Compare commits

..
Author SHA1 Message Date
Dylan M. Taylor 6cdd6db150 engine: show overlong format codes as General without hashing them per cell
The display cache hashed the full format code on every lookup, so one
huge code shared by many cells cost its length for every cell drawn.
A code with more than 4 × 255 bytes can't be within Excel's 255
characters and is now formatted as General straight away.
2026-11-03 14:07:22 +00:00
3 changed files with 27 additions and 27 deletions
+22
View File
@@ -10,6 +10,11 @@ use gridcraft_numfmt::{Formatted, NumberFormat, format_value};
/// Parsed number formats are cached by code.
fn parsed(code: &str) -> NumberFormat {
static CACHE: std::sync::LazyLock<Mutex<HashMap<String, NumberFormat>>> = std::sync::LazyLock::new(|| Mutex::new(HashMap::new()));
// A code of more bytes than 4 × the character limit can't be within it: it shows as General
// without being hashed or scanned again for every cell that shares it.
if code.len() > 4 * gridcraft_numfmt::MAX_FORMAT_CODE_CHARS {
return NumberFormat::parse("General");
}
let lock = || CACHE.lock().unwrap_or_else(std::sync::PoisonError::into_inner);
if let Some(f) = lock().get(code) {
return f.clone();
@@ -226,3 +231,20 @@ pub fn stats(sheet: &Sheet, ranges: &[RangeRef]) -> Stats {
}
s
}
#[cfg(test)]
mod tests {
use super::*;
/// A format code far past Excel's 255 characters shows its values as General, for every cell
/// that shares it, without being hashed into the cache each time.
#[test]
fn an_overlong_code_formats_as_general() {
let wb = Workbook::new();
let code = format!("0.00{}", "\"x\"".repeat(400_000));
for _ in 0..1_000 {
assert_eq!(format(&Value::Number(1.5), &code, &wb).text, "1.5");
}
assert_eq!(format(&Value::Number(1.5), "0.00", &wb).text, "1.50");
}
}
-18
View File
@@ -85,21 +85,3 @@ 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);
}
+5 -9
View File
@@ -6,7 +6,6 @@ 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};
@@ -113,9 +112,6 @@ 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,
@@ -146,14 +142,15 @@ impl Out {
/// 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 key = (ext.to_string(), sha2::Sha256::digest(data).into());
if let Some(file) = self.media.get(&key) {
return file.clone();
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());
self.media.insert(key, file.clone());
file
}
pub fn default_type(&mut self, ext: &str, ct: &str) {
@@ -252,7 +249,6 @@ pub fn write_xlsx_named(wb: &Workbook, kind: XlsxKind, book_name: Option<&str>)
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![],