diff --git a/crates/image_viewer/src/image_viewer.rs b/crates/image_viewer/src/image_viewer.rs index a683ff1986..33f286c820 100644 --- a/crates/image_viewer/src/image_viewer.rs +++ b/crates/image_viewer/src/image_viewer.rs @@ -12,8 +12,8 @@ use editor::{ }; use file_icons::FileIcons; use gpui::{ - AnyElement, App, Bounds, Context, DismissEvent, DispatchPhase, DragMoveEvent, Element, - ElementId, Entity, EventEmitter, FocusHandle, Focusable, Font, GlobalElementId, + AnyElement, App, Bounds, Context, CursorStyle, DismissEvent, DispatchPhase, DragMoveEvent, + Element, ElementId, Entity, EventEmitter, FocusHandle, Focusable, Font, GlobalElementId, InspectorElementId, InteractiveElement, IntoElement, KeyDownEvent, LayoutId, MouseButton, MouseDownEvent, MouseMoveEvent, MouseUpEvent, ParentElement, PinchEvent, Pixels, Point, Render, RenderImage, ScrollDelta, ScrollWheelEvent, Style, Styled, Subscription, Task, WeakEntity, @@ -113,21 +113,21 @@ const MAX_ZOOM: f32 = 20.0; const ZOOM_STEP: f32 = 1.1; const SCROLL_LINE_MULTIPLIER: f32 = 20.0; -/// Scroll offset that keeps the document point under `anchor_rel_y` (offset from the -/// viewport top) stationary while the zoom scales by `zoom_ratio`. `max_offset_y` and -/// `viewport_height` describe the pre-zoom layout; the result is clamped to the scaled -/// scrollable range. -fn pdf_anchored_scroll_y( - offset_y: Pixels, - anchor_rel_y: Pixels, +/// Scroll offset that keeps the document point under `anchor_rel` (offset from the +/// viewport top/left on one axis) stationary while the zoom scales by `zoom_ratio`. +/// `max_offset` and `viewport_extent` describe the pre-zoom layout on that axis; the +/// result is clamped to the scaled scrollable range. +fn pdf_anchored_scroll_axis( + offset: Pixels, + anchor_rel: Pixels, zoom_ratio: f32, - max_offset_y: Pixels, - viewport_height: Pixels, + max_offset: Pixels, + viewport_extent: Pixels, ) -> Pixels { - // Zooming out below the fit height leaves the content shorter than the viewport, + // Zooming out below the fit size leaves the content shorter than the viewport, // where the scrollable range collapses to zero rather than going negative. - let max_y = (((max_offset_y + viewport_height) * zoom_ratio) - viewport_height).max(px(0.0)); - (anchor_rel_y * (1.0 - zoom_ratio) + offset_y * zoom_ratio).clamp(-max_y, px(0.0)) + let max = (((max_offset + viewport_extent) * zoom_ratio) - viewport_extent).max(px(0.0)); + (anchor_rel * (1.0 - zoom_ratio) + offset * zoom_ratio).clamp(-max, px(0.0)) } const BASE_SQUARE_SIZE: f32 = 32.0; @@ -1689,15 +1689,23 @@ impl ImageView { Some(position) => position - viewport.origin, None => point(viewport.size.width / 2.0, viewport.size.height / 2.0), }; - let new_y = pdf_anchored_scroll_y( - self.pdf_scroll_handle.offset().y, + let offset = self.pdf_scroll_handle.offset(); + let max_offset = self.pdf_scroll_handle.max_offset(); + let new_x = pdf_anchored_scroll_axis( + offset.x, + anchor_rel.x, + zoom_ratio, + max_offset.x, + viewport.size.width, + ); + let new_y = pdf_anchored_scroll_axis( + offset.y, anchor_rel.y, zoom_ratio, - self.pdf_scroll_handle.max_offset().y, + max_offset.y, viewport.size.height, ); - // The container only scrolls vertically; horizontal placement is layout-driven. - self.pdf_scroll_handle.set_offset(point(px(0.0), new_y)); + self.pdf_scroll_handle.set_offset(point(new_x, new_y)); cx.notify(); } @@ -3624,8 +3632,12 @@ impl Render for ImageView { .id("pdf-scroll-container") .debug_selector(|| "pdf-scroll-container".into()) .track_scroll(&self.pdf_scroll_handle) - .overflow_y_scroll() + .overflow_scroll() .size_full() + .when( + self.pdf_middle_pan.is_some(), + |this| this.cursor(CursorStyle::ClosedHand), + ) .on_scroll_wheel(cx.listener(Self::handle_pdf_scroll_wheel)) .on_pinch(cx.listener(Self::handle_pdf_pinch)) .on_mouse_down( @@ -5094,17 +5106,25 @@ mod tests { // Zooming in 2x with the anchor 100px below the viewport top must keep the // document point at that screen position: content coord 1100px (100 - (-1000)) // maps back to 1100 * 2 - 2100 = 100. - let scrolled = pdf_anchored_scroll_y(px(-1000.0), px(100.0), 2.0, px(2000.0), px(500.0)); + let scrolled = pdf_anchored_scroll_axis(px(-1000.0), px(100.0), 2.0, px(2000.0), px(500.0)); assert_eq!(scrolled, px(-2100.0)); + // The same math anchors the horizontal axis around its own extent. + let scrolled = pdf_anchored_scroll_axis(px(0.0), px(700.0), 2.0, px(300.0), px(800.0)); + assert_eq!(scrolled, px(-700.0)); + // Zooming out above the top edge clamps to the top instead of overscrolling. - let scrolled = pdf_anchored_scroll_y(px(-100.0), px(300.0), 0.5, px(100.0), px(500.0)); + let scrolled = pdf_anchored_scroll_axis(px(-100.0), px(300.0), 0.5, px(100.0), px(500.0)); + assert_eq!(scrolled, px(0.0)); + + // Zooming out below the fit size collapses the scrollable range entirely. + let scrolled = pdf_anchored_scroll_axis(px(-100.0), px(300.0), 0.5, px(0.0), px(500.0)); assert_eq!(scrolled, px(0.0)); // Zooming out with the viewport at the bottom edge clamps to the scaled bottom // boundary instead of following the anchor: raw target 100*0.5 + (-2000)*0.5 = // -950 exceeds the scaled max of (2000+500)*0.5-500 = 750. - let scrolled = pdf_anchored_scroll_y(px(-2000.0), px(100.0), 0.5, px(2000.0), px(500.0)); + let scrolled = pdf_anchored_scroll_axis(px(-2000.0), px(100.0), 0.5, px(2000.0), px(500.0)); assert_eq!(scrolled, px(-750.0)); } @@ -5763,6 +5783,63 @@ mod tests { offset_pan.y < offset_after.y, "dragging down should scroll the viewport up, got {offset_pan:?} vs {offset_after:?}" ); + + // Zoom in twice more so the page overflows horizontally by a drag-worthy + // margin, then drag right: the content should move right, scrolling the + // viewport left. + for _ in 0..2 { + cx.simulate_event(ScrollWheelEvent { + position: anchor, + delta: ScrollDelta::Lines(point(0.0, 3.0)), + modifiers: Modifiers { + control: true, + ..Modifiers::default() + }, + touch_phase: TouchPhase::Moved, + }); + cx.run_until_parked(); + } + cx.update(|window, _| window.refresh()); + cx.run_until_parked(); + + let offset_zoomed = cx.read(|cx| image_view.read(cx).pdf_scroll_handle.offset()); + let drag_from = point( + container.origin.x + container.size.width * 0.5, + container.origin.y + container.size.height * 0.5, + ); + cx.simulate_event(MouseDownEvent { + position: drag_from, + button: MouseButton::Middle, + modifiers: Modifiers::default(), + click_count: 1, + first_mouse: false, + }); + cx.run_until_parked(); + let pan_started = cx.read(|cx| image_view.read(cx).pdf_middle_pan.is_some()); + assert!(pan_started, "middle down should start the pan"); + cx.simulate_event(MouseMoveEvent { + position: drag_from + point(px(200.0), px(0.0)), + pressed_button: Some(MouseButton::Middle), + modifiers: Modifiers::default(), + }); + cx.run_until_parked(); + cx.simulate_event(MouseUpEvent { + position: drag_from + point(px(200.0), px(0.0)), + button: MouseButton::Middle, + modifiers: Modifiers::default(), + click_count: 1, + }); + cx.run_until_parked(); + + let offset_h = cx.read(|cx| image_view.read(cx).pdf_scroll_handle.offset()); + assert!( + offset_h.x < offset_zoomed.x, + "dragging right should scroll the viewport left, got {offset_h:?} vs {offset_zoomed:?}" + ); + assert!( + offset_h.x >= -cx.read(|cx| image_view.read(cx).pdf_scroll_handle.max_offset().x), + "horizontal pan should stay within the scrollable range" + ); } #[gpui::test]