diff --git a/docs/HANDOFF.md b/docs/HANDOFF.md index d23e0e1..7cc33f7 100644 --- a/docs/HANDOFF.md +++ b/docs/HANDOFF.md @@ -8,12 +8,13 @@ found is in `docs/LAYOUT_LOG.md`. Bryan clarified that the built-in attribute should replace the wrapper. Work is in `/home/bob/repos/iris-deferred`, on `layout/one-ask`, at -`2ac0843` (pushed). Bounds now constrain the offer in `Placing::ask`; declarations -are retained as resolved window lengths, so movement does not decide the -bound again. `.max_width`/`.max_height` now edit the same widget, like the -minimum helpers, and `MaxSize` is removed. Preferred lengths and bounds -are independent, so either order of `.width` and `.max_width` works. The -app pin is unchanged. +`ea1f836` (pushed), which is the eleventh sweep over `2ac0843`. Bounds now +constrain the offer in `Placing::ask`; declarations are retained as resolved +window lengths, so movement does not decide the bound again. +`.max_width`/`.max_height` now edit the same widget, like the minimum +helpers, and `MaxSize` is removed. Preferred lengths and bounds are +independent, so either order of `.width` and `.max_width` works. The app pin +is unchanged. The review fixed three cases: `window_holds` erasing an ask's bound crossing (seed 104, depth 5), inactive bound edits failing to update the parent's offer @@ -22,9 +23,10 @@ fractional intrinsic cap being read against its already-allocated slot. The latter has an absolute 75-in-300 geometry check; warm/cold agreement alone cannot catch it. -207 ordinary and 211 diagnostic tests pass, with warning-clean Clippy both -ways. All three layout scans pass (400 depth 5, 1,000 depth 6, 2,000 depth 4), -as do 400 depth-5 trees in each of three deferred-request corpora. All 34,986 +206 ordinary and 210 diagnostic tests pass, with warning-clean Clippy both +ways (207 and 211 at `2ac0843`, less the duplicate test the sweep merged). +All three layout scans pass (400 depth 5, 1,000 depth 6, 2,000 depth 4), as +do 400 depth-5 trees in each of three deferred-request corpora. All 34,986 unbounded cold boxes match `4cb6f68`; bounded offers intentionally change. The final unbounded-path optimization preserves every bounded cold box too. @@ -36,6 +38,22 @@ widget; all three allocation fixtures remain allocation-free after warm-up. `docs/LAYOUT.md` records the contract and measurements. Artifacts are in `/tmp/attribute-bounds/`. +## A quality sweep of the bounds work is submitted on PR #19 + +The eleventh sweep landed at `ea1f836`. It is the first review of `2ac0843` +-- `SizeRule` from an enum to a preferred length beside an independent bound, +the offer constrained in `Placing::ask`, and `MaxSize` removed. Eight +findings, the two largest being a hint read that answered "cannot say" above +the diagnostics and so counted as no read at all, and five spellings of +reading one environment variable across the rigs, which is the class the +tenth sweep found one commit earlier -- there is now one `env` in +`tests/rig/`, used by all six. `bounds_cost` also verified 128 regions inside +its measured loop; the check moved out, and the 0.65% it measured is written +beside it. `docs/LAYOUT_LOG.md` has all eight, with the two rules it tripped +and left. Verified: format, clippy both ways, 206 and 210 tests, 400 depth-5 +trees warm against cold in 64.19s, and the cold dump byte-identical to +`2ac0843` across all 34,986 boxes. + ## A quality sweep of the deferred system is submitted on PR #19 The tenth sweep landed at `4cb6f68` in `/home/bob/repos/iris-deferred`. It is @@ -147,6 +165,7 @@ comparison work above, its review rounds past `cadfba0` are described in above the root, which would pin the tree to pixels. - **The deferred request system's own sweep**, `4cb6f68` -- described at the top of this file and in full in `docs/LAYOUT_LOG.md`. +- **The bounds work's own sweep**, `ea1f836` -- the same, over `2ac0843`. The settled design of the vocabulary rounds is in `docs/LAYOUT.md` under "Three names, and the one argument that says them". Bryan settled the API @@ -202,8 +221,8 @@ 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,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 +since `76aaf06` grew bounds in the trees, and is unchanged through `ea1f836`. +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 diff --git a/docs/LAYOUT_LOG.md b/docs/LAYOUT_LOG.md index bf75e86..387320b 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`. +## Eleventh sweep: the built-in bounds work (2026-09-20) + +Over `2ac0843`, which no earlier round reviewed -- `SizeRule` from an enum to +a preferred length beside an independent `Bound`, the offer constrained in +`Placing::ask`, and `MaxSize` removed. 554 inserted lines over 18 files. Eight +findings, all in `ea1f836`. + +- **A hint read that answered "cannot say" was counted as no read at all.** + The bound check moved out of `Painter::size_request` and into + `Painter::size_hint`, where it returns above `diag::hint_read` and the + `HintHits`/`HintMisses` bump. So a bounded child was neither a hit nor a + miss and left no trace event, in the one place the counters exist to watch. + It is a miss now -- the bound makes the hint `None` rather than returning + early -- with the reason written on it, since the obvious simplification + back to an early return silently loses the counter again. +- **Two loops over both axes, writing the same value.** `draw_widget` grew a + second `for axis in Axis::BOTH` when the rel-base pin moved up to join the + bound's, separated only by a comment. One loop; the pin goes first because + the bound block `continue`s on a share. +- **A twelve-line comment left describing the wrong line.** The new + one-liner about combining the ask's holds was inserted between the comment + about holding an answer to its bound and the code that comment is about. +- **Three things nothing reads.** `Declared::from_axes` lost its only caller + with `Widgets::declared_lens`; `Bounds::from_axes` and `SizeRule::declared` + never had one. `PlaceDesc::from_axes` is the only member of that family + anything calls, so nothing is left half-complete. +- **The shrinker printed a rule with derived `Debug`.** `describe`'s + hand-written printer was replaced by `format!("{r:?}")` and the comment + saying why it was hand-written deleted. Measured: one axis prints as + `SizeRule { request: Some(Linear(LayoutLen { px: 0, rel: 0, leftover: 1 })), + bound: Bound { min: None, max: Some(Len { rel: 0, px: 80 }) } }` -- 130 + characters, twice per widget, in a line that joins every ancestor with + `" < "`. That is the one function whose stated job is output a tree can be + rebuilt from. It prints its parts again: `[x:1 leftover;<80 px;,y:-]`. +- **Five spellings of reading one environment variable.** `bounds_cost` wrote + three in one function (`var().unwrap_or_else`, `var().is_ok_and(== "1")`, + `var().ok().and_then(parse)`) where an identical `fn env` + already stood in `layout_dump.rs`, `layout_diagnostics.rs`, + `revision_cost.rs` and `scenario/mod.rs`. This is the class the tenth sweep + found one commit earlier and fixed one instance of. It is now one function + in `tests/rig/`, used by all six rigs, and a switch is `env(NAME, 0_u8) != 0` + everywhere. +- **A measurement rig verifying inside its measured loop.** `bounds_cost` + asserted 128 regions every frame, where `revision_cost` prints its geometry + once before the loop and asserts nothing inside it. Measured at `MODE=cap + FRAMES=2000`: 5.780B instructions with the per-frame assertions against + 5.743B without, three runs each, stable to 0.005% -- 0.65%, which is far + less than it looked and still not layout. The check now runs once on each + side of the crossing before the loop, and the number is in the comment so + nobody re-adds it or deletes it for the wrong reason. Its module comment + also said it compares against "the former wrapper", which this tree no + longer contains, without saying the other side has to be run at an earlier + commit; and it documented `MODE` and `REDRAW` but not `FRAMES`, and gave no + invocation line where every neighbouring rig gives one. +- **The same fixture built by two tests, and a half-test that could no longer + decide anything.** `a_bound_holds_what_a_widget_answers` and + `a_cap_holds_an_answer_that_overflowed_its_box` both built a 250 window + holding two 200-wide rects in a row under a 300 cap; the first lost its doc + comment in `2ac0843` and the second kept one. They are one test, with the + corners assertion folded into the cap arm and a corrected explanation -- + corrected because the old one said a bound "leaves the box alone", which is + no longer true in general and is true of this fixture only because 300 does + not bind a 250 box. Separately, the second half of + `a_cap_attribute_narrows_the_widgets_box` lost the assertion that + distinguished it when `MaxSize` went (the wrapper measuring 400 while its + child measured 300), leaving `rect().max_width(300)` at root in a 400 + window -- which is the opening of + `a_cap_attribute_is_decided_again_on_either_side_of_the_crossing`, verbatim. + It is now `width(leftover(1)).max_width(300)`, the allocator's path, which + nothing covered at the root. + +Two things the sweep **looked at and left**: + +- **`SizeRule` now holds `Option` inline** where the enum held + `Request(Arc)`, so `at_least` and `at_most` clone the request + to edit a bound. Left because `SizeRequest` is `Linear(LayoutLen)` or an + `Arc` pair, so that clone is a discriminant copy or a refcount bump, never + a deep copy. `size_of::()` is 40 bytes, which the rig prints. +- **`Widgets::set_len` writes the `SizeRule` literally** where `set_min_len` + and `set_max_len` go through `SizeRule::at_least`/`at_most`. A matching + builder was considered and rejected: the two bound helpers exist because a + bound is half of a pair that has to be preserved, and a request has no + other half to preserve -- a method would be a name in front of one field + assignment. + +Verified at `ea1f836`: format, workspace clippy under `-D warnings` with and +without `layout-diagnostics`, 206 ordinary and 210 diagnostic tests (207 and +211 before, less the merged test), 400 depth-5 trees warm against cold in +64.19s, and the cold dump byte-identical to `2ac0843` across all **34,986** +boxes. The scan was run because the `size_hint` restructure touches what a +container records as a dependency; the dump because nothing here was meant to +move a box. + ## Tenth sweep: the deferred request system (2026-09-20) Over the part of #19 no earlier round reviewed -- `76aaf06` and `de1eb7e`