Merge pull request #2790 from dotCooCoo/fix/pdf-export-skips-undecodable-images

PDF export: an image that can't be decoded is left out with a note instead of failing the export

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

@@ -131,6 +131,56 @@ fn options_not_applied_yet_and_document_features_warn() {
     assert!(warnings(&s.execute("document.export", &json!({"format": "png"})).unwrap()).is_empty());
 }
 
+/// A placed image whose pixel data can't be decoded is left out of every PDF the engine writes,
+/// and the rest of the page is written. Each reply that carries the PDF's warnings has the note
+/// raster exports give, once.
+#[test]
+fn every_pdf_leaves_out_an_image_that_cant_be_decoded() {
+    const NOTE: &str = "an image couldn't be decoded and is missing from the picture";
+    let mut s = session(1);
+    let cut = crate::tests_menucmds::place_undecodable_png(&mut s);
+    // Whether each object on the PDF's pages is an image. The pages should hold the rectangle and
+    // no image.
+    let drawn = |bytes: &[u8]| {
+        let mut images = vec![];
+        vectorcraft_pdf::import(bytes).unwrap().walk(|n| {
+            if !n.is_container() && !matches!(n.kind, vectorcraft_doc::NodeKind::Path { clipping: true, .. }) {
+                images.push(matches!(n.kind, vectorcraft_doc::NodeKind::Image(_)));
+            }
+        });
+        images
+    };
+    for (cmd, p) in [
+        ("document.exportPdf", json!({})),
+        ("document.exportPdf", json!({"preset": "High Quality Print"})),
+        ("document.exportPdf", json!({"standard": "pdfX1a"})),
+        ("document.export", json!({"format": "pdf"})),
+        ("file.saveCopy", json!({"format": "pdf"})),
+        ("file.saveCopy", json!({"format": "ai"})),
+        ("file.print", json!({})),
+    ] {
+        let v = s.execute(cmd, &p).unwrap_or_else(|e| panic!("{cmd} {p}: {e}"));
+        assert_eq!(warnings(&v).iter().filter(|w| *w == NOTE).count(), 1, "{cmd} {p}: {}", v["warnings"]);
+        assert_eq!(drawn(&b64(&v)), [false], "{cmd} {p}");
+    }
+    // The .ai file reopens with the image as it was placed, from the image bytes in its editing data.
+    let ai = b64(&s.execute("file.saveCopy", &json!({"format": "ai"})).unwrap());
+    let placed = s.doc().unwrap().doc.images.clone();
+    s.execute("document.open", &json!({"name": "cut.ai", "dataBase64": vectorcraft_format::base64_encode(&ai)})).unwrap();
+    let reopened = &s.doc().unwrap().doc;
+    assert!(reopened.node(cut).is_some_and(|n| matches!(n.kind, vectorcraft_doc::NodeKind::Image(_))));
+    assert_eq!(reopened.images, placed);
+    // Export for Screens, a native file carrying a PDF, and the PDF that Copy offers are written.
+    let r = s.execute("document.exportForScreens", &json!({"formats": [{"format": "pdf"}]})).unwrap();
+    assert_eq!(drawn(&b64(&r["files"][0])), [false]);
+    assert!(s.execute("file.saveCopy", &json!({"format": "vectorcraft", "pdfCompatible": true})).is_ok());
+    s.execute("select.set", &json!({"ids": [cut.0]})).unwrap();
+    s.execute("edit.copy", &json!({})).unwrap();
+    assert!(b64(&s.execute("clipboard.exportPdf", &json!({})).unwrap()).starts_with(b"%PDF"));
+    s.execute("prefs.set", &json!({"key": "copyAsPdf", "value": true})).unwrap();
+    assert!(s.clipboard_flavours().iter().any(|f| f.mime == crate::cmd::clipboard::PDF));
+}
+
 #[test]
 fn export_pdf_writes_a_path() {
     let mut s = session(2);

crates/pdf/src/export.rs

@@ -38,10 +38,26 @@ pub fn export(doc: &Document, opts: &PdfOptions) -> Result<Vec<u8>, PdfError> {
 /// Like [`export`], also returning warnings: options accepted but not applied yet (see
 /// [`crate::PdfSettings::warnings`]) and features approximated or dropped.
 pub fn export_with_report(doc: &Document, opts: &PdfOptions) -> Result<ExportReport, PdfError> {
-    write::guard(|| export_with_report_inner(doc, opts))
+    write::guard(|| leaving_out_undecodable(|left_out| export_with_report_inner(doc, opts, left_out)))
 }
 
-fn export_with_report_inner(doc: &Document, opts: &PdfOptions) -> Result<ExportReport, PdfError> {
+/// Run `write` (a whole PDF written by a [`Writer`]), and run it again each time it fails on an
+/// image whose pixel data the writer couldn't decode, with that image left out: [`Writer::finish`]
+/// adds the image's key to the set `write` is given, and the exporters leave out the images it
+/// holds. The writer reports such an image (a PNG, GIF or WebP kept as it is, whose header reads)
+/// only when it finishes the file. Each repeated run leaves out at least one more image.
+pub(crate) fn leaving_out_undecodable<T>(mut write: impl FnMut(&mut HashSet<String>) -> Result<T, PdfError>) -> Result<T, PdfError> {
+    let mut left_out = HashSet::new();
+    loop {
+        let known = left_out.len();
+        match write(&mut left_out) {
+            Err(_) if left_out.len() > known => {}
+            done => return done,
+        }
+    }
+}
+
+fn export_with_report_inner(doc: &Document, opts: &PdfOptions, left_out: &mut HashSet<String>) -> Result<ExportReport, PdfError> {
     let set = &opts.settings;
     set.check()?;
     vectorcraft_doc::cancel::check().map_err(|_| PdfError::Cancelled)?;
@@ -68,6 +84,7 @@ fn export_with_report_inner(doc: &Document, opts: &PdfOptions) -> Result<ExportR
     let mut ex = Exporter::new(doc, set);
     ex.non_printing = set.include_non_printing || set.create_layers;
     ex.out = w.out.clone();
+    ex.left_out = left_out.clone();
     // A knockout page group draws the layers' objects one by one, not layer by layer.
     ex.layers = set.writes_layers() && !doc.page_knockout;
     if set.writes_layers() && doc.page_knockout {
@@ -113,7 +130,7 @@ fn export_with_report_inner(doc: &Document, opts: &PdfOptions) -> Result<ExportR
     vectorcraft_doc::cancel::check().map_err(|_| PdfError::Cancelled)?;
     let (layers, overprinted) = (ex.layers, ex.overprinted);
     w.absorb(ex);
-    let (bytes, more) = w.finish()?;
+    let (bytes, more) = w.finish(left_out)?;
     let bytes = crate::forms::finish(bytes, doc, set, layers, overprinted)?;
     warnings.extend(more);
     let bytes = crate::post::user_units(bytes, &units)?;
@@ -207,6 +224,9 @@ pub(crate) struct Writer {
     rgb_in_cmyk: RgbInCmyk,
     /// A PDF 1.3 file ([`crate::PdfSettings::pdf13`]): flat, without page groups.
     pdf13: bool,
+    /// The document's images the exporters wrote, by key, gathered from the exporters
+    /// ([`Self::absorb`]): [`Self::finish`] finds the key of an image the writer couldn't decode.
+    images: Vec<(String, Image)>,
 }
 
 /// One page as [`Writer::page`] draws it: the art placed in a drawing space ([`Self::place`]),
@@ -288,6 +308,7 @@ impl Writer {
             warnings,
             rgb_in_cmyk: RgbInCmyk::default(),
             pdf13: set.pdf13(),
+            images: vec![],
         })
     }
 
@@ -370,7 +391,7 @@ impl Writer {
         Ok(())
     }
 
-    /// Take an exporter's warnings and Lab spot colours once its pages are drawn.
+    /// Take an exporter's warnings, Lab spot colours and images once its pages are drawn.
     pub(crate) fn absorb(&mut self, ex: Exporter) {
         for w in ex.warnings {
             if !self.warnings.contains(&w) {
@@ -383,13 +404,28 @@ impl Writer {
                 self.lab_spots.push(spot);
             }
         }
+        // The exporter caches images by their key, followed by their size and mask after a NUL.
+        for (cache, image) in ex.images {
+            if let (Some(key), Some(image)) = (cache.split('\0').next(), image) {
+                self.images.push((key.to_string(), image));
+            }
+        }
     }
 
     /// The finished file (with the output intent and Trapped entries, and checked against its
     /// standard) and the warnings of its drawing. PDF/X-1a spot colours keep their CMYK
-    /// alternates.
-    pub(crate) fn finish(mut self) -> Result<(Vec<u8>, Vec<String>), PdfError> {
-        let mut bytes = self.pdf.finish()?;
+    /// alternates. When the writer couldn't decode one of the images, its key is added to
+    /// `left_out` ([`leaving_out_undecodable`]) and the writer's error is returned.
+    pub(crate) fn finish(mut self, left_out: &mut HashSet<String>) -> Result<(Vec<u8>, Vec<String>), PdfError> {
+        let images = std::mem::take(&mut self.images);
+        let mut bytes = self.pdf.finish().map_err(|e| {
+            if let write::WriteError::Serialization(k) = &e
+                && let krilla::error::KrillaError::Image(failed, ..) = &**k
+            {
+                left_out.extend(images.iter().filter(|(_, image)| image == failed).map(|(key, _)| key.clone()));
+            }
+            PdfError::from(e)
+        })?;
         if self.cmyk {
             cmyk_blending(&mut bytes, self.out.tagged);
         }
@@ -557,11 +593,18 @@ pub(crate) struct Exporter<'a> {
     /// Inline graphics being drawn inside inline graphics (text in a symbol shown inline in
     /// text…): deeper ones are left out.
     inline_depth: u32,
+    /// Keys of the images left out because the writer couldn't decode them
+    /// ([`leaving_out_undecodable`]).
+    pub left_out: HashSet<String>,
 }
 
 /// How deep inline graphics nest before they are left out.
 const MAX_INLINE_DEPTH: u32 = 4;
 
+/// The warning for an image left out because it can't be decoded: the note raster exports give
+/// (`vectorcraft-engine`'s `rasterfx::undecoded_warning`).
+const UNDECODED: &str = "an image couldn't be decoded and is missing from the picture";
+
 impl<'a> Exporter<'a> {
     /// An exporter of `doc` writing images as `set` says; layers whose Print option is off are
     /// left out.
@@ -590,6 +633,7 @@ impl<'a> Exporter<'a> {
             overprint: false,
             overprinted: false,
             inline_depth: 0,
+            left_out: HashSet::new(),
         }
     }
 
@@ -1696,7 +1740,8 @@ impl Exporter<'_> {
 
     /// Image `key` placed `size` points wide and high, resampled and compressed as the Compression
     /// settings say ([`crate::images::recode`], [`crate::images::recode_cmyk`] for CMYK images);
-    /// cached by key (and size when images are resampled).
+    /// cached by key (and size when images are resampled). `None`, with [`UNDECODED`] among the
+    /// warnings, for an image that is missing, can't be decoded or is in [`Self::left_out`].
     fn load_image(&mut self, key: &str, size: (f64, f64)) -> Option<Image> {
         let c = self.compression;
         let sized = [c.color.downsample, c.gray.downsample, c.mono.downsample].iter().any(|d| *d != crate::Downsample::None);
@@ -1707,7 +1752,7 @@ impl Exporter<'_> {
         let interpolate = self.interpolate;
         let doc = self.doc;
         let out = self.out.clone();
-        let img = doc.images.get(key).and_then(|blob| {
+        let img = doc.images.get(key).filter(|_| !self.left_out.contains(key)).and_then(|blob| {
             let cmyk_image = blob.is_cmyk();
             if !cmyk_image && self.convert && doc.color_mode == vectorcraft_doc::ColorMode::Cmyk {
                 self.rgb_in_cmyk.images.insert(key.to_string());
@@ -1723,7 +1768,7 @@ impl Exporter<'_> {
             r.image.or_else(|| embed(blob, interpolate))
         });
         if img.is_none() {
-            self.warn(format!("image '{key}' could not be decoded and was skipped"));
+            self.warn(UNDECODED);
         }
         self.images.insert(cache, img.clone());
         img

crates/pdf/src/print/mod.rs

@@ -30,6 +30,7 @@ mod settings;
 mod tiling;
 
 use std::borrow::Cow;
+use std::collections::HashSet;
 use std::sync::Arc;
 
 use kurbo::{Affine, Rect};
@@ -44,7 +45,7 @@ pub use plates::print_inks;
 pub use settings::*;
 pub use tiling::{TilingPage, tiling};
 
-use crate::export::{Exporter, Sheet, Writer};
+use crate::export::{Exporter, Sheet, Writer, leaving_out_undecodable};
 use crate::marks::PageBoxes;
 use crate::{AdvancedSettings, ColorConversion, CompressionSettings, OutputSettings, Overprint, PdfError, PdfSettings};
 use layout::Layout;
@@ -391,10 +392,10 @@ pub fn plan<'a>(doc: &'a Document, opts: &PrintOptions) -> Result<PrintPlan<'a>,
 
 /// Print `doc` to a PDF as `opts` say.
 pub fn print(doc: &Document, opts: &PrintOptions) -> Result<PrintReport, PdfError> {
-    craft_pdf::write::guard(|| print_inner(doc, opts))
+    craft_pdf::write::guard(|| leaving_out_undecodable(|left_out| print_inner(doc, opts, left_out)))
 }
 
-fn print_inner(doc: &Document, opts: &PrintOptions) -> Result<PrintReport, PdfError> {
+fn print_inner(doc: &Document, opts: &PrintOptions, left_out: &mut HashSet<String>) -> Result<PrintReport, PdfError> {
     let plan = plan(doc, opts)?;
     let doc = &*plan.doc;
     let separations = opts.settings.output.mode == OutputMode::Separations;
@@ -413,6 +414,7 @@ fn print_inner(doc: &Document, opts: &PrintOptions) -> Result<PrintReport, PdfEr
         ex.out = w.out.clone();
         // Composite output keeps overprints as the Advanced option says (plates darken them).
         ex.overprint = !separations && pdf.advanced.overprint == Overprint::Preserve;
+        ex.left_out = left_out.clone();
     }
     for &i in &plan.order {
         let Some(s) = plan.sheets.get(i) else { continue };
@@ -423,7 +425,7 @@ fn print_inner(doc: &Document, opts: &PrintOptions) -> Result<PrintReport, PdfEr
     for ex in exporters {
         w.absorb(ex);
     }
-    let (bytes, more) = w.finish()?;
+    let (bytes, more) = w.finish(left_out)?;
     let bytes = crate::forms::finish(bytes, doc, &pdf, false, overprinted)?;
     let mut warnings = plan.warnings;
     for m in more {

crates/pdf/src/tests.rs

@@ -538,6 +538,53 @@ fn image_roundtrip() {
     assert_eq!(decoded.get_pixel(3, 0).0, [0, 0, 0, 255]);
 }
 
+/// A 4 × 2 red image encoded as `format`, cut short `keep` bytes past the first `tag` in it: its
+/// header reads, and its pixel data can't be decoded.
+fn cut_image(format: image::ImageFormat, tag: &[u8], keep: usize) -> Vec<u8> {
+    let mut out = vec![];
+    image::RgbaImage::from_pixel(4, 2, image::Rgba([255, 0, 0, 255])).write_to(&mut std::io::Cursor::new(&mut out), format).unwrap();
+    let at = out.windows(tag.len()).position(|w| w == tag).expect("tag");
+    out.truncate(at + tag.len() + keep);
+    out
+}
+
+/// An image whose pixel data can't be decoded (a PNG, GIF or WebP whose header reads) is left out
+/// of the PDF and of a print job, with one note however many there are, and the rest of the page is
+/// written.
+#[test]
+fn an_image_whose_pixels_cant_be_decoded_is_left_out_with_one_note() {
+    const NOTE: &str = "an image couldn't be decoded and is missing from the picture";
+    let mut d = doc(200.0, 300.0);
+    let blobs = [
+        ("good", ImageBlob::new("image/png", tiny_png())),
+        ("png", ImageBlob::new("image/png", cut_image(image::ImageFormat::Png, b"IDAT", 4))),
+        ("png2", ImageBlob::new("image/png", cut_image(image::ImageFormat::Png, b"IDAT", 6))),
+        ("gif", ImageBlob::new("image/gif", cut_image(image::ImageFormat::Gif, b"GIF89a", 7))),
+        ("webp", ImageBlob::new("image/webp", cut_image(image::ImageFormat::WebP, b"VP8L", 9))),
+    ];
+    for (i, (key, blob)) in blobs.into_iter().enumerate() {
+        d.images.insert(key.into(), blob);
+        let xf = Affine::translate((20.0, 20.0 + 50.0 * i as f64)) * Affine::scale(10.0);
+        let im = ImageObject { key: key.into(), width: 4, height: 2, xf, link: None, placement: Default::default() };
+        add(&mut d, Node::new(NodeId(0), NodeKind::Image(Box::new(im))));
+    }
+    add(&mut d, rect_node(Rect::new(100.0, 100.0, 150.0, 150.0), Color::BLACK));
+    let written = |bytes: &[u8]| {
+        let back = import(bytes).expect("import");
+        let l = leaves(&back);
+        let images: Vec<Rect> = l.iter().filter(|n| matches!(n.kind, NodeKind::Image(_))).filter_map(|n| n.geometric_bounds()).collect();
+        (images, l.iter().filter(|n| n.path_data().is_some()).count())
+    };
+    let r = export_with_report(&d, &PdfOptions::default()).expect("export");
+    assert_eq!(r.warnings, [NOTE]);
+    let (images, paths) = written(&r.bytes);
+    assert!(images.len() == 1 && close(images[0], Rect::new(20.0, 20.0, 60.0, 40.0), 0.01), "{images:?}");
+    assert_eq!(paths, 1);
+    let p = print(&d, &PrintOptions::default()).expect("print");
+    assert_eq!(p.warnings.iter().filter(|w| *w == NOTE).count(), 1, "{:?}", p.warnings);
+    assert_eq!(written(&p.bytes).0.len(), 1);
+}
+
 /// #2023: a group's Transform effect copies an image among its members as it does the paths: the
 /// PDF draws the image three times, where the effect put it.
 #[test]

crates/ui-egui/src/dialogs/save_pdf.rs

@@ -940,6 +940,26 @@ mod tests {
         assert!(r["warnings"].is_array());
     }
 
+    /// A placed image whose pixel data can't be decoded is left out of the PDF, and the status bar
+    /// shows the note raster exports give.
+    #[test]
+    fn an_image_that_couldnt_be_decoded_is_named_in_the_status() {
+        let (mut app, written, _) = app();
+        // A PNG whose header reads (so it places at its size) but whose pixel data is cut short.
+        let mut cut = vec![];
+        image::RgbaImage::from_pixel(64, 32, image::Rgba([200, 30, 30, 255]))
+            .write_to(&mut std::io::Cursor::new(&mut cut), image::ImageFormat::Png)
+            .unwrap();
+        let idat = cut.windows(4).position(|w| w == b"IDAT").unwrap();
+        cut.truncate(idat + 8);
+        // The image is placed on the first artboard.
+        app.run("file.place", json!({"name": "cut.png", "dataBase64": vectorcraft_format::base64_encode(&cut), "at": [60, 50]})).unwrap();
+        app.run("ui.savePdfDialog", json!({"path": "/tmp/cut.pdf"})).unwrap();
+        super::super::confirm(&mut app).unwrap();
+        assert_eq!(app.ui.status, "Saved /tmp/cut.pdf with 1 note(s): an image couldn't be decoded and is missing from the picture");
+        assert_eq!(written.borrow().len(), 1);
+    }
+
     #[test]
     fn presets_reset_the_settings_and_fields_fall_back_to_defaults() {
         let (mut app, _, _) = app();

docs/mcp.md

@@ -562,9 +562,11 @@ Long-tail commands:
 PDF files take the Save PDF dialog's options: `preset`, `standard`, `compatibility`, the General toggles and the
 `compression`, `marks`, `bleed`, `output`, `advanced` and `security` sections (`list_commands` with filter `exportPdf`
 documents every field). `document.exportPdf` and `export` (format `pdf`, the options in `options`) return `warnings`:
-options accepted but not applied yet, and features approximated or left out. A standard with a PDF version it doesn't
-allow is refused (PDF/A-2b at 1.3 or 2.0, PDF/X-4 above 1.6, PDF/X-1a and PDF/X-3 above 1.4); choosing a
-standard without giving `compatibility` sets its version (PDF 1.3 for PDF/X-1a and PDF/X-3).
+options accepted but not applied yet, and features approximated or left out. An image whose pixel data can't be
+decoded (a PNG cut short) is left out of the pages, and `warnings` holds the note "an image couldn't be decoded and
+is missing from the picture" once, as raster exports give it; `.ai` saves and `file.print` do the same. A standard with a
+PDF version it doesn't allow is refused (PDF/A-2b at 1.3 or 2.0, PDF/X-4 above 1.6, PDF/X-1a and PDF/X-3 above 1.4);
+choosing a standard without giving `compatibility` sets its version (PDF 1.3 for PDF/X-1a and PDF/X-3).
 A `security` password encrypts the file (RC4 40-bit at PDF 1.3, RC4 128-bit at 1.4–1.5, AES-128 at 1.6, AES-256 at
 1.7 and 2.0) with its permissions; either password opens it (`document.open {password}`), and a password with PDF/A or PDF/X is refused.
 Pattern fills and strokes are written as their tiles clipped to the area they paint (a stroke's outline, with its