Merge pull request #13141 from rmsff/fix/convert-point-drag-segment-12741

Convert Point: drag a segment as Direct Selection does

crates/engine/src/cmd/path_edit_cmds.rs

@@ -116,10 +116,11 @@ fn move_anchors(s: &mut Session, p: &Value) -> Result<Value> {
         })
         .collect::<Result<_>>()?;
     let d = offset(CMD, p)?;
+    let label = step_name(CMD, p, "Drag Anchor Points", "Drag Path")?;
     if p.get("curvature").and_then(Value::as_bool).unwrap_or(false) {
-        return edit_path(s, CMD, p, "Drag Anchor Points", |path| edit::move_anchors_curvature(path, &anchors, d));
+        return edit_path(s, CMD, p, label, |path| edit::move_anchors_curvature(path, &anchors, d));
     }
-    edit_path(s, CMD, p, "Drag Anchor Points", |path| edit::move_anchors(path, &anchors, d))
+    edit_path(s, CMD, p, label, |path| edit::move_anchors(path, &anchors, d))
 }
 
 fn move_handle(s: &mut Session, p: &Value) -> Result<Value> {
@@ -283,7 +284,7 @@ pub fn specs() -> Vec<CommandSpec> {
         journal: true,
     };
     vec![
-        spec("path.moveAnchors", "Drag Anchor Points", r#""anchors":[[subpath,knot],…],"move":[dx,dy],"curvature":bool=false (anchors move with their handles; Direct Selection drag. curvature = the Curvature Pen's drag: every smooth knot of the touched subpaths gets its handles recomputed so the curve flows through the anchors)"#.into(), move_anchors),
+        spec("path.moveAnchors", "Drag Anchor Points", r#""anchors":[[subpath,knot],…],"move":[dx,dy],"curvature":bool=false (anchors move with their handles; Direct Selection drag. curvature = the Curvature Pen's drag: every smooth knot of the touched subpaths gets its handles recomputed so the curve flows through the anchors),"stepName":"Drag Path"? (History step name; the Convert Point tool sends it when it drags a whole path; default "Drag Anchor Points"; any other value is refused)"#.into(), move_anchors),
         spec(
             "path.moveHandle",
             "Drag Direction Point",

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

@@ -10,8 +10,9 @@
 //!   The arrow keys nudge the selected anchors 1 px (⇧ 10 px), one history step per press.
 //! * Convert Point (the tool, or Pen + ⌥): click an anchor to make it a corner, drag from it to
 //!   pull out smooth handles, drag a handle to move it alone. The tool selects the anchor it
-//!   converts, so its handles show; a press on empty canvas or a segment does nothing, and ⌘/Ctrl
-//!   is Direct Selection, as with the Pen.
+//!   converts, so its handles show; a press on empty canvas does nothing, and ⌘/Ctrl is Direct
+//!   Selection, as with the Pen. The tool drags a straight segment with nothing selected, or the
+//!   whole path when all of it is selected, as Direct Selection would (#12741).
 //! * While a Pen path is being drawn, ⌘/Ctrl edits it in place (its anchors, handles and
 //!   segments) and the Pen carries on drawing from its last anchor once ⌘/Ctrl is let go; a
 //!   ⌘/Ctrl-press anywhere else ends it, as in Photoshop (#4022).
@@ -267,6 +268,8 @@ pub enum Edit {
     /// A handle, where it was at the press, and whether it moves alone (⌥).
     Handle([usize; 2], Handle, Point, bool),
     Segment([usize; 2], f64),
+    /// The Convert Point tool dragging a whole selected path by a segment: its anchors, all of them.
+    Path(Vec<[usize; 2]>),
     /// Pen + ⌥ on an anchor: a click makes a corner, a drag pulls out smooth handles.
     Convert([usize; 2]),
 }
@@ -287,6 +290,7 @@ impl Edit {
                 ("path.moveHandle", json!({"subpath": s, "knot": k, "handle": h.name(), "to": [from.x + d[0], from.y + d[1]], "independent": independent}))
             }
             Edit::Segment([s, k], t) => ("path.bendSegment", json!({"subpath": s, "knot": k, "t": t, "move": d})),
+            Edit::Path(a) => ("path.moveAnchors", json!({"anchors": a, "move": d, "stepName": "Drag Path"})),
             Edit::Convert([s, k]) => ("path.convertPoint", json!({"subpath": s, "knot": k, "out": end})),
         }
     }
@@ -296,7 +300,7 @@ impl Edit {
         let d = [end[0] - start[0], end[1] - start[1]];
         let mut p = base.clone();
         match self {
-            Edit::Anchors(a) => edit::move_anchors(&mut p, a, d),
+            Edit::Anchors(a) | Edit::Path(a) => edit::move_anchors(&mut p, a, d),
             Edit::Curvature(a) => edit::move_anchors_curvature(&mut p, a, d),
             Edit::Handle(k, h, from, independent) => edit::move_handle(&mut p, *k, *h, Point::new(from.x + d[0], from.y + d[1]), *independent),
             Edit::Segment(k, t) => edit::bend_segment(&mut p, *k, *t, d),
@@ -654,8 +658,21 @@ fn press(app: &mut PhotocraftApp, p: [f64; 2], mods: Modifiers, convert: bool) -
             }
             Edit::Convert(k)
         }
-        // Pen + ⌥ on a segment does nothing.
-        Hit::Segment(..) if convert => return true,
+        // Pen + ⌥ on a segment does nothing; so does the tool where the reference was not
+        // measured: with ⇧ or ⌥, on a curve, or with part of the path selected.
+        Hit::Segment(k, t) if convert => {
+            if !own_tool || mods.alt || mods.shift || !straight(&base, k) {
+                return true;
+            }
+            let all: Vec<[usize; 2]> = (0..base.subpaths.len()).flat_map(|s| edit::subpath_anchors(&base, s)).collect();
+            if ds.anchors.is_empty() {
+                Edit::Segment(k, t)
+            } else if all.iter().all(|a| ds.anchors.contains(a)) {
+                Edit::Path(all)
+            } else {
+                return true;
+            }
+        }
         Hit::Anchor(k) => {
             ds.segment = None;
             if mods.alt {
@@ -687,6 +704,18 @@ fn press(app: &mut PhotocraftApp, p: [f64; 2], mods: Modifiers, convert: bool) -
     true
 }
 
+/// Is the segment leaving knot `k` straight: both of its handles retracted? The distance test is
+/// the one `edit::bend_segment` uses, so a segment counted straight here moves as straight there.
+fn straight(path: &Path, [s, k]: [usize; 2]) -> bool {
+    let Some(sub) = path.subpaths.get(s) else { return false };
+    let next = if k + 1 < sub.knots.len() { k + 1 } else { 0 };
+    let near = |c: Point, a: Point| (c.x - a.x).hypot(c.y - a.y) < 1e-9;
+    match (sub.knots.get(k), sub.knots.get(next)) {
+        (Some(a), Some(b)) => near(a.out_ctrl, a.anchor) && near(b.in_ctrl, b.anchor),
+        _ => false,
+    }
+}
+
 /// What the idle Curvature Pen has under document point `p` on the active paths: an anchor or a
 /// segment, with the path it is on. Handles don't count.
 fn curvature_target(app: &PhotocraftApp, p: [f64; 2]) -> Option<(PathRef, Path, Hit)> {
@@ -953,6 +982,9 @@ mod convert_point_targeted_path_tests;
 #[cfg(test)]
 mod convert_point_tolerance_tests;
 
+#[cfg(test)]
+mod convert_point_drag_segment_tests;
+
 #[cfg(test)]
 mod freeform_command_tests {
     use super::*;

crates/ui-egui/src/tools/direct_select/convert_point_drag_segment_tests.rs

@@ -0,0 +1,172 @@
+//! #12741: a drag that starts on a segment with the Convert Point tool works as a temporary
+//! Direct Selection, as in the reference: with nothing selected the straight segment moves
+//! ("Drag Segment"), with the whole path selected the whole path moves ("Drag Path"). A click,
+//! and the cases the reference was not measured on, do nothing.
+
+use egui::Modifiers;
+use photocraft_doc::vector::Path;
+use photocraft_geom::Point;
+use serde_json::json;
+
+use super::convert_point_history_tests::{added, click, drag};
+use crate::PhotocraftApp;
+use crate::canvas::{ToolEvent, tool_event};
+use crate::state::Tool;
+
+const BOTTOM_EDGE: [f64; 2] = [200.0, 500.0];
+
+/// The reference document: 800×600 px with a saved path, targeted in the Paths panel: a closed
+/// corner square, an open straight line and an open S-curve of two smooth points.
+fn app(tool: Tool) -> PhotocraftApp {
+    let mut app = PhotocraftApp::new(photocraft_engine::Session::new(), crate::Services::default());
+    app.session.execute("file.new", json!({"width": 800, "height": 600})).unwrap();
+    let path = json!({"subpaths": [
+        {"closed": true, "knots": [[100, 300], [300, 300], [300, 500], [100, 500]]},
+        {"knots": [[400, 300], [700, 300]]},
+        {"knots": [
+            {"anchor": [400, 450], "in": [300, 550], "out": [500, 350], "smooth": true},
+            {"anchor": [700, 450], "in": [600, 550], "out": [800, 350], "smooth": true},
+        ]},
+    ]});
+    app.run("path.set", json!({"name": "Path 1", "path": path})).unwrap();
+    app.ui.selected_path = Some("Path 1".into());
+    app.sync_views();
+    app.ui.tool = tool;
+    app
+}
+
+fn path(app: &PhotocraftApp) -> Path {
+    app.session.active().unwrap().doc.paths[0].path.clone()
+}
+
+/// `p` with knots `which` (all when `None`) moved by `d`, handles included.
+fn moved(p: &Path, which: Option<&[[usize; 2]]>, d: [f64; 2]) -> Path {
+    let mut p = p.clone();
+    for (s, sub) in p.subpaths.iter_mut().enumerate() {
+        for (k, kn) in sub.knots.iter_mut().enumerate() {
+            if which.is_none_or(|w| w.contains(&[s, k])) {
+                for q in [&mut kn.anchor, &mut kn.in_ctrl, &mut kn.out_ctrl] {
+                    *q = Point::new(q.x + d[0], q.y + d[1]);
+                }
+            }
+        }
+    }
+    p
+}
+
+/// Every anchor of the path selected, by a ⌘-marquee (Direct Selection in this tool).
+fn select_all(app: &mut PhotocraftApp) {
+    drag(app, [0.0, 0.0], [800.0, 600.0], Modifiers::COMMAND);
+    assert_eq!(app.ui.direct_selection.anchors.len(), 8);
+}
+
+#[test]
+fn with_nothing_selected_a_segment_drag_moves_only_that_segment() {
+    let mut app = app(Tool::ConvertPoint);
+    let before = path(&app);
+    let steps = added(&mut app, |a| drag(a, BOTTOM_EDGE, [200.0, 540.0], Modifiers::NONE));
+    assert_eq!(steps, ["Drag Segment"]);
+    let after = path(&app);
+    assert_eq!(after.subpaths[0].knots[2].anchor, Point::new(300.0, 540.0));
+    assert_eq!(after.subpaths[0].knots[3].anchor, Point::new(100.0, 540.0));
+    assert_eq!(after, moved(&before, Some(&[[0, 2], [0, 3]]), [0.0, 40.0]), "the other anchors stay");
+    assert!(app.session.undo());
+    assert_eq!(path(&app), before);
+}
+
+#[test]
+fn the_closing_segment_of_a_closed_subpath_moves_too() {
+    let mut app = app(Tool::ConvertPoint);
+    let before = path(&app);
+    assert_eq!(added(&mut app, |a| drag(a, [100.0, 400.0], [60.0, 400.0], Modifiers::NONE)), ["Drag Segment"]);
+    assert_eq!(path(&app), moved(&before, Some(&[[0, 3], [0, 0]]), [-40.0, 0.0]));
+}
+
+#[test]
+fn with_the_whole_path_selected_a_segment_drag_moves_the_whole_path() {
+    let mut app = app(Tool::ConvertPoint);
+    select_all(&mut app);
+    let before = path(&app);
+    let steps = added(&mut app, |a| drag(a, BOTTOM_EDGE, [200.0, 540.0], Modifiers::NONE));
+    assert_eq!(steps, ["Drag Path"]);
+    assert_eq!(path(&app), moved(&before, None, [0.0, 40.0]), "all three subpaths moved");
+    assert_eq!(app.ui.direct_selection.anchors.len(), 8, "the selection stays");
+}
+
+#[test]
+fn a_click_on_a_segment_does_nothing() {
+    for all in [false, true] {
+        let mut app = app(Tool::ConvertPoint);
+        if all {
+            select_all(&mut app);
+        }
+        let (before, selection) = (path(&app), app.ui.direct_selection.anchors.clone());
+        assert!(added(&mut app, |a| click(a, BOTTOM_EDGE, Modifiers::NONE)).is_empty(), "all selected: {all}");
+        assert_eq!((path(&app), &app.ui.direct_selection.anchors), (before, &selection));
+        assert!(app.ui.direct_selection.drag.is_none());
+    }
+}
+
+#[test]
+fn unmeasured_segment_drags_still_do_nothing() {
+    let nothing = |mut app: PhotocraftApp, from: [f64; 2], mods: Modifiers, what: &str| {
+        let before = path(&app);
+        let steps = added(&mut app, |a| drag(a, from, [from[0], from[1] + 40.0], mods));
+        assert!(steps.is_empty(), "{what}: {steps:?}");
+        assert_eq!(path(&app), before, "{what}");
+    };
+    nothing(app(Tool::ConvertPoint), [550.0, 450.0], Modifiers::NONE, "a curved segment");
+    let mut all = app(Tool::ConvertPoint);
+    select_all(&mut all);
+    nothing(all, [550.0, 450.0], Modifiers::NONE, "a curved segment, the whole path selected");
+    nothing(app(Tool::ConvertPoint), BOTTOM_EDGE, Modifiers::SHIFT, "⇧");
+    nothing(app(Tool::ConvertPoint), BOTTOM_EDGE, Modifiers::ALT, "⌥");
+    let mut some = app(Tool::ConvertPoint);
+    click(&mut some, [100.0, 300.0], Modifiers::COMMAND);
+    assert_eq!(some.ui.direct_selection.anchors, vec![[0, 0]]);
+    nothing(some, BOTTOM_EDGE, Modifiers::NONE, "part of the path selected");
+
+    // Pen + ⌥ on a segment is the Pen's own: it adds an anchor there, and the segment stays.
+    let mut pen = app(Tool::Pen);
+    assert_eq!(added(&mut pen, |a| drag(a, BOTTOM_EDGE, [200.0, 540.0], Modifiers::ALT)), ["Add Anchor Point"]);
+    let bottom = &path(&pen).subpaths[0].knots;
+    assert!(bottom.iter().all(|k| k.anchor.y <= 500.0), "the segment did not move: {bottom:?}");
+}
+
+#[test]
+fn direct_selection_segment_drags_keep_their_behaviour() {
+    for all in [false, true] {
+        let mut app = app(Tool::DirectSelection);
+        if all {
+            select_all(&mut app);
+        }
+        let before = path(&app);
+        assert_eq!(added(&mut app, |a| drag(a, BOTTOM_EDGE, [200.0, 540.0], Modifiers::NONE)), ["Drag Segment"]);
+        assert_eq!(path(&app), moved(&before, Some(&[[0, 2], [0, 3]]), [0.0, 40.0]), "all selected: {all}");
+    }
+}
+
+#[test]
+fn a_command_anchor_drag_released_without_command_keeps_its_own_step_name() {
+    let mut app = app(Tool::ConvertPoint);
+    select_all(&mut app);
+    tool_event(&mut app, ToolEvent::Down { x: 300.0, y: 300.0, pressure: 1.0 }, Modifiers::COMMAND);
+    tool_event(&mut app, ToolEvent::Move { x: 320.0, y: 300.0, pressure: 1.0 }, Modifiers::COMMAND);
+    let steps = added(&mut app, |a| {
+        tool_event(a, ToolEvent::Up { x: 320.0, y: 300.0 }, Modifiers::NONE);
+    });
+    assert_eq!(steps, ["Drag Anchor Points"]);
+}
+
+#[test]
+fn move_anchors_takes_the_drag_path_step_name_and_refuses_any_other() {
+    let mut app = app(Tool::ConvertPoint);
+    let params = |name: serde_json::Value| json!({"name": "Path 1", "anchors": [[0, 0]], "move": [1, 1], "stepName": name});
+    assert_eq!(added(&mut app, |a| a.run("path.moveAnchors", params(json!("Drag Path"))).map(|_| ()).unwrap()), ["Drag Path"]);
+    let before = path(&app);
+    for bad in [json!("Drag Segment"), json!("Drag Path "), json!(1)] {
+        let e = app.run("path.moveAnchors", params(bad.clone())).unwrap_err();
+        assert!(e.contains("stepName"), "{bad}: {e}");
+    }
+    assert_eq!(path(&app), before);
+}

crates/ui-egui/src/tools/direct_select/convert_point_history_tests.rs

@@ -35,7 +35,7 @@ pub(super) fn drag(app: &mut PhotocraftApp, from: [f64; 2], to: [f64; 2], mods:
     tool_event(app, ToolEvent::Up { x: to[0], y: to[1] }, mods);
 }
 
-fn click(app: &mut PhotocraftApp, at: [f64; 2], mods: Modifiers) {
+pub(super) fn click(app: &mut PhotocraftApp, at: [f64; 2], mods: Modifiers) {
     drag(app, at, at, mods);
 }
 
@@ -44,7 +44,7 @@ fn names(app: &PhotocraftApp) -> Vec<String> {
 }
 
 /// Runs `gesture` and returns the History steps it added.
-fn added(app: &mut PhotocraftApp, gesture: impl FnOnce(&mut PhotocraftApp)) -> Vec<String> {
+pub(super) fn added(app: &mut PhotocraftApp, gesture: impl FnOnce(&mut PhotocraftApp)) -> Vec<String> {
     let n = names(app).len();
     gesture(app);
     names(app).split_off(n)