From e10582a2cd3df4d976f7ee86f3c2787a56a752ca Mon Sep 17 00:00:00 2001 From: iris <2+iris@noreply.localhost> Date: Mon, 7 Sep 2026 20:58:35 -0400 Subject: [PATCH] iris: three layer-1 tests that could not fail in the direction the bug goes docs/REVIEW-2026-09-07.md's T1, T2 and T3. Each was confirmed by breaking its subject on purpose and watching the new assertion fire, and each of those breaks is recorded beside the assertion. **T1** (`phone_screen.rs`) bounded the fling's duration with `FlingCalculator::new(PHONE_SCALE).duration(velocity)` -- the calculator under test -- and only from above, so it could fail when a fling ran too long and never when one stopped dead, which is the symptom Iris actually reported. The companion `assert_ne!(before, after)` passes on one pixel of travel. It now takes both bounds from `fling_spline_reference.py`, which gains this case's own line (`density=2.55 v=15250.0: distance=11057.424px duration=2.0716s`), and measures travel in pixels from a row's own on-screen extent -- 10527px against the reference's 11057, the 5% shortfall being the frames a tracked row leaves the screen on. Scaling `tick_fling`'s elapsed by 1000 reports "stopped after 8ms"; scaling its delta by 0.01 reports "travelled 111px". **T2** (`top_edge.rs`) asserted the per-row box only on the return leg, so a regression that drew rows in the wrong place while travelling *backwards* was checked by the row count alone. The first leg still cannot assert it (an unmeasured row has to be drawn to be measured), so there is now a third leg -- back again, every height known. Widening `intersects_viewport` downwards passes all 40 forward steps and fails at "back 6", which is the leg that did not exist. **T3** (`top_edge.rs`) asserted a mask exists and sits inside the list's box, never that any row primitive references it, so a broken `Mask::parent` chain -- what d507ae4 introduced -- left it green while a code fence drew unclipped. It now walks every row primitive's chain and requires the list's own mask slot on it (and rejects a chain that loops). Forcing `Painter::set_mask`'s `parent` to `NONE` fails it with "clips to [Id(1)], a chain that never reaches the list's own mask Id(0)". Verified: `cargo test -p transcript-fixture` (12) and `cargo test --lib -p iris` (103) pass, fmt and clippy clean. Co-Authored-By: Claude Fable 5.1 --- iris/benches/fling_spline_reference.py | 6 +- iris/transcript-fixture/tests/phone_screen.rs | 58 ++++++++-- iris/transcript-fixture/tests/top_edge.rs | 105 +++++++++++++++--- 3 files changed, 145 insertions(+), 24 deletions(-) diff --git a/iris/benches/fling_spline_reference.py b/iris/benches/fling_spline_reference.py index a62508b..3a9af55 100644 --- a/iris/benches/fling_spline_reference.py +++ b/iris/benches/fling_spline_reference.py @@ -130,7 +130,11 @@ if __name__ == "__main__": # 2.55 is Iris's Pixel 9 Pro XL (docs/bench/iris-phone-v2-2026-09-06.md); # 2.75 is this checkout's emulator. for density in (2.55, 2.75): - for velocity in (5000.0, 11064.0): + # 15250 is `transcript-fixture/touch/flick-120hz.touch`'s own + # release velocity (velocity_reference.py), so `phone_screen.rs` + # can bound the fling it produces from *here* rather than from the + # `FlingCalculator` under test (docs/REVIEW-2026-09-07.md's T1). + for velocity in (5000.0, 11064.0, 15250.0): dur = fling_duration_s(velocity, density) print( f"density={density} v={velocity}: " diff --git a/iris/transcript-fixture/tests/phone_screen.rs b/iris/transcript-fixture/tests/phone_screen.rs index 2c354c9..872d373 100644 --- a/iris/transcript-fixture/tests/phone_screen.rs +++ b/iris/transcript-fixture/tests/phone_screen.rs @@ -58,16 +58,42 @@ fn a_recorded_flick_releases_with_a_velocity_and_flings_the_list() { "expected ~-15250px/s from velocity_reference.py, got {velocity}" ); - // Android's own spline says how long a fling at this speed runs. The - // list learns its density from the painter, so this is the same - // curve it is using. - let expected = FlingCalculator::new(PHONE_SCALE).duration(velocity); - let end = flick.end_ms() + expected.as_millis() as u64 * 2; + // `iris/benches/fling_spline_reference.py`'s own line for this exact + // case -- `density=2.55 v=15250.0: distance=11057.424px + // duration=2.0716s`. **Not** `FlingCalculator::new(PHONE_SCALE)`, + // which is the calculator under test: bounding a fling with the thing + // being measured is the "compared the code with itself" shape 73f956f + // found in the spline's own tests, and it left this one able to fail + // in the "ran too long" direction only -- never in the "stopped dead" + // direction, which is what Iris actually reported + // (docs/REVIEW-2026-09-07.md's T1). + const REFERENCE_MS: u64 = 2071; + const REFERENCE_PX: f32 = 11057.0; + let end = flick.end_ms() + REFERENCE_MS * 2; let mut settled_at = None; let mut t = flick.end_ms(); + // Travel in pixels, measured from a row's own on-screen extent, since + // `List` has no travel accessor and this needs none: follow whatever + // row is under the viewport's middle until it leaves, then pick + // another. Deliberately an *under*-count -- the frame a row leaves on + // contributes nothing -- which is why it is only ever a lower bound. + let middle = phone_size().y / 2.0; + let mut travelled = 0.0f32; + let mut tracked: Option<(RowKey, f32)> = None; while t <= end { h.frame(t); - if settled_at.is_none() && !(screen.list)(&mut h.rsc).is_scrolling() { + let list = (screen.list)(&mut h.rsc); + tracked = + match tracked.and_then(|(key, was)| list.extent(key).map(|(now, _)| (key, was, now))) { + Some((key, was, now)) => { + travelled += (now - was).abs(); + Some((key, now)) + } + None => list + .key_at(middle) + .and_then(|key| list.extent(key).map(|(top, _)| (key, top))), + }; + if settled_at.is_none() && !list.is_scrolling() { settled_at = Some(t); } t += PHONE_FRAME_MS; @@ -80,10 +106,24 @@ fn a_recorded_flick_releases_with_a_velocity_and_flings_the_list() { ); let settled_at = settled_at.expect("the fling must stop on its own, not run forever"); let ran_for = settled_at - flick.end_ms(); + // Both directions. The lower bound is the one that fails when a fling + // settles on its first tick; the upper is the one that was here. assert!( - ran_for <= expected.as_millis() as u64 + PHONE_FRAME_MS * 2, - "the fling ran {ran_for}ms against the spline's own {}ms", - expected.as_millis() + ran_for >= REFERENCE_MS - PHONE_FRAME_MS * 2, + "the fling stopped after {ran_for}ms against the spline reference's {REFERENCE_MS}ms" + ); + assert!( + ran_for <= REFERENCE_MS + PHONE_FRAME_MS * 2, + "the fling ran {ran_for}ms against the spline reference's {REFERENCE_MS}ms" + ); + // 80% of the reference, against 10527px measured today -- the 5% + // shortfall is the frames a tracked row leaves the screen on. A fling + // that moves one row's worth fails this; scaling `tick_fling`'s delta + // by 0.01 reports 111px, which is how it was confirmed to fail in the + // direction the bug goes. + assert!( + travelled >= REFERENCE_PX * 0.8, + "the fling travelled {travelled:.0}px against the spline reference's {REFERENCE_PX:.0}px" ); } diff --git a/iris/transcript-fixture/tests/top_edge.rs b/iris/transcript-fixture/tests/top_edge.rs index 90d0202..f0589c7 100644 --- a/iris/transcript-fixture/tests/top_edge.rs +++ b/iris/transcript-fixture/tests/top_edge.rs @@ -123,6 +123,70 @@ fn the_list_is_clipped_to_its_own_box() { "the clip {clip:?} reaches outside the list's own box {list:?}, so a row straddling an \ edge still draws past it", ); + + // And the mask has to *reach* what the rows draw. The two above say a + // mask exists and sits in the right place; neither says any primitive + // references it, so a broken `Mask::parent` chain -- what d507ae4 + // introduced -- would leave them green while a code fence inside a row + // drew unclipped again (docs/REVIEW-2026-09-07.md's T3). + let rows = h + .render + .active + .get(&screen.list.id()) + .expect("the list is drawn") + .children + .clone(); + let mut checked = 0; + for row in rows { + for prim in primitives_under(&h, row) { + assert!( + mask_chain(&h, prim).contains(&active.mask), + "a primitive of row {row:?} clips to {:?}, a chain that never reaches the list's \ + own mask {:?}", + mask_chain(&h, prim), + active.mask, + ); + checked += 1; + } + } + assert!( + checked > 0, + "no row primitive was checked, so this test asserted nothing", + ); +} + +/// Every primitive `id` and its descendants drew, as `MaskIdx`es -- images +/// excluded, since they live in a separate instance array with their own +/// indices (`Primitives::free`). +fn primitives_under(h: &Harness, id: WidgetId) -> Vec { + let Some(active) = h.render.active.get(&id) else { + return Vec::new(); + }; + let mut out: Vec = active + .primitives + .iter() + .filter(|p| p.binding != IMAGE_BINDING) + .map(|p| h.render.layers[p.layer].instances()[p.inst_idx].mask_idx) + .collect(); + for child in &active.children { + out.extend(primitives_under(h, *child)); + } + out +} + +/// The chain the fragment stage walks from `mask`, outermost last. +fn mask_chain(h: &Harness, mask: MaskIdx) -> Vec { + let mut chain = Vec::new(); + let mut at = mask; + while at != MaskIdx::NONE { + assert!( + !chain.contains(&at), + "the mask chain from {mask:?} loops back to {at:?}", + ); + chain.push(at); + at = h.rsc.ui.masks[at.idx()].parent; + } + chain } /// A row that has left the viewport entirely is not drawn at all. Before @@ -132,12 +196,16 @@ fn the_list_is_clipped_to_its_own_box() { /// a 2012px viewport, ~59 of them off screen and painting over the /// header. /// -/// Asserted strictly on the way *back*, because a row whose height has -/// never been measured has to be drawn to be measured (`List::place`'s -/// doc), which on the outbound leg is every row entering from the top. -/// The return leg crosses the same rows with every height already known, -/// which is also the ordinary state of a transcript being panned around -/// in. The bound on how many rows are placed at once holds on both. +/// The box is asserted on every leg *except the first*, because a row +/// whose height has never been measured has to be drawn to be measured +/// (`List::place`'s doc), which on the first walk back is every row +/// entering from the top. Every later leg crosses the same rows with +/// every height already known -- including the second walk *back*, which +/// is there because a regression that draws rows in the wrong place while +/// travelling backwards would otherwise be checked only by the row count +/// (docs/REVIEW-2026-09-07.md's T2). That is also the ordinary state of a +/// transcript being panned around in. The bound on how many rows are +/// placed at once holds on all three. #[test] fn rows_that_have_left_the_viewport_are_not_drawn() { let (mut h, screen) = opened(); @@ -152,22 +220,31 @@ fn rows_that_have_left_the_viewport_are_not_drawn() { rows.len(), ); }; + let inside = |rows: &[(f32, f32)], leg: &str, step: usize| { + for &(top, bottom) in rows { + assert!( + bottom > list.top_left.y - 0.5 && top < list.bot_right.y + 0.5, + "{leg} {step}: a row at ({top:.1}, {bottom:.1}) is outside the list's box \ + {list:?} and was drawn anyway", + ); + } + }; for step in 0..40 { t = scrolled(&mut h, &screen, -400.0, t); - bounded(&drawn_rows(&h, &screen), "back", step); + bounded(&drawn_rows(&h, &screen), "measuring", step); } for step in 0..40 { t = scrolled(&mut h, &screen, 400.0, t); let rows = drawn_rows(&h, &screen); bounded(&rows, "forward", step); - for &(top, bottom) in &rows { - assert!( - bottom > list.top_left.y - 0.5 && top < list.bot_right.y + 0.5, - "forward {step}: a row at ({top:.1}, {bottom:.1}) is outside the list's box \ - {list:?} and was drawn anyway", - ); - } + inside(&rows, "forward", step); + } + for step in 0..40 { + t = scrolled(&mut h, &screen, -400.0, t); + let rows = drawn_rows(&h, &screen); + bounded(&rows, "back", step); + inside(&rows, "back", step); } }