From af751599f695e630542d95a8e0d3ead71b2a2336 Mon Sep 17 00:00:00 2001 From: iris-ai <4+iris-ai@noreply.localhost> Date: Tue, 15 Sep 2026 14:32:47 -0400 Subject: [PATCH] Make the handoff enough to start the leftover work from cold Rewrites the opening as state rather than a chronicle of the last few commits, and gives the parked rule its own section: what the owner asked for in her words, the half that is written and works with the code in it, the four ways of asking that were measured and what each did, the seed that shows it most clearly, and the design question underneath -- which is the same one holding OrthoSize. Also folds the declared-length rule into the invariants, brings the vocabulary to leftover, says the suite is eight seconds and must stay that way, and writes out the image replay rather than pointing at a file in /tmp. Co-Authored-By: Claude Opus 5 --- docs/HANDOFF.md | 269 +++++++++++++++++++++++++++--------------------- 1 file changed, 154 insertions(+), 115 deletions(-) diff --git a/docs/HANDOFF.md b/docs/HANDOFF.md index c70ec38..46c6a3a 100644 --- a/docs/HANDOFF.md +++ b/docs/HANDOFF.md @@ -10,118 +10,123 @@ into a standalone framework. Canonical `main` is **`ca2b4b2`** (#17, the headless rig). Sixteen slices are in. -**#18 `split/18-position-chain`** is open and finished apart from one decision: -worktree `/home/bob/repos/iris-pr18`, head `f61e893`, thirty-four commits, -workspace tests passing, fmt and clippy clean. It is LAYOUT.md §2's position -chain, generalised to boxes. After the performance pass (`e5f8b6b` to -`3f7cd82`) came the fuzzers and what they found: the shrinking fuzzer -(`b7caab3`, `386a0d1`), `SetSize` measuring in its declared length +**#18 `split/18-position-chain`** is open: worktree `/home/bob/repos/iris-pr18`, +head `691e3eb`, forty-four commits. 70 tests pass and 8 are ignored, fmt and +clippy are clean, the 100-seed sweep passes, and every reference render is +byte-identical to `upstream/main` -- `tabs`, `view`, `minimal` and `text` at +1920x1200, `tabs` cold at 900x1200, `tabs` resized from 1920x1200 down to it, +and `tabs` with the image replay. `/home/bob/repos/ai-app-2` is on `rustify`, +worktree clean. + +It is LAYOUT.md §2's position chain, generalised to boxes, plus what the +fuzzers found on the way: `SetSize` measuring in its declared length (`c596bf1`), text re-breaking across the interval a break holds for -(`9913194`), and a dirty widget measured where its parent asked rather than -in a box its own answer decided (`02ff8c7`). `f61e893` restores `abs()` in -the rect shader, which `7c50a3e`'s rename had taken with it so that every -window failed shader validation while `cargo test` stayed green, and adds -the device-free shader validation test that would have caught it. The five -reference renders and the resize render are byte-identical across all of -it, and the hundred-seed sweep passes. `/home/bob/repos/ai-app-2` is on -`rustify`, worktree clean. +(`9913194`), a dirty widget measured where its parent asked rather than in a +box its own answer decided (`02ff8c7`), and `abs()` restored in the rect +shader (`f61e893`), which a rename had taken with it so that every window +failed shader validation while `cargo test` stayed green. The last five +commits are 2026-09-15: hairline tests (`4063635`), the shader composing a +position the way the CPU does (`169db7f`), a declared length resolved where +the widget is drawn (`de9ddc0`), the glyph write inlined (`9644971`), and +`rest` renamed to `leftover` (`691e3eb`). -**Fixed, 2026-09-15, by `de9ddc0`: a declared length is resolved where the -widget is drawn.** `Painter` takes a child's `size_hint` in its own box before -drawing it there -- which is what a fraction of a length means, and is the -identity for a caller that already reserved the space -- and `SetSize::draw` -keeps none of it. `rest` is deliberately not resolved there: a part of what is -left over is only a length to the widget dividing one, so it passes up in the -size, the way it already passes up out of a span. A declared length is then -part of the box its parent decided, so a change to one redraws the parent; the -lengths resolved into a box are kept beside it in `ActiveData::declared` and -compared. Assuming instead that any dirty widget which declares a length needs -its parent costs 17% of a frame that dirties 130 of 260 widgets and buys -nothing. All five reference renders, the resize render and the image replay are -byte-identical to `upstream/main`, and the 100-seed sweep passes. +**An earlier reading that `tabs` differed from `upstream/main` by 1,283 pixels +is withdrawn.** It does not reproduce at any commit reachable now; a target +directory shared between two checkouts is the trap the render section warns +about, and both sides render the same bytes twice. -`9644971` is the other half of the cost. Whether the inliner took -`DrawLayers::write` into `Painter::glyphs` turned out to depend on unrelated -code in `iris-core`: the resolution above pushed it out, and a call per glyph -cost 12% of a resize frame with every counter -- widget draws, primitive -writes, text renders -- unchanged. `#[inline]` says it directly. The -random-tree rig is 12.59B instructions per 500 resize frames where it was -13.25B before either change, and the `revision_cost` fixture is 1.286 ms -against 1.289. +## Next: a `leftover` with nothing left over should not draw -The defect it fixed, for the record: +This is the piece to pick up, and it is not a small one. The owner asked for +it on 2026-09-15: -**A declared `rel` length used to be applied twice.** `.width(rel(0.5))` in a 400-wide span puts the `SetSize` at -0..200 and its child at 0..100: the span sizes the box from the hint, and -`c596bf1` then measures the child in the declared length again *inside* that -box. `px` is unaffected, since 200 px of a box already 200 px wide is the whole -box, and `rest` is unaffected because `Len::apply_rest` turns any rest into -`rel(1.0)`, which is the whole box too. Only `rel` doubles, and it doubles -under every placer that honours a hint -- `Span`, `Aligned` and `Scroll` all -call `known_len` -- while a `Pad` or the root, which do not, are correct. It shows in the `text` -example, whose "same words in half the width" panel is 474 px where -`upstream/main` draws 948, and that is 111,923 pixels of difference, so **the -claim above that the five reference renders are byte-identical is wrong for -`text`**; `tabs`, `view` and `minimal` are identical. The structural fix is the -built-in size already queued below -- a declared length resolved once, by the -parent -- rather than a second rule inside `SetSize`, which cannot tell whether -its box is its own declared length or the whole of what its parent had. -Minimal repro: `(inner.width(rel(0.5)), rect()).span(Dir::RIGHT)` at 400 wide, -asking where `inner` landed. +> it shouldn't be a clamp, if it's 0 wide for rest then it shouldn't draw in +> the first place. rel and abs should continue to overflow like before -**`rest` is now `leftover`** (`691e3eb`): `Len::leftover(2)`, `Len::LEFTOVER`, -`Size::LEFTOVER`, the field, and `apply_leftover`. The owner renamed it on -2026-09-15 because "rest" reads as "the remainder of the list" as often as "the -remaining space". ai-app's own calls to `rest(..)` will need the same rename -when its submodule pin moves. +(both quotes predate the rename, so "rest" in them is `leftover`.) -**Parked, and wanted: a `leftover` with nothing left over should not draw.** The -owner asked for it on 2026-09-15 -- "if it's 0 wide for rest then it shouldn't -draw in the first place", with `rel` and `px` continuing to overflow forward as -they do now, and only a length that is *nothing but* `rest` skipped. Written -and reverted: `Span` needs `painter.px_len(axis)` to know whether anything is -left over, since `rel(0.5)` beside 300 px is full at 600 and overfull at 400 -and the difference is not expressible in fractions, and that makes a span's -drawing depend on its own box. Inside a `Scroll`, whose box comes from the -content, seed 13 of `adding_and_removing_span_children` then lands warm and -cold on different answers: the same span holds the same region in both, and -`ActiveData::px` says it drew against 63 px warm and 900 cold, so the leftover -question is answered in two different boxes. `Painter::undraw` -- drop a child -drawn only to measure it, so it is not one of this widget's children this -frame -- is the other half and worked. +and, on what "0 wide" means: -Four ways of asking were measured on 2026-09-15 and none of them holds: +> it looks like you're skipping a child if it has any rest and there is no +> rest space. That is not correct, it should only skip if the child *only* has +> rest space. + +So: a child whose length is nothing but `leftover`, in a span with nothing left +over, is not drawn at all. One that also asked for pixels or a fraction keeps +those and overflows forward, which is what a span does today when nothing in it +is elastic -- three 60 px children in a 100 px box land at 0..60, 60..120 and +120..180. What is wrong today is the other case: put a `leftover` child between +each pair and their lengths come out at -40 px, so the children walk *backwards* +to 0..60, 20..80 and 40..100, which is how a squeezed layout ends up with +things drawn over each other. + +**Half of it is written and works.** `Painter::undraw` takes back a child that +was drawn only to measure it, which a span needs because its first pass draws +any child whose length it cannot get from `size_hint`: + +```rust +/// Takes back a child that was drawn only to find out how long it is: its +/// drawing is dropped and it is not one of this widget's children this +/// frame, as though it had never been asked for. +pub fn undraw(&mut self, id: &StrongWidget) { + self.children.retain(|child| *child != id.id()); + self.size_deps.retain(|child| *child != id.id()); + self.state.remove_rec(id.id(), self.rsc); +} +``` + +`remove_rec` has to become `pub(super)` for it. In `Span::draw`: + +```rust +let any_leftover = total.leftover > 0.0 + && (1.0 - total.rel) * painter.px_len(axis) - total.px > 0.0; +... + if len.leftover > 0.0 && !any_leftover && len.px == 0.0 && len.rel == 0.0 { + painter.undraw(child); + continue; + } + ... + if len.leftover > 0.0 && any_leftover { // the arithmetic already there +``` + +**The half that does not work is knowing whether anything is left over.** It +needs pixels -- `rel(0.5)` beside 300 px is full at 600 and overfull at 400, +and the difference is not expressible in fractions -- so a span's drawing +becomes a function of its own pixel length, and a span's box can be decided +from what it reports. Four ways of asking were measured on 2026-09-15 and none +of them holds: | what the span asks | what happens | | --- | --- | -| its own box (`px_len`), `Span` keeping `OnResize::Scale` | seeds 8, 10 and 13 diverge: a scaled drawing keeps a leftover answer taken in another box | -| its own box, `Span` saying `Redraw` on its own axis when it holds a `leftover` child | ordinary tests pass, the 100-seed sweep still fails at seed 10, and the `many` load goes from 12.59B to 23.57B instructions | -| the box it was offered, threaded through `draw_inner` so it is the parent's current offer rather than last frame's | four generated cases diverge, worse than asking its own box | -| the box it was offered, with a general rule that a drawing which read its box in pixels cannot be scaled along that axis | two generated cases diverge | +| its own box (`px_len`), `Span` keeping `OnResize::Scale` | seeds 8, 10 and 13 of `tests/generated.rs` diverge: a scaled drawing keeps an answer taken in another box | +| its own box, `Span` saying `Redraw` on its own axis when it holds a `leftover` child | the ordinary tests pass, the 100-seed sweep still fails at seed 10, and the `many` load goes from 12.59B to 23.57B instructions a frame | +| the box it was offered, threaded into `draw_inner` so it is the parent's current offer rather than the one on last frame's `ActiveData` | four generated cases diverge, worse than asking its own box | +| the box it was offered, plus a general rule that a drawing which read its box in pixels cannot be scaled along that axis (`on_resize(axis) == Scale && !size_box_inputs[axis]`) | two generated cases diverge | -**The shape of the problem is the one already parked for -`OrthoSize::{Fill, Children}`**: a span's drawing becoming a function of its -own pixel length, when its box can be decided from what it reports. `Branch` -in `iris::random` makes a pixel-dependent choice about *which child to draw* -and the fuzzer is happy with it, so a pixel-dependent draw is not itself the -problem -- what is missing is a way for such an answer to settle when the box -that decided it moves. That is the piece of design to do before this rule, -and it would take `OrthoSize` off the shelf with it. +The failures all look the same from outside: warm draws a widget cold does not, +or the other way round. Seed 13 of `adding_and_removing_span_children` is the +clearest -- the span holds an identical region in both trees while +`ActiveData::px` says it drew against 63 px warm and 900 px cold, so the same +question was answered in two boxes. It is a `Span{dir:X-}` inside a `Scroll`, +which draws its child once to measure it and once in the content length. -**The `tabs` divergence does not reproduce, so nothing waits on the owner.** -Re-measured 2026-09-15 at `f61e893` against `upstream/main` built in its own -worktree: `tabs` at 1920x1200 is byte-identical, and so are a cold 900x1200 and -a resize from 1920x1200 down to it. Both sides render the same bytes twice, so -it is not a flaky shot. The earlier reading of 1,283 differing pixels at x=1056 -and x=1337 is withdrawn; it was taken before `02ff8c7` and `f61e893`, and a -target directory shared between the two checkouts is the trap the render -section already warns about. Byte-identical against `upstream/main` is met. +**The design question, which is the owner's.** A pixel-dependent draw is not +itself the problem: `Branch` in `iris::random` chooses which child to draw from +a measurement and the fuzzer is happy with it. What is missing is a way for +such an answer to *settle* when the box that decided it moves. That is the same +thing already parked for `OrthoSize::{Fill, Children}` -- "reads the span's own +box to rank candidates, which is this cycle with a second face" -- so whatever +shape it takes would take both off the shelf. Agree the shape before building +it; the four rows above are what a variation costs to find out. -`4063635` adds the two hairline tests to `tests/layout.rs`, and `169db7f` -writes the shader's `scalar_within` the way `UiScalar::within` writes it -- -`from + (to - from) * t` rather than `mix` -- so the CPU and the GPU compose a -position with the same arithmetic and one multiplication fewer. All five -renders are unchanged by it. +Reproducing takes: + +```sh +cargo test --workspace # seeds 8 and 13 show here +cargo test --release --test generated -- --ignored a_long_run_of_seeds +IRIS_PHASE=many IRIS_DEPTH=8 IRIS_FRAMES=500 IRIS_DIRTY=130 \ + perf stat -e instructions:u --ignored +``` Check for a review before starting anything, and read the newest `submitted_at` rather than the first result: @@ -186,6 +191,21 @@ Invariants, not history. Everything in `core/src/ui` rests on them. Marking descendants for redraw instead does not terminate: the mark escalates to that descendant's size reader, which re-places the child, which marks it again. Asking first and giving up the whole reuse adds no marks and stops. +- **A declared length is resolved where the widget is drawn, not inside it.** + `Painter` takes a child's `size_hint` in its own box before drawing it there, + which is what a fraction of a length means and is the identity for a caller + that already reserved the space; `SetSize::draw` keeps none of it. `leftover` + is deliberately not resolved there -- a part of what is left over is only a + length to the widget dividing one, so it passes up in the size the way it + already passes up out of a span. A declared length is therefore part of the + box its parent decided, so a change to one redraws the parent: the lengths + resolved into a box are kept in `ActiveData::declared` and compared there. + Assuming instead that any dirty widget which declares a length needs its + parent costs 17% of a frame that dirties 130 of 260 widgets and buys nothing. + Before this, `.width(rel(0.5))` in a 400-wide span drew its child 100 wide: + the span sized the box from the hint and `SetSize` took the fraction of it + again. `px` hid it, since 200 px of a 200 px box is all of it, and so did + `leftover`, which `apply_leftover` turns into the whole box. - **A fixed pixel length keeps its pixel width through any chain.** Both edges of such a part share their box's `rel`, and `within` is a function of that `rel` alone, so the shader's `floor(rel * dim)` is the same for both edges @@ -194,12 +214,12 @@ Invariants, not history. Everything in `core/src/ui` rests on them. `floor`; it cannot be widened or deleted by one. Only a length expressed as a fraction can round away, and that was as true before the chain. Measured 2026-09-15 by `tests/layout.rs`, which composes sixteen hairlines under four - levels of span and padding on `rest(3)/rest(7)/rest(5)` weights and applies + levels of span and padding on `leftover(3)/leftover(7)/leftover(5)` weights and applies the shader's own snapping at four output sizes. **A span short of room takes it from its shares and never from a fixed length**, though a share that has run out does not stop at zero: three 60 px children in a 100 px box overflow forward to 0..60, 60..120 and 120..180, which is what the owner intends, - but putting a `rest` share between each pair gives the shares -40 px and + but putting a `leftover` child between each pair gives them -40 px and walks the children *backwards* to 0..60, 20..80 and 40..100. Clamping a share at zero is the fix, and is not in yet: the shares reach zero and then go past it, so the fixed parts start overlapping each other -- at 400 @@ -886,8 +906,12 @@ cargo clippy --workspace --all-targets -- -D warnings cargo test --workspace ``` -50 tests pass on #18's head. `--workspace` matters: `rig-input` is a crate of -its own. +70 tests pass and 8 are ignored on #18's head. `--workspace` matters: +`rig-input` is a crate of its own. Nothing enabled by default takes a minute -- +the whole suite is about eight seconds, of which `generated.rs` is six -- and +the owner asked on 2026-09-15 that it stay that way, so anything slower than a +minute is `#[ignore]`d. The 100-seed sweep at 68 seconds is the reason that +rule exists. Render checks are the last pass, not the iteration loop -- the owner asked for that on 2026-09-14, since the layout tests cover the CPU part and the shots @@ -900,10 +924,21 @@ cost real time: ``` - The reference shots are `tabs`, `view`, `minimal` and `text` at 1920x1200, - plus `tabs` with a replay that switches to the image tab and adds two images. - A `.touch` line is ` down|move|up ` in the output's own pixels; the + `tabs` cold at 900x1200, `tabs` resized from 1920x1200 down to it, and `tabs` + with a replay that switches to the image tab and adds two images. Compare + them against the same shots taken from a worktree at `upstream/main`; the + bar every slice has met is byte-identical. +- A `.touch` line is ` down|move|up ` in the output's own pixels; the tab strip is at y=24 and the five tabs at x = 192, 576, 960, 1344 and 1728, - with the image tab's add button near (1836, 1116). + with the image tab's add button near (1836, 1116). The image replay is six + lines, and writing it out again is quicker than looking for the file: + + 0 down 1728 24 + 80 up 1728 24 + 400 down 1836 1116 + 480 up 1836 1116 + 800 down 1836 1116 + 880 up 1836 1116 - **A resize is its own case**, and `--resize` is it: the output changes under the running app, and what it lands on must match a cold start at that size byte for byte. That caught both of #16's defects and nothing in `cargo test` @@ -989,14 +1024,16 @@ is no longer part of it -- a child offset is just placing the child, which Then, roughly in dependency order: -- **Built-in alignment, and probably size**, which the owner moved ahead of the - rest on 2026-09-14. Reproduced in the harness: `.width(rel(0.5))` inside a - `Dir::DOWN` span reports 200 of 400 and is handed the whole 400, and a `Pad` - in between does not change that. **Do not "fix" it by reading the child's - ortho `size_hint`** -- a `Pad` between the `SetSize` and the span has no hint - of its own, so the declared width silently goes back to filling. It works - only when nothing is in the way. Alignment has to belong to the widget rather - than be discovered through whatever happens to sit on top of it. +- **Built-in alignment**, which the owner moved ahead of everything else on + 2026-09-14. Half of the size question is answered -- a declared length is + resolved once, by whoever draws the widget -- but alignment still is not. + Reproduced in the harness: `.width(rel(0.5))` inside a `Dir::DOWN` span + reports 200 of 400 and is handed the whole 400 on the *other* axis, and a + `Pad` in between does not change that. **Do not "fix" it by reading the + child's ortho `size_hint`** -- a `Pad` between the `SetSize` and the span has + no hint of its own, so the declared width silently goes back to filling. It + works only when nothing is in the way. Alignment has to belong to the widget + rather than be discovered through whatever happens to sit on top of it. Two things beyond the bug argue for it. Built-in size removes `SetSize`, and with it a wrapper reporting one size while handing its child the whole box. @@ -1016,7 +1053,7 @@ Then, roughly in dependency order: unchanged drawing should sit, which is the alignment work above. - **`UiRenderState` behind `Rc>`**, queued by the owner on 2026-09-13 as fundamental, and especially so for text. -- **`Len`, `LayoutLen` and dp.** The archive splits the type so that `rest` is +- **`Len`, `LayoutLen` and dp.** The archive splits the type so that `leftover` is unrepresentable where it is meaningless (a padding), and folds a density in at resolve time. 21 files mention `Len`, so it is wide but shallow. - **The input restructure** -- `src/default/sense.rs` becomes `src/rsc/sense.rs` @@ -1123,7 +1160,9 @@ was measured. Copy `tests/revision_cost.rs` into either to compare. `iris-old-cmp` (`43ce8c7`) and `iris-main-cmp` (`upstream/main`) are unpatched worktrees with target directories of their own, for the instruction counts and the reference renders above; `43ce8c7` calls it `Len::abs` where this branch -says `Len::px`, which is the only edit a copied fixture needs. +says `Len::px`, which is the only edit a copied fixture needs. Keep +`iris-main-cmp` -- every render comparison starts by taking the same shot +there. ## Cautions