Merge pull request #6103 from dotCooCoo/fix/svg-missing-links-key

SVG import: each linked file keeps image keys of its own

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#"<svg xmlns="http://www.w3.org/2000/svg" width="40" height="20"><image href="a.png" width="20" height="20"/><image href="b.png" x="20" width="20" height="20"/></svg>"#;
+    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#"<svg xmlns="http://www.w3.org/2000/svg" width="40" height="20"><image href="a.png" width="20" height="20"/><image href="b.png" x="20" width="20" height="20"/></svg>"#;
+    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.

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<Vec<u8>> {
-    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);
-    }
 }

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 }

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#"<image href="img/a.png" width="10" height="10"/><image href="img/b.png" x="20" width="10" height="10"/><image href="img/a.png" x="40" width="10" height="10"/>"#;
+    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#"<image href="img/a.png" width="10" height="10"/><image href="img/b.png" x="20" width="10" height="10"/><image href="img/a.png" x="40" width="10" height="10"/>"#;
+    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");