iris: the input clock anchors on the first event's oldest sample, not its own time

docs/REVIEW-2026-09-07.md's D4. `on_touch_event` took its one anchor as
`(Instant::now(), event.event_time_nanos())` from the first MotionEvent the
view ever sees, and dated every later sample as `anchor_at + (sample -
anchor).max(0)`. An event's historical samples are by definition *older*
than its own event_time, so if that first event is a Move -- the Down went
to another view, or the view was attached mid-gesture -- its whole batch
clamps onto one instant: three samples at the same time make the Lsq2 fit
degenerate and the flick reads 0 px/s. In a debug build the ordering
debug_assert fired first, and it was comparing against `anchor_nanos`,
a value from a different event, so it was also the wrong comparison for
the first sample of every later event.

The arithmetic moves into `sense::PointerClock`, which anchors at
`now - (event_time - oldest_sample)` and carries the last sample seen
across events, so `sample()`'s ordering assert compares against the
previous event's last sample. It lives in `sense` rather than in the
android backend because `iris::android` is cfg'd out everywhere but the
device, and this is exactly the arithmetic that wanted a test off one:
`the_first_events_batched_samples_are_dated_apart` reports [0ns, 0ns, 0ns]
against the old anchoring.

The assert stays a `debug_assert!` and now says why in a comment: it runs
once per touch sample, hundreds a second on a batching 120Hz screen, and a
mis-ordered sample degrades a velocity rather than drawing something wrong.

Also drops the stale reference to `VelocityTracker::add_sample` in the
comment above it (the review's rule finding); the method is `add_position`.

Verified: `cargo test --lib -p iris` and `cargo ndk -t x86_64 -P 29 check
-p iris` clean, fmt and clippy clean.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
This commit is contained in:
irisandClaude Fable 5.1 committed 2026-09-07 20:36:41 -04:00
1 parent 992c472975
commit 2ec0fee84c
3 files changed
+154 -42

No files matched your search

+34 -41
View File
@@ -7,7 +7,7 @@ use android_view::{
jni::{
JNIEnv, JavaVM,
objects::{GlobalRef, JValue},
sys::{jint, jlong},
sys::jint,
},
ndk::event::{Axis, Keycode, MotionAction},
};
@@ -20,7 +20,7 @@ use std::{
marker::{PhantomData, Sized},
rc::Rc,
sync::Arc,
time::{Duration, Instant},
time::Instant,
};
use super::{
@@ -312,12 +312,11 @@ pub struct IrisViewPeer<State: AndroidAppState> {
pub(super) render: UiRenderState,
pub(super) state: State,
task_recv: TaskMsgReceiver<AndroidRsc<State>>,
/// `(an Instant, the input-event nanosecond stamp it was taken at)`,
/// captured from the first `MotionEvent` this view receives and never
/// changed after -- how `on_touch_event` dates every touch sample. Its
/// path out is the peer's own drop: it holds nothing but two numbers
/// and is meaningless to any other view.
input_clock: Option<(Instant, jlong)>,
/// Anchored on the first `MotionEvent` this view receives and never
/// re-anchored after -- how `on_touch_event` dates every touch sample.
/// Its path out is the peer's own drop: it holds nothing but three
/// numbers and is meaningless to any other view.
input_clock: Option<PointerClock>,
}
impl<State: 'static, I: RscIdx<AndroidRsc<State>>> std::ops::Index<I> for AndroidRsc<State> {
@@ -639,17 +638,22 @@ impl<State: AndroidAppState> ViewPeer for IrisViewPeer<State> {
// `CLOCK_MONOTONIC` an `Instant` reads, so a single
// `(Instant, nanos)` pair converts every later sample exactly.
// Anchoring **once** rather than per event is what keeps the times
// ordered: a fresh `Instant::now()` per event, minus each sample's
// age inside it, can date a later event's first historical sample
// before the previous event's last one whenever delivery jitters by
// more than the batch spans -- and `VelocityTracker::add_sample`'s
// debug assert would rightly fire on that. See `CursorState::time`.
// ordered, and anchoring on the first event's *oldest* sample
// rather than on its own time is what keeps that event's batch
// from collapsing onto one instant -- `sense::PointerClock`'s doc
// has both, and owns the arithmetic so it can be unit-tested off a
// device (`sense_tests.rs`). See `CursorState::time`.
let event_time = event.event_time_nanos(&mut ctx.env);
let (anchor_at, anchor_nanos) =
*self.input_clock.get_or_insert((Instant::now(), event_time));
let at = |sample_time: jlong| {
anchor_at + Duration::from_nanos(sample_time.saturating_sub(anchor_nanos).max(0) as u64)
};
if self.input_clock.is_none() {
let history = event.history_size(&mut ctx.env);
let oldest = if history > 0 {
event.historical_event_time_nanos(&mut ctx.env, 0)
} else {
event_time
};
self.input_clock = Some(PointerClock::anchored(Instant::now(), event_time, oldest));
}
let mut clock = self.input_clock.expect("anchored just above");
// `iris::input`'s own doc (`sense::log_input_event`): collected
// only when tracing is on, since this is otherwise a `Vec` per
// `MotionEvent` for a line nobody is reading -- the JNI reads
@@ -658,7 +662,6 @@ impl<State: AndroidAppState> ViewPeer for IrisViewPeer<State> {
// function does regardless of tracing.
let trace_input = crate::diagnostics::trace_enabled();
let mut historical_ms: Vec<(u64, f32, f32)> = Vec::new();
let ms_since_anchor = |t: jlong| (t.saturating_sub(anchor_nanos).max(0) as u64) / 1_000_000;
// **Historical samples first.** A flick on a 120Hz screen is
// delivered as one or two `MotionEvent`s with the intermediate
@@ -678,34 +681,30 @@ impl<State: AndroidAppState> ViewPeer for IrisViewPeer<State> {
// the event's own sample as the newest of the batch; everything
// downstream (`VelocityTracker`, `DragArbiter`'s long-press
// clock) assumes it, so say so here rather than at each reader.
let mut previous = anchor_nanos;
// `PointerClock::sample` is what asserts it, and it carries the
// last sample seen *across* events, so the first sample of
// every event is checked against the previous event's last one
// rather than against the anchor.
for pos in 0..history {
let hx = event.historical_axis(&mut ctx.env, Axis::X, 0, pos);
let hy = event.historical_axis(&mut ctx.env, Axis::Y, 0, pos);
let ht = event.historical_event_time_nanos(&mut ctx.env, pos);
debug_assert!(
ht >= previous,
"historical sample {pos} of {history} is dated {ht}ns, before the {previous}ns \
sample ahead of it -- the input clock is not what this assumes"
);
previous = ht;
let sample_at = clock.sample(ht);
if trace_input {
historical_ms.push((ms_since_anchor(ht), hx, hy));
historical_ms.push((clock.ms_since_anchor(ht), hx, hy));
}
let ui_state = self.state.android_state_mut();
ui_state.cursor.pos = vec2(hx, hy);
ui_state.cursor.time = at(ht);
ui_state.cursor.time = sample_at;
self.run_input_frame(ctx);
}
debug_assert!(
event_time >= previous,
"the event's own sample is dated {event_time}ns, before its last historical \
sample at {previous}ns"
);
}
let event_at = clock.sample(event_time);
let event_ms = clock.ms_since_anchor(event_time);
self.input_clock = Some(clock);
let ui_state = self.state.android_state_mut();
ui_state.cursor.time = at(event_time);
ui_state.cursor.time = event_at;
match action {
MotionAction::Down => {
ui_state.cursor.pos = vec2(x, y);
@@ -736,13 +735,7 @@ impl<State: AndroidAppState> ViewPeer for IrisViewPeer<State> {
MotionAction::Cancel => "cancel",
_ => "other",
};
crate::sense::log_input_event(
action_word,
x,
y,
ms_since_anchor(event_time),
&historical_ms,
);
crate::sense::log_input_event(action_word, x, y, event_ms, &historical_ms);
}
self.after_input(ctx);
true
+72
View File
@@ -505,6 +505,78 @@ pub fn log_input_event(action: &str, x: f32, y: f32, t_ms: u64, historical: &[(u
);
}
/// Converts a platform's own monotonic input timestamps into [`Instant`]s
/// through **one** anchor taken at the first event, so that every sample
/// this process ever sees is dated on a single ruler.
///
/// A fresh `Instant::now()` per event, minus each sample's age inside it,
/// can date a later event's first sample before the previous event's last
/// one whenever delivery jitters by more than the batch spans -- which
/// [`VelocityTracker`] would rightly reject.
///
/// The anchor is taken from the **earliest sample of the first event**,
/// not from that event's own time: an event batches samples that are by
/// definition older than itself, and anchoring on the event's own time
/// leaves them before the anchor, where they clamp onto one instant. Three
/// samples sharing a timestamp make the Lsq2 fit degenerate, so the flick
/// that produced them reads 0 px/s -- reachable whenever the first event a
/// view sees is a `Move` (the `Down` went to another view, or the view was
/// attached mid-gesture). Found by review, 2026-09-07 (docs/REVIEW-2026-09-07.md's D4).
#[derive(Clone, Copy)]
pub struct PointerClock {
anchor_at: Instant,
anchor_nanos: i64,
last_nanos: i64,
}
impl PointerClock {
/// `now` is when the first event arrived, `event_time` its own
/// timestamp, and `oldest` the timestamp of the earliest sample it
/// carries -- equal to `event_time` when it batches none.
pub fn anchored(now: Instant, event_time: i64, oldest: i64) -> Self {
let batch_span = Duration::from_nanos(event_time.saturating_sub(oldest).max(0) as u64);
Self {
// `checked_sub` rather than `-`: an `Instant` taken very early
// in a process's life has nothing to subtract from, and the
// saturating answer (everything in the first batch at `now`)
// is the old behaviour rather than a panic.
anchor_at: now.checked_sub(batch_span).unwrap_or(now),
anchor_nanos: oldest,
last_nanos: oldest,
}
}
/// Dates one sample, in the order the platform delivers them.
///
/// The `debug_assert!` is deliberately not an `assert!`: this runs once
/// per touch sample, which on a 120Hz screen with batching is hundreds
/// a second, and a mis-ordered sample degrades a velocity rather than
/// drawing something wrong (CODE_RULES' hot-loop exception).
pub fn sample(&mut self, nanos: i64) -> Instant {
debug_assert!(
nanos >= self.last_nanos,
"input sample is dated {nanos}ns, before the {}ns sample ahead of it -- the input \
clock is not what this assumes",
self.last_nanos,
);
self.last_nanos = self.last_nanos.max(nanos);
self.at(nanos)
}
/// Dates a sample without treating it as the newest one seen -- for
/// reads that are out of band, such as looking at a batch before
/// replaying it.
pub fn at(&self, nanos: i64) -> Instant {
self.anchor_at + Duration::from_nanos(nanos.saturating_sub(self.anchor_nanos).max(0) as u64)
}
/// Milliseconds since the anchor, which is the column an `iris::input`
/// line and a `.touch` file both carry (see [`log_input_event`]).
pub fn ms_since_anchor(&self, nanos: i64) -> u64 {
(nanos.saturating_sub(self.anchor_nanos).max(0) as u64) / 1_000_000
}
}
/// How long a stationary press has to be held before it is treated as a
/// long-press rather than the start of a pan.
pub const LONG_PRESS: Duration = Duration::from_millis(500);
+48 -1
View File
@@ -7,7 +7,7 @@
//! impl need no GPU or window.
use crate::prelude::*;
use std::{cell::Cell, rc::Rc};
use std::{cell::Cell, rc::Rc, time::Instant};
struct SenseRsc {
ui: UiData,
@@ -317,3 +317,50 @@ fn a_finger_drag_over_a_scroll_area_pans_it() {
// widget even once the finger leaves its box.
assert_eq!(render.captured_pointer(), Some(scroll.id()));
}
/// docs/REVIEW-2026-09-07.md's D4. The first `MotionEvent` a view sees can
/// be a `Move` -- the `Down` went to another view, or the view was attached
/// mid-gesture -- and its batched samples are older than its own
/// timestamp. Anchoring on that timestamp clamped every one of them onto
/// the anchor, so the tracker saw three samples at one instant, the Lsq2
/// fit went degenerate, and the flick read 0 px/s.
#[test]
fn the_first_events_batched_samples_are_dated_apart() {
const MS: i64 = 1_000_000;
let now = Instant::now();
// A 120Hz batch: three historical samples at 0/4/8ms and the event's
// own at 12ms.
let clock = PointerClock::anchored(now, 12 * MS, 0);
assert_eq!(
clock.at(12 * MS),
now,
"the event's own sample is the one that arrived now"
);
let batch = [clock.at(0), clock.at(4 * MS), clock.at(8 * MS)];
assert!(
batch[0] < batch[1] && batch[1] < batch[2] && batch[2] < now,
"the batch must keep the 4ms between its samples, got {:?}",
batch
.iter()
.map(|t| now.duration_since(*t))
.collect::<Vec<_>>()
);
assert_eq!(clock.ms_since_anchor(8 * MS), 8);
}
/// The same clock has to keep ordering *across* events: the sample it
/// compares a new event's first sample against is the previous event's
/// last one, never the anchor.
#[test]
fn the_clock_orders_samples_across_events() {
const MS: i64 = 1_000_000;
let mut clock = PointerClock::anchored(Instant::now(), 12 * MS, 0);
let first = clock.sample(12 * MS);
let second = clock.sample(28 * MS);
assert!(second > first);
assert_eq!(
second.duration_since(first),
std::time::Duration::from_millis(16)
);
}