From c2725f052c3ebc98f25aeba2d4377ab6d6a1f14c Mon Sep 17 00:00:00 2001 From: JamesGoslings <3248175240@qq.com> Date: Fri, 15 May 2026 20:17:50 +0800 Subject: [PATCH] fix(textkit): align line-box height and font metrics with browser layout MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 #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/cjk-line-box-and-typo-metrics.md | 5 ++ packages/textkit/src/run/ascent.ts | 4 +- packages/textkit/src/run/descent.ts | 4 +- packages/textkit/src/run/height.ts | 3 +- packages/textkit/src/run/lineGap.ts | 6 +- packages/textkit/src/run/typoMetrics.ts | 46 +++++++++++ packages/textkit/tests/run/ascent.test.ts | 20 +++++ packages/textkit/tests/run/descent.test.ts | 19 +++++ packages/textkit/tests/run/height.test.ts | 58 ++++++++++++++ packages/textkit/tests/run/lineGap.test.ts | 23 ++++++ .../textkit/tests/run/typoMetrics.test.ts | 76 +++++++++++++++++++ 11 files changed, 259 insertions(+), 5 deletions(-) create mode 100644 .changeset/cjk-line-box-and-typo-metrics.md create mode 100644 packages/textkit/src/run/typoMetrics.ts create mode 100644 packages/textkit/tests/run/typoMetrics.test.ts diff --git a/.changeset/cjk-line-box-and-typo-metrics.md b/.changeset/cjk-line-box-and-typo-metrics.md new file mode 100644 index 000000000..7344303db --- /dev/null +++ b/.changeset/cjk-line-box-and-typo-metrics.md @@ -0,0 +1,5 @@ +--- +"@react-pdf/textkit": patch +--- + +fix(textkit): align line-box height and font metrics with browser layout diff --git a/packages/textkit/src/run/ascent.ts b/packages/textkit/src/run/ascent.ts index 5e69b2dd9..ef354e17d 100644 --- a/packages/textkit/src/run/ascent.ts +++ b/packages/textkit/src/run/ascent.ts @@ -1,5 +1,6 @@ import { Run } from '../types'; import scale from './scale'; +import resolveTypoMetrics from './typoMetrics'; /** * Get run ascent @@ -10,7 +11,8 @@ import scale from './scale'; const ascent = (run: Run) => { const { font, attachment } = run.attributes; const attachmentHeight = attachment?.height || 0; - const fontAscent = typeof font === 'string' ? 0 : font?.[0]?.ascent || 0; + const fontAscent = + typeof font === 'string' ? 0 : resolveTypoMetrics(font?.[0]).ascent; return Math.max(attachmentHeight, fontAscent * scale(run)); }; diff --git a/packages/textkit/src/run/descent.ts b/packages/textkit/src/run/descent.ts index 10b16511a..fd1433291 100644 --- a/packages/textkit/src/run/descent.ts +++ b/packages/textkit/src/run/descent.ts @@ -1,5 +1,6 @@ import { Run } from '../types'; import scale from './scale'; +import resolveTypoMetrics from './typoMetrics'; /** * Get run descent @@ -9,7 +10,8 @@ import scale from './scale'; */ const descent = (run: Run) => { const font = run.attributes?.font; - const fontDescent = typeof font === 'string' ? 0 : font?.[0]?.descent || 0; + const fontDescent = + typeof font === 'string' ? 0 : resolveTypoMetrics(font?.[0]).descent; return scale(run) * fontDescent; }; diff --git a/packages/textkit/src/run/height.ts b/packages/textkit/src/run/height.ts index 0398f1c2b..e976e9606 100644 --- a/packages/textkit/src/run/height.ts +++ b/packages/textkit/src/run/height.ts @@ -11,7 +11,8 @@ import lineGap from './lineGap'; */ const height = (run: Run) => { const lineHeight = run.attributes?.lineHeight; - return lineHeight || lineGap(run) + ascent(run) - descent(run); + const intrinsic = lineGap(run) + ascent(run) - descent(run); + return Math.max(lineHeight || 0, intrinsic); }; export default height; diff --git a/packages/textkit/src/run/lineGap.ts b/packages/textkit/src/run/lineGap.ts index 910b7f88e..08bbb93e8 100644 --- a/packages/textkit/src/run/lineGap.ts +++ b/packages/textkit/src/run/lineGap.ts @@ -1,5 +1,6 @@ import { Run } from '../types'; import scale from './scale'; +import resolveTypoMetrics from './typoMetrics'; /** * Get run lineGap @@ -9,8 +10,9 @@ import scale from './scale'; */ const lineGap = (run: Run) => { const font = run.attributes?.font; - const lineGap = typeof font === 'string' ? 0 : font?.[0]?.lineGap || 0; - return lineGap * scale(run); + const fontLineGap = + typeof font === 'string' ? 0 : resolveTypoMetrics(font?.[0]).lineGap; + return fontLineGap * scale(run); }; export default lineGap; diff --git a/packages/textkit/src/run/typoMetrics.ts b/packages/textkit/src/run/typoMetrics.ts new file mode 100644 index 000000000..2f3cd8d19 --- /dev/null +++ b/packages/textkit/src/run/typoMetrics.ts @@ -0,0 +1,46 @@ +import { Font } from '../types'; + +type Metrics = { + ascent: number; + descent: number; + lineGap: number; +}; + +/** + * Prefer OS/2 typographic metrics over hhea, falling back when absent. + * + * @param font - Font + * @returns Metrics + */ +const resolveTypoMetrics = (font: Font | undefined): Metrics => { + const os2 = font?.['OS/2'] as + | { + typoAscender?: number; + typoDescender?: number; + typoLineGap?: number; + } + | undefined; + + if ( + !os2 || + typeof os2.typoAscender !== 'number' || + typeof os2.typoDescender !== 'number' + ) { + return { + ascent: font?.ascent || 0, + descent: font?.descent || 0, + lineGap: font?.lineGap || 0, + }; + } + + return { + ascent: os2.typoAscender, + descent: os2.typoDescender, + lineGap: + typeof os2.typoLineGap === 'number' + ? os2.typoLineGap + : font?.lineGap || 0, + }; +}; + +export default resolveTypoMetrics; diff --git a/packages/textkit/tests/run/ascent.test.ts b/packages/textkit/tests/run/ascent.test.ts index 279ac72d9..b28f534fb 100644 --- a/packages/textkit/tests/run/ascent.test.ts +++ b/packages/textkit/tests/run/ascent.test.ts @@ -80,4 +80,24 @@ describe('run ascent operator', () => { expect(ascent(run)).toBe(70); }); + + test('should prefer OS/2 typoAscender over hhea ascent', () => { + const run = { + start: 0, + end: 0, + attributes: { + fontSize: 12, + font: [ + { + ascent: 1160, + unitsPerEm: 1000, + 'OS/2': { typoAscender: 880, typoDescender: -120 }, + } as unknown as Font, + ], + }, + }; + + // 880 (typoAscender) * 12 / 1000, not 1160 * 12 / 1000. + expect(ascent(run)).toBe((880 * 12) / 1000); + }); }); diff --git a/packages/textkit/tests/run/descent.test.ts b/packages/textkit/tests/run/descent.test.ts index 5fa95dc9f..6d6eed754 100644 --- a/packages/textkit/tests/run/descent.test.ts +++ b/packages/textkit/tests/run/descent.test.ts @@ -52,4 +52,23 @@ describe('run descent operator', () => { expect(descent(run)).toBe(-(10 * 12) / 2); }); + + test('should prefer OS/2 typoDescender over hhea descent', () => { + const run = { + start: 0, + end: 0, + attributes: { + fontSize: 12, + font: [ + { + descent: -288, + unitsPerEm: 1000, + 'OS/2': { typoAscender: 880, typoDescender: -120 }, + } as unknown as Font, + ], + }, + }; + + expect(descent(run)).toBe((-120 * 12) / 1000); + }); }); diff --git a/packages/textkit/tests/run/height.test.ts b/packages/textkit/tests/run/height.test.ts index e588fcfb0..c36940566 100644 --- a/packages/textkit/tests/run/height.test.ts +++ b/packages/textkit/tests/run/height.test.ts @@ -74,4 +74,62 @@ describe('run height operator', () => { expect(height(run)).toBe(((15 + 10 + 2) * 12) / 2); }); + + test('should return user lineHeight when it exceeds the intrinsic height', () => { + const run = { + start: 0, + end: 0, + attributes: { + fontSize: 12, + lineHeight: 200, + font: [{ descent: -10, ascent: 15, lineGap: 2, unitsPerEm: 2 } as Font], + }, + }; + + // intrinsic = (15 + 10 + 2) * 12 / 2 = 162; user lineHeight = 200 wins. + expect(height(run)).toBe(200); + }); + + test('should clamp user lineHeight up to the intrinsic height (CSS line-box rule)', () => { + const run = { + start: 0, + end: 0, + attributes: { + fontSize: 12, + lineHeight: 5, + font: [{ descent: -10, ascent: 15, lineGap: 2, unitsPerEm: 2 } as Font], + }, + }; + + // intrinsic = 162 > user lineHeight = 5; line-box must grow. + expect(height(run)).toBe(((15 + 10 + 2) * 12) / 2); + }); + + test('should use OS/2 typo metrics when computing the intrinsic height', () => { + const run = { + start: 0, + end: 0, + attributes: { + fontSize: 12, + font: [ + { + // hhea inflated (typical Source Han Sans behaviour). + ascent: 1160, + descent: -288, + lineGap: 0, + unitsPerEm: 1000, + 'OS/2': { + typoAscender: 880, + typoDescender: -120, + typoLineGap: 0, + }, + } as unknown as Font, + ], + }, + }; + + // typo intrinsic = (880 - (-120) + 0) * 12 / 1000 = 12; + // hhea intrinsic would have been (1160 - (-288)) * 12 / 1000 = 17.376. + expect(height(run)).toBe(12); + }); }); diff --git a/packages/textkit/tests/run/lineGap.test.ts b/packages/textkit/tests/run/lineGap.test.ts index 563ded2b0..a2f270974 100644 --- a/packages/textkit/tests/run/lineGap.test.ts +++ b/packages/textkit/tests/run/lineGap.test.ts @@ -54,4 +54,27 @@ describe('run lineGap operator', () => { expect(lineGap(run)).toBe((10 * 12) / 2); }); + + test('should prefer OS/2 typoLineGap over hhea lineGap', () => { + const run = { + start: 0, + end: 0, + attributes: { + fontSize: 12, + font: [ + { + lineGap: 200, + unitsPerEm: 1000, + 'OS/2': { + typoAscender: 800, + typoDescender: -200, + typoLineGap: 0, + }, + } as unknown as Font, + ], + }, + }; + + expect(lineGap(run)).toBe(0); + }); }); diff --git a/packages/textkit/tests/run/typoMetrics.test.ts b/packages/textkit/tests/run/typoMetrics.test.ts new file mode 100644 index 000000000..d308ad520 --- /dev/null +++ b/packages/textkit/tests/run/typoMetrics.test.ts @@ -0,0 +1,76 @@ +import { describe, expect, test } from 'vitest'; + +import resolveTypoMetrics from '../../src/run/typoMetrics'; +import { Font } from '../../src/types'; + +describe('run typoMetrics helper', () => { + test('returns zeroed metrics for an undefined font', () => { + expect(resolveTypoMetrics(undefined)).toEqual({ + ascent: 0, + descent: 0, + lineGap: 0, + }); + }); + + test('falls back to hhea metrics when no OS/2 table is present', () => { + const font = { ascent: 800, descent: -200, lineGap: 0 } as Font; + + expect(resolveTypoMetrics(font)).toEqual({ + ascent: 800, + descent: -200, + lineGap: 0, + }); + }); + + test('falls back to hhea when OS/2 lacks numeric typo metrics', () => { + const font = { + ascent: 800, + descent: -200, + lineGap: 0, + 'OS/2': {} as never, + } as Font; + + expect(resolveTypoMetrics(font)).toEqual({ + ascent: 800, + descent: -200, + lineGap: 0, + }); + }); + + test('prefers OS/2 typo metrics when fully populated', () => { + const font = { + ascent: 1160, + descent: -288, + lineGap: 0, + 'OS/2': { + typoAscender: 880, + typoDescender: -120, + typoLineGap: 0, + } as never, + } as Font; + + expect(resolveTypoMetrics(font)).toEqual({ + ascent: 880, + descent: -120, + lineGap: 0, + }); + }); + + test('falls back to hhea lineGap when OS/2 typoLineGap is missing', () => { + const font = { + ascent: 1000, + descent: -200, + lineGap: 50, + 'OS/2': { + typoAscender: 800, + typoDescender: -150, + } as never, + } as Font; + + expect(resolveTypoMetrics(font)).toEqual({ + ascent: 800, + descent: -150, + lineGap: 50, + }); + }); +});