From 7635b38c3634800e0754bea93c122d8f68f51c0a Mon Sep 17 00:00:00 2001 From: mahmatias Date: Sun, 18 Oct 2026 11:56:27 -0300 Subject: [PATCH 1/9] feat(guided): select and delete one Guided Upright guide MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A click near a guide selects it (white, thicker, larger endpoints); drawing a new guide or clicking away clears the selection, and so does leaving the photo or the tool. Delete / Backspace with the tool open removes only the selected guide (geometry.guides minus its index, one undo step); with none selected it removes nothing — the photo itself is never deleted while Guided Upright is open. selected_guide is UI-only (serde skip). Coverage: tests_guided_guides 5/5. --- crates/ui-egui/src/lib.rs | 2 + crates/ui-egui/src/panels/detail.rs | 67 +++++++++--- crates/ui-egui/src/shortcuts.rs | 15 +++ crates/ui-egui/src/state.rs | 5 + crates/ui-egui/src/tests_guided_guides.rs | 125 ++++++++++++++++++++++ 5 files changed, 202 insertions(+), 12 deletions(-) create mode 100644 crates/ui-egui/src/tests_guided_guides.rs diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index a08fb6151..b7e778543 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -99,6 +99,8 @@ mod tests_grading; #[cfg(test)] mod tests_grid; #[cfg(test)] +mod tests_guided_guides; +#[cfg(test)] mod tests_info_dialogs; #[cfg(test)] mod tests_keymap; diff --git a/crates/ui-egui/src/panels/detail.rs b/crates/ui-egui/src/panels/detail.rs index b3f0ff386..2dae69630 100644 --- a/crates/ui-egui/src/panels/detail.rs +++ b/crates/ui-egui/src/panels/detail.rs @@ -1535,8 +1535,12 @@ fn general_interaction( // ------------------------------------------------------------------------ crop fn crop_overlay(app: &mut LightcraftApp, ui: &mut egui::Ui, resp: &egui::Response, map: &CanvasMap, frame: &Frame, d: &DevelopSettings, id: PhotoId) { + // the selected guide belongs to this photo and to the Guided Upright tool + if app.ui.tool != "guidedUpright" || app.ui.selected_guide.is_some_and(|(photo, _)| photo != id.0) { + app.ui.selected_guide = None; + } if app.ui.tool == "guidedUpright" { - guided_overlay(app, ui, resp, map, frame, d); + guided_overlay(app, ui, resp, map, frame, d, id); return; } if app.ui.tool == "straighten" { @@ -1691,7 +1695,6 @@ fn crop_overlay(app: &mut LightcraftApp, ui: &mut egui::Ui, resp: &egui::Respons // the canvas (the loupe's own rect), not the photo: rotating happens in the margin around it angle_readout(ui, at, shown_angle, resp.rect); } - let _ = id; } /// The crop handles' names for screen readers, in `crop.drag`'s numbering: 0..3 corners, 4..7 @@ -1830,30 +1833,62 @@ fn angle_readout(ui: &egui::Ui, at: Pos2, angle: f64, bounds: Rect) { /// Guided Upright: draw up to four guides along lines that should be vertical or horizontal. Guides are /// stored in lens-corrected (pre-perspective) coordinates, so they stay attached to the image as it warps. -fn guided_overlay(app: &mut LightcraftApp, ui: &mut egui::Ui, resp: &egui::Response, map: &CanvasMap, frame: &Frame, d: &DevelopSettings) { +fn guided_overlay( + app: &mut LightcraftApp, + ui: &mut egui::Ui, + resp: &egui::Response, + map: &CanvasMap, + frame: &Frame, + d: &DevelopSettings, + id: PhotoId, +) { let p = ui.painter_at(map.rect.expand(8.0)); let col = Color32::from_rgb(255, 196, 40); - let draw = |a: Pos2, b: Pos2| { - p.line_segment([a, b], Stroke::new(3.0, Color32::from_black_alpha(120))); - p.line_segment([a, b], Stroke::new(1.5, col)); + // the selected guide: brighter, so the Delete key has a visible target + let selected = app.ui.selected_guide.filter(|(photo, _)| *photo == id.0).map(|(_, i)| i); + let draw = |a: Pos2, b: Pos2, sel: bool| { + let (edge, core, at) = if sel { (3.5, 2.5, 5.0) } else { (3.0, 1.5, 4.0) }; + p.line_segment([a, b], Stroke::new(edge, Color32::from_black_alpha(120))); + p.line_segment([a, b], Stroke::new(core, if sel { Color32::WHITE } else { col })); for q in [a, b] { - p.circle_filled(q, 4.0, col); - p.circle_stroke(q, 4.0, Stroke::new(1.0, Color32::BLACK)); + p.circle_filled(q, at, if sel { Color32::WHITE } else { col }); + p.circle_stroke(q, at, Stroke::new(1.0, Color32::BLACK)); } }; - for (i, (a, b)) in d.geometry.guides.iter().enumerate() { - let (sa, sb) = (map.screen(frame.corrected_to_transformed(*a)), map.screen(frame.corrected_to_transformed(*b))); + let on_screen = |guide: &(Point, Point)| { + let (sa, sb) = (map.screen(frame.corrected_to_transformed(guide.0)), map.screen(frame.corrected_to_transformed(guide.1))); + (sa, sb) + }; + for (i, guide) in d.geometry.guides.iter().enumerate() { + let (sa, sb) = on_screen(guide); register(ui.ctx(), format!("uprightGuide:{i}"), Rect::from_two_pos(sa, sb)); - draw(sa, sb); + draw(sa, sb, selected == Some(i)); } ui.ctx().set_cursor_icon(egui::CursorIcon::Crosshair); + // a click near a guide selects it (Delete / Backspace removes it alone); a click away clears + if resp.clicked() + && let Some(q) = resp.interact_pointer_pos() + { + let hit = d.geometry.guides.iter().position(|guide| { + let (a, b) = on_screen(guide); + point_segment_dist(q, a, b) <= 6.0 + }); + app.ui.selected_guide = match hit { + Some(i) if selected != Some(i) => Some((id.0, i)), + _ => None, + }; + } + if resp.drag_started() { + // drawing a new guide leaves nothing selected + app.ui.selected_guide = None; + } if resp.drag_started() && let Some(q) = resp.interact_pointer_pos() { app.gesture = Some(Gesture::Guide { a: map.norm(q) }); } if let (Some(Gesture::Guide { a }), Some(q)) = (&app.gesture, resp.interact_pointer_pos()) { - draw(map.screen(*a), q); + draw(map.screen(*a), q, false); } if resp.drag_stopped() && let (Some(Gesture::Guide { a }), Some(q)) = (app.gesture.clone(), resp.interact_pointer_pos()) @@ -1875,6 +1910,14 @@ fn to_straight(n: Point, angle: f64, frame: &Frame) -> Point { Point::new(r.x / w, r.y / h) } + +/// How far `q` is from the segment `a`–`b` (screen pixels): the click that selects a guide. +fn point_segment_dist(q: Pos2, a: Pos2, b: Pos2) -> f32 { + let ab = b - a; + let t = if ab.length_sq() > 0.0 { ((q - a).dot(ab) / ab.length_sq()).clamp(0.0, 1.0) } else { 0.0 }; + (a + ab * t).distance(q) +} + /// A crop box's corners (top-left, top-right, bottom-right, bottom-left of the unflipped photo) /// in the order they appear on screen when the photo is shown flipped. fn flipped_corners(pts: &[Pos2], flip_h: bool, flip_v: bool) -> [Pos2; 4] { diff --git a/crates/ui-egui/src/shortcuts.rs b/crates/ui-egui/src/shortcuts.rs index b9d1874a1..fb483f61c 100644 --- a/crates/ui-egui/src/shortcuts.rs +++ b/crates/ui-egui/src/shortcuts.rs @@ -558,6 +558,21 @@ pub fn handle(app: &mut LightcraftApp, ctx: &egui::Context) { } continue; } + R::Crop if app.ui.tool == "guidedUpright" => { + // Delete removes the selected guide alone; with none selected it removes + // nothing (the photo itself is never deleted while the tool is open) + if let (Some((photo, i)), Some(active)) = (app.ui.selected_guide, app.session.active()) { + if photo == active.0 + && let Some(d) = app.session.develop_of(active) + { + let guides: Vec<[f64; 4]> = + d.geometry.guides.iter().enumerate().filter(|(k, _)| *k != i).map(|(_, (a, b))| [a.x, a.y, b.x, b.y]).collect(); + let _ = app.run("geometry.guides", json!({"guides": guides})); + } + app.ui.selected_guide = None; + } + continue; + } R::RedEye => continue, _ => {} } diff --git a/crates/ui-egui/src/state.rs b/crates/ui-egui/src/state.rs index 7268d8a2b..9b0e8ee82 100644 --- a/crates/ui-egui/src/state.rs +++ b/crates/ui-egui/src/state.rs @@ -815,6 +815,10 @@ pub struct UiState { pub grading_hidden: Option<(lightcraft_catalog::PhotoId, [bool; 3])>, /// Active on-canvas tool: "", "brush", "linear", "radial", "wbPicker", "straighten", "remove". pub tool: String, + /// Guided Upright: the selected guide (photo id + index) while the tool is open. A Delete / + /// Backspace on it removes that guide alone; a click away or leaving the tool clears it. + #[serde(skip)] + pub selected_guide: Option<(u64, usize)>, pub brush_size: f32, pub brush_feather: f32, pub brush_flow: f32, @@ -1345,6 +1349,7 @@ impl Default for UiState { grading_mode: "3way".into(), grading_hidden: None, tool: String::new(), + selected_guide: None, brush_size: 0.04, brush_feather: 50.0, brush_flow: 60.0, diff --git a/crates/ui-egui/src/tests_guided_guides.rs b/crates/ui-egui/src/tests_guided_guides.rs new file mode 100644 index 000000000..65e7be6b9 --- /dev/null +++ b/crates/ui-egui/src/tests_guided_guides.rs @@ -0,0 +1,125 @@ +//! Headless tests of Guided Upright guide selection and Delete (issue: remove one guide): a click +//! near a guide selects it, Delete removes the selected guide alone, a click away clears the +//! selection, and with no selection Delete removes nothing (never the photo). + +use std::time::Duration; + +use serde_json::json; + +use crate::headless::Headless; +use crate::{LightcraftApp, Services}; + +const T: Duration = Duration::from_secs(20); +const SETTLE: Duration = Duration::from_secs(120); + +/// Crop with Guided Upright on and two horizontal guides drawn (in lens-corrected coordinates). +fn guided() -> Headless { + let app = LightcraftApp::new(lightcraft_engine::Session::with_demo(), Services { png: None, ..Default::default() }); + let mut h = Headless::new(app, [1200.0, 800.0], 1.0); + for (method, params) in [ + ("ui.set", json!({"view": "detail"})), + ("engine.execute", json!({"command": "tool.guidedUpright"})), + ("engine.execute", json!({"command": "geometry.guides", "params": {"guides": [[0.2, 0.3, 0.8, 0.3], [0.2, 0.7, 0.8, 0.7]]}})), + ] { + let r = h.request(method, params, T); + assert_eq!(r["ok"], true, "{r}"); + } + h.settle(SETTLE); + h +} + +fn guides_len(h: &Headless) -> usize { + let id = h.app.session.active().expect("a photo"); + h.app.session.catalog.photo(id).expect("the photo").develop.geometry.guides.len() +} + +fn guide_at(h: &Headless, i: usize) -> egui::Pos2 { + h.app.widgets.iter().rev().find(|(w, _)| *w == format!("uprightGuide:{i}")).map(|(_, r)| r.center()).expect("the guide is on screen") +} + +/// A click near a guide selects it (Delete has a visible target); the guides stay put. +#[test] +fn a_click_near_a_guide_selects_it() { + let mut h = guided(); + assert_eq!(guides_len(&h), 2); + let id = h.app.session.active().expect("a photo"); + let r = h.request("ui.click", json!({"x": guide_at(&h, 0).x, "y": guide_at(&h, 0).y}), T); + assert_eq!(r["ok"], true, "{r}"); + h.settle(SETTLE); + assert_eq!(h.app.ui.selected_guide, Some((id.0, 0)), "the clicked guide is selected"); + assert_eq!(guides_len(&h), 2, "selecting removes nothing"); +} + +/// Delete removes the selected guide alone; the other one stays, and nothing stays selected. +#[test] +fn delete_removes_the_selected_guide() { + let mut h = guided(); + let r = h.request("ui.click", json!({"x": guide_at(&h, 1).x, "y": guide_at(&h, 1).y}), T); + assert_eq!(r["ok"], true, "{r}"); + h.settle(SETTLE); + assert_eq!(h.app.ui.selected_guide.map(|(_, i)| i), Some(1)); + let r = h.request("ui.key", json!({"key": "Delete"}), T); + assert_eq!(r["ok"], true, "{r}"); + h.settle(SETTLE); + assert_eq!(guides_len(&h), 1, "one guide is gone"); + assert_eq!(h.app.ui.selected_guide, None, "and nothing is selected afterwards"); + // the survivor is the first guide: the second one's endpoints are gone + let id = h.app.session.active().expect("a photo"); + let d = h.app.session.catalog.photo(id).expect("the photo"); + let (a, b) = d.develop.geometry.guides[0]; + assert!( + (a.x - 0.2).abs() < 1e-6 && (a.y - 0.3).abs() < 1e-6 && (b.x - 0.8).abs() < 1e-6 && (b.y - 0.3).abs() < 1e-6, + "the first guide survived: {a:?}–{b:?}" + ); +} + +/// A click away from every guide clears the selection (and removes nothing). +#[test] +fn a_click_away_clears_the_selection() { + let mut h = guided(); + let r = h.request("ui.click", json!({"x": guide_at(&h, 0).x, "y": guide_at(&h, 0).y}), T); + assert_eq!(r["ok"], true, "{r}"); + h.settle(SETTLE); + assert!(h.app.ui.selected_guide.is_some()); + let img = h.app.image_rect.expect("the photo on screen"); + // below both guides, far from them + let r = h.request("ui.click", json!({"x": img.center().x, "y": img.top() + 0.95 * img.height()}), T); + assert_eq!(r["ok"], true, "{r}"); + h.settle(SETTLE); + assert_eq!(h.app.ui.selected_guide, None); + assert_eq!(guides_len(&h), 2, "and removes nothing"); +} + +/// With nothing selected Delete removes no guide and never deletes the photo: the tool is open. +#[test] +fn delete_with_no_selection_removes_nothing() { + let mut h = guided(); + let undo = h.app.session.undo.len(); + let r = h.request("ui.key", json!({"key": "Delete"}), T); + assert_eq!(r["ok"], true, "{r}"); + h.settle(SETTLE); + assert_eq!(guides_len(&h), 2, "no guide is removed"); + assert_eq!(h.app.session.undo.len(), undo, "no edit either"); + assert!(h.app.session.active().is_some(), "the photo is still active"); +} + +/// Drawing a new guide clears the selection first, so Delete after a draw removes the new guide. +#[test] +fn drawing_a_new_guide_clears_the_selection() { + let mut h = guided(); + let r = h.request("ui.click", json!({"x": guide_at(&h, 0).x, "y": guide_at(&h, 0).y}), T); + assert_eq!(r["ok"], true, "{r}"); + h.settle(SETTLE); + assert!(h.app.ui.selected_guide.is_some()); + let img = h.app.image_rect.expect("the photo on screen"); + // a drag across the middle of the photo (clear of both guides) draws a third guide + let r = h.request( + "ui.drag", + json!({"x": img.left() + 0.3 * img.width(), "y": img.top() + 0.5 * img.height(), "toX": img.left() + 0.7 * img.width(), "toY": img.top() + 0.5 * img.height()}), + T, + ); + assert_eq!(r["ok"], true, "{r}"); + h.settle(SETTLE); + assert_eq!(guides_len(&h), 3, "the drawn guide is added"); + assert_eq!(h.app.ui.selected_guide, None, "and the click's selection is gone"); +} From a22c496d038bcf961d0e641e66e1105356d73a56 Mon Sep 17 00:00:00 2001 From: mahmatias Date: Mon, 19 Oct 2026 10:43:30 -0300 Subject: [PATCH 2/9] feat(guided): guides start on the press and their endpoints can be dragged A press starts what it means right away from the pointer's press point: on an endpoint (within 7 px) it drags that endpoint, committing one geometry.guides edit on release (the guide must stay at least 0.02 long; a press without a real move only selects, so a click never leaves a no-op undo entry); on a line (within 6 px) it selects the guide (a press on the selected one deselects); anywhere else it starts a new guide from the press point itself instead of where egui's drag threshold was crossed, leaving nothing selected. The endpoint being dragged is drawn bright while the pointer moves. tests_guided_guides 8/8. --- crates/ui-egui/src/panels/detail.rs | 102 ++++++++++++++++------ crates/ui-egui/src/tests_guided_guides.rs | 87 +++++++++++++++++- 2 files changed, 162 insertions(+), 27 deletions(-) diff --git a/crates/ui-egui/src/panels/detail.rs b/crates/ui-egui/src/panels/detail.rs index 2dae69630..6a6c74e5b 100644 --- a/crates/ui-egui/src/panels/detail.rs +++ b/crates/ui-egui/src/panels/detail.rs @@ -60,6 +60,11 @@ pub enum Gesture { Guide { a: Point, }, + /// Guided Upright: dragging the endpoint `end` (0 = `guide.0`, 1 = `guide.1`) of guide `i`. + GuideEndpoint { + i: usize, + end: usize, + }, /// Red Eye: an ellipse being dragged from corner `a` (normalized). Eye { a: Point, @@ -1865,39 +1870,86 @@ fn guided_overlay( draw(sa, sb, selected == Some(i)); } ui.ctx().set_cursor_icon(egui::CursorIcon::Crosshair); - // a click near a guide selects it (Delete / Backspace removes it alone); a click away clears - if resp.clicked() - && let Some(q) = resp.interact_pointer_pos() + // A press starts what it means, right away (the pointer's press point, not where a drag + // threshold would have been crossed): on an endpoint it drags that endpoint (the guide is + // selected when it lands), on a line it selects the guide (a press on the selected one + // deselects), anywhere else it starts a new guide from the press point. + if ui.input(|i| i.pointer.any_pressed()) + && let Some(q) = ui.input(|i| i.pointer.press_origin()).or(resp.interact_pointer_pos()) + && resp.rect.contains(q) { - let hit = d.geometry.guides.iter().position(|guide| { + let endpoint = d.geometry.guides.iter().enumerate().find_map(|(i, guide)| { + let (a, b) = on_screen(guide); + if a.distance(q) <= 7.0 { + Some((i, 0)) + } else if b.distance(q) <= 7.0 { + Some((i, 1)) + } else { + None + } + }); + let line = d.geometry.guides.iter().position(|guide| { let (a, b) = on_screen(guide); point_segment_dist(q, a, b) <= 6.0 }); - app.ui.selected_guide = match hit { - Some(i) if selected != Some(i) => Some((id.0, i)), - _ => None, - }; + match (endpoint, line) { + (Some((i, end)), _) => app.gesture = Some(Gesture::GuideEndpoint { i, end }), + (None, Some(i)) => app.ui.selected_guide = if selected == Some(i) { None } else { Some((id.0, i)) }, + // drawing a new guide leaves nothing selected + (None, None) => { + app.ui.selected_guide = None; + app.gesture = Some(Gesture::Guide { a: map.norm(q) }); + } + } } - if resp.drag_started() { - // drawing a new guide leaves nothing selected - app.ui.selected_guide = None; - } - if resp.drag_started() - && let Some(q) = resp.interact_pointer_pos() - { - app.gesture = Some(Gesture::Guide { a: map.norm(q) }); - } - if let (Some(Gesture::Guide { a }), Some(q)) = (&app.gesture, resp.interact_pointer_pos()) { + let q = resp.interact_pointer_pos().or_else(|| ui.input(|i| i.pointer.latest_pos())); + if let (Some(Gesture::Guide { a }), Some(q)) = (&app.gesture, q) { draw(map.screen(*a), q, false); } - if resp.drag_stopped() - && let (Some(Gesture::Guide { a }), Some(q)) = (app.gesture.clone(), resp.interact_pointer_pos()) + // the endpoint being dragged: the guide follows the pointer, bright + if let (Some(Gesture::GuideEndpoint { i, end }), Some(q)) = (&app.gesture, q) + && let Some(guide) = d.geometry.guides.get(*i) { - app.gesture = None; - let b = map.norm(q); - if a.dist(b) > 0.02 { - let (ca, cb) = (frame.transformed_to_corrected(a), frame.transformed_to_corrected(b)); - let _ = app.run("geometry.guides", json!({"guides": [[ca.x, ca.y, cb.x, cb.y]], "add": true})); + let fixed = if *end == 0 { guide.1 } else { guide.0 }; + draw(map.screen(frame.corrected_to_transformed(fixed)), q, true); + } + if ui.input(|i| i.pointer.any_released()) { + match app.gesture.take() { + // the endpoint lands: the guide stays (one undo step) and is selected. A press + // without a real drag only selects — no no-op undo entry for a click. + Some(Gesture::GuideEndpoint { i, end }) => { + app.ui.selected_guide = Some((id.0, i)); + if let (Some(q), Some(guide)) = (q, d.geometry.guides.get(i)) { + let (old_t, fixed_t) = if end == 0 { (guide.0, guide.1) } else { (guide.1, guide.0) }; + let new_t = frame.transformed_to_corrected(map.norm(q)); + let (a, b) = if end == 0 { (new_t, fixed_t) } else { (fixed_t, new_t) }; + let (na, nb) = (frame.corrected_to_transformed(a), frame.corrected_to_transformed(b)); + let moved = map.screen(frame.corrected_to_transformed(old_t)).distance(q) > 2.0; + if moved && na.dist(nb) > 0.02 { + let all: Vec<[f64; 4]> = d + .geometry + .guides + .iter() + .enumerate() + .map(|(j, g)| { + let (x, y) = if j == i { (a, b) } else { *g }; + [x.x, x.y, y.x, y.y] + }) + .collect(); + let _ = app.run("geometry.guides", json!({"guides": all})); + } + } + } + Some(Gesture::Guide { a }) => { + if let Some(q) = q + && map.norm(q).dist(a) > 0.02 + { + let b = map.norm(q); + let (ca, cb) = (frame.transformed_to_corrected(a), frame.transformed_to_corrected(b)); + let _ = app.run("geometry.guides", json!({"guides": [[ca.x, ca.y, cb.x, cb.y]], "add": true})); + } + } + other => app.gesture = other, } } } diff --git a/crates/ui-egui/src/tests_guided_guides.rs b/crates/ui-egui/src/tests_guided_guides.rs index 65e7be6b9..6d6b8df0d 100644 --- a/crates/ui-egui/src/tests_guided_guides.rs +++ b/crates/ui-egui/src/tests_guided_guides.rs @@ -34,7 +34,11 @@ fn guides_len(h: &Headless) -> usize { } fn guide_at(h: &Headless, i: usize) -> egui::Pos2 { - h.app.widgets.iter().rev().find(|(w, _)| *w == format!("uprightGuide:{i}")).map(|(_, r)| r.center()).expect("the guide is on screen") + guide_rect(h, i).center() +} + +fn guide_rect(h: &Headless, i: usize) -> egui::Rect { + h.app.widgets.iter().rev().find(|(w, _)| *w == format!("uprightGuide:{i}")).map(|(_, r)| *r).expect("the guide is on screen") } /// A click near a guide selects it (Delete has a visible target); the guides stay put. @@ -69,7 +73,7 @@ fn delete_removes_the_selected_guide() { let (a, b) = d.develop.geometry.guides[0]; assert!( (a.x - 0.2).abs() < 1e-6 && (a.y - 0.3).abs() < 1e-6 && (b.x - 0.8).abs() < 1e-6 && (b.y - 0.3).abs() < 1e-6, - "the first guide survived: {a:?}–{b:?}" + "the first guide survived: {a:?}–{b:?}" ); } @@ -123,3 +127,82 @@ fn drawing_a_new_guide_clears_the_selection() { assert_eq!(guides_len(&h), 3, "the drawn guide is added"); assert_eq!(h.app.ui.selected_guide, None, "and the click's selection is gone"); } + +/// A press on an endpoint drags it: the guide follows the pointer — the other end and the other +/// guide untouched — the moved guide ends up selected, and nothing is added (one edit, on release). +#[test] +fn dragging_a_guide_endpoint_moves_it() { + let mut h = guided(); + assert_eq!(guides_len(&h), 2); + let id = h.app.session.active().expect("a photo"); + let img = h.app.image_rect.expect("the photo on screen"); + let before = guide_rect(&h, 0); + let from = before.left_top(); + let to = egui::pos2(from.x + 24.0, from.y + 60.0); + // ui.pointer speaks normalized image coordinates + let norm = |p: egui::Pos2| (((p.x - img.left()) / img.width()) as f64, ((p.y - img.top()) / img.height()) as f64); + let (fx, fy) = norm(from); + let (tx, ty) = norm(to); + let r = h.request( + "ui.pointer", + json!({"events": [{"kind": "down", "x": fx, "y": fy}, {"kind": "drag", "x": tx, "y": ty}, {"kind": "up", "x": tx, "y": ty}]}), + T, + ); + assert_eq!(r["ok"], true, "{r}"); + h.settle(SETTLE); + assert_eq!(guides_len(&h), 2, "no guide is added"); + let d = h.app.session.catalog.photo(id).expect("the photo"); + let (a, b) = d.develop.geometry.guides[0]; + assert!((a.x - tx).abs() < 1e-3 && (a.y - ty).abs() < 1e-3, "the pressed end followed the pointer: {a:?} vs ({tx}, {ty})"); + assert!((b.x - 0.8).abs() < 1e-6 && (b.y - 0.3).abs() < 1e-6, "the far end stays: {b:?}"); + assert_eq!(h.app.ui.selected_guide, Some((id.0, 0)), "the moved guide is selected"); +} + +/// A press-release on an endpoint without moving it selects the guide and changes nothing: no +/// coordinate moves and no undo entry appears for a click. +#[test] +fn clicking_an_endpoint_selects_it_without_changing_it() { + let mut h = guided(); + let id = h.app.session.active().expect("a photo"); + let undo = h.app.session.undo.len(); + let img = h.app.image_rect.expect("the photo on screen"); + let at = guide_rect(&h, 1).left_top(); + let x = ((at.x - img.left()) / img.width()) as f64; + let y = ((at.y - img.top()) / img.height()) as f64; + let r = h.request("ui.pointer", json!({"events": [{"kind": "down", "x": x, "y": y}, {"kind": "up", "x": x, "y": y}]}), T); + assert_eq!(r["ok"], true, "{r}"); + h.settle(SETTLE); + assert_eq!(h.app.ui.selected_guide, Some((id.0, 1)), "the endpoint's guide is selected"); + assert_eq!(guides_len(&h), 2); + let d = h.app.session.catalog.photo(id).expect("the photo"); + let (a, b) = d.develop.geometry.guides[1]; + assert!( + (a.x - 0.2).abs() < 1e-6 && (a.y - 0.7).abs() < 1e-6 && (b.x - 0.8).abs() < 1e-6 && (b.y - 0.7).abs() < 1e-6, + "the guide is untouched: {a:?}–{b:?}" + ); + assert_eq!(h.app.session.undo.len(), undo, "a click is not an edit"); +} + +/// A new guide starts at the press point itself: a press, one far jump, a release — the guide +/// spans exactly those two points (it used to start where the drag threshold was crossed, so the +/// anchor was already at the far end). +#[test] +fn a_new_guide_starts_at_the_press_point() { + let mut h = guided(); + assert_eq!(guides_len(&h), 2); + let img = h.app.image_rect.expect("the photo on screen"); + let p1 = egui::pos2(img.left() + 0.35 * img.width(), img.top() + 0.55 * img.height()); + let p2 = egui::pos2(img.left() + 0.6 * img.width(), img.top() + 0.55 * img.height()); + let r = h.request( + "ui.pointer", + json!({"events": [{"kind": "down", "x": 0.35, "y": 0.55}, {"kind": "drag", "x": 0.6, "y": 0.55}, {"kind": "up", "x": 0.6, "y": 0.55}]}), + T, + ); + assert_eq!(r["ok"], true, "{r}"); + h.settle(SETTLE); + assert_eq!(guides_len(&h), 3, "the drawn guide is added"); + assert_eq!(h.app.ui.selected_guide, None, "drawing a new guide leaves nothing selected"); + let rect = guide_rect(&h, 2); + assert!(rect.left_top().distance(p1) < 2.0, "the guide starts at the press point: {rect:?} vs {p1:?}"); + assert!(rect.right_top().distance(p2) < 2.0, "and ends at the release point: {rect:?} vs {p2:?}"); +} From aa0f9b92a7c0627c4203785e92da925e6fc3b1d8 Mon Sep 17 00:00:00 2001 From: mahmatias Date: Mon, 19 Oct 2026 15:58:02 -0300 Subject: [PATCH 3/9] docs(parity): LR-GUIDED-SELECT/DELETE rows land as done (press semantics) --- docs/parity-checklist.md | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/docs/parity-checklist.md b/docs/parity-checklist.md index 02f5bf08f..84723da69 100644 --- a/docs/parity-checklist.md +++ b/docs/parity-checklist.md @@ -33,7 +33,7 @@ cited here still exists. Formerly docs/parity.md. | E. Metadata (META) | 5 | 1 | 0 | 0 | 2/2 (100%) | 2/2 (100%) | | F. Edit panel — global adjustments (EDIT) | 44 | 2 | 3 | 1 | 28/28 (100%) | 13/14 (93%) | | G. Profiles (PROF) | 7 | 1 | 3 | 0 | 3/4 (75%) | 3/3 (100%) | -| H. Crop & rotate (CROP) | 9 | 0 | 0 | 1 | 6/6 (100%) | 3/3 (100%) | +| H. Crop & rotate (CROP) | 11 | 0 | 0 | 1 | 8/8 (100%) | 3/3 (100%) | | I. Remove / healing (REM) | 8 | 1 | 2 | 2 | 4/4 (100%) | 2/3 (67%) | | J. Red eye (EYE) | 2 | 0 | 0 | 0 | — | 1/1 (100%) | | K. Masking (MASK) | 15 | 5 | 5 | 0 | 7/8 (88%) | 7/7 (100%) | @@ -53,9 +53,9 @@ cited here still exists. Formerly docs/parity.md. | Y. Menus | 84 | 0 | 4 | 8 | 47/47 (100%) | 23/23 (100%) | | Z. Keyboard shortcuts (desktop) | 78 | 4 | 2 | 1 | 49/52 (94%) | 24/26 (92%) | | Lightroom Classic extras | 40 | 23 | 23 | 8 | — | 24/26 (92%) | -| **Total** | 419 | 61 | 59 | 35 | 194/201 (97%) | 148/161 (92%) | +| **Total** | 421 | 61 | 59 | 35 | 196/203 (97%) | 148/161 (92%) | -Weighted completion (✅ = 1, 🟡 = ½, 🚫 left out): **83.4%** of 539 in-scope rows — P0 98.3% of 201 · P1 95.0% of 161 · P2 55.7% of 176. +Weighted completion (✅ = 1, 🟡 = ½, 🚫 left out): **83.5%** of 541 in-scope rows — P0 98.3% of 203 · P1 95.0% of 161 · P2 55.7% of 176. ## Top gaps and shortcut notes @@ -254,6 +254,8 @@ differences and missing bindings are in [ui-parity.md](ui-parity.md#shortcuts). | LR-CROP-STRAIGHTEN | Straighten tool | P0 | ✅ | `cmd:crop.straighten`, `cmd:crop.autoStraighten`, `crates/ui-egui/src/panels/detail.rs` (`rotate_cursor`, `angle_readout`), `crates/ui-egui/src/panels/right.rs` (Angle field), `crates/ui-egui/src/tests_crop_rotate.rs` | Straighten Tool button: drag along a horizon/vertical; double-click or Auto levels automatically. Dragging outside the crop box rotates it: there the pointer is a curved double arrow that bulges away from the side of the box it is on, diagonally off a corner and following a rotated box (`rotate_cursor_turn`, tested by `rotate_cursor_faces_away_from_the_crop_box`) (with ⌘ held, the straighten crosshair), and while rotating the angle shows next to it, kept on the canvas (issue #534); an exact angle goes in the Angle field under Straighten (`cropAngleField`: applies on Return or leaving it, Esc keeps the old angle, takes a decimal comma; a drag on it is one undo step) or by clicking the Straighten value (its hover says so). Every way of setting the angle (rotate handle, Straighten slider, Angle field, `crop.straighten`, `crop.set {angle}`, and the generic `develop.set` / `develop.adjust` / `develop.quickAdjust` / `develop.resetControl` on `crop.angle`, as MCP, the control channel and `lightcraft-cli render --set` use them) keeps the crop if it still fits, else shrinks it about its own centre, keeping its aspect, until it fits; the full photo (or a crop an earlier turn shrank to the largest of its aspect) stays the largest, so it grows back towards 0° (issue #1388), and no other crop ever grows (issue #1794: a crop dragged in from a corner turned into nearly the whole photo) (`CropGeometry::with_angle`, `crates/engine/src/tests_crop_angle.rs`; before issue #742 the generic path left the box, and the export, past the photo with smeared edge pixels in the corners). The dimming outside the box is the photo minus the (rotated) box, drawn as one mesh (`outside_crop_mesh`), so rotating never dims wedges inside the box or doubles them outside (issue #731) | | LR-CROP-AUTO | Auto straighten | P1 | ✅ | `cmd:crop.autoStraighten` | crop-angle leveling from detected horizon/plumb lines (consensus required) | | LR-CROP-ANGLE | Angle slider | P0 | ✅ | `ctl:crop.angle`, `crates/ui-egui/src/panels/right.rs` | the Straighten slider; an exact angle can also be typed in the Angle field under it (see LR-CROP-STRAIGHTEN); setting `crop.angle` through `develop.set` fits the crop inside the photo as Straighten does (issue #742) | +| LR-GUIDED-SELECT | Guide selection in Guided Upright mode | P0 | ✅ | `cmd:tool.guidedUpright`, `cmd:geometry.guides`, `crates/ui-egui/src/panels/detail.rs`, `crates/ui-egui/src/state.rs` (`selected_guide`), `crates/ui-egui/src/tests_guided_guides.rs` | a press near a guide's line selects it (`selected_guide = Some((photo id, index))`; another press on it deselects), a press on an empty spot starts a new guide at that point, and an endpoint drags to reshape the guide (`Gesture::GuideEndpoint`, one undo step; clicking an endpoint only selects); tested in `tests_guided_guides` 8/8 | +| LR-GUIDED-DELETE | Delete removes the selected guide alone | P0 | ✅ | `cmd:photo.delete`, `crates/ui-egui/src/shortcuts.rs`, `crates/ui-egui/src/tests_guided_guides.rs` | with the Guided Upright tool open (`RightPanel::Crop && tool == "guidedUpright"`), Delete / Backspace removes only the selected guide and clears the selection; with nothing selected it removes nothing (the photo itself is never deleted while the tool is open); tested in `tests_guided_guides` | | LR-CROP-ROTATE90 | Rotate 90° | P0 | ✅ | `cmd:photo.rotateLeft`, `cmd:photo.rotateRight` | | | LR-CROP-FLIP | Flip | P0 | ✅ | `cmd:photo.flipHorizontal`, `cmd:photo.flipVertical`, `crates/pipeline/src/geometry.rs` | the crop tool's uncropped view shows the flips too, with the crop box and its dimming mirrored to match | | LR-CROP-OVERLAY | Crop overlays | P1 | ✅ | `cmd:view.cropOverlay`, `cmd:view.cropOverlayOrientation`, `crates/ui-egui/src/panels/crop_overlay.rs` | thirds, grid, golden ratio, diagonal, triangle, golden spiral (mirrored with ⇧O while cropping); no aspect-ratio overlays | @@ -896,3 +898,4 @@ Latin stays upright. Advanced Japanese composition remains open. Regression cove | 2026-10-14 | patch | LRC-LIB-CATALOG-IMPORT: reconciled with upstream phased progress, cancellation kinds, WAL-only import, translations and current progress tracking. Status and summary counts unchanged. | | 2026-10-13 | minor | KEYC-COMPARE: `=` / `-` (also `+`) increase / decrease the grid thumbnail size (`view.thumbSizeUp` / `view.thumbSizeDown`, View menu), grids only; status unchanged (🟡) | | 2026-10-16 | patch | LR-META-XMP: a TIFF or JPEG imported beside a raw of its stem reads its own `.xmp` and takes nothing from the raw's `IMG_0001.xmp`, whether it arrives with the raw or after it (a stem sidecar belongs to one file: a raw first, then the first by name); status unchanged (✅) | +| 2026-10-19 | minor | LR-GUIDED-SELECT and LR-GUIDED-DELETE: Guided Upright guides select on the press near their line (another press deselects), start a new guide where the press lands (not where egui's drag threshold was crossed), and an endpoint drags to reshape the guide (`Gesture::GuideEndpoint`, one undo step); Delete / Backspace with the tool open removes only the selected guide; new rows land as ✅ (`tests_guided_guides` 8/8) | From 331e0d2103b999038afe1c37e50e528515216e16 Mon Sep 17 00:00:00 2001 From: mahmatias Date: Mon, 19 Oct 2026 15:58:39 -0300 Subject: [PATCH 4/9] style: fmt (blank line before point_segment_dist) --- crates/ui-egui/src/panels/detail.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/crates/ui-egui/src/panels/detail.rs b/crates/ui-egui/src/panels/detail.rs index 6a6c74e5b..e5d362423 100644 --- a/crates/ui-egui/src/panels/detail.rs +++ b/crates/ui-egui/src/panels/detail.rs @@ -1962,7 +1962,6 @@ fn to_straight(n: Point, angle: f64, frame: &Frame) -> Point { Point::new(r.x / w, r.y / h) } - /// How far `q` is from the segment `a`–`b` (screen pixels): the click that selects a guide. fn point_segment_dist(q: Pos2, a: Pos2, b: Pos2) -> f32 { let ab = b - a; From ca26c4046a343e832b8c9fdb22e1effeada9655b Mon Sep 17 00:00:00 2001 From: mahmatias Date: Mon, 19 Oct 2026 17:11:57 -0300 Subject: [PATCH 5/9] fix(guided): a stale selected index does not fire an empty guides edit; docs: un-mojibake the dashes --- crates/ui-egui/src/shortcuts.rs | 1 + crates/ui-egui/src/tests_guided_guides.rs | 10 +++++----- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/crates/ui-egui/src/shortcuts.rs b/crates/ui-egui/src/shortcuts.rs index fb483f61c..109ff7023 100644 --- a/crates/ui-egui/src/shortcuts.rs +++ b/crates/ui-egui/src/shortcuts.rs @@ -564,6 +564,7 @@ pub fn handle(app: &mut LightcraftApp, ctx: &egui::Context) { if let (Some((photo, i)), Some(active)) = (app.ui.selected_guide, app.session.active()) { if photo == active.0 && let Some(d) = app.session.develop_of(active) + && i < d.geometry.guides.len() { let guides: Vec<[f64; 4]> = d.geometry.guides.iter().enumerate().filter(|(k, _)| *k != i).map(|(_, (a, b))| [a.x, a.y, b.x, b.y]).collect(); diff --git a/crates/ui-egui/src/tests_guided_guides.rs b/crates/ui-egui/src/tests_guided_guides.rs index 6d6b8df0d..89fe6fbbe 100644 --- a/crates/ui-egui/src/tests_guided_guides.rs +++ b/crates/ui-egui/src/tests_guided_guides.rs @@ -73,7 +73,7 @@ fn delete_removes_the_selected_guide() { let (a, b) = d.develop.geometry.guides[0]; assert!( (a.x - 0.2).abs() < 1e-6 && (a.y - 0.3).abs() < 1e-6 && (b.x - 0.8).abs() < 1e-6 && (b.y - 0.3).abs() < 1e-6, - "the first guide survived: {a:?}–{b:?}" + "the first guide survived: {a:?}–{b:?}" ); } @@ -128,8 +128,8 @@ fn drawing_a_new_guide_clears_the_selection() { assert_eq!(h.app.ui.selected_guide, None, "and the click's selection is gone"); } -/// A press on an endpoint drags it: the guide follows the pointer — the other end and the other -/// guide untouched — the moved guide ends up selected, and nothing is added (one edit, on release). +/// A press on an endpoint drags it: the guide follows the pointer — the other end and the other +/// guide untouched — the moved guide ends up selected, and nothing is added (one edit, on release). #[test] fn dragging_a_guide_endpoint_moves_it() { let mut h = guided(); @@ -178,12 +178,12 @@ fn clicking_an_endpoint_selects_it_without_changing_it() { let (a, b) = d.develop.geometry.guides[1]; assert!( (a.x - 0.2).abs() < 1e-6 && (a.y - 0.7).abs() < 1e-6 && (b.x - 0.8).abs() < 1e-6 && (b.y - 0.7).abs() < 1e-6, - "the guide is untouched: {a:?}–{b:?}" + "the guide is untouched: {a:?}–{b:?}" ); assert_eq!(h.app.session.undo.len(), undo, "a click is not an edit"); } -/// A new guide starts at the press point itself: a press, one far jump, a release — the guide +/// A new guide starts at the press point itself: a press, one far jump, a release — the guide /// spans exactly those two points (it used to start where the drag threshold was crossed, so the /// anchor was already at the far end). #[test] From b1c607d7aca6d07f8b3bea5ded91d586f5d36c9c Mon Sep 17 00:00:00 2001 From: mahmatias Date: Mon, 19 Oct 2026 22:08:52 -0300 Subject: [PATCH 6/9] fix(guided): the primary button and the canvas only; the press point is the gesture's own - a secondary/middle press or one on the navigator panned the photo but also selected or drew a guide: the gestures take the primary button and skip the navigator's rect - egui forgets the press origin by the release, so the endpoint drag compared against the guide's old position: a press that landed within the grab radius and never moved wrote a no-op guide; the gesture now carries the press point itself - a selection left behind by a Delete past the last guide clears with the photo/tool check in crop_overlay --- crates/ui-egui/src/panels/detail.rs | 28 +++++++++++------- crates/ui-egui/src/tests_guided_guides.rs | 35 +++++++++++++++++++++++ 2 files changed, 53 insertions(+), 10 deletions(-) diff --git a/crates/ui-egui/src/panels/detail.rs b/crates/ui-egui/src/panels/detail.rs index e5d362423..abd123476 100644 --- a/crates/ui-egui/src/panels/detail.rs +++ b/crates/ui-egui/src/panels/detail.rs @@ -60,10 +60,12 @@ pub enum Gesture { Guide { a: Point, }, - /// Guided Upright: dragging the endpoint `end` (0 = `guide.0`, 1 = `guide.1`) of guide `i`. + /// Guided Upright: dragging the endpoint `end` (0 = `guide.0`, 1 = `guide.1`) of guide `i`, + /// from where the pointer went down (`from` — egui forgets the press origin at the release). GuideEndpoint { i: usize, end: usize, + from: Pos2, }, /// Red Eye: an ellipse being dragged from corner `a` (normalized). Eye { @@ -1540,8 +1542,9 @@ fn general_interaction( // ------------------------------------------------------------------------ crop fn crop_overlay(app: &mut LightcraftApp, ui: &mut egui::Ui, resp: &egui::Response, map: &CanvasMap, frame: &Frame, d: &DevelopSettings, id: PhotoId) { - // the selected guide belongs to this photo and to the Guided Upright tool - if app.ui.tool != "guidedUpright" || app.ui.selected_guide.is_some_and(|(photo, _)| photo != id.0) { + // the selected guide belongs to this photo and to the Guided Upright tool, and to a guide + // that still exists (a Delete after the last one leaves the selection behind) + if app.ui.tool != "guidedUpright" || app.ui.selected_guide.is_some_and(|(photo, i)| photo != id.0 || i >= d.geometry.guides.len()) { app.ui.selected_guide = None; } if app.ui.tool == "guidedUpright" { @@ -1873,10 +1876,13 @@ fn guided_overlay( // A press starts what it means, right away (the pointer's press point, not where a drag // threshold would have been crossed): on an endpoint it drags that endpoint (the guide is // selected when it lands), on a line it selects the guide (a press on the selected one - // deselects), anywhere else it starts a new guide from the press point. - if ui.input(|i| i.pointer.any_pressed()) + // deselects), anywhere else it starts a new guide from the press point. The primary button + // only, and never on the navigator — it pans the photo, it draws no guide. + let nav = app.widgets.iter().rev().find(|(w, _)| w == "canvas:navigator").map(|(_, r)| *r); + if ui.input(|i| i.pointer.primary_pressed()) && let Some(q) = ui.input(|i| i.pointer.press_origin()).or(resp.interact_pointer_pos()) && resp.rect.contains(q) + && !nav.is_some_and(|n| n.expand(5.0).contains(q)) { let endpoint = d.geometry.guides.iter().enumerate().find_map(|(i, guide)| { let (a, b) = on_screen(guide); @@ -1893,7 +1899,7 @@ fn guided_overlay( point_segment_dist(q, a, b) <= 6.0 }); match (endpoint, line) { - (Some((i, end)), _) => app.gesture = Some(Gesture::GuideEndpoint { i, end }), + (Some((i, end)), _) => app.gesture = Some(Gesture::GuideEndpoint { i, end, from: q }), (None, Some(i)) => app.ui.selected_guide = if selected == Some(i) { None } else { Some((id.0, i)) }, // drawing a new guide leaves nothing selected (None, None) => { @@ -1907,7 +1913,7 @@ fn guided_overlay( draw(map.screen(*a), q, false); } // the endpoint being dragged: the guide follows the pointer, bright - if let (Some(Gesture::GuideEndpoint { i, end }), Some(q)) = (&app.gesture, q) + if let (Some(Gesture::GuideEndpoint { i, end, .. }), Some(q)) = (&app.gesture, q) && let Some(guide) = d.geometry.guides.get(*i) { let fixed = if *end == 0 { guide.1 } else { guide.0 }; @@ -1917,14 +1923,16 @@ fn guided_overlay( match app.gesture.take() { // the endpoint lands: the guide stays (one undo step) and is selected. A press // without a real drag only selects — no no-op undo entry for a click. - Some(Gesture::GuideEndpoint { i, end }) => { + Some(Gesture::GuideEndpoint { i, end, from }) => { app.ui.selected_guide = Some((id.0, i)); if let (Some(q), Some(guide)) = (q, d.geometry.guides.get(i)) { - let (old_t, fixed_t) = if end == 0 { (guide.0, guide.1) } else { (guide.1, guide.0) }; + let fixed_t = if end == 0 { guide.1 } else { guide.0 }; let new_t = frame.transformed_to_corrected(map.norm(q)); let (a, b) = if end == 0 { (new_t, fixed_t) } else { (fixed_t, new_t) }; let (na, nb) = (frame.corrected_to_transformed(a), frame.corrected_to_transformed(b)); - let moved = map.screen(frame.corrected_to_transformed(old_t)).distance(q) > 2.0; + // a press that only selected (it landed within the endpoint's grab radius + // and never moved) writes no guide: compare against where it went down + let moved = from.distance(q) > 2.0; if moved && na.dist(nb) > 0.02 { let all: Vec<[f64; 4]> = d .geometry diff --git a/crates/ui-egui/src/tests_guided_guides.rs b/crates/ui-egui/src/tests_guided_guides.rs index 89fe6fbbe..dea1fcd93 100644 --- a/crates/ui-egui/src/tests_guided_guides.rs +++ b/crates/ui-egui/src/tests_guided_guides.rs @@ -206,3 +206,38 @@ fn a_new_guide_starts_at_the_press_point() { assert!(rect.left_top().distance(p1) < 2.0, "the guide starts at the press point: {rect:?} vs {p1:?}"); assert!(rect.right_top().distance(p2) < 2.0, "and ends at the release point: {rect:?} vs {p2:?}"); } + +/// The right button is the pan / context menu's, never the guides': on a guide it selects +/// nothing and draws nothing. +#[test] +fn a_right_click_on_a_guide_selects_nothing() { + let mut h = guided(); + let r = h.request("ui.click", json!({"x": guide_at(&h, 0).x, "y": guide_at(&h, 0).y, "button": "right"}), T); + assert_eq!(r["ok"], true, "{r}"); + h.settle(SETTLE); + assert_eq!(h.app.ui.selected_guide, None, "the right button selects nothing"); + assert_eq!(guides_len(&h), 2, "and draws nothing"); +} + +/// The navigator pans the zoomed photo: a press on it never reaches the guides — it neither +/// selects one under it nor starts drawing a new guide from it. +#[test] +fn a_press_on_the_navigator_draws_no_guide() { + let mut h = guided(); + let r = h.request("engine.execute", json!({"command": "view.zoom100"}), T); + assert_eq!(r["ok"], true, "{r}"); + h.settle(SETTLE); + let nav = h.app.widgets.iter().rev().find(|(w, _)| *w == "canvas:navigator").map(|(_, r)| *r).expect("navigator shown when zoomed"); + let img = h.app.image_rect.expect("the photo on screen"); + let c = nav.center(); + let (nx, ny) = ((c.x - img.left()) / img.width(), (c.y - img.top()) / img.height()); + let r = h.request( + "ui.pointer", + json!({"events": [{"kind": "down", "x": nx, "y": ny}, {"kind": "drag", "x": nx + 0.05, "y": ny}, {"kind": "up", "x": nx + 0.05, "y": ny}]}), + T, + ); + assert_eq!(r["ok"], true, "{r}"); + h.settle(SETTLE); + assert_eq!(guides_len(&h), 2, "the press on the navigator drew no guide"); + assert_eq!(h.app.ui.selected_guide, None, "and selected nothing"); +} From f2e97d39fe36ec751e67d240b76f1a08467b2a79 Mon Sep 17 00:00:00 2001 From: mahmatias Date: Wed, 21 Oct 2026 00:50:20 -0300 Subject: [PATCH 7/9] fix(guided): the loupe's press is layer-aware, not just inside its rect MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `primary_pressed` plus `resp.rect.contains` reads the raw pointer, so anything floating above the photo — the context menu, a dropdown, a dialog — also started a guide gesture under it. The gate now asks `resp.contains_pointer()`, which the hit test keeps honest about covering widgets; the navigator check stays. Two tests press on a menu over a line and over an endpoint: nothing is selected, moved or drawn. --- crates/ui-egui/src/panels/detail.rs | 6 ++- crates/ui-egui/src/tests_guided_guides.rs | 65 +++++++++++++++++++++++ 2 files changed, 70 insertions(+), 1 deletion(-) diff --git a/crates/ui-egui/src/panels/detail.rs b/crates/ui-egui/src/panels/detail.rs index abd123476..63f17dec2 100644 --- a/crates/ui-egui/src/panels/detail.rs +++ b/crates/ui-egui/src/panels/detail.rs @@ -1877,10 +1877,14 @@ fn guided_overlay( // threshold would have been crossed): on an endpoint it drags that endpoint (the guide is // selected when it lands), on a line it selects the guide (a press on the selected one // deselects), anywhere else it starts a new guide from the press point. The primary button - // only, and never on the navigator — it pans the photo, it draws no guide. + // only, never on the navigator — it pans the photo, it draws no guide — and only where the + // loupe itself takes the press: `contains_pointer` is layer-aware (the raw pointer is not), + // so anything floating above the photo — a context menu, a dropdown, a dialog — owns the + // press and the guide under it stays untouched. let nav = app.widgets.iter().rev().find(|(w, _)| w == "canvas:navigator").map(|(_, r)| *r); if ui.input(|i| i.pointer.primary_pressed()) && let Some(q) = ui.input(|i| i.pointer.press_origin()).or(resp.interact_pointer_pos()) + && resp.contains_pointer() && resp.rect.contains(q) && !nav.is_some_and(|n| n.expand(5.0).contains(q)) { diff --git a/crates/ui-egui/src/tests_guided_guides.rs b/crates/ui-egui/src/tests_guided_guides.rs index dea1fcd93..7cdb8bebd 100644 --- a/crates/ui-egui/src/tests_guided_guides.rs +++ b/crates/ui-egui/src/tests_guided_guides.rs @@ -241,3 +241,68 @@ fn a_press_on_the_navigator_draws_no_guide() { assert_eq!(guides_len(&h), 2, "the press on the navigator drew no guide"); assert_eq!(h.app.ui.selected_guide, None, "and selected nothing"); } + +/// The context menu (anything floating above the photo) owns its own press: a left press on the +/// menu that sits over a guide line selects nothing under it and starts no guide gesture — the +/// canvas has to actually receive the press, not merely lie under the pointer. +#[test] +fn a_press_on_a_menu_over_a_guide_selects_nothing() { + let mut h = guided(); + let id = h.app.session.active().expect("a photo"); + let undo = h.app.session.undo.len(); + let line = guide_at(&h, 0); + // right-click 30px up-left of that point: the context menu opens with its top-left corner + // at the pointer (gap 0), so its frame covers the guide from 30px inside every edge + let r = h.request("ui.click", json!({"x": line.x - 30.0, "y": line.y - 30.0, "button": "right"}), T); + assert_eq!(r["ok"], true, "{r}"); + h.settle(SETTLE); + assert!(h.view.ctx.any_popup_open(), "the context menu is open over the photo"); + let img = h.app.image_rect.expect("the photo on screen"); + let (lx, ly) = ((line.x - img.left()) / img.width(), (line.y - img.top()) / img.height()); + let r = h.request("ui.pointer", json!({"events": [{"kind": "down", "x": lx, "y": ly}, {"kind": "up", "x": lx, "y": ly}]}), T); + assert_eq!(r["ok"], true, "{r}"); + h.settle(SETTLE); + assert_eq!(h.app.ui.selected_guide, None, "the press under the menu selects nothing"); + assert!(h.app.gesture.is_none(), "and starts no gesture"); + assert_eq!(guides_len(&h), 2, "and draws nothing"); + assert_eq!(h.app.session.undo.len(), undo, "no edit was written"); + let d = h.app.session.catalog.photo(id).expect("the photo"); + let (a, b) = d.develop.geometry.guides[0]; + assert!( + (a.x - 0.2).abs() < 1e-6 && (a.y - 0.3).abs() < 1e-6 && (b.x - 0.8).abs() < 1e-6 && (b.y - 0.3).abs() < 1e-6, + "the guide is untouched: {a:?}–{b:?}" + ); +} + +/// Same for an endpoint under the menu: a press-drag on it moves nothing and selects nothing — +/// the endpoint-drag gesture never starts because the menu, not the canvas, took the press. +#[test] +fn a_press_on_a_menu_over_an_endpoint_moves_nothing() { + let mut h = guided(); + let id = h.app.session.active().expect("a photo"); + let undo = h.app.session.undo.len(); + let end = guide_rect(&h, 0).left_top(); + let r = h.request("ui.click", json!({"x": end.x - 30.0, "y": end.y - 30.0, "button": "right"}), T); + assert_eq!(r["ok"], true, "{r}"); + h.settle(SETTLE); + assert!(h.view.ctx.any_popup_open(), "the context menu is open over the photo"); + let img = h.app.image_rect.expect("the photo on screen"); + let (ex, ey) = ((end.x - img.left()) / img.width(), (end.y - img.top()) / img.height()); + let r = h.request( + "ui.pointer", + json!({"events": [{"kind": "down", "x": ex, "y": ey}, {"kind": "drag", "x": ex + 0.05, "y": ey + 0.08}, {"kind": "up", "x": ex + 0.05, "y": ey + 0.08}]}), + T, + ); + assert_eq!(r["ok"], true, "{r}"); + h.settle(SETTLE); + assert_eq!(h.app.ui.selected_guide, None, "the endpoint's press under the menu selects nothing"); + assert!(h.app.gesture.is_none(), "and starts no gesture"); + assert_eq!(guides_len(&h), 2, "and draws nothing"); + assert_eq!(h.app.session.undo.len(), undo, "no edit was written"); + let d = h.app.session.catalog.photo(id).expect("the photo"); + let (a, b) = d.develop.geometry.guides[0]; + assert!( + (a.x - 0.2).abs() < 1e-6 && (a.y - 0.3).abs() < 1e-6 && (b.x - 0.8).abs() < 1e-6 && (b.y - 0.3).abs() < 1e-6, + "the endpoint under the menu did not move: {a:?}–{b:?}" + ); +} From c19265dee05e2657f1ea6fa18be95f48763d7ccc Mon Sep 17 00:00:00 2001 From: mahmatias Date: Wed, 21 Oct 2026 08:00:42 -0300 Subject: [PATCH 8/9] fix(guided): the press point, not the current pointer, owns the gesture `contains_pointer` reads where the pointer is NOW; a slow frame can deliver the press on a menu and the move off it in the same pass, and the gate would then pass with the saved press origin under the menu. `layer_id_at(press_origin) == resp.layer_id` asks which layer owns the press itself: the menu's area (or any modal) wins there and the guide under it stays untouched. The navigator check stays for its own widget. --- crates/ui-egui/src/panels/detail.rs | 9 ++++-- crates/ui-egui/src/tests_guided_guides.rs | 34 +++++++++++++++++++++++ 2 files changed, 40 insertions(+), 3 deletions(-) diff --git a/crates/ui-egui/src/panels/detail.rs b/crates/ui-egui/src/panels/detail.rs index 63f17dec2..9c0215bd8 100644 --- a/crates/ui-egui/src/panels/detail.rs +++ b/crates/ui-egui/src/panels/detail.rs @@ -1878,13 +1878,16 @@ fn guided_overlay( // selected when it lands), on a line it selects the guide (a press on the selected one // deselects), anywhere else it starts a new guide from the press point. The primary button // only, never on the navigator — it pans the photo, it draws no guide — and only where the - // loupe itself takes the press: `contains_pointer` is layer-aware (the raw pointer is not), - // so anything floating above the photo — a context menu, a dropdown, a dialog — owns the - // press and the guide under it stays untouched. + // loupe itself takes the press: `contains_pointer` is layer-aware over the current pointer + // (the raw pointer is not), and `layer_id_at` asks the same question at the press point — + // a menu can receive the press and the pointer can leave it within the same frame, so the + // press itself must belong to the loupe's layer. Anything floating above the photo — a + // context menu, a dropdown, a dialog — owns its own press; the guide under it stays put. let nav = app.widgets.iter().rev().find(|(w, _)| w == "canvas:navigator").map(|(_, r)| *r); if ui.input(|i| i.pointer.primary_pressed()) && let Some(q) = ui.input(|i| i.pointer.press_origin()).or(resp.interact_pointer_pos()) && resp.contains_pointer() + && ui.ctx().layer_id_at(q) == Some(resp.layer_id) && resp.rect.contains(q) && !nav.is_some_and(|n| n.expand(5.0).contains(q)) { diff --git a/crates/ui-egui/src/tests_guided_guides.rs b/crates/ui-egui/src/tests_guided_guides.rs index 7cdb8bebd..84e9def7e 100644 --- a/crates/ui-egui/src/tests_guided_guides.rs +++ b/crates/ui-egui/src/tests_guided_guides.rs @@ -306,3 +306,37 @@ fn a_press_on_a_menu_over_an_endpoint_moves_nothing() { "the endpoint under the menu did not move: {a:?}–{b:?}" ); } + +/// The press can leave the menu within the same frame it started (one batch: down on the menu, +/// pointer onto bare photo, release): containment of the *current* pointer would pass then, but +/// the press belongs to the menu — the press point decides, so no guide is drawn from it. +#[test] +fn a_press_that_leaves_the_menu_in_one_frame_draws_nothing() { + let mut h = guided(); + let undo = h.app.session.undo.len(); + let end = guide_rect(&h, 0).left_top(); + // right-click up-left of the endpoint: the menu opens with its top-left at the pointer + let r = h.request("ui.click", json!({"x": end.x - 30.0, "y": end.y - 30.0, "button": "right"}), T); + assert_eq!(r["ok"], true, "{r}"); + h.settle(SETTLE); + assert!(h.view.ctx.any_popup_open(), "the context menu is open over the photo"); + // one batch: go down 1px inside the menu, then straight off it to bare photo, far below + // both guides, and release there — the current pointer never sees the menu again + let img = h.app.image_rect.expect("the photo on screen"); + let menu = end - egui::vec2(29.0, 29.0); + let bare = egui::pos2(img.center().x, img.top() + 0.95 * img.height()); + let rel = |p: egui::Pos2| (((p.x - img.left()) / img.width()) as f64, ((p.y - img.top()) / img.height()) as f64); + let (mx, my) = rel(menu); + let (bx, by) = rel(bare); + let r = h.request( + "ui.pointer", + json!({"events": [{"kind": "down", "x": mx, "y": my}, {"kind": "drag", "x": bx, "y": by}, {"kind": "up", "x": bx, "y": by}]}), + T, + ); + assert_eq!(r["ok"], true, "{r}"); + h.settle(SETTLE); + assert_eq!(guides_len(&h), 2, "the press that left the menu drew no guide"); + assert_eq!(h.app.ui.selected_guide, None, "and selected nothing"); + assert!(h.app.gesture.is_none(), "and leaves no gesture behind"); + assert_eq!(h.app.session.undo.len(), undo, "no edit was written"); +} From 77b020433148bf64e702a9b4529b086d966f8774 Mon Sep 17 00:00:00 2001 From: mahmatias Date: Wed, 21 Oct 2026 08:30:32 -0300 Subject: [PATCH 9/9] fix(guided): the press position survives press+release in one frame MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit egui clears `press_origin` on the release, so a slow frame that swallows press, move and release into one batch reports `primary_pressed` with no origin: the gesture fell back to the release position and a real drag wrote nothing (its length check saw a zero-length stub). The gate now reads the primary press back from the frame's own events before that fallback — the press decides what it means, wherever the batch ended. --- crates/ui-egui/src/panels/detail.rs | 14 ++++++++++++- crates/ui-egui/src/tests_guided_guides.rs | 25 +++++++++++++++++++++++ 2 files changed, 38 insertions(+), 1 deletion(-) diff --git a/crates/ui-egui/src/panels/detail.rs b/crates/ui-egui/src/panels/detail.rs index 9c0215bd8..f2fe8c92d 100644 --- a/crates/ui-egui/src/panels/detail.rs +++ b/crates/ui-egui/src/panels/detail.rs @@ -1883,9 +1883,21 @@ fn guided_overlay( // a menu can receive the press and the pointer can leave it within the same frame, so the // press itself must belong to the loupe's layer. Anything floating above the photo — a // context menu, a dropdown, a dialog — owns its own press; the guide under it stays put. + // A slow frame can also swallow the whole press-move-release into one batch, and egui + // clears `press_origin` on the release: then the press position is read back from the + // frame's own events, so the gesture still starts where the pointer went down. let nav = app.widgets.iter().rev().find(|(w, _)| w == "canvas:navigator").map(|(_, r)| *r); if ui.input(|i| i.pointer.primary_pressed()) - && let Some(q) = ui.input(|i| i.pointer.press_origin()).or(resp.interact_pointer_pos()) + && let Some(q) = ui + .input(|i| { + i.pointer.press_origin().or_else(|| { + i.events.iter().rev().find_map(|e| match e { + egui::Event::PointerButton { pos, button: egui::PointerButton::Primary, pressed: true, .. } => Some(*pos), + _ => None, + }) + }) + }) + .or(resp.interact_pointer_pos()) && resp.contains_pointer() && ui.ctx().layer_id_at(q) == Some(resp.layer_id) && resp.rect.contains(q) diff --git a/crates/ui-egui/src/tests_guided_guides.rs b/crates/ui-egui/src/tests_guided_guides.rs index 84e9def7e..d59698350 100644 --- a/crates/ui-egui/src/tests_guided_guides.rs +++ b/crates/ui-egui/src/tests_guided_guides.rs @@ -207,6 +207,31 @@ fn a_new_guide_starts_at_the_press_point() { assert!(rect.right_top().distance(p2) < 2.0, "and ends at the release point: {rect:?} vs {p2:?}"); } +/// A slow frame can swallow press, move and release into a single batch: egui then reports +/// `primary_pressed` with `press_origin` already cleared by the release, and the gesture must +/// still start at the press point — the guide spans the drag, not a zero-length stub at the +/// release. The control channel spreads pointer events one per frame, so the batch is pushed +/// raw into one frame here. +#[test] +fn a_drag_whose_press_and_release_share_one_frame_still_draws() { + let mut h = guided(); + assert_eq!(guides_len(&h), 2); + let undo = h.app.session.undo.len(); + let img = h.app.image_rect.expect("the photo on screen"); + let p1 = egui::pos2(img.left() + 0.35 * img.width(), img.top() + 0.55 * img.height()); + let p2 = egui::pos2(img.left() + 0.6 * img.width(), img.top() + 0.55 * img.height()); + h.events.push(egui::Event::PointerMoved(p1)); + h.events.push(egui::Event::PointerButton { pos: p1, button: egui::PointerButton::Primary, pressed: true, modifiers: egui::Modifiers::NONE }); + h.events.push(egui::Event::PointerMoved(p2)); + h.events.push(egui::Event::PointerButton { pos: p2, button: egui::PointerButton::Primary, pressed: false, modifiers: egui::Modifiers::NONE }); + h.settle(SETTLE); + assert_eq!(guides_len(&h), 3, "the drag that shared one frame still draws its guide"); + assert_eq!(h.app.session.undo.len(), undo + 1, "one edit was written"); + let rect = guide_rect(&h, 2); + assert!(rect.left_top().distance(p1) < 2.0, "the guide starts at the press point: {rect:?} vs {p1:?}"); + assert!(rect.right_top().distance(p2) < 2.0, "and ends at the release point: {rect:?} vs {p2:?}"); +} + /// The right button is the pan / context menu's, never the guides': on a guide it selects /// nothing and draws nothing. #[test]