From b863f9f3df41037cc7a94671f811bebde399bc34 Mon Sep 17 00:00:00 2001 From: iris <2+iris@noreply.localhost> Date: Tue, 8 Sep 2026 13:44:51 -0400 Subject: [PATCH] iris: a capture cancels every other gesture, and the pointer leaves UiRenderState Two defects Iris reported from her phone on 2026-09-08, one root cause each, both in how a gesture ends. A widget that takes pointer capture cuts every other widget off from the press completely -- no PressEnd, no Drop -- so anything else tracking it was left with an open gesture at a stale origin, and the *next* touch anywhere was measured from that origin. That is the transcript jumping on a tap after a code fence was panned sideways. CursorSense::Cancel is the missing state: delivered once to each loser of a capture race, the way Android sends ACTION_CANCEL and the web sends pointercancel. And registered click_or_drag|unclick, which never matches a Drop, so a Scroll that had captured never saw its own gesture end and stayed panning from where the finger left. That is the horizontal snap back. CursorSense::drag_senses() states the rule once for every widget driving a DragGesture instead of per call site. The pointer's own state (who holds capture, who is tracking the press) no longer lives in a Mutex on UiRenderState. It is Event::Global for the cursor senses -- owned by the event manager that runs the dispatch, reached by &mut, with a per-dispatch PointerRequests slot for handlers -- per Iris: never reach for locks first, and input-wide state belongs to the general input handler. What had forced the lock was a Data: Send bound on task_on that nothing needed; the spawned future never sees the event's data. Co-Authored-By: Claude Opus 5 --- iris/core/src/event/manager.rs | 3 + iris/core/src/event/mod.rs | 14 ++ iris/core/src/ui/render_state.rs | 55 +---- iris/src/attr.rs | 14 +- iris/src/event.rs | 12 +- iris/src/sense.rs | 344 ++++++++++++++++++++++++---- iris/src/sense_tests.rs | 184 ++++++++++++++- iris/src/widget/position/scroll.rs | 17 +- iris/src/widget/trait_fns.rs | 15 +- iris/transcript-ui/src/lib.rs | 4 +- iris/transcript-ui/src/row.rs | 2 +- iris/transcript-ui/src/selection.rs | 17 +- iris/transcript-ui/src/tool.rs | 2 +- 13 files changed, 554 insertions(+), 129 deletions(-) diff --git a/iris/core/src/event/manager.rs b/iris/core/src/event/manager.rs index 9f04a32..94a5776 100644 --- a/iris/core/src/event/manager.rs +++ b/iris/core/src/event/manager.rs @@ -79,6 +79,8 @@ type EventData = (E, Rc EventFn::Data<'a>> pub struct TypeEventManager { // TODO: reduce visiblity!! pub active: HashMap>, + /// This event's own input-wide state -- see [`Event::Global`]. + pub global: E::Global, map: HashMap>>, } @@ -107,6 +109,7 @@ impl Default for TypeEventManager { fn default() -> Self { Self { active: Default::default(), + global: Default::default(), map: Default::default(), } } diff --git a/iris/core/src/event/mod.rs b/iris/core/src/event/mod.rs index 7b038f4..5619fb4 100644 --- a/iris/core/src/event/mod.rs +++ b/iris/core/src/event/mod.rs @@ -9,6 +9,20 @@ pub use rsc::*; pub trait Event: Sized + 'static + Clone { type Data<'a>: Clone = (); type State: Default = (); + /// State this event owns that belongs to no single widget -- what the + /// thing dispatching the event knows about the *input*, rather than + /// about a listener. `()` for almost every event; the cursor's is + /// `iris::sense::PointerInput` (which widget holds pointer capture, + /// and who is tracking the press in flight). + /// + /// It lives here so that such state has one owner, reached by `&mut` + /// through the event manager, instead of being parked on whatever + /// structure a handler happens to be able to reach and guarded with a + /// lock. Iris asked for that on 2026-09-08, of the pointer capture + /// that used to sit in a `Mutex` on `UiRenderState`: "everything + /// global should be stored in the general input handler, not in + /// specific senses with locking stuff." + type Global: Default = (); #[allow(unused_variables)] fn should_run<'a>(&self, data: &Self::Data<'a>) -> Option> { Some(data.clone()) diff --git a/iris/core/src/ui/render_state.rs b/iris/core/src/ui/render_state.rs index d83976a..f29c1ed 100644 --- a/iris/core/src/ui/render_state.rs +++ b/iris/core/src/ui/render_state.rs @@ -57,21 +57,6 @@ pub struct UiRenderState { /// ever drawn and was never emptied. draw_started: HashSet, - /// The widget currently holding exclusive pointer input, if any -- - /// `iris::sense::SensorUi::run_sensors` reads and clears this every - /// call. Interior mutability (a `Mutex`, not a bare `Cell`, since a - /// `CursorData` reaching this through an async `task_on` handler needs - /// `Send`/`Sync`) because `run_sensors` takes `&self` (widgets are - /// dispatched to, not owned, at that layer) and this render state is - /// the one structure both backends (winit, android-view) already hold - /// across frames, the same way `old_root`/`resized` are -- see - /// `iris::sense`'s pointer-capture doc for why a drag needs this: once - /// a gesture has committed to panning or selecting, every later sample - /// of it must reach the same widget even if the finger has moved off - /// whatever hit region first noticed the press. Never held across an - /// await or another lock -- every access here is a single get/set. - captured: std::sync::Mutex>, - /// `Widget::draw` calls and `Primitives::region_mut` rewrites since the /// last `take_counters`. LAYOUT.md section 8's pass conditions are /// stated in terms of these two: an unchanged frame must cost 0 of @@ -103,10 +88,9 @@ pub struct UiRenderState { /// When the sensor dispatch (`SensorUi::run_sensors`, in the `iris` /// crate) last saw an input sample, dated by the sample's own clock /// (`CursorState::time`) rather than when the dispatch ran -- same - /// reasoning as that field's own doc. A `Mutex` rather than a - /// `Cell` for the same reason `captured` is: `run_sensors` takes `&self` - /// and this is the one render state both backends already share across - /// frames. + /// reasoning as that field's own doc. A `Mutex` because `run_sensors` + /// takes `&self` and this is the one render state both backends + /// already share across frames. last_input_at: Mutex>, } @@ -138,7 +122,6 @@ impl UiRenderState { old_root: None, resized: false, draw_started: Default::default(), - captured: Default::default(), draw_count: 0, region_mut_count: 0, mov_count: 0, @@ -719,11 +702,12 @@ impl UiRenderState { if undraw { // A captured widget that goes away mid-gesture (List's // virtualisation retiring a row, a rebuild) must not leave - // the pointer permanently captured by an id nothing will - // ever draw again -- `captured`'s own path out. - if *self.captured.lock().unwrap() == Some(id) { - *self.captured.lock().unwrap() = None; - } + // the pointer captured by an id nothing will ever draw + // again. That path out is the sensor pass's, not this + // one's: `iris::sense::SensorUi::run_sensors` releases a + // capture whose widget no longer resolves to a region, + // which covers this case and every other way an id can + // stop being drawn. // Permanent removal: retire this widget's own move slot // (the self-ownership ref taken when it was allocated) and // the up-link ref it held on its parent's slot -- read from @@ -926,27 +910,6 @@ impl UiRenderState { ) } - /// Give `id` exclusive pointer input from the next `run_sensors` call - /// on -- see `captured`'s field doc. Overwrites any previous capture - /// (a gesture that starts a new one has already decided the old one - /// is over). - pub fn capture_pointer(&self, id: WidgetId) { - *self.captured.lock().unwrap() = Some(id); - } - - /// Release exclusive pointer input, if any is held -- called once - /// `run_sensors` has delivered the terminal `Drop` to the capturing - /// widget, or by that widget itself if it decides the gesture is over - /// some other way. - pub fn release_pointer(&self) { - *self.captured.lock().unwrap() = None; - } - - /// The widget currently holding exclusive pointer input, if any. - pub fn captured_pointer(&self) -> Option { - *self.captured.lock().unwrap() - } - pub fn debug(&self, widgets: &Widgets, label: &str) -> impl Iterator { self.active.iter().filter_map(move |(&id, inst)| { let l = widgets.label(id); diff --git a/iris/src/attr.rs b/iris/src/attr.rs index 3c8a473..5f7a4dd 100644 --- a/iris/src/attr.rs +++ b/iris/src/attr.rs @@ -52,8 +52,16 @@ pub fn recent_click(last_click: &mut Instant) -> bool { /// rather than reacting to `PressStart` alone the way `click_or_drag`'s /// consumer used to (Iris, 2026-09-06: "if I swipe over the input bar it /// brings up the keyboard"). +/// `CursorSense::Cancel` is in the set for the same reason `DragGesture` +/// registers it: if a scroll area or a list takes the pointer mid-gesture, +/// this field sees no `PressEnd`, and a `press_origin` left set is then +/// compared against the *next* press -- a stray selection, or a keyboard +/// summoned by a tap somewhere else entirely. fn press_track() -> CursorSenses { - CursorSense::click() | CursorSense::Pressing(CursorButton::Left) | CursorSense::unclick() + CursorSense::click() + | CursorSense::Pressing(CursorButton::Left) + | CursorSense::unclick() + | CursorSense::Cancel } pub struct Selector; @@ -185,6 +193,7 @@ fn on_press( state.focus_gained(render.window_region(&id, &*rsc)); } } + CursorSense::Cancel => id.edit(rsc).text.press_origin = None, _ => {} } return; @@ -205,6 +214,9 @@ fn on_press( ctx.text.press_origin = None; } } + // The gesture was taken by somebody else, so it is not a tap and + // must not grant focus when it ends out of this widget's sight. + CursorSense::Cancel => id.edit(rsc).text.press_origin = None, CursorSense::PressEnd(_) => { let was_tap = id.edit(rsc).text.press_origin.take().is_some(); if was_tap { diff --git a/iris/src/event.rs b/iris/src/event.rs index cee7f92..d79d32f 100644 --- a/iris/src/event.rs +++ b/iris/src/event.rs @@ -44,13 +44,19 @@ impl, Rsc: HasEvents, Tag> Eventable for WL { widget_trait! { pub trait TaskEventable; - fn task_on<'a, E: EventLike, F: AsyncWidgetEventFn>( + /// No `Data: Send` bound, deliberately: the registered handler below + /// takes `|_, rsc|` and the event's data never crosses into the + /// spawned future -- `AsyncEventIdCtx` carries the widget id and the + /// task handle and nothing else. The bound used to be here anyway, and + /// it was the whole reason `CursorData`'s pointer state was behind a + /// `Mutex` rather than owned by the input handler (Iris, 2026-09-08: + /// never reach for a lock first). + fn task_on>( self, event: E, f: F, ) -> impl WidgetIdFn - where ::Data<'a>: Send, - for<'b> F::CallRefFuture<'b>: Send, + where for<'b> F::CallRefFuture<'b>: Send, { let f = Arc::new(f); move |rsc| { diff --git a/iris/src/sense.rs b/iris/src/sense.rs index be2ee57..2639a63 100644 --- a/iris/src/sense.rs +++ b/iris/src/sense.rs @@ -30,6 +30,20 @@ pub enum CursorSense { /// never call `capture_pointer` and have no use for it) to receive it /// at all; ordinary hit-tested widgets keep seeing `PressEnd`. Drop, + /// Delivered exactly once to a widget that was tracking this press + /// when **another** widget took pointer capture + /// (`UiRenderState::capture_pointer`): the gesture it was following + /// has been taken away and it will see no further frame of it, not + /// even a `PressEnd` or a `Drop`. + /// + /// A separate sense rather than a second meaning for `Drop`, because + /// the two say opposite things to the widget reading them: `Drop` is + /// "your gesture finished", and a widget acts on it (a fling, a tap, + /// a link followed), while `Cancel` is "your gesture was never + /// yours", and acting on it is exactly the bug -- a horizontal pan of + /// a code fence would follow whatever markdown link the finger + /// happened to go down on. Registered explicitly, like `Drop`. + Cancel, } #[derive(Clone)] @@ -38,6 +52,7 @@ pub struct CursorSenses(Vec); impl Event for CursorSenses { type Data<'a> = CursorData<'a>; type State = SensorState; + type Global = PointerInput; fn should_run<'a>(&self, data: &Self::Data<'a>) -> Option> { // `Drop` is never derived from raw cursor/hover state below (the // free `should_run`'s own arm for it is only ever asked here, @@ -51,8 +66,13 @@ impl Event for CursorSenses { // `PressEnd` matches on, so falling through to the loop below // would let whichever of the two happens to be registered first // win, silently swallowing the `Drop` a caller relied on. - if data.sense == CursorSense::Drop { - return self.contains(&CursorSense::Drop).then(|| data.clone()); + // The same argument as `Drop` immediately above, for the same + // reason: `run_sensors` has already decided this frame is a + // cancellation for this widget, and the registration list very + // likely also carries `Pressing`, which the loop below would + // match against a button that is still down. + if data.sense == CursorSense::Drop || data.sense == CursorSense::Cancel { + return self.contains(&data.sense).then(|| data.clone()); } if let Some(sense) = should_run(self, &data.cursor, data.hover) { let mut data = data.clone(); @@ -74,6 +94,21 @@ impl CursorSense { pub fn unclick() -> Self { Self::PressEnd(CursorButton::Left) } + /// What a widget driving a [`DragGesture`] must register: the frames + /// of the gesture, plus **both** of the ways it can end for that + /// widget -- its own [`Self::Drop`] once it has captured the pointer, + /// and [`Self::Cancel`] if somebody else captured it first. + /// + /// One function rather than a set spelled out per call site, because + /// the two terminal senses are exactly what gets forgotten: a `Scroll` + /// registered `click_or_drag | unclick` and so never saw the end of + /// any gesture it had captured, which left its arbiter panning from a + /// stale position and made the *next* drag jump by the distance + /// between them -- Iris's "it keeps snapping back to some position + /// when horizontally scrolling" (docs/RUST.md, 2026-09-08). + pub fn drag_senses() -> CursorSenses { + Self::click_or_drag() | Self::unclick() | Self::Drop | Self::Cancel + } pub fn is_dragging(&self) -> bool { matches!(self, CursorSense::Pressing(CursorButton::Left)) } @@ -207,6 +242,99 @@ pub struct CursorData<'a> { /// the first sense that triggered this pub sense: CursorSense, pub render: &'a UiRenderState, + /// The pointer itself, for the length of this dispatch -- who holds + /// exclusive input and how to ask for it. See [`PointerRequests`]. + pub pointer: &'a PointerRequests, +} + +/// What the sensor pass knows about the pointer itself rather than about +/// any one listener: who has exclusive input, and who is tracking the +/// press in flight. [`Event::Global`] for [`CursorSenses`], so it is owned +/// by the event manager that runs the dispatch and reached by `&mut` -- +/// there is no lock and no copy of it anywhere else. +/// +/// **Capture** ([`PointerRequests::capture`]) gives one widget every later +/// sample of the gesture, so a pan or a selection keeps going once the +/// finger has moved off whatever hit region first noticed the press -- +/// including right off the end of it, which is what used to leave a fling +/// never started because no widget saw the release. +/// +/// **`pressed`** is capture's other half: every widget that has been +/// handed a frame of this press and not yet been told it ended. Taking the +/// pointer is a one-way door for all of them -- they see no `PressEnd` and +/// no `Drop` -- so each is sent one [`CursorSense::Cancel`], the way +/// Android sends `ACTION_CANCEL` and the web sends `pointercancel`. +/// Without it a gesture is left open forever with a stale origin, and the +/// *next* touch anywhere on screen is measured from it: Iris's 2026-09-08 +/// phone report, where a horizontal pan inside a code fence made the +/// transcript jump on the following tap (docs/RUST.md). +#[derive(Default)] +pub struct PointerInput { + captured: Option, + pressed: Vec, +} + +impl PointerInput { + /// Which widget holds exclusive pointer input between dispatches. + pub fn holder(&self) -> Option { + self.captured + } + + /// Hand the pointer to `id` from outside the sensor pass -- a test + /// setting a gesture up, or a backend tearing one down with `None`. + /// A handler *inside* the pass uses [`PointerRequests::capture`] + /// instead, which is the same state seen through the dispatch. + pub fn set_holder(&mut self, id: Option) { + self.captured = id; + } +} + +/// The pointer state of `rsc`'s cursor dispatch, for a caller outside the +/// sensor pass. Inside it, a handler has [`PointerRequests`] on its +/// [`CursorData`] and should use that. +pub fn pointer_input(rsc: &mut Rsc) -> &mut PointerInput { + &mut rsc.events_mut().get_type::().global +} + +/// The pointer, as a handler sees it during one dispatch: what it may ask +/// of the capture, and who holds it. Owned by [`SensorUi::run_sensors`] +/// for the length of the dispatch and folded back into [`PointerInput`] +/// straight after, so a handler's request never races anything and nothing +/// global is reachable from a widget. +/// +/// A `Cell`, not a lock: this is one frame of one thread's dispatch, and +/// the interior mutability is only here because a handler is handed +/// `CursorData` by shared reference. +#[derive(Default)] +pub struct PointerRequests { + holder: std::cell::Cell>, +} + +impl PointerRequests { + /// Give `id` exclusive pointer input from the next dispatch on. `id` + /// must be a 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. Overwrites any previous capture: a gesture that + /// starts a new one has already decided the old one is over, and the + /// old holder is told so with [`CursorSense::Cancel`]. + pub fn capture(&self, id: WidgetId) { + self.holder.set(Some(id)); + } + + /// Give up exclusive pointer input. Called for the capturing widget by + /// `run_sensors` itself once it has delivered the terminal + /// [`CursorSense::Drop`], or by that widget if it decides the gesture + /// is over some other way. + pub fn release(&self) { + self.holder.set(None); + } + + /// Which widget holds exclusive pointer input, as of this moment in + /// the dispatch. What a widget checks before releasing, so it cannot + /// drop a capture that is somebody else's. + pub fn holder(&self) -> Option { + self.holder.get() + } } pub trait SensorUi { @@ -232,27 +360,44 @@ impl SensorUi for UiRenderState { // so recording it once in the one place they share is what keeps // it from needing a copy per backend. self.note_input(cursor.time); - // Exclusive pointer capture (`UiRenderState::capture_pointer`, - // `DragGesture`): once some widget has committed to a drag, every - // other widget sees nothing from this pointer at all -- no hover, - // no click, no press -- until it releases. This is what lets a - // fast pan or a selection keep going once the finger has moved - // off whatever hit region first noticed the press (including - // right off the end of the gesture, at `PressEnd`/`Cancel`): a - // per-widget hit test would otherwise silently stop delivering to - // *anyone* the moment the pointer left every registered region, - // which is exactly what used to leave a fling never started (no - // widget ever saw the release). The captured widget keeps getting - // ordinary `Pressing` frames while the button is down and gets - // exactly one `Drop` -- not `PressEnd` -- the frame it lifts, - // which also releases the capture. - if let Some(id) = self.captured_pointer() { + + // The pointer's own state, taken out of the event manager for the + // length of this dispatch and put back at the end -- the same + // `mem::take` the `active` map below uses, and for the same + // borrow reason. `PointerRequests` is what a handler sees of it. + let mut pointer: PointerInput = + std::mem::take(&mut rsc.events_mut().get_type::().global); + let requests = PointerRequests { + holder: std::cell::Cell::new(pointer.captured), + }; + let button_down = cursor.buttons.select(&CursorButton::Left).is_on(); + + // Exclusive pointer capture (`PointerRequests::capture`): once + // some widget has committed to a drag, every other widget sees + // nothing from this pointer at all -- no hover, no click, no press + // -- until it releases. That is what lets a fast pan or a + // selection keep going once the finger has moved off whatever hit + // region first noticed the press (including right off the end of + // the gesture, at `PressEnd`/`Cancel`): a per-widget hit test + // would otherwise silently stop delivering to *anyone* the moment + // the pointer left every registered region, which is exactly what + // used to leave a fling never started (no widget ever saw the + // release). The captured widget keeps getting ordinary `Pressing` + // frames while the button is down and gets exactly one `Drop` -- + // not `PressEnd` -- the frame it lifts, which also releases the + // capture. + if let Some(id) = requests.holder() { + // The capture's path out for a widget that stopped being + // drawn mid-gesture -- a `List` row retired by virtualisation, + // a rebuilt subtree. Nothing can be delivered to an id with no + // region, so the gesture ends here for everyone. let Some(shape) = self.resolved_region(&id, rsc) else { - self.release_pointer(); + pointer.captured = None; + pointer.pressed.clear(); + rsc.events_mut().get_type::().global = pointer; return; }; let region = shape.to_px(window_size); - let button_down = cursor.buttons.select(&CursorButton::Left).is_on(); let sense = if button_down { CursorSense::Pressing(CursorButton::Left) } else { @@ -266,11 +411,15 @@ impl SensorUi for UiRenderState { cursor: cursor.clone(), sense, render: self, + pointer: &requests, }; rsc.run_event::(id, data, state); if !button_down { - self.release_pointer(); + requests.release(); + pointer.pressed.clear(); } + pointer.captured = requests.holder(); + rsc.events_mut().get_type::().global = pointer; return; } @@ -352,17 +501,88 @@ impl SensorUi for UiRenderState { // might wanna set up Event to have a prepare stage sense: CursorSense::Hovering, render: self, + pointer: &requests, }; rsc.run_event::(*id, data, state); + // Anything handed a frame while the button is down may + // have opened a gesture on it, and is owed a `Cancel` if + // somebody else captures the pointer -- see + // `PointerInput`. Recorded for every such widget rather + // than only the ones known to drag, because this layer + // cannot see what a handler did with the frame. + if button_down && !pointer.pressed.contains(id) { + pointer.pressed.push(*id); + } } if sensed { break; } } rsc.events_mut().get_type::().active = active; + + pointer.captured = requests.holder(); + match pointer.captured { + // A capture taken during this frame's dispatch: every other + // widget tracking the same press is told, once, that it is + // over for them. Delivered after `active` is restored, since + // these are ordinary registered handlers being run outside + // the loop. + Some(winner) => { + let losers: Vec = pointer + .pressed + .iter() + .copied() + .filter(|&id| id != winner) + .collect(); + pointer.pressed.retain(|&id| id == winner); + for loser in losers { + deliver_cancel(self, rsc, state, loser, &cursor, window_size, &requests); + } + } + // The press ended without anyone capturing: everybody who saw + // it got their own `PressEnd`, so there is nothing to cancel + // and nothing to remember. + None if !button_down => pointer.pressed.clear(), + None => {} + } + // A cancel handler may itself have captured (a widget deciding + // the gesture is now its own); `requests` is still the truth. + pointer.captured = requests.holder(); + rsc.events_mut().get_type::().global = pointer; } } +/// Hand one widget a [`CursorSense::Cancel`] -- the gesture it was +/// tracking has been taken by whoever captured the pointer. Silent if +/// the widget has no resolved region any more (it was retired in the +/// same frame), which is the same "nothing to deliver to" case +/// `run_sensors`' capture branch handles by releasing. +fn deliver_cancel( + render: &UiRenderState, + rsc: &mut Rsc, + state: &mut Rsc::State, + id: WidgetId, + cursor: &CursorState, + window_size: Vec2, + pointer: &PointerRequests, +) { + let Some(shape) = render.resolved_region(&id, rsc) else { + return; + }; + let region = shape.to_px(window_size); + let data = CursorData { + pos: cursor.pos - region.top_left, + size: region.bot_right - region.top_left, + scroll_delta: cursor.scroll_delta, + hover: ActivationState::On, + cursor: cursor.clone(), + sense: CursorSense::Cancel, + render, + pointer, + }; + rsc.run_event::(id, data, state); +} + pub fn should_run( senses: &CursorSenses, cursor: &CursorState, @@ -385,7 +605,10 @@ pub fn should_run( // happened to register `Drop` (with no capture involved at // all) would see it fire on every plain button-up under the // cursor. - CursorSense::Drop => false, + // Neither is ever derived from raw state -- see the `Drop` + // note above; both are set by `run_sensors` alone, for the one + // widget it is delivering to this frame. + CursorSense::Drop | CursorSense::Cancel => false, } { return Some(*sense); } @@ -879,6 +1102,12 @@ pub enum GestureOutcome { /// same units as `Pan`, so a caller hands it to `List::fling` with /// whatever sign flip it already applies to `Pan`. Released(Option), + /// Another widget took the pointer (`CursorSense::Cancel`), so this + /// gesture is over and **nothing** should be acted on: not a tap, not + /// a fling, not a selection. Distinct from `Released(None)`, which is + /// a gesture of this widget's own that simply ended with nothing to + /// hand on. + Cancelled, } /// Bundles a [`DragArbiter`] and a [`VelocityTracker`] into the one thing @@ -960,7 +1189,7 @@ impl DragGesture { /// 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, + CursorSense::Drop | CursorSense::PressEnd(_) | CursorSense::Cancel => false, _ => self.arbiter.is_idle(), } } @@ -969,13 +1198,13 @@ impl DragGesture { /// 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 + /// retired mid-drag as content scrolls). `pointer` is `CursorData`'s /// 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, + pointer: &PointerRequests, id: WidgetId, sense: CursorSense, pos_window: Vec2, @@ -983,6 +1212,22 @@ impl DragGesture { press: PressState, ) -> GestureOutcome { match sense { + // Somebody else won this gesture. Forget it completely -- + // leaving the arbiter open is the fault this sense was added + // for, since its origin then measures the *next* touch and + // pans by the distance between two unrelated fingers. + CursorSense::Cancel => { + if crate::diagnostics::trace_enabled() { + log::debug!( + target: "iris::input", + "iris gesture: cancelled (pointer captured elsewhere)", + ); + } + self.arbiter.release(); + self.catch_unmoved = false; + self.velocity.reset(); + GestureOutcome::Cancelled + } CursorSense::Drop | CursorSense::PressEnd(_) => { // Once: a `velocity()` is a full Lsq2 fit, and the log // line below wants the same number the outcome carries. @@ -1036,7 +1281,14 @@ impl DragGesture { } self.arbiter.release(); self.catch_unmoved = false; - render.release_pointer(); + // Only if this gesture is the one holding it. A widget + // that never captured (it stayed `Undecided`, so this + // release is a tap) would otherwise drop somebody else's + // capture mid-drag, which is the same lost-gesture bug + // `CursorSense::Cancel` exists to prevent, in reverse. + if pointer.holder() == Some(id) { + pointer.release(); + } outcome } // A `Pressing` frame can arrive with no matching `PressStart` @@ -1070,15 +1322,15 @@ impl DragGesture { pos_window.x, pos_window.y, press.scrolling, ); } - self.dispatch(render, id, pos_window, now) + self.dispatch(pointer, id, pos_window, now) } - _ => self.dispatch(render, id, pos_window, now), + _ => self.dispatch(pointer, id, pos_window, now), } } fn dispatch( &mut self, - render: &UiRenderState, + pointer: &PointerRequests, id: WidgetId, pos: Vec2, now: Instant, @@ -1086,7 +1338,7 @@ impl DragGesture { match self.arbiter.update(pos, now) { DragOutcome::Undecided => GestureOutcome::Undecided, DragOutcome::Pan(dy) => { - render.capture_pointer(id); + pointer.capture(id); if dy != 0.0 { // The catch has moved something, so its release is an // ordinary pan release again -- see `catch_unmoved`. @@ -1100,11 +1352,11 @@ impl DragGesture { GestureOutcome::Pan(dy) } DragOutcome::SelectStart => { - render.capture_pointer(id); + pointer.capture(id); GestureOutcome::SelectStart } DragOutcome::SelectExtend => { - render.capture_pointer(id); + pointer.capture(id); GestureOutcome::SelectExtend } } @@ -2275,14 +2527,14 @@ mod drag_gesture_tests { *BASE + Duration::from_millis(ms) } - /// A `UiRenderState` with nothing in it. `DragGesture` only ever calls - /// `capture_pointer`/`release_pointer` on it, which are bookkeeping on - /// a `Cell` and need no widget tree behind them. - fn render() -> UiRenderState { - UiRenderState::new() + /// The pointer as a handler sees it, with nothing captured. + /// `DragGesture` only ever reads and sets the holder, which needs no + /// widget tree behind it. + fn pointer() -> PointerRequests { + PointerRequests::default() } - /// The id `capture_pointer` records. Any id will do -- nothing here + /// The id a capture records. Any id will do -- nothing here /// resolves it -- so it comes from a real (empty) widget registry /// rather than being fabricated. fn some_id(ui: &mut UiData) -> WidgetId { @@ -2300,7 +2552,7 @@ mod drag_gesture_tests { fn a_flick_delivered_as_two_move_frames_releases_with_a_velocity() { let mut ui = UiData::default(); let id = some_id(&mut ui); - let r = render(); + let r = pointer(); let mut g = DragGesture::new(); g.handle( @@ -2358,7 +2610,7 @@ mod drag_gesture_tests { fn a_flick_delivered_as_one_move_frame_carries_no_velocity_to_fit() { let mut ui = UiData::default(); let id = some_id(&mut ui); - let r = render(); + let r = pointer(); let mut g = DragGesture::new(); g.handle( @@ -2395,7 +2647,7 @@ mod drag_gesture_tests { fn a_tap_is_still_a_tap_and_flings_nothing() { let mut ui = UiData::default(); let id = some_id(&mut ui); - let r = render(); + let r = pointer(); let mut g = DragGesture::new(); g.handle( @@ -2425,7 +2677,7 @@ mod drag_gesture_tests { fn a_selection_release_carries_no_velocity() { let mut ui = UiData::default(); let id = some_id(&mut ui); - let r = render(); + let r = pointer(); let mut g = DragGesture::new(); g.handle( @@ -2463,7 +2715,7 @@ mod drag_gesture_tests { 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 r = pointer(); let mut g = DragGesture::new(); let caught = PressState { scrolling: true, @@ -2504,7 +2756,7 @@ mod drag_gesture_tests { fn the_same_press_on_settled_content_stays_undecided() { let mut ui = UiData::default(); let id = some_id(&mut ui); - let r = render(); + let r = pointer(); let mut g = DragGesture::new(); g.handle( @@ -2538,7 +2790,7 @@ mod drag_gesture_tests { 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 r = pointer(); let mut g = DragGesture::new(); let caught = PressState { scrolling: true, @@ -2574,7 +2826,7 @@ mod drag_gesture_tests { fn a_catch_that_then_drags_still_flings() { let mut ui = UiData::default(); let id = some_id(&mut ui); - let r = render(); + let r = pointer(); let mut g = DragGesture::new(); let caught = PressState { scrolling: true, @@ -2625,7 +2877,7 @@ mod drag_gesture_tests { 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 r = pointer(); let mut g = DragGesture::new(); let caught = PressState { diff --git a/iris/src/sense_tests.rs b/iris/src/sense_tests.rs index 172ac28..35c2254 100644 --- a/iris/src/sense_tests.rs +++ b/iris/src/sense_tests.rs @@ -130,7 +130,7 @@ fn a_button_over_a_list_scrolls_the_list_and_still_clicks() { /// the press, in a gap, or off the loaded content entirely. Before pointer /// capture, `run_sensors`' hit test simply delivered nothing that frame, /// so a widget mid-drag never saw its release and never got a chance to -/// start a fling. `UiRenderState::capture_pointer`/`DragGesture` fix this +/// start a fling. `PointerRequests::capture`/`DragGesture` fix this /// by giving the drag's widget every frame regardless of where the /// pointer is, including the terminal `Drop` in place of `PressEnd`. #[test] @@ -157,7 +157,7 @@ fn a_release_outside_every_hit_region_still_reaches_the_captured_widget() { // would gate this on a `DragArbiter`/`DragGesture` // decision, but this test only needs to exercise the // capture-and-release mechanics themselves. - ctx.data.render.capture_pointer(draggable_weak.id()); + ctx.data.pointer.capture(draggable_weak.id()); let _ = rsc; } CursorSense::Drop => dropped.set(true), @@ -175,7 +175,7 @@ fn a_release_outside_every_hit_region_still_reaches_the_captured_widget() { press.buttons.left = ActivationState::Start; render.run_sensors(&mut rsc, &mut state, press, (100.0, 100.0).into()); assert_eq!( - render.captured_pointer(), + pointer_input(&mut rsc).holder(), Some(draggable.id()), "the press should have taken capture" ); @@ -192,7 +192,7 @@ fn a_release_outside_every_hit_region_still_reaches_the_captured_widget() { the widget holding pointer capture" ); assert_eq!( - render.captured_pointer(), + pointer_input(&mut rsc).holder(), None, "Drop must release the capture" ); @@ -236,7 +236,7 @@ fn capturing_one_widget_starves_every_other_widget_of_events() { let mut render = UiRenderState::new(); render.resize((100.0, 100.0)); render.update(&root, &mut rsc); - render.capture_pointer(a_weak.id()); + pointer_input(&mut rsc).set_holder(Some(a_weak.id())); let mut state = (); let cursor = cursor_at((50.0, 50.0).into()); @@ -315,7 +315,7 @@ fn a_finger_drag_over_a_scroll_area_pans_it() { // And the gesture holds the pointer, so the rest of it reaches this // widget even once the finger leaves its box. - assert_eq!(render.captured_pointer(), Some(scroll.id())); + assert_eq!(pointer_input(&mut rsc).holder(), Some(scroll.id())); } /// docs/REVIEW-2026-09-07.md's D4. The first `MotionEvent` a view sees can @@ -364,3 +364,175 @@ fn the_clock_orders_samples_across_events() { std::time::Duration::from_millis(16) ); } + +/// Iris's 2026-09-08 phone report, first half: "it keeps snapping back to +/// some position when horizontally scrolling." +/// +/// A `Scroll` that has committed to a pan holds the pointer, so the +/// gesture's end arrives as `CursorSense::Drop` -- and `scrollable_on` +/// used to register `click_or_drag | unclick` only, which `should_run` +/// never matches a `Drop` against. So the widget never learned its own +/// gesture had ended: its `DragArbiter` stayed `Panning` at the position +/// the finger left, and the *next* drag's first frame was measured from +/// there and applied in one step. The registration is +/// `CursorSense::drag_senses()` now, which is the rule for every widget +/// driving a `DragGesture` rather than a fact about this one. +#[test] +fn a_scroll_area_that_captured_the_pointer_learns_its_gesture_ended() { + let mut rsc = SenseRsc { + ui: UiData::default(), + events: EventManager::default(), + }; + let scroll_strong = rect(UiColor::WHITE) + .height(Len::abs(1000.0)) + .scrollable() + .add_strong(&mut rsc); + let scroll = scroll_strong.weak(); + let root = scroll_strong.any(); + + let mut render = UiRenderState::new(); + render.resize((100.0, 100.0)); + render.update(&root, &mut rsc); + rsc.ui.widgets.get_mut(&scroll).unwrap().scroll(0.0); + render.update(&root, &mut rsc); + + let mut state = (); + let win = Vec2::new(100.0, 100.0); + let mut send = |render: &UiRenderState, rsc: &mut SenseRsc, y: f32, button| { + let mut c = cursor_at((50.0, y).into()); + c.buttons.left = button; + render.run_sensors(rsc, &mut state, c, win); + }; + + // One pan of 40px past the slop, then a release well outside the + // widget -- the ordinary shape of a flick. + send(&render, &mut rsc, 80.0, ActivationState::Start); + send( + &render, + &mut rsc, + 80.0 - (DRAG_SLOP + 40.0), + ActivationState::On, + ); + let after_first = rsc.ui.widgets.get(&scroll).unwrap().amt(); + assert!((after_first - 40.0).abs() < 0.01, "amt={after_first}"); + send(&render, &mut rsc, 400.0, ActivationState::End); + assert_eq!( + pointer_input(&mut rsc).holder(), + None, + "the release must give the pointer back" + ); + + // A second gesture, starting where the first one did. If the arbiter + // were still panning from the release position, this first frame + // would apply the whole distance between the two at once. + send(&render, &mut rsc, 80.0, ActivationState::Start); + let after_second = rsc.ui.widgets.get(&scroll).unwrap().amt(); + assert!( + (after_second - after_first).abs() < 0.01, + "a fresh touch-down moved the content by {} -- the previous \ + gesture was never closed", + after_second - after_first, + ); +} + +/// The second half of the same report: "tapping sometimes seems to make +/// the scrolling jump, particularly when tapping on things that have +/// events like horizontal scrolling." +/// +/// Two widgets see the same press -- a scroll area and, under it, +/// something tracking the gesture for a list. When the scroll area +/// captures, the other one is cut off completely: no `PressEnd`, no +/// `Drop`. It has to be told, or its gesture stays open at an origin +/// belonging to a finger that has long gone, and the next unrelated touch +/// is measured from it. +#[test] +fn taking_the_pointer_cancels_everyone_else_tracking_the_press() { + let mut rsc = SenseRsc { + ui: UiData::default(), + events: EventManager::default(), + }; + + // The bystander *contains* the capturer, which is the real shape: a + // transcript's `List` and one row's own text both track the same + // press, and a `Stack`'s siblings would be on separate layers where + // only the topmost is dispatched to at all. + let capturer = rsc.ui.widgets.add_strong(Rect::new(UiColor::WHITE)); + let capturer_weak = capturer.weak(); + let bystander = rsc.ui.widgets.add_strong(Stack { + children: vec![capturer.any()], + size: StackSize::default(), + }); + let bystander_weak = bystander.weak(); + + let capturer_saw = Rc::new(Cell::new(0u32)); + { + let capturer_saw = capturer_saw.clone(); + rsc.register_event( + capturer_weak, + CursorSense::drag_senses(), + move |ctx, _rsc| { + capturer_saw.set(capturer_saw.get() + 1); + if matches!(ctx.data.sense, CursorSense::Pressing(_)) { + ctx.data.pointer.capture(capturer_weak.id()); + } + }, + ); + } + let cancelled = Rc::new(Cell::new(0u32)); + let ended = Rc::new(Cell::new(0u32)); + { + let (cancelled, ended) = (cancelled.clone(), ended.clone()); + rsc.register_event( + bystander_weak, + CursorSense::drag_senses(), + move |ctx, _rsc| match ctx.data.sense { + CursorSense::Cancel => cancelled.set(cancelled.get() + 1), + CursorSense::PressEnd(_) | CursorSense::Drop => ended.set(ended.get() + 1), + _ => {} + }, + ); + } + + let root = bystander.any(); + let mut render = UiRenderState::new(); + render.resize((100.0, 100.0)); + render.update(&root, &mut rsc); + + let mut state = (); + let win = Vec2::new(100.0, 100.0); + let mut down = cursor_at((50.0, 50.0).into()); + down.buttons.left = ActivationState::Start; + render.run_sensors(&mut rsc, &mut state, down, win); + assert_eq!(cancelled.get(), 0, "nothing has captured yet"); + + let mut moved = cursor_at((50.0, 20.0).into()); + moved.buttons.left = ActivationState::On; + render.run_sensors(&mut rsc, &mut state, moved, win); + assert!(capturer_saw.get() > 0, "the capturer never saw the press"); + assert_eq!( + pointer_input(&mut rsc).holder(), + Some(capturer_weak.id()), + "the capture should have been taken on this frame" + ); + assert_eq!( + cancelled.get(), + 1, + "the widget that lost the gesture must be told exactly once" + ); + + // And exactly once: the frames after the capture reach the capturer + // alone, so there is nothing left to cancel. + let mut more = cursor_at((50.0, 10.0).into()); + more.buttons.left = ActivationState::On; + render.run_sensors(&mut rsc, &mut state, more, win); + let mut up = cursor_at((50.0, 10.0).into()); + up.buttons.left = ActivationState::End; + render.run_sensors(&mut rsc, &mut state, up, win); + assert_eq!(cancelled.get(), 1, "cancelled more than once"); + assert_eq!( + ended.get(), + 0, + "a cancelled widget must not also be told the gesture ended \ + normally -- acting on that is the tap it never made" + ); +} diff --git a/iris/src/widget/position/scroll.rs b/iris/src/widget/position/scroll.rs index a77658e..40a82d5 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, PressState}; +use crate::sense::{DragGesture, GestureOutcome, PointerRequests, PressState}; use std::time::Instant; pub struct Scroll { @@ -113,7 +113,7 @@ impl Scroll { /// released velocity is deliberately dropped rather than approximated. pub fn drag( &mut self, - render: &UiRenderState, + pointer: &PointerRequests, id: WidgetId, sense: CursorSense, pos_window: Vec2, @@ -128,7 +128,7 @@ impl Scroll { // `DragGesture` itself instead. match self .gesture - .handle(render, id, sense, pos_window, now, PressState::default()) + .handle(pointer, 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 @@ -143,6 +143,7 @@ impl Scroll { | GestureOutcome::Tapped | GestureOutcome::SelectStart | GestureOutcome::SelectExtend + | GestureOutcome::Cancelled | GestureOutcome::Released(_) => {} } } @@ -191,7 +192,7 @@ mod tests { fn press( s: &mut Scroll, - render: &UiRenderState, + render: &PointerRequests, id: WidgetId, sense: CursorSense, y: f32, @@ -203,7 +204,7 @@ mod tests { #[test] fn a_vertical_finger_drag_pans_the_content_with_the_finger() { let (_ui, mut s, id) = area(); - let render = UiRenderState::new(); + let render = PointerRequests::default(); let t = Instant::now(); press( &mut s, @@ -246,7 +247,7 @@ mod tests { #[test] fn a_press_that_stays_inside_the_slop_does_not_scroll() { let (_ui, mut s, id) = area(); - let render = UiRenderState::new(); + let render = PointerRequests::default(); let t = Instant::now(); press( &mut s, @@ -286,7 +287,7 @@ mod tests { #[test] fn a_horizontal_drag_does_not_scroll() { let (_ui, mut s, id) = area(); - let render = UiRenderState::new(); + let render = PointerRequests::default(); let t = Instant::now(); s.drag( &render, @@ -311,7 +312,7 @@ mod tests { #[test] fn a_pan_past_the_end_clamps_instead_of_running_off() { let (_ui, mut s, id) = area(); - let render = UiRenderState::new(); + let render = PointerRequests::default(); let t = Instant::now(); s.drag( &render, diff --git a/iris/src/widget/trait_fns.rs b/iris/src/widget/trait_fns.rs index 89eea22..5a59aa2 100644 --- a/iris/src/widget/trait_fns.rs +++ b/iris/src/widget/trait_fns.rs @@ -103,15 +103,12 @@ widget_trait! { // has the arbitration and why there is no fling. The wheel // above and this are the two inputs of one scroll, so they // are registered together rather than left to each caller. - .on( - CursorSense::click_or_drag() | CursorSense::unclick(), - |ctx, rsc| { - let id = ctx.widget.id(); - let (sense, pos) = (ctx.data.sense, ctx.data.cursor.pos); - ctx.widget(rsc) - .drag(ctx.data.render, id, sense, pos, ctx.data.cursor.time); - }, - ) + .on(CursorSense::drag_senses(), |ctx, rsc| { + let id = ctx.widget.id(); + let (sense, pos) = (ctx.data.sense, ctx.data.cursor.pos); + ctx.widget(rsc) + .drag(ctx.data.pointer, id, sense, pos, ctx.data.cursor.time); + }) .add(state) } } diff --git a/iris/transcript-ui/src/lib.rs b/iris/transcript-ui/src/lib.rs index 0bd3ee7..b6306f1 100644 --- a/iris/transcript-ui/src/lib.rs +++ b/iris/transcript-ui/src/lib.rs @@ -397,7 +397,7 @@ where { let selection = selection.clone(); list.on( - CursorSense::Pressing(CursorButton::Left) | CursorSense::Drop, + CursorSense::Pressing(CursorButton::Left) | CursorSense::Drop | CursorSense::Cancel, move |ctx, rsc| { // Which *block* the finger is over, resolved from its // drawn box rather than from the row's extent -- a row is @@ -413,7 +413,7 @@ where ctx.data.cursor.pos, ctx.data.sense, ctx.data.cursor.time, - ctx.data.render, + ctx.data.pointer, ); }, ) diff --git a/iris/transcript-ui/src/row.rs b/iris/transcript-ui/src/row.rs index 7823f8d..157d057 100644 --- a/iris/transcript-ui/src/row.rs +++ b/iris/transcript-ui/src/row.rs @@ -238,7 +238,7 @@ where cursor, ctx.data.sense, ctx.data.cursor.time, - ctx.data.render, + ctx.data.pointer, ); // A *tap*, decided by the same `DragArbiter` the pan and // the selection are: a gesture that panned the list past diff --git a/iris/transcript-ui/src/selection.rs b/iris/transcript-ui/src/selection.rs index f0d16e8..1f537a1 100644 --- a/iris/transcript-ui/src/selection.rs +++ b/iris/transcript-ui/src/selection.rs @@ -242,7 +242,7 @@ impl Selection { /// selection, where it is rare and the frame is simply dropped. /// `pos_window` is in window space, since a pan's delta has to stay /// meaningful even when this frame's event landed on a different row - /// than the last one. `render` is `CursorData`'s own field -- what + /// than the last one. `pointer` is `CursorData`'s own field -- what /// `DragGesture` needs to take pointer capture. #[allow(clippy::too_many_arguments)] /// Returns what the gesture decided this frame, so a caller with its @@ -257,7 +257,7 @@ impl Selection { pos_window: Vec2, sense: CursorSense, now: Instant, - render: &UiRenderState, + pointer: &PointerRequests, ) -> GestureOutcome { // A fresh touch-down cancels any fling still coasting from the // previous gesture -- `List::fling`'s own doc, and Android's @@ -278,9 +278,14 @@ impl Selection { press.already_selected = self.has_selection(ui); let outcome = self .gesture - .handle(render, list.id(), sense, pos_window, now, press); + .handle(pointer, list.id(), sense, pos_window, now, press); match outcome { - GestureOutcome::Undecided => {} + // Somebody else took the gesture (a code fence panning + // sideways under the finger). Nothing here acted on it, and + // `DragGesture` has already forgotten it, so there is nothing + // to undo either -- the point is that no tap, fling or + // selection follows from a gesture that was never ours. + GestureOutcome::Cancelled | GestureOutcome::Undecided => {} GestureOutcome::Pan(dy) => list(ui).scroll(-dy), GestureOutcome::SelectStart => { if let Some((key, pos_row, size)) = row { @@ -428,7 +433,7 @@ mod tests { sel.register((1, 0), field); assert!(sel.gesture.is_idle()); - let render = UiRenderState::new(); + let pointer = PointerRequests::default(); let now = Instant::now(); let size = Vec2::new(100.0, 20.0); // No `PressStart` is ever sent -- only the `Pressing` frames a @@ -440,7 +445,7 @@ mod tests { Vec2::new(540.0, 700.0), CursorSense::Pressing(CursorButton::Left), now, - &render, + &pointer, ); assert!( !sel.gesture.is_idle(), diff --git a/iris/transcript-ui/src/tool.rs b/iris/transcript-ui/src/tool.rs index 550df77..e9210c7 100644 --- a/iris/transcript-ui/src/tool.rs +++ b/iris/transcript-ui/src/tool.rs @@ -209,7 +209,7 @@ fn on_tap( ctx.data.cursor.pos, ctx.data.sense, ctx.data.cursor.time, - ctx.data.render, + ctx.data.pointer, ); if outcome == GestureOutcome::Tapped { f(rsc);