diff --git a/crates/engine/src/cmd/file.rs b/crates/engine/src/cmd/file.rs index 1ed3793..a528e98 100644 --- a/crates/engine/src/cmd/file.rs +++ b/crates/engine/src/cmd/file.rs @@ -662,6 +662,13 @@ fn open(s: &mut Session, p: &Value) -> Result { 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}); diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index c2dad90..5da8d7d 100644 --- a/crates/engine/src/lib.rs +++ b/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; diff --git a/crates/engine/src/open_issues_tests.rs b/crates/engine/src/open_issues_tests.rs new file mode 100644 index 0000000..d7f2372 --- /dev/null +++ b/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()); +} diff --git a/crates/format/src/image_tests.rs b/crates/format/src/image_tests.rs new file mode 100644 index 0000000..bc925ef --- /dev/null +++ b/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 { + 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) -> Option>) -> Vec { + let mut z = zip::ZipArchive::new(std::io::Cursor::new(bytes)).unwrap(); + let names: Vec = 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")); +} diff --git a/crates/format/src/lib.rs b/crates/format/src/lib.rs index acd2253..77ef253 100644 --- a/crates/format/src/lib.rs +++ b/crates/format/src/lib.rs @@ -465,11 +465,17 @@ pub fn load_bytes_with_versions(bytes: &[u8]) -> Result { let (mut doc, mut issues, own_images) = parse_json(&text)?; let names: Vec = 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 { 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, + unread: &[(String, FormatError)], + listed: Option<&BTreeSet>, + images: &BTreeMap, +) { + let mut lost: Vec = 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), Form #[cfg(test)] mod branch_tests; #[cfg(test)] +mod image_tests; +#[cfg(test)] mod large_tests; #[cfg(test)] mod version_tests; diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index 3b27505..a9a989d 100644 --- a/crates/ui-egui/src/lib.rs +++ b/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; diff --git a/crates/ui-egui/src/open_issues_tests.rs b/crates/ui-egui/src/open_issues_tests.rs new file mode 100644 index 0000000..c1e69f3 --- /dev/null +++ b/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); +}