Move the round detail out of the handoff, and say when a scan is owed
The handoff said "three rounds" over four bullets and carried the detail of each, which belongs in the findings log. It now names the four and their commit ranges and points at `LAYOUT_LOG.md`, which gains the naming and sweep round in full. The check section split in two: the cheap gate runs on every change including a rename, because the cold dump is the only thing that catches two same-typed values being swapped; the seed scans run only when the change can alter what layout computes, and never with the tree still moving under them. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
1 parent
d12cbcd499
commit
1442443e78
2 files changed
+81
-39
No files matched your search
+29
-39
@@ -7,32 +7,19 @@ found is in `docs/LAYOUT_LOG.md`.
|
|||||||
## The Iris layout repair is submitted
|
## The Iris layout repair is submitted
|
||||||
|
|
||||||
**Iris PR #19** (`layout/one-ask`) replaces closed #18. The tip is `6c84b6f`,
|
**Iris PR #19** (`layout/one-ask`) replaces closed #18. The tip is `6c84b6f`,
|
||||||
and past the reviewed `cadfba0` it is three rounds:
|
and past the reviewed `cadfba0` it is four rounds, each described in
|
||||||
|
`docs/LAYOUT_LOG.md`:
|
||||||
|
|
||||||
- **The repair.** `add6774` fixes collapsed-share placement, retained mask
|
- **The repair**, `add6774` and `84dad21` -- collapsed-share placement,
|
||||||
ownership and a redraw-on-reparent defect; `84dad21` removes repeated work
|
retained mask ownership, a redraw-on-reparent defect, and repeated work in
|
||||||
in the test harness.
|
the test harness.
|
||||||
- **The vocabulary and the container API**, `5642f20` through `58ce74d`. What
|
- **The vocabulary and the container API**, `5642f20` through `58ce74d`.
|
||||||
they settled is described as design in `docs/LAYOUT.md` under "Three names,
|
- **Naming**, `55df32a` through `40b89c1`.
|
||||||
and the one argument that says them" -- region, placement and rel_base, and
|
- **A sweep over the logic those names exposed**, `8d2b7a5` and `6c84b6f`.
|
||||||
the single `impl Into<PlaceDesc>` argument that replaced `Place`, `Part`,
|
|
||||||
`widget_within` and the separate rel-base argument. **Bryan is still
|
The settled design of the last three is in `docs/LAYOUT.md` under "Three
|
||||||
refining this**, so treat the API as current rather than frozen.
|
names, and the one argument that says them". Bryan settled the API over
|
||||||
- **Named operations and `Index<Axis>`**, `55df32a` and `2807a92`, settled
|
2026-09-17 to 19; it is current, not frozen.
|
||||||
with Bryan on 2026-09-19. A description is built by chaining off the value
|
|
||||||
that says it (`UiSpan::shifted_desc`, `Len::as_desc`), never by a
|
|
||||||
constructor naming the type; `on` is reserved for events, so the lift is
|
|
||||||
`PlaceDescAxis::on_axis`. Arithmetic that needed a comment became a name:
|
|
||||||
`UiSpan::place`, `LayoutLen::without_leftover`/`is_px`/`is_only_leftover`/
|
|
||||||
`declared`, `Holds::covers`. `LayoutHolds` became `AxisHolds` on `x`/`y`,
|
|
||||||
every pair got `Index<Axis>` through `impl_axis_index!`, and the bare
|
|
||||||
`[Option<LayoutLen>; 2]` became `Declared` of `Option<Len>`.
|
|
||||||
- **A sweep over the logic the clearer methods exposed**, `8d2b7a5` and
|
|
||||||
`6c84b6f`. A `Span` carried `start` as a third accumulator when it was
|
|
||||||
always `fixed` and `taken` read together, so both ends of a slot are now
|
|
||||||
read where they are used. A `Scroll`'s draw writes `amt` and `snap_end`,
|
|
||||||
so warm and cold agree only because re-clamping is idempotent; the seed
|
|
||||||
scans build scrolls and never scroll one, so a test now pins it.
|
|
||||||
|
|
||||||
The core design remains sound. Round-to-nearest is still unchanged.
|
The core design remains sound. Round-to-nearest is still unchanged.
|
||||||
|
|
||||||
@@ -40,26 +27,29 @@ Two checkouts share one Git storage: `/home/bob/repos/iris` is the active
|
|||||||
`layout/one-ask` worktree, and `ai-app-2/iris` stays on `main` at the app's
|
`layout/one-ask` worktree, and `ai-app-2/iris` stays on `main` at the app's
|
||||||
`32f6ad8` pin until the integration below is ready.
|
`32f6ad8` pin until the integration below is ready.
|
||||||
|
|
||||||
### How each round was checked
|
### How to check a round
|
||||||
|
|
||||||
Format, workspace clippy under `-D warnings` with and without
|
**Always**, because they cost nothing: format, workspace clippy under
|
||||||
`layout-diagnostics`, the workspace tests both ways, and the three prescribed
|
`-D warnings` with and without `layout-diagnostics`, the workspace tests, and
|
||||||
seed scans -- 400 at depth 5, 1,000 at depth 6, 2,000 at depth 4.
|
the **cold dump**. `layout_dump` over 400 depth-5 trees is 34,492 boxes, and
|
||||||
|
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`.
|
||||||
|
|
||||||
The gate that matters for anything shaped like a rename or a refactor is the
|
**Only when the change can alter what layout computes**: the three seed scans
|
||||||
**cold dump**: `layout_dump` over 400 depth-5 trees is 34,492 boxes, and the
|
-- 400 at depth 5, 1,000 at depth 6, 2,000 at depth 4. They cost about a
|
||||||
compiler cannot catch two same-typed values being swapped. The repair moved
|
quarter of an hour and they exist to find logic that is wrong on some tree
|
||||||
650 of those boxes, all from the collapsed-share correction. Every commit
|
shape, so a rename has nothing for them to find (Bryan, 2026-09-19). Never
|
||||||
since has been byte-identical to `84dad21`, including the whole API rewrite
|
start one and then edit the tree: cargo rebuilds mid-flight and exits 1 from
|
||||||
and the naming work above.
|
a compile error, which reads exactly like a fuzzer failure.
|
||||||
|
|
||||||
## What is next, in order
|
## What is next, in order
|
||||||
|
|
||||||
1. **Bryan's review of #19.** Fixes are themselves unreviewed code: repeat
|
1. **Bryan's review of #19.** Fixes are themselves unreviewed code: repeat
|
||||||
`pre-submit-review` over each round's changes, and re-run the three
|
`pre-submit-review` over each round's changes, and apply the gate above to
|
||||||
fuzzers and the dump diff for edits to `Span`, `Painter`, or
|
whatever each one touched. The ordinary oracle does not replace absolute
|
||||||
`render_state`. The ordinary oracle does not replace absolute geometry and
|
geometry and retained-primitive expectations.
|
||||||
retained-primitive expectations.
|
|
||||||
2. **Integrate the app's Iris capabilities before changing its pin.**
|
2. **Integrate the app's Iris capabilities before changing its pin.**
|
||||||
`32f6ad8` has 45 commits not reachable from the review branch; shared UI
|
`32f6ad8` has 45 commits not reachable from the review branch; shared UI
|
||||||
ownership, richer masks, Android support, and app-side performance work
|
ownership, richer masks, Android support, and app-side performance work
|
||||||
|
|||||||
@@ -6,6 +6,58 @@ nothing here is rediscovered. Each entry says who found it and when.
|
|||||||
it (settled design, the measurement method) belongs in `docs/LAYOUT.md`, and
|
it (settled design, the measurement method) belongs in `docs/LAYOUT.md`, and
|
||||||
the current plan is in `docs/HANDOFF.md`.
|
the current plan is in `docs/HANDOFF.md`.
|
||||||
|
|
||||||
|
## Naming and logic sweep (2026-09-19)
|
||||||
|
|
||||||
|
Settled with Bryan across one session, on the branch past `58ce74d`. Nothing
|
||||||
|
here changed what layout computes: the cold dump is byte-identical to
|
||||||
|
`58ce74d` at every commit.
|
||||||
|
|
||||||
|
**How a description is said.** A `PlaceDescAxis` is built by chaining off the
|
||||||
|
value that says it -- `UiSpan::within_desc`/`shifted_desc`, `Len::as_desc` --
|
||||||
|
never by a constructor naming the type, because a constructor sends the
|
||||||
|
reader back to the start of the line. The `_desc` suffix is what says which
|
||||||
|
type comes out. `PlaceDescAxis::on_axis(axis)` lifts one axis into a pair
|
||||||
|
with the whole box across it; `on` alone was rejected as contentless and
|
||||||
|
reserved for events. `from_axes` is the constructor taking a function, beside
|
||||||
|
the `from_axis` taking one axis and two values.
|
||||||
|
|
||||||
|
**Arithmetic that needed a comment became a name.** `UiSpan::place` was the
|
||||||
|
aligned-placement rule written out three times; `LayoutLen::without_leftover`
|
||||||
|
was the sibling `apply_leftover` never had, at six sites; `is_px`,
|
||||||
|
`is_only_leftover` and `declared` name field comparisons the surrounding
|
||||||
|
comments had to translate; `Holds::covers` was interval containment by hand.
|
||||||
|
Seven module-level functions became methods on the value each took first.
|
||||||
|
|
||||||
|
**Every pair is a struct of two per-axis values, read with `[axis]`.**
|
||||||
|
`LayoutHolds` was four two-element arrays, so none of its own operations
|
||||||
|
could be written once; it is `AxisHolds` on `x` and `y`, and `and`, `covers`
|
||||||
|
and `contains` lost their loops. `impl_axis_index!` gives every pair
|
||||||
|
`Index<Axis>`/`IndexMut<Axis>`, replacing eighteen `axis`/`axis_mut`
|
||||||
|
methods -- `const_index` keeps them usable in const context. The bare
|
||||||
|
`[Option<LayoutLen>; 2]` became `Declared` of `Option<Len>`, which makes
|
||||||
|
"a share is never a declaration" structural rather than two filters and a
|
||||||
|
comment.
|
||||||
|
|
||||||
|
**Two findings in the logic, both one mistake.** A value computed from other
|
||||||
|
state was being stored as if it were state, and in both cases the visible
|
||||||
|
symptom was something that looked like an off-by-one:
|
||||||
|
|
||||||
|
- A `Span` carried `start` as a third accumulator beside `fixed` and `taken`,
|
||||||
|
assigned at three points, when every assignment was `reached(fixed,
|
||||||
|
taken)`. Both ends of a slot are now read where they are used; the variable
|
||||||
|
and two of the three calls per child go, and the gap added after the last
|
||||||
|
child derives nothing rather than needing to be subtracted.
|
||||||
|
- The measuring loop's `cursor` added `px` and `rel` by hand where the
|
||||||
|
placing loop below said `fixed += len.without_leftover()` -- the same sum,
|
||||||
|
one of them named.
|
||||||
|
|
||||||
|
**One property that held but nothing guarded.** A `Scroll`'s draw writes
|
||||||
|
`amt` and `snap_end`, so a second draw at another viewport reads what the
|
||||||
|
first wrote. Warm matches cold only because re-clamping is idempotent and
|
||||||
|
monotone. The seed scans build `Scroll`s and never scroll one, so this was
|
||||||
|
untested; `a_scrolled_view_resized_lands_where_a_cold_layout_puts_it` scrolls
|
||||||
|
four distances, one past the end, then widens. It passes.
|
||||||
|
|
||||||
## Follow-up implementation review (2026-09-19)
|
## Follow-up implementation review (2026-09-19)
|
||||||
|
|
||||||
The ask/place split, window-unit frames, exact validity preimages, and
|
The ask/place split, window-unit frames, exact validity preimages, and
|
||||||
|
|||||||
Reference in new issue
Block a user