diff --git a/.kilo/plans/1784668619682-iced-drawing-performance-regression.md b/.kilo/plans/1784668619682-iced-drawing-performance-regression.md new file mode 100644 index 0000000..b95080a --- /dev/null +++ b/.kilo/plans/1784668619682-iced-drawing-performance-regression.md @@ -0,0 +1,159 @@ +# 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>>` 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>` 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>>` so shader/view construction is O(1). +2. Add a separate `Option>>` 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. diff --git a/hcie-brush-engine/Cargo.toml b/hcie-brush-engine/Cargo.toml index d8b1ba9..deddd38 100644 --- a/hcie-brush-engine/Cargo.toml +++ b/hcie-brush-engine/Cargo.toml @@ -16,4 +16,4 @@ rstest = "0.23" proptest = "1.5" approx = "0.5" egui = "0.34" -eframe = { version = "0.34", default-features = false, features = ["default_fonts", "glow"] } +eframe = { version = "0.34", default-features = false, features = ["default_fonts", "glow", "x11"] } diff --git a/hcie-brush-engine/src/lib.rs b/hcie-brush-engine/src/lib.rs index 9220218..30837c4 100644 --- a/hcie-brush-engine/src/lib.rs +++ b/hcie-brush-engine/src/lib.rs @@ -1223,49 +1223,89 @@ pub fn draw_watercolor_brush( mask: Option<&[u8]>, ) { let mut rng = rand::thread_rng(); + draw_watercolor_brush_with_rng( + pixels, width, height, cx, cy, size, pressure, color, opacity, hardness, is_eraser, mask, + &mut rng, + ); +} + +/// Maximum raster dabs emitted for one interpolated watercolor sample. +const WATERCOLOR_MAX_DABS_PER_SAMPLE: usize = 8; + +/// Computes bounded watercolor detail counts for one effective brush size. +/// +/// **Arguments:** `effective_size` is pressure-adjusted diameter and `rng` supplies the optional +/// bloom decision. **Returns:** Satellite, splatter, and bloom counts whose total plus one core is +/// at most eight. **Side Effects / Dependencies:** Advances the provided random generator once for +/// eligible blooms. +fn watercolor_detail_counts( + effective_size: f32, + rng: &mut R, +) -> (usize, usize, usize) { + let satellites = ((effective_size / 24.0).ceil() as usize).clamp(1, 2); + let splatters = ((effective_size / 16.0).ceil() as usize).clamp(1, 4); + let bloom = usize::from(effective_size > 25.0 && rng.gen_bool(0.3)); + debug_assert!(1 + satellites + splatters + bloom <= WATERCOLOR_MAX_DABS_PER_SAMPLE); + (satellites, splatters, bloom) +} + +/// Renders one bounded watercolor sample using a caller-provided random generator. +/// +/// **Arguments:** Pixel target, dimensions, center, brush settings, optional selection mask, and +/// random generator. **Returns:** Nothing. **Logic & Workflow:** Draws one irregular full-size core, +/// up to two satellites, up to four tiny splatters, and at most one faint bloom. **Side Effects / +/// Dependencies:** Mutates target pixels and advances `rng`; never exceeds eight raster dabs. +#[allow(clippy::too_many_arguments)] +fn draw_watercolor_brush_with_rng( + pixels: &mut [u8], + width: u32, + height: u32, + cx: f32, + cy: f32, + size: f32, + pressure: f32, + color: [u8; 4], + opacity: f32, + hardness: f32, + is_eraser: bool, + mask: Option<&[u8]>, + rng: &mut R, +) { let effective_size = size * pressure; if effective_size < 0.5 { return; } - let base_opacity = opacity * 0.2 * pressure; + let base_opacity = opacity * 0.3 * pressure; let center_color = color; + let (satellite_count, splatter_count, bloom_count) = + watercolor_detail_counts(effective_size, rng); // ── Central irregular core ────────────────────────────────────────────── - // Build the main mark from several overlapping soft dabs so the boundary - // is lumpy instead of a perfect circle. - let core_dabs = rng.gen_range(2..=4); - for _ in 0..core_dabs { - let angle = rng.gen_range(0.0..std::f32::consts::TAU); - let offset = effective_size * rng.gen_range(0.0..0.12); - let dx = angle.cos() * offset; - let dy = angle.sin() * offset; - let s = effective_size * rng.gen_range(0.85..1.15); - let a = base_opacity * rng.gen_range(0.7..1.3); - draw_dab( - pixels, - width, - height, - cx + dx, - cy + dy, - s, - hardness, - center_color, - a, - is_eraser, - mask, - ); - } + let core_angle = rng.gen_range(0.0..std::f32::consts::TAU); + let core_offset = effective_size * rng.gen_range(0.0..0.08); + draw_dab( + pixels, + width, + height, + cx + core_angle.cos() * core_offset, + cy + core_angle.sin() * core_offset, + effective_size * rng.gen_range(0.95..1.1), + hardness, + center_color, + base_opacity * rng.gen_range(0.85..1.15), + is_eraser, + mask, + ); // ── Satellite puddles ─────────────────────────────────────────────────── // Secondary dabs pulled away from the stroke path by water tension, // giving the edge its characteristic ragged bloom. - let satellite_count = (effective_size / 6.0).clamp(2.0, 8.0) as i32; for _ in 0..satellite_count { let angle = rng.gen_range(0.0..std::f32::consts::TAU); let dist = effective_size * rng.gen_range(0.25..0.65); let dx = angle.cos() * dist; let dy = angle.sin() * dist; - let s = effective_size * rng.gen_range(0.2..0.55); + let s = effective_size * rng.gen_range(0.18..0.38); let a = base_opacity * rng.gen_range(0.25..0.75); draw_dab( pixels, @@ -1284,7 +1324,6 @@ pub fn draw_watercolor_brush( // ── Tiny splatter particles ─────────────────────────────────────────── // Small droplets flung outward, visible on high-resolution strokes. - let splatter_count = (effective_size * 1.2).clamp(3.0, 22.0) as i32; for _ in 0..splatter_count { let angle = rng.gen_range(0.0..std::f32::consts::TAU); let dist = effective_size * rng.gen_range(0.45..1.15); @@ -1310,17 +1349,12 @@ pub fn draw_watercolor_brush( // ── Back-run / blossom bursts ─────────────────────────────────────────── // A few very large, faint "blooms" that extend beyond the main stroke, // simulating water pushing pigment to the edges. - let bloom_count = if effective_size > 25.0 { - rng.gen_range(1..=3) - } else { - 0 - }; for _ in 0..bloom_count { let angle = rng.gen_range(0.0..std::f32::consts::TAU); let dist = effective_size * rng.gen_range(0.5..0.9); let dx = angle.cos() * dist; let dy = angle.sin() * dist; - let s = effective_size * rng.gen_range(0.6..1.0); + let s = effective_size * rng.gen_range(0.45..0.7); let a = base_opacity * rng.gen_range(0.15..0.35); draw_dab( pixels, @@ -3345,3 +3379,88 @@ fn alpha_blend(dst: [u8; 4], src: [u8; 4]) -> [u8; 4] { (out_a * 255.0).round() as u8, ] } + +#[cfg(test)] +mod watercolor_performance_tests { + use super::{ + draw_dab, draw_watercolor_brush_with_rng, watercolor_detail_counts, + WATERCOLOR_MAX_DABS_PER_SAMPLE, + }; + use rand::{rngs::StdRng, SeedableRng}; + + /// Ensures every supported size stays within the fixed raster-work budget. + #[test] + fn watercolor_detail_count_is_hard_bounded() { + let mut rng = StdRng::seed_from_u64(0x0057_4154_4552); + for size in [0.5, 1.0, 8.0, 24.0, 64.0, 128.0, 1024.0] { + for _ in 0..1000 { + let (satellites, splatters, blooms) = watercolor_detail_counts(size, &mut rng); + let total = 1 + satellites + splatters + blooms; + assert!(satellites <= 2); + assert!(splatters <= 4); + assert!(blooms <= 1); + assert!(total <= WATERCOLOR_MAX_DABS_PER_SAMPLE); + } + } + } + + /// Reports p50/p95/max raster time for one round dab and the bounded watercolor sample. + #[test] + #[ignore = "diagnostic benchmark; run with --ignored --nocapture"] + fn diagnostic_watercolor_sample_latency() { + let mut pixels = vec![0u8; 512 * 512 * 4]; + let mut rng = StdRng::seed_from_u64(0x0050_4149_4e54); + for size in [24.0, 64.0, 128.0] { + let mut round_samples = Vec::with_capacity(100); + let mut watercolor_samples = Vec::with_capacity(100); + for sample in 0..110 { + let started = std::time::Instant::now(); + draw_dab( + &mut pixels, + 512, + 512, + 256.0, + 256.0, + size, + 0.5, + [20, 80, 180, 255], + 0.8, + false, + None, + ); + let round = started.elapsed().as_secs_f64() * 1000.0; + let started = std::time::Instant::now(); + draw_watercolor_brush_with_rng( + &mut pixels, + 512, + 512, + 256.0, + 256.0, + size, + 1.0, + [20, 80, 180, 255], + 0.8, + 0.5, + false, + None, + &mut rng, + ); + let watercolor = started.elapsed().as_secs_f64() * 1000.0; + if sample >= 10 { + round_samples.push(round); + watercolor_samples.push(watercolor); + } + } + round_samples.sort_by(f64::total_cmp); + watercolor_samples.sort_by(f64::total_cmp); + let p95_index = (round_samples.len() as f64 * 0.95).floor() as usize; + println!( + "watercolor size={size:.0}: round_p95={:.3}ms watercolor_p50={:.3}ms watercolor_p95={:.3}ms watercolor_max={:.3}ms", + round_samples[p95_index.min(round_samples.len() - 1)], + watercolor_samples[watercolor_samples.len() / 2], + watercolor_samples[p95_index.min(watercolor_samples.len() - 1)], + watercolor_samples[watercolor_samples.len() - 1], + ); + } + } +} diff --git a/hcie-engine-api/src/lib.rs b/hcie-engine-api/src/lib.rs index d17f814..580014b 100644 --- a/hcie-engine-api/src/lib.rs +++ b/hcie-engine-api/src/lib.rs @@ -13,7 +13,7 @@ pub mod svg_editor; // Performance-critical engine paths isolated into dedicated modules. mod layer_property_ops; pub mod partial_composite; -mod stroke_brush; +pub mod stroke_brush; mod stroke_cache; use crate::dynamic_loader::svg_import::import_svg; diff --git a/hcie-iced-app/crates/hcie-iced-gui/src/app.rs b/hcie-iced-app/crates/hcie-iced-gui/src/app.rs index dc63023..937f60f 100644 --- a/hcie-iced-app/crates/hcie-iced-gui/src/app.rs +++ b/hcie-iced-app/crates/hcie-iced-gui/src/app.rs @@ -404,13 +404,11 @@ enum PendingFileOperation { /// Per-document state wrapping an engine instance. pub struct IcedDocument { pub engine: Engine, - /// Composite RGBA pixel data shared with the shader pipeline via Arc. - /// The shader's `prepare()` reads this and uploads only the dirty region - /// via `queue.write_texture()`. Updated in `refresh_composite_if_needed()`. - pub composite_pixels: Arc>, - /// Mutable composite buffer for partial-copy from engine. - /// After updating, this is wrapped in a new Arc for the shader. - composite_raw: Vec, + /// Stable full RGBA recovery image shared with the persistent shader pipeline. + /// Normal frames use packed dirty payloads; this is read only for full texture recovery. + pub composite_pixels: crate::canvas::texture_update::SharedCompositePixels, + /// Mutable composite buffer for partial copies and packed dirty-update construction. + pub(crate) composite_raw: Vec, /// Document display name. pub name: String, /// Original file path, if loaded from disk. @@ -466,9 +464,13 @@ pub struct IcedDocument { /// When true, the pending_paste pixels should be placed as a new layer /// instead of entering transform mode (set by "Paste as New Layer"). pub pending_paste_as_new_layer: bool, - /// Selection mask (alpha channel) for the current selection - pub selection_mask: Option>, - /// Whether the selection mask has changed since last GPU upload + /// Raw selection mask retained for transform and selection-history workflows. + pub selection_mask: Option>>, + /// Encoded R8 selection texture: zero unselected, 128 interior, 255 border. + pub selection_texture: Option>>, + /// Whether a selection engine/local mutation still needs cache synchronization. + pub selection_model_dirty: bool, + /// Whether the encoded selection texture has changed since last GPU upload. pub selection_mask_dirty: std::cell::Cell, /// Selection bounds (x, y, width, height) pub selection_bounds: Option<(u32, u32, u32, u32)>, @@ -488,12 +490,6 @@ pub struct IcedDocument { pub vision_rect: Option<(f32, f32, f32, f32)>, /// Draft text state for the on-canvas text tool. pub text_draft: Option, - /// Cache for extracted selection edges (marching ants). - pub selection_edge_cache: crate::canvas::edge_cache::SelectionEdgeCache, - /// Extracted selection edges for overlay rendering. - pub marching_ants_edges: Option>>, - /// Horizontal runs of selected pixels used to fill irregular masks exactly. - pub selection_fill_spans: Option>>, /// How newly generated selection masks modify the existing mask. pub selection_mode: selection::SelectionMode, /// In-memory saved selection for the document's Load/Save Selection commands. @@ -1563,8 +1559,7 @@ mod transform_transaction_tests { /// /// **Logic & Workflow:** Restores only the previous transformed AABB from the stable base, invokes /// the shared inverse-affine compositor for the current AABB, unions both with any pending dirty -/// region, and updates the shader Arc in place when uniquely owned. A full-buffer Arc clone is used -/// only when Iced still owns the prior frame's immutable Arc. +/// region, and copies only that union into the stable shader recovery image. /// /// **Arguments:** `doc` contains transform/composite state and `previous_bounds` identifies stale /// preview pixels. **Returns:** Nothing. **Side Effects / Dependencies:** Mutates preview buffers, @@ -1593,15 +1588,13 @@ fn render_transform_preview(doc: &mut IcedDocument, previous_bounds: Option<[u32 let pending = doc.dirty_region.replace(None); let upload_bounds = union_regions(pending, bounds); doc.dirty_region.replace(Some(upload_bounds)); - if let Some(shader_pixels) = Arc::get_mut(&mut doc.composite_pixels) { - restore_preview_region( - shader_pixels, - &doc.composite_raw, - canvas_width, - upload_bounds, - ); - } else { - doc.composite_pixels = Arc::new(doc.composite_raw.clone()); + if let Err(error) = + doc.composite_pixels + .copy_region_from(&doc.composite_raw, canvas_width, upload_bounds) + { + log::error!("failed to stage transform preview: {error}"); + doc.composite_pixels.replace(&doc.composite_raw); + doc.full_upload.replace(true); } doc.render_generation = doc.render_generation.wrapping_add(1); log::debug!( @@ -1611,6 +1604,60 @@ fn render_transform_preview(doc: &mut IcedDocument, previous_bounds: Option<[u32 } } +/// Rebuilds all GUI selection-rendering state from one raw mask. +/// +/// **Purpose:** Makes selection texture work proportional to selection mutations rather than brush +/// refreshes. **Logic & Workflow:** Valid masks are encoded once into interior/border values and +/// cached behind `Arc`; empty or invalid masks clear all overlay state. **Arguments:** `doc` is the +/// target document and `mask` is an optional locally owned alpha mask. **Returns:** Nothing. +/// **Side Effects / Dependencies:** Updates selection caches and schedules exactly one GPU upload. +fn set_document_selection_mask(doc: &mut IcedDocument, mask: Option>) { + let width = doc.engine.canvas_width(); + let height = doc.engine.canvas_height(); + let valid_mask = mask.filter(|mask| mask.len() == (width * height) as usize); + if let Some(mask) = valid_mask { + let encoded = selection::state::encode_selection_texture(&mask, width, height); + if encoded.bounds.is_some() { + doc.selection_mask = Some(Arc::new(mask)); + doc.selection_texture = Some(Arc::new(encoded.pixels)); + doc.selection_bounds = encoded.bounds; + } else { + doc.selection_mask = None; + doc.selection_texture = None; + doc.selection_bounds = None; + } + } else { + doc.selection_mask = None; + doc.selection_texture = None; + doc.selection_bounds = None; + } + doc.selection_model_dirty = false; + doc.selection_mask_dirty.set(true); +} + +/// Consumes one pending selection-cache synchronization request. +/// +/// **Arguments:** `requested` is the document's mutation marker. **Returns:** Whether a rebuild must +/// run now. **Side Effects / Dependencies:** Resets the marker so ordinary composite refreshes are +/// constant-time until another selection operation explicitly marks it. +fn consume_selection_sync_request(requested: &mut bool) -> bool { + std::mem::take(requested) +} + +/// Synchronizes cached selection rendering state after an engine selection mutation. +/// +/// **Arguments:** `doc` is the document whose engine owns the source mask. **Returns:** Nothing. +/// **Logic & Workflow:** A clean marker is a constant-time no-op; a dirty marker clones and encodes +/// the engine mask once. **Side Effects / Dependencies:** Reads `hcie-engine-api` selection state and +/// may schedule a selection texture upload. +fn sync_document_selection_if_needed(doc: &mut IcedDocument) { + if !consume_selection_sync_request(&mut doc.selection_model_dirty) { + return; + } + let mask = doc.engine.get_selection_mask().map(ToOwned::to_owned); + set_document_selection_mask(doc, mask); +} + impl HcieIcedApp { /// Combines the engine's newly generated selection with the previous mask. /// @@ -1628,11 +1675,13 @@ impl HcieIcedApp { doc.selection_history.push(previous.clone()); let Some(new_mask) = doc.engine.get_selection_mask().map(ToOwned::to_owned) else { + set_document_selection_mask(doc, None); return; }; let combined = selection::state::combine_masks(previous.as_deref(), &new_mask, doc.selection_mode); doc.engine.set_selection_mask(combined); + doc.selection_model_dirty = true; } /// Create the initial application state. @@ -1656,7 +1705,9 @@ impl HcieIcedApp { let doc = IcedDocument { engine, - composite_pixels: Arc::new(composite_raw.clone()), + composite_pixels: crate::canvas::texture_update::SharedCompositePixels::new( + composite_raw.clone(), + ), composite_raw, name: "Untitled".to_string(), source_path: None, @@ -1684,6 +1735,8 @@ impl HcieIcedApp { pending_paste: None, pending_paste_as_new_layer: false, selection_mask: None, + selection_texture: None, + selection_model_dirty: false, selection_mask_dirty: std::cell::Cell::new(false), selection_bounds: None, selection_history: Vec::new(), @@ -1694,9 +1747,6 @@ impl HcieIcedApp { gradient_drag: None, vision_rect: None, text_draft: None, - selection_edge_cache: Default::default(), - marching_ants_edges: None, - selection_fill_spans: None, selection_mode: selection::SelectionMode::Replace, saved_selection_mask: None, quick_mask: false, @@ -1955,8 +2005,14 @@ impl HcieIcedApp { app.documents[0].engine.canvas_height() ); app.documents[0].composite_raw = composite; - app.documents[0].composite_pixels = - std::sync::Arc::new(app.documents[0].composite_raw.clone()); + app.documents[0] + .composite_pixels + .replace(&app.documents[0].composite_raw); + let selection = app.documents[0] + .engine + .get_selection_mask() + .map(ToOwned::to_owned); + set_document_selection_mask(&mut app.documents[0], selection); app.documents[0].full_upload.replace(true); app.documents[0].render_generation = app.documents[0].render_generation.wrapping_add(1); @@ -2183,6 +2239,8 @@ impl HcieIcedApp { } if let Some(next) = self.documents.iter().position(|document| document.modified) { self.active_doc = next; + self.documents[next].full_upload.replace(true); + self.documents[next].selection_mask_dirty.set(true); self.active_dialog = ActiveDialog::CloseConfirm; Task::none() } else { @@ -2214,6 +2272,8 @@ impl HcieIcedApp { self.vector_drag_last_angle = None; if let Some(doc) = self.documents.get_mut(self.active_doc) { doc.vector_drag_preview = None; + doc.full_upload.replace(true); + doc.selection_mask_dirty.set(true); } return; } @@ -2228,6 +2288,8 @@ impl HcieIcedApp { self.vector_drag_last = None; if let Some(doc) = self.documents.get_mut(self.active_doc) { doc.vector_drag_preview = None; + doc.full_upload.replace(true); + doc.selection_mask_dirty.set(true); } } @@ -2270,7 +2332,6 @@ impl HcieIcedApp { // structure is no longer valid, and a partial copy would leave zeros in the new buffer. if let Some(rect) = dirty_rect.filter(|_| !is_full_copy) { let w = doc.engine.canvas_width() as usize; - let _h = doc.engine.canvas_height() as usize; let [x0, y0, x1, y1] = rect; let x0 = x0 as usize; let y0 = y0 as usize; @@ -2280,42 +2341,25 @@ impl HcieIcedApp { let row_bytes = (x1 - x0) * 4; if row_bytes > 0 { unsafe { - let src_ptr = ptr; let dst_raw_ptr = doc.composite_raw.as_mut_ptr(); - - let mut pixels_ptr = std::ptr::null_mut(); - let mut need_new_arc = false; - - if let Some(pixels) = Arc::get_mut(&mut doc.composite_pixels) { - // Important: resize arc if needed before taking mutable pointer - if pixels.len() != len { - pixels.resize(len, 0); - } - pixels_ptr = pixels.as_mut_ptr(); - } else { - need_new_arc = true; - } - for y in y0..y1 { let offset = (y * w + x0) * 4; std::ptr::copy_nonoverlapping( - src_ptr.add(offset), + ptr.add(offset), dst_raw_ptr.add(offset), row_bytes, ); - if !pixels_ptr.is_null() { - std::ptr::copy_nonoverlapping( - src_ptr.add(offset), - pixels_ptr.add(offset), - row_bytes, - ); - } - } - - if need_new_arc { - doc.composite_pixels = Arc::new(doc.composite_raw.clone()); } } + if let Err(error) = doc.composite_pixels.copy_region_from( + &doc.composite_raw, + w as u32, + rect, + ) { + log::error!("failed to stage dirty composite region: {error}"); + doc.composite_pixels.replace(&doc.composite_raw); + doc.full_upload.replace(true); + } } let current = doc.dirty_region.replace(None); @@ -2325,16 +2369,7 @@ impl HcieIcedApp { unsafe { std::ptr::copy_nonoverlapping(ptr, doc.composite_raw.as_mut_ptr(), len); } - if let Some(pixels) = Arc::get_mut(&mut doc.composite_pixels) { - if pixels.len() != len { - pixels.resize(len, 0); - } - unsafe { - std::ptr::copy_nonoverlapping(ptr, pixels.as_mut_ptr(), len); - } - } else { - doc.composite_pixels = Arc::new(doc.composite_raw.clone()); - } + doc.composite_pixels.replace(&doc.composite_raw); doc.full_upload.replace(true); doc.dirty_region.replace(None); } @@ -2345,37 +2380,7 @@ impl HcieIcedApp { doc.engine.clear_dirty_flags(); } - // Update selection edge cache if mask changed - if let Some(mask) = doc.engine.get_selection_mask() { - doc.selection_mask = Some(mask.to_vec()); - doc.selection_mask_dirty.set(true); - doc.selection_bounds = selection::state::mask_bounds( - mask, - doc.engine.canvas_width(), - doc.engine.canvas_height(), - ); - doc.selection_fill_spans = Some(Arc::new(selection::state::selected_spans( - mask, - doc.engine.canvas_width(), - doc.engine.canvas_height(), - ))); - let edges = doc.selection_edge_cache.get(mask).unwrap_or_else(|| { - doc.selection_edge_cache.rebuild( - mask, - doc.engine.canvas_width(), - doc.engine.canvas_height(), - ) - }); - doc.marching_ants_edges = Some(edges); - } else { - doc.selection_mask = None; - doc.selection_mask_dirty.set(true); - doc.selection_bounds = None; - doc.selection_fill_spans = None; - doc.marching_ants_edges = None; - // Clear cache if selection is dropped so next time it won't mistakenly hit - doc.selection_edge_cache = Default::default(); - } + sync_document_selection_if_needed(doc); // Always refresh cached panel data (cheap operation) doc.cached_layers = doc.engine.layer_infos(); @@ -2896,11 +2901,7 @@ impl HcieIcedApp { self.documents[self.active_doc] .engine .create_selection_magic_wand(cx, cy, tol); - self.documents[self.active_doc].selection_bounds = { - let w = self.documents[self.active_doc].engine.canvas_width(); - let h = self.documents[self.active_doc].engine.canvas_height(); - Some((0, 0, w, h)) - }; + self.documents[self.active_doc].selection_model_dirty = true; self.refresh_composite_if_needed(); } Tool::Lasso => { @@ -2937,6 +2938,7 @@ impl HcieIcedApp { self.documents[self.active_doc] .engine .create_selection_polygon(&pts_ref); + self.documents[self.active_doc].selection_model_dirty = true; self.refresh_composite_if_needed(); } } else { @@ -2984,8 +2986,7 @@ impl HcieIcedApp { ); doc.engine.set_active_layer_pixels(cut_layer); doc.engine.selection_clear(); - doc.selection_mask = Some(mask_data.clone()); - doc.selection_mask_dirty.set(true); + set_document_selection_mask(doc, Some(mask_data.clone())); doc.transform_source = Some(tr.clone()); doc.transform_layer_baseline = Some(original_layer); doc.transform_selection_baseline = Some(mask_data); @@ -3307,8 +3308,6 @@ impl HcieIcedApp { .engine .create_selection_rect(sx, sy, ex, ey); self.combine_current_selection(None); - self.documents[self.active_doc].selection_bounds = - Some((sx, sy, ex - sx, ey - sy)); self.refresh_composite_if_needed(); } } @@ -3455,7 +3454,7 @@ impl HcieIcedApp { } else { doc.engine.selection_clear(); } - doc.selection_mask_dirty.set(true); + doc.selection_model_dirty = true; } } else { doc.engine.undo(); @@ -5211,6 +5210,8 @@ impl HcieIcedApp { let doc = &mut self.documents[self.active_doc]; let as_new_layer = doc.pending_paste_as_new_layer; doc.engine.resize_canvas(paste_w, paste_h); + doc.engine.selection_clear(); + set_document_selection_mask(doc, None); let canvas_w = doc.engine.canvas_width(); let canvas_h = doc.engine.canvas_height(); if let Some((pixels, w, h)) = doc.pending_paste.take() { @@ -5328,7 +5329,9 @@ impl HcieIcedApp { let history_current = engine.history_current(); let doc = IcedDocument { engine, - composite_pixels: Arc::new(composite_raw.clone()), + composite_pixels: crate::canvas::texture_update::SharedCompositePixels::new( + composite_raw.clone(), + ), composite_raw, name, source_path: None, @@ -5356,6 +5359,8 @@ impl HcieIcedApp { pending_paste: None, pending_paste_as_new_layer: false, selection_mask: None, + selection_texture: None, + selection_model_dirty: false, selection_mask_dirty: std::cell::Cell::new(false), selection_bounds: None, selection_history: Vec::new(), @@ -5366,9 +5371,6 @@ impl HcieIcedApp { gradient_drag: None, vision_rect: None, text_draft: None, - selection_edge_cache: Default::default(), - marching_ants_edges: None, - selection_fill_spans: None, selection_mode: selection::SelectionMode::Replace, saved_selection_mask: None, quick_mask: false, @@ -5505,6 +5507,7 @@ impl HcieIcedApp { } // Selection operations mutate the existing mask in place, so force overlay cache // synchronization before the next marching-ants frame. + self.documents[self.active_doc].selection_model_dirty = true; self.refresh_composite_if_needed(); self.active_dialog = ActiveDialog::None; return Task::perform(async {}, |_| Message::CompositeRefresh); @@ -5532,6 +5535,8 @@ impl HcieIcedApp { let w = self.dialog_image_size_width.max(1); let h = self.dialog_image_size_height.max(1); self.documents[self.active_doc].engine.resize_canvas(w, h); + self.documents[self.active_doc].engine.selection_clear(); + set_document_selection_mask(&mut self.documents[self.active_doc], None); self.documents[self.active_doc].engine.pre_tile_all_layers(); self.refresh_composite_if_needed(); self.documents[self.active_doc].full_upload.replace(true); @@ -5555,6 +5560,8 @@ impl HcieIcedApp { self.documents[self.active_doc] .engine .resize_canvas(new_w, new_h); + self.documents[self.active_doc].engine.selection_clear(); + set_document_selection_mask(&mut self.documents[self.active_doc], None); self.documents[self.active_doc].engine.pre_tile_all_layers(); self.refresh_composite_if_needed(); self.documents[self.active_doc].full_upload.replace(true); @@ -5586,9 +5593,6 @@ impl HcieIcedApp { .engine .create_selection_rect(sx, sy, ex, ey); self.combine_current_selection(previous); - // Store selection bounds for marching ants display - self.documents[self.active_doc].selection_bounds = - Some((sx, sy, ex - sx, ey - sy)); // Clear the drag-preview rect so the blue rectangle // disappears and the marching ants overlay takes over. self.documents[self.active_doc].selection_rect = None; @@ -5613,10 +5617,9 @@ impl HcieIcedApp { .selection_history .push(previous); - let w = self.documents[self.active_doc].engine.canvas_width(); - let h = self.documents[self.active_doc].engine.canvas_height(); self.documents[self.active_doc].engine.selection_all(); - self.documents[self.active_doc].selection_bounds = Some((0, 0, w, h)); + self.documents[self.active_doc].selection_model_dirty = true; + self.refresh_composite_if_needed(); } Message::Deselect => { let doc = &mut self.documents[self.active_doc]; @@ -5633,14 +5636,12 @@ impl HcieIcedApp { doc.selection_transform = None; doc.transform_drag_handle = TransformHandle::None; doc.engine.selection_clear(); - doc.selection_mask = None; - doc.selection_mask_dirty.set(true); - doc.selection_fill_spans = None; - doc.marching_ants_edges = None; + set_document_selection_mask(doc, None); return Task::perform(async {}, |_| Message::CompositeRefresh); } Message::SelectInverse => { self.documents[self.active_doc].engine.selection_invert(); + self.documents[self.active_doc].selection_model_dirty = true; self.refresh_composite_if_needed(); return Task::perform(async {}, |_| Message::CompositeRefresh); } @@ -5659,6 +5660,7 @@ impl HcieIcedApp { self.documents[self.active_doc] .engine .set_selection_mask(mask); + self.documents[self.active_doc].selection_model_dirty = true; self.refresh_composite_if_needed(); } } @@ -5762,9 +5764,7 @@ impl HcieIcedApp { }, ); doc.engine.selection_clear(); - doc.selection_bounds = None; - doc.selection_mask = None; - doc.selection_mask_dirty.set(true); + set_document_selection_mask(doc, None); doc.transform_drag_handle = TransformHandle::None; doc.transform_drag_start = None; doc.transform_original = None; @@ -5791,8 +5791,7 @@ impl HcieIcedApp { } if let Some(mask) = doc.transform_selection_baseline.take() { doc.engine.set_selection_mask(mask.clone()); - doc.selection_mask = Some(mask); - doc.selection_mask_dirty.set(true); + set_document_selection_mask(doc, Some(mask)); } doc.selection_transform = None; doc.transform_source = None; @@ -7060,11 +7059,6 @@ impl HcieIcedApp { .engine .create_selection_lasso(&pts_ref); self.combine_current_selection(previous); - self.documents[self.active_doc].selection_bounds = { - let w = self.documents[self.active_doc].engine.canvas_width(); - let h = self.documents[self.active_doc].engine.canvas_height(); - Some((0, 0, w, h)) - }; self.refresh_composite_if_needed(); } } @@ -7087,11 +7081,6 @@ impl HcieIcedApp { .engine .create_selection_polygon(&pts_ref); self.combine_current_selection(previous); - self.documents[self.active_doc].selection_bounds = { - let w = self.documents[self.active_doc].engine.canvas_width(); - let h = self.documents[self.active_doc].engine.canvas_height(); - Some((0, 0, w, h)) - }; self.refresh_composite_if_needed(); } } @@ -7157,6 +7146,8 @@ impl HcieIcedApp { Message::CropConfirm => { if let Some((x, y, w, h)) = self.documents[self.active_doc].crop_state.confirm() { self.documents[self.active_doc].engine.crop(x, y, w, h); + self.documents[self.active_doc].engine.selection_clear(); + set_document_selection_mask(&mut self.documents[self.active_doc], None); self.documents[self.active_doc].crop_state.cancel(); self.refresh_composite_if_needed(); return Task::perform(async {}, |_| Message::CompositeRefresh); @@ -7589,8 +7580,7 @@ impl HcieIcedApp { } Message::PasteImage => { // Check internal clipboard first - if let Some(transform) = self.documents[self.active_doc].internal_clipboard.clone() - { + if let Some(transform) = self.documents[self.active_doc].internal_clipboard.clone() { // Enter transform mode with the clipboard content let mut placed = transform; placed.pos.x += 10.0; @@ -7615,7 +7605,8 @@ impl HcieIcedApp { } Message::PasteAsNewLayer => { // Check internal clipboard first - if let Some(transform) = self.documents[self.active_doc].internal_clipboard.clone() { + if let Some(transform) = self.documents[self.active_doc].internal_clipboard.clone() + { let doc = &mut self.documents[self.active_doc]; let canvas_w = doc.engine.canvas_width(); let canvas_h = doc.engine.canvas_height(); @@ -7828,6 +7819,7 @@ impl HcieIcedApp { Ok(()) => { self.show_welcome = false; self.documents[self.active_doc].engine.pre_tile_all_layers(); + self.documents[self.active_doc].selection_model_dirty = true; self.documents[self.active_doc].name = path .file_name() .map(|n| n.to_string_lossy().to_string()) @@ -8203,7 +8195,9 @@ impl HcieIcedApp { let history_current = engine.history_current(); self.documents.push(IcedDocument { engine, - composite_pixels: Arc::new(composite_raw.clone()), + composite_pixels: crate::canvas::texture_update::SharedCompositePixels::new( + composite_raw.clone(), + ), composite_raw, name: "Untitled".to_string(), source_path: None, @@ -8231,6 +8225,8 @@ impl HcieIcedApp { pending_paste: None, pending_paste_as_new_layer: false, selection_mask: None, + selection_texture: None, + selection_model_dirty: false, selection_mask_dirty: std::cell::Cell::new(false), selection_bounds: None, selection_history: Vec::new(), @@ -8241,9 +8237,6 @@ impl HcieIcedApp { gradient_drag: None, vision_rect: None, text_draft: None, - selection_edge_cache: Default::default(), - marching_ants_edges: None, - selection_fill_spans: None, selection_mode: selection::SelectionMode::Replace, saved_selection_mask: None, quick_mask: false, @@ -8268,6 +8261,8 @@ impl HcieIcedApp { self.filter_params = serde_json::json!({}); } self.active_doc = idx; + self.documents[idx].full_upload.replace(true); + self.documents[idx].selection_mask_dirty.set(true); } } @@ -8289,6 +8284,8 @@ impl HcieIcedApp { } if modified { self.active_doc = idx; + self.documents[idx].full_upload.replace(true); + self.documents[idx].selection_mask_dirty.set(true); self.pending_document_close = Some(idx); self.active_dialog = ActiveDialog::CloseConfirm; } else { @@ -8384,8 +8381,16 @@ impl HcieIcedApp { .engine .get_composite_pixels(); self.documents[self.active_doc].composite_raw = composite; - self.documents[self.active_doc].composite_pixels = std::sync::Arc::new( - self.documents[self.active_doc].composite_raw.clone(), + self.documents[self.active_doc] + .composite_pixels + .replace(&self.documents[self.active_doc].composite_raw); + let selection = self.documents[self.active_doc] + .engine + .get_selection_mask() + .map(ToOwned::to_owned); + set_document_selection_mask( + &mut self.documents[self.active_doc], + selection, ); self.documents[self.active_doc].full_upload.replace(true); self.documents[self.active_doc].render_generation = self.documents @@ -8459,15 +8464,14 @@ impl HcieIcedApp { return Task::perform(async {}, |_| Message::LayerFlatten) } MenuCommand::SelectAll => { - let w = self.documents[self.active_doc].engine.canvas_width() as f32; - let h = self.documents[self.active_doc].engine.canvas_height() as f32; - self.documents[self.active_doc].selection_rect = Some((0.0, 0.0, w, h)); + return self.update(Message::SelectAll); } MenuCommand::Deselect => { - self.documents[self.active_doc].selection_rect = None; + return self.update(Message::Deselect); } MenuCommand::InvertSelection => { self.documents[self.active_doc].engine.selection_invert(); + self.documents[self.active_doc].selection_model_dirty = true; self.refresh_composite_if_needed(); return Task::perform(async {}, |_| Message::CompositeRefresh); } @@ -8669,6 +8673,7 @@ impl HcieIcedApp { doc.engine.set_active_layer_pixels(layer_pixels); } doc.engine.selection_clear(); + set_document_selection_mask(doc, Some(mask_data)); doc.selection_transform = Some(tr); self.refresh_composite_if_needed(); } @@ -8976,9 +8981,14 @@ impl HcieIcedApp { let composite = self.documents[document_index].engine.get_composite_pixels(); self.documents[document_index].composite_raw = composite; - self.documents[document_index].composite_pixels = std::sync::Arc::new( - self.documents[document_index].composite_raw.clone(), - ); + self.documents[document_index] + .composite_pixels + .replace(&self.documents[document_index].composite_raw); + let selection = self.documents[document_index] + .engine + .get_selection_mask() + .map(ToOwned::to_owned); + set_document_selection_mask(&mut self.documents[document_index], selection); self.documents[document_index].full_upload.replace(true); self.documents[document_index].render_generation = self.documents [document_index] @@ -10014,8 +10024,9 @@ fn active_index_after_document_close( #[cfg(test)] mod cycle_one_ux_tests { use super::{ - active_index_after_document_close, document_close_disposition, overlay_escape_target, - DocumentCloseDisposition, OverlayEscapeTarget, + active_index_after_document_close, consume_selection_sync_request, + document_close_disposition, overlay_escape_target, union_regions, DocumentCloseDisposition, + OverlayEscapeTarget, }; /// Confirms Escape dismisses subtools before menus and leaves editor cancellation untouched. @@ -10062,6 +10073,26 @@ mod cycle_one_ux_tests { assert_eq!(active_index_after_document_close(2, 2, 2), 1); assert_eq!(active_index_after_document_close(0, 2, 2), 0); } + + /// Ensures an unchanged selection cannot trigger work on every drawing refresh. + #[test] + fn selection_sync_request_is_consumed_once() { + let mut requested = true; + assert!(consume_selection_sync_request(&mut requested)); + for _ in 0..120 { + assert!(!consume_selection_sync_request(&mut requested)); + } + requested = true; + assert!(consume_selection_sync_request(&mut requested)); + } + + /// Protects the multi-event dirty accumulator from dropping an earlier update. + #[test] + fn dirty_region_union_retains_all_updates_before_view() { + let first = union_regions(None, [10, 20, 30, 40]); + let union = union_regions(Some(first), [25, 5, 50, 35]); + assert_eq!(union, [10, 5, 50, 40]); + } } /// Find the topmost text layer whose rasterized pixels contain the point diff --git a/hcie-iced-app/crates/hcie-iced-gui/src/canvas/canvas.wgsl b/hcie-iced-app/crates/hcie-iced-gui/src/canvas/canvas.wgsl index 8c563e0..5b6e757 100644 --- a/hcie-iced-app/crates/hcie-iced-gui/src/canvas/canvas.wgsl +++ b/hcie-iced-app/crates/hcie-iced-gui/src/canvas/canvas.wgsl @@ -109,28 +109,10 @@ fn fs_main(in: VertexOutput) -> @location(0) vec4 { if uniforms.has_selection > 0.5 { let sel_val = textureSample(selection_texture, selection_sampler, in.uv).r; - if sel_val > 0.5 { - // This pixel is selected — compute distance to nearest border - // by sampling 8 neighbors at 1-pixel offset in canvas space - let texel_w = 1.0 / uniforms.canvas_w; - let texel_h = 1.0 / uniforms.canvas_h; - - // Sample 8 neighbors to detect border (unrolled — WGSL forbids variable-indexed textureSample) - var is_border = false; - let n0 = textureSample(selection_texture, selection_sampler, in.uv + vec2(-texel_w, 0.0)).r; - let n1 = textureSample(selection_texture, selection_sampler, in.uv + vec2( texel_w, 0.0)).r; - let n2 = textureSample(selection_texture, selection_sampler, in.uv + vec2(0.0, -texel_h)).r; - let n3 = textureSample(selection_texture, selection_sampler, in.uv + vec2(0.0, texel_h)).r; - let n4 = textureSample(selection_texture, selection_sampler, in.uv + vec2(-texel_w, -texel_h)).r; - let n5 = textureSample(selection_texture, selection_sampler, in.uv + vec2( texel_w, -texel_h)).r; - let n6 = textureSample(selection_texture, selection_sampler, in.uv + vec2(-texel_w, texel_h)).r; - let n7 = textureSample(selection_texture, selection_sampler, in.uv + vec2( texel_w, texel_h)).r; - - if n0 <= 0.5 || n1 <= 0.5 || n2 <= 0.5 || n3 <= 0.5 || n4 <= 0.5 || n5 <= 0.5 || n6 <= 0.5 || n7 <= 0.5 { - is_border = true; - } - - if is_border { + if sel_val > 0.25 { + // Border membership is pre-encoded on selection changes so ordinary + // animation frames need one mask sample instead of nine. + if sel_val > 0.75 { // ── Marching ants border ────────────────────────────────── // Use canvas-space position for the dash pattern let canvas_x = in.uv.x * uniforms.canvas_w; diff --git a/hcie-iced-app/crates/hcie-iced-gui/src/canvas/edge_cache.rs b/hcie-iced-app/crates/hcie-iced-gui/src/canvas/edge_cache.rs deleted file mode 100644 index 8477177..0000000 --- a/hcie-iced-app/crates/hcie-iced-gui/src/canvas/edge_cache.rs +++ /dev/null @@ -1,78 +0,0 @@ -//! Cached extraction of exact selection-mask edges. -//! -//! **Purpose:** Avoids rescanning a full selection mask every animation frame. **Logic & Workflow:** -//! A content fingerprint, dimensions, and length identify the current mask; changed content is -//! rescanned into pixel-boundary line segments. **Side Effects / Dependencies:** Allocates only -//! when the mask changes and shares the result through `Arc`. - -use std::sync::Arc; - -#[derive(Clone, Default)] -pub struct SelectionEdgeCache { - fingerprint: u64, - mask_len: usize, - edges: Arc>, -} - -impl SelectionEdgeCache { - /// Returns cached edges only when mask content and dimensions still match. - pub fn get(&self, mask: &[u8]) -> Option>> { - if mask_fingerprint(mask) == self.fingerprint && mask.len() == self.mask_len { - Some(Arc::clone(&self.edges)) - } else { - None - } - } - - /// Rebuilds and stores exact pixel-boundary edges for `mask`. - pub fn rebuild(&mut self, mask: &[u8], w: u32, h: u32) -> Arc> { - let edges = Arc::new(extract_selection_edges(mask, w, h)); - self.fingerprint = mask_fingerprint(mask); - self.mask_len = mask.len(); - self.edges = Arc::clone(&edges); - edges - } -} - -/// Computes a fast deterministic fingerprint that detects in-place mask mutations. -fn mask_fingerprint(mask: &[u8]) -> u64 { - mask.iter().fold(0xcbf2_9ce4_8422_2325, |hash, byte| { - (hash ^ u64::from(*byte)).wrapping_mul(0x1000_0000_01b3) - }) -} - -pub fn extract_selection_edges( - selection_mask: &[u8], - canvas_width: u32, - canvas_height: u32, -) -> Vec<(u32, u32, u32, u32)> { - let w = canvas_width as usize; - let h = canvas_height as usize; - if selection_mask.len() != w * h { - return Vec::new(); - } - - let threshold: u8 = 128; - let mut edges = Vec::with_capacity(1024); - for y in 0..h { - for x in 0..w { - let idx = y * w + x; - if selection_mask[idx] <= threshold { - continue; - } - if x == 0 || selection_mask[idx - 1] <= threshold { - edges.push((x as u32, y as u32, x as u32, (y + 1) as u32)); - } - if x == w - 1 || selection_mask[idx + 1] <= threshold { - edges.push(((x + 1) as u32, y as u32, (x + 1) as u32, (y + 1) as u32)); - } - if y == 0 || selection_mask[idx - w] <= threshold { - edges.push((x as u32, y as u32, (x + 1) as u32, y as u32)); - } - if y == h - 1 || selection_mask[idx + w] <= threshold { - edges.push((x as u32, (y + 1) as u32, (x + 1) as u32, (y + 1) as u32)); - } - } - } - edges -} diff --git a/hcie-iced-app/crates/hcie-iced-gui/src/canvas/mod.rs b/hcie-iced-app/crates/hcie-iced-gui/src/canvas/mod.rs index 087f9f0..ca0a085 100644 --- a/hcie-iced-app/crates/hcie-iced-gui/src/canvas/mod.rs +++ b/hcie-iced-app/crates/hcie-iced-gui/src/canvas/mod.rs @@ -19,8 +19,8 @@ //! - Only dirty sub-regions are uploaded — never the full 33MB buffer //! - The checkerboard is procedural (in the fragment shader) — no CPU geometry -pub mod edge_cache; pub mod shader_canvas; +pub mod texture_update; // pub mod viewport; // pub mod render; @@ -34,6 +34,7 @@ use iced::{Element, Length, Point, Rectangle, Size, Vector}; use hcie_engine_api::{ZOOM_MAX, ZOOM_MIN}; use shader_canvas::CanvasShaderProgram; +use texture_update::TextureUpdate; // ─── Overlay (selection rect, vector preview, crosshair) ───────────────────── @@ -81,10 +82,6 @@ struct OverlayProgram { lasso_points: Vec<(u32, u32)>, /// Polygon selection points accumulated during click-by-click (canvas-space). polygon_points: Vec<(u32, u32)>, - /// Extracted edges for marching ants rendering. - _marching_ants_edges: Option>>, - /// Exact horizontal selected-pixel spans for irregular selection fill. - _selection_fill_spans: Option>>, /// Whether selected pixels use quick-mask red instead of the normal blue tint. _quick_mask: bool, /// Active brush diameter in canvas pixels for the footprint cursor. @@ -716,17 +713,11 @@ impl OverlayProgram { draw_ellipse(frame, top_cy); draw_ellipse(frame, bot_cy); frame.stroke( - &Path::line( - Point::new(left_scr, top_cy), - Point::new(left_scr, bot_cy), - ), + &Path::line(Point::new(left_scr, top_cy), Point::new(left_scr, bot_cy)), stroke_style.clone(), ); frame.stroke( - &Path::line( - Point::new(right_scr, top_cy), - Point::new(right_scr, bot_cy), - ), + &Path::line(Point::new(right_scr, top_cy), Point::new(right_scr, bot_cy)), stroke_style.clone(), ); } @@ -872,8 +863,8 @@ impl OverlayProgram { let end_angle = std::f32::consts::PI * 1.2; let crescent_path = Path::new(|b| { for i in 0..=segments { - let a = start_angle - + (end_angle - start_angle) * i as f32 / segments as f32; + let a = + start_angle + (end_angle - start_angle) * i as f32 / segments as f32; let hx = cx + rx * a.cos(); let hy = cy + ry * a.sin(); let (rpx, rpy) = rotate(hx, hy); @@ -1787,9 +1778,9 @@ impl canvas::Program for OverlayProgram { // clearly visible during fast pointer movements. if let Some(preview) = self.vector_drag_preview.as_ref() { let stroke_style = Stroke { - style: canvas::stroke::Style::Solid( - iced::Color::from_rgba(0.2, 0.9, 0.4, 0.95), - ), + style: canvas::stroke::Style::Solid(iced::Color::from_rgba( + 0.2, 0.9, 0.4, 0.95, + )), width: (2.5 / self.zoom.max(1.0)).max(1.0), ..Default::default() }; @@ -1874,10 +1865,7 @@ impl canvas::Program for OverlayProgram { let end_y = cy + indicator_radius * snap_angle.sin(); let radial_line = Path::line( Point::new(origin_x + cx * self.zoom, origin_y + cy * self.zoom), - Point::new( - origin_x + end_x * self.zoom, - origin_y + end_y * self.zoom, - ), + Point::new(origin_x + end_x * self.zoom, origin_y + end_y * self.zoom), ); frame.stroke( &radial_line, @@ -1895,10 +1883,7 @@ impl canvas::Program for OverlayProgram { // Small dot at arc end frame.fill( &Path::circle( - Point::new( - origin_x + end_x * self.zoom, - origin_y + end_y * self.zoom, - ), + Point::new(origin_x + end_x * self.zoom, origin_y + end_y * self.zoom), 3.0, ), snap_color, @@ -2226,16 +2211,32 @@ pub fn view<'a>( let pan_offset = doc.pan_offset; // ── Shader canvas (main rendering) ─────────────────────────────────── + let dirty_region = doc.dirty_region.take(); + let full_upload = doc.full_upload.replace(false); + let texture_update = if full_upload { + None + } else { + dirty_region.and_then(|region| { + match TextureUpdate::pack(&doc.composite_raw, engine_w, region) { + Ok(update) => Some(update), + Err(error) => { + log::error!("failed to pack canvas texture update: {error}"); + doc.full_upload.replace(true); + None + } + } + }) + }; let shader_program = CanvasShaderProgram { engine_w, engine_h, zoom, pan_offset, composite_pixels: doc.composite_pixels.clone(), - dirty_region: doc.dirty_region.take(), - full_upload: doc.full_upload.replace(false), + texture_update, + full_upload, space_pan: tool_state.space_pan, - selection_mask: doc.selection_mask.clone(), + selection_mask: doc.selection_texture.clone(), selection_dirty: doc.selection_mask_dirty.get(), anim_time: marching_ants_offset * 2.0, // Convert 0..1 to seconds (2s cycle) quick_mask: doc.quick_mask, @@ -2301,8 +2302,6 @@ pub fn view<'a>( gradient_drag: doc.gradient_drag, lasso_points: doc.lasso_points.clone(), polygon_points: doc.polygon_points.clone(), - _marching_ants_edges: doc.marching_ants_edges.clone(), - _selection_fill_spans: doc.selection_fill_spans.clone(), _quick_mask: doc.quick_mask, brush_size: tool_state.brush_size, selected_vector_bounds: sel_vec_bounds, diff --git a/hcie-iced-app/crates/hcie-iced-gui/src/canvas/shader_canvas.rs b/hcie-iced-app/crates/hcie-iced-gui/src/canvas/shader_canvas.rs index b888119..80383ec 100644 --- a/hcie-iced-app/crates/hcie-iced-gui/src/canvas/shader_canvas.rs +++ b/hcie-iced-app/crates/hcie-iced-gui/src/canvas/shader_canvas.rs @@ -16,6 +16,7 @@ //! All automated agents must preserve these structures and keep the wgpu/advanced feature dependencies. //! +use super::texture_update::{SharedCompositePixels, TextureUpdate}; use crate::app::Message; use iced::advanced::Shell; use iced::mouse; @@ -573,37 +574,9 @@ impl CanvasShaderPipeline { /// /// ## Arguments /// * `queue` — wgpu queue for the upload - /// * `region` — dirty bounds [x0, y0, x1, y1] in canvas pixels - /// * `full_buffer` — the complete composite_raw buffer (row-major RGBA) - /// * `canvas_w` — engine canvas width - fn upload_dirty_region( - &self, - queue: &wgpu::Queue, - region: [u32; 4], - full_buffer: &[u8], - canvas_w: u32, - ) { - let [x0, y0, x1, y1] = region; - let rw = x1.saturating_sub(x0); - let rh = y1.saturating_sub(y0); - - if rw == 0 || rh == 0 { - return; - } - - // Extract the sub-region rows from the full buffer into a contiguous block - let row_bytes = (rw * 4) as usize; - let mut region_data = vec![0u8; (rw * rh * 4) as usize]; - for row in 0..rh { - let src_offset = ((y0 + row) as usize * canvas_w as usize + x0 as usize) * 4; - let dst_offset = (row as usize) * row_bytes; - let src_end = src_offset + row_bytes; - if src_end <= full_buffer.len() && dst_offset + row_bytes <= region_data.len() { - region_data[dst_offset..dst_offset + row_bytes] - .copy_from_slice(&full_buffer[src_offset..src_end]); - } - } - + /// * `update` — tightly packed immutable dirty-region pixels + fn upload_dirty_region(&self, queue: &wgpu::Queue, update: &TextureUpdate) { + let [x0, y0, _, _] = update.region; let t0 = std::time::Instant::now(); queue.write_texture( wgpu::ImageCopyTexture { @@ -612,22 +585,22 @@ impl CanvasShaderPipeline { origin: wgpu::Origin3d { x: x0, y: y0, z: 0 }, aspect: wgpu::TextureAspect::All, }, - ®ion_data, + &update.pixels, wgpu::ImageDataLayout { offset: 0, - bytes_per_row: Some(rw * 4), - rows_per_image: Some(rh), + bytes_per_row: Some(update.bytes_per_row), + rows_per_image: Some(update.height()), }, wgpu::Extent3d { - width: rw, - height: rh, + width: update.width(), + height: update.height(), depth_or_array_layers: 1, }, ); let elapsed = t0.elapsed(); log::debug!( "[CanvasShaderPipeline::upload_dirty_region] {}×{} region at ({},{}) → {:.2}ms, {} bytes", - rw, rh, x0, y0, elapsed.as_secs_f64() * 1000.0, region_data.len() + update.width(), update.height(), x0, y0, elapsed.as_secs_f64() * 1000.0, update.pixels.len() ); } @@ -701,17 +674,16 @@ pub struct CanvasShaderPrimitive { pub zoom: f32, /// Pan offset in screen pixels pub pan_offset: Vector, - /// Dirty region from the engine [x0, y0, x1, y1], if any - pub dirty_region: Option<[u32; 4]>, - /// Full composite RGBA pixel data (shared reference via Arc) - /// Only the dirty sub-region is actually uploaded - pub composite_pixels: std::sync::Arc>, + /// Tightly packed dirty-region pixels for a normal partial upload. + pub texture_update: Option, + /// Stable complete RGBA recovery image for pipeline creation and resize. + pub composite_pixels: SharedCompositePixels, /// Whether this is a full texture upload (resize or first frame) pub full_upload: bool, /// Absolute widget bounds in the window pub bounds: Rectangle, - /// Selection mask data (1 byte per pixel, >128 = selected), if any - pub selection_mask: Option>, + /// Encoded selection data (0 unselected, 128 interior, 255 border), if any. + pub selection_mask: Option>>, /// Whether the selection mask has changed since last upload pub selection_dirty: bool, /// Animation time in seconds for marching ants @@ -740,15 +712,17 @@ impl shader::Primitive for CanvasShaderPrimitive { let t0 = std::time::Instant::now(); // ── Create or retrieve pipeline ────────────────────────────────── - if !storage.has::() { + let created_pipeline = !storage.has::(); + if created_pipeline { log::info!("[CanvasShaderPrimitive::prepare] First frame — creating pipeline"); + let pixels = self.composite_pixels.read(); let pipeline = CanvasShaderPipeline::new( device, queue, format, self.canvas_w, self.canvas_h, - &self.composite_pixels, + &pixels, ); storage.store(pipeline); } @@ -756,26 +730,20 @@ impl shader::Primitive for CanvasShaderPrimitive { let pipeline = storage.get_mut::().unwrap(); // ── Resize texture if canvas dimensions changed ────────────────── - if pipeline.texture_w != self.canvas_w || pipeline.texture_h != self.canvas_h { - pipeline.resize_texture( - device, - queue, - self.canvas_w, - self.canvas_h, - &self.composite_pixels, - ); - } else if self.full_upload { + if !created_pipeline + && (pipeline.texture_w != self.canvas_w || pipeline.texture_h != self.canvas_h) + { + let pixels = self.composite_pixels.read(); + pipeline.resize_texture(device, queue, self.canvas_w, self.canvas_h, &pixels); + } else if !created_pipeline && self.full_upload { // Full upload requested (e.g., after loading a file) - pipeline.resize_texture( - device, - queue, - self.canvas_w, - self.canvas_h, - &self.composite_pixels, - ); - } else if let Some(region) = self.dirty_region { + let pixels = self.composite_pixels.read(); + pipeline.resize_texture(device, queue, self.canvas_w, self.canvas_h, &pixels); + } else if !created_pipeline { // ── Partial upload: only the dirty sub-region ──────────────── - pipeline.upload_dirty_region(queue, region, &self.composite_pixels, self.canvas_w); + if let Some(update) = self.texture_update.as_ref() { + pipeline.upload_dirty_region(queue, update); + } } // ── Upload selection mask if changed ────────────────────────────── @@ -939,16 +907,16 @@ pub struct CanvasShaderProgram { pub zoom: f32, /// Pan offset in screen pixels (relative to centered position). pub pan_offset: Vector, - /// Full composite RGBA pixel buffer (shared via Arc to avoid copies). - pub composite_pixels: std::sync::Arc>, - /// Dirty region from the last engine composite [x0, y0, x1, y1]. - pub dirty_region: Option<[u32; 4]>, + /// Stable complete RGBA recovery image used only for full uploads. + pub composite_pixels: SharedCompositePixels, + /// Tightly packed dirty-region pixels for the next partial upload. + pub texture_update: Option, /// Whether a full texture upload is needed (first frame, resize, file load). pub full_upload: bool, /// Whether Space temporarily changes left-drag into panning. pub space_pan: bool, - /// Selection mask data (1 byte per pixel, >128 = selected), if any. - pub selection_mask: Option>, + /// Encoded selection data (0 unselected, 128 interior, 255 border), if any. + pub selection_mask: Option>>, /// Whether the selection mask has changed since last upload. pub selection_dirty: bool, /// Animation time in seconds for marching ants. @@ -1279,7 +1247,7 @@ impl shader::Program for CanvasShaderProgram { canvas_h: self.engine_h, zoom: self.zoom, pan_offset: self.pan_offset, - dirty_region: self.dirty_region, + texture_update: self.texture_update.clone(), composite_pixels: self.composite_pixels.clone(), full_upload: self.full_upload, bounds, @@ -1304,3 +1272,13 @@ impl shader::Program for CanvasShaderProgram { } } } + +#[cfg(test)] +mod shader_regression_tests { + /// Prevents restoration of the nine-sample per-fragment selection border detector. + #[test] + fn selection_overlay_uses_one_texture_sample() { + let shader = include_str!("canvas.wgsl"); + assert_eq!(shader.matches("textureSample(selection_texture").count(), 1); + } +} diff --git a/hcie-iced-app/crates/hcie-iced-gui/src/canvas/texture_update.rs b/hcie-iced-app/crates/hcie-iced-gui/src/canvas/texture_update.rs new file mode 100644 index 0000000..d0d21d5 --- /dev/null +++ b/hcie-iced-app/crates/hcie-iced-gui/src/canvas/texture_update.rs @@ -0,0 +1,290 @@ +//! CPU-side staging for persistent canvas texture updates. +//! +//! **Purpose:** Keeps the shader's recovery image in stable shared storage while carrying ordinary +//! frame updates as tightly packed dirty rectangles. **Logic & Workflow:** Engine pixels are copied +//! into the shared full image by row, then the final per-frame dirty union is packed once for wgpu. +//! **Side Effects / Dependencies:** Uses a standard-library read/write lock because iced may retain +//! shader primitives across application updates; no lock is held during normal partial GPU uploads. + +use std::sync::{Arc, RwLock, RwLockReadGuard}; + +/// Stable full-canvas pixels used only for pipeline creation, resize, and renderer recovery. +#[derive(Clone, Debug)] +pub struct SharedCompositePixels { + pixels: Arc>>, +} + +impl SharedCompositePixels { + /// Creates shared storage from one complete RGBA image. + /// + /// **Arguments:** `pixels` is a row-major RGBA image. **Returns:** Shared stable storage. + /// **Side Effects / Dependencies:** Allocates once and initializes an `RwLock`. + pub fn new(pixels: Vec) -> Self { + Self { + pixels: Arc::new(RwLock::new(pixels)), + } + } + + /// Replaces the complete recovery image after a document or canvas-size change. + /// + /// **Arguments:** `pixels` is the new complete RGBA image. **Returns:** Nothing. + /// **Side Effects / Dependencies:** Takes the write lock and may resize the backing allocation. + pub fn replace(&self, pixels: &[u8]) { + let mut target = self + .pixels + .write() + .unwrap_or_else(|error| error.into_inner()); + target.clear(); + target.extend_from_slice(pixels); + } + + /// Copies one RGBA rectangle from a complete source image into the recovery image. + /// + /// **Arguments:** `source` is a full row-major RGBA image, `canvas_w` is its pixel width, and + /// `region` is an exclusive `[x0, y0, x1, y1]` rectangle. **Returns:** Copied byte count, or an + /// error for malformed dimensions/bounds. **Side Effects / Dependencies:** Takes the write lock + /// only while copying the selected rows. + pub fn copy_region_from( + &self, + source: &[u8], + canvas_w: u32, + region: [u32; 4], + ) -> Result { + let validated = ValidatedRegion::new(source.len(), canvas_w, region, 4)?; + let mut target = self + .pixels + .write() + .unwrap_or_else(|error| error.into_inner()); + if target.len() != source.len() { + return Err("shared composite length does not match source"); + } + for row in 0..validated.height { + let offset = validated.source_offset(row); + let end = offset + validated.row_bytes; + target[offset..end].copy_from_slice(&source[offset..end]); + } + Ok(validated.row_bytes * validated.height) + } + + /// Borrows the complete image for a rare full GPU upload. + /// + /// **Arguments:** None. **Returns:** A read guard exposing the full RGBA bytes. + /// **Side Effects / Dependencies:** Takes a read lock; callers must keep its scope short. + pub fn read(&self) -> RwLockReadGuard<'_, Vec> { + self.pixels + .read() + .unwrap_or_else(|error| error.into_inner()) + } + + #[cfg(test)] + fn allocation_identity(&self) -> *const RwLock> { + Arc::as_ptr(&self.pixels) + } +} + +/// One tightly packed RGBA rectangle ready for `wgpu::Queue::write_texture`. +#[derive(Clone, Debug)] +pub struct TextureUpdate { + /// Exclusive canvas-space bounds `[x0, y0, x1, y1]`. + pub region: [u32; 4], + /// Number of bytes in one packed row. + pub bytes_per_row: u32, + /// Immutable packed RGBA rows. + pub pixels: Arc<[u8]>, +} + +impl TextureUpdate { + /// Packs one dirty RGBA rectangle from a complete image. + /// + /// **Arguments:** `source` is a full row-major RGBA image, `canvas_w` its width, and `region` + /// exclusive canvas bounds. **Returns:** A tightly packed upload payload or a validation error. + /// **Side Effects / Dependencies:** Allocates exactly `dirty_width * dirty_height * 4` bytes. + pub fn pack(source: &[u8], canvas_w: u32, region: [u32; 4]) -> Result { + let validated = ValidatedRegion::new(source.len(), canvas_w, region, 4)?; + let mut pixels = vec![0; validated.row_bytes * validated.height]; + for row in 0..validated.height { + let source_offset = validated.source_offset(row); + let target_offset = row * validated.row_bytes; + pixels[target_offset..target_offset + validated.row_bytes] + .copy_from_slice(&source[source_offset..source_offset + validated.row_bytes]); + } + Ok(Self { + region, + bytes_per_row: validated.row_bytes as u32, + pixels: pixels.into(), + }) + } + + /// Returns the packed rectangle width. + /// + /// **Arguments:** None. **Returns:** Width in pixels. **Side Effects / Dependencies:** None. + pub fn width(&self) -> u32 { + self.region[2] - self.region[0] + } + + /// Returns the packed rectangle height. + /// + /// **Arguments:** None. **Returns:** Height in pixels. **Side Effects / Dependencies:** None. + pub fn height(&self) -> u32 { + self.region[3] - self.region[1] + } +} + +/// Validated row-copy geometry shared by recovery and upload staging. +struct ValidatedRegion { + canvas_w: usize, + x0: usize, + y0: usize, + height: usize, + row_bytes: usize, + bytes_per_pixel: usize, +} + +impl ValidatedRegion { + /// Validates a rectangle against a complete tightly packed image. + /// + /// **Arguments:** Image length, width, exclusive region, and channel byte count. **Returns:** + /// Safe row-copy geometry or an error. **Side Effects / Dependencies:** None. + fn new( + source_len: usize, + canvas_w: u32, + region: [u32; 4], + bytes_per_pixel: usize, + ) -> Result { + let [x0, y0, x1, y1] = region; + if canvas_w == 0 || x0 >= x1 || y0 >= y1 || x1 > canvas_w { + return Err("invalid dirty rectangle"); + } + let row_stride = canvas_w as usize * bytes_per_pixel; + if row_stride == 0 || !source_len.is_multiple_of(row_stride) { + return Err("source length does not match canvas width"); + } + let canvas_h = source_len / row_stride; + if y1 as usize > canvas_h { + return Err("dirty rectangle exceeds canvas height"); + } + Ok(Self { + canvas_w: canvas_w as usize, + x0: x0 as usize, + y0: y0 as usize, + height: (y1 - y0) as usize, + row_bytes: (x1 - x0) as usize * bytes_per_pixel, + bytes_per_pixel, + }) + } + + /// Computes one source row's byte offset. + /// + /// **Arguments:** `row` is relative to the validated rectangle. **Returns:** Byte offset into + /// the complete image. **Side Effects / Dependencies:** None. + fn source_offset(&self, row: usize) -> usize { + ((self.y0 + row) * self.canvas_w + self.x0) * self.bytes_per_pixel + } +} + +#[cfg(test)] +mod tests { + use super::{SharedCompositePixels, TextureUpdate}; + use std::sync::Arc; + + const WIDTH: u32 = 3840; + const HEIGHT: u32 = 2160; + const FULL_BYTES: usize = WIDTH as usize * HEIGHT as usize * 4; + + /// Verifies retained shader references cannot trigger a full-canvas clone in the new path. + #[test] + fn retained_shader_reference_keeps_stable_allocation_and_dirty_budget() { + let mut source = vec![0; FULL_BYTES]; + let shared = SharedCompositePixels::new(source.clone()); + let shader_reference = shared.clone(); + let identity = shared.allocation_identity(); + let mut copied = 0usize; + let mut uploaded = 0usize; + + for sample in 0..120u32 { + let x0 = 100 + sample; + let region = [x0, 200, x0 + 48, 248]; + let marker = sample as u8; + for y in region[1]..region[3] { + let start = ((y * WIDTH + region[0]) * 4) as usize; + source[start..start + 48 * 4].fill(marker); + } + copied += shared.copy_region_from(&source, WIDTH, region).unwrap(); + let update = TextureUpdate::pack(&source, WIDTH, region).unwrap(); + copied += update.pixels.len(); + uploaded += update.pixels.len(); + assert_eq!(update.pixels.len(), 48 * 48 * 4); + } + + assert_eq!(identity, shader_reference.allocation_identity()); + assert_eq!(copied, 120 * 48 * 48 * 4 * 2); + assert_eq!(uploaded, 120 * 48 * 48 * 4); + assert_eq!(&*shared.read(), &source); + } + + /// Reproduces why the removed copy-on-write fallback cloned 33 MB per ordinary 4K update. + #[test] + fn legacy_arc_copy_on_write_reproduction_is_full_canvas() { + let mut pixels = Arc::new(vec![0u8; FULL_BYTES]); + let _shader_reference = pixels.clone(); + assert!(Arc::get_mut(&mut pixels).is_none()); + let fallback = Arc::new(pixels.as_ref().clone()); + assert_eq!(fallback.len(), 33_177_600); + } + + /// Confirms packed uploads preserve exact rows and reject malformed rectangles. + #[test] + fn packed_update_is_exact_and_bounds_checked() { + let source: Vec = (0..6 * 4 * 4).map(|value| value as u8).collect(); + let update = TextureUpdate::pack(&source, 6, [2, 1, 5, 3]).unwrap(); + assert_eq!(update.width(), 3); + assert_eq!(update.height(), 2); + assert_eq!(update.bytes_per_row, 12); + assert_eq!(&update.pixels[..12], &source[32..44]); + assert_eq!(&update.pixels[12..], &source[56..68]); + assert!(TextureUpdate::pack(&source, 6, [5, 1, 2, 3]).is_err()); + assert!(TextureUpdate::pack(&source, 6, [0, 0, 7, 1]).is_err()); + assert!(TextureUpdate::pack(&source, 6, [0, 0, 1, 5]).is_err()); + } + + /// Reports repeatable GUI staging latency and byte volume for a 4K stroke workload. + #[test] + #[ignore = "diagnostic benchmark; run with --ignored --nocapture"] + fn diagnostic_4k_dirty_staging_latency() { + let mut source = vec![0u8; FULL_BYTES]; + let shared = SharedCompositePixels::new(source.clone()); + let mut samples = Vec::with_capacity(120); + let mut dirty_bytes = 0usize; + for sample in 0..130u32 { + let x0 = 200 + sample * 8; + let region = [x0, 700, x0 + 48, 748]; + let start = std::time::Instant::now(); + for y in region[1]..region[3] { + let offset = ((y * WIDTH + region[0]) * 4) as usize; + source[offset..offset + 48 * 4].fill(sample as u8); + } + shared.copy_region_from(&source, WIDTH, region).unwrap(); + let update = TextureUpdate::pack(&source, WIDTH, region).unwrap(); + if sample >= 10 { + samples.push(start.elapsed().as_secs_f64() * 1000.0); + dirty_bytes += update.pixels.len(); + } + } + samples.sort_by(f64::total_cmp); + let percentile = |fraction: f64| { + let index = ((samples.len() - 1) as f64 * fraction).round() as usize; + samples[index] + }; + let legacy_bytes = samples.len() * FULL_BYTES; + println!( + "4K iced staging: p50={:.3}ms p95={:.3}ms max={:.3}ms dirty={}B legacy_cow={}B reduction={:.0}x", + percentile(0.5), + percentile(0.95), + samples[samples.len() - 1], + dirty_bytes, + legacy_bytes, + legacy_bytes as f64 / dirty_bytes as f64, + ); + assert_eq!(dirty_bytes, 120 * 48 * 48 * 4); + } +} diff --git a/hcie-iced-app/crates/hcie-iced-gui/src/selection/state.rs b/hcie-iced-app/crates/hcie-iced-gui/src/selection/state.rs index 9be9230..5b0cd38 100644 --- a/hcie-iced-app/crates/hcie-iced-gui/src/selection/state.rs +++ b/hcie-iced-app/crates/hcie-iced-gui/src/selection/state.rs @@ -73,11 +73,89 @@ pub fn mask_bounds(mask: &[u8], width: u32, height: u32) -> Option<(u32, u32, u3 )) } -/// Compresses selected pixels into horizontal spans for efficient irregular overlay filling. +/// Encoded R8 selection texture and exact non-zero source bounds. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct EncodedSelection { + /// Per-pixel values: zero is unselected, 128 is interior, and 255 is border. + pub pixels: Vec, + /// Exact `(x, y, width, height)` bounds of the source mask. + pub bounds: Option<(u32, u32, u32, u32)>, +} + +/// Encodes selection membership and its eight-neighbor border into one GPU texture. +/// +/// **Arguments:** `mask` is one alpha byte per pixel and `width`/`height` are its dimensions. +/// **Returns:** One R8 byte per pixel plus exact selected bounds; invalid dimensions return an empty +/// texture. **Logic & Workflow:** Selected pixels become interior unless an in-bounds neighboring +/// pixel is unselected. Out-of-bounds neighbors are ignored to match the shader's clamp-to-edge +/// sampling behavior. **Side Effects / Dependencies:** None. +pub fn encode_selection_texture(mask: &[u8], width: u32, height: u32) -> EncodedSelection { + if width == 0 || height == 0 || mask.len() != (width * height) as usize { + return EncodedSelection { + pixels: Vec::new(), + bounds: None, + }; + } + let mut pixels = vec![0; mask.len()]; + let mut min_x = width; + let mut min_y = height; + let mut max_x = 0; + let mut max_y = 0; + let mut found = false; + for y in 0..height { + for x in 0..width { + let index = (y * width + x) as usize; + if mask[index] <= 127 { + continue; + } + found = true; + min_x = min_x.min(x); + min_y = min_y.min(y); + max_x = max_x.max(x); + max_y = max_y.max(y); + + let mut border = false; + for dy in -1i32..=1 { + for dx in -1i32..=1 { + if dx == 0 && dy == 0 { + continue; + } + let nx = x as i32 + dx; + let ny = y as i32 + dy; + if nx >= 0 + && ny >= 0 + && nx < width as i32 + && ny < height as i32 + && mask[(ny as u32 * width + nx as u32) as usize] <= 127 + { + border = true; + break; + } + } + if border { + break; + } + } + pixels[index] = if border { 255 } else { 128 }; + } + } + EncodedSelection { + pixels, + bounds: found.then_some(( + min_x, + min_y, + max_x.saturating_sub(min_x).saturating_add(1), + max_y.saturating_sub(min_y).saturating_add(1), + )), + } +} + +/// Compresses selected pixels into horizontal spans for non-GPU callers. /// /// **Arguments:** `mask` is one alpha byte per pixel and `width`/`height` are its dimensions. /// **Returns:** Inclusive-exclusive `(x_start, y, x_end)` spans for alpha values above 127. -/// **Side Effects / Dependencies:** None. +/// **Side Effects / Dependencies:** None. The iced canvas no longer invokes this during frames; +/// the function remains part of the testable selection-state API. pub fn selected_spans(mask: &[u8], width: u32, height: u32) -> Vec<(u32, u32, u32)> { if width == 0 || height == 0 || mask.len() != (width * height) as usize { return Vec::new(); @@ -100,3 +178,40 @@ pub fn selected_spans(mask: &[u8], width: u32, height: u32) -> Vec<(u32, u32, u3 } spans } + +#[cfg(test)] +mod encoded_selection_tests { + use super::encode_selection_texture; + + /// Verifies interior, border, empty, and clamp-to-edge classifications. + #[test] + fn selection_texture_encodes_membership_and_border_once() { + let mut mask = vec![0; 25]; + for y in 1..4 { + for x in 1..4 { + mask[y * 5 + x] = 255; + } + } + let encoded = encode_selection_texture(&mask, 5, 5); + assert_eq!(encoded.bounds, Some((1, 1, 3, 3))); + assert_eq!(encoded.pixels[2 * 5 + 2], 128); + assert_eq!(encoded.pixels[1 * 5 + 1], 255); + assert_eq!(encoded.pixels[0], 0); + + let full = encode_selection_texture(&vec![255; 25], 5, 5); + assert!(full.pixels.iter().all(|value| *value == 128)); + assert_eq!(full.bounds, Some((0, 0, 5, 5))); + + let empty = encode_selection_texture(&vec![0; 25], 5, 5); + assert!(empty.pixels.iter().all(|value| *value == 0)); + assert_eq!(empty.bounds, None); + } + + /// Confirms feathered coverage follows the shader's greater-than-127 membership threshold. + #[test] + fn feathered_mask_uses_existing_threshold() { + let encoded = encode_selection_texture(&[127, 128, 255], 3, 1); + assert_eq!(encoded.pixels, vec![0, 255, 128]); + assert_eq!(encoded.bounds, Some((1, 0, 2, 1))); + } +}