From c8a7420b93f685e381c14e7e86e7f351f2536530 Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Thu, 29 Oct 2026 18:54:38 +0700 Subject: [PATCH] Place: an image whose pixels can't be read is refused instead of placed blank Squashed from #4257 onto main. --- crates/doc/src/links.rs | 66 ++++++++++++++- crates/engine/src/cmd/clipboard/flavours.rs | 8 +- crates/engine/src/cmd/fileio/tests_raster.rs | 12 +-- crates/engine/src/cmd/links.rs | 62 ++++++++++++-- crates/engine/src/cmd/place.rs | 33 +++++--- crates/engine/src/tests/clipflavours.rs | 36 ++++++++ crates/engine/src/tests/links.rs | 39 +++++++++ crates/engine/src/tests/linkspanel.rs | 27 +++++- crates/engine/src/tests/menucmds.rs | 32 ++++++- crates/engine/src/tests/pattern.rs | 13 +-- crates/engine/src/tests/place.rs | 83 +++++++++++++++++++ crates/engine/src/tests/webexport.rs | 13 +-- crates/render/src/images.rs | 71 ++++++++++++++++ crates/render/src/lib.rs | 3 +- crates/render/src/paint.rs | 22 ++++- crates/render/src/tests.rs | 33 ++++++++ crates/testkit/src/fixtures.rs | 32 +++++++ crates/ui-egui/src/dialogs/flatten.rs | 6 +- crates/ui-egui/src/dialogs/save_pdf/tests.rs | 10 +-- .../src/dialogs/tests_raster_formats.rs | 6 +- .../ui-egui/src/dialogs/tests_save_for_web.rs | 6 +- crates/ui-egui/src/dialogs/tests_screens.rs | 6 +- .../src/panels/tests_css_properties.rs | 6 +- crates/ui-egui/src/placing/tests.rs | 42 +++++++++- crates/ui-egui/src/tests/place.rs | 7 +- crates/ui-egui/src/tests/sysclip.rs | 6 +- docs/mcp.md | 11 ++- docs/target-app-parity.md | 2 + 28 files changed, 589 insertions(+), 104 deletions(-) diff --git a/crates/doc/src/links.rs b/crates/doc/src/links.rs index 04bad52c6..2330e95ac 100644 --- a/crates/doc/src/links.rs +++ b/crates/doc/src/links.rs @@ -126,6 +126,40 @@ impl ImageBlob { self } + /// This blob after its pixels are decoded once, within [`crate::MAX_RASTER_ALLOC`], to check + /// that they can be read; with `proxy`, it also gets the preview [`Self::with_proxy`] makes, + /// made from that decode. `None` when the pixels can't be read, as with a file cut short that + /// has a header but not all its pixels. An image whose pixels would take more than the + /// allowance as RGBA is not decoded, and one whose decode needs more than the allowance stops + /// decoding; either is returned as it is. + pub fn checked_with_proxy(self, proxy: bool) -> Option { + self.checked_with_pixels(proxy).map(|(blob, _)| blob) + } + + /// [`Self::checked_with_proxy`], with the pixels it decoded: `None` beside the blob when it + /// decoded none (an image past the allowance). + pub fn checked_with_pixels(mut self, proxy: bool) -> Option<(Self, Option)> { + let max = crate::MAX_RASTER_ALLOC; + let reader = || image::ImageReader::new(Cursor::new(self.bytes.as_slice())).with_guessed_format().ok(); + let (w, h) = reader()?.into_dimensions().ok()?; + if u64::from(w) * u64::from(h) > max / 4 { + return Some((self, None)); + } + let mut r = reader()?; + let mut limits = image::Limits::default(); + limits.max_alloc = Some(max); + r.limits(limits); + let img = match r.decode() { + Ok(img) => img, + Err(image::ImageError::Limits(_)) => return Some((self, None)), + Err(_) => return None, + }; + if proxy && self.proxy.is_none() && w.max(h) > PROXY_SIZE { + self.proxy = proxy_of(&img).map(Arc::new); + } + Some((self, Some(img))) + } + /// Are the bytes the preview (the linked file hasn't been read)? pub fn is_proxy(&self) -> bool { self.proxy.as_ref().is_some_and(|p| Arc::ptr_eq(p, &self.bytes)) @@ -149,7 +183,12 @@ fn proxy_png(bytes: &[u8]) -> Option> { let mut limits = image::Limits::default(); limits.max_alloc = Some(crate::MAX_RASTER_ALLOC); reader.limits(limits); - let small = reader.decode().ok()?.thumbnail(PROXY_SIZE, PROXY_SIZE); + proxy_of(&reader.decode().ok()?) +} + +/// Decoded image `img` shrunk to fit [`PROXY_SIZE`] as a PNG. +fn proxy_of(img: &image::DynamicImage) -> Option> { + let small = img.thumbnail(PROXY_SIZE, PROXY_SIZE); let mut png = Vec::new(); small.write_to(&mut Cursor::new(&mut png), image::ImageFormat::Png).ok()?; Some(png) @@ -442,6 +481,31 @@ mod tests { assert!(ImageBlob::new("image/png", vec![1, 2, 3]).with_proxy().proxy.is_none(), "undecodable"); } + /// A checked image's pixels are decoded once: the preview comes from that decode, and an image + /// whose header reads but whose pixels don't is refused. + #[test] + fn a_checked_image_is_refused_when_its_pixels_cant_be_read() { + let full = png(600, 300); + let big = ImageBlob::new("image/png", full.clone()).checked_with_proxy(true).unwrap(); + let p = image::load_from_memory(big.proxy.as_ref().unwrap()).unwrap(); + assert_eq!((p.width(), p.height()), (256, 128)); + assert!(ImageBlob::new("image/png", full.clone()).checked_with_proxy(false).unwrap().proxy.is_none(), "no preview asked for"); + assert!(ImageBlob::new("image/png", png(40, 20)).checked_with_proxy(true).unwrap().proxy.is_none(), "small images keep their pixels"); + let cut = full[..full.len() / 2].to_vec(); + assert_eq!(image::ImageReader::new(Cursor::new(&cut)).with_guessed_format().unwrap().into_dimensions().unwrap(), (600, 300)); + assert!(ImageBlob::new("image/png", cut).checked_with_proxy(true).is_none(), "cut to half its length"); + assert!(ImageBlob::new("image/png", vec![1, 2, 3]).checked_with_proxy(false).is_none(), "not an image"); + } + + /// A checked image hands on the pixels it decoded, so they need not be decoded again. + #[test] + fn a_checked_image_hands_on_its_pixels() { + let full = png(600, 300); + let (blob, pixels) = ImageBlob::new("image/png", full.clone()).checked_with_pixels(true).unwrap(); + assert!(blob.proxy.is_some(), "the preview is made as checked_with_proxy makes it"); + assert_eq!(pixels.map(|p| p.to_rgba8()), Some(image::load_from_memory(&full).unwrap().to_rgba8())); + } + /// PNG CRC-32 (the same polynomial ZIP uses), so a hand-written header is one a decoder accepts. fn crc32(data: &[u8]) -> u32 { let mut crc = !0u32; diff --git a/crates/engine/src/cmd/clipboard/flavours.rs b/crates/engine/src/cmd/clipboard/flavours.rs index b7b046824..c4d5a0712 100644 --- a/crates/engine/src/cmd/clipboard/flavours.rs +++ b/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 { "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 { 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 { diff --git a/crates/engine/src/cmd/fileio/tests_raster.rs b/crates/engine/src/cmd/fileio/tests_raster.rs index 1c9b47abc..0c0184446 100644 --- a/crates/engine/src/cmd/fileio/tests_raster.rs +++ b/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}"); diff --git a/crates/engine/src/cmd/links.rs b/crates/engine/src/cmd/links.rs index b26cfb557..42aafc21d 100644 --- a/crates/engine/src/cmd/links.rs +++ b/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 { + 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 { + 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 { + 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 { 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 { } } // 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 { 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 })) } diff --git a/crates/engine/src/cmd/place.rs b/crates/engine/src/cmd/place.rs index ed4d0aba4..2ab05f69e 100644 --- a/crates/engine/src/cmd/place.rs +++ b/crates/engine/src/cmd/place.rs @@ -63,7 +63,7 @@ pub fn specs() -> Vec { "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 { 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) -> Result { 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 }); diff --git a/crates/engine/src/tests/clipflavours.rs b/crates/engine/src/tests/clipflavours.rs index a72e87c1c..8783c28f9 100644 --- a/crates/engine/src/tests/clipflavours.rs +++ b/crates/engine/src/tests/clipflavours.rs @@ -150,6 +150,42 @@ fn import_image_then_paste_gives_an_image() { assert!(s.execute("clipboard.importImage", &json!({"dataBase64": "!!"})).is_err()); } +/// A pasted PNG cut to half its length (a copied file still downloading) has a header but not +/// all its pixels: importing it is an error, and the clipboard keeps what it held. +#[test] +fn a_bitmap_whose_pixels_cant_be_read_is_not_pasted() { + let mut s = session(); + copy_red_square(&mut s); + let held = s.clipboard.clone(); + let cut = crate::tests_links::cut_png(600, 300); + let e = s.execute("clipboard.importImage", &json!({"dataBase64": vectorcraft_format::base64_encode(&cut), "mime": "image/png"})).unwrap_err(); + assert!(e.to_string().ends_with("the image's pixels can't be read"), "{e}"); + assert_eq!(s.clipboard.nodes, held.nodes); +} + +/// The pixels `clipboard.importImage` decodes to check a pasted bitmap are kept in the renderers' +/// image store, so the first render of the pasted image draws them without decoding it again. +#[test] +fn a_pasted_bitmap_is_drawn_without_decoding_it_again() { + // The image has pixels no other test uses, since every test shares the image store. + let img = image::RgbImage::from_fn(300, 200, |x, y| image::Rgb([(x % 253) as u8, (y % 233) as u8, 17])); + let mut png = vec![]; + img.write_to(&mut std::io::Cursor::new(&mut png), image::ImageFormat::Png).unwrap(); + let mut s = session(); + run(&mut s, "clipboard.importImage", json!({"dataBase64": vectorcraft_format::base64_encode(&png), "mime": "image/png"})); + run(&mut s, "edit.paste", json!({})); + let n = &pasted(&s)[0]; + let NodeKind::Image(im) = &n.kind else { panic!("not an image: {:?}", n.kind) }; + let d = s.doc().unwrap().doc.clone(); + assert!(vectorcraft_render::image_cached(&d.images[&im.key]), "kept before it is drawn"); + let decodes = vectorcraft_render::image_decodes(); + let r = vectorcraft_render::Renderer::new().render(&d, 400, 300, vectorcraft_geom::Affine::IDENTITY, &Default::default()); + assert_eq!(vectorcraft_render::image_decodes(), decodes, "drawn without a decode"); + let at = im.xf * Point::new(150.0, 100.0); + let px = r.pixel(at.x as u32, at.y as u32); + assert!(px[3] > 0 && px[2] < 100, "the image is drawn at its middle: {px:?}"); +} + #[test] fn copied_svgz_files_paste_as_svg() { let svg = ""; diff --git a/crates/engine/src/tests/links.rs b/crates/engine/src/tests/links.rs index 2ac7d5972..eabf5f7ef 100644 --- a/crates/engine/src/tests/links.rs +++ b/crates/engine/src/tests/links.rs @@ -255,6 +255,45 @@ fn a_missing_link_renders_its_preview_and_relinks() { assert!(s.execute("links.relink", &json!({"ids": [id.0]})).is_err(), "path or folder"); } +/// A `w`×`h` PNG of noise cut to half its length: its header reads, its pixels don't. +pub(crate) fn cut_png(w: u32, h: u32) -> Vec { + let img = image::RgbImage::from_fn(w, h, |x, y| { + let v = x.wrapping_mul(2_654_435_761) ^ y.wrapping_mul(40_503).rotate_left(13); + image::Rgb([v as u8, (v >> 8) as u8, (v >> 16) as u8]) + }); + let mut out = vec![]; + img.write_to(&mut std::io::Cursor::new(&mut out), image::ImageFormat::Png).unwrap(); + out.truncate(out.len() / 2); + out +} + +/// A linked file whose pixels can't be read (cut short) isn't shown: Relink is an error, and +/// Update Links and Open (Update Links: Automatically) count it as missing. The image keeps what +/// it showed. +#[test] +fn a_linked_file_whose_pixels_cant_be_read_is_not_shown() { + let dir = Folder::new("cut"); + let pic = dir.file("photo.png"); + write(&pic, &png(600, 300, RED)); + let mut s = session(); + let id = place(&mut s, &pic); + let shown = image(&s, id); + let doc_path = dir.file("poster.vectorcraft"); + save(&mut s, &doc_path); + let cut = dir.file("cut.png"); + write(&cut, &cut_png(300, 100)); + let e = s.execute("links.relink", &json!({"ids": [id.0], "path": cut})).unwrap_err(); + assert!(e.to_string().ends_with("the image's pixels can't be read"), "{e}"); + write(&pic, &cut_png(300, 100)); + assert_eq!(s.execute("links.update", &json!({})).unwrap(), json!({"updated": [], "missing": [id.0]})); + assert_eq!(image(&s, id), shown); + assert!(near(centre_colour(&s.doc().unwrap().doc), RED)); + s.execute("prefs.set", &json!({"key": "updateLinks", "value": "automatically"})).unwrap(); + let r = open(&mut s, &doc_path); + assert_eq!((r["missingLinks"][0]["ids"].clone(), r["updatedLinks"].clone()), (json!([id.0]), json!([])), "{r}"); + assert!(near(centre_colour(&s.doc().unwrap().doc), RED), "the image still shows its preview"); +} + #[test] fn a_modified_link_follows_the_update_links_preference() { let dir = Folder::new("modified"); diff --git a/crates/engine/src/tests/linkspanel.rs b/crates/engine/src/tests/linkspanel.rs index 56c6f904e..b08d9acab 100644 --- a/crates/engine/src/tests/linkspanel.rs +++ b/crates/engine/src/tests/linkspanel.rs @@ -68,7 +68,8 @@ fn an_image_too_large_to_draw_is_reported() { /// A gray image is counted as RGBA, 4 bytes a pixel: 23,171 × 23,170 1-bit pixels decode to 537 MB /// of gray and are 2,147,488,280 bytes as RGBA, past the 2 GiB allowance (shown at reduced size); -/// 23,170 × 23,170 are 2,147,395,600 bytes, within it. +/// 23,170 × 23,170 are 2,147,395,600 bytes, within it. The file within the allowance holds no +/// pixels, so Place decodes it and refuses it without the note. #[test] fn a_gray_image_too_large_as_rgba_is_reported_on_place() { const NOTE: &str = "an image of 23171 × 23170 pixels is shown at reduced size"; @@ -78,10 +79,12 @@ fn a_gray_image_too_large_as_rgba_is_reported_on_place() { write(&within, &png_header(23_170, 23_170)); let mut s = session(); assert_eq!(run(&mut s, "file.place", json!({"path": over, "at": [200, 150], "link": false}))["warnings"], json!([NOTE])); - assert_eq!(run(&mut s, "file.place", json!({"path": within, "at": [20, 20], "link": false}))["warnings"], json!([])); + assert_eq!(run(&mut s, "file.place.info", json!({"path": within}))["warnings"], json!([]), "the image within the allowance has nothing to say"); + let e = s.execute("file.place", &json!({"path": within, "at": [20, 20], "link": false})).unwrap_err(); + assert_eq!(e.to_string(), "the image's pixels can't be read"); let l = run(&mut s, "links.list", json!({})); let notes: Vec<&Value> = l["links"].as_array().unwrap().iter().map(|r| &r["note"]).collect(); - assert_eq!(notes, [&Value::Null, &json!(NOTE)], "top first: the image within the allowance has nothing to say"); + assert_eq!(notes, [&json!(NOTE)]); } /// File › Open says the same of a gray image past the allowance as RGBA, and nothing of one within it. @@ -182,6 +185,24 @@ fn unembed_writes_the_bytes_and_links_to_them() { assert!(s.execute("links.unembed", &json!({"id": id.0, "path": dir.file("photo.psd")})).is_err(), "not a format it writes"); } +/// An embedded image whose pixels can't be read (a document stores its bytes without decoding +/// them) is not unembedded, and no file is written. +#[test] +fn unembed_of_pixels_that_cant_be_read_writes_no_file() { + let dir = Folder::new("unembed-unread"); + let pic = dir.file("photo.png"); + write(&pic, &png(60, 30, RED)); + let mut s = session(); + let id = place_embedded(&mut s, &pic); + let key = image(&s, id).key; + std::sync::Arc::make_mut(&mut s.doc_mut().unwrap().doc).images.insert(key, vectorcraft_doc::ImageBlob::png(png_header(60, 30))); + let out = dir.file("out.png"); + let e = s.execute("links.unembed", &json!({"id": id.0, "path": out})).unwrap_err(); + assert!(e.to_string().ends_with("the image's pixels can't be read"), "{e}"); + assert!(!std::path::Path::new(&out).exists(), "no file left behind"); + assert!(image(&s, id).link.is_none()); +} + #[test] fn go_to_selects_the_image() { let dir = Folder::new("goto"); diff --git a/crates/engine/src/tests/menucmds.rs b/crates/engine/src/tests/menucmds.rs index 47d12d343..32ea11330 100644 --- a/crates/engine/src/tests/menucmds.rs +++ b/crates/engine/src/tests/menucmds.rs @@ -38,8 +38,8 @@ fn sel(s: &mut Session, ids: &[NodeId]) { s.execute("select.set", &json!({"ids": ids.iter().map(|i| i.0).collect::>()})).unwrap(); } -/// A placed 64 × 32 px 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. +/// A placed 64 × 32 px PNG whose header reads but whose pixel data is cut short +/// ([`cut_pixels`]): the renderer can't decode it and draws nothing. pub(crate) fn place_undecodable_png(s: &mut Session) -> NodeId { place_undecodable_png_sized(s, 64, 32) } @@ -51,10 +51,34 @@ pub(crate) fn place_undecodable_png_sized(s: &mut Session, w: u32, h: u32) -> No image::RgbaImage::from_pixel(w, h, image::Rgba([200, 30, 30, 255])) .write_to(&mut std::io::Cursor::new(&mut png), image::ImageFormat::Png) .unwrap(); + let r = s.execute("file.place", &json!({"name": "cut.png", "dataBase64": vectorcraft_format::base64_encode(&png)})).unwrap(); + let id = NodeId(r["ids"][0].as_u64().unwrap()); + cut_pixels(s, id); + id +} + +/// Cut the pixel data of the PNG that placed image `id` shows short, right after its first `IDAT` +/// chunk's type, as a document saved with a damaged image holds it: the header still reads, but +/// the renderer can't decode the image. Place refuses such a file, so tests cut a placed one. The +/// image is stored under the key of its new bytes. +pub(crate) fn cut_pixels(s: &mut Session, id: NodeId) { + let doc = std::sync::Arc::make_mut(&mut s.doc_mut().unwrap().doc); + let Some(NodeKind::Image(im)) = doc.node(id).map(|n| &n.kind) else { panic!("{id:?} is not an image") }; + let old = im.key.clone(); + let mut png = doc.images[&old].bytes.to_vec(); let idat = png.windows(4).position(|w| w == b"IDAT").unwrap(); png.truncate(idat + 8); - let r = s.execute("file.place", &json!({"name": "cut.png", "dataBase64": vectorcraft_format::base64_encode(&png)})).unwrap(); - NodeId(r["ids"][0].as_u64().unwrap()) + let cut = vectorcraft_doc::ImageBlob::new("image/png", png); + let key = cut.content_key(); + doc.images.insert(key.clone(), cut); + if let Some(Node { kind: NodeKind::Image(im), .. }) = doc.node_mut(id) { + im.key = key; + } + let mut shown = false; + doc.visit_images(|_, im| shown |= im.key == old); + if !shown { + doc.images.remove(&old); + } } /// Whether reply `r` warns that an image couldn't be decoded, as raster exports do. diff --git a/crates/engine/src/tests/pattern.rs b/crates/engine/src/tests/pattern.rs index 2b72f6602..f993372b8 100644 --- a/crates/engine/src/tests/pattern.rs +++ b/crates/engine/src/tests/pattern.rs @@ -480,17 +480,10 @@ fn pdf_export_writes_patterns_nested_too_deep_as_tiles_with_a_warning() { #[test] fn every_raster_export_says_when_a_pattern_holds_an_image_that_could_not_be_decoded() { - // A PNG whose header reads (so it places at its size) but whose pixel data is cut short, made - // into a pattern and then cleared from the page: only the pattern's tile draws it. + // A PNG whose header reads but whose pixel data is cut short, made into a pattern and then + // cleared from the page: only the pattern's tile draws it. let mut s = session(); - let mut png = Vec::new(); - image::RgbaImage::from_pixel(20, 20, image::Rgba([200, 30, 30, 255])) - .write_to(&mut std::io::Cursor::new(&mut png), image::ImageFormat::Png) - .unwrap(); - let idat = png.windows(4).position(|w| w == b"IDAT").unwrap(); - png.truncate(idat + 8); - let placed = s.execute("file.place", &json!({"name": "cut.png", "dataBase64": vectorcraft_format::base64_encode(&png)})).unwrap(); - let placed = NodeId(placed["ids"][0].as_u64().unwrap()); + let placed = crate::tests_menucmds::place_undecodable_png_sized(&mut s, 20, 20); sel(&mut s, &[placed]); s.execute("object.pattern.make", &json!({"name": "Cut", "width": 20, "height": 20})).unwrap(); s.execute("object.pattern.done", &json!({})).unwrap(); diff --git a/crates/engine/src/tests/place.rs b/crates/engine/src/tests/place.rs index 1d3558eaa..7c3ff56ab 100644 --- a/crates/engine/src/tests/place.rs +++ b/crates/engine/src/tests/place.rs @@ -378,6 +378,59 @@ fn a_linked_image_keeps_the_preview_made_when_its_file_was_read() { assert!(d.images["embedded"].proxy.is_none()); } +/// Keeping an image's pixels decodes nothing: a linked image read without a preview keeps none, +/// and pixels already in the document take the preview the file was read with. +#[test] +fn keeping_an_image_decodes_nothing() { + let bytes = png(600, 300, [9, 9, 9, 255], None); + let mut d = vectorcraft_doc::Document::new(100.0, 100.0); + cmd::links::store_image(&mut d, "k", vectorcraft_doc::ImageBlob::new("image/png", bytes.clone()), true); + assert!(d.images["k"].proxy.is_none(), "no preview is made here"); + let read = vectorcraft_doc::ImageBlob::new("image/png", bytes).with_proxy(); + let made = read.proxy.clone().unwrap(); + cmd::links::store_image(&mut d, "k", read, true); + assert!(d.images["k"].proxy.as_ref().is_some_and(|p| Arc::ptr_eq(p, &made)), "the preview it was read with"); +} + +/// `w`×`h` pixels of noise, which compress little, encoded as `format`. +fn noise(w: u32, h: u32, format: image::ImageFormat) -> Vec { + let img = image::RgbImage::from_fn(w, h, |x, y| { + let v = x.wrapping_mul(2_654_435_761) ^ y.wrapping_mul(40_503).rotate_left(13); + image::Rgb([v as u8, (v >> 8) as u8, (v >> 16) as u8]) + }); + let mut out = vec![]; + img.write_to(&mut std::io::Cursor::new(&mut out), format).unwrap(); + out +} + +/// A PNG cut to half its length, as a download or a sync still running leaves it, has a header +/// but not all its pixels: placing it, embedded or linked, is an error and places nothing. A JPEG +/// cut short still decodes, with its missing rows empty, and is placed. +#[test] +fn an_image_whose_pixels_cant_be_read_is_not_placed() { + let mut cut = noise(600, 300, image::ImageFormat::Png); + cut.truncate(cut.len() / 2); + let mut s = session(); + let info = s.execute("file.place.info", &file("cut.png", &cut)).unwrap(); + assert_eq!((info["pixelWidth"].as_u64(), info["pixelHeight"].as_u64()), (Some(600), Some(300)), "its header reads"); + let dir = crate::tests_links::Folder::new("cut-png"); + let path = dir.file("cut.png"); + crate::tests_links::write(&path, &cut); + for p in [file("cut.png", &cut), json!({"path": path}), json!({"path": path, "link": false})] { + let (undo, journal) = (s.doc().unwrap().history.undo.len(), s.journal.len()); + let e = s.execute("file.place", &p).unwrap_err(); + assert_eq!(e.to_string(), "the image's pixels can't be read", "{p}"); + assert!(s.prepare_place(&p).unwrap().read().is_err(), "{p}: read apart from the session"); + assert!(top_children(&s).is_empty(), "{p}: nothing placed"); + assert_eq!((s.doc().unwrap().history.undo.len(), s.journal.len()), (undo, journal), "{p}"); + } + let mut jpeg = noise(600, 300, image::ImageFormat::Jpeg); + jpeg.truncate(jpeg.len() / 2); + let r = place(&mut s, "cut.jpg", &jpeg, json!({})); + assert_eq!(r["format"], "jpg"); + assert_eq!(top_children(&s).len(), 1); +} + /// Type and a DXF drawing fitted to the artboard, read apart from the session while the artboard /// under `at` changes size, are placed against the artboard as it is when they are placed: as /// `file.place` places them after the change, which is what a replay of the journal does. @@ -724,6 +777,36 @@ fn files_read_for_the_place_cursor_apart_from_the_session_load_it_as_file_place_ assert_eq!(c.tool_id(), "selection"); } +/// A file whose pixels `file.place` can't read (a PNG cut short) is skipped by the place cursor's +/// queue, read with the session or apart from it. +#[test] +fn the_place_queue_skips_an_image_whose_pixels_cant_be_read() { + let mut cut = noise(600, 300, image::ImageFormat::Png); + cut.truncate(cut.len() / 2); + let p = json!({"files": [file("a.png", &png(40, 20, [9, 9, 9, 255], None)), file("cut.png", &cut)], "link": false}); + let (mut a, mut b) = (session(), session()); + let r = a.execute("file.place.queue", &p).unwrap(); + assert_eq!(r["count"], 1); + assert_eq!(r["skipped"], json!([{"name": "cut.png", "error": "the image's pixels can't be read"}])); + let read = b.prepare_place_queue(&p).unwrap().read().unwrap(); + assert_eq!(b.place_queue_read(read).unwrap(), r); +} + +/// The place queue's thumbnail of an image is drawn from the pixels its check decoded, so each +/// image is decoded once. +#[test] +fn the_place_queue_draws_a_thumbnail_without_decoding_the_image_again() { + // Pixels no other test uses, since every test shares the image store. + let img = image::RgbImage::from_fn(320, 160, |x, y| image::Rgb([(x % 251) as u8, 23, (y % 241) as u8])); + let mut bytes = vec![]; + img.write_to(&mut std::io::Cursor::new(&mut bytes), image::ImageFormat::Png).unwrap(); + let p = json!({"files": [file("q.png", &bytes)], "link": false, "thumbnail": 64}); + let decodes = vectorcraft_render::image_decodes(); + let r = session().execute("file.place.queue", &p).unwrap(); + assert!(r["files"][0]["thumbnailBase64"].as_str().is_some_and(|t| !t.is_empty()), "{r}"); + assert_eq!(vectorcraft_render::image_decodes(), decodes, "the thumbnail is drawn without a decode"); +} + #[test] fn bad_place_params_are_errors() { let mut s = session(); diff --git a/crates/engine/src/tests/webexport.rs b/crates/engine/src/tests/webexport.rs index 761d5d42e..c16896ac7 100644 --- a/crates/engine/src/tests/webexport.rs +++ b/crates/engine/src/tests/webexport.rs @@ -308,17 +308,6 @@ fn files_are_written_beside_the_path() { assert_eq!(r["bytes"].as_u64().unwrap(), std::fs::metadata(&page).unwrap().len() + std::fs::metadata(dir.join("images/page.png")).unwrap().len()); } -/// A 64×32 PNG whose header reads (so it places at its size) but whose pixel data is cut short. -fn cut_png() -> Vec { - 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 idat = png.windows(4).position(|w| w == b"IDAT").unwrap(); - png.truncate(idat + 8); - png -} - #[test] fn an_export_says_when_an_image_could_not_be_decoded() { let undecoded = @@ -328,7 +317,7 @@ fn an_export_says_when_an_image_could_not_be_decoded() { assert_eq!(r["warnings"], json!([]), "{r}"); assert_eq!(run(&mut s, "document.exportForWeb.preview", json!({}))["warnings"], json!([])); // The renderer can't decode the placed image and draws nothing in its place. - run(&mut s, "file.place", json!({"name": "cut.png", "dataBase64": vectorcraft_format::base64_encode(&cut_png())})); + crate::tests_menucmds::place_undecodable_png(&mut s); for format in ["gif", "jpg", "png8", "png24"] { let r = run(&mut s, "document.exportForWeb", json!({"format": format})); assert!(undecoded(&r), "{format}: {}", r["warnings"]); diff --git a/crates/render/src/images.rs b/crates/render/src/images.rs index d70b517d4..61f0f740c 100644 --- a/crates/render/src/images.rs +++ b/crates/render/src/images.rs @@ -198,6 +198,18 @@ impl ImageStore { pm } + /// Keep `pm` as `key` as [`ImageStore::insert`] does, holding no reference to it afterwards, + /// then drop entries over budget → whether the store holds `key`'s pixels. Pixels larger than + /// the budget are not kept. + pub(crate) fn keep(&mut self, key: ImageKey, pm: Arc) -> bool { + if pixmap_bytes(&pm) > self.budget { + return false; + } + drop(self.insert(key.clone(), pm)); + self.evict(); + self.entries.contains_key(&key) + } + /// Drop the least recently used entries no one else holds until within budget. fn evict(&mut self) { if self.bytes <= self.budget { @@ -271,6 +283,42 @@ pub fn image_cache_bytes() -> usize { store().bytes() } +/// Keep `img`, the decoded pixels of `blob`, in the store as a renderer keeps the pixels it +/// decodes, so the first render that draws `blob` uses them instead of decoding it. Returns whether +/// `img` was kept or the store already held `blob`'s pixels. It returns false, without looking at +/// the store, for a CMYK image and for pixels a renderer would refuse to decode: more than the +/// decode allowance as RGBA, or more than fits beside the pixels being drawn now. The store then +/// drops the least recently used entries beyond its budget, as it does whenever a renderer adds +/// pixels, and returns false when that leaves `img` out: pixels larger than the budget are never +/// kept. +pub fn keep_decoded_image(blob: &ImageBlob, img: image::DynamicImage) -> bool { + let (key, lut, _) = crate::blobs::blob_key(blob); + if lut.is_some() || !crate::paint::within_allowance(img.width(), img.height()) { + return false; + } + let kept = store().get(&key); + let size = match kept { + Some(pm) => (pm.width(), pm.height()), + None => { + let Some(pm) = crate::paint::decoded_pixmap(img) else { return false }; + let size = (pm.width(), pm.height()); + if !store().keep(key, Arc::new(pm)) { + return false; + } + size + } + }; + remember_size(blob, size); + true +} + +/// Whether the store holds the decoded pixels of `blob` at full size. +pub fn image_cached(blob: &ImageBlob) -> bool { + // The key first: working it out takes the store's lock too. + let (key, _, _) = crate::blobs::blob_key(blob); + store().peek(&key).is_some() +} + /// `blob`'s content key and kind, hashed and sniffed once per blob allocation. pub(crate) fn blob_id(blob: &ImageBlob) -> BlobId { if let Some(id) = store().known_id(&blob.bytes) { @@ -370,6 +418,29 @@ mod tests { assert_eq!(s.bytes(), 400); } + /// Pixels kept for a first frame are dropped at once when the budget has no room for them, + /// since nothing draws them yet. + #[test] + fn kept_pixels_larger_than_the_budget_are_not_kept() { + // 10×10 pixmaps: 400 bytes each. + let mut s = ImageStore::new(300); + assert!(!s.keep(key("big"), Arc::new(Pixmap::new(10, 10)))); + assert_eq!(s.bytes(), 0); + assert!(s.peek(&key("big")).is_none()); + // Room for one: the older one, held by no one, goes. + let mut s = ImageStore::new(400); + assert!(s.keep(key("a"), Arc::new(Pixmap::new(10, 10)))); + assert!(s.keep(key("b"), Arc::new(Pixmap::new(10, 10)))); + assert!(s.peek(&key("a")).is_none() && s.peek(&key("b")).is_some()); + assert_eq!(s.bytes(), 400); + // The one kept is held while drawn: a new one that doesn't fit beside it is dropped. + let held = s.get(&key("b")); + assert!(!s.keep(key("c"), Arc::new(Pixmap::new(10, 10)))); + assert!(s.peek(&key("c")).is_none()); + assert_eq!(s.bytes(), 400); + drop(held); + } + #[test] #[cfg(not(target_arch = "wasm32"))] fn default_budget_is_an_eighth_of_installed_memory_at_most_1_gib() { diff --git a/crates/render/src/lib.rs b/crates/render/src/lib.rs index f7541c07a..d76c1a52c 100644 --- a/crates/render/src/lib.rs +++ b/crates/render/src/lib.rs @@ -14,11 +14,12 @@ mod freeform; mod fx; mod group; mod images; +pub use images::{image_cached, keep_decoded_image}; mod ink; mod live; mod lru; mod paint; -pub use paint::{decode_failures, reduced_size}; +pub use paint::{decode_failures, image_decodes, reduced_size}; mod pattern; pub mod placed_document; pub mod proof; diff --git a/crates/render/src/paint.rs b/crates/render/src/paint.rs index dcaffadbc..e3c5ed30b 100644 --- a/crates/render/src/paint.rs +++ b/crates/render/src/paint.rs @@ -75,6 +75,8 @@ thread_local! { /// Images that couldn't be decoded (and so drew nothing) on this thread, counted for exports to /// report. static DECODE_FAILURES: std::cell::Cell = const { std::cell::Cell::new(0) }; + /// Images decoded, or tried, on this thread ([`image_decodes`]). + static DECODES: std::cell::Cell = const { std::cell::Cell::new(0) }; } #[cfg(test)] @@ -116,6 +118,12 @@ pub fn decode_failures() -> u64 { DECODE_FAILURES.with(std::cell::Cell::get) } +/// How many images [`decode_pixmap_or_why`] and [`decode_pixmap_fit`] have decoded on this thread so +/// far, failed decodes included. Images other threads decode are not counted. +pub fn image_decodes() -> u64 { + DECODES.with(std::cell::Cell::get) +} + /// [`decode_in_room`]'s image or error. #[cfg(test)] pub(crate) fn decode_within_allowance(bytes: &[u8]) -> image::ImageResult { @@ -157,6 +165,12 @@ fn room_for(rgba: u64, pinned: u64) -> Result<(), Undecoded> { } } +/// Whether `w`×`h` pixels may be decoded whole ([`room_for`]) beside the images paints hold now. +pub(crate) fn within_allowance(w: u32, h: u32) -> bool { + let bytes = (u64::from(w) * u64::from(h)).saturating_mul(4); + room_for(bytes, crate::images::store().pinned_bytes() as u64).is_ok() +} + /// Whether `bytes` of decoded pixels fit beside the images paints hold, which can't be evicted from /// the cache: a new image is decoded only while those and it fit twice [`MAX_DECODE_ALLOC`]. fn fits_beside_pinned(bytes: u64) -> bool { @@ -397,7 +411,7 @@ pub fn decode_pixmap(bytes: &[u8]) -> Option { /// [`reduced_size`] when it has one; a failure is counted in [`decode_failures`]. pub(crate) fn decode_pixmap_or_why(bytes: &[u8]) -> Result { counted(match decode_or_reduction(bytes) { - Ok(img) => pixmap_of(fit_pixmap(img).to_rgba8()).ok_or(Undecoded::Unreadable), + Ok(img) => decoded_pixmap(img).ok_or(Undecoded::Unreadable), Err(r) => r.and_then(|r| reduced_pixmap(bytes, &r)), }) } @@ -407,6 +421,7 @@ pub(crate) fn decode_pixmap_or_why(bytes: &[u8]) -> Result Result> { #[cfg(test)] DECODE_ATTEMPTS.with(|n| n.set(n.get() + 1)); + DECODES.with(|n| n.set(n.get() + 1)); match decode_in_room(bytes) { Ok(img) => Ok(img), Err((image::ImageError::Limits(_), Undecoded::Unreadable)) => Err(reduction(bytes, allowance()).ok_or(Undecoded::Unreadable)), @@ -471,6 +486,11 @@ fn pixmap_of(img: image::RgbaImage) -> Option { Some(vello_cpu::Pixmap::from_parts(data, w as u16, h as u16)) } +/// Decoded image `img` as [`decode_pixmap_or_why`] makes it a pixmap. +pub(crate) fn decoded_pixmap(img: image::DynamicImage) -> Option { + pixmap_of(fit_pixmap(img).to_rgba8()) +} + /// The substitution highlights' pink: Preferences › Highlight Substituted Fonts and Highlight /// Alternate Glyphs, and Document Setup's Highlight Substituted Glyphs (screen only). pub(crate) const SUBSTITUTED: peniko::Color = peniko::Color::from_rgba8(255, 120, 190, 110); diff --git a/crates/render/src/tests.rs b/crates/render/src/tests.rs index e299a7d1f..1155f7d92 100644 --- a/crates/render/src/tests.rs +++ b/crates/render/src/tests.rs @@ -903,6 +903,39 @@ fn decode_failures_counts_this_threads_images_only() { assert_eq!(decode_failures(), before + 1, "this thread's failure"); } +/// Pixels decoded elsewhere and kept with [`keep_decoded_image`] are the pixels a renderer decodes, +/// premultiplied alike, and a render that draws the image uses them without decoding it. A CMYK +/// image's pixels are not kept. +#[test] +fn a_kept_decoded_image_is_drawn_without_decoding_it() { + use vectorcraft_doc::cmyk::Inks; + use vectorcraft_doc::{ImageBlob, ImageObject, NodeKind}; + // The image has pixels no other test uses, since every test shares the store, and half of + // them are partly transparent. + let img = image::RgbaImage::from_fn(37, 23, |x, y| image::Rgba([(x * 7) as u8, (y * 11) as u8, 99, if (x + y) % 2 == 0 { 255 } else { 77 }])); + let mut png = vec![]; + img.write_to(&mut std::io::Cursor::new(&mut png), image::ImageFormat::Png).unwrap(); + let blob = ImageBlob::new("image/png", png.clone()); + assert!(!image_cached(&blob)); + assert!(keep_decoded_image(&blob, image::load_from_memory(&png).unwrap())); + assert!(image_cached(&blob)); + let key = blobs::blob_key(&blob).0; + let kept = images::store().peek(&key).unwrap(); + assert!(kept.data() == paint::decode_pixmap(&png).unwrap().data(), "the pixels a renderer decodes"); + let mut d = Document::new(37.0, 23.0); + d.images.insert("kept".into(), blob); + let id = d.alloc_id(); + let im = ImageObject { key: "kept".into(), width: 37, height: 23, xf: Affine::IDENTITY, link: None, placement: Default::default(), pixels: None }; + d.insert(Some(d.layers[0].id), 0, Node::new(id, NodeKind::Image(Box::new(im)))).unwrap(); + let decodes = image_decodes(); + let r = Renderer::new().render(&d, 37, 23, Affine::IDENTITY, &RenderOptions::default()); + assert_eq!(image_decodes(), decodes, "drawn from the kept pixels"); + assert!(r.pixel(18, 11)[3] > 0, "the image is drawn"); + let cmyk = ImageBlob::cmyk_tiff(&Inks::new(2, 2, [10, 20, 30, 40].repeat(4)).unwrap()).unwrap(); + assert!(!keep_decoded_image(&cmyk, image::DynamicImage::new_rgba8(2, 2))); + assert!(!image_cached(&cmyk)); +} + /// An image drawn larger than its pixels samples smoothly, or with `smooth_images` off each screen /// pixel takes its nearest image pixel (Pixel Preview with File Handling › Display Bitmaps as /// Anti-aliased Images off, #394). diff --git a/crates/testkit/src/fixtures.rs b/crates/testkit/src/fixtures.rs index d79615cd8..656bf4a8b 100644 --- a/crates/testkit/src/fixtures.rs +++ b/crates/testkit/src/fixtures.rs @@ -52,6 +52,38 @@ pub fn select(s: &mut Session, ids: &[NodeId]) { exec(s, "select.set", json!({"ids": ids.iter().map(|i| i.0).collect::>()})); } +/// Place `png` as `cut.png` with `params` (`file.place`'s, besides `name` and `dataBase64`), then +/// cut the placed image's pixel data short right after its first `IDAT` chunk's type, as a +/// document saved with a damaged image holds it: the header still reads, but the renderer can't +/// decode the image and draws nothing. Place refuses such a file, so tests cut a placed one. The +/// image is stored under the key of its new bytes. → `file.place`'s reply. +pub fn place_cut_png(s: &mut Session, png: &[u8], params: Value) -> Value { + let mut p = json!({"name": "cut.png", "dataBase64": vectorcraft_format::base64_encode(png)}); + if let (Some(o), Value::Object(extra)) = (p.as_object_mut(), params) { + o.extend(extra); + } + let r = exec(s, "file.place", p); + let id = NodeId(r["ids"][0].as_u64().unwrap()); + let doc = Arc::make_mut(&mut s.doc_mut().unwrap().doc); + let Some(NodeKind::Image(im)) = doc.node(id).map(|n| &n.kind) else { panic!("{id:?} is not an image") }; + let old = im.key.clone(); + let mut bytes = doc.images[&old].bytes.to_vec(); + let idat = bytes.windows(4).position(|w| w == b"IDAT").unwrap(); + bytes.truncate(idat + 8); + let cut = vectorcraft_doc::ImageBlob::new("image/png", bytes); + let key = cut.content_key(); + doc.images.insert(key.clone(), cut); + if let Some(Node { kind: NodeKind::Image(im), .. }) = doc.node_mut(id) { + im.key = key; + } + let mut shown = false; + doc.visit_images(|_, im| shown |= im.key == old); + if !shown { + doc.images.remove(&old); + } + r +} + /// The three standard fixture states for sweeping commands. #[derive(Clone, Copy, Debug, PartialEq, Eq)] pub enum Fixture { diff --git a/crates/ui-egui/src/dialogs/flatten.rs b/crates/ui-egui/src/dialogs/flatten.rs index b4c54ed2f..3cd0a5111 100644 --- a/crates/ui-egui/src/dialogs/flatten.rs +++ b/crates/ui-egui/src/dialogs/flatten.rs @@ -319,16 +319,14 @@ mod tests { #[test] fn ok_says_in_the_status_bar_when_an_image_could_not_be_decoded() { const WARNING: &str = "an image couldn't be decoded and is missing from the picture"; - // A PNG whose header reads (so it places at its size) but whose pixel data is cut short. + // A PNG whose header reads but whose pixel data is cut short. 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 idat = png.windows(4).position(|w| w == b"IDAT").unwrap(); - png.truncate(idat + 8); for preview in [true, false] { let mut app = scene(); - let placed = app.run("file.place", json!({"name": "cut.png", "dataBase64": vectorcraft_format::base64_encode(&png)})).unwrap(); + let placed = vectorcraft_testkit::fixtures::place_cut_png(&mut app.session, &png, json!({})); // The image is half transparent itself, so Flatten Transparency flattens it wherever it was placed. app.run("transparency.set", json!({"ids": placed["ids"], "opacity": 50})).unwrap(); app.run("select.all", json!({})).unwrap(); diff --git a/crates/ui-egui/src/dialogs/save_pdf/tests.rs b/crates/ui-egui/src/dialogs/save_pdf/tests.rs index fc766c6d6..6ef85620a 100644 --- a/crates/ui-egui/src/dialogs/save_pdf/tests.rs +++ b/crates/ui-egui/src/dialogs/save_pdf/tests.rs @@ -131,15 +131,13 @@ fn warnings_reach_the_status_and_errors_keep_the_dialog_open() { #[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![]; + // A PNG whose header reads but whose pixel data is cut short. + let mut png = 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) + .write_to(&mut std::io::Cursor::new(&mut png), 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(); + vectorcraft_testkit::fixtures::place_cut_png(&mut app.session, &png, json!({"at": [60, 50]})); 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"); diff --git a/crates/ui-egui/src/dialogs/tests_raster_formats.rs b/crates/ui-egui/src/dialogs/tests_raster_formats.rs index 15fa77fda..24ffefbba 100644 --- a/crates/ui-egui/src/dialogs/tests_raster_formats.rs +++ b/crates/ui-egui/src/dialogs/tests_raster_formats.rs @@ -140,14 +140,12 @@ fn save_for_office_and_export_selection_say_when_an_image_could_not_be_decoded() let (mut app, written) = app(1); app.run("document.exportForOffice", json!({"path": "/x/o.png", "ppi": 72})).unwrap(); assert_eq!(app.ui.status, "Saved /x/o.png"); - // A PNG whose header reads (so it places at its size) but whose pixel data is cut short. + // A PNG whose header reads but whose pixel data is cut short. 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 idat = png.windows(4).position(|w| w == b"IDAT").unwrap(); - png.truncate(idat + 8); - app.run("file.place", json!({"name": "cut.png", "dataBase64": vectorcraft_format::base64_encode(&png), "at": [10, 10]})).unwrap(); + vectorcraft_testkit::fixtures::place_cut_png(&mut app.session, &png, json!({"at": [10, 10]})); app.run("document.exportForOffice", json!({"path": "/x/o.png", "ppi": 72})).unwrap(); assert_eq!(app.ui.status, format!("Saved /x/o.png {NOTE}")); // Export Selection of the placed image, which stays selected. diff --git a/crates/ui-egui/src/dialogs/tests_save_for_web.rs b/crates/ui-egui/src/dialogs/tests_save_for_web.rs index e5c834c10..3341d4340 100644 --- a/crates/ui-egui/src/dialogs/tests_save_for_web.rs +++ b/crates/ui-egui/src/dialogs/tests_save_for_web.rs @@ -578,11 +578,7 @@ fn saving_says_in_the_status_bar_when_an_image_could_not_be_decoded() { 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 idat = png.windows(4).position(|w| w == b"IDAT").unwrap(); - png.truncate(idat + 8); - let place = |app: &mut VectorcraftApp| { - app.run("file.place", json!({"name": "cut.png", "dataBase64": vectorcraft_format::base64_encode(&png), "at": [30, 20]})).unwrap() - }; + let place = |app: &mut VectorcraftApp| vectorcraft_testkit::fixtures::place_cut_png(&mut app.session, &png, json!({"at": [30, 20]})); let dir = vectorcraft_testkit::temp_dir("sfw-undecoded"); let (mut desktop, _) = app(&dir, false); desktop.run("file.saveForWeb", json!({"format": "png24"})).unwrap(); diff --git a/crates/ui-egui/src/dialogs/tests_screens.rs b/crates/ui-egui/src/dialogs/tests_screens.rs index 74132e4db..bf2aed988 100644 --- a/crates/ui-egui/src/dialogs/tests_screens.rs +++ b/crates/ui-egui/src/dialogs/tests_screens.rs @@ -456,11 +456,7 @@ fn the_status_bar_says_when_an_image_could_not_be_decoded() { 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 idat = png.windows(4).position(|w| w == b"IDAT").unwrap(); - png.truncate(idat + 8); - let place = |app: &mut VectorcraftApp| { - app.run("file.place", json!({"name": "cut.png", "dataBase64": vectorcraft_format::base64_encode(&png), "at": [10, 10]})).unwrap() - }; + let place = |app: &mut VectorcraftApp| vectorcraft_testkit::fixtures::place_cut_png(&mut app.session, &png, json!({"at": [10, 10]})); let rows = json!([{"format": "png", "scale": "1x", "suffix": ""}, {"format": "jpg", "scale": "2x", "suffix": "@2x"}]); let dir = temp_folder("undecoded"); let folder = dir.to_string_lossy().replace('\\', "/"); diff --git a/crates/ui-egui/src/panels/tests_css_properties.rs b/crates/ui-egui/src/panels/tests_css_properties.rs index 41cb6c0ab..1034daeb3 100644 --- a/crates/ui-egui/src/panels/tests_css_properties.rs +++ b/crates/ui-egui/src/panels/tests_css_properties.rs @@ -108,14 +108,12 @@ fn export_says_in_the_status_bar_when_an_image_could_not_be_decoded() { let (mut app, log) = app(); app.run("css.exportFile", json!({"path": "/tmp/site/site.css", "scope": "all", "rasterize": true})).unwrap(); assert_eq!(app.ui.status, "Exported /tmp/site/site.css"); - // A PNG whose header reads (so it places at its size) but whose pixel data is cut short. + // A PNG whose header reads but whose pixel data is cut short. 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 idat = png.windows(4).position(|w| w == b"IDAT").unwrap(); - png.truncate(idat + 8); - app.run("file.place", json!({"name": "cut.png", "dataBase64": vectorcraft_format::base64_encode(&png)})).unwrap(); + vectorcraft_testkit::fixtures::place_cut_png(&mut app.session, &png, json!({})); let r = app.run("css.exportFile", json!({"path": "/tmp/site/site.css", "scope": "all", "rasterize": true})).unwrap(); assert_eq!(r["warnings"], json!([WARNING])); assert_eq!(app.ui.status, format!("Exported /tmp/site/site.css with 1 note(s): {WARNING}")); diff --git a/crates/ui-egui/src/placing/tests.rs b/crates/ui-egui/src/placing/tests.rs index 0627b37bc..96f8f82a3 100644 --- a/crates/ui-egui/src/placing/tests.rs +++ b/crates/ui-egui/src/placing/tests.rs @@ -241,14 +241,49 @@ fn files_dropped_together_are_read_one_at_a_time() { assert_eq!(app.placing.reading[1].load(std::sync::atomic::Ordering::SeqCst), 1, "the most reads at once"); } +/// A PNG read off the UI thread is decoded there once: its pixels are in the image store the +/// renderers share before the canvas draws it, and the canvas's first frame with it decodes +/// nothing on the UI thread. +#[test] +fn a_placed_image_is_drawn_without_decoding_it_on_the_ui_thread() { + // The image has pixels no other test uses, since every test shares the image store. + let noise = image::RgbImage::from_fn(2000, 2000, |x, y| image::Rgb([(x % 251) as u8, (y % 241) as u8, ((x + y) % 239) as u8])); + let mut png = vec![]; + noise.write_to(&mut std::io::Cursor::new(&mut png), Png).unwrap(); + let path = saved("seeded", "photo.png", &png); + let mut app = app(true); + start(&mut app, &json!({"path": path}), false).unwrap(); + placed(&mut app); + assert_eq!(names(&app), ["photo.png"]); + let st = app.session.doc().unwrap(); + let blob = st.doc.images.values().next().unwrap().clone(); + assert!(vectorcraft_render::image_cached(&blob), "kept before the canvas draws it"); + let decodes = vectorcraft_render::image_decodes(); + let ctx = egui::Context::default(); + let raw = egui::RawInput { screen_rect: Some(egui::Rect::from_min_size(egui::Pos2::ZERO, egui::vec2(800.0, 600.0))), ..Default::default() }; + let mut out = ctx.run_ui(raw, |ui| crate::canvas::show(&mut app, ui)); + out.textures_delta.clear(); + assert!(app.canvas_rect.is_some(), "the canvas drew a frame"); + assert_eq!(vectorcraft_render::image_decodes(), decodes, "no decode on the UI thread"); +} + /// A file that can't be read shows the message it shows when read at once, dropped or placed -/// from the dialog, and nothing is placed. +/// from the dialog, and nothing is placed: a PNG cut to half its length, whose header reads but +/// whose pixels don't, too. #[test] fn a_file_that_cant_be_read_off_the_ui_thread_reports_as_at_once() { let gone = saved("unread", "gone.tif", b""); std::fs::remove_file(&gone).unwrap(); let unknown = saved("unread", "notes.xyz", b"hello"); - for path in [gone, unknown] { + let noise = image::RgbImage::from_fn(600, 300, |x, y| { + let v = x.wrapping_mul(2_654_435_761) ^ y.wrapping_mul(40_503).rotate_left(13); + image::Rgb([v as u8, (v >> 8) as u8, (v >> 16) as u8]) + }); + let mut png = vec![]; + noise.write_to(&mut std::io::Cursor::new(&mut png), image::ImageFormat::Png).unwrap(); + png.truncate(png.len() / 2); + let cut = saved("unread", "cut.png", &png); + for path in [gone, unknown, cut] { let p = json!({"path": path}); let mut at_once = app(false); crate::place::run_for_user(&mut at_once, &p).unwrap_err(); @@ -268,6 +303,9 @@ fn a_file_that_cant_be_read_off_the_ui_thread_reports_as_at_once() { } assert_eq!(app.ui.status, at_once.ui.status); assert!(app.ui.status.starts_with(&format!("Couldn't place {name}: ")), "{}", app.ui.status); + if name == "cut.png" { + assert_eq!(app.ui.status, "Couldn't place cut.png: the image's pixels can't be read"); + } } } diff --git a/crates/ui-egui/src/tests/place.rs b/crates/ui-egui/src/tests/place.rs index 7c887e1fe..1c99b2fd9 100644 --- a/crates/ui-egui/src/tests/place.rs +++ b/crates/ui-egui/src/tests/place.rs @@ -722,12 +722,9 @@ fn crop_image_shows_a_box_that_apply_crops_to() { #[test] fn crop_and_rasterize_say_in_the_status_bar_when_an_image_could_not_be_decoded() { const WARNING: &str = "an image couldn't be decoded and is missing from the picture"; - // A PNG whose header reads (so it places at its size) but whose pixel data is cut short. - let mut cut = png(100, 50, 72.0); - let idat = cut.windows(4).position(|w| w == b"IDAT").unwrap(); - cut.truncate(idat + 8); + // A PNG whose header reads but whose pixel data is cut short. let mut app = app(); - app.run("file.place", json!({"name": "cut.png", "dataBase64": vectorcraft_format::base64_encode(&cut)})).unwrap(); + vectorcraft_testkit::fixtures::place_cut_png(&mut app.session, &png(100, 50, 72.0), json!({})); let whole = selected_image(&app).geometric_bounds().unwrap(); app.run("ui.cropImage", json!({})).unwrap(); app.run("tool.setOption", json!({"key": "rect", "value": [whole.x0, whole.y0, whole.width() / 2.0, whole.height()]})).unwrap(); diff --git a/crates/ui-egui/src/tests/sysclip.rs b/crates/ui-egui/src/tests/sysclip.rs index 2e2197481..6a013e928 100644 --- a/crates/ui-egui/src/tests/sysclip.rs +++ b/crates/ui-egui/src/tests/sysclip.rs @@ -298,20 +298,18 @@ fn a_copy_while_the_worker_saves_writes_the_formats_at_once() { #[test] fn copy_says_in_the_status_bar_when_an_image_could_not_be_decoded() { const WARNING: &str = "an image couldn't be decoded and is missing from the picture"; - // A PNG whose header reads (so it places at its size) but whose pixel data is cut short. + // A PNG whose header reads but whose pixel data is cut short. 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 idat = png.windows(4).position(|w| w == b"IDAT").unwrap(); - png.truncate(idat + 8); for worker in [false, true] { let (mut app, board) = if worker { app_with_worker() } else { app() }; app.status("Ready"); copy_rect(&mut app); background::wait_all(&mut app); assert_eq!(app.ui.status, "Ready", "worker {worker}: no note"); - run(&mut app, "file.place", json!({"name": "cut.png", "dataBase64": vectorcraft_format::base64_encode(&png)})); + vectorcraft_testkit::fixtures::place_cut_png(&mut app.session, &png, json!({})); app.status("Ready"); run(&mut app, "edit.copy", json!({})); background::wait_all(&mut app); diff --git a/docs/mcp.md b/docs/mcp.md index 93d2eafbd..df5ae1fc5 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -3241,6 +3241,11 @@ The art's bounds are the bounds of the art that shows, with its strokes and effe hidden objects, guides and template layers do not count, and symbol instances are measured by their art. A page or artboard holds the objects whose shown art lies on it; a hidden object on it is still placed and stays hidden, as do the hidden members of the groups placed. `replace` and the `width` and `height` that `file.place` answers also measure symbol instances by their art. +`file.place` decodes the pixels of a PNG, JPEG, GIF or WebP file within the decode allowance before placing it. When +they can't be read, as with a PNG cut short by a download or a sync still running, it fails with "the image's pixels +can't be read" and places nothing; `clipboard.importImage` fails the same way. A JPEG cut short is placed when its +pixels still decode. Both keep the decoded pixels (except a CMYK JPEG's) in the image store the renderers share, so the first render +that draws the image does not decode the file again. `file.place.info` describes a file without placing it and `image.info` reports a placed image's link, colour mode and effective ppi. `file.place.queue` loads the place cursor (the `place` tool) with several files: headless, drive it with `pointer_gesture` (a click places at 100% with the top-left corner there, a drag at the dragged size) and @@ -3524,8 +3529,10 @@ The result lists `missingLinks` path isn't followed) when it isn't. The desktop app also checks the active document's links when its window comes back to the front, following the same preference. `links.check` reports every link's `status` (`ok`, `modified`, `missing`), `links.update {ids?}` reads modified files again and `links.relink {ids?, path | folder}` points images at -another file (or each at the file of its name in a folder); images keep their bounds, one undo step each. Without a -file system (the web), linked images show their previews. +another file (or each at the file of its name in a folder); images keep their bounds, one undo step each. A PNG, JPEG, +GIF or WebP file whose pixels can't be read is not shown: `links.relink` fails with "the image's pixels can't be +read", and `links.update`, or `document.open` when it reads the file, lists it as missing, its images keeping what they +showed. Without a file system (the web), linked images show their previews. `links.findFiles {folder}` looks in an absolute folder and its subfolders, on separate threads, for the files of the active document's missing links, or of the images and placed documents `ids` whatever their status, and returns at once diff --git a/docs/target-app-parity.md b/docs/target-app-parity.md index 2e0b27418..4080df73d 100644 --- a/docs/target-app-parity.md +++ b/docs/target-app-parity.md @@ -1013,6 +1013,8 @@ Moved from the ROADMAP's "Shipped so far" on 2026-10-10. Update it when a featur - File › Place reads Word `.docx` documents as area type with the same formatting, taken from the document defaults, the paragraph and character styles (their toggle properties reversing each other) and direct formatting, the theme's fonts included. Table rows are paragraphs with their cells separated by tabs; hidden and deleted text, field instructions, pictures, headers, footers and notes are left out (`file.place`, `crates/engine/src/tests/textimport.rs`). - A Word paragraph justified with a low, medium or high kashida length places justified with Insert Kashidas set to Short, Medium or Long (`file.place`, `crates/engine/src/tests/textimport.rs`). - On the desktop, File › Place reads and decodes files off the UI thread, one at a time, and places each once it is read, with "Placing" in the status bar and the busy spinner until then: the Place dialog's OK, a file dropped on the canvas, a click or drag of the place cursor, and the OK of the PDF, DXF and text import options. The files and thumbnails the place cursor is loaded with are read the same way, and the Place dialog's lines about its files are read on a thread of their own. Files are placed in the order they were started; a file is not placed while a dialog is open or a drag is under way, and a file whose document was closed meanwhile is not placed. `file.place` and `file.place.queue` over MCP and scripts reply as before. + - The pixels of a PNG, JPEG, GIF or WebP file that Place decodes to check them are kept in the image store the renderers share, so the canvas's first frame with the placed image draws them without decoding the file again on the UI thread: File › Place, a file dropped on the canvas, a click or drag of the place cursor, `file.place`, and Paste of such an image. A CMYK JPEG is still decoded when it is first drawn (`crates/ui-egui/src/placing.rs`, `crates/render/src/tests.rs`). + - File › Place and a file dropped on the canvas refuse a PNG, JPEG, GIF or WebP file whose pixels can't be read, such as one cut short by a download or a sync still running, and the status bar shows "Couldn't place : the image's pixels can't be read". Paste and Relink refuse such an image too, and Update Links, or Open when it reads the file again, counts it as missing. A JPEG cut short still places when its pixels can be decoded (`file.place`, `crates/engine/src/tests_place.rs`). - On the desktop, Copy and Cut make the system clipboard's formats (text, SVG, PDF, PNG) from a copy of the clipboard on the thread that writes background saves and exports, and write them when they are ready; while that thread has a save, an export, a print or a recovery copy to finish, they are made and written at once. Until they are written, Paste in VectorCraft takes the internal clipboard, and while a text field or the Type tool has the keyboard the app waits for them. Another Copy or Cut, or a copy of text in VectorCraft, drops them. - On the desktop with Enable Background Export on (Preferences › File Handling), File › Package… collects the linked files and fonts and writes the package from the document as it was at OK on the thread that writes background saves and exports, with "Packaging" in the status bar and the busy spinner until it is written; a save of unsaved changes that Package starts is written first. `file.package` over MCP and scripts replies as before. - On the desktop with Enable Background Export on, Export for Screens and the Asset Export panel's Export encode and write their files (into the folder, or one ZIP in it) from the document as it was at OK on the thread that writes background saves and exports, with "Exporting" in the status bar and the export progress dialog, which counts the files and whose Cancel stops before the next file. `document.exportForScreens` and `assets.export` over MCP and scripts reply as before.