diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index d309729f4..d14446e63 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -142,6 +142,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 db50118bf..41971c6d2 100644 --- a/crates/ui-egui/src/menubar.rs +++ b/crates/ui-egui/src/menubar.rs @@ -162,6 +162,7 @@ const LAYOUT: &[(&str, &[&str])] = &[ "view.maskOverlayMode", "view.maskOverlayColor", "view.maskPins", + "@Tool Overlay", "view.cropOverlay", "---", "@Sort", @@ -551,6 +552,18 @@ pub(crate) 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 791ced73b..9b8067570 100644 --- a/crates/ui-egui/src/menus.rs +++ b/crates/ui-egui/src/menus.rs @@ -103,6 +103,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"), @@ -345,6 +349,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}) @@ -968,6 +984,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 76b3bcd9d..508720a6e 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; @@ -1935,6 +1935,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() { @@ -1987,10 +1993,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)); } @@ -2218,6 +2225,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)); @@ -2227,15 +2240,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)); @@ -2252,7 +2278,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 2e8220066..e26646223 100644 --- a/crates/ui-egui/src/panels/masking.rs +++ b/crates/ui-egui/src/panels/masking.rs @@ -503,6 +503,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 1b7fb9079..6a363278a 100644 --- a/crates/ui-egui/src/panels/right.rs +++ b/crates/ui-egui/src/panels/right.rs @@ -485,6 +485,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})); @@ -493,6 +495,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 de1136efb..2a20b11a8 100644 --- a/crates/ui-egui/src/state.rs +++ b/crates/ui-egui/src/state.rs @@ -91,6 +91,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")] @@ -601,6 +665,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, @@ -1179,6 +1246,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 3414e11f7..2ffef503f 100644 --- a/docs/parity-checklist.md +++ b/docs/parity-checklist.md @@ -271,7 +271,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 | ⬜ | | | @@ -311,7 +311,7 @@ differences and missing bindings are in [ui-parity.md](ui-parity.md#shortcuts). | LR-MASK-EFFECTS | Local adjustment (effect / brush) presets | P1 | ✅ | `cmd:mask.effects`, `cmd:mask.applyEffect`, `cmd:mask.saveEffect`, `cmd:mask.updateEffect`, `cmd:mask.renameEffect`, `cmd:mask.duplicateEffect`, `cmd:mask.deleteEffect`, `cmd:mask.favoriteEffect`, `cmd:mask.setEffectGroup`, `cmd:mask.renameEffectGroup`, `cmd:mask.toolEffect`, `cmd:mask.importEffects`, `cmd:mask.exportEffects`, `cmd:dialog.saveMaskEffect`, `cmd:dialog.manageMaskEffects`, `cmd:file.exportMaskEffects`, `crates/engine/src/cmd/mask_effects.rs`, `crates/engine/src/cmd/mask_effect_files.rs`, `crates/ui-egui/src/panels/mask_effects.rs` | Effect menu above the mask sliders: grouped (folders, ★ Favorites first), searchable, scrolls, ↑ ↓ Return, ✓ on the matching effect (else Custom), resting on an effect previews it on the photo; right-click: apply to all masks / to the same-named mask on the selected photos (one undo step), update, rename, duplicate, star, move to group, delete (asks first); Reset Sliders; New Effect Preset dialog: which sliders (All / None / Changed Only), Amount, brush size/feather/flow; any mask type; Manage Effect Presets window (search, inline rename, update, duplicate, stars, groups, export, delete with confirmation); a new mask starts with the effect last applied to a mask of its kind (per tool, kept with the library); imports Lightroom Classic local `.lrtemplate` presets, Camera Raw local-correction `.xmp` presets, `.lceffect` and `.zip` bundles, through File ▸ Import Profiles & Presets… and drag & drop too; folders and LrC divider presets (`10 ----- DETAIL ----- 10`) become groups; settings with no local slider here (`grain`, Point Color `pointColors`, `toningLuminance`) are reported and kept with the effect for export, not applied (🟡 for those); brush size/feather/flow aren't in LrC presets (it keeps them per brush), so only `.lceffect` carries them; no built-in effects | | 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 | ⬜ | | | @@ -682,7 +682,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 @@ -810,6 +810,7 @@ Latin stays upright. Advanced Japanese composition remains open. Regression cove | 2026-10-13 | minor | LR-PROF-CAMERACOLOR: bundled ILCA-99M2 profile with its held-out measurement | | 2026-10-12 | minor | LR-MASK-EFFECTS (new, ✅): local adjustment (effect / brush) presets: Effect menu, New Effect Preset dialog, Manage window, groups and favorites; imports Lightroom Classic local `.lrtemplate`, Camera Raw local `.xmp`, `.lceffect` and `.zip`. LR-IMP-PROFILES and LR-MASK-SLIDERS notes updated | | 2026-10-14 | minor | LR-IMP-CAMERA-COVERAGE: 21 Sony and Leica Adobe models verified on one CC0 file each; status unchanged (🟡) | +| 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 ff2463e63..b6e8b0c08 100644 --- a/docs/ui-parity.md +++ b/docs/ui-parity.md @@ -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). @@ -127,6 +127,7 @@ a real screen reader on any platform yet. | Date | Change | Summary | |---|---|---| | 2026-10-12 | minor | Accessibility: crop handles, spots, gradient handles and mask pins operable by keyboard with AccessKit names and values; Tab order test for the right panels; hand-drawn icons and rows named; 55% → 75%; UI remaining 34–58 h → 32–54 h | +| 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 |