160 lines
16 KiB
Markdown
160 lines
16 KiB
Markdown
|
|
# 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.
|