From e4fdc10b994a978c1a5369786787efd50f22ebe4 Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Thu, 12 Nov 2026 21:30:06 -0800 Subject: [PATCH 1/2] Place: a DCS file that regains its clipping path clips the group that stayed When a linked file lost its clipping path, the clip group holding the image stayed as a plain group of the image. When the file had a clipping path again, the image went into a new clip group inside that plain group, so each round nested one more group. The plain group holding just the image now becomes its clip group again, in the layers and in symbol, pattern, graph design and mask art. --- crates/engine/src/cmd/links.rs | 63 ++++++++++++++++++++++++++-------- crates/engine/src/tests/dcs.rs | 36 +++++++++++++++++++ docs/mcp.md | 2 +- 3 files changed, 85 insertions(+), 16 deletions(-) diff --git a/crates/engine/src/cmd/links.rs b/crates/engine/src/cmd/links.rs index 720a906462..91ec153049 100644 --- a/crates/engine/src/cmd/links.rs +++ b/crates/engine/src/cmd/links.rs @@ -738,12 +738,16 @@ impl Nested<'_> { let mut slots: Vec<(usize, Arc)> = vec![]; // The object a clip group that loses its clipping path holds alone. let mut declipped = None; + // The clipping path and object of a plain group holding just the object, which becomes a + // clip group again. + let mut reclipped = None; if let Some(children) = n.children() { // A clip group holding just the object: its clipping path changes. let clip_path = match (&n.kind, &children[..]) { (NodeKind::Group { clip: true, .. }, [c, _]) if matches!(c.kind, NodeKind::Path { clipping: true, .. }) => Some(c), _ => None, }; + let plain = matches!((&n.kind, &children[..]), (NodeKind::Group { clip: false, .. }, [_])); for (i, c) in children.iter().enumerate() { if !(nested && (self.pick)(c)) { let mut new = Arc::clone(c); @@ -769,6 +773,7 @@ impl Nested<'_> { } slots.extend([(0, Arc::new(path)), (1, new)]); } + (ClipTo::Path(clip), None) if plain => reclipped = Some((clip, new)), (ClipTo::Path(clip) | ClipTo::Nest(clip), _) => { let group = self.clip_group(new, clip); slots.push((i, Arc::new(group))); @@ -778,13 +783,21 @@ impl Nested<'_> { } } } - if mask.is_none() && slots.is_empty() && declipped.is_none() { + if mask.is_none() && slots.is_empty() && declipped.is_none() && reclipped.is_none() { return; } + let reclipped = reclipped.map(|(clip, object)| (Arc::new(self.clipping_path(clip)), object)); let n = Arc::make_mut(n); if let (Some(art), Some(m)) = (mask, n.extras_if_any().and_then(|e| e.mask.as_mut())) { m.art = art; } + // A plain group of just the object that becomes a clip group again. + if let Some((path, object)) = reclipped { + if let NodeKind::Group { children, clip } = &mut n.kind { + (*children, *clip) = (vec![path, object], true); + } + return; + } // A clip group that stays as a plain group of just the object. if let Some(object) = declipped { if let NodeKind::Group { children, clip } = &mut n.kind { @@ -803,15 +816,18 @@ impl Nested<'_> { /// A clip group of `object` clipped to `clip`, as [`clip_image_to`] makes one in the layers. fn clip_group(&mut self, object: Arc, clip: PathData) -> Node { - let (path_id, group_id) = (NodeId(self.next), NodeId(self.next + 1)); - self.next += 2; - let none = vectorcraft_color::Paint::None; - let mut path = Node::path(path_id, clip, Appearance::basic(none.clone(), none, 0.0)); - if let NodeKind::Path { clipping, .. } = &mut path.kind { - *clipping = true; - } + let path = self.clipping_path(clip); + let group_id = NodeId(self.next); + self.next += 1; Node::new(group_id, NodeKind::Group { children: vec![Arc::new(path), object], clip: true }) } + + /// A clipping path of `clip`, with the next node id. + fn clipping_path(&mut self, clip: PathData) -> Node { + let id = NodeId(self.next); + self.next += 1; + clipping_path(id, clip) + } } /// Placed documents `ids` show `src`, read from the file `link` points at, each placed as its @@ -936,23 +952,40 @@ 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, -/// else it goes into a new clip group. +/// 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_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) + let plain = d.parent_of(id).filter(|g| match d.node(*g).map(|n| &n.kind) { + Some(NodeKind::Group { children, clip: false }) => matches!(&children[..], [o] if o.id == id), + _ => false, + }); + let Some(g) = plain else { return new_clip_group(d, id, path) }; + let clip = clipping_path(d.alloc_id(), path); + d.insert(Some(g), 0, clip)?; + if let Some(Node { kind: NodeKind::Group { clip, .. }, .. }) = d.node_mut(g) { + *clip = true; + } + Ok(()) +} + +/// A clipping path `id` of `path`, unpainted. +fn clipping_path(id: NodeId, path: PathData) -> Node { + let none = vectorcraft_color::Paint::None; + let mut clip = Node::path(id, path, Appearance::basic(none.clone(), none, 0.0)); + if let NodeKind::Path { clipping, .. } = &mut clip.kind { + *clipping = true; + } + clip } /// Put object `id` of `d` into a new clip group of `path`, where it is. fn new_clip_group(d: &mut Document, id: NodeId, path: PathData) -> Result<()> { let (par, index, _) = d.position(id).ok_or(EngineError::NoNode(id))?; let image = d.remove(id)?; - let none = vectorcraft_color::Paint::None; - let mut clip = Node::path(d.alloc_id(), path, Appearance::basic(none.clone(), none, 0.0)); - if let NodeKind::Path { clipping, .. } = &mut clip.kind { - *clipping = true; - } + let clip = clipping_path(d.alloc_id(), path); let group = Node::new(d.alloc_id(), NodeKind::Group { children: vec![std::sync::Arc::new(clip), image], clip: true }); d.insert(par, index, group)?; Ok(()) diff --git a/crates/engine/src/tests/dcs.rs b/crates/engine/src/tests/dcs.rs index 2801432a0a..8d4e22ba85 100644 --- a/crates/engine/src/tests/dcs.rs +++ b/crates/engine/src/tests/dcs.rs @@ -357,6 +357,42 @@ fn a_file_that_lost_its_clipping_path_unclips_the_image() { ); } +#[test] +fn a_file_that_gains_its_clipping_path_again_clips_the_group_that_stayed() { + let dir = temp_dir("update-regained-clip"); + let path = write(&dir, "pic.eps", &two_plates()); + let mut s = session(); + let r = s.execute("file.place", &json!({ "path": path, "at": [100, 100] })).unwrap(); + let placed = NodeId(r["ids"][0].as_u64().unwrap()); + let id = image_of(&s, &r["ids"][0]).id; + let other = s.execute("file.place", &json!({ "path": path, "at": [200, 100] })).unwrap(); + s.execute("symbol.new", &json!({ "name": "Pic", "ids": [other["ids"][0]] })).unwrap(); + let unclipped = String::from_utf8(two_plates()).unwrap().replacen("closepath clip newpath", "closepath newpath ", 1); + // The symbol's art: how deep the image is. + let depth = |s: &Session| { + fn find(n: &Node, at: usize) -> Option { + if matches!(n.kind, NodeKind::Image(_)) { + return Some(at); + } + n.children().and_then(|ch| ch.iter().find_map(|c| find(c, at + 1))) + } + find(&s.doc().unwrap().doc.symbols.iter().find(|w| w.name == "Pic").expect("the symbol").art, 0) + }; + let clipped_depth = depth(&s); + for _ in 0..2 { + std::fs::write(&path, unclipped.as_bytes()).unwrap(); + s.execute("links.update", &json!({ "ids": [id.0] })).unwrap(); + std::fs::write(&path, two_plates()).unwrap(); + s.execute("links.update", &json!({ "ids": [id.0] })).unwrap(); + let doc = &s.doc().unwrap().doc; + assert_eq!(doc.parent_of(id), Some(placed), "the placed group holds the image itself"); + let NodeKind::Group { children, clip: true } = &doc.node(placed).unwrap().kind else { panic!("a clip group again") }; + assert!(matches!(&children[..], [c, im] if matches!(c.kind, NodeKind::Path { clipping: true, .. }) && im.id == id)); + assert_eq!(depth(&s), clipped_depth, "the symbol's image is not nested deeper"); + assert!(symbol_clip_width(&s, "Pic").is_some(), "the symbol's image is clipped again"); + } +} + /// The kind of group `g` is, as (clip, the kinds of its children are just one image). fn plain_group_of_image(g: &Node) -> Option<(bool, bool)> { match &g.kind { diff --git a/docs/mcp.md b/docs/mcp.md index 20e37222c6..00e0e435f8 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -5965,7 +5965,7 @@ while such an image that the art shows has a plate of its name; deleting the swa file's composite is drawn through clips the image (a clipping group, its path under the nonzero fill rule, an even-odd clipping path kept as the same area), and `file.place` answers the image's id as `image`, since the Links commands take the image and not the group. `links.update` and `links.relink` clip the image -by the file's clipping path again (in a new clipping group when none holds it, and inside a clipping mask made in the +by the file's clipping path again (a plain group holding just the image becomes its clipping group, a new clipping group holds it when no group does, and it goes inside a clipping mask made in the document, whose path is kept), unless Placement Options clip the image, the same way for the images in symbols, graph designs, patterns and opacity masks; when the new file has no clipping path, a clipping group holding just the image whose path is the old file's clipping path loses that path and stays as a plain group of the image, with its id, name, visibility and transparency (one whose path was given the From dc92be239173ba7f7a50a43d2d11bd036d6615be Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Thu, 12 Nov 2026 22:02:31 -0800 Subject: [PATCH 2/2] 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");