Place: only a DCS clipping path turns the plain group back into a clip group

A plain group holding just the image becomes its clip group again only
when the image takes its DCS file's clipping path. When Placement Options
clip the image, it goes into a new clip group as before, so a group the
user made around the image alone stays a plain group.

crates/engine/src/cmd/links.rs

@@ -609,9 +609,13 @@ fn clip_shape((frame, room): Clip) -> PathData {
 enum ClipTo {
     /// It stays as it is.
     Keep,
-    /// It is clipped to this path: the clipping path of the clip group holding just it changes,
-    /// else it goes into a new clip group.
+    /// It is clipped to this path (Placement Options' clip): the clipping path of the clip group
+    /// holding just it changes, else it goes into a new clip group.
     Path(PathData),
+    /// It is clipped to this path, the clipping path its DCS file draws it through: as [`ClipTo::Path`],
+    /// except that a plain group holding just it (as [`ClipTo::Off`] leaves one) becomes a clip group
+    /// again.
+    File(PathData),
     /// It goes into a new clip group of this path, inside the clip group that holds it.
     Nest(PathData),
     /// The clipping path of the clip group holding just it goes (a DCS file's clipping path that
@@ -673,7 +677,7 @@ fn clip_to(
     match img.clip.as_ref().map(|path| path.transformed(new_xf)) {
         // A clipping mask made in the document stays, with the file's clip group inside it.
         Some(path) if held.is_some() && !file_held => ClipTo::Nest(path),
-        Some(path) => ClipTo::Path(path),
+        Some(path) => ClipTo::File(path),
         None if file_held => ClipTo::Off,
         None => ClipTo::Keep,
     }
@@ -722,7 +726,7 @@ impl Nested<'_> {
         let mut new = Arc::new(new);
         self.update(&mut new, true);
         *n = match clip {
-            ClipTo::Path(path) | ClipTo::Nest(path) => Arc::new(self.clip_group(new, path)),
+            ClipTo::Path(path) | ClipTo::File(path) | ClipTo::Nest(path) => Arc::new(self.clip_group(new, path)),
             ClipTo::Keep | ClipTo::Off => new,
         };
     }
@@ -766,15 +770,15 @@ impl Nested<'_> {
                 self.update(&mut new, true);
                 match (clip, clip_path) {
                     (ClipTo::Keep, _) => slots.push((i, new)),
-                    (ClipTo::Path(clip), Some(path)) if i == 1 => {
+                    (ClipTo::Path(clip) | ClipTo::File(clip), Some(path)) if i == 1 => {
                         let mut path = Node::clone(path);
                         if let NodeKind::Path { path: p, .. } = &mut path.kind {
                             *p = clip;
                         }
                         slots.extend([(0, Arc::new(path)), (1, new)]);
                     }
-                    (ClipTo::Path(clip), None) if plain => reclipped = Some((clip, new)),
-                    (ClipTo::Path(clip) | ClipTo::Nest(clip), _) => {
+                    (ClipTo::File(clip), None) if plain => reclipped = Some((clip, new)),
+                    (ClipTo::Path(clip) | ClipTo::File(clip) | ClipTo::Nest(clip), _) => {
                         let group = self.clip_group(new, clip);
                         slots.push((i, Arc::new(group)));
                     }
@@ -952,9 +956,18 @@ fn clip_image(d: &mut Document, id: NodeId, clip: Clip) -> Result<()> {
 }
 
 /// Clip object `id` of `d` to `path`: the clipping path of the clip group holding just it changes,
-/// a plain group holding just it (as [`unclip`] leaves one) becomes a clip group again, else it
-/// goes into a new clip group.
+/// else it goes into a new clip group.
 fn clip_image_to(d: &mut Document, id: NodeId, path: PathData) -> Result<()> {
+    if set_clip_path(d, id, path.clone()) {
+        return Ok(());
+    }
+    new_clip_group(d, id, path)
+}
+
+/// Clip object `id` of `d` to `path`, the clipping path its DCS file draws it through: the clipping
+/// path of the clip group holding just it changes, a plain group holding just it (as [`unclip`]
+/// leaves one) becomes a clip group again, else it goes into a new clip group.
+fn clip_file_to(d: &mut Document, id: NodeId, path: PathData) -> Result<()> {
     if set_clip_path(d, id, path.clone()) {
         return Ok(());
     }
@@ -1050,6 +1063,7 @@ fn install(d: &mut Document, ids: &[NodeId], (link, img): FileRead, force: Optio
     clips.into_iter().try_for_each(|(id, clip)| match clip {
         ClipTo::Keep => Ok(()),
         ClipTo::Path(path) => clip_image_to(d, id, path),
+        ClipTo::File(path) => clip_file_to(d, id, path),
         ClipTo::Nest(path) => new_clip_group(d, id, path),
         ClipTo::Off => unclip(d, id),
     })

crates/engine/src/tests/links.rs

@@ -576,6 +576,42 @@ fn update_links_reads_files_again_inside_symbols_patterns_and_masks() {
     assert!(images(&s).iter().all(|(_, _, _, size)| *size == (20, 10)), "{:?}", images(&s));
 }
 
+/// An image that Placement Options clip, alone in a group the user made, goes into a new clip group
+/// inside that group when its file grows, in the layers and in a symbol's art; the user's group
+/// stays a plain group.
+#[test]
+fn placement_options_clip_an_image_alone_in_a_group_inside_that_group() {
+    let dir = Folder::new("clip-in-group");
+    let photo = dir.file("photo.png");
+    write(&photo, &png(20, 10, RED));
+    let mut s = session();
+    let mut groups = vec![];
+    for _ in 0..2 {
+        let id = place(&mut s, &photo);
+        s.execute("links.placementOptions", &json!({"ids": [id.0], "preserve": "fill", "clip": true})).unwrap();
+        let g = NodeId(s.execute("object.group", &json!({"ids": [id.0]})).unwrap()["id"].as_u64().unwrap());
+        groups.push((g, id));
+    }
+    let (in_symbol, _) = groups[1];
+    s.execute("symbol.new", &json!({"name": "Mark", "ids": [in_symbol.0]})).unwrap();
+    write(&photo, &png(60, 20, BLUE));
+    s.execute("links.update", &json!({})).unwrap();
+    // A plain group holding just a clip group of a clipping path and an image.
+    let plain_of_clip = |n: &vectorcraft_doc::Node| match &n.kind {
+        NodeKind::Group { children, clip: false } => matches!(&children[..], [c] if matches!(&c.kind,
+            NodeKind::Group { children, clip: true } if matches!(&children[..], [p, im]
+                if matches!(p.kind, NodeKind::Path { clipping: true, .. }) && matches!(im.kind, NodeKind::Image(_))))),
+        _ => false,
+    };
+    let d = &s.doc().unwrap().doc;
+    let (g, id) = groups[0];
+    assert!(plain_of_clip(d.node(g).unwrap()), "{:?}", d.node(g));
+    assert_ne!(d.parent_of(id), Some(g), "the image is in a new clip group");
+    let mut found = false;
+    d.symbols[0].art.walk(&mut |n| found |= plain_of_clip(n));
+    assert!(found, "{:?}", d.symbols[0].art);
+}
+
 #[test]
 fn a_missing_link_renders_its_preview_and_relinks() {
     let dir = Folder::new("missing");