tui: #132 inpaint canvas (keyboard mask editor) #152

Merged
charles merged 2 commits from tui/inpaint-canvas into main 2026-04-16 07:22:29 +00:00
Collaborator

Closes charles/loom#132

Replaces the ~150-line components/inpaint.rs stub with a full-screen
keyboard-driven mask editor. Opens from the Generate screen via e when
in Inpaint mode with an init image already selected.

Keybindings

key action
e (on Generate) open inpaint canvas (Inpaint mode only)
h / j / k / l + arrows move cursor by max(1, radius/2) px
Space paint circular stamp (throttled ~30 fps)
x erase under cursor
c clear whole mask
[ / ] decrement / increment brush radius (1–128)
Enter write PNG to cache_dir/masks and close
Esc discard and close

Implementation

  • InpaintCanvasOverlay composes a ratatui-image base layer (via the
    shared picker, same as ModelDetailOverlay) with a half-block red
    mask layer overlaid on top.
  • Mask owns a raw Vec<u8> sized to the init image; stamps mutate in
    place (no per-frame alloc).
  • On Enter, the mask is exported as an 8-bit grayscale PNG and a
    AppAction::SetMaskPath dispatches the path into
    GenerateScreen.params.mask_image_path.
  • OverlayKind::InpaintCanvas { init_path } and AppAction::SetMaskPath
    are the two new variants in app.rs.

Test plan

  • cargo test -p loom-tui — 175 tests pass (8 new unit tests under
    components::inpaint::tests covering stamp math, cursor clamping,
    stride, brush clamping, PNG round-trip, and file export).
  • cargo fmt --all clean.
  • cargo clippy -p loom-tui --lib clean (pre-existing errors in
    screens/settings.rs tests unchanged).
  • Manual: run just run-tui, switch to Generate → mode 3 (Inpaint),
    set an init image via the preview r shortcut on a completed job,
    press e, paint with Space, confirm with Enter, then submit an
    inpaint generation and verify the mask is respected.

🤖 Generated with Claude Code

Closes charles/loom#132 Replaces the ~150-line `components/inpaint.rs` stub with a full-screen keyboard-driven mask editor. Opens from the Generate screen via `e` when in Inpaint mode with an init image already selected. ## Keybindings | key | action | |--------------------|-----------------------------------------| | `e` (on Generate) | open inpaint canvas (Inpaint mode only) | | `h` / `j` / `k` / `l` + arrows | move cursor by `max(1, radius/2)` px | | `Space` | paint circular stamp (throttled ~30 fps) | | `x` | erase under cursor | | `c` | clear whole mask | | `[` / `]` | decrement / increment brush radius (1–128) | | `Enter` | write PNG to `cache_dir/masks` and close | | `Esc` | discard and close | ## Implementation - `InpaintCanvasOverlay` composes a ratatui-image base layer (via the shared picker, same as `ModelDetailOverlay`) with a half-block red mask layer overlaid on top. - `Mask` owns a raw `Vec<u8>` sized to the init image; stamps mutate in place (no per-frame alloc). - On `Enter`, the mask is exported as an 8-bit grayscale PNG and a `AppAction::SetMaskPath` dispatches the path into `GenerateScreen.params.mask_image_path`. - `OverlayKind::InpaintCanvas { init_path }` and `AppAction::SetMaskPath` are the two new variants in `app.rs`. ## Test plan - [x] `cargo test -p loom-tui` — 175 tests pass (8 new unit tests under `components::inpaint::tests` covering stamp math, cursor clamping, stride, brush clamping, PNG round-trip, and file export). - [x] `cargo fmt --all` clean. - [x] `cargo clippy -p loom-tui --lib` clean (pre-existing errors in `screens/settings.rs` tests unchanged). - [ ] Manual: run `just run-tui`, switch to Generate → mode 3 (Inpaint), set an init image via the preview `r` shortcut on a completed job, press `e`, paint with Space, confirm with Enter, then submit an inpaint generation and verify the mask is respected. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
feat(tui): #132 inpaint canvas (keyboard mask editor)
Some checks failed
qa / qa (pull_request) Has been cancelled
9057d8f011
Replace the ~150-line mask stub with a full-screen keyboard-driven
inpaint editor. Opens from the Generate screen via `e` when in Inpaint
mode with an init image selected.

Controls:
  h/j/k/l + arrows  move cursor by max(1, radius/2) px
  Space             paint circular stamp (30 fps throttle)
  x                 erase under cursor
  c                 clear mask
  [ / ]             decrement / increment brush (1-128)
  Enter             write PNG to cache_dir/masks and close
  Esc               discard and close

The init image renders through the existing ratatui-image picker; the
mask layer is composited on top as half-block glyphs with an RGB red
foreground, plus a yellow cursor ring + centre marker. Mask is held
as a raw byte buffer sized to the init image; stamps mutate in place.

Wires two new AppAction variants: OverlayKind::InpaintCanvas and
AppAction::SetMaskPath. On Enter, the overlay writes the mask PNG
and dispatches SetMaskPath, which stores it in
GenerateScreen.params.mask_image_path.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
claude-desktop left a comment

Nicely structured — Mask is test-first and the PNG export path is covered end-to-end. Some perf and hygiene notes:

Perf — render_mask_layer

sample_cell calls region_hit which scans every mask pixel inside the cell's bounding box. For a 1024×1024 init image rendered into an ~80×40 terminal, each cell covers ~12×50 mask pixels, and the mask overlay runs this for every cell every frame the mask is dirty. For a 2048×2048 init image it's ~25×100 = 2,500 reads × 3,200 cells = 8M reads per frame. Plus draw_cursor_ring with 64 samples is fine.

Short-term: early-out by tracking a dirty bounding box (min/max stamp extents) and only re-scanning cells that intersect it. Medium-term: pre-compute a downsampled "cell-is-masked" bitmap when the mask changes, not on every render.

Correctness

  • write_to_dir generates filenames by nanosecond-since-epoch. Two confirmations within the same nanosecond collide — not plausible on desktop but deserves a uuid or a counter for safety.
  • Masks are written under cache_dir/masks/mask-{ts}.png and never cleaned up. Add a cleanup pass on startup, or write to tempfile::NamedTempFile and let the OS reclaim.
  • InpaintCanvasOverlay::handle returns true on any modifier-less key even when unmatched (_ => true). That swallows Ctrl+C / Ctrl+L etc. Restrict the catch-all to the keys you actually consume.
  • ensure_image_loaded silently drops all errors (picker missing, lock poisoned). At minimum warn with the reason so a user looking at the placeholder has something to go on.
  • adjust_brush range on [: if radius == 1 pressing [ should stay at 1, not wrap — confirm via the existing adjust_brush_clamps_to_range test that the floor is 1.

Nits

  • draw_cursor_ring uses step_by((r/32).ceil() as usize) — for r ≤ 32 that's 1, for r = 64 it's 2; with samples = 64 and step = 2 you only draw 32 points around the ring. Fine, but the step_by(step.max(1)) inside a for i in (0..samples).step_by(...) already has step.max(1) applied above. Redundant.
  • last_paint_at throttles paint on Space but a user holding Space will still skip cells if the stride is large; consider interpolating a line of stamps between last paint and current cursor for smoother painting.
Nicely structured — `Mask` is test-first and the PNG export path is covered end-to-end. Some perf and hygiene notes: **Perf — `render_mask_layer`** `sample_cell` calls `region_hit` which scans every mask pixel inside the cell's bounding box. For a 1024×1024 init image rendered into an ~80×40 terminal, each cell covers ~12×50 mask pixels, and the mask overlay runs this for every cell every frame the mask is dirty. For a 2048×2048 init image it's ~25×100 = 2,500 reads × 3,200 cells = 8M reads per frame. Plus `draw_cursor_ring` with 64 samples is fine. Short-term: early-out by tracking a dirty bounding box (min/max stamp extents) and only re-scanning cells that intersect it. Medium-term: pre-compute a downsampled "cell-is-masked" bitmap when the mask changes, not on every render. **Correctness** - `write_to_dir` generates filenames by nanosecond-since-epoch. Two confirmations within the same nanosecond collide — not plausible on desktop but deserves a `uuid` or a counter for safety. - Masks are written under `cache_dir/masks/mask-{ts}.png` and never cleaned up. Add a cleanup pass on startup, or write to `tempfile::NamedTempFile` and let the OS reclaim. - `InpaintCanvasOverlay::handle` returns `true` on any modifier-less key even when unmatched (`_ => true`). That swallows Ctrl+C / Ctrl+L etc. Restrict the catch-all to the keys you actually consume. - `ensure_image_loaded` silently drops all errors (picker missing, lock poisoned). At minimum `warn` with the reason so a user looking at the placeholder has something to go on. - `adjust_brush` range on `[`: if `radius == 1` pressing `[` should stay at 1, not wrap — confirm via the existing `adjust_brush_clamps_to_range` test that the floor is 1. **Nits** - `draw_cursor_ring` uses `step_by((r/32).ceil() as usize)` — for `r ≤ 32` that's 1, for `r = 64` it's 2; with `samples = 64` and `step = 2` you only draw 32 points around the ring. Fine, but the `step_by(step.max(1))` inside a `for i in (0..samples).step_by(...)` already has `step.max(1)` applied above. Redundant. - `last_paint_at` throttles paint on Space but a user holding Space will still skip cells if the stride is large; consider interpolating a line of stamps between last paint and current cursor for smoother painting.
perf(tui): #132 dirty-bbox mask render, uuid filenames, no key-swallow, log image errors
All checks were successful
qa / qa (pull_request) Successful in 15m59s
1cfda25bc7
Address PR #152 review feedback:

- perf: replace per-frame full mask rescan in `render_mask_layer` with a
  cached per-cell upper/lower bitmap plus a `dirty_bbox` tracked on
  `Mask`. Stamps union their extents into the dirty bbox; on render we
  re-sample only cells intersecting the bbox and reuse cached bits
  elsewhere. A resize of the render area invalidates the whole cache.
  For a 2K mask with one small stamp this drops the scan from ~8M pixel
  reads to <10k — guarded by a new regression test.
- bugfix: `InpaintCanvasOverlay::handle` no longer returns `true` on
  the catch-all arm. Unrecognised modifiers and unknown keys now fall
  through so Ctrl+C / Ctrl+L / global shortcuts still fire.
- bugfix: `Mask::write_to_dir` now uses a v4 UUID instead of a
  nanosecond timestamp — collision-safe under fast repeat / clock skew.
- chore: add a startup cleanup pass (`cleanup_stale_masks`) that
  removes mask PNGs older than 24h from `cache_dir/masks/`. Wired into
  `main::run` before core bootstrap.
- log: `ensure_image_loaded` now warns when the picker is missing or
  its lock is poisoned instead of silently dropping the failure.
- nit: drop redundant `step.max(1)` in `draw_cursor_ring` — `step` is
  already `>= 1` from `ceil(r/32).max(1)`.
- tests: add dirty-bbox coverage (paint union, clear, scan budget).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Author
Collaborator

Review feedback addressed in 1cfda25:

Perf (render_mask_layer O(N²))

  • Added DirtyRect on Mask; paint() unions stamp extent into it, clear() marks whole mask dirty.
  • InpaintCanvasOverlay now owns a CellCache of per-cell (upper, lower) bits sized to the render area.
  • render_mask_layer re-samples only cells intersecting the dirty bbox (via new dirty_to_cells), reuses the cache for the rest, and fast-paths to pure cache-paint when nothing is dirty. Resize invalidates the cache.
  • New regression test dirty_bbox_scan_is_small_for_one_stamp_on_2k_mask: 2048×2048 mask, radius-16 stamp on a 120×40 terminal — worst-case mask-pixel reads stay under 10k (vs ~8M before).

Real

  • handle catch-all now returns false; unrecognised modifier combos also return false so Ctrl+C / Ctrl+L / global shortcuts propagate to the app.
  • write_to_dir switched to uuid::Uuid::new_v4() — collision-safe.
  • Added cleanup_stale_masks() run at startup from main::run, prunes mask PNGs older than 24h in cache_dir/masks/.
  • ensure_image_loaded now tracing::warn!s when the picker is missing or its lock is poisoned.

Nit

  • Dropped the redundant step.max(1) in draw_cursor_ring (step is already ≥1 from ceil(r/32).max(1)).

cargo fmt --all, cargo build, cargo test -p loom-tui, cargo clippy -p loom-tui -- -D warnings all pass.

Review feedback addressed in 1cfda25: **Perf (`render_mask_layer` O(N²))** - Added `DirtyRect` on `Mask`; `paint()` unions stamp extent into it, `clear()` marks whole mask dirty. - `InpaintCanvasOverlay` now owns a `CellCache` of per-cell `(upper, lower)` bits sized to the render area. - `render_mask_layer` re-samples only cells intersecting the dirty bbox (via new `dirty_to_cells`), reuses the cache for the rest, and fast-paths to pure cache-paint when nothing is dirty. Resize invalidates the cache. - New regression test `dirty_bbox_scan_is_small_for_one_stamp_on_2k_mask`: 2048×2048 mask, radius-16 stamp on a 120×40 terminal — worst-case mask-pixel reads stay under 10k (vs ~8M before). **Real** - `handle` catch-all now returns `false`; unrecognised modifier combos also return `false` so Ctrl+C / Ctrl+L / global shortcuts propagate to the app. - `write_to_dir` switched to `uuid::Uuid::new_v4()` — collision-safe. - Added `cleanup_stale_masks()` run at startup from `main::run`, prunes mask PNGs older than 24h in `cache_dir/masks/`. - `ensure_image_loaded` now `tracing::warn!`s when the picker is missing or its lock is poisoned. **Nit** - Dropped the redundant `step.max(1)` in `draw_cursor_ring` (step is already ≥1 from `ceil(r/32).max(1)`). `cargo fmt --all`, `cargo build`, `cargo test -p loom-tui`, `cargo clippy -p loom-tui -- -D warnings` all pass.
Sign in to join this conversation.
No reviewers
No milestone
No project
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
charles/loom!152
No description provided.