diff --git a/crates/engine/src/cmd/edit.rs b/crates/engine/src/cmd/edit.rs index 9ca8f6e9..3d6325d6 100644 --- a/crates/engine/src/cmd/edit.rs +++ b/crates/engine/src/cmd/edit.rs @@ -128,7 +128,7 @@ pub fn specs() -> Vec { "Clear", ["Edit"], Some("Delete"), - "{ids?} delete the objects `ids`, else the selection (direct-selected anchors, else objects, else ruler guides)", + "{ids?} delete the objects `ids`, else the selection (fully selected objects and direct-selected anchors together, else ruler guides)", has_selection_or_guides, clear ), @@ -368,7 +368,7 @@ fn cut(s: &mut Session, p: &Value) -> Result { fn clear(s: &mut Session, p: &Value) -> Result { let explicit = if p.get("ids").is_some() { Some(checked_ids_param(s, p, "ids", "edit.clear")?) } else { None }; - // Direct-selected anchors or segments: remove those components instead of whole objects. + // Direct-selected anchors or segments: remove those, and any objects selected whole with them. if explicit.is_none() && !s.doc()?.selection.anchors.is_empty() { return super::path::delete_anchors(s, p); } diff --git a/crates/engine/src/cmd/path.rs b/crates/engine/src/cmd/path.rs index 3adc84cc..bf1dcc10 100644 --- a/crates/engine/src/cmd/path.rs +++ b/crates/engine/src/cmd/path.rs @@ -91,7 +91,7 @@ pub fn specs() -> Vec { "Delete Anchor Points", [], None, - "{} delete the direct-selected anchors and their segments, opening closed paths there (the Delete key)", + "{} delete the direct-selected anchors and their segments, opening closed paths there, and delete objects selected whole alongside them; what remains of a path stays selected, so a second Delete removes it (the Delete key)", has_anchors, delete_anchors ), @@ -521,10 +521,27 @@ pub(crate) fn delete_anchors(s: &mut Session, _: &Value) -> Result { let st = s.doc()?; let sel = st.selection.anchors.clone(); let segments = st.selection.segments.clone(); + // Every anchor selected stores the path as an object, with no `anchors` entry. A marquee that + // also catches part of another path must still delete those whole objects. + let whole = st.selection.whole(); s.edit("Clear", |d, selection| { + let targets: BTreeSet = whole.into_iter().filter(|id| d.node(*id).is_some_and(|n| !n.is_layer())).collect(); + let whole_ids: Vec = targets + .iter() + .copied() + .filter(|id| !d.ancestry(*id).unwrap_or_default().iter().rev().skip(1).any(|ancestor| targets.contains(ancestor))) + .collect(); + if !whole_ids.is_empty() { + d.remove_many(&whole_ids)?; + } let mut survivors = vec![]; - let mut emptied = Beside::replacing(d, &sel.keys().copied().collect::>()); + let remaining: Vec = sel.keys().copied().filter(|id| d.node(*id).is_some()).collect(); + let mut emptied = Beside::replacing(d, &remaining); for (id, set) in &sel { + // The path went with a fully selected group around it. + if d.node(*id).is_none() { + continue; + } let remove_node = { let path = path_mut(d, *id)?; if let Some(refs) = segments.get(id) { @@ -538,39 +555,73 @@ pub(crate) fn delete_anchors(s: &mut Session, _: &Value) -> Result { }) .collect(); } else { - // Deleting an anchor from a closed path opens it there (Illustrator behaviour on Delete). - for &(si, ai) in set.iter().rev() { - if let Some(sp) = path.subpaths.get_mut(si) - && ai < sp.anchors.len() - { - if sp.closed { - sp.anchors.rotate_left(ai + 1); - sp.anchors.pop(); - sp.closed = false; - } else { - sp.anchors.remove(ai); - } - } - } + // Remove every selected anchor against the original indexes. A closed path opens + // across each deleted run and keeps the surviving stretches in their old order. + path.subpaths = std::mem::take(&mut path.subpaths) + .into_iter() + .enumerate() + .flat_map(|(si, sp)| { + let gone = set.iter().filter_map(|&(s, ai)| (s == si).then_some(ai)).collect(); + without_anchors(sp, &gone) + }) + .collect(); } path.subpaths.retain(|s| s.anchors.len() >= 2); path.subpaths.is_empty() }; if remove_node { emptied.put(d, *id, vec![])?; - } else if segments.contains_key(id) { + } else { + // The remainder is selected whole, so the next Delete removes it. survivors.push(*id); } } emptied.finish(d)?; - // Another Delete removes the remaining path after a segment cut. Anchor Delete keeps - // its existing deselection behavior. selection.set(survivors); Ok(()) })?; ok() } +/// Remove anchors `removed`. An open path stays one path and joins across the gap. A closed path +/// opens there: each stretch of anchors that were not deleted becomes its own open path, in the +/// original order, including a stretch that wraps past the last anchor. +fn without_anchors(sp: SubPath, removed: &BTreeSet) -> Vec { + let n = sp.anchors.len(); + let gone: BTreeSet = removed.iter().copied().filter(|&i| i < n).collect(); + if gone.is_empty() { + return vec![sp]; + } + let keep = |anchors: Vec| (anchors.len() >= 2).then(|| SubPath::new(anchors, false)); + if !sp.closed { + return keep(sp.anchors.into_iter().enumerate().filter(|(i, _)| !gone.contains(i)).map(|(_, a)| a).collect()).into_iter().collect(); + } + let mut runs: Vec> = vec![]; + let mut run = vec![]; + for (i, anchor) in sp.anchors.into_iter().enumerate() { + if gone.contains(&i) { + if !run.is_empty() { + runs.push(std::mem::take(&mut run)); + } + } else { + run.push(anchor); + } + } + // The anchors before the first deletion and after the last are one stretch. + if gone.contains(&0) { + if !run.is_empty() { + runs.push(run); + } + } else if !runs.is_empty() { + let head = runs.remove(0); + run.extend(head); + runs.insert(0, run); + } else if !run.is_empty() { + runs.push(run); + } + runs.into_iter().filter_map(keep).collect() +} + /// Remove connections, splitting open/closed contours into their surviving open runs. fn without_segments(sp: SubPath, removed: &BTreeSet) -> Vec { let cuts: BTreeSet = removed.iter().copied().filter(|&i| i < sp.segment_count()).collect(); diff --git a/crates/engine/tests/clear_commands.rs b/crates/engine/tests/clear_commands.rs index 80fa4660..1cb639e2 100644 --- a/crates/engine/tests/clear_commands.rs +++ b/crates/engine/tests/clear_commands.rs @@ -17,6 +17,66 @@ fn compound() -> (Session, NodeId, NodeId, NodeId) { (s, a, b, c) } +/// The two right-hand corners of a square are indexes 1 and 2. Delete must remove those corners +/// and leave the left side, not a diagonal from shifting indexes as the path opens. +#[test] +fn clear_deletes_every_selected_anchor_of_a_closed_path() { + let mut s = Session::new(); + s.execute("file.new", &json!({})).unwrap(); + let id = NodeId(s.execute("shape.rectangle", &json!({"x": 10, "y": 10, "width": 30, "height": 30})).unwrap()["id"].as_u64().unwrap()); + let points = |s: &Session| { + s.active().unwrap().doc.node(id).unwrap().path_data().unwrap().subpaths[0].anchors.iter().map(|a| (a.p.x, a.p.y)).collect::>() + }; + let before = points(&s); + s.execute("select.anchors", &json!({"id": id.0, "anchors": [[0, 1], [0, 2]]})).unwrap(); + s.execute("edit.clear", &json!({})).unwrap(); + let sp = &s.active().unwrap().doc.node(id).unwrap().path_data().unwrap().subpaths[0]; + assert!(!sp.closed); + assert_eq!(points(&s), vec![before[3], before[0]]); + assert_eq!(s.active().unwrap().selection.objects, vec![id]); + assert!(s.active().unwrap().selection.anchors.is_empty()); + s.execute("edit.clear", &json!({})).unwrap(); + assert!(s.active().unwrap().doc.node(id).is_none(), "a second Delete removes the remainder"); + s.execute("edit.undo", &json!({})).unwrap(); + s.execute("edit.undo", &json!({})).unwrap(); + assert_eq!(points(&s), before); + // One corner still opens the square into the other three, in order. + s.execute("select.anchors", &json!({"id": id.0, "anchors": [[0, 1]]})).unwrap(); + s.execute("edit.clear", &json!({})).unwrap(); + assert_eq!(points(&s), vec![before[2], before[3], before[0]]); +} + +/// A marquee that covers one path completely and only some anchors of another stores the first as +/// a whole object. Delete removes that object and the selected anchors of the other. +#[test] +fn clear_deletes_fully_selected_objects_along_with_partial_anchors() { + let mut s = Session::new(); + s.execute("file.new", &json!({})).unwrap(); + let mut rectangle = + |x| NodeId(s.execute("shape.rectangle", &json!({"x": x, "y": 10, "width": 30, "height": 30})).unwrap()["id"].as_u64().unwrap()); + let whole = rectangle(10); + let partial = rectangle(80); + let refs = |s: &Session, id| { + s.active().unwrap().doc.node(id).unwrap().path_data().unwrap().anchors().map(|(si, ai, _)| vec![si, ai]).collect::>() + }; + let all = refs(&s, whole); + let one = refs(&s, partial)[0].clone(); + let before = refs(&s, partial).len(); + s.execute("select.anchorsMany", &json!({"items": [{"id": whole.0, "anchors": all}, {"id": partial.0, "anchors": [one]}]})).unwrap(); + assert!(s.active().unwrap().selection.contains(whole) && s.active().unwrap().selection.partial(whole).is_none()); + s.execute("edit.clear", &json!({})).unwrap(); + assert!(s.active().unwrap().doc.node(whole).is_none()); + assert_eq!(refs(&s, partial).len(), before - 1); + assert_eq!(s.active().unwrap().selection.objects, vec![partial]); + assert!(s.active().unwrap().selection.anchors.is_empty()); + s.execute("edit.clear", &json!({})).unwrap(); + assert!(s.active().unwrap().doc.node(partial).is_none(), "a second Delete removes what the first left"); + s.execute("edit.undo", &json!({})).unwrap(); + s.execute("edit.undo", &json!({})).unwrap(); + assert!(s.active().unwrap().doc.node(whole).is_some()); + assert_eq!(refs(&s, partial).len(), before); +} + #[test] fn clear_selected_compound_member_preserves_its_sibling_and_is_undoable() { let (mut s, a, b, c) = compound();