iris: fix the intermittent touch-scroll dropout (missed ACTION_DOWN hit-test)
Root-caused via temporary logcat tracing (touch events, DragArbiter state, Selection::drag dispatch), reproduced against a real sandbox session: a gesture's ACTION_DOWN can land on a row's own padding/gap or its header, which CursorSense has no sensor over, so the widget that ends up handling the gesture only ever sees Pressing frames and DragArbiter never gets press_start -- leaving it stuck in Idle (answers Undecided forever) for the rest of that gesture. Not the previously-suspected coalesced first ACTION_MOVE, which is now ruled out. DragArbiter::is_idle() lets Selection::drag notice a Pressing frame with no matching press_start and recover the press there instead. Four new unit tests, one of which fails on the pre-fix code. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
This commit is contained in:
1 parent
68b48cfd14
commit
e6924298bc
4 files changed
+168
No files matched your search
@@ -146,6 +146,24 @@ on a row's own rendered text now pans the list correctly instead of
|
||||
always starting a selection. 8 new unit tests in `iris/src/sense.rs`'s
|
||||
`drag_arbiter_tests` module.
|
||||
|
||||
### 2026-09-05, later: `DragArbiter::is_idle()`, recovering a missed `press_start`
|
||||
|
||||
Follow-up to the above, from a real touch-scroll dropout: a gesture's
|
||||
`ACTION_DOWN` can land on a caller's own dead space (a row's padding, a
|
||||
gap, a header with no handler) that never calls `press_start`, so the
|
||||
first frame the arbiter actually sees is a `Pressing`-shaped `update`
|
||||
with no matching start. Before this, `update`'s `Idle` arm had no way to
|
||||
tell that apart from "nothing is happening" and answered `Undecided`
|
||||
forever for the rest of that gesture. `is_idle(&self) -> bool` lets a
|
||||
caller notice the gap and recover: if `is_idle()` is true on a frame the
|
||||
caller knows a press is genuinely down (its own `Pressing`/equivalent
|
||||
sense fired), call `press_start` right there instead of assuming one
|
||||
already happened. `transcript-ui`'s `Selection::drag` is the reference
|
||||
caller — one new match arm, checked before the ordinary `update`-only
|
||||
case. Any other `DragArbiter` caller with the same "one sensor per
|
||||
sub-region, no fallback for dead space" shape has the same gap and wants
|
||||
the same recovery.
|
||||
|
||||
## 2026-09-05: `SpanStyle`, per-range text styling (RUST.md's I5)
|
||||
|
||||
A `TextBuffer` used to have exactly one style (`TextAttrs`: colour, size,
|
||||
|
||||
@@ -251,6 +251,20 @@ order and what "done" looks like. Tick and date them in place.
|
||||
--workspace` and `cargo ndk` (both `iris` and `transcript-ui`) all
|
||||
clean; `run-headless.sh` screenshot byte-identical to before the
|
||||
change (38578 bytes). See RUST.md's I5 box, "Gap closed, 2026-09-05".
|
||||
- [x] **Intermittent touch-scroll dropout — root-caused and fixed,
|
||||
2026-09-05.** Not the coalesced-`ACTION_MOVE` hypothesis the earlier
|
||||
pass suspected (ruled out): a gesture's `ACTION_DOWN` can land on a
|
||||
row's own padding/gap or its header, which no `CursorSense` covers,
|
||||
so `DragArbiter` never gets `press_start` and sits in `Idle`
|
||||
(answers `Undecided` forever) for that whole gesture. Fixed via a new
|
||||
`DragArbiter::is_idle()` that `Selection::drag`
|
||||
(`transcript-ui/src/selection.rs`) checks to recover a missed press
|
||||
on the next `Pressing` frame. Four new unit tests. See RUST.md's I5
|
||||
box, "Touch-scroll dropout root-caused, 2026-09-05", for the trace and
|
||||
what a peer session sharing this checkout's emulator mid-pass
|
||||
prevented from being re-verified end-to-end (the aggregate
|
||||
`iris-scroll.sh` three-run confirmation and a re-taken FrameReport
|
||||
row) — a future pass should finish that once the emulator is free.
|
||||
- [ ] **Row-level accessibility names.** The composer carries
|
||||
`.label("Message")`; transcript rows do not carry a `.label()` of
|
||||
their own yet, so `Widgets::named()` (I4) does not include them —
|
||||
|
||||
@@ -444,8 +444,31 @@ impl DragArbiter {
|
||||
self.state = ArbiterState::Undecided { already_selected };
|
||||
}
|
||||
|
||||
/// Whether this arbiter has no press in flight -- either it has never
|
||||
/// seen [`Self::press_start`], or the last one it saw was released.
|
||||
/// What a caller whose own `PressStart` sense can miss (see
|
||||
/// [`Self::update`]'s doc) uses to notice that a `Pressing` frame has
|
||||
/// arrived with no matching start, and recover by starting one now.
|
||||
pub fn is_idle(&self) -> bool {
|
||||
matches!(self.state, ArbiterState::Idle)
|
||||
}
|
||||
|
||||
/// The press continues (still down) at `pos`. Call once per frame
|
||||
/// while the button/finger is down; returns what this frame means.
|
||||
///
|
||||
/// A caller must not call this while [`Self::is_idle`] is true for a
|
||||
/// press that is genuinely still down -- `Idle` has no way to tell
|
||||
/// "no press is happening" from "a press is happening but this
|
||||
/// arbiter never got its `press_start`," so it always answers
|
||||
/// `Undecided` and never leaves `Idle` on its own. That second case is
|
||||
/// real: a touch's `ACTION_DOWN` lands whatever pixel the finger
|
||||
/// actually hit, which is not guaranteed to be inside the same
|
||||
/// row-local sensor region a later `ACTION_MOVE` in the same gesture
|
||||
/// lands in (a row's own padding/gap, or its non-selectable header, is
|
||||
/// pointer-transparent to `CursorSense`) -- so the widget that
|
||||
/// receives the gesture's first `Pressing` frame may never have seen
|
||||
/// its `PressStart`. `iris::transcript_ui::selection::Selection::drag`
|
||||
/// is the caller that recovers from this, via `is_idle`.
|
||||
pub fn update(&mut self, pos: Vec2, now: Instant) -> DragOutcome {
|
||||
match self.state {
|
||||
ArbiterState::Idle => DragOutcome::Undecided,
|
||||
@@ -584,6 +607,50 @@ mod drag_arbiter_tests {
|
||||
);
|
||||
}
|
||||
|
||||
/// A fresh arbiter that never saw `press_start` -- the state a widget's
|
||||
/// own arbiter is left in when a gesture's `ACTION_DOWN` landed on a
|
||||
/// pixel no sensor covered (a row's padding/gap, or its header) and
|
||||
/// only a later `ACTION_MOVE` reached this widget. `update` must not
|
||||
/// silently swallow the whole rest of the gesture here; `is_idle` is
|
||||
/// what a caller checks to notice and recover (RUST.md's I5
|
||||
/// intermittent-touch-scroll-dropout finding, 2026-09-05) --
|
||||
/// `transcript_ui::selection::Selection::drag` is the real caller,
|
||||
/// this is the pure-state half of the fix.
|
||||
#[test]
|
||||
fn is_idle_reports_a_press_that_was_never_started() {
|
||||
let a = DragArbiter::new();
|
||||
assert!(a.is_idle());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn update_on_an_idle_arbiter_stays_undecided_forever_without_recovery() {
|
||||
// Documents the failure this fix works around: calling `update`
|
||||
// (as if the arbiter were mid-gesture) without ever having called
|
||||
// `press_start` leaves it stuck answering `Undecided`, even for a
|
||||
// movement well past `DRAG_SLOP` that would otherwise pan
|
||||
// immediately.
|
||||
let mut a = DragArbiter::new();
|
||||
assert_eq!(
|
||||
a.update(Vec2::new(0.0, 100.0), t(10)),
|
||||
DragOutcome::Undecided
|
||||
);
|
||||
assert!(a.is_idle());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_caller_can_recover_a_missed_press_start_via_is_idle() {
|
||||
let mut a = DragArbiter::new();
|
||||
// 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);
|
||||
assert_eq!(
|
||||
a.update(Vec2::new(0.0, 720.0), t(10)),
|
||||
DragOutcome::Pan(20.0)
|
||||
);
|
||||
assert!(!a.is_idle());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn release_resets_to_idle() {
|
||||
let mut a = DragArbiter::new();
|
||||
|
||||
@@ -184,6 +184,22 @@ impl Selection {
|
||||
self.arbiter.release();
|
||||
return;
|
||||
}
|
||||
// A `Pressing` frame with the arbiter still `Idle` means this
|
||||
// gesture's `ACTION_DOWN` landed somewhere no row's sensor
|
||||
// covers (a row's own padding/gap, or a header with no
|
||||
// selection handler) and this row is only now getting the
|
||||
// touch as it moves across it -- the touch is definitely still
|
||||
// down (that's what `Pressing` means), so without this the
|
||||
// arbiter would sit in `Idle` answering `Undecided` for the
|
||||
// rest of the gesture (`DragArbiter::update`'s own doc).
|
||||
// Recovered by starting the press here instead of where it
|
||||
// was missed -- RUST.md's I5 intermittent-touch-scroll-dropout
|
||||
// finding, 2026-09-05.
|
||||
_ if self.arbiter.is_idle() => {
|
||||
let already_selected = self.has_selection(ui);
|
||||
self.arbiter.press_start(pos_window, now, already_selected);
|
||||
self.arbiter.update(pos_window, now)
|
||||
}
|
||||
_ => self.arbiter.update(pos_window, now),
|
||||
};
|
||||
match outcome {
|
||||
@@ -273,6 +289,59 @@ mod tests {
|
||||
}
|
||||
}
|
||||
|
||||
/// The RUST.md I5 intermittent-touch-scroll-dropout regression: a
|
||||
/// gesture whose `ACTION_DOWN` landed where no row's sensor covers
|
||||
/// (padding, a gap, a header with no handler) delivers this row only
|
||||
/// `Pressing` frames, never `PressStart`. Before the fix, the shared
|
||||
/// `DragArbiter` stayed `Idle` for the whole gesture (`DragArbiter::
|
||||
/// update`'s own doc), which is exactly what a real device trace
|
||||
/// showed for four of twenty-four otherwise-identical swipes in one
|
||||
/// run -- `ui-trace`-driven touch coordinates land on different row
|
||||
/// content each time the list actually scrolls, so whether `DOWN`
|
||||
/// happens to hit a sensor is intermittent by construction. This test
|
||||
/// fails on the code before `Selection::drag`'s `_ if self.arbiter.
|
||||
/// is_idle()` branch existed, because the arbiter would still report
|
||||
/// `is_idle()` after both calls below.
|
||||
#[test]
|
||||
fn a_missed_press_start_recovers_on_the_next_pressing_frame() {
|
||||
let mut rsc = TestRsc {
|
||||
ui: UiData::default(),
|
||||
};
|
||||
let field = rsc
|
||||
.ui
|
||||
.widgets
|
||||
.add_strong(TextEdit::new(
|
||||
TextView::new(TextBuffer::new_empty(), TextAttrs::default(), None),
|
||||
EditMode::MultiLine,
|
||||
))
|
||||
.weak();
|
||||
let list = rsc.ui.widgets.add_strong(List::new(Axis::Y)).weak();
|
||||
|
||||
let mut sel = Selection::new();
|
||||
sel.register(1, field);
|
||||
assert!(sel.arbiter.is_idle());
|
||||
|
||||
let now = Instant::now();
|
||||
let size = Vec2::new(100.0, 20.0);
|
||||
// No `PressStart` is ever sent -- only the `Pressing` frames a
|
||||
// widget whose sensor missed the `ACTION_DOWN` would actually see.
|
||||
sel.drag(
|
||||
&mut rsc,
|
||||
list,
|
||||
1,
|
||||
Vec2::ZERO,
|
||||
size,
|
||||
Vec2::new(540.0, 700.0),
|
||||
CursorSense::Pressing(CursorButton::Left),
|
||||
now,
|
||||
);
|
||||
assert!(
|
||||
!sel.arbiter.is_idle(),
|
||||
"a Pressing frame with the arbiter still Idle must recover \
|
||||
the press rather than leaving it stuck"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn unregister_forgets_the_row_and_clears_a_matching_anchor() {
|
||||
let mut rsc = TestRsc {
|
||||
|
||||
Reference in new issue
Block a user