diff --git a/crates/xlsx/src/sheet_write.rs b/crates/xlsx/src/sheet_write.rs index 78e1afaae..f961baac3 100644 --- a/crates/xlsx/src/sheet_write.rs +++ b/crates/xlsx/src/sheet_write.rs @@ -498,12 +498,10 @@ pub fn write_sheet(wb: &Workbook, si: usize, selected: bool, out: &mut Out) -> ( if img.data.is_empty() { continue; } - out.images += 1; let ext = image_ext(&img.mime, &img.data); - let name = format!("xl/media/image{}.{ext}", out.images); out.default_type(ext, image_content_type(ext)); - out.part(&name, None, img.data.clone()); - let rid = drels.add("image", &format!("../media/image{}.{ext}", out.images)); + let file = out.image_part(ext, &img.data); + let rid = drels.add("image", &format!("../media/{file}")); objs.push(Obj::Image(img, rid)); } for ch in &sheet.charts { diff --git a/crates/xlsx/src/tests/mod.rs b/crates/xlsx/src/tests/mod.rs index 517113d0f..387e7d603 100644 --- a/crates/xlsx/src/tests/mod.rs +++ b/crates/xlsx/src/tests/mod.rs @@ -13,6 +13,7 @@ mod passthrough_tests; mod pivot_tests; mod richdata_tests; mod roundtrip; +mod shared_media; mod sheet_names; mod smooth_roundtrip; mod trendline_roundtrip; diff --git a/crates/xlsx/src/tests/shared_media.rs b/crates/xlsx/src/tests/shared_media.rs new file mode 100644 index 000000000..1422a2663 --- /dev/null +++ b/crates/xlsx/src/tests/shared_media.rs @@ -0,0 +1,87 @@ +//! A picture used on several sheets is written as one media part that every drawing points to. + +use std::io::Read; +use std::sync::Arc; + +use gridcraft_core::CellRef; +use gridcraft_model::{Anchor, Image, Sheet, Workbook}; + +use crate::{read_xlsx, write_xlsx}; + +fn png(fill: u8) -> Vec { + let mut v = b"\x89PNG\r\n\x1a\n".to_vec(); + v.extend(std::iter::repeat_n(fill, 4096)); + v +} + +fn sheet_with(name: &str, pictures: &[(u32, &[u8])]) -> Arc { + let mut s = Sheet::new(name); + for (id, data) in pictures { + let anchor = Anchor { cell: CellRef::parse("B2").unwrap(), width: 40.0, height: 30.0, ..Default::default() }; + s.images.push(Image { id: *id, anchor, data: data.to_vec(), mime: "image/png".into(), alt: String::new() }); + } + Arc::new(s) +} + +fn names(bytes: &[u8]) -> Vec { + let z = zip::ZipArchive::new(std::io::Cursor::new(bytes.to_vec())).unwrap(); + z.file_names().map(str::to_string).collect() +} + +fn text(bytes: &[u8], name: &str) -> String { + let mut z = zip::ZipArchive::new(std::io::Cursor::new(bytes.to_vec())).unwrap(); + let mut s = String::new(); + z.by_name(name).unwrap().read_to_string(&mut s).unwrap(); + s +} + +#[test] +fn a_picture_on_several_sheets_is_one_media_part() { + let (a, b) = (png(1), png(2)); + // `a` on three sheets (and twice on one of them), `b` on one: two media parts, not five. + let wb = Workbook { + sheets: vec![sheet_with("One", &[(1, &a)]), sheet_with("Two", &[(1, &a), (2, &b)]), sheet_with("Three", &[(1, &a), (2, &a)])], + ..Default::default() + }; + let bytes = write_xlsx(&wb).unwrap(); + let media: Vec = names(&bytes).into_iter().filter(|n| n.starts_with("xl/media/")).collect(); + assert_eq!(media.len(), 2, "{media:?}"); + // Every drawing relationship points to an existing part, and the three sheets that show `a` + // point to the same one. + let mut targets = vec![]; + for d in 1..=3 { + let rels = text(&bytes, &format!("xl/drawings/_rels/drawing{d}.xml.rels")); + for t in rels.split("Target=\"").skip(1) { + let t = t.split('"').next().unwrap(); + let part = format!("xl/media/{}", t.trim_start_matches("../media/")); + assert!(media.contains(&part), "drawing{d}: dangling {t}"); + targets.push((d, part)); + } + } + assert_eq!(targets.len(), 5, "{targets:?}"); + let distinct: std::collections::BTreeSet<&String> = targets.iter().map(|(_, p)| p).collect(); + assert_eq!(distinct.len(), 2, "{targets:?}"); + let first_of = |d: usize| targets.iter().find(|(x, _)| *x == d).map(|(_, p)| p.clone()); + assert_eq!(first_of(1), first_of(2), "sheets One and Two show the same picture"); + assert_eq!(first_of(1), first_of(3)); + // They read back with the right bytes on the right sheets. + let back = read_xlsx(&bytes).unwrap().0; + let data = |i: usize| back.sheets[i].images.iter().map(|im| im.data.clone()).collect::>(); + assert_eq!(data(0), vec![a.clone()]); + assert_eq!(data(1), vec![a.clone(), b.clone()]); + assert_eq!(data(2), vec![a.clone(), a.clone()]); + // A second save of what was read back is stable. + let again = write_xlsx(&back).unwrap(); + assert_eq!(names(&again).iter().filter(|n| n.starts_with("xl/media/")).count(), 2); +} + +#[test] +fn different_pictures_with_the_same_length_stay_separate() { + let (a, b) = (png(1), png(2)); + assert_eq!(a.len(), b.len()); + let wb = Workbook { sheets: vec![sheet_with("One", &[(1, &a), (2, &b)])], ..Default::default() }; + let bytes = write_xlsx(&wb).unwrap(); + assert_eq!(names(&bytes).iter().filter(|n| n.starts_with("xl/media/")).count(), 2); + let back = read_xlsx(&bytes).unwrap().0; + assert_eq!(back.sheets[0].images.iter().map(|i| i.data.clone()).collect::>(), vec![a, b]); +} diff --git a/crates/xlsx/src/write.rs b/crates/xlsx/src/write.rs index 06761f01d..7c63e59b4 100644 --- a/crates/xlsx/src/write.rs +++ b/crates/xlsx/src/write.rs @@ -135,6 +135,20 @@ impl Out { } self.parts.push((name.to_string(), data)); } + /// The `xl/media` file name (`imageN.ext`) holding `data`. A picture used on several sheets, + /// or twice on one, is written once and every drawing points to that part, as in Excel. + pub fn image_part(&mut self, ext: &str, data: &[u8]) -> String { + let existing = self.parts.iter().find(|(name, bytes)| { + name.strip_prefix("xl/media/image").is_some_and(|rest| rest.ends_with(&format!(".{ext}"))) && bytes.as_slice() == data + }); + if let Some((name, _)) = existing { + return name.trim_start_matches("xl/media/").to_string(); + } + self.images += 1; + let file = format!("image{}.{ext}", self.images); + self.part(&format!("xl/media/{file}"), None, data.to_vec()); + file + } pub fn default_type(&mut self, ext: &str, ct: &str) { self.defaults.entry(ext.to_string()).or_insert_with(|| ct.to_string()); }