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 <noreply@anthropic.com>
This commit is contained in:
1 parent
8310431497
commit
94d8373289
2 files changed
+67
-25
No files matched your search
+22
-7
@@ -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
|
rather than the renderer, and every frame number the bench has produced was
|
||||||
measured against a gesture no hand would make.
|
measured against a gesture no hand would make.
|
||||||
|
|
||||||
Fix: drive one step per **real frame**, from the frame callback the
|
**Fixed, 2026-09-08.** The bench does not synthesise touch samples at
|
||||||
renderer already runs on, and derive each step's delta from the elapsed
|
all -- it calls `List::fling(velocity)` and then *drove the fling itself*,
|
||||||
time it reports rather than from a constant. Pass condition: the bench's
|
calling `List::tick_fling` from an async loop every 16ms. A real fling is
|
||||||
sample rate matches the display's refresh (say so in the report, next to
|
advanced once per frame by `UiData::tick_animations`, from the frame
|
||||||
the adapter line, so a number is never read without it), and the report
|
callback, which on her phone is 120Hz; so every second frame redrew the
|
||||||
states the rate it drove at so an old report cannot be compared to a new
|
list at a position it had already drawn. The rig, not the renderer.
|
||||||
one by accident.
|
|
||||||
|
`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).
|
||||||
@@ -72,13 +72,15 @@ const TYPE_CHAR_MS: u64 = 50;
|
|||||||
const KEYBOARD_CYCLES: usize = 5;
|
const KEYBOARD_CYCLES: usize = 5;
|
||||||
const KEYBOARD_WAIT_MS: u64 = 1_000;
|
const KEYBOARD_WAIT_MS: u64 = 1_000;
|
||||||
|
|
||||||
/// One animation step's target cadence -- close enough to 60Hz that a
|
/// How often this file *asks a question of* the running app -- polls for
|
||||||
/// fling/scroll is many small moves rather than one jump, so frames are
|
/// a `ctx.update` closure's answer, or for a fling to have settled.
|
||||||
/// actually rendered along the way, and close enough that a `ctx.update`
|
///
|
||||||
/// closure's effect (only applied once the next frame callback drains the
|
/// It is not an animation cadence and nothing on screen moves at this
|
||||||
/// task channel -- `IrisViewPeer::drain_tasks`) is visible again quickly
|
/// rate: the frame loop advances animations once per frame at the
|
||||||
/// when a later step in the same phase needs to read state back.
|
/// display's own refresh (`UiData::tick_animations`). It used to be both,
|
||||||
const ANIM_STEP_MS: u64 = 16;
|
/// 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
|
/// 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
|
/// 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
|
/// drained everything queued before this call existed. Cost a real hang
|
||||||
/// in this file's first version of the fling phase: every loop iteration
|
/// in this file's first version of the fling phase: every loop iteration
|
||||||
/// after the first sat forever with nothing scheduled to drain it.
|
/// 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.
|
/// slow device's frame callback can lag further than that.
|
||||||
async fn read_from_state<T, F>(
|
async fn read_from_state<T, F>(
|
||||||
ctx: &mut iris::task::TaskCtx<Rsc>,
|
ctx: &mut iris::task::TaskCtx<Rsc>,
|
||||||
@@ -900,7 +902,7 @@ where
|
|||||||
if let Ok(value) = rx.try_recv() {
|
if let Ok(value) = rx.try_recv() {
|
||||||
return value;
|
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();
|
redraw.request_redraw();
|
||||||
// Lets the next frame's `repair_anchor` resolve `jump_to_end`'s
|
// Lets the next frame's `repair_anchor` resolve `jump_to_end`'s
|
||||||
// `anchor = None` into a real slot before `start` is read.
|
// `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;
|
let start = read_anchor_position(ctx, redraw).await;
|
||||||
|
|
||||||
for _ in 0..FLING_COUNT {
|
for _ in 0..FLING_COUNT {
|
||||||
ctx.update(|state: &mut BenchClient, rsc| {
|
ctx.update(|state: &mut BenchClient, rsc| {
|
||||||
if let Some(screen) = &state.screen {
|
if let Some(screen) = &state.screen {
|
||||||
(screen.list)(rsc).fling(-FLING_VELOCITY_PX_S);
|
(screen.list)(rsc).fling(-FLING_VELOCITY_PX_S);
|
||||||
|
animate_list(screen.list, rsc);
|
||||||
}
|
}
|
||||||
});
|
});
|
||||||
redraw.request_redraw();
|
redraw.request_redraw();
|
||||||
@@ -947,6 +950,7 @@ async fn run_fling_phase(
|
|||||||
ctx.update(|state: &mut BenchClient, rsc| {
|
ctx.update(|state: &mut BenchClient, rsc| {
|
||||||
if let Some(screen) = &state.screen {
|
if let Some(screen) = &state.screen {
|
||||||
(screen.list)(rsc).fling(FLING_VELOCITY_PX_S);
|
(screen.list)(rsc).fling(FLING_VELOCITY_PX_S);
|
||||||
|
animate_list(screen.list, rsc);
|
||||||
}
|
}
|
||||||
});
|
});
|
||||||
redraw.request_redraw();
|
redraw.request_redraw();
|
||||||
@@ -955,7 +959,10 @@ async fn run_fling_phase(
|
|||||||
}
|
}
|
||||||
let end = read_anchor_position(ctx, redraw).await;
|
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(
|
async fn read_anchor_position(
|
||||||
@@ -969,11 +976,31 @@ async fn read_anchor_position(
|
|||||||
.await
|
.await
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Ticks the fling forward in ~60Hz steps (the same shape
|
/// Register the list with the frame loop, exactly as a finger's own
|
||||||
/// `run_stream_phase`'s per-event loop and the old `animate_scroll` used)
|
/// release does (`transcript_ui::Selection::drag`'s `Released` arm) --
|
||||||
/// until it settles or `FLING_SETTLE_CAP_MS` passes -- belt-and-suspenders
|
/// `List::fling` sets a velocity and drives nothing by itself.
|
||||||
/// the same way `BenchRun.kt`'s own `waitForSettle` is, since a fling's
|
fn animate_list(list: iris::prelude::WeakWidget<iris::prelude::List>, rsc: &mut Rsc) {
|
||||||
/// own spline-decided `duration()` already caps how long it can run.
|
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(
|
async fn wait_for_fling_settle(
|
||||||
ctx: &mut iris::task::TaskCtx<Rsc>,
|
ctx: &mut iris::task::TaskCtx<Rsc>,
|
||||||
redraw: &Arc<dyn RequestRedraw>,
|
redraw: &Arc<dyn RequestRedraw>,
|
||||||
@@ -982,14 +1009,14 @@ async fn wait_for_fling_settle(
|
|||||||
let started = Instant::now();
|
let started = Instant::now();
|
||||||
while started.elapsed() < cap {
|
while started.elapsed() < cap {
|
||||||
let still_scrolling = read_from_state(ctx, redraw, |state, rsc| match &state.screen {
|
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,
|
None => false,
|
||||||
})
|
})
|
||||||
.await;
|
.await;
|
||||||
if !still_scrolling {
|
if !still_scrolling {
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
tokio::time::sleep(Duration::from_millis(ANIM_STEP_MS)).await;
|
tokio::time::sleep(Duration::from_millis(POLL_MS)).await;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
Reference in new issue
Block a user