From e29e01b6505c387b94c13a42329e06210d8bc98c Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Sat, 7 Nov 2026 03:38:06 -0800 Subject: [PATCH] SVG import: each linked file keeps image keys of its own Images were keyed by their bytes alone, so two elements that link different files with the same bytes shared one image key. Every missing file shows the same placeholder, and files can also be copies of each other. Export reads a linked file's pixels into the image key when the document holds only a preview, so once one of the files was found or changed, the images of the other file exported and printed that file's pixels. A linked image's key now also holds a hash of its link's path, as the .ai reader's linked images do. Images of one file still share a key. The missing-file placeholder comes from vectorcraft_doc::links::missing_placeholder instead of a copy in the SVG importer. --- crates/engine/src/tests/links.rs | 57 +++++++++++++++++++++++++++ crates/svg/src/import/files.rs | 25 +----------- crates/svg/src/import/importer.rs | 12 +++++- crates/svg/tests/it/import_details.rs | 34 ++++++++++++++++ 4 files changed, 104 insertions(+), 24 deletions(-) diff --git a/crates/engine/src/tests/links.rs b/crates/engine/src/tests/links.rs index a4c222090..3fe46ad35 100644 --- a/crates/engine/src/tests/links.rs +++ b/crates/engine/src/tests/links.rs @@ -452,6 +452,63 @@ fn low_res_proxy_eps_export_uses_full_resolution_from_the_linked_file() { assert!(near(centre_colour(&s.doc().unwrap().doc), RED), "canvas still shows the preview"); } +/// An SVG linking two missing files: once one of them is found, export reads its pixels into its +/// own images only, and the other file's image keeps its placeholder. +#[test] +fn export_reads_a_found_svg_link_into_its_own_images_only() { + let dir = Folder::new("svg-missing-keys"); + let svg = r#""#; + let svg_path = dir.file("art.svg"); + write(&svg_path, svg.as_bytes()); + let mut s = session(); + open(&mut s, &svg_path); + let mut found = vec![]; + s.doc().unwrap().doc.visit_images(|_, im| found.push(im.clone())); + let [a, b] = &found[..] else { panic!("two images: {found:?}") }; + assert!(a.link.as_ref().unwrap().path.ends_with("a.png") && b.link.as_ref().unwrap().path.ends_with("b.png")); + let placeholder = s.doc().unwrap().doc.images[&b.key].bytes.clone(); + + let bytes = png(20, 20, RED); + write(&dir.file("a.png"), &bytes); + let (full, warnings) = cmd::links::full_linked_images(&s.doc().unwrap().doc, None); + assert_eq!(warnings.len(), 1, "b.png is still missing: {warnings:?}"); + assert_eq!(*full.images[&a.key].bytes, bytes, "a.png's pixels"); + assert_eq!(full.images[&b.key].bytes, placeholder, "b.png keeps its placeholder"); +} + +/// An SVG linking two files with the same pixels, saved with previews only: once one file is gone +/// and the other has changed, export reads the changed file into its own images only. +#[test] +fn export_reads_a_changed_svg_link_into_its_own_images_only() { + let dir = Folder::new("svg-same-pixels"); + let svg = r#""#; + let svg_path = dir.file("art.svg"); + write(&svg_path, svg.as_bytes()); + write(&dir.file("a.png"), &png(600, 300, RED)); + write(&dir.file("b.png"), &png(600, 300, RED)); + let mut s = session(); + open(&mut s, &svg_path); + let doc_path = dir.file("art.vectorcraft"); + save(&mut s, &doc_path); + std::fs::remove_file(dir.file("a.png")).unwrap(); + let blue = png(600, 300, BLUE); + write(&dir.file("b.png"), &blue); + + open(&mut s, &doc_path); + let mut found = vec![]; + s.doc().unwrap().doc.visit_images(|_, im| found.push(im.clone())); + let [a, b] = &found[..] else { panic!("two images: {found:?}") }; + assert!(a.link.as_ref().unwrap().path.ends_with("a.png") && b.link.as_ref().unwrap().path.ends_with("b.png")); + assert_ne!(a.key, b.key, "two files, two keys"); + let doc = &s.doc().unwrap().doc; + assert!(doc.images[&a.key].is_proxy() && doc.images[&b.key].is_proxy(), "previews"); + let preview = doc.images[&a.key].bytes.clone(); + let (full, warnings) = cmd::links::full_linked_images(doc, None); + assert_eq!(warnings.len(), 1, "a.png is missing: {warnings:?}"); + assert_eq!(*full.images[&b.key].bytes, blue, "b.png's new pixels"); + assert_eq!(full.images[&a.key].bytes, preview, "a.png keeps its preview"); +} + /// The half of opening that the desktop app runs on a thread of its own (`read_for_open`) reads /// the linked files and lays out the type. The half left for the UI thread (`open_read`) reads no /// file: with a linked file moved away in between, it opens the document that `document.open` opens. diff --git a/crates/svg/src/import/files.rs b/crates/svg/src/import/files.rs index b6d488c9e..9de3bebcd 100644 --- a/crates/svg/src/import/files.rs +++ b/crates/svg/src/import/files.rs @@ -11,6 +11,7 @@ use std::sync::atomic::{AtomicBool, AtomicU64, AtomicUsize, Ordering}; use usvg::{ImageHrefResolver, ImageKind, roxmltree}; use vectorcraft_doc::LinkInfo; +use vectorcraft_doc::links::missing_placeholder; use super::{Edits, depth, href}; use crate::{ImportOptions, MAX_ELEMENT_DEPTH, MAX_SVGZ, SvgError}; @@ -62,22 +63,6 @@ pub(in crate::import) struct Linked { pub missing: bool, } -/// The side of the placeholder image, in pixels. -const MARKER: u32 = 64; - -/// The placeholder of a missing linked image: a light tile, crossed out (drawn here). -fn marker() -> Option> { - let last = MARKER - 1; - let img = image::RgbaImage::from_fn(MARKER, MARKER, |x, y| { - let edge = x == 0 || y == 0 || x == last || y == last; - let cross = x.abs_diff(y) <= 1 || (x + y).abs_diff(last) <= 1; - image::Rgba(if edge || cross { [200, 60, 60, 255] } else { [236, 236, 236, 255] }) - }); - let mut png = Vec::new(); - img.write_to(&mut std::io::Cursor::new(&mut png), image::ImageFormat::Png).ok()?; - Some(png) -} - /// `s` with `%XX` escapes decoded (an href is a URL). fn percent_decoded(s: &str) -> String { if !s.contains('%') { @@ -159,7 +144,7 @@ impl Files { }; let source = source.or_else(|why| { warnings.push(format!("linked image '{h}' {why}: a placeholder keeps its place until it is relinked")); - let bytes = Arc::new(placeholder.get_or_insert_with(marker).clone().ok_or(why)?); + let bytes = Arc::new(placeholder.get_or_insert_with(missing_placeholder).clone().ok_or(why)?); files.links.insert(key(&bytes), Linked { link: LinkInfo::new(path), missing: true }); Ok::<_, &str>(Source::Raster(ImageKind::PNG, bytes)) }); @@ -479,10 +464,4 @@ mod tests { assert!(!read("2147483647")); assert!(read("1")); } - - #[test] - fn the_placeholder_is_a_png() { - let png = marker().unwrap(); - assert_eq!(image::load_from_memory(&png).unwrap().width(), MARKER); - } } diff --git a/crates/svg/src/import/importer.rs b/crates/svg/src/import/importer.rs index 49bcfaab7..55cf0aa91 100644 --- a/crates/svg/src/import/importer.rs +++ b/crates/svg/src/import/importer.rs @@ -26,6 +26,13 @@ pub(super) struct SymbolMade { reuse: Reuse, } +/// The image key of a linked image of the file at `path` that shows `bytes` (the file's pixels or +/// a placeholder): the key of the bytes and the path's. Images of other files that show the same +/// pixels have keys of their own, and reading one file's pixels in changes that file's images only. +fn linked_key(bytes: &[u8], path: &str) -> String { + format!("img-{:016x}-{}", fnv1a(bytes), vectorcraft_doc::links::hash_bytes(path.as_bytes())) +} + impl Importer { pub(super) fn warn(&mut self, s: String) { if !self.warnings.contains(&s) { @@ -721,11 +728,14 @@ impl Importer { .ok() .and_then(|r| r.into_dimensions().ok()) .unwrap_or((size.width().round().max(1.0) as u32, size.height().round().max(1.0) as u32)); - let key = format!("img-{:016x}", fnv1a(bytes)); // A linked file keeps a preview; a placeholder is all a missing one has (its preview). let linked = self.files.linked(i.kind()); let missing = linked.is_some_and(|l| l.missing); let link = linked.map(|l| l.link.clone()); + let key = match &link { + Some(l) => linked_key(bytes, &l.path), + None => format!("img-{:016x}", fnv1a(bytes)), + }; self.doc.images.entry(key.clone()).or_insert_with(|| { let blob = ImageBlob { mime: mime.into(), bytes: bytes.clone(), proxy: missing.then(|| bytes.clone()) }; if link.is_some() && !missing { blob.with_proxy() } else { blob } diff --git a/crates/svg/tests/it/import_details.rs b/crates/svg/tests/it/import_details.rs index 13518a6fd..7857ba07c 100644 --- a/crates/svg/tests/it/import_details.rs +++ b/crates/svg/tests/it/import_details.rs @@ -105,6 +105,40 @@ fn a_missing_link_warns_and_keeps_a_placeholder_in_its_box() { assert!(!w.is_empty()); } +/// Missing files show the same placeholder, each under a key of its own: the images of one file +/// share a key, and those of another file have a different one. +#[test] +fn each_missing_file_has_its_own_placeholder_key() { + let dir = folder("missing-keys"); + let body = r#""#; + let (d, _) = open_in(&dir, body); + let [(_, a), (_, b), (_, a2)] = images(&d)[..] else { panic!("{:?}", art(&d)) }; + assert_eq!(Path::new(&a.link.as_ref().unwrap().path), dir.join("img").join("a.png")); + assert_eq!(Path::new(&b.link.as_ref().unwrap().path), dir.join("img").join("b.png")); + assert_ne!(a.key, b.key, "two files, two keys"); + assert_eq!(a.key, a2.key, "one file, one key"); + assert!(d.images[&a.key].is_proxy() && d.images[&b.key].is_proxy(), "placeholders, not the files"); + assert_eq!(d.images[&a.key].bytes, d.images[&b.key].bytes, "the same placeholder"); +} + +/// Two files with the same pixels are linked under keys of their own: the images of one file share +/// a key, and those of the other file have a different one. +#[test] +fn files_with_the_same_pixels_have_keys_of_their_own() { + let dir = folder("same-pixels"); + let bytes = png(40, 20); + std::fs::write(dir.join("img").join("a.png"), &bytes).unwrap(); + std::fs::write(dir.join("img").join("b.png"), &bytes).unwrap(); + let body = r#""#; + let (d, w) = open_in(&dir, body); + assert!(w.is_empty(), "{w:?}"); + let [(_, a), (_, b), (_, a2)] = images(&d)[..] else { panic!("{:?}", art(&d)) }; + assert_ne!(a.key, b.key, "two files, two keys"); + assert_eq!(a.key, a2.key, "one file, one key"); + assert_eq!(*d.images[&a.key].bytes, bytes); + assert_eq!(*d.images[&b.key].bytes, bytes); +} + #[test] fn svg_images_import_as_vector_art() { let dir = folder("svgimage");