From a6d092ceb00e1ace8f7b87f198aa44d657cddaf6 Mon Sep 17 00:00:00 2001 From: Elliot Graham Date: Fri, 9 Oct 2026 15:32:04 +0100 Subject: [PATCH] LR-MASK-BRUSH: separate brush subtraction from erasing (#516) --- crates/engine/src/cmd/masks.rs | 2 +- crates/ui-egui/src/menus.rs | 14 +++++ crates/ui-egui/src/panels/masking.rs | 16 ++--- crates/ui-egui/src/tests_masking.rs | 93 ++++++++++++++++++++++++++++ docs/control-protocol.md | 6 ++ docs/parity.md | 10 +-- 6 files changed, 128 insertions(+), 13 deletions(-) diff --git a/crates/engine/src/cmd/masks.rs b/crates/engine/src/cmd/masks.rs index dd1fa77c..d7299652 100644 --- a/crates/engine/src/cmd/masks.rs +++ b/crates/engine/src/cmd/masks.rs @@ -255,7 +255,7 @@ pub fn specs() -> Vec { } }; let m = &mut masks[i]; - if let Some(MaskComponent { name: None, shape: MaskShape::Brush { strokes }, .. }) = + if let Some(MaskComponent { shape: MaskShape::Brush { strokes }, .. }) = m.components.iter_mut().rev().find(|c| matches!(c.shape, MaskShape::Brush { .. })) { strokes.push(stroke); diff --git a/crates/ui-egui/src/menus.rs b/crates/ui-egui/src/menus.rs index 5b79548b..7cfbd47c 100644 --- a/crates/ui-egui/src/menus.rs +++ b/crates/ui-egui/src/menus.rs @@ -821,6 +821,20 @@ pub fn run_ui_command(app: &mut LightcraftApp, id: &str, p: &Value) -> Option { + // A new mask and Add/Subtract are selection operations, not the brush's + // Erase mode. Paint positively into a component with the requested op. + let create = if p.get("new").and_then(Value::as_bool) == Some(true) { + Some(app.run("mask.add", json!({"kind": "brush"}))) + } else { + p.get("op").map(|op| app.run("mask.addComponent", json!({"kind": "brush", "op": op}))) + }; + if let Some(result) = create { + if let Err(e) = result { + return Some(Err(e)); + } + app.ui.brush_erase = false; + app.ui.mask_overlay = true; + } app.ui.right = RightPanel::Masking; app.ui.view = ViewMode::Detail; app.ui.tool = "brush".into(); diff --git a/crates/ui-egui/src/panels/masking.rs b/crates/ui-egui/src/panels/masking.rs index 90fe284f..640bc4f1 100644 --- a/crates/ui-egui/src/panels/masking.rs +++ b/crates/ui-egui/src/panels/masking.rs @@ -138,11 +138,12 @@ pub fn show(app: &mut LightcraftApp, ui: &mut egui::Ui, id: PhotoId) { } "object" => start_object(app, ui.ctx(), "new"), "prompt" => start_describe(app, "new"), - "brush" | "linear" | "radial" => { + "brush" => { + let _ = app.run("tool.brush", json!({"new": true})); + } + "linear" | "radial" => { app.ui.tool = kind.to_string(); - if *kind != "brush" { - let _ = app.run("mask.add", json!({"kind": kind})); - } + let _ = app.run("mask.add", json!({"kind": kind})); } k => { let _ = app.run("mask.add", json!({"kind": k})); @@ -798,10 +799,11 @@ fn component_menu(app: &mut LightcraftApp, ui: &mut egui::Ui, op: &str) { if kind == "brush" && op == "intersect" { continue; } - if ui.button(label).clicked() { + let b = ui.button(label); + register(ui.ctx(), format!("maskComp:{op}:{kind}"), b.rect); + if b.clicked() { if kind == "brush" { - app.ui.tool = "brush".into(); - app.ui.brush_erase = op == "subtract"; + let _ = app.run("tool.brush", json!({"op": op})); } else { let _ = app.run("mask.addComponent", json!({"op": op, "kind": kind})); } diff --git a/crates/ui-egui/src/tests_masking.rs b/crates/ui-egui/src/tests_masking.rs index 0729a854..f0c5a762 100644 --- a/crates/ui-egui/src/tests_masking.rs +++ b/crates/ui-egui/src/tests_masking.rs @@ -39,6 +39,99 @@ fn develop(h: &Headless) -> lightcraft_develop::DevelopSettings { (*h.app.session.develop_of(id).unwrap_or_default()).clone() } +fn click(h: &mut Headless, id: &str) { + let r = h.request("ui.clickWidget", json!({"id": id}), T); + assert_eq!(r["ok"], true, "{id}: {r}"); +} + +fn brush_at(h: &mut Headless, x: f64, y: f64) { + pointer(h, json!([{"kind": "down", "x": x, "y": y}, {"kind": "up", "x": x, "y": y}])); +} + +/// Evaluate the real composite, with a deterministic embedded sky matte (no AI model required). +fn mask_coverage(h: &Headless, mask: usize, x: usize, y: usize) -> f32 { + use lightcraft_pipeline::{ + geometry::Frame, + masks::{MatteKind, Mattes, evaluate_one}, + }; + use lightcraft_raster::{Image, Plane, Rgb32f}; + let d = develop(h); + let mut sky = Image::::new(100, 100); + sky.data.fill(255); + let mut mattes = Mattes::default(); + mattes.push(MatteKind::Sky, sky); + let a = evaluate_one( + &d.masks[mask], + &Frame::new(100, 100, &Default::default(), true), + 100, + 100, + &Rgb32f::new(100, 100), + &Plane::new(100, 100), + 0.0, + Some(&mattes), + ); + a.get(x, y) +} + +#[test] +fn subtract_brush_removes_sky_coverage_and_add_restores_it() { + let mut h = detail("panel.masking"); + exec(&mut h, "mask.add", json!({"kind": "sky"})); + let r = h.request("ui.set", json!({"brushSize": 0.15, "brushFeather": 0.0, "brushFlow": 100.0}), T); + assert_eq!(r["ok"], true, "{r}"); + click(&mut h, "button:maskMinus:1"); + click(&mut h, "maskComp:subtract:brush"); + brush_at(&mut h, 0.5, 0.5); + assert!(mask_coverage(&h, 0, 50, 50) < 0.01, "Subtract > Brush must remove sky coverage"); + assert!(mask_coverage(&h, 0, 10, 10) > 0.99, "unpainted sky stays selected"); + let subtracted = develop(&h); + exec(&mut h, "edit.undo", json!({})); + assert!(mask_coverage(&h, 0, 50, 50) > 0.99); + exec(&mut h, "edit.redo", json!({})); + assert_eq!(develop(&h).masks, subtracted.masks); + + // Renaming must not detach subsequent strokes from the subtract component. + exec(&mut h, "mask.component", json!({"component": 1, "action": "rename", "name": "Sky cleanup"})); + brush_at(&mut h, 0.25, 0.5); + assert!(mask_coverage(&h, 0, 25, 50) < 0.01); + assert_eq!(develop(&h).masks[0].components.len(), 2); + + // Erase removes paint from the subtract component, restoring the underlying sky. + h.app.ui.brush_erase = true; + brush_at(&mut h, 0.5, 0.5); + assert!(mask_coverage(&h, 0, 50, 50) > 0.99); + click(&mut h, "button:maskPlus:1"); + click(&mut h, "maskComp:add:brush"); + brush_at(&mut h, 0.25, 0.5); + assert!(mask_coverage(&h, 0, 25, 50) > 0.99, "Add must be composed after Subtract"); + assert_eq!(develop(&h).masks[0].components.len(), 3); + h.settle(SETTLE); +} + +#[test] +fn new_brush_starts_a_separate_mask_in_paint_mode() { + let mut h = detail("panel.masking"); + exec(&mut h, "mask.add", json!({"kind": "sky"})); + let original = develop(&h).masks[0].clone(); + h.app.ui.brush_erase = true; + h.app.ui.mask_overlay = false; + click(&mut h, "maskNew:brush"); + assert_eq!(h.app.session.active_mask, Some(2)); + assert!(!h.app.ui.brush_erase); + assert!(h.app.ui.mask_overlay); + let r = h.request("ui.set", json!({"brushSize": 0.15, "brushFeather": 0.0, "brushFlow": 100.0}), T); + assert_eq!(r["ok"], true, "{r}"); + brush_at(&mut h, 0.5, 0.5); + assert!(mask_coverage(&h, 1, 50, 50) > 0.99); + h.app.ui.brush_erase = true; + brush_at(&mut h, 0.5, 0.5); + assert!(mask_coverage(&h, 1, 50, 50) < 0.01); + exec(&mut h, "edit.undo", json!({})); + assert!(mask_coverage(&h, 1, 50, 50) > 0.99); + assert_eq!(develop(&h).masks[0], original); + h.settle(SETTLE); +} + #[test] fn mask_overlay_keys_and_pins() { use lightcraft_pipeline::{MaskView, Overlay}; diff --git a/docs/control-protocol.md b/docs/control-protocol.md index 4247b559..14f04409 100644 --- a/docs/control-protocol.md +++ b/docs/control-protocol.md @@ -55,6 +55,12 @@ scrolling pans in both axes and respects the operating system's scrolling direct These gestures work in Detail (including editing tools and full-screen preview), Compare and Reference views, and only apply over their image areas. Panning stops at the image edges. +Mask brushing uses the UI command `tool.brush` through `engine.execute`. With `{}` it activates +painting on the current mask. `{"new": true}` creates a separate brush mask; `{"op": "add"}` or +`{"op": "subtract"}` appends a brush component to the selected mask. Creating either starts in +paint mode with the overlay visible. Brush Erase (or Alt while painting) removes strokes' coverage +within that brush component; it is distinct from subtracting the component from the whole mask. + ### When the library can't be saved With a persistent library, every command that changes something is written to the catalog journal (fsynced) before diff --git a/docs/parity.md b/docs/parity.md index 4b992c99..df0d667a 100644 --- a/docs/parity.md +++ b/docs/parity.md @@ -31,7 +31,7 @@ and prints the summary; `cargo xtask parity --write` refreshes the summary table | H. Crop & rotate (CROP) | 9 | 0 | 0 | 1 | 6/6 (100%) | 3/3 (100%) | | I. Remove / healing (REM) | 7 | 1 | 2 | 2 | 4/4 (100%) | 2/3 (67%) | | J. Red eye (EYE) | 2 | 0 | 0 | 0 | โ€” | 1/1 (100%) | -| K. Masking (MASK) | 14 | 4 | 5 | 0 | 8/8 (100%) | 5/5 (100%) | +| K. Masking (MASK) | 13 | 5 | 5 | 0 | 7/8 (88%) | 5/5 (100%) | | L. Presets (PRE) | 6 | 0 | 1 | 1 | 2/2 (100%) | 2/2 (100%) | | M. Versions & history (VER) | 5 | 0 | 0 | 0 | 1/1 (100%) | 3/3 (100%) | | N. Copy / paste / sync (SYNC) | 5 | 0 | 0 | 0 | 3/3 (100%) | 1/1 (100%) | @@ -48,9 +48,9 @@ and prints the summary; `cargo xtask parity --write` refreshes the summary table | Y. Menus | 82 | 0 | 4 | 8 | 47/47 (100%) | 23/23 (100%) | | Z. Keyboard shortcuts (desktop) | 75 | 3 | 3 | 1 | 49/52 (94%) | 23/24 (96%) | | Lightroom Classic extras | 32 | 11 | 39 | 9 | โ€” | 22/24 (92%) | -| **Total** | 394 | 38 | 84 | 36 | 193/200 (97%) | 142/152 (93%) | +| **Total** | 393 | 39 | 84 | 36 | 192/200 (96%) | 142/152 (93%) | -Weighted completion (โœ… = 1, ๐ŸŸก = ยฝ, ๐Ÿšซ left out): **80.0%** of 516 in-scope rows โ€” P0 98.2% of 200 ยท P1 95.7% of 152 ยท P2 42.9% of 163. +Weighted completion (โœ… = 1, ๐ŸŸก = ยฝ, ๐Ÿšซ left out): **79.9%** of 516 in-scope rows โ€” P0 98.0% of 200 ยท P1 95.7% of 152 ยท P2 42.9% of 163. ## Top gaps @@ -352,13 +352,13 @@ under the UI command that wraps it (a no-op error); the UI command now wins. | LR-MASK-OBJECTS | Object selection | P2 | ๐ŸŸก | `cmd:mask.add` (`object`, `prompt`), `cmd:mask.objectPoint`, `cmd:mask.refineDetail`, `cmd:segment.prepare`, `cmd:segment.model.status`, `cmd:segment.model.download`, `cmd:segment.model.cancel`, `crates/segment`, `crates/engine/src/segment/mod.rs` | SAM 3 in pure Rust (candle; Metal on macOS, CPU elsewhere): Object tile โ†’ click to include, โŒฅ-click to leave out; Describe tile โ†’ a text prompt selects every instance ("sky", "the red car"); both also as Add/Subtract/Intersect components; + / โˆ’ under the selected mask; comma lists (`car, road`); a zoomed-in detail pass for 5โ€“10ร— finer edges on small objects; per-selection Edge (hard โ†” soft). The model runs on its own worker thread (the UI never waits; panics become errors), is unloaded after 10 min idle, and is optional: the segmentation is stored with the mask (288ยฒ logits), so renders and exports never need it. Weights are never bundled (SAM License): without them the app offers a consented background download (mirrors, resume, timeouts, SHA-256) โ€” **but no default download location is configured yet** (users set `LIGHTCRAFT_SAM3_MIRRORS` or install by hand: the dialog names the three files, shows the model folder and links docs/ai-masks.md). Not verified against Lightroom's Select Object; no brush/box object mode; the first click on a photo waits for its analysis (~4 s on an M4 Pro, much longer on CPU) | | LR-MASK-PEOPLE | People parts | P2 | โฌœ | | shape exists (no UI): uses the photo's person / skin / hair / teeth / glasses mattes when it carries them (DNG semantic masks), else the subject heuristic | | LR-MASK-LANDSCAPE | Landscape classes | P2 | โฌœ | | shape exists, evaluates empty | -| LR-MASK-BRUSH | Brush mask | P0 | โœ… | `cmd:tool.brush`, `cmd:mask.brushStroke` (`autoMask`), `crates/pipeline/src/masks.rs` | size/feather/flow/density/erase; Auto Mask: dabs weighted by similarity to the colour under the dab centre, refined by a guided filter on luminance (CPU + GPU); no A/B brushes, no pressure | +| LR-MASK-BRUSH | Brush mask | P0 | ๐ŸŸก | `cmd:tool.brush`, `cmd:mask.brushStroke` (`autoMask`), `crates/pipeline/src/masks.rs` | size/feather/flow/density/erase; Create New Mask starts a separate brush in paint mode; live-preview gaps (#517) and stationary accumulation (#518) remain; Auto Mask: dabs weighted by similarity to the colour under the dab centre, refined by a guided filter on luminance (CPU + GPU); no A/B brushes, no pressure | | LR-MASK-LINEAR | Linear gradient | P0 | โœ… | `cmd:tool.linear`, `cmd:mask.update` | | | LR-MASK-RADIAL | Radial gradient | P0 | โœ… | `cmd:tool.radial`, `cmd:mask.update` | | | LR-MASK-COLORRANGE | Colour range | P1 | โœ… | `cmd:mask.add` (`colorRange`), `cmd:mask.sampleColor`, `cmd:mask.update`, `crates/pipeline/src/lib.rs` (`color_range_sample`) | Color tile โ†’ click the photo to sample (โ‡ง-click adds, up to 5; samples taken in the space the mask compares in), Pick button, Refine slider | | LR-MASK-LUMRANGE | Luminance range | P1 | โœ… | `cmd:mask.add` (`luminanceRange`), `cmd:mask.update`, `crates/ui-egui/src/panels/masking.rs` (`range_controls`) | range bar with two handles (one undo step per drag), Smoothness, Show Luminance Map (B&W photo with the range tinted) | | LR-MASK-DEPTHRANGE | Depth range | P2 | โฌœ | | shape exists, needs depth data | -| LR-MASK-COMBINE | Add / subtract / intersect | P0 | โœ… | `cmd:mask.addComponent` | | +| LR-MASK-COMBINE | Add / subtract / intersect | P0 | โœ… | `cmd:mask.addComponent`, `cmd:tool.brush` | Add/Subtract > Brush creates a component with the chosen operation; Erase removes paint within that brush component | | LR-MASK-INVERT | Invert | P0 | โœ… | `cmd:mask.invert` | | | LR-MASK-AMOUNT | Mask amount | P1 | โœ… | `cmd:mask.adjust` (`amount`), `crates/ui-egui/src/panels/masking.rs` | | | LR-MASK-FEATHER-EDGE | Refine mask edges | P2 | โœ… | `cmd:mask.refine`, `crates/pipeline/src/masks.rs` (`evaluate_one`) | Refine Edges slider per mask: guided filter on the photo's luminance snaps soft mask edges to the photo's edges (rendered on the CPU) |