iris: the guards against silently wrong output survive into release
docs/REVIEW-2026-09-07.md's R1. Every invariant guard added on 2026-09-07
was a `debug_assert!`, and every build anybody runs on this project is
release -- the bench APK must be (the debug `libmain.so` is 325 MB and
will not install) and Iris's phone gets release too. So a `List` drawn
without a mask painted over its surroundings again, in exactly the build
the fault was found in, with nothing saying so.
Promoted to `assert!`, each O(1) or a handful per *draw* and each
protecting against output that is wrong on screen with no other symptom:
`List::draw`'s `painter.is_masked()`, `List`'s `extents`-are-on-screen
check, `Painter::set_mask`'s doubled-call check (the second call replaces
rather than nests, i.e. an unclipped widget), `Painter::glyphs`'s atlas
generation (glyphs sampled from coordinates now holding other letters),
and `List::fling`'s finiteness (one comparison per gesture; NaN
propagates into `deceleration_for`'s `ln()` and the fling never settles).
Left as `debug_assert!` and now saying so in a comment: `List::place`'s
slot-exists precondition (once per row placed per frame, and its release
failure is the `.expect` below rather than something wrong on screen) and
`poly_fit_least_squares`'s two preconditions (run on every velocity query,
with `MIN_SAMPLE_SIZE` and the `is_finite` check giving release a defined
outcome either way). `PointerClock::sample`'s ordering assert was already
annotated in 2ec0fee for the same reason.
Verified: `cargo test --lib -p iris` (103) and `cargo test -p
transcript-fixture` (12) pass in both debug *and* `--release`, which is
what says the promoted asserts do not fire on a real replayed flick;
fmt, clippy and `cargo ndk check -p iris` clean.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
This commit is contained in:
1 parent
7e79ec11e0
commit
551c01398f
3 files changed
+35
-8
No files matched your search
@@ -65,7 +65,12 @@ impl<'a> Painter<'a> {
|
||||
/// so keeps pointing at whichever slot it was drawn under. See
|
||||
/// `ActiveData::own_mask` for what pushing a fresh one cost.
|
||||
pub fn set_mask(&mut self, region: UiRegion) {
|
||||
debug_assert!(
|
||||
// `assert!`, not `debug_assert!`: one comparison per widget draw,
|
||||
// and the second call silently *replacing* the first is a widget
|
||||
// drawn unclipped -- which reaches the screen and nothing says so.
|
||||
// Every build anybody runs here is release
|
||||
// (docs/REVIEW-2026-09-07.md's R1).
|
||||
assert!(
|
||||
self.own_mask == MaskIdx::NONE || self.mask != self.own_mask,
|
||||
"set_mask called twice while drawing one widget: the second would replace the first \
|
||||
rather than nest inside it",
|
||||
@@ -264,8 +269,10 @@ impl<'a> Painter<'a> {
|
||||
// A caller re-emitting quads placed against an atlas that has since
|
||||
// been cleared draws every glyph from coordinates now holding
|
||||
// something else. Caught at the submission rather than on screen,
|
||||
// where it reads as fragments of unrelated letters.
|
||||
debug_assert_eq!(
|
||||
// where it reads as fragments of unrelated letters. `assert_eq!`
|
||||
// for R1's reason: two integers per laid-out string, not per
|
||||
// glyph, and the failure is unreadable text on a release build.
|
||||
assert_eq!(
|
||||
text.generation,
|
||||
self.atlas_generation(),
|
||||
"glyphs placed against atlas generation {} submitted against {}: the holder did not \
|
||||
|
||||
@@ -1221,6 +1221,11 @@ impl VelocityTracker {
|
||||
/// points than coefficients; [`MIN_SAMPLE_SIZE`] makes that unreachable
|
||||
/// from the only caller, so the truncation is an assert instead of a
|
||||
/// branch that could never be exercised.
|
||||
// Both guards stay `debug_assert!` under docs/REVIEW-2026-09-07.md's R1:
|
||||
// they are preconditions of a fit run on every velocity query, and the two
|
||||
// callers between them already answer 0 below `MIN_SAMPLE_SIZE` and check
|
||||
// the result with `is_finite`, so a release build has a defined outcome
|
||||
// rather than a silently wrong one.
|
||||
fn poly_fit_least_squares(x: &[f32], y: &[f32]) -> [f32; FIT_COEFFICIENTS] {
|
||||
debug_assert_eq!(x.len(), y.len());
|
||||
debug_assert!(
|
||||
|
||||
+20
-5
@@ -487,8 +487,12 @@ impl List {
|
||||
// through) would propagate silently into `deceleration_for`'s
|
||||
// `.ln()` -- the fling either never settles or jumps to NaN
|
||||
// positions with nothing on screen saying why (docs/
|
||||
// REVIEW-2026-09-06.md finding 3).
|
||||
debug_assert!(velocity_px_per_s.is_finite());
|
||||
// REVIEW-2026-09-06.md finding 3). A plain `assert!` rather than a
|
||||
// `debug_assert!`: it is one comparison per *gesture*, and every
|
||||
// build anybody runs -- the emulator's and Iris's phone's -- is
|
||||
// release, where a debug-only guard against silently wrong output
|
||||
// is no guard at all (docs/REVIEW-2026-09-07.md's R1).
|
||||
assert!(velocity_px_per_s.is_finite());
|
||||
// Compose's two thresholds at a release, and **only** those two.
|
||||
//
|
||||
// The maximum is `ViewConfiguration.getScaledMaximumFlingVelocity()`
|
||||
@@ -1011,7 +1015,10 @@ impl List {
|
||||
// sites, not by this function, which would otherwise fail with a
|
||||
// bare "index out of bounds" and no context (docs/
|
||||
// REVIEW-2026-09-06.md finding 2). `slot_widget`, called from
|
||||
// here, is what actually indexes/`.expect`s on it.
|
||||
// here, is what actually indexes/`.expect`s on it. Stays a
|
||||
// `debug_assert!` under R1's rule: this runs once per row placed
|
||||
// per frame, and its release failure is the `.expect` below rather
|
||||
// than something silently wrong on screen.
|
||||
debug_assert!(
|
||||
self.slot_exists(slot),
|
||||
"place() called with a slot that doesn't exist: {slot:?}"
|
||||
@@ -1161,7 +1168,12 @@ impl Widget for List {
|
||||
// cannot set the mask itself, since `Painter::set_mask` allows one
|
||||
// mask per widget and rows of this list already use their own
|
||||
// (`transcript-ui`'s `row.rs`, `tool.rs`). So it checks instead.
|
||||
debug_assert!(
|
||||
//
|
||||
// `assert!`, not `debug_assert!`: one bool per draw, and what it
|
||||
// catches is a `List` painting over its surroundings with nothing
|
||||
// on screen saying so -- the fault e922b73 was written to fix.
|
||||
// Every build that runs is release (docs/REVIEW-2026-09-07.md's R1).
|
||||
assert!(
|
||||
painter.is_masked(),
|
||||
"a `List` must be drawn inside something `.masked()`: it draws rows straddling both \
|
||||
edges in full, so the parts outside its own box reach the screen otherwise",
|
||||
@@ -1223,7 +1235,10 @@ impl Widget for List {
|
||||
// what overlaps the viewport, and nothing above or below it can
|
||||
// be seen. The first failed silently for a whole build -- an
|
||||
// off-screen row draws correctly, it is just in the wrong place.
|
||||
debug_assert!(
|
||||
// `assert!` for R1's reason: it walks the rows *on screen*, a
|
||||
// handful, once per draw, and a release build is the only build
|
||||
// this fault has ever been seen in.
|
||||
assert!(
|
||||
self.extents
|
||||
.values()
|
||||
.all(|e| self.intersects_viewport(e.top, e.bottom)),
|
||||
|
||||
Reference in new issue
Block a user