diff --git a/assets/bugfixes/issue-452/after.jpg b/assets/bugfixes/issue-452/after.jpg new file mode 100644 index 00000000..ae96b4f6 Binary files /dev/null and b/assets/bugfixes/issue-452/after.jpg differ diff --git a/assets/bugfixes/issue-452/before.jpg b/assets/bugfixes/issue-452/before.jpg new file mode 100644 index 00000000..b07ae38f Binary files /dev/null and b/assets/bugfixes/issue-452/before.jpg differ diff --git a/assets/bugfixes/issue-452/gt.jpg b/assets/bugfixes/issue-452/gt.jpg new file mode 100644 index 00000000..4c897d86 Binary files /dev/null and b/assets/bugfixes/issue-452/gt.jpg differ diff --git a/crates/office2pdf/src/parser/docx.rs b/crates/office2pdf/src/parser/docx.rs index 001101ac..3613bfd1 100644 --- a/crates/office2pdf/src/parser/docx.rs +++ b/crates/office2pdf/src/parser/docx.rs @@ -70,11 +70,16 @@ mod text; /// Parser for DOCX (Office Open XML Word) documents. pub struct DocxParser; -/// Word supplies this built-in paragraph spacing when neither a paragraph -/// nor its style hierarchy specifies an override. Line height is left to the -/// renderer, which derives Word's single-spacing pitch from the actual font -/// metrics (issue #354). -pub(super) const WORD_COMPATIBLE_PARAGRAPH_SPACE_AFTER_PT: f64 = 8.0; +/// The paragraph spacing Word applies when neither a paragraph nor its +/// style hierarchy specifies `w:spacing w:after`: ECMA-376 leaves the gap +/// at zero, and a Word PDF export of a document whose `styles.xml` defines +/// no `Normal` spacing confirms it. Recording it explicitly (rather than +/// leaving `space_after` unset) also pins the paragraph block's `below`, so +/// Typst's own 1.2em default block spacing cannot leak into the gap. +/// +/// Line height is left to the renderer, which derives Word's single-spacing +/// pitch from the actual font metrics (issues #354, #452). +pub(super) const WORD_COMPATIBLE_PARAGRAPH_SPACE_AFTER_PT: f64 = 0.0; fn apply_word_compatible_paragraph_defaults(style: &mut ParagraphStyle) { style diff --git a/crates/office2pdf/src/parser/docx_foundation_tests.rs b/crates/office2pdf/src/parser/docx_foundation_tests.rs index 9b69899a..91d75eb8 100644 --- a/crates/office2pdf/src/parser/docx_foundation_tests.rs +++ b/crates/office2pdf/src/parser/docx_foundation_tests.rs @@ -595,9 +595,11 @@ fn test_paragraph_uses_word_default_spacing_when_unspecified() { let paragraph = first_paragraph(&doc); // Line height stays unset in the IR: the renderer derives Word's - // single-spacing pitch from the actual font metrics (issue #354). + // single-spacing pitch from the actual font metrics (issue #354). The + // gap below is zero, because neither the paragraph nor the style + // hierarchy sets `w:spacing w:after` (issue #452). assert_eq!(paragraph.style.line_box, None); - assert_eq!(paragraph.style.space_after, Some(8.0)); + assert_eq!(paragraph.style.space_after, Some(0.0)); } #[test] diff --git a/crates/office2pdf/src/parser/docx_table_tests.rs b/crates/office2pdf/src/parser/docx_table_tests.rs index 7977370b..69652d2e 100644 --- a/crates/office2pdf/src/parser/docx_table_tests.rs +++ b/crates/office2pdf/src/parser/docx_table_tests.rs @@ -400,9 +400,10 @@ fn test_table_cell_paragraph_uses_word_default_line_box_and_spacing() { other => panic!("expected table cell paragraph, got {other:?}"), }; - // Line height stays unset in the IR (issue #354). + // Line height stays unset in the IR (issue #354); an unspecified + // `w:spacing w:after` is zero (issue #452). assert_eq!(paragraph(0).style.line_box, None); - assert_eq!(paragraph(0).style.space_after, Some(8.0)); + assert_eq!(paragraph(0).style.space_after, Some(0.0)); assert_eq!(paragraph(1).style.line_box, None); assert_eq!(paragraph(1).style.space_after, Some(6.0)); diff --git a/crates/office2pdf/src/parser/docx_tests.rs b/crates/office2pdf/src/parser/docx_tests.rs index 93a42f31..60bde30a 100644 --- a/crates/office2pdf/src/parser/docx_tests.rs +++ b/crates/office2pdf/src/parser/docx_tests.rs @@ -304,7 +304,7 @@ fn test_list_paragraphs_use_word_compatible_spacing_when_unspecified() { }) .expect("numbered paragraphs should become a list"); - assert_eq!(list.items[0].content[0].style.space_after, Some(8.0)); + assert_eq!(list.items[0].content[0].style.space_after, Some(0.0)); assert_eq!(list.items[1].content[0].style.space_after, Some(6.0)); } diff --git a/crates/office2pdf/src/render/typst_gen.rs b/crates/office2pdf/src/render/typst_gen.rs index 31ddb086..cf0e07ae 100644 --- a/crates/office2pdf/src/render/typst_gen.rs +++ b/crates/office2pdf/src/render/typst_gen.rs @@ -1564,20 +1564,19 @@ fn generate_block(out: &mut String, block: &Block, ctx: &mut GenCtx) -> Result<( // Grid-snapped line height applies to list items too (Word's // document grid covers all body text). let first_paragraph = list.items.first().and_then(|item| item.content.first()); - let metric_leading_pt: Option = first_paragraph.and_then(|paragraph| { - word_line_leading_pt(¶graph.runs, ¶graph.style, ctx.line_grid_pitch) - }); let settings: Option = first_paragraph.and_then(|paragraph| { word_line_height_settings(¶graph.runs, ¶graph.style, ctx.line_grid_pitch) }); if let Some(settings) = settings { out.push_str("#block(width: 100%)[\n"); out.push_str(&settings); - generate_list(out, list, metric_leading_pt)?; + // The wrapper's line box spans Word's full line advance, so + // the item gaps are the raw `w:spacing` values. + generate_list(out, list, true)?; out.push_str("]\n"); Ok(()) } else { - generate_list(out, list, None) + generate_list(out, list, false) } } Block::MathEquation(math) => { diff --git a/crates/office2pdf/src/render/typst_gen_list_tests.rs b/crates/office2pdf/src/render/typst_gen_list_tests.rs index f9c9d058..191b780d 100644 --- a/crates/office2pdf/src/render/typst_gen_list_tests.rs +++ b/crates/office2pdf/src/render/typst_gen_list_tests.rs @@ -839,12 +839,13 @@ fn test_generate_list_uses_first_item_level_marker_when_list_starts_nested() { } #[test] -fn test_generate_list_metric_spacing_adds_gap_to_single_space_leading() { +fn test_generate_list_metric_spacing_is_the_raw_paragraph_gap() { // Word adds `w:spacing w:after` directly to the single-space line // advance between list items: next item top = previous line advance + - // after. Under metric text edges that whitespace is the single-space - // leading plus the paragraph gap; adding a whole line height instead - // stretched every list block by ~8pt per item (issue #384). + // after. The wrapper's line box already spans that advance, so the + // Typst list `spacing` that replaces the automatic leading is the raw + // paragraph gap — adding a whole line height instead stretched every + // list block by ~8pt per item (issues #384, #452). use crate::ir::List; let Some((ascender, descender, word_pitch_em)) = @@ -853,8 +854,7 @@ fn test_generate_list_metric_spacing_adds_gap_to_single_space_leading() { return; // no font book available (e.g. exotic CI sandbox) }; let font_size: f64 = 10.0; - let single_space_leading_pt: f64 = - ((word_pitch_em - (ascender + descender)) * font_size).max(0.0); + let metric_em: f64 = ascender + descender; let make_item = |text: &str| ListItem { content: vec![Paragraph { @@ -886,21 +886,14 @@ fn test_generate_list_metric_spacing_adds_gap_to_single_space_leading() { .unwrap() .source; + let advance_pt: f64 = (word_pitch_em * font_size).max(metric_em * font_size); + assert_line_advance(&source, "Libertinus Serif", font_size, advance_pt); assert!( - source.contains(&format!( - "top-edge: {}em, bottom-edge: -{}em", - format_f64(ascender), - format_f64(descender) - )), - "fixed nominal-metric em edges expected in: {source}" + source.contains("spacing: 4pt"), + "expected inter-item spacing to be the raw 4pt gap in: {source}" ); - let expected = format_f64(single_space_leading_pt + 4.0); assert!( - source.contains(&format!("spacing: {expected}pt")), - "expected inter-item spacing {expected}pt (leading + gap) in: {source}" - ); - assert!( - source.contains(&format!("below: {expected}pt")), - "expected list below spacing {expected}pt (leading + gap) in: {source}" + source.contains("below: 4pt"), + "expected list below spacing to be the raw 4pt gap in: {source}" ); } diff --git a/crates/office2pdf/src/render/typst_gen_lists.rs b/crates/office2pdf/src/render/typst_gen_lists.rs index 40a9b2e5..e4205fc6 100644 --- a/crates/office2pdf/src/render/typst_gen_lists.rs +++ b/crates/office2pdf/src/render/typst_gen_lists.rs @@ -242,7 +242,7 @@ fn paragraph_line_height(paragraph: &Paragraph) -> f64 { fn list_boundary_spacing( previous: &crate::ir::ListItem, next: &crate::ir::ListItem, - metric_leading_pt: Option, + wrapper_spans_full_line: bool, ) -> Option { let previous_paragraph = previous.content.last()?; let next_paragraph = next.content.first()?; @@ -256,16 +256,16 @@ fn list_boundary_spacing( if previous_paragraph.style.line_box.is_some() && next_paragraph.style.line_box.is_some() { return Some(paragraph_gap); } - // Under Word metric text edges the enclosing wrapper's leading already - // tops each line box up to the single-space (or grid) advance; Word adds - // w:spacing before/after on top of that advance, so the item gap is the - // leading plus the paragraph gap. Adding a whole line height instead - // stretched every spaced list by roughly a line per item (issue #384). - if let Some(leading) = metric_leading_pt - && paragraph_uses_metric_edges(previous_paragraph) - && paragraph_uses_metric_edges(next_paragraph) + // The enclosing wrapper's line box already spans Word's full single-space + // (or grid) advance, and Word adds w:spacing before/after on top of that + // advance, so the item gap is exactly the paragraph gap. Adding a whole + // line height instead stretched every spaced list by roughly a line per + // item (issues #384, #452). + if wrapper_spans_full_line + && paragraph_uses_full_line_box(previous_paragraph) + && paragraph_uses_full_line_box(next_paragraph) { - return Some(leading + paragraph_gap); + return Some(paragraph_gap); } // An explicit Typst list spacing replaces its automatic paragraph @@ -276,16 +276,16 @@ fn list_boundary_spacing( Some(line_height + paragraph_gap) } -/// Whether the paragraph renders under the wrapper's metric text edges +/// Whether the paragraph renders under the wrapper's full-advance line box /// (no explicit line spacing or fixed line box of its own). -fn paragraph_uses_metric_edges(paragraph: &Paragraph) -> bool { +fn paragraph_uses_full_line_box(paragraph: &Paragraph) -> bool { paragraph.style.line_spacing.is_none() && paragraph.style.line_box.is_none() } fn common_list_level_spacing( items: &[crate::ir::ListItem], level: u32, - metric_leading_pt: Option, + wrapper_spans_full_line: bool, ) -> Option { let level_items = items .iter() @@ -293,7 +293,7 @@ fn common_list_level_spacing( .collect::>(); let mut boundaries = level_items .windows(2) - .map(|pair| list_boundary_spacing(pair[0], pair[1], metric_leading_pt)); + .map(|pair| list_boundary_spacing(pair[0], pair[1], wrapper_spans_full_line)); let first = boundaries.next()??; (first > 0.0001 @@ -304,7 +304,7 @@ fn common_list_level_spacing( fn list_edge_spacing( list: &List, level: u32, - metric_leading_pt: Option, + wrapper_spans_full_line: bool, ) -> (Option, Option) { let first = list.items.iter().find(|item| item.level == level); let last = list.items.iter().rev().find(|item| item.level == level); @@ -313,13 +313,11 @@ fn list_edge_spacing( return Some(spacing.unwrap_or(0.0).max(0.0)); } // Same Word semantics as the item boundaries: before/after extends - // the line advance, so under metric edges the whitespace is the - // wrapper leading plus the gap (issue #384). - if let Some(leading) = metric_leading_pt - && paragraph_uses_metric_edges(paragraph) - { + // the line advance, which the wrapper's line box already spans, so + // the whitespace is the gap alone (issues #384, #452). + if wrapper_spans_full_line && paragraph_uses_full_line_box(paragraph) { return spacing - .map(|spacing| leading + spacing.max(0.0)) + .map(|spacing| spacing.max(0.0)) .filter(|spacing| *spacing > 0.0001); } spacing @@ -352,14 +350,14 @@ fn common_list_line_box(list: &List) -> Option { pub(super) fn generate_list( out: &mut String, list: &List, - metric_leading_pt: Option, + wrapper_spans_full_line: bool, ) -> Result<(), ConvertError> { let root_level: u32 = list_root_level(list); let style = list_style_for_level(list, root_level); let fallback_marker_style = common_list_level_text_style(&list.items, root_level); let indent = common_list_level_indent(&list.items, root_level); - let spacing_pt = common_list_level_spacing(&list.items, root_level, metric_leading_pt); - let (space_before, space_after) = list_edge_spacing(list, root_level, metric_leading_pt); + let spacing_pt = common_list_level_spacing(&list.items, root_level, wrapper_spans_full_line); + let (space_before, space_after) = list_edge_spacing(list, root_level, wrapper_spans_full_line); let line_box = common_list_line_box(list); let start_at = list.items.first().and_then(|item| item.start_at); if space_before.is_some() || space_after.is_some() { @@ -384,7 +382,7 @@ pub(super) fn generate_list( spacing_pt, start_at, ); - generate_list_items(out, list, &list.items, root_level, metric_leading_pt)?; + generate_list_items(out, list, &list.items, root_level, wrapper_spans_full_line)?; out.push_str(")\n"); if space_before.is_some() || space_after.is_some() { out.push_str("]\n"); @@ -1155,7 +1153,7 @@ fn generate_list_items( list: &List, items: &[crate::ir::ListItem], base_level: u32, - metric_leading_pt: Option, + wrapper_spans_full_line: bool, ) -> Result<(), ConvertError> { let style = list_style_for_level(list, base_level); let (_, item_func) = list_funcs(style.kind); @@ -1202,7 +1200,7 @@ fn generate_list_items( let spacing_pt = common_list_level_spacing( &items[nested_start..nested_end], base_level + 1, - metric_leading_pt, + wrapper_spans_full_line, ); let nested_start_at = items[nested_start].start_at; write_list_open( @@ -1219,7 +1217,7 @@ fn generate_list_items( list, &items[nested_start..nested_end], base_level + 1, - metric_leading_pt, + wrapper_spans_full_line, )?; out.push(')'); i = nested_end; diff --git a/crates/office2pdf/src/render/typst_gen_paragraph_tests.rs b/crates/office2pdf/src/render/typst_gen_paragraph_tests.rs index da62b3da..f097e6ad 100644 --- a/crates/office2pdf/src/render/typst_gen_paragraph_tests.rs +++ b/crates/office2pdf/src/render/typst_gen_paragraph_tests.rs @@ -652,11 +652,9 @@ fn test_document_grid_pitch_snaps_line_height() { // line cannot inflate its advance past the grid (issue #398); the // baseline splits the box by the font's ascender/descender ratio. Uses // a font from Typst's embedded set so the test is environment-free. - let Some((ascender, descender, _)) = - crate::render::pdf::font_line_metrics_em("Libertinus Serif") - else { + if crate::render::pdf::font_line_metrics_em("Libertinus Serif").is_none() { return; // no font book available (e.g. exotic CI sandbox) - }; + } let mut page = match make_flow_page(vec![Block::Paragraph(Paragraph { style: ParagraphStyle::default(), runs: vec![Run { @@ -677,18 +675,10 @@ fn test_document_grid_pitch_snaps_line_height() { let doc = make_doc(vec![Page::Flow(page)]); let result = generate_typst(&doc).unwrap().source; - let expected_leading = 18.0 - (ascender + descender) * 10.0; + assert_line_advance(&result, "Libertinus Serif", 10.0, 18.0); assert!( - result.contains(&format!( - "top-edge: {}em, bottom-edge: -{}em", - format_f64(ascender), - format_f64(descender) - )), - "fixed nominal-metric em edges expected (clamps fallback glyphs): {result}" - ); - assert!( - result.contains(&format!("leading: {}pt", format_f64(expected_leading))), - "grid leading {expected_leading} unchanged from the metric-edge model: {result}" + result.contains("leading: 0pt"), + "the grid advance is carried by the box, not by leading: {result}" ); } @@ -717,23 +707,18 @@ fn test_latin_paragraph_ignores_document_grid() { let doc = make_doc(vec![Page::Flow(page)]); let result = generate_typst(&doc).unwrap().source; - // The paragraph keeps Word's metric single-spacing leading; the 18pt - // grid top-up (leading = 18 - line box) must not appear. + // The paragraph keeps Word's hhea single-spacing advance; the 18pt grid + // pitch must not appear in its line box. let Some((ascender, descender, word_pitch)) = crate::render::pdf::font_line_metrics_em("Libertinus Serif") else { return; }; - let box_pt = (ascender + descender) * 10.0; - let single_leading = (word_pitch * 10.0 - box_pt).max(0.0); - let grid_leading = 18.0 - box_pt; + let single_pt: f64 = (word_pitch * 10.0).max((ascender + descender) * 10.0); + assert_line_advance(&result, "Libertinus Serif", 10.0, single_pt); assert!( - result.contains(&format!("leading: {}pt", format_f64(single_leading))), - "Latin paragraphs keep Word single spacing: {result}" - ); - assert!( - !result.contains(&format!("leading: {}pt", format_f64(grid_leading))), - "Latin paragraphs must not snap to the grid: {result}" + (single_pt - 18.0).abs() > 0.01, + "the fixture only proves anything if the grid pitch differs from single spacing" ); } @@ -760,18 +745,11 @@ fn test_no_document_grid_uses_word_single_spacing() { else { return; }; - let single_leading = (word_pitch * 10.0 - (ascender + descender) * 10.0).max(0.0); - assert!( - result.contains(&format!( - "top-edge: {}em, bottom-edge: -{}em", - format_f64(ascender), - format_f64(descender) - )), - "fixed nominal-metric em edges expected: {result}" - ); + let single_pt: f64 = (word_pitch * 10.0).max((ascender + descender) * 10.0); + assert_line_advance(&result, "Libertinus Serif", 10.0, single_pt); assert!( - result.contains(&format!("leading: {}pt", format_f64(single_leading))), - "Word single-spacing leading expected: {result}" + result.contains("leading: 0pt"), + "the advance is carried by the box, not by leading: {result}" ); } @@ -922,10 +900,9 @@ fn test_tab_advance_defaults_to_36pt_without_grid() { #[test] fn test_latin_paragraph_space_after_stays_raw_gap() { - // Latin single-spacing paragraphs (no document grid) keep their raw - // w:spacing w:after: Word places that gap directly below the metric - // box, so adding the hhea leading here overshoots (issue #394 is scoped - // to grid paragraphs; measured Western fixtures confirm the raw gap). + // Word places `w:spacing w:after` directly below the full line box, + // which the paragraph's own line box already spans, so the gap reaches + // the block unchanged and needs no leading top-up (issues #394, #452). let make_para = |text: &str| { Block::Paragraph(Paragraph { style: ParagraphStyle { @@ -957,14 +934,74 @@ fn test_latin_paragraph_space_after_stays_raw_gap() { } #[test] -fn test_grid_paragraph_space_after_extends_grid_advance() { - // Grid variant: the after-gap sits below the snapped grid line box, so - // the block's `below` is the grid top-up leading plus the gap. - let Some((ascender, descender, _)) = - crate::render::pdf::font_line_metrics_em("Libertinus Serif") - else { +fn test_consecutive_paragraphs_each_advance_by_the_full_font_line() { + // The shaded command lines of a technical manual are separate 9pt + // Courier New paragraphs with `w:spacing w:after="0"`. Word advances + // each by the font's full single-spacing line; Typst only inserts + // `par(leading:)` *between* the lines of one paragraph, so recovering + // the advance that way left every paragraph one leading short and + // consecutive command lines packed 28% tighter than Word (issue #452). + // The line box must therefore span the whole advance on its own. + let Some(advance_pt) = single_spacing_advance_pt(LINE_GAP_FONT, 9.0) else { return; }; + let make_code_line = |text: &str| { + Block::Paragraph(Paragraph { + style: ParagraphStyle { + space_after: Some(0.0), + ..ParagraphStyle::default() + }, + runs: vec![Run { + text: text.to_string(), + style: TextStyle { + font_family: Some(LINE_GAP_FONT.to_string()), + font_size: Some(9.0), + ..TextStyle::default() + }, + href: None, + footnote: None, + }], + }) + }; + let doc = make_doc(vec![make_flow_page(vec![ + make_code_line("$ cargo install office2pdf-cli"), + make_code_line("$ office2pdf --version"), + ])]); + let result = generate_typst(&doc).unwrap().source; + + assert_line_advance(&result, LINE_GAP_FONT, 9.0, advance_pt); + assert!( + result.contains("leading: 0pt"), + "the advance belongs to the box, not to leading: {result}" + ); + assert!( + result.contains("below: 0pt"), + "a zero w:spacing w:after stays zero: {result}" + ); +} + +/// A Typst-embedded font whose hhea line is taller than its typographic +/// metric box, so the single-spacing advance is strictly larger than the +/// metric box. Libertinus Serif - the default test font here - has no line +/// gap at all, which would make the assertions above pass vacuously. +const LINE_GAP_FONT: &str = "DejaVu Sans Mono"; + +/// Word's single-spacing advance for `family` at `font_size`, or `None` +/// when the font is unavailable or its hhea line adds no gap over the +/// typographic metric box (which would make the assertion vacuous). +fn single_spacing_advance_pt(family: &str, font_size: f64) -> Option { + let (ascender, descender, word_pitch) = crate::render::pdf::font_line_metrics_em(family)?; + (word_pitch - (ascender + descender) > 0.001).then_some(word_pitch * font_size) +} + +#[test] +fn test_grid_paragraph_space_after_stays_raw_gap() { + // Grid variant: the snapped line box already spans the full grid pitch, + // so Word's after-gap sits directly below it and reaches the block + // unchanged (issues #394, #452). + if crate::render::pdf::font_line_metrics_em("Libertinus Serif").is_none() { + return; + } let mut page = match make_flow_page(vec![Block::Paragraph(Paragraph { style: ParagraphStyle { space_after: Some(4.0), @@ -988,10 +1025,8 @@ fn test_grid_paragraph_space_after_extends_grid_advance() { let doc = make_doc(vec![Page::Flow(page)]); let result = generate_typst(&doc).unwrap().source; - let grid_leading = 18.0 - (ascender + descender) * 10.0; - let expected = format!("below: {}pt", format_f64(grid_leading + 4.0)); assert!( - result.contains(&expected), - "expected grid paragraph {expected} in: {result}" + result.contains("below: 4pt"), + "grid paragraph keeps the raw 4pt gap: {result}" ); } diff --git a/crates/office2pdf/src/render/typst_gen_tables.rs b/crates/office2pdf/src/render/typst_gen_tables.rs index 0666ecbc..f871ef99 100644 --- a/crates/office2pdf/src/render/typst_gen_tables.rs +++ b/crates/office2pdf/src/render/typst_gen_tables.rs @@ -596,7 +596,7 @@ fn generate_cell_content( if can_render_fixed_text_list_inline(list) { generate_fixed_text_list(out, list, true, None)?; } else { - generate_list(out, list, None)?; + generate_list(out, list, false)?; } } Block::MathEquation(math) => generate_math_equation(out, math), diff --git a/crates/office2pdf/src/render/typst_gen_tests.rs b/crates/office2pdf/src/render/typst_gen_tests.rs index a9fe2979..b8f2f729 100644 --- a/crates/office2pdf/src/render/typst_gen_tests.rs +++ b/crates/office2pdf/src/render/typst_gen_tests.rs @@ -40,6 +40,37 @@ fn make_paragraph(text: &str) -> Block { }) } +/// The `(top_edge_em, bottom_edge_em)` of the first line box the generator +/// emits, or `None` when the source declares no fixed text edges. +fn emitted_line_box_em(source: &str) -> Option<(f64, f64)> { + let after_top: &str = source.split_once("top-edge: ")?.1; + let (top, rest) = after_top.split_once("em")?; + let after_bottom: &str = rest.split_once("bottom-edge: -")?.1; + let (bottom, _) = after_bottom.split_once("em")?; + Some((top.parse().ok()?, bottom.parse().ok()?)) +} + +/// Assert the generated line box spans `expected_pt` and splits it by the +/// font's own ascender-to-descender ratio, which is where Word places the +/// baseline inside the line. Compared numerically rather than as a +/// formatted string so float noise in the em split cannot break the +/// assertion. +fn assert_line_advance(source: &str, family: &str, font_size: f64, expected_pt: f64) { + let (top, bottom) = + emitted_line_box_em(source).unwrap_or_else(|| panic!("no line box emitted in: {source}")); + let advance_pt: f64 = (top + bottom) * font_size; + assert!( + (advance_pt - expected_pt).abs() < 0.01, + "line advance {advance_pt}pt should be {expected_pt}pt in: {source}" + ); + let (ascender, descender, _) = + crate::render::pdf::font_line_metrics_em(family).expect("font metrics should resolve"); + assert!( + (top / bottom - ascender / descender).abs() < 0.01, + "baseline should split the box by the font's ascender/descender ratio: {source}" + ); +} + #[path = "typst_gen_paragraph_tests.rs"] mod paragraph_tests; diff --git a/crates/office2pdf/src/render/typst_gen_text.rs b/crates/office2pdf/src/render/typst_gen_text.rs index 0bfa037e..710acccd 100644 --- a/crates/office2pdf/src/render/typst_gen_text.rs +++ b/crates/office2pdf/src/render/typst_gen_text.rs @@ -37,25 +37,14 @@ pub(super) fn generate_paragraph( word_line_height_settings(¶.runs, style, line_grid_pitch); let has_para_style = needs_block_wrapper(style) || line_height_settings.is_some(); - // Word measures w:spacing w:before/w:after from the bottom of the full - // grid line box, while the metric text edges end at the descender: a - // grid-snapped paragraph must carry the grid top-up leading on its - // block above/below or every paragraph boundary loses it and the page - // rhythm drifts (issue #394). - let mut boundary_style = std::borrow::Cow::Borrowed(style); - if let Some(leading) = word_grid_boundary_leading_pt(¶.runs, style, line_grid_pitch) - && (style.space_before.is_some() || style.space_after.is_some()) - { - let adjusted = boundary_style.to_mut(); - adjusted.space_before = style.space_before.map(|gap| gap.max(0.0) + leading); - adjusted.space_after = style.space_after.map(|gap| gap.max(0.0) + leading); - } - if has_para_style { // The wrapper must span the full line width: Typst blocks shrink to // their content by default, which would defeat the inner #align. + // Word measures `w:spacing w:before/w:after` from the edges of the + // full line box, which `word_line_height_settings` spans directly, + // so those gaps reach the block unmodified (issues #394, #452). out.push_str("#block(width: 100%"); - write_block_params_continuation(out, &boundary_style); + write_block_params_continuation(out, style); out.push_str(")[\n"); write_paragraph_double_border_overlays(out, &style.border); write_line_box_settings(out, style.line_box); @@ -120,37 +109,47 @@ pub(super) fn needs_block_wrapper(style: &ParagraphStyle) -> bool { || matches!(style.direction, Some(TextDirection::Rtl)) } -/// Word snaps body lines to the section's document grid (`w:docGrid` -/// `w:linePitch`); Typst's glyph-tight default renders such documents -/// 20-30% shorter and shifts every page break. When the section carries a -/// grid pitch, the paragraph has no explicit line spacing, and the font's -/// metrics are known, emit metric line edges plus leading that tops the -/// line box up to the next grid multiple. +/// Line-box settings for a body paragraph: a fixed box spanning Word's full +/// line advance — the font's hhea line, or the document grid pitch for East +/// Asian text under a `w:docGrid` — split by the ascender/descender ratio, +/// with zero leading. Typst's glyph-tight default renders grid documents +/// 20-30% shorter and shifts every page break (issue #354). +/// +/// Carrying the advance inside the box, rather than recovering the +/// remainder as `par(leading:)`, is what makes a paragraph's height match +/// Word's. Typst inserts `leading` only *between* the lines of one +/// paragraph, so an n-line paragraph came out one whole leading short +/// however many lines it had, and consecutive 9pt Courier New paragraphs +/// advanced 28% tighter than Word (issue #452). It also lets `w:spacing +/// w:before/w:after` reach the block unchanged, because the block edges now +/// sit exactly where Word measures those gaps from (issue #394). pub(super) fn word_line_height_settings( runs: &[Run], style: &ParagraphStyle, line_grid_pitch: Option, ) -> Option { - let (ascender_em, descender_em, leading_pt) = + let (ascender_em, descender_em, leading_em) = word_line_box_and_leading(runs, style, line_grid_pitch)?; + let metric_em: f64 = ascender_em + descender_em; + if metric_em <= 0.0 { + return None; + } + let pitch_em: f64 = metric_em + leading_em; // Pin the line box to the nominal font's own metric edges as fixed em // values rather than the "ascender"/"descender" keywords. The keywords // let Typst resolve the box against the tallest font on each line, so a // bullet marker or em dash pulled from a taller fallback font inflated // that one line's advance past the grid/single-spacing (issue #398). - // Fixed nominal-metric edges keep every normal line identical to before - // (same baseline, same leading) while clamping the fallback-glyph line. Some(format!( - "#set text(top-edge: {}em, bottom-edge: -{}em)\n#set par(leading: {}pt)\n", - format_f64(ascender_em), - format_f64(descender_em), - format_f64(leading_pt) + "#set text(top-edge: {}em, bottom-edge: -{}em)\n#set par(leading: 0pt)\n", + format_f64(pitch_em * ascender_em / metric_em), + format_f64(pitch_em * descender_em / metric_em) )) } /// The nominal font's `(ascender_em, descender_em)` metric edges plus the -/// leading that tops the line box up to Word's single-spacing or grid -/// advance. `None` when the metric-edge treatment does not apply. +/// leading, in em, that tops the line box up to Word's single-spacing or +/// grid advance. `None` when the metric-edge treatment does not apply. fn word_line_box_and_leading( runs: &[Run], style: &ParagraphStyle, @@ -162,7 +161,18 @@ fn word_line_box_and_leading( .find_map(|run| run.style.font_family.as_deref())?; let (ascender_em, descender_em, _word_pitch_em) = crate::render::pdf::font_line_metrics_em(family)?; - Some((ascender_em, descender_em, leading_pt)) + let font_size: f64 = paragraph_font_size_pt(runs); + Some((ascender_em, descender_em, leading_pt / font_size)) +} + +/// The font size Word resolves a paragraph's line box against: the largest +/// size among its runs, falling back to the Word default when unset. +fn paragraph_font_size_pt(runs: &[Run]) -> f64 { + let largest: f64 = runs + .iter() + .filter_map(|run| run.style.font_size) + .fold(f64::NAN, f64::max); + if largest.is_nan() { 11.0 } else { largest } } /// Line-box settings for a table cell: a fixed box spanning the font's @@ -193,11 +203,15 @@ pub(super) fn word_cell_line_box_settings(runs: &[Run], style: &ParagraphStyle) )) } -/// The leading that accompanies Word metric text edges: the whitespace -/// Typst must insert between metric line boxes so consecutive lines land at -/// Word's single-space advance (or the document grid pitch). `None` when -/// the paragraph carries explicit line spacing or the font's metrics are -/// unknown — the metric-edge treatment does not apply then. +/// The top-up that raises the font's typographic metric box to Word's line +/// advance — its hhea single-space line, or the document grid pitch for East +/// Asian text under a `w:docGrid`. `word_line_height_settings` folds this +/// into the fixed line-box height rather than emitting it as `par(leading:)` +/// whitespace between boxes, because Typst inserts that only *between* the +/// lines of one paragraph and every paragraph then came up one top-up short +/// (issues #354, #452). `None` when the paragraph carries explicit line +/// spacing or the font's metrics are unknown — the treatment does not apply +/// then. pub(super) fn word_line_leading_pt( runs: &[Run], style: &ParagraphStyle, @@ -241,26 +255,6 @@ pub(super) fn word_line_leading_pt( Some(leading_pt) } -/// The grid top-up leading a paragraph carries only when its lines snap to -/// the document grid (East Asian text under a `w:docGrid`). Word measures -/// `w:spacing w:before/w:after` from the bottom of the full grid line box, -/// so a metric-edge paragraph must add this leading to its block -/// above/below or every paragraph boundary loses it and the page rhythm -/// drifts (issue #394). Latin single-spacing paragraphs are excluded: -/// their `w:spacing` sits directly below the metric box in Word, so adding -/// the hhea leading there overshoots (measured on Western fixtures). -pub(super) fn word_grid_boundary_leading_pt( - runs: &[Run], - style: &ParagraphStyle, - line_grid_pitch: Option, -) -> Option { - let pitch: f64 = line_grid_pitch.filter(|pitch| *pitch > 0.0)?; - if !runs.iter().any(|run| run.text.chars().any(is_cjk_like)) { - return None; - } - word_line_leading_pt(runs, style, Some(pitch)) -} - pub(super) fn write_block_params(out: &mut String, style: &ParagraphStyle) { let mut first = true; diff --git a/crates/office2pdf/tests/docx_fixtures.rs b/crates/office2pdf/tests/docx_fixtures.rs index 667872f3..dc0aba3c 100644 --- a/crates/office2pdf/tests/docx_fixtures.rs +++ b/crates/office2pdf/tests/docx_fixtures.rs @@ -236,8 +236,10 @@ fn acceptance_pr_187_contributor_acceptance_table_row_metrics() { .expect("fixture table cells should contain a paragraph"); // Line height stays unset in the IR: the renderer derives Word's // single-spacing pitch from the actual font metrics (issue #354). + // The fixture sets no `w:spacing w:after`, which Word reads as zero + // (issue #452). assert_eq!(paragraph.style.line_box, None); - assert_eq!(paragraph.style.space_after, Some(8.0)); + assert_eq!(paragraph.style.space_after, Some(0.0)); } } @@ -283,7 +285,7 @@ fn acceptance_pr_187_contributor_acceptance_style_inherited_numbering() { let style = &item.content[0].style; assert_eq!(style.indent_left, Some(36.0)); assert_eq!(style.indent_first_line, Some(-18.0)); - assert_eq!(style.space_after, Some(8.0)); + assert_eq!(style.space_after, Some(0.0)); } } }