diff --git a/packages/layout-engine/layout-engine/src/index.test.ts b/packages/layout-engine/layout-engine/src/index.test.ts index f6e3f4fe97..17a987c358 100644 --- a/packages/layout-engine/layout-engine/src/index.test.ts +++ b/packages/layout-engine/layout-engine/src/index.test.ts @@ -2458,6 +2458,48 @@ describe('layoutDocument', () => { expect(layout.pages[1].fragments[0].blockId).toBe('p2'); }); + it('skips empty sectPr marker paragraph before continuous section break (SD-2735)', () => { + // IT-945 shape: "This is my first section" + empty sectPr-marker + + // continuous break into 2-col section. The marker is logically a + // section-properties container, not a visible paragraph. Rendering it + // at docDefaults height would cost a row per column on page 1. + const blocks: FlowBlock[] = [ + { kind: 'paragraph', id: 'p1', runs: [{ text: 'First section', fontFamily: 'Arial', fontSize: 12 }] }, + { + kind: 'paragraph', + id: 'p-marker', + runs: [{ text: '', fontFamily: 'Arial', fontSize: 12 }], + attrs: { sectPrMarker: true }, + }, + { + kind: 'sectionBreak', + id: 'sb1', + type: 'continuous', + margins: {}, + pageSize: { w: 612, h: 792 }, + columns: { count: 2, gap: 36 }, + attrs: { source: 'sectPr' }, + }, + { kind: 'paragraph', id: 'p2', runs: [{ text: 'In 2-col section', fontFamily: 'Arial', fontSize: 12 }] }, + ]; + const measures: Measure[] = [ + { kind: 'paragraph', lines: [makeLine(20)], totalHeight: 20 }, + { kind: 'paragraph', lines: [makeLine(16)], totalHeight: 16 }, + { kind: 'sectionBreak' }, + { kind: 'paragraph', lines: [makeLine(20)], totalHeight: 20 }, + ]; + + const layout = layoutDocument(blocks, measures, { + pageSize: { w: 612, h: 792 }, + margins: { top: 72, right: 72, bottom: 72, left: 72 }, + }); + + // Both visible paragraphs land on page 1; the marker does not appear. + expect(layout.pages).toHaveLength(1); + const fragmentIds = layout.pages[0].fragments.map((f) => f.blockId); + expect(fragmentIds).toEqual(['p1', 'p2']); + }); + it('skips empty sectPr marker paragraph before forced section break', () => { const blocks: FlowBlock[] = [ { kind: 'paragraph', id: 'p1', runs: [{ text: 'Content', fontFamily: 'Arial', fontSize: 12 }] }, diff --git a/packages/layout-engine/layout-engine/src/index.ts b/packages/layout-engine/layout-engine/src/index.ts index c6d5e91903..17997dab08 100644 --- a/packages/layout-engine/layout-engine/src/index.ts +++ b/packages/layout-engine/layout-engine/src/index.ts @@ -2170,25 +2170,23 @@ export function layoutDocument(blocks: FlowBlock[], measures: Measure[], options if (isEmpty) { const isSectPrMarker = paraBlock.attrs?.sectPrMarker === true; - // Check if previous block was pageBreak and next block is sectionBreak const prevBlock = index > 0 ? blocks[index - 1] : null; const nextBlock = index < blocks.length - 1 ? blocks[index + 1] : null; - - const nextSectionBreak = nextBlock?.kind === 'sectionBreak' ? (nextBlock as SectionBreakBlock) : null; - const nextBreakType = - nextSectionBreak?.type ?? (nextSectionBreak?.attrs?.source === 'sectPr' ? 'nextPage' : undefined); - const nextBreakForcesPage = - nextSectionBreak && - (nextBreakType === 'nextPage' || - nextBreakType === 'evenPage' || - nextBreakType === 'oddPage' || - nextSectionBreak.attrs?.requirePageBoundary === true); - - if (isSectPrMarker && nextBreakForcesPage) { + const nextIsSectionBreak = nextBlock?.kind === 'sectionBreak'; + + // A section-break marker paragraph (`` with + // no runs) carries only section metadata in Word — it's not a visible + // paragraph. Skip it regardless of break type (nextPage/continuous/ + // evenPage/oddPage) so intro spacing matches Word for all section + // transitions. Per ECMA-376 §17.6.17 the sectPr defines where the + // section ends, not a rendered paragraph. + if (isSectPrMarker && nextIsSectionBreak) { continue; } - if (prevBlock?.kind === 'pageBreak' && nextBlock?.kind === 'sectionBreak') { + // Pre-sectPrMarker path: empty paragraph sandwiched between an + // explicit page break and a section break — same "pure marker" shape. + if (prevBlock?.kind === 'pageBreak' && nextIsSectionBreak) { continue; } } diff --git a/packages/layout-engine/measuring/dom/src/fontMetricsCache.test.ts b/packages/layout-engine/measuring/dom/src/fontMetricsCache.test.ts index d9aec22840..8f146b9f03 100644 --- a/packages/layout-engine/measuring/dom/src/fontMetricsCache.test.ts +++ b/packages/layout-engine/measuring/dom/src/fontMetricsCache.test.ts @@ -359,4 +359,48 @@ describe('fontMetricsCache', () => { expect(metrics.ascent).toBeGreaterThan(metrics.descent); }); }); + + describe('naturalSingleLine calibration (SD-2735)', () => { + // Per ECMA-376 §17.18.48, spacing multipliers are relative to the font's + // "natural single line height". JSDOM's Canvas implementation cannot read + // the actual ascent/descent from Microsoft's Aptos/Calibri font files, so + // we calibrate these fonts by measured-vs-Word ratio. + + it('populates naturalSingleLine for Aptos at 12pt matching Word (~19.5px)', () => { + const metrics = getFontMetrics(ctx, { fontFamily: 'Aptos', fontSize: 16 }, 'browser', defaultFonts); + expect(metrics.naturalSingleLine).toBeDefined(); + // 16px × 1.218 = 19.488 — Word renders Aptos 12pt at ~19.5px + expect(metrics.naturalSingleLine!).toBeCloseTo(19.488, 1); + }); + + it('populates naturalSingleLine for Calibri at 12pt (~19.5px)', () => { + const metrics = getFontMetrics(ctx, { fontFamily: 'Calibri', fontSize: 16 }, 'browser', defaultFonts); + expect(metrics.naturalSingleLine).toBeDefined(); + expect(metrics.naturalSingleLine!).toBeCloseTo(19.504, 1); + }); + + it('falls back to canvas measurement for fonts without calibration', () => { + const metrics = getFontMetrics(ctx, { fontFamily: 'Arial', fontSize: 16 }, 'browser', defaultFonts); + // Arial has no entry in the calibration table; naturalSingleLine may + // still come through from fontBoundingBox* when the canvas provides it. + // Either a canvas-sourced value or undefined is acceptable here. + if (metrics.naturalSingleLine != null) { + expect(metrics.naturalSingleLine).toBeGreaterThan(0); + expect(metrics.naturalSingleLine).toBeLessThan(30); + } + }); + + it('scales naturalSingleLine linearly with font size for Aptos', () => { + const m12 = getFontMetrics(ctx, { fontFamily: 'Aptos', fontSize: 16 }, 'browser', defaultFonts); + const m24 = getFontMetrics(ctx, { fontFamily: 'Aptos', fontSize: 32 }, 'browser', defaultFonts); + // Calibration is a pure multiplier: doubling fontSize doubles naturalSingle. + expect(m24.naturalSingleLine! / m12.naturalSingleLine!).toBeCloseTo(2, 2); + }); + + it('matches Aptos Display and Aptos to the same calibration', () => { + const m1 = getFontMetrics(ctx, { fontFamily: 'Aptos', fontSize: 16 }, 'browser', defaultFonts); + const m2 = getFontMetrics(ctx, { fontFamily: 'Aptos Display', fontSize: 16 }, 'browser', defaultFonts); + expect(m1.naturalSingleLine).toBeCloseTo(m2.naturalSingleLine!, 2); + }); + }); }); diff --git a/packages/layout-engine/measuring/dom/src/fontMetricsCache.ts b/packages/layout-engine/measuring/dom/src/fontMetricsCache.ts index 1c8aa75f60..5ebec84c41 100644 --- a/packages/layout-engine/measuring/dom/src/fontMetricsCache.ts +++ b/packages/layout-engine/measuring/dom/src/fontMetricsCache.ts @@ -23,10 +23,54 @@ export type FontInfo = { /** * Measured font metrics from the Canvas API. + * + * `ascent`/`descent` are painted bounds from `actualBoundingBox*` (used for + * glyph-clipping guards). `naturalSingleLine` is the font's intrinsic + * single-line height used by `w:lineRule="auto"` — from the calibration + * table when present, else from `fontBoundingBox*` when the browser exposes + * it. Undefined signals "caller pick a fallback". */ export type FontMetricsResult = { ascent: number; descent: number; + naturalSingleLine?: number; +}; + +/** + * Natural-single-line multipliers for fonts whose Word-rendered metrics + * differ from what the browser Canvas can measure. The browser does not have + * access to Microsoft's internal font metrics unless the font is bundled, + * so Aptos/Calibri/Cambria must be calibrated against Word's actual output. + * + * Keyed by primary family name lowercased. Add entries when corpus evidence + * shows a systematic gap between Canvas measurement and Word. + * + * Transitional empirical data. The table should move to a fixture-backed + * FontMetricsProvider (generated from Word-native fixtures with provenance + * and a drift test) — tracked in SD-3296. Do not grow this table + * indefinitely; once the provider lands, retire it. + */ +const FONT_NATURAL_LINE_CALIBRATION: Record = { + aptos: 1.218, + 'aptos display': 1.218, + calibri: 1.219, + 'calibri light': 1.219, + cambria: 1.219, +}; + +const primaryFontFamily = (fontFamily: string): string => { + if (typeof fontFamily !== 'string') return ''; + const first = fontFamily.split(',')[0] ?? ''; + return first + .trim() + .replace(/^['"]|['"]$/g, '') + .toLowerCase(); +}; + +export const getCalibratedNaturalSingleLine = (fontFamily: string, fontSizePx: number): number | undefined => { + const multiplier = FONT_NATURAL_LINE_CALIBRATION[primaryFontFamily(fontFamily)]; + if (multiplier == null || !Number.isFinite(fontSizePx) || fontSizePx <= 0) return undefined; + return fontSizePx * multiplier; }; /** @@ -219,7 +263,19 @@ export function getFontMetrics( descent = fontInfo.fontSize * 0.2; } - const result: FontMetricsResult = { ascent, descent }; + // Prefer explicit Word-calibration; fall back to the browser's + // fontBoundingBox* when it's exposed; otherwise leave undefined. + let naturalSingleLine = getCalibratedNaturalSingleLine(fontInfo.fontFamily, fontInfo.fontSize); + if ( + naturalSingleLine == null && + typeof textMetrics.fontBoundingBoxAscent === 'number' && + typeof textMetrics.fontBoundingBoxDescent === 'number' && + textMetrics.fontBoundingBoxAscent > 0 + ) { + naturalSingleLine = textMetrics.fontBoundingBoxAscent + textMetrics.fontBoundingBoxDescent; + } + + const result: FontMetricsResult = { ascent, descent, naturalSingleLine }; // Cache the result (with size limit) if (fontMetricsCache.size >= MAX_CACHE_SIZE) { diff --git a/packages/layout-engine/measuring/dom/src/index.test.ts b/packages/layout-engine/measuring/dom/src/index.test.ts index a2a0ee80f6..b31e7b319e 100644 --- a/packages/layout-engine/measuring/dom/src/index.test.ts +++ b/packages/layout-engine/measuring/dom/src/index.test.ts @@ -1002,9 +1002,13 @@ describe('measureBlock', () => { }; const measure = expectParagraphMeasure(await measureBlock(block, 400)); - // `lineUnit: "multiplier"` applies directly to fontSize. - // (pm-adapter already bakes the OOXML auto 1.15 factor into the multiplier value.) - expect(measure.lines[0].lineHeight).toBeCloseTo(1.5 * fontSize, 1); + // `lineUnit: "multiplier"` now applies to naturalSingleLine per + // ECMA-376 §17.18.48 auto rule `(line/240) × naturalSingle`. For fonts + // without explicit calibration, naturalSingle falls back to + // WORD_SINGLE_LINE_SPACING_MULTIPLIER × fontSize = 1.15 × 16 = 18.4. + // → 1.5 × 18.4 = 27.6. + const baseNaturalSingle = 1.15 * fontSize; // 18.4 fallback for Arial + expect(measure.lines[0].lineHeight).toBeCloseTo(1.5 * baseNaturalSingle, 1); }); it('applies higher auto multipliers to the baseline line height', async () => { @@ -1025,7 +1029,10 @@ describe('measureBlock', () => { }; const measure = expectParagraphMeasure(await measureBlock(block, 400)); - expect(measure.lines[0].lineHeight).toBeCloseTo(2 * fontSize, 1); + // ECMA-376 §17.18.48 auto rule: multiplier × naturalSingle (not fontSize). + // For uncalibrated Arial, naturalSingle falls back to 1.15 × fontSize. + const baseNaturalSingle = 1.15 * fontSize; // 18.4 + expect(measure.lines[0].lineHeight).toBeCloseTo(2 * baseNaturalSingle, 1); }); it('applies large auto values as multipliers', async () => { @@ -1045,7 +1052,8 @@ describe('measureBlock', () => { }; const measure = expectParagraphMeasure(await measureBlock(block, 400)); - expect(measure.lines[0].lineHeight).toBeCloseTo(42 * 16, 1); + // 42 × naturalSingle (fallback 1.15 × 16 = 18.4) = 772.8. + expect(measure.lines[0].lineHeight).toBeCloseTo(42 * 1.15 * 16, 1); }); it('does not clamp line height for very small fonts', async () => { @@ -1135,6 +1143,145 @@ describe('measureBlock', () => { expect(measure.lines[0].lineHeight).toBeCloseTo(baseLineHeight, 1); }); + it('honours auto lineRule with a sub-baseline multiplier (no silent min-clamp) per ECMA-376 §17.18.48', async () => { + // SD-2735 regression: `auto` explicitly has "no predetermined minimum + // or maximum" per spec. A multiplier of 0.5 should tighten lines, + // not be silently inflated to the baseline 1.15 × fontSize as the + // pre-fix code did (it reused the atLeast max-clamp branch for auto). + const fontSize = 16; + const block: FlowBlock = { + kind: 'paragraph', + id: 'tight-spacing', + runs: [ + { + text: 'Tight auto spacing', + fontFamily: 'Arial', + fontSize, + }, + ], + attrs: { + spacing: { line: 0.5, lineUnit: 'multiplier', lineRule: 'auto' }, + }, + }; + + const measure = expectParagraphMeasure(await measureBlock(block, 400)); + // 0.5 × naturalSingle (= 0.5 × 18.4 = 9.2 for uncalibrated Arial). + // Previously the `auto` path max-clamped this up to 18.4, eating the + // author's explicit half-spacing intent. After the §17.18.48 rule + // fix the multiplier applies cleanly, even if the result is still + // smaller than the font's cap height (caller is responsible for + // glyph-clipping guards elsewhere). + const baseNaturalSingle = 1.15 * fontSize; // 18.4 fallback + expect(measure.lines[0].lineHeight).toBeCloseTo(0.5 * baseNaturalSingle, 1); + }); + + it('applies multiplier × naturalSingle to empty paragraphs in calibrated fonts (SD-2735)', async () => { + // Empty calibrated-font cell paragraph (Aptos): `naturalSingleLine` + // must thread through `calculateEmptyParagraphMetrics` so empty cells + // match the height of text-bearing cells in the same table. For + // uncalibrated fonts the bug hides because naturalSingle ≈ + // WORD_SINGLE_LINE_SPACING_MULTIPLIER × fontSize (18.4 px at 16 px) + // happens to match the fallback floor inside `resolveLineHeight`. + // For Aptos (calibrated to ~19.5 px at 16 px), the gap surfaces: + // empty rows would measure 1.15 × fontSize instead of the calibrated + // value, rendering shorter than text-bearing cells. Symmetric with + // `calculateTypographyMetrics`: populate naturalSingle from the + // calibration table when fontInfo is present, and forward it to + // resolveLineHeight. + const fontSize = 16; // 12 pt + const block: FlowBlock = { + kind: 'paragraph', + id: 'empty-aptos', + runs: [ + { + text: '', + fontFamily: 'Aptos', + fontSize, + }, + ], + attrs: { + spacing: { line: 1.5, lineUnit: 'multiplier', lineRule: 'auto' }, + }, + }; + + const measure = expectParagraphMeasure(await measureBlock(block, 400)); + // Aptos calibrated naturalSingle ≈ 19.488 px (see fontMetricsCache.test.ts). + // 1.5 × 19.488 ≈ 29.2. + const aptosNaturalSingle = 19.488; + expect(measure.lines[0].lineHeight).toBeCloseTo(1.5 * aptosNaturalSingle, 0); + }); + + it('threads naturalSingle through drop-cap height for calibrated fonts (SD-2735)', async () => { + // SD-2735 follow-up: `measureDropCap` previously called + // `resolveLineHeight` without `naturalSingleLine`, so drop caps for + // calibrated fonts used the 1.15 × fontSize fallback floor rather than + // the calibrated single-line height. This rendered drop caps shorter + // than the body lines they span. After plumbing naturalSingle through, + // drop-cap height scales off the same intrinsic line height as the + // surrounding paragraph. + const fontSize = 16; // 12 pt + const dropCapDescriptor = { + mode: 'drop' as const, + lines: 3, + run: { + text: 'A', + fontFamily: 'Aptos', + fontSize, + }, + }; + const block: FlowBlock = { + kind: 'paragraph', + id: 'drop-cap-aptos', + runs: [ + { + text: 'The rest of the paragraph.', + fontFamily: 'Aptos', + fontSize, + }, + ], + attrs: { + spacing: { line: 1.5, lineUnit: 'multiplier', lineRule: 'auto' }, + dropCapDescriptor, + }, + }; + + const measure = expectParagraphMeasure(await measureBlock(block, 400)); + // Aptos calibrated naturalSingle ≈ 19.488 px. With line=1.5 / auto and + // lines=3: expected drop-cap height ≈ 3 × 1.5 × 19.488 ≈ 87.7. The + // pre-fix code returned 3 × 1.5 × 18.4 ≈ 82.8 (1.15 × fontSize floor). + const aptosNaturalSingle = 19.488; + expect(measure.dropCap?.height).toBeCloseTo(3 * 1.5 * aptosNaturalSingle, 0); + }); + + it('expands atLeast line height to fit a larger run in a mixed-size paragraph (SD-2735)', async () => { + // SD-2735 follow-up: `lineRule="atLeast"` per ECMA-376 §17.18.48 must + // expand to fit content when the content's natural line height + // exceeds the `line` floor. With mixed font sizes on a single line, + // `line.maxFontInfo` should drive `naturalSingle` to the larger run's + // metrics so `resolveLineHeight` returns the larger value rather than + // the floor. Anchors the invariant against future refactors of the + // maxFontInfo plumbing. + const block: FlowBlock = { + kind: 'paragraph', + id: 'mixed-atleast', + runs: [ + { text: 'small ', fontFamily: 'Arial', fontSize: 16 }, // 12 pt + { text: 'BIG', fontFamily: 'Arial', fontSize: 48 }, // 36 pt + ], + attrs: { + // 18 px floor — well below the 36 pt run's natural single-line + // height (≈ 48 × 1.15 = 55.2 px for uncalibrated Arial). + spacing: { line: 18, lineUnit: 'px', lineRule: 'atLeast' }, + }, + }; + + const measure = expectParagraphMeasure(await measureBlock(block, 800)); + // Expect the line to expand to the larger run's natural single line, + // not the 18 px floor. + expect(measure.lines[0].lineHeight).toBeGreaterThan(50); + expect(measure.lines[0].lineHeight).toBeLessThan(60); + }); + it('ensures line height is never smaller than glyph bounds to prevent clipping', async () => { // This test verifies the clamp: Math.max(fontSize * 1.15, ascent + descent) // For any font, line height must be >= ascent + descent to prevent glyph overlap diff --git a/packages/layout-engine/measuring/dom/src/index.ts b/packages/layout-engine/measuring/dom/src/index.ts index 39e85716cf..6d8860cc24 100644 --- a/packages/layout-engine/measuring/dom/src/index.ts +++ b/packages/layout-engine/measuring/dom/src/index.ts @@ -9,14 +9,19 @@ * - Calculate typography metrics (ascent, descent, lineHeight) * - Return Measure with positioned line boundaries * - * Typography Approximations (v0.1.0): - * - ascent ≈ fontSize * 0.8 (baseline to top) - * - descent ≈ fontSize * 0.2 (baseline to bottom) - * - lineHeight = fontSize * 1.15 (Word 2007+ "single" line spacing) - * - empty paragraphs use fontSize as the base line height + * Typography model (post-SD-2735, ECMA-376 §17.18.48): + * - ascent / descent come from Canvas `actualBoundingBox*` when fontInfo + * is available; fall back to fontSize × 0.8 / 0.2 only when it is not. + * - lineHeight for `w:lineRule="auto"` is `line × naturalSingleLine`. The + * spec leaves "a line" undefined; we source it from `fontMetricsCache` + * (calibrated value for fonts the browser cannot measure — Aptos, + * Calibri, Cambria — else `fontBoundingBox*`). `naturalSingleLine` is + * floored at 1.15 × fontSize as a Word 2007+ Normal-style compatibility + * choice, not a spec minimum. + * - empty paragraphs follow the same path via `calculateEmptyParagraphMetrics`. * - * These are documented heuristics; we can swap in precise font metrics later - * if needed via libraries like opentype.js. + * The calibration table is transitional empirical data, tracked for + * migration to a fixture-backed FontMetricsProvider in SD-3296. * * Line Breaking Strategy: * - Greedy algorithm: accumulate words until exceeding maxWidth @@ -134,12 +139,6 @@ export function configureMeasurement(options: Partial): void export { clearMeasurementCache }; -/** - * Future: Font-specific calibration factors could be added here if Canvas measurements - * consistently diverge from MS Word after all precision fixes (bounding box, fractional pt→px, etc.) - * are applied. Currently not needed. - */ - /** * Global canvas context cache for text measurement * Reused across calls to avoid repeated canvas creation @@ -355,15 +354,12 @@ function measureText( */ /** - * Word 2007+ default line spacing multiplier for "single" line spacing. - * - * Microsoft Word changed its line spacing algorithm in Word 2007 to use 1.15× font size - * as the baseline for "single" line spacing, rather than just using ascent + descent. - * This provides more breathing room and matches the industry standard for readability. - * - * For example, 12pt (16px) font: 16 × 1.15 = 18.4px line height. - * - * Reference: Word 2007+ line spacing behavior + * Word 2007+ "single" line spacing multiplier, used as a Word-fidelity floor + * on `naturalSingleLine` in `resolveLineHeight`. ECMA-376 §17.18.48 leaves + * "a line" undefined for `lineRule="auto"`, so this floor is a compatibility + * choice (Word's Normal style assumes 1.15 × fontSize), not a spec minimum. + * It guards against fonts whose `fontBoundingBox*` reports a tighter natural + * line than Word would render. */ const WORD_SINGLE_LINE_SPACING_MULTIPLIER = 1.15; @@ -383,13 +379,15 @@ const WORD_SINGLE_LINE_SPACING_MULTIPLIER = 1.15; * - Browser doesn't support actualBoundingBox* metrics (legacy browsers) * * **Line Height Calculation:** - * Uses Word 2007+'s default "single" line spacing of fontSize × 1.15 as the base. - * This base is then modified by paragraph spacing rules (lineRule: auto/exact/atLeast). - * A minimum line height clamp is intentionally NOT enforced to match Word's behavior - * for small font sizes and empty paragraphs. + * Resolved via `resolveLineHeight` per ECMA-376 §17.18.48: + * - `auto`: `line × naturalSingleLine` + * - `atLeast`: `max(line, naturalSingleLine)` + * - `exact`: `line` (clipping is the painter's responsibility, see SD-3297) * - * The 1.15 multiplier provides consistent spacing that matches Word's behavior and - * accounts for the line gap that Canvas TextMetrics doesn't expose directly. + * `naturalSingleLine` comes from `fontMetricsCache` — the calibrated value + * for fonts the browser cannot measure (Aptos, Calibri, Cambria), else + * `fontBoundingBox*`. It is floored at 1.15 × fontSize as a Word-fidelity + * choice (see `WORD_SINGLE_LINE_SPACING_MULTIPLIER`). * * @param fontSize - The font size in pixels * @param spacing - Optional paragraph spacing configuration (lineRule, line value) @@ -397,19 +395,19 @@ const WORD_SINGLE_LINE_SPACING_MULTIPLIER = 1.15; * @returns Object containing ascent, descent, and lineHeight in pixels * * @example - * // Basic usage with 16px font + * // 16 px Arial (uncalibrated), no spacing — naturalSingle floors at 1.15 × 16 = 18.4 * const metrics = calculateTypographyMetrics(16); - * // Returns: { ascent: ~12.8, descent: ~3.2, lineHeight: 18.4 } + * // Returns: { ascent: ~12.8, descent: ~3.2, lineHeight: ~18.4 } * * @example - * // With 1.5 line spacing multiplier - * const metrics = calculateTypographyMetrics(16, { line: 1.5, lineRule: 'auto' }); - * // Returns: { ascent: ~12.8, descent: ~3.2, lineHeight: 27.6 } // 16 × 1.15 × 1.5 + * // 16 px Aptos (calibrated to 1.218 × fontSize), `line=1.5 auto` + * const metrics = calculateTypographyMetrics(16, { line: 1.5, lineUnit: 'multiplier', lineRule: 'auto' }, aptosFontInfo); + * // Returns: { lineHeight: ~29.2 } // 1.5 × (1.218 × 16) = 1.5 × 19.488 * * @example - * // With exact line height override - * const metrics = calculateTypographyMetrics(16, { line: 24, lineRule: 'exact' }); - * // Returns: { ascent: ~12.8, descent: ~3.2, lineHeight: 24 } + * // Exact line height override + * const metrics = calculateTypographyMetrics(16, { line: 24, lineUnit: 'px', lineRule: 'exact' }); + * // Returns: { lineHeight: 24 } */ function calculateTypographyMetrics( fontSize: number, @@ -423,6 +421,12 @@ function calculateTypographyMetrics( const resolvedFontSize = normalizeFontSize(fontSize); let ascent: number; let descent: number; + // `naturalSingle` is the font's intrinsic single-line-height — what Word + // uses as the base for `w:lineRule="auto"` rendering. Per-font calibration + // table in fontMetricsCache populates this for fonts where Word's intrinsic + // metrics diverge from the browser's (Aptos, Calibri). Absent calibration, + // we fall back to ascent + descent below. + let naturalSingle: number | undefined; if ( fontInfo && @@ -435,13 +439,14 @@ function calculateTypographyMetrics( const metrics = getFontMetrics(ctx, fontInfo, measurementConfig.mode, measurementConfig.fonts); ascent = roundValue(metrics.ascent); descent = roundValue(metrics.descent); + naturalSingle = metrics.naturalSingleLine; } else { // Fallback approximations for empty paragraphs or missing font info ascent = roundValue(resolvedFontSize * 0.8); descent = roundValue(resolvedFontSize * 0.2); } - const lineHeight = resolveLineHeight(spacing, fontSize, ascent + descent); + const lineHeight = resolveLineHeight(spacing, fontSize, naturalSingle ?? ascent + descent); return { ascent, @@ -502,6 +507,13 @@ function calculateEmptyParagraphMetrics( const resolvedFontSize = normalizeFontSize(fontSize); let ascent: number; let descent: number; + // Symmetric with `calculateTypographyMetrics`: when fontInfo is present, + // forward the calibrated `naturalSingleLine` so `resolveLineHeight` scales + // a multiplier off Word's intrinsic single-line height (Aptos/Calibri etc.) + // instead of the WORD_SINGLE_LINE_SPACING_MULTIPLIER × fontSize fallback. + // Otherwise empty cells render shorter than text-bearing cells under the + // same auto multiplier. + let naturalSingle: number | undefined; if ( fontInfo && @@ -513,14 +525,15 @@ function calculateEmptyParagraphMetrics( const metrics = getFontMetrics(ctx, fontInfo, measurementConfig.mode, measurementConfig.fonts); ascent = roundValue(metrics.ascent); descent = roundValue(metrics.descent); + naturalSingle = metrics.naturalSingleLine; } else { ascent = roundValue(resolvedFontSize * 0.8); descent = roundValue(resolvedFontSize * 0.2); } // Word treats empty paragraphs as a single font-sized line unless line spacing is explicitly set. - const maxLineHeight = Math.max(resolvedFontSize, ascent + descent); - const lineHeight = roundValue(resolveLineHeight(spacing, resolvedFontSize, maxLineHeight)); + const fallbackNaturalSingle = Math.max(resolvedFontSize, ascent + descent); + const lineHeight = roundValue(resolveLineHeight(spacing, resolvedFontSize, naturalSingle ?? fallbackNaturalSingle)); return { ascent, @@ -3543,17 +3556,38 @@ const appendSegment = ( }); }; -const resolveLineHeight = (spacing: ParagraphSpacing | undefined, fontSize: number, maxHeight: number = -1): number => { - let computedHeight = spacing?.line ?? WORD_SINGLE_LINE_SPACING_MULTIPLIER; - if (spacing?.lineUnit === 'multiplier') { - computedHeight = computedHeight * fontSize; - } +/** + * Resolve paragraph line height per ECMA-376 §17.18.48 (ST_LineSpacingRule). + * + * | lineRule | Result | + * |----------|---------------------------------------------------------| + * | auto | `target` — no min, no max | + * | atLeast | `max(target, naturalSingle)` | + * | exact | `target` (paint-time clipping tracked in SD-3297) | + * + * For `lineUnit='multiplier'`, `target = line × naturalSingle` — the adapter + * has already divided `w:line` by 240, so `line` is the dimensionless ratio. + * + * `naturalSingleLine` is floored at `WORD_SINGLE_LINE_SPACING_MULTIPLIER × + * fontSize` because many text fonts measure smaller than Word's Normal-style + * baseline; without the floor, unspaced paragraphs would render tighter than + * users expect. + */ +const resolveLineHeight = ( + spacing: ParagraphSpacing | undefined, + fontSize: number, + naturalSingleLine: number = -1, +): number => { + const fallbackFloor = WORD_SINGLE_LINE_SPACING_MULTIPLIER * fontSize; + const naturalSingle = naturalSingleLine > 0 ? Math.max(naturalSingleLine, fallbackFloor) : fallbackFloor; - const lineRule = spacing?.lineRule ?? 'auto'; - if (['atLeast', 'auto'].includes(lineRule)) { - return Math.max(computedHeight, maxHeight, WORD_SINGLE_LINE_SPACING_MULTIPLIER * fontSize); - } - return computedHeight; + if (!spacing || spacing.line == null) return naturalSingle; + + const target = spacing.lineUnit === 'multiplier' ? spacing.line * naturalSingle : spacing.line; + const rule = spacing.lineRule ?? 'auto'; + + if (rule === 'atLeast') return Math.max(target, naturalSingle); + return target; }; const sanitizePositive = (value: number | undefined): number => @@ -3618,9 +3652,21 @@ const measureDropCap = ( // Add padding for spacing between drop cap and text const width = roundValue(textWidth + DROP_CAP_PADDING_PX); - // Calculate height based on the number of lines the drop cap should span - // This uses the base line height calculation from the paragraph's spacing - const lineHeight = resolveLineHeight(spacing, run.fontSize); + // Calculate height based on the number of lines the drop cap should span. + // Thread the drop-cap font's `naturalSingleLine` through `resolveLineHeight` + // (matching `calculateTypographyMetrics`) so calibrated fonts (Aptos, + // Calibri) size drop caps off the same intrinsic line height as the body + // lines they span. Without this, drop caps fell back to the 1.15 × fontSize + // floor and rendered shorter than the calibrated body. + const dropCapFontInfo: FontInfo = { + fontFamily: normalizeFontFamily(run.fontFamily), + fontSize: normalizeFontSize(run.fontSize), + bold: run.bold, + italic: run.italic, + }; + const dropCapMetrics = getFontMetrics(ctx, dropCapFontInfo, measurementConfig.mode, measurementConfig.fonts); + const naturalSingle = dropCapMetrics.naturalSingleLine ?? dropCapMetrics.ascent + dropCapMetrics.descent; + const lineHeight = resolveLineHeight(spacing, dropCapFontInfo.fontSize, naturalSingle); const height = roundValue(lineHeight * lines); return { diff --git a/packages/layout-engine/pm-adapter/src/attributes/paragraph.ts b/packages/layout-engine/pm-adapter/src/attributes/paragraph.ts index d7dd40ce57..5631b0b1d4 100644 --- a/packages/layout-engine/pm-adapter/src/attributes/paragraph.ts +++ b/packages/layout-engine/pm-adapter/src/attributes/paragraph.ts @@ -14,7 +14,8 @@ import { type ParagraphIndent, type DropCapDescriptor, type DropCapRun, - type ParagraphFrame, ParagraphDirectionContext + type ParagraphFrame, + ParagraphDirectionContext, } from '@superdoc/contracts'; import type { PMNode, ParagraphFont } from '../types.js'; import type { ResolvedRunProperties } from '@superdoc/word-layout'; diff --git a/packages/layout-engine/pm-adapter/src/attributes/spacing-indent.test.ts b/packages/layout-engine/pm-adapter/src/attributes/spacing-indent.test.ts index 0ad57727a7..4e9b804a59 100644 --- a/packages/layout-engine/pm-adapter/src/attributes/spacing-indent.test.ts +++ b/packages/layout-engine/pm-adapter/src/attributes/spacing-indent.test.ts @@ -45,7 +45,12 @@ describe('normalizeParagraphSpacing', () => { it('converts auto line values > 10 from 240ths of a line', () => { const spacing = { line: 360, lineRule: 'auto' as const } as ParagraphSpacing; // 1.5x const result = normalizeParagraphSpacing(spacing, false); - expect(result?.line).toBeCloseTo(1.725, 5); + // Per ECMA-376 §17.18.48: w:line=360 with lineRule=auto means 360/240 = 1.5 + // multiplier of natural single-line height. The pre-fix code multiplied by an + // extra 1.15 so measuring-dom (which used to do mult × fontSize) coincidentally + // matched naturalSingle. Now that measuring-dom does mult × naturalSingle, the + // 1.15 fudge would double-apply. + expect(result?.line).toBeCloseTo(1.5, 5); expect(result?.lineRule).toBe('auto'); }); @@ -83,7 +88,10 @@ describe('normalizeParagraphSpacing', () => { it('returns undefined for empty or invalid inputs', () => { expect(normalizeParagraphSpacing(undefined, false)).toBeUndefined(); expect(normalizeParagraphSpacing(null as never, false)).toBeUndefined(); - expect(normalizeParagraphSpacing({} as ParagraphSpacing, false)).toEqual({ line: 1.15, lineUnit: 'multiplier' }); + // Empty spacing → single-spacing (1.0 × naturalSingle) per spec. The pre-fix + // default of 1.15 was load-bearing only because measuring-dom didn't have the + // naturalSingle concept; with the §17.18.48 model the canonical default is 1.0. + expect(normalizeParagraphSpacing({} as ParagraphSpacing, false)).toEqual({ line: 1.0, lineUnit: 'multiplier' }); }); it('skips non-numeric values but preserves valid ones', () => { diff --git a/packages/layout-engine/pm-adapter/src/attributes/spacing-indent.ts b/packages/layout-engine/pm-adapter/src/attributes/spacing-indent.ts index 426e10ad03..b85d0eb531 100644 --- a/packages/layout-engine/pm-adapter/src/attributes/spacing-indent.ts +++ b/packages/layout-engine/pm-adapter/src/attributes/spacing-indent.ts @@ -9,8 +9,18 @@ import type { ParagraphAttrs, ParagraphSpacing } from '@superdoc/contracts'; import type { ParagraphSpacing as OoxmlParagraphSpacing } from '@superdoc/style-engine/ooxml'; import { twipsToPx, pickNumber } from '../utilities.js'; -const AUTO_SPACING_DEFAULT_MULTIPLIER = 1.15; - +// Per ECMA-376 §17.18.48 (ST_LineSpacingRule), `lineRule="auto"` means: +// `lineHeight = (w:line / 240) × naturalSingleLineHeight`. +// `w:line/240` is a dimensionless ratio of the font's intrinsic single-line +// height; the multiplication against natural-single happens in measuring-dom +// (`resolveLineHeight`). Earlier versions baked an extra 1.15 here so that +// measuring-dom's pre-§17.18.48 formula `mult × fontSize` coincidentally +// produced approximately the right pixels for Word's Normal style. After +// SD-2735 measuring-dom does `mult × naturalSingle` end-to-end; the fudge +// would double-apply, so we keep this normalization at the spec-canonical +// pure ratio. +const SINGLE_LINE_AUTO_DEFAULT = 1.0; +const AUTO_SPACING_BEFORE_AFTER_DEFAULT_MULTIPLIER = 1.15; const AUTO_SPACING_LINE_DEFAULT = 240; // Default OOXML auto line spacing in twips /** @@ -104,14 +114,14 @@ export const normalizeParagraphSpacing = ( if (isList) { before = undefined; } else { - before = (lineRaw ?? AUTO_SPACING_LINE_DEFAULT) * AUTO_SPACING_DEFAULT_MULTIPLIER; + before = (lineRaw ?? AUTO_SPACING_LINE_DEFAULT) * AUTO_SPACING_BEFORE_AFTER_DEFAULT_MULTIPLIER; } } if (afterAutospacing) { if (isList) { after = undefined; } else { - after = (lineRaw ?? AUTO_SPACING_LINE_DEFAULT) * AUTO_SPACING_DEFAULT_MULTIPLIER; + after = (lineRaw ?? AUTO_SPACING_LINE_DEFAULT) * AUTO_SPACING_BEFORE_AFTER_DEFAULT_MULTIPLIER; } } @@ -127,23 +137,28 @@ export const normalizeParagraphSpacing = ( }; /** - * Normalizes line spacing value based on line rule. - * Converts OOXML line spacing values to a multiplier of font size. - * @param value - OOXML line spacing value in twips + * Normalizes the OOXML `w:line` value into the canonical pipeline format + * consumed by `measuring-dom`. Per ECMA-376 §17.18.48 (ST_LineSpacingRule): + * + * - `auto`: `w:line` is in 240ths-of-a-line. Output is the dimensionless + * ratio `value/240`; measuring-dom multiplies it by the font's natural + * single-line height. + * - `atLeast`/`exact`: `w:line` is in twentieths-of-a-point. Output is in + * pixels (twips → px). + * - missing rule: treat like `auto` per OOXML default but emit the pure + * ratio (no fudge). + * + * @param value - OOXML line spacing value * @param lineRule - Line rule ('auto', 'exact', 'atLeast') - * @returns Normalized line spacing value as a multiplier, or undefined */ export const normalizeLineValue = ( value: number | undefined, lineRule: ParagraphSpacing['lineRule'] | undefined, ): { value: number; unit: 'multiplier' | 'px' } => { - if (value == null) return { value: AUTO_SPACING_DEFAULT_MULTIPLIER, unit: 'multiplier' }; + if (value == null) return { value: SINGLE_LINE_AUTO_DEFAULT, unit: 'multiplier' }; if (lineRule == 'exact' || lineRule == 'atLeast') { return { value: twipsToPx(value), unit: 'px' }; } - if (lineRule === 'auto') { - return { value: (value * AUTO_SPACING_DEFAULT_MULTIPLIER) / AUTO_SPACING_LINE_DEFAULT, unit: 'multiplier' }; - } return { value: value / AUTO_SPACING_LINE_DEFAULT, unit: 'multiplier' }; }; diff --git a/packages/layout-engine/pm-adapter/src/converters/table.test.ts b/packages/layout-engine/pm-adapter/src/converters/table.test.ts index 19b1a5496d..a1dbe20e25 100644 --- a/packages/layout-engine/pm-adapter/src/converters/table.test.ts +++ b/packages/layout-engine/pm-adapter/src/converters/table.test.ts @@ -2362,3 +2362,99 @@ describe('tableCellNodeToBlock — SD-2516: documentPartObject children', () => expect((cellBlocks[0] as ParagraphBlock).runs[0].text).toBe('Inner DPO'); }); }); + +// SD-2735 P2: previously every table-cell paragraph in a styled table was +// flattened to spacing.line=1.0 by a TableGrid-only workaround that the +// converter applied to ANY tableStyleId. Custom table styles (e.g. a +// "1.5×-spacing" table style) lost their explicit multiplier. With the +// pm-adapter normalization producing spec-correct ratios end-to-end, the +// style-engine cascade already pulls the table style's pPr.spacing — the +// workaround is redundant and must not flatten cell paragraphs whose source +// has no inline spacing. +describe('parseTableCell — SD-2735 P2: preserves custom table-style line multiplier', () => { + const mockBlockIdGenerator: BlockIdGenerator = vi.fn((kind) => `test-${kind}`); + const mockPositionMap: PositionMap = new Map(); + + it('does not flatten a 1.5× line multiplier produced by a custom table-style cascade', () => { + // mockParagraphConverter simulates the cascade output: a cell paragraph + // whose source had no explicit `` but whose tableStyle pPr + // resolves to `line=360 lineRule=auto` (= 1.5× multiplier post-fix). + const mockParagraphConverter = vi.fn( + (params: { para: PMNode }) => + [ + { + kind: 'paragraph', + id: 'p1', + attrs: { + spacing: { line: 1.5, lineUnit: 'multiplier', lineRule: 'auto' }, + }, + runs: [ + { + text: (params.para.content?.[0] as { text?: string } | undefined)?.text || 'cell', + fontFamily: 'Arial', + fontSize: 12, + }, + ], + }, + ] as ParagraphBlock[], + ); + + const node: PMNode = { + type: 'table', + attrs: { + tableStyleId: 'CustomLineSpacing', + tableProperties: { tableStyleId: 'CustomLineSpacing', tblLook: { noHBand: true, noVBand: true } }, + }, + content: [ + { + type: 'tableRow', + content: [ + { + type: 'tableCell', + // Source paragraph has NO `` — explicit-spacing guard + // does not apply, so the workaround would flatten this to 1.0. + content: [{ type: 'paragraph', content: [{ type: 'text', text: 'Cell' }] }], + }, + ], + }, + ], + }; + + // Register the custom style so `resolveExistingTableEffectiveStyleId` + // resolves `tableStyleId` to a non-null value and the cascade gate fires. + const converterContext: ConverterContext = { + ...DEFAULT_CONVERTER_CONTEXT, + translatedLinkedStyles: { + ...DEFAULT_CONVERTER_CONTEXT.translatedLinkedStyles, + styles: { + CustomLineSpacing: { + type: 'table', + paragraphProperties: { spacing: { line: 360, lineRule: 'auto' } }, + tableProperties: {}, + }, + }, + }, + }; + + const result = tableNodeToBlock( + node, + mockBlockIdGenerator, + mockPositionMap, + 'Arial', + 16, + undefined, + undefined, + undefined, + undefined, + mockParagraphConverter, + converterContext, + ) as TableBlock; + + const cell = result.rows[0].cells[0]; + const cellBlocks = cell.blocks ?? (cell.paragraph ? [cell.paragraph] : []); + const para = cellBlocks[0] as ParagraphBlock; + expect(para.attrs?.spacing?.line).toBe(1.5); + expect(para.attrs?.spacing?.lineUnit).toBe('multiplier'); + expect(para.attrs?.spacing?.lineRule).toBe('auto'); + }); +}); diff --git a/packages/layout-engine/pm-adapter/src/converters/table.ts b/packages/layout-engine/pm-adapter/src/converters/table.ts index c4d63bb834..4ac5ec77ae 100644 --- a/packages/layout-engine/pm-adapter/src/converters/table.ts +++ b/packages/layout-engine/pm-adapter/src/converters/table.ts @@ -333,17 +333,6 @@ const parseTableCell = (args: ParseTableCellArgs): TableCell | null => { const paragraphToFlowBlocks = context.converters.paragraphToFlowBlocks; const tableNodeToBlock = context.converters?.tableNodeToBlock; - /** - * Appends converted paragraph blocks to the cell's blocks array. - * - * This helper: - * 1. Applies SDT metadata to paragraph blocks (for structured content inheritance) - * 2. Filters to only include supported block types (paragraph, image, drawing) - * 3. Appends the filtered blocks to the cell's blocks array - * - * @param paragraphBlocks - The converted flow blocks from a paragraph node - * @param sdtMetadata - Optional SDT metadata to apply (from parent structuredContentBlock) - */ const appendParagraphBlocks = ( paragraphBlocks: FlowBlock[], sdtMetadata?: ReturnType, diff --git a/packages/layout-engine/style-engine/src/ooxml/index.test.ts b/packages/layout-engine/style-engine/src/ooxml/index.test.ts index 7cd3505210..52c3a7cad2 100644 --- a/packages/layout-engine/style-engine/src/ooxml/index.test.ts +++ b/packages/layout-engine/style-engine/src/ooxml/index.test.ts @@ -389,6 +389,40 @@ describe('ooxml - resolveParagraphProperties', () => { expect(result.spacing).toEqual({ before: 120, after: 240 }); expect(result.keepNext).toBe(true); }); + + it('lets a table style override docDefaults line spacing for cell paragraphs without inline spacing (SD-2735)', () => { + // Pins the cascade path that pm-adapter's old `applyTableGridSpacingCascade` + // hack was working around: Word's Normal template sets docDefaults + // `` (≈ 1.15 multiplier) and + // TableGrid sets `` (= 1.0 + // multiplier). For a cell paragraph with no inline spacing, the cascade + // must yield TableGrid's 240, not docDefaults' 276 — otherwise table + // rows render ~5 % taller than Word. + const params = buildParams({ + translatedLinkedStyles: { + ...emptyStyles, + docDefaults: { paragraphProperties: { spacing: { line: 276, lineRule: 'auto' } } }, + styles: { + Normal: { default: true, paragraphProperties: {} }, + TableGrid: { + type: 'table', + paragraphProperties: { spacing: { line: 240, lineRule: 'auto' } }, + tableProperties: {}, + }, + }, + }, + }); + const tableInfo = { + tableProperties: { tableStyleId: 'TableGrid', tblLook: { noHBand: true, noVBand: true } }, + rowIndex: 0, + cellIndex: 0, + numRows: 1, + numCells: 1, + }; + const result = resolveParagraphProperties(params, {}, tableInfo); + expect(result.spacing?.line).toBe(240); + expect(result.spacing?.lineRule).toBe('auto'); + }); }); describe('ooxml - resolveCellStyles', () => { diff --git a/tests/visual/tests/rendering/sd-2735-it945-aptos-empty-table-cells.spec.ts b/tests/visual/tests/rendering/sd-2735-it945-aptos-empty-table-cells.spec.ts new file mode 100644 index 0000000000..773f74668f --- /dev/null +++ b/tests/visual/tests/rendering/sd-2735-it945-aptos-empty-table-cells.spec.ts @@ -0,0 +1,28 @@ +import fs from 'node:fs'; +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { test } from '../fixtures/superdoc.js'; + +const __dirname = path.dirname(fileURLToPath(import.meta.url)); +const DOC_PATH_CANDIDATES = [ + path.resolve(__dirname, '../../test-data/rendering/sd-2735-it945-aptos-empty-table-cells.docx'), + path.resolve(__dirname, '../../../../test-corpus/rendering/sd-2735-it945-aptos-empty-table-cells.docx'), +]; +const DOC_PATH = DOC_PATH_CANDIDATES.find((candidate) => fs.existsSync(candidate)) ?? DOC_PATH_CANDIDATES[0]; + +test.skip(!fs.existsSync(DOC_PATH), 'Test document not available'); + +// SD-2735 / IT-945: Aptos paragraphs inside a TableGrid-styled table with +// empty and text-bearing cells. The TableGrid style carries +// w:line="240" w:lineRule="auto" (single spacing) over docDefaults +// w:line="278". Empty cells must match the height of text-bearing cells — +// measuring-dom has to thread Aptos's calibrated naturalSingleLine through +// calculateEmptyParagraphMetrics rather than fall back to 1.15 × fontSize. +// +// To regenerate baselines: +// pnpm --filter @superdoc-testing/visual docs:upload \ +// --issue SD-2735 --description it945-aptos-empty-table-cells +test('@rendering SD-2735 IT-945 Aptos empty table cells match text-bearing cells', async ({ superdoc }) => { + await superdoc.loadDocument(DOC_PATH); + await superdoc.screenshotPages('rendering/sd-2735-it945-aptos-empty-table-cells'); +});