Merge pull request #5990 from dotCooCoo/fix/ai-mask-lone-object

.ai import: opacity mask art of one object opens as that object

crates/engine/src/cmd/fileio/tests_pdfedit.rs

@@ -598,36 +598,17 @@ fn editing_copy(bytes: &[u8]) -> vectorcraft_doc::Document {
     l.doc
 }
 
-/// `nodes` as the editing copy reads them back: an opacity mask's art that isn't a group comes
-/// back as a plain group holding it.
-fn mask_art_grouped(nodes: &mut [std::sync::Arc<vectorcraft_doc::Node>]) {
-    use vectorcraft_doc::{Node, NodeKind};
-    for n in nodes {
-        let n = std::sync::Arc::make_mut(n);
-        if let Some(m) = n.extras_mut().mask.as_mut()
-            && !matches!(m.art.kind, NodeKind::Group { .. })
-        {
-            let art = m.art.clone();
-            m.art = std::sync::Arc::new(Node::new(art.id, NodeKind::Group { children: vec![art], clip: false }));
-        }
-        if let Some(c) = n.children_mut() {
-            mask_art_grouped(c);
-        }
-    }
-}
-
 /// Transparency, a compound path, a clipping group, an opacity mask and a clipping layer come back
 /// from the editing copy for other apps alone, its page matching it, with every save option that
 /// writes the copy; the version 7 `.ai` keeps the compound path and the clipping group.
 #[test]
 fn transparency_clipping_and_opacity_masks_come_back_from_the_editing_copy() {
     let mut s = see_through_session();
-    let mut before = (*s.doc().unwrap().doc).clone();
-    // The mask's art is one path; it reads back as a group holding the path.
+    let before = (*s.doc().unwrap().doc).clone();
+    // The mask's art is one path, and it reads back as that path.
     let mut arts = vec![];
     before.walk(|n| arts.extend(n.extras().mask.as_ref().map(|m| matches!(m.art.kind, vectorcraft_doc::NodeKind::Path { .. }))));
     assert_eq!(arts, [true]);
-    mask_art_grouped(&mut before.layers);
     let want = see_through_outline(&before);
     for opts in [json!({"format": "ai"}), json!({"format": "ai", "compress": false}), json!({"format": "ai", "pdfCompatible": true})] {
         let bytes = b64(&s.execute("document.export", &opts).unwrap());

crates/eps/src/ai_editing/tests.rs

@@ -1244,52 +1244,88 @@ fn same_lines(got: &[String], want: &[String]) {
     assert!(differ.is_empty(), "{}", differ.join("\n"));
 }
 
-/// `nodes` as the reader reads them back: an opacity mask's art that isn't a group comes back as
-/// a group holding it.
-fn mask_art_grouped(nodes: &mut [Arc<Node>]) {
-    for n in nodes {
-        let n = Arc::make_mut(n);
-        if let Some(m) = n.extras_mut().mask.as_mut()
-            && !matches!(m.art.kind, NodeKind::Group { .. })
-        {
-            let art = m.art.clone();
-            m.art = Arc::new(Node::new(art.id, NodeKind::Group { children: vec![art], clip: false }));
-        }
-        if let Some(c) = n.children_mut() {
-            mask_art_grouped(c);
-        }
-    }
-}
-
 #[test]
 fn transparency_clipping_and_masks_are_read_back() {
-    let mut doc = transparent();
+    let doc = transparent();
     let out = export_ai_editing(&doc, &opts()).unwrap();
     assert!(out.warnings.is_empty(), "{:?}", out.warnings);
     let back = read_back(&doc);
     same_lines(&lines(&back), &lines(&doc));
-    // The disabled mask's art is one path: it reads back as a group holding the path.
-    let mask_arts = |d: &Document| {
-        let mut out = vec![];
-        d.walk(|n| {
-            if let Some(m) = &n.extras().mask {
-                out.push(match &m.art.kind {
-                    NodeKind::Group { children, .. } => format!("group of {}", children.len()),
-                    NodeKind::Path { .. } => "path".into(),
-                    _ => "other".into(),
-                });
-            }
-        });
-        out
-    };
-    assert_eq!(
-        (mask_arts(&doc), mask_arts(&back)),
-        (vec!["group of 2".to_string(), "path".into()], vec!["group of 2".to_string(), "group of 1".into()])
-    );
-    mask_art_grouped(&mut doc.layers);
     same_lines(&all_looks(&back), &all_looks(&doc));
 }
 
+/// An opacity mask's art reads back in its own shape: one object, a group of one object, a group
+/// of two, a group of one group, a see-through group of two, and a group around a hidden object or
+/// a guide.
+#[test]
+fn opacity_mask_art_keeps_its_shape_when_read_back() {
+    let mut doc = Document::new(200.0, 100.0);
+    doc.swatches = vec![Swatch { name: "Blue".into(), paint: solid(Color::rgb(0.0, 0.0, 1.0)), global: false, spot: false }].into();
+    doc.swatch_groups = vec![].into();
+    let layer = doc.layers[0].id;
+    let art = |doc: &mut Document| square(doc, Rect::new(5.0, 5.0, 10.0, 10.0), Color::gray(0.5));
+    let path = art(&mut doc);
+    let one = art(&mut doc);
+    let of_one = group(&mut doc, vec![one], false);
+    let (a, b) = (art(&mut doc), art(&mut doc));
+    let of_two = group(&mut doc, vec![a, b], false);
+    let (a, b) = (art(&mut doc), art(&mut doc));
+    let inner = group(&mut doc, vec![a, b], false);
+    let of_group = group(&mut doc, vec![inner], false);
+    let (a, b) = (art(&mut doc), art(&mut doc));
+    let faded = see_through(group(&mut doc, vec![a, b], false), 0.5, BlendMode::Normal);
+    let mut off = art(&mut doc);
+    off.visible = false;
+    let of_hidden = group(&mut doc, vec![off], false);
+    let line = Node::new(
+        doc.alloc_id(),
+        NodeKind::Path {
+            path: shapes::rectangle(Rect::new(5.0, 5.0, 10.0, 10.0)),
+            rule: FillRule::NonZero,
+            live: None,
+            clipping: false,
+            guide: true,
+        },
+    );
+    let of_guide = group(&mut doc, vec![line], false);
+    for mask_art in [path, of_one, of_two, of_group, faded, of_hidden, of_guide] {
+        let mut owner = square(&mut doc, Rect::new(0.0, 0.0, 30.0, 30.0), Color::rgb(1.0, 0.0, 0.0));
+        owner.extras_mut().mask = Some(Box::new(OpacityMask::new(mask_art, true)));
+        doc.insert(Some(layer), usize::MAX, owner).unwrap();
+    }
+    let back = read_back(&doc);
+    same_lines(&all_looks(&back), &all_looks(&doc));
+    let first = String::from_utf8(export_ai_editing(&doc, &opts()).unwrap().program).unwrap();
+    let again = String::from_utf8(export_ai_editing(&back, &opts()).unwrap().program).unwrap();
+    same_lines(&again.lines().map(String::from).collect::<Vec<_>>(), &first.lines().map(String::from).collect::<Vec<_>>());
+}
+
+/// Hidden art of a mask is written shown: the canvas draws the art's root either way, and the reader
+/// puts a hidden object in a group that doesn't draw it.
+#[test]
+fn a_hidden_opacity_mask_art_root_reads_back_shown() {
+    let mut doc = Document::new(200.0, 100.0);
+    doc.swatches = vec![Swatch { name: "Blue".into(), paint: solid(Color::rgb(0.0, 0.0, 1.0)), global: false, spot: false }].into();
+    doc.swatch_groups = vec![].into();
+    let layer = doc.layers[0].id;
+    let art = |doc: &mut Document| square(doc, Rect::new(5.0, 5.0, 10.0, 10.0), Color::gray(0.5));
+    let mut path = art(&mut doc);
+    path.visible = false;
+    let (a, b) = (art(&mut doc), art(&mut doc));
+    let mut of_two = group(&mut doc, vec![a, b], false);
+    of_two.visible = false;
+    for mask_art in [path, of_two] {
+        let mut owner = square(&mut doc, Rect::new(0.0, 0.0, 30.0, 30.0), Color::rgb(1.0, 0.0, 0.0));
+        owner.extras_mut().mask = Some(Box::new(OpacityMask::new(mask_art, true)));
+        doc.insert(Some(layer), usize::MAX, owner).unwrap();
+    }
+    let back = read_back(&doc);
+    let roots: Vec<_> = back.layers[0].children().unwrap().iter().map(|o| o.extras().mask.as_ref().unwrap().art.clone()).collect();
+    assert!(matches!(roots.as_slice(), [p, g] if p.visible && g.visible
+        && matches!(p.kind, NodeKind::Path { .. })
+        && matches!(&g.kind, NodeKind::Group { children, clip: false } if children.len() == 2 && children.iter().all(|c| c.visible))));
+}
+
 #[test]
 fn transparency_clipping_and_masks_write_the_same_bytes_again() {
     let doc = transparent();
@@ -2610,8 +2646,6 @@ fn images_doc() -> Document {
         shapes::rectangle(Rect::new(10.0, 60.0, 40.0, 90.0)),
         Appearance::basic(solid(Color::rgb(0.0, 0.5, 1.0)), Paint::None, 0.0),
     );
-    // A mask's art is read into a group.
-    let mask_art = Node::group(doc.alloc_id(), vec![Arc::new(mask_art)]);
     square.extras_mut().mask = Some(Box::new(OpacityMask::new(mask_art, false)));
     doc.insert(Some(layer), usize::MAX, square).unwrap();
     let links = Node::layer(doc.alloc_id(), "Links", LayerColor::Preset(1));

crates/eps/src/ai_editing/writer.rs

@@ -543,8 +543,19 @@ impl<'a> Writer<'a> {
         let outer = std::mem::take(&mut self.body);
         let (state, clip) = (self.state, self.clip.take());
         (self.state, self.gs) = (Some((false, false)), Some(Xy::OPAQUE));
-        // A plain group holding the art is the group the art is read into.
-        let art = &m.art;
+        // The canvas draws the art's root even when it is hidden, and the reader puts a hidden object
+        // in a group that doesn't draw it, so a hidden root is written shown.
+        let shown;
+        let art = if m.art.visible {
+            &m.art
+        } else {
+            let mut a = (*m.art).clone();
+            a.visible = true;
+            shown = Arc::new(a);
+            &shown
+        };
+        // A plain group of two or more objects, or of none, is the group the art is read into. A group
+        // of one object is written as a group, because the reader makes one shown object the art.
         let plain = matches!(art.kind, NodeKind::Group { clip: false, .. })
             && Xy::of(art) == Xy::PLAIN
             && art.visible
@@ -552,7 +563,7 @@ impl<'a> Writer<'a> {
             && art.name.as_deref().is_none_or(str::is_empty)
             && art.extras().mask.is_none();
         let members = match art.children() {
-            Some(c) if plain => c.as_slice(),
+            Some(c) if plain && c.len() != 1 => c.as_slice(),
             _ => std::slice::from_ref(art),
         };
         self.nest += 1;

crates/eps/src/import/ai/read/containers.rs

@@ -381,7 +381,12 @@ impl<'a> Reader<'a> {
         let f = self.close("X+", |k| matches!(k, Kind::Mask))?;
         // The mask's art is one level under the object it masks.
         let (children, clip) = self.clip_first(f.children, &f.clips, false, self.nesting() + 2)?;
-        let art = self.new_node(NodeKind::Group { children, clip }, &f.gs);
+        // One shown object is the art itself. A hidden object or a guide stays in a group: the canvas draws the art's root
+        // whether it is shown or not and whether guides show, and a group doesn't draw a hidden member or a left-out guide.
+        let art = match children.as_slice() {
+            [only] if !clip && only.visible && !matches!(only.kind, NodeKind::Path { guide: true, .. }) => only.clone(),
+            _ => Arc::new(self.new_node(NodeKind::Group { children, clip }, &f.gs)),
+        };
         self.gs = f.gs;
         self.pending_unread_fill = f.pending_unread_fill;
         self.pending_unread_stroke = f.pending_unread_stroke;
@@ -390,7 +395,7 @@ impl<'a> Reader<'a> {
         let Some(Frame { kind: Kind::Obj(o), .. }) = self.frames.last_mut() else {
             return Err("mask art ends outside a mask dictionary".into());
         };
-        if o.art.replace(Arc::new(art)).is_some() {
+        if o.art.replace(art).is_some() {
             return Err("a mask has more than one art block".into());
         }
         o.unread = f.unread;

crates/eps/src/import/ai/tests.rs

@@ -6555,7 +6555,7 @@ fn vector_opacity_mask_keeps_its_art_and_does_not_paint_as_a_sibling() {
         assert_eq!(fill(&a[1]).color(), Some(Color::rgb(1.0, 0.0, 0.0)), "mask colour must not leak");
         let m = a[0].extras().mask.as_ref().unwrap();
         assert_eq!((m.clip, m.invert, m.disabled, m.linked), (true, invert, disabled, linked));
-        let mask_path = &m.art.children().unwrap()[0].children().unwrap()[0];
+        let mask_path = &m.art.children().unwrap()[0];
         assert!(matches!(mask_path.kind, NodeKind::Path { .. }));
         assert_eq!(mask_path.geometric_bounds(), Some(Rect::new(10.0, 40.0, 30.0, 80.0)));
         assert_ne!(mask_path.id, a[0].id);
@@ -6565,6 +6565,37 @@ fn vector_opacity_mask_keeps_its_art_and_does_not_paint_as_a_sibling() {
     }
 }
 
+/// An opacity mask's art of one shown object is that object; other art is read into a group.
+#[test]
+fn opacity_mask_art_of_one_object_is_that_object() {
+    let owner = format!("{RED}{}f\n", rect_path(10.0, 20.0, 50.0, 60.0));
+    let square = "%_1 g\n%_10 20 m\n%_10 60 L\n%_30 60 L\n%_30 20 L\n%_f\n";
+    let read_art = |ops: &str| {
+        let mask = format!("%_/ArtDictionary :\n%_/Mask :\n%_X=\n{ops}%_X+\n%_(Art) ,\n%_1 /Bool (Clipping) ,\n%_; (AI9_Mask) ,\n%_;\n");
+        let r = read_ok(&layer(&format!("{owner}{mask}")));
+        art(&r.doc)[0].extras().mask.as_ref().unwrap().art.clone()
+    };
+    assert!(matches!(read_art(square).kind, NodeKind::Path { .. }));
+    // A group is one object: it stays a group, holding its path.
+    let grouped = read_art(&format!("%_u\n{square}%_U\n"));
+    let NodeKind::Group { children, clip: false } = &grouped.kind else { panic!("{:?}", grouped.kind) };
+    assert!(matches!(children.as_slice(), [p] if matches!(p.kind, NodeKind::Path { .. })));
+    // Two objects are held in a group.
+    let two = read_art(&format!("{square}{square}"));
+    assert!(matches!(&two.kind, NodeKind::Group { children, clip: false } if children.len() == 2), "{:?}", two.kind);
+    // A clipping path alone keeps its group, which clips.
+    let clipped = read_art("%_10 20 m\n%_10 60 L\n%_30 60 L\n%_30 20 L\n%_h\n%_W\n%_n\n");
+    assert!(matches!(&clipped.kind, NodeKind::Group { children, clip: true } if children.len() == 1), "{:?}", clipped.kind);
+    // A hidden object stays in a group, which doesn't draw it.
+    let hidden = read_art(&format!("%_1 Xw\n{square}%_0 Xw\n"));
+    let NodeKind::Group { children, clip: false } = &hidden.kind else { panic!("{:?}", hidden.kind) };
+    assert!(matches!(children.as_slice(), [p] if !p.visible && matches!(p.kind, NodeKind::Path { .. })));
+    // So does a guide, which a group leaves out when guides aren't drawn.
+    let guide = read_art("%_10 20 m\n%_30 60 L\n%_(N) *\n");
+    let NodeKind::Group { children, clip: false } = &guide.kind else { panic!("{:?}", guide.kind) };
+    assert!(matches!(children.as_slice(), [p] if matches!(p.kind, NodeKind::Path { guide: true, .. })));
+}
+
 /// `9 () XW` also follows legacy compound member groups. The final gradient paints
 /// every member; the initial black paint is a placeholder, not a separate black object.
 #[test]

docs/file-format-parity.md

@@ -47,6 +47,8 @@ private editing data. VectorCraft:
   objects keep what only the format records (locks, for example); it opens as its page when the layer can't be used.
   A multi-layer mask (`Mb` … `Md` … `MB`) comes in as a clipping group in each layer it covers, each
   clipped by a copy of the mask's path.
+  An opacity mask's art of one shown object, other than a guide or a clipping path, opens as that object in the
+  editing data of a `.ai`, EPS or PDF; other art opens in a group.
   RGB in the PDF page's `ICCBased` spaces keeps the file's numbers in a `.ai` (the `uncalibrated_rgb`
   import option, on for editor documents, off for PDFs), so `.ai` files tagged with a non-sRGB RGB profile
   open with different, unconverted RGB colours than before.