Merge pull request #6718 from dotCooCoo/fix/issue-6634

Fix #6634: Pen closing drag no longer gives the first anchor symmetric handles

crates/engine/src/cmd/addons/pen/tests.rs

@@ -285,3 +285,54 @@ fn the_tool_draws_a_path_one_step_per_anchor_and_closes_it() {
     assert_eq!(path(&s, id).subpaths[0].anchors.len(), 3);
     assert_eq!(steps(&s), n + 8);
 }
+
+/// #6634: the tool's closing drag on the first anchor sets that anchor's incoming handle to the
+/// mirror of the pointer and does not give it symmetric handles: a corner keeps its outgoing
+/// handle, a smooth anchor turns it to stay in line at its own length, and Alt pressed mid-drag
+/// keeps it where the drag had turned it.
+#[test]
+fn the_tools_closing_drag_does_not_make_the_first_anchor_symmetric() {
+    // An open path whose first anchor, at (50, 50), has the handles `first`, drawn on from its last
+    // anchor; the closing gesture is then up to the caller.
+    let draw = |first: Value| {
+        let mut s = session();
+        let mut a = json!({"x": 50, "y": 50});
+        a.as_object_mut().unwrap().extend(first.as_object().unwrap().clone());
+        let anchors = json!([a, {"x": 150, "y": 50}, {"x": 150, "y": 150}]);
+        let id = NodeId(s.execute("path.create", &json!({"anchors": anchors})).unwrap()["id"].as_u64().unwrap());
+        s.select_tool("smartPen", view()).unwrap();
+        gesture(&mut s, &[(150.0, 150.0)], Mods::default());
+        (s, id)
+    };
+    let first = |s: &Session, id: NodeId| {
+        let sp = &path(s, id).subpaths[0];
+        assert!(sp.closed, "{sp:?}");
+        sp.anchors[0]
+    };
+    let near = |a: Point, b: Point| a.distance(b) < 1e-6;
+
+    // A corner: the first segment keeps its shape.
+    let (mut s, id) = draw(json!({"out": [80, 30]}));
+    gesture(&mut s, &[(50.0, 50.0), (50.0, 100.0)], Mods::default());
+    let a = first(&s, id);
+    assert_eq!((a.h_in, a.h_out, a.kind), (Point::new(50.0, 0.0), Point::new(80.0, 30.0), AnchorKind::Corner), "{a:?}");
+
+    // A smooth anchor: the outgoing handle turns opposite the incoming one, keeping its length 30.
+    let (mut s, id) = draw(json!({"in": [20, 50], "out": [80, 50]}));
+    assert_eq!(path(&s, id).subpaths[0].anchors[0].kind, AnchorKind::Smooth);
+    gesture(&mut s, &[(50.0, 50.0), (50.0, 100.0)], Mods::default());
+    let a = first(&s, id);
+    assert_eq!((a.h_in, a.kind), (Point::new(50.0, 0.0), AnchorKind::Smooth), "{a:?}");
+    assert!(near(a.h_out, Point::new(50.0, 80.0)), "{a:?}");
+
+    // Alt pressed mid-drag: a corner, its outgoing handle where the drag had turned it.
+    let (mut s, id) = draw(json!({"in": [20, 50], "out": [80, 50]}));
+    let alt = Mods { alt: true, ..Mods::default() };
+    s.pointer(&PointerEvent::new(PointerKind::Down, 50.0, 50.0), view()).unwrap();
+    s.pointer(&PointerEvent::new(PointerKind::Drag, 50.0, 100.0), view()).unwrap();
+    s.pointer(&PointerEvent::new(PointerKind::Drag, 80.0, 100.0).with_mods(alt), view()).unwrap();
+    s.pointer(&PointerEvent::new(PointerKind::Up, 80.0, 100.0).with_mods(alt), view()).unwrap();
+    let a = first(&s, id);
+    assert_eq!((a.h_in, a.kind), (Point::new(20.0, 0.0), AnchorKind::Corner), "{a:?}");
+    assert!(near(a.h_out, Point::new(50.0, 80.0)), "{a:?}");
+}

crates/engine/src/cmd/path.rs

@@ -32,7 +32,7 @@ pub fn specs() -> Vec<CommandSpec> {
             "Close Path",
             [],
             None,
-            "{id, subpath?: usize (the open subpath to close, 0 included; default: the last open one), in?: [x,y] (handle into the first anchor), independent?: bool}",
+            "{id, subpath?: usize (the open subpath to close, 0 included; default: the last open one), in?: [x,y] (handle into the first anchor, placed as path.setHandle places it: a smooth first anchor keeps its outgoing handle in line at its own length, a corner keeps it where it is), out?: [x,y] (the first anchor's outgoing handle, placed after `in`; the anchor becomes a corner), independent?: bool (`in` makes the first anchor a corner and leaves its outgoing handle)}",
             has_doc,
             close
         ),
@@ -202,14 +202,18 @@ fn append_anchor(s: &mut Session, p: &Value) -> Result<Value> {
     ok()
 }
 
-/// Close the active open subpath of `id`, optionally setting the incoming handle of the first anchor.
+/// Close the active open subpath of `id`, optionally setting the handles of the first anchor. The
+/// incoming handle moves as Move Direction Handle moves it: the outgoing handle turns with it only
+/// on a smooth anchor, so the first segment of a path that starts at a corner keeps its shape.
 fn close(s: &mut Session, p: &Value) -> Result<Value> {
     let id = id_param(p, "id").ok_or_else(|| bad("path.close", "missing id"))?;
     let subpath = p.get("subpath").and_then(Value::as_u64).map(|u| u as usize);
-    let h_in = match p.get("in").filter(|h| !h.is_null()) {
-        None => None,
-        Some(h) => Some(finite_numbers(h).map(|[x, y]| Point::new(x, y)).ok_or_else(|| bad("path.close", "`in` must be [x, y]"))?),
+    let handle = |key: &str| match p.get(key).filter(|h| !h.is_null()) {
+        None => Ok(None),
+        Some(h) => finite_numbers(h).map(|[x, y]| Some(Point::new(x, y))).ok_or_else(|| bad("path.close", format!("`{key}` must be [x, y]"))),
     };
+    let h_in = handle("in")?;
+    let h_out = handle("out")?;
     let independent = switch_or(p, "independent", false, "path.close")?;
     s.edit("Close Path", |d, sel| {
         let path = path_mut(d, id)?;
@@ -218,11 +222,13 @@ fn close(s: &mut Session, p: &Value) -> Result<Value> {
             None => path.subpaths.iter_mut().rev().find(|s| !s.closed).ok_or_else(|| EngineError::Other("path is already closed".into()))?,
         };
         sp.closed = true;
-        if let (Some(h), Some(first)) = (h_in, sp.anchors.first_mut()) {
-            first.h_in = h;
-            if !independent {
-                first.h_out = first.p - (h - first.p);
-                first.kind = AnchorKind::Smooth;
+        if let Some(first) = sp.anchors.first_mut() {
+            if let Some(h) = h_in {
+                first.set_handle(false, h, independent);
+            }
+            if let Some(h) = h_out {
+                first.h_out = h;
+                first.kind = AnchorKind::Corner;
             }
         }
         let top = compound_or_self(d, id);

crates/engine/src/tests/draw2.rs

@@ -3146,7 +3146,7 @@ fn shift_reshapes_the_closing_segment_of_a_rotated_square_and_undoes() {
 fn pen_drag_from_the_first_anchor_closes_the_path() {
     // #501: closing a path by dragging out of its first anchor flickered between a close and a new
     // path, and released at the wrong moment left the shape open. The press decides: the drag
-    // shapes the closing curve, symmetric, and the release closes the path wherever it ends.
+    // shapes the closing curve, and the release closes the path wherever it ends.
     let none = Mods::default();
     let v = view();
     let mut s = session();
@@ -3167,13 +3167,119 @@ fn pen_drag_from_the_first_anchor_closes_the_path() {
     let sp = &path(&s, id).subpaths[0];
     assert!(sp.closed && sp.anchors.len() == 3, "{sp:?}");
     let a = sp.anchors[0];
-    assert_eq!((a.h_in, a.h_out), (Point::new(40.0, 348.0), Point::new(160.0, 252.0)), "symmetric");
+    assert_eq!((a.h_in, a.h_out), (Point::new(40.0, 348.0), Point::new(100.0, 300.0)), "only the closing segment curves");
     assert_eq!(paths(&s).len(), 1);
     // The next click starts a new path.
     gesture(&mut s, &[(500.0, 500.0)], none);
     assert_eq!(paths(&s).len(), 2);
 }
 
+/// #6634: closing a path by dragging on its first anchor does not give that anchor symmetric
+/// handles. Its incoming handle mirrors the pointer; its outgoing handle stays where it is on a
+/// corner, so the first segment keeps its shape, and on a smooth anchor turns to stay in line at
+/// its own length. Alt keeps the outgoing handle where it was when Alt went down, Shift keeps the
+/// drag to 45° steps, and a click leaves the anchor a corner with the closing segment straight
+/// into it. Each close is one undo step.
+#[test]
+fn pen_closing_drag_does_not_make_the_first_anchor_symmetric() {
+    use vectorcraft_geom::AnchorKind;
+    let (none, alt, shift) = (Mods::default(), Mods { alt: true, ..Mods::default() }, Mods { shift: true, ..Mods::default() });
+    let v = view();
+    // Draws a path from (100, 300), dragged out to `start` when given (a smooth anchor), through
+    // (200, 200) and (300, 300), and returns it with its first anchor as drawn.
+    let draw = |s: &mut Session, start: Option<(f64, f64)>| {
+        s.select_tool("pen", v).unwrap();
+        let first = match start {
+            Some(out) => vec![(100.0, 300.0), out],
+            None => vec![(100.0, 300.0)],
+        };
+        gesture(s, &first, none);
+        gesture(s, &[(200.0, 200.0)], none);
+        gesture(s, &[(300.0, 300.0)], none);
+        let id = s.doc().unwrap().selection.objects[0];
+        let anchor = path(s, id).subpaths[0].anchors[0];
+        (id, anchor)
+    };
+    let first = |s: &Session, id: NodeId| {
+        let sp = &path(s, id).subpaths[0];
+        assert!(sp.closed, "{sp:?}");
+        sp.anchors[0]
+    };
+
+    // A corner: the first segment stays straight however the drag turns.
+    let mut s = session();
+    let (id, before) = draw(&mut s, None);
+    assert_eq!(gesture(&mut s, &[(100.0, 300.0), (80.0, 250.0), (60.0, 200.0)], none), 1, "one undo step");
+    let a = first(&s, id);
+    assert_eq!((a.h_in, a.h_out, a.kind), (Point::new(140.0, 400.0), before.h_out, AnchorKind::Corner), "{a:?}");
+    s.execute("edit.undo", &json!({})).unwrap();
+    assert!(!path(&s, id).subpaths[0].closed, "the undo opens the path again");
+
+    // A smooth anchor: the outgoing handle turns opposite the incoming one, keeping its length 50.
+    let mut s = session();
+    let (id, before) = draw(&mut s, Some((150.0, 300.0)));
+    assert_eq!((before.h_out, before.kind), (Point::new(150.0, 300.0), AnchorKind::Smooth));
+    gesture(&mut s, &[(100.0, 300.0), (100.0, 250.0), (100.0, 200.0)], none);
+    let a = first(&s, id);
+    assert_eq!((a.h_in, a.kind), (Point::new(100.0, 400.0), AnchorKind::Smooth), "{a:?}");
+    assert!(near(a.h_out, Point::new(100.0, 250.0)), "in line, at its own length: {a:?}");
+
+    // Alt held from the press: a corner, its outgoing handle where it was.
+    let mut s = session();
+    let (id, before) = draw(&mut s, Some((150.0, 300.0)));
+    gesture(&mut s, &[(100.0, 300.0), (100.0, 250.0), (100.0, 200.0)], alt);
+    let a = first(&s, id);
+    assert_eq!((a.h_in, a.h_out, a.kind), (Point::new(100.0, 400.0), before.h_out, AnchorKind::Corner), "{a:?}");
+
+    // Alt pressed mid-drag: the outgoing handle stays where the drag had turned it.
+    let mut s = session();
+    let (id, _) = draw(&mut s, Some((150.0, 300.0)));
+    s.pointer(&PointerEvent::new(PointerKind::Down, 100.0, 300.0), v).unwrap();
+    s.pointer(&PointerEvent::new(PointerKind::Drag, 100.0, 200.0), v).unwrap();
+    s.pointer(&PointerEvent::new(PointerKind::Drag, 130.0, 200.0).with_mods(alt), v).unwrap();
+    s.pointer(&PointerEvent::new(PointerKind::Up, 130.0, 200.0).with_mods(alt), v).unwrap();
+    let a = first(&s, id);
+    assert_eq!((a.h_in, a.kind), (Point::new(70.0, 400.0), AnchorKind::Corner), "{a:?}");
+    assert!(near(a.h_out, Point::new(100.0, 250.0)), "{a:?}");
+
+    // Shift: the drag keeps to 45° steps (straight up here), at the pointer's distance from the
+    // anchor, mirrored into the incoming handle.
+    let mut s = session();
+    let (id, before) = draw(&mut s, None);
+    gesture(&mut s, &[(100.0, 300.0), (95.0, 250.0), (90.0, 200.0)], shift);
+    let a = first(&s, id);
+    assert!(near(a.h_in, Point::new(100.0, 300.0 + 10f64.hypot(100.0))) && a.h_out == before.h_out, "{a:?}");
+
+    // A click: the closing segment comes straight in, and a smooth anchor keeps its outgoing
+    // handle and becomes a corner.
+    let mut s = session();
+    let (id, before) = draw(&mut s, Some((150.0, 300.0)));
+    assert_eq!(gesture(&mut s, &[(100.0, 300.0)], none), 1);
+    let a = first(&s, id);
+    assert_eq!((a.h_in, a.h_out, a.kind), (Point::new(100.0, 300.0), before.h_out, AnchorKind::Corner), "{a:?}");
+}
+
+/// Close Path's `in` moves the first anchor's incoming handle as Move Direction Handle does, and
+/// `out` places its outgoing handle, making it a corner; `out` that is not [x, y] is refused.
+#[test]
+fn close_path_places_the_first_anchors_handles() {
+    use vectorcraft_geom::AnchorKind;
+    let mut s = session();
+    let id = curve(&mut s);
+    let a = path(&s, id).subpaths[0].anchors[0];
+    assert!(a.has_out() && a.kind == AnchorKind::Corner, "{a:?}");
+    for h in [json!("1, 2"), json!([1]), json!([1, "2"])] {
+        refuses_with(&mut s, "path.close", &json!({"id": id.0, "out": h}), "`out` must be [x, y]");
+    }
+    s.execute("path.close", &json!({"id": id.0, "in": [60.0, 140.0]})).unwrap();
+    let b = path(&s, id).subpaths[0].anchors[0];
+    assert_eq!((b.h_in, b.h_out, b.kind), (Point::new(60.0, 140.0), a.h_out, AnchorKind::Corner), "a corner keeps its outgoing handle");
+    s.execute("edit.undo", &json!({})).unwrap();
+    s.execute("path.close", &json!({"id": id.0, "in": [60.0, 140.0], "out": [120.0, 60.0]})).unwrap();
+    let b = path(&s, id).subpaths[0].anchors[0];
+    assert_eq!((b.h_in, b.h_out, b.kind), (Point::new(60.0, 140.0), Point::new(120.0, 60.0), AnchorKind::Corner));
+}
+
 #[test]
 fn pen_with_alt_converts_the_anchors_of_the_path_being_drawn() {
     // #504: while drawing, Alt-click on a smooth anchor of the path makes it a corner and Alt-drag