Files

160 lines
16 KiB
Markdown
Raw Permalink Normal View History

# Iced Drawing Performance Regression Plan
## Goal
Restore low-latency 4K drawing while preserving the protected tiled compositor, persistent wgpu texture, dirty-bound `RefCell` union, viewport mapping, and nearest-neighbor sampler. Add deterministic transfer/work-budget tests and a repeatable latency diagnostic so future UI changes cannot silently restore full-canvas work.
## Historical Root Cause
The history contains three cumulative causes, not one isolated regression:
| Commit | Date | Finding | Scope |
|---|---|---|---|
| `266183a` | 2026-07-12 | Introduced the persistent wgpu texture, but also introduced `Arc::get_mut` for the shader-visible full composite. During iced `update()`, the existing widget/primitive still owns another `Arc`, so uniqueness fails and the fallback clones the full 4K RGBA canvas. The GPU upload is partial, but CPU staging can still copy 33,177,600 bytes per refresh. | All raster drawing; latent flaw in the original optimization |
| `b2c88e5` | 2026-07-17 | Added GPU selection rendering and retained/reworked the full-canvas Arc fallback. Every composite refresh clones and rescans the unchanged 8,294,400-byte 4K selection mask, marks it GPU-dirty, and uploads it again. The fragment shader also performs one center plus eight neighbor mask samples for every selected fragment, about 75 million R8 samples per frame for a full 4K selection. | Severe when drawing with an active selection |
| `7b4073e` | 2026-07-21 | Changed watercolor from one raster dab per sample to roughly 7-37 dabs (core, satellites, splatter, blooms). At common 24-64 px sizes it usually performs about 28-37 dabs per interpolated sample. | Watercolor brush |
Supporting evidence:
- `git log -G 'Arc::get_mut'` identifies `266183a`, `b2c88e5`, and later transform work as the ownership-sensitive composite path history.
- `git blame` attributes all nine `selection_texture` samples in `canvas.wgsl` and unconditional selection dirty marking to `b2c88e5`.
- `git diff 7b4073e^ 7b4073e -- hcie-brush-engine/src/lib.rs` shows the one-dab to multi-dab watercolor expansion.
- `hcie-engine-api/tests/performance_stroke_4k.rs` only exercises a round brush and engine compositing. It is explicitly non-gating and excludes iced staging, selection synchronization, fragment cost, GPU payloads, and input-to-present delay.
- The recorded engine baseline is approximately 18.8 ms average composite and 53 segments/s. It is useful as an engine reference, not as an iced end-to-end latency baseline.
- Preserve the unrelated current worktree edit that exposes `stroke_brush`; never overwrite or revert it.
## Concrete Transfer Design
Add `hcie-iced-app/crates/hcie-iced-gui/src/canvas/texture_update.rs` with two focused types:
- `SharedCompositePixels`: stable `Arc<RwLock<Vec<u8>>>` full-canvas backing storage used for pipeline creation/recreation and full uploads.
- `TextureUpdate`: a tightly packed immutable partial payload containing `[x0, y0, x1, y1]`, `bytes_per_row`, and `Arc<[u8]>` pixels.
The data flow will be:
1. `refresh_composite_if_needed()` calls `render_composite_region()` exactly as today.
2. For a normal dirty rectangle, it copies only dirty rows into both existing `composite_raw` and `SharedCompositePixels`; no copy-on-write and no full-canvas allocation occurs.
3. The existing `dirty_region: RefCell<Option<[u32; 4]>>` continues unioning every update received before `view()`.
4. `canvas::view()` consumes the union once and packs that final rectangle from `composite_raw` into one `TextureUpdate`. This costs one additional dirty-area copy and avoids stale payloads when several Elm updates occur in one frame.
5. `CanvasShaderPrimitive::prepare()` sends `TextureUpdate` directly to `queue.write_texture()` with no full-buffer scan, row extraction, or per-prepare allocation.
6. If shader storage is absent, dimensions changed, or `full_upload` is set, `prepare()` takes a short read lock on `SharedCompositePixels` and creates/replaces the full texture. It does not also apply the partial payload because the shared backing already contains the latest pixels.
7. Normal `prepare()` calls never lock the full buffer. The lock is held only for small row writes in `update()` or rare full texture creation/replacement, never across engine compositing and never longer than the corresponding `queue.write_texture()` input borrow requires.
Expected ordinary 48x48 update budget:
- Dirty RGBA payload: 9,216 bytes.
- CPU copies: at most 27,648 bytes (engine scratch to `composite_raw`, engine scratch to shared recovery backing, `composite_raw` to packed upload).
- GPU upload: exactly 9,216 bytes.
- Forbidden ordinary-frame behavior: a 33,177,600-byte clone, texture recreation, or full upload.
## Selection Design
Make selection rendering event-driven and reduce the shader to one mask sample:
1. Keep the GUI's raw selection mask available for move/transform flows, but store it as `Option<Arc<Vec<u8>>>` so shader/view construction is O(1).
2. Add a separate `Option<Arc<Vec<u8>>>` encoded selection texture and a `selection_model_dirty` marker. Continue using `selection_mask_dirty` only for “encoded texture requires GPU upload.”
3. Add one synchronization helper that runs only after a selection mutation. It clones the engine/local raw mask once, computes exact bounds, and builds one R8 encoded texture:
- `0`: unselected
- `128`: selected interior
- `255`: selected border using the existing eight-neighbor, alpha-greater-than-127 semantics
4. Preserve current edge behavior at canvas boundaries by treating out-of-bounds neighbors as clamped/selected, matching the current `ClampToEdge` shader rather than introducing a new outer border.
5. Update `canvas.wgsl` to sample `selection_texture` once. Values above `0.75` receive animated black/white border color; values above `0.25` receive normal/quick-mask fill.
6. Animation remains uniform-only. Marching-ants ticks do not rebuild, clone, fingerprint, or upload selection data, and remain disabled while drawing.
7. Remove obsolete CPU structures that became dead when `b2c88e5` moved selection rendering to the shader: `SelectionEdgeCache`, `marching_ants_edges`, `selection_fill_spans`, their underscored overlay fields, and `selected_spans()`. Delete `canvas/edge_cache.rs` after confirming no remaining consumers.
8. Mark selection state dirty at every engine/local mutation family: rectangle, magic wand, lasso, polygon, select-all, combine mode, invert, grow/shrink/feather/border/smooth, load, clear/deselect, selection undo, move-start engine clear with local retained mask, transform apply/cancel, and menu/script paths.
9. Keep conditional synchronization at the end of `refresh_composite_if_needed()` so selection-only changes still reach the GPU even when the document composite is clean. The condition is a cheap boolean on normal brush refreshes.
## Watercolor Design
Retain the richer watercolor appearance but impose a hard per-sample budget:
- Unlock `hcie-brush-engine` with `unlock.sh hcie-brush-engine`, make the change, and relock it with `lock.sh hcie-brush-engine`.
- Refactor watercolor planning into an RNG-parameterized private helper so a seeded unit test can verify its maximum work and footprint.
- Budget at most eight raster dabs per interpolated sample: one irregular full-size core, up to two smaller satellites, up to four tiny splatters, and at most one probabilistic faint bloom.
- Keep all non-core dabs materially smaller than the core; do not permit multiple near-full-size core/bloom dabs per sample.
- Preserve pressure, mask, eraser, opacity, and hardness behavior.
- If the measured p95 still exceeds the budget after this cap, restore the pre-`7b4073e` one-dab implementation. Do not weaken GPU/tile/composite protections to compensate for brush simulation cost.
## Diagnostic And Regression Tests
### 1. Deterministic Iced Transfer Tests
Place unit tests beside `texture_update.rs` so private binary canvas code remains testable without expanding the public library API.
- Hold shader-side clones of `SharedCompositePixels`, apply 120 48x48 updates to a 3840x2160 buffer, and assert the shared allocation identity remains stable.
- Assert each packed payload length is `dirty_width * dirty_height * 4` and no ordinary update is classified as full.
- Assert cumulative CPU copy accounting is no more than three times cumulative dirty bytes and cumulative GPU bytes equal cumulative dirty bytes.
- Include a test-only reproduction of the old COW branch: hold a second `Arc`, show that `Arc::get_mut` fails, and account for a 33,177,600-byte clone. This consistently demonstrates the regression without retaining legacy behavior in production.
- Verify two or more dirty rectangles accumulated before `view()` produce the exact union payload containing final pixels from all updates.
- Verify first frame, resize, document replacement, and file load still select the full-upload path and contain valid complete pixels.
- Verify malformed/out-of-bounds rectangles are rejected or clamped safely without pointer arithmetic underflow.
### 2. Deterministic Selection Tests
- Encode empty, rectangular, irregular, feathered, and edge-touching masks; assert exact `0/128/255` classification and bounds.
- Synchronize one 4K mask, then process 120 normal drawing refreshes with `selection_model_dirty == false`; assert zero additional raw clones, encodes, bounds scans, and selection texture uploads.
- Replace and clear a selection; assert exactly one upload per mutation and no stale `has_selection` state.
- Add a shader contract test using `include_str!("canvas.wgsl")` that asserts exactly one `textureSample(selection_texture, ...)` occurrence. This deterministic gate catches reintroduction of per-fragment neighbor sampling.
### 3. Watercolor Work And Timing Tests
- Seed the private watercolor planner and assert every generated sample contains one core and no more than eight total dabs across small, normal, and large sizes.
- Assert planned satellite/splatter/bloom extents remain within the dirty-radius contract used by `stroke_brush.rs`.
- Add an ignored diagnostic benchmark for Round and Watercolor at 24, 64, and 128 px, warming up before collecting 100 samples and printing p50/p95/max.
- Gate only the deterministic maximum dab count and a generous catastrophic timing ratio; do not use a tight absolute wall-clock threshold in CI.
### 4. End-To-End Diagnostic Harness
Add an ignored iced drawing-pipeline unit test, runnable with `--ignored --nocapture`, that creates a 3840x2160 10-layer engine document and replays 120 pointer samples in four scenarios:
- Round, no selection.
- Round, full-canvas selection.
- Watercolor, no selection.
- Watercolor, full-canvas selection.
Discard the first 10 samples as warm-up. Report p50/p95/max separately for stroke, engine composite, GUI row copy, upload packing, selection synchronization, and total event processing, plus dirty bytes, CPU copied bytes, GPU payload bytes, full-upload count, and estimated sustainable FPS. This benchmark is diagnostic; deterministic byte/work tests are the CI gate.
For historical confirmation, run the standalone/reproducible portions in isolated temporary worktrees at `266183a^`, `266183a`, `b2c88e5^`, `b2c88e5`, `7b4073e^`, and `7b4073e` where those revisions build. Do not check out commits in the user's dirty main worktree. Static `git diff`/`blame` evidence remains authoritative if an old revision no longer builds.
## File-Level Change Map
- Add `hcie-iced-app/crates/hcie-iced-gui/src/canvas/texture_update.rs`: shared recovery backing, packed update type, byte accounting, deterministic tests, diagnostic test.
- Update `hcie-iced-app/crates/hcie-iced-gui/src/canvas/mod.rs`: register the module, consume dirty union, build packed payload, pass encoded selection texture, remove dead edge/span overlay fields.
- Update `hcie-iced-app/crates/hcie-iced-gui/src/canvas/shader_canvas.rs`: accept shared full backing plus optional packed payload; direct partial upload; rare full recovery upload; remove current region extraction allocation.
- Update `hcie-iced-app/crates/hcie-iced-gui/src/canvas/canvas.wgsl`: one-sample encoded selection branch.
- Update `hcie-iced-app/crates/hcie-iced-gui/src/app.rs`: document fields/constructors, partial row writes, full replacement paths, conditional selection synchronization and mutation markers.
- Update `hcie-iced-app/crates/hcie-iced-gui/src/selection/state.rs`: encoded overlay generation and tests; remove unused span extraction.
- Delete `hcie-iced-app/crates/hcie-iced-gui/src/canvas/edge_cache.rs` after removing all consumers.
- Update `hcie-brush-engine/src/lib.rs`: bounded watercolor planner/renderer and internal deterministic tests.
- Extend or add a focused engine benchmark without changing protected engine caching behavior.
## Validation Sequence
1. Run focused unit tests for transfer packing, dirty union, selection encoding, shader sample contract, and watercolor work budget.
2. Run the ignored four-scenario diagnostic with `--nocapture` and save before/after numbers in the final report.
3. Run `cargo test -p hcie-engine-api --test visual_regression`.
4. Run `cargo test -p hcie-engine-api --test performance_stroke_4k -- --nocapture`.
5. Run all `hcie-iced-gui` tests, `cargo check -p hcie-iced-gui`, and `cargo clippy -p hcie-iced-gui`.
6. Run focused brush-engine tests/checks, then confirm the crate is relocked.
7. Launch iced in release mode with a 4K 10-layer document. Replay Round and Watercolor strokes with no selection and a full-canvas selection. Confirm no ordinary full upload, no repeated selection upload, correct dirty payload sizes, and materially improved p95 input processing.
8. Capture and inspect an iced screenshot after the GUI change. Verify canvas alignment, nearest-neighbor output, selection tint/ants, brush cursor, and drawn pixels. Screenshot correctness complements, but does not replace, latency telemetry.
## Acceptance Criteria
- A normal 48x48 4K drawing refresh uploads 9,216 bytes and performs at most 27,648 bytes of GUI CPU copies; it never clones/uploads 33,177,600 bytes.
- Multiple updates before one `view()` preserve the complete dirty union and final pixels.
- An unchanged active 4K selection causes zero mask clone/encode/upload work during drawing after initial synchronization.
- The selection shader contains one selection texture sample, not nine, while preserving tint, quick-mask color, and animated border appearance.
- Watercolor performs at most eight planned raster dabs per interpolated sample and shows no catastrophic p95 regression against `7b4073e^`.
- Round-brush output and the protected tile/composite/GPU mechanisms remain unchanged.
- Visual regression, focused tests, compile/lint checks, manual 4K replay, and screenshot inspection pass without missing tiles, coordinate drift, seams, stale selection state, or first-frame failure.
## Risks And Controls
- Renderer-thread lock contention: normal partial uploads use immutable packed payloads and do not lock; full backing locks are limited to short row writes or rare recreation.
- Lost dirty regions: retain the existing `RefCell` union and consume only in `view()`; test multiple updates per frame.
- Pipeline recreation with only a partial payload: initialize from `SharedCompositePixels`, which always contains the latest full image.
- Selection mutation coverage: centralize marking/synchronization and audit all 32 current selection-engine call sites found in `app.rs`.
- GPU timing variability: gate shader sample count and transfer volume deterministically; use runtime timings only for trend diagnosis.
- Watercolor visual change: use seeded planner tests and screenshot/visual regression checks; fall back to the known pre-regression renderer if the bounded richer version misses the latency target.