Record the eleventh sweep, over the built-in bounds work
This commit is contained in:
1 parent
17423a57b8
commit
5f01dc65c8
2 files changed
+123
-11
No files matched your search
+30
-11
@@ -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
|
||||
|
||||
@@ -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<T: FromStr>`
|
||||
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<SizeRequest>` inline** where the enum held
|
||||
`Request(Arc<SizeRequest>)`, 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::<SizeRule>()` 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`
|
||||
|
||||
Reference in new issue
Block a user