Record what making the frame a window length corrected

Step 2 of the layout plan is done in the experiment checkout, at 1512d84
and 23523ee. The handoff carries the commits and what is left; LAYOUT.md
carries the settled rule, which is now one coordinate unit with the frame
a length of it and both pins beside each other; the log carries what
implementing the plan corrected in it, including the four that would have
shipped as wrong layout and the two rig gaps that hid them.
This commit is contained in:
iris-ai committed 2026-09-19 00:21:44 -04:00
1 parent da1807664a
commit 36ce66554b
3 files changed
+196 -146

No files matched your search

+61 -131
View File
@@ -14,24 +14,31 @@ 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 **`0ef87eb`**, two commits over `4328eac` (the head of
`wip/transparent-frames`, which is unchanged). It replaces the step 3 plan
below with a smaller protocol change, and it passes every check:
**`wip/one-ask`** at **`23523ee`**, four 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, and `1512d84` and `23523ee` make the
frame a length of the window. It passes every check:
| check at `0ef87eb` | result |
| check at `23523ee` | result |
| --- | --- |
| `cargo fmt --all --check`, clippy `-D warnings`, with and without `layout-diagnostics` | clean |
| `cargo test --workspace` (debug) | 115 suite, 20 core, 11 generated, all green |
| the two decided-box pins from step 1, and the new seed 946 pin | **pass** (the first two were red at `4328eac`) |
| `cargo test --workspace` (debug) | 122 suite, 20 core, 11 generated, all green |
| `cargo test --release --test generated` | 11/11 |
| shrinker, 400 seeds, depth 5, all fifteen cases | agree, 56 s |
| 2000-seed depth-4 scan, all fifteen cases | agree, 262 s |
| 1000 seeds at depth 6 | agree, 148 s (found seed 946 at `3091fb8`, fixed and pinned in `0ef87eb`) |
| shrinker, 400 seeds, depth 5, all sixteen cases | agree, 66 s |
| 1000 seeds at depth 6 | agree |
| 2000-seed depth-4 scan, all sixteen cases | agree |
Not done: the renders and the `tabs` replay (step 2), and the pre-submit
review of the two commits.
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
losing the padding twice, a root resolving its own rule twice, a rule changed
over two pads relocating the column under them instead of dividing it again,
and a resize leaving a short scroll's window-tall content where it was. Each
is pinned as a named test.
The worker's uncommitted step 3/4 experiment is preserved as branch
Not done: the renders and the `tabs` replay, the retained-cost work, the
renames, and a pre-submit review of the three commits as one diff.
The worker's older step 3/4 experiment is preserved as branch
`wip/step3-experiment` (one commit over `4328eac`) and as
`~/repos/iris-step3-experiment.patch`. It is evidence, not the protocol.
The app's Iris pin is unchanged.
@@ -77,7 +84,7 @@ is the third plan for this repair and the next one should not repeat it:
**A widget draws once, in the box it is asked in. Its answer is placed inside
that box by re-expressing the drawing. Nothing is drawn again in a box an
answer chose.** `Holds` is a contract about the ask box alone, consulted only
to decide whether a re-ask can be skipped. This is `draw_inner` at `3091fb8`:
to decide whether a re-ask can be skipped. This is `draw_inner` at `1512d84`:
```rust
let reused = (!stale)
@@ -85,8 +92,7 @@ let reused = (!stale)
.flatten()
.and_then(|answer| {
let extent = placed_extent(part, answer.0, declared, info.fill(), align);
self.try_reuse(id, frame, part, extent, info, rsc)
.map(|()| answer)
self.try_reuse(id, part, extent, info, rsc).map(|()| answer)
});
let answer = reused.unwrap_or_else(|| {
if old.is_none() {
@@ -95,7 +101,7 @@ let answer = reused.unwrap_or_else(|| {
let answer = self.draw_at(id, part, info, old.take(), rsc);
let extent = placed_extent(part, answer.0, declared, info.fill(), align);
if extent != part {
self.reposition(id, frame, extent, info, rsc);
self.relocate(id, extent, info, rsc);
}
answer
});
@@ -118,8 +124,10 @@ with a new call that never runs the body:
pub fn place_at<W: ?Sized>(&mut self, id: &StrongWidget<W>, place: [Place; 2])
```
A frame is narrowed by a *length* of the parent's frame, never a region, and
is put back into the part by the child's alignment on every placement:
A frame is narrowed by a *length of the window*, never a region and never a
fraction of the parent's frame -- a row's slot cannot be written as a
fraction of the row. The box stays whatever `place` names; only a declaration
places the box inside it, by the child's alignment, on every placement:
```rust
pub fn widget_at<'s, W: ?Sized>(
@@ -150,9 +158,11 @@ let used = match len.leftover > Weight::ZERO && shares {
};
```
`Stack` asks non-sizing children in the box the sizing child decided, with
the frame narrowed to it on every axis that is not a share, and `Scroll` asks
its content once in the viewport and `place_at`s it to the scrolled offset.
`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
`Scroll` asks its content once in the viewport and `place_at`s it to the
scrolled offset.
A local redraw asks the retained question again -- the same place of the box
the parent was *asked* in -- and, if the answer stands, puts the fresh drawing
@@ -216,12 +226,11 @@ row by exactly the icon's width. With the pad in a share instead, both are
the share less 32. An icon *after* the padded text overflows; a user who
wanted otherwise meant `leftover`.
**The experiment does not implement this**, and cannot in its representation:
a narrowed frame is its own box there (`frame_and_extent` sets the extent to
`FULL` of it), so 868 for `rel` and 844 for the wrap cannot both hold. `Pad`
still insets only the box through `Part::Of` and forwards the frame whole,
which makes `rel(1.0)` under it 900. Step 2 below is the representation
change that expresses the rule.
Implemented at `1512d84`. `Pad` reads its own frame (`Painter::frame_len`,
which pins it), takes the padding off, and hands that down as the child's
frame, while the box it gives is the inset part of its own box. The two are
different lengths whenever the box is narrower than the frame -- which is
exactly the wrapping case above.
### A share never adds room beyond the deciding box
@@ -235,114 +244,31 @@ Unchanged; see `docs/LAYOUT.md`.
## Implementation plan
Work in `/home/bob/repos/iris-layout-experiment` on `wip/one-ask` from
`3091fb8`. Make each step a warning-clean commit and run its named checks
`23523ee`. 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.
### 1. Review the two commits
Anything a fuzzer finds is shrunk first (`SHRINK_SEED=<seed>
SHRINK_DEPTH=<depth> SHRINK_CASE=<case>`, which now prints each level's
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.
Run the pre-submit review over `3091fb8` and `0ef87eb` as one diff against
`4328eac`; they were written as a probe and reviewed only by their tests.
Anything a fuzzer finds later is shrunk first (`SHRINK_SEED=<seed>
SHRINK_DEPTH=<depth> SHRINK_CASE=<case>`), pinned as a named test in
`tests/cases/unsettled.rs`, then fixed under the rule above. Do not add a
second draw back.
### 1. Review the four commits
Run the pre-submit review over `3091fb8`, `0ef87eb`, `1512d84` and `23523ee`
as one diff against `4328eac`. `1512d84` rewrote much of what the first two did to
`painter.rs` and `render_state.rs` and was reviewed as it was written, but
the three have never been read as one change, and the parts of the one-ask
protocol it did not touch -- `place_at`, the twice-asked deferral, `Span`'s
two passes -- were written as a probe and reviewed only by their tests.
### 2. Make the frame a length and the box a region
The frame stops being a coordinate system. **There is one coordinate unit,
the window**: every box in the tree is a region in window units, a widget's
frame is a *length* in the same units and is only what fractions resolve
against, and a region node's move entry is a *translation* -- a region `rel
1` long anchored at the node's box start -- rather than a box. The last
point is forced, not chosen: a node whose unit was its frame would need its
box expressed in frame units, which is a division of two lengths and not a
`rel + px` length (a `.region_node()` child of a pad in a span room is the
case). **Decided by Bryan, 2026-09-18: yes.** It changes one cost. A node that
moves, or scrolls, is one entry write whatever narrowed it (today only a
narrowed node gets that; a transparent one is re-expressed), and a node
whose box changes *length* at the same window size is re-expressed as a
subtree (today a narrowed node is one write). The shader's `resolve_move`
composes a `rel 1` entry as a translation already and does not change.
```rust
pub struct Painter<'a> {
/// This widget's frame as a length of its region node's box, per axis:
/// what a fraction it declares or reports, and a `From` span under it,
/// are fractions of. A length and not a region, so the box need not be
/// the frame -- padding takes from both without either becoming the other.
pub(super) frame: UiVec2,
/// Where this widget's drawing goes, in the node's coordinates.
pub(super) extent: UiRegion,
/// The window in pixels: the frame in pixels is `frame.to_px(window)`
/// and the box's is `extent.size().to_px(window)`. Every `Holds` range
/// is in these pixels, so `in_parent` combines them without `through`,
/// and a widget's own frame reads convert once: `Holds::at(frame_px)
/// .through(frame)`.
pub(super) window: PxVec2,
...
}
```
`Part::of` does not change: with one unit a `From` span is already a
window-unit length, so a span's cursor, `far`, `room` and slots stay as they
are and only its shares boundary reads `room.to_px(window)`. Two places
resolve through a frame: a reported fraction (`in_parent_frame`, now
`size.within_len(child_frame)`) and a declared one (`narrowed_by`, then a
`Sized` part placed by alignment). A widget's own frame reads convert once,
`Holds::at(frame_px).through(frame)`, the exact preimage of the `to_px` that
made them. `recompose_subtree` and `reposition` become one operation, since
a child's box is always `Part::of(parent box, place)`; `asked_px` is the
window.
A child's frame is `narrow.within_len(parent.frame)` where the caller or a
rule narrowed it, else the parent's; a declared length also decides the box
(`len` placed in the part by alignment, as `frame_and_extent` does today),
where a caller's `narrow` decides the frame alone and the place decides the
box. `Pad` then says both halves of the rule:
```rust
let inset = |lead: Px, trail: Px| {
Place::Within(Part::Of(UiSpan::new(
Len::from_parts(Rel::ZERO, lead),
Len::from_parts(Rel::ONE, -trail),
)))
};
let less = |lead: Px, trail: Px| Some(Len::from_parts(Rel::ONE, -(lead + trail)));
let inner = painter
.widget_at(
&self.inner,
[less(self.padding.left, self.padding.right), less(self.padding.top, self.padding.bottom)],
[inset(self.padding.left, self.padding.right), inset(self.padding.top, self.padding.bottom)],
)
.size();
```
What this changes in the core, and why it is not merely more code:
- `Painter::resolve` becomes `region.within(&self.extent)`; the second
`within` through the frame goes.
- `in_parent` no longer maps a child's frame holds through the frame length
per level: every widget holds for the window's pixels, so the holds `and`
directly, at region nodes too.
- `DrawInfo::frame`/`frame_abs`, `ActiveData::frame`/`frame_abs`, `local`
in `Placing`, `recompose_subtree` and `asked_px` all change meaning:
`frame` is a `UiVec2` length, `frame_abs` goes, a node's move entry is a
`rel 1` region at the node's box start, and `asked_px` becomes the window
size.
- `Stack` needs a part that is "a box of this length, placed by the child's
alignment" (`Part::Sized(Len)`, the same thing a declared length does),
since `narrow` no longer makes the box.
The random rig and the shrunk tests do not change. Expected precision is the
same as today (one `rel × rel` per level of narrowing). Checks: the suite,
the fast oracle, the 400/5 shrinker, then both long fuzzers; the `many`,
`resize` and `cold` counters must not move by more than the reposition work
this removes; and two new tests, `rel(1.0)` inside `.pad(16)` after a 24 px
icon is 868 px while the wrapped text beside it wraps at 844, and the same
inside a 450 px share is 418 for both. `examples/text.rs` then renders
inside its padding.
**Done at `1512d84` and `23523ee`.** The settled rule is in `docs/LAYOUT.md` under
"Frames, decided boxes and padding"; what implementing it corrected in the
plan -- three of them wrong layout that would have shipped -- is in
`docs/LAYOUT_LOG.md`.
### 3. Render and replay
@@ -358,8 +284,12 @@ likely to show here; inspect every intentional difference and record it.
Rename `ActiveData::offer_part` to `part`, `offer_place` to `asked`, `place`
to `placed`, and `DrawInfo` likewise; delete `ActiveData::measured` in favour
of reading `answer`; delete `answers_at` if `resize` is its only caller and
inline it. Every use was written against the old names on purpose to keep
the probe's diff readable; do this as one mechanical commit. Suite, oracle.
inline it. Rename `LayoutHolds::frame` to `window` and `Painter::frame_own`
with it: since `1512d84` those ranges are window pixels, and the only thing
that writes them is `Painter::window_holds`; the frame's own entry is the
`frame_len` pin beside it. Every use was written against the old names on
purpose to keep the probe's diff readable; do this as one mechanical commit.
Suite, oracle.
### 5. Restore the expected retained cost
@@ -423,7 +353,7 @@ SHRINK_CASE=all SHRINK_SEEDS=2000 SHRINK_DEPTH=4 \
cargo test --release --test shrink -- --ignored --nocapture
```
The last line is the 2000-seed depth-4 scan over all fifteen cases; the
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.