Fix uncached raster_bounds computation and font selection (#3328)

Max Brunsfeld created 2 years ago

This fixes some slowness we noticed when scrolling an editor in `zed2`.

Change summary

crates/gpui2/src/platform.rs                 |  6 ++++
crates/gpui2/src/platform/mac/text_system.rs | 15 +++++++++---
crates/gpui2/src/text_system.rs              | 26 ++++++++++++++++-----
3 files changed, 35 insertions(+), 12 deletions(-)

Detailed changes

crates/gpui2/src/platform.rs

@@ -184,7 +184,11 @@ pub trait PlatformTextSystem: Send + Sync {
     fn advance(&self, font_id: FontId, glyph_id: GlyphId) -> Result<Size<f32>>;
     fn glyph_for_char(&self, font_id: FontId, ch: char) -> Option<GlyphId>;
     fn glyph_raster_bounds(&self, params: &RenderGlyphParams) -> Result<Bounds<DevicePixels>>;
-    fn rasterize_glyph(&self, params: &RenderGlyphParams) -> Result<(Size<DevicePixels>, Vec<u8>)>;
+    fn rasterize_glyph(
+        &self,
+        params: &RenderGlyphParams,
+        raster_bounds: Bounds<DevicePixels>,
+    ) -> Result<(Size<DevicePixels>, Vec<u8>)>;
     fn layout_line(&self, text: &str, font_size: Pixels, runs: &[FontRun]) -> LineLayout;
     fn wrap_line(
         &self,

crates/gpui2/src/platform/mac/text_system.rs

@@ -116,7 +116,9 @@ impl PlatformTextSystem for MacTextSystem {
                 },
             )?;
 
-            Ok(candidates[ix])
+            let font_id = candidates[ix];
+            lock.font_selections.insert(font.clone(), font_id);
+            Ok(font_id)
         }
     }
 
@@ -145,8 +147,9 @@ impl PlatformTextSystem for MacTextSystem {
     fn rasterize_glyph(
         &self,
         glyph_id: &RenderGlyphParams,
+        raster_bounds: Bounds<DevicePixels>,
     ) -> Result<(Size<DevicePixels>, Vec<u8>)> {
-        self.0.read().rasterize_glyph(glyph_id)
+        self.0.read().rasterize_glyph(glyph_id, raster_bounds)
     }
 
     fn layout_line(&self, text: &str, font_size: Pixels, font_runs: &[FontRun]) -> LineLayout {
@@ -247,8 +250,11 @@ impl MacTextSystemState {
             .into())
     }
 
-    fn rasterize_glyph(&self, params: &RenderGlyphParams) -> Result<(Size<DevicePixels>, Vec<u8>)> {
-        let glyph_bounds = self.raster_bounds(params)?;
+    fn rasterize_glyph(
+        &self,
+        params: &RenderGlyphParams,
+        glyph_bounds: Bounds<DevicePixels>,
+    ) -> Result<(Size<DevicePixels>, Vec<u8>)> {
         if glyph_bounds.size.width.0 == 0 || glyph_bounds.size.height.0 == 0 {
             Err(anyhow!("glyph bounds are empty"))
         } else {
@@ -260,6 +266,7 @@ impl MacTextSystemState {
             if params.subpixel_variant.y > 0 {
                 bitmap_size.height += DevicePixels(1);
             }
+            let bitmap_size = bitmap_size;
 
             let mut bytes;
             let cx;

crates/gpui2/src/text_system.rs

@@ -39,6 +39,7 @@ pub struct TextSystem {
     platform_text_system: Arc<dyn PlatformTextSystem>,
     font_ids_by_font: RwLock<HashMap<Font, FontId>>,
     font_metrics: RwLock<HashMap<FontId, FontMetrics>>,
+    raster_bounds: RwLock<HashMap<RenderGlyphParams, Bounds<DevicePixels>>>,
     wrapper_pool: Mutex<HashMap<FontIdWithSize, Vec<LineWrapper>>>,
     font_runs_pool: Mutex<Vec<Vec<FontRun>>>,
 }
@@ -48,10 +49,11 @@ impl TextSystem {
         TextSystem {
             line_layout_cache: Arc::new(LineLayoutCache::new(platform_text_system.clone())),
             platform_text_system,
-            font_metrics: RwLock::new(HashMap::default()),
-            font_ids_by_font: RwLock::new(HashMap::default()),
-            wrapper_pool: Mutex::new(HashMap::default()),
-            font_runs_pool: Default::default(),
+            font_metrics: RwLock::default(),
+            raster_bounds: RwLock::default(),
+            font_ids_by_font: RwLock::default(),
+            wrapper_pool: Mutex::default(),
+            font_runs_pool: Mutex::default(),
         }
     }
 
@@ -252,14 +254,24 @@ impl TextSystem {
     }
 
     pub fn raster_bounds(&self, params: &RenderGlyphParams) -> Result<Bounds<DevicePixels>> {
-        self.platform_text_system.glyph_raster_bounds(params)
+        let raster_bounds = self.raster_bounds.upgradable_read();
+        if let Some(bounds) = raster_bounds.get(params) {
+            Ok(bounds.clone())
+        } 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)
+        }
     }
 
     pub fn rasterize_glyph(
         &self,
-        glyph_id: &RenderGlyphParams,
+        params: &RenderGlyphParams,
     ) -> Result<(Size<DevicePixels>, Vec<u8>)> {
-        self.platform_text_system.rasterize_glyph(glyph_id)
+        let raster_bounds = self.raster_bounds(params)?;
+        self.platform_text_system
+            .rasterize_glyph(params, raster_bounds)
     }
 }