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