editor: Fix autoscroll sometimes obscuring the current row behind a sticky header (#53165)

Fixes an autoscroll bug that sometimes obscures the current row behind a
sticky header.

The bug was caused by `Editor::autoscroll_vertically` using the
`sticky_header_line_count` value that was cached during the previous
render, which isn't necessarily the number of sticky headers that the
scroll target needs, e.g.
- when jumping to a definition
- when pressing the up arrow key when the selection is in the topmost
visible row with `"vertical_scroll_margin": 0`

This fixes that by not caching `sticky_header_line_count` and instead
querying Tree-sitter on the fly to get the number of sticky headers that
the scroll target needs. Performance-wise this seem okay, I measured it
to take less than 200µs on my machine using a very large Rust file (and
there are still some possible ways to optimize this if necessary). In
particular, the pathological huge single-line file case as discussed in
#48450 shouldn't be an issue here because unlike the main sticky header
Tree-sitter query, here we query the outline items that contain a
particular point rather than those that intersect an entire row.

The `ScrollCursorTop` handler is also simplified to use the same shared
function for counting the number of sticky headers, since that action
doesn't rely on autoscroll.

Before:


https://github.com/user-attachments/assets/efb12776-82d9-4b94-baa5-347ec769fb98

After:


https://github.com/user-attachments/assets/236deb9f-fe06-43bd-b167-7bd3ab719e4c

Self-Review Checklist:

- [x] I've reviewed my own diff for quality, security, and reliability
- [x] Unsafe blocks (if any) have justifying comments
- [x] The content is consistent with the [UI/UX
checklist](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist)
- [x] Tests cover the new/changed behavior
- [x] Performance impact has been considered and is acceptable

Closes #50803 

Release Notes:

- Fixed sticky headers sometimes obscuring the current row.

---------

Co-authored-by: Conrad Irwin <conrad.irwin@gmail.com>

crates/editor/src/editor_tests.rs

@@ -32693,6 +32693,80 @@ async fn test_no_duplicated_sticky_headers(cx: &mut TestAppContext) {
     assert_eq!(sticky_headers(5.0), vec![]);
 }
 
+#[gpui::test]
+async fn test_autoscroll_keeps_cursor_visible_below_sticky_headers(cx: &mut TestAppContext) {
+    init_test(cx, |_| {});
+    update_test_editor_settings(cx, &|settings| {
+        settings.vertical_scroll_margin = Some(0.0);
+        settings.scroll_beyond_last_line = Some(ScrollBeyondLastLine::OnePage);
+        settings.sticky_scroll = Some(settings::StickyScrollContent {
+            enabled: Some(true),
+        });
+    });
+    let mut cx = EditorTestContext::new(cx).await;
+
+    cx.set_state(indoc! {"
+        impl Foo { fn bar() {
+            let x = 1;
+            fn baz() {
+                let y = 2;
+            }
+        } }
+        ˇ
+    "});
+
+    let mut previous_cursor_row = cx.update_editor(|editor, window, cx| {
+        editor
+            .buffer()
+            .read(cx)
+            .as_singleton()
+            .unwrap()
+            .update(cx, |buffer, cx| buffer.set_language(Some(rust_lang()), cx));
+        let cursor_row = editor
+            .selections
+            .newest_display(&editor.display_snapshot(cx))
+            .head()
+            .row();
+        editor.set_scroll_top_row(cursor_row, window, cx);
+        cursor_row
+    });
+
+    for _ in 0..6 {
+        cx.update_editor(|editor, window, cx| editor.move_up(&MoveUp, window, cx));
+        cx.run_until_parked();
+
+        cx.update_editor(|editor, window, cx| {
+            let snapshot = editor.snapshot(window, cx);
+            let scroll_top = snapshot.scroll_position().y;
+            let sticky_header_count = EditorElement::sticky_headers(editor, &snapshot).len();
+            let cursor_row = editor
+                .selections
+                .newest_display(&snapshot.display_snapshot)
+                .head()
+                .row();
+            assert_eq!(
+                cursor_row,
+                previous_cursor_row
+                    .previous_row()
+                    .max(DisplayRow(scroll_top as u32) + DisplayRow(sticky_header_count as u32))
+            );
+            previous_cursor_row = cursor_row;
+        });
+
+        // The `ScrollCursorTop` action shouldn't change the scroll position, as the cursor is
+        // already as high up as the sticky headers allow.
+        let scroll_top_before =
+            cx.update_editor(|editor, window, cx| editor.snapshot(window, cx).scroll_position().y);
+        cx.update_editor(|editor, window, cx| {
+            editor.scroll_cursor_top(&ScrollCursorTop, window, cx)
+        });
+        cx.run_until_parked();
+        let scroll_top_after =
+            cx.update_editor(|editor, window, cx| editor.snapshot(window, cx).scroll_position().y);
+        assert_eq!(scroll_top_before, scroll_top_after);
+    }
+}
+
 #[gpui::test]
 fn test_relative_line_numbers(cx: &mut TestAppContext) {
     init_test(cx, |_| {});

crates/editor/src/element.rs

@@ -10484,11 +10484,6 @@ impl Element for EditorElement {
                     } else {
                         None
                     };
-                    self.editor.update(cx, |editor, _| {
-                        editor.scroll_manager.set_sticky_header_line_count(
-                            sticky_headers.as_ref().map_or(0, |h| h.lines.len()),
-                        );
-                    });
                     let indent_guides =
                         if scroll_pixel_position != preliminary_scroll_pixel_position {
                             self.layout_indent_guides(

crates/editor/src/scroll.rs

@@ -201,8 +201,6 @@ pub struct ScrollManager {
     /// Each side separately clamps the x component using its own scroll_max_x when reading from the SharedScrollAnchor.
     scroll_max_x: Option<f64>,
     ongoing: OngoingScroll,
-    /// Number of sticky header lines currently being rendered for the current scroll position.
-    sticky_header_line_count: usize,
     /// The second element indicates whether the autoscroll request is local
     /// (true) or remote (false). Local requests are initiated by user actions,
     /// while remote requests come from external sources.
@@ -234,7 +232,6 @@ impl ScrollManager {
             anchor,
             scroll_max_x: None,
             ongoing: OngoingScroll::new(),
-            sticky_header_line_count: 0,
             autoscroll_request: None,
             show_scrollbars: true,
             hide_scrollbar_task: None,
@@ -273,7 +270,6 @@ impl ScrollManager {
             this.display_map_id = Some(my_snapshot.display_map_id);
         });
         self.ongoing = other.ongoing;
-        self.sticky_header_line_count = other.sticky_header_line_count;
     }
 
     pub fn offset(&self, cx: &App) -> gpui::Point<f64> {
@@ -360,14 +356,6 @@ impl ScrollManager {
         pos
     }
 
-    pub fn sticky_header_line_count(&self) -> usize {
-        self.sticky_header_line_count
-    }
-
-    pub fn set_sticky_header_line_count(&mut self, count: usize) {
-        self.sticky_header_line_count = count;
-    }
-
     fn set_scroll_position(
         &mut self,
         scroll_position: gpui::Point<ScrollOffset>,

crates/editor/src/scroll/actions.rs

@@ -1,12 +1,10 @@
 use super::Axis;
 use crate::{
-    Autoscroll, Editor, EditorMode, EditorSettings, NextScreen, NextScrollCursorCenterTopBottom,
+    Autoscroll, Editor, EditorMode, NextScreen, NextScrollCursorCenterTopBottom,
     SCROLL_CENTER_TOP_BOTTOM_DEBOUNCE_TIMEOUT, ScrollCursorBottom, ScrollCursorCenter,
     ScrollCursorCenterTopBottom, ScrollCursorTop, display_map::DisplayRow, scroll::ScrollOffset,
 };
 use gpui::{Context, Point, Window};
-use settings::Settings;
-use text::ToOffset;
 
 impl Editor {
     pub fn next_screen(&mut self, _: &NextScreen, window: &mut Window, cx: &mut Context<Editor>) {
@@ -77,23 +75,9 @@ impl Editor {
         let scroll_margin_rows = self.vertical_scroll_margin() as u32;
         let selection_head = self.selections.newest_display(&display_snapshot).head();
 
-        let sticky_headers_len = if EditorSettings::get_global(cx).sticky_scroll.enabled
-            && let Some(buffer_snapshot) = display_snapshot.buffer_snapshot().as_singleton()
-        {
-            let select_head_point =
-                rope::Point::new(selection_head.to_point(&display_snapshot).row, 0);
-            buffer_snapshot
-                .outline_items_containing(select_head_point..select_head_point, false, None)
-                .iter()
-                .filter(|outline| {
-                    outline.range.start.offset
-                        < select_head_point.to_offset(&buffer_snapshot) as u32
-                })
-                .collect::<Vec<_>>()
-                .len()
-        } else {
-            0
-        } as u32;
+        let sticky_headers_len =
+            self.visible_sticky_header_count_for_point(&display_snapshot, selection_head, cx)
+                as u32;
 
         let new_screen_top = selection_head.row().0;
         let header_offset = display_snapshot

crates/editor/src/scroll/autoscroll.rs

@@ -1,12 +1,15 @@
 use crate::{
-    DisplayRow, Editor, EditorMode, LineWithInvisibles, RowExt, SelectionEffects,
-    display_map::ToDisplayPoint,
+    DisplayPoint, DisplayRow, Editor, EditorMode, EditorSettings, LineWithInvisibles, RowExt,
+    SelectionEffects,
+    display_map::{DisplaySnapshot, ToDisplayPoint},
     scroll::{ScrollOffset, WasScrolled},
 };
-use gpui::{Bounds, Context, Pixels, Window};
+use gpui::{App, Bounds, Context, Pixels, Window};
 use language::Point;
 use multi_buffer::Anchor;
+use settings::Settings;
 use std::cmp;
+use text::Bias;
 
 #[derive(Debug, PartialEq, Eq, Clone, Copy)]
 pub enum Autoscroll {
@@ -140,23 +143,24 @@ impl Editor {
             return (NeedsHorizontalAutoscroll(false), editor_was_scrolled);
         };
 
+        let mut target_point;
         let mut target_top;
         let mut target_bottom;
         if let Some(first_highlighted_row) =
             self.highlighted_display_row_for_autoscroll(&display_map)
         {
-            target_top = first_highlighted_row.as_f64();
+            target_point = DisplayPoint::new(first_highlighted_row, 0);
+            target_top = target_point.row().as_f64();
             target_bottom = target_top + 1.;
         } else {
             let selections = self.selections.all::<Point>(&display_map);
 
-            target_top = selections
+            target_point = selections
                 .first()
                 .unwrap()
                 .head()
-                .to_display_point(&display_map)
-                .row()
-                .as_f64();
+                .to_display_point(&display_map);
+            target_top = target_point.row().as_f64();
             target_bottom = selections
                 .last()
                 .unwrap()
@@ -173,21 +177,17 @@ impl Editor {
             ) || (matches!(autoscroll, Autoscroll::Strategy(AutoscrollStrategy::Fit, _))
                 && !selections_fit)
             {
-                let newest_selection_top = selections
+                target_point = selections
                     .iter()
                     .max_by_key(|s| s.id)
                     .unwrap()
                     .head()
-                    .to_display_point(&display_map)
-                    .row()
-                    .as_f64();
-                target_top = newest_selection_top;
-                target_bottom = newest_selection_top + 1.;
+                    .to_display_point(&display_map);
+                target_top = target_point.row().as_f64();
+                target_bottom = target_top + 1.;
             }
         }
 
-        let visible_sticky_headers = self.scroll_manager.sticky_header_line_count();
-
         let margin = if matches!(self.mode, EditorMode::AutoHeight { .. }) {
             0.
         } else {
@@ -209,10 +209,14 @@ impl Editor {
                 .unwrap_or_default(),
         };
         if let Autoscroll::Strategy(_, Some(anchor)) = autoscroll {
-            target_top = anchor.to_display_point(&display_map).row().as_f64();
+            target_point = anchor.to_display_point(&display_map);
+            target_top = target_point.row().as_f64();
             target_bottom = target_top + 1.;
         }
 
+        let visible_sticky_headers =
+            self.visible_sticky_header_count_for_point(&display_map, target_point, cx);
+
         let was_autoscrolled = match strategy {
             AutoscrollStrategy::Fit | AutoscrollStrategy::Newest => {
                 let margin = margin.min(self.scroll_manager.vertical_scroll_margin);
@@ -274,6 +278,54 @@ impl Editor {
         (NeedsHorizontalAutoscroll(true), was_scrolled)
     }
 
+    pub(crate) fn visible_sticky_header_count_for_point(
+        &self,
+        display_map: &DisplaySnapshot,
+        target_point: DisplayPoint,
+        cx: &App,
+    ) -> usize {
+        let sticky_scroll = EditorSettings::get_global(cx).sticky_scroll;
+        if !sticky_scroll.enabled {
+            return 0;
+        }
+
+        let Some(buffer_snapshot) = display_map.buffer_snapshot().as_singleton() else {
+            return 0;
+        };
+
+        let point = target_point.to_point(display_map);
+        let mut item_ranges = buffer_snapshot
+            .outline_ranges_containing(point..point)
+            .collect::<Vec<_>>();
+        item_ranges.sort_by_key(|item_range| item_range.start);
+
+        let mut previous_sticky_row = None;
+        let mut num_visible_sticky_headers = 0;
+
+        for item_range in item_ranges {
+            let sticky_row = display_map
+                .point_to_display_point(item_range.start, Bias::Left)
+                .row();
+            if sticky_row >= target_point.row() {
+                break;
+            }
+            if previous_sticky_row.replace(sticky_row) == Some(sticky_row) {
+                continue;
+            }
+
+            let end_row = display_map
+                .point_to_display_point(item_range.end, Bias::Left)
+                .row();
+            if end_row <= target_point.row() {
+                continue;
+            }
+
+            num_visible_sticky_headers += 1;
+        }
+
+        num_visible_sticky_headers
+    }
+
     pub(crate) fn autoscroll_horizontally(
         &mut self,
         start_row: DisplayRow,

crates/language/src/buffer.rs

@@ -4251,7 +4251,10 @@ impl BufferSnapshot {
         items
     }
 
-    pub fn outline_range_containing<T: ToOffset>(&self, range: Range<T>) -> Option<Range<Point>> {
+    pub fn outline_ranges_containing<T: ToOffset>(
+        &self,
+        range: Range<T>,
+    ) -> impl Iterator<Item = Range<Point>> + '_ {
         let range = range.to_offset(self);
         let mut matches = self.syntax.matches(range.clone(), &self.text, |grammar| {
             grammar.outline_config.as_ref().map(|c| &c.query)
@@ -4262,35 +4265,41 @@ impl BufferSnapshot {
             .map(|g| g.outline_config.as_ref().unwrap())
             .collect::<Vec<_>>();
 
-        while let Some(mat) = matches.peek() {
-            let config = &configs[mat.grammar_index];
-            let containing_item_node = maybe!({
-                let item_node = mat.captures.iter().find_map(|cap| {
-                    if cap.index == config.item_capture_ix {
-                        Some(cap.node)
-                    } else {
+        std::iter::from_fn(move || {
+            while let Some(mat) = matches.peek() {
+                let config = &configs[mat.grammar_index];
+                let containing_item_node = maybe!({
+                    let item_node = mat.captures.iter().find_map(|cap| {
+                        if cap.index == config.item_capture_ix {
+                            Some(cap.node)
+                        } else {
+                            None
+                        }
+                    })?;
+
+                    let item_byte_range = item_node.byte_range();
+                    if item_byte_range.end < range.start || item_byte_range.start > range.end {
                         None
+                    } else {
+                        Some(item_node)
                     }
-                })?;
+                });
 
-                let item_byte_range = item_node.byte_range();
-                if item_byte_range.end < range.start || item_byte_range.start > range.end {
-                    None
-                } else {
-                    Some(item_node)
-                }
-            });
-
-            if let Some(item_node) = containing_item_node {
-                return Some(
+                let range = containing_item_node.as_ref().map(|item_node| {
                     Point::from_ts_point(item_node.start_position())
-                        ..Point::from_ts_point(item_node.end_position()),
-                );
+                        ..Point::from_ts_point(item_node.end_position())
+                });
+                matches.advance();
+                if range.is_some() {
+                    return range;
+                }
             }
+            None
+        })
+    }
 
-            matches.advance();
-        }
-        None
+    pub fn outline_range_containing<T: ToOffset>(&self, range: Range<T>) -> Option<Range<Point>> {
+        self.outline_ranges_containing(range).next()
     }
 
     pub fn outline_items_containing<T: ToOffset>(