Bring the handoff up to date: #18's state, the open question, and the rigs
This commit is contained in:
1 parent
e90b026969
commit
c683e62e4f
1 file changed
+113
-63
+113
-63
@@ -6,48 +6,84 @@ Not a decisions log; delete it when the extraction is done.
|
||||
## Where things stand
|
||||
|
||||
Canonical `main` is **`ca2b4b2`** (#17, the headless rig). Sixteen slices are
|
||||
in, and #16's size work and #17's rig both merged on 2026-09-14.
|
||||
in; #16's size work and #17's rig both merged on 2026-09-14.
|
||||
|
||||
- **#18** `split/18-position-chain`, worktree `/home/bob/repos/iris-pr18`, head
|
||||
`8223a55`. LAYOUT.md §2's O(1) subtree movement: every active widget owns a
|
||||
slot in `UiRenderState::moves`, `prelude.wgsl` walks the chain, and
|
||||
`try_reuse`'s translation case writes one slot instead of remapping a
|
||||
subtree's primitives. Measured at 100 primitive region writes to 0 on a span
|
||||
of 20 rows five primitives deep. `Vec2` is `repr(align(8))` so a GPU struct
|
||||
holding one matches WGSL without saying so itself.
|
||||
**#18 `split/18-position-chain`** is open, worktree `/home/bob/repos/iris-pr18`,
|
||||
head `db1751f`, five commits. LAYOUT.md §2's O(1) subtree movement, plus the
|
||||
`Remap` retirement the owner asked for on top of it.
|
||||
|
||||
Not in it, both wanting the above first: `Painter::set_child_offset` for a
|
||||
container that moves its children as a group (the scrolling case), and
|
||||
`LazySpan`. Built-in alignment is after those -- the owner reordered it
|
||||
behind the chain on 2026-09-14 as the more important of the two.
|
||||
- Every active widget owns a slot in `UiRenderState::moves` -- a translation in
|
||||
physical pixels and the slot it is relative to. A primitive instance and a
|
||||
mask each name one, and `prelude.wgsl` walks the chain and adds the
|
||||
accumulated delta. A mask resolves its own chain rather than the drawn
|
||||
primitive's, so a stationary viewport can clip content moving inside it.
|
||||
- `try_reuse`'s translation case writes one slot instead of remapping a
|
||||
subtree: 100 primitive region writes to 0 on a span of 20 rows five
|
||||
primitives deep.
|
||||
- `window_region` walks the same chain on the CPU, so hit testing and anyone
|
||||
asking in window pixels agree with the shader. `Moves::resolve` stops at
|
||||
`CHAIN_LIMIT` as the shader does, and debug-asserts that it reached the end.
|
||||
- `Vec2` is `repr(align(8))`, WGSL's alignment for a `vec2<f32>`, so a GPU
|
||||
struct holding one is laid out the way its shader reads it without saying so
|
||||
itself. `GlyphPrimitive` no longer states its own alignment; both it and
|
||||
`MoveOffset` keep a manual `unsafe impl Pod`, because the trailing padding
|
||||
that alignment introduces is what `derive(Pod)` refuses. **No manual padding
|
||||
fields** -- the owner rejected one on 2026-09-14.
|
||||
- `Remap` is gone, with `UiScalar::outside`, `UiSpan::outside` and
|
||||
`LerpUtil::lerp_inv`. A translation never needed an inversion: shifting a box
|
||||
shifts everything composed into it by the same amount, since
|
||||
`lerp(s + d, e + d, t) == lerp(s, e, t) + d` on both channels, whatever the
|
||||
box's relative extent. Only a change of length needs each part's fraction
|
||||
recovered, which is `UiRegion::stretch` behind `UiRegion::stretchable`. The
|
||||
decision is made once before the walk and neither relocation method
|
||||
branches, which is how the owner asked for it.
|
||||
|
||||
The invariant the chain rests on: `resolved = region + resolve(slot)`, so
|
||||
anything that rewrites a region owes that slot a zero. `mov` does it for the
|
||||
subtree it rewrites and `draw_inner` for the widget it draws.
|
||||
**Two invariants everything here rests on.** `resolved = region +
|
||||
resolve(slot)`, so anything that rewrites a region owes that slot a zero --
|
||||
`stretch` does it for the subtree it rewrites, `draw_inner` for the widget it
|
||||
draws. And a stretch is only expressible out of a box with a relative extent;
|
||||
a fixed length holds its parts as offsets from its start and keeps no fraction.
|
||||
|
||||
**Waiting on the owner, and measured first.** `tests/chain_cost.rs` times the
|
||||
pass on the GPU with timestamp queries: at 200,000 instances the walk is free
|
||||
to about depth 8 (+5%) and then costs roughly 3 us per level -- +43% at 16,
|
||||
+221% at 64. Each step is a storage load addressed by the previous one, so it
|
||||
is the chaining that costs and not the arithmetic at a level; a slot carrying
|
||||
a whole region would measure the same. Since #18 gives every active widget a
|
||||
slot, a primitive resolves through its full tree depth, which LAYOUT.md notes
|
||||
has exceeded 16. Irrelevant at an example's couple of hundred primitives,
|
||||
squarely in that regime for a transcript's glyphs.
|
||||
### The open question on #18, and the numbers for it
|
||||
|
||||
So the open question is how to keep the chain shallow -- a slot only where
|
||||
something is actually moved (needs re-parenting when an intermediate ancestor
|
||||
later gains one), against §6's rejected flattening, which the number partly
|
||||
rehabilitates. Retiring `Remap` by way of region chaining is cheap on top of a
|
||||
shallow chain and expensive without one. Put to her on #18; do not act before
|
||||
she picks.
|
||||
The owner proposed, and I agree with, **opt-in chaining that `Scroll` would
|
||||
choose** plus **recalculating rather than repositioning** when a region cannot
|
||||
be moved. Neither is implemented. What is settled and what is not:
|
||||
|
||||
- **Recalculating rather than repositioning is in**, as the `Remap`
|
||||
retirement above. It costs the per-axis carry: a box that changed length on
|
||||
one axis and not the other is now redrawn rather than remapped. Six of
|
||||
`tabs`'s fourteen relocations and five of `text`'s sixteen, and one extra
|
||||
redraw per frame on `replace_cost` -- 354,310,889 instructions against
|
||||
354,272,387, noise.
|
||||
- **Opt-in chaining is not**, and the design question is *who may opt in*. A
|
||||
widget can only move its subtree if its descendants chain through it, so
|
||||
opting in has to be done by whoever performs the move. My recommendation on
|
||||
the PR: any container that re-places a child after drawing it (`Span`,
|
||||
`Aligned`, `Scroll` all do), not only `Scroll`. That keeps the chain 2-4
|
||||
deep instead of full tree depth, and keeps the slot write for ordinary
|
||||
re-placement.
|
||||
- Giving **every** widget a slot, which is what #18 does and what I read §2 to
|
||||
say, is the thing to change: it puts a primitive's walk at full tree depth
|
||||
for no benefit, since almost every slot is zero.
|
||||
|
||||
Measured, so the next attempt is compared rather than argued:
|
||||
|
||||
| rig | what it says |
|
||||
| --- | --- |
|
||||
| `tests/chain_cost.rs` | GPU pass time by chain depth at 200k instances: free to depth 8 (+5%), then ~3 us per level -- +42.6% at 16, +221% at 64. Each step is a storage load addressed by the previous one, so it is the chaining that costs, not the arithmetic at a level; a slot carrying a whole region would measure the same. |
|
||||
| `tests/replace_cost.rs` | Instructions per frame re-placing 200 rows: 1.98M writing each row's slot, 2.38M rewriting its regions, 7.13M redrawing it. A load for `perf`, not a check. |
|
||||
| `tests/draw_cost.rs` | Pre-existing: what recording a frame costs on the CPU by layer count. |
|
||||
|
||||
Irrelevant at an example's couple of hundred primitives; a transcript's glyphs
|
||||
are tens of thousands, which is the regime `chain_cost` measures.
|
||||
|
||||
Check for a review before starting anything, and read the newest
|
||||
`submitted_at` rather than the first result:
|
||||
|
||||
```sh
|
||||
TOKEN=$(cat ~/.config/gitea/token)
|
||||
N=12
|
||||
N=18
|
||||
curl -s -H "Authorization: token $TOKEN" \
|
||||
https://git.arirex.me/api/v1/repos/iris/iris/pulls/$N/reviews
|
||||
curl -s -H "Authorization: token $TOKEN" \
|
||||
@@ -115,8 +151,10 @@ Other standing instructions from the owner:
|
||||
|
||||
## The next slice
|
||||
|
||||
**`set_child_offset` and `LazySpan`**, finishing LAYOUT.md §2 on top of #18,
|
||||
then built-in alignment.
|
||||
**Nothing, until the owner answers the opt-in question on #18** -- the shape of
|
||||
the slot set decides what `set_child_offset` even is, so building it first
|
||||
risks writing it twice. After that: `set_child_offset` and `LazySpan` to finish
|
||||
LAYOUT.md §2, then built-in alignment.
|
||||
|
||||
The archive is not a patch here: it writes `Widget::draw` against
|
||||
`painter.set_size`, which #16 replaced with a returned `Size`, and it writes
|
||||
@@ -130,9 +168,8 @@ Still in the target, roughly in dependency order:
|
||||
`LazySpan`.
|
||||
- **Built-in alignment, and probably size**, after the chain rather than
|
||||
before it: the owner reordered the two on 2026-09-14. Reproduced in the
|
||||
harness -- a child of a span is handed the full extent on the ortho axis, so
|
||||
`.width(rel(0.5))` inside a `Dir::DOWN` span changes what the child reports
|
||||
and not the box it gets. **Do not "fix" that by reading the child's ortho `size_hint`**: a `Pad`
|
||||
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" that 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
|
||||
@@ -205,8 +242,8 @@ Current invariants, not history. Worth reading before touching `core/render`.
|
||||
knows nothing else. Dispatch per list was measured at 6 instructions, 0.1% of
|
||||
a frame at 256 and at 1024 layers, against the ~5,400 wgpu spends recording
|
||||
one list; `tests/draw_cost.rs` is that measurement.
|
||||
- **The shared bind group is the window and the masks**, given to every draw.
|
||||
A mask texture would go here too. What a primitive samples is its own group,
|
||||
- **The shared bind group is the window, the masks and the move chain**, given
|
||||
to every draw. A mask texture would go here too. What a primitive samples is its own group,
|
||||
and a primitive that samples nothing has no such group in its pipeline.
|
||||
- **Every binding size is stated.** A `None` minimum puts the binding on
|
||||
wgpu-core's late-sized list, which `is_ready` scans on every draw.
|
||||
@@ -254,48 +291,59 @@ Current invariants, not history. Worth reading before touching `core/render`.
|
||||
|
||||
```sh
|
||||
cd /home/bob/repos/iris && git fetch upstream
|
||||
git worktree add -b split/15-name /home/bob/repos/iris-pr15 upstream/main
|
||||
git worktree add -b split/19-name /home/bob/repos/iris-pr19 upstream/main
|
||||
```
|
||||
|
||||
`/home/bob/repos/iris-pr18` is the live one. Every other `iris-pr*` worktree
|
||||
holds a merged branch; they are readable references, not places to build.
|
||||
|
||||
Every other `/home/bob/repos/iris-pr*` worktree holds a merged branch. They
|
||||
are readable references; do not build new work on them.
|
||||
|
||||
## Verifying a slice
|
||||
|
||||
`iris` runs its own rig now (#17), so a rendering claim no longer has to be
|
||||
driven from ai-app's submodule:
|
||||
|
||||
```sh
|
||||
cd <iris-worktree>
|
||||
./scripts/run-headless.sh tabs --mode 1920x1200@60Hz --shot /tmp/out.png
|
||||
./scripts/run-headless.sh tabs --replay /tmp/taps.touch --shot /tmp/out.png
|
||||
```
|
||||
|
||||
The reference shots this session compared against 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 `<ms> down|move|up <x> <y>` 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).
|
||||
|
||||
**A resize is its own case and the harness cannot see it.** Start an example,
|
||||
change the output mode under it with `swaymsg output HEADLESS-1 mode WxH@60Hz`,
|
||||
screenshot, and compare against a cold start at that size -- they must match
|
||||
byte for byte. That is what caught both of #16's defects, and neither showed up
|
||||
in 40 tests.
|
||||
|
||||
```sh
|
||||
cd <iris-worktree>
|
||||
cargo fmt --all --check
|
||||
cargo clippy --all-targets -- -D warnings
|
||||
cargo test
|
||||
cargo clippy --workspace --all-targets -- -D warnings
|
||||
cargo test --workspace
|
||||
```
|
||||
|
||||
`--workspace` when the slice crosses workspace crates. A rendering claim needs
|
||||
a real run, which until the rig is extracted means driving it from ai-app's
|
||||
submodule:
|
||||
42 tests pass on #18's head. `--workspace` matters: `rig-input` is a crate of
|
||||
its own.
|
||||
|
||||
```sh
|
||||
cd /home/bob/repos/ai-app-2/iris
|
||||
./scripts/run-headless.sh tabs --dir /home/bob/repos/iris-prNN \
|
||||
--shot /tmp/out.png --seconds 3
|
||||
./scripts/run-headless.sh tabs --dir ... --replay /tmp/taps.touch --shot ...
|
||||
```
|
||||
Two drawing paths still have no shot of their own, and each needs a ui the
|
||||
examples do not have, so both are throwaway examples written into the worktree
|
||||
and deleted after:
|
||||
|
||||
A `.touch` file is `<ms> down|move|up <x> <y>` in the output's own pixels. The
|
||||
`tabs` example's five tabs sit at x = 192, 576, 960, 1344 and 1728 on a
|
||||
1920x1200 output; tapping the third and then the bottom-right button twice
|
||||
adds two images.
|
||||
|
||||
Three renders cover the drawing paths, and each needs a different ui, so two
|
||||
of them are throwaway examples written into the worktree and deleted after:
|
||||
|
||||
1. `tabs` with that replay — rects, glyphs and images together.
|
||||
2. An image alone in a layer, which is the case that failed GPU validation
|
||||
1. An image alone in a layer, which is the case that failed GPU validation
|
||||
when every other test happened to have a rectangle in the same layer.
|
||||
3. Six lines of 400px text, which forces the atlas to four pages and proves
|
||||
2. Six lines of 400px text, which forces the atlas to four pages and proves
|
||||
the array grew and its group was rebuilt.
|
||||
|
||||
Those three become ordinary tests with the harness slice, which is the
|
||||
argument for doing it next.
|
||||
`tabs` with the image replay covers rects, glyphs and images together, so that
|
||||
one is an ordinary check now.
|
||||
|
||||
## Cautions
|
||||
|
||||
@@ -332,5 +380,7 @@ argument for doing it next.
|
||||
| #14 | Rename the `Sized` widget to `SetSize` (`32b1038`) |
|
||||
| #15 | Run a ui without a window, and test one (`c8ac669`) |
|
||||
| #12 | Route pointer input per kind (`43ce8c7`) |
|
||||
| #16 | Size a widget while drawing it, not in a pass of its own (`f942385`) |
|
||||
| #17 | Bring the headless rig into the repository (`ca2b4b2`) |
|
||||
|
||||
URLs are `https://git.arirex.me/iris/iris/pulls/{number}`.
|
||||
Reference in new issue
Block a user