From dc92be239173ba7f7a50a43d2d11bd036d6615be Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Thu, 12 Nov 2026 22:02:31 -0800 Subject: [PATCH] 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 | 32 ++++++++++++++++++++-------- crates/engine/src/tests/links.rs | 36 ++++++++++++++++++++++++++++++++ 2 files changed, 59 insertions(+), 9 deletions(-) diff --git a/crates/engine/src/cmd/links.rs b/crates/engine/src/cmd/links.rs index 91ec153049..6167ca6f71 100644 --- a/crates/engine/src/cmd/links.rs +++ b/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), }) diff --git a/crates/engine/src/tests/links.rs b/crates/engine/src/tests/links.rs index a3c7a72aee..9787401285 100644 --- a/crates/engine/src/tests/links.rs +++ b/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");