fix(xlsx): carry the workbook Normal font onto unstyled cells#469
Merged
Conversation
A cell with no `s` attribute uses `cellXfs[0]`, whose font is the workbook Normal font. umya reports no font at all for such a cell, so the style path returned `TextStyle::default()` and the renderer picked its own family and size: an Excel-saved workbook whose Normal font is Malgun Gothic 12 rendered its title in Typst's serif default at 11pt. Read the Normal font from `styles.xml` - `extract_normal_font` already did, for column print metrics - carry it on `SheetContext`, and fall back to it whenever a cell's own style names no font or no size. Two special cases went with it. The family was discarded when it was "Calibri" and the size when it was 11.0, on the grounds that they were "the Excel default"; both left the most common workbooks to the renderer's serif default rather than to Calibri, which font substitution resolves perfectly well. A rich-text run naming Calibri was ignored for the same reason and now counts like any other family. `extract_normal_font` returns a `NormalFont` struct rather than a bare `(String, f64)`, and `prepare_sheet_context` takes it directly instead of a precomputed max-digit-width, so the print-metric derivation lives next to the font it derives from. Fixes #462 Signed-off-by: Yonghye Kwon <developer.0hye@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
styles.xmlfont 0) onto cells whose own style names no font or sizeNormalFontfromextract_normal_fontand derive the column print metric next to itRelated issue
Fixes #462
Details
A cell with no
sattribute usescellXfs[0], whose font is the workbook Normal font. umya reports no font at all for such a cell, so the style path returnedTextStyle::default()and the renderer picked its own family and size.Two special cases compounded it: the family was discarded when it was
"Calibri"and the size when it was 11.0, on the grounds that they were "the Excel default". Both left the most common workbooks to the renderer's serif default rather than to Calibri, which font substitution resolves perfectly well. A rich-text run naming Calibri was ignored for the same reason.pdffontson the regression fixture, whose Normal font is Malgun Gothic 12:LibertinusSerif-Regular,CalibriMalgunGothic,CalibriThe Libertinus fallback is gone: nothing on the page falls back to the renderer default any more.
Testing
cargo fmt --all -- --checkcargo test --workspacecargo clippy --workspace --all-targets --all-featurespdffontsbefore/after comparison ontests/fixtures/xlsx/theme_color_drawing.xlsxpdftoppm -r 150rendering, matched full-resolution title crops, andmagick compare -metric AE -fuzz 5%Visual impact
Visual audit
tests/fixtures/xlsx/theme_color_drawing.xlsxpdftoppm, 150 DPIfixassets/bugfixes/issue-462/gt.jpgassets/bugfixes/issue-462/before.jpgassets/bugfixes/issue-462/after.jpgThe GT is the native Microsoft Excel export already captured for this fixture in #434, reused unchanged: same workbook, same page, same renderer, same 150 DPI.
Visual comparison
Required inspection
The 5% fuzz sweep against the size-normalized GT moves from 290,500 to 290,889 differing pixels: the drawing geometry defects tracked in #459 and #460 displace all three shapes and dominate the count, and the now-wider sans-serif title covers slightly more pixels than the narrow serif it replaced. The matched title crops at full resolution show the change directly — the GT and the after image both set the A1 title in the workbook's 12pt sans-serif Normal font, the before image in an 11pt serif. The fixture contains no hairlines, borders, or dashed strokes, and every run is regular weight with no italic or underline.
Deviation audit
Checklist
Signed-off-byline