Record the report rule, the stale-answer guard, and two open seeds
The `rel` reading Bryan decided landed, and with it a defect the retained machinery had all along: a span that can overflow itself makes the two asks' boxes differ far enough that a reused answer is read before the subtree under it has settled. Widening the fuzzers to 2000 seeds at depth 4 then found two warm/cold divergences that predate both commits, so the branch is not correct and the routine verification does not say otherwise. They lead "Next".
This commit is contained in:
1 parent
4763e1a70d
commit
f64d6a8d4d
1 file changed
+124
-115
+124
-115
@@ -10,10 +10,37 @@ stayed.
|
||||
|
||||
Canonical Iris `main` is **`ca2b4b2`** (#17, the headless rig). **#18
|
||||
`split/18-position-chain`** is open in `/home/bob/repos/iris-pr18`, head
|
||||
**`ea6dbae`**, pushed. It holds LAYOUT.md §2's position chain, `leftover`,
|
||||
**`ffd79f3`**, pushed. It holds LAYOUT.md §2's position chain, `leftover`,
|
||||
the `Holds` retained-layout contract, region nodes, built-in alignment and
|
||||
size rules, fixed-point layout, and a box in pixels threaded down the draw.
|
||||
No PR review was present when checked on 2026-09-15.
|
||||
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
|
||||
present when checked on 2026-09-15.
|
||||
|
||||
**The branch has a warm/cold divergence nothing on it explains yet**, found
|
||||
2026-09-17 by widening the fuzzers rather than by any change: seeds **1121**
|
||||
and **1839** at **depth 4** fail `shuffle-swap-for-three` and
|
||||
`shuffle-all-but-first` respectively, and they fail the same way on
|
||||
`ea6dbae`, before either of the day's commits. Nothing in the routine
|
||||
verification reaches them -- the fast oracle takes ten seeds, the shrinker
|
||||
400 at depth 5 and the long oracle 1000 at depth 6, and these are past 400
|
||||
at a depth neither long run uses. Chasing them is first in "Next". The scan
|
||||
that found them, which is worth keeping as a pattern:
|
||||
|
||||
```rust
|
||||
// tests/scan.rs, deleted once it had done its job
|
||||
over_seeds((1..=2000).collect(), |seed| {
|
||||
let grown = plan(seed, 4, &Edits::default());
|
||||
for &case in ALL.iter() {
|
||||
if let Some(how) = diverges(&grown, case, seed) {
|
||||
println!("HIT seed {seed} case {} {how}", case.name());
|
||||
}
|
||||
}
|
||||
});
|
||||
```
|
||||
|
||||
262 s for 2000 seeds at depth 4 over all fifteen cases. `Rng::new` is
|
||||
`seed | 1`, so an even seed and the odd one above it are one tree: 1120 and
|
||||
1121 are the same counterexample, as are 1838 and 1839.
|
||||
|
||||
**Two ideas outrank everything else in this document** (Bryan, 2026-09-17).
|
||||
First, a changed tree lays out exactly as if it had been drawn that way from
|
||||
@@ -38,9 +65,9 @@ equality. Threading the pixel box down the draw, with `Holds::through` the
|
||||
exact preimage of that one multiply, is the strongest idea in the code:
|
||||
layout has one route to every length and the reuse test is its exact
|
||||
inverse. Offer, given and placed is the ordinary measure-then-arrange model.
|
||||
What needs work is the bookkeeping around the second ask, one boundary in
|
||||
What needed work was the bookkeeping around the second ask, one boundary in
|
||||
`Span` computed by an expression other than the drawing it guards, and a
|
||||
`rel` that means two things. The two-step residual is structural and no
|
||||
`rel` that meant two things. The two-step residual is structural and no
|
||||
grid width fixes it; where it becomes visible is the shader's snap.
|
||||
|
||||
### Decided by Bryan
|
||||
@@ -56,100 +83,64 @@ grid width fixes it; where it becomes visible is the shader's snap.
|
||||
place them exactly where they were measured and so draw once, and that
|
||||
must survive. The smaller items below come first.
|
||||
|
||||
### `rel` means two things today
|
||||
### A report is a fraction of the containing widget (landed, `ffd79f3`)
|
||||
|
||||
Measured in a 400 px row whose first child is 100 px wide:
|
||||
`rel(0.5)` is half the span whatever else is in it and wherever the child
|
||||
sits. A report used to come back composed through the box it was offered,
|
||||
and a span offers each child the room from its cursor, so a nested span
|
||||
taking half of what it was given took a quarter of a row whose first half
|
||||
was spoken for -- where the same half written as a rule on the child took
|
||||
half the row.
|
||||
|
||||
| second child | box | meaning |
|
||||
| --- | --- | --- |
|
||||
| `rect.width(rel(0.5))`, declared | 100 to 300 | half the row |
|
||||
| nested span reporting `rel(0.5)` | 100 to 250 | half the remainder |
|
||||
|
||||
The declared case is the rule; the reported case is wrong, and
|
||||
`tests/cases/layout.rs::a_span_reads_a_child_report_as_a_fraction_of_what_it_offered`
|
||||
pins the wrong behaviour and has to change. The two placement functions in
|
||||
`core/src/ui/painter.rs` differ by one line:
|
||||
The offer is still the remainder, because a text has to wrap at the width
|
||||
actually there. What separated from it is the base a report's fractions are
|
||||
of, which the ask now carries as `reports_of`:
|
||||
|
||||
```rust
|
||||
// placed_box: a reported length is composed through the offered box
|
||||
let len = lens.axis(axis).within_len(span.len());
|
||||
|
||||
// declared_box: a declared length is a fraction of the parent's own box
|
||||
let len = Len::from_parts(len.rel, len.px);
|
||||
pub fn widget_at<'s, W: ?Sized>(
|
||||
&'s mut self,
|
||||
id: &'s StrongWidget<W>,
|
||||
region: UiRegion,
|
||||
reports_of: UiVec2,
|
||||
decided: [bool; 2],
|
||||
) -> DrawResult<'s, 'a, W>
|
||||
```
|
||||
|
||||
Making `declared_box` compose the same way would be the one-line fix in the
|
||||
*wrong* direction. The composition `in_parent_frame` does is right where the
|
||||
offer is the child's whole area -- `Pad`'s inset, a `Stack` child, `Scroll`'s
|
||||
content -- and wrong where the offer is a positional remainder, which is
|
||||
`Span` along its axis. `Span` offers each child the room from the cursor to
|
||||
the end because a text has to wrap at the width actually left, so the pixel
|
||||
width of the offer and the base its fractions are taken of have to be
|
||||
separated: the remainder for one, the row for the other. Candidate: the ask
|
||||
carries what a report's fractions are of, defaulting to the offered box, and
|
||||
`Span` passes its own extent along the row. A child that drew at half of its
|
||||
remainder is then placed at half the row and redrawn there by the placing
|
||||
ask, which is one draw more, and exactly what `place` already does for any
|
||||
child whose placed box differs from its offer. A nested span whose child
|
||||
asks for half of it reports half of the row, gets it, and its child gets
|
||||
half of that: no circularity, because the base is the row and not the
|
||||
nested span's own answer. Not designed yet; it is first in "Next".
|
||||
It is `region.size()` wherever the box offered is the child's whole area --
|
||||
`Pad`'s inset, a `Stack` child, `Scroll`'s content -- and `Span` passes
|
||||
`UiVec2::FULL_SIZE` along its row. `widget_decided` is gone; `widget_at`
|
||||
says both things about an ask rather than one of them.
|
||||
|
||||
### The offer's answer is overwritten by every ask
|
||||
**A span can now overflow itself without bound**, which is the consequence
|
||||
Bryan's rule asks for: two children reporting half each take the whole row
|
||||
and a third starts past the end. Under the old reading `total.rel` could
|
||||
not exceed one, so `Span`'s `fixed <= 0` branches were only reachable
|
||||
through declared fractions; they are ordinary now, and with them boxes of
|
||||
negative length passed down to children.
|
||||
|
||||
`ActiveData::answer` is documented as what the widget answered at its
|
||||
offer. `Painter::widget_at` guards its write with `answers_offer`, but
|
||||
`UiRenderState::draw_inner` writes the field unconditionally and returns
|
||||
the same value, so the guard is dead and the placing second ask overwrites
|
||||
the offer's answer with one about the placed box. The guard is from
|
||||
`29c7881`; the unconditional write arrived with `d3b0ebf`.
|
||||
### An answer is not an answer while anything under it is dirty (`0e0d4af`)
|
||||
|
||||
```rust
|
||||
// painter.rs, widget_at
|
||||
let answers_offer = self.at_offer && px == offered_px;
|
||||
let (size, holds) = self.state.draw_inner(...);
|
||||
if answers_offer {
|
||||
self.state.active.get_mut(&id.id()).unwrap().answer = (size, holds);
|
||||
}
|
||||
`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
|
||||
while anything the widget read a size from was dirty. A widget whose drawing
|
||||
happened to be reusable therefore handed back the answer it gave before that
|
||||
descendant changed, and nothing puts it right: the comparison that tells a
|
||||
reader its child's answer moved is in `redraw`, and a widget settled inside
|
||||
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
|
||||
number the tree no longer agrees with.
|
||||
|
||||
// render_state.rs, draw_inner, on every ask
|
||||
active.answer = settled;
|
||||
```
|
||||
So `dirty_size_under` is not the optimization its comment claimed. It is
|
||||
what makes an answer an answer, and it is asked once in `draw_inner` for
|
||||
both retained routes. Twenty-five rig counters are unchanged on `cold`,
|
||||
`repaint`, `scroll`, `resize` and `size`; `many` makes 18 fewer reuse
|
||||
attempts, 17 of which already reported "dirty".
|
||||
|
||||
`known_len`'s first-ask write and `update`'s resize path also write the
|
||||
field as though `draw_inner` did not; `known_len`'s stores the value it just
|
||||
read from the same field. The oracle passes, so layout is not wrong. What it
|
||||
can cost is churn: an answer about the placed box can miss
|
||||
`retained_answer` on the next offer ask and fall through to a remap and
|
||||
back. Not confirmed against the counters. Write the field in one place, and
|
||||
keep the guarded one.
|
||||
|
||||
### `redraw` reassembles `DrawInfo` by hand
|
||||
|
||||
`ActiveData` copies eight fields of `DrawInfo` and `redraw` rebuilds the
|
||||
struct field by field. This is where the mask defect fixed in `ea6dbae`
|
||||
lived for as long as there was a local-redraw path.
|
||||
|
||||
```rust
|
||||
let info = DrawInfo {
|
||||
layer: active.layer,
|
||||
parent: active.parent,
|
||||
depth: active.depth,
|
||||
parent_move: active.parent_move,
|
||||
region_node: rsc.widgets().is_region_node(id),
|
||||
mask: active.parent_mask,
|
||||
given_len: active.given_len,
|
||||
offer_len: active.offer_len,
|
||||
px: given_px,
|
||||
offered_px,
|
||||
decided: active.decided,
|
||||
};
|
||||
```
|
||||
|
||||
Store the `DrawInfo` on `ActiveData` and write
|
||||
`DrawInfo { px, offered_px, ..active.info }`: the duplicated fields and the
|
||||
reconstruction go, and a new field cannot be forgotten. The pixel pair stays
|
||||
out on purpose; see "px is not stored" below.
|
||||
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
|
||||
divergence needs a reader drawing while a size dependency two levels under
|
||||
it is dirty, which no hand-built tree has reproduced yet, and the seeds at
|
||||
depth 4 above fail for some other reason.
|
||||
|
||||
### `Span`'s leftover boundary is a third expression for the room
|
||||
|
||||
@@ -455,22 +446,26 @@ above for what closing the rest would cost.
|
||||
|
||||
## Verification at the current head
|
||||
|
||||
At `32542d0`, then `ea6dbae` on top:
|
||||
At `ffd79f3`:
|
||||
|
||||
- `cargo fmt --all --check`, `cargo clippy --workspace --all-targets --
|
||||
-D warnings`, `cargo test --workspace`: green, 87 suite tests, 18 core
|
||||
-D warnings`, `cargo test --workspace`: green, 89 suite tests, 18 core
|
||||
unit tests, 11 generated cases. Only the long runs and the profiling rigs
|
||||
are ignored; no known defect is.
|
||||
- The release oracle at 100 seeds in 14.4 s, and **120 seeds in debug** in
|
||||
- The release oracle at 100 seeds in 14.3 s, and **120 seeds in debug** in
|
||||
59 s -- the debug run exercises the `Holds` assertion in `draw_at`.
|
||||
- All fifteen shrinker cases at 400 seeds of depth 5 in 58 s, and at 1000
|
||||
seeds of depth 6 in 147 s.
|
||||
- All fifteen shrinker cases at 400 seeds of depth 5 in 56 s, and the
|
||||
oracle at 1000 seeds of depth 6 in 142 s.
|
||||
- `view`, `minimal`, `random`, `tabs` and `text` byte-identical at
|
||||
1920x1200 against `5b78002`, and the `tabs` touch replay before and after
|
||||
the gesture. `random` live-resized from 1920x1200 to 1280x800 is
|
||||
byte-identical to a cold 1280x800 render.
|
||||
- Twenty-five rig work counters identical on the `cold` and `resize`
|
||||
phases, which is what makes those rows under "Performance" a measurement.
|
||||
1920x1200 against `ea6dbae`. `random` live-resized from 1920x1200 to
|
||||
1280x800 is byte-identical to a cold 1280x800 render.
|
||||
- Twenty-five rig work counters identical on `cold`, `repaint`, `scroll`,
|
||||
`resize` and `size` across `0e0d4af`; `many` differs only in reuse
|
||||
attempts.
|
||||
|
||||
**None of that reaches seeds 1121 and 1839 at depth 4**, which fail on this
|
||||
head and on `ea6dbae` alike. Everything below is verification of what was
|
||||
changed, not a claim that the branch is correct.
|
||||
|
||||
**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
|
||||
@@ -487,9 +482,10 @@ IRIS_GENERATED_SEEDS=1000 IRIS_GENERATED_DEPTH=6 \
|
||||
cargo test --release --test generated -- --ignored a_long_run_of_seeds_agrees
|
||||
```
|
||||
|
||||
Depth is what finds things: nothing failed at 100 seeds of depth 4, which
|
||||
was all the oracle ever routinely ran, and every late defect surfaced at
|
||||
depth 5 or 6.
|
||||
Depth is what finds things, but so is breadth: every late defect before
|
||||
2026-09-17 surfaced at depth 5 or 6, and the two open ones were found by
|
||||
running 2000 seeds at depth 4, which nothing routine does. Widen one axis
|
||||
at a time and record which.
|
||||
|
||||
## Performance
|
||||
|
||||
@@ -636,18 +632,27 @@ The replay used for the reference check:
|
||||
|
||||
In order, from the review above and Bryan's steer (2026-09-17):
|
||||
|
||||
1. **`rel` as a fraction of the containing widget's whole area**, on
|
||||
`Span`'s row axis, per the decision above. Fix the test that pins the
|
||||
remainder reading, and add the two-child case from the table.
|
||||
2. Write `ActiveData::answer` in one place.
|
||||
3. Keep the `DrawInfo` on `ActiveData`; delete the copied fields and the
|
||||
1. **Seeds 1121 and 1839 at depth 4**, which fail on this head and on
|
||||
`ea6dbae` alike -- a warm/cold divergence older than either of the
|
||||
day's commits and the only known correctness defect on the branch.
|
||||
Reduce each with `SHRINK_SEED`/`SHRINK_DEPTH=4`/`SHRINK_CASE`, turn it
|
||||
into a fast test, and add the seed to `generated.rs`'s `SEEDS` the way
|
||||
86 and 20 are there. Then re-scan 2000 seeds at depth 4 before calling
|
||||
it done.
|
||||
2. **A fast test for `0e0d4af`**, the stale-answer guard, which went in
|
||||
with only a fuzz seed behind it. The shape wanted is a widget drawing
|
||||
while a size dependency two levels under it is dirty, where its
|
||||
drawing is reusable at the measuring box and not at the placing one.
|
||||
3. Write `ActiveData::answer` in one place.
|
||||
4. Keep the `DrawInfo` on `ActiveData`; delete the copied fields and the
|
||||
reconstruction in `redraw`.
|
||||
4. `Span`'s leftover boundary through `Holds::through`.
|
||||
5. **Round on the CPU and snap to the nearest pixel in the shader**, as
|
||||
5. `Span`'s leftover boundary through `Holds::through`.
|
||||
6. **Round on the CPU and snap to the nearest pixel in the shader**, as
|
||||
one change with one verification. Bryan approved the snap on 2026-09-17
|
||||
(rendering may change wherever it brings the screen closer to what the
|
||||
user's code says: three equal sections of 1000 px need one of them
|
||||
rounded up). CPU rounding is recommended for a different reason: a
|
||||
rounded up), and to CPU rounding the same day. The reason for that one
|
||||
is different: a
|
||||
`Rel` is off by at most `2^-25` of its box, so with round-to-nearest
|
||||
every product whose true value is a whole number of steps is exact for
|
||||
boxes under about 8,000 px, where truncation leaves half of them one
|
||||
@@ -658,17 +663,21 @@ In order, from the review above and Bryan's steer (2026-09-17):
|
||||
bounds move by half a `Rel` step); check with `nm` that `UiSpan::within`
|
||||
still inlines; expect a couple of percent of instructions and re-run the
|
||||
long fuzzers and the render set once for both.
|
||||
6. The smaller items: the stale `f32` comment, the gap of an undrawn child,
|
||||
confirm nested `leftover` weights, one zero-divisor fallback.
|
||||
7. `LazySpan`, the next LAYOUT.md §2 item. Region nodes cover the movable
|
||||
7. The smaller items: the stale `f32` comment, the gap of an undrawn child,
|
||||
confirm nested `leftover` weights, one zero-divisor fallback. Add to
|
||||
them: a span that overflows itself hands a child a box of negative
|
||||
length, which is ordinary now rather than a corner, and nothing states
|
||||
what a widget may assume about one.
|
||||
8. `LazySpan`, the next LAYOUT.md §2 item. Region nodes cover the movable
|
||||
subtree case; do not restore a separate child-placement API.
|
||||
8. `SizeRule::{Min, Max, Clamp}`, restoring the `max_width`/`max_height`
|
||||
9. `SizeRule::{Min, Max, Clamp}`, restoring the `max_width`/`max_height`
|
||||
builders `8220a78` deleted. The clamp boundary is a hard layout decision
|
||||
with an exact `Holds` split at the crossover, both sides in `Px`. Still
|
||||
awaiting Bryan: whether a `Max` narrows the box the child draws in, or
|
||||
only what the parent reports for it.
|
||||
9. `Scroll` taking a direction rather than one axis.
|
||||
10. The measure/draw split, once the above is in.
|
||||
10. `Scroll` taking a direction rather than one axis.
|
||||
11. The measure/draw split, once the above is in. It deletes the two-ask
|
||||
protocol, which is what `0e0d4af` had to put a guard around.
|
||||
|
||||
`docs/LAYOUT.md` §4, §5 and the density section are stale: they name
|
||||
`Painter::place`, `SetSize`, `desired_width`, `apply_rest`, `Len::dp`,
|
||||
|
||||
Reference in new issue
Block a user