fix(textkit): align line-box height and font metrics with browser layout#3425
Open
JamesGoslings wants to merge 1 commit into
Open
fix(textkit): align line-box height and font metrics with browser layout#3425JamesGoslings wants to merge 1 commit into
JamesGoslings wants to merge 1 commit into
Conversation
Resolves descender clipping for CJK glyphs (and any other font whose hhea table is inflated for legacy GDI compatibility) when a user supplies a lineHeight smaller than the font's natural line height. Two independent gaps relative to CSS line-box rules are addressed: 1. `height(run)` now applies `max(line-height, content-area)` instead of short-circuiting whenever a user lineHeight is set. Previously a tightened lineHeight produced a line-box smaller than the run's actual ascent + descent, while the line baseline was still computed from the real (CJK-aware) ascent in finalizeLine — so descenders fell outside the box and overlapped the next line. Spec ref: https://www.w3.org/TR/CSS22/visudet.html#line-height. 2. `ascent` / `descent` / `lineGap` now prefer the OS/2 typographic triple (sTypoAscender / sTypoDescender / sTypoLineGap) when present and fall back to fontkit's hhea defaults otherwise. This mirrors how Chromium reads font metrics for line-box content height, restoring the v3-era visual density for CJK fonts whose hhea is inflated for Windows GDI compatibility — notably Source Han Sans / Serif (a.k.a. Noto Sans / Serif CJK), where hhea reports 1.448 em vs the typo metrics' 1.000 em. The OS/2 path is opt-in: any font without numeric typoAscender / typoDescender values falls through to the existing hhea computation, so StandardFont (Helvetica / Courier / Times-Roman) and renderer snapshot tests are byte-identical to before. Closes diegomura#3424. Reported in detail (with reproducer and metrics tables) downstream at amruthpillai/reactive-resume#3069 and validated there as a pnpm patch against @react-pdf/textkit@6.3.0. - 794 textkit unit tests pass (783 pre-existing + 11 new covering the OS/2 fallback ladder and the Math.max line-box rule). - 2246 monorepo tests pass — including renderer integration tests that rasterise to PDF — confirming no regression for fonts on the hhea fallback path.
🦋 Changeset detectedLatest commit: c2725f0 The changes in this PR will be included in the next version bump. This PR includes changesets to release 9 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
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
1 participant
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.
Closes #3424.
Summary
Fixes two independent root causes that make CJK glyphs (and any other font with an inflated hhea table) get their descenders clipped or overlap the next line whenever a user supplies a lineHeight smaller than the font's natural line height.
Root cause
height(run) ignores the CSS max(line-height, content-area) rule. Today it short-circuits to the user-supplied lineHeight whenever one is set, but the line baseline is still computed in finalizeLine from the actual ascent. A tightened lineHeight therefore produces a line-box smaller than ascent − descent + lineGap, and descenders fall outside it. Spec: https://www.w3.org/TR/CSS22/visudet.html#line-height.
ascent / descent / lineGap read only hhea. Many CJK fonts — most notably Source Han Sans / Serif (Noto Sans / Serif CJK) — set hhea to ~1.448 em for legacy Windows GDI compatibility, while the OS/2 typographic triple (sTypoAscender / sTypoDescender / sTypoLineGap) reports the true ~1.000 em. Chromium uses the OS/2 triple for line-box content height; textkit currently doesn't, which is what produces the visibly looser layout vs. browsers and the descender clipping when combined with #1.
Fix
New helper resolveTypoMetrics(font) prefers OS/2 sTypoAscender / sTypoDescender / sTypoLineGap when all three are numeric, falling back to fontkit's hhea-derived ascent / descent / lineGap otherwise.
ascent.ts / descent.ts / lineGap.ts go through the helper.
height.ts returns Math.max(lineHeight || 0, lineGap + ascent − descent).
The OS/2 path is fully opt-in: any font without numeric typo metrics (e.g. PDF StandardFont — Helvetica / Courier / Times-Roman, whose 'OS/2' is {}) falls through to the existing computation, so existing snapshots are byte-identical.
Validation
794 textkit unit tests pass (783 pre-existing + 11 new covering the OS/2 fallback ladder and the Math.max line-box rule).
2246 monorepo tests pass — including renderer integration tests that rasterise to PDF — confirming no regression for fonts on the hhea fallback path.
Validated end-to-end downstream as a pnpm patch against @react-pdf/textkit@6.3.0 in amruthpillai/reactive-resume#3069 (reproducer + before/after screenshots + metrics tables in that thread).
Changeset
@react-pdf/textkit: patch.