Merge pull request #3681 from dotCooCoo/fix/package-skips-page-parsing-without-fonts

Package: reads placed pages and document previews only for the options that use them

crates/engine/src/cmd/package.rs

@@ -104,12 +104,23 @@ fn safe_asset_name(name: &str) -> String {
 /// A file placed: (page, bounding, page box) of each placed object showing it.
 type PlacedPages = Vec<(u32, bool, vectorcraft_doc::PageBox)>;
 
+#[cfg(test)]
+thread_local! {
+    /// Tests: how many placed PDF or `.ai` pages [`page_fonts`] read on this thread.
+    pub(crate) static PAGES_READ: std::cell::Cell<usize> = const { std::cell::Cell::new(0) };
+    /// Tests: how many linked `.vectorcraft` files had their preview and PDF read on this thread,
+    /// to keep them in the rewritten copy.
+    pub(crate) static PARTS_READ: std::cell::Cell<usize> = const { std::cell::Cell::new(0) };
+}
+
 /// The fonts of the type on `pages` of the PDF or `.ai` file `name` (`bytes`, found at `path`);
 /// none for other files.
 fn page_fonts(name: &str, bytes: &[u8], path: &str, pages: &PlacedPages) -> BTreeSet<super::fonts::UsedFont> {
     let mut fonts = BTreeSet::new();
     if super::place::document::is_pdf(name, bytes) {
         for &(page, bounding, page_box) in pages {
+            #[cfg(test)]
+            PAGES_READ.with(|c| c.set(c.get() + 1));
             if let Ok(a) = super::place::document::document_art(name, bytes, Some(path), page, bounding, page_box, C) {
                 fonts.extend(super::fonts::art_fonts(&a.doc, &a.nodes));
             }
@@ -231,7 +242,9 @@ impl<'a> Collector<'a> {
                         self.renamed(&mut lines, source, &file.path, &file.name, &existing.name);
                     }
                     // Another document may place other pages of a PDF copied already.
-                    if let (Some(pages), Some(bytes)) = (placed.get(&file.path), &file.bytes) {
+                    if self.opts.copy_fonts
+                        && let (Some(pages), Some(bytes)) = (placed.get(&file.path), &file.bytes)
+                    {
                         placed_fonts.extend(page_fonts(&file.name, bytes, found_path, pages));
                     }
                     local.insert(file.path.clone(), existing);
@@ -262,7 +275,9 @@ impl<'a> Collector<'a> {
                     } else {
                         self.collect(&mut art, found_path, &destination_file, depth + 1, true)?;
                     }
-                } else if let Some(pages) = pages {
+                } else if self.opts.copy_fonts
+                    && let Some(pages) = pages
+                {
                     placed_fonts.extend(page_fonts(&file.name, &bytes, found_path, pages));
                 }
                 if placed_file && super::place::document::is_native(&file.name, &bytes) {
@@ -271,18 +286,18 @@ impl<'a> Collector<'a> {
                     } else {
                         match vectorcraft_format::load_file(&bytes) {
                             Ok(saved) => {
-                                let preview = vectorcraft_format::preview(&bytes);
-                                let pdf = vectorcraft_format::pdf_content(&bytes);
                                 let compressed = bytes.starts_with(&[0x1f, 0x8b]);
                                 let include_linked =
                                     saved.doc.linked_only_images().iter().any(|key| saved.doc.images.get(key).is_some_and(|blob| !blob.is_proxy()));
                                 let mut nested = saved.doc;
                                 self.collect(&mut nested, found_path, &destination_file, depth + 1, false)?;
                                 if self.opts.relink {
+                                    #[cfg(test)]
+                                    PARTS_READ.with(|c| c.set(c.get() + 1));
                                     // Preserve auxiliary native-file content when rewriting.
                                     let mut options = vectorcraft_format::SaveOptions::for_doc(&nested);
-                                    options.preview = preview;
-                                    options.pdf = pdf;
+                                    options.preview = vectorcraft_format::preview(&bytes);
+                                    options.pdf = vectorcraft_format::pdf_content(&bytes);
                                     options.profiles = saved.profiles;
                                     options.compress = compressed;
                                     options.include_linked = include_linked;

crates/engine/src/tests_package.rs

@@ -471,6 +471,35 @@ fn package_copies_the_fonts_of_each_page_of_a_pdf_that_two_documents_place() {
     assert_eq!((r["links"].as_u64(), r["fonts"].as_u64(), r["skippedFonts"].clone()), (Some(2), Some(2), json!([])), "{r}");
 }
 
+/// With Copy Fonts off, Package reads no placed PDF page for its type: neither a page of a PDF it
+/// copies nor a page of one another document placed and it copied already.
+#[test]
+fn package_reads_no_placed_pdf_page_without_copy_fonts() {
+    use cmd::package::PAGES_READ;
+    let dir = Folder::new("package-placed-pdf-no-fonts");
+    let mut src = session();
+    run(&mut src, "text.create", json!({"x": 10, "y": 40, "text": "One"}));
+    run(&mut src, "artboard.new", json!({"x": 500, "y": 0, "width": 400, "height": 300}));
+    run(&mut src, "text.create", json!({"x": 510, "y": 40, "text": "Two"}));
+    let pdf = run(&mut src, "document.serialize", json!({"format": "pdf", "preserveEditing": false}));
+    let path = dir.file("art/flyer.pdf");
+    write(&path, &vectorcraft_format::base64_decode(pdf["dataBase64"].as_str().unwrap()).unwrap());
+    let mut part = session();
+    run(&mut part, "file.place", json!({"path": path, "page": 2}));
+    let part_path = dir.file("art/part.vectorcraft");
+    save(&mut part, &part_path);
+    let mut s = session();
+    run(&mut s, "file.place", json!({"path": path, "page": 1}));
+    run(&mut s, "file.place", json!({"path": part_path}));
+    save(&mut s, &dir.file("poster.vectorcraft"));
+    for (copy_links, copy_fonts, fonts, reads, out) in [(true, false, 0, 0, "links"), (false, false, 0, 0, "neither"), (true, true, 1, 2, "both")] {
+        let before = PAGES_READ.with(std::cell::Cell::get);
+        let r = run(&mut s, "file.package", json!({"folder": dir.file(out), "copyLinks": copy_links, "copyFonts": copy_fonts}));
+        let read = PAGES_READ.with(std::cell::Cell::get) - before;
+        assert_eq!((r["fonts"].as_u64(), read), (Some(fonts), reads), "{out}: {r}");
+    }
+}
+
 /// `dir/art/badge.ai`, saved without its PDF part, its art placing each of `pics` (600 × 300 px,
 /// with a 2×1 preview) → its path.
 fn ai_placing(dir: &Folder, pics: &[&str]) -> String {
@@ -664,6 +693,21 @@ fn packaging_nested_files_preserves_embedded_preview_pdf_and_compression() {
     assert_eq!(images[0].as_ref().unwrap().relative.as_deref(), Some("photo.png"));
 }
 
+/// Without Relink, Package copies a placed document as it is and reads no preview or PDF from it.
+#[test]
+fn package_reads_no_preview_or_pdf_of_a_placed_document_without_relink() {
+    use cmd::package::PARTS_READ;
+    let dir = Folder::new("package-native-no-relink");
+    let mut parent = poster_with_placed_document(&dir);
+    let original = std::fs::read(dir.file("art/part.vectorcraft")).unwrap();
+    for (relink, reads, out) in [(false, 0, "as is"), (true, 1, "relinked")] {
+        let before = PARTS_READ.with(std::cell::Cell::get);
+        let r = run(&mut parent, "file.package", json!({"folder": dir.file(out), "relink": relink}));
+        assert_eq!((r["links"].as_u64(), PARTS_READ.with(std::cell::Cell::get) - before), (Some(2), reads), "{out}: {r}");
+    }
+    assert_eq!(std::fs::read(dir.0.join("as is/poster Folder/Links/part.vectorcraft")).unwrap(), original);
+}
+
 /// The report keeps the packaged document's sections as they were, then gives each placed
 /// document's own, headed by its file, instead of nesting them inside the parent's list.
 #[test]