diff --git a/assets/bugfixes/issue-461/after.jpg b/assets/bugfixes/issue-461/after.jpg new file mode 100644 index 00000000..44f7f775 Binary files /dev/null and b/assets/bugfixes/issue-461/after.jpg differ diff --git a/assets/bugfixes/issue-461/before.jpg b/assets/bugfixes/issue-461/before.jpg new file mode 100644 index 00000000..a8306408 Binary files /dev/null and b/assets/bugfixes/issue-461/before.jpg differ diff --git a/assets/bugfixes/issue-461/gt.jpg b/assets/bugfixes/issue-461/gt.jpg new file mode 100644 index 00000000..9dc9e0b9 Binary files /dev/null and b/assets/bugfixes/issue-461/gt.jpg differ diff --git a/crates/office2pdf/src/parser/drawingml.rs b/crates/office2pdf/src/parser/drawingml.rs index 6405aa42..cc106830 100644 --- a/crates/office2pdf/src/parser/drawingml.rs +++ b/crates/office2pdf/src/parser/drawingml.rs @@ -297,6 +297,72 @@ pub(crate) fn parse_theme_color_scheme(xml: &str) -> HashMap { colors } +/// The Latin typefaces a theme part's `` declares. A slot the +/// theme leaves empty stays `None`, because DrawingML spells "inherit" as an +/// empty `typeface` and an empty family would select nothing. +#[derive(Debug, Default, Clone, PartialEq, Eq)] +pub(crate) struct ThemeFontScheme { + pub(crate) major_latin: Option, + pub(crate) minor_latin: Option, +} + +impl ThemeFontScheme { + /// Resolve a DrawingML `typeface` attribute. The placeholders `+mj-lt` + /// and `+mn-lt` name this scheme's major and minor Latin fonts; any + /// other value is a literal family name. + pub(crate) fn resolve_typeface(&self, typeface: &str) -> Option { + match typeface { + "" => None, + "+mj-lt" => self.major_latin.clone(), + "+mn-lt" => self.minor_latin.clone(), + literal => Some(literal.to_string()), + } + } +} + +/// Parse just the `` Latin typefaces out of a theme part +/// (`theme1.xml`). +/// +/// The pptx parser keeps its own combined single-pass reader because it +/// also collects colors and fill styles from the same document. +pub(crate) fn parse_theme_font_scheme(xml: &str) -> ThemeFontScheme { + let mut fonts = ThemeFontScheme::default(); + let mut reader = Reader::from_str(xml); + let mut in_major: bool = false; + let mut in_minor: bool = false; + + loop { + match reader.read_event() { + Ok(Event::Start(ref e)) => match e.local_name().as_ref() { + b"majorFont" => in_major = true, + b"minorFont" => in_minor = true, + _ => {} + }, + Ok(Event::End(ref e)) => match e.local_name().as_ref() { + b"majorFont" => in_major = false, + b"minorFont" => in_minor = false, + _ => {} + }, + Ok(Event::Empty(ref e)) if e.local_name().as_ref() == b"latin" => { + let typeface: Option = + get_attr_str(e, b"typeface").filter(|typeface| !typeface.is_empty()); + // Only a slot's first `` is its Latin face; the + // script-specific `` entries that follow are not. + if in_major { + fonts.major_latin = fonts.major_latin.take().or(typeface); + } else if in_minor { + fonts.minor_latin = fonts.minor_latin.take().or(typeface); + } + } + Ok(Event::Eof) => break, + Err(_) => break, + _ => {} + } + } + + fonts +} + #[cfg(test)] #[path = "drawingml_tests.rs"] mod tests; diff --git a/crates/office2pdf/src/parser/xlsx_drawing.rs b/crates/office2pdf/src/parser/xlsx_drawing.rs index 53107936..550ed74b 100644 --- a/crates/office2pdf/src/parser/xlsx_drawing.rs +++ b/crates/office2pdf/src/parser/xlsx_drawing.rs @@ -3,6 +3,7 @@ use std::io::Cursor; use crate::ir::Chart; use crate::parser::chart::parse_chart_xml; +use crate::parser::drawingml::ThemeFontScheme; use crate::parser::xml_util; /// Extract charts from the XLSX ZIP with their anchor positions per sheet. @@ -671,7 +672,7 @@ pub(super) fn extract_text_boxes_with_anchors( let sheet_rids = parse_workbook_sheet_rids(&workbook_xml); let workbook_rels_xml = read_zip_entry_string(&mut archive, "xl/_rels/workbook.xml.rels"); let rid_to_target = parse_rels_targets(&workbook_rels_xml); - let theme_colors = workbook_theme_colors(&mut archive, &workbook_rels_xml); + let (theme_colors, theme_fonts) = workbook_theme(&mut archive, &workbook_rels_xml); let mut result: HashMap> = HashMap::new(); @@ -693,7 +694,7 @@ pub(super) fn extract_text_boxes_with_anchors( if drawing_xml.is_empty() { continue; } - let boxes = parse_drawing_text_boxes(&drawing_xml, &theme_colors); + let boxes = parse_drawing_text_boxes(&drawing_xml, &theme_colors, &theme_fonts); if !boxes.is_empty() { result.entry(sheet_name.clone()).or_default().extend(boxes); } @@ -746,10 +747,12 @@ fn resolved_or_legacy( } /// Parse `` text boxes from a worksheet drawing, resolving scheme -/// colors against the workbook theme palette. +/// colors against the workbook theme palette and run typefaces against its +/// font scheme. pub(super) fn parse_drawing_text_boxes( xml: &str, theme_colors: &HashMap, + theme_fonts: &ThemeFontScheme, ) -> Vec { use crate::ir::{ Alignment, BorderLineStyle, BorderSide, Paragraph, ParagraphStyle, Run, TextStyle, @@ -895,6 +898,13 @@ pub(super) fn parse_drawing_text_boxes( } } } + b"latin" if in_run => { + if let Some(typeface) = xml_util::get_attr_str(e, b"typeface") + && let Some(family) = theme_fonts.resolve_typeface(&typeface) + { + current_style.font_family = Some(family); + } + } b"pPr" if current_para.is_some() => { for attr in e.attributes().flatten() { if attr.key.local_name().as_ref() == b"algn" @@ -948,9 +958,17 @@ pub(super) fn parse_drawing_text_boxes( Ok(quick_xml::events::Event::Text(ref t)) => { if in_text && let Ok(text) = t.xml_content() { if let Some(para) = current_para.as_mut() { + let mut style = current_style.clone(); + // An `` without `` - how Excel writes + // plain shape labels - resolves to the theme's minor + // Latin font in DrawingML, not to a renderer default + // (issue #461). + if style.font_family.is_none() { + style.font_family = theme_fonts.minor_latin.clone(); + } para.runs.push(Run { text: text.to_string(), - style: current_style.clone(), + style, href: None, footnote: None, }); @@ -1025,19 +1043,24 @@ pub(super) fn parse_drawing_text_boxes( result } -/// Read the workbook theme palette (`xl/theme/theme1.xml` or the -/// rels-declared target). Missing or unreadable themes yield an empty map, -/// which downgrades scheme resolution to the legacy light/dark fallback. -fn workbook_theme_colors( +/// Read the workbook theme's color palette and Latin font scheme +/// (`xl/theme/theme1.xml`, or the rels-declared target). A missing or +/// unreadable theme yields an empty palette, which downgrades scheme +/// resolution to the legacy light/dark fallback, and an empty font scheme, +/// which leaves font selection to the renderer. +fn workbook_theme( archive: &mut zip::ZipArchive>, workbook_rels_xml: &str, -) -> HashMap { +) -> (HashMap, ThemeFontScheme) { let theme_path = parse_rels_by_type(workbook_rels_xml, "theme") .first() .map(|target| resolve_relative_xl_path("xl", target)) .unwrap_or_else(|| "xl/theme/theme1.xml".to_string()); let theme_xml = read_zip_entry_string(archive, &theme_path); - crate::parser::drawingml::parse_theme_color_scheme(&theme_xml) + ( + crate::parser::drawingml::parse_theme_color_scheme(&theme_xml), + crate::parser::drawingml::parse_theme_font_scheme(&theme_xml), + ) } #[cfg(test)] diff --git a/crates/office2pdf/src/parser/xlsx_drawing_tests.rs b/crates/office2pdf/src/parser/xlsx_drawing_tests.rs index b7d4a2f4..0edbc9bc 100644 --- a/crates/office2pdf/src/parser/xlsx_drawing_tests.rs +++ b/crates/office2pdf/src/parser/xlsx_drawing_tests.rs @@ -2,6 +2,7 @@ use std::collections::HashMap; use super::*; use crate::ir::Color; +use crate::parser::drawingml::ThemeFontScheme; fn accent_theme() -> HashMap { // Office default theme slots a workbook actually ships in theme1.xml. @@ -36,7 +37,11 @@ fn drawing_with_fill(color_markup: &str) -> String { } fn fill_of(color_markup: &str, theme: &HashMap) -> Option { - let boxes = parse_drawing_text_boxes(&drawing_with_fill(color_markup), theme); + let boxes = parse_drawing_text_boxes( + &drawing_with_fill(color_markup), + theme, + &ThemeFontScheme::default(), + ); assert_eq!(boxes.len(), 1, "fixture should yield one text box"); boxes[0].fill } @@ -131,3 +136,128 @@ fn theme_color_scheme_parses_from_theme_xml() { assert_eq!(colors.get("hlink"), Some(&Color::new(0x05, 0x63, 0xC1))); assert_eq!(colors.get("accent2"), None); } + +fn office_theme_fonts() -> ThemeFontScheme { + // The font scheme an Excel-saved workbook ships in theme1.xml. + ThemeFontScheme { + major_latin: Some("Calibri Light".to_string()), + minor_latin: Some("Calibri".to_string()), + } +} + +fn drawing_with_run_properties(run_properties: &str) -> String { + format!( + r#" + + 1010 + 4040 + + + + {run_properties}accent1 + + + + +"# + ) +} + +fn font_of(run_properties: &str, fonts: &ThemeFontScheme) -> Option { + let boxes = parse_drawing_text_boxes( + &drawing_with_run_properties(run_properties), + &HashMap::new(), + fonts, + ); + assert_eq!(boxes.len(), 1, "fixture should yield one text box"); + boxes[0].paragraphs[0].runs[0].style.font_family.clone() +} + +#[test] +fn drawing_run_without_a_typeface_inherits_the_theme_minor_font() { + // Excel writes shape labels as `` with no + // typeface at all; DrawingML resolves that to the theme's minor Latin + // font, not to the renderer's serif default (issue #461). + assert_eq!( + font_of(r#""#, &office_theme_fonts()), + Some("Calibri".to_string()) + ); +} + +#[test] +fn drawing_run_without_run_properties_inherits_the_theme_minor_font() { + assert_eq!( + font_of("", &office_theme_fonts()), + Some("Calibri".to_string()) + ); +} + +#[test] +fn drawing_run_uses_its_explicit_latin_typeface() { + assert_eq!( + font_of( + r#""#, + &office_theme_fonts() + ), + Some("Georgia".to_string()) + ); +} + +#[test] +fn drawing_run_resolves_theme_typeface_placeholders() { + // `+mj-lt` and `+mn-lt` name the theme's major and minor Latin fonts. + assert_eq!( + font_of( + r#""#, + &office_theme_fonts() + ), + Some("Calibri Light".to_string()) + ); + assert_eq!( + font_of( + r#""#, + &office_theme_fonts() + ), + Some("Calibri".to_string()) + ); +} + +#[test] +fn drawing_run_font_stays_unset_without_a_theme_font_scheme() { + // A workbook with no readable theme part leaves font selection to the + // renderer's existing fallback rather than inventing a family. + assert_eq!( + font_of(r#""#, &ThemeFontScheme::default()), + None + ); +} + +#[test] +fn theme_font_scheme_parses_from_theme_xml() { + let theme_xml = r#" + + + + + + +"#; + let fonts = crate::parser::drawingml::parse_theme_font_scheme(theme_xml); + assert_eq!(fonts.major_latin.as_deref(), Some("Calibri Light")); + assert_eq!(fonts.minor_latin.as_deref(), Some("Calibri")); +} + +#[test] +fn theme_font_scheme_ignores_empty_typefaces() { + // Themes spell "inherit" as an empty typeface; it must not become a font. + let theme_xml = r#" + + + + + + +"#; + let fonts = crate::parser::drawingml::parse_theme_font_scheme(theme_xml); + assert_eq!(fonts, ThemeFontScheme::default()); +} diff --git a/crates/office2pdf/src/render/font_subst.rs b/crates/office2pdf/src/render/font_subst.rs index e48b066d..a243959a 100644 --- a/crates/office2pdf/src/render/font_subst.rs +++ b/crates/office2pdf/src/render/font_subst.rs @@ -515,6 +515,11 @@ fn collect_document_font_families(doc: &Document) -> BTreeSet { collect_header_footer_fonts(footer, &mut fonts); } collect_table_fonts(&page.table, &mut fonts); + for text_box in &page.text_boxes { + for paragraph in &text_box.paragraphs { + collect_block_fonts(&Block::Paragraph(paragraph.clone()), &mut fonts); + } + } } } } @@ -553,10 +558,22 @@ pub(crate) fn document_requests_font_families(doc: &Document) -> bool { .as_ref() .is_some_and(header_footer_requests_font_family) || table_requests_font_family(&page.table) + // Worksheet drawings carry their own runs; a workbook whose + // only font request comes from a shape label still needs the + // font search context, or the compiler never sees the + // directories that hold the requested face (issue #461). + || page.text_boxes.iter().any(sheet_text_box_requests_font_family) } }) } +fn sheet_text_box_requests_font_family(text_box: &crate::ir::SheetTextBox) -> bool { + text_box + .paragraphs + .iter() + .any(|paragraph| block_requests_font_family(&Block::Paragraph(paragraph.clone()))) +} + fn resolve_available_fallback(font_family: &str, context: &FontSearchContext) -> Option { if context.has_family(font_family) { return None;