diff --git a/assets/bugfixes/issue-459/after.jpg b/assets/bugfixes/issue-459/after.jpg new file mode 100644 index 00000000..55ace5f6 Binary files /dev/null and b/assets/bugfixes/issue-459/after.jpg differ diff --git a/assets/bugfixes/issue-459/before.jpg b/assets/bugfixes/issue-459/before.jpg new file mode 100644 index 00000000..44f7f775 Binary files /dev/null and b/assets/bugfixes/issue-459/before.jpg differ diff --git a/assets/bugfixes/issue-459/gt.jpg b/assets/bugfixes/issue-459/gt.jpg new file mode 100644 index 00000000..9dc9e0b9 Binary files /dev/null and b/assets/bugfixes/issue-459/gt.jpg differ diff --git a/crates/office2pdf/src/render/typst_gen.rs b/crates/office2pdf/src/render/typst_gen.rs index cf0e07ae..43cb23ac 100644 --- a/crates/office2pdf/src/render/typst_gen.rs +++ b/crates/office2pdf/src/render/typst_gen.rs @@ -569,10 +569,19 @@ fn generate_table_with_anchors( out.push('\n'); row_start = row_end + 1; } - // Emit the anchored element - generate_sheet_anchor(out, &sorted_charts[chart_idx].1, ctx); - out.push('\n'); - chart_idx += 1; + // Emit every drawing anchored to this row together, so same-row + // shapes overlay rather than stack (issue #459). + let row_end_idx: usize = sorted_charts[chart_idx..] + .iter() + .take_while(|(row, _)| *row == sorted_charts[chart_idx].0) + .count() + + chart_idx; + let row_anchors: Vec<&SheetAnchor> = sorted_charts[chart_idx..row_end_idx] + .iter() + .map(|(_, anchor)| anchor) + .collect(); + generate_sheet_anchor_row(out, &row_anchors, ctx); + chart_idx = row_end_idx; } } @@ -596,24 +605,81 @@ fn generate_table_with_anchors( out.push('\n'); } - // Emit any remaining anchors (anchored beyond last row, e.g., u32::MAX) + // Emit any remaining anchors (anchored beyond last row, e.g., u32::MAX), + // still grouped by the row they share. while chart_idx < sorted_charts.len() { - generate_sheet_anchor(out, &sorted_charts[chart_idx].1, ctx); - out.push('\n'); - chart_idx += 1; + let row_end_idx: usize = sorted_charts[chart_idx..] + .iter() + .take_while(|(row, _)| *row == sorted_charts[chart_idx].0) + .count() + + chart_idx; + let row_anchors: Vec<&SheetAnchor> = sorted_charts[chart_idx..row_end_idx] + .iter() + .map(|(_, anchor)| anchor) + .collect(); + generate_sheet_anchor_row(out, &row_anchors, ctx); + chart_idx = row_end_idx; } Ok(()) } -fn generate_sheet_anchor(out: &mut String, anchor: &SheetAnchor, ctx: &mut GenCtx) { +/// Emit every drawing anchored to one worksheet row. +/// +/// Excel lays same-row shapes out side by side over the grid. Reserving a +/// flow box per shape advanced the flow between them, so three shapes on row +/// 3 formed a diagonal stack and reserved three times the height, spilling +/// onto a blank second page (issue #459). One reserved box, as tall as the +/// tallest shape, holds them all; each keeps its own horizontal offset. +/// +/// Charts are ordinary flow content and keep their own placement. +fn generate_sheet_anchor_row(out: &mut String, anchors: &[&SheetAnchor], ctx: &mut GenCtx) { + for anchor in anchors.iter() { + if let SheetAnchor::Chart(chart) = anchor { + generate_chart(out, chart); + out.push('\n'); + } + } + + let placed: Vec<&&SheetAnchor> = anchors + .iter() + .filter(|anchor| !matches!(anchor, SheetAnchor::Chart(_))) + .collect(); + if placed.is_empty() { + return; + } + let reserved_height_pt: f64 = placed + .iter() + .map(|anchor| sheet_anchor_height_pt(anchor)) + .fold(0.0, f64::max); + let _ = write!( + out, + "#box(width: 100%, height: {}pt)[", + format_f64(reserved_height_pt) + ); + for anchor in placed { + write_placed_sheet_anchor(out, anchor, ctx); + } + out.push_str("]\n"); +} + +/// The flow height a placed drawing occupies. +fn sheet_anchor_height_pt(anchor: &SheetAnchor) -> f64 { match anchor { - SheetAnchor::Chart(chart) => generate_chart(out, chart), + SheetAnchor::TextBox(text_box) => text_box.height, + SheetAnchor::Image(sheet_image) => sheet_image.image.height.unwrap_or(100.0), + SheetAnchor::Chart(_) => 0.0, + } +} + +/// Place one drawing at its horizontal offset inside the row's reserved box. +fn write_placed_sheet_anchor(out: &mut String, anchor: &SheetAnchor, ctx: &mut GenCtx) { + match anchor { + SheetAnchor::Chart(_) => {} SheetAnchor::TextBox(text_box) => { let _ = write!( out, - "#box(width: 100%, height: {}pt)[#place(top + left, dx: {}pt)[#box(width: {}pt, height: {}pt", - format_f64(text_box.height), + "#place(top + left, dx: {}pt)[#box(width: {}pt, height: {}pt", format_f64(text_box.x_offset_pt), format_f64(text_box.width), format_f64(text_box.height), @@ -639,20 +705,16 @@ fn generate_sheet_anchor(out: &mut String, anchor: &SheetAnchor, ctx: &mut GenCt if text_box.vertical_center { out.push(']'); } - out.push_str("]]]\n"); + out.push_str("]]"); } SheetAnchor::Image(sheet_image) => { - // Keep the anchor's horizontal position: reserve the image height - // in the flow and place the image at its column offset. - let height: f64 = sheet_image.image.height.unwrap_or(100.0); let _ = write!( out, - "#box(width: 100%, height: {}pt)[#place(top + left, dx: {}pt)[", - format_f64(height), + "#place(top + left, dx: {}pt)[", format_f64(sheet_image.x_offset_pt), ); generate_image(out, &sheet_image.image, ctx); - out.push_str("]]\n"); + out.push(']'); } } } diff --git a/crates/office2pdf/src/render/typst_gen_advanced_tests.rs b/crates/office2pdf/src/render/typst_gen_advanced_tests.rs index f28d31f9..34fe7273 100644 --- a/crates/office2pdf/src/render/typst_gen_advanced_tests.rs +++ b/crates/office2pdf/src/render/typst_gen_advanced_tests.rs @@ -1339,3 +1339,107 @@ fn test_non_arrow_icon_set_still_renders_as_text() { assert!(output.source.contains("text(fill: rgb(214, 85, 50)")); assert!(!output.source.contains("polygon(")); } + +/// A worksheet text box anchored after `anchor_row` at `x_offset_pt`. +fn make_sheet_text_box(anchor_row: u32, x_offset_pt: f64, height: f64) -> crate::ir::SheetTextBox { + crate::ir::SheetTextBox { + anchor_row, + x_offset_pt, + width: 100.0, + height, + paragraphs: vec![Paragraph { + style: ParagraphStyle::default(), + runs: vec![Run { + text: format!("shape at {x_offset_pt}"), + style: TextStyle::default(), + href: None, + footnote: None, + }], + }], + fill: None, + border: None, + vertical_center: false, + } +} + +fn sheet_page_with_text_boxes(text_boxes: Vec) -> Page { + Page::Sheet(SheetPage { + name: "Sheet1".to_string(), + size: PageSize::default(), + margins: Margins::default(), + table: Table { + rows: vec![TableRow { + cells: vec![TableCell::default()], + height: None, + }], + column_widths: vec![100.0], + ..Table::default() + }, + header: None, + footer: None, + charts: vec![], + images: Vec::new(), + text_boxes, + }) +} + +#[test] +fn test_same_row_sheet_drawings_share_one_reserved_box() { + // Excel draws shapes anchored to the same row side by side. Emitting one + // reserved flow box each stacked them diagonally and reserved three + // times the height, which spilled onto a blank second page (issue #459). + let doc = make_doc(vec![sheet_page_with_text_boxes(vec![ + make_sheet_text_box(1, 0.0, 60.0), + make_sheet_text_box(1, 200.0, 60.0), + make_sheet_text_box(1, 400.0, 60.0), + ])]); + let source = generate_typst(&doc).unwrap().source; + + assert_eq!( + source.matches("#box(width: 100%, height: 60pt)").count(), + 1, + "same-row drawings share one reserved box: {source}" + ); + for dx in ["dx: 0pt", "dx: 200pt", "dx: 400pt"] { + assert!( + source.contains(dx), + "each drawing keeps its own horizontal offset ({dx}): {source}" + ); + } +} + +#[test] +fn test_same_row_sheet_drawings_reserve_the_tallest_height() { + // The row's reserved height is the tallest shape on it, not the sum. + let doc = make_doc(vec![sheet_page_with_text_boxes(vec![ + make_sheet_text_box(1, 0.0, 40.0), + make_sheet_text_box(1, 200.0, 90.0), + ])]); + let source = generate_typst(&doc).unwrap().source; + + assert_eq!( + source.matches("#box(width: 100%, height: ").count(), + 1, + "the row reserves a single box: {source}" + ); + assert!( + source.contains("#box(width: 100%, height: 90pt)"), + "the reserved height is the tallest shape, not the sum: {source}" + ); +} + +#[test] +fn test_drawings_on_different_rows_keep_separate_boxes() { + // Triangulation: shapes on different rows still advance the flow. + let doc = make_doc(vec![sheet_page_with_text_boxes(vec![ + make_sheet_text_box(1, 0.0, 60.0), + make_sheet_text_box(5, 0.0, 60.0), + ])]); + let source = generate_typst(&doc).unwrap().source; + + assert_eq!( + source.matches("#box(width: 100%, height: 60pt)").count(), + 2, + "different rows reserve their own boxes: {source}" + ); +}