From abf0caf2017e79108fa9a7d67efabad27232b269 Mon Sep 17 00:00:00 2001 From: helgehelge123 <117832631+helgehelge123@users.noreply.github.com> Date: Fri, 16 Oct 2026 23:33:22 +0200 Subject: [PATCH 1/5] KEY-TOGGLEZOOM: hold Space over a tool to pan and click-zoom (#1895) With Crop, Masking, Remove, Red Eye or a picker on the photo, a held Space lends the pointer to the view until the key is up, as Lightroom's temporary Hand / Zoom: a click zooms to the click-zoom ratio at the point clicked (or back to Fit), a drag pans the zoomed photo, and the tool gets neither. A tap (under 0.4 s, no click) still toggles the zoom, on its release. Space's auto-repeat no longer toggles the zoom again. ui.key takes action press|down|up so a script can hold a key over a ui.pointer drag. Co-Authored-By: Claude Opus 5.5 --- crates/ui-egui/src/control.rs | 13 +- crates/ui-egui/src/lib.rs | 22 ++- crates/ui-egui/src/panels/detail.rs | 18 +- crates/ui-egui/src/shortcuts.rs | 9 + crates/ui-egui/src/space_hand.rs | 99 +++++++++++ crates/ui-egui/src/tests_space_hand.rs | 220 +++++++++++++++++++++++++ docs/control-protocol.md | 2 +- docs/parity-checklist.md | 3 +- docs/ui-parity.md | 8 +- 9 files changed, 383 insertions(+), 11 deletions(-) create mode 100644 crates/ui-egui/src/space_hand.rs create mode 100644 crates/ui-egui/src/tests_space_hand.rs diff --git a/crates/ui-egui/src/control.rs b/crates/ui-egui/src/control.rs index 7e04f1356..372c135dc 100644 --- a/crates/ui-egui/src/control.rs +++ b/crates/ui-egui/src/control.rs @@ -291,8 +291,17 @@ pub fn handle(app: &mut LightcraftApp, ctx: &egui::Context, req: &ControlRequest "ui.key" => { let Some(k) = s("key").and_then(key_from) else { return err("unknown or missing `key`") }; let m = modifiers(p); - app.synthetic.push(egui::Event::Key { key: k, physical_key: None, pressed: true, repeat: false, modifiers: m }); - app.synthetic.push(egui::Event::Key { key: k, physical_key: None, pressed: false, repeat: false, modifiers: m }); + // {action?: "press" | "down" | "up"}: down … up holds the key across other input (Space + // held over a drag); a press is both in one frame + let (down, up) = match s("action").unwrap_or("press") { + "press" => (true, true), + "down" => (true, false), + "up" => (false, true), + other => return err(format!("ui.key: action {other:?} (press, down or up)")), + }; + for pressed in [down.then_some(true), up.then_some(false)].into_iter().flatten() { + app.synthetic.push(egui::Event::Key { key: k, physical_key: None, pressed, repeat: false, modifiers: m }); + } ctx.request_repaint(); ok(Value::Null) } diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index 216d6cb61..4ca7b1bdd 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -44,6 +44,7 @@ pub mod shortcuts; pub mod sidebar; pub mod slideshow_export; pub mod softpaint; +pub mod space_hand; pub mod state; pub mod sync; pub mod tasks; @@ -192,6 +193,8 @@ mod tests_slideshow; #[cfg(test)] mod tests_solo_sections; #[cfg(test)] +mod tests_space_hand; +#[cfg(test)] mod tests_switch_library; #[cfg(test)] mod tests_sync; @@ -440,6 +443,8 @@ pub struct LightcraftApp { pub widgets: Vec<(String, egui::Rect)>, /// In-progress on-canvas gesture (brush stroke points, gradient drag…). pub gesture: Option, + /// Space is held over a tool on the photo: the view's hand has the pointer ([`space_hand`]). + pub space_hand: Option, /// The brush stroke being painted, as drawn on the photo until it's committed (issue #517). pub brush_live: Option, /// What the loupe drew last frame: photo and source ("render", "cached", "embedded", "small", @@ -569,6 +574,7 @@ impl LightcraftApp { film_scroll: None, widgets: vec![], gesture: None, + space_hand: None, brush_live: None, loupe_shown: None, region_view: None, @@ -1451,9 +1457,19 @@ impl LightcraftApp { if self.synthetic.is_empty() { return; } - let n = match self.synthetic[0] { - egui::Event::PointerMoved(_) | egui::Event::PointerButton { .. } | egui::Event::MouseWheel { .. } => 1, - _ => self.synthetic.iter().position(|e| matches!(e, egui::Event::Key { pressed: false, .. })).map_or(self.synthetic.len(), |i| i + 1), + let pointer = + |e: &egui::Event| matches!(e, egui::Event::PointerMoved(_) | egui::Event::PointerButton { .. } | egui::Event::MouseWheel { .. }); + let n = if pointer(&self.synthetic[0]) { + 1 + } else { + // a key sequence up to its release; a key held down (no release yet) stops before the + // pointer events that follow, which then still come one per frame + let release = self.synthetic.iter().position(|e| matches!(e, egui::Event::Key { pressed: false, .. })).map(|i| i + 1); + let next_pointer = self.synthetic.iter().position(pointer); + match (release, next_pointer) { + (Some(r), Some(q)) => r.min(q), + (r, q) => r.or(q).unwrap_or(self.synthetic.len()), + } }; if let Some(egui::Event::PointerMoved(p) | egui::Event::PointerButton { pos: p, .. }) = self.synthetic.first() { raw.events.push(egui::Event::PointerMoved(*p)); diff --git a/crates/ui-egui/src/panels/detail.rs b/crates/ui-egui/src/panels/detail.rs index e655e4969..0315de8ad 100644 --- a/crates/ui-egui/src/panels/detail.rs +++ b/crates/ui-egui/src/panels/detail.rs @@ -941,6 +941,8 @@ pub fn show(app: &mut LightcraftApp, ui: &mut egui::Ui) { } } } + // Space held over a tool: the view's hand takes the pointer from it (#1895) + crate::space_hand::interact(app, ui, canvas, (main_area, img_rect), split.then(|| (areas[0], before_img))); // drawn and hit-tested above the loupe and its tools: clicks on it pan let nav = navigator(app, ui, canvas, img_rect, id); // what is on screen isn't the render on its way (a stand-in, the last render after an edit, @@ -1644,8 +1646,19 @@ fn general_interaction( } return; } - // click toggles Fit ↔ the chosen click-zoom ratio at the clicked point; drag pans when zoomed - let zoomed = img.width() > canvas.width() + 1.0 || img.height() > canvas.height() + 1.0; + click_zoom_and_pan(app, resp, img, canvas); + let _ = (native, aspect); +} + +/// The photo `img` reaches past its pane `canvas`: a drag pans it. +pub(crate) fn zoomed(img: Rect, canvas: Rect) -> bool { + img.width() > canvas.width() + 1.0 || img.height() > canvas.height() + 1.0 +} + +/// The loupe's own click and drag (also Space's hand over a tool, `space_hand`): a click toggles +/// Fit ↔ the chosen click-zoom ratio at the clicked point, a drag pans when zoomed. +pub(crate) fn click_zoom_and_pan(app: &mut LightcraftApp, resp: &egui::Response, img: Rect, canvas: Rect) { + let zoomed = zoomed(img, canvas); if resp.double_clicked() || (resp.clicked() && !zoomed) { if let Some(q) = resp.interact_pointer_pos() { let u = ((q.x - img.left()) / img.width()).clamp(0.0, 1.0); @@ -1661,7 +1674,6 @@ fn general_interaction( if zoomed && resp.dragged() { pan_image(app, canvas, img, resp.drag_delta()); } - let _ = (native, aspect); } // ------------------------------------------------------------------------ crop diff --git a/crates/ui-egui/src/shortcuts.rs b/crates/ui-egui/src/shortcuts.rs index 032548f6a..3dbf680f6 100644 --- a/crates/ui-egui/src/shortcuts.rs +++ b/crates/ui-egui/src/shortcuts.rs @@ -350,6 +350,8 @@ fn keyboard_owned(app: &LightcraftApp, ctx: &egui::Context) -> bool { } pub fn handle(app: &mut LightcraftApp, ctx: &egui::Context) { + // Space held over a tool ends when the key is up, whoever has the keyboard by then + crate::space_hand::tick(app, ctx); if !matches!(app.ui.dialog, Some(crate::state::Dialog::Shortcuts)) { app.recording_shortcut = None; } @@ -402,9 +404,12 @@ pub fn handle(app: &mut LightcraftApp, ctx: &egui::Context) { fire.push(id.to_string()); } } + // a held Space repeats: it toggles the zoom (or takes the hand) once per press + let space_repeat = !i.events.iter().any(|e| matches!(e, egui::Event::Key { key: Key::Space, pressed: true, repeat: false, .. })); for (sc, id, params) in ALIASES { if let Some((m, k)) = parse(sc).filter(|_| !native(sc)) && !(k == Key::Escape && popup_open) + && !(k == Key::Space && space_repeat) && !taken.contains(&(m, k)) && matches(i, m, k) { @@ -459,6 +464,10 @@ pub fn handle(app: &mut LightcraftApp, ctx: &egui::Context) { } continue; } + // over a tool on the photo, held Space is the view's hand; a tap toggles on its release + if id == "view.zoomToggle" && crate::space_hand::begin(app, ctx) { + continue; + } // flag/rate aliases (Shift+X…) go through the culling path: active photo in Compare/Survey, // `advance` moves to the next candidate there if matches!(id, "photo.flag" | "photo.rate" | "photo.label") { diff --git a/crates/ui-egui/src/space_hand.rs b/crates/ui-egui/src/space_hand.rs new file mode 100644 index 000000000..bd905e183 --- /dev/null +++ b/crates/ui-egui/src/space_hand.rs @@ -0,0 +1,99 @@ +//! Space held over a tool on the photo: the view's hand, as in Lightroom (issue #1895). +//! +//! Crop, Masking, Remove, Red Eye and the pickers (white balance, colour range, point colour, a +//! targeted adjustment) take the primary click and drag on the photo, so the loupe's own click +//! zoom and drag pan are out of reach while one is open. Holding Space lends them back until the +//! key goes up: a click zooms (Fit ↔ the click-zoom ratio, at the point clicked), a drag pans the +//! zoomed photo, and the tool sees neither. A quick tap with no click still toggles the zoom, on +//! its release. Without a tool Space toggles the zoom on the press, as before. A held key's +//! auto-repeat toggles neither (`shortcuts::handle` lets only a fresh press through). + +use egui::{CursorIcon, Key, Rect, Sense}; +use serde_json::json; + +use crate::LightcraftApp; +use crate::state::{RightPanel, ViewMode}; + +/// The longest Space press (s) that still toggles the zoom when it lets go without a click. +pub(crate) const TAP: f64 = 0.4; + +/// Space is held over a tool on the photo. +#[derive(Clone, Copy, Debug, PartialEq)] +pub struct SpaceHand { + /// When the key went down (egui input time, s). + since: f64, + /// A pointer button went down while it was held: letting go is not a tap. + used: bool, +} + +/// A tool on the photo takes the primary click and drag, so Space has to lend them to the view. +pub(crate) fn tool_owns_pointer(app: &LightcraftApp) -> bool { + // the full-screen preview shows no tool overlays, but a picker still takes its click there + let panel = !app.ui.fullscreen && matches!(app.ui.right, RightPanel::Crop | RightPanel::Masking | RightPanel::Remove | RightPanel::RedEye); + let picker = matches!(app.ui.tool.as_str(), "wbPicker" | "colorRange" | "pointColor") || app.ui.tool.starts_with("tat:"); + app.ui.view == ViewMode::Detail && app.ui.slideshow.is_none() && app.session.active().is_some() && (panel || picker) +} + +/// Space went down (a fresh press, not a repeat). Over a tool the hand takes over until the key +/// is up and the press does nothing else (true); otherwise false leaves it to Toggle Zoom. +pub(crate) fn begin(app: &mut LightcraftApp, ctx: &egui::Context) -> bool { + if !tool_owns_pointer(app) { + return false; + } + // in the middle of a stroke or a drag the press neither zooms nor lends the pointer + let (now, busy) = ctx.input(|i| (i.time, i.pointer.any_down())); + if !busy && app.space_hand.is_none() { + app.space_hand = Some(SpaceHand { since: now, used: false }); + } + true +} + +/// Runs every frame before the shortcuts: ends the hold once Space is up and a drag it started has +/// ended, or at once when the tool closes or a dialog opens. A quick tap with no click toggles the +/// zoom as it ends. +pub(crate) fn tick(app: &mut LightcraftApp, ctx: &egui::Context) { + let Some(mut hand) = app.space_hand else { return }; + let (held, pointer, now) = ctx.input(|i| (i.key_down(Key::Space), i.pointer.any_down(), i.time)); + hand.used |= pointer; + let tool = tool_owns_pointer(app) && app.ui.dialog.is_none(); + // held, or let go in the middle of a pan: the hand stays until the button is up + if tool && (held || pointer) { + app.space_hand = Some(hand); + return; + } + app.space_hand = None; + if tool && !hand.used && now - hand.since <= TAP { + let _ = app.run("view.zoomToggle", json!({})); + } +} + +/// The hand over the photo while Space is held. Registered above the loupe and the tool overlays +/// (the navigator stays above it), so it takes the click or the drag and they get neither: they +/// still draw, without the pointer. `main` is the pane the photo `img` is laid out in; `before` +/// the Before pane of a side-by-side or stacked view with its photo, where a press there acts. +pub(crate) fn interact(app: &mut LightcraftApp, ui: &mut egui::Ui, canvas: Rect, main: (Rect, Rect), before: Option<(Rect, Rect)>) { + if app.space_hand.is_none() { + return; + } + let resp = ui.interact(canvas, egui::Id::new("loupe-hand"), Sense::click_and_drag()); + crate::widgets::register(ui.ctx(), "loupe:hand", canvas); + let at = resp.interact_pointer_pos().or(resp.hover_pos()); + let (pane, img) = match (before, at) { + (Some((pane, img)), Some(q)) if pane.contains(q) => (pane, img), + _ => main, + }; + if resp.is_pointer_button_down_on() + && let Some(hand) = app.space_hand.as_mut() + { + hand.used = true; + } + let zoomed = crate::panels::detail::zoomed(img, pane); + crate::panels::detail::click_zoom_and_pan(app, &resp, img, pane); + if resp.hovered() || resp.dragged() { + ui.ctx().set_cursor_icon(match (zoomed, resp.dragged()) { + (true, true) => CursorIcon::Grabbing, + (true, false) => CursorIcon::Grab, + (false, _) => CursorIcon::ZoomIn, + }); + } +} diff --git a/crates/ui-egui/src/tests_space_hand.rs b/crates/ui-egui/src/tests_space_hand.rs new file mode 100644 index 000000000..8bf3ba992 --- /dev/null +++ b/crates/ui-egui/src/tests_space_hand.rs @@ -0,0 +1,220 @@ +//! Headless tests of Space held over a tool on the photo (issue #1895): the view's hand pans and +//! click-zooms while the tool (Remove, a mask brush, Crop) gets neither, a quick tap still toggles +//! the zoom, and a held key's auto-repeat never toggles it again. + +use std::time::Duration; + +use serde_json::json; + +use crate::headless::Headless; +use crate::state::{RightPanel, Zoom}; +use crate::{LightcraftApp, Services}; + +const T: Duration = Duration::from_secs(20); + +fn detail(panel: Option<&str>) -> Headless { + let services = Services { png: None, ..Default::default() }; + let mut app = LightcraftApp::new(lightcraft_engine::Session::with_demo(), services); + app.ui.settings.gpu = false; + // Space and click zoom go straight to the ratio, not to the camera's focus point + app.ui.settings.zoom_to_focus = false; + let mut h = Headless::new(app, [1200.0, 800.0], 1.0); + let r = h.request("ui.set", json!({"view": "detail", "right": "edit"}), T); + assert_eq!(r["ok"], true, "{r}"); + if let Some(panel) = panel { + let r = h.request("engine.execute", json!({"command": panel}), T); + assert_eq!(r["ok"], true, "{r}"); + } + h.step(); + h +} + +fn space(h: &mut Headless, action: &str) { + let r = h.request("ui.key", json!({"key": "Space", "action": action}), T); + assert_eq!(r["ok"], true, "{r}"); + h.step(); +} + +fn pointer(h: &mut Headless, events: serde_json::Value) { + let r = h.request("ui.pointer", json!({"events": events}), T); + assert_eq!(r["ok"], true, "{r}"); + h.step(); +} + +/// A point `dx`, `dy` (screen points) from the middle of the canvas, in the normalized image +/// coordinates `ui.pointer` takes: zoomed in, the photo's own middle can be off screen. +fn at(h: &Headless, dx: f32, dy: f32) -> (f32, f32) { + let (img, canvas) = (h.app.image_rect.unwrap(), h.app.canvas_rect.unwrap()); + let q = canvas.center() + egui::vec2(dx, dy); + ((q.x - img.left()) / img.width(), (q.y - img.top()) / img.height()) +} + +/// A drag of 120 × 40 points across the middle of the canvas. +fn drag(h: &mut Headless) { + let p: Vec<(f32, f32)> = [(0.0, 0.0), (20.0, 5.0), (60.0, 20.0), (120.0, 40.0)].iter().map(|&(x, y)| at(h, x, y)).collect(); + pointer( + h, + json!([ + {"kind": "down", "x": p[0].0, "y": p[0].1}, + {"kind": "drag", "x": p[1].0, "y": p[1].1}, + {"kind": "drag", "x": p[2].0, "y": p[2].1}, + {"kind": "drag", "x": p[3].0, "y": p[3].1}, + {"kind": "up", "x": p[3].0, "y": p[3].1} + ]), + ); +} + +fn click(h: &mut Headless, (x, y): (f32, f32)) { + pointer(h, json!([{"kind": "down", "x": x, "y": y}, {"kind": "up", "x": x, "y": y}])); +} + +fn develop(h: &Headless) -> lightcraft_develop::DevelopSettings { + let id = h.app.session.active().expect("active photo"); + (*h.app.session.develop_of(id).unwrap_or_default()).clone() +} + +fn zoom_in(h: &mut Headless) { + h.app.run("view.navigate", json!({"zoom": {"percent": 200.0}, "pan": [0.5, 0.5]})).unwrap(); + h.step(); + h.step(); + let (img, canvas) = (h.app.image_rect.unwrap(), h.app.canvas_rect.unwrap()); + assert!(img.width() > canvas.width(), "not zoomed: {img:?} in {canvas:?}"); +} + +fn hand_shown(h: &Headless) -> bool { + h.app.widgets.iter().any(|(id, _)| id == "loupe:hand") +} + +#[test] +fn a_held_space_toggles_the_zoom_once_however_long_it_repeats() { + // no tool: the press toggles at once, as before; the key's auto-repeat used to flip it back + // and forth for as long as it was held + let mut h = detail(None); + assert_eq!(h.app.ui.zoom, Zoom::Fit); + space(&mut h, "down"); + assert_eq!(h.app.ui.zoom, Zoom::Percent(100.0)); + assert!(h.app.space_hand.is_none(), "no tool: no hand"); + // further presses of a key that is down are the system's auto-repeat + for _ in 0..3 { + space(&mut h, "down"); + } + assert_eq!(h.app.ui.zoom, Zoom::Percent(100.0), "a repeat toggled the zoom"); + space(&mut h, "up"); + assert_eq!(h.app.ui.zoom, Zoom::Percent(100.0), "the release toggled the zoom"); + // the next press is a new one + space(&mut h, "press"); + assert_eq!(h.app.ui.zoom, Zoom::Fit); +} + +#[test] +fn space_held_over_the_healing_brush_pans_and_paints_no_spot() { + let mut h = detail(Some("panel.remove")); + assert_eq!(h.app.ui.right, RightPanel::Remove); + zoom_in(&mut h); + let spots = develop(&h).spots.len(); + let pan = h.app.ui.pan; + space(&mut h, "down"); + assert!(h.app.space_hand.is_some()); + assert_eq!(h.app.ui.zoom, Zoom::Percent(200.0), "the press zoomed"); + pointer(&mut h, json!([{"kind": "move", "x": 0.5, "y": 0.5}])); + assert!(hand_shown(&h)); + assert_eq!(h.last_cursor, egui::CursorIcon::Grab); + drag(&mut h); + assert_eq!(develop(&h).spots.len(), spots, "the drag painted a spot"); + assert_ne!(h.app.ui.pan, pan, "the drag did not pan"); + assert_eq!(h.app.ui.zoom, Zoom::Percent(200.0)); + // letting go after a drag is not a tap: the zoom stays + space(&mut h, "up"); + assert!(h.app.space_hand.is_none()); + assert!(!hand_shown(&h)); + assert_eq!(h.app.ui.zoom, Zoom::Percent(200.0), "the release toggled the zoom"); + // the tool has its pointer back: the same drag paints a spot and leaves the view alone + let pan = h.app.ui.pan; + drag(&mut h); + assert_eq!(develop(&h).spots.len(), spots + 1, "the brush did not paint after the release"); + assert_eq!(h.app.ui.pan, pan); +} + +#[test] +fn space_held_click_zooms_to_the_point_clicked_and_back() { + let mut h = detail(Some("panel.remove")); + let spots = develop(&h).spots.len(); + space(&mut h, "down"); + pointer(&mut h, json!([{"kind": "move", "x": 0.25, "y": 0.3}])); + assert_eq!(h.last_cursor, egui::CursorIcon::ZoomIn, "at Fit the hand zooms in"); + click(&mut h, (0.25, 0.3)); + assert_eq!(h.app.ui.zoom, Zoom::Percent(h.app.ui.click_zoom as f32)); + let (u, v) = h.app.ui.pan; + assert!((u - 0.25).abs() < 0.02 && (v - 0.3).abs() < 0.02, "zoomed to {:?}, not the point clicked", h.app.ui.pan); + // zoomed in, a click goes back to Fit + for _ in 0..30 { + h.step(); + } + let middle = at(&h, 0.0, 0.0); + click(&mut h, middle); + assert_eq!(h.app.ui.zoom, Zoom::Fit); + space(&mut h, "up"); + assert_eq!(h.app.ui.zoom, Zoom::Fit, "the release toggled the zoom"); + assert_eq!(develop(&h).spots.len(), spots, "a click painted a spot"); +} + +#[test] +fn a_tap_over_a_tool_toggles_on_release_and_a_long_hold_does_not() { + let mut h = detail(Some("panel.remove")); + let spots = develop(&h).spots.len(); + space(&mut h, "press"); + for _ in 0..3 { + h.step(); + } + assert_eq!(h.app.ui.zoom, Zoom::Percent(100.0), "a tap did not toggle the zoom"); + assert!(h.app.space_hand.is_none()); + space(&mut h, "down"); + space(&mut h, "up"); + assert_eq!(h.app.ui.zoom, Zoom::Fit, "a quick press and release did not toggle back"); + // held a while with no click, then let go: changed one's mind, nothing happens + space(&mut h, "down"); + let steps = (crate::space_hand::TAP * 60.0).ceil() as usize + 10; + for _ in 0..steps { + h.step(); + } + // the key's auto-repeat in the meantime keeps the hand + space(&mut h, "down"); + assert!(h.app.space_hand.is_some(), "a repeat ended the hand"); + space(&mut h, "up"); + assert_eq!(h.app.ui.zoom, Zoom::Fit, "a long hold toggled the zoom"); + assert_eq!(develop(&h).spots.len(), spots); +} + +#[test] +fn space_held_over_a_mask_brush_paints_nothing() { + let mut h = detail(Some("panel.masking")); + let r = h.request("ui.clickWidget", json!({"id": "maskNew:brush"}), T); + assert_eq!(r["ok"], true, "{r}"); + let r = h.request("ui.set", json!({"brushSize": 0.15, "brushFeather": 0.0, "brushFlow": 100.0}), T); + assert_eq!(r["ok"], true, "{r}"); + zoom_in(&mut h); + let before = develop(&h); + let pan = h.app.ui.pan; + space(&mut h, "down"); + drag(&mut h); + space(&mut h, "up"); + assert_eq!(develop(&h), before, "the drag painted the mask"); + assert_ne!(h.app.ui.pan, pan, "the drag did not pan"); + assert!(h.app.brush_live.is_none()); +} + +#[test] +fn space_held_over_the_crop_tool_pans_and_leaves_the_crop() { + let mut h = detail(Some("panel.crop")); + assert_eq!(h.app.ui.right, RightPanel::Crop); + let r = h.request("engine.execute", json!({"command": "crop.set", "params": {"rect": [0.1, 0.1, 0.8, 0.9]}}), T); + assert_eq!(r["ok"], true, "{r}"); + zoom_in(&mut h); + let before = develop(&h); + let pan = h.app.ui.pan; + space(&mut h, "down"); + drag(&mut h); + space(&mut h, "up"); + assert_eq!(develop(&h), before, "the drag moved the crop"); + assert_ne!(h.app.ui.pan, pan, "the drag did not pan"); +} diff --git a/docs/control-protocol.md b/docs/control-protocol.md index 7a8033aa5..8acbbe8a5 100644 --- a/docs/control-protocol.md +++ b/docs/control-protocol.md @@ -82,7 +82,7 @@ filmstrip is visible. | `ui.clickWidget` / `ui.dragWidget` / `ui.hoverWidget` | `{id, count?, button?, fx?, fy?}` (`button: "right"` right-clicks) / `{id, toX?, toY?, dx?, dy?, steps?}` / `{id, fx?, fy?}` | Real egui input on a widget (hover: the pointer rests on it, e.g. for preset/profile previews) | | `ui.move` / `ui.click` / `ui.drag` | `{x, y, count?, button?}` / `{x, y, toX, toY, steps?}` | Raw pointer input, screen points | | `ui.pointer` | `{events: [{kind: down\|drag\|up, x, y}], alt?, shift?, cmd?}` | Gesture in normalized image coordinates (Detail view) | -| `ui.key` | `{key, cmd?, shift?, alt?, ctrl?}` | Key press | +| `ui.key` | `{key, action?: press\|down\|up, cmd?, shift?, alt?, ctrl?}` | Key press (default), or the key going `down` and staying held until an `up` (Space held over a `ui.pointer` drag) | | `ui.text` | `{text}` | Text input | | `ui.clipboard` | `{action: cut\|copy\|paste, text?}` | Cut, copy or paste in the focused text field, as the system does for ⌘X / ⌘C / ⌘V; `paste` pastes `text`, or the system clipboard without it. Refused when no text field has the focus (outside one, ⌘C / ⌘V are the commands `develop.copy` / `develop.paste`). In the desktop app cut and copy write the system clipboard. `ui.inspect` → `copied` is the text the UI last put on the clipboard | | `ui.scroll` | `{dx, dy, cmd?, ctrl?, shift?, alt?}` | Wheel / two-finger scroll at the current pointer; pans over the image, modifier-scroll zooms | diff --git a/docs/parity-checklist.md b/docs/parity-checklist.md index 7fa952efb..17d1143cc 100644 --- a/docs/parity-checklist.md +++ b/docs/parity-checklist.md @@ -629,7 +629,7 @@ From `06-shortcuts.md` part 1. Evidence is our binding; conflicts are explained | KEY-WB | White-balance selector — W | P0 | ✅ | `cmd:tool.wbPicker` | | | KEY-FILMSTRIP | Filmstrip — / | P0 | ✅ | `cmd:view.filmstrip` | | | KEY-SHOWORIGINAL | Show original — \ | P0 | ✅ | `cmd:view.showOriginal` | | -| KEY-TOGGLEZOOM | Toggle zoom — Space | P0 | ✅ | `cmd:view.zoomToggle` | Space is a secondary binding (primary Z) | +| KEY-TOGGLEZOOM | Toggle zoom — Space | P0 | ✅ | `cmd:view.zoomToggle`, `crates/ui-egui/src/space_hand.rs`, `crates/ui-egui/src/tests_space_hand.rs` | Space is a secondary binding (primary Z); held, it toggles once (the key's auto-repeat no longer flips the zoom back and forth). With a tool on the photo (Crop, Masking, Remove, Red Eye, the white-balance / colour-range / point-colour pickers, a targeted adjustment) held Space is the view's hand until the key is up, as in Lightroom: a click zooms to the click-zoom ratio at the point clicked (or back to Fit), a drag pans the zoomed photo (grab cursor; zoom-in cursor at Fit), and the tool gets neither; a tap (under 0.4 s, no click) toggles the zoom on its release (issue #1895) | | KEY-MASKCOLOR | Cycle mask colour — ⇧O | P1 | ✅ | `cmd:view.maskOverlayColor`, `crates/ui-egui/src/shortcuts.rs` | ⇧O while masking cycles the overlay colour (while cropping: the guides' orientation); the overlay mode is in View ▸ Cycle Mask Overlay Mode | | KEY-EXPORTPREV | Export with previous — ⌘E | P0 | ✅ | `cmd:app.exportPrevious` | ⌘E (alias) and ⌥⇧⌘E (Classic) | | KEY-EXPORTDIALOG | Export dialog — ⇧E | P0 | ✅ | `cmd:dialog.export` | secondary binding (primary ⌘⇧E) | @@ -841,6 +841,7 @@ Latin stays upright. Advanced Japanese composition remains open. Regression cove | 2026-10-13 | minor | LRC-LIB-KEYWORDS: the Keyword List marks a keyword that has an applied keyword inside it (a dot in its box); status unchanged | | 2026-10-12 | minor | LR-IMP-FORMATS: folder discovery includes SR2/SRF; MCP and CLI reuse the engine extension list. DSC-F828 remains preview-only; decoder support and row status unchanged | | 2026-10-15 | patch | KEYC-RATING: `\` shows / hides the Filter Bar in Photo Grid and Square Grid (Show Original in Detail); status unchanged (🟡) | +| 2026-10-16 | minor | KEY-TOGGLEZOOM: Space held over a tool on the photo is the view's hand (click zooms, drag pans, the tool gets neither), a tap still toggles; a held Space toggles once (issue #1895); status unchanged | | 2026-10-12 | minor | LR-EDIT-OPTICS-PROFILE: Sony ARW shading, CA and distortion tables decoded with settings and provenance (raw IFD, `SR2SubIFD` fallback); applied corrections unchanged (ILCE-7RM4A and ILCE-7RM6 distortion); status unchanged (🟡) | | 2026-10-14 | minor | LR-IMP-CAMERA-COVERAGE: Camera-coverage sweep (#1327): 30 models verified on 40 CC0 files (coverage.py 515 → 545 verified); sensor verification does not establish colour parity (#1277); status unchanged (🟡) | | 2026-10-13 | minor | LR-MERGE-PANO: a panorama's DNG takes its `AsShotNeutral` from the median of its frames' vendor white-balance multipliers (CR2, NEF, ARW…) instead of the first frame's tags, and frames now carry the multipliers (issue #1144); status unchanged (✅) | diff --git a/docs/ui-parity.md b/docs/ui-parity.md index 16665f3d4..ab5b724f0 100644 --- a/docs/ui-parity.md +++ b/docs/ui-parity.md @@ -27,7 +27,7 @@ assessment and the notes that don't fit a row. Visual reference for agents: `pla | Crop, Upright, transform handles | handles, aspect lock, angle, overlays (O / ⇧O), guided Upright | ✅ all of them; overlay cycle on ⇧O | 85% | crop opacity of cut-out area (15.5) | | Masking tools | brush (A / B, flow, density, auto mask, pressure), linear / radial, range masks, AI masks, pins | brush, linear, radial, colour / luminance range, add / subtract / intersect / invert, masks panel, pins | 60% | brush live preview (#517 / #518), no pen pressure, no depth range, AI masks, Feather / Edge controls (15.5) | | Remove / heal | content-aware remove, heal, clone, visualize spots | heal, clone, auto source, visualize spots, red / pet eye | 65% | content-aware (patch synthesis) remove, generative remove, detect distractions | -| Zoom and navigation | Fit / Fill / 1:1 / 2:1 … 11:1, Navigator, spacebar pan | Fit to 800%, pinch zoom, pan, Navigator-like; every view, mask and diagnostic overlays (Visualize Spots, Point Color range, HDR range) and soft proofing included, is sharp at 1:1–8:1 | 60% | zoom stops at 800 % (Adobe's reference goes to 11:1) | +| Zoom and navigation | Fit / Fill / 1:1 / 2:1 … 11:1, Navigator, spacebar pan | Fit to 800%, pinch zoom, pan, Navigator-like, Space-held hand over tools; every view, mask and diagnostic overlays (Visualize Spots, Point Color range, HDR range) and soft proofing included, is sharp at 1:1–8:1 | 60% | zoom stops at 800 % (Adobe's reference goes to 11:1) | | Library culling and metadata | ratings, flags, labels with keys, painter, keywording, metadata panel, filter bar | ratings / flags / labels and advance, keyword list and sets, keywording box, metadata fields, smart albums, filter bar | 85% | painter tool, quick collection, some Classic keys | | Shortcuts | Lightroom desktop and Classic keymaps | Lightroom-desktop keymap with Classic additions, editable | 85% | deliberate differences below; no Classic keymap layer | | Context menus | everywhere | 31 sites (grid, filmstrip, albums, folders, keywords, masks, presets, text fields…), all through `widgets::context_menu` (`crates/ui-egui/src/widgets.rs`): a right-click elsewhere while a menu is open closes it and opens the menu there, also on the same widget (issue #1877: that one flashed at the new spot and closed, as egui counts the opening click as a click while open) | 70% | unaudited against Lightroom's menus item by item | @@ -51,6 +51,11 @@ Export dialog, Space Toggle zoom, ⇧M Create Version, ⇧X Reject + advance, Before/After Split and the History panel; History lost the binding. `W` and `⇧⌘I` also fired the engine command under the UI command that wraps it (a no-op error); the UI command now wins. +**Fixed (#1895):** Space held over a tool on the photo (Crop, Masking, Remove, Red Eye, a picker) is the view's hand +until the key is up, as Lightroom's temporary Hand / Zoom: a click zooms at the point clicked (or back to Fit), a drag +pans, the tool gets neither; a tap still toggles the zoom (on its release). A held Space toggles once; its +auto-repeat used to flip the zoom back and forth (`crates/ui-egui/src/space_hand.rs`). + **Deliberate differences (our key → Lightroom desktop key)** — each is a conflict with another binding we have: | Action | Ours | Lightroom desktop | Why | @@ -151,6 +156,7 @@ establish OS keyboard delivery or cross-platform acceptance. | 2026-10-15 | patch | Context menus: a right-click elsewhere on the same widget while its menu is open moves the menu to the pointer; before it flashed at the new spot and closed (issue #1877); every site uses `widgets::context_menu`; 70% unchanged | | 2026-10-23 | patch | KEYC-RATING: ⌘L / Ctrl+L (Library ▸ Enable Filters) switches the filter off and on, restoring it as it was | | 2026-10-15 | patch | KEYC-RATING: `\` shows / hides the Filter Bar in Photo Grid and Square Grid (Show Original in Detail); ⌘L is the filter-bar key left | +| 2026-10-24 | minor | Shortcuts: Space held over a tool on the photo pans and click-zooms (Lightroom's temporary hand), a tap still toggles, auto-repeat no longer toggles (issue #1895) | | 2026-10-14 | minor | Zoom row: diagnostic overlays render zoom windows too | | 2026-10-14 | patch | Shortcuts: Visualize HDR loses its default ⇧H, which cycles the Tool Overlay (issue #224); both PRs had claimed it | | 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 | From 9e305d642e3e091235e8a651079f6dafd798c224 Mon Sep 17 00:00:00 2001 From: helgehelge123 <117832631+helgehelge123@users.noreply.github.com> Date: Sun, 18 Oct 2026 09:39:32 +0200 Subject: [PATCH 2/5] KEY-TOGGLEZOOM: a click down and up within one frame keeps its zoom on Space's release A real mouse can deliver the press and the release in the same egui frame: no button is down then, so the hand was not marked used and a quick release of Space ran the tap fallback, zooming straight back. tick() now also counts a press seen this frame. New headless test sends both button events in one frame (the synthetic pointer queue sends one per frame, which is why the existing tests missed it). Reported by AlexChambers in review. Co-Authored-By: Claude Opus 5.5 --- crates/ui-egui/src/space_hand.rs | 5 +++-- crates/ui-egui/src/tests_space_hand.rs | 20 ++++++++++++++++++++ 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/crates/ui-egui/src/space_hand.rs b/crates/ui-egui/src/space_hand.rs index bd905e183..a02f2ac84 100644 --- a/crates/ui-egui/src/space_hand.rs +++ b/crates/ui-egui/src/space_hand.rs @@ -53,8 +53,9 @@ pub(crate) fn begin(app: &mut LightcraftApp, ctx: &egui::Context) -> bool { /// zoom as it ends. pub(crate) fn tick(app: &mut LightcraftApp, ctx: &egui::Context) { let Some(mut hand) = app.space_hand else { return }; - let (held, pointer, now) = ctx.input(|i| (i.key_down(Key::Space), i.pointer.any_down(), i.time)); - hand.used |= pointer; + let (held, pointer, pressed, now) = ctx.input(|i| (i.key_down(Key::Space), i.pointer.any_down(), i.pointer.any_pressed(), i.time)); + // a quick click can go down and up within one frame: no button is down then, but it pressed + hand.used |= pointer || pressed; let tool = tool_owns_pointer(app) && app.ui.dialog.is_none(); // held, or let go in the middle of a pan: the hand stays until the button is up if tool && (held || pointer) { diff --git a/crates/ui-egui/src/tests_space_hand.rs b/crates/ui-egui/src/tests_space_hand.rs index 8bf3ba992..701408e34 100644 --- a/crates/ui-egui/src/tests_space_hand.rs +++ b/crates/ui-egui/src/tests_space_hand.rs @@ -158,6 +158,26 @@ fn space_held_click_zooms_to_the_point_clicked_and_back() { assert_eq!(develop(&h).spots.len(), spots, "a click painted a spot"); } +#[test] +fn a_click_down_and_up_within_one_frame_is_not_a_tap() { + // a real mouse can deliver the press and the release in the same frame: no button is down + // then, yet the click zoomed, and letting go of Space quickly must not zoom back + let mut h = detail(Some("panel.remove")); + space(&mut h, "down"); + pointer(&mut h, json!([{"kind": "move", "x": 0.25, "y": 0.3}])); + let img = h.app.image_rect.unwrap(); + let pos = egui::pos2(img.left() + 0.25 * img.width(), img.top() + 0.3 * img.height()); + for pressed in [true, false] { + h.events.push(egui::Event::PointerButton { pos, button: egui::PointerButton::Primary, pressed, modifiers: egui::Modifiers::NONE }); + } + h.step(); + let zoomed = Zoom::Percent(h.app.ui.click_zoom as f32); + assert_eq!(h.app.ui.zoom, zoomed, "the click did not zoom"); + space(&mut h, "up"); + assert!(h.app.space_hand.is_none()); + assert_eq!(h.app.ui.zoom, zoomed, "the release toggled the zoom back"); +} + #[test] fn a_tap_over_a_tool_toggles_on_release_and_a_long_hold_does_not() { let mut h = detail(Some("panel.remove")); From c9c3870ab38718a63c5faaa7b677b39cbbc002eb Mon Sep 17 00:00:00 2001 From: helgehelge123 <117832631+helgehelge123@users.noreply.github.com> Date: Sun, 25 Oct 2026 00:50:21 +0200 Subject: [PATCH 3/5] KEY-TOGGLEZOOM: Toggle Zoom bound to Space in the keymap takes the hand and toggles once When the user gives view.zoomToggle the Space key, the Space alias yields to it and the bindable-command loop dispatched the zoom directly: the hand never started over a tool and the key's auto-repeat toggled again. The binding now goes through the alias's handoff and repeat check. Co-Authored-By: Claude Opus 5.5 --- crates/ui-egui/src/shortcuts.rs | 14 +++++++++++--- crates/ui-egui/src/tests_space_hand.rs | 23 +++++++++++++++++++++++ 2 files changed, 34 insertions(+), 3 deletions(-) diff --git a/crates/ui-egui/src/shortcuts.rs b/crates/ui-egui/src/shortcuts.rs index 3dbf680f6..6a9e19a4f 100644 --- a/crates/ui-egui/src/shortcuts.rs +++ b/crates/ui-egui/src/shortcuts.rs @@ -393,6 +393,8 @@ pub fn handle(app: &mut LightcraftApp, ctx: &egui::Context) { { fire.push("view.detail".to_string()); } + // a held Space repeats: it toggles the zoom (or takes the hand) once per press + let space_repeat = !i.events.iter().any(|e| matches!(e, egui::Event::Key { key: Key::Space, pressed: true, repeat: false, .. })); for b in bindable() { if let Some(sc) = binding(keymap, b.id, b.default) && let Some((m, k)) = parse(sc) @@ -401,11 +403,17 @@ pub fn handle(app: &mut LightcraftApp, ctx: &egui::Context) { && matches(i, m, k) && let Some(id) = grid_key_command(keymap, grid, app.ui.fullscreen, b.id, (m, k)) { - fire.push(id.to_string()); + if id == "view.zoomToggle" && k == Key::Space { + // Toggle Zoom given Space in the keymap is the Space alias: the same hand and + // the same once-per-press + if !space_repeat { + aliased.push(("view.zoomToggle", json!({}))); + } + } else { + fire.push(id.to_string()); + } } } - // a held Space repeats: it toggles the zoom (or takes the hand) once per press - let space_repeat = !i.events.iter().any(|e| matches!(e, egui::Event::Key { key: Key::Space, pressed: true, repeat: false, .. })); for (sc, id, params) in ALIASES { if let Some((m, k)) = parse(sc).filter(|_| !native(sc)) && !(k == Key::Escape && popup_open) diff --git a/crates/ui-egui/src/tests_space_hand.rs b/crates/ui-egui/src/tests_space_hand.rs index 701408e34..02c90705b 100644 --- a/crates/ui-egui/src/tests_space_hand.rs +++ b/crates/ui-egui/src/tests_space_hand.rs @@ -106,6 +106,29 @@ fn a_held_space_toggles_the_zoom_once_however_long_it_repeats() { assert_eq!(h.app.ui.zoom, Zoom::Fit); } +#[test] +fn toggle_zoom_given_space_in_the_keymap_takes_the_hand_and_toggles_once() { + // with Space as Toggle Zoom's own shortcut the Space alias yields to it: the binding gets the + // same hand over a tool and the same once-per-press + let mut h = detail(None); + crate::shortcuts::assign(&mut h.app.ui.settings.keymap, "view.zoomToggle", Some("Space")).unwrap(); + space(&mut h, "down"); + assert_eq!(h.app.ui.zoom, Zoom::Percent(100.0)); + for _ in 0..3 { + space(&mut h, "down"); + } + assert_eq!(h.app.ui.zoom, Zoom::Percent(100.0), "a repeat toggled the zoom"); + space(&mut h, "up"); + let r = h.request("engine.execute", json!({"command": "panel.remove"}), T); + assert_eq!(r["ok"], true, "{r}"); + h.step(); + zoom_in(&mut h); + space(&mut h, "down"); + assert!(h.app.space_hand.is_some(), "the binding did not take the hand"); + assert_eq!(h.app.ui.zoom, Zoom::Percent(200.0), "the press zoomed"); + space(&mut h, "up"); +} + #[test] fn space_held_over_the_healing_brush_pans_and_paints_no_spot() { let mut h = detail(Some("panel.remove")); From 28ad99d77132caa63c6b0fef53223a4a16cf100a Mon Sep 17 00:00:00 2001 From: helgehelge123 <117832631+helgehelge123@users.noreply.github.com> Date: Mon, 26 Oct 2026 03:16:56 +0100 Subject: [PATCH 4/5] KEY-TOGGLEZOOM: the Space hand follows egui 0.37's ids and the registry's Ui Co-Authored-By: Claude Opus 5.5 --- crates/ui-egui/src/space_hand.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/ui-egui/src/space_hand.rs b/crates/ui-egui/src/space_hand.rs index a02f2ac84..3c5f36835 100644 --- a/crates/ui-egui/src/space_hand.rs +++ b/crates/ui-egui/src/space_hand.rs @@ -76,8 +76,8 @@ pub(crate) fn interact(app: &mut LightcraftApp, ui: &mut egui::Ui, canvas: Rect, if app.space_hand.is_none() { return; } - let resp = ui.interact(canvas, egui::Id::new("loupe-hand"), Sense::click_and_drag()); - crate::widgets::register(ui.ctx(), "loupe:hand", canvas); + let resp = ui.interact(canvas, egui::Id::unique("loupe-hand"), Sense::click_and_drag()); + crate::widgets::register(ui, "loupe:hand", canvas); let at = resp.interact_pointer_pos().or(resp.hover_pos()); let (pane, img) = match (before, at) { (Some((pane, img)), Some(q)) if pane.contains(q) => (pane, img), From fa098806417e396e52c8c9a1f95963cef2cbc574 Mon Sep 17 00:00:00 2001 From: helgehelge123 <117832631+helgehelge123@users.noreply.github.com> Date: Tue, 27 Oct 2026 03:35:41 +0100 Subject: [PATCH 5/5] KEY-TOGGLEZOOM: the checklist row names the Space-hand tests where main's layout put them Co-Authored-By: Claude Opus 5.5 --- docs/parity-checklist.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/parity-checklist.md b/docs/parity-checklist.md index 4ef7b8cb2..02a01d97f 100644 --- a/docs/parity-checklist.md +++ b/docs/parity-checklist.md @@ -629,7 +629,7 @@ From `06-shortcuts.md` part 1. Evidence is our binding; conflicts are explained | KEY-WB | White-balance selector — W | P0 | ✅ | `cmd:tool.wbPicker` | | | KEY-FILMSTRIP | Filmstrip — / | P0 | ✅ | `cmd:view.filmstrip` | | | KEY-SHOWORIGINAL | Show original — \ | P0 | ✅ | `cmd:view.showOriginal` | | -| KEY-TOGGLEZOOM | Toggle zoom — Space | P0 | ✅ | `cmd:view.zoomToggle`, `crates/ui-egui/src/space_hand.rs`, `crates/ui-egui/src/tests_space_hand.rs` | Space is a secondary binding (primary Z); held, it toggles once (the key's auto-repeat no longer flips the zoom back and forth). With a tool on the photo (Crop, Masking, Remove, Red Eye, the white-balance / colour-range / point-colour pickers, a targeted adjustment) held Space is the view's hand until the key is up, as in Lightroom: a click zooms to the click-zoom ratio at the point clicked (or back to Fit), a drag pans the zoomed photo (grab cursor; zoom-in cursor at Fit), and the tool gets neither; a tap (under 0.4 s, no click) toggles the zoom on its release (issue #1895) | +| KEY-TOGGLEZOOM | Toggle zoom — Space | P0 | ✅ | `cmd:view.zoomToggle`, `crates/ui-egui/src/space_hand.rs`, `crates/ui-egui/src/tests/space_hand.rs` | Space is a secondary binding (primary Z); held, it toggles once (the key's auto-repeat no longer flips the zoom back and forth). With a tool on the photo (Crop, Masking, Remove, Red Eye, the white-balance / colour-range / point-colour pickers, a targeted adjustment) held Space is the view's hand until the key is up, as in Lightroom: a click zooms to the click-zoom ratio at the point clicked (or back to Fit), a drag pans the zoomed photo (grab cursor; zoom-in cursor at Fit), and the tool gets neither; a tap (under 0.4 s, no click) toggles the zoom on its release (issue #1895) | | KEY-MASKCOLOR | Cycle mask colour — ⇧O | P1 | ✅ | `cmd:view.maskOverlayColor`, `crates/ui-egui/src/shortcuts.rs` | ⇧O while masking cycles the overlay colour (while cropping: the guides' orientation); the overlay mode is in View ▸ Cycle Mask Overlay Mode | | KEY-EXPORTPREV | Export with previous — ⌘E | P0 | ✅ | `cmd:app.exportPrevious` | ⌘E (alias) and ⌥⇧⌘E (Classic) | | KEY-EXPORTDIALOG | Export dialog — ⇧E | P0 | ✅ | `cmd:dialog.export` | secondary binding (primary ⌘⇧E) |