Fix relative line numbers in sticky headers (#45164)

Andrew Farkas created

Closes #42586 

This includes a rewrite of `calculate_relative_line_numbers()`. Now it's
linear-time with respect to the number of rows displayed, instead of
linear time with respect to the number of rows displayed _plus_ the
distance to the base row.

Release Notes:

- Improved performance when using relative line numbers in large files
- Fixed relative line numbers not appearing in sticky headers

Change summary

crates/editor/src/editor.rs       |  66 ++++++++++++
crates/editor/src/editor_tests.rs | 127 +++++++++++++++++++++++
crates/editor/src/element.rs      | 176 ++++++++++++--------------------
3 files changed, 258 insertions(+), 111 deletions(-)

Detailed changes

crates/editor/src/editor.rs 🔗

@@ -20475,7 +20475,7 @@ impl Editor {
         EditorSettings::get_global(cx).gutter.line_numbers
     }
 
-    pub fn relative_line_numbers(&self, cx: &mut App) -> RelativeLineNumbers {
+    pub fn relative_line_numbers(&self, cx: &App) -> RelativeLineNumbers {
         match (
             self.use_relative_line_numbers,
             EditorSettings::get_global(cx).relative_line_numbers,
@@ -25294,6 +25294,70 @@ impl EditorSnapshot {
         let digit_count = self.widest_line_number().ilog10() + 1;
         column_pixels(style, digit_count as usize, window)
     }
+
+    /// Returns the line delta from `base` to `line` in the multibuffer, ignoring wrapped lines.
+    ///
+    /// This is positive if `base` is before `line`.
+    fn relative_line_delta(&self, base: DisplayRow, line: DisplayRow) -> i64 {
+        let point = DisplayPoint::new(line, 0).to_point(self);
+        self.relative_line_delta_to_point(base, point)
+    }
+
+    /// Returns the line delta from `base` to `point` in the multibuffer, ignoring wrapped lines.
+    ///
+    /// This is positive if `base` is before `point`.
+    pub fn relative_line_delta_to_point(&self, base: DisplayRow, point: Point) -> i64 {
+        let base_point = DisplayPoint::new(base, 0).to_point(self);
+        point.row as i64 - base_point.row as i64
+    }
+
+    /// Returns the line delta from `base` to `line` in the multibuffer, counting wrapped lines.
+    ///
+    /// This is positive if `base` is before `line`.
+    fn relative_wrapped_line_delta(&self, base: DisplayRow, line: DisplayRow) -> i64 {
+        let point = DisplayPoint::new(line, 0).to_point(self);
+        self.relative_wrapped_line_delta_to_point(base, point)
+    }
+
+    /// Returns the line delta from `base` to `point` in the multibuffer, counting wrapped lines.
+    ///
+    /// This is positive if `base` is before `point`.
+    pub fn relative_wrapped_line_delta_to_point(&self, base: DisplayRow, point: Point) -> i64 {
+        let base_point = DisplayPoint::new(base, 0).to_point(self);
+        let wrap_snapshot = self.wrap_snapshot();
+        let base_wrap_row = wrap_snapshot.make_wrap_point(base_point, Bias::Left).row();
+        let wrap_row = wrap_snapshot.make_wrap_point(point, Bias::Left).row();
+        wrap_row.0 as i64 - base_wrap_row.0 as i64
+    }
+
+    /// Returns the unsigned relative line number to display for each row in `rows`.
+    ///
+    /// Wrapped rows are excluded from the hashmap if `count_relative_lines` is `false`.
+    pub fn calculate_relative_line_numbers(
+        &self,
+        rows: &Range<DisplayRow>,
+        relative_to: DisplayRow,
+        count_wrapped_lines: bool,
+    ) -> HashMap<DisplayRow, u32> {
+        let initial_offset = if count_wrapped_lines {
+            self.relative_wrapped_line_delta(relative_to, rows.start)
+        } else {
+            self.relative_line_delta(relative_to, rows.start)
+        };
+        let display_row_infos = self
+            .row_infos(rows.start)
+            .take(rows.len())
+            .enumerate()
+            .map(|(i, row_info)| (DisplayRow(rows.start.0 + i as u32), row_info));
+        display_row_infos
+            .filter(|(_row, row_info)| {
+                row_info.buffer_row.is_some()
+                    || (count_wrapped_lines && row_info.wrapped_buffer_row.is_some())
+            })
+            .enumerate()
+            .map(|(i, (row, _row_info))| (row, (initial_offset + i as i64).unsigned_abs() as u32))
+            .collect()
+    }
 }
 
 pub fn column_pixels(style: &EditorStyle, column: usize, window: &Window) -> Pixels {

crates/editor/src/editor_tests.rs 🔗

@@ -36,7 +36,8 @@ use languages::markdown_lang;
 use languages::rust_lang;
 use lsp::CompletionParams;
 use multi_buffer::{
-    IndentGuide, MultiBufferFilterMode, MultiBufferOffset, MultiBufferOffsetUtf16, PathKey,
+    ExcerptRange, IndentGuide, MultiBuffer, MultiBufferFilterMode, MultiBufferOffset,
+    MultiBufferOffsetUtf16, PathKey,
 };
 use parking_lot::Mutex;
 use pretty_assertions::{assert_eq, assert_ne};
@@ -28633,6 +28634,130 @@ async fn test_sticky_scroll(cx: &mut TestAppContext) {
     assert_eq!(sticky_headers(10.0), vec![]);
 }
 
+#[gpui::test]
+fn test_relative_line_numbers(cx: &mut TestAppContext) {
+    init_test(cx, |_| {});
+
+    let buffer_1 = cx.new(|cx| Buffer::local("aaaaaaaaaa\nbbb\n", cx));
+    let buffer_2 = cx.new(|cx| Buffer::local("cccccccccc\nddd\n", cx));
+    let buffer_3 = cx.new(|cx| Buffer::local("eee\nffffffffff\n", cx));
+
+    let multibuffer = cx.new(|cx| {
+        let mut multibuffer = MultiBuffer::new(ReadWrite);
+        multibuffer.push_excerpts(
+            buffer_1.clone(),
+            [ExcerptRange::new(Point::new(0, 0)..Point::new(2, 0))],
+            cx,
+        );
+        multibuffer.push_excerpts(
+            buffer_2.clone(),
+            [ExcerptRange::new(Point::new(0, 0)..Point::new(2, 0))],
+            cx,
+        );
+        multibuffer.push_excerpts(
+            buffer_3.clone(),
+            [ExcerptRange::new(Point::new(0, 0)..Point::new(2, 0))],
+            cx,
+        );
+        multibuffer
+    });
+
+    // wrapped contents of multibuffer:
+    //    aaa
+    //    aaa
+    //    aaa
+    //    a
+    //    bbb
+    //
+    //    ccc
+    //    ccc
+    //    ccc
+    //    c
+    //    ddd
+    //
+    //    eee
+    //    fff
+    //    fff
+    //    fff
+    //    f
+
+    let (editor, cx) = cx.add_window_view(|window, cx| build_editor(multibuffer, window, cx));
+    editor.update_in(cx, |editor, window, cx| {
+        editor.set_wrap_width(Some(30.0.into()), cx); // every 3 characters
+
+        // includes trailing newlines.
+        let expected_line_numbers = [2, 6, 7, 10, 14, 15, 18, 19, 23];
+        let expected_wrapped_line_numbers = [
+            2, 3, 4, 5, 6, 7, 10, 11, 12, 13, 14, 15, 18, 19, 20, 21, 22, 23,
+        ];
+
+        editor.change_selections(SelectionEffects::no_scroll(), window, cx, |s| {
+            s.select_ranges([
+                Point::new(7, 0)..Point::new(7, 1), // second row of `ccc`
+            ]);
+        });
+
+        let snapshot = editor.snapshot(window, cx);
+
+        // these are all 0-indexed
+        let base_display_row = DisplayRow(11);
+        let base_row = 3;
+        let wrapped_base_row = 7;
+
+        // test not counting wrapped lines
+        let expected_relative_numbers = expected_line_numbers
+            .into_iter()
+            .enumerate()
+            .map(|(i, row)| (DisplayRow(row), i.abs_diff(base_row) as u32))
+            .collect_vec();
+        let actual_relative_numbers = snapshot
+            .calculate_relative_line_numbers(
+                &(DisplayRow(0)..DisplayRow(24)),
+                base_display_row,
+                false,
+            )
+            .into_iter()
+            .sorted()
+            .collect_vec();
+        assert_eq!(expected_relative_numbers, actual_relative_numbers);
+        // check `calculate_relative_line_numbers()` against `relative_line_delta()` for each line
+        for (display_row, relative_number) in expected_relative_numbers {
+            assert_eq!(
+                relative_number,
+                snapshot
+                    .relative_line_delta(display_row, base_display_row)
+                    .unsigned_abs() as u32,
+            );
+        }
+
+        // test counting wrapped lines
+        let expected_wrapped_relative_numbers = expected_wrapped_line_numbers
+            .into_iter()
+            .enumerate()
+            .map(|(i, row)| (DisplayRow(row), i.abs_diff(wrapped_base_row) as u32))
+            .collect_vec();
+        let actual_relative_numbers = snapshot
+            .calculate_relative_line_numbers(
+                &(DisplayRow(0)..DisplayRow(24)),
+                base_display_row,
+                true,
+            )
+            .into_iter()
+            .sorted()
+            .collect_vec();
+        assert_eq!(expected_wrapped_relative_numbers, actual_relative_numbers);
+        // check `calculate_relative_line_numbers()` against `relative_wrapped_line_delta()` for each line
+        for (display_row, relative_number) in expected_wrapped_relative_numbers {
+            assert_eq!(
+                relative_number,
+                snapshot
+                    .relative_wrapped_line_delta(display_row, base_display_row)
+                    .unsigned_abs() as u32,
+            );
+        }
+    });
+}
+
 #[gpui::test]
 async fn test_scroll_by_clicking_sticky_header(cx: &mut TestAppContext) {
     init_test(cx, |_| {});

crates/editor/src/element.rs 🔗

@@ -66,7 +66,7 @@ use project::{
 };
 use settings::{
     GitGutterSetting, GitHunkStyleSetting, IndentGuideBackgroundColoring, IndentGuideColoring,
-    Settings,
+    RelativeLineNumbers, Settings,
 };
 use smallvec::{SmallVec, smallvec};
 use std::{
@@ -194,8 +194,6 @@ pub struct EditorElement {
     style: EditorStyle,
 }
 
-type DisplayRowDelta = u32;
-
 impl EditorElement {
     pub(crate) const SCROLLBAR_WIDTH: Pixels = px(15.);
 
@@ -3225,64 +3223,6 @@ impl EditorElement {
             .collect()
     }
 
-    fn calculate_relative_line_numbers(
-        &self,
-        snapshot: &EditorSnapshot,
-        rows: &Range<DisplayRow>,
-        relative_to: Option<DisplayRow>,
-        count_wrapped_lines: bool,
-    ) -> HashMap<DisplayRow, DisplayRowDelta> {
-        let mut relative_rows: HashMap<DisplayRow, DisplayRowDelta> = Default::default();
-        let Some(relative_to) = relative_to else {
-            return relative_rows;
-        };
-
-        let start = rows.start.min(relative_to);
-        let end = rows.end.max(relative_to);
-
-        let buffer_rows = snapshot
-            .row_infos(start)
-            .take(1 + end.minus(start) as usize)
-            .collect::<Vec<_>>();
-
-        let head_idx = relative_to.minus(start);
-        let mut delta = 1;
-        let mut i = head_idx + 1;
-        let should_count_line = |row_info: &RowInfo| {
-            if count_wrapped_lines {
-                row_info.buffer_row.is_some() || row_info.wrapped_buffer_row.is_some()
-            } else {
-                row_info.buffer_row.is_some()
-            }
-        };
-        while i < buffer_rows.len() as u32 {
-            if should_count_line(&buffer_rows[i as usize]) {
-                if rows.contains(&DisplayRow(i + start.0)) {
-                    relative_rows.insert(DisplayRow(i + start.0), delta);
-                }
-                delta += 1;
-            }
-            i += 1;
-        }
-        delta = 1;
-        i = head_idx.min(buffer_rows.len().saturating_sub(1) as u32);
-        while i > 0 && buffer_rows[i as usize].buffer_row.is_none() && !count_wrapped_lines {
-            i -= 1;
-        }
-
-        while i > 0 {
-            i -= 1;
-            if should_count_line(&buffer_rows[i as usize]) {
-                if rows.contains(&DisplayRow(i + start.0)) {
-                    relative_rows.insert(DisplayRow(i + start.0), delta);
-                }
-                delta += 1;
-            }
-        }
-
-        relative_rows
-    }
-
     fn layout_line_numbers(
         &self,
         gutter_hitbox: Option<&Hitbox>,
@@ -3292,7 +3232,7 @@ impl EditorElement {
         rows: Range<DisplayRow>,
         buffer_rows: &[RowInfo],
         active_rows: &BTreeMap<DisplayRow, LineHighlightSpec>,
-        newest_selection_head: Option<DisplayPoint>,
+        relative_line_base: Option<DisplayRow>,
         snapshot: &EditorSnapshot,
         window: &mut Window,
         cx: &mut App,
@@ -3304,32 +3244,16 @@ impl EditorElement {
             return Arc::default();
         }
 
-        let (newest_selection_head, relative) = self.editor.update(cx, |editor, cx| {
-            let newest_selection_head = newest_selection_head.unwrap_or_else(|| {
-                let newest = editor
-                    .selections
-                    .newest::<Point>(&editor.display_snapshot(cx));
-                SelectionLayout::new(
-                    newest,
-                    editor.selections.line_mode(),
-                    editor.cursor_offset_on_selection,
-                    editor.cursor_shape,
-                    &snapshot.display_snapshot,
-                    true,
-                    true,
-                    None,
-                )
-                .head
-            });
-            let relative = editor.relative_line_numbers(cx);
-            (newest_selection_head, relative)
-        });
+        let relative = self.editor.read(cx).relative_line_numbers(cx);
 
         let relative_line_numbers_enabled = relative.enabled();
-        let relative_to = relative_line_numbers_enabled.then(|| newest_selection_head.row());
+        let relative_rows = if relative_line_numbers_enabled && let Some(base) = relative_line_base
+        {
+            snapshot.calculate_relative_line_numbers(&rows, base, relative.wrapped())
+        } else {
+            Default::default()
+        };
 
-        let relative_rows =
-            self.calculate_relative_line_numbers(snapshot, &rows, relative_to, relative.wrapped());
         let mut line_number = String::new();
         let segments = buffer_rows.iter().enumerate().flat_map(|(ix, row_info)| {
             let display_row = DisplayRow(rows.start.0 + ix as u32);
@@ -4652,6 +4576,8 @@ impl EditorElement {
         gutter_hitbox: &Hitbox,
         text_hitbox: &Hitbox,
         style: &EditorStyle,
+        relative_line_numbers: RelativeLineNumbers,
+        relative_to: Option<DisplayRow>,
         window: &mut Window,
         cx: &mut App,
     ) -> Option<StickyHeaders> {
@@ -4681,9 +4607,21 @@ impl EditorElement {
             );
 
             let line_number = show_line_numbers.then(|| {
-                let number = (start_point.row + 1).to_string();
+                let relative_number = relative_to.and_then(|base| match relative_line_numbers {
+                    RelativeLineNumbers::Disabled => None,
+                    RelativeLineNumbers::Enabled => {
+                        Some(snapshot.relative_line_delta_to_point(base, start_point))
+                    }
+                    RelativeLineNumbers::Wrapped => {
+                        Some(snapshot.relative_wrapped_line_delta_to_point(base, start_point))
+                    }
+                });
+                let number = relative_number
+                    .filter(|&delta| delta != 0)
+                    .map(|delta| delta.unsigned_abs() as u32)
+                    .unwrap_or(start_point.row + 1);
                 let color = cx.theme().colors().editor_line_number;
-                self.shape_line_number(SharedString::from(number), color, window)
+                self.shape_line_number(SharedString::from(number.to_string()), color, window)
             });
 
             lines.push(StickyHeaderLine::new(
@@ -9436,6 +9374,28 @@ impl Element for EditorElement {
                             window,
                             cx,
                         );
+
+                    // relative rows are based on newest selection, even outside the visible area
+                    let relative_row_base =  self.editor.update(cx, |editor, cx| {
+                        if editor.selections.count()==0 {
+                            return None;
+                        }
+                            let newest = editor
+                                .selections
+                                .newest::<Point>(&editor.display_snapshot(cx));
+                            Some(SelectionLayout::new(
+                                newest,
+                                editor.selections.line_mode(),
+                                editor.cursor_offset_on_selection,
+                                editor.cursor_shape,
+                                &snapshot.display_snapshot,
+                                true,
+                                true,
+                                None,
+                            )
+                            .head.row())
+                        });
+
                     let mut breakpoint_rows = self.editor.update(cx, |editor, cx| {
                         editor.active_breakpoints(start_row..end_row, window, cx)
                     });
@@ -9453,7 +9413,7 @@ impl Element for EditorElement {
                         start_row..end_row,
                         &row_infos,
                         &active_rows,
-                        newest_selection_head,
+                        relative_row_base,
                         &snapshot,
                         window,
                         cx,
@@ -9773,6 +9733,7 @@ impl Element for EditorElement {
                         && is_singleton
                         && EditorSettings::get_global(cx).sticky_scroll.enabled
                     {
+                        let relative = self.editor.read(cx).relative_line_numbers(cx);
                         self.layout_sticky_headers(
                             &snapshot,
                             editor_width,
@@ -9784,6 +9745,8 @@ impl Element for EditorElement {
                             &gutter_hitbox,
                             &text_hitbox,
                             &style,
+                            relative,
+                            relative_row_base,
                             window,
                             cx,
                         )
@@ -11631,7 +11594,7 @@ mod tests {
     }
 
     #[gpui::test]
-    fn test_shape_line_numbers(cx: &mut TestAppContext) {
+    fn test_layout_line_numbers(cx: &mut TestAppContext) {
         init_test(cx, |_| {});
         let window = cx.add_window(|window, cx| {
             let buffer = MultiBuffer::build_simple(&sample_text(6, 6, 'a'), cx);
@@ -11671,7 +11634,7 @@ mod tests {
                         })
                         .collect::<Vec<_>>(),
                     &BTreeMap::default(),
-                    Some(DisplayPoint::new(DisplayRow(0), 0)),
+                    Some(DisplayRow(0)),
                     &snapshot,
                     window,
                     cx,
@@ -11683,10 +11646,9 @@ mod tests {
         let relative_rows = window
             .update(cx, |editor, window, cx| {
                 let snapshot = editor.snapshot(window, cx);
-                element.calculate_relative_line_numbers(
-                    &snapshot,
+                snapshot.calculate_relative_line_numbers(
                     &(DisplayRow(0)..DisplayRow(6)),
-                    Some(DisplayRow(3)),
+                    DisplayRow(3),
                     false,
                 )
             })
@@ -11702,10 +11664,9 @@ mod tests {
         let relative_rows = window
             .update(cx, |editor, window, cx| {
                 let snapshot = editor.snapshot(window, cx);
-                element.calculate_relative_line_numbers(
-                    &snapshot,
+                snapshot.calculate_relative_line_numbers(
                     &(DisplayRow(3)..DisplayRow(6)),
-                    Some(DisplayRow(1)),
+                    DisplayRow(1),
                     false,
                 )
             })
@@ -11719,10 +11680,9 @@ mod tests {
         let relative_rows = window
             .update(cx, |editor, window, cx| {
                 let snapshot = editor.snapshot(window, cx);
-                element.calculate_relative_line_numbers(
-                    &snapshot,
+                snapshot.calculate_relative_line_numbers(
                     &(DisplayRow(0)..DisplayRow(3)),
-                    Some(DisplayRow(6)),
+                    DisplayRow(6),
                     false,
                 )
             })
@@ -11759,7 +11719,7 @@ mod tests {
                         })
                         .collect::<Vec<_>>(),
                     &BTreeMap::default(),
-                    Some(DisplayPoint::new(DisplayRow(0), 0)),
+                    Some(DisplayRow(0)),
                     &snapshot,
                     window,
                     cx,
@@ -11774,7 +11734,7 @@ mod tests {
     }
 
     #[gpui::test]
-    fn test_shape_line_numbers_wrapping(cx: &mut TestAppContext) {
+    fn test_layout_line_numbers_wrapping(cx: &mut TestAppContext) {
         init_test(cx, |_| {});
         let window = cx.add_window(|window, cx| {
             let buffer = MultiBuffer::build_simple(&sample_text(6, 6, 'a'), cx);
@@ -11819,7 +11779,7 @@ mod tests {
                         })
                         .collect::<Vec<_>>(),
                     &BTreeMap::default(),
-                    Some(DisplayPoint::new(DisplayRow(0), 0)),
+                    Some(DisplayRow(0)),
                     &snapshot,
                     window,
                     cx,
@@ -11831,10 +11791,9 @@ mod tests {
         let relative_rows = window
             .update(cx, |editor, window, cx| {
                 let snapshot = editor.snapshot(window, cx);
-                element.calculate_relative_line_numbers(
-                    &snapshot,
+                snapshot.calculate_relative_line_numbers(
                     &(DisplayRow(0)..DisplayRow(6)),
-                    Some(DisplayRow(3)),
+                    DisplayRow(3),
                     true,
                 )
             })
@@ -11871,7 +11830,7 @@ mod tests {
                         })
                         .collect::<Vec<_>>(),
                     &BTreeMap::from_iter([(DisplayRow(0), LineHighlightSpec::default())]),
-                    Some(DisplayPoint::new(DisplayRow(0), 0)),
+                    Some(DisplayRow(0)),
                     &snapshot,
                     window,
                     cx,
@@ -11886,10 +11845,9 @@ mod tests {
         let relative_rows = window
             .update(cx, |editor, window, cx| {
                 let snapshot = editor.snapshot(window, cx);
-                element.calculate_relative_line_numbers(
-                    &snapshot,
+                snapshot.calculate_relative_line_numbers(
                     &(DisplayRow(0)..DisplayRow(6)),
-                    Some(DisplayRow(3)),
+                    DisplayRow(3),
                     true,
                 )
             })