xlsx: find a reused picture through a content hash instead of comparing every media part (#3214)

* xlsx: find a reused picture through a content hash instead of comparing every media part

Out::image_part compared each new picture byte by byte with every
media part written so far, so a workbook of many distinct pictures
sharing a long prefix was exported in quadratic time. Media parts are
now indexed by extension and SHA-256.

* address review: index preserved media parts when they are added

* address review: do not hash an image twice when it is first added
This commit is contained in:
Dylan M. Taylor authored and GitHub committed 2026-11-11 06:32:44 -05:00
1 parent 76e53085e3
commit f79a425ca9
2 files changed
+34 -6

No files matched your search

+18
View File
@@ -85,3 +85,21 @@ fn different_pictures_with_the_same_length_stay_separate() {
let back = read_xlsx(&bytes).unwrap().0;
assert_eq!(back.sheets[0].images.iter().map(|i| i.data.to_vec()).collect::<Vec<_>>(), vec![a, b]);
}
/// Many distinct pictures that share a long prefix: each is matched against the earlier ones
/// through a content hash, not by comparing bytes with every one of them.
#[test]
fn many_distinct_pictures_with_a_common_prefix_are_each_written_once() {
let pictures: Vec<Vec<u8>> = (0..400u32)
.map(|i| {
let mut v = b"\x89PNG\r\n\x1a\n".to_vec();
v.extend(std::iter::repeat_n(7u8, 65_536));
v.extend(i.to_le_bytes());
v
})
.collect();
let refs: Vec<(u32, &[u8])> = pictures.iter().enumerate().map(|(i, p)| (i as u32 + 1, p.as_slice())).collect();
let wb = Workbook { sheets: vec![sheet_with("One", &refs), sheet_with("Two", &refs)], ..Default::default() };
let bytes = write_xlsx(&wb).unwrap();
assert_eq!(names(&bytes).iter().filter(|n| n.starts_with("xl/media/")).count(), 400);
}
+16 -6
View File
@@ -6,6 +6,7 @@ use std::fmt::Write as _;
use gridcraft_core::RangeRef;
use gridcraft_formula::quote_sheet;
use gridcraft_model::{CalcMode, CalcSettings, PhoneticString, ShowObjects, Style, Visibility, Workbook, WorkbookView};
use sha2::Digest as _;
use crate::package::ZipOut;
use crate::xml::{esc, esc_attr};
@@ -112,6 +113,9 @@ pub struct Out {
pub dxfs: Vec<Style>,
pub sst: Sst,
pub images: u32,
/// Media parts written so far, by extension and SHA-256 of their bytes: a picture used again
/// is found here instead of being compared byte by byte with every earlier one.
media: HashMap<(String, [u8; 32]), String>,
pub charts: u32,
/// The workbook's file name, which a PivotChart's link to its PivotTable names in brackets.
pub book_name: String,
@@ -145,20 +149,25 @@ impl Out {
if let Some(ct) = content_type {
self.overrides.push((format!("/{name}"), ct.to_string()));
}
// Every `xl/media/image*.ext` part is indexed as it is added, preserved ones included, so a
// later identical picture finds it instead of writing a second copy.
if let Some((ext, file)) = name.strip_prefix("xl/media/").filter(|f| f.starts_with("image")).and_then(|f| Some((f.rsplit_once('.')?.1, f))) {
self.media.entry((ext.to_string(), sha2::Sha256::digest(&data).into())).or_insert_with(|| file.to_string());
}
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();
let key = (ext.to_string(), sha2::Sha256::digest(data).into());
if let Some(file) = self.media.get(&key) {
return file.clone();
}
self.images += 1;
let file = format!("image{}.{ext}", self.images);
self.part(&format!("xl/media/{file}"), None, data.to_vec());
// Not through `part`: that would hash the same bytes again to index them.
self.parts.push((format!("xl/media/{file}"), data.to_vec()));
self.media.insert(key, file.clone());
file
}
pub fn default_type(&mut self, ext: &str, ct: &str) {
@@ -257,6 +266,7 @@ pub fn write_xlsx_named_with(wb: &Workbook, kind: XlsxKind, book_name: Option<&s
dxfs: vec![],
sst: Sst::default(),
images: 0,
media: HashMap::new(),
charts: 0,
book_name: book_name.filter(|n| !n.is_empty() && !n.contains(['[', ']'])).unwrap_or(DEFAULT_BOOK_NAME).to_string(),
chart_names: vec![],