From a2717954081936294cd7a30ba6a455fece35272c Mon Sep 17 00:00:00 2001 From: iris-ai <4+iris-ai@noreply.localhost> Date: Wed, 16 Sep 2026 17:09:45 -0400 Subject: [PATCH] Record the partial-repaint fix, the truncating multiply, and what is left Three commits on iris #18, now at `08c9d5a`. `aea878d` fixes the ignored `redrawing_one_widget_does_not_move_what_scrolls_ around_it`, and the bisect this document reported for it was a red herring: `95fb4f9` cannot be the cause, since the failing tree has no `Masked` in it. The cause was `redraw` skipping a dirty widget's second ask whenever the offer and the final box were the same *length*. It also closed three of the four unreduced shrinker leads and two of the three depth-6 oracle seeds; the two that are left are written up with their reductions. `60367d8` stops `Holds::through` allowing for a rounding that did not happen, which compounded down a chain of widgets each taking the whole of its parent. `08c9d5a` makes `Fixed::mul` drop to the step below (Bryan: truncation is preferable at this point), with the two short-circuits priced against the old multiply. Measured against the float head in `iris-float-cmp`, which is the comparison that was asked for: 1,800M instructions and 715M cycles against 1,761M and 688M, from 1,915M and 777M -- three quarters of the instruction gap and two thirds of the cycle gap closed. The float pair's work counters differ and the fixed-point pair's twenty-five are identical, so only the second number is same-work-at-a-different-speed; both are in the table. Also recorded: `cycles:u` returns garbage readings in this VM the way `instructions:u` does, and a whole set of three can be garbage at once; `tabs` is no longer a byte-identical render and why; and Bryan's idea of composing in `i64` and narrowing only when storing, with what is already protected, what is not, and the test that would say it worked. Co-Authored-By: Claude Opus 5 --- docs/HANDOFF.md | 271 ++++++++++++++++++++++++++++++++++-------------- 1 file changed, 195 insertions(+), 76 deletions(-) diff --git a/docs/HANDOFF.md b/docs/HANDOFF.md index 953256e..4136c90 100644 --- a/docs/HANDOFF.md +++ b/docs/HANDOFF.md @@ -8,9 +8,9 @@ log. Canonical Iris `main` is **`ca2b4b2`** (#17, the headless rig). **#18 `split/18-position-chain`** is open in `/home/bob/repos/iris-pr18`; its local -head is **`98d4e98`**, seventy-two commits. Built-in alignment is complete there; -see "Built-in alignment" below for the retained-layout details. No PR reviews -were present when checked on 2026-09-15. +head is **`08c9d5a`**, seventy-five commits, pushed. Built-in alignment is +complete there; see "Built-in alignment" below for the retained-layout +details. No PR reviews were present when checked on 2026-09-15. The current head completes LAYOUT.md ยง2's position chain and the requested `leftover` behavior. A child whose length is only `leftover` is not drawn when @@ -89,26 +89,30 @@ rewrite: `core/src/ui/holds.rs`, the retained tests, and the generated cold-layout oracle pin those rules. Seeds 10 and 86 are now in the ordinary generated set. -## A warm frame moves a scrolled span, and the rigs that found it +## A warm frame moved a scrolled span, and the rigs that found it -**The defect.** Marking one widget for redraw moves a span inside a scroll by -24px against a cold tree of the same state. Five widgets, in -`tests/cases/unsettled.rs` as -`redrawing_one_widget_does_not_move_what_scrolls_around_it`, **ignored until -it is fixed** so the suite stays honest rather than red: +**Fixed in `aea878d`, and the bisect was a red herring.** `git bisect` named +`95fb4f9`, the commit that made `Masked` report `Size::LEFTOVER`; the failing +tree has no `Masked` in it and that commit changes nothing else but a +`debug_assert`, so the attribution was wrong and the write-up built on it +("`Scroll` clips through a `Masked`") was too. Read the tree rather than the +bisect next time a commit that cannot be the cause is named. + +The tree, five widgets, now running in `tests/cases/unsettled.rs` as +`redrawing_one_widget_does_not_move_what_scrolls_around_it`: Scroll(Y) { Span(DOWN) { Scroll(X){ text } as a region node, rect 87x24 } } -`git bisect` names **`95fb4f9`**, which made `Masked` report `Size::LEFTOVER` -rather than its inner's size. `Scroll` clips through a `Masked`, so what the -outer scroll is told its content measures now depends on whether the inner -subtree was redrawn this frame. The reasoning in that commit is sound -- a -widget that clipped its contents cannot honestly ask for more than its box -- -so the fix is not to revert it but to stop the outer scroll's content -measurement from going through the clipped report. Not attempted yet. - -24px is exactly the sized child's height, so this is a whole child's worth of -layout rather than a rounding. +`UiRenderState::redraw` asks a dirty widget in the box its parent asked it in +and then again in the box its parent chose from that answer, and it skipped +that second ask whenever the two were the same **length**. They are not the +same box: the outer scroll offers its whole viewport and places the span 24px +above it, snapping to the end of a content 24px longer than its box, so the +offer was as long as the final box and 24px below it. A region node writes +the box it drew in into its own move entry, so the inner scroll stayed where +the offer put it. `d3b0ebf` had already compared whole boxes for +`parent_must_place` and left this one a length comparison -- one decision with +its two halves disagreeing. **The rigs.** `98d4e98` splits `iris::random::grow` into `plan(seed, depth, &edits)` and `build(rsc, &plan)`. A seed cannot be made smaller, which is why @@ -124,19 +128,53 @@ whatever it failed on. Seeds still mean the trees they meant: the 1000-seed depth-6 run gives the same three failures with the same boxes before and after the split, which is the check to repeat if the generator is touched again. -**Unreduced leads**, from the shrinker at 400 seeds of depth 5 -- a level it -had never run, since it used to grow its own trees and know five cases: -**seeds 2 and 288 on `region-node`**, and **174 and 175 on `repaint-some`**. -The `repaint-some` pair is probably the defect above. The `region-node` pair -is not obviously the same thing and is the next one to reduce. +**What is left, at `08c9d5a`.** Two failures, both older than the fix above +and neither reduced to a test yet: + +- **Shrinker seed 288 on `region-node`**, 400 seeds of depth 5. Reduces to 11 + widgets from 42, and widget 34 -- a `Text` under `Span(Y-)` under two + `Stack`s -- sits at y 1032.80..1261.60 warm against 1024..1270.40 cold, so + it is inset 8.8px at each end rather than displaced. The plan is a + `Scroll(X)` over a `Span` of three rects with `region_node` set on the + first and third, inside two `Stack`s. +- **Oracle seed 326 at depth 6 on `repaint-some`**. Reduces only to 43 + widgets from 205, around two `Branch`es -- the widget whose shape depends + on a measured length -- with the differing `Text` at + `Text < Branch < Branch < Scroll < Span < Pad`, 88px out. The `Branch`es + are why it will not reduce further, and probably why it diverges. + +Fixed with `aea878d`, from what this section used to list as unreduced leads: +shrinker seeds 174 and 175 on `repaint-some` and seed 2 on `region-node`, and +oracle seeds 18 and 190 at depth 6. The `repaint-some` guess was right. **Depth is what finds these.** Nothing fails at 100 seeds of depth 4, which is -all the oracle has ever routinely run. Seeds 18, 190 and 326 fail at depth 6 -of 1000. Before fixed point, `5ed9e87` fails that same run on seed 40 after a -resize and on nothing else, so the partial-repaint family is new since then -and the resize one is older than all of it. +all the oracle has ever routinely run. Before fixed point, `5ed9e87` fails the +1000-seed depth-6 run on seed 40 after a resize and on nothing else, so the +partial-repaint family is newer than that and the resize one is older than all +of it. -## Verification at `394d514` +## Verification + +**At `08c9d5a`**, the current head: + +- `cargo fmt --all --check`, `cargo clippy --workspace --all-targets -- + -D warnings`, `cargo test --workspace`: sixteen targets green, 80 suite + tests, 17 core unit tests, 11 generated cases. What stays ignored is the + long runs and the profiling rigs; no known defect is ignored any more. +- The release oracle at 100 seeds, in 14.3 s. +- All fifteen shrinker cases at 400 seeds of depth 5, in 81 s: seed 288 on + `region-node` fails and nothing else does. +- Depth-6 oracle seeds 18, 190 and 326 run one at a time: 18 and 190 pass, + 326 fails. +- `view`, `minimal`, `text`, `random` and the `tabs` touch replay render + byte-identical at 1920x1200 against `98d4e98`. `tabs` cold and + `tabs`-before-the-gesture differ on 4,664 of 2,304,000 pixels; see + "Performance" for why, and take the oracle rather than `tabs` as the check + from here. +- Twenty-five rig work counters identical between `60367d8` and `08c9d5a` on + the `many` fixture, which is what makes their timings comparable. + +**At `394d514`**, kept because several of these have not been re-run since: `4febabf` on top of it -- `Fixed` wrapping rather than saturating -- passed the workspace tests with all features (105), the release 100-seed oracle in @@ -209,21 +247,30 @@ an X span's `OrthoSize::Full` became a size rule. Measured on the fixed-shape fixture, seed 1, depth 8, 500 frames of `many`: -| | drawn widgets | primitive writes | instructions | cycles | -| --- | ---: | ---: | ---: | ---: | -| `5ed9e87`, before fixed point | 100 | 4,272 | 1,763M | 693M | -| `4cbb242` | 97 | 3,951 | 2,093M | ~819M | -| `4febabf`, wrapping | 97 | 3,951 | 1,912M | 771M | +| | drawn widgets | primitive writes | instructions | cycles | IPC | +| --- | ---: | ---: | ---: | ---: | ---: | +| `5ed9e87`, before fixed point | 100 | 4,272 | 1,761M | 688M | 2.561 | +| `4cbb242` | 97 | 3,951 | 2,093M | ~819M | | +| `4febabf`, wrapping | 97 | 3,951 | 1,912M | 771M | 2.48 | +| `60367d8`, rounding | 97 | 3,951 | 1,915M | 777M | 2.465 | +| `08c9d5a`, truncating | 97 | 3,951 | 1,800M | 715M | 2.516 | -The two ends were re-measured 2026-09-16 as medians of nine runs of two -freshly built binaries and are the numbers above; `4cbb242`'s row is still -the single reading it was taken from. Raw, the head is 1.085x the float head -on instructions and 1.113x on cycles. Normalised by primitive writes it is -1.17x and 1.20x; per drawn widget 1.12x. **Take none of those for the grid's -cost.** The two heads do different work rather than the same work at -different speeds -- `4febabf` remaps 53 subtrees a frame and redraws 27 -where `5ed9e87` remaps 38 and redraws 43 -- and dividing by primitive writes -does not correct for a different mix of remapping and redrawing. +The three ends were measured as medians of 25 runs of binaries built for the +reading; `4cbb242`'s row is still the single reading it was taken from, and +`4febabf`'s is medians of nine from 2026-09-16. `5ed9e87`'s numbers +reproduced to within 0.1% and 0.7% across those two sessions, which is what +says the rig is sound. + +**`60367d8` and `08c9d5a` are the one pair here that can be compared +directly**: all twenty-five of the rig's work counters are identical between +them, so -6.0% instructions and -8.0% cycles is the same work at a different +speed. Against the float head the head is +2.2% and +3.9%, down from +8.7% +and +12.9% -- but **take neither for the grid's cost.** Those two do +different work: the float head draws 100 widgets to the head's 97, writes +4,272 primitives to 3,951, remaps 38 subtrees a frame and redraws 43 where +the head remaps 53 and redraws 27, and renders 49 texts to 42. Dividing by +primitive writes does not correct for a different mix of remapping and +redrawing. **The fused multiply-add this section used to name does not exist, withdrawn 2026-09-16.** `5ed9e87`'s binary holds no FMA instruction at all, @@ -250,6 +297,9 @@ on, and six of them checked for the others. | all three | **1,803M** | **715M** | | branchless `shift_round`, both short-circuits out | 1,964M | 773M | +The "all three" row is the head as of `08c9d5a`; it re-measures there at +1,800M and 715M, which is the 0.2% a rebuild moves. + Taking the rounding out alone removes 43M instructions and moves cycles by +2M, which is inside the noise: the grid's arithmetic is instructions the machine has spare issue width for, not time. @@ -273,27 +323,18 @@ pays, and only the whole set of cuts reaches it. That is where to push: a cheaper `Holds` or a smaller `Len::within` is worth trying for this reason rather than for its own instruction count. -**What truncating costs was then measured rather than asserted.** The first -write-up of this section said it "breaks the one rounding the grid is for" -and left it there, which is the fault it withdraws the fused multiply-add -for. Run, it fails one exactness test by a step -- -`an_uneven_nesting_still_gives_every_share_the_same_length`, where a share -reaches 199.99902 of a 200px row -- and it does **not** fail the -warm-against-cold oracle any harder than the head does: 100 seeds pass, and -1000 seeds of depth 6 give the same three failures on the same seeds, -differing by 0.002px. So the case against it is a share stopping a -thousandth of a pixel short of its row, which is below what an even number -of pixels draws anyway, rather than layout drift. Whether that buys 57M -cycles is a decision, not a defect. - **Cycle noise is larger than this section used to claim.** Across ten sets of nine to eleven runs, each of one unchanged binary, cycles spread 1-3% usually and 6.7% in the worst set -- not the 0.23% recorded from three runs, which was a lucky set read as the machine's precision. Worse, `instructions:u` in this VM intermittently returns a garbage value -- 11.0e9, 25.8e9 and 28.4e9 against a true 1.9e9, in roughly a quarter of -readings. Take medians of nine or more, drop anything off by a factor, and -do not trust a single `perf stat`. `ex_div_busy` was the one counter that +readings. **`cycles:u` does it too**, seen 2026-09-16: 7.1e9 and 11.3e9 +against a true 715M, and in one set of three runs all three cycle readings +were garbage while all three instruction readings were good -- so a filter +that drops anything off by a factor has to report how many readings it kept, +or an empty sample reads as a zero. Take medians of nine or more and do not +trust a single `perf stat`. `ex_div_busy` was the one counter that held to 0.1% across runs. **Instruction counts hold to 0.02% within a binary and move 0.5% across a @@ -364,12 +405,34 @@ the 2.7% the instruction share predicted. `from_f32` still clamps and `Holds` keeps its saturating `narrow`, since a range of box lengths past `i32` really is unbounded. -**One posture question is still open, with its number.** `shift_round` -rounds halves away from zero so a value and its negation round to the same -distance, which is a sign branch on every multiply. Rounding halves up -instead measured **-1.6% instructions, -2% cycles** (1,888M, ~757M), and -would let a flipped span land a step from its mirror image at exact halves. -Not applied; Bryan's call. +**`08c9d5a` makes `Fixed::mul` drop to the step below rather than round** +(Bryan, 2026-09-16: truncation is preferable at this point). The three cuts +this section priced together landed as one commit, since separately they are +worth 61M instructions and 8M cycles and together 115M and 62M -- the whole +of the superadditivity above. `Fixed::scaled` was its own zero short-circuit +and is gone; `UiSpan::within` lost both `is_full` tests and is inlined again, +which `nm` confirms. + +What it costs, all of it measured rather than asserted: + +- A share lands a thousandth of a pixel short of its row rather than on it. + `an_uneven_nesting_still_gives_every_share_the_same_length` now says what + is still exact -- each share starts where the last ended, the row ends at + its own edge, each edge is on the even division or one step below. +- A value and its negation are no longer the same distance from where they + came, since the drop is toward negative infinity on both sides of zero, so + a flipped span can sit a step from its mirror image. The alternative that + keeps that symmetry is the sign branch, which is part of what was bought. +- `tabs` is no longer a byte-identical render: 4,664 of 2,304,000 pixels + differ, in single-pixel-wide runs along 80 columns of one band of rounded + rects, which is an antialiased edge moved less than a pixel. `view`, + `minimal`, `text`, `random` and the tab replay are unchanged. +- The warm-against-cold oracle is **not** worse: 100 seeds pass, the fifteen + shrinker cases at 400 seeds of depth 5 fail only on seed 288 as before, and + depth-6 seeds 18 and 190 pass with 326 failing as before. +- `Holds::through` had to be re-derived, and the derivation for one + truncation either side is measurably too narrow. See the item under + "Retained-layout invariants". Earlier, against #18's own history: the retained rewrite took `many` from 25.17M instructions a frame at `691e3eb` to 6.14M at `29c7881`, and `resize` @@ -427,7 +490,10 @@ across it. - Equal box lengths do not imply equal placement. An ordinary widget whose offered and current boxes differ in position must involve its parent again; a region node can settle itself only when its own alignment, rather than a - container override, determines the final box. + container override, determines the final box. **Both halves of that + decision compare whole boxes** -- whether the parent must place it, and + whether the offer already was the final box. Comparing lengths for the + second left a region node drawn at its offer (`aea878d`). - A retained drawing can be reused only when its `Holds` interval contains the new pixel box on both axes, its parent node is unchanged, its region-node choice matches the retained structure, and the widget is clean. A valid @@ -571,9 +637,12 @@ the number, `4e28f10` positions, `bd6de71` lengths, `39e4ca2` the last of the pixels and `Holds`. Decided with Bryan on 2026-09-15. - **`Fixed` is an `i32` counting `1 / 2^SHIFT`.** Adding and - subtracting are exact; a multiply or a conversion rounds once, back onto - the same steps. Two routes to one place that come within half a step land - on the same number, so everything downstream compares for equality. + subtracting are exact; a multiply drops to the step below and a conversion + between grids takes the nearest step. Two routes to one place that land on + one number are the same place, so everything downstream compares for + equality. **The multiply truncates as of `08c9d5a`** -- see the item in + "Performance" for what that bought and what it costs. `shift_round` is + still there for `to_scale`, which has no caller outside its own test. - **`Px` is `1/1024` px, `Rel` is `1/2^24` of a box, `Weight` is `1/65536` of a share.** `PX_SHIFT` and `REL_SHIFT` are the only statement of the first two, and the shader's copy is prepended from them by @@ -592,9 +661,10 @@ pixels and `Holds`. Decided with Bryan on 2026-09-15. to move a box, and at `1/1024` it is a thousandth of a pixel. Range is +/-2.1M px and conversion to `f32` is exact to 16,384 px. - **What the fuzzers ask for is a step per level of nesting**, which is two - for these trees. Traced on 2026-09-16 to the same box reached two ways, - each rounding where the other does not -- not accumulation, and not one - place. Two of them are fixed in `bdab558`: + for these trees. Truncating made each of those steps a whole one rather + than half of one; it did not add a level. Traced on 2026-09-16 to the same + box reached two ways, each rounding where the other does not -- not + accumulation, and not one place. Two of them are fixed in `bdab558`: - `Scroll` wrote a box it had been given back out as its own length in pixels. Centring a part in `rel 1` lands a step from centring it in `px 900`, because `a(x - y)` and `ax - ay` do not round alike. Content @@ -609,10 +679,27 @@ pixels and `Holds`. Decided with Bryan on 2026-09-15. means alignment resolved in pixels everywhere -- which costs the retained resize path, since it is the fractional form that re-centres a subtree without redrawing it. Not worth it at a thousandth of a pixel. -- `Holds::through` inverts `px + rel * box`, which rounds, so the answer is - an interval even for a single length. It maps the half step either side, - plus one more for the two ways above; inverting the length alone gives a - point that need not contain the box the part was drawn in. +- `Holds::through` inverts `px + rel * box`, which drops a step, so the answer + is an interval even for a single length: inverting the length alone gives a + point that need not contain the box the part was drawn in. What it allows + for has two parts and they are not the same shape. + - **The two routes to a length** -- composed down the chain against + measured against the window -- get two steps either side. One step either + side is the derivation for a single rounding on each route, and it is + measurably too narrow: each route is a *chain* of multiplies, and a + generated tree wanted 2 steps where the derivation allowed 1. + - **The multiply on the way in** gets one more step at the top of the range + and nothing at the bottom, since truncation only ever drops. The whole of + a box has no multiply in it, however many pixels are added to it, and + allowing for one there compounded a step per level down a chain of + widgets each taking the whole of its parent (`60367d8`). + + Too wide is the unsound direction: it admits reusing a drawing where it + does not hold. Too narrow costs a redraw and fires the `Holds` assertion in + a debug build, which is how both of these were found -- **run the generated + cases in debug before trusting a release measurement of a rounding + change**, since `debug_assert` is what says the contract broke and the + release oracle passed both of these anyway. - A pointer, a wheel notch, a shaped glyph advance and a window size arrive as floats and are put on the grid where they arrive. `Vec2` stays what the GPU and the platform speak; `PxVec2` is what layout decides in. @@ -700,7 +787,18 @@ the flag is the only way to measure the two on one tree. Build it and rather than through cargo. A comparison is only worth reading when the `--features layout-diagnostics` counters match on both sides; check `primitive writes`, `widget draws`, `reuse remapped` and `placed by -redrawing` before any timing. +redrawing` before any timing. **Against the float head they do not match and +will not**, so that comparison is a bound rather than a measurement -- the +pair worth measuring is two fixed-point commits, where all twenty-five can +be diffed. Dump them with + +```sh +IRIS_SEED=1 IRIS_DEPTH=8 IRIS_FRAMES=500 IRIS_PHASE=many \ + --ignored --nocapture \ + | grep -E '^ +[a-z].*[0-9.]+$' | grep -v ' ms$' | sort +``` + +and `diff` the two, which is the check that says a change is free. The headless reference set must be run one process at a time because the rig reuses one compositor. Comparison worktrees need separate target directories. @@ -780,6 +878,27 @@ Queued from this work, in order: since `a8898aa`. Still awaiting Bryan: whether a `Max` narrows the box the child draws in, or only what the parent reports for it. +- **Compose in `i64` and narrow only when storing** (Bryan, 2026-09-16). The + range inside a single operation is already protected -- `Fixed::mul` + widens to `i64` for the product, `Holds::through` does all of its + arithmetic in `i64` and narrows once at the end, and `div_toward` takes + `i64`. What is not protected is *precision across a chain*: + `UiSpan::within` composes a box through its parent with four multiplies, + and every one of them comes back to the `i32` grid before the next starts, + which is exactly the "step per level of nesting" in "Fixed point" and what + forces `Holds::through`'s allowance for two routes. A `Wide` held + through a composition and narrowed when it is written to `ActiveData` or a + primitive would round once instead of once a level, at no cost in stored + size, since storage stays `i32`. + + Two things to decide before starting: how many fractional bits the + intermediate keeps, since chained multiplies accumulate them and an `i64` + runs out too; and whether it helps `UiSpan::within` stay inlined, which is + the thing that actually moves cycles there -- a `Wide` that makes the body + bigger loses on the axis the truncation change won on. The success test is + narrowing `Holds::through`'s two-route allowance back to a step, which is a + check the generated oracle can answer in debug. + Other queued work, in dependency order: - `UiRenderState` behind `Rc>`.