Scroll: a touch zoom keeps zooming after its keys are let go (#6130)

crates/ui-egui/src/canvas.rs

@@ -2141,19 +2141,18 @@ fn single_page_paging(ui: &egui::Ui, view: &mut DocView, cx: &PagingCx<'_>) -> (
         single_paging = can_turn && fits;
         // Every wheel event goes to the pager, so it follows each trackpad touch to its end
         // even while the page can't turn.
-        ui.input(|i| {
-            for e in &i.events {
-                if let egui::Event::MouseWheel { unit, delta, phase, modifiers, .. } = e {
-                    // Zooming (⌘ or Ctrl) and sideways scrolling (Shift) never turn pages.
-                    let dy = if modifiers.command || modifiers.ctrl || modifiers.shift { 0.0 } else { delta.y };
-                    if fits {
-                        view.single_page_wheel(*unit, dy, *phase, i.time, can_turn);
-                    } else {
-                        edge_turned |= view.single_page_wheel_edge(*unit, dy, *phase, i.time, can_turn);
-                    }
+        let now = ui.input(|i| i.time);
+        for e in &crate::motion::gesture_wheel_events(ui.ctx()) {
+            if let egui::Event::MouseWheel { unit, delta, phase, modifiers, .. } = e {
+                // Zooming (⌘ or Ctrl) and sideways scrolling (Shift) never turn pages.
+                let dy = if modifiers.command || modifiers.ctrl || modifiers.shift { 0.0 } else { delta.y };
+                if fits {
+                    view.single_page_wheel(*unit, dy, *phase, now, can_turn);
+                } else {
+                    edge_turned |= view.single_page_wheel_edge(*unit, dy, *phase, now, can_turn);
                 }
             }
-        });
+        }
         if single_paging {
             // Paging owns vertical wheel motion here. A diagonal gesture keeps its sideways
             // part, as `ScrollArea` itself handles each axis.
@@ -2202,10 +2201,10 @@ fn read_wheel(ui: &mut egui::Ui, view: &mut DocView, avail: Rect, single_paging:
         }
         return Wheel { precise_scroll: Vec2::ZERO, line_base: Vec2::ZERO, coarse_scroll: Vec2::ZERO, has_page: false, paging_owns_vertical };
     }
-    let consumed = ui.input(|i| {
-        i.events.iter().any(|event| {
-            matches!(event, egui::Event::MouseWheel { delta, modifiers, .. } if *delta != Vec2::ZERO && delta.is_finite() && !modifiers.command && !modifiers.ctrl)
-        })
+    // A touch gesture zooms to its end once it began with ⌘ or Ctrl, the keys let go or not.
+    let events = crate::motion::gesture_wheel_events(ui.ctx());
+    let consumed = events.iter().any(|event| {
+        matches!(event, egui::Event::MouseWheel { delta, modifiers, .. } if *delta != Vec2::ZERO && delta.is_finite() && !modifiers.command && !modifiers.ctrl)
     });
     if consumed {
         wheel_reset::consumed(ui.ctx());
@@ -2214,7 +2213,7 @@ fn read_wheel(ui: &mut egui::Ui, view: &mut DocView, avail: Rect, single_paging:
         // Match the line distance used by egui's other scroll areas.
         let live = ui.ctx().options(|o| o.input_options.line_scroll_speed);
         let line_points = if live.is_finite() && live > 0.0 { live } else { crate::scroll_speed::EGUI_LINE_POINTS };
-        ui.input(|i| crate::wheel_boost::accumulate(&i.events, line_points, single_paging))
+        crate::wheel_boost::accumulate(&events, line_points, single_paging)
     };
     let mut precise_scroll = input.precise;
     let mut line_base = input.line;
@@ -2870,7 +2869,7 @@ struct GlideCx {
 fn step_glide(ui: &egui::Ui, view: &mut DocView, avail: Rect, cx: &GlideCx) -> Vec2 {
     let GlideCx { now, may_glide, key_scroll, key_side, auto_delta, zoom_delta, edge_delta, wheel_scrolled } = *cx;
     let over_page = ui.rect_contains_pointer(avail);
-    ui.input(|i| view.fling.read(&i.events, now, may_glide && over_page));
+    view.fling.read(&crate::motion::gesture_wheel_events(ui.ctx()), now, may_glide && over_page);
     let interrupted = glide_interrupted(ui);
     if !may_glide || interrupted || key_scroll != 0.0 || key_side != 0.0 || auto_delta != Vec2::ZERO || zoom_delta != 1.0 {
         view.fling.stop();

crates/ui-egui/src/platform/motion.rs

@@ -19,7 +19,7 @@
 //! How far anything scrolls or zooms doesn't change, only how long it takes.
 
 use egui::style::ScrollAnimation;
-use egui::{Event, Modifiers, MouseWheelUnit, TouchPhase, Vec2, vec2};
+use egui::{Event, Modifiers, MouseWheelSource, MouseWheelUnit, TouchPhase, Vec2, vec2};
 
 /// Preferences ▸ Animation effects: whether anything in the window is animated, the wheel's
 /// scrolling and zooming included.
@@ -119,12 +119,6 @@ impl Motion {
         }
         let animate = effects.animates(self.reduced);
         self.sync_style(ctx, animate);
-        if animate {
-            // A gesture begun while easing is egui's to finish.
-            self.wheel = WheelReader::default();
-            self.frame = None;
-            return;
-        }
         if crate::canvas::wheel_reset::reset_this_pass(ctx) {
             // The document's hidden cancel resets egui before fresh input. Its app reader
             // must forget the same older gesture modifiers before reading those events.
@@ -146,11 +140,18 @@ impl Motion {
         let motion = match self.frame {
             Some((read_in, motion)) if read_in == frame => motion,
             _ => {
-                let motion = ctx.input(|i| self.wheel.read(&i.events, i.viewport_rect().height(), &options));
+                let motion = ctx.input(|i| self.wheel.read(&i.events, i.time, i.viewport_rect().height(), &options));
                 self.frame = Some((frame, motion));
+                let modifiers = self.wheel.modifiers.clone();
+                ctx.data_mut(|d| d.insert_temp(gesture_id(), (frame, modifiers)));
                 motion
             }
         };
+        if animate {
+            // egui eases the wheel itself. The reader still follows each touch gesture, as
+            // egui does, for the modifiers [`gesture_wheel_events`] hands the document.
+            return;
+        }
         let (scroll, zoom) = if first_pass { (motion.scroll, motion.zoom) } else { (Vec2::ZERO, 1.0) };
         ctx.input_mut(|i| i.smooth_scroll_delta = scroll);
         let pass = ctx.cumulative_pass_nr();
@@ -202,6 +203,34 @@ pub(crate) fn zoom_delta(ctx: &egui::Context) -> f32 {
     })
 }
 
+/// Return the stable egui id holding the modifiers of this frame's wheel events.
+fn gesture_id() -> egui::Id {
+    egui::Id::unique("pdfcraft-motion-wheel-gesture")
+}
+
+/// This pass's wheel events, each with the modifiers it acts with: a touch gesture keeps
+/// ⌘/Ctrl or Shift from its `Start` until its `End`, as egui and [`WheelReader`] read it. A view
+/// that reads the raw wheel then scrolls none of a gesture that egui or [`zoom_delta`] zooms,
+/// after the keys are let go too. The events keep their own modifiers when
+/// [`Motion::begin_pass`] did not read this frame's wheel.
+pub(crate) fn gesture_wheel_events(ctx: &egui::Context) -> Vec<Event> {
+    let read: Option<(u64, Vec<Modifiers>)> = ctx.data(|d| d.get_temp(gesture_id()));
+    let frame = ctx.cumulative_frame_nr();
+    let mut events: Vec<Event> = ctx.input(|i| i.events.iter().filter(|e| matches!(e, Event::MouseWheel { .. })).cloned().collect());
+    if let Some((at, held)) = read
+        && at == frame
+        // A later pass of the frame has no events left; only the pass that was read matches.
+        && events.len() == held.len()
+    {
+        for (event, held) in events.iter_mut().zip(held) {
+            if let Event::MouseWheel { modifiers, .. } = event {
+                *modifiers = held;
+            }
+        }
+    }
+    events
+}
+
 /// One pass's wheel input.
 #[derive(Clone, Copy, Debug, PartialEq)]
 struct WheelMotion {
@@ -221,20 +250,44 @@ struct WheelReader {
     /// The modifiers of the touch gesture in progress (`TouchPhase::Start` to `End`). They hold
     /// for the whole gesture, as in egui: its momentum arrives after the keys are let go.
     touch: Option<Modifiers>,
+    /// The time of the last wheel event and what drove it. A touch whose `End` never arrives
+    /// (some mouse wheels on Wayland send `Start` alone) ends after egui's timeout.
+    last: Option<(f64, MouseWheelSource)>,
+    /// The modifiers each wheel event of the last [`Self::read`] acted with, in order.
+    modifiers: Vec<Modifiers>,
 }
 
+/// egui's `SCROLL_ACTION_TIMEOUT`: seconds without a wheel event that end a touch.
+const TOUCH_TIMEOUT: f64 = 0.150;
+
+/// egui's `RESTING_FINGER_TIMEOUT`: the same for a touch driven by fingers on a trackpad.
+const RESTING_FINGER_TIMEOUT: f64 = 1.0;
+
 impl WheelReader {
-    /// `page_height` is what one `MouseWheelUnit::Page` scrolls: the window's height in points.
-    fn read(&mut self, events: &[Event], page_height: f32, options: &egui::InputOptions) -> WheelMotion {
+    /// `time` is the pass's input time; `page_height` is what one `MouseWheelUnit::Page`
+    /// scrolls: the window's height in points.
+    fn read(&mut self, events: &[Event], time: f64, page_height: f32, options: &egui::InputOptions) -> WheelMotion {
         let mut delta = Vec2::ZERO;
         let mut zoom = 1.0_f32;
         let mut modifiers = self.touch.unwrap_or_default();
+        self.modifiers.clear();
         for event in events {
+            if let Event::MouseWheel { source, .. } = event {
+                if let Some((at, last_source)) = self.last {
+                    let timeout = if last_source == MouseWheelSource::Trackpad { RESTING_FINGER_TIMEOUT } else { TOUCH_TIMEOUT };
+                    // `>` is false for a NaN time, which then keeps the touch, as egui does.
+                    if time - at > timeout {
+                        self.touch = None;
+                    }
+                }
+                self.last = Some((time, *source));
+            }
             match event {
                 Event::MouseWheel { unit, delta: step, phase, modifiers: latest, .. } => match phase {
                     TouchPhase::Start => {
                         self.touch = Some(*latest);
                         modifiers = *latest;
+                        self.modifiers.push(modifiers);
                     }
                     TouchPhase::Move => {
                         modifiers = match &mut self.touch {
@@ -244,6 +297,7 @@ impl WheelReader {
                             }
                             None => *latest,
                         };
+                        self.modifiers.push(modifiers);
                         let mut step = match unit {
                             MouseWheelUnit::Point => *step,
                             MouseWheelUnit::Line => options.line_scroll_speed * *step,
@@ -264,6 +318,8 @@ impl WheelReader {
                     }
                     // egui drops what the gesture moved earlier in the pass it ends in.
                     TouchPhase::End | TouchPhase::Cancel => {
+                        // The end belongs to the gesture: it carries the gesture's modifiers.
+                        self.modifiers.push(self.touch.unwrap_or(*latest));
                         self.touch = None;
                         modifiers = Modifiers::NONE;
                         delta = Vec2::ZERO;

crates/ui-egui/src/platform/motion/tests.rs

@@ -11,7 +11,7 @@ fn notch(dy: f32, modifiers: Modifiers) -> Event {
 }
 
 fn read(events: &[Event]) -> WheelMotion {
-    WheelReader::default().read(events, 600.0, &egui::InputOptions::default())
+    WheelReader::default().read(events, 0.0, 600.0, &egui::InputOptions::default())
 }
 
 #[test]
@@ -46,14 +46,33 @@ fn a_touch_gesture_keeps_its_modifiers_and_ends_clean() {
     let options = egui::InputOptions::default();
     let point = |dy: f32, phase, modifiers| wheel(MouseWheelUnit::Point, vec2(0.0, dy), phase, modifiers);
     // ⌘ held as the fingers land, let go before the momentum: still zooming.
-    let began = reader.read(&[point(0.0, TouchPhase::Start, Modifiers::COMMAND), point(20.0, TouchPhase::Move, Modifiers::COMMAND)], 600.0, &options);
+    let began =
+        reader.read(&[point(0.0, TouchPhase::Start, Modifiers::COMMAND), point(20.0, TouchPhase::Move, Modifiers::COMMAND)], 0.0, 600.0, &options);
     assert!(began.zoom > 1.0 && began.scroll == Vec2::ZERO, "{began:?}");
-    let momentum = reader.read(&[point(20.0, TouchPhase::Move, Modifiers::NONE)], 600.0, &options);
+    let momentum = reader.read(&[point(20.0, TouchPhase::Move, Modifiers::NONE)], 0.0, 600.0, &options);
     assert!(momentum.zoom > 1.0 && momentum.scroll == Vec2::ZERO, "{momentum:?}");
+    assert_eq!(reader.modifiers, vec![Modifiers::COMMAND], "the momentum acts with the gesture's modifiers");
     // The pass it ends in moves nothing, and the next gesture starts without the modifier.
-    let ended = reader.read(&[point(20.0, TouchPhase::Move, Modifiers::NONE), point(0.0, TouchPhase::End, Modifiers::NONE)], 600.0, &options);
+    let ended = reader.read(&[point(20.0, TouchPhase::Move, Modifiers::NONE), point(0.0, TouchPhase::End, Modifiers::NONE)], 0.0, 600.0, &options);
     assert_eq!(ended, WheelMotion { scroll: Vec2::ZERO, zoom: 1.0 });
-    assert_eq!(reader.read(&[point(-20.0, TouchPhase::Move, Modifiers::NONE)], 600.0, &options).scroll, vec2(0.0, -20.0));
+    assert_eq!(reader.read(&[point(-20.0, TouchPhase::Move, Modifiers::NONE)], 0.0, 600.0, &options).scroll, vec2(0.0, -20.0));
+}
+
+/// A touch whose `End` never arrives (some mouse wheels on Wayland) ends after egui's timeout,
+/// so its modifiers don't latch onto later scrolling.
+#[test]
+fn a_touch_without_an_end_lets_its_modifiers_go_after_a_pause() {
+    let options = egui::InputOptions::default();
+    for (source, pause) in [(egui::MouseWheelSource::Unknown, 0.2), (egui::MouseWheelSource::Trackpad, 1.1)] {
+        let mut reader = WheelReader::default();
+        let point = |dy: f32, phase, modifiers| Event::MouseWheel { unit: MouseWheelUnit::Point, delta: vec2(0.0, dy), phase, modifiers, source };
+        reader.read(&[point(0.0, TouchPhase::Start, Modifiers::COMMAND), point(20.0, TouchPhase::Move, Modifiers::COMMAND)], 1.0, 600.0, &options);
+        let soon = reader.read(&[point(20.0, TouchPhase::Move, Modifiers::NONE)], 1.1, 600.0, &options);
+        assert!(soon.zoom > 1.0 && soon.scroll == Vec2::ZERO, "{source:?}: still the same touch: {soon:?}");
+        let later = reader.read(&[point(-20.0, TouchPhase::Move, Modifiers::NONE)], 1.1 + pause, 600.0, &options);
+        assert_eq!(later, WheelMotion { scroll: vec2(0.0, -20.0), zoom: 1.0 }, "{source:?}");
+        assert_eq!(reader.modifiers, vec![Modifiers::NONE], "{source:?}");
+    }
 }
 
 #[test]
@@ -103,7 +122,7 @@ fn reading_at_once_goes_as_far_as_egui_eases() {
             });
         }
         let page_height = ctx.input(|i| i.viewport_rect().height());
-        let at_once = WheelReader::default().read(&[event], page_height, &options);
+        let at_once = WheelReader::default().read(&[event], 0.0, page_height, &options);
         assert!((scrolled - at_once.scroll).length() < 0.01, "{modifiers:?} {unit:?}: egui scrolled {scrolled:?}, read {:?}", at_once.scroll);
         assert!((zoomed / at_once.zoom - 1.0).abs() < 1e-3, "{modifiers:?} {unit:?}: egui zoomed {zoomed}, read {}", at_once.zoom);
         // What makes the reader necessary: egui's first pass holds part of the step back.

crates/ui-egui/tests/it/view.rs

@@ -1016,6 +1016,51 @@ fn the_wheel_zooms_a_whole_step_at_once_with_animation_effects_off() {
     }
 }
 
+/// A touch gesture begun with ⌘/Ctrl zooms to its end after the keys are let go, as egui reads
+/// it: its later motion neither scrolls the page as well nor turns the rest of it into scrolling.
+#[test]
+fn a_zoom_gesture_zooms_to_its_end_after_its_keys_are_let_go() {
+    /// The view's zoom and page 1 on screen, settled, after a gesture of six 5-point motions,
+    /// the keys let go after the second one or held throughout.
+    fn zoom_gesture(effects: &'static str, let_go: bool) -> (f32, f32, egui::Rect) {
+        let mut h = harness_stepping(1.0 / 60.0, &[("layout", "continuous"), ("zoom", "300")]);
+        h.state_mut().set_option("animation_effects", effects).unwrap();
+        let at = h.state().views[0].viewport_rect().center();
+        h.hover_at(at);
+        h.run_steps(2);
+        let before = h.state().views[0].zoom;
+        let touch = |phase, dy, modifiers| egui::Event::MouseWheel {
+            unit: MouseWheelUnit::Point,
+            delta: egui::vec2(0.0, dy),
+            phase,
+            modifiers,
+            source: egui::MouseWheelSource::Unknown,
+        };
+        h.event(touch(TouchPhase::Start, 0.0, Modifiers::COMMAND));
+        h.step();
+        for k in 0..6 {
+            let modifiers = if let_go && k >= 2 { Modifiers::NONE } else { Modifiers::COMMAND };
+            h.event(touch(TouchPhase::Move, 5.0, modifiers));
+            h.step();
+        }
+        h.event(touch(TouchPhase::End, 0.0, Modifiers::NONE));
+        h.run_steps(40);
+        (before, h.state().views[0].zoom, rect(&h, 0).expect("page 1 on screen"))
+    }
+    for effects in ["off", "on"] {
+        let (before, held, held_page) = zoom_gesture(effects, false);
+        assert!(held > before, "{effects}: the gesture zoomed in: {before} to {held}");
+        let (_, let_go, let_go_page) = zoom_gesture(effects, true);
+        assert!((let_go / held - 1.0).abs() < 1e-4, "{effects}: the whole gesture zooms: {let_go}, held throughout {held}");
+        assert!(
+            (let_go_page.min - held_page.min).length() < 0.5,
+            "{effects}: and nothing scrolls: page 1 at {:?}, held throughout {:?}",
+            let_go_page.min,
+            held_page.min
+        );
+    }
+}
+
 #[test]
 fn page_display_commands_switch_layouts() {
     let mut h = harness(&[]);