Merge pull request #12270 from DuBearte/fix/11980-ruler-click-keeps

Ruler: a plain click keeps the current ruler instead of deleting it

crates/ui-egui/src/tools/analysis_ruler_click_tests.rs

@@ -0,0 +1,71 @@
+//! #11980: a plain click with the Ruler leaves the current ruler alone; a drag still replaces it.
+
+use super::{PhotocraftApp, Ruler, Tool, ToolEvent, json};
+
+/// A 600 × 400 document with the Ruler from 100,100 to 300,250 and the Ruler tool active.
+fn ruler_app(protractor: Option<[f64; 2]>) -> PhotocraftApp {
+    let mut app = PhotocraftApp::new(photocraft_engine::Session::new(), crate::Services::default());
+    app.run("file.new", json!({"width": 600, "height": 400})).unwrap();
+    app.run("image.analysis.rulerTool", json!({"start": [100, 100], "end": [300, 250], "protractor": protractor})).unwrap();
+    app.ui.tool = Tool::Ruler;
+    app
+}
+
+fn ruler(app: &PhotocraftApp) -> Option<Ruler> {
+    app.session.active().unwrap().doc.measurement.ruler
+}
+
+fn history(app: &PhotocraftApp) -> usize {
+    app.session.active().unwrap().history.past_len()
+}
+
+/// A press at `from` and a release at `to`, moving between them when they differ.
+fn gesture(app: &mut PhotocraftApp, [x, y]: [f64; 2], [end_x, end_y]: [f64; 2]) {
+    crate::canvas::tool_event(app, ToolEvent::Down { x, y, pressure: 1.0 }, egui::Modifiers::NONE);
+    if [x, y] != [end_x, end_y] {
+        crate::canvas::tool_event(app, ToolEvent::Move { x: end_x, y: end_y, pressure: 1.0 }, egui::Modifiers::NONE);
+    }
+    crate::canvas::tool_event(app, ToolEvent::Up { x: end_x, y: end_y }, egui::Modifiers::NONE);
+}
+
+#[test]
+fn a_click_away_from_the_ruler_keeps_it() {
+    for protractor in [None, Some([150.0, 50.0])] {
+        let mut app = ruler_app(protractor);
+        let before = ruler(&app);
+        let steps = history(&app);
+        gesture(&mut app, [500.0, 350.0], [500.0, 350.0]);
+        assert_eq!(ruler(&app), before, "protractor {protractor:?}");
+        assert_eq!(history(&app), steps);
+    }
+}
+
+#[test]
+fn a_click_on_an_end_point_keeps_the_ruler() {
+    let mut app = ruler_app(None);
+    let before = ruler(&app);
+    let steps = history(&app);
+    gesture(&mut app, [300.0, 250.0], [300.0, 250.0]);
+    gesture(&mut app, [100.0, 100.0], [100.0, 100.0]);
+    assert_eq!(ruler(&app), before);
+    assert_eq!(history(&app), steps);
+}
+
+#[test]
+fn a_drag_away_from_the_ruler_replaces_it() {
+    let mut app = ruler_app(Some([150.0, 50.0]));
+    let steps = history(&app);
+    gesture(&mut app, [400.0, 300.0], [550.0, 320.0]);
+    assert_eq!(ruler(&app), Some(Ruler { start: [400.0, 300.0], end: [550.0, 320.0], protractor: None }));
+    assert_eq!(history(&app), steps);
+}
+
+/// Not measured, so as today: a click with no ruler yet leaves none.
+#[test]
+fn a_click_with_no_ruler_leaves_none() {
+    let mut app = PhotocraftApp::new(photocraft_engine::Session::new(), crate::Services::default());
+    app.run("file.new", json!({"width": 600, "height": 400})).unwrap();
+    app.ui.tool = Tool::Ruler;
+    gesture(&mut app, [500.0, 350.0], [500.0, 350.0]);
+    assert_eq!(ruler(&app), None);
+}

crates/ui-egui/src/tools/analysis_ui.rs

@@ -25,8 +25,8 @@ use crate::theme::Tokens;
 /// What a pointer drag is doing (not serialised).
 #[derive(Clone, Copy, Debug, PartialEq)]
 pub enum Drag {
-    /// New ruler line from a point.
-    RulerNew,
+    /// New ruler line from a point; the ruler it replaces comes back if the press doesn't move (#11980).
+    RulerNew(Option<Ruler>),
     /// Moving a ruler handle: 0 start, 1 end, 2 protractor end.
     RulerHandle(u8),
     /// Moving count marker (group, index) from where it was.
@@ -247,6 +247,10 @@ fn set_ruler(app: &mut PhotocraftApp, r: Ruler) {
     let _ = app.run("image.analysis.rulerTool", json!({"start": r.start, "end": r.end, "protractor": r.protractor}));
 }
 
+#[cfg(test)]
+#[path = "analysis_ruler_click_tests.rs"]
+mod analysis_ruler_click_tests;
+
 #[cfg(test)]
 #[path = "note_show_on_new_tests.rs"]
 mod note_show_on_new_tests;
@@ -300,14 +304,14 @@ pub fn pointer(app: &mut PhotocraftApp, ev: ToolEvent, mods: egui::Modifiers) ->
                 Some(r) if near(r.start) => app.ui.analysis.drag = Some(Drag::RulerHandle(0)),
                 _ => {
                     set_ruler(app, Ruler { start: p, end: p, protractor: None });
-                    app.ui.analysis.drag = Some(Drag::RulerNew);
+                    app.ui.analysis.drag = Some(Drag::RulerNew(cur));
                 }
             }
         }
         (Tool::Ruler, ToolEvent::Move { x, y, .. }) => {
             let (Some(drag), Some(mut r)) = (app.ui.analysis.drag, doc.measurement.ruler) else { return true };
             match drag {
-                Drag::RulerNew | Drag::RulerHandle(1) => r.end = constrain(r.start, [x, y], mods.shift),
+                Drag::RulerNew(_) | Drag::RulerHandle(1) => r.end = constrain(r.start, [x, y], mods.shift),
                 Drag::RulerHandle(0) => r.start = constrain(r.end, [x, y], mods.shift),
                 Drag::RulerHandle(_) => r.protractor = Some(constrain(r.start, [x, y], mods.shift)),
                 _ => return true,
@@ -316,8 +320,16 @@ pub fn pointer(app: &mut PhotocraftApp, ev: ToolEvent, mods: egui::Modifiers) ->
         }
         (Tool::Ruler, ToolEvent::Up { .. }) => {
             let drag = app.ui.analysis.drag.take();
-            if drag == Some(Drag::RulerNew) && doc.measurement.ruler.is_some_and(|r| dist(r.start, r.end) < 0.5) {
-                let _ = app.run("image.analysis.rulerTool", json!({"clear": true}));
+            if let Some(Drag::RulerNew(before)) = drag
+                && doc.measurement.ruler.is_some_and(|r| dist(r.start, r.end) < 0.5)
+            {
+                // A click leaves the ruler that was there; with none, it leaves none.
+                match before {
+                    Some(r) => set_ruler(app, r),
+                    None => {
+                        let _ = app.run("image.analysis.rulerTool", json!({"clear": true}));
+                    }
+                }
             }
         }
         (Tool::Count, ToolEvent::Down { x, y, .. }) => {

crates/ui-egui/src/tools/analysis_ui/tests.rs

@@ -92,9 +92,12 @@ fn ruler_drag_protractor_and_clear() {
     up(&mut app, 60.0, 10.0);
     let info = photocraft_engine::analysis_cmds::ruler_info(&ruler(&app).unwrap(), &Default::default());
     assert!((info["angle"].as_f64().unwrap() - 90.0).abs() < 1e-6, "{info}");
-    // A click without a drag far away starts a new line and clears it.
+    // A click without a drag far away keeps the ruler (#11980); Clear removes it.
+    let before = ruler(&app);
     down(&mut app, 150.0, 80.0, none);
     up(&mut app, 150.0, 80.0);
+    assert_eq!(ruler(&app), before);
+    app.run("image.analysis.rulerTool", json!({"clear": true})).unwrap();
     assert!(ruler(&app).is_none());
     assert_eq!(app.session.active().unwrap().history.past_len(), 0, "the ruler adds no history");
 }