diff --git a/crates/engine/src/cmd/links.rs b/crates/engine/src/cmd/links.rs index 019f9baf73..74818a7604 100644 --- a/crates/engine/src/cmd/links.rs +++ b/crates/engine/src/cmd/links.rs @@ -629,9 +629,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 @@ -693,7 +697,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, } @@ -742,7 +746,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, }; } @@ -758,12 +762,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); @@ -782,14 +790,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) | 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))); } @@ -798,13 +807,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 { @@ -823,15 +840,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 @@ -964,15 +984,41 @@ fn clip_image_to(d: &mut Document, id: NodeId, path: PathData) -> Result<()> { 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(()); + } + 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(()) @@ -1037,6 +1083,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/dcs.rs b/crates/engine/src/tests/dcs.rs index 9853ed5da7..6c17f04c0d 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/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"); diff --git a/docs/mcp.md b/docs/mcp.md index ffbdcbf013..1232ffb131 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -6072,7 +6072,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