From 9d9bce08a7acb25352f110af5404811ef30708c6 Mon Sep 17 00:00:00 2001 From: Andrew Lygin Date: Fri, 19 Apr 2024 23:18:37 +0300 Subject: [PATCH] Fix scroll thumb (#10667) 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 --- crates/editor/src/element.rs | 61 +++++++++++++----------------------- 1 file changed, 21 insertions(+), 40 deletions(-) diff --git a/crates/editor/src/element.rs b/crates/editor/src/element.rs index 810657632c..b8ee1643c6 100644 --- a/crates/editor/src/element.rs +++ b/crates/editor/src/element.rs @@ -980,8 +980,7 @@ impl EditorElement { snapshot: &EditorSnapshot, bounds: Bounds, scroll_position: gpui::Point, - line_height: Pixels, - height_in_lines: f32, + rows_per_page: f32, cx: &mut ElementContext, ) -> Option { 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, 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 { - 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,