editor: Use `em_advance` everywhere for horizontal scroll position computations (#33514)

Finn Evers created 5 months ago

Closes #33472

This PR fixes some regressions that were introduced in
https://github.com/zed-industries/zed/pull/32558, which updated the
editor scrolling to use `em_advance` instead of `em_width` for the
horizontal scroll position calculation.
However, not all occurrences were updated, which caused issues with wrap
guides and some small stuttering with horizontal autoscroll whilst
typing/navigating with the keyboard.

Release Notes:

- Fixed an issue where horizontal autoscrolling would stutter and indent
guides would drift when scrolling horizontally.

Change summary

crates/editor/src/editor.rs             | 29 ++++++++++++++++++--------
crates/editor/src/element.rs            | 14 ++++++------
crates/editor/src/mouse_context_menu.rs |  4 +-
3 files changed, 29 insertions(+), 18 deletions(-)

Detailed changes

crates/editor/src/editor.rs

@@ -1215,6 +1215,12 @@ impl GutterDimensions {
     }
 }
 
+struct CharacterDimensions {
+    em_width: Pixels,
+    em_advance: Pixels,
+    line_height: Pixels,
+}
+
 #[derive(Debug)]
 pub struct RemoteSelection {
     pub replica_id: ReplicaId,
@@ -20520,15 +20526,20 @@ impl Editor {
             .and_then(|item| item.to_any_mut()?.downcast_mut::<T>())
     }
 
-    fn character_size(&self, window: &mut Window) -> gpui::Size<Pixels> {
+    fn character_dimensions(&self, window: &mut Window) -> CharacterDimensions {
         let text_layout_details = self.text_layout_details(window);
         let style = &text_layout_details.editor_style;
         let font_id = window.text_system().resolve_font(&style.text.font());
         let font_size = style.text.font_size.to_pixels(window.rem_size());
         let line_height = style.text.line_height_in_pixels(window.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();
 
-        gpui::Size::new(em_width, line_height)
+        CharacterDimensions {
+            em_width,
+            em_advance,
+            line_height,
+        }
     }
 
     pub fn wait_for_diff_to_load(&self) -> Option<Shared<Task<()>>> {
@@ -22542,19 +22553,19 @@ impl EntityInputHandler for Editor {
         cx: &mut Context<Self>,
     ) -> Option<gpui::Bounds<Pixels>> {
         let text_layout_details = self.text_layout_details(window);
-        let gpui::Size {
-            width: em_width,
-            height: line_height,
-        } = self.character_size(window);
+        let CharacterDimensions {
+            em_width,
+            em_advance,
+            line_height,
+        } = self.character_dimensions(window);
 
         let snapshot = self.snapshot(window, cx);
         let scroll_position = snapshot.scroll_position();
-        let scroll_left = scroll_position.x * em_width;
+        let scroll_left = scroll_position.x * em_advance;
 
         let start = OffsetUtf16(range_utf16.start).to_display_point(&snapshot);
         let x = snapshot.x_for_display_point(start, &text_layout_details) - scroll_left
-            + self.gutter_dimensions.width
-            + self.gutter_dimensions.margin;
+            + self.gutter_dimensions.full_width();
         let y = line_height * (start.row().as_f32() - scroll_position.y);
 
         Some(Bounds {

crates/editor/src/element.rs

@@ -5238,8 +5238,8 @@ impl EditorElement {
                     paint_highlight(range.start, range.end, color, edges);
                 }
 
-                let scroll_left =
-                    layout.position_map.snapshot.scroll_position().x * layout.position_map.em_width;
+                let scroll_left = layout.position_map.snapshot.scroll_position().x
+                    * layout.position_map.em_advance;
 
                 for (wrap_position, active) in layout.wrap_guides.iter() {
                     let x = (layout.position_map.text_hitbox.origin.x
@@ -6676,7 +6676,7 @@ impl EditorElement {
                         let position_map: &PositionMap = &position_map;
 
                         let line_height = position_map.line_height;
-                        let max_glyph_width = position_map.em_width;
+                        let max_glyph_advance = position_map.em_advance;
                         let (delta, axis) = match delta {
                             gpui::ScrollDelta::Pixels(mut pixels) => {
                                 //Trackpad
@@ -6687,15 +6687,15 @@ impl EditorElement {
                             gpui::ScrollDelta::Lines(lines) => {
                                 //Not trackpad
                                 let pixels =
-                                    point(lines.x * max_glyph_width, lines.y * line_height);
+                                    point(lines.x * max_glyph_advance, lines.y * line_height);
                                 (pixels, None)
                             }
                         };
 
                         let current_scroll_position = position_map.snapshot.scroll_position();
-                        let x = (current_scroll_position.x * max_glyph_width
+                        let x = (current_scroll_position.x * max_glyph_advance
                             - (delta.x * scroll_sensitivity))
-                            / max_glyph_width;
+                            / max_glyph_advance;
                         let y = (current_scroll_position.y * line_height
                             - (delta.y * scroll_sensitivity))
                             / line_height;
@@ -8591,7 +8591,7 @@ impl Element for EditorElement {
                                 start_row,
                                 editor_content_width,
                                 scroll_width,
-                                em_width,
+                                em_advance,
                                 &line_layouts,
                                 cx,
                             )

crates/editor/src/mouse_context_menu.rs

@@ -275,10 +275,10 @@ pub fn deploy_context_menu(
             cx,
         ),
         None => {
-            let character_size = editor.character_size(window);
+            let character_size = editor.character_dimensions(window);
             let menu_position = MenuPosition::PinnedToEditor {
                 source: source_anchor,
-                offset: gpui::point(character_size.width, character_size.height),
+                offset: gpui::point(character_size.em_width, character_size.line_height),
             };
             Some(MouseContextMenu::new(
                 editor,