From 64ccc208537832fb0440466b66f957723ca20f30 Mon Sep 17 00:00:00 2001 From: Atmik Shetty Date: Tue, 13 Oct 2026 13:10:57 +0530 Subject: [PATCH] KEY-HIDEPINS: Tool Overlay modes (Auto / Always / Selected / Never) for spot and mask pins, Shift+H cycles them (fixes #224) The Remove tool's spot pins (Remove / Content-Aware / Heal / Clone) and the mask pins now follow a Tool Overlay mode, saved with the UI state: - Auto (default): shown while the pointer is over the photo (or a pin is dragged), hidden while a new spot or brush stroke is painted - Always: all of them - Selected: only the selected spot's or mask's - Never: none; the tool still works and a click paints a new spot A hidden spot can't be grabbed. Mask pins also need Show Pins (view.maskPins), which keeps its meaning and reply. Command view.toolOverlay {mode?: auto|always|selected|never} (no mode steps to the next), Shift+H (H stays Remove), View > Tool Overlay with the modes and the cycle item, and a Tool Overlay dropdown in the Remove and Masking panels. Headless tests cover each mode with the pointer over and away from the photo, grabbing, the key, the command, the menu, the dropdown, mask pins with Show Pins, and the ui.json round trip. --- crates/ui-egui/src/lib.rs | 2 + crates/ui-egui/src/menubar.rs | 13 ++ crates/ui-egui/src/menus.rs | 29 +++ crates/ui-egui/src/panels/detail.rs | 34 ++- crates/ui-egui/src/panels/masking.rs | 3 + crates/ui-egui/src/panels/right.rs | 24 +++ crates/ui-egui/src/state.rs | 68 ++++++ crates/ui-egui/src/tests_tool_overlay.rs | 259 +++++++++++++++++++++++ docs/parity-checklist.md | 17 +- docs/ui-parity.md | 5 +- 10 files changed, 440 insertions(+), 14 deletions(-) create mode 100644 crates/ui-egui/src/tests_tool_overlay.rs diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index 8248ee5a1..13f437441 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -118,6 +118,8 @@ mod tests_temp_slider; #[cfg(test)] mod tests_titlebar; #[cfg(test)] +mod tests_tool_overlay; +#[cfg(test)] mod tests_unsaved; #[cfg(test)] mod tests_wb_selector; diff --git a/crates/ui-egui/src/menubar.rs b/crates/ui-egui/src/menubar.rs index 04eea2439..e1e96e197 100644 --- a/crates/ui-egui/src/menubar.rs +++ b/crates/ui-egui/src/menubar.rs @@ -161,6 +161,7 @@ const LAYOUT: &[(&str, &[&str])] = &[ "view.maskOverlayMode", "view.maskOverlayColor", "view.maskPins", + "@Tool Overlay", "view.cropOverlay", "---", "@Sort", @@ -507,6 +508,18 @@ fn expanded(app: &LightcraftApp, name: &str) -> Option> { } v } + "Tool Overlay" => { + // when the spot and mask pins show; the last item steps to the next mode (Shift+H) + use crate::state::ToolOverlay; + let mut v: Vec = ToolOverlay::ALL + .into_iter() + .map(|m| item("view.toolOverlay", json!({"mode": m.name()}), m.label(), None, true, Some(app.ui.tool_overlay == m))) + .collect(); + v.push(MenuNode::Separator); + let shortcut = crate::shortcuts::shortcut_of(&app.ui.settings.keymap, "view.toolOverlay"); + v.push(item("view.toolOverlay", Value::Null, "Cycle Tool Overlay", shortcut, true, None)); + v + } "Grid Info" => ["filename", "exposure", "date"] .into_iter() .zip(["File Name", "Exposure (shutter · aperture · ISO)", "Capture Date"]) diff --git a/crates/ui-egui/src/menus.rs b/crates/ui-egui/src/menus.rs index 141d6d782..72e0d714c 100644 --- a/crates/ui-egui/src/menus.rs +++ b/crates/ui-egui/src/menus.rs @@ -98,6 +98,10 @@ pub const UI_COMMANDS: &[UiCommand] = &[ ("view.maskOverlayMode", "Cycle Mask Overlay Mode", None, "View"), ("view.maskOverlayColor", "Cycle Mask Overlay Color", None, "View"), ("view.maskPins", "Show Mask Pins", None, "View"), + // {mode?: auto | always | selected | never}: when spot and mask pins show (steps to the next + // mode when omitted; Shift+H, like Classic's H for pins, since H opens Remove). View ▸ Tool + // Overlay and the Remove panel list the modes. + ("view.toolOverlay", "Cycle Tool Overlay", Some("Shift+H"), ""), ("view.visualizeSpots", "Visualize Spots", Some("A"), "View"), ("view.cropOverlay", "Cycle Crop Overlay", Some("Shift+O"), "View"), ("view.cropOverlayOrientation", "Cycle Crop Overlay Orientation", None, "View"), @@ -319,6 +323,18 @@ fn bool_param(command: &str, p: &Value, key: &str) -> Result, Strin } } +/// What Shift+H says it switched to (the pins themselves may be off screen). +fn tool_overlay_toast(mode: crate::state::ToolOverlay) -> String { + use crate::state::ToolOverlay; + crate::i18n::tr(match mode { + ToolOverlay::Auto => "Tool Overlay: Auto", + ToolOverlay::Always => "Tool Overlay: Always", + ToolOverlay::Selected => "Tool Overlay: Selected", + ToolOverlay::Never => "Tool Overlay: Never", + }) + .to_string() +} + fn appearance_json(app: &LightcraftApp) -> Value { let s = &app.ui.settings; json!({"mode": s.appearance_mode, "darkTheme": s.dark_theme, "lightTheme": s.light_theme}) @@ -894,6 +910,19 @@ pub fn run_ui_command(app: &mut LightcraftApp, id: &str, p: &Value) -> Option { + use crate::state::ToolOverlay; + let mode = match p.get("mode") { + None | Some(Value::Null) => app.ui.tool_overlay.next(), + Some(v) => match v.as_str().and_then(ToolOverlay::parse) { + Some(m) => m, + None => return Some(Err(format!("view.toolOverlay: unknown mode {v} (auto|always|selected|never)"))), + }, + }; + app.ui.tool_overlay = mode; + app.toast(&ctx, tool_overlay_toast(mode)); + Ok(json!({"mode": mode.name()})) + } "view.visualizeSpots" => { // like Lightroom's A: opens the Remove tool with the view on, or toggles it there if app.ui.right == RightPanel::Remove { diff --git a/crates/ui-egui/src/panels/detail.rs b/crates/ui-egui/src/panels/detail.rs index 66f9c7997..c92767c12 100644 --- a/crates/ui-egui/src/panels/detail.rs +++ b/crates/ui-egui/src/panels/detail.rs @@ -10,7 +10,7 @@ use serde_json::json; use crate::LightcraftApp; use crate::render::Slot; -use crate::state::{BeforeAfter, RightPanel, Zoom}; +use crate::state::{BeforeAfter, RightPanel, ToolOverlay, Zoom}; use crate::theme::Tokens; use crate::widgets::register; @@ -1834,6 +1834,12 @@ fn mask_overlay(app: &mut LightcraftApp, ui: &mut egui::Ui, resp: &egui::Respons let p = &ui.painter_at(clip); // (mask, component, handle, screen position) of everything that can be grabbed let mut grips: Vec<(u32, usize, u8, Pos2)> = Vec::new(); + // pins follow Show Pins and the Tool Overlay mode (Shift+H); Auto hides them while brushing + let (mode, live) = (app.ui.tool_overlay, pins_live(app, resp, map)); + let mut pins = p.clone(); + if mode == ToolOverlay::Auto && matches!(app.gesture, Some(Gesture::Brush { .. })) { + pins.set_invisible(); + } for m in &d.masks { let sel = Some(m.id) == active; for (ci, c) in m.components.iter().enumerate() { @@ -1886,10 +1892,11 @@ fn mask_overlay(app: &mut LightcraftApp, ui: &mut egui::Ui, resp: &egui::Respons _ => {} } if app.ui.mask_pins + && mode.shows(sel, live) && let Some(at) = component_pin(&c.shape) { let q = map.screen(at); - pin(p, q, sel); + pin(&pins, q, sel); register(ui.ctx(), format!("maskPin:{}:{ci}", m.id), Rect::from_center_size(q, vec2(14.0, 14.0))); grips.push((m.id, ci, 0, q)); } @@ -2066,6 +2073,12 @@ fn frame_long_norm(map: &CanvasMap) -> (f64, f64) { (l / x.max(1e-9), l / y.max(1e-9)) } +/// Auto's condition for the Tool Overlay (Shift+H): the pointer is over the photo, or a pin is +/// being dragged. +fn pins_live(app: &LightcraftApp, resp: &egui::Response, map: &CanvasMap) -> bool { + resp.hover_pos().is_some_and(|q| map.rect.contains(q)) || matches!(app.gesture, Some(Gesture::SpotMove { .. } | Gesture::MaskHandle { .. })) +} + fn pin(p: &egui::Painter, c: Pos2, sel: bool) { p.circle_filled(c, 6.0, if sel { Color32::from_rgb(1, 101, 221) } else { Color32::from_gray(200) }); p.circle_stroke(c, 6.0, Stroke::new(1.5, Color32::WHITE)); @@ -2075,15 +2088,28 @@ fn pin(p: &egui::Painter, c: Pos2, sel: bool) { /// The Remove tool on the photo: every spot's outline and pin (the selected one with its source), /// click a pin to select its spot, drag a target or source to move it, paint elsewhere to add one. +/// The Tool Overlay mode (Shift+H) says which spots show; a hidden spot can't be grabbed, so a +/// click there adds a new one. fn remove_overlay(app: &mut LightcraftApp, ui: &mut egui::Ui, resp: &egui::Response, map: &CanvasMap, d: &DevelopSettings) { let long = (map.rect.width().max(map.rect.height())) as f64; - let p = ui.painter_at(app.canvas_rect.unwrap_or(map.rect)); let active = app.session.active_spot.filter(|i| *i < d.spots.len()); + let (mode, live) = (app.ui.tool_overlay, pins_live(app, resp, map)); + // Auto also hides them while a new spot is painted + let painting = mode == ToolOverlay::Auto && matches!(app.gesture, Some(Gesture::Spot { .. })); + let p = ui.painter_at(app.canvas_rect.unwrap_or(map.rect)); + let mut pins = p.clone(); + if painting { + pins.set_invisible(); + } // (spot, is source, screen centre, radius) of everything that can be grabbed let mut grips: Vec<(usize, bool, Pos2, f32)> = Vec::new(); for (i, s) in d.spots.iter().enumerate() { let r = (s.size * long) as f32; let sel = Some(i) == active; + if !mode.shows(sel, live) { + continue; + } + let p = &pins; let col = Color32::from_white_alpha(if sel { 230 } else { 150 }); for q in &s.points { p.circle_stroke(map.screen(*q), r, Stroke::new(if sel { 1.5 } else { 1.0 }, col)); @@ -2100,7 +2126,7 @@ fn remove_overlay(app: &mut LightcraftApp, ui: &mut egui::Ui, resp: &egui::Respo register(ui.ctx(), format!("spotSource:{i}"), Rect::from_center_size(sq, vec2(14.0, 14.0))); grips.push((i, true, sq, r)); } - pin(&p, tq, sel); + pin(p, tq, sel); register(ui.ctx(), format!("spotPin:{i}"), Rect::from_center_size(tq, vec2(14.0, 14.0))); grips.push((i, false, tq, r)); } diff --git a/crates/ui-egui/src/panels/masking.rs b/crates/ui-egui/src/panels/masking.rs index 534a207ca..4095625f6 100644 --- a/crates/ui-egui/src/panels/masking.rs +++ b/crates/ui-egui/src/panels/masking.rs @@ -498,6 +498,9 @@ fn overlay_options(app: &mut LightcraftApp, ui: &mut egui::Ui) { if ui.checkbox(&mut pins, crate::i18n::tr("Show Pins")).changed() { let _ = app.run("view.maskPins", json!({"show": pins})); } + if app.ui.mask_pins { + super::right::tool_overlay_options(app, ui); + } } /// Controls for a range component: the selected range (two handles over a dark → light bar), diff --git a/crates/ui-egui/src/panels/right.rs b/crates/ui-egui/src/panels/right.rs index 8d0cc0d3a..330b8d885 100644 --- a/crates/ui-egui/src/panels/right.rs +++ b/crates/ui-egui/src/panels/right.rs @@ -454,6 +454,8 @@ fn remove(app: &mut LightcraftApp, ui: &mut egui::Ui, id: PhotoId) { app.ui.spots_threshold = v as f32; } padded(ui, |ui| { + tool_overlay_options(app, ui); + ui.add_space(4.0); if !d.spots.is_empty() && text_button(ui, "removeClear", crate::i18n::tr("Delete all spots"), false).clicked() { for i in (0..d.spots.len()).rev() { let _ = app.run("spot.delete", json!({"index": i})); @@ -462,6 +464,28 @@ fn remove(app: &mut LightcraftApp, ui: &mut egui::Ui, id: PhotoId) { }); } +/// Tool Overlay: when the spot and mask pins show on the photo (Auto / Always / Selected / Never; +/// Shift+H steps through them). Remove and Masking panels. +pub(crate) fn tool_overlay_options(app: &mut LightcraftApp, ui: &mut egui::Ui) { + use crate::state::ToolOverlay; + let t = Tokens::get(ui.ctx()); + ui.horizontal(|ui| { + ui.label(egui::RichText::new(crate::i18n::tr("Tool Overlay")).color(t.text_dim)); + let current = app.ui.tool_overlay; + let r = crate::widgets::dropdown(ui, "toolOverlay", crate::i18n::tr(current.label()), t.font(12.5), t.text_label) + .on_hover_text(crate::i18n::tr("When pins show on the photo (Shift+H cycles)")); + egui::Popup::menu(&r).show(|ui| { + for m in ToolOverlay::ALL { + let item = ui.selectable_label(m == current, crate::i18n::tr(m.label())); + register(ui.ctx(), format!("toolOverlay:{}", m.name()), item.rect); + if item.clicked() { + let _ = app.run("view.toolOverlay", json!({"mode": m.name()})); + } + } + }); + }); +} + fn red_eye(app: &mut LightcraftApp, ui: &mut egui::Ui, id: PhotoId) { let d = app.session.develop_of(id).unwrap_or_default(); header(ui, "Red Eye"); diff --git a/crates/ui-egui/src/state.rs b/crates/ui-egui/src/state.rs index 6c4dd3f6a..84ac60780 100644 --- a/crates/ui-egui/src/state.rs +++ b/crates/ui-egui/src/state.rs @@ -83,6 +83,70 @@ pub enum CropOverlay { None, } +/// Tool Overlay: when the retouching tools' pins (Remove / Heal / Clone spots, mask pins) show on +/// the photo. Shift+H steps through the modes in this order (`view.toolOverlay`). +#[derive(Clone, Copy, Debug, Default, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "camelCase")] +pub enum ToolOverlay { + /// While the pointer is over the photo (hidden while a new spot or brush stroke is painted). + #[default] + Auto, + Always, + /// Only the selected spot's or mask's. + Selected, + /// None; the tool still works (a click still adds a spot). + Never, +} + +impl ToolOverlay { + pub const ALL: [ToolOverlay; 4] = [ToolOverlay::Auto, ToolOverlay::Always, ToolOverlay::Selected, ToolOverlay::Never]; + + /// The id used by `view.toolOverlay {mode}` and the saved settings. + pub fn name(self) -> &'static str { + match self { + ToolOverlay::Auto => "auto", + ToolOverlay::Always => "always", + ToolOverlay::Selected => "selected", + ToolOverlay::Never => "never", + } + } + + pub fn parse(s: &str) -> Option { + ToolOverlay::ALL.into_iter().find(|m| m.name().eq_ignore_ascii_case(s.trim())) + } + + /// Menu label. + pub fn label(self) -> &'static str { + match self { + ToolOverlay::Auto => "Auto", + ToolOverlay::Always => "Always", + ToolOverlay::Selected => "Selected", + ToolOverlay::Never => "Never", + } + } + + /// The mode Shift+H steps to. + pub fn next(self) -> ToolOverlay { + match self { + ToolOverlay::Auto => ToolOverlay::Always, + ToolOverlay::Always => ToolOverlay::Selected, + ToolOverlay::Selected => ToolOverlay::Never, + ToolOverlay::Never => ToolOverlay::Auto, + } + } + + /// Whether a pin shows: `selected` = its spot or mask is the selected one, `live` = the pointer + /// is over the photo (or a pin is being dragged), which is what Auto waits for. + pub fn shows(self, selected: bool, live: bool) -> bool { + match self { + ToolOverlay::Auto => live, + ToolOverlay::Always => true, + ToolOverlay::Selected => selected, + ToolOverlay::Never => false, + } + } +} + /// Which view the app opens in. #[derive(Clone, Copy, Debug, Default, PartialEq, Eq, Serialize, Deserialize)] #[serde(rename_all = "camelCase")] @@ -544,6 +608,9 @@ pub struct UiState { pub mask_overlay_color: [u8; 3], pub mask_overlay_opacity: f32, pub mask_pins: bool, + /// When the Remove tool's spot pins and the mask pins show (Shift+H cycles). Mask pins also + /// need [`mask_pins`](Self::mask_pins). + pub tool_overlay: ToolOverlay, /// Masking: show every visible mask's region as one overlay at once (Ctrl+Shift+O / Cmd+Shift+O), /// instead of just the selected mask's ([`mask_overlay`]). pub mask_overlay_all: bool, @@ -1062,6 +1129,7 @@ impl Default for UiState { mask_overlay_color: [230, 30, 40], mask_overlay_opacity: 50.0, mask_pins: true, + tool_overlay: ToolOverlay::Auto, mask_overlay_all: false, crop_overlay: CropOverlay::Thirds, crop_default_aspect: None, diff --git a/crates/ui-egui/src/tests_tool_overlay.rs b/crates/ui-egui/src/tests_tool_overlay.rs new file mode 100644 index 000000000..35426ac44 --- /dev/null +++ b/crates/ui-egui/src/tests_tool_overlay.rs @@ -0,0 +1,259 @@ +//! Headless tests of the Tool Overlay (issue #224): when the Remove tool's spot pins and the mask +//! pins show (Auto / Always / Selected / Never), Shift+H and `view.toolOverlay` to change it. + +use std::time::Duration; + +use serde_json::json; + +use crate::headless::Headless; +use crate::menubar::MenuNode; +use crate::state::{RightPanel, ToolOverlay}; +use crate::{LightcraftApp, Services}; + +const T: Duration = Duration::from_secs(20); + +fn detail(panel: &str) -> Headless { + let services = Services { png: None, ..Default::default() }; + let app = LightcraftApp::new(lightcraft_engine::Session::with_demo(), services); + let mut h = Headless::new(app, [1200.0, 800.0], 1.0); + let r = h.request("ui.set", json!({"view": "detail"}), T); + assert_eq!(r["ok"], true, "{r}"); + let r = h.request("engine.execute", json!({"command": panel}), T); + assert_eq!(r["ok"], true, "{r}"); + h +} + +fn exec(h: &mut Headless, command: &str, params: serde_json::Value) -> serde_json::Value { + let r = h.request("engine.execute", json!({"command": command, "params": params}), T); + assert_eq!(r["ok"], true, "{command}: {r}"); + r["result"].clone() +} + +/// The Remove tool with three spots (the last one selected). +fn three_spots() -> Headless { + let mut h = detail("panel.remove"); + assert_eq!(h.app.ui.right, RightPanel::Remove); + for (x, y) in [(0.25, 0.3), (0.5, 0.6), (0.75, 0.3)] { + exec(&mut h, "spot.add", json!({"mode": "heal", "points": [[x, y]], "size": 0.03})); + } + let id = h.app.session.active().expect("active photo"); + assert_eq!(h.app.session.develop_of(id).unwrap_or_default().spots.len(), 3); + assert_eq!(h.app.session.active_spot, Some(2)); + h +} + +/// Pointer over the photo (normalized image coordinates), or away from it (the top bar). +fn hover(h: &mut Headless, over: bool) { + let r = if over { + h.request("ui.pointer", json!({"events": [{"kind": "move", "x": 0.1, "y": 0.9}]}), T) + } else { + h.request("ui.move", json!({"x": 4.0, "y": 4.0}), T) + }; + assert_eq!(r["ok"], true, "{r}"); + h.step(); +} + +/// The ids of the pins drawn this frame with `prefix` (`spotPin:`, `maskPin:`), sorted. +fn pins(h: &Headless, prefix: &str) -> Vec { + let mut v: Vec = h.app.widgets.iter().filter(|(id, _)| id.starts_with(prefix)).map(|(id, _)| id.clone()).collect(); + v.sort(); + v.dedup(); + v +} + +fn spot_pins(h: &Headless) -> Vec { + pins(h, "spotPin:") +} + +fn all_three() -> Vec { + vec!["spotPin:0".to_string(), "spotPin:1".to_string(), "spotPin:2".to_string()] +} + +#[test] +fn spot_pins_follow_each_tool_overlay_mode() { + let mut h = three_spots(); + // Auto (the default): only while the pointer is over the photo + assert_eq!(h.app.ui.tool_overlay, ToolOverlay::Auto); + hover(&mut h, false); + assert_eq!(spot_pins(&h), Vec::::new(), "Auto: hidden with the pointer away"); + assert!(pins(&h, "spotSource:").is_empty()); + hover(&mut h, true); + assert_eq!(spot_pins(&h), all_three(), "Auto: shown with the pointer over the photo"); + assert_eq!(pins(&h, "spotSource:"), vec!["spotSource:2".to_string()], "the selected spot's source"); + // Always: whatever the pointer does + exec(&mut h, "view.toolOverlay", json!({"mode": "always"})); + hover(&mut h, false); + assert_eq!(spot_pins(&h), all_three(), "Always: shown with the pointer away"); + // Selected: the selected spot only + exec(&mut h, "view.toolOverlay", json!({"mode": "selected"})); + hover(&mut h, true); + assert_eq!(spot_pins(&h), vec!["spotPin:2".to_string()]); + exec(&mut h, "spot.select", json!({"index": 0})); + h.step(); + assert_eq!(spot_pins(&h), vec!["spotPin:0".to_string()]); + // Never: none, even over the photo + exec(&mut h, "view.toolOverlay", json!({"mode": "never"})); + hover(&mut h, true); + assert_eq!(spot_pins(&h), Vec::::new()); + assert!(pins(&h, "spotSource:").is_empty()); +} + +/// Hidden pins can't be grabbed: with Never a click on a spot paints a new spot (the tool still +/// works); with Selected, clicking a hidden spot does too, while the selected one can be dragged. +#[test] +fn hidden_spot_pins_are_not_grabbed_and_the_tool_still_works() { + let mut h = three_spots(); + let spots = |h: &Headless| h.app.session.develop_of(h.app.session.active().unwrap()).unwrap_or_default().spots.clone(); + exec(&mut h, "view.toolOverlay", json!({"mode": "never"})); + let r = h.request("ui.pointer", json!({"events": [{"kind": "down", "x": 0.25, "y": 0.3}, {"kind": "up", "x": 0.25, "y": 0.3}]}), T); + assert_eq!(r["ok"], true, "{r}"); + assert_eq!(spots(&h).len(), 4, "Never: a click adds a spot"); + assert_eq!(h.app.session.active_spot, Some(3)); + exec(&mut h, "edit.undo", json!({})); + assert_eq!(spots(&h).len(), 3); + // Selected: the selected spot's pin still drags it + exec(&mut h, "view.toolOverlay", json!({"mode": "selected"})); + exec(&mut h, "spot.select", json!({"index": 1})); + let r = h.request( + "ui.pointer", + json!({"events": [{"kind": "down", "x": 0.5, "y": 0.6}, {"kind": "drag", "x": 0.52, "y": 0.6}, {"kind": "drag", "x": 0.55, "y": 0.6}, {"kind": "up", "x": 0.55, "y": 0.6}]}), + T, + ); + assert_eq!(r["ok"], true, "{r}"); + let moved = spots(&h)[1].points[0]; + assert!((moved.x - 0.55).abs() < 0.01 && (moved.y - 0.6).abs() < 0.01, "{moved:?}"); + assert_eq!(spots(&h).len(), 3); + // …and a click on a hidden spot adds one rather than selecting it + let r = h.request("ui.pointer", json!({"events": [{"kind": "down", "x": 0.75, "y": 0.3}, {"kind": "up", "x": 0.75, "y": 0.3}]}), T); + assert_eq!(r["ok"], true, "{r}"); + assert_eq!(spots(&h).len(), 4); +} + +#[test] +fn shift_h_and_the_command_cycle_the_tool_overlay() { + let mut h = three_spots(); + // Shift+H steps Auto → Always → Selected → Never → Auto + for want in [ToolOverlay::Always, ToolOverlay::Selected, ToolOverlay::Never, ToolOverlay::Auto] { + let r = h.request("ui.key", json!({"key": "h", "shift": true}), T); + assert_eq!(r["ok"], true, "{r}"); + assert_eq!(h.app.ui.tool_overlay, want); + assert_eq!(h.app.ui.right, RightPanel::Remove, "Shift+H leaves the panel alone"); + } + let toast = h.app.ui.toast.as_ref().map(|t| t.0.clone()); + assert_eq!(toast.as_deref(), Some("Tool Overlay: Auto")); + // the pins follow the key: Never hides them over the photo + for _ in 0..3 { + h.request("ui.key", json!({"key": "h", "shift": true}), T); + } + hover(&mut h, true); + assert_eq!(h.app.ui.tool_overlay, ToolOverlay::Never); + assert!(spot_pins(&h).is_empty()); + // the command: no params steps, `mode` sets, a bad mode is refused and changes nothing + assert_eq!(exec(&mut h, "view.toolOverlay", json!({})), json!({"mode": "auto"})); + assert_eq!(exec(&mut h, "view.toolOverlay", json!({"mode": "Selected"})), json!({"mode": "selected"})); + for bad in [json!({"mode": "sometimes"}), json!({"mode": 3})] { + let r = h.request("engine.execute", json!({"command": "view.toolOverlay", "params": bad}), T); + assert_eq!(r["ok"], false, "{r}"); + } + assert_eq!(h.app.ui.tool_overlay, ToolOverlay::Selected); + // plain H still opens Remove (from Edit), and leaves the mode alone + exec(&mut h, "panel.edit", json!({})); + h.request("ui.key", json!({"key": "h"}), T); + assert_eq!(h.app.ui.right, RightPanel::Remove); + assert_eq!(h.app.ui.tool_overlay, ToolOverlay::Selected); + assert_eq!(crate::shortcuts::shortcut_of(&h.app.ui.settings.keymap, "view.toolOverlay"), Some("Shift+H")); +} + +/// View ▸ Tool Overlay lists the modes (the current one checked) and the cycle item with its key; +/// the Remove panel's dropdown sets the mode. +#[test] +fn view_menu_and_remove_panel_choose_the_tool_overlay() { + let mut h = three_spots(); + exec(&mut h, "view.toolOverlay", json!({"mode": "never"})); + let bar = crate::menubar::menu_bar(&h.app); + let view = &bar.iter().find(|(t, _)| t == "View").unwrap().1; + let Some(MenuNode::Submenu { children, .. }) = view.iter().find(|n| matches!(n, MenuNode::Submenu { label, .. } if label == "Tool Overlay")) + else { + panic!("View ▸ Tool Overlay") + }; + let items: Vec<(String, serde_json::Value, Option, Option)> = children + .iter() + .filter_map(|n| match n { + MenuNode::Item { id, params, checked, shortcut, .. } if id == "view.toolOverlay" => { + Some((id.clone(), params.clone(), *checked, shortcut.clone())) + } + _ => None, + }) + .collect(); + assert_eq!(items.len(), 5, "{items:?}"); + let checked: Vec<&serde_json::Value> = items.iter().filter(|i| i.2 == Some(true)).map(|i| &i.1).collect(); + assert_eq!(checked, vec![&json!({"mode": "never"})]); + assert_eq!(items[4].1, serde_json::Value::Null, "the last item cycles"); + assert_eq!(items[4].3.as_deref(), Some("Shift+H")); + // the menu items run the command like the shortcut does + let r = h.request("ui.menu.invoke", json!({"id": "view.toolOverlay", "params": {"mode": "always"}}), T); + assert_eq!(r["ok"], true, "{r}"); + assert_eq!(h.app.ui.tool_overlay, ToolOverlay::Always); + // the Remove panel's dropdown + let r = h.request("ui.clickWidget", json!({"id": "dropdown:toolOverlay"}), T); + assert_eq!(r["ok"], true, "{r}"); + let r = h.request("ui.clickWidget", json!({"id": "toolOverlay:selected"}), T); + assert_eq!(r["ok"], true, "{r}"); + assert_eq!(h.app.ui.tool_overlay, ToolOverlay::Selected); +} + +/// Mask pins follow the same mode; Show Pins (`view.maskPins`) off still hides them all. +#[test] +fn mask_pins_follow_the_tool_overlay_and_show_pins() { + let mut h = detail("panel.masking"); + exec(&mut h, "mask.add", json!({"kind": "radial", "center": [0.3, 0.4], "rx": 0.1, "ry": 0.1})); + exec(&mut h, "mask.add", json!({"kind": "linear", "start": [0.7, 0.2], "end": [0.7, 0.6]})); + assert_eq!(h.app.session.active_mask, Some(2)); + let both = vec!["maskPin:1:0".to_string(), "maskPin:2:0".to_string()]; + hover(&mut h, false); + assert!(pins(&h, "maskPin:").is_empty(), "Auto: hidden with the pointer away"); + hover(&mut h, true); + assert_eq!(pins(&h, "maskPin:"), both); + exec(&mut h, "view.toolOverlay", json!({"mode": "always"})); + hover(&mut h, false); + assert_eq!(pins(&h, "maskPin:"), both); + exec(&mut h, "view.toolOverlay", json!({"mode": "selected"})); + h.step(); + assert_eq!(pins(&h, "maskPin:"), vec!["maskPin:2:0".to_string()]); + exec(&mut h, "view.toolOverlay", json!({"mode": "never"})); + hover(&mut h, true); + assert!(pins(&h, "maskPin:").is_empty()); + // Show Pins off hides them in every mode; view.maskPins answers as before + exec(&mut h, "view.toolOverlay", json!({"mode": "always"})); + assert_eq!(exec(&mut h, "view.maskPins", json!({"show": false})), json!({"maskPins": false})); + h.step(); + assert!(pins(&h, "maskPin:").is_empty()); + assert_eq!(exec(&mut h, "view.maskPins", json!({})), json!({"maskPins": true})); + h.step(); + assert_eq!(pins(&h, "maskPin:"), both); + assert_eq!(h.app.ui.tool_overlay, ToolOverlay::Always, "Show Pins leaves the mode alone"); +} + +/// The mode is saved with the UI state (`ui.json`); settings from before it open in Auto. +#[test] +fn tool_overlay_is_saved_and_old_settings_open_in_auto() { + let ui = crate::UiState { tool_overlay: ToolOverlay::Selected, ..Default::default() }; + let saved = serde_json::to_value(&ui).unwrap(); + assert_eq!(saved["toolOverlay"], "selected"); + let back: crate::UiState = serde_json::from_value(saved.clone()).unwrap(); + assert_eq!(back.sanitized().tool_overlay, ToolOverlay::Selected); + let mut old = saved; + old.as_object_mut().unwrap().remove("toolOverlay"); + let back: crate::UiState = serde_json::from_value(old).unwrap(); + assert_eq!(back.tool_overlay, ToolOverlay::Auto); + // through the control channel too + let mut h = detail("panel.remove"); + let r = h.request("ui.set", json!({"toolOverlay": "never"}), T); + assert_eq!(r["ok"], true, "{r}"); + assert_eq!(h.app.ui.tool_overlay, ToolOverlay::Never); + for m in ToolOverlay::ALL { + assert_eq!(ToolOverlay::parse(m.name()), Some(m)); + assert_eq!(serde_json::to_value(m).unwrap(), m.name()); + } + assert_eq!(ToolOverlay::parse("nope"), None); +} diff --git a/docs/parity-checklist.md b/docs/parity-checklist.md index 76d91ba3e..1d3c9a303 100644 --- a/docs/parity-checklist.md +++ b/docs/parity-checklist.md @@ -1,6 +1,6 @@ # Lightroom parity checklist -> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-11 · **Change:** minor (LR-MERGE-HDR: merges of raws without colour matrices keep their colour (#965); rapid slider entry; LR-PROF-CAMERACOLOR: tone curve from black-and-white camera JPEGs; LR-VIEW-ZOOM, LR-VIEW-COMPARE: Compare and Reference zoom windows; LR-IMP-FORMATS, LR-IMP-CAMERA-COVERAGE: EOS R5 Mark II CR3 decodes; renamed from `parity.md`; Top gaps moved to gaps.md, shortcut notes to ui-parity.md; filesystem history retry/Undo/Redo evidence; renamed from `parity.md`; Top gaps moved to gaps.md, shortcut notes to ui-parity.md) · **Target:** Adobe Lightroom Classic 15.5 / Lightroom 9.6 +> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-13 · **Change:** minor (KEY-HIDEPINS, LR-MASK-PINS, LR-REM-SPOT-EDIT: Tool Overlay modes for spot and mask pins, ⇧H cycles (#224); LR-MERGE-HDR: merges of raws without colour matrices keep their colour (#965); rapid slider entry; LR-PROF-CAMERACOLOR: tone curve from black-and-white camera JPEGs; LR-VIEW-ZOOM, LR-VIEW-COMPARE: Compare and Reference zoom windows; LR-IMP-FORMATS, LR-IMP-CAMERA-COVERAGE: EOS R5 Mark II CR3 decodes; renamed from `parity.md`; Top gaps moved to gaps.md, shortcut notes to ui-parity.md; filesystem history retry/Undo/Redo evidence; renamed from `parity.md`; Top gaps moved to gaps.md, shortcut notes to ui-parity.md) · **Target:** Adobe Lightroom Classic 15.5 / Lightroom 9.6 The measured, row-by-row checklist: one row per Lightroom feature, menu item and shortcut, what we have and what is missing. It answers *does the feature exist?* (feature breadth). Whether it is good @@ -36,7 +36,7 @@ cited here still exists. Formerly docs/parity.md. | H. Crop & rotate (CROP) | 9 | 0 | 0 | 1 | 6/6 (100%) | 3/3 (100%) | | I. Remove / healing (REM) | 8 | 1 | 2 | 2 | 4/4 (100%) | 2/3 (67%) | | J. Red eye (EYE) | 2 | 0 | 0 | 0 | — | 1/1 (100%) | -| K. Masking (MASK) | 13 | 5 | 5 | 0 | 7/8 (88%) | 5/5 (100%) | +| K. Masking (MASK) | 14 | 5 | 5 | 0 | 7/8 (88%) | 6/6 (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%) | @@ -51,11 +51,11 @@ cited here still exists. Formerly docs/parity.md. | W. Cloud & AI infrastructure (CLOUD / AI) | 1 | 0 | 1 | 2 | — | — | | X. Cross-cutting behaviours (BEHAV) | 23 | 6 | 1 | 1 | 8/8 (100%) | 7/10 (70%) | | Y. Menus | 84 | 0 | 4 | 8 | 47/47 (100%) | 23/23 (100%) | -| Z. Keyboard shortcuts (desktop) | 76 | 3 | 3 | 1 | 49/52 (94%) | 23/24 (96%) | +| Z. Keyboard shortcuts (desktop) | 77 | 3 | 2 | 1 | 49/52 (94%) | 23/24 (96%) | | Lightroom Classic extras | 34 | 16 | 35 | 8 | — | 24/26 (92%) | -| **Total** | 408 | 52 | 72 | 35 | 193/201 (96%) | 145/157 (92%) | +| **Total** | 410 | 52 | 71 | 35 | 193/201 (96%) | 146/158 (92%) | -Weighted completion (✅ = 1, 🟡 = ½, 🚫 left out): **81.6%** of 532 in-scope rows — P0 98.0% of 201 · P1 95.2% of 157 · P2 50.0% of 173. +Weighted completion (✅ = 1, 🟡 = ½, 🚫 left out): **81.8%** of 533 in-scope rows — P0 98.0% of 201 · P1 95.3% of 158 · P2 50.6% of 173. ## Top gaps and shortcut notes @@ -266,7 +266,7 @@ differences and missing bindings are in [ui-parity.md](ui-parity.md#shortcuts). | LR-REM-GEN | Generative remove | OOS | 🚫 | | | | LR-REM-DETECT | Object detection for remove | P2 | ⬜ | | | | LR-REM-BRUSH-PARAMS | Brush size / feather / opacity | P0 | ✅ | `cmd:spot.add` (`size`, `feather`, `opacity`), `cmd:brush.smaller`, `cmd:brush.larger`, `cmd:brush.featherLess`, `cmd:brush.featherMore`, `crates/ui-egui/src/panels/right.rs` | Size / Feather / Opacity sliders (for new spots and the selected one); `[` `]` size, ⇧`[` ⇧`]` feather (also the Masking brush) | -| LR-REM-SPOT-EDIT | Edit existing spots | P0 | ✅ | `cmd:spot.select`, `cmd:spot.update`, `cmd:spot.refreshSource`, `cmd:spot.delete` | click a pin to select, drag target or source, ⌫ deletes the selected spot, `/` picks another source; automatic sources are resolved when the spot is added | +| LR-REM-SPOT-EDIT | Edit existing spots | P0 | ✅ | `cmd:spot.select`, `cmd:spot.update`, `cmd:spot.refreshSource`, `cmd:spot.delete`, `cmd:view.toolOverlay` | click a pin to select, drag target or source, ⌫ deletes the selected spot, `/` picks another source; automatic sources are resolved when the spot is added; Tool Overlay (Remove panel, View menu, ⇧H) shows the spots' pins Auto (pointer over the photo, hidden while painting) / Always / Selected / Never, and a hidden spot can't be grabbed (#224) | | LR-REM-VISUALIZE | Visualize spots | P1 | ✅ | `cmd:view.visualizeSpots` | | | LR-REM-PEOPLE | Remove people (generative) | OOS | 🚫 | | | | LR-REM-REFLECT | Remove reflections | P2 | ⬜ | | | @@ -305,7 +305,7 @@ differences and missing bindings are in [ui-parity.md](ui-parity.md#shortcuts). | LR-MASK-SLIDERS | Local adjustment sliders | P0 | ✅ | `cmd:mask.adjust`, `crates/pipeline/src/finish.rs` | every slider renders (CPU + GPU), incl. Noise, Moiré, Defringe (negative Defringe has no effect); no local curve or effect presets | | LR-MASK-OVERLAY | Mask overlay | P0 | ✅ | `cmd:view.maskOverlay`, `cmd:view.maskOverlayMode`, `cmd:view.maskOverlayColor`, `crates/pipeline/src/visualize.rs` | rendered alpha of the selected mask (CPU + GPU): colour, colour on B&W, image on black/white, white on black; ⇧O cycles while masking; successful local adjustments (including Amount) hide the overlay until manually shown again with O / the eye; shape/edge edits retain it; zoomed in, the overlay is rendered on the same sharp window as the plain view (zoom and pan unchanged; `crates/ui-egui/src/tests_preview_limit.rs`, `crates/pipeline/src/tests_window.rs`); no auto-show on hover | | LR-MASK-OVERLAYALL | Show all mask overlays | P1 | ✅ | `cmd:view.maskOverlayAll`, `crates/ui-egui/src/panels/masking.rs`, `crates/pipeline/src/visualize.rs` | one diagnostic tint over every visible mask at once (CPU + GPU): the masks still stack their adjustments as before — this only paints each mask's region together, so you can see all masked areas at a glance (⌘⇧O / Ctrl+Shift+O) | -| LR-MASK-PINS | Pins | P1 | ✅ | `cmd:view.maskPins`, `crates/ui-egui/src/panels/detail.rs` | a pin per radial/linear/brush component: click selects its mask, drag moves it; no pins for range/AI components, no Auto mode | +| LR-MASK-PINS | Pins | P1 | ✅ | `cmd:view.maskPins`, `cmd:view.toolOverlay`, `crates/ui-egui/src/panels/detail.rs`, `crates/ui-egui/src/tests_tool_overlay.rs` | a pin per radial/linear/brush component: click selects its mask, drag moves it; Show Pins on follows the Tool Overlay mode (Auto: while the pointer is over the photo, hidden while brushing; Always; Selected: the selected mask's; Never; ⇧H cycles, #224); no pins for range/AI components | | LR-MASK-UPDATE | Recompute AI masks | P2 | ⬜ | | | | LR-MASK-SYNC | Copy masks to other photos | P1 | ✅ | `cmd:develop.copy` (`groups`) | | | LR-MASK-ADAPTIVE | Adaptive presets | P2 | ⬜ | | | @@ -675,7 +675,7 @@ From `06-shortcuts.md` part 1. Evidence is our binding; conflicts are explained | KEY-ESC | Leave tool / view — Esc | P0 | ✅ | `cmd:view.back`, `crates/ui-egui/src/icons.rs` (`navigation_arrowheads_have_both_halves`), `crates/ui-egui/src/tests_crop_tool.rs` | top-bar navigation arrowheads render both halves at 1× and 2× scale; in the crop tool Esc closes the tool first (the crop stays), the next Esc goes back to the grid; Esc that ends an edit in a text field does nothing else | | KEY-COMMIT | Commit tool — Return | P1 | ✅ | `cmd:tool.done` | Return closes Crop / Remove / Red Eye / Masking back to Edit (edits apply live, as in Lightroom) | | KEY-DELETE-PIN | Delete selected pin — ⌫ | P0 | ✅ | `cmd:mask.delete`, `cmd:spot.delete` | ⌫ deletes the active mask (Masking) or the selected spot (Remove), never the photo while retouching | -| KEY-HIDEPINS | Hide pins — H | P2 | ⬜ | | H = Remove panel | +| KEY-HIDEPINS | Hide pins — H | P2 | ✅ | `cmd:view.toolOverlay`, `crates/ui-egui/src/tests_tool_overlay.rs` | ⇧H cycles the Tool Overlay (Auto → Always → Selected → Never) for spot and mask pins, since H opens Remove; also View ▸ Tool Overlay and the Remove / Masking panels (#224) | ## Lightroom Classic extras @@ -789,6 +789,7 @@ Latin stays upright. Advanced Japanese composition remains open. Regression cove | Date | Change | Summary | |---|---|---| +| 2026-10-13 | minor | KEY-HIDEPINS ⬜ → ✅: Tool Overlay (Auto / Always / Selected / Never, `view.toolOverlay`, ⇧H cycles; H stays Remove) for the Remove tool's spot pins and the mask pins (issue #224); LR-MASK-PINS loses its "no Auto mode" note, LR-REM-SPOT-EDIT notes the modes | | 2026-10-11 | minor | LR-MERGE-HDR: HDR and panorama merges of raws without colour matrices (CR2, NEF, ARW…) write the colour they open with as DNG tags (issue #965); status unchanged | | 2026-10-12 | minor | LR-META-XMP: Read Metadata from File no longer auto-writes the sidecar it has just read, which added `lc:settings` and hid another app's later `crs:` edits (#934); status unchanged (✅) | | 2026-10-12 | patch | LR-IMP-CAMERA-COVERAGE: CR2 white level from the active area, not the saturated masked rows above it (issue #1272); status unchanged (🟡) | diff --git a/docs/ui-parity.md b/docs/ui-parity.md index 2236f02a7..84bea0df0 100644 --- a/docs/ui-parity.md +++ b/docs/ui-parity.md @@ -1,6 +1,6 @@ # UI parity -> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-12 · **Change:** minor (accessibility audit: test over the AccessKit tree; fields, sliders and dialogs named) · **Target:** Adobe Lightroom Classic 15.5 / Lightroom 9.6 +> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-13 · **Change:** minor (Tool Overlay: ⇧H cycles when spot and mask pins show, #224; accessibility audit: test over the AccessKit tree; fields, sliders and dialogs named) · **Target:** Adobe Lightroom Classic 15.5 / Lightroom 9.6 How LightCraft's interaction compares with Lightroom's: views, panels, tools and handles, modifier keys, shortcuts, zoom, context menus and feel. Rows for individual menu items and shortcuts live in @@ -60,6 +60,7 @@ under the UI command that wraps it (a no-op error); the UI command now wins. | Expand/collapse edit sections | ⌘⌥1–5 | ⌘1–6 | ⌘0 = Zoom to Fit, ⌘1 = Zoom 100 %; Geometry lives in the Crop panel | | Histogram | ⌘⇧H | ⌘0 | ⌘0 = Zoom to Fit | | Square Grid | ⇧G | (none; ⇧G = Guided Upright) | Guided Upright is a button in the Crop panel | +| Hide / show pins (Tool Overlay) | ⇧H (cycles Auto, Always, Selected, Never) | H (Classic) | H opens Remove; View ▸ Tool Overlay and the Remove / Masking panels pick a mode | | Export dialog | ⌘⇧E (+ ⇧E) | ⇧E | ⌘⇧E is "Edit in Photoshop" there; no external-editor command yet | | Crop overlay cycle | ⇧O | O | O = mask overlay; ⇧O (mask colour / overlay orientation) unused otherwise | | Create Version | ⌘⇧S (+ ⇧M) | ⇧M (Windows: Ctrl+⇧S) | — | @@ -70,7 +71,6 @@ under the UI command that wraps it (a no-op error); the UI command now wins. **Still missing / broken:** - No command yet: F1 help (verify the rest of the old list: full screen, settings, stacks, visualize spots and merges have commands now). -- `H` opens Remove; Lightroom also uses it (Classic) to hide pins — pins toggle from View → Show Mask Pins. - ⌘M / ⌘H / ⌘Q / ⌘W rely on the platform window defaults (unverified). @@ -105,6 +105,7 @@ left panel, canvas, right panel, which is sensible but not tuned). | Date | Change | Summary | |---|---|---| +| 2026-10-13 | minor | Shortcuts: Tool Overlay modes for spot and mask pins on ⇧H (H stays Remove; issue #224), listed with the deliberate differences; dropped from the missing list | | 2026-10-12 | minor | Accessibility: an AccessKit tree audit test over every view, Develop tool and 13 dialogs; unnamed fields, slider ranges and the Auto Stack slider fixed; 30% → 55%; UI remaining 40–70 h → 34–58 h | | 2026-10-11 | minor | Zoom row: Compare and Reference get zoom windows; what remains of LR-VIEW-ZOOM is overlays and soft proofing | | 2026-10-10 | major | Created: measured command and shortcut counts, assessment by area, estimates; moved the shortcut conflicts section here from the checklist |