Merge pull request #2352 from VL420/fix/remove-anchors-select-next

G1: Remove Anchor Points selects the next anchor (#2241)

crates/engine/src/cmd/draw2.rs

@@ -1,6 +1,7 @@
 //! Commands backing the drawing tools (pencil, curvature, anchor tools, scissors, knife, eraser,
 //! blob brush, smooth, path eraser, join).
 
+use std::collections::BTreeSet;
 use std::sync::Arc;
 
 use kurbo::{CubicBez, ParamCurve, ParamCurveNearest};
@@ -60,7 +61,7 @@ pub fn specs() -> Vec<CommandSpec> {
             "Remove Anchor Points",
             ["Object", "Path"],
             None,
-            "{} remove the direct-selected anchors without opening their paths, refitting the curve round each → {removedObjects}",
+            "{} remove the direct-selected anchors without opening their paths, refitting the curve round each; the anchor after each removed one is selected, so running it again goes on along the path → {removedObjects}",
             has_anchors,
             remove_anchors
         ),
@@ -772,7 +773,16 @@ fn remove_anchors(s: &mut Session, _: &Value) -> Result<Value> {
         let mut removed = 0;
         let mut emptied = Beside::replacing(d, &jobs.iter().map(|(id, _)| *id).collect::<Vec<_>>());
         for (id, anchors) in jobs {
-            removed += usize::from(remove_anchors_of(d, sel, &mut emptied, id, anchors)?);
+            let shape: Vec<(usize, bool)> =
+                d.node(id).and_then(Node::path_data).map(|p| p.subpaths.iter().map(|sp| (sp.anchors.len(), sp.closed)).collect()).unwrap_or_default();
+            let next = following_anchors(&shape, &anchors);
+            if remove_anchors_of(d, sel, &mut emptied, id, anchors)? {
+                removed += 1;
+            } else if !next.is_empty() {
+                // The next anchor along is selected, so the next Remove Anchor Points goes on from
+                // there without picking it first (#2241).
+                sel.anchors.insert(id, next);
+            }
         }
         emptied.finish(d)?;
         Ok(removed)
@@ -780,6 +790,30 @@ fn remove_anchors(s: &mut Session, _: &Value) -> Result<Value> {
     Ok(json!({ "removedObjects": removed }))
 }
 
+/// Where the selection goes once Remove Anchor Points takes `removed` (subpath, anchor) out of a
+/// path whose subpaths have `shape` (anchor count, closed): in each subpath, the anchor after its
+/// last removed one, round the end of a closed subpath, or the one before it at an open subpath's
+/// end. Indexes are those after the removal, without the subpaths left with fewer than two anchors.
+fn following_anchors(shape: &[(usize, bool)], removed: &[(usize, usize)]) -> BTreeSet<(usize, usize)> {
+    let mut next = BTreeSet::new();
+    let mut kept = 0;
+    for (si, &(n, closed)) in shape.iter().enumerate() {
+        let gone: BTreeSet<usize> = removed.iter().filter(|&&(s, a)| s == si && a < n).map(|&(_, a)| a).collect();
+        if n - gone.len() < 2 {
+            continue;
+        }
+        if let Some(&last) = gone.last() {
+            let wrap = if closed { 0..last } else { 0..0 };
+            let after = (last + 1..n).chain(wrap).find(|a| !gone.contains(a));
+            if let Some(a) = after.or_else(|| (0..last).rev().find(|a| !gone.contains(a))) {
+                next.insert((kept, a - gone.range(..a).count()));
+            }
+        }
+        kept += 1;
+    }
+    next
+}
+
 /// Remove `anchors` (subpath, anchor) of path `id` without opening it, refitting the curve round
 /// each one. A subpath left with one anchor is removed, and a path left with no subpath is removed
 /// through `emptied`. True when the path went.

crates/engine/src/tests_draw2.rs

@@ -900,6 +900,99 @@ fn remove_anchor_points_keeps_the_paths_closed() {
     assert_eq!(spec.menu, &["Object", "Path"][..]);
 }
 
+fn selected_anchors(s: &Session, id: NodeId) -> Vec<(usize, usize)> {
+    s.doc().unwrap().selection.anchors.get(&id).map(|set| set.iter().copied().collect()).unwrap_or_default()
+}
+
+fn anchor_points(s: &Session, id: NodeId) -> Vec<Point> {
+    path(s, id).subpaths.iter().flat_map(|sp| sp.anchors.iter().map(|a| a.p)).collect()
+}
+
+#[test]
+fn remove_anchor_points_selects_the_next_anchor_so_it_can_go_on_along_the_path() {
+    // #2241: after Remove Anchor Points the next anchor along is selected, so running it again
+    // removes that one without picking it first. The path stays selected the whole way.
+    let mut s = session();
+    let anchors: Vec<_> = (0..5).map(|i| json!({"x": 100 * i, "y": 100 * (i % 2)})).collect();
+    let id = NodeId(s.execute("path.create", &json!({"anchors": anchors})).unwrap()["id"].as_u64().unwrap());
+    let original = anchor_points(&s, id);
+    s.execute("select.anchors", &json!({"id": id.0, "anchors": [[0, 1]]})).unwrap();
+    for (remaining, next) in [(4, 2), (3, 3), (2, 4)] {
+        assert_eq!(s.execute("path.removeAnchors", &json!({})).unwrap()["removedObjects"], 0);
+        assert_eq!(path(&s, id).subpaths[0].anchors.len(), remaining);
+        assert_eq!(selected_anchors(&s, id), [(0, 1)], "anchor {next} of the original is selected");
+        assert_eq!(path(&s, id).subpaths[0].anchors[1].p, original[next]);
+        assert_eq!(s.doc().unwrap().selection.objects, [id]);
+    }
+    // One anchor can't make a path: the last run removes it, and nothing is left selected.
+    assert_eq!(s.execute("path.removeAnchors", &json!({})).unwrap()["removedObjects"], 1);
+    assert!(s.doc().unwrap().doc.node(id).is_none());
+    assert!(s.doc().unwrap().selection.is_empty());
+}
+
+#[test]
+fn remove_anchor_points_selects_the_anchor_before_an_open_end_and_wraps_a_closed_path() {
+    // An open path's last anchor has nothing after it: the one before is selected.
+    let mut s = session();
+    let anchors: Vec<_> = (0..4).map(|i| json!({"x": 100 * i, "y": 0})).collect();
+    let open = NodeId(s.execute("path.create", &json!({"anchors": anchors})).unwrap()["id"].as_u64().unwrap());
+    let original = anchor_points(&s, open);
+    s.execute("select.anchors", &json!({"id": open.0, "anchors": [[0, 3]]})).unwrap();
+    s.execute("path.removeAnchors", &json!({})).unwrap();
+    assert_eq!(selected_anchors(&s, open), [(0, 2)]);
+    assert_eq!(anchor_points(&s, open)[2], original[2]);
+
+    // A closed path goes round: removing its last anchor selects its first.
+    let r = rect(&mut s, 0.0, 200.0, 100.0, 80.0);
+    let corners = anchor_points(&s, r);
+    s.execute("select.anchors", &json!({"id": r.0, "anchors": [[0, 3]]})).unwrap();
+    s.execute("path.removeAnchors", &json!({})).unwrap();
+    assert_eq!(selected_anchors(&s, r), [(0, 0)]);
+    assert_eq!(anchor_points(&s, r)[0], corners[0]);
+}
+
+#[test]
+fn remove_anchor_points_selects_one_next_anchor_per_subpath_and_per_path() {
+    // Several anchors of one subpath: the anchor after the last of them. Each path and each
+    // subpath gets its own, at its index once a subpath left too short to keep is gone.
+    let mut s = session();
+    let made = s.execute("path.create", &json!({"d": "M0 0 L50 0 M0 100 L50 100 L100 100 L150 100 L200 100 L250 100"})).unwrap();
+    let id = NodeId(made["id"].as_u64().unwrap());
+    let original = anchor_points(&s, id);
+    let anchors: Vec<_> = (0..4).map(|i| json!({"x": 100 * i, "y": 400})).collect();
+    let other = NodeId(s.execute("path.create", &json!({"anchors": anchors})).unwrap()["id"].as_u64().unwrap());
+    s.execute("select.anchors", &json!({"id": id.0, "anchors": [[0, 0], [1, 1], [1, 2]]})).unwrap();
+    s.execute("select.anchors", &json!({"id": other.0, "anchors": [[0, 0]], "mode": "add"})).unwrap();
+    assert_eq!(s.execute("path.removeAnchors", &json!({})).unwrap()["removedObjects"], 0);
+    let sp = &path(&s, id).subpaths;
+    assert_eq!(sp.len(), 1, "the first subpath, down to one anchor, is gone");
+    assert_eq!(selected_anchors(&s, id), [(0, 1)]);
+    assert_eq!(sp[0].anchors[1].p, original[5], "the anchor after the two removed ones");
+    assert_eq!(selected_anchors(&s, other), [(0, 0)]);
+
+    // One undo step brings back the anchors and the selection they were removed from.
+    s.execute("edit.undo", &json!({})).unwrap();
+    assert_eq!(anchor_points(&s, id), original);
+    assert_eq!(selected_anchors(&s, id), [(0, 0), (1, 1), (1, 2)]);
+    assert_eq!(selected_anchors(&s, other), [(0, 0)]);
+}
+
+#[test]
+fn remove_anchor_point_tool_and_delete_still_select_the_remainder_whole() {
+    // Only Remove Anchor Points moves the selection on. The Delete Anchor Point tool leaves the
+    // path selected whole, and so does the Delete key, so that a second Delete removes the rest.
+    for command in ["path.removeAnchor", "path.deleteAnchors"] {
+        let mut s = session();
+        let anchors: Vec<_> = (0..4).map(|i| json!({"x": 100 * i, "y": 0})).collect();
+        let id = NodeId(s.execute("path.create", &json!({"anchors": anchors})).unwrap()["id"].as_u64().unwrap());
+        s.execute("select.anchors", &json!({"id": id.0, "anchors": [[0, 1]]})).unwrap();
+        let params = if command == "path.removeAnchor" { json!({"id": id.0, "subpath": 0, "anchor": 1}) } else { json!({}) };
+        s.execute(command, &params).unwrap();
+        assert!(selected_anchors(&s, id).is_empty(), "{command}");
+        assert_eq!(s.doc().unwrap().selection.objects, [id], "{command}");
+    }
+}
+
 fn undo_steps(s: &Session) -> usize {
     s.doc().unwrap().history.undo.len()
 }

crates/ui-egui/src/tests_removeanchors.rs

@@ -150,7 +150,9 @@ fn the_path_menu_and_bars_run_it_for_direct_selected_anchors() {
     assert!(has(&texts, LABEL), "context menu: {texts:?}");
     let texts = click_canvas(&mut app, &ctx, at(&texts, LABEL), PointerButton::Primary);
     assert_eq!(anchor_count(&app, id), 3);
-    assert!(!has(&texts, LABEL), "the menu closes after the command");
+    assert!(!has(&texts, "Cut Path at Selected Anchor Points"), "the menu closes after the command");
+    // The next anchor along is selected (#2241), so the task bar offers the command again.
+    assert_eq!(texts.iter().filter(|(t, _)| t == LABEL).count(), 1, "task bar: {texts:?}");
 }
 
 #[test]

docs/mcp.md

@@ -1608,6 +1608,8 @@ paths and compound paths, and `path.reverse {reversed?}` makes subpaths run coun
 {id, anchors: [[subpath, anchor]…], mode?}`) without opening their paths, in one undo step, and answers
 `{removedObjects}` (paths left with no segment go). Each removed point's neighbours keep their handle directions and
 their facing handles are refitted so one cubic follows the two old segments; two straight sides become one.
+The anchor after each removed one is then selected (the one before at an open path's end), so running
+`path.removeAnchors` again goes on along the path.
 `path.removeAnchor {id, subpath?, anchor}` (the Delete Anchor Point tool) removes one anchor the same way, and
 `path.deleteAnchors {}` (the Delete key) deletes the selected anchors with their segments, opening closed paths there.
 

docs/ui-parity.md

@@ -75,7 +75,7 @@ tools, never checked side by side) isn't captured row by row.
 | Drag a handle with Shift (45° about its anchor), Alt (move one handle alone) | done | handles snap to smart guides |
 | Show handles when multiple anchors are selected; Highlight anchors on mouse over; anchor and handle size | done | Preferences › Selection & Anchor Display |
 | Convert selected anchors to corner/smooth from the Control bar and Properties | done | |
-| Cut path at selected anchors, Remove Anchor Points, Delete removes segments | done | Open-path anchor Delete removes adjoining segments without bridging the gap; targeted core and full-frame Delete/Backspace regressions pass for survivor selection and Undo/Redo. Explicit Remove Anchor Points keeps its reconnect/refit behavior; #1278: segment Delete preserves neighboring edges and endpoint anchors; a second Delete removes the remainder; explicit anchor Delete retains its behavior; #1674: Delete removes every selected anchor of a closed path and objects selected whole along with them, and keeps the remainder selected |
+| Cut path at selected anchors, Remove Anchor Points, Delete removes segments | done | Open-path anchor Delete removes adjoining segments without bridging the gap; targeted core and full-frame Delete/Backspace regressions pass for survivor selection and Undo/Redo. Explicit Remove Anchor Points keeps its reconnect/refit behavior; #1278: segment Delete preserves neighboring edges and endpoint anchors; a second Delete removes the remainder; explicit anchor Delete retains its behavior; #1674: Delete removes every selected anchor of a closed path and objects selected whole along with them, and keeps the remainder selected; #2241: Remove Anchor Points then selects the next anchor along the path (the one before at an open end), so it can be run again without picking each anchor |
 | Direct Selection and Group Selection pick only inside the isolated object in isolation mode | done | #1666 |
 | Escape cancels a Direct Selection drag | done | #1803 (#1743) |
 | Live Corners widgets (radius readout, Alt-click cycles kind, double-click the Corners dialog) | done | a Direct Selection click on a widget selects its corner (#1239); a press-and-drag on a widget of a path selected whole still rounds every corner (#908), and adding widgets to the selection by Shift-click (#1232) is missing; the bounding box hides while a widget is dragged with the Selection tool (#1229); a round corner next to a curved side curves smoothly instead of sticking out of the shape (#1512) |