Record the resize walk and the re-parenting defects, and what verified them
The first item on the plan landed as two commits on #18. The re-parenting half turned out to be two defects rather than the predicted one -- the old parent's child list erasing a subtree it no longer draws, and the settling walk ordering by a stale depth -- and the `depth()` assertion the note expected to catch it catches only the second. `dirty_size_under` is deleted, so the section that argued for it is marked superseded rather than removed: it says what the bottom-up walk is buying.
This commit is contained in:
1 parent
8c16f9d0fd
commit
3084491b9b
1 file changed
+71
-100
+71
-100
@@ -10,7 +10,7 @@ stayed.
|
|||||||
|
|
||||||
Canonical Iris `main` is **`ca2b4b2`** (#17, the headless rig). **#18
|
Canonical Iris `main` is **`ca2b4b2`** (#17, the headless rig). **#18
|
||||||
`split/18-position-chain`** is open in `/home/bob/repos/iris-pr18`, head
|
`split/18-position-chain`** is open in `/home/bob/repos/iris-pr18`, head
|
||||||
**`25e456e`**, pushed. It holds LAYOUT.md §2's position chain, `leftover`,
|
**`e44dea3`**, pushed. It holds LAYOUT.md §2's position chain, `leftover`,
|
||||||
the `Holds` retained-layout contract, region nodes, built-in alignment and
|
the `Holds` retained-layout contract, region nodes, built-in alignment and
|
||||||
size rules, fixed-point layout, a box in pixels threaded down the draw, and
|
size rules, fixed-point layout, a box in pixels threaded down the draw, and
|
||||||
a report read as a fraction of the containing widget. No PR review was
|
a report read as a fraction of the containing widget. No PR review was
|
||||||
@@ -132,7 +132,13 @@ not exceed one, so `Span`'s `fixed <= 0` branches were only reachable
|
|||||||
through declared fractions; they are ordinary now, and with them boxes of
|
through declared fractions; they are ordinary now, and with them boxes of
|
||||||
negative length passed down to children.
|
negative length passed down to children.
|
||||||
|
|
||||||
### An answer is not an answer while anything under it is dirty (`0e0d4af`)
|
### An answer is not an answer while anything under it is dirty (`0e0d4af`, superseded by `a0693ac`)
|
||||||
|
|
||||||
|
**Superseded: `dirty_size_under` is deleted.** Once a resize goes through
|
||||||
|
the settling walk the ordering makes the whole category unreachable rather
|
||||||
|
than checked, which is Bryan's steer and the better answer. The defect and
|
||||||
|
its reasoning are kept below because they say what the ordering is buying.
|
||||||
|
|
||||||
|
|
||||||
`draw_inner` took an answer from `try_reuse`, which checks only whether the
|
`draw_inner` took an answer from `try_reuse`, which checks only whether the
|
||||||
widget itself is marked, where `retained_answer` beside it also refused one
|
widget itself is marked, where `retained_answer` beside it also refused one
|
||||||
@@ -144,17 +150,13 @@ its parent's own draw never goes through it. The placing ask redraws the
|
|||||||
subtree, the descendant's mark is cleared there, and the parent keeps a
|
subtree, the descendant's mark is cleared there, and the parent keeps a
|
||||||
number the tree no longer agrees with.
|
number the tree no longer agrees with.
|
||||||
|
|
||||||
So `dirty_size_under` is not the optimization its comment claimed. It is
|
So `dirty_size_under` was not the optimization its comment claimed: it was
|
||||||
what makes an answer an answer, and it is asked once in `draw_inner` for
|
what made an answer an answer, until the walk made the state it guarded
|
||||||
both retained routes. Twenty-five rig counters are unchanged on `cold`,
|
against impossible to be in.
|
||||||
`repaint`, `scroll`, `resize` and `size`; `many` makes 18 fewer reuse
|
|
||||||
attempts, 17 of which already reported "dirty".
|
|
||||||
|
|
||||||
Found at seed 564, depth 6, `shuffle-every-other`, reachable only once a
|
Found at seed 564, depth 6, `shuffle-every-other`, reachable only once a
|
||||||
span could overflow itself. **The fast test for it is still owed**: the
|
span could overflow itself. No hand-built tree ever reproduced it, and the
|
||||||
divergence needs a reader drawing while a size dependency two levels under
|
seeds at depth 4 above fail for some other reason.
|
||||||
it is dirty, which no hand-built tree has reproduced yet, and the seeds at
|
|
||||||
depth 4 above fail for some other reason.
|
|
||||||
|
|
||||||
### A text is handed back a box its own line fits in (landed, `4bd8607`)
|
### A text is handed back a box its own line fits in (landed, `4bd8607`)
|
||||||
|
|
||||||
@@ -216,78 +218,51 @@ loop {
|
|||||||
Bryan's, 2026-09-17, and the right answer where `0e0d4af` was a check:
|
Bryan's, 2026-09-17, and the right answer where `0e0d4af` was a check:
|
||||||
"then that entire category of issue can't even occur".
|
"then that entire category of issue can't even occur".
|
||||||
|
|
||||||
**The walk is sound on its own, and the guard is only there for a second
|
**The walk is sound on its own, and the second entry point is closed**
|
||||||
entry point** (read on 2026-09-17, Bryan asking for either a breaking case
|
(`a0693ac`). By induction on depth: when a widget at depth d draws fresh,
|
||||||
or a proof). By induction on depth: when a widget at depth d draws fresh,
|
|
||||||
every dirty widget deeper has been popped, so each is settled or deferred,
|
every dirty widget deeper has been popped, so each is settled or deferred,
|
||||||
and a deferred one has marked its parent. A clean child asked by that draw
|
and a deferred one has marked its parent. A clean child asked by that draw
|
||||||
therefore has a clean subtree, because anything dirty under it would have a
|
therefore has a clean subtree, because anything dirty under it would have a
|
||||||
dirty parent, and so on up to the child itself. `dirty_size_under` cannot
|
dirty parent, and so on up to the child itself.
|
||||||
fire inside `redraw_updates`, which is what dropping it from the suite,
|
|
||||||
the shrinker at 400 seeds of depth 5, the oracle at 1000 of depth 6 and
|
|
||||||
2000 seeds at depth 4 measured.
|
|
||||||
|
|
||||||
The entry point it guards is `update` drawing the root for a resize before
|
The entry point that was left was `update` drawing the root for a resize
|
||||||
the walk runs, top-down over a tree with dirty widgets still in it. Most
|
before the walk ran, top-down over a tree with dirty widgets still in it.
|
||||||
routes through that are safe anyway: `retained_answer` succeeding at the
|
It is closed by marking the root instead, so layout is one walk a frame and
|
||||||
offer implies `try_reuse` succeeds at the placed box, since the settled
|
`dirty_size_under` is gone at both call sites. **The root is marked only
|
||||||
holds are cut by the drawing's holds taken through the placing length, so
|
where the new output falls outside what its answer holds for**: that range
|
||||||
a stale answer is reused
|
is the intersection of everything under it, so admitting the new output
|
||||||
whole and the dirty descendant later reaches its readers through
|
says the whole tree stands, and nothing above the root moved. Marking it
|
||||||
`redraw`'s comparison. The route that is not safe is the one asymmetry
|
unconditionally cost the root its own `Holds` -- a leaf root that scales
|
||||||
between the two: `try_reuse` refuses a drawing on another layer and
|
with its box was drawn again on every resize, which two tests caught.
|
||||||
`retained_answer` does not. A resize frame in which a structural edit
|
|
||||||
shifted a clean child's layer, with a dirty descendant under that child
|
|
||||||
whose answer changes, hands back the old answer, redraws the subtree fresh
|
|
||||||
in `place`, clears the descendant's mark there, and tells nobody.
|
|
||||||
|
|
||||||
**Close the entry point rather than test the contrived case.** A resize
|
The rig says the cost is scheduling only: on `resize`, one queue pop, one
|
||||||
marks the root and nothing else, so layout has one walk and the induction
|
local redraw and one depth read appear and one failed reuse attempt goes,
|
||||||
covers everything:
|
and every other counter on `cold`, `repaint`, `many`, `size`, `scroll` and
|
||||||
|
`resize` is identical.
|
||||||
|
|
||||||
```rust
|
### A subtree that changes hands is recorded on both sides (landed, `e44dea3`)
|
||||||
pub fn resize(&mut self, size: impl Into<Vec2>, widgets: &mut Widgets) {
|
|
||||||
let size = PxVec2::from_f32(size.into());
|
|
||||||
if size == self.output_size {
|
|
||||||
return;
|
|
||||||
}
|
|
||||||
self.output_size = size;
|
|
||||||
if let Some(root) = self.old_root {
|
|
||||||
widgets.needs_redraw.insert(root);
|
|
||||||
}
|
|
||||||
}
|
|
||||||
```
|
|
||||||
|
|
||||||
`redraw` already asks a parentless widget again in `root_region` against
|
A subtree can be reused whole under a different parent -- same box, same
|
||||||
the output. Then `dirty_size_under` goes at both call sites, `draw_inner`
|
layer, same region node, clean -- and nothing in the drawing says it moved.
|
||||||
and `retained_size`. Bryan's question of whether a resize could instead
|
Two things read who its parent is, and both were wrong after one of these.
|
||||||
mark "the important widgets" up front answers itself: `needs_redraw` is the
|
|
||||||
mark, and `Holds` is the exact, per-widget, lazy computation of which
|
|
||||||
widgets a new output invalidates; nothing has to be worked out ahead of the
|
|
||||||
walk. What it costs: the root always runs its own `draw` on a resize where
|
|
||||||
today the whole tree can remap in one `try_reuse`, and a widget dirty in
|
|
||||||
the same frame as a resize may draw twice if the root's layout then gives
|
|
||||||
it a different box. Drawn widgets should otherwise be identical on the rig;
|
|
||||||
check the counters.
|
|
||||||
|
|
||||||
**Two facts the induction relies on are not pinned, and both are the shape
|
- The **old parent still listed it**, and a parent's next draw undraws
|
||||||
of `0e0d4af`: state that is right only because something else set it up.**
|
whatever is missing from that list. Two spans under one root, with the
|
||||||
|
root swapping which of them it holds, drew the subtree under the new span
|
||||||
|
and then erased it when the old one drew.
|
||||||
|
- Its **depth** was the one it had under the old parent, which is what the
|
||||||
|
settling walk orders by, so a change made under it afterwards settled at
|
||||||
|
the wrong point in the frame.
|
||||||
|
|
||||||
- `try_reuse` never writes `active.parent`, and neither does the tail of
|
Both are written where `draw_inner` already records what the ask decided:
|
||||||
`draw_inner`. A subtree reused under a different parent inside the same
|
`active.parent` is replaced and the old parent's `children` repaired, and
|
||||||
region node keeps the old parent, so a later deferral marks the wrong
|
`try_reuse` re-walks the subtree's depths -- only where the top of it
|
||||||
widget. The fuzzer never re-parents: `reshuffle` only trades children
|
moved, which is what makes that free in the ordinary case. Pinned by
|
||||||
between a span and its own spares. A hand-built test moving a child
|
`retained::a_subtree_that_changed_parents_is_not_undrawn_by_the_one_it_left`
|
||||||
between two spans should miss a redraw or trip the `depth()` assertion.
|
and `..._settles_at_the_depth_it_moved_to`; each fails without one half.
|
||||||
- `remap_subtree` never updates descendants' `depth`. The debug assertion
|
|
||||||
in `depth()` catches it only for a widget that later goes dirty.
|
|
||||||
|
|
||||||
Both fixes are one line where `draw_inner` already writes `given` and
|
The fuzzer never re-parents (`reshuffle` only trades children between a
|
||||||
`answer`, plus a walk in `remap_subtree` or a depth kept relative to the
|
span and its own spares), which is why nothing generated reached either.
|
||||||
nearest region node.
|
|
||||||
|
|
||||||
Drawn widgets, widget draws and primitive writes are unchanged on every rig
|
|
||||||
phase; `many` pays 51 queue pops for 27 and 1059 depth reads for 410.
|
|
||||||
|
|
||||||
### A span's leftover boundary is its own inverse (landed, `53b00c6`)
|
### A span's leftover boundary is its own inverse (landed, `53b00c6`)
|
||||||
|
|
||||||
@@ -799,34 +774,32 @@ above for what closing the rest would cost.
|
|||||||
|
|
||||||
## Verification at the current head
|
## Verification at the current head
|
||||||
|
|
||||||
At `25e456e`:
|
At `e44dea3`:
|
||||||
|
|
||||||
- `cargo fmt --all --check`, `cargo clippy --workspace --all-targets --
|
- `cargo fmt --all --check`, `cargo clippy --workspace --all-targets --
|
||||||
-D warnings`, `cargo test --workspace`: green, 90 suite tests, 19 core
|
-D warnings`, `cargo test --workspace`: green, 92 suite tests, 19 core
|
||||||
unit tests, 11 generated cases. Only the long runs and the profiling rigs
|
unit tests, 11 generated cases. Only the long runs and the profiling rigs
|
||||||
are ignored; no known defect is.
|
are ignored; no known defect is.
|
||||||
- The release oracle at 100 seeds in 14.3 s, and **120 seeds in debug** in
|
- The release oracle at 100 seeds in 14.2 s, and **120 seeds in debug** in
|
||||||
59 s -- the debug run exercises the `Holds` assertion in `draw_at`.
|
59 s -- the debug run exercises the `Holds` assertion in `draw_at`.
|
||||||
- All fifteen shrinker cases at 400 seeds of depth 5 in 56 s, the oracle at
|
- All fifteen shrinker cases at 400 seeds of depth 5 in 57 s, the oracle at
|
||||||
1000 seeds of depth 6 in 142 s, and **2000 seeds at depth 4 over all
|
1000 seeds of depth 6 in 143 s, and **2000 seeds at depth 4 over all
|
||||||
fifteen cases** in 262 s. The last is not routine and should be: it is
|
fifteen cases** in 260 s. The last is not routine and should be: it is
|
||||||
the only run that has ever found anything past seed 400.
|
the only run that has ever found anything past seed 400.
|
||||||
- `view`, `minimal`, `random` and `tabs` byte-identical at 1920x1200
|
- `view`, `minimal`, `random`, `tabs` and `text` byte-identical at
|
||||||
against `ea6dbae`. `random` live-resized from 1920x1200 to 1280x800 is
|
1920x1200 against `25e456e`, as is `tabs` under the recorded replay.
|
||||||
byte-identical to a cold 1280x800 render. **`text` is a new picture**:
|
`random` live-resized from 1920x1200 to 1280x800 is byte-identical to a
|
||||||
`4bd8607` moved its lower paragraph one pixel, the box being a step wider
|
cold 1280x800 render. Rendered through Venus on the host's RX 7900 XT,
|
||||||
and its left edge crossing the shader's snap, and `c8beca5` rewrote the
|
confirmed against `vulkaninfo --summary` in the same session -- an
|
||||||
alignment panel, which had all three labels in the middle of a box the
|
llvmpipe fallback makes the same PNG and nothing in it says so.
|
||||||
width of the widest of them.
|
- All rig counters identical on `cold`, `repaint`, `many`, `size` and
|
||||||
- Twenty-five rig work counters identical on `cold`, `repaint`, `scroll`,
|
`scroll` across `25e456e`. `resize` gains one queue pop, one local
|
||||||
`resize` and `size` across `0e0d4af`; `many` differs only in reuse
|
redraw and one depth read and loses one failed reuse attempt, which is
|
||||||
attempts. Across `a92c6ac` drawn widgets, widget draws and primitive
|
the root going through the walk; drawn widgets, widget draws, draw
|
||||||
writes are identical on every phase, and only the scheduling counters
|
requests and primitive writes do not move.
|
||||||
move.
|
|
||||||
|
|
||||||
**None of that reaches seeds 1121 and 1839 at depth 4**, which fail on this
|
Everything above is verification of what was changed, not a claim that the
|
||||||
head and on `ea6dbae` alike. Everything below is verification of what was
|
branch is correct.
|
||||||
changed, not a claim that the branch is correct.
|
|
||||||
|
|
||||||
**A claim about a render holds for the commit it was checked at and no
|
**A claim about a render holds for the commit it was checked at and no
|
||||||
further.** `tabs` changed twice across `d3b0ebf` with nobody looking; take
|
further.** `tabs` changed twice across `d3b0ebf` with nobody looking; take
|
||||||
@@ -993,12 +966,10 @@ The replay used for the reference check:
|
|||||||
|
|
||||||
In order, from the review above and Bryan's steer (2026-09-17):
|
In order, from the review above and Bryan's steer (2026-09-17):
|
||||||
|
|
||||||
1. **A resize marks the root and goes through the walk**; delete
|
1. ~~A resize marks the root and goes through the walk.~~ Landed as
|
||||||
`dirty_size_under` at both call sites; write `active.parent` where
|
`a0693ac` and `e44dea3`; see the two sections above. The re-parenting
|
||||||
`draw_inner` writes `given`, and descendants' `depth` in
|
half turned out to be two defects rather than the predicted one, and
|
||||||
`remap_subtree`; pin re-parenting with a test that moves a child between
|
neither was the `depth()` assertion.
|
||||||
two spans. Small, argued by the induction above, verified by the rig's
|
|
||||||
counters.
|
|
||||||
2. **Frame and extent**, as written above. This is the fundamental change
|
2. **Frame and extent**, as written above. This is the fundamental change
|
||||||
and comes before anything built on the placing ask. It lands the two
|
and comes before anything built on the placing ask. It lands the two
|
||||||
parked branches' tests (the stack test unchanged, `Inset`/`Outset` with
|
parked branches' tests (the stack test unchanged, `Inset`/`Outset` with
|
||||||
|
|||||||
Reference in new issue
Block a user