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 <noreply@anthropic.com>
This commit is contained in:
1 parent
551c01398f
commit
e10582a2cd
3 files changed
+145
-24
No files matched your search
@@ -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}: "
|
||||
|
||||
@@ -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"
|
||||
);
|
||||
}
|
||||
|
||||
|
||||
@@ -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<MaskIdx> {
|
||||
let Some(active) = h.render.active.get(&id) else {
|
||||
return Vec::new();
|
||||
};
|
||||
let mut out: Vec<MaskIdx> = 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<MaskIdx> {
|
||||
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);
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
Reference in new issue
Block a user