diff --git a/crates/engine/src/cmd/draw2.rs b/crates/engine/src/cmd/draw2.rs index 8d699798d..24ffb39db 100644 --- a/crates/engine/src/cmd/draw2.rs +++ b/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 { "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 { let mut removed = 0; let mut emptied = Beside::replacing(d, &jobs.iter().map(|(id, _)| *id).collect::>()); 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 { 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 = 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. diff --git a/crates/engine/src/tests_draw2.rs b/crates/engine/src/tests_draw2.rs index 053f6b2f4..bf600eeca 100644 --- a/crates/engine/src/tests_draw2.rs +++ b/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 { + 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, ¶ms).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() } diff --git a/crates/ui-egui/src/tests_removeanchors.rs b/crates/ui-egui/src/tests_removeanchors.rs index bb298edcc..a809f174d 100644 --- a/crates/ui-egui/src/tests_removeanchors.rs +++ b/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] diff --git a/docs/mcp.md b/docs/mcp.md index 8fe40600f..344fef8a6 100644 --- a/docs/mcp.md +++ b/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. diff --git a/docs/ui-parity.md b/docs/ui-parity.md index d465f3d86..e88640d86 100644 --- a/docs/ui-parity.md +++ b/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) |