Scope line layout cache to each window (#7235)

Antonio Scandurra and Max Brunsfeld created

This improves a performance problem we were observing when having
multiple windows updating at the same time, where each window would
invalidate the other window's layout cache.

Release Notes:

- Improved performance when having multiple Zed windows open.

Co-authored-by: Max Brunsfeld <max@zed.dev>

Change summary

crates/editor/src/movement.rs                |   4 
crates/gpui/src/text_system.rs               | 154 ++++++++++++---------
crates/gpui/src/text_system/line_wrapper.rs  |   4 
crates/gpui/src/window.rs                    |  10 +
crates/terminal_view/src/terminal_element.rs |   6 
5 files changed, 102 insertions(+), 76 deletions(-)

Detailed changes

crates/editor/src/movement.rs 🔗

@@ -3,7 +3,7 @@
 
 use super::{Bias, DisplayPoint, DisplaySnapshot, SelectionGoal, ToDisplayPoint};
 use crate::{char_kind, CharKind, EditorStyle, ToOffset, ToPoint};
-use gpui::{px, Pixels, TextSystem};
+use gpui::{px, Pixels, WindowTextSystem};
 use language::Point;
 
 use std::{ops::Range, sync::Arc};
@@ -22,7 +22,7 @@ pub enum FindRange {
 /// TextLayoutDetails encompasses everything we need to move vertically
 /// taking into account variable width characters.
 pub struct TextLayoutDetails {
-    pub(crate) text_system: Arc<TextSystem>,
+    pub(crate) text_system: Arc<WindowTextSystem>,
     pub(crate) editor_style: EditorStyle,
     pub(crate) rem_size: Pixels,
     pub anchor: Anchor,

crates/gpui/src/text_system.rs 🔗

@@ -15,6 +15,7 @@ use crate::{
 use anyhow::anyhow;
 use collections::{BTreeSet, FxHashMap, FxHashSet};
 use core::fmt;
+use derive_more::Deref;
 use itertools::Itertools;
 use parking_lot::{Mutex, RwLock, RwLockUpgradableReadGuard};
 use smallvec::{smallvec, SmallVec};
@@ -38,9 +39,8 @@ pub struct FontFamilyId(pub usize);
 
 pub(crate) const SUBPIXEL_VARIANTS: u8 = 4;
 
-/// The GPUI text layout and rendering sub system.
+/// The GPUI text rendering sub system.
 pub struct TextSystem {
-    line_layout_cache: Arc<LineLayoutCache>,
     platform_text_system: Arc<dyn PlatformTextSystem>,
     font_ids_by_font: RwLock<FxHashMap<Font, Result<FontId>>>,
     font_metrics: RwLock<FxHashMap<FontId, FontMetrics>>,
@@ -53,7 +53,6 @@ pub struct TextSystem {
 impl TextSystem {
     pub(crate) fn new(platform_text_system: Arc<dyn PlatformTextSystem>) -> Self {
         TextSystem {
-            line_layout_cache: Arc::new(LineLayoutCache::new(platform_text_system.clone())),
             platform_text_system,
             font_metrics: RwLock::default(),
             raster_bounds: RwLock::default(),
@@ -234,43 +233,66 @@ impl TextSystem {
         }
     }
 
-    pub(crate) fn with_view<R>(&self, view_id: EntityId, f: impl FnOnce() -> R) -> R {
-        self.line_layout_cache.with_view(view_id, f)
+    /// Returns a handle to a line wrapper, for the given font and font size.
+    pub fn line_wrapper(self: &Arc<Self>, font: Font, font_size: Pixels) -> LineWrapperHandle {
+        let lock = &mut self.wrapper_pool.lock();
+        let font_id = self.resolve_font(&font);
+        let wrappers = lock
+            .entry(FontIdWithSize { font_id, font_size })
+            .or_default();
+        let wrapper = wrappers.pop().unwrap_or_else(|| {
+            LineWrapper::new(font_id, font_size, self.platform_text_system.clone())
+        });
+
+        LineWrapperHandle {
+            wrapper: Some(wrapper),
+            text_system: self.clone(),
+        }
     }
 
-    /// Layout the given line of text, at the given font_size.
-    /// Subsets of the line can be styled independently with the `runs` parameter.
-    /// Generally, you should prefer to use `TextLayout::shape_line` instead, which
-    /// can be painted directly.
-    pub fn layout_line(
-        &self,
-        text: &str,
-        font_size: Pixels,
-        runs: &[TextRun],
-    ) -> Result<Arc<LineLayout>> {
-        let mut font_runs = self.font_runs_pool.lock().pop().unwrap_or_default();
-        for run in runs.iter() {
-            let font_id = self.resolve_font(&run.font);
-            if let Some(last_run) = font_runs.last_mut() {
-                if last_run.font_id == font_id {
-                    last_run.len += run.len;
-                    continue;
-                }
-            }
-            font_runs.push(FontRun {
-                len: run.len,
-                font_id,
-            });
+    /// Get the rasterized size and location of a specific, rendered glyph.
+    pub(crate) fn raster_bounds(&self, params: &RenderGlyphParams) -> Result<Bounds<DevicePixels>> {
+        let raster_bounds = self.raster_bounds.upgradable_read();
+        if let Some(bounds) = raster_bounds.get(params) {
+            Ok(*bounds)
+        } else {
+            let mut raster_bounds = RwLockUpgradableReadGuard::upgrade(raster_bounds);
+            let bounds = self.platform_text_system.glyph_raster_bounds(params)?;
+            raster_bounds.insert(params.clone(), bounds);
+            Ok(bounds)
         }
+    }
 
-        let layout = self
-            .line_layout_cache
-            .layout_line(text, font_size, &font_runs);
+    pub(crate) fn rasterize_glyph(
+        &self,
+        params: &RenderGlyphParams,
+    ) -> Result<(Size<DevicePixels>, Vec<u8>)> {
+        let raster_bounds = self.raster_bounds(params)?;
+        self.platform_text_system
+            .rasterize_glyph(params, raster_bounds)
+    }
+}
 
-        font_runs.clear();
-        self.font_runs_pool.lock().push(font_runs);
+/// The GPUI text layout subsystem.
+#[derive(Deref)]
+pub struct WindowTextSystem {
+    line_layout_cache: Arc<LineLayoutCache>,
+    #[deref]
+    text_system: Arc<TextSystem>,
+}
 
-        Ok(layout)
+impl WindowTextSystem {
+    pub(crate) fn new(text_system: Arc<TextSystem>) -> Self {
+        Self {
+            line_layout_cache: Arc::new(LineLayoutCache::new(
+                text_system.platform_text_system.clone(),
+            )),
+            text_system,
+        }
+    }
+
+    pub(crate) fn with_view<R>(&self, view_id: EntityId, f: impl FnOnce() -> R) -> R {
+        self.line_layout_cache.with_view(view_id, f)
     }
 
     /// Shape the given line, at the given font_size, for painting to the screen.
@@ -429,43 +451,39 @@ impl TextSystem {
         self.line_layout_cache.finish_frame(reused_views)
     }
 
-    /// Returns a handle to a line wrapper, for the given font and font size.
-    pub fn line_wrapper(self: &Arc<Self>, font: Font, font_size: Pixels) -> LineWrapperHandle {
-        let lock = &mut self.wrapper_pool.lock();
-        let font_id = self.resolve_font(&font);
-        let wrappers = lock
-            .entry(FontIdWithSize { font_id, font_size })
-            .or_default();
-        let wrapper = wrappers.pop().unwrap_or_else(|| {
-            LineWrapper::new(font_id, font_size, self.platform_text_system.clone())
-        });
-
-        LineWrapperHandle {
-            wrapper: Some(wrapper),
-            text_system: self.clone(),
+    /// Layout the given line of text, at the given font_size.
+    /// Subsets of the line can be styled independently with the `runs` parameter.
+    /// Generally, you should prefer to use `TextLayout::shape_line` instead, which
+    /// can be painted directly.
+    pub fn layout_line(
+        &self,
+        text: &str,
+        font_size: Pixels,
+        runs: &[TextRun],
+    ) -> Result<Arc<LineLayout>> {
+        let mut font_runs = self.font_runs_pool.lock().pop().unwrap_or_default();
+        for run in runs.iter() {
+            let font_id = self.resolve_font(&run.font);
+            if let Some(last_run) = font_runs.last_mut() {
+                if last_run.font_id == font_id {
+                    last_run.len += run.len;
+                    continue;
+                }
+            }
+            font_runs.push(FontRun {
+                len: run.len,
+                font_id,
+            });
         }
-    }
 
-    /// Get the rasterized size and location of a specific, rendered glyph.
-    pub(crate) fn raster_bounds(&self, params: &RenderGlyphParams) -> Result<Bounds<DevicePixels>> {
-        let raster_bounds = self.raster_bounds.upgradable_read();
-        if let Some(bounds) = raster_bounds.get(params) {
-            Ok(*bounds)
-        } else {
-            let mut raster_bounds = RwLockUpgradableReadGuard::upgrade(raster_bounds);
-            let bounds = self.platform_text_system.glyph_raster_bounds(params)?;
-            raster_bounds.insert(params.clone(), bounds);
-            Ok(bounds)
-        }
-    }
+        let layout = self
+            .line_layout_cache
+            .layout_line(text, font_size, &font_runs);
 
-    pub(crate) fn rasterize_glyph(
-        &self,
-        params: &RenderGlyphParams,
-    ) -> Result<(Size<DevicePixels>, Vec<u8>)> {
-        let raster_bounds = self.raster_bounds(params)?;
-        self.platform_text_system
-            .rasterize_glyph(params, raster_bounds)
+        font_runs.clear();
+        self.font_runs_pool.lock().push(font_runs);
+
+        Ok(layout)
     }
 }
 

crates/gpui/src/text_system/line_wrapper.rs 🔗

@@ -143,7 +143,7 @@ impl Boundary {
 #[cfg(test)]
 mod tests {
     use super::*;
-    use crate::{font, TestAppContext, TestDispatcher, TextRun, WrapBoundary};
+    use crate::{font, TestAppContext, TestDispatcher, TextRun, WindowTextSystem, WrapBoundary};
     use rand::prelude::*;
 
     #[test]
@@ -218,7 +218,7 @@ mod tests {
     #[crate::test]
     fn test_wrap_shaped_line(cx: &mut TestAppContext) {
         cx.update(|cx| {
-            let text_system = cx.text_system().clone();
+            let text_system = WindowTextSystem::new(cx.text_system().clone());
 
             let normal = TextRun {
                 len: 0,

crates/gpui/src/window.rs 🔗

@@ -7,7 +7,7 @@ use crate::{
     MouseMoveEvent, MouseUpEvent, Pixels, PlatformAtlas, PlatformDisplay, PlatformInput,
     PlatformWindow, Point, PromptLevel, Render, ScaledPixels, SharedString, Size, SubscriberSet,
     Subscription, TaffyLayoutEngine, Task, View, VisualContext, WeakView, WindowBounds,
-    WindowOptions,
+    WindowOptions, WindowTextSystem,
 };
 use anyhow::{anyhow, Context as _, Result};
 use collections::FxHashSet;
@@ -251,6 +251,7 @@ pub struct Window {
     pub(crate) platform_window: Box<dyn PlatformWindow>,
     display_id: DisplayId,
     sprite_atlas: Arc<dyn PlatformAtlas>,
+    text_system: Arc<WindowTextSystem>,
     pub(crate) rem_size: Pixels,
     pub(crate) viewport_size: Size<Pixels>,
     layout_engine: Option<TaffyLayoutEngine>,
@@ -337,6 +338,7 @@ impl Window {
         let content_size = platform_window.content_size();
         let scale_factor = platform_window.scale_factor();
         let bounds = platform_window.bounds();
+        let text_system = Arc::new(WindowTextSystem::new(cx.text_system().clone()));
 
         platform_window.on_request_frame(Box::new({
             let mut cx = cx.to_async();
@@ -393,6 +395,7 @@ impl Window {
             platform_window,
             display_id,
             sprite_atlas,
+            text_system,
             rem_size: px(16.),
             viewport_size: content_size,
             layout_engine: Some(TaffyLayoutEngine::new()),
@@ -530,6 +533,11 @@ impl<'a> WindowContext<'a> {
         self.window.focus_enabled = false;
     }
 
+    /// Accessor for the text system.
+    pub fn text_system(&self) -> &Arc<WindowTextSystem> {
+        &self.window.text_system
+    }
+
     /// Dispatch the given action on the currently focused element.
     pub fn dispatch_action(&mut self, action: Box<dyn Action>) {
         let focus_handle = self.focused();

crates/terminal_view/src/terminal_element.rs 🔗

@@ -4,8 +4,8 @@ use gpui::{
     ElementContext, ElementId, FocusHandle, Font, FontStyle, FontWeight, HighlightStyle, Hsla,
     InputHandler, InteractiveBounds, InteractiveElement, InteractiveElementState, Interactivity,
     IntoElement, LayoutId, Model, ModelContext, ModifiersChangedEvent, MouseButton, MouseMoveEvent,
-    Pixels, Point, ShapedLine, StatefulInteractiveElement, Styled, TextRun, TextStyle, TextSystem,
-    UnderlineStyle, WeakView, WhiteSpace, WindowContext,
+    Pixels, Point, ShapedLine, StatefulInteractiveElement, Styled, TextRun, TextStyle,
+    UnderlineStyle, WeakView, WhiteSpace, WindowContext, WindowTextSystem,
 };
 use itertools::Itertools;
 use language::CursorShape;
@@ -185,7 +185,7 @@ impl TerminalElement {
         grid: &Vec<IndexedCell>,
         text_style: &TextStyle,
         // terminal_theme: &TerminalStyle,
-        text_system: &TextSystem,
+        text_system: &WindowTextSystem,
         hyperlink: Option<(HighlightStyle, &RangeInclusive<AlacPoint>)>,
         cx: &WindowContext<'_>,
     ) -> (Vec<LayoutCell>, Vec<LayoutRect>) {