fix(xlsx): price a bold cell's own digit advance for weight-sensitive faces - #1768
Merged
Merged
Conversation
… faces Cambria, Verdana, Segoe UI/Selawik, and Malgun Gothic each measurably widen their decimal digits when bold (verified directly against Excel's own Cambria.ttc/Cambriab.ttf, Verdana.ttf/Verdana Bold.ttf, malgun.ttf/malgunbd.ttf, and this crate's bundled Selawik-Regular.ttf/Selawik-Bold.ttf with fontTools), so a bold cell in one of these families was priced on the regular-weight digit width and undershot its whole-point left inset by a full step. A 42pt bold Cambria title landed 1.255pt left of a native Excel for Mac export. Thread bold: bool through reference_digit_advance_em, digit_advance_em, column_unit_pt, cell_left_inset_pt, cell_right_inset_pt, and max_digit_advance_em, and add a matching typst::text::FontVariant to the live-face lookup so an unlisted family also resolves its bold digit advance from the real face. Reuse first_face_in_chain (issue #1629's interleaved in-memory-then-disk order) from the new weight-aware lookup instead of duplicating that resolution order inline. Calibri/Carlito, Arial/Helvetica/Liberation Sans, Times New Roman, and Courier New are confirmed weight-invariant the same way and left untouched. Fixes #1623 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Signed-off-by: Yonghye Kwon <developer.0hye@gmail.com>
Fresh native Excel for Mac 300 DPI GT/before/after for page 2 of tests/fixtures/xlsx/issue_1181_fit_to_height.xlsx, plus a --fine-shift 0.5 layout audit and a strict render-cluster report for the same page. The title's measured dx moves from -1.255pt to -0.475pt; 348 of 354 render clusters are reused byte-for-byte (exact bbox match) from PR #1766's report for #1622 on this same fixture/page, and the 6 new clusters are exactly the text this fix moves. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> 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.
File submission policy
Summary
Cambria.ttc/Cambriab.ttfandVerdana.ttf/Verdana Bold.ttfwithfontTools), so a bold cell in either family was priced on the regular-weight digit width and undershot its whole-point left inset by a full step.bold: boolthroughreference_digit_advance_em,digit_advance_em,column_unit_pt,cell_left_inset_pt,cell_right_inset_pt, andmax_digit_advance_em; add a matchingtypst::text::FontVariantto the live-face lookup so an unlisted family also resolves its bold digit advance from the real face.first_face_in_chain(issue PPTX: line-box metrics take the bundled Noto Serif over a face supplied on --font-path, seating Avenir Next LT Pro 2pt high #1629's interleaved in-memory-then-disk order) from the new weight-aware live-face lookup instead of duplicating that resolution order inline, so the two can never drift apart.Related issue
Fixes #1623
Testing
python3 -c "from fontTools.ttLib import TTFont, TTCollection; ..."overCambria.ttc/Cambriab.ttf/Verdana.ttf/Verdana Bold.ttfgives Cambria regular 1134/2048em, Cambria bold 1213/2048em, Verdana regular 1302/2048em, Verdana bold 1456/2048em — exactly the constants this PR adds.probe_harness.pySPEC — a fresh one-cellumyaworkbook round-tripped throughscripts/macos/export_excel_pdfs.applescript): Cambria 36pt regular vs bold shifts the printed digit start by +1.0pt, 48pt by +0.0pt, matching this PR's table (6→7pt, 8→8pt) exactly.reference_digit_advance_emignorebold, confirmedtest_bold_cell_font_prices_its_own_bold_digit_advanceandtest_reference_digit_advance_em_is_bold_aware_only_where_measuredboth fail (and the Calibri negative-control test still passes), then restored the fix — the new tests are not vacuously green.cargo test --locked -p office2pdf --lib(3092 passed)cargo fmt --all -- --checkcargo clippy --locked -p office2pdf --all-targets(stable 1.98, matching CI)cargo check --locked -p office2pdf --target wasm32-unknown-unknownpython3 scripts/compare_layout.py gt.pdf before.pdf --page 2 --audit --fine-shift 0.5 --noise-floor 0.5 --json(Monthlycollegebudgetdx -1.255pt)python3 scripts/compare_layout.py gt.pdf after.pdf --page 2 --audit --fine-shift 0.5 --noise-floor 0.5 --json(same instance dx -0.475pt; the other 546 fine-shift entries are byte-identical before/after — nothing else moved)python3 scripts/compare_render.py gt.pdf after.pdf --page 2 --dpi 300 --fine-shift 0.5 --artifacts-dir artifacts-page2 --cluster-report assets/bugfixes/issue-1623/render-clusters-page-2.json --cluster-dispositions final-dispositions.json --strict-clusters(354/354 dispositioned, PASS; 348 reused by exact bbox match from PR fix(xlsx): scale Excel worksheet bar-chart value-axis label gap with font size #1766's report for XLSX: fitted horizontal bar-chart value labels sit about 0.61pt too high #1622 on this same fixture/page, 6 new clusters are this fix's own effect — the title and the "Monthly Cash After Expenses" sub-heading, both bold Cambria — dispositioned to XLSX: fitted worksheet text keeps the physical margin origin while its paint layer snaps to Excel's sheet-space origin #1719, the already-tracked fitted-text-origin residual that both now land on)Visual impact
Visual audit
tests/fixtures/xlsx/issue_1181_fit_to_height.xlsx(unmodified; SHA-2562b4a2d8dceda58758593c88409875efbda05780559154c02bd13fef4f7a1c65b)fixassets/bugfixes/issue-1623/layout-audit.jsonassets/bugfixes/issue-1623/render-clusters-page-2.jsonassets/bugfixes/issue-1623/gt.jpg/before.jpg/after.jpg), plus a zoomed crop stacking the GT and output title regions and a separate crop of the "Monthly Cash After Expenses" sub-heading (both bold Cambria, both benefit from this fix). At normal viewing scale the two renders are visually indistinguishable; the measured title dx moves from -1.255pt to -0.475pt against the fresh native export, with dy unchanged at -0.5999pt (that residual is the title's own baseline seat; the issue body deferred it to XLSX: fitted fixed-row cell baselines sit up to 2.16pt above Excel on the budget sheet #1545, which closed without actually fixing this row — now re-filed as XLSX: fitted sheet's row-1 title baseline still sits 0.6pt above Excel after #1545 #1767, out of scope for this PR). The strict render-cluster audit confirms this numerically: 354/354 clusters dispositioned, of which 348 are reused byte-for-byte (exact bbox match) from PR fix(xlsx): scale Excel worksheet bar-chart value-axis label gap with font size #1766's report for XLSX: fitted horizontal bar-chart value labels sit about 0.61pt too high #1622 on this same fixture/page — meaning nothing else on the page moved a pixel — and the 6 new clusters are exactly the title and sub-heading text this fix shifts, still carrying the small XLSX: fitted worksheet text keeps the physical margin origin while its paint layer snaps to Excel's sheet-space origin #1719 residual visible in the crops.assets/bugfixes/issue-1623/gt.jpgassets/bugfixes/issue-1623/before.jpgassets/bugfixes/issue-1623/after.jpgassets/bugfixes/issue-1623/compare.jpg(filed with the original issue)Visual comparison
Required inspection
Deviation audit
Checklist
Signed-off-bylineRelated: #1545, #1621, #1631, #1632, #1719, #1721, #1757, #1759, #1760, #1761, #1767