iris/ai-app
1
0 Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Record the partial-repaint fix, the truncating multiply, and what is left

Browse Source 
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 <noreply@anthropic.com>
This commit is contained in:
iris-aiandClaude Opus 5 committed 2026-09-16 17:09:45 -04:00
1 parent 06f9ae7799
commit a271795408
1 file changed
+195 -76
Download Patch File Download Diff File Expand all files Collapse all files
docs/HANDOFF.md
+195 -76
View File
@@ -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<SHIFT>` 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 \
<instrumented binary> --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<SHIFT>` 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<RefCell<_>>`.