Place: an image whose pixels can't be read is refused instead of placed blank

Squashed from #4257 onto main.

crates/engine/src/cmd/clipboard/flavours.rs

@@ -10,8 +10,8 @@ use serde_json::{Value, json};
 use vectorcraft_doc::{Document, ImageObject, Node, NodeId, NodeKind, TextObject};
 use vectorcraft_geom::{Affine, Point};
 
-use super::super::fileio;
 use super::super::place::pt_per_px;
+use super::super::{fileio, links};
 use super::{Clipboard, CommandSpec, Result, Session, always, bad, cmd, f64_or, has_doc, looks_like_svg, point_param, str_param};
 
 /// Plain text: a type-only copy's text, else the SVG markup (Include SVG Code).
@@ -178,7 +178,7 @@ pub fn specs() -> Vec<CommandSpec> {
             "Load Image into Clipboard",
             [],
             None,
-            "{dataBase64 (PNG, JPEG, GIF, WebP, TIFF or BMP bytes), mime?: the type the system clipboard gave (an image/… type; the format is read from the bytes), center?: [x, y]} replace the clipboard with the image, embedded at 100% of its physical size (its resolution, else 72 ppi), centred on `center` (default: the first artboard) → {count, width, height (pt), pixelWidth, pixelHeight}; then run edit.paste",
+            "{dataBase64 (PNG, JPEG, GIF, WebP, TIFF or BMP bytes), mime?: the type the system clipboard gave (an image/… type; the format is read from the bytes), center?: [x, y]} replace the clipboard with the image, embedded at 100% of its physical size (its resolution, else 72 ppi), centred on `center` (default: the first artboard) → {count, width, height (pt), pixelWidth, pixelHeight}; then run edit.paste. A PNG, JPEG, GIF or WebP image whose pixels can't be read (a file cut short) is an error",
             has_doc,
             import_image
         ),
@@ -418,7 +418,9 @@ fn import_image(s: &mut Session, p: &Value) -> Result<Value> {
     if let Some(m) = str_param(p, "mime").filter(|m| !m.starts_with("image/") || *m == SVG) {
         return Err(bad(C, format!("`{m}` isn't a bitmap: use clipboard.importSvg, importPdf or importText")));
     }
-    let img = fileio::raster_image(&data_param(p, C)?).map_err(|e| bad(C, e.to_string()))?;
+    let data = data_param(p, C)?;
+    let mut img = fileio::raster_image(&data).map_err(|e| bad(C, e.to_string()))?;
+    img.blob = links::readable_and_kept(img.blob, links::kept_unread(&data), false).map_err(|e| bad(C, e.to_string()))?;
     let (sx, sy) = pt_per_px(img.ppi);
     let (w, h) = (img.width, img.height);
     let image = ImageObject {

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

@@ -282,16 +282,10 @@ fn a_raster_export_says_nothing_of_images_another_thread_could_not_decode() {
 
 #[test]
 fn a_raster_export_says_when_an_image_could_not_be_decoded() {
-    // A PNG whose header reads (so it places at its size) but whose pixel data is cut short:
-    // the renderer can't decode it and draws nothing, which the export must say.
+    // A PNG whose header reads but whose pixel data is cut short: the renderer can't decode it
+    // and draws nothing, which the export must say.
     let mut s = session(72.0, 36.0, 1);
-    let mut png = Vec::new();
-    image::RgbaImage::from_pixel(64, 32, image::Rgba([200, 30, 30, 255]))
-        .write_to(&mut std::io::Cursor::new(&mut png), image::ImageFormat::Png)
-        .unwrap();
-    let cut = png_chunk(&png, b"IDAT").map(|d| d.as_ptr() as usize - png.as_ptr() as usize).unwrap() + 4;
-    png.truncate(cut);
-    s.execute("file.place", &json!({"name": "cut.png", "dataBase64": vectorcraft_format::base64_encode(&png)})).unwrap();
+    crate::tests_menucmds::place_undecodable_png(&mut s);
     let r = s.execute("document.export", &json!({"format": "png", "background": "transparent"})).unwrap();
     let warnings = r["warnings"].as_array().cloned().unwrap_or_default();
     assert!(warnings.iter().any(|w| w.as_str().is_some_and(|w| w.contains("couldn't be decoded"))), "{r}");

crates/engine/src/cmd/links.rs

@@ -399,14 +399,56 @@ pub(crate) fn files_to_find(st: &crate::DocState, ids: Option<&[NodeId]>) -> Vec
 // ---------- reading files into the document ----------
 
 /// Keep `full` (a file's pixels) as image `key` of `d`, unless pixels are there already; a
-/// preview there is kept, and a linked image gets one (the one `full` carries, when it has one).
+/// preview there is kept, and a linked image gets the one `full` carries ([`readable`] makes it
+/// when the file is read). Nothing is decoded here.
 pub(crate) fn store_image(d: &mut Document, key: &str, full: ImageBlob, linked: bool) {
     let made = full.proxy.clone().filter(|_| linked);
     let blob = match d.images.get(key) {
-        Some(b) if !b.is_proxy() => b.clone(),
+        Some(b) if !b.is_proxy() => ImageBlob { proxy: b.proxy.clone().or(made), ..b.clone() },
         old => ImageBlob { proxy: old.and_then(|b| b.proxy.clone()).or(made), ..full },
     };
-    d.images.insert(key.to_string(), if linked { blob.with_proxy() } else { blob });
+    d.images.insert(key.to_string(), blob);
+}
+
+/// Whether `bytes` are a PNG, JPEG, GIF or WebP file: the formats [`fileio::raster_image`] keeps
+/// as they are after reading only their header.
+pub(crate) fn kept_unread(bytes: &[u8]) -> bool {
+    use image::ImageFormat as F;
+    matches!(image::guess_format(bytes), Ok(F::Png | F::Jpeg | F::Gif | F::WebP))
+}
+
+/// `blob`, an image [`fileio::raster_image`] read from a file, ready to keep. When `unread` (the
+/// file is one [`kept_unread`] names), its pixels are decoded once to check that they can be read,
+/// and a `linked` image's preview is made from that decode ([`ImageBlob::checked_with_proxy`]);
+/// otherwise a `linked` image gets its preview ([`ImageBlob::with_proxy`]). An error when the
+/// pixels can't be read.
+pub(crate) fn readable(blob: ImageBlob, unread: bool, linked: bool) -> Result<ImageBlob> {
+    if !unread {
+        return Ok(if linked { blob.with_proxy() } else { blob });
+    }
+    blob.checked_with_proxy(linked).ok_or_else(|| EngineError::Other("the image's pixels can't be read".into()))
+}
+
+/// [`readable`], and the pixels it decoded are kept in the image store the renderers share
+/// ([`vectorcraft_render::keep_decoded_image`]), so the first frame that draws the image finds
+/// them there and does not decode the file again.
+pub(crate) fn readable_and_kept(blob: ImageBlob, unread: bool, linked: bool) -> Result<ImageBlob> {
+    if !unread {
+        return readable(blob, unread, linked);
+    }
+    let (blob, pixels) = blob.checked_with_pixels(linked).ok_or_else(|| EngineError::Other("the image's pixels can't be read".into()))?;
+    if let Some(pixels) = pixels {
+        vectorcraft_render::keep_decoded_image(&blob, pixels);
+    }
+    Ok(blob)
+}
+
+/// The linked image file `bytes`, read ([`fileio::raster_image`]) with its preview; an error when
+/// its pixels can't be read ([`readable`]).
+fn linked_image(bytes: &[u8]) -> Result<RasterImage> {
+    let mut img = fileio::raster_image(bytes)?;
+    img.blob = readable(img.blob, kept_unread(bytes), true)?;
+    Ok(img)
 }
 
 /// The file read for a link: the link to it and its image.
@@ -415,7 +457,7 @@ type FileRead = (LinkInfo, RasterImage);
 /// The file at `f` read for a link like `base` (its image decoded).
 fn read_image(f: &mut Found, base: &LinkInfo) -> Result<FileRead> {
     let bytes = f.read()?;
-    Ok((f.link(&bytes, base), fileio::raster_image(&bytes)?))
+    Ok((f.link(&bytes, base), linked_image(&bytes)?))
 }
 
 /// A linked file read for the objects that show it.
@@ -553,7 +595,7 @@ pub(crate) fn set_image_from_path(d: &mut Document, id: NodeId, path: &str, doc_
     // the app connect to another computer.
     let path = fileio::document_path(path, doc_path).map_err(|r| EngineError::Other(format!("the image `{path}` isn't followed: {r}")))?;
     let bytes = read_file(&path)?;
-    let read = (link_info(&path, &bytes), fileio::raster_image(&bytes)?);
+    let read = (link_info(&path, &bytes), linked_image(&bytes)?);
     install(d, &[id], read, Some(Preserve::Bounds))
 }
 
@@ -678,11 +720,11 @@ pub fn resolve(d: &mut Document, doc_path: Option<&str>, update: bool, low_res_p
                 // Read when the images show the preview, or when the file's details changed.
                 let bytes = if (preview && !eps_preview) || f.bytes.is_some() { f.read().ok() } else { None };
                 if preview && !eps_preview {
-                    let Some((b, img)) = bytes.as_deref().and_then(|b| Some((b, fileio::raster_image(b).ok()?))) else {
+                    let Some((b, img)) = bytes.as_deref().and_then(|b| Some((b, linked_image(b).ok()?))) else {
                         out.missing.extend(row);
                         continue;
                     };
-                    let blob = img.blob.with_proxy();
+                    let blob = img.blob;
                     read_images.insert(f.path.clone(), (blob.clone(), hash_bytes(b)));
                     store_image(d, &g.key, blob, true);
                 } else if let Some(b) = d.images.get(&g.key).filter(|b| b.proxy.is_none()) {
@@ -935,7 +977,7 @@ fn relink(s: &mut Session, p: &Value) -> Result<Value> {
             }
         }
         // Read before it is copied: a file that can't be shown isn't copied.
-        let img = if images.is_empty() { None } else { Some(fileio::raster_image(&bytes).map_err(|e| bad(C, format!("{path}: {e}")))?) };
+        let img = if images.is_empty() { None } else { Some(linked_image(&bytes).map_err(|e| bad(C, format!("{path}: {e}")))?) };
         let mut sources = vec![];
         for ((page, bounding, page_box), ids) in documents {
             sources.push((page, ids, super::place::document::source(&path, &bytes, Some(&path), page, bounding, page_box, C)?));
@@ -1275,8 +1317,10 @@ fn unembed(s: &mut Session, p: &Value) -> Result<Value> {
         return Ok(json!({ "name": format!("{stem}.{ext}"), "dataBase64": vectorcraft_format::base64_encode(&bytes) }));
     };
     let path = absolute_path(&if fileio::extension(path).is_empty() { format!("{path}.{ext}") } else { path.to_string() });
+    // Read before writing: pixels that can't be read leave no file behind.
+    let img = linked_image(&bytes)?;
     write_file(&path, &bytes)?;
-    let read = (link_info(&path, &bytes), fileio::raster_image(&bytes)?);
+    let read = (link_info(&path, &bytes), img);
     s.edit("Unembed", |d, _| install(d, &[id], read, Some(Preserve::Bounds)))?;
     Ok(json!({ "id": id.0, "path": path }))
 }

crates/engine/src/cmd/place.rs

@@ -63,7 +63,7 @@ pub fn specs() -> Vec<CommandSpec> {
             "Place…",
             ["File"],
             Some("Cmd+Shift+P"),
-            "{path | name | dataBase64 (a file's path, or its name with its bytes), link?: true (a raster image keeps its file's path; a VectorCraft document at path becomes one locked placed document showing artboard `page`, linked to the file and read again when it changes (links.update; saving it in this app updates it at once), drawn as vectors in every output; so does a PDF/.ai page at path, showing its `crop` box, unless password or textAs: \"outlines\" is given; false: an editable copy of its art, as from dataBase64; other files are embedded), text?: {characterSet?: \"unicode\" (UTF-8, or UTF-16 with a byte-order mark; other bytes as the platform's 8-bit set) | \"ansi\" (the platform's 8-bit set), platform?: \"windows\" (Windows-1252) | \"mac\" (Mac Roman), removeLineReturns?: false (each block of lines becomes one paragraph; blank lines end paragraphs), removeParagraphReturns?: false (drop blank lines), replaceSpaces?: n (runs of n ≥ 2 spaces become a tab)} (a .txt file, placed as area type filling rect, the replaced object's bounds, or else the artboard less a 36 pt margin), template?: false (onto a new locked template layer below the current layer), replace?: false (swap the one selected object, keeping its stacking place and transform; no at/rect), at?: [x, y] centre (default: the first artboard's centre), rect?: [x, y, width, height] fit inside; aspect kept (wins over at), page?: 1 (PDF/.ai page, or a native document's artboard), crop?: \"crop\" (clipped to that page or artboard, default; an SVG, only when given: framed and clipped by its viewBox, else by its art's bounds) | \"bounding\" (the bounds of the art that shows, with its strokes and effects, as an export measures them: hidden objects and guides left out) | \"art\" | \"trim\" | \"bleed\" | \"media\" (a PDF page's boxes), password? (an encrypted PDF), dxf?: {…the DXF options of document.open} (a .dxf drawing: fit fills the artboard under at, else the first; fitted or uncentred art lands with its drawing's artboard on that artboard, else it is centred on at)} → {ids, name, format, linked, width, height, warnings}. Raster images come in at 100% of their physical size (the file's ppi, else 72); SVG, DXF, PDF/.ai, EMF/WMF (clipped to the picture's frame), EPS (clipped to its bounding box; as document.open reads it), a .ai saved without PDF compatibility (from its editing data, as document.open reads it: the art of the layers that show on artboard `page`, clipped to that artboard unless crop is \"bounding\") and native documents as one group, with the images, symbols, patterns and swatches they use. One undo step; selects what it placed (unless on a template layer); never touches the clipboard",
+            "{path | name | dataBase64 (a file's path, or its name with its bytes), link?: true (a raster image keeps its file's path; a VectorCraft document at path becomes one locked placed document showing artboard `page`, linked to the file and read again when it changes (links.update; saving it in this app updates it at once), drawn as vectors in every output; so does a PDF/.ai page at path, showing its `crop` box, unless password or textAs: \"outlines\" is given; false: an editable copy of its art, as from dataBase64; other files are embedded), text?: {characterSet?: \"unicode\" (UTF-8, or UTF-16 with a byte-order mark; other bytes as the platform's 8-bit set) | \"ansi\" (the platform's 8-bit set), platform?: \"windows\" (Windows-1252) | \"mac\" (Mac Roman), removeLineReturns?: false (each block of lines becomes one paragraph; blank lines end paragraphs), removeParagraphReturns?: false (drop blank lines), replaceSpaces?: n (runs of n ≥ 2 spaces become a tab)} (a .txt file, placed as area type filling rect, the replaced object's bounds, or else the artboard less a 36 pt margin), template?: false (onto a new locked template layer below the current layer), replace?: false (swap the one selected object, keeping its stacking place and transform; no at/rect), at?: [x, y] centre (default: the first artboard's centre), rect?: [x, y, width, height] fit inside; aspect kept (wins over at), page?: 1 (PDF/.ai page, or a native document's artboard), crop?: \"crop\" (clipped to that page or artboard, default; an SVG, only when given: framed and clipped by its viewBox, else by its art's bounds) | \"bounding\" (the bounds of the art that shows, with its strokes and effects, as an export measures them: hidden objects and guides left out) | \"art\" | \"trim\" | \"bleed\" | \"media\" (a PDF page's boxes), password? (an encrypted PDF), dxf?: {…the DXF options of document.open} (a .dxf drawing: fit fills the artboard under at, else the first; fitted or uncentred art lands with its drawing's artboard on that artboard, else it is centred on at)} → {ids, name, format, linked, width, height, warnings}. Raster images come in at 100% of their physical size (the file's ppi, else 72); SVG, DXF, PDF/.ai, EMF/WMF (clipped to the picture's frame), EPS (clipped to its bounding box; as document.open reads it), a .ai saved without PDF compatibility (from its editing data, as document.open reads it: the art of the layers that show on artboard `page`, clipped to that artboard unless crop is \"bounding\") and native documents as one group, with the images, symbols, patterns and swatches they use. One undo step; selects what it placed (unless on a template layer); never touches the clipboard. A PNG, JPEG, GIF or WebP image whose pixels can't be read (a file cut short) is an error, and nothing is placed",
             has_doc,
             place
         ),
@@ -479,15 +479,17 @@ impl PlaceJob {
         str_param(&self.params, "path").or_else(|| str_param(&self.params, "name")).map(fileio::file_name).unwrap_or_else(|| "Untitled".into())
     }
 
-    /// Read and decode the file, as `file.place` does, without the session. A linked image's
-    /// preview is made here too.
+    /// Read and decode the file, as `file.place` does, without the session. The pixels of a PNG,
+    /// JPEG, GIF or WebP file are decoded here to check that they can be read (an error when they
+    /// can't), a linked image's preview is made from that decode, and the decoded pixels are kept in
+    /// the renderers' image store for the first frame that draws them
+    /// ([`super::links::readable_and_kept`]).
     pub fn read(self) -> Result<PlaceRead> {
         let mut loaded = load(&self.params, PLACE, self.target, true)?;
-        if loaded.link.is_some()
-            && bool_or(&self.params, "link", true)
-            && let Art::Image(img) = &mut loaded.art
-        {
-            img.blob = img.blob.clone().with_proxy();
+        let linked = loaded.link.is_some() && bool_or(&self.params, "link", true);
+        if let Art::Image(img) = &mut loaded.art {
+            let unread = super::links::kept_unread(&img.blob.bytes);
+            img.blob = super::links::readable_and_kept(img.blob.clone(), unread, linked)?;
         }
         Ok(PlaceRead { job: self, loaded })
     }
@@ -719,8 +721,19 @@ fn read_queue(p: Value, queued: Vec<Value>) -> Result<QueueRead> {
                 q[k] = v.clone();
             }
         }
-        // Read for its size and thumbnail: each click places it with file.place.
-        match load(&q, QUEUE, None, false) {
+        // Read for its size and thumbnail: each click places it with file.place, so a file whose
+        // pixels file.place can't read is skipped. With a thumbnail, the decoded pixels are kept
+        // so that drawing it does not decode the file again.
+        let checked = load(&q, QUEUE, None, false).and_then(|mut l| {
+            let linked = l.link.is_some() && bool_or(&q, "link", true);
+            if let Art::Image(img) = &mut l.art {
+                let unread = super::links::kept_unread(&img.blob.bytes);
+                let check = if thumbnail.is_some() { super::links::readable_and_kept } else { super::links::readable };
+                img.blob = check(img.blob.clone(), unread, linked)?;
+            }
+            Ok(l)
+        });
+        match checked {
             Ok(l) => {
                 let (w, h) = (l.natural.width(), l.natural.height());
                 let mut f = json!({ "name": l.name, "format": l.format.id, "width": w, "height": h });