editor: Fix soft wrap premature wrapping with certain fonts (#45206)

Mayank Verma and Lukas Wirth created

Closes #45107

Release Notes:

- Fixed soft wrap prematurely wrapping with certain fonts

---------

Co-authored-by: Lukas Wirth <lukas@zed.dev>

Change summary

crates/editor/src/element.rs                | 24 ++++++++------
crates/gpui/src/text_system.rs              | 29 ++++++++++++++++-
crates/gpui/src/text_system/line_wrapper.rs | 37 ++++++----------------
3 files changed, 50 insertions(+), 40 deletions(-)

Detailed changes

crates/editor/src/element.rs 🔗

@@ -7521,7 +7521,7 @@ impl EditorElement {
                         let position_map: &PositionMap = &position_map;
 
                         let line_height = position_map.line_height;
-                        let max_glyph_advance = position_map.em_advance;
+                        let glyph_width = position_map.em_layout_width;
                         let (delta, axis) = match delta {
                             gpui::ScrollDelta::Pixels(mut pixels) => {
                                 //Trackpad
@@ -7531,17 +7531,15 @@ impl EditorElement {
 
                             gpui::ScrollDelta::Lines(lines) => {
                                 //Not trackpad
-                                let pixels =
-                                    point(lines.x * max_glyph_advance, lines.y * line_height);
+                                let pixels = point(lines.x * glyph_width, lines.y * line_height);
                                 (pixels, None)
                             }
                         };
 
                         let current_scroll_position = position_map.snapshot.scroll_position();
-                        let x = (current_scroll_position.x
-                            * ScrollPixelOffset::from(max_glyph_advance)
+                        let x = (current_scroll_position.x * ScrollPixelOffset::from(glyph_width)
                             - ScrollPixelOffset::from(delta.x * scroll_sensitivity))
-                            / ScrollPixelOffset::from(max_glyph_advance);
+                            / ScrollPixelOffset::from(glyph_width);
                         let y = (current_scroll_position.y * ScrollPixelOffset::from(line_height)
                             - ScrollPixelOffset::from(delta.y * scroll_sensitivity))
                             / ScrollPixelOffset::from(line_height);
@@ -9539,6 +9537,7 @@ impl Element for EditorElement {
                     let line_height = style.text.line_height_in_pixels(rem_size);
                     let em_width = window.text_system().em_width(font_id, font_size).unwrap();
                     let em_advance = window.text_system().em_advance(font_id, font_size).unwrap();
+                    let em_layout_width = window.text_system().em_layout_width(font_id, font_size);
                     let glyph_grid_cell = size(em_advance, line_height);
 
                     let gutter_dimensions =
@@ -9592,7 +9591,7 @@ impl Element for EditorElement {
                             let wrap_width = calculate_wrap_width(
                                 editor.soft_wrap_mode(cx),
                                 editor_width,
-                                em_advance,
+                                em_layout_width,
                             );
 
                             if editor.set_wrap_width(wrap_width, cx) {
@@ -10248,7 +10247,7 @@ impl Element for EditorElement {
 
                     let scroll_max: gpui::Point<ScrollPixelOffset> = point(
                         ScrollPixelOffset::from(
-                            ((scroll_width - editor_width) / em_advance).max(0.0),
+                            ((scroll_width - editor_width) / em_layout_width).max(0.0),
                         ),
                         max_scroll_top,
                     );
@@ -10275,7 +10274,7 @@ impl Element for EditorElement {
                     });
 
                     let scroll_pixel_position = point(
-                        scroll_position.x * f64::from(em_advance),
+                        scroll_position.x * f64::from(em_layout_width),
                         scroll_position.y * f64::from(line_height),
                     );
                     let sticky_headers = if !is_minimap
@@ -10779,6 +10778,7 @@ impl Element for EditorElement {
                         line_height,
                         em_width,
                         em_advance,
+                        em_layout_width,
                         snapshot,
                         text_align: self.style.text.text_align,
                         content_width: text_hitbox.size.width,
@@ -11626,6 +11626,7 @@ pub(crate) struct PositionMap {
     pub scroll_max: gpui::Point<ScrollOffset>,
     pub em_width: Pixels,
     pub em_advance: Pixels,
+    pub em_layout_width: Pixels,
     pub visible_row_range: Range<DisplayRow>,
     pub line_layouts: Vec<LineWithInvisibles>,
     pub snapshot: EditorSnapshot,
@@ -11689,7 +11690,7 @@ impl PositionMap {
         let scroll_position = self.snapshot.scroll_position();
         let position = position - text_bounds.origin;
         let y = position.y.max(px(0.)).min(self.size.height);
-        let x = position.x + (scroll_position.x as f32 * self.em_advance);
+        let x = position.x + (scroll_position.x as f32 * self.em_layout_width);
         let row = ((y / self.line_height) as f64 + scroll_position.y) as u32;
 
         let (column, x_overshoot_after_line_end) = if let Some(line) = self
@@ -11711,7 +11712,8 @@ impl PositionMap {
         let previous_valid = self.snapshot.clip_point(exact_unclipped, Bias::Left);
         let next_valid = self.snapshot.clip_point(exact_unclipped, Bias::Right);
 
-        let column_overshoot_after_line_end = (x_overshoot_after_line_end / self.em_advance) as u32;
+        let column_overshoot_after_line_end =
+            (x_overshoot_after_line_end / self.em_layout_width) as u32;
         *exact_unclipped.column_mut() += column_overshoot_after_line_end;
         PointForPosition {
             previous_valid,

crates/gpui/src/text_system.rs 🔗

@@ -205,6 +205,23 @@ impl TextSystem {
         Ok(result * font_size)
     }
 
+    // Consider removing this?
+    /// Returns the shaped layout width of for the given character, in the given font and size.
+    pub fn layout_width(&self, font_id: FontId, font_size: Pixels, ch: char) -> Pixels {
+        let mut buffer = [0; 4];
+        let buffer = ch.encode_utf8(&mut buffer);
+        self.platform_text_system
+            .layout_line(
+                buffer,
+                font_size,
+                &[FontRun {
+                    len: buffer.len(),
+                    font_id,
+                }],
+            )
+            .width
+    }
+
     /// Returns the width of an `em`.
     ///
     /// Uses the width of the `m` character in the given font and size.
@@ -219,6 +236,12 @@ impl TextSystem {
         Ok(self.advance(font_id, font_size, 'm')?.width)
     }
 
+    // Consider removing this?
+    /// Returns the shaped layout width of an `em`.
+    pub fn em_layout_width(&self, font_id: FontId, font_size: Pixels) -> Pixels {
+        self.layout_width(font_id, font_size, 'm')
+    }
+
     /// Returns the width of an `ch`.
     ///
     /// Uses the width of the `0` character in the given font and size.
@@ -295,9 +318,9 @@ impl TextSystem {
         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())
-        });
+        let wrapper = wrappers
+            .pop()
+            .unwrap_or_else(|| LineWrapper::new(font_id, font_size, self.clone()));
 
         LineWrapperHandle {
             wrapper: Some(wrapper),

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

@@ -1,4 +1,4 @@
-use crate::{FontId, FontRun, Pixels, PlatformTextSystem, SharedString, TextRun, px};
+use crate::{FontId, Pixels, SharedString, TextRun, TextSystem, px};
 use collections::HashMap;
 use std::{borrow::Cow, iter, sync::Arc};
 
@@ -13,7 +13,7 @@ pub enum TruncateFrom {
 
 /// The GPUI line wrapper, used to wrap lines of text to a given width.
 pub struct LineWrapper {
-    platform_text_system: Arc<dyn PlatformTextSystem>,
+    text_system: Arc<TextSystem>,
     pub(crate) font_id: FontId,
     pub(crate) font_size: Pixels,
     cached_ascii_char_widths: [Option<Pixels>; 128],
@@ -24,13 +24,9 @@ impl LineWrapper {
     /// The maximum indent that can be applied to a line.
     pub const MAX_INDENT: u32 = 256;
 
-    pub(crate) fn new(
-        font_id: FontId,
-        font_size: Pixels,
-        text_system: Arc<dyn PlatformTextSystem>,
-    ) -> Self {
+    pub(crate) fn new(font_id: FontId, font_size: Pixels, text_system: Arc<TextSystem>) -> Self {
         Self {
-            platform_text_system: text_system,
+            text_system,
             font_id,
             font_size,
             cached_ascii_char_widths: [None; 128],
@@ -254,33 +250,22 @@ impl LineWrapper {
             if let Some(cached_width) = self.cached_ascii_char_widths[c as usize] {
                 cached_width
             } else {
-                let width = self.compute_width_for_char(c);
+                let width = self
+                    .text_system
+                    .layout_width(self.font_id, self.font_size, c);
                 self.cached_ascii_char_widths[c as usize] = Some(width);
                 width
             }
         } else if let Some(cached_width) = self.cached_other_char_widths.get(&c) {
             *cached_width
         } else {
-            let width = self.compute_width_for_char(c);
+            let width = self
+                .text_system
+                .layout_width(self.font_id, self.font_size, c);
             self.cached_other_char_widths.insert(c, width);
             width
         }
     }
-
-    fn compute_width_for_char(&self, c: char) -> Pixels {
-        let mut buffer = [0; 4];
-        let buffer = c.encode_utf8(&mut buffer);
-        self.platform_text_system
-            .layout_line(
-                buffer,
-                self.font_size,
-                &[FontRun {
-                    len: buffer.len(),
-                    font_id: self.font_id,
-                }],
-            )
-            .width
-    }
 }
 
 fn update_runs_after_truncation(
@@ -401,7 +386,7 @@ mod tests {
         let dispatcher = TestDispatcher::new(0);
         let cx = TestAppContext::build(dispatcher, None);
         let id = cx.text_system().resolve_font(&font(".ZedMono"));
-        LineWrapper::new(id, px(16.), cx.text_system().platform_text_system.clone())
+        LineWrapper::new(id, px(16.), cx.text_system().clone())
     }
 
     fn generate_test_runs(input_run_len: &[usize]) -> Vec<TextRun> {