diff --git a/assets/bugfixes/issue-620/after.jpg b/assets/bugfixes/issue-620/after.jpg new file mode 100644 index 00000000..c6ff2d0b Binary files /dev/null and b/assets/bugfixes/issue-620/after.jpg differ diff --git a/assets/bugfixes/issue-620/before.jpg b/assets/bugfixes/issue-620/before.jpg new file mode 100644 index 00000000..8df2d705 Binary files /dev/null and b/assets/bugfixes/issue-620/before.jpg differ diff --git a/assets/bugfixes/issue-620/gt.jpg b/assets/bugfixes/issue-620/gt.jpg new file mode 100644 index 00000000..feec149e Binary files /dev/null and b/assets/bugfixes/issue-620/gt.jpg differ diff --git a/assets/bugfixes/issue-713/compare.jpg b/assets/bugfixes/issue-713/compare.jpg new file mode 100644 index 00000000..28aaa1c4 Binary files /dev/null and b/assets/bugfixes/issue-713/compare.jpg differ diff --git a/assets/bugfixes/issue-714/compare.jpg b/assets/bugfixes/issue-714/compare.jpg new file mode 100644 index 00000000..67c04d29 Binary files /dev/null and b/assets/bugfixes/issue-714/compare.jpg differ diff --git a/assets/bugfixes/issue-715/compare.jpg b/assets/bugfixes/issue-715/compare.jpg new file mode 100644 index 00000000..28aaa1c4 Binary files /dev/null and b/assets/bugfixes/issue-715/compare.jpg differ diff --git a/crates/office2pdf/src/parser/xlsx.rs b/crates/office2pdf/src/parser/xlsx.rs index 9d7e418c..8cef546c 100644 --- a/crates/office2pdf/src/parser/xlsx.rs +++ b/crates/office2pdf/src/parser/xlsx.rs @@ -254,17 +254,27 @@ fn anchored_image( /// Context stand-in for sheets with no used cells, so drawing anchors can /// still resolve against default column widths and row heights. -fn empty_sheet_context() -> SheetContext { +/// +/// The column metric must come from the workbook Normal font exactly as it +/// does for populated sheets: hardcoding 7px laid every drawing-only sheet +/// out on 44.2575pt columns while the workbook's own Calibri-11 metric says +/// 50.58pt, shrinking anchors 12.5% and distorting picture aspect ratios +/// (issue #620). Without a readable Normal font the shared fallback inspects +/// cell fonts, finds none on an empty sheet, and keeps the legacy 7px. +fn empty_sheet_context( + sheet: &umya_spreadsheet::Worksheet, + normal_font: Option<&NormalFont>, +) -> SheetContext { SheetContext { col_start: 1, col_end: 0, num_cols: 0, column_widths: Vec::new(), - max_digit_width_px: 7.0, + max_digit_width_px: resolve_max_digit_width_px(sheet, normal_font), merge_tops: std::collections::HashMap::new(), merge_skips: std::collections::HashSet::new(), cond_fmt_overrides: std::collections::HashMap::new(), - normal_font: None, + normal_font: normal_font.cloned(), row_stripes: Vec::new(), } } @@ -355,7 +365,7 @@ impl XlsxParser { let raw_text_boxes = text_box_map.remove(&sheet_name); let raw_charts = chart_map.remove(&sheet_name); if raw_images.is_some() || raw_text_boxes.is_some() || raw_charts.is_some() { - let stub_ctx = empty_sheet_context(); + let stub_ctx = empty_sheet_context(sheet, normal_font.as_ref()); let images: Vec = raw_images .unwrap_or_default() .into_iter() @@ -568,7 +578,7 @@ impl Parser for XlsxParser { let raw_text_boxes = text_box_map.remove(&sheet_name); let raw_charts = chart_map.remove(&sheet_name); if raw_images.is_some() || raw_text_boxes.is_some() || raw_charts.is_some() { - let stub_ctx = empty_sheet_context(); + let stub_ctx = empty_sheet_context(sheet, normal_font.as_ref()); let images: Vec = raw_images .unwrap_or_default() .into_iter() diff --git a/crates/office2pdf/src/parser/xlsx_cells.rs b/crates/office2pdf/src/parser/xlsx_cells.rs index 68aea6fa..10ec5a2b 100644 --- a/crates/office2pdf/src/parser/xlsx_cells.rs +++ b/crates/office2pdf/src/parser/xlsx_cells.rs @@ -751,6 +751,22 @@ pub(super) fn build_rows_for_range( rows } +/// The pixel metric every column width is scaled by. Excel derives it from +/// the workbook Normal font; cell fonts do not participate (issue #366). +/// When `xl/styles.xml` was unreadable, fall back to the dominant cell font +/// — which on a sheet with no cells lands on the legacy 7px default. Shared +/// by populated and drawing-only sheets so both scale from the same digit +/// metric (issue #620); drawing-only sheets still price every column at the +/// default width because their context carries no `` overrides. +pub(super) fn resolve_max_digit_width_px( + sheet: &umya_spreadsheet::Worksheet, + normal_font: Option<&NormalFont>, +) -> f64 { + normal_font + .map(|font| max_digit_width_px_for_normal_font(&font.family, font.size_pt)) + .unwrap_or_else(|| sheet_max_digit_width_px(sheet)) +} + /// Prepare the shared context for processing a sheet (dimensions, merges, styles, etc.). /// Returns (SheetContext, row_start, row_end) or None if the sheet is empty. pub(super) fn prepare_sheet_context( @@ -782,11 +798,7 @@ pub(super) fn prepare_sheet_context( (1, max_col, 1, max_row) }; - // Excel derives every column print metric from the workbook Normal - // font; cell fonts do not participate (issue #366). - let max_digit_width_px = normal_font - .map(|font| max_digit_width_px_for_normal_font(&font.family, font.size_pt)) - .unwrap_or_else(|| sheet_max_digit_width_px(sheet)); + let max_digit_width_px = resolve_max_digit_width_px(sheet, normal_font); let column_widths: Vec = (col_start..=col_end) .map(|col| { sheet diff --git a/crates/office2pdf/src/parser/xlsx_tests.rs b/crates/office2pdf/src/parser/xlsx_tests.rs index 39f5ffd7..c8adbe15 100644 --- a/crates/office2pdf/src/parser/xlsx_tests.rs +++ b/crates/office2pdf/src/parser/xlsx_tests.rs @@ -643,3 +643,177 @@ fn rewrite_first_styles_font(data: &[u8], family: &str, size_pt: f64) -> Vec } out.finish().expect("finished zip").into_inner() } + +// ----- Drawing-only sheets (issue #620) ----- + +/// A workbook whose only sheet has no cells but carries one picture anchored +/// C1:F9 (cols 2..5, rows 0..8, zero offsets). umya cannot author drawings, +/// so the drawing parts are spliced into the zip it writes. +fn build_drawing_only_sheet_xlsx() -> Vec { + let book = umya_spreadsheet::new_file(); + let mut cursor = Cursor::new(Vec::new()); + umya_spreadsheet::writer::xlsx::write_writer(&book, &mut cursor).unwrap(); + splice_picture_drawing(&cursor.into_inner()) +} + +/// Splice `xl/drawings/drawing1.xml` (one twoCellAnchor picture), its rels, +/// and a 1x1 PNG into a workbook zip, wiring the first worksheet to it. +fn splice_picture_drawing(data: &[u8]) -> Vec { + const DRAWING_XML: &str = r#" +20005080"#; + const DRAWING_RELS: &str = r#" +"#; + const SHEET_RELS: &str = r#" +"#; + /// Smallest valid PNG: 1x1 RGBA. + const PNG_1X1: &[u8] = &[ + 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, 0x62, 0x00, + 0x01, 0x00, 0x00, 0x05, 0x00, 0x01, 0x0D, 0x0A, 0x2D, 0xB4, 0x00, 0x00, 0x00, 0x00, 0x49, + 0x45, 0x4E, 0x44, 0xAE, 0x42, 0x60, 0x82, + ]; + + let mut archive = zip::ZipArchive::new(Cursor::new(data)).expect("readable zip"); + let mut out = zip::ZipWriter::new(Cursor::new(Vec::new())); + let mut has_sheet_rels = false; + for index in 0..archive.len() { + let mut entry = archive.by_index(index).expect("readable entry"); + let name = entry.name().to_string(); + let mut bytes = Vec::new(); + std::io::Read::read_to_end(&mut entry, &mut bytes).expect("readable entry body"); + match name.as_str() { + "xl/worksheets/sheet1.xml" => { + let xml = String::from_utf8(bytes).expect("sheet1.xml is utf-8"); + bytes = xml + .replace( + "", + r#""#, + ) + .into_bytes(); + } + "xl/worksheets/_rels/sheet1.xml.rels" => { + has_sheet_rels = true; + let xml = String::from_utf8(bytes).expect("sheet rels is utf-8"); + bytes = xml + .replace( + "", + r#""#, + ) + .into_bytes(); + } + "[Content_Types].xml" => { + let xml = String::from_utf8(bytes).expect("content types is utf-8"); + bytes = xml + .replace( + "", + r#""#, + ) + .into_bytes(); + } + _ => {} + } + out.start_file(name, zip::write::FileOptions::default()) + .expect("writable entry"); + std::io::Write::write_all(&mut out, &bytes).expect("writable entry body"); + } + if !has_sheet_rels { + out.start_file( + "xl/worksheets/_rels/sheet1.xml.rels", + zip::write::FileOptions::default(), + ) + .expect("writable sheet rels"); + std::io::Write::write_all(&mut out, SHEET_RELS.as_bytes()).expect("writable sheet rels"); + } + for (path, body) in [ + ("xl/drawings/drawing1.xml", DRAWING_XML.as_bytes()), + ( + "xl/drawings/_rels/drawing1.xml.rels", + DRAWING_RELS.as_bytes(), + ), + ("xl/media/image1.png", PNG_1X1), + ] { + out.start_file(path, zip::write::FileOptions::default()) + .expect("writable drawing part"); + std::io::Write::write_all(&mut out, body).expect("writable drawing part body"); + } + out.finish().expect("finished zip").into_inner() +} + +/// A sheet with no cells must resolve its drawing anchors against the +/// workbook Normal font, producing the same column metric as a populated +/// sheet. umya writes Calibri 11 as the Normal font, which the converter's +/// own model maps to an 8px max digit width -> 50.58pt default columns; the +/// legacy hardcoded 7px metric produced 44.2575pt (issue #620). +#[test] +fn test_drawing_only_sheet_resolves_anchors_with_normal_font_metric() { + let data = build_drawing_only_sheet_xlsx(); + let (doc, _warnings) = XlsxParser.parse(&data, &ConvertOptions::default()).unwrap(); + + let page = get_sheet_page(&doc, 0); + assert_eq!( + page.images.len(), + 1, + "the spliced picture must survive parse" + ); + let image: &crate::ir::SheetImage = &page.images[0]; + + let column_pt: f64 = column_width_to_pt(DEFAULT_COLUMN_WIDTH, 8.0); + assert!( + (column_pt - 50.58).abs() < 0.01, + "Calibri-11 default column must be 50.58pt, got {column_pt}" + ); + // Anchor spans cols 2..5 with zero offsets: x = 2 columns, width = 3. + assert!( + (image.x_offset_pt - 2.0 * column_pt).abs() < 0.01, + "x_offset_pt {} != 2 x {column_pt}", + image.x_offset_pt + ); + let width: f64 = image.image.width.expect("twoCellAnchor resolves a width"); + assert!( + (width - 3.0 * column_pt).abs() < 0.01, + "width {width} != 3 x {column_pt}" + ); + // Negative: the legacy hardcoded-7px metric must be gone. + let legacy_column_pt: f64 = column_width_to_pt(DEFAULT_COLUMN_WIDTH, 7.0); + assert!( + (width - 3.0 * legacy_column_pt).abs() > 1.0, + "width {width} still matches the legacy 44.2575pt column metric" + ); +} + +/// Triangulation for issue #620: the empty-sheet context must derive its +/// metric from whatever Normal font it is given — not a hardcoded 8px — and +/// fall back to the legacy 7px only when no Normal font is readable. The +/// carried `normal_font` keeps the stub structurally consistent with a +/// populated-sheet context; nothing on the drawing-only path reads it today +/// (text boxes take their fonts from DrawingML run properties and the theme). +#[test] +fn test_empty_sheet_context_derives_metric_from_normal_font() { + let book = umya_spreadsheet::new_file(); + let sheet: &umya_spreadsheet::Worksheet = book.get_sheet(&0).unwrap(); + + let calibri_11 = NormalFont { + family: "Calibri".to_string(), + size_pt: 11.0, + }; + let calibri_ctx = empty_sheet_context(sheet, Some(&calibri_11)); + assert_eq!(calibri_ctx.max_digit_width_px, 8.0); + assert_eq!(calibri_ctx.normal_font, Some(calibri_11)); + + // A smaller Normal font must shrink the metric with it. + let calibri_8 = NormalFont { + family: "Calibri".to_string(), + size_pt: 8.0, + }; + assert_eq!( + empty_sheet_context(sheet, Some(&calibri_8)).max_digit_width_px, + 6.0 + ); + + // No readable Normal font: the shared cell-font fallback finds no cells + // on an empty sheet and keeps the legacy 7px default. + let fallback_ctx = empty_sheet_context(sheet, None); + assert_eq!(fallback_ctx.max_digit_width_px, 7.0); + assert_eq!(fallback_ctx.normal_font, None); +}