Merge pull request #5197 from dotCooCoo/fix/cleanup-leaves-hidden-locked-art

Artwork Cleanup: Simplify Anchors and Deep Ungroup leave hidden and locked art alone

crates/engine/src/cmd/addons/cleanup.rs

@@ -37,7 +37,7 @@ pub fn specs() -> Vec<CommandSpec> {
             "Simplify Anchors (Keep Shape)…",
             ["Object", "Plug-ins", "Artwork Cleanup"],
             None,
-            "{tolerance?: 1..100 (20; higher removes more anchors and lets the outline move further from where it was, up to tolerance × 0.02 pt), protectCorners?: bool (false; anchors where the path turns by more than 30° stay), ids?: [id…] (default: the selection)} remove the anchors each path under the objects can do without, refitting the handles beside each so the outline keeps its shape, as one undo step; open ends stay → {ids, before, after} anchor counts",
+            "{tolerance?: 1..100 (20; higher removes more anchors and lets the outline move further from where it was, up to tolerance × 0.02 pt), protectCorners?: bool (false; anchors where the path turns by more than 30° stay), ids?: [id…] (default: the selection)} remove the anchors each path under the objects can do without, refitting the handles beside each so the outline keeps its shape, as one undo step; open ends stay, and locked and hidden paths are left out → {ids, before, after} anchor counts",
             has_selection,
             simplify
         ),
@@ -73,7 +73,7 @@ pub fn specs() -> Vec<CommandSpec> {
             "Deep Ungroup…",
             ["Object", "Plug-ins", "Artwork Cleanup"],
             None,
-            "{includeClipGroups?: bool (false: clipping groups, and the art inside them, stay as they are; true: they are ungrouped too and their clipping paths become plain paths), ids?: [id…] (default: the selection)} ungroup every level of the groups among the objects, as one undo step; each object keeps the opacity of the groups it was in → {ids (the objects left, selected), ungrouped (groups)}",
+            "{includeClipGroups?: bool (false: clipping groups, and the art inside them, stay as they are; true: they are ungrouped too and their clipping paths become plain paths), ids?: [id…] (default: the selection)} ungroup every level of the groups among the objects, as one undo step; each object keeps the opacity of the groups it was in; locked and hidden groups stay whole, and locked and hidden objects stay locked or hidden and are not selected → {ids (the objects selected: those left, or with no group to take apart the selection as it was), ungrouped (groups)}",
             has_selection,
             deep_ungroup
         ),
@@ -91,7 +91,8 @@ pub fn specs() -> Vec<CommandSpec> {
     v
 }
 
-/// The paths (not guides) under `roots`, compound path members included.
+/// The paths (not guides) under `roots`, compound path members included; locked and hidden
+/// paths, and those in locked or hidden groups, stay out.
 fn paths_under(d: &Document, roots: &[NodeId]) -> Vec<NodeId> {
     let mut out = vec![];
     for n in roots.iter().filter_map(|id| d.node(*id)) {
@@ -101,6 +102,8 @@ fn paths_under(d: &Document, roots: &[NodeId]) -> Vec<NodeId> {
             }
         });
     }
+    let editable = d.editable();
+    out.retain(|id| editable(*id));
     out
 }
 
@@ -256,9 +259,9 @@ fn ungroupable(n: &Node, clips: bool) -> bool {
 
 /// `n` with every level of grouping Deep Ungroup takes apart removed, into `out`; `count` counts
 /// the groups. Members take on the opacity of the groups they were in, and a clipping path
-/// becomes a plain path.
+/// becomes a plain path. A locked or hidden group stays whole.
 fn flatten(n: Node, clips: bool, out: &mut Vec<Node>, count: &mut usize) {
-    if !ungroupable(&n, clips) {
+    if !ungroupable(&n, clips) || n.locked || !n.visible {
         out.push(n);
         return;
     }
@@ -282,6 +285,11 @@ fn deep_ungroup(s: &mut Session, p: &Value) -> Result<Value> {
     let clips = switch_or(p, "includeClipGroups", false, C)?;
     let roots = checked_ids_or_selection(s, p, C)?;
     let d = &s.doc()?.doc;
+    // Locked and hidden objects, and those in locked or hidden groups, stay as they are.
+    let roots: Vec<NodeId> = {
+        let editable = d.editable();
+        roots.into_iter().filter(|id| editable(*id)).collect()
+    };
     let (mut items, mut selected, mut count) = (vec![], vec![], 0);
     for id in &roots {
         match d.node(*id).filter(|n| ungroupable(n, clips)) {
@@ -294,13 +302,18 @@ fn deep_ungroup(s: &mut Session, p: &Value) -> Result<Value> {
             None => selected.push(*id),
         }
     }
-    // With no group to take apart, the document and its undo history stay as they are.
-    if !items.is_empty() {
-        let ids = selected.clone();
-        s.edit("Deep Ungroup", |d, sel| {
+    // With no group to take apart, the document, the selection and the undo history stay as they
+    // are, and the reply lists the selection.
+    if items.is_empty() {
+        selected = s.doc()?.selection.objects.to_vec();
+    } else {
+        selected = s.edit("Deep Ungroup", move |d, sel| {
             d.insert_beside(items, true)?;
-            sel.set(ids);
-            Ok(())
+            // Members that are locked or hidden themselves keep that, and are not selected.
+            let editable = d.editable();
+            selected.retain(|id| editable(*id));
+            sel.set(selected.iter().copied());
+            Ok(selected)
         })?;
     }
     Ok(json!({ "ids": selected.iter().map(|i| i.0).collect::<Vec<_>>(), "ungrouped": count }))

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

@@ -104,6 +104,39 @@ fn simplify_keeps_the_shape_with_fewer_anchors_in_one_step() {
     }
 }
 
+#[test]
+fn simplify_leaves_locked_and_hidden_paths_alone() {
+    let mut s = session();
+    let mut ids = vec![];
+    for x in [20.0, 140.0, 260.0] {
+        ids.push(id_of(&s.execute("shape.ellipse", &json!({"x": x, "y": 50, "width": 100, "height": 100})).unwrap()));
+        s.execute("object.path.addAnchorPoints", &json!({})).unwrap();
+        s.execute("object.path.addAnchorPoints", &json!({})).unwrap();
+    }
+    select(&mut s, &ids);
+    let g = id_of(&s.execute("object.group", &json!({})).unwrap());
+    change(&mut s, ids[1], |n| n.locked = true);
+    change(&mut s, ids[2], |n| n.visible = false);
+    select(&mut s, &[g]);
+    let r = s.execute("addon.cleanup.simplify", &json!({})).unwrap();
+    assert_eq!(r["ids"], json!([ids[0].0]));
+    assert_eq!(r["before"], 16);
+    assert!(anchors(&s, ids[0]) < 16);
+    assert_eq!((anchors(&s, ids[1]), anchors(&s, ids[2])), (16, 16));
+    // Named by `ids`, or in a locked group, a path stays too, with no undo step.
+    let steps = undo_steps(&s);
+    s.execute("addon.cleanup.simplify", &json!({"ids": [ids[1].0, ids[2].0]})).unwrap();
+    assert_eq!((anchors(&s, ids[1]), anchors(&s, ids[2])), (16, 16));
+    assert_eq!(undo_steps(&s), steps);
+    change(&mut s, ids[1], |n| n.locked = false);
+    change(&mut s, g, |n| n.locked = true);
+    let steps = undo_steps(&s);
+    let r = s.execute("addon.cleanup.simplify", &json!({"ids": [g.0]})).unwrap();
+    assert_eq!((r["before"].as_u64(), r["after"].as_u64()), (Some(0), Some(0)));
+    assert_eq!(anchors(&s, ids[1]), 16);
+    assert_eq!(undo_steps(&s), steps);
+}
+
 /// Each kind: what makes the issue in a fresh document (beside a clean rectangle).
 fn make_issue(s: &mut Session, kind: &str) {
     match kind {
@@ -333,6 +366,51 @@ fn deep_ungroup_takes_apart_every_level_but_clipping_groups() {
     assert!(matches!(d.node(clip).unwrap().kind, NodeKind::Path { clipping: false, .. }));
 }
 
+#[test]
+fn deep_ungroup_keeps_hidden_art_hidden_and_locked_art_locked() {
+    let mut s = session();
+    let a = rect(&mut s, 10.0, 10.0, 20.0, 20.0);
+    let b = rect(&mut s, 40.0, 10.0, 20.0, 20.0);
+    let c = rect(&mut s, 70.0, 10.0, 20.0, 20.0);
+    let e = rect(&mut s, 100.0, 10.0, 20.0, 20.0);
+    select(&mut s, &[b]);
+    let hidden = id_of(&s.execute("object.group", &json!({})).unwrap());
+    select(&mut s, &[c]);
+    let locked = id_of(&s.execute("object.group", &json!({})).unwrap());
+    change(&mut s, hidden, |n| n.visible = false);
+    change(&mut s, locked, |n| n.locked = true);
+    change(&mut s, e, |n| n.locked = true);
+    s.edit("t", |d, _| {
+        let w = d.alloc_id();
+        Ok(d.wrap(&[a, hidden, locked, e], |ch| Node::group(w, ch))?)
+    })
+    .unwrap();
+    let outer = s.doc().unwrap().doc.layers[0].children().unwrap()[0].id;
+    select(&mut s, &[outer]);
+    let r = s.execute("addon.cleanup.deepUngroup", &json!({})).unwrap();
+    assert_eq!(r["ungrouped"], 1);
+    assert_eq!(r["ids"], json!([a.0]));
+    let d = &s.doc().unwrap().doc;
+    let layer: Vec<NodeId> = d.layers[0].children().unwrap().iter().map(|n| n.id).collect();
+    assert_eq!(layer, [a, hidden, locked, e], "the hidden and locked groups stay whole");
+    assert!(!d.node(hidden).unwrap().visible && d.node(b).unwrap().visible);
+    assert!(d.node(locked).unwrap().locked && d.node(e).unwrap().locked);
+    assert_eq!(s.doc().unwrap().selection.objects, [a]);
+    // Named by `ids`, a locked group stays as it is, with no undo step.
+    let steps = undo_steps(&s);
+    let r = s.execute("addon.cleanup.deepUngroup", &json!({"ids": [locked.0]})).unwrap();
+    assert_eq!(r["ungrouped"], 0);
+    assert!(node(&s, locked).is_some());
+    assert_eq!(undo_steps(&s), steps);
+    // With nothing ungrouped, the selection stays and the reply lists it.
+    assert_eq!(r["ids"], json!([a.0]));
+    let f = rect(&mut s, 130.0, 10.0, 20.0, 20.0);
+    select(&mut s, &[a]);
+    let r = s.execute("addon.cleanup.deepUngroup", &json!({"ids": [f.0]})).unwrap();
+    assert_eq!((r["ids"].clone(), r["ungrouped"].clone()), (json!([a.0]), json!(0)));
+    assert_eq!(s.doc().unwrap().selection.objects, [a]);
+}
+
 #[test]
 fn rejoin_joins_pieces_within_the_gap_only() {
     let mut s = session();