Fix scroll thumb (#10667)

Andrew Lygin created 2 years ago

Editor scrollbar has several issues that show up on large files:

- The thumb scrolls beyond the window.
- When dragged, the thumb goes out of sync with the mouse pointer.
- When the scrollbar trunk is clicked, the thumb jumps incorrectly.


https://github.com/zed-industries/zed/assets/2101250/320dba59-a526-4e68-99b3-1186271ba839

The reason is that the scrollbar now has two modes:
1. The "basic mode" for small files, when the thumb height correctly
represents the visible area, i.e. the top of the thumb matches the top
visible row (let's call it top-to-top sync), and the bottom of the thumb
matches the bottom visible row.
2. The "extended mode" for large files, when thumb becomes too small and
we have to impose minimal height to it. In this mode we have a vertical
offset of the first row position inside the scrollbar, we try to
position the thumb center-to-center with the editor.

...and the second mode is not implemented correctly. Also, mouse event
handlers ignore it. It is possible to fix this implementation, but I'm
not sure if it worth doing because it a) leads to some confusing cases
(for instance, in the extended mode the first row marker is not at the
top of the scrollbar), and b) differs from what all other editors do.

Here's a previous mentioning of this problem:
https://github.com/zed-industries/zed/pull/9080#pullrequestreview-1927465293

This PR changes the "extended mode", making it synchronize the thumb
top-to-top with the editor. It solves all the mentioned problems and
makes the scroll thumb work the same whay as in other editors.

But if you want to stick to the idea of the center-to-center sync for
large files, I can do that too.

Release Notes:

- Fixed scroll thumb behaviour.

Optionally, include screenshots / media showcasing your addition that
can be included in the release notes.

- N/A

Change summary

crates/editor/src/element.rs | 61 +++++++++++++------------------------
1 file changed, 21 insertions(+), 40 deletions(-)

Detailed changes

crates/editor/src/element.rs

@@ -980,8 +980,7 @@ impl EditorElement {
         snapshot: &EditorSnapshot,
         bounds: Bounds<Pixels>,
         scroll_position: gpui::Point<f32>,
-        line_height: Pixels,
-        height_in_lines: f32,
+        rows_per_page: f32,
         cx: &mut ElementContext,
     ) -> Option<ScrollbarLayout> {
         let scrollbar_settings = EditorSettings::get_global(cx).scrollbar;
@@ -1012,7 +1011,7 @@ impl EditorElement {
             return None;
         }
 
-        let visible_row_range = scroll_position.y..scroll_position.y + height_in_lines;
+        let visible_row_range = scroll_position.y..scroll_position.y + rows_per_page;
 
         // If a drag took place after we started dragging the scrollbar,
         // cancel the scrollbar drag.
@@ -1027,27 +1026,18 @@ impl EditorElement {
             point(bounds.lower_right().x, bounds.lower_left().y),
         );
 
-        let scroll_height = snapshot.max_point().row() as f32 + height_in_lines;
-        let mut height = bounds.size.height;
-        let mut first_row_y_offset = px(0.0);
-
-        // Impose a minimum height on the scrollbar thumb
-        let row_height = height / scroll_height;
-        let min_thumb_height = line_height;
-        let thumb_height = height_in_lines * row_height;
-        if thumb_height < min_thumb_height {
-            first_row_y_offset = (min_thumb_height - thumb_height) / 2.0;
-            height -= min_thumb_height - thumb_height;
-        }
+        let height = bounds.size.height;
+        let total_rows = snapshot.max_point().row() as f32 + rows_per_page;
+        let px_per_row = height / total_rows;
+        let thumb_height = (rows_per_page * px_per_row).max(ScrollbarLayout::MIN_THUMB_HEIGHT);
+        let row_height = (height - thumb_height) / snapshot.max_point().row() as f32;
 
         Some(ScrollbarLayout {
             hitbox: cx.insert_hitbox(track_bounds, false),
             visible_row_range,
-            height,
-            scroll_height,
-            first_row_y_offset,
             row_height,
             visible: show_scrollbars,
+            thumb_height,
         })
     }
 
@@ -2507,8 +2497,7 @@ impl EditorElement {
 
         cx.set_cursor_style(CursorStyle::Arrow, &scrollbar_layout.hitbox);
 
-        let scroll_height = scrollbar_layout.scroll_height;
-        let height = scrollbar_layout.height;
+        let row_height = scrollbar_layout.row_height;
         let row_range = scrollbar_layout.visible_row_range.clone();
 
         cx.on_mouse_event({
@@ -2528,14 +2517,13 @@ impl EditorElement {
                         let new_y = event.position.y;
                         if (hitbox.top()..hitbox.bottom()).contains(&y) {
                             let mut position = editor.scroll_position(cx);
-                            position.y += (new_y - y) * scroll_height / height;
+                            position.y += (new_y - y) / row_height;
                             if position.y < 0.0 {
                                 position.y = 0.0;
                             }
                             editor.set_scroll_position(position, cx);
                         }
 
-                        mouse_position = event.position;
                         cx.stop_propagation();
                     } else {
                         editor.scroll_manager.set_is_dragging_scrollbar(false, cx);
@@ -2543,6 +2531,7 @@ impl EditorElement {
                             editor.scroll_manager.show_scrollbar(cx);
                         }
                     }
+                    mouse_position = event.position;
                 })
             }
         });
@@ -2575,8 +2564,7 @@ impl EditorElement {
 
                         let y = event.position.y;
                         if y < thumb_bounds.top() || thumb_bounds.bottom() < y {
-                            let center_row =
-                                ((y - hitbox.top()) * scroll_height / height).round() as u32;
+                            let center_row = ((y - hitbox.top()) / row_height).round() as u32;
                             let top_row = center_row
                                 .saturating_sub((row_range.end - row_range.start) as u32 / 2);
                             let mut position = editor.scroll_position(cx);
@@ -3664,14 +3652,8 @@ impl Element for EditorElement {
                     cx,
                 );
 
-                let scrollbar_layout = self.layout_scrollbar(
-                    &snapshot,
-                    bounds,
-                    scroll_position,
-                    line_height,
-                    height_in_lines,
-                    cx,
-                );
+                let scrollbar_layout =
+                    self.layout_scrollbar(&snapshot, bounds, scroll_position, height_in_lines, cx);
 
                 let folds = cx.with_element_id(Some("folds"), |cx| {
                     self.layout_folds(
@@ -3929,19 +3911,18 @@ struct ScrollbarLayout {
     hitbox: Hitbox,
     visible_row_range: Range<f32>,
     visible: bool,
-    height: Pixels,
-    scroll_height: f32,
-    first_row_y_offset: Pixels,
     row_height: Pixels,
+    thumb_height: Pixels,
 }
 
 impl ScrollbarLayout {
     const BORDER_WIDTH: Pixels = px(1.0);
     const MIN_MARKER_HEIGHT: Pixels = px(2.0);
+    const MIN_THUMB_HEIGHT: Pixels = px(20.0);
 
     fn thumb_bounds(&self) -> Bounds<Pixels> {
-        let thumb_top = self.y_for_row(self.visible_row_range.start) - self.first_row_y_offset;
-        let thumb_bottom = self.y_for_row(self.visible_row_range.end) + self.first_row_y_offset;
+        let thumb_top = self.y_for_row(self.visible_row_range.start);
+        let thumb_bottom = thumb_top + self.thumb_height;
         Bounds::from_corners(
             point(self.hitbox.left(), thumb_top),
             point(self.hitbox.right(), thumb_bottom),
@@ -3949,7 +3930,7 @@ impl ScrollbarLayout {
     }
 
     fn y_for_row(&self, row: f32) -> Pixels {
-        self.hitbox.top() + self.first_row_y_offset + row * self.row_height
+        self.hitbox.top() + row * self.row_height
     }
 
     fn marker_quads_for_ranges(
@@ -3966,8 +3947,8 @@ impl ScrollbarLayout {
         let mut background_pixel_ranges = row_ranges
             .into_iter()
             .map(|range| {
-                let start_y = self.first_row_y_offset + range.start as f32 * self.row_height;
-                let end_y = self.first_row_y_offset + (range.end + 1) as f32 * self.row_height;
+                let start_y = range.start as f32 * self.row_height;
+                let end_y = (range.end + 1) as f32 * self.row_height;
                 ColoredRange {
                     start: start_y,
                     end: end_y,