fix(ui): Delete on an anchor selected with Add Anchor Point removes it, not the layer (#12703) (#12796)

* fix(ui): Delete on an anchor selected with Add Anchor Point removes it, not the layer (#12703)

With the Add Anchor Point tool and one anchor selected (Ctrl+click), Delete
and Backspace fell through to Edit > Clear and deleted the active layer.
They now remove the anchor and its two segments, as in the reference
application: a closed subpath opens there. One history step, Delete Anchor
Point.

- vector: edit::delete_anchor_and_segments (a closed subpath opens at the
  anchor; an open one loses an end or splits at an inner anchor)
- engine: path.deleteAnchor takes "segments": bool (default false keeps
  joining the neighbours, as the tool's click does)
- ui: delete_key_deletes_anchor next to delete_key_deletes_work_path, and a
  Delete/Backspace branch before the work path's

* test(vector): Delete on one anchor of a closed two-anchor subpath leaves the other, open (#12703)

The kept anchor retains its handles and smooth flag, and the subpath keeps its path operation.
This commit is contained in:
RickiNano authored and GitHub committed 2026-11-10 20:08:21 +01:00
1 parent 3e55928e63
commit 4af62e8abb
8 files changed
+269 -5

No files matched your search

@@ -50,6 +50,21 @@ fn deleting_a_saved_paths_last_anchor_leaves_it_empty_in_one_undoable_step() {
assert_eq!(s.execute("path.list", json!({})).unwrap()["paths"], json!([{"name": "Path 1", "knots": 0, "subpaths": 0}]));
}
/// ⌫ / Delete on the Add Anchor Point tool's selected anchor (`segments`, #12703) follows the same
/// rule for the path's only anchor, which has no segments.
#[test]
fn deleting_the_only_anchor_with_its_segments_empties_a_saved_path_and_refuses_the_work_path() {
let s = &mut session();
s.execute("path.set", json!({"name": "Path 1", "path": lone()})).unwrap();
let r = s.execute("path.deleteAnchor", json!({"name": "Path 1", "subpath": 0, "knot": 0, "segments": true})).unwrap();
assert_eq!(r["subpathRemoved"], json!(true));
assert!(doc(s).paths[0].path.subpaths.is_empty());
s.execute("path.set", json!({"path": lone()})).unwrap();
let e = s.execute("path.deleteAnchor", json!({"subpath": 0, "knot": 0, "segments": true})).unwrap_err();
assert!(e.to_string().contains("only anchor"), "{e}");
assert_eq!(doc(s).work_path.as_ref().map(|p| p.subpaths.len()), Some(1));
}
#[test]
fn the_work_paths_and_a_vector_masks_last_anchor_are_still_refused() {
let mut s = session();
+27 -2
View File
@@ -178,9 +178,18 @@ fn delete_anchor(s: &mut Session, p: &Value) -> Result<Value> {
// A saved path may lose its last anchor and stay listed, empty; the Work Path, a shape's path,
// a vector mask and a Type Path keep refusing it.
let saved = p.get("name").and_then(Value::as_str).is_some_and(|n| n != "layer" && !crate::vector_cmds::is_work(Some(n)));
let segments = p.get("segments").and_then(Value::as_bool).unwrap_or(false);
let mut removed = false;
let mut r = edit_path(s, CMD, p, "Delete Anchor Point", |path| {
removed = if saved { edit::delete_anchor_allow_empty(path, k)? } else { edit::delete_anchor(path, k)? };
// The path's only anchor has no segments: it goes, or is refused, as without `segments`.
let only = path.subpaths.iter().map(|sp| sp.knots.len()).fold(0, usize::saturating_add) <= 1;
removed = if segments && !only {
edit::delete_anchor_and_segments(path, k)?
} else if saved {
edit::delete_anchor_allow_empty(path, k)?
} else {
edit::delete_anchor(path, k)?
};
Ok(())
})?;
if let Some(o) = r.as_object_mut() {
@@ -294,7 +303,7 @@ pub fn specs() -> Vec<CommandSpec> {
spec(
"path.deleteAnchor",
"Delete Anchor Point",
r#""subpath":i,"knot":j (its neighbours are joined, keeping their handles; a subpath's last anchor removes the subpath; the path's only anchor empties a saved path and is refused elsewhere) (the result also has subpathRemoved:bool)"#.into(),
r#""subpath":i,"knot":j,"segments":bool=false (its neighbours are joined, keeping their handles; segments = ⌫/Delete on a selected anchor: its two segments go too, so a closed subpath opens there and an open one splits; a subpath's last anchor removes the subpath; the path's only anchor empties a saved path and is refused elsewhere) (the result also has subpathRemoved:bool)"#.into(),
delete_anchor,
),
spec(
@@ -470,6 +479,22 @@ mod tests {
assert_eq!((k.len(), k[0].out_ctrl, k[1].in_ctrl), (2, before.subpaths[0].knots[0].out_ctrl, before.subpaths[0].knots[2].in_ctrl));
}
#[test]
fn deleting_an_anchor_with_its_segments_opens_the_subpath_in_one_step() {
let mut s = session();
s.execute("path.set", json!({"path": square()})).unwrap();
let (before, steps) = (work(&s), s.active().unwrap().history.entries().len());
let r = s.execute("path.deleteAnchor", json!({"subpath": 0, "knot": 1, "segments": true})).unwrap();
assert_eq!(r["subpathRemoved"], json!(false));
let sp = &work(&s).subpaths[0];
assert_eq!((sp.knots.len(), sp.closed), (3, false));
assert_eq!(sp.knots[0].anchor, before.subpaths[0].knots[2].anchor, "it starts after the deleted anchor");
assert_eq!(s.active().unwrap().history.entries().len(), steps + 1);
assert_eq!(s.active().unwrap().history.undo_label(), Some("Delete Anchor Point"));
assert!(s.undo());
assert_eq!(work(&s), before);
}
#[test]
fn anchor_tools_edit_saved_paths_shapes_and_vector_masks() {
let mut s = session();
+23
View File
@@ -713,6 +713,29 @@ fn handle_pen_and_work_path_keys(app: &mut PhotocraftApp, ctx: &egui::Context, f
crate::shape_bounds_box::cancel(app);
return true;
}
// An anchor selected with the Add Anchor Point tool: ⌫/Delete removes it and its two segments,
// as in Photoshop (#12703), never the active layer.
if focus != crate::shortcut_dispatch::Focus::DeletingWidget
&& let Some((target, [s, k])) = crate::vector_ui::delete_key_deletes_anchor(app)
{
let mods = ctx.input(|i| i.modifiers);
if !mods.command && !mods.ctrl && !mods.shift && !mods.alt && ctx.input_mut(|i| i.consume_key(mods, Key::Backspace) || i.consume_key(mods, Key::Delete))
{
let mut params = json!({"subpath": s, "knot": k, "segments": true});
if let (Some(p), Some(t)) = (params.as_object_mut(), target.params(app).as_object()) {
p.extend(t.clone());
}
match app.run("path.deleteAnchor", params) {
// The anchors shift: the selection would point at the wrong ones.
Ok(_) => app.ui.direct_selection = crate::direct_select::DirectSelection::default(),
Err(e) => {
app.ui.status = e;
app.ui.status_error = true;
}
}
return true;
}
}
// A finished work path shown by the Pen or Path Selection: ⌫/Delete deletes it, as in
// Photoshop (#5702), never Edit › Clear on the pixels under it.
if focus != crate::shortcut_dispatch::Focus::DeletingWidget && crate::vector_ui::delete_key_deletes_work_path(app) {
+3
View File
@@ -187,3 +187,6 @@ fn draw_hit(painter: &egui::Painter, to_scr: &dyn Fn([f64; 2]) -> Pos2, accent:
#[cfg(test)]
mod tests;
#[cfg(test)]
mod add_anchor_delete_key_tests;
@@ -0,0 +1,98 @@
//! #12703: with the Add Anchor Point tool, ⌫/Delete on a selected anchor removes the anchor and
//! its two segments (a closed subpath opens there), never the active layer.
use egui::Modifiers;
use photocraft_doc::vector::Path;
use serde_json::json;
use crate::PhotocraftApp;
use crate::canvas::{ToolEvent, tool_event};
use crate::state::Tool;
/// 800 x 600 on white, pixel layer Red active, saved path Path 1 (a closed corner square)
/// targeted in the Paths panel, the Add Anchor Point tool on.
fn app() -> PhotocraftApp {
let mut app = PhotocraftApp::new(photocraft_engine::Session::new(), crate::Services::default());
app.session.execute("file.new", json!({"width": 800, "height": 600, "background": "white"})).unwrap();
app.sync_views();
app.run("layer.new.layer", json!({"name": "Red"})).unwrap();
let square = json!({"subpaths": [{"closed": true, "knots": [[100, 300], [300, 300], [300, 500], [100, 500]]}]});
app.run("path.set", json!({"name": "Path 1", "path": square})).unwrap();
app.ui.selected_path = Some("Path 1".into());
app.ui.tool = Tool::AddAnchorPoint;
app
}
fn path1(app: &PhotocraftApp) -> Path {
app.session.active().unwrap().doc.paths.iter().find(|p| p.name == "Path 1").unwrap().path.clone()
}
fn layers(app: &PhotocraftApp) -> usize {
app.session.active().unwrap().doc.layers.len()
}
fn steps(app: &PhotocraftApp) -> usize {
app.session.active().unwrap().history.past_len()
}
fn ctrl_click(app: &mut PhotocraftApp, at: [f64; 2]) {
tool_event(app, ToolEvent::Down { x: at[0], y: at[1], pressure: 1.0 }, Modifiers::COMMAND);
tool_event(app, ToolEvent::Up { x: at[0], y: at[1] }, Modifiers::COMMAND);
}
/// A key through the app's shortcut handling, as the canvas gets it.
fn press(app: &mut PhotocraftApp, key: egui::Key) {
let ctx = egui::Context::default();
let m = Modifiers::NONE;
let raw = egui::RawInput {
events: vec![egui::Event::ModifiersChanged(m), egui::Event::Key { key, physical_key: None, pressed: true, repeat: false, modifiers: m }],
..Default::default()
};
ctx.begin_pass(raw);
crate::shortcuts::handle(app, &ctx);
ctx.end_pass().textures_delta.clear();
}
fn anchors(path: &Path) -> Vec<[f64; 2]> {
path.subpaths.iter().flat_map(|sp| sp.knots.iter().map(|k| [k.anchor.x, k.anchor.y])).collect()
}
#[test]
fn delete_on_a_selected_anchor_removes_it_and_its_segments_not_the_layer() {
for key in [egui::Key::Delete, egui::Key::Backspace] {
let mut app = app();
let (before, n, count) = (path1(&app), steps(&app), layers(&app));
ctrl_click(&mut app, [300.0, 500.0]);
assert_eq!(app.ui.direct_selection.anchors, vec![[0, 2]], "{key:?}: Ctrl+click selects the anchor");
press(&mut app, key);
assert_eq!(layers(&app), count, "{key:?}: every layer is still there");
let p = path1(&app);
assert_eq!((p.subpaths.len(), p.subpaths[0].closed), (1, false), "{key:?}: the square opens");
assert_eq!(anchors(&p), vec![[100.0, 500.0], [100.0, 300.0], [300.0, 300.0]], "{key:?}");
let st = app.session.active().unwrap();
assert_eq!((st.history.past_len(), st.history.undo_label()), (n + 1, Some("Delete Anchor Point")), "{key:?}");
assert!(app.session.undo());
assert_eq!(path1(&app), before, "{key:?}: Undo restores the closed square");
}
}
#[test]
fn without_a_selected_anchor_delete_keeps_its_usual_route() {
let mut app = app();
let before = path1(&app);
press(&mut app, egui::Key::Delete);
assert_eq!(path1(&app), before, "no anchor selected: the path stays");
assert_ne!(app.session.active().unwrap().history.undo_label(), Some("Delete Anchor Point"));
}
#[test]
fn other_tools_keep_their_delete() {
for tool in [Tool::Pen, Tool::FreeformPen, Tool::PathSelection] {
let mut app = app();
ctrl_click(&mut app, [300.0, 500.0]);
app.ui.tool = tool;
let before = path1(&app);
press(&mut app, egui::Key::Delete);
assert_eq!(path1(&app), before, "{tool:?}: no anchor is deleted");
}
}
+2 -1
View File
@@ -45,7 +45,8 @@ pub use path_ops::{
active_path_name, make_shape, path_selection_finish, path_to_selection, pick_shape_at, selected_document_path, takes_path_mask, with_active_path,
};
pub(crate) use path_ops::{
active_shape_path, active_type_path, delete_key_deletes_work_path, deselect_loaded_path, free_transform_path, targeted_vector_mask, work_path_shown,
active_shape_path, active_type_path, delete_key_deletes_anchor, delete_key_deletes_work_path, deselect_loaded_path, free_transform_path,
targeted_vector_mask, work_path_shown,
};
#[cfg(test)]
use paths_panel::path_context_actions;
@@ -216,6 +216,25 @@ pub(crate) fn delete_key_deletes_work_path(app: &PhotocraftApp) -> bool {
&& matches!(target_path(app), Some((PathTarget::Work, _)))
}
/// The anchor ⌫ / Delete removes, with its two segments: with the Add Anchor Point tool, the one
/// anchor selected on it (⌘/Ctrl+click) while that selection still holds and its path is shown,
/// as in Photoshop (#12703). The key reached Edit › Clear and deleted the active layer. Several
/// selected anchors weren't measured and keep the key's other routes.
pub(crate) fn delete_key_deletes_anchor(app: &PhotocraftApp) -> Option<(crate::direct_select::PathRef, [usize; 2])> {
let ds = &app.ui.direct_selection;
let ([[s, k]], Some(target)) = (ds.anchors.as_slice(), ds.target) else { return None };
let usable = app.ui.tool == Tool::AddAnchorPoint
&& app.ui.pen.is_none()
&& ds.drag.is_none()
&& app.ui.view.shows(app.ui.view.show.target_path)
&& crate::direct_select::still_valid(app, ds)
&& crate::direct_select::writable(app, &target);
usable.then_some(())?;
let (_, path) = crate::direct_select::candidates(app).into_iter().find(|(r, _)| *r == target)?;
path.subpaths.get(*s)?.knots.get(*k)?;
Some((target, [*s, *k]))
}
/// The path Path Selection edits: the targeted vector mask, the active shape layer's path, the
/// saved path selected in the Paths panel (#8478), else the work path.
pub(super) fn target_path(app: &PhotocraftApp) -> Option<(PathTarget, Path)> {
+82 -2
View File
@@ -6,7 +6,7 @@
//! Knots are addressed as `[subpath, knot]`. Every edit validates its indices and returns an
//! error message instead of panicking.
use photocraft_doc::{Knot, Path};
use photocraft_doc::{Knot, Path, Subpath};
use photocraft_geom::Point;
/// One of a knot's two direction handles.
@@ -266,6 +266,35 @@ pub fn delete_anchor_allow_empty(path: &mut Path, [s, k]: [usize; 2]) -> Result<
Ok(false)
}
/// ⌫ / Delete on a selected anchor: removes the knot and the two segments that meet at it, where
/// Delete Anchor Point joins its neighbours. A closed subpath opens there and starts at the next
/// anchor; an open subpath loses an end segment, or splits in two at an inner anchor. The other
/// knots keep their handles. A subpath's last anchor and the path's only anchor go as in
/// [`delete_anchor`]. Returns whether the subpath was removed.
pub fn delete_anchor_and_segments(path: &mut Path, [s, k]: [usize; 2]) -> Result<bool, String> {
knot_mut(path, [s, k])?;
let total: usize = path.subpaths.iter().map(|sp| sp.knots.len()).fold(0, usize::saturating_add);
let sp = path.subpaths.get_mut(s).ok_or_else(|| format!("no subpath {s}"))?;
if total <= 1 || sp.knots.len() <= 1 {
return delete_anchor(path, [s, k]);
}
// `k` < len: `knot_mut` found it.
let after = sp.knots.split_off(k);
let after = after.get(1..).unwrap_or_default().to_vec();
if sp.closed {
// Open at `k`: the knots after it, then those before.
sp.knots = after.into_iter().chain(std::mem::take(&mut sp.knots)).collect();
sp.closed = false;
} else if sp.knots.is_empty() {
sp.knots = after;
} else if !after.is_empty() {
let rest = Subpath { closed: false, knots: after, op: sp.op };
let at = s.checked_add(1).ok_or_else(|| format!("no subpath after {s}"))?;
path.subpaths.insert(at, rest);
}
Ok(false)
}
/// Every anchor of subpath `s` (⌥-click with Direct Selection selects the whole subpath).
pub fn subpath_anchors(path: &Path, s: usize) -> Vec<[usize; 2]> {
path.subpaths.get(s).map_or_else(Vec::new, |sp| (0..sp.knots.len()).map(|k| [s, k]).collect())
@@ -375,7 +404,7 @@ pub fn hit(path: &Path, p: Point, tol: f64, handles: &[([usize; 2], Handle)]) ->
#[cfg(test)]
mod tests {
use super::*;
use photocraft_doc::Subpath;
use photocraft_doc::{PathOp, Subpath};
fn tri() -> Path {
Path::new(vec![Subpath::polygon(&[(0.0, 0.0), (100.0, 0.0), (50.0, 80.0)])])
@@ -599,6 +628,57 @@ mod tests {
assert_eq!(c.subpaths[0].segments(), vec![[Point::new(0.0, 50.0), Point::new(10.0, 20.0), Point::new(90.0, 20.0), Point::new(100.0, 50.0)]]);
}
#[test]
fn deleting_with_segments_opens_a_closed_subpath_at_the_anchor() {
let square = || Path::new(vec![Subpath::polygon(&[(100.0, 300.0), (300.0, 300.0), (300.0, 500.0), (100.0, 500.0)])]);
let mut p = square();
assert!(!delete_anchor_and_segments(&mut p, [0, 2]).unwrap());
assert_eq!(p, Path::new(vec![Subpath::polyline(&[(100.0, 500.0), (100.0, 300.0), (300.0, 300.0)])]));
let mut p = square();
delete_anchor_and_segments(&mut p, [0, 1]).unwrap();
assert_eq!(p, Path::new(vec![Subpath::polyline(&[(300.0, 500.0), (100.0, 500.0), (100.0, 300.0)])]));
}
#[test]
fn deleting_with_segments_leaves_one_open_anchor_of_a_closed_pair() {
let a = Knot::smooth(Point::new(0.0, 0.0), Point::new(-5.0, 5.0), Point::new(5.0, -5.0));
let b = Knot::smooth(Point::new(10.0, 0.0), Point::new(15.0, 5.0), Point::new(5.0, 5.0));
let pair = Subpath { closed: true, knots: vec![a, b], op: PathOp::Subtract };
for (k, kept) in [(0, b), (1, a)] {
let mut p = Path::new(vec![pair.clone()]);
assert!(!delete_anchor_and_segments(&mut p, [0, k]).unwrap());
assert_eq!(p, Path::new(vec![Subpath { closed: false, knots: vec![kept], op: PathOp::Subtract }]), "k={k}");
}
}
#[test]
fn deleting_with_segments_trims_or_splits_an_open_subpath() {
let line = || Path::new(vec![Subpath::polyline(&[(0.0, 0.0), (10.0, 0.0), (20.0, 0.0), (30.0, 0.0)])]);
let mut p = line();
delete_anchor_and_segments(&mut p, [0, 0]).unwrap();
assert_eq!(p, Path::new(vec![Subpath::polyline(&[(10.0, 0.0), (20.0, 0.0), (30.0, 0.0)])]));
let mut p = line();
delete_anchor_and_segments(&mut p, [0, 3]).unwrap();
assert_eq!(p, Path::new(vec![Subpath::polyline(&[(0.0, 0.0), (10.0, 0.0), (20.0, 0.0)])]));
let mut p = line();
delete_anchor_and_segments(&mut p, [0, 1]).unwrap();
assert_eq!(p, Path::new(vec![Subpath::polyline(&[(0.0, 0.0)]), Subpath::polyline(&[(20.0, 0.0), (30.0, 0.0)])]));
}
#[test]
fn deleting_with_segments_keeps_the_last_anchor_rules_and_bad_indices() {
let mut p = tri();
p.subpaths.push(Subpath::polyline(&[(5.0, 5.0)]));
assert!(delete_anchor_and_segments(&mut p, [1, 0]).unwrap());
assert_eq!(p, tri());
let mut one = Path::new(vec![Subpath::polyline(&[(5.0, 5.0)])]);
assert!(delete_anchor_and_segments(&mut one, [0, 0]).is_err());
for k in [[0, 3], [1, 0], [usize::MAX, usize::MAX]] {
assert!(delete_anchor_and_segments(&mut p, k).is_err(), "{k:?}");
}
assert_eq!(p, tri());
}
#[test]
fn deleting_a_subpaths_last_anchor_removes_it_but_never_the_last_anchor() {
let mut p = tri();