Record the step 5 result: hints skip the room ask, and the far pin is not the cost

Measured at a888717 and at the new f6242aa on wip/hint-first, with the
inset experiment recorded as tested and rejected. The plan is now review,
render and land.
This commit is contained in:
iris-ai committed 2026-09-19 02:24:09 -04:00
1 parent 1ef31bc9e7
commit e4fada298d
2 files changed
+242 -124

No files matched your search

+170 -124
View File
@@ -14,21 +14,29 @@ detached comparison checkout is `/home/bob/repos/iris-layout-baseline`. It is
the reviewed baseline this work must preserve or improve.
The continuation is `/home/bob/repos/iris-layout-experiment`, now on branch
**`wip/one-ask`** at **`a888717`**, eight commits over `4328eac` (the head of
`wip/transparent-frames`, which is unchanged). It replaces the old step 3
plan with the one-ask protocol below, `1512d84` and `23523ee` make the frame
a length of the window, and `e8a5792` is what the step 1 review found. It
passes every check:
**`wip/hint-first`** at **`f6242aa`**, one commit over `wip/one-ask` at
`a888717` (nine commits over `4328eac`, the head of `wip/transparent-frames`,
which is unchanged). Both branches are pushed to `origin`. `wip/one-ask`
replaced the old step 3 plan with the one-ask protocol below, `1512d84` and
`23523ee` made the frame a length of the window, and `e8a5792` is what the
step 1 review found. `f6242aa` is the step 5 result: a span takes a child's
length from its hint or rule and asks it once, in its slot. It passes every
check:
| check at `a888717` | result |
| check at `f6242aa` | result |
| --- | --- |
| `cargo fmt --all --check`, clippy `-D warnings`, with and without `layout-diagnostics` | clean |
| `cargo test --workspace` (debug) | 123 suite, 20 core, 11 generated, all green |
| `cargo test --release --test generated` | 11/11 |
| shrinker, 400 seeds, depth 5, all sixteen cases | agree, 73 s (34,488 widgets) |
| 1000 seeds at depth 6 | agree, 187 s |
| 2000-seed depth-4 scan, all sixteen cases | agree, 347 s (82,203 widgets) |
| renders, the `tabs` replay and the `random` resize against #18 | inspected, see below |
| shrinker, 400 seeds, depth 5, all sixteen cases | agree, 63 s (34,488 widgets) |
| 1000 seeds at depth 6 | agree, 277 s |
| 2000-seed depth-4 scan, all sixteen cases | agree, 283 s (82,203 widgets) |
| cold layout of 400 trees at depth 5, dumped and diffed against `a888717` | byte-identical (34,488 widgets) |
| renders, the `tabs` replay and the `random` resize | **not re-run since `a888717`**; step 3 below |
The cold-layout dump is `tests/layout_dump.rs`, new at `f6242aa`: it prints
every widget's box for many grown trees so two commits can be diffed on cold
layout, which the warm/cold oracle cannot see a change to.
What implementing it corrected in the plan is in `docs/LAYOUT_LOG.md`; the
four that would have shipped as wrong layout are a share inside padding
@@ -45,8 +53,10 @@ matches a cold render at that size byte for byte. Re-run at `a888717`, all
five are byte-identical to the same renders at `a30971e`, so the scroll fix
below changed none of them.
Not done: the retained-cost work (step 5). Step 1's review is done and what
it found is in `docs/LAYOUT_LOG.md`.
Not done: the pre-submit review of `f6242aa`, the render set at it, the doc
pruning and the landing (steps 1, 3 and 6 below). Step 5's measurements and
the two ideas it tested are in `docs/LAYOUT_LOG.md` under "What the step 5
measurements found".
The worker's older step 3/4 experiment is preserved as branch
`wip/step3-experiment` (one commit over `4328eac`) and as
@@ -119,9 +129,8 @@ let answer = reused.unwrap_or_else(|| {
`try_reuse` checks the drawing against `part` and relocates it to `extent`;
the old `place` (redraw in the answer box) is gone, and with it every offer
field's purpose. `ActiveData` keeps `offer_part` as the ask box, `offer_place`
as where it was asked and `place` as where it was put; the names are the old
ones and should be renamed (`part`, `asked`, `placed`) when this lands.
field's purpose. `ActiveData` keeps `part` as the ask box, `asked` as where
it was asked and `placed` as where it was put (renamed in step 4).
A container that puts an answer somewhere other than where it asked says so
with a new call that never runs the body:
@@ -148,26 +157,53 @@ pub fn widget_at<'s, W: ?Sized>(
) -> DrawResult<'s, 'a, W>
```
`Span` asks every child once from its cursor (`Within(From(cursor..far))`),
then moves fixed children to their slots and asks share children once more
in their decided slot with the frame narrowed to it:
`Span` takes each child's length along itself from its hint where one says
-- a rule, or a widget that always reports the whole of its box -- and
otherwise asks the child once from its cursor (`Within(From(cursor..far))`).
Then it moves a drawn fixed child to its slot, and asks every other child
once in its decided slot, a share with the frame narrowed to it. This is
`Span::draw` at `f6242aa`:
```rust
let size = match painter.size_hint(child, axis) {
Some(len) => {
measured.push(None);
len
}
None => {
let room = Place::Within(Part::From(along(cursor, far)));
let size = painter
.widget_at(child, [None; 2], axis.pair(room, across))
.size();
measured.push(Some(size));
size.axis(axis)
}
};
...
let slot = along(from, start);
let place = axis.pair(Place::Fill(Part::From(slot)), across);
let used = match len.leftover > Weight::ZERO && shares {
true => {
let mut narrow = [None; 2];
narrow[axis as usize] = Some(slot.len());
painter.widget_at(child, narrow, place).len(!axis)
}
false => {
let mut narrow = [None; 2];
if len.leftover > Weight::ZERO && shares {
narrow[axis as usize] = Some(slot.len());
}
let used = match (measured, narrow[axis as usize]) {
(Some(size), None) => {
painter.place_at(child, place);
size.axis(!axis)
}
_ => painter.widget_at(child, narrow, place).len(!axis),
};
```
A hint is the length the child's draw would report: `Painter::size_hint`
resolves a fraction in it against the asking widget's frame, which is the
frame a child asked with nothing narrowed gets, and pins that frame where it
did. `Scroll`, `Masked`, a `Stack` without a sizing child and the fixture's
`Branch` hint `LEFTOVER`, since each always reports it; `Rect` and `()`
already did. A widget that reports a share its children gave it -- a span
with a `Rect` in it -- has no hint and is still asked twice, which is the
whole of what step 5 left.
`Stack` asks non-sizing children with `Part::Sized(len)` of what its sizing
child decided, on every axis that is not a share -- a box of that length
where their own alignment puts it, and that length as their frame -- and
@@ -253,10 +289,13 @@ Unchanged; see `docs/LAYOUT.md`.
## Implementation plan
Work in `/home/bob/repos/iris-layout-experiment` on `wip/one-ask` from
`a888717`. Make each step a warning-clean commit and run its named checks
before the next. If a step exposes a different mechanism, stop and update
this handoff rather than papering over it.
Work in `/home/bob/repos/iris-layout-experiment` on `wip/hint-first` from
`f6242aa`. Make each step a warning-clean commit, push it, and run its named
checks before the next. If a step exposes a different mechanism, stop and
update this handoff rather than papering over it.
The order is 1, 3, 6: review the step 5 commit, render at it, then land.
Steps 2, 4 and 5 are done and kept below for what they decided.
Anything a fuzzer finds is shrunk first (`SHRINK_SEED=<seed>
SHRINK_DEPTH=<depth> SHRINK_CASE=<case>`, which now prints each level's
@@ -264,14 +303,32 @@ frame, ask, box and size warm against cold), pinned as a named test in
`tests/cases/unsettled.rs`, then fixed under the two rules above. Do not add
a second draw back.
### 1. Review the six commits
### 1. Review the step 5 commit
**Done at `a888717`.** The pre-submit review over the six commits as one
diff against `4328eac` found one wrong layout -- a scroll placing content
that fits into a window-length box rather than the viewport -- and three
comments left describing lengths as fractions of the frame. Both are in
`docs/LAYOUT_LOG.md`; the fix is `e8a5792`, pinned by
`scroll::content_that_fits_is_placed_in_the_viewport_and_not_in_the_window`.
The six commits `4328eac..a888717` were reviewed as one diff (done at
`a888717`; it found one wrong layout, fixed at `e8a5792` and pinned by
`scroll::content_that_fits_is_placed_in_the_viewport_and_not_in_the_window`,
and three stale comments). **Still to do: the pre-submit review of
`a888717..f6242aa`**, which the planner wrote and measured but did not
review as a separate pass. It touches `Span::draw`, `Painter::size_hint`,
four `size_hint` implementations and the dump rig. Things to look at:
- `Painter::size_hint`'s pin: it sets `frame_own_len` where the hint has a
fraction. A child asked in its slot afterwards pins the same frame through
`in_parent` anyway, so the pin only matters for a caller that reads a hint
and never asks the child. Confirm that reading is right, or delete the pin
and say why.
- A hinted share child with no room is never asked and `Span` calls
`painter.undraw` on it, which is what takes its last frame's drawing down.
A drawn-in-the-room child got the same call before; check nothing else
relied on the room draw having happened first (`size_deps` records the
hint read, and `draw_at`'s `asked` handles a dep that is not a child).
- The two comments in `Span::draw` above the passes, and the `Stack` hint's
comment.
Check: the ordinary suite, then `cargo test --release --test generated`.
Anything the review changes in `Span` or `Painter` re-runs the three long
fuzzers and the cold dump (commands under step 6).
### 2. Make the frame a length and the box a region
@@ -282,11 +339,17 @@ the plan -- four of them wrong layout that would have shipped -- is in
### 3. Render and replay
**Done at `adbedaf`**, against `e44dea3` (#18); what each render showed is
in `docs/LAYOUT_LOG.md`. `view` has no counterpart in the baseline, so it
was rendered but not compared. Run the set again after any further change
here -- the commands are under **Full verification** below, and a text
placed by re-expression rather than by a second draw is what shows first.
Done at `adbedaf` and re-run at `a888717`, against `e44dea3` (#18); what
each render showed is in `docs/LAYOUT_LOG.md`. **Not yet run at `f6242aa`.**
Render the five examples, the `tabs` replay and the `random` resize at the
reviewed head and compare with the same renders at `a888717` (a
`git worktree add ../iris-a888717 a888717` with its own target directory,
one process at a time). Expected: byte-identical, every one -- the cold
dump already says so for 400 random trees, but the fuzzer grows no `rel`
rules and the examples do, so `text` is the render that can still differ.
Any difference is a finding: read the records before touching anything.
`view` has no counterpart in #18 and is compared with `a888717` only. The
commands are under step 6.
### 4. Rename and delete
@@ -301,24 +364,24 @@ is worth a name.
### 5. Restore the expected retained cost
Work counters, seed 1 and 13, depth 8, widget draws / distinct widgets,
beside `e44dea3` (#18) and `0ef87eb` (before the frame became a length). The
`a888717` column was re-measured after the step 1 review and is identical to
`adbedaf` at every phase, so the scroll fix cost nothing:
**Done at `f6242aa`**, by one change: a span takes a child's length from
its hint or rule and asks it once, in its slot, instead of drawing it in the
measuring room first. Work counters, seeds 1 and 13, depth 8, widget draws /
distinct widgets, beside `e44dea3` (#18) and `a888717` (before it):
| seed 1 | e44dea3 | 0ef87eb | a888717 |
| seed 1 | e44dea3 | a888717 | f6242aa |
| --- | --- | --- | --- |
| cold | 369/261 | 331/288 | 342/288 |
| many | 157/95 | 110/92 | 118/95 |
| cold | 369/261 | 342/288 | 264/232 |
| many | 157/95 | 118/95 | 41/41 |
| size | 16/12 | 3/3 | 3/3 |
| scroll | 2 | 1 | 1 |
| resize | 13/13 | 40/15 | 44/13 |
| resize | 13/13 | 44/13 | 36/13 |
| seed 13 | e44dea3 | 0ef87eb | a888717 |
| --- | --- | --- | --- |
| cold | 1330/707 | 1179/982 | 1278/982 |
| many | 524/159 | 424/364 | 429/366 |
| resize | nothing | nothing | nothing |
| seed 13 | e44dea3 | a888717 | f6242aa |
| --- | --- | --- | --- |
| cold | 1330/707 | 1278/982 | 758/627 |
| many | 524/159 | 429/366 | 16/16 |
| resize | nothing | nothing | nothing |
The command is
@@ -327,74 +390,37 @@ IRIS_SEED=1 IRIS_DEPTH=8 cargo test --release --features layout-diagnostics \
--test layout_diagnostics -- --ignored --nocapture layout_cost
```
reading `widget draws` and `distinct widgets` from each phase's block.
reading `widget draws` and `distinct widgets` from each phase's block, and
`hottest widget draws` with `IRIS_PHASE=resize IRIS_FRAMES=2` for who is
drawn how often.
`many`, `size` and `scroll` are better than #18 and within a few draws of
`0ef87eb`; `cold` is 3% and 8% more than `0ef87eb` for the same distinct
widgets, which is the frame pins making a widget answer again where it used
to be reused on a box that happened to match. `resize` at seed 1 still draws
44 times where #18 drew 13.
`many`, `size` and `scroll` are at their floor: every draw is a marked
widget, or the parent a marked widget deferred to. `cold` is under #18 at
both seeds. What is left is `resize` at seed 1, 36 draws of 13 widgets where
#18 drew 13, and the `cold` draws over the distinct count (32 at seed 1,
131 at seed 13). Both are one mechanism, understood and not yet worth a
change: a **reported share** -- a span whose children report `leftover`, a
stack sized by a child, a wrapper round either -- has no hint, so its parent
still asks it in the room and again in its slot, and each of those asks
draws, since the room drawing divided the room and does not hold for the
slot. Under the root's resize redraw that multiplies down the tree: the
hottest widget at seed 1 is a `Span` drawn 8 times in one frame, two
levels of reported shares under it drawn 4. The design that would remove it
is in `docs/LAYOUT_LOG.md` ("two answers per record"); do it only once an
app screen shows the cost, and measure that screen first.
Measuring this is what found `adbedaf`: the fuzzer's own `Branch` pinned the
window rather than saying which side of its threshold it was on, which put
seed 1's resize at 131 and seed 13's at 828. A fixture that redraws
everything on a resize cannot tell a change that reuses well from one that
does not, so check the fixture before believing a regression.
Two ideas from the previous version of this step were tested and are **not
to be done**; the measurements are in `docs/LAYOUT_LOG.md`:
Three mechanisms behind what is left, all understood:
- **A share child is asked twice per span draw** -- in the measuring room
with the frame forwarded, then in its slot with the frame narrowed. Each
ask that reads pixels or pins a length draws, and since `0ef87eb` every
local change inside a share child redraws its span as well. Give `Span` a
measure-only ask for the first pass: reuse the retained *answer* when its
holds contain the room, without validating or relocating the drawing, and
let the placing ask settle the drawing. The answer contract must then
carry no symbolic pin (a span's total does not depend on `far`; only its
slots do), which is the separation the worker's experiment made with
`answer_extent_len`. With that, a twice-asked child could keep both
answers and settle locally by re-asking both questions instead of
deferring. Measure `many` at seed 13 before and after; the seed 946 pin
must stay green throughout.
- **A positive-direction span with no shares pins `far`** it does not need,
so a resize redraws it. Read `extent_len` only where a slot depends on it
(shares, or `Sign::Neg`), and express the measuring room's far end without
the length.
**Measured at `a888717` (worker, 2026-09-19)**, by replacing `far` with a
deliberately unsound read that returns the length without pinning: seed 1's
`resize` goes from **44 draws of 13 distinct widgets to 20 of 9**, and the
median frame from 0.063 ms to 0.022 ms. So the pin is worth the step, and
it is the pin rather than the twice-asked multiplication -- the hottest
widgets are drawn 8, 8, 4, 4, 4, 4, 2, 2 times, and only 3 of 98 reuse
attempts are rejected by a frame length. A symbolic pin fails on a resize
because a symbolic length is not window-independent: a scroll's content box
is `rel 0, px content_len`, and `content_len` is measured against the
window.
**What blocks the sound version.** The measuring room is
`Part::From(UiSpan::new(cursor, far))`, and no existing `Part` says "from
the cursor to the end of the box" without naming the box's length. A new
variant would -- an inset of `lead` and `trail` *window* lengths, which
also subsumes `Part::Of`'s pure-pixel use in `Pad` -- but `in_parent` needs
an arm for it, and the mapping is not a plain `Holds::through`: the inset
is a window length, so the child's box length changes with the window and
the offset cannot be written as a `Len` of the parent's box.
There is a second, deeper question under it. Dropping the pin is only
sound if nothing else has to notice the span's box changing, and today
`in_parent`'s `_` arm converts a child's box range into a range on the
*window* through the child's box length -- which assumes that length is a
fixed expression of the window, which is exactly what `far` changing
violates. Putting that dependency on the parent's own box instead would
let the span stop reading `far` for soundness and read it only where a
slot depends on it, but it will also invalidate more elsewhere. Decide
that before writing the `Part` variant.
- **A rule that is a fraction of the frame pins the frame**, which is what
`cold` grew by. The answer for such an axis is a pure function of the rule
and the frame, so a reuse could resolve it again from the record instead
of redrawing -- `placed_extent` already takes `declared`. Worth trying
before anything subtler; the pin stays for the axes that read the frame.
- Not reading the span's own length where no slot depends on it
(`wip/inset` at `5b181bc`, pushed): identical counters at every phase.
The symbolic pin survives a resize because every ask box is symbolically
stable across one; the earlier "44 to 20" came from share spans reusing
their room drawing in their slot, which is wrong layout rather than saved
work.
- Resolving a rule that is a fraction of the frame from the record instead
of redrawing: `reuse outside: a frame length` is 3 draws of 264 cold at
seed 1 and 1 of 758 at seed 13. Nothing to recover.
Report every phase at both seeds, work counters first, medians only when the
work agrees.
@@ -420,7 +446,21 @@ SHRINK_CASE=all SHRINK_SEEDS=2000 SHRINK_DEPTH=4 \
The last line is the 2000-seed depth-4 scan over all sixteen cases; the
shrinker runs the same cases as the scan and reduces anything it finds, so
no temporary test body is needed any more. `Rng::new` uses `seed | 1`, so
adjacent even/odd seed pairs describe the same tree.
adjacent even/odd seed pairs describe the same tree. All of these passed at
`f6242aa`; re-run whatever a later commit could affect.
A change that could move cold layout also gets the dump diff, which the
oracle cannot replace:
```sh
IRIS_DUMP_SEEDS=400 IRIS_DUMP_DEPTH=5 cargo test --release --test layout_dump \
-- --ignored --nocapture | grep -E '^[0-9]+ [0-9]+ ' > /tmp/after.txt
```
once at the commit before and once after (check the earlier commit out in
the same checkout so the build is incremental; the rig file is untracked
there, copy it in with `git show wip/hint-first:tests/layout_dump.rs`), then
`diff` the two. Zero differing lines is the expectation for a cost change.
Render `view`, `minimal`, `random`, `tabs` and `text` at 1920x1200 and inspect
every intentional change. Also replay `tabs` and compare a live resize of
@@ -448,10 +488,16 @@ The reference replay is:
880 up 1836 1116
```
Before submitting, run the pre-submit review. Once the protocol lands, move
any surviving fact from `docs/LAYOUT_LOG.md` into `docs/LAYOUT.md`, delete
the log, update this handoff to the next actual task, update the app's Iris
pin only when the Iris change is ready, and push every coherent commit.
Before submitting, run the pre-submit review. Then land: move any surviving
fact from `docs/LAYOUT_LOG.md` into `docs/LAYOUT.md` (the settled protocol,
the measurement method including the dump rig, and the "not to be done"
results under step 5), delete the log, update this handoff to the next
actual task, and push every coherent commit. How the branch reaches
upstream is Bryan's call and has not been asked: `wip/hint-first` is some
sixty commits over #18's `split/18-position-chain`, which is still open, so
either #18 lands first and this follows as one PR, or this replaces #18.
Ask before opening anything. Update the app's Iris pin only when the Iris
change is ready.
## Follow-on work, not part of this repair