iris: give masks/move_offsets their own bind group, fixing O(N) image append
GpuTextures folded the masks and move_offsets storage buffers into every standalone image's own bind group (group 2), alongside that image's texture view. Since ArrBuf::update hands back a new Buffer identity whenever either buffer's length changes -- which a widget getting its first move-offset slot can trigger, unrelated to any image -- every live image's bind group had to be rebuilt whenever either buffer grew. Appending a 1,001st image to 1,000 already-settled ones cost 1,001 bind-group creates, not 1 (IRIS_TODO.md, run-bench.sh images). Moved both buffers into their own bind group (group 3 in shader.wgsl and UiRenderNode), bound once per frame in draw() rather than once per per-image bind group. GpuTextures's image bind groups now only reference the atlas array view, the image's own view and the sampler -- none of which change when masks/move_offsets resize -- so a resize touches exactly one bind group regardless of how many images are live. This also closes the "two frames to reach steady state" item, which was the same bug measured a second way. Verified: cargo build/clippy/test clean (19 tests), cargo ndk build/clippy clean, run-headless.sh tabs --shot byte-identical (27266 bytes). New run-bench.sh images numbers: cold load unchanged at 1000/0/0/0, append now 1 instead of 1001. Both Fix items in IRIS_TODO.md ticked with the before/after numbers. Co-Authored-By: Claude Sonnet <noreply@anthropic.com>
This commit is contained in:
1 parent
e2873df92e
commit
19c36e37f2
4 files changed
+185
-149
No files matched your search
+59
-31
@@ -26,43 +26,61 @@ order and what "done" looks like. Tick and date them in place.
|
||||
still reaches the button — confirmed to fail on the pre-fix code and
|
||||
pass after.
|
||||
|
||||
- [ ] **Appending one image to an already-loaded list rebuilds every other
|
||||
image's bind group (2026-09-05).** Found by the benchmark below, not
|
||||
designed against: `GpuTextures::update` (`core/src/render/texture.rs`)
|
||||
triggers `rebuild_image_bind_groups` — a loop over *every live
|
||||
- [x] **Appending one image to an already-loaded list rebuilds every other
|
||||
image's bind group (2026-09-05, fixed 2026-09-05).** Found by the
|
||||
benchmark below: `GpuTextures::update` (`core/src/render/texture.rs`)
|
||||
triggered `rebuild_image_bind_groups` — a loop over *every live
|
||||
standalone image*, rebuilding its `BindGroup` — whenever the shared
|
||||
`masks` or `move_offsets` GPU buffer is resized (`masks_resized ||
|
||||
`masks` or `move_offsets` GPU buffer was resized (`masks_resized ||
|
||||
moves_resized` in `UiRenderNode::update`, `core/src/render/mod.rs`), and
|
||||
a widget getting its *first* move-offset slot (LAYOUT.md section 2 —
|
||||
every widget gets one on first draw) can be exactly what grows that
|
||||
every widget gets one on first draw) could be exactly what grows that
|
||||
buffer. So one new message with one new image, appended to a transcript
|
||||
that already has N images loaded, does not cost O(1): it costs one
|
||||
that already has N images loaded, did not cost O(1): it cost one
|
||||
`create_image` for the new image plus one `make_image_bind_group` per
|
||||
*existing* image, because the new widget's own move slot pushed the
|
||||
arena past its capacity. Measured directly in
|
||||
`iris/examples/bench_images.rs`: appending a 1,001st image to 1,000
|
||||
already-settled ones reports **1,001** bind-group creates for that one
|
||||
already-settled ones reported **1,001** bind-group creates for that one
|
||||
frame, not 1 (`./run-bench.sh images`, frame 5 in the transcript below).
|
||||
This is the same class of cost LAYOUT.md's move chain exists to avoid
|
||||
elsewhere in the codebase, just not yet closed off here — the fix is
|
||||
presumably to size `masks`/`move_offsets` with headroom (the array
|
||||
texture already grows by doubling, `grow_array`, for the same reason) so
|
||||
an ordinary append does not cross a capacity boundary, or to stop tying
|
||||
the *image* bind group's contents to a buffer that changes on every new
|
||||
widget in the whole tree, image or not. Not designed further here per
|
||||
the "do not redesign, record it" instruction this benchmark was built
|
||||
under.
|
||||
- [ ] **Bind-group creation takes two frames to reach the steady state, not
|
||||
one (2026-09-05).** Same benchmark: loading 1,000 images cold reports
|
||||
1,000 creates on frame 1 (expected — this is `create_image`, one per
|
||||
new image) *and again* 1,000 on frame 2, with nothing between the two
|
||||
frames marked dirty, before settling to 0 from frame 3. The second
|
||||
frame's 1,000 is `rebuild_image_bind_groups` again, for the same
|
||||
masks/move-offsets buffer-growth reason as the item above — the arena
|
||||
apparently does not finish growing to its steady size within the first
|
||||
frame the tree is drawn. Not chased further; recorded so whoever fixes
|
||||
the item above checks whether the fix also closes this one, since they
|
||||
look like the same root cause measured two different ways.
|
||||
|
||||
**Fix**: `masks`/`move_offsets` never belonged in a standalone image's own
|
||||
bind group (group 2) in the first place — the group also holds that
|
||||
image's own texture view, which is the only thing that is genuinely
|
||||
per-image, so a buffer shared by *everything* forced a rebuild of
|
||||
*every* group the moment it moved. Gave masks/move_offsets their own
|
||||
bind group (group 3 in `shader.wgsl` and `UiRenderNode`: `masks_layout`/
|
||||
`masks_group`), bound once per frame in `UiRenderNode::draw` rather than
|
||||
once per draw call, instead of duplicating them into every per-image
|
||||
group. `GpuTextures` and its image bind groups now know nothing about
|
||||
either buffer — `rebuild_image_bind_groups` is called only from
|
||||
`grow_array` (the atlas array texture growing, which genuinely does
|
||||
change what every image's own bind group must reference) — so a
|
||||
masks/move_offsets resize now touches exactly one bind group, ever,
|
||||
regardless of how many images are live. Numbers after the fix, same
|
||||
benchmark and command:
|
||||
|
||||
./run-bench.sh images
|
||||
frame=1 bind_group_creates=1000 (cold load, unchanged)
|
||||
frame=2 bind_group_creates=0 (was 1000 -- see the item below)
|
||||
frame=3 bind_group_creates=0
|
||||
frame=4 bind_group_creates=0
|
||||
(append one image here)
|
||||
frame=5 bind_group_creates=1 (was 1001)
|
||||
frame=6 bind_group_creates=0
|
||||
|
||||
`run-headless.sh tabs --shot` still 27266 bytes, byte-for-byte unchanged,
|
||||
confirming the bind-group restructuring changed nothing about what is
|
||||
drawn.
|
||||
- [x] **Bind-group creation takes two frames to reach the steady state, not
|
||||
one (2026-09-05, closed by the fix above, 2026-09-05).** Same benchmark:
|
||||
loading 1,000 images cold used to report 1,000 creates on frame 1
|
||||
(expected — `create_image`, one per new image) *and again* 1,000 on
|
||||
frame 2, before settling to 0 from frame 3. This was `rebuild_image_bind_groups`
|
||||
firing a second time for the same masks/move-offsets buffer-growth
|
||||
reason as the item above, confirming the guess recorded here — the two
|
||||
were exactly the same root cause measured two different ways. Frame 2
|
||||
now reports 0 (see the numbers above); not a separate fix.
|
||||
|
||||
## Build
|
||||
|
||||
@@ -123,7 +141,7 @@ order and what "done" looks like. Tick and date them in place.
|
||||
draws=320 rewrites=40 moves=160 (identical at every N)
|
||||
per-line average: 0.0012-0.0013ms (identical at every N)
|
||||
|
||||
cd iris && ./run-bench.sh images
|
||||
cd iris && ./run-bench.sh images (2026-09-05, before the fix)
|
||||
frame=1 bind_group_creates=1000 (cold load)
|
||||
frame=2 bind_group_creates=1000 (see Fix item above)
|
||||
frame=3 bind_group_creates=0
|
||||
@@ -132,6 +150,15 @@ order and what "done" looks like. Tick and date them in place.
|
||||
frame=5 bind_group_creates=1001 (see Fix item above)
|
||||
frame=6 bind_group_creates=0
|
||||
|
||||
cd iris && ./run-bench.sh images (2026-09-05, after the fix)
|
||||
frame=1 bind_group_creates=1000 (cold load, unchanged -- genuine work)
|
||||
frame=2 bind_group_creates=0
|
||||
frame=3 bind_group_creates=0
|
||||
frame=4 bind_group_creates=0
|
||||
(append one image here)
|
||||
frame=5 bind_group_creates=1 (one image's own create_image, O(1))
|
||||
frame=6 bind_group_creates=0
|
||||
|
||||
**Reading it**: (a) is real, necessary work — shaping and laying out N
|
||||
never-before-seen text rows — and scales with N as it must, ~10x cost
|
||||
per 10x N. (b) and (c) are the pass conditions that matter: both are
|
||||
@@ -140,8 +167,9 @@ order and what "done" looks like. Tick and date them in place.
|
||||
the message list — draws/moves per tick or per line do not grow with
|
||||
list size, and the per-operation cost (a fraction of a microsecond) is
|
||||
nowhere near a frame budget. (d)'s cold-load and steady-state halves
|
||||
behave as designed; its *append* half did not, which is the two Fix
|
||||
items above.
|
||||
behave as designed; its *append* half did not, until the fix above moved
|
||||
masks/move_offsets out of the per-image bind group — now flat at O(1)
|
||||
the same way (b) and (c) are.
|
||||
- [ ] **Masks defined relative to each other.** Wanted: mask A multiplies
|
||||
by something *and also* applies mask B — a mask can reference a parent
|
||||
mask, the way the move chain references a parent offset. Today masks
|
||||
|
||||
Reference in new issue
Block a user