diff --git a/crates/ui-egui/src/app.rs b/crates/ui-egui/src/app.rs index fa10210f0..906c7aca7 100644 --- a/crates/ui-egui/src/app.rs +++ b/crates/ui-egui/src/app.rs @@ -88,6 +88,7 @@ impl LightcraftApp { next_widgets: vec![], next_widget_hits: vec![], gesture: None, + space_hand: None, brush_live: None, loupe_shown: None, region_view: None, diff --git a/crates/ui-egui/src/app/input.rs b/crates/ui-egui/src/app/input.rs index 35b7991b4..8234b78fd 100644 --- a/crates/ui-egui/src/app/input.rs +++ b/crates/ui-egui/src/app/input.rs @@ -186,9 +186,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()), + } }; // A press needs the pointer at its position before the button event; the move that is // itself the first event is delivered by the drain below, so it must not be sent twice. diff --git a/crates/ui-egui/src/control.rs b/crates/ui-egui/src/control.rs index caacbc590..73c8bc2c2 100644 --- a/crates/ui-egui/src/control.rs +++ b/crates/ui-egui/src/control.rs @@ -693,8 +693,17 @@ pub(crate) fn gesture_events(app: &mut LightcraftApp, ctx: &egui::Context, metho "ui.key" => { let Some(k) = s("key").and_then(key_from) else { return Err("unknown or missing `key`".into()) }; 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(GestureOutcome::done(Value::Null)) } diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index 8bf57b341..f3a800178 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -41,6 +41,7 @@ pub mod render; pub mod shortcuts; pub mod sidebar; pub mod softpaint; +pub mod space_hand; pub mod state; pub mod sync; pub mod tasks; @@ -268,6 +269,8 @@ pub struct LightcraftApp { next_widget_hits: Vec<(String, widgets::WidgetHit)>, /// 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", diff --git a/crates/ui-egui/src/panels/detail.rs b/crates/ui-egui/src/panels/detail.rs index 6840afdf0..eef7a4bec 100644 --- a/crates/ui-egui/src/panels/detail.rs +++ b/crates/ui-egui/src/panels/detail.rs @@ -334,6 +334,8 @@ pub fn show(app: &mut LightcraftApp, ui: &mut egui::Ui) { let tv = ToolView { right, map: &map, frame: &frame, d: &d, id, split, native, aspect }; tool_interaction(app, ui, &resp, &tv, &Views { img_rect, target_rect, before_img, before_target, areas: &areas, main_area, canvas }); } + // 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, @@ -1028,8 +1030,19 @@ fn general_interaction(app: &mut LightcraftApp, ui: &mut egui::Ui, resp: &egui:: } 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); @@ -1045,7 +1058,6 @@ fn general_interaction(app: &mut LightcraftApp, ui: &mut egui::Ui, resp: &egui:: if zoomed && resp.dragged() { pan_image(app, canvas, img, resp.drag_delta()); } - let _ = (native, aspect); } #[cfg(test)] diff --git a/crates/ui-egui/src/shortcuts.rs b/crates/ui-egui/src/shortcuts.rs index 2f0b07d45..02c4dab2f 100644 --- a/crates/ui-egui/src/shortcuts.rs +++ b/crates/ui-egui/src/shortcuts.rs @@ -367,6 +367,8 @@ fn keyboard_owned(app: &LightcraftApp, ctx: &egui::Context) -> bool { /// text field, name edit or shortcut recording owns input, then fires saved command bindings, /// aliases and fixed photo keys, respecting native menu ownership and open popups. 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; } @@ -425,6 +427,8 @@ fn pressed_commands(app: &LightcraftApp, ctx: &egui::Context) -> (Vec, V { 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) @@ -433,12 +437,21 @@ fn pressed_commands(app: &LightcraftApp, ctx: &egui::Context) -> (Vec, V && 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()); + } } } 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) { @@ -505,6 +518,10 @@ fn fire_alias(app: &mut LightcraftApp, ctx: &egui::Context, id: &str, params: se } return; } + // 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) { + return; + } // 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..3c5f36835 --- /dev/null +++ b/crates/ui-egui/src/space_hand.rs @@ -0,0 +1,100 @@ +//! 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, 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) { + 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::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), + _ => 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.rs b/crates/ui-egui/src/tests.rs index 3a102a29a..f96313608 100644 --- a/crates/ui-egui/src/tests.rs +++ b/crates/ui-egui/src/tests.rs @@ -71,6 +71,7 @@ mod scroll; mod slider_reset; mod slideshow; mod solo_sections; +mod space_hand; mod switch_library; mod sync; mod tallies; 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..02c90705b --- /dev/null +++ b/crates/ui-egui/src/tests/space_hand.rs @@ -0,0 +1,263 @@ +//! 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 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")); + 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_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")); + 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 36294c787..340d6d54f 100644 --- a/docs/control-protocol.md +++ b/docs/control-protocol.md @@ -84,7 +84,7 @@ filmstrip is visible. | `ui.clickWidget` / `ui.dragWidget` / `ui.hoverWidget` | `{id, count?, button?, fx?, fy?}` (`button: "right"` right-clicks; `count` is capped at 16) / `{id, toX?, toY?, dx?, dy?, steps?}` (steps are capped at 1000) / `{id, fx?, fy?}` | Real egui input on a widget (hover: the pointer rests on it, e.g. for preset/profile previews). `fx`/`fy` are fractions of the registered rect and may lie outside `[0,1]`; the resulting point must still be inside egui's clipped interaction rect, and a non-finite fraction or resolved point is rejected before any input is queued. Geometry comes from the latest completed layout; if the widget moves before the first event or before a press, or its latest clipped rect no longer contains the point, the request fails with an error instead of clicking another widget or retargeting. A target below an ordinary modal dialog may succeed as a no-op or a backdrop click, because egui routes that input to the dialog; a target covered by a non-modal overlay fails. The blocking library-problem/launch-list window is explicit: its dim takes the modal layer, but a background target fails instead of reporting a no-op success, and only that window's own controls (`button:library…`) pass. Its creation frame is shielded and redrawn with empty input like a prompt's, so background text/pointer input cannot act while the new modal areas settle. A quit prompt or settings notice is a real egui modal scope with a foreground backdrop. Only its own buttons (`button:quit…`, `button:noticeOk`) accept a `ui.clickWidget`; background targets fail, and background keystrokes, text/clipboard input, wheel/zoom input and AccessKit actions are dropped before panels see them. The prompt itself takes Tab/Shift+Tab, Enter/Space on the focused button, and Escape/Enter for the safe default Cancel; AccessKit Focus/Click works when targeted at a prompt button. Its first creation frame (including a Tasks→Unsaved or notice→next-notice transition) is shielded and redrawn with empty input, so a press/release in that frame, or a held press from before it, cannot act underneath it; a synthetic drag already in progress is canceled. Once settled, raw `ui.click`/`ui.drag`/`ui.pointer` and backdrop clicks are still delivered to the prompt backdrop as no-ops, and the prompt's own button clicks work normally. The area/layer check cannot see a different widget covering the point in the same layer. A `ui.clickWidget` whose widget disappears before its release also fails, after canceling the pointer press without a click or drag (the same rules apply when a click spans several host frames) | | `ui.move` / `ui.click` / `ui.drag` | `{x, y, count?, button?}` / `{x, y, toX, toY, steps?}` | Raw pointer input, screen points (the same click and step caps apply) | | `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; while a prompt is up only its Tab/Shift+Tab, Enter, Space and Escape are delivered, every other key is dropped | +| `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); while a prompt is up only its Tab/Shift+Tab, Enter, Space and Escape are delivered, every other key is dropped | | `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. Refused while a prompt or the blocking library window owns input, including a window in its creation frame. A `paste` without `text` queues egui's `RequestPaste` viewport command for the host: egui 0.37 gives that command no completion/correlation id, and egui-winit emits a later `Event::Paste` only for non-empty text (or a `PasteImage` fallback; asynchronous clipboard reads finish later), so the reply confirms the request was queued in egui, not that the host dispatched it or that it was delivered; a barrier created after the request can drop that later event. `paste` with `text` is synthetic and is canceled like other injected input. `ui.inspect` → `copied` is the text the UI last put on the clipboard | | `ui.scroll` | `{dx, dy, source?, unit?, phase?, cmd?, ctrl?, shift?, alt?}` | Wheel / two-finger scroll at the current pointer; pans over the image, modifier-scroll zooms. `source` is `unknown` (default), `wheel`, `trackpad` or `momentum`; `unit` is `point` (default), `line` or `page`; `phase` is `move` (default), `start`, `end` or `cancel`. Wheel/line events drive the grid/filmstrip one-third wheel step and the loupe's notch zoom; trackpad/momentum phases drive kinetic scrolling and rubber-band boundaries. Invalid values are refused and queue nothing | diff --git a/docs/parity-checklist.md b/docs/parity-checklist.md index 08dab300b..80896e26f 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) | @@ -846,6 +846,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 24099eb63..435486014 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 | @@ -158,6 +163,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 |