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 9eae0cd77..cb757a6f6 100644 --- a/crates/ui-egui/src/panels/detail.rs +++ b/crates/ui-egui/src/panels/detail.rs @@ -60,6 +60,13 @@ pub enum Gesture { Guide { a: Point, }, + /// 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 { a: Point, @@ -1535,8 +1542,13 @@ 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, 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" { - 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 +1703,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,39 +1841,142 @@ 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); - if resp.drag_started() - && 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. 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 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. + // 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_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) + && !nav.is_some_and(|n| n.expand(5.0).contains(q)) { - app.gesture = Some(Gesture::Guide { a: map.norm(q) }); + 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 + }); + match (endpoint, line) { + (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) => { + app.ui.selected_guide = None; + 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); + 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, from }) => { + app.ui.selected_guide = Some((id.0, i)); + if let (Some(q), Some(guide)) = (q, d.geometry.guides.get(i)) { + 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)); + // 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 + .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, } } } @@ -1875,6 +1989,13 @@ 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 9458c94c6..1b48514f9 100644 --- a/crates/ui-egui/src/shortcuts.rs +++ b/crates/ui-egui/src/shortcuts.rs @@ -559,6 +559,22 @@ 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) + && 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(); + 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..d59698350 --- /dev/null +++ b/crates/ui-egui/src/tests_guided_guides.rs @@ -0,0 +1,367 @@ +//! 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 { + 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. +#[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"); +} + +/// 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:?}"); +} + +/// 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] +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"); +} + +/// 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:?}" + ); +} + +/// 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"); +} diff --git a/docs/parity-checklist.md b/docs/parity-checklist.md index de92318af..05c0ef675 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 | @@ -901,3 +903,4 @@ Latin stays upright. Advanced Japanese composition remains open. Regression cove | 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-18 | minor | LR-EDIT-OPTICS-PROFILE: raws that name no lens model (Nikon DSLRs record only the lens specification in their maker note) look up the lens database by a name built from it (`24-70mm f/2.8`) and use only a profile named exactly that; 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) |