From 94d8373289f07983eaf8021d90777f1f89a2cfd3 Mon Sep 17 00:00:00 2001 From: iris <2+iris@noreply.localhost> Date: Tue, 8 Sep 2026 13:48:17 -0400 Subject: [PATCH] iris-android-app: the bench observes the fling instead of driving it at 60Hz The fling phase called List::tick_fling itself every 16ms, so on a 120Hz phone every second frame redrew a position already drawn -- Iris saw the benchmark scroll visibly less smoothly than her own finger, and it was the rig rather than the renderer. A real fling is advanced once per frame by UiData::tick_animations from the frame callback, so the phase now starts one the way a gesture does (fling + animate) and polls is_scrolling to know when it settled. ANIM_STEP_MS becomes POLL_MS, which is what it always was here. Co-Authored-By: Claude Opus 5 --- docs/RUST.md | 29 +++++++++---- iris/android-app/src/bench_client.rs | 63 ++++++++++++++++++++-------- 2 files changed, 67 insertions(+), 25 deletions(-) diff --git a/docs/RUST.md b/docs/RUST.md index b5d3514..03106a1 100644 --- a/docs/RUST.md +++ b/docs/RUST.md @@ -7994,10 +7994,25 @@ scroll really is coarser than hers, the difference she sees is the rig rather than the renderer, and every frame number the bench has produced was measured against a gesture no hand would make. -Fix: drive one step per **real frame**, from the frame callback the -renderer already runs on, and derive each step's delta from the elapsed -time it reports rather than from a constant. Pass condition: the bench's -sample rate matches the display's refresh (say so in the report, next to -the adapter line, so a number is never read without it), and the report -states the rate it drove at so an old report cannot be compared to a new -one by accident. +**Fixed, 2026-09-08.** The bench does not synthesise touch samples at +all -- it calls `List::fling(velocity)` and then *drove the fling itself*, +calling `List::tick_fling` from an async loop every 16ms. A real fling is +advanced once per frame by `UiData::tick_animations`, from the frame +callback, which on her phone is 120Hz; so every second frame redrew the +list at a position it had already drawn. The rig, not the renderer. + +`wait_for_fling_settle` now **observes** instead: the phase starts the +fling the way a finger's release does (`List::fling` **plus** +`UiData::animate`, the two halves `List::fling`'s own doc says have +different owners) and polls `is_scrolling()` to know when it is over. So +the phase measures the same path a gesture takes, at the display's own +rate. `ANIM_STEP_MS` is renamed `POLL_MS` and its doc says it is how +often a question is asked, not a cadence anything moves at -- the name +was half the reason it was used for both. The fling phase's line in the +report gains `ticked=frame-loop`, so a report from before this cannot be +compared with one after it by accident; the `frames:` block already +states the refresh it ran at. + +**Not yet confirmed on the phone** -- that needs a build in her hands, +and the emulator cannot answer it (it is a 60Hz GLES rig, so the defect +is invisible there by construction). diff --git a/iris/android-app/src/bench_client.rs b/iris/android-app/src/bench_client.rs index 78ac2c9..9bb2127 100644 --- a/iris/android-app/src/bench_client.rs +++ b/iris/android-app/src/bench_client.rs @@ -72,13 +72,15 @@ const TYPE_CHAR_MS: u64 = 50; const KEYBOARD_CYCLES: usize = 5; const KEYBOARD_WAIT_MS: u64 = 1_000; -/// One animation step's target cadence -- close enough to 60Hz that a -/// fling/scroll is many small moves rather than one jump, so frames are -/// actually rendered along the way, and close enough that a `ctx.update` -/// closure's effect (only applied once the next frame callback drains the -/// task channel -- `IrisViewPeer::drain_tasks`) is visible again quickly -/// when a later step in the same phase needs to read state back. -const ANIM_STEP_MS: u64 = 16; +/// How often this file *asks a question of* the running app -- polls for +/// a `ctx.update` closure's answer, or for a fling to have settled. +/// +/// It is not an animation cadence and nothing on screen moves at this +/// rate: the frame loop advances animations once per frame at the +/// display's own refresh (`UiData::tick_animations`). It used to be both, +/// and that is the defect Iris reported on 2026-09-08 -- see +/// `wait_for_fling_settle`. +const POLL_MS: u64 = 16; /// How much of the screen a *filled* benchmark report may take before it /// scrolls instead of growing -- roughly a third of a phone screen, the @@ -880,7 +882,7 @@ impl BenchClient { /// drained everything queued before this call existed. Cost a real hang /// in this file's first version of the fling phase: every loop iteration /// after the first sat forever with nothing scheduled to drain it. -/// Polls rather than assuming one `ANIM_STEP_MS` sleep is enough, since a +/// Polls rather than assuming one `POLL_MS` sleep is enough, since a /// slow device's frame callback can lag further than that. async fn read_from_state( ctx: &mut iris::task::TaskCtx, @@ -900,7 +902,7 @@ where if let Ok(value) = rx.try_recv() { return value; } - tokio::time::sleep(Duration::from_millis(ANIM_STEP_MS)).await; + tokio::time::sleep(Duration::from_millis(POLL_MS)).await; } } @@ -928,13 +930,14 @@ async fn run_fling_phase( redraw.request_redraw(); // Lets the next frame's `repair_anchor` resolve `jump_to_end`'s // `anchor = None` into a real slot before `start` is read. - tokio::time::sleep(Duration::from_millis(ANIM_STEP_MS * 2)).await; + tokio::time::sleep(Duration::from_millis(POLL_MS * 2)).await; let start = read_anchor_position(ctx, redraw).await; for _ in 0..FLING_COUNT { ctx.update(|state: &mut BenchClient, rsc| { if let Some(screen) = &state.screen { (screen.list)(rsc).fling(-FLING_VELOCITY_PX_S); + animate_list(screen.list, rsc); } }); redraw.request_redraw(); @@ -947,6 +950,7 @@ async fn run_fling_phase( ctx.update(|state: &mut BenchClient, rsc| { if let Some(screen) = &state.screen { (screen.list)(rsc).fling(FLING_VELOCITY_PX_S); + animate_list(screen.list, rsc); } }); redraw.request_redraw(); @@ -955,7 +959,10 @@ async fn run_fling_phase( } let end = read_anchor_position(ctx, redraw).await; - format!("start={start} outward={outward} end={end}") + // Says how the fling was advanced, because that is what changed on + // 2026-09-08 and a report from before then is not comparable: the + // phase used to tick the fling itself at ~60Hz. + format!("start={start} outward={outward} end={end} ticked=frame-loop") } async fn read_anchor_position( @@ -969,11 +976,31 @@ async fn read_anchor_position( .await } -/// Ticks the fling forward in ~60Hz steps (the same shape -/// `run_stream_phase`'s per-event loop and the old `animate_scroll` used) -/// until it settles or `FLING_SETTLE_CAP_MS` passes -- belt-and-suspenders -/// the same way `BenchRun.kt`'s own `waitForSettle` is, since a fling's -/// own spline-decided `duration()` already caps how long it can run. +/// Register the list with the frame loop, exactly as a finger's own +/// release does (`transcript_ui::Selection::drag`'s `Released` arm) -- +/// `List::fling` sets a velocity and drives nothing by itself. +fn animate_list(list: iris::prelude::WeakWidget, rsc: &mut Rsc) { + let id = list.id(); + rsc.ui_mut().animate(id); +} + +/// Waits for the fling started above to settle, or for +/// `FLING_SETTLE_CAP_MS` -- belt-and-suspenders the same way +/// `BenchRun.kt`'s own `waitForSettle` is, since a fling's own +/// spline-decided `duration()` already caps how long it can run. +/// +/// **It observes; it does not drive.** Until 2026-09-08 this loop called +/// `List::tick_fling` itself every `POLL_MS`, which advanced the +/// fling in 16ms steps -- so on Iris's 120Hz phone every second frame +/// redrew the list at a position it had already drawn, and the benchmark +/// looked distinctly less smooth than the same list under her finger. +/// That is what she reported that day, and it was the rig rather than the +/// renderer: a real fling is ticked once per frame by +/// `UiData::tick_animations`, from the frame callback. So the bench now +/// starts the fling the way a gesture does (`fling` + `UiData::animate`) +/// and polls `is_scrolling` to know when it is over, which makes the +/// phase measure the same path a finger takes. The poll interval is only +/// how often the *question* is asked and has no bearing on the animation. async fn wait_for_fling_settle( ctx: &mut iris::task::TaskCtx, redraw: &Arc, @@ -982,14 +1009,14 @@ async fn wait_for_fling_settle( let started = Instant::now(); while started.elapsed() < cap { let still_scrolling = read_from_state(ctx, redraw, |state, rsc| match &state.screen { - Some(screen) => (screen.list)(rsc).tick_fling(Instant::now()), + Some(screen) => (screen.list)(rsc).is_scrolling(), None => false, }) .await; if !still_scrolling { return; } - tokio::time::sleep(Duration::from_millis(ANIM_STEP_MS)).await; + tokio::time::sleep(Duration::from_millis(POLL_MS)).await; } }