diff --git a/iris/src/sense.rs b/iris/src/sense.rs index 32f88f1..3598ada 100644 --- a/iris/src/sense.rs +++ b/iris/src/sense.rs @@ -600,6 +600,25 @@ pub enum DragOutcome { SelectExtend, } +/// What the thing a press landed on looked like at the moment it landed +/// -- the two facts a [`DragArbiter`] cannot see for itself and that +/// decide what the press is allowed to become. One struct rather than two +/// boolean parameters because they are read together, on exactly one call +/// (`press_start`), and a bare `false, false` at a call site says nothing +/// about which is which. +#[derive(Debug, Clone, Copy, Default, PartialEq, Eq)] +pub struct PressState { + /// Whether anything was already selected *before* this press -- it + /// decides whether an early horizontal move extends that selection + /// instead of waiting for a long-press. + pub already_selected: bool, + /// Whether the target was already moving under its own momentum (a + /// `List` with a fling in flight, `List::is_scrolling`). See + /// [`DragArbiter::press_start`]: a press on moving content is a catch, + /// and catches skip the slop entirely. + pub scrolling: bool, +} + #[derive(Clone, Copy, PartialEq)] enum ArbiterState { Idle, @@ -665,15 +684,31 @@ impl DragArbiter { } } - /// A fresh press-down at `pos`. `already_selected` is whatever the - /// caller's selection state was *before* this press -- it decides - /// whether an early horizontal move extends that selection instead of - /// waiting for a long-press. - pub fn press_start(&mut self, pos: Vec2, now: Instant, already_selected: bool) { + /// A fresh press-down at `pos`, with whatever the caller's target + /// looked like at that moment ([`PressState`]). + /// + /// `press.scrolling` short-circuits the whole decision: the press + /// commits to panning on this very sample, with no [`DRAG_SLOP`] and + /// no long-press timer, because a finger put down on content that is + /// already moving means "stop it here" and nothing else. That is + /// Compose's `scrollable`, whose `startDragImmediately` is + /// `ScrollingLogic.shouldScrollImmediately()` -- + /// `scrollableState.isScrollInProgress` -- and whose + /// `DragGestureNode.processInitialDownState` then consumes the DOWN + /// on the `Initial` pass and calls `sendDragStart` + + /// `sendDragEvent(Offset.Zero)` on the `Main` pass of that same + /// event, rather than moving to its await-touch-slop state. + pub fn press_start(&mut self, pos: Vec2, now: Instant, press: PressState) { self.origin = pos; self.origin_at = now; self.last = pos; - self.state = ArbiterState::Undecided { already_selected }; + self.state = if press.scrolling { + ArbiterState::Panning + } else { + ArbiterState::Undecided { + already_selected: press.already_selected, + } + }; } /// Whether this arbiter has no press in flight -- either it has never @@ -849,6 +884,14 @@ pub enum GestureOutcome { pub struct DragGesture { arbiter: DragArbiter, velocity: VelocityTracker, + /// Set when a press began as a *catch* (`PressState::scrolling`) and + /// cleared by the first frame that actually moves the content. While + /// it is set the gesture is a pan that has panned nothing, so its + /// release is `Released(None)`: not a `Tapped`, because Compose's + /// scrollable consumed that DOWN and no click or long-press detector + /// under it ever saw the gesture at all, and not a + /// `Released(Some(v))`, because there is no velocity to hand on. + catch_unmoved: bool, } impl Default for DragGesture { @@ -867,6 +910,7 @@ impl DragGesture { Self { arbiter: DragArbiter::on(axis), velocity: VelocityTracker::new(), + catch_unmoved: false, } } @@ -879,14 +923,42 @@ impl DragGesture { self.arbiter.is_idle() } + /// Whether feeding `sense` to [`Self::handle`] right now would begin + /// a **new press**: any frame that is not a release, arriving while + /// no press is in flight. That covers a `PressStart` and equally the + /// first `Pressing` frame of a gesture whose `PressStart` never + /// arrived (`handle`'s recovery branch below). A caller reads this to + /// prepare the thing being dragged on exactly the frames `handle` + /// will call `DragArbiter::press_start` -- + /// `transcript_ui::Selection::drag` stops its list's fling and + /// reports whether there was one -- rather than keeping its own copy + /// of that rule, which is the one place the two could disagree. + /// + /// **A `PressStart` is not special-cased to `true`**, which it was + /// for one afternoon: one touch-down reaches every sensor under the + /// finger, and a transcript row's block and the tool row containing + /// it both drive this same shared `DragGesture`, so `handle` sees one + /// `PressStart` twice. Restarting on the second delivery re-reads the + /// caller's [`PressState`] *after* the first delivery has already + /// acted on it -- the list's fling is cancelled by then, so + /// `scrolling` comes back false and a catch silently becomes an + /// ordinary slop-waiting press. The second delivery is a continuation + /// of a press already in flight, and this says so. + pub fn starts_press(&self, sense: CursorSense) -> bool { + match sense { + CursorSense::Drop | CursorSense::PressEnd(_) => false, + _ => self.arbiter.is_idle(), + } + } + /// Feed one frame of a gesture through. `id` is the widget iris should /// give exclusive pointer input to once this gesture commits to /// panning or selecting -- a stable widget that outlives the gesture /// (a `List`'s own id, not one of its virtualised rows, which can be /// retired mid-drag as content scrolls). `render` is `CursorData`'s - /// own field, already in hand at every call site. `already_selected` - /// only matters for the first frame of a gesture (`PressStart`, or the - /// recovery branch below) -- see `DragArbiter::press_start`'s doc. + /// own field, already in hand at every call site. `press` only matters + /// on the frames [`Self::starts_press`] answers true for -- see + /// `DragArbiter::press_start`'s doc. pub fn handle( &mut self, render: &UiRenderState, @@ -894,35 +966,25 @@ impl DragGesture { sense: CursorSense, pos_window: Vec2, now: Instant, - already_selected: bool, + press: PressState, ) -> GestureOutcome { match sense { - CursorSense::PressStart(_) => { - self.velocity.reset(); - // Where the finger went down is a sample, exactly as - // Compose's `DragGestureNode.sendDragStart` feeds the DOWN - // change to its tracker before any move. It is one of the - // three a fit needs, and it is the one that fixes the - // origin of the curve; without it a 120Hz flick delivering - // its whole motion in two frames has too few points and - // does not fling at all. - self.velocity - .add_position(pos_window.axis(self.arbiter.axis()), now); - self.arbiter.press_start(pos_window, now, already_selected); - if crate::diagnostics::trace_enabled() { - log::debug!( - target: "iris::input", - "iris gesture: press start pos=({:.1},{:.1})", - pos_window.x, pos_window.y, - ); - } - self.dispatch(render, id, pos_window, now) - } CursorSense::Drop | CursorSense::PressEnd(_) => { // Once: a `velocity()` is a full Lsq2 fit, and the log // line below wants the same number the outcome carries. let released = self.velocity.velocity(); - let outcome = if self.arbiter.is_panning() { + // `catch_unmoved` is only ever set beside `Panning` (a + // catch enters it on the down) and only ever left set + // while nothing has moved, so `Panning` is the one state + // it can be observed in. If that stops holding, the + // branch below is silently swallowing a tap. + debug_assert!( + !self.catch_unmoved || self.arbiter.is_panning(), + "a caught press that never moved must still be panning at release", + ); + let outcome = if self.catch_unmoved { + GestureOutcome::Released(None) + } else if self.arbiter.is_panning() { GestureOutcome::Released(Some(released)) } else if self.arbiter.is_undecided() { GestureOutcome::Tapped @@ -959,22 +1021,39 @@ impl DragGesture { ); } self.arbiter.release(); + self.catch_unmoved = false; render.release_pointer(); outcome } - // See `DragArbiter::update`'s own doc: a `Pressing` frame can - // arrive with no matching `PressStart` if the touch-down - // landed outside whichever hit region first noticed it. - _ if self.arbiter.is_idle() => { + // A `Pressing` frame can arrive with no matching `PressStart` + // if the touch-down landed outside whichever hit region first + // noticed it -- `DragArbiter::update`'s own doc. Both that + // recovery and an ordinary `PressStart` open a press the same + // way, so they are one branch: `starts_press` is the rule, and + // it is the same one the caller reads. + _ if self.starts_press(sense) => { self.velocity.reset(); + // Where the finger went down is a sample, exactly as + // Compose's `DragGestureNode.sendDragStart` feeds the DOWN + // change to its tracker before any move. It is one of the + // three a fit needs, and it is the one that fixes the + // origin of the curve; without it a 120Hz flick delivering + // its whole motion in two frames has too few points and + // does not fling at all. self.velocity .add_position(pos_window.axis(self.arbiter.axis()), now); - self.arbiter.press_start(pos_window, now, already_selected); + self.arbiter.press_start(pos_window, now, press); + self.catch_unmoved = press.scrolling; if crate::diagnostics::trace_enabled() { + let how = if matches!(sense, CursorSense::PressStart(_)) { + "" + } else { + " (recovered, no PressStart seen)" + }; log::debug!( target: "iris::input", - "iris gesture: press start (recovered, no PressStart seen) pos=({:.1},{:.1})", - pos_window.x, pos_window.y, + "iris gesture: press start{how} pos=({:.1},{:.1}) scrolling={}", + pos_window.x, pos_window.y, press.scrolling, ); } self.dispatch(render, id, pos_window, now) @@ -994,6 +1073,11 @@ impl DragGesture { DragOutcome::Undecided => GestureOutcome::Undecided, DragOutcome::Pan(dy) => { render.capture_pointer(id); + if dy != 0.0 { + // The catch has moved something, so its release is an + // ordinary pan release again -- see `catch_unmoved`. + self.catch_unmoved = false; + } // The raw position, not `dy`: `dy` has the touch slop // subtracted out of the frame that crossed it, and the // tracker fits a curve through where the finger *was*. @@ -1922,14 +2006,14 @@ mod drag_arbiter_tests { #[test] fn small_jitter_stays_undecided() { let mut a = DragArbiter::new(); - a.press_start(Vec2::new(0.0, 0.0), t(0), false); + a.press_start(Vec2::new(0.0, 0.0), t(0), PressState::default()); assert_eq!(a.update(Vec2::new(1.0, 1.0), t(10)), DragOutcome::Undecided); } #[test] fn a_vertical_drag_pans_immediately() { let mut a = DragArbiter::new(); - a.press_start(Vec2::new(0.0, 0.0), t(0), false); + a.press_start(Vec2::new(0.0, 0.0), t(0), PressState::default()); // The transition frame applies only the motion past `DRAG_SLOP` // (20 - 8 = 12), not the full 20px since `press_start` -- see the // `Pan` arm's own comment for why replaying the whole withheld @@ -1954,7 +2038,7 @@ mod drag_arbiter_tests { #[test] fn crossing_the_slop_by_a_little_pans_by_a_little() { let mut a = DragArbiter::new(); - a.press_start(Vec2::new(0.0, 0.0), t(0), false); + a.press_start(Vec2::new(0.0, 0.0), t(0), PressState::default()); assert_eq!( a.update(Vec2::new(0.0, DRAG_SLOP + 0.5), t(10)), DragOutcome::Pan(0.5) @@ -1964,7 +2048,7 @@ mod drag_arbiter_tests { #[test] fn a_horizontal_drag_with_nothing_selected_does_not_select() { let mut a = DragArbiter::new(); - a.press_start(Vec2::new(0.0, 0.0), t(0), false); + a.press_start(Vec2::new(0.0, 0.0), t(0), PressState::default()); // Horizontal movement alone, with no prior selection, is not any // of the three named gestures -- it stays undecided rather than // guessing (it will resolve to a long-press-selection if the @@ -1978,7 +2062,7 @@ mod drag_arbiter_tests { #[test] fn a_long_press_without_moving_starts_a_selection() { let mut a = DragArbiter::new(); - a.press_start(Vec2::new(5.0, 5.0), t(0), false); + a.press_start(Vec2::new(5.0, 5.0), t(0), PressState::default()); assert_eq!(a.update(Vec2::new(5.0, 5.0), t(10)), DragOutcome::Undecided); assert_eq!( a.update(Vec2::new(6.0, 5.0), t(LONG_PRESS.as_millis() as u64 + 1)), @@ -1989,7 +2073,7 @@ mod drag_arbiter_tests { #[test] fn after_a_long_press_any_further_drag_extends() { let mut a = DragArbiter::new(); - a.press_start(Vec2::new(0.0, 0.0), t(0), false); + a.press_start(Vec2::new(0.0, 0.0), t(0), PressState::default()); assert_eq!( a.update(Vec2::new(0.0, 0.0), t(LONG_PRESS.as_millis() as u64 + 1)), DragOutcome::SelectStart @@ -2006,7 +2090,14 @@ mod drag_arbiter_tests { #[test] fn a_horizontal_drag_on_already_selected_text_extends_immediately() { let mut a = DragArbiter::new(); - a.press_start(Vec2::new(0.0, 0.0), t(0), true); + a.press_start( + Vec2::new(0.0, 0.0), + t(0), + PressState { + already_selected: true, + ..Default::default() + }, + ); assert_eq!( a.update(Vec2::new(20.0, 2.0), t(10)), DragOutcome::SelectExtend @@ -2016,7 +2107,14 @@ mod drag_arbiter_tests { #[test] fn a_vertical_drag_still_pans_even_with_a_prior_selection() { let mut a = DragArbiter::new(); - a.press_start(Vec2::new(0.0, 0.0), t(0), true); + a.press_start( + Vec2::new(0.0, 0.0), + t(0), + PressState { + already_selected: true, + ..Default::default() + }, + ); assert_eq!( a.update(Vec2::new(0.0, 20.0), t(10)), DragOutcome::Pan(12.0) @@ -2059,7 +2157,7 @@ mod drag_arbiter_tests { // Simulates the real call site: a `Pressing` frame arrives with no // matching `PressStart` ever having reached this arbiter. assert!(a.is_idle()); - a.press_start(Vec2::new(0.0, 700.0), t(0), false); + a.press_start(Vec2::new(0.0, 700.0), t(0), PressState::default()); assert_eq!( a.update(Vec2::new(0.0, 720.0), t(10)), DragOutcome::Pan(12.0) @@ -2074,7 +2172,7 @@ mod drag_arbiter_tests { #[test] fn a_press_that_never_moved_is_still_undecided_at_release() { let mut a = DragArbiter::new(); - a.press_start(Vec2::new(0.0, 0.0), t(0), false); + a.press_start(Vec2::new(0.0, 0.0), t(0), PressState::default()); a.update(Vec2::new(1.0, 1.0), t(10)); assert!(a.is_undecided()); assert!(!a.is_panning()); @@ -2086,7 +2184,7 @@ mod drag_arbiter_tests { #[test] fn a_press_that_panned_is_not_undecided_at_release() { let mut a = DragArbiter::new(); - a.press_start(Vec2::new(0.0, 0.0), t(0), false); + a.press_start(Vec2::new(0.0, 0.0), t(0), PressState::default()); a.update(Vec2::new(0.0, 40.0), t(10)); assert!(a.is_panning()); assert!(!a.is_undecided()); @@ -2096,7 +2194,7 @@ mod drag_arbiter_tests { #[test] fn a_long_press_that_selected_is_not_undecided() { let mut a = DragArbiter::new(); - a.press_start(Vec2::new(0.0, 0.0), t(0), false); + a.press_start(Vec2::new(0.0, 0.0), t(0), PressState::default()); assert_eq!( a.update(Vec2::new(0.0, 1.0), t(LONG_PRESS.as_millis() as u64 + 10)), DragOutcome::SelectStart @@ -2111,14 +2209,14 @@ mod drag_arbiter_tests { #[test] fn a_horizontal_arbiter_pans_on_the_drag_a_vertical_one_ignores() { let mut across = DragArbiter::on(Axis::X); - across.press_start(Vec2::new(0.0, 0.0), t(0), false); + across.press_start(Vec2::new(0.0, 0.0), t(0), PressState::default()); assert_eq!( across.update(Vec2::new(20.0, 0.0), t(10)), DragOutcome::Pan(12.0) ); let mut down = DragArbiter::new(); - down.press_start(Vec2::new(0.0, 0.0), t(0), false); + down.press_start(Vec2::new(0.0, 0.0), t(0), PressState::default()); assert_eq!( down.update(Vec2::new(20.0, 0.0), t(10)), DragOutcome::Undecided @@ -2128,7 +2226,7 @@ mod drag_arbiter_tests { // which is what lets the list behind a code fence still be // panned by a finger that started on the fence. let mut across = DragArbiter::on(Axis::X); - across.press_start(Vec2::new(0.0, 0.0), t(0), false); + across.press_start(Vec2::new(0.0, 0.0), t(0), PressState::default()); assert_eq!( across.update(Vec2::new(0.0, 20.0), t(10)), DragOutcome::Undecided @@ -2138,7 +2236,7 @@ mod drag_arbiter_tests { #[test] fn release_resets_to_idle() { let mut a = DragArbiter::new(); - a.press_start(Vec2::new(0.0, 0.0), t(0), false); + a.press_start(Vec2::new(0.0, 0.0), t(0), PressState::default()); a.update(Vec2::new(0.0, 20.0), t(10)); a.release(); assert_eq!( @@ -2197,7 +2295,7 @@ mod drag_gesture_tests { CursorSense::PressStart(CursorButton::Left), Vec2::ZERO, t(0), - false, + PressState::default(), ); g.handle( &r, @@ -2205,7 +2303,7 @@ mod drag_gesture_tests { CursorSense::Pressing(CursorButton::Left), Vec2::new(0.0, 100.0), t(8), - false, + PressState::default(), ); g.handle( &r, @@ -2213,7 +2311,7 @@ mod drag_gesture_tests { CursorSense::Pressing(CursorButton::Left), Vec2::new(0.0, 220.0), t(16), - false, + PressState::default(), ); let out = g.handle( &r, @@ -2221,7 +2319,7 @@ mod drag_gesture_tests { CursorSense::PressEnd(CursorButton::Left), Vec2::new(0.0, 220.0), t(24), - false, + PressState::default(), ); // (0, 0) (8, 100) (16, 220) through Compose's Lsq2 fit. Note the @@ -2255,7 +2353,7 @@ mod drag_gesture_tests { CursorSense::PressStart(CursorButton::Left), Vec2::ZERO, t(0), - false, + PressState::default(), ); g.handle( &r, @@ -2263,7 +2361,7 @@ mod drag_gesture_tests { CursorSense::Pressing(CursorButton::Left), Vec2::new(0.0, 100.0), t(8), - false, + PressState::default(), ); let out = g.handle( &r, @@ -2271,7 +2369,7 @@ mod drag_gesture_tests { CursorSense::PressEnd(CursorButton::Left), Vec2::new(0.0, 100.0), t(16), - false, + PressState::default(), ); assert_eq!(out, GestureOutcome::Released(Some(0.0))); } @@ -2292,7 +2390,7 @@ mod drag_gesture_tests { CursorSense::PressStart(CursorButton::Left), Vec2::ZERO, t(0), - false, + PressState::default(), ); let out = g.handle( &r, @@ -2300,7 +2398,7 @@ mod drag_gesture_tests { CursorSense::PressEnd(CursorButton::Left), Vec2::ZERO, t(20), - false, + PressState::default(), ); assert_eq!(out, GestureOutcome::Tapped); } @@ -2322,7 +2420,7 @@ mod drag_gesture_tests { CursorSense::PressStart(CursorButton::Left), Vec2::ZERO, t(0), - false, + PressState::default(), ); // Held still past LONG_PRESS, which is what starts a selection. g.handle( @@ -2331,7 +2429,7 @@ mod drag_gesture_tests { CursorSense::Pressing(CursorButton::Left), Vec2::ZERO, t(0) + LONG_PRESS, - false, + PressState::default(), ); let out = g.handle( &r, @@ -2339,8 +2437,219 @@ mod drag_gesture_tests { CursorSense::PressEnd(CursorButton::Left), Vec2::new(0.0, 50.0), t(0) + LONG_PRESS + Duration::from_millis(10), - false, + PressState::default(), ); assert_eq!(out, GestureOutcome::Released(None)); } + + /// A catch: the press lands on content that is already moving, so it + /// pans from its very first sample with no `DRAG_SLOP` withheld -- + /// docs/IRIS_TODO.md's "it fails to stop & snap to where finger is". + #[test] + fn a_press_on_moving_content_pans_from_the_first_sample() { + let mut ui = UiData::default(); + let id = some_id(&mut ui); + let r = render(); + let mut g = DragGesture::new(); + let caught = PressState { + scrolling: true, + ..Default::default() + }; + + assert_eq!( + g.handle( + &r, + id, + CursorSense::PressStart(CursorButton::Left), + Vec2::ZERO, + t(0), + caught, + ), + // The down itself moves nothing; it only stops the fling. + GestureOutcome::Pan(0.0), + ); + // A move of 2px, a quarter of `DRAG_SLOP` -- an ordinary press + // would still be `Undecided` here. + assert_eq!( + g.handle( + &r, + id, + CursorSense::Pressing(CursorButton::Left), + Vec2::new(0.0, 2.0), + t(8), + caught, + ), + GestureOutcome::Pan(2.0), + ); + } + + /// The sibling of the case above, and the pair that says the catch is + /// not simply "every press pans": the same two samples with nothing + /// moving underneath stay inside the slop and decide nothing. + #[test] + fn the_same_press_on_settled_content_stays_undecided() { + let mut ui = UiData::default(); + let id = some_id(&mut ui); + let r = render(); + let mut g = DragGesture::new(); + + g.handle( + &r, + id, + CursorSense::PressStart(CursorButton::Left), + Vec2::ZERO, + t(0), + PressState::default(), + ); + assert_eq!( + g.handle( + &r, + id, + CursorSense::Pressing(CursorButton::Left), + Vec2::new(0.0, 2.0), + t(8), + PressState::default(), + ), + GestureOutcome::Undecided, + ); + } + + /// A catch released without moving is `Released(None)`: not a + /// `Tapped`, because Compose's scrollable consumed that DOWN and no + /// click detector under it ever saw the gesture -- so stopping a + /// fling with a finger must not also follow the link it landed on -- + /// and not a `Released(Some(v))`, because there is no velocity to + /// hand on. + #[test] + fn a_catch_released_without_moving_is_neither_a_tap_nor_a_fling() { + let mut ui = UiData::default(); + let id = some_id(&mut ui); + let r = render(); + let mut g = DragGesture::new(); + let caught = PressState { + scrolling: true, + ..Default::default() + }; + + g.handle( + &r, + id, + CursorSense::PressStart(CursorButton::Left), + Vec2::ZERO, + t(0), + caught, + ); + let out = g.handle( + &r, + id, + CursorSense::PressEnd(CursorButton::Left), + Vec2::ZERO, + t(20), + caught, + ); + assert_eq!(out, GestureOutcome::Released(None)); + // The identical gesture with nothing moving underneath is the + // tap it looks like -- `a_tap_is_still_a_tap_and_flings_nothing` + // above. The two differ by `scrolling` and nothing else. + } + + /// Once a catch has actually moved the content, its release is an + /// ordinary pan release again and hands on a velocity -- otherwise + /// "catch it, then keep flicking" would stop dead every time. + #[test] + fn a_catch_that_then_drags_still_flings() { + let mut ui = UiData::default(); + let id = some_id(&mut ui); + let r = render(); + let mut g = DragGesture::new(); + let caught = PressState { + scrolling: true, + ..Default::default() + }; + + g.handle( + &r, + id, + CursorSense::PressStart(CursorButton::Left), + Vec2::ZERO, + t(0), + caught, + ); + for (i, y) in [100.0, 220.0].into_iter().enumerate() { + g.handle( + &r, + id, + CursorSense::Pressing(CursorButton::Left), + Vec2::new(0.0, y), + t(8 * (i as u64 + 1)), + caught, + ); + } + let out = g.handle( + &r, + id, + CursorSense::PressEnd(CursorButton::Left), + Vec2::new(0.0, 220.0), + t(24), + caught, + ); + assert!( + matches!(out, GestureOutcome::Released(Some(v)) if v.abs() > 1.0), + "a catch that dragged must release with a velocity, got {out:?}" + ); + } + + /// One touch-down reaches every sensor under the finger, and a + /// transcript row's block and the tool row containing it share one + /// `DragGesture` -- so `handle` sees the same `PressStart` twice, and + /// the second delivery carries a `PressState` the first has already + /// acted on (`Selection::drag` has cancelled the fling by then, so + /// `scrolling` is false). It must be a continuation, not a restart; + /// as a restart it silently turned every catch back into an ordinary + /// slop-waiting press, which is how this was found. + #[test] + fn a_second_delivery_of_one_press_start_does_not_restart_it() { + let mut ui = UiData::default(); + let id = some_id(&mut ui); + let r = render(); + let mut g = DragGesture::new(); + + let caught = PressState { + scrolling: true, + ..Default::default() + }; + assert!(g.starts_press(CursorSense::PressStart(CursorButton::Left))); + g.handle( + &r, + id, + CursorSense::PressStart(CursorButton::Left), + Vec2::ZERO, + t(0), + caught, + ); + assert!( + !g.starts_press(CursorSense::PressStart(CursorButton::Left)), + "the second sensor must be told this press is already in flight" + ); + g.handle( + &r, + id, + CursorSense::PressStart(CursorButton::Left), + Vec2::ZERO, + t(0), + PressState::default(), + ); + assert_eq!( + g.handle( + &r, + id, + CursorSense::Pressing(CursorButton::Left), + Vec2::new(0.0, 2.0), + t(8), + PressState::default(), + ), + GestureOutcome::Pan(2.0), + "the catch survived only if the second delivery left it panning" + ); + } } diff --git a/iris/src/widget/position/scroll.rs b/iris/src/widget/position/scroll.rs index 725efd3..a77658e 100644 --- a/iris/src/widget/position/scroll.rs +++ b/iris/src/widget/position/scroll.rs @@ -1,5 +1,5 @@ use crate::prelude::*; -use crate::sense::{DragGesture, GestureOutcome}; +use crate::sense::{DragGesture, GestureOutcome, PressState}; use std::time::Instant; pub struct Scroll { @@ -119,14 +119,16 @@ impl Scroll { pos_window: Vec2, now: Instant, ) { - // `already_selected: false` -- a scroll area has no selection of - // its own to extend, so a horizontal drag stays `Undecided` and a + // A default `PressState`: a scroll area has no selection of its + // own to extend, so a horizontal drag stays `Undecided` and a // vertical one past the slop pans, which is the whole contract - // here. A caller that *does* own a selection (the transcript's - // `Selection`) drives `DragGesture` itself instead. + // here; and it never flings (see this method's doc), so there is + // never a moving target to catch either. A caller that *does* own + // a selection, or a fling (the transcript's `Selection`), drives + // `DragGesture` itself instead. match self .gesture - .handle(render, id, sense, pos_window, now, false) + .handle(render, id, sense, pos_window, now, PressState::default()) { // `scroll(dy)`, not `scroll(-dy)` -- `Selection::drag` passes // `-dy` to `List::scroll` because a `List`'s anchor offset and diff --git a/iris/transcript-fixture/tests/catch_a_fling.rs b/iris/transcript-fixture/tests/catch_a_fling.rs new file mode 100644 index 0000000..54848c1 --- /dev/null +++ b/iris/transcript-fixture/tests/catch_a_fling.rs @@ -0,0 +1,211 @@ +//! Layer 1 of docs/RUST.md's "Three test layers", for catching a fling: +//! the real transcript screen over the real bench fixture, at the phone's +//! size and density, with no window, no compositor and no GPU. +//! +//! docs/IRIS_TODO.md's 2026-09-07 night report -- "sometimes when I try to +//! catch it while it's still moving (particularly if I drag) then it fails +//! to stop & snap to where finger is". The finger goes down on content +//! that is still travelling and the content does not follow it until +//! `DRAG_SLOP` has been crossed, which at a fling's speed is several +//! frames of the content sliding *away* from a finger that is already +//! down. Compose does not do that: a down while `isScrollInProgress` +//! starts the drag immediately (`scrollable`'s `startDragImmediately`). + +use iris::harness::{Harness, TouchAction, TouchScript}; +use iris::prelude::*; +use iris::sense::DRAG_SLOP; +use transcript_fixture::{PHONE_FRAME_MS, PHONE_SCALE, phone_size}; + +/// The screen open on the fixture, framed twice -- once to draw, once for +/// `List::repair_anchor` to resolve the opening `snap_end` into a real +/// anchor, which is what every assertion about scroll position reads. +fn opened() -> (Harness, transcript_ui::TranscriptScreen) { + let mut h = Harness::new(phone_size(), PHONE_SCALE); + let opened = transcript_fixture::open(&mut h.rsc, &mut h.state).expect("the fixture folds"); + h.frame(0); + h.frame(PHONE_FRAME_MS); + (h, opened.screen) +} + +/// Where the content is, in window pixels: the top of whichever row is +/// under the middle of the viewport. `List` has no travel accessor and +/// this needs none -- a row's own extent moves exactly as far as the +/// content does, and the row is picked once so the two readings compare. +fn tracked_row(h: &mut Harness, screen: &transcript_ui::TranscriptScreen) -> (RowKey, f32) { + let middle = phone_size().y / 2.0; + let list = (screen.list)(&mut h.rsc); + let key = list.key_at(middle).expect("a row under the viewport"); + let (top, _) = list.extent(key).expect("that row has an extent"); + (key, top) +} + +fn row_top(h: &mut Harness, screen: &transcript_ui::TranscriptScreen, key: RowKey) -> f32 { + (screen.list)(&mut h.rsc) + .extent(key) + .expect("the tracked row is still loaded") + .0 +} + +/// Three finger samples 8ms apart, each moving `STEP` further down the +/// screen. `STEP * 3` is deliberately **under** `DRAG_SLOP`: a gesture +/// this small moves nothing at all on a settled list (the control below), +/// so anything it moves here is the catch and not the slop being crossed. +const STEP: f32 = 2.0; +const SAMPLES: usize = 3; +const CATCH_X: f32 = 540.0; + +/// Feeds the down and its `SAMPLES` moves from `y0` at `t0`, asserting +/// after each one that the content moved by exactly the finger's own +/// delta. Returns the release time. +fn drag_from( + h: &mut Harness, + screen: &transcript_ui::TranscriptScreen, + key: RowKey, + y0: f32, + t0: u64, + expect_tracking: bool, +) -> u64 { + let before = row_top(h, screen, key); + h.touch(TouchAction::Down, Vec2::new(CATCH_X, y0), t0); + assert_eq!( + row_top(h, screen, key), + before, + "the down itself must not move the content, only stop it" + ); + let mut t = t0; + for i in 1..=SAMPLES { + let moved = STEP * i as f32; + t = t0 + 8 * i as u64; + h.touch(TouchAction::Move, Vec2::new(CATCH_X, y0 + moved), t); + let travelled = row_top(h, screen, key) - before; + if expect_tracking { + assert!( + (travelled - moved).abs() < 0.5, + "sample {i}: the finger has moved {moved}px since the down and the content \ + {travelled:.1}px -- it is not pinned to the finger" + ); + } else { + assert!( + travelled.abs() < 0.5, + "sample {i}: a {moved}px drag is inside DRAG_SLOP ({DRAG_SLOP}px) and must move \ + nothing, but the content moved {travelled:.1}px" + ); + } + } + t += 8; + h.touch( + TouchAction::Up, + Vec2::new(CATCH_X, y0 + STEP * SAMPLES as f32), + t, + ); + t +} + +/// The report itself: flick, let the fling run for 150ms, then put a +/// finger down and drag it a little. From the down onwards the content is +/// pinned to the finger, sample for sample -- no slop, and no coasting +/// past the place the finger stopped it. +#[test] +fn a_press_on_a_flinging_list_pins_the_content_to_the_finger() { + let (mut h, screen) = opened(); + + let flick = TouchScript::parse(include_str!("../touch/flick-120hz.touch")) + .unwrap_or_else(|e| panic!("flick-120hz.touch: {e}")); + h.replay(&flick); + + // Frames, not a file: the second half of this gesture has to arrive + // *while* the fling is ticking, and a `.touch` replay inserts no + // frames between its samples, so a fling recorded that way would be + // running on paper and stationary in fact. + let catch_at = flick.end_ms() + 150; + h.frames_until( + flick.end_ms() + PHONE_FRAME_MS, + catch_at - PHONE_FRAME_MS, + PHONE_FRAME_MS, + ); + assert!( + (screen.list)(&mut h.rsc).is_scrolling(), + "the fling must still be running 150ms in, or this test catches nothing" + ); + + let (key, _) = tracked_row(&mut h, &screen); + drag_from(&mut h, &screen, key, 1200.0, catch_at, true); +} + +/// The other half of the same rule: a catch that never moved at all is a +/// `Released(None)`, not a tap. Compose's scrollable consumes that DOWN, +/// so no click detector under it ever sees the gesture -- stopping a +/// fling with a finger must not also follow the link it landed on, and +/// must not hand the list a velocity to start again with. +#[test] +fn a_catch_that_never_moved_is_not_a_tap_and_does_not_fling() { + let (mut h, screen) = opened(); + + let flick = TouchScript::parse(include_str!("../touch/flick-120hz.touch")) + .unwrap_or_else(|e| panic!("flick-120hz.touch: {e}")); + h.replay(&flick); + let catch_at = flick.end_ms() + 150; + h.frames_until( + flick.end_ms() + PHONE_FRAME_MS, + catch_at - PHONE_FRAME_MS, + PHONE_FRAME_MS, + ); + assert!( + (screen.list)(&mut h.rsc).is_scrolling(), + "the fling must still be running 150ms in, or this test catches nothing" + ); + + let (key, _) = tracked_row(&mut h, &screen); + h.touch(TouchAction::Down, Vec2::new(CATCH_X, 1200.0), catch_at); + let stopped_at = row_top(&mut h, &screen, key); + h.touch(TouchAction::Up, Vec2::new(CATCH_X, 1200.0), catch_at + 8); + + assert_eq!( + (screen.list)(&mut h.rsc).fling_velocity(), + None, + "a press that stopped a fling and moved nothing must not start another" + ); + h.frames_until(catch_at + 16, catch_at + 500, PHONE_FRAME_MS); + assert!( + (row_top(&mut h, &screen, key) - stopped_at).abs() < 0.5, + "the content moved after a catch was released without moving" + ); + assert_eq!( + h.state.opened_urls, + Vec::::new(), + "a catch is not a tap: nothing under it may be followed" + ); +} + +/// The half this change had no reason to touch: on a list that is *not* +/// moving, the same tiny drag is still inside `DRAG_SLOP` and still moves +/// nothing. Without this, making every press pin the content would pass +/// the test above and take the slop away from every ordinary press. +#[test] +fn the_same_small_drag_on_a_settled_list_moves_nothing() { + let (mut h, screen) = opened(); + + let flick = TouchScript::parse(include_str!("../touch/flick-120hz.touch")) + .unwrap_or_else(|e| panic!("flick-120hz.touch: {e}")); + h.replay(&flick); + // Long past the spline's own 2071ms for this recording. + let settled = h.frames_until( + flick.end_ms() + PHONE_FRAME_MS, + flick.end_ms() + 4000, + PHONE_FRAME_MS, + ); + assert!( + !(screen.list)(&mut h.rsc).is_scrolling(), + "the fling must have stopped, or this is the same case as the test above" + ); + + let (key, _) = tracked_row(&mut h, &screen); + drag_from( + &mut h, + &screen, + key, + 1200.0, + settled + PHONE_FRAME_MS, + false, + ); +} diff --git a/iris/transcript-ui/src/selection.rs b/iris/transcript-ui/src/selection.rs index 5f35f09..f0d16e8 100644 --- a/iris/transcript-ui/src/selection.rs +++ b/iris/transcript-ui/src/selection.rs @@ -259,16 +259,26 @@ impl Selection { now: Instant, render: &UiRenderState, ) -> GestureOutcome { - if matches!(sense, CursorSense::PressStart(_)) { - // A fresh touch-down cancels any fling still coasting from - // the previous gesture -- `List::fling`'s own doc, and - // Android's `Scroller::abortAnimation` for the same reason. + // A fresh touch-down cancels any fling still coasting from the + // previous gesture -- `List::fling`'s own doc, and Android's + // `Scroller::abortAnimation` for the same reason -- and, since + // 2026-09-07, *tells the gesture there was one*. A press that + // caught moving content is a catch: it pans from this very sample + // rather than waiting out `DRAG_SLOP`, which is what pins the + // content to the finger instead of leaving it coasting for the + // first few frames (Iris's "it fails to stop & snap to where + // finger is"). See `DragArbiter::press_start` for Compose's own + // mechanism. `starts_press` rather than a `PressStart` test of our + // own, so this fires on the recovered-press frames too. + let mut press = PressState::default(); + if self.gesture.starts_press(sense) { + press.scrolling = list(ui).is_scrolling(); list(ui).cancel_fling(); } - let already_selected = self.has_selection(ui); - let outcome = - self.gesture - .handle(render, list.id(), sense, pos_window, now, already_selected); + press.already_selected = self.has_selection(ui); + let outcome = self + .gesture + .handle(render, list.id(), sense, pos_window, now, press); match outcome { GestureOutcome::Undecided => {} GestureOutcome::Pan(dy) => list(ui).scroll(-dy),