M1: report images a native file lost on loading (#207) (#242)

A `.figcraft` image member that is damaged, too large or missing was skipped
without a word, and saving the file again dropped it for good. The load now
lists each lost image (up to 20, then a count) with its hash and the reason,
file.open puts it in `repairs` and sets a session message, and the app shows
that as a toast.

Co-authored-by: Claude Sonnet 5.5 <noreply@anthropic.com>
Co-authored-by: bflatastic <bombayapirat@gmail.com>

crates/engine/src/cmd/file.rs

@@ -662,6 +662,13 @@ fn open(s: &mut Session, p: &Value) -> Result<Value> {
     remember(s, &path);
     let native = !rest && matches!(ext(&path).as_str(), "figcraft" | "json");
     let warnings = rest || ext(&path) == "svg";
+    // A native file that lost something (a damaged image, say): the app says so, not only the result.
+    if native && !loaded.issues.is_empty() {
+        s.message = Some(match loaded.issues.as_slice() {
+            [one] => format!("{}: {one}", file_stem(&path)),
+            more => format!("{}: {} problems were repaired or left out while opening it, e.g. {}", file_stem(&path), more.len(), more[0]),
+        });
+    }
     s.docs.push(DocState::new(loaded.doc, native.then_some(path)).with_versions(loaded.versions).with_branches(loaded.branches));
     s.active = s.docs.len() - 1;
     let mut out = json!({"index": s.active});

crates/engine/src/lib.rs

@@ -1494,6 +1494,8 @@ mod nested_override_tests;
 #[cfg(test)]
 mod online_tests;
 #[cfg(test)]
+mod open_issues_tests;
+#[cfg(test)]
 mod outline_options_tests;
 #[cfg(test)]
 mod paint_reorder_tests;

crates/engine/src/open_issues_tests.rs

@@ -0,0 +1,51 @@
+//! Opening a native file that lost something (a damaged image): the session says so, with the
+//! reason, and the file still opens.
+
+use figcraft_doc::Document;
+use serde_json::json;
+
+use crate::Session;
+
+/// A 1×1 PNG.
+const PNG: [u8; 67] = [
+    0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a, 0x00, 0x00, 0x00, 0x0d, 0x49, 0x48, 0x44, 0x52, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x01,
+    0x08, 0x06, 0x00, 0x00, 0x00, 0x1f, 0x15, 0xc4, 0x89, 0x00, 0x00, 0x00, 0x0d, 0x49, 0x44, 0x41, 0x54, 0x78, 0x9c, 0x63, 0x60, 0x00, 0x00, 0x02,
+    0x00, 0x00, 0x01, 0xe2, 0x21, 0xbc, 0x33, 0x00, 0x00, 0x00, 0x00, 0x49, 0x45, 0x4e, 0x44, 0xae, 0x42, 0x60, 0x82,
+];
+
+fn file(name: &str, damage: bool) -> std::path::PathBuf {
+    let dir = std::env::temp_dir().join(format!("figcraft-open-issues-{}-{name}", std::process::id()));
+    std::fs::create_dir_all(&dir).unwrap();
+    let mut d = Document::new("Pics");
+    figcraft_format::add_image(&mut d, PNG.to_vec()).unwrap();
+    let mut bytes = figcraft_format::save_bytes(&d, None).unwrap();
+    if damage {
+        // Images are stored as they are: change one byte of the PNG, not its checksum.
+        let at = bytes.windows(PNG.len()).position(|w| w == PNG).unwrap();
+        bytes[at + 30] ^= 0xff;
+    }
+    let path = dir.join("pics.figcraft");
+    std::fs::write(&path, bytes).unwrap();
+    path
+}
+
+#[test]
+fn a_file_with_a_damaged_image_opens_and_says_what_was_lost() {
+    let path = file("damaged", true);
+    let mut s = Session::new();
+    let r = s.execute("file.open", &json!({"path": path.to_string_lossy()})).unwrap();
+    assert!(r["repairs"].as_array().is_some_and(|a| a.len() == 1), "{r}");
+    let m = s.message.take().expect("the session says so");
+    assert!(m.starts_with("pics: image ") && m.contains("couldn't be read") && m.contains("drops it"), "{m}");
+    let _ = std::fs::remove_dir_all(path.parent().unwrap());
+}
+
+#[test]
+fn an_intact_file_opens_without_a_message() {
+    let path = file("intact", false);
+    let mut s = Session::new();
+    let r = s.execute("file.open", &json!({"path": path.to_string_lossy()})).unwrap();
+    assert!(r["repairs"].as_array().is_some_and(Vec::is_empty), "{r}");
+    assert!(s.message.is_none());
+    let _ = std::fs::remove_dir_all(path.parent().unwrap());
+}

crates/format/src/image_tests.rs

@@ -0,0 +1,88 @@
+//! Images a native file can't give back: damaged or missing members are reported, not dropped
+//! without a word.
+
+use std::io::Write;
+
+use figcraft_doc::{Document, NodeKind};
+
+use super::*;
+
+fn png() -> Vec<u8> {
+    let img = image::RgbaImage::from_pixel(2, 2, image::Rgba([9, 0, 0, 255]));
+    let mut out = Vec::new();
+    img.write_to(&mut std::io::Cursor::new(&mut out), image::ImageFormat::Png).unwrap();
+    out
+}
+
+fn doc_with_image() -> (Document, String) {
+    let mut d = Document::new("File");
+    let p = d.first_page().unwrap();
+    d.create(p, NodeKind::Rectangle, "R", 0.0, 0.0, 10.0, 10.0).unwrap();
+    let (hash, ..) = add_image(&mut d, png()).unwrap();
+    (d, hash)
+}
+
+/// The archive of `d` rebuilt with `edit` applied to each member (None drops it).
+fn rebuild(bytes: &[u8], edit: impl Fn(&str, Vec<u8>) -> Option<Vec<u8>>) -> Vec<u8> {
+    let mut z = zip::ZipArchive::new(std::io::Cursor::new(bytes)).unwrap();
+    let names: Vec<String> = z.file_names().map(str::to_string).collect();
+    let mut buf = std::io::Cursor::new(Vec::new());
+    let mut w = zip::ZipWriter::new(&mut buf);
+    for n in names {
+        let data = read_member(&mut z, &n, u64::MAX).unwrap();
+        if let Some(data) = edit(&n, data) {
+            w.start_file(n, zip::write::SimpleFileOptions::default().compression_method(zip::CompressionMethod::Stored)).unwrap();
+            w.write_all(&data).unwrap();
+        }
+    }
+    w.finish().unwrap();
+    buf.into_inner()
+}
+
+#[test]
+fn an_intact_file_loads_without_image_issues() {
+    let (d, hash) = doc_with_image();
+    let (loaded, issues) = load_bytes(&save_bytes(&d, None).unwrap()).unwrap();
+    assert!(issues.is_empty(), "{issues:?}");
+    assert!(loaded.images.contains_key(&hash));
+}
+
+#[test]
+fn a_missing_image_member_is_reported() {
+    let (d, hash) = doc_with_image();
+    let bytes = rebuild(&save_bytes(&d, None).unwrap(), |n, data| (!n.starts_with("images/")).then_some(data));
+    let (loaded, issues) = load_bytes(&bytes).unwrap();
+    assert!(!loaded.images.contains_key(&hash));
+    assert!(issues.iter().any(|i| i.contains(&hash) && i.contains("missing") && i.contains("drops it")), "{issues:?}");
+}
+
+#[test]
+fn a_damaged_image_member_is_reported_with_its_reason() {
+    let (d, hash) = doc_with_image();
+    let bytes = save_bytes(&d, None).unwrap();
+    // The member's bytes change but not its checksum: reading it fails.
+    let mut z = zip::ZipArchive::new(std::io::Cursor::new(&bytes)).unwrap();
+    let name = format!("images/{hash}");
+    let stored = read_member(&mut z, &name, u64::MAX).unwrap();
+    let at = bytes.windows(stored.len()).position(|w| w == stored).unwrap();
+    let mut damaged = bytes.clone();
+    damaged[at + stored.len() / 2] ^= 0xff;
+    let (loaded, issues) = load_bytes(&damaged).unwrap();
+    assert!(!loaded.images.contains_key(&hash));
+    assert!(issues.iter().any(|i| i.contains(&hash) && i.contains("couldn't be read") && i.contains("drops it")), "{issues:?}");
+}
+
+#[test]
+fn many_lost_images_are_counted_not_listed_one_by_one() {
+    let mut d = Document::new("File");
+    for k in 0..40u8 {
+        let img = image::RgbaImage::from_pixel(2, 2, image::Rgba([k, 1, 0, 255]));
+        let mut out = Vec::new();
+        img.write_to(&mut std::io::Cursor::new(&mut out), image::ImageFormat::Png).unwrap();
+        add_image(&mut d, out).unwrap();
+    }
+    let bytes = rebuild(&save_bytes(&d, None).unwrap(), |n, data| (!n.starts_with("images/")).then_some(data));
+    let (_, issues) = load_bytes(&bytes).unwrap();
+    assert_eq!(issues.len(), MAX_IMAGE_ISSUES + 1, "{issues:?}");
+    assert_eq!(issues.last().map(String::as_str), Some("20 more images couldn't be loaded"));
+}

crates/format/src/lib.rs

@@ -465,11 +465,17 @@ pub fn load_bytes_with_versions(bytes: &[u8]) -> Result<Loaded, FormatError> {
     let (mut doc, mut issues, own_images) = parse_json(&text)?;
     let names: Vec<String> = z.file_names().filter(|n| n.starts_with("images/")).map(str::to_string).collect();
     let mut images = BTreeMap::new();
+    let mut unread = Vec::new();
     for name in names {
-        let Ok(data) = read_member(&mut z, &name, MAX_MEMBER) else { continue };
         let hash = name.trim_start_matches("images/").to_string();
-        images.insert(hash, image_data(data));
+        match read_member(&mut z, &name, MAX_MEMBER) {
+            Ok(data) => {
+                images.insert(hash, image_data(data));
+            }
+            Err(e) => unread.push((hash, e)),
+        }
     }
+    report_lost_images(&mut issues, &unread, own_images.as_ref(), &images);
     match library::read(&mut z, &images, u64::MAX) {
         Ok(lib) => doc.published = lib.map(Arc::new),
         Err(e) => issues.push(format!("the published library was skipped: {e}")),
@@ -490,6 +496,42 @@ pub fn load_bytes_with_versions(bytes: &[u8]) -> Result<Loaded, FormatError> {
     Ok(Loaded { doc, issues, versions, branches })
 }
 
+/// Most lost images reported one by one (the rest as a count).
+const MAX_IMAGE_ISSUES: usize = 20;
+
+/// Report the images a file lost on loading: members that couldn't be read (damaged, too large)
+/// and images its document lists but the archive doesn't hold. They are left out, so saving the
+/// file again would drop them for good.
+fn report_lost_images(
+    issues: &mut Vec<String>,
+    unread: &[(String, FormatError)],
+    listed: Option<&BTreeSet<String>>,
+    images: &BTreeMap<String, ImageData>,
+) {
+    let mut lost: Vec<String> = unread
+        .iter()
+        .map(|(hash, e)| {
+            let why = match e {
+                FormatError::Corrupt(m) | FormatError::Io(m) => m.clone(),
+                FormatError::Newer(v) => format!("format version {v}"),
+            };
+            format!("image {hash} couldn't be read ({why}): it is left out, and saving this file again drops it")
+        })
+        .collect();
+    let unread_hashes: BTreeSet<&str> = unread.iter().map(|(h, _)| h.as_str()).collect();
+    for hash in listed.into_iter().flatten() {
+        if !images.contains_key(hash) && !unread_hashes.contains(hash.as_str()) {
+            lost.push(format!("image {hash} is used by the file but its data is missing: it is left out, and saving this file again drops it"));
+        }
+    }
+    let more = lost.len().saturating_sub(MAX_IMAGE_ISSUES);
+    lost.truncate(MAX_IMAGE_ISSUES);
+    issues.extend(lost);
+    if more > 0 {
+        issues.push(format!("{more} more images couldn't be loaded"));
+    }
+}
+
 /// The version history kept under `dir` in an archive (none when it has no index), reading at
 /// most `budget` bytes of snapshots. Damaged entries are skipped and reported in `issues`; nothing
 /// here fails the load.
@@ -630,6 +672,8 @@ pub fn load_file(path: &std::path::Path) -> Result<(Document, Vec<String>), Form
 #[cfg(test)]
 mod branch_tests;
 #[cfg(test)]
+mod image_tests;
+#[cfg(test)]
 mod large_tests;
 #[cfg(test)]
 mod version_tests;

crates/ui-egui/src/lib.rs

@@ -113,6 +113,8 @@ mod live_text_tests;
 #[cfg(test)]
 mod online_tests;
 #[cfg(test)]
+mod open_issues_tests;
+#[cfg(test)]
 mod panel_reorder_tests;
 #[cfg(test)]
 mod panel_width_tests;

crates/ui-egui/src/open_issues_tests.rs

@@ -0,0 +1,37 @@
+//! Opening a native file with a damaged image: the app says what was lost.
+
+use base64::Engine as _;
+use figcraft_engine::Session;
+use serde_json::json;
+
+use crate::layers_drag_tests::Ui;
+
+/// A 1×1 PNG.
+const PNG: [u8; 67] = [
+    0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a, 0x00, 0x00, 0x00, 0x0d, 0x49, 0x48, 0x44, 0x52, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x01,
+    0x08, 0x06, 0x00, 0x00, 0x00, 0x1f, 0x15, 0xc4, 0x89, 0x00, 0x00, 0x00, 0x0d, 0x49, 0x44, 0x41, 0x54, 0x78, 0x9c, 0x63, 0x60, 0x00, 0x00, 0x02,
+    0x00, 0x00, 0x01, 0xe2, 0x21, 0xbc, 0x33, 0x00, 0x00, 0x00, 0x00, 0x49, 0x45, 0x4e, 0x44, 0xae, 0x42, 0x60, 0x82,
+];
+
+#[test]
+fn opening_a_file_with_a_damaged_image_toasts_what_was_lost() {
+    let dir = std::env::temp_dir().join(format!("figcraft-ui-open-issues-{}", std::process::id()));
+    std::fs::create_dir_all(&dir).unwrap();
+    let path = dir.join("pics.figcraft");
+    let mut s = Session::new();
+    let data = base64::engine::general_purpose::STANDARD.encode(PNG);
+    s.execute("file.import", &json!({"data": data, "name": "dot.png"})).unwrap();
+    s.execute("file.saveAs", &json!({"path": path.to_string_lossy()})).unwrap();
+    // Images are stored as they are: change one byte of the PNG, not its checksum.
+    let mut bytes = std::fs::read(&path).unwrap();
+    let at = bytes.windows(PNG.len()).position(|w| w == PNG).unwrap();
+    bytes[at + 30] ^= 0xff;
+    std::fs::write(&path, bytes).unwrap();
+
+    let mut u = Ui::new(Session::new());
+    u.app.run("file.open", json!({"path": path.to_string_lossy()}));
+    u.frame(vec![]);
+    let toast = u.app.ui.toast.as_ref().map(|t| t.0.clone()).unwrap_or_default();
+    assert!(toast.starts_with("pics: image ") && toast.contains("couldn't be read"), "{toast}");
+    let _ = std::fs::remove_dir_all(&dir);
+}