diff --git a/docs/HANDOFF.md b/docs/HANDOFF.md index 95a21a2..1446254 100644 --- a/docs/HANDOFF.md +++ b/docs/HANDOFF.md @@ -4,10 +4,22 @@ Where the work in flight stands. The settled layout design, the vocabulary and the measurement method are in `docs/LAYOUT.md`; what the review of #19 found is in `docs/LAYOUT_LOG.md`. +## A quality sweep of the deferred system is submitted on PR #19 + +`layout/one-ask` is at `4cb6f68` in `/home/bob/repos/iris-deferred`. The tenth +sweep is the first review of `76aaf06` through `0e838e9` -- the bound rules, +the deferred request path and the invalidation fix. Ten findings, the largest +two being a bound's held length looked up a second time through a value that +could not promise the end it named (`Bound::at` `expect`ed it), and a bound in +pixels pinning the rel base it is not read against. `docs/LAYOUT_LOG.md` has +them all, with the two rules it tripped and left and two corpus questions for +Bryan. Verified: format, clippy both ways, 197 and 201 tests, all three seed +scans (69.02s, 174.87s, 330.39s), and the cold dump byte-identical to +`0e838e9` across all 34,986 boxes. + ## Performance sweep is submitted on PR #19 -`layout/one-ask` is at `0e838e9` in `/home/bob/repos/iris-deferred`. The fix -keeps dependencies only for size requests the allocator uses, and borrows +The performance fix is `0e838e9`. It keeps dependencies only for size requests the allocator uses, and borrows `Widgets::size_rules` rather than cloning both axes at every lookup. Used hints retain their reader dependency; adding a cap after layout is checked for both hinted and measured shares. Ordinary text measurement still tracks its @@ -95,6 +107,8 @@ comparison work above, its review rounds past `cadfba0` are described in overflow its pixels asked for, said as the place the parent gives and sharing one comparison with the span; and `Image` grown in the generated trees, which is the only widget here whose hint is a length in pixels. +- **The deferred request system's own sweep**, `4cb6f68` -- described at the + top of this file and in full in `docs/LAYOUT_LOG.md`. - **One ask, the root's included**, `0d03267` -- the root had a layout path of its own, so a rule that reads the box it is offered reached every widget but that one. `Placing::WINDOW` is the box nobody drew and `Placing::ask` @@ -155,12 +169,11 @@ the name it is reachable by. **Always**, because they cost nothing: format, workspace clippy under `-D warnings` with and without `layout-diagnostics`, the workspace tests, and -the **cold dump**. `layout_dump` over 400 depth-5 trees is **34,571** boxes -since `2dba90b` grew images in the trees, and `0d03267` left every one of them -byte-identical. It was 34,488 before the images; the fourth through eighth -sweeps all repeated 34,492, which is a `wc -l` of the whole run rather than of -its box lines, so count the lines that are a box -(`grep -cE '^[0-9]+ [0-9]+ '`). It is the only thing that catches two +the **cold dump**. `layout_dump` over 400 depth-5 trees is **34,986** boxes +since `76aaf06` grew bounds in the trees. It was 34,571 from `2dba90b`, which +grew the images, and 34,488 before those; the fourth through eighth sweeps all +repeated 34,492, which is a `wc -l` of the whole run rather than of its box +lines, so count the lines that are a box (`grep -cE '^[0-9]+ [0-9]+ '`). It is the only thing that catches two same-typed values being swapped, which is the failure mode of a rename or a move. The repair moved 650 of those boxes, all from the collapsed-share correction; every commit since has been byte-identical to `84dad21`. diff --git a/docs/LAYOUT_LOG.md b/docs/LAYOUT_LOG.md index a9dbd3f..bf75e86 100644 --- a/docs/LAYOUT_LOG.md +++ b/docs/LAYOUT_LOG.md @@ -6,6 +6,99 @@ nothing here is rediscovered. Each entry says who found it and when. it (settled design, the measurement method) belongs in `docs/LAYOUT.md`, and the current plan is in `docs/HANDOFF.md`. +## Tenth sweep: the deferred request system (2026-09-20) + +Over the part of #19 no earlier round reviewed -- `76aaf06` and `de1eb7e` +(the bound rules and the rule/widget split), `8780b40` (deferred comparisons) +and `0e838e9` (the invalidation fix) -- 2,243 inserted lines over 32 files, +with `core/src/widget/request.rs`, `MaxSize` and the deferred cases new. Ten +findings, all in `4cb6f68`. + +- **A bound's held length was looked up a second time, through a value that + could not say which end.** `Bound::outside` returned `Option` and + the caller asked `Bound::at(outside)` for the length, which `expect`s an end + the enum says nothing about -- `at(Shorter)` on a bound with no floor + panics, and only the pairing of the two calls kept it from happening. + `outside` already had the length in hand (`held`), so it now returns it; + `Outside` and `Bound::at` are deleted, and the state that could panic + cannot be written. +- **A pixel bound pinned the rel base it was not read against.** + `Painter::measured_request` recorded a rel-base dependency for any bound at + all, so a measured share under `Min(px(80))` was invalidated by a change to + a base its answer does not depend on. This is the class `0e838e9` fixed one + instance of; `Placing::ask` already asked the narrower question inline, so + the question is now `Bound::has_fraction` and all three callers ask it. +- **One question about a pair, written out three times.** `[bound.min, + bound.max].into_iter().flatten().any(|len| len.rel != Rel::ZERO)` appeared + in `SizeRule::has_fraction` and again in `Placing::ask`, over a pair the + framework names. It is `Bound::has_fraction` once. +- **Three buffers reused for their capacity, on an invariant nothing + stated.** `draw_at` now hands the painter the old draw's `textures`, + `primitives` and `request_deps`, which is only sound because every path to + it goes through `remove`, which drains them. A drawing over a buffer that + still held primitives would record them twice and free them once. A + `debug_assert` says so where they are taken. +- **Two names and two spellings for one switch in the rigs.** The walk that + drops a generated tree's bounds was written out in `layout_dump.rs` and + `layout_diagnostics.rs`, under `IRIS_DUMP_UNBOUNDED` read with + `var_os().is_some()` in one and `IRIS_UNBOUNDED` read with the file's own + `env` helper in the other. It is `Plan::drop_bounds` and `IRIS_UNBOUNDED` + in both, and the dump's module comment says so. +- **`Span` wrote its gaps twice and read its allocation three ways.** The gap + total is `Span::gaps`. In the placement loop `len`, `shares` and "not drawn + at all" were three separate matches on whether the row was allocated, two + of them deciding one child's length: one match now gives all three, so the + allocated and plain rules are read side by side. +- **`Stack` spelled "the child that sizes it" a second way** in + `size_request`, with two arms answering `LEFTOVER`, where `draw` resolves + the same thing once. +- **`Pad` spelled its own padding a second way.** `Padding::along(axis)` is + the sum of the two sides, which `draw` adds back and `size_request` insets + by. +- **Comments that described something else.** `with_requests` said nested + painters keep discovery from overwriting its buffers, where the hazard is a + child *drawn* mid-row; `Painter::allocate`'s doc described the window it + holds for rather than what it does; `minimum_request` had none; + `RequestArena::allocate` said "one scope of nonnegative shares", which is + not this codebase's vocabulary; and `redraw_updates` was left mid-rewrap, + with "the set" naming a `BTreeSet` that is now a `BinaryHeap`. +- **A live explanation deleted with the path it was not about.** The comment + saying a span carries its children's leftover weight whole rather than + collapsing it per level -- the rule the handoff still lists as wanting + confirmation -- was replaced by one about discovery. Both paths run; both + are now described. + +Two findings of the `as i32` kind were **looked at and left**. The allocator +narrows i128 prefix sums and i64 segment totals to `Px` with `as i32` rather +than `fixed::narrow`, which clamps; but `Fixed::add`, `sub` and `mul` all +wrap deliberately (`MAX` is documented as "compared against, never added +to"), so wrapping is what the ordinary path does with the same overflow, and +`narrow` is for ranges. The epoch check in `RequestArena::segment` stays a +release `assert`: a retained `RequestedLen` would otherwise read a node +belonging to another widget and answer silently, which is worse than the +compare it costs. The negative-weight check beside it is a caller bug like +`div_int`'s and is now a `debug_assert`. + +Two things the sweep did not change and somebody should decide: + +- **The generator's leftover density halved** when `76aaf06` grew bounds: + `Sow::rule` draws `LEFTOVER` 1 time in 8 where `Sow::len` drew it 1 in 4, + and `Free` 2 in 8 where it was 1 in 2. The seed scans are the main defence + for share logic, so the corpus now exercises it half as often. Restoring it + costs a new dump baseline and three fresh scans. +- **`MaxSize` holds `x` and `y` with a hand-written axis match**, where the + framework names every other pair (`Bounds`, `Declared`, `Size`) and indexes + it. Left because the fields are the constructor surface `max_width` and + `max_height` build, and `impl_axis_index!` on a widget reads oddly; it is + three call sites inside one file. + +Verified at `4cb6f68`: format, workspace clippy under `-D warnings` with and +without `layout-diagnostics`, 197 ordinary and 201 diagnostic workspace +tests, the cold dump byte-identical to `0e838e9` across all 34,986 boxes, and +all three seed scans (400 at depth 5 in 69.02s, 1,000 at depth 6 in 174.87s, +2,000 at depth 4 in 330.39s). The scans were run because the rel-base fix +changes what is invalidated, which is exactly what warm-against-cold checks. + ## Performance sweep of #19 (2026-09-20) `0e838e9`, by Codex.