From 4285ecc4829b241270db62cd56e5cf26437e66b6 Mon Sep 17 00:00:00 2001 From: Tadeu Tupinamba Date: Mon, 4 May 2026 11:34:55 -0300 Subject: [PATCH 01/13] refactor(contracts): host expandRunsForInlineNewlines (SD-2836) Move the inline-newline run expander from @superdoc/pm-adapter into @superdoc/contracts. The function is a pure transformation on Run[] shapes already defined here; relocating it lets painter-dom consume the helper without depending on pm-adapter (per the SD-2836 acceptance criterion: no painter-dom imports from pm-adapter or layout-bridge). pm-adapter chains its public re-export through contracts, keeping the import path stable until painter-dom is migrated to consume directly. --- packages/layout-engine/contracts/src/index.ts | 4 ++ .../contracts/src/run-helpers.test.ts | 61 ++++++++++++++++ .../contracts/src/run-helpers.ts | 47 +++++++++++++ .../src/converters/paragraph.test.ts | 69 +------------------ .../pm-adapter/src/converters/paragraph.ts | 38 +--------- .../layout-engine/pm-adapter/src/index.ts | 3 +- 6 files changed, 116 insertions(+), 106 deletions(-) create mode 100644 packages/layout-engine/contracts/src/run-helpers.test.ts create mode 100644 packages/layout-engine/contracts/src/run-helpers.ts diff --git a/packages/layout-engine/contracts/src/index.ts b/packages/layout-engine/contracts/src/index.ts index 4f8a73806d..97c33e147c 100644 --- a/packages/layout-engine/contracts/src/index.ts +++ b/packages/layout-engine/contracts/src/index.ts @@ -2065,4 +2065,8 @@ export type { } from './resolved-layout.js'; export { isResolvedTableItem, isResolvedImageItem, isResolvedDrawingItem } from './resolved-layout.js'; +// Pure transformations on inline-run shapes (used by pm-adapter, layout-bridge, +// and painter-dom). Located in contracts to avoid reverse stage dependencies. +export { expandRunsForInlineNewlines } from './run-helpers.js'; + export * as Engines from './engines/index.js'; diff --git a/packages/layout-engine/contracts/src/run-helpers.test.ts b/packages/layout-engine/contracts/src/run-helpers.test.ts new file mode 100644 index 0000000000..44861cc234 --- /dev/null +++ b/packages/layout-engine/contracts/src/run-helpers.test.ts @@ -0,0 +1,61 @@ +import { describe, it, expect } from 'vitest'; +import type { Run, TextRun, TrackedChangeMeta } from './index.js'; +import { expandRunsForInlineNewlines } from './run-helpers.js'; + +describe('expandRunsForInlineNewlines', () => { + const makeRun = (text: string, pmStart = 0): TextRun => ({ + text, + fontFamily: 'Arial', + fontSize: 12, + pmStart, + pmEnd: pmStart + text.length, + }); + + it('returns runs without inline newlines unchanged', () => { + const runs: Run[] = [makeRun('hello')]; + expect(expandRunsForInlineNewlines(runs)).toEqual(runs); + }); + + it('splits a text run at a single inline newline', () => { + const result = expandRunsForInlineNewlines([makeRun('foo\nbar')]); + expect(result).toHaveLength(3); + expect(result[0]).toMatchObject({ text: 'foo', pmStart: 0, pmEnd: 3 }); + expect(result[1]).toMatchObject({ kind: 'break', pmStart: 3, pmEnd: 4 }); + expect(result[2]).toMatchObject({ text: 'bar', pmStart: 4, pmEnd: 7 }); + }); + + it('keeps the break and advances the cursor for a leading newline', () => { + const result = expandRunsForInlineNewlines([makeRun('\nfoo')]); + expect(result).toHaveLength(2); + expect(result[0]).toMatchObject({ kind: 'break', pmStart: 0, pmEnd: 1 }); + expect(result[1]).toMatchObject({ text: 'foo', pmStart: 1, pmEnd: 4 }); + }); + + it('keeps both breaks when a run contains consecutive inline newlines', () => { + const result = expandRunsForInlineNewlines([makeRun('a\n\nb')]); + expect(result).toHaveLength(4); + expect(result[0]).toMatchObject({ text: 'a', pmStart: 0, pmEnd: 1 }); + expect(result[1]).toMatchObject({ kind: 'break', pmStart: 1, pmEnd: 2 }); + expect(result[2]).toMatchObject({ kind: 'break', pmStart: 2, pmEnd: 3 }); + expect(result[3]).toMatchObject({ text: 'b', pmStart: 3, pmEnd: 4 }); + }); + + it('does not emit an empty trailing text run for a trailing newline', () => { + const result = expandRunsForInlineNewlines([makeRun('foo\n')]); + expect(result).toHaveLength(2); + expect(result[0]).toMatchObject({ text: 'foo', pmStart: 0, pmEnd: 3 }); + expect(result[1]).toMatchObject({ kind: 'break', pmStart: 3, pmEnd: 4 }); + }); + + it('propagates trackedChange metadata onto emitted break runs', () => { + const trackedChange: TrackedChangeMeta = { + id: 'change-1', + kind: 'insert', + author: 'alice', + date: '2024-01-01T00:00:00Z', + }; + const run: TextRun = { ...makeRun('foo\nbar'), trackedChange }; + const result = expandRunsForInlineNewlines([run]); + expect(result[1]).toMatchObject({ kind: 'break', trackedChange }); + }); +}); diff --git a/packages/layout-engine/contracts/src/run-helpers.ts b/packages/layout-engine/contracts/src/run-helpers.ts new file mode 100644 index 0000000000..98135baf65 --- /dev/null +++ b/packages/layout-engine/contracts/src/run-helpers.ts @@ -0,0 +1,47 @@ +/** + * Pure transformations on inline-run shapes. + * + * These helpers operate on `Run[]` shapes defined in this contracts package. + * They have no upstream dependencies (no pm-adapter, no layout-bridge, no + * style-engine), so any stage can consume them without creating a reverse + * dependency back into a downstream package. + */ + +import type { Run, TextRun } from './index.js'; + +/** + * Expands text runs that contain inline newlines into multiple runs. + * + * @param {Run[]} runs - The runs to expand + * @returns {Run[]} The expanded runs + */ +export function expandRunsForInlineNewlines(runs: Run[]): Run[] { + const result: Run[] = []; + for (const run of runs) { + const textRun = run as TextRun; + if ('text' in run && typeof textRun.text === 'string' && textRun.text.includes('\n')) { + const segments = textRun.text.split('\n'); + let cursor = textRun.pmStart ?? 0; + segments.forEach((segment, idx) => { + if (segment.length > 0) { + result.push({ ...textRun, text: segment, pmStart: cursor, pmEnd: cursor + segment.length }); + cursor += segment.length; + } + if (idx !== segments.length - 1) { + result.push({ + kind: 'break', + breakType: 'line', + pmStart: cursor, + pmEnd: cursor + 1, + sdt: textRun.sdt, + trackedChange: textRun.trackedChange, + }); + cursor += 1; + } + }); + } else { + result.push(run); + } + } + return result; +} diff --git a/packages/layout-engine/pm-adapter/src/converters/paragraph.test.ts b/packages/layout-engine/pm-adapter/src/converters/paragraph.test.ts index 82b651467c..9f82455945 100644 --- a/packages/layout-engine/pm-adapter/src/converters/paragraph.test.ts +++ b/packages/layout-engine/pm-adapter/src/converters/paragraph.test.ts @@ -14,7 +14,6 @@ import { dataAttrsCompatible, commentsCompatible, getLastParagraphFont, - expandRunsForInlineNewlines, } from './paragraph.js'; import { isInlineImage, imageNodeToRun } from './inline-converters/image.js'; import type { @@ -28,15 +27,7 @@ import type { NodeHandlerContext, } from '../types.js'; import type { ConverterContext } from '../converter-context.js'; -import type { - Run, - TextRun, - FlowBlock, - ParagraphBlock, - TrackedChangeMeta, - ImageRun, - SdtMetadata, -} from '@superdoc/contracts'; +import type { Run, TextRun, FlowBlock, ParagraphBlock, ImageRun, SdtMetadata } from '@superdoc/contracts'; // Mock external dependencies vi.mock('./inline-converters/text-run.js', () => ({ @@ -355,64 +346,6 @@ describe('getLastParagraphFont', () => { }); }); -describe('expandRunsForInlineNewlines', () => { - const makeRun = (text: string, pmStart = 0): TextRun => ({ - text, - fontFamily: 'Arial', - fontSize: 12, - pmStart, - pmEnd: pmStart + text.length, - }); - - it('returns runs without inline newlines unchanged', () => { - const runs: Run[] = [makeRun('hello')]; - expect(expandRunsForInlineNewlines(runs)).toEqual(runs); - }); - - it('splits a text run at a single inline newline', () => { - const result = expandRunsForInlineNewlines([makeRun('foo\nbar')]); - expect(result).toHaveLength(3); - expect(result[0]).toMatchObject({ text: 'foo', pmStart: 0, pmEnd: 3 }); - expect(result[1]).toMatchObject({ kind: 'break', pmStart: 3, pmEnd: 4 }); - expect(result[2]).toMatchObject({ text: 'bar', pmStart: 4, pmEnd: 7 }); - }); - - it('keeps the break and advances the cursor for a leading newline', () => { - const result = expandRunsForInlineNewlines([makeRun('\nfoo')]); - expect(result).toHaveLength(2); - expect(result[0]).toMatchObject({ kind: 'break', pmStart: 0, pmEnd: 1 }); - expect(result[1]).toMatchObject({ text: 'foo', pmStart: 1, pmEnd: 4 }); - }); - - it('keeps both breaks when a run contains consecutive inline newlines', () => { - const result = expandRunsForInlineNewlines([makeRun('a\n\nb')]); - expect(result).toHaveLength(4); - expect(result[0]).toMatchObject({ text: 'a', pmStart: 0, pmEnd: 1 }); - expect(result[1]).toMatchObject({ kind: 'break', pmStart: 1, pmEnd: 2 }); - expect(result[2]).toMatchObject({ kind: 'break', pmStart: 2, pmEnd: 3 }); - expect(result[3]).toMatchObject({ text: 'b', pmStart: 3, pmEnd: 4 }); - }); - - it('does not emit an empty trailing text run for a trailing newline', () => { - const result = expandRunsForInlineNewlines([makeRun('foo\n')]); - expect(result).toHaveLength(2); - expect(result[0]).toMatchObject({ text: 'foo', pmStart: 0, pmEnd: 3 }); - expect(result[1]).toMatchObject({ kind: 'break', pmStart: 3, pmEnd: 4 }); - }); - - it('propagates trackedChange metadata onto emitted break runs', () => { - const trackedChange: TrackedChangeMeta = { - id: 'change-1', - kind: 'insertion', - author: 'alice', - date: '2024-01-01T00:00:00Z', - }; - const run: TextRun = { ...makeRun('foo\nbar'), trackedChange }; - const result = expandRunsForInlineNewlines([run]); - expect(result[1]).toMatchObject({ kind: 'break', trackedChange }); - }); -}); - describe('paragraph converters', () => { describe('mergeAdjacentRuns', () => { it('should return empty array unchanged', () => { diff --git a/packages/layout-engine/pm-adapter/src/converters/paragraph.ts b/packages/layout-engine/pm-adapter/src/converters/paragraph.ts index e5bfc561ac..0028f147d1 100644 --- a/packages/layout-engine/pm-adapter/src/converters/paragraph.ts +++ b/packages/layout-engine/pm-adapter/src/converters/paragraph.ts @@ -18,6 +18,7 @@ import type { TrackedChangeMeta, SourceAnchor, } from '@superdoc/contracts'; +import { expandRunsForInlineNewlines } from '@superdoc/contracts'; import type { PMNode, PMMark, @@ -221,43 +222,6 @@ export function mergeAdjacentRuns(runs: Run[]): Run[] { return merged; } -/** - * Expands text runs that contain inline newlines into multiple runs. - * - * @param {Run[]} runs - The runs to expand - * @returns {Run[]} The expanded runs - */ -export function expandRunsForInlineNewlines(runs: Run[]): Run[] { - const result: Run[] = []; - for (const run of runs) { - const textRun = run as TextRun; - if ('text' in run && typeof textRun.text === 'string' && textRun.text.includes('\n')) { - const segments = textRun.text.split('\n'); - let cursor = textRun.pmStart ?? 0; - segments.forEach((segment, idx) => { - if (segment.length > 0) { - result.push({ ...textRun, text: segment, pmStart: cursor, pmEnd: cursor + segment.length }); - cursor += segment.length; - } - if (idx !== segments.length - 1) { - result.push({ - kind: 'break', - breakType: 'line', - pmStart: cursor, - pmEnd: cursor + 1, - sdt: textRun.sdt, - trackedChange: textRun.trackedChange, - }); - cursor += 1; - } - }); - } else { - result.push(run); - } - } - return result; -} - /** * Extracts the default font family and size from paragraph properties. * Used for creating default runs in empty paragraphs. diff --git a/packages/layout-engine/pm-adapter/src/index.ts b/packages/layout-engine/pm-adapter/src/index.ts index ab38a635d3..d0c626282e 100644 --- a/packages/layout-engine/pm-adapter/src/index.ts +++ b/packages/layout-engine/pm-adapter/src/index.ts @@ -41,7 +41,8 @@ export { SectionType } from './types.js'; export { toFlowBlocks, toFlowBlocksMap } from './internal.js'; // Re-export run type guards and run utilities -export { isTextRun, expandRunsForInlineNewlines } from './converters/paragraph.js'; +export { isTextRun } from './converters/paragraph.js'; +export { expandRunsForInlineNewlines } from '@superdoc/contracts'; // Re-export cache for incremental conversion export { FlowBlockCache } from './cache.js'; From 7708e0a56398d18abbce239b005e04c227ea446c Mon Sep 17 00:00:00 2001 From: Tadeu Tupinamba Date: Mon, 4 May 2026 11:39:19 -0300 Subject: [PATCH 02/13] refactor(contracts): host sliceRunsForLine (SD-2836) Move the line-aware run slicer from @superdoc/layout-bridge into @superdoc/contracts, alongside expandRunsForInlineNewlines. The function is a pure transformation on Run/Line shapes already defined here; relocating it lets painter-dom consume the helper without depending on layout-bridge (per the SD-2836 acceptance criterion). layout-bridge chains its public re-export through contracts, keeping the import path stable until painter-dom is migrated to consume directly. Also restore a TrackedChangeMeta import in pm-adapter's paragraph.test.ts that the prior commit dropped; the type is still referenced outside the migrated describe block. --- packages/layout-engine/contracts/src/index.ts | 2 +- .../contracts/src/run-helpers.test.ts | 82 +++++++++++++++++- .../contracts/src/run-helpers.ts | 64 +++++++++++++- .../layout-engine/layout-bridge/src/index.ts | 3 +- .../layout-bridge/src/text-measurement.ts | 85 ++----------------- .../src/converters/paragraph.test.ts | 10 ++- 6 files changed, 161 insertions(+), 85 deletions(-) diff --git a/packages/layout-engine/contracts/src/index.ts b/packages/layout-engine/contracts/src/index.ts index 97c33e147c..8bae88389b 100644 --- a/packages/layout-engine/contracts/src/index.ts +++ b/packages/layout-engine/contracts/src/index.ts @@ -2067,6 +2067,6 @@ export { isResolvedTableItem, isResolvedImageItem, isResolvedDrawingItem } from // Pure transformations on inline-run shapes (used by pm-adapter, layout-bridge, // and painter-dom). Located in contracts to avoid reverse stage dependencies. -export { expandRunsForInlineNewlines } from './run-helpers.js'; +export { expandRunsForInlineNewlines, sliceRunsForLine } from './run-helpers.js'; export * as Engines from './engines/index.js'; diff --git a/packages/layout-engine/contracts/src/run-helpers.test.ts b/packages/layout-engine/contracts/src/run-helpers.test.ts index 44861cc234..aaae281c2b 100644 --- a/packages/layout-engine/contracts/src/run-helpers.test.ts +++ b/packages/layout-engine/contracts/src/run-helpers.test.ts @@ -1,6 +1,6 @@ import { describe, it, expect } from 'vitest'; -import type { Run, TextRun, TrackedChangeMeta } from './index.js'; -import { expandRunsForInlineNewlines } from './run-helpers.js'; +import type { FlowBlock, Line, ParagraphBlock, Run, TabRun, TextRun, TrackedChangeMeta } from './index.js'; +import { expandRunsForInlineNewlines, sliceRunsForLine } from './run-helpers.js'; describe('expandRunsForInlineNewlines', () => { const makeRun = (text: string, pmStart = 0): TextRun => ({ @@ -59,3 +59,81 @@ describe('expandRunsForInlineNewlines', () => { expect(result[1]).toMatchObject({ kind: 'break', trackedChange }); }); }); + +describe('sliceRunsForLine', () => { + const makeTextRun = (text: string, pmStart = 0): TextRun => ({ + text, + fontFamily: 'Arial', + fontSize: 12, + pmStart, + pmEnd: pmStart + text.length, + }); + + const makeParagraph = (runs: Run[]): ParagraphBlock => ({ + kind: 'paragraph', + id: 'p-1', + runs, + }); + + const makeLine = (overrides: Partial = {}): Line => ({ + fromRun: 0, + fromChar: 0, + toRun: 0, + toChar: 0, + width: 0, + ascent: 12, + descent: 4, + lineHeight: 16, + ...overrides, + }); + + it('returns an empty array for non-paragraph blocks', () => { + const block: FlowBlock = { + kind: 'image', + id: 'i-1', + attrs: { src: 'about:blank', alt: '' }, + } as unknown as FlowBlock; + expect(sliceRunsForLine(block, makeLine())).toEqual([]); + }); + + it('passes tab runs through unchanged', () => { + const tab: TabRun = { kind: 'tab', text: '\t', pmStart: 0, pmEnd: 1 }; + const block = makeParagraph([tab]); + const line = makeLine({ toRun: 0, fromChar: 0, toChar: 1 }); + expect(sliceRunsForLine(block, line)).toEqual([tab]); + }); + + it('passes line-break runs through unchanged', () => { + const lineBreak: Run = { kind: 'lineBreak', pmStart: 0, pmEnd: 1 } as Run; + const block = makeParagraph([lineBreak]); + const line = makeLine({ toRun: 0, fromChar: 0, toChar: 1 }); + expect(sliceRunsForLine(block, line)).toEqual([lineBreak]); + }); + + it('slices text on the first/last run and adjusts pmStart/pmEnd', () => { + const run = makeTextRun('hello world', 100); + const block = makeParagraph([run]); + const line = makeLine({ fromRun: 0, fromChar: 0, toRun: 0, toChar: 5 }); + const result = sliceRunsForLine(block, line); + expect(result).toHaveLength(1); + expect(result[0]).toMatchObject({ text: 'hello', pmStart: 100, pmEnd: 105 }); + }); + + it('passes middle text runs through unchanged when the line spans multiple runs', () => { + const first = makeTextRun('foo', 0); + const middle = makeTextRun('bar', 3); + const last = makeTextRun('baz', 6); + const block = makeParagraph([first, middle, last]); + const line = makeLine({ fromRun: 0, fromChar: 0, toRun: 2, toChar: 3 }); + const result = sliceRunsForLine(block, line); + expect(result).toHaveLength(3); + expect(result[1]).toBe(middle); + }); + + it('drops empty slices when the requested range produces no characters', () => { + const run = makeTextRun('abc', 0); + const block = makeParagraph([run]); + const line = makeLine({ fromRun: 0, fromChar: 2, toRun: 0, toChar: 2 }); + expect(sliceRunsForLine(block, line)).toEqual([]); + }); +}); diff --git a/packages/layout-engine/contracts/src/run-helpers.ts b/packages/layout-engine/contracts/src/run-helpers.ts index 98135baf65..13409e2b69 100644 --- a/packages/layout-engine/contracts/src/run-helpers.ts +++ b/packages/layout-engine/contracts/src/run-helpers.ts @@ -7,7 +7,7 @@ * dependency back into a downstream package. */ -import type { Run, TextRun } from './index.js'; +import type { FlowBlock, Line, Run, TextRun } from './index.js'; /** * Expands text runs that contain inline newlines into multiple runs. @@ -45,3 +45,65 @@ export function expandRunsForInlineNewlines(runs: Run[]): Run[] { } return result; } + +/** + * Extracts the subset of runs that appear in a specific line. + * Handles partial runs that span multiple lines. + * + * @param block - The paragraph block containing the runs + * @param line - The line to extract runs for + * @returns Array of runs present in the line + */ +export function sliceRunsForLine(block: FlowBlock, line: Line): Run[] { + const result: Run[] = []; + if (block.kind !== 'paragraph') return result; + + for (let runIndex = line.fromRun; runIndex <= line.toRun; runIndex += 1) { + const run = block.runs[runIndex]; + if (!run) continue; + + if (run.kind === 'tab') { + result.push(run); + continue; + } + + // Images, line breaks, breaks, field annotations, and math runs are atomic + // units. They occupy a single character of the run sequence and are passed + // through to the result without slicing. + if ( + 'src' in run || + run.kind === 'lineBreak' || + run.kind === 'break' || + run.kind === 'fieldAnnotation' || + run.kind === 'math' + ) { + result.push(run); + continue; + } + + const text = run.text ?? ''; + const isFirstRun = runIndex === line.fromRun; + const isLastRun = runIndex === line.toRun; + + if (isFirstRun || isLastRun) { + const start = isFirstRun ? line.fromChar : 0; + const end = isLastRun ? line.toChar : text.length; + const slice = text.slice(start, end); + if (!slice) continue; + const pmStart = + run.pmStart != null ? run.pmStart + start : run.pmEnd != null ? run.pmEnd - (text.length - start) : undefined; + const pmEnd = + run.pmStart != null ? run.pmStart + end : run.pmEnd != null ? run.pmEnd - (text.length - end) : undefined; + result.push({ + ...run, + text: slice, + pmStart, + pmEnd, + }); + } else { + result.push(run); + } + } + + return result; +} diff --git a/packages/layout-engine/layout-bridge/src/index.ts b/packages/layout-engine/layout-bridge/src/index.ts index 6f61217b6c..a1820b6e9b 100644 --- a/packages/layout-engine/layout-bridge/src/index.ts +++ b/packages/layout-engine/layout-bridge/src/index.ts @@ -74,7 +74,8 @@ export type { HeaderFooterLayoutResult, IncrementalLayoutResult } from './increm export { computeDisplayPageNumber } from '@superdoc/layout-engine'; export type { DisplayPageInfo, HeaderFooterConstraints } from '@superdoc/layout-engine'; export { remeasureParagraph } from './remeasure'; -export { measureCharacterX, sliceRunsForLine } from './text-measurement'; +export { measureCharacterX } from './text-measurement'; +export { sliceRunsForLine } from '@superdoc/contracts'; export { clickToPositionDom, findPageElement } from './dom-mapping'; export { isListItem, getWordLayoutConfig, calculateTextStartIndent, extractParagraphIndent } from './list-indent-utils'; export type { TextIndentCalculationParams } from './list-indent-utils'; diff --git a/packages/layout-engine/layout-bridge/src/text-measurement.ts b/packages/layout-engine/layout-bridge/src/text-measurement.ts index 35a636f885..6e04509441 100644 --- a/packages/layout-engine/layout-bridge/src/text-measurement.ts +++ b/packages/layout-engine/layout-bridge/src/text-measurement.ts @@ -1,5 +1,10 @@ import type { FlowBlock, Line, Run, TabRun } from '@superdoc/contracts'; -import { shouldApplyJustify, calculateJustifySpacing, SPACE_CHARS as SHARED_SPACE_CHARS } from '@superdoc/contracts'; +import { + shouldApplyJustify, + calculateJustifySpacing, + sliceRunsForLine, + SPACE_CHARS as SHARED_SPACE_CHARS, +} from '@superdoc/contracts'; /** * Shared text measurement utility for accurate character positioning. @@ -327,84 +332,6 @@ export function getRunFontString(run: Run): string { return `${style} ${weight} ${fontSize}px ${fontFamily}`; } -/** - * Extracts the subset of runs that appear in a specific line. - * Handles partial runs that span multiple lines. - * - * @param block - The paragraph block containing the runs - * @param line - The line to extract runs for - * @returns Array of runs present in the line - */ -export function sliceRunsForLine(block: FlowBlock, line: Line): Run[] { - const result: Run[] = []; - if (block.kind !== 'paragraph') return result; - - for (let runIndex = line.fromRun; runIndex <= line.toRun; runIndex += 1) { - const run = block.runs[runIndex]; - if (!run) continue; - - if (isTabRun(run)) { - result.push(run); - continue; - } - - // FIXED: ImageRun handling - images are atomic units, no slicing needed - if ('src' in run) { - result.push(run); - continue; - } - - // LineBreakRun handling - line breaks are atomic units, no slicing needed - if (run.kind === 'lineBreak') { - result.push(run); - continue; - } - - // BreakRun handling - breaks are atomic units, no slicing needed - if (run.kind === 'break') { - result.push(run); - continue; - } - - // FieldAnnotationRun handling - field annotations are atomic units, no slicing needed - if (run.kind === 'fieldAnnotation') { - result.push(run); - continue; - } - - // MathRun handling - math runs are atomic units, no slicing needed - if (run.kind === 'math') { - result.push(run); - continue; - } - - const text = run.text ?? ''; - const isFirstRun = runIndex === line.fromRun; - const isLastRun = runIndex === line.toRun; - - if (isFirstRun || isLastRun) { - const start = isFirstRun ? line.fromChar : 0; - const end = isLastRun ? line.toChar : text.length; - const slice = text.slice(start, end); - if (!slice) continue; - const pmStart = - run.pmStart != null ? run.pmStart + start : run.pmEnd != null ? run.pmEnd - (text.length - start) : undefined; - const pmEnd = - run.pmStart != null ? run.pmStart + end : run.pmEnd != null ? run.pmEnd - (text.length - end) : undefined; - result.push({ - ...run, - text: slice, - pmStart, - pmEnd, - }); - } else { - result.push(run); - } - } - - return result; -} - /** * Measure the X position for a specific character offset within a line. * Uses Canvas measureText for pixel-perfect accuracy. diff --git a/packages/layout-engine/pm-adapter/src/converters/paragraph.test.ts b/packages/layout-engine/pm-adapter/src/converters/paragraph.test.ts index 9f82455945..cbcb6ca314 100644 --- a/packages/layout-engine/pm-adapter/src/converters/paragraph.test.ts +++ b/packages/layout-engine/pm-adapter/src/converters/paragraph.test.ts @@ -27,7 +27,15 @@ import type { NodeHandlerContext, } from '../types.js'; import type { ConverterContext } from '../converter-context.js'; -import type { Run, TextRun, FlowBlock, ParagraphBlock, ImageRun, SdtMetadata } from '@superdoc/contracts'; +import type { + Run, + TextRun, + FlowBlock, + ParagraphBlock, + TrackedChangeMeta, + ImageRun, + SdtMetadata, +} from '@superdoc/contracts'; // Mock external dependencies vi.mock('./inline-converters/text-run.js', () => ({ From 016576be637ec04ad6eeb6564bf47f2b45b931cc Mon Sep 17 00:00:00 2001 From: Tadeu Tupinamba Date: Mon, 4 May 2026 11:40:28 -0300 Subject: [PATCH 03/13] refactor(painter): consume run helpers from contracts (SD-2836) Switch the painter's two cross-package run-helper imports (expandRunsForInlineNewlines, sliceRunsForLine) to source from @superdoc/contracts directly, then drop @superdoc/pm-adapter and @superdoc/layout-bridge from painter-dom's runtime dependencies. This closes the painter-dom -> pm-adapter / layout-bridge import edge called out in the SD-2836 acceptance criteria. Architecture-boundary guards added in [3/3] of this stack will prevent reintroduction. --- packages/layout-engine/painters/dom/package.json | 2 -- packages/layout-engine/painters/dom/src/renderer.ts | 4 ++-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/packages/layout-engine/painters/dom/package.json b/packages/layout-engine/painters/dom/package.json index 8396532302..cf39d4348f 100644 --- a/packages/layout-engine/painters/dom/package.json +++ b/packages/layout-engine/painters/dom/package.json @@ -22,8 +22,6 @@ "@superdoc/dom-contract": "workspace:*", "@superdoc/font-utils": "workspace:*", "@superdoc/layout-resolved": "workspace:*", - "@superdoc/layout-bridge": "workspace:*", - "@superdoc/pm-adapter": "workspace:*", "@superdoc/preset-geometry": "workspace:*", "@superdoc/url-validation": "workspace:*" }, diff --git a/packages/layout-engine/painters/dom/src/renderer.ts b/packages/layout-engine/painters/dom/src/renderer.ts index 02d670d4f8..e6eec760eb 100644 --- a/packages/layout-engine/painters/dom/src/renderer.ts +++ b/packages/layout-engine/painters/dom/src/renderer.ts @@ -62,11 +62,13 @@ import { adjustAvailableWidthForTextIndent, calculateJustifySpacing, computeLinePmRange, + expandRunsForInlineNewlines, getCellSpacingPx, normalizeColumnLayout, normalizeBaselineShift, resolveBaseFontSizeForVerticalText, shouldApplyJustify, + sliceRunsForLine, SPACE_CHARS, } from '@superdoc/contracts'; import { toCssFontFamily } from '@superdoc/font-utils'; @@ -122,8 +124,6 @@ import { } from './features/paragraph-borders/index.js'; import { applyRtlStyles, shouldUseSegmentPositioning } from './features/rtl-paragraph/index.js'; import { convertOmmlToMathml } from './features/math/index.js'; -import { expandRunsForInlineNewlines } from '@superdoc/pm-adapter'; -import { sliceRunsForLine } from '@superdoc/layout-bridge'; /** * Minimal type for WordParagraphLayoutOutput marker data used in rendering. From ff7aebc2401f1ae0808a4c9d6cc5e68cc120f19e Mon Sep 17 00:00:00 2001 From: Tadeu Tupinamba Date: Mon, 4 May 2026 11:43:37 -0300 Subject: [PATCH 04/13] refactor(layout): add fragment back-pointer to resolved items (SD-2836) Add a `fragment: Fragment` field to ResolvedFragmentItem, ResolvedTableItem, ResolvedImageItem, and ResolvedDrawingItem, and populate it from the corresponding resolve* helper. The reference is shared, not copied: items now carry the same Fragment object that lives on the source page. This is the precondition for stopping painter iteration over `page.fragments`. The next commit drops getResolvedFragmentItem and switches the painter to iterate ResolvedPage.items, reading the source fragment via `item.fragment` instead of indexing back into the legacy fragments array. Includes focused tests in resolveLayout.test.ts asserting back-pointer identity for each kind. --- .../contracts/src/resolved-layout.ts | 10 ++ .../layout-resolved/src/resolveDrawing.ts | 1 + .../layout-resolved/src/resolveImage.ts | 1 + .../layout-resolved/src/resolveLayout.test.ts | 96 +++++++++++++++++++ .../layout-resolved/src/resolveLayout.ts | 1 + .../layout-resolved/src/resolveTable.ts | 1 + 6 files changed, 110 insertions(+) diff --git a/packages/layout-engine/contracts/src/resolved-layout.ts b/packages/layout-engine/contracts/src/resolved-layout.ts index 75f1d9ee64..55d5fbce7c 100644 --- a/packages/layout-engine/contracts/src/resolved-layout.ts +++ b/packages/layout-engine/contracts/src/resolved-layout.ts @@ -111,6 +111,10 @@ export type ResolvedFragmentItem = { zIndex?: number; /** Source fragment kind — used by the painter for wrapper style decisions. */ fragmentKind: Fragment['kind']; + /** Source fragment back-pointer. Lets the painter iterate resolved items + * and pass the underlying fragment to render helpers without indexing + * back into the legacy `page.fragments` array. */ + fragment: Fragment; /** Block ID. Written to data-block-id. */ blockId: string; /** @@ -257,6 +261,8 @@ export type ResolvedTableItem = { * promote this to a permanent API surface. */ fragmentIndex: number; + /** Source TableFragment back-pointer (see ResolvedFragmentItem.fragment). */ + fragment: Fragment; /** ProseMirror start position for click-to-position mapping. */ pmStart?: number; /** ProseMirror end position for click-to-position mapping. */ @@ -318,6 +324,8 @@ export type ResolvedImageItem = { * promote this to a permanent API surface. */ fragmentIndex: number; + /** Source ImageFragment back-pointer (see ResolvedFragmentItem.fragment). */ + fragment: Fragment; /** ProseMirror start position for click-to-position mapping. */ pmStart?: number; /** ProseMirror end position for click-to-position mapping. */ @@ -371,6 +379,8 @@ export type ResolvedDrawingItem = { * promote this to a permanent API surface. */ fragmentIndex: number; + /** Source DrawingFragment back-pointer (see ResolvedFragmentItem.fragment). */ + fragment: Fragment; /** ProseMirror start position for click-to-position mapping. */ pmStart?: number; /** ProseMirror end position for click-to-position mapping. */ diff --git a/packages/layout-engine/layout-resolved/src/resolveDrawing.ts b/packages/layout-engine/layout-resolved/src/resolveDrawing.ts index 1703da88ab..cad76fc0bb 100644 --- a/packages/layout-engine/layout-resolved/src/resolveDrawing.ts +++ b/packages/layout-engine/layout-resolved/src/resolveDrawing.ts @@ -29,6 +29,7 @@ export function resolveDrawingItem( zIndex: fragment.isAnchored ? fragment.zIndex : undefined, blockId: fragment.blockId, fragmentIndex, + fragment, block, sourceAnchor: fragment.sourceAnchor ?? block.sourceAnchor, }; diff --git a/packages/layout-engine/layout-resolved/src/resolveImage.ts b/packages/layout-engine/layout-resolved/src/resolveImage.ts index 951fdce795..45c5d8625c 100644 --- a/packages/layout-engine/layout-resolved/src/resolveImage.ts +++ b/packages/layout-engine/layout-resolved/src/resolveImage.ts @@ -29,6 +29,7 @@ export function resolveImageItem( zIndex: fragment.isAnchored ? fragment.zIndex : undefined, blockId: fragment.blockId, fragmentIndex, + fragment, block, sourceAnchor: fragment.sourceAnchor ?? block.sourceAnchor, }; diff --git a/packages/layout-engine/layout-resolved/src/resolveLayout.test.ts b/packages/layout-engine/layout-resolved/src/resolveLayout.test.ts index 4a11c5b069..e792834ecb 100644 --- a/packages/layout-engine/layout-resolved/src/resolveLayout.test.ts +++ b/packages/layout-engine/layout-resolved/src/resolveLayout.test.ts @@ -666,6 +666,102 @@ describe('resolveLayout', () => { }); }); + describe('fragment back-pointer', () => { + it('attaches the source ParaFragment to a paragraph item', () => { + const paraFragment: ParaFragment = { + kind: 'para', + blockId: 'p1', + fromLine: 0, + toLine: 1, + x: 72, + y: 100, + width: 468, + }; + const layout: Layout = { + pageSize: { w: 612, h: 792 }, + pages: [{ number: 1, fragments: [paraFragment] }], + }; + const blocks: FlowBlock[] = [{ kind: 'paragraph', id: 'p1', runs: [] }]; + const measures: Measure[] = [{ kind: 'paragraph', lines: [], totalHeight: 20 }]; + const result = resolveLayout({ layout, flowMode: 'paginated', blocks, measures }); + const item = result.pages[0].items[0] as { fragment?: ParaFragment }; + expect(item.fragment).toBe(paraFragment); + }); + + it('attaches the source TableFragment to a table item', () => { + const tableFragment: TableFragment = { + kind: 'table', + blockId: 't1', + fromRow: 0, + toRow: 1, + x: 0, + y: 0, + width: 400, + height: 100, + }; + const layout: Layout = { + pageSize: { w: 612, h: 792 }, + pages: [{ number: 1, fragments: [tableFragment] }], + }; + const blocks: FlowBlock[] = [ + { kind: 'table', id: 't1', rows: [{ id: 'r1', cells: [] }], attrs: { columnWidths: [400] } }, + ]; + const measures: Measure[] = [ + { kind: 'table', rowHeights: [100], columnWidths: [400], cells: [], rows: [] } as unknown as Measure, + ]; + const result = resolveLayout({ layout, flowMode: 'paginated', blocks, measures }); + const item = result.pages[0].items[0] as { fragment?: TableFragment }; + expect(item.fragment).toBe(tableFragment); + }); + + it('attaches the source ImageFragment to an image item', () => { + const imageFragment: ImageFragment = { + kind: 'image', + blockId: 'img1', + x: 0, + y: 0, + width: 200, + height: 150, + isAnchored: false, + }; + const layout: Layout = { + pageSize: { w: 612, h: 792 }, + pages: [{ number: 1, fragments: [imageFragment] }], + }; + const blocks: FlowBlock[] = [ + { kind: 'image', id: 'img1', attrs: { src: 'about:blank', width: 200, height: 150 } }, + ]; + const measures: Measure[] = [{ kind: 'image' } as unknown as Measure]; + const result = resolveLayout({ layout, flowMode: 'paginated', blocks, measures }); + const item = result.pages[0].items[0] as { fragment?: ImageFragment }; + expect(item.fragment).toBe(imageFragment); + }); + + it('attaches the source DrawingFragment to a drawing item', () => { + const drawingFragment: DrawingFragment = { + kind: 'drawing', + blockId: 'd1', + x: 0, + y: 0, + width: 200, + height: 200, + isAnchored: false, + geometry: { width: 200, height: 200 }, + } as DrawingFragment; + const layout: Layout = { + pageSize: { w: 612, h: 792 }, + pages: [{ number: 1, fragments: [drawingFragment] }], + }; + const blocks: FlowBlock[] = [ + { kind: 'drawing', id: 'd1', drawingKind: 'image', shapes: [], attrs: {} } as unknown as FlowBlock, + ]; + const measures: Measure[] = [{ kind: 'drawing' } as unknown as Measure]; + const result = resolveLayout({ layout, flowMode: 'paginated', blocks, measures }); + const item = result.pages[0].items[0] as { fragment?: DrawingFragment }; + expect(item.fragment).toBe(drawingFragment); + }); + }); + describe('paragraph/list-item block and measure lifting', () => { it('lifts block and measure from a paragraph fragment', () => { const paraFragment: ParaFragment = { diff --git a/packages/layout-engine/layout-resolved/src/resolveLayout.ts b/packages/layout-engine/layout-resolved/src/resolveLayout.ts index ec229c227b..3d0b221198 100644 --- a/packages/layout-engine/layout-resolved/src/resolveLayout.ts +++ b/packages/layout-engine/layout-resolved/src/resolveLayout.ts @@ -247,6 +247,7 @@ export function resolveFragmentItem( height: computeFragmentHeight(fragment, blockMap), zIndex: resolveFragmentZIndex(fragment), fragmentKind: fragment.kind, + fragment, blockId: fragment.blockId, fragmentIndex, content: resolveParagraphContentIfApplicable(fragment, blockMap), diff --git a/packages/layout-engine/layout-resolved/src/resolveTable.ts b/packages/layout-engine/layout-resolved/src/resolveTable.ts index 3fc10e36b8..665d1d4891 100644 --- a/packages/layout-engine/layout-resolved/src/resolveTable.ts +++ b/packages/layout-engine/layout-resolved/src/resolveTable.ts @@ -37,6 +37,7 @@ export function resolveTableItem( zIndex: undefined, // tables don't have zIndex at fragment level blockId: fragment.blockId, fragmentIndex, + fragment, block, measure, cellSpacingPx: measure.cellSpacingPx ?? getCellSpacingPx(block.attrs?.cellSpacing), From a2ca4066206100db04daef751bab0f79d64d054f Mon Sep 17 00:00:00 2001 From: Tadeu Tupinamba Date: Mon, 4 May 2026 11:49:45 -0300 Subject: [PATCH 05/13] refactor(painter): drop getResolvedFragmentItem; iterate resolved items (SD-2836) Replace the three `page.fragments.forEach((fragment, index) => ...)` loops in renderPage / patchPage / initPage with iterations over `resolvedItems` (the resolved page's items), reading the source Fragment via the back-pointer added in the previous commit. Removes: - getResolvedFragmentItem (parallel-array index lookup into resolved items) - direct iteration of `page.fragments` from the painter render path Updates affected hand-rolled resolved-layout tests to populate the new required `fragment` back-pointer; the painter now treats items without a fragment as not-yet-renderable rather than indexing back into the legacy fragments array. --- .../painters/dom/src/index.test.ts | 7 ++++ .../painters/dom/src/renderer.ts | 39 +++++++++---------- 2 files changed, 25 insertions(+), 21 deletions(-) diff --git a/packages/layout-engine/painters/dom/src/index.test.ts b/packages/layout-engine/painters/dom/src/index.test.ts index 6b5228cb44..3e01304fdf 100644 --- a/packages/layout-engine/painters/dom/src/index.test.ts +++ b/packages/layout-engine/painters/dom/src/index.test.ts @@ -5324,6 +5324,7 @@ describe('DomPainter', () => { width: 260, height: 20, fragmentKind: 'list-item', + fragment: initialLayout.pages[0].fragments[0], blockId: 'list-1', fragmentIndex: 0, block: listBlock as import('@superdoc/contracts').ListBlock, @@ -5355,6 +5356,7 @@ describe('DomPainter', () => { width: 280, height: 20, fragmentKind: 'list-item', + fragment: updatedLayout.pages[0].fragments[0], blockId: 'list-1', fragmentIndex: 0, block: listBlock as import('@superdoc/contracts').ListBlock, @@ -5474,6 +5476,7 @@ describe('DomPainter', () => { height: 15, zIndex: 7, fragmentKind: 'drawing', + fragment: drawingLayout.pages[0].fragments[0], blockId: 'drawing-anchored', fragmentIndex: 0, block: anchoredDrawingBlock as import('@superdoc/contracts').DrawingBlock, @@ -5488,6 +5491,7 @@ describe('DomPainter', () => { height: 15, zIndex: 1, fragmentKind: 'drawing', + fragment: drawingLayout.pages[0].fragments[1], blockId: 'drawing-inline', fragmentIndex: 1, block: inlineDrawingBlock as import('@superdoc/contracts').DrawingBlock, @@ -5621,6 +5625,7 @@ describe('DomPainter', () => { const resolvedLayout = createSinglePageResolvedLayout({ kind: 'fragment', id: 'para:resolved-indent:0:2', + fragment: paragraphLayout.pages[0].fragments[0], pageIndex: 0, x: 30, y: 40, @@ -5716,6 +5721,7 @@ describe('DomPainter', () => { const resolvedLayout = createSinglePageResolvedLayout({ kind: 'fragment', id: 'para:resolved-marker:0:1', + fragment: paragraphLayout.pages[0].fragments[0], pageIndex: 0, x: 30, y: 40, @@ -5811,6 +5817,7 @@ describe('DomPainter', () => { const resolvedLayout = createSinglePageResolvedLayout({ kind: 'fragment', id: 'para:resolved-drop-cap:0:1', + fragment: paragraphLayout.pages[0].fragments[0], pageIndex: 0, x: 30, y: 40, diff --git a/packages/layout-engine/painters/dom/src/renderer.ts b/packages/layout-engine/painters/dom/src/renderer.ts index e6eec760eb..ed25021f12 100644 --- a/packages/layout-engine/painters/dom/src/renderer.ts +++ b/packages/layout-engine/painters/dom/src/renderer.ts @@ -1462,14 +1462,6 @@ export class DomPainter { return this.resolvedLayout?.pages[pageIndex] ?? null; } - /** Returns the resolved fragment item for a given page/fragment index, or undefined. */ - private getResolvedFragmentItem(pageIndex: number, fragmentIndex: number): ResolvedPaintItem | undefined { - const page = this.getResolvedPage(pageIndex); - if (!page) return undefined; - const item = page.items[fragmentIndex]; - return item?.kind === 'fragment' ? item : undefined; - } - /** * Returns the latest painter snapshot captured during the last paint cycle. */ @@ -2231,9 +2223,10 @@ export class DomPainter { const sdtBoundaries = computeSdtBoundaries(page.fragments, resolvedItems, this.sdtLabelsRendered); const betweenBorderFlags = computeBetweenBorderFlags(page.fragments, resolvedItems); - page.fragments.forEach((fragment, index) => { + resolvedItems.forEach((resolvedItem, index) => { + if (resolvedItem.kind !== 'fragment') return; + const fragment = resolvedItem.fragment; const sdtBoundary = sdtBoundaries.get(index); - const resolvedItem = this.getResolvedFragmentItem(pageIndex, index); el.appendChild( this.renderFragment(fragment, contextBase, sdtBoundary, betweenBorderFlags.get(index), resolvedItem), ); @@ -2798,12 +2791,13 @@ export class DomPainter { pageIndex, }; - page.fragments.forEach((fragment, index) => { + resolvedItems.forEach((resolvedItem, index) => { + if (resolvedItem.kind !== 'fragment') return; + const fragment = resolvedItem.fragment; const key = fragmentKey(fragment); const current = existing.get(key); const sdtBoundary = sdtBoundaries.get(index); const betweenInfo = betweenBorderFlags.get(index); - const resolvedItem = this.getResolvedFragmentItem(pageIndex, index); const resolvedSig = resolvedPaintCacheSignature(resolvedItem); if (current) { @@ -2958,9 +2952,10 @@ export class DomPainter { const resolvedItems = resolvedPage?.items ?? []; const sdtBoundaries = computeSdtBoundaries(page.fragments, resolvedItems, this.sdtLabelsRendered); const betweenBorderFlags = computeBetweenBorderFlags(page.fragments, resolvedItems); - const fragmentStates: FragmentDomState[] = page.fragments.map((fragment, index) => { + const fragmentStates: FragmentDomState[] = resolvedItems.flatMap((resolvedItem, index) => { + if (resolvedItem.kind !== 'fragment') return []; + const fragment = resolvedItem.fragment; const sdtBoundary = sdtBoundaries.get(index); - const resolvedItem = this.getResolvedFragmentItem(pageIndex, index); const fragmentEl = this.renderFragment( fragment, contextBase, @@ -2970,13 +2965,15 @@ export class DomPainter { ); el.appendChild(fragmentEl); const initSig = resolvedPaintCacheSignature(resolvedItem); - return { - key: fragmentKey(fragment), - signature: initSig, - fragment, - element: fragmentEl, - context: contextBase, - }; + return [ + { + key: fragmentKey(fragment), + signature: initSig, + fragment, + element: fragmentEl, + context: contextBase, + }, + ]; }); this.renderDecorationsForPage(el, page, pageIndex, resolvedPage); From db03d4deaff9291f47124491e15825d70fa5f384 Mon Sep 17 00:00:00 2001 From: Tadeu Tupinamba Date: Mon, 4 May 2026 12:07:18 -0300 Subject: [PATCH 06/13] refactor(painter): paint() body reads ResolvedLayout (SD-2836) Switch paint()'s top-level reads from input.sourceLayout to input.resolvedLayout for: layoutEpoch, page count, and the mounted-page indices. The local `layout = input.sourceLayout` binding stays for one more commit because the four legacy-Layout helpers (beginPaintSnapshot, fullRender, patchLayout, renderHorizontal, renderBookMode, renderVirtualized) still take a Layout argument. The next commit migrates those signatures and removes the binding entirely. --- .../painters/dom/src/renderer.ts | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/packages/layout-engine/painters/dom/src/renderer.ts b/packages/layout-engine/painters/dom/src/renderer.ts index ed25021f12..406feaa242 100644 --- a/packages/layout-engine/painters/dom/src/renderer.ts +++ b/packages/layout-engine/painters/dom/src/renderer.ts @@ -1636,8 +1636,13 @@ export class DomPainter { } public paint(input: DomPainterInput, mount: HTMLElement, mapping?: PositionMapping): void { + const resolvedLayout = input.resolvedLayout; + this.resolvedLayout = resolvedLayout; + // Local Layout-shaped binding kept until the four legacy-Layout methods + // (beginPaintSnapshot, fullRender, patchLayout, renderHorizontal, + // renderBookMode, renderVirtualized) migrate to ResolvedLayout in the + // following commit. After that, this binding goes away entirely. const layout = input.sourceLayout; - this.resolvedLayout = input.resolvedLayout; this.changedBlocks.clear(); if (!(mount instanceof HTMLElement)) { @@ -1683,11 +1688,11 @@ export class DomPainter { } this.layoutVersion += 1; - this.layoutEpoch = this.resolvedLayout?.layoutEpoch ?? layout.layoutEpoch ?? 0; + this.layoutEpoch = resolvedLayout.layoutEpoch ?? 0; this.mount = mount; this.beginPaintSnapshot(layout); - this.totalPages = layout.pages.length; + this.totalPages = resolvedLayout.pages.length; if (this.isSemanticFlow) { // Semantic mode always renders as a single continuous surface. applyStyles(mount, containerStyles); @@ -1698,7 +1703,7 @@ export class DomPainter { } else { this.patchLayout(layout); } - this.setMountedPageIndices(this.createAllPageIndices(layout.pages.length)); + this.setMountedPageIndices(this.createAllPageIndices(resolvedLayout.pages.length)); this.currentLayout = layout; this.changedBlocks.clear(); this.currentMapping = null; @@ -1713,7 +1718,7 @@ export class DomPainter { mount.style.gap = `${this.pageGap}px`; this.renderHorizontal(layout, mount); this.finalizePaintSnapshotFromBuilder(mount); - this.setMountedPageIndices(this.createAllPageIndices(layout.pages.length)); + this.setMountedPageIndices(this.createAllPageIndices(resolvedLayout.pages.length)); this.currentLayout = layout; this.pageStates = []; this.changedBlocks.clear(); @@ -1724,7 +1729,7 @@ export class DomPainter { applyStyles(mount, containerStyles); this.renderBookMode(layout, mount); this.finalizePaintSnapshotFromBuilder(mount); - this.setMountedPageIndices(this.createAllPageIndices(layout.pages.length)); + this.setMountedPageIndices(this.createAllPageIndices(resolvedLayout.pages.length)); this.currentLayout = layout; this.pageStates = []; this.changedBlocks.clear(); @@ -1752,7 +1757,7 @@ export class DomPainter { this.patchLayout(layout); useDomSnapshotFallback = true; } - this.setMountedPageIndices(this.createAllPageIndices(layout.pages.length)); + this.setMountedPageIndices(this.createAllPageIndices(resolvedLayout.pages.length)); } if (useDomSnapshotFallback) { From 796a5c3f92887e048b14b270c68e3b48a3f4ac8f Mon Sep 17 00:00:00 2001 From: Tadeu Tupinamba Date: Mon, 4 May 2026 12:12:11 -0300 Subject: [PATCH 07/13] refactor(painter): drop fragments param from sdt+border helpers (SD-2836) computeSdtBoundaries and computeBetweenBorderFlags previously took both the raw `fragments` array and the parallel resolvedItems array. They now take only resolvedItems and read each fragment via the back-pointer added in [PR1#4]. Updates all four call sites in renderer.ts plus the between-borders test fixture. This eliminates the painter's lookup into `page.fragments` from the helper-call layer, leaving only the deeper-method Layout dependency (beginPaintSnapshot, fullRender, patchLayout, renderHorizontal, renderBookMode, renderVirtualized) to migrate next. --- .../painters/dom/src/between-borders.test.ts | 3 +- .../paragraph-borders/group-analysis.ts | 23 +++++---- .../painters/dom/src/renderer.ts | 49 +++++++++++-------- 3 files changed, 44 insertions(+), 31 deletions(-) diff --git a/packages/layout-engine/painters/dom/src/between-borders.test.ts b/packages/layout-engine/painters/dom/src/between-borders.test.ts index 2218a18b0d..fbbfb97cdc 100644 --- a/packages/layout-engine/painters/dom/src/between-borders.test.ts +++ b/packages/layout-engine/painters/dom/src/between-borders.test.ts @@ -102,6 +102,7 @@ const buildResolvedItems = (fragments: readonly Fragment[], blocks: TestBlockLis width: fragment.width, height: 'height' in fragment && typeof fragment.height === 'number' ? fragment.height : 0, fragmentKind: fragment.kind, + fragment, blockId: fragment.blockId, fragmentIndex: index, paragraphBorders: borders, @@ -113,7 +114,7 @@ const buildResolvedItems = (fragments: readonly Fragment[], blocks: TestBlockLis /** Test helper: run computeBetweenBorderFlags given fragments and the underlying blocks. */ const runFlags = (fragments: readonly Fragment[], blocks: TestBlockList) => - computeBetweenBorderFlags(fragments, buildResolvedItems(fragments, blocks)); + computeBetweenBorderFlags(buildResolvedItems(fragments, blocks)); const paraFragment = (blockId: string, overrides?: Partial): ParaFragment => ({ kind: 'para', diff --git a/packages/layout-engine/painters/dom/src/features/paragraph-borders/group-analysis.ts b/packages/layout-engine/painters/dom/src/features/paragraph-borders/group-analysis.ts index caae556b0d..eaa1327e91 100644 --- a/packages/layout-engine/painters/dom/src/features/paragraph-borders/group-analysis.ts +++ b/packages/layout-engine/painters/dom/src/features/paragraph-borders/group-analysis.ts @@ -8,7 +8,7 @@ * @ooxml w:pPr/w:pBdr/w:between — between border for grouped paragraphs * @spec ECMA-376 §17.3.1.24 (pBdr) */ -import type { Fragment, ListItemFragment, ResolvedPaintItem, ResolvedFragmentItem } from '@superdoc/contracts'; +import type { ListItemFragment, ResolvedPaintItem, ResolvedFragmentItem } from '@superdoc/contracts'; import { hashParagraphBorders } from '../../paragraph-hash-utils.js'; /** @@ -69,23 +69,25 @@ function isResolvedFragmentWithBorders( * Middle fragments in a chain of 3+ get both flags. */ export const computeBetweenBorderFlags = ( - fragments: readonly Fragment[], resolvedItems: readonly ResolvedPaintItem[], ): Map => { // Phase 1: determine which consecutive pairs form between-border groups const pairFlags = new Set(); const noBetweenPairs = new Set(); - for (let i = 0; i < fragments.length - 1; i += 1) { - const frag = fragments[i]; + for (let i = 0; i < resolvedItems.length - 1; i += 1) { + const resolvedCur = resolvedItems[i]; + if (resolvedCur.kind !== 'fragment') continue; + const frag = resolvedCur.fragment; if (frag.kind !== 'para' && frag.kind !== 'list-item') continue; if (frag.continuesOnNext) continue; - const resolvedCur = resolvedItems[i]; if (!isResolvedFragmentWithBorders(resolvedCur)) continue; const borders = resolvedCur.paragraphBorders; - const next = fragments[i + 1]; + const resolvedNext = resolvedItems[i + 1]; + if (resolvedNext.kind !== 'fragment') continue; + const next = resolvedNext.fragment; if (next.kind !== 'para' && next.kind !== 'list-item') continue; if (next.continuesFromPrev) continue; if (next.blockId === frag.blockId && next.kind === 'para') continue; @@ -97,7 +99,6 @@ export const computeBetweenBorderFlags = ( ) continue; - const resolvedNext = resolvedItems[i + 1]; if (!isResolvedFragmentWithBorders(resolvedNext)) continue; const nextBorders = resolvedNext.paragraphBorders; @@ -129,10 +130,12 @@ export const computeBetweenBorderFlags = ( const result = new Map(); for (const i of pairFlags) { - const frag = fragments[i]; - const next = fragments[i + 1]; const resolvedCur = resolvedItems[i]; - const fragHeight = resolvedCur && 'height' in resolvedCur && resolvedCur.height != null ? resolvedCur.height : 0; + const resolvedNext = resolvedItems[i + 1]; + if (resolvedCur.kind !== 'fragment' || resolvedNext.kind !== 'fragment') continue; + const frag = resolvedCur.fragment; + const next = resolvedNext.fragment; + const fragHeight = 'height' in resolvedCur && resolvedCur.height != null ? resolvedCur.height : 0; const gapBelow = Math.max(0, next.y - (frag.y + fragHeight)); const isNoBetween = noBetweenPairs.has(i); diff --git a/packages/layout-engine/painters/dom/src/renderer.ts b/packages/layout-engine/painters/dom/src/renderer.ts index 406feaa242..53dff2ee53 100644 --- a/packages/layout-engine/painters/dom/src/renderer.ts +++ b/packages/layout-engine/painters/dom/src/renderer.ts @@ -2225,8 +2225,8 @@ export class DomPainter { }; const resolvedItems = resolvedPage?.items ?? []; - const sdtBoundaries = computeSdtBoundaries(page.fragments, resolvedItems, this.sdtLabelsRendered); - const betweenBorderFlags = computeBetweenBorderFlags(page.fragments, resolvedItems); + const sdtBoundaries = computeSdtBoundaries(resolvedItems, this.sdtLabelsRendered); + const betweenBorderFlags = computeBetweenBorderFlags(resolvedItems); resolvedItems.forEach((resolvedItem, index) => { if (resolvedItem.kind !== 'fragment') return; @@ -2592,7 +2592,7 @@ export class DomPainter { // Compute between-border flags for header/footer paragraph fragments const decorationItems = data.items ?? []; - const betweenBorderFlags = computeBetweenBorderFlags(data.fragments, decorationItems); + const betweenBorderFlags = computeBetweenBorderFlags(decorationItems); // Separate behindDoc fragments from normal fragments. // Prefer explicit fragment.behindDoc when present. Keep zIndex===0 as a @@ -2785,8 +2785,8 @@ export class DomPainter { const existing = new Map(state.fragments.map((frag) => [frag.key, frag])); const nextFragments: FragmentDomState[] = []; const resolvedItems = resolvedPage?.items ?? []; - const sdtBoundaries = computeSdtBoundaries(page.fragments, resolvedItems, this.sdtLabelsRendered); - const betweenBorderFlags = computeBetweenBorderFlags(page.fragments, resolvedItems); + const sdtBoundaries = computeSdtBoundaries(resolvedItems, this.sdtLabelsRendered); + const betweenBorderFlags = computeBetweenBorderFlags(resolvedItems); const contextBase: FragmentRenderContext = { pageNumber: page.number, @@ -2955,8 +2955,8 @@ export class DomPainter { }; const resolvedItems = resolvedPage?.items ?? []; - const sdtBoundaries = computeSdtBoundaries(page.fragments, resolvedItems, this.sdtLabelsRendered); - const betweenBorderFlags = computeBetweenBorderFlags(page.fragments, resolvedItems); + const sdtBoundaries = computeSdtBoundaries(resolvedItems, this.sdtLabelsRendered); + const betweenBorderFlags = computeBetweenBorderFlags(resolvedItems); const fragmentStates: FragmentDomState[] = resolvedItems.flatMap((resolvedItem, index) => { if (resolvedItem.kind !== 'fragment') return []; const fragment = resolvedItem.fragment; @@ -7266,13 +7266,11 @@ export class DomPainter { } const computeSdtBoundaries = ( - fragments: readonly Fragment[], resolvedItems: readonly ResolvedPaintItem[], sdtLabelsRendered: Set, ): Map => { const boundaries = new Map(); - const containerKeys: (string | null)[] = fragments.map((_frag, idx) => { - const item = resolvedItems[idx]; + const containerKeys: (string | null)[] = resolvedItems.map((item) => { if (item && 'sdtContainerKey' in item) { const key = (item as { sdtContainerKey?: string | null }).sdtContainerKey; return key ?? null; @@ -7280,38 +7278,49 @@ const computeSdtBoundaries = ( return null; }); + const fragmentOf = (idx: number): Fragment | null => { + const item = resolvedItems[idx]; + return item && item.kind === 'fragment' ? item.fragment : null; + }; + let i = 0; - while (i < fragments.length) { + while (i < resolvedItems.length) { const currentKey = containerKeys[i]; - if (!currentKey) { + const startFrag = fragmentOf(i); + if (!currentKey || !startFrag) { i += 1; continue; } - let groupRight = fragments[i].x + fragments[i].width; + let groupRight = startFrag.x + startFrag.width; let j = i; - while (j + 1 < fragments.length && containerKeys[j + 1] === currentKey) { + while (j + 1 < resolvedItems.length && containerKeys[j + 1] === currentKey) { j += 1; - const fragmentRight = fragments[j].x + fragments[j].width; + const nextFrag = fragmentOf(j); + if (!nextFrag) break; + const fragmentRight = nextFrag.x + nextFrag.width; if (fragmentRight > groupRight) { groupRight = fragmentRight; } } for (let k = i; k <= j; k += 1) { - const fragment = fragments[k]; + const fragment = fragmentOf(k); + if (!fragment) continue; const isStart = k === i; const isEnd = k === j; let paddingBottomOverride: number | undefined; if (!isEnd) { - const nextFragment = fragments[k + 1]; + const nextFragment = fragmentOf(k + 1); const currentHeight = (resolvedItems[k] as { height?: number } | undefined)?.height ?? 0; const currentBottom = fragment.y + currentHeight; - const gapToNext = nextFragment.y - currentBottom; - if (gapToNext > 0) { - paddingBottomOverride = gapToNext; + if (nextFragment) { + const gapToNext = nextFragment.y - currentBottom; + if (gapToNext > 0) { + paddingBottomOverride = gapToNext; + } } } From 119408612fc484efea72b142a7ce71895790cf2f Mon Sep 17 00:00:00 2001 From: Tadeu Tupinamba Date: Mon, 4 May 2026 12:42:53 -0300 Subject: [PATCH 08/13] refactor(painter): migrate internals to ResolvedLayout (SD-2836) Switch DomPainter's internal state and helpers from raw Layout/Page to ResolvedLayout/ResolvedPage: * The six top-level legacy-Layout methods (beginPaintSnapshot, fullRender, patchLayout, renderHorizontal, renderBookMode, renderVirtualized) take ResolvedLayout. paint() drops the `const layout = input.sourceLayout` binding and calls every method with input.resolvedLayout. * The page-level helpers (renderPage, createPageState, patchPage, renderDecorationsForPage, renderDecorationSection, getDecorationAnchorPageOriginY, renderPageRuler, renderColumnSeparators) take ResolvedPage and read width/height/ margins/numberText/etc. directly. Drops every redundant `resolvedPage?: ResolvedPage | null` parameter. * `currentLayout: Layout | null` -> `ResolvedLayout | null`. Strips the `(page.size ?? layout.pageSize)` cascades inside virtualization + page-iteration code; uses ResolvedPage.width/.height directly. * Lifts `columns` and `columnRegions` onto ResolvedPage so the column-separator renderer can read them without falling back to raw Page. Couples PageDecorationProvider (planned PR2 work) since the painter now passes ResolvedPage to the third callback argument: * `PageDecorationProvider`'s `page?` parameter is `ResolvedPage`. * `HeaderFooterSessionManager.rebuildRegions` takes ResolvedLayout. `updateDecorationProviders(layout, resolvedLayout)` plumbed through PresentationEditor. * The provider closure body and internal helpers (`#stripFootnoteReserveFromBottomMargin`, `#computeExpectedSectionType`) now operate on ResolvedPage; `page?.size?.h` -> `page?.height`. * `buildLegacyPaintInput` always calls `resolveLayout` so the legacy paint(Layout) path produces ResolvedPages even when no body blocks/measures are supplied (preserves page-level metadata). Skips a "decoration item synthesis" describe block that exercises `paint(Layout)` + `setData` + `setResolvedLayout`. Those legacy entry points get deleted in the next commit; the block is being preserved as .skip so the deletion is visible in diff. --- .../contracts/src/resolved-layout.ts | 6 + .../layout-resolved/src/resolveLayout.ts | 2 + .../painters/dom/src/index.test.ts | 2 +- .../layout-engine/painters/dom/src/index.ts | 5 +- .../painters/dom/src/renderer.ts | 175 +++++++----------- .../presentation-editor/PresentationEditor.ts | 7 +- .../HeaderFooterSessionManager.ts | 31 ++-- 7 files changed, 100 insertions(+), 128 deletions(-) diff --git a/packages/layout-engine/contracts/src/resolved-layout.ts b/packages/layout-engine/contracts/src/resolved-layout.ts index 55d5fbce7c..fb564a57e3 100644 --- a/packages/layout-engine/contracts/src/resolved-layout.ts +++ b/packages/layout-engine/contracts/src/resolved-layout.ts @@ -1,4 +1,6 @@ import type { + ColumnLayout, + ColumnRegion, DrawingBlock, FlowMode, Fragment, @@ -66,6 +68,10 @@ export type ResolvedPage = { }; /** Page orientation. */ orientation?: 'portrait' | 'landscape'; + /** Column layout configuration for this page (reflects page-start config). */ + columns?: ColumnLayout; + /** Vertical column regions when continuous section breaks change column layout mid-page. */ + columnRegions?: ColumnRegion[]; }; /** Union of all resolved paint item kinds. */ diff --git a/packages/layout-engine/layout-resolved/src/resolveLayout.ts b/packages/layout-engine/layout-resolved/src/resolveLayout.ts index 3d0b221198..28f63bb75b 100644 --- a/packages/layout-engine/layout-resolved/src/resolveLayout.ts +++ b/packages/layout-engine/layout-resolved/src/resolveLayout.ts @@ -308,6 +308,8 @@ export function resolveLayout(input: ResolveLayoutInput): ResolvedLayout { const pages: ResolvedPage[] = layout.pages.map((page, pageIndex) => ({ id: `page-${pageIndex}`, index: pageIndex, + columns: page.columns, + columnRegions: page.columnRegions, number: page.number, width: page.size?.w ?? layout.pageSize.w, height: page.size?.h ?? layout.pageSize.h, diff --git a/packages/layout-engine/painters/dom/src/index.test.ts b/packages/layout-engine/painters/dom/src/index.test.ts index 3e01304fdf..5b63e1640b 100644 --- a/packages/layout-engine/painters/dom/src/index.test.ts +++ b/packages/layout-engine/painters/dom/src/index.test.ts @@ -10858,7 +10858,7 @@ describe('applyRunDataAttributes', () => { }); }); - describe('decoration item synthesis', () => { + describe.skip('decoration item synthesis (legacy, deleted in PR2 of SD-2836)', () => { let mount: HTMLElement; beforeEach(() => { diff --git a/packages/layout-engine/painters/dom/src/index.ts b/packages/layout-engine/painters/dom/src/index.ts index ecaea9e6c6..a6567c59a9 100644 --- a/packages/layout-engine/painters/dom/src/index.ts +++ b/packages/layout-engine/painters/dom/src/index.ts @@ -235,9 +235,10 @@ function buildLegacyPaintInput( let resolvedLayout: ResolvedLayout; if (legacyState.resolvedLayout) { resolvedLayout = legacyState.resolvedLayout; - } else if (legacyState.blocks.length === 0 && legacyState.measures.length === 0) { - resolvedLayout = createEmptyResolvedLayout(flowMode, pageGap); } else { + // resolveLayout handles empty blocks/measures gracefully and preserves + // page-level metadata (size, margins, columns/columnRegions, etc.) needed + // by the painter even when no body content has been provided yet. resolvedLayout = resolveLayout({ layout, flowMode: flowMode ?? 'paginated', diff --git a/packages/layout-engine/painters/dom/src/renderer.ts b/packages/layout-engine/painters/dom/src/renderer.ts index 53dff2ee53..35a09afbe6 100644 --- a/packages/layout-engine/painters/dom/src/renderer.ts +++ b/packages/layout-engine/painters/dom/src/renderer.ts @@ -291,13 +291,13 @@ export type PageDecorationPayload = { * * @param {number} pageNumber - The page number (1-indexed) * @param {PageMargins} [pageMargins] - Page margin configuration - * @param {Page} [page] - Full page object from the layout + * @param {ResolvedPage} [page] - Resolved page from the layout * @returns {PageDecorationPayload | null} Decoration payload containing fragments and layout info, or null if no decoration */ export type PageDecorationProvider = ( pageNumber: number, pageMargins?: PageMargins, - page?: Page, + page?: ResolvedPage, ) => PageDecorationPayload | null; /** @@ -1291,7 +1291,7 @@ export class DomPainter { private mount: HTMLElement | null = null; private doc: Document | null = null; private pageStates: PageDomState[] = []; - private currentLayout: Layout | null = null; + private currentLayout: ResolvedLayout | null = null; private changedBlocks = new Set(); private readonly layoutMode: LayoutMode; private readonly isSemanticFlow: boolean; @@ -1492,7 +1492,7 @@ export class DomPainter { this.onPaintSnapshotCallback?.(snapshot); } - private beginPaintSnapshot(layout: Layout): void { + private beginPaintSnapshot(layout: ResolvedLayout): void { this.paintSnapshotBuilder = { formatVersion: 1, lineCount: 0, @@ -1638,11 +1638,6 @@ export class DomPainter { public paint(input: DomPainterInput, mount: HTMLElement, mapping?: PositionMapping): void { const resolvedLayout = input.resolvedLayout; this.resolvedLayout = resolvedLayout; - // Local Layout-shaped binding kept until the four legacy-Layout methods - // (beginPaintSnapshot, fullRender, patchLayout, renderHorizontal, - // renderBookMode, renderVirtualized) migrate to ResolvedLayout in the - // following commit. After that, this binding goes away entirely. - const layout = input.sourceLayout; this.changedBlocks.clear(); if (!(mount instanceof HTMLElement)) { @@ -1690,7 +1685,7 @@ export class DomPainter { this.layoutEpoch = resolvedLayout.layoutEpoch ?? 0; this.mount = mount; - this.beginPaintSnapshot(layout); + this.beginPaintSnapshot(resolvedLayout); this.totalPages = resolvedLayout.pages.length; if (this.isSemanticFlow) { @@ -1699,12 +1694,12 @@ export class DomPainter { mount.style.gap = '0px'; mount.style.alignItems = 'stretch'; if (!this.currentLayout || this.pageStates.length === 0) { - this.fullRender(layout); + this.fullRender(resolvedLayout); } else { - this.patchLayout(layout); + this.patchLayout(resolvedLayout); } this.setMountedPageIndices(this.createAllPageIndices(resolvedLayout.pages.length)); - this.currentLayout = layout; + this.currentLayout = resolvedLayout; this.changedBlocks.clear(); this.currentMapping = null; return; @@ -1716,10 +1711,10 @@ export class DomPainter { applyStyles(mount, containerStylesHorizontal); // Use configured page gap for horizontal rendering mount.style.gap = `${this.pageGap}px`; - this.renderHorizontal(layout, mount); + this.renderHorizontal(resolvedLayout, mount); this.finalizePaintSnapshotFromBuilder(mount); this.setMountedPageIndices(this.createAllPageIndices(resolvedLayout.pages.length)); - this.currentLayout = layout; + this.currentLayout = resolvedLayout; this.pageStates = []; this.changedBlocks.clear(); this.currentMapping = null; @@ -1727,10 +1722,10 @@ export class DomPainter { } if (mode === 'book') { applyStyles(mount, containerStyles); - this.renderBookMode(layout, mount); + this.renderBookMode(resolvedLayout, mount); this.finalizePaintSnapshotFromBuilder(mount); this.setMountedPageIndices(this.createAllPageIndices(resolvedLayout.pages.length)); - this.currentLayout = layout; + this.currentLayout = resolvedLayout; this.pageStates = []; this.changedBlocks.clear(); this.currentMapping = null; @@ -1743,18 +1738,18 @@ export class DomPainter { if (this.virtualEnabled) { // Keep container gap at 0 so spacer elements don't introduce extra offsets. mount.style.gap = '0px'; - this.renderVirtualized(layout, mount); + this.renderVirtualized(resolvedLayout, mount); useDomSnapshotFallback = true; - this.currentLayout = layout; + this.currentLayout = resolvedLayout; this.changedBlocks.clear(); this.currentMapping = null; } else { // Use configured page gap for normal vertical rendering mount.style.gap = `${this.pageGap}px`; if (!this.currentLayout || this.pageStates.length === 0) { - this.fullRender(layout); + this.fullRender(resolvedLayout); } else { - this.patchLayout(layout); + this.patchLayout(resolvedLayout); useDomSnapshotFallback = true; } this.setMountedPageIndices(this.createAllPageIndices(resolvedLayout.pages.length)); @@ -1767,7 +1762,7 @@ export class DomPainter { this.finalizePaintSnapshotFromBuilder(mount); } - this.currentLayout = layout; + this.currentLayout = resolvedLayout; this.changedBlocks.clear(); this.currentMapping = null; } @@ -1775,7 +1770,7 @@ export class DomPainter { // ---------------- // Virtualized path // ---------------- - private renderVirtualized(layout: Layout, mount: HTMLElement): void { + private renderVirtualized(layout: ResolvedLayout, mount: HTMLElement): void { if (!this.doc) return; // Always keep the latest layout reference for handlers this.currentLayout = layout; @@ -1884,10 +1879,7 @@ export class DomPainter { if (!this.currentLayout) return; const N = this.currentLayout.pages.length; if (N !== this.virtualHeights.length) { - this.virtualHeights = this.currentLayout.pages.map((p, i) => { - const resolved = this.getResolvedPage(i); - return resolved?.height ?? p.size?.h ?? this.currentLayout!.pageSize.h; - }); + this.virtualHeights = this.currentLayout.pages.map((p) => p.height); } // Build offsets where offsets[i] = sum_{k < i} (height[k] + gap). // Use virtualGap to match CSS gap on virtualPagesEl. @@ -2046,8 +2038,7 @@ export class DomPainter { // Insert or patch needed pages for (const i of mounted) { const page = layout.pages[i]; - const resolved = this.getResolvedPage(i); - const pageSize = resolved ? { w: resolved.width, h: resolved.height } : (page.size ?? layout.pageSize); + const pageSize = { w: page.width, h: page.height }; const existing = this.pageIndexToState.get(i); if (!existing) { const newState = this.createPageState(page, pageSize, i); @@ -2144,28 +2135,23 @@ export class DomPainter { this.virtualGapSpacers = []; } - private renderHorizontal(layout: Layout, mount: HTMLElement): void { + private renderHorizontal(layout: ResolvedLayout, mount: HTMLElement): void { if (!this.doc) return; mount.innerHTML = ''; layout.pages.forEach((page, pageIndex) => { - const resolved = this.getResolvedPage(pageIndex); - const pageSize = resolved ? { w: resolved.width, h: resolved.height } : (page.size ?? layout.pageSize); - const pageEl = this.renderPage(pageSize.w, pageSize.h, page, pageIndex); + const pageEl = this.renderPage(page.width, page.height, page, pageIndex); mount.appendChild(pageEl); }); } - private renderBookMode(layout: Layout, mount: HTMLElement): void { + private renderBookMode(layout: ResolvedLayout, mount: HTMLElement): void { if (!this.doc) return; mount.innerHTML = ''; const pages = layout.pages; if (pages.length === 0) return; - const firstResolved = this.getResolvedPage(0); - const firstPageSize = firstResolved - ? { w: firstResolved.width, h: firstResolved.height } - : (pages[0].size ?? layout.pageSize); - const firstPageEl = this.renderPage(firstPageSize.w, firstPageSize.h, pages[0], 0); + const firstPage = pages[0]; + const firstPageEl = this.renderPage(firstPage.width, firstPage.height, firstPage, 0); mount.appendChild(firstPageEl); for (let i = 1; i < pages.length; i += 2) { @@ -2174,20 +2160,12 @@ export class DomPainter { applyStyles(spreadEl, spreadStyles); const leftPage = pages[i]; - const leftResolved = this.getResolvedPage(i); - const leftPageSize = leftResolved - ? { w: leftResolved.width, h: leftResolved.height } - : (leftPage.size ?? layout.pageSize); - const leftPageEl = this.renderPage(leftPageSize.w, leftPageSize.h, leftPage, i); + const leftPageEl = this.renderPage(leftPage.width, leftPage.height, leftPage, i); spreadEl.appendChild(leftPageEl); if (i + 1 < pages.length) { const rightPage = pages[i + 1]; - const rightResolved = this.getResolvedPage(i + 1); - const rightPageSize = rightResolved - ? { w: rightResolved.width, h: rightResolved.height } - : (rightPage.size ?? layout.pageSize); - const rightPageEl = this.renderPage(rightPageSize.w, rightPageSize.h, rightPage, i + 1); + const rightPageEl = this.renderPage(rightPage.width, rightPage.height, rightPage, i + 1); spreadEl.appendChild(rightPageEl); } @@ -2195,11 +2173,10 @@ export class DomPainter { } } - private renderPage(width: number, height: number, page: Page, pageIndex: number): HTMLElement { + private renderPage(width: number, height: number, page: ResolvedPage, pageIndex: number): HTMLElement { if (!this.doc) { throw new Error('DomPainter: document is not available'); } - const resolvedPage = this.getResolvedPage(pageIndex); const el = this.doc.createElement('div'); el.classList.add(CLASS_NAMES.page); applyStyles(el, pageStyles(width, height, this.getEffectivePageStyles())); @@ -2210,7 +2187,7 @@ export class DomPainter { // Render per-page ruler if enabled (suppressed in semantic flow mode) if (!this.isSemanticFlow && this.options.ruler?.enabled) { - const rulerEl = this.renderPageRuler(width, page, resolvedPage); + const rulerEl = this.renderPageRuler(width, page); if (rulerEl) { el.appendChild(rulerEl); } @@ -2220,11 +2197,11 @@ export class DomPainter { pageNumber: page.number, totalPages: this.totalPages, section: 'body', - pageNumberText: resolvedPage?.numberText ?? page.numberText, + pageNumberText: page.numberText, pageIndex, }; - const resolvedItems = resolvedPage?.items ?? []; + const resolvedItems = page.items; const sdtBoundaries = computeSdtBoundaries(resolvedItems, this.sdtLabelsRendered); const betweenBorderFlags = computeBetweenBorderFlags(resolvedItems); @@ -2236,8 +2213,8 @@ export class DomPainter { this.renderFragment(fragment, contextBase, sdtBoundary, betweenBorderFlags.get(index), resolvedItem), ); }); - this.renderDecorationsForPage(el, page, pageIndex, resolvedPage); - this.renderColumnSeparators(el, page, width, height, resolvedPage); + this.renderDecorationsForPage(el, page, pageIndex); + this.renderColumnSeparators(el, page, width, height); return el; } @@ -2259,13 +2236,13 @@ export class DomPainter { * - Uses DEFAULT_PAGE_HEIGHT_PX (1056px = 11 inches) if page.size.h is not available * - Defaults margins to 0 if not explicitly provided */ - private renderPageRuler(pageWidthPx: number, page: Page, resolvedPage?: ResolvedPage | null): HTMLElement | null { + private renderPageRuler(pageWidthPx: number, page: ResolvedPage): HTMLElement | null { if (!this.doc) { console.warn('[renderPageRuler] Cannot render ruler: document is not available.'); return null; } - const margins = resolvedPage?.margins ?? page.margins; + const margins = page.margins; if (!margins) { console.warn(`[renderPageRuler] Cannot render ruler for page ${page.number}: margins not available.`); return null; @@ -2277,7 +2254,7 @@ export class DomPainter { try { const rulerDefinition = generateRulerDefinitionFromPx({ pageWidthPx, - pageHeightPx: page.size?.h ?? DEFAULT_PAGE_HEIGHT_PX, + pageHeightPx: page.height ?? DEFAULT_PAGE_HEIGHT_PX, leftMarginPx: leftMargin, rightMarginPx: rightMargin, }); @@ -2318,17 +2295,11 @@ export class DomPainter { } } - private renderColumnSeparators( - pageEl: HTMLElement, - page: Page, - pageWidth: number, - pageHeight: number, - resolvedPage?: ResolvedPage | null, - ): void { + private renderColumnSeparators(pageEl: HTMLElement, page: ResolvedPage, pageWidth: number, pageHeight: number): void { if (!this.doc) return; pageEl.querySelectorAll('[data-superdoc-column-separator="true"]').forEach((separator) => separator.remove()); - const pageMargins = resolvedPage?.margins ?? page.margins; + const pageMargins = page.margins; if (!pageMargins) return; const leftMargin = pageMargins.left ?? 0; @@ -2424,15 +2395,10 @@ export class DomPainter { return separatorPositions; } - private renderDecorationsForPage( - pageEl: HTMLElement, - page: Page, - pageIndex: number, - resolvedPage?: ResolvedPage | null, - ): void { + private renderDecorationsForPage(pageEl: HTMLElement, page: ResolvedPage, pageIndex: number): void { if (this.isSemanticFlow) return; - this.renderDecorationSection(pageEl, page, pageIndex, 'header', resolvedPage); - this.renderDecorationSection(pageEl, page, pageIndex, 'footer', resolvedPage); + this.renderDecorationSection(pageEl, page, pageIndex, 'header'); + this.renderDecorationSection(pageEl, page, pageIndex, 'footer'); } /** @@ -2463,21 +2429,17 @@ export class DomPainter { */ private getDecorationAnchorPageOriginY( pageEl: HTMLElement, - page: Page, + page: ResolvedPage, kind: 'header' | 'footer', effectiveOffset: number, - resolvedPage?: ResolvedPage | null, ): number { if (kind === 'header') { return effectiveOffset; } - const pageMargins = resolvedPage?.margins ?? page.margins; + const pageMargins = page.margins; const styledPageHeight = Number.parseFloat(pageEl.style.height || ''); - const pageHeight = - page.size?.h ?? - this.currentLayout?.pageSize?.h ?? - (Number.isFinite(styledPageHeight) ? styledPageHeight : pageEl.clientHeight); + const pageHeight = page.height ?? (Number.isFinite(styledPageHeight) ? styledPageHeight : pageEl.clientHeight); const footerDistance = pageMargins?.footer; if (typeof footerDistance === 'number' && Number.isFinite(footerDistance)) { @@ -2489,7 +2451,7 @@ export class DomPainter { return effectiveOffset; } - const footnoteReserve = resolvedPage?.footnoteReserved ?? page.footnoteReserved ?? 0; + const footnoteReserve = page.footnoteReserved ?? 0; const adjustedBottomMargin = Math.max(0, bottomMargin - footnoteReserve); return Math.max(0, pageHeight - adjustedBottomMargin); @@ -2497,16 +2459,14 @@ export class DomPainter { private renderDecorationSection( pageEl: HTMLElement, - page: Page, + page: ResolvedPage, pageIndex: number, kind: 'header' | 'footer', - resolvedPage?: ResolvedPage | null, ): void { if (!this.doc) return; const provider = kind === 'header' ? this.headerProvider : this.footerProvider; const className = kind === 'header' ? CLASS_NAMES.pageHeader : CLASS_NAMES.pageFooter; const existing = pageEl.querySelector(`.${className}`); - // Provider still receives legacy page — its signature is not changed in this PR const data = provider ? provider(page.number, page.margins, page) : null; if (!data || data.fragments.length === 0) { @@ -2519,7 +2479,7 @@ export class DomPainter { container.innerHTML = ''; const baseOffset = data.offset ?? (kind === 'footer' ? pageEl.clientHeight - data.height : 0); const marginLeft = data.marginLeft ?? 0; - const pageMargins = resolvedPage?.margins ?? page.margins; + const pageMargins = page.margins; const marginRight = pageMargins?.right ?? 0; // For footers, if content is taller than reserved space, expand container upward @@ -2560,7 +2520,7 @@ export class DomPainter { // Header page-relative anchors use raw inner-layout Y and are handled with // the simpler effectiveOffset subtraction (unchanged from the baseline). const footerAnchorPageOriginY = - kind === 'footer' ? this.getDecorationAnchorPageOriginY(pageEl, page, kind, effectiveOffset, resolvedPage) : 0; + kind === 'footer' ? this.getDecorationAnchorPageOriginY(pageEl, page, kind, effectiveOffset) : 0; const footerAnchorContainerOffsetY = kind === 'footer' ? footerAnchorPageOriginY - effectiveOffset : 0; // For footers, calculate offset to push content to bottom of container @@ -2586,7 +2546,7 @@ export class DomPainter { pageNumber: page.number, totalPages: this.totalPages, section: kind, - pageNumberText: resolvedPage?.numberText ?? page.numberText, + pageNumberText: page.numberText, pageIndex, }; @@ -2727,14 +2687,13 @@ export class DomPainter { this.mountedPageIndices = []; } - private fullRender(layout: Layout): void { + private fullRender(layout: ResolvedLayout): void { if (!this.mount || !this.doc) return; this.mount.innerHTML = ''; this.pageStates = []; layout.pages.forEach((page, pageIndex) => { - const resolved = this.getResolvedPage(pageIndex); - const pageSize = resolved ? { w: resolved.width, h: resolved.height } : (page.size ?? layout.pageSize); + const pageSize = { w: page.width, h: page.height }; const pageState = this.createPageState(page, pageSize, pageIndex); pageState.element.dataset.pageNumber = String(page.number); pageState.element.dataset.pageIndex = String(pageIndex); @@ -2743,14 +2702,13 @@ export class DomPainter { }); } - private patchLayout(layout: Layout): void { + private patchLayout(layout: ResolvedLayout): void { if (!this.mount || !this.doc) return; const nextStates: PageDomState[] = []; layout.pages.forEach((page, index) => { - const resolved = this.getResolvedPage(index); - const pageSize = resolved ? { w: resolved.width, h: resolved.height } : (page.size ?? layout.pageSize); + const pageSize = { w: page.width, h: page.height }; const prevState = this.pageStates[index]; if (!prevState) { const newState = this.createPageState(page, pageSize, index); @@ -2773,8 +2731,12 @@ export class DomPainter { this.pageStates = nextStates; } - private patchPage(state: PageDomState, page: Page, pageSize: { w: number; h: number }, pageIndex: number): void { - const resolvedPage = this.getResolvedPage(pageIndex); + private patchPage( + state: PageDomState, + page: ResolvedPage, + pageSize: { w: number; h: number }, + pageIndex: number, + ): void { const pageEl = state.element; applyStyles(pageEl, pageStyles(pageSize.w, pageSize.h, this.getEffectivePageStyles())); this.applySemanticPageOverrides(pageEl); @@ -2784,7 +2746,7 @@ export class DomPainter { const existing = new Map(state.fragments.map((frag) => [frag.key, frag])); const nextFragments: FragmentDomState[] = []; - const resolvedItems = resolvedPage?.items ?? []; + const resolvedItems = page.items; const sdtBoundaries = computeSdtBoundaries(resolvedItems, this.sdtLabelsRendered); const betweenBorderFlags = computeBetweenBorderFlags(resolvedItems); @@ -2792,7 +2754,7 @@ export class DomPainter { pageNumber: page.number, totalPages: this.totalPages, section: 'body', - pageNumberText: resolvedPage?.numberText ?? page.numberText, + pageNumberText: page.numberText, pageIndex, }; @@ -2874,8 +2836,8 @@ export class DomPainter { }); state.fragments = nextFragments; - this.renderDecorationsForPage(pageEl, page, pageIndex, resolvedPage); - this.renderColumnSeparators(pageEl, page, pageSize.w, pageSize.h, resolvedPage); + this.renderDecorationsForPage(pageEl, page, pageIndex); + this.renderColumnSeparators(pageEl, page, pageSize.w, pageSize.h); } /** @@ -2935,11 +2897,10 @@ export class DomPainter { } } - private createPageState(page: Page, pageSize: { w: number; h: number }, pageIndex: number): PageDomState { + private createPageState(page: ResolvedPage, pageSize: { w: number; h: number }, pageIndex: number): PageDomState { if (!this.doc) { throw new Error('DomPainter.createPageState requires a document'); } - const resolvedPage = this.getResolvedPage(pageIndex); const el = this.doc.createElement('div'); el.classList.add(CLASS_NAMES.page); applyStyles(el, pageStyles(pageSize.w, pageSize.h, this.getEffectivePageStyles())); @@ -2950,11 +2911,11 @@ export class DomPainter { pageNumber: page.number, totalPages: this.totalPages, section: 'body', - pageNumberText: resolvedPage?.numberText ?? page.numberText, + pageNumberText: page.numberText, pageIndex, }; - const resolvedItems = resolvedPage?.items ?? []; + const resolvedItems = page.items; const sdtBoundaries = computeSdtBoundaries(resolvedItems, this.sdtLabelsRendered); const betweenBorderFlags = computeBetweenBorderFlags(resolvedItems); const fragmentStates: FragmentDomState[] = resolvedItems.flatMap((resolvedItem, index) => { @@ -2981,8 +2942,8 @@ export class DomPainter { ]; }); - this.renderDecorationsForPage(el, page, pageIndex, resolvedPage); - this.renderColumnSeparators(el, page, pageSize.w, pageSize.h, resolvedPage); + this.renderDecorationsForPage(el, page, pageIndex); + this.renderColumnSeparators(el, page, pageSize.w, pageSize.h); return { element: el, fragments: fragmentStates }; } diff --git a/packages/super-editor/src/editors/v1/core/presentation-editor/PresentationEditor.ts b/packages/super-editor/src/editors/v1/core/presentation-editor/PresentationEditor.ts index ebd67cab4f..5c4a46eeb1 100644 --- a/packages/super-editor/src/editors/v1/core/presentation-editor/PresentationEditor.ts +++ b/packages/super-editor/src/editors/v1/core/presentation-editor/PresentationEditor.ts @@ -131,6 +131,7 @@ import type { Layout, Measure, Page, + ResolvedLayout, SectionMetadata, TrackedChangesMode, Fragment, @@ -6188,7 +6189,7 @@ export class PresentationEditor extends EventEmitter { // Process per-rId header/footer content and decoration providers (paginated only) if (!isSemanticFlow) { await this.#layoutPerRIdHeaderFooters(headerFooterInput, layout, sectionMetadata); - this.#updateDecorationProviders(layout); + this.#updateDecorationProviders(layout, resolvedLayout); } this.#ensurePainter(); @@ -7371,8 +7372,8 @@ export class PresentationEditor extends EventEmitter { * Update decoration providers for header/footer. * Delegates to HeaderFooterSessionManager which handles provider creation. */ - #updateDecorationProviders(layout: Layout) { - this.#headerFooterSession?.updateDecorationProviders(layout); + #updateDecorationProviders(layout: Layout, resolvedLayout: ResolvedLayout) { + this.#headerFooterSession?.updateDecorationProviders(layout, resolvedLayout); } /** diff --git a/packages/super-editor/src/editors/v1/core/presentation-editor/header-footer/HeaderFooterSessionManager.ts b/packages/super-editor/src/editors/v1/core/presentation-editor/header-footer/HeaderFooterSessionManager.ts index 3cda734b30..9c05a63fc5 100644 --- a/packages/super-editor/src/editors/v1/core/presentation-editor/header-footer/HeaderFooterSessionManager.ts +++ b/packages/super-editor/src/editors/v1/core/presentation-editor/header-footer/HeaderFooterSessionManager.ts @@ -19,7 +19,7 @@ import type { SectionMetadata, Fragment, ResolvedHeaderFooterLayout, - ResolvedPaintItem, + ResolvedPaintItem, ResolvedLayout, ResolvedPage } from '@superdoc/contracts'; import type { PageDecorationProvider } from '@superdoc/painter-dom'; import { resolveHeaderFooterLayout } from '@superdoc/layout-resolved'; @@ -733,19 +733,20 @@ export class HeaderFooterSessionManager { // =========================================================================== /** - * Rebuild header/footer regions from layout. + * Rebuild header/footer regions from the resolved layout. */ - rebuildRegions(layout: Layout): void { + rebuildRegions(resolvedLayout: ResolvedLayout): void { this.#headerRegions.clear(); this.#footerRegions.clear(); const layoutOptions = this.#deps?.getLayoutOptions() ?? {}; - const pageHeight = layout.pageSize?.h ?? layoutOptions.pageSize?.h ?? this.#options.defaultPageSize.h; - if (pageHeight <= 0) return; + const fallbackPageHeight = + resolvedLayout.pages[0]?.height ?? layoutOptions.pageSize?.h ?? this.#options.defaultPageSize.h; + if (fallbackPageHeight <= 0) return; // Build section first page numbers map const sectionFirstPageNumbers = new Map(); - for (const p of layout.pages) { + for (const p of resolvedLayout.pages) { const idx = p.sectionIndex ?? 0; if (!sectionFirstPageNumbers.has(idx)) { sectionFirstPageNumbers.set(idx, p.number); @@ -757,9 +758,9 @@ export class HeaderFooterSessionManager { const defaultMargins = this.#options.defaultMargins; - layout.pages.forEach((page, pageIndex) => { + resolvedLayout.pages.forEach((page, pageIndex) => { const margins = page.margins ?? layoutOptions.margins ?? defaultMargins; - const actualPageHeight = page.size?.h ?? pageHeight; + const actualPageHeight = page.height ?? fallbackPageHeight; const sectionIndex = page.sectionIndex ?? 0; const sectionId = sectionIdBySectionIndex.get(sectionIndex) ?? `section-${sectionIndex}`; @@ -1685,7 +1686,7 @@ export class HeaderFooterSessionManager { #computeExpectedSectionType( kind: 'header' | 'footer', - page: Page, + page: ResolvedPage, sectionFirstPageNumbers: Map, ): string { const pageNumber = page.number; @@ -1714,10 +1715,10 @@ export class HeaderFooterSessionManager { #stripFootnoteReserveFromBottomMargin( margins: HeaderFooterLayoutOptions['margins'], - page: Page, + page: ResolvedPage | null, ): HeaderFooterLayoutOptions['margins'] { // Note: property is 'footnoteReserved' (with 'd') as defined in @superdoc/contracts - const footnoteReserved = page.footnoteReserved ?? 0; + const footnoteReserved = page?.footnoteReserved ?? 0; if (footnoteReserved <= 0) return margins; const currentBottom = margins?.bottom ?? this.#options.defaultMargins.bottom ?? 0; @@ -2262,10 +2263,10 @@ export class HeaderFooterSessionManager { * Update decoration providers for header and footer. * Creates new providers based on layout results and sets them on this manager. */ - updateDecorationProviders(layout: Layout): void { + updateDecorationProviders(layout: Layout, resolvedLayout: ResolvedLayout): void { this.#headerDecorationProvider = this.createDecorationProvider('header', layout); this.#footerDecorationProvider = this.createDecorationProvider('footer', layout); - this.rebuildRegions(layout); + this.rebuildRegions(resolvedLayout); } private resolveAlignedDecorationItems( @@ -2398,7 +2399,7 @@ export class HeaderFooterSessionManager { resolvedLayout, `rId '${rIdLayoutKey}' page ${pageNumber}`, ); - const pageHeight = page?.size?.h ?? layout.pageSize?.h ?? layoutOptions.pageSize?.h ?? defaultPageSize.h; + const pageHeight = page?.height ?? layout.pageSize?.h ?? layoutOptions.pageSize?.h ?? defaultPageSize.h; const margins = pageMargins ?? layout.pages[0]?.margins ?? layoutOptions.margins ?? defaultMargins; const decorationMargins = kind === 'footer' ? this.#stripFootnoteReserveFromBottomMargin(margins, page ?? null) : margins; @@ -2459,7 +2460,7 @@ export class HeaderFooterSessionManager { `variant '${headerFooterType}' page ${pageNumber}`, ); - const pageHeight = page?.size?.h ?? layout.pageSize?.h ?? layoutOptions.pageSize?.h ?? defaultPageSize.h; + const pageHeight = page?.height ?? layout.pageSize?.h ?? layoutOptions.pageSize?.h ?? defaultPageSize.h; const margins = pageMargins ?? layout.pages[0]?.margins ?? layoutOptions.margins ?? defaultMargins; const decorationMargins = kind === 'footer' ? this.#stripFootnoteReserveFromBottomMargin(margins, page ?? null) : margins; From 5eb2fd719a0aacec77abbd94fecc184bc6a99d95 Mon Sep 17 00:00:00 2001 From: Tadeu Tupinamba Date: Mon, 4 May 2026 14:48:00 -0300 Subject: [PATCH 09/13] chore(deps): regenerate lockfile after painter-dom dep cleanup (SD-2836) Drops the now-unused @superdoc/layout-bridge and @superdoc/pm-adapter entries from pnpm-lock.yaml so CI's --frozen-lockfile install matches package.json. --- pnpm-lock.yaml | 6 ------ 1 file changed, 6 deletions(-) diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 0b0fefa0cb..df5e23aaab 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -2542,15 +2542,9 @@ importers: '@superdoc/font-utils': specifier: workspace:* version: link:../../../../shared/font-utils - '@superdoc/layout-bridge': - specifier: workspace:* - version: link:../../layout-bridge '@superdoc/layout-resolved': specifier: workspace:* version: link:../../layout-resolved - '@superdoc/pm-adapter': - specifier: workspace:* - version: link:../../pm-adapter '@superdoc/preset-geometry': specifier: workspace:* version: link:../../../preset-geometry From 58cf4b43fdc0c23fe4473670c063e1d5ca33ebc0 Mon Sep 17 00:00:00 2001 From: Tadeu Tupinamba Date: Mon, 4 May 2026 13:44:32 -0300 Subject: [PATCH 10/13] test(super-editor): synthesize ResolvedLayout in PresentationEditor mock (SD-2836) rebuildRegions now iterates resolvedLayout.pages (was layout.pages). The PresentationEditor test mocked resolveLayout to return empty pages, which caused the header/footer region tests to populate zero regions and bookmark navigation to fail. The mock now synthesizes a minimal ResolvedPage per source Layout page so region tests stay green. --- .../tests/PresentationEditor.test.ts | 29 ++++++++++++++++++- 1 file changed, 28 insertions(+), 1 deletion(-) diff --git a/packages/super-editor/src/editors/v1/core/presentation-editor/tests/PresentationEditor.test.ts b/packages/super-editor/src/editors/v1/core/presentation-editor/tests/PresentationEditor.test.ts index c67d57260b..7f64d8f50d 100644 --- a/packages/super-editor/src/editors/v1/core/presentation-editor/tests/PresentationEditor.test.ts +++ b/packages/super-editor/src/editors/v1/core/presentation-editor/tests/PresentationEditor.test.ts @@ -182,7 +182,34 @@ const { getActiveEditorHost: vi.fn(() => null), destroy: vi.fn(), })), - mockResolveLayout: vi.fn(() => ({ version: 1, flowMode: 'paginated', pageGap: 0, pages: [] })), + // SD-2836: rebuildRegions now iterates resolvedLayout.pages, so the mock + // must synthesize a ResolvedPage per source Layout page to keep header/footer + // region tests from going empty. + mockResolveLayout: vi.fn( + ({ layout }: { layout: { pages: Array>; pageSize: { w: number; h: number } } }) => ({ + version: 1, + flowMode: 'paginated', + pageGap: 0, + pages: (layout?.pages ?? []).map((p, i) => ({ + id: `page-${i}`, + index: i, + number: (p.number as number) ?? i + 1, + width: ((p.size as { w?: number } | undefined)?.w ?? layout.pageSize?.w) as number, + height: ((p.size as { h?: number } | undefined)?.h ?? layout.pageSize?.h) as number, + items: [], + margins: p.margins, + sectionRefs: p.sectionRefs, + sectionIndex: p.sectionIndex, + numberText: p.numberText, + footnoteReserved: p.footnoteReserved, + vAlign: p.vAlign, + baseMargins: p.baseMargins, + orientation: p.orientation, + columns: p.columns, + columnRegions: p.columnRegions, + })), + }), + ), mockFlowBlockCacheInstances, MockFlowBlockCache, }; From 3e734662608e89b2c5e8bc060ca66936da2c556e Mon Sep 17 00:00:00 2001 From: Tadeu Tupinamba Date: Tue, 5 May 2026 11:18:09 -0300 Subject: [PATCH 11/13] refactor(painter,super-editor): address PR review feedback (SD-2836) - Drop redundant pageSize parameter from createPageState/patchPage; read page.width/height directly from ResolvedPage. - Migrate createDecorationProvider to ResolvedLayout; updateDecorationProviders no longer needs the legacy Layout parameter. - Add direct rebuildRegions(resolvedLayout) tests covering footnoteReserved>0, per-page height variation, and sectionIndex>0. - Assert columns and columnRegions forward through to ResolvedPage in resolveLayout tests. --- .../layout-resolved/src/resolveLayout.test.ts | 15 ++ .../painters/dom/src/renderer.ts | 32 ++--- .../presentation-editor/PresentationEditor.ts | 6 +- .../HeaderFooterSessionManager.ts | 31 ++-- .../tests/HeaderFooterSessionManager.test.ts | 134 ++++++++++++++++-- 5 files changed, 172 insertions(+), 46 deletions(-) diff --git a/packages/layout-engine/layout-resolved/src/resolveLayout.test.ts b/packages/layout-engine/layout-resolved/src/resolveLayout.test.ts index e792834ecb..3b1a4667a9 100644 --- a/packages/layout-engine/layout-resolved/src/resolveLayout.test.ts +++ b/packages/layout-engine/layout-resolved/src/resolveLayout.test.ts @@ -66,6 +66,21 @@ describe('resolveLayout', () => { expect(result.pages[2].height).toBe(1600); }); + it('forwards page-level columns and columnRegions onto ResolvedPage', () => { + const columns = { count: 2, gap: 24, withSeparator: true } as const; + const columnRegions = [ + { yStart: 0, yEnd: 400, columns: { count: 1, gap: 0 } }, + { yStart: 400, yEnd: 1000, columns: { count: 3, gap: 12 } }, + ] as const; + const layout: Layout = { + pageSize: { w: 800, h: 1000 }, + pages: [{ number: 1, fragments: [], columns, columnRegions: [...columnRegions] }], + }; + const result = resolveLayout({ layout, flowMode: 'paginated', blocks: [], measures: [] }); + expect(result.pages[0].columns).toEqual(columns); + expect(result.pages[0].columnRegions).toEqual(columnRegions); + }); + it('falls back to layout.pageSize when page.size is undefined', () => { const layout: Layout = { pageSize: { w: 612, h: 792 }, diff --git a/packages/layout-engine/painters/dom/src/renderer.ts b/packages/layout-engine/painters/dom/src/renderer.ts index 35a09afbe6..3a80e25752 100644 --- a/packages/layout-engine/painters/dom/src/renderer.ts +++ b/packages/layout-engine/painters/dom/src/renderer.ts @@ -2038,19 +2038,18 @@ export class DomPainter { // Insert or patch needed pages for (const i of mounted) { const page = layout.pages[i]; - const pageSize = { w: page.width, h: page.height }; const existing = this.pageIndexToState.get(i); if (!existing) { - const newState = this.createPageState(page, pageSize, i); + const newState = this.createPageState(page, i); newState.element.dataset.pageNumber = String(page.number); newState.element.dataset.pageIndex = String(i); // Ensure virtualization uses page margin 0 - applyStyles(newState.element, pageStyles(pageSize.w, pageSize.h, this.getEffectivePageStyles())); + applyStyles(newState.element, pageStyles(page.width, page.height, this.getEffectivePageStyles())); this.virtualPagesEl.appendChild(newState.element); this.pageIndexToState.set(i, newState); } else { // Patch in place - this.patchPage(existing, page, pageSize, i); + this.patchPage(existing, page, i); } } @@ -2693,8 +2692,7 @@ export class DomPainter { this.pageStates = []; layout.pages.forEach((page, pageIndex) => { - const pageSize = { w: page.width, h: page.height }; - const pageState = this.createPageState(page, pageSize, pageIndex); + const pageState = this.createPageState(page, pageIndex); pageState.element.dataset.pageNumber = String(page.number); pageState.element.dataset.pageIndex = String(pageIndex); this.mount!.appendChild(pageState.element); @@ -2708,17 +2706,16 @@ export class DomPainter { const nextStates: PageDomState[] = []; layout.pages.forEach((page, index) => { - const pageSize = { w: page.width, h: page.height }; const prevState = this.pageStates[index]; if (!prevState) { - const newState = this.createPageState(page, pageSize, index); + const newState = this.createPageState(page, index); newState.element.dataset.pageNumber = String(page.number); newState.element.dataset.pageIndex = String(index); this.mount!.insertBefore(newState.element, this.mount!.children[index] ?? null); nextStates.push(newState); return; } - this.patchPage(prevState, page, pageSize, index); + this.patchPage(prevState, page, index); nextStates.push(prevState); }); @@ -2731,14 +2728,9 @@ export class DomPainter { this.pageStates = nextStates; } - private patchPage( - state: PageDomState, - page: ResolvedPage, - pageSize: { w: number; h: number }, - pageIndex: number, - ): void { + private patchPage(state: PageDomState, page: ResolvedPage, pageIndex: number): void { const pageEl = state.element; - applyStyles(pageEl, pageStyles(pageSize.w, pageSize.h, this.getEffectivePageStyles())); + applyStyles(pageEl, pageStyles(page.width, page.height, this.getEffectivePageStyles())); this.applySemanticPageOverrides(pageEl); pageEl.dataset.pageNumber = String(page.number); pageEl.dataset.layoutEpoch = String(this.layoutEpoch); @@ -2837,7 +2829,7 @@ export class DomPainter { state.fragments = nextFragments; this.renderDecorationsForPage(pageEl, page, pageIndex); - this.renderColumnSeparators(pageEl, page, pageSize.w, pageSize.h); + this.renderColumnSeparators(pageEl, page, page.width, page.height); } /** @@ -2897,13 +2889,13 @@ export class DomPainter { } } - private createPageState(page: ResolvedPage, pageSize: { w: number; h: number }, pageIndex: number): PageDomState { + private createPageState(page: ResolvedPage, pageIndex: number): PageDomState { if (!this.doc) { throw new Error('DomPainter.createPageState requires a document'); } const el = this.doc.createElement('div'); el.classList.add(CLASS_NAMES.page); - applyStyles(el, pageStyles(pageSize.w, pageSize.h, this.getEffectivePageStyles())); + applyStyles(el, pageStyles(page.width, page.height, this.getEffectivePageStyles())); this.applySemanticPageOverrides(el); el.dataset.layoutEpoch = String(this.layoutEpoch); @@ -2943,7 +2935,7 @@ export class DomPainter { }); this.renderDecorationsForPage(el, page, pageIndex); - this.renderColumnSeparators(el, page, pageSize.w, pageSize.h); + this.renderColumnSeparators(el, page, page.width, page.height); return { element: el, fragments: fragmentStates }; } diff --git a/packages/super-editor/src/editors/v1/core/presentation-editor/PresentationEditor.ts b/packages/super-editor/src/editors/v1/core/presentation-editor/PresentationEditor.ts index 5c4a46eeb1..0d35be5d89 100644 --- a/packages/super-editor/src/editors/v1/core/presentation-editor/PresentationEditor.ts +++ b/packages/super-editor/src/editors/v1/core/presentation-editor/PresentationEditor.ts @@ -6189,7 +6189,7 @@ export class PresentationEditor extends EventEmitter { // Process per-rId header/footer content and decoration providers (paginated only) if (!isSemanticFlow) { await this.#layoutPerRIdHeaderFooters(headerFooterInput, layout, sectionMetadata); - this.#updateDecorationProviders(layout, resolvedLayout); + this.#updateDecorationProviders(resolvedLayout); } this.#ensurePainter(); @@ -7372,8 +7372,8 @@ export class PresentationEditor extends EventEmitter { * Update decoration providers for header/footer. * Delegates to HeaderFooterSessionManager which handles provider creation. */ - #updateDecorationProviders(layout: Layout, resolvedLayout: ResolvedLayout) { - this.#headerFooterSession?.updateDecorationProviders(layout, resolvedLayout); + #updateDecorationProviders(resolvedLayout: ResolvedLayout) { + this.#headerFooterSession?.updateDecorationProviders(resolvedLayout); } /** diff --git a/packages/super-editor/src/editors/v1/core/presentation-editor/header-footer/HeaderFooterSessionManager.ts b/packages/super-editor/src/editors/v1/core/presentation-editor/header-footer/HeaderFooterSessionManager.ts index 9c05a63fc5..5fa7025f3e 100644 --- a/packages/super-editor/src/editors/v1/core/presentation-editor/header-footer/HeaderFooterSessionManager.ts +++ b/packages/super-editor/src/editors/v1/core/presentation-editor/header-footer/HeaderFooterSessionManager.ts @@ -19,7 +19,9 @@ import type { SectionMetadata, Fragment, ResolvedHeaderFooterLayout, - ResolvedPaintItem, ResolvedLayout, ResolvedPage + ResolvedPaintItem, + ResolvedLayout, + ResolvedPage, } from '@superdoc/contracts'; import type { PageDecorationProvider } from '@superdoc/painter-dom'; import { resolveHeaderFooterLayout } from '@superdoc/layout-resolved'; @@ -2263,9 +2265,9 @@ export class HeaderFooterSessionManager { * Update decoration providers for header and footer. * Creates new providers based on layout results and sets them on this manager. */ - updateDecorationProviders(layout: Layout, resolvedLayout: ResolvedLayout): void { - this.#headerDecorationProvider = this.createDecorationProvider('header', layout); - this.#footerDecorationProvider = this.createDecorationProvider('footer', layout); + updateDecorationProviders(resolvedLayout: ResolvedLayout): void { + this.#headerDecorationProvider = this.createDecorationProvider('header', resolvedLayout); + this.#footerDecorationProvider = this.createDecorationProvider('footer', resolvedLayout); this.rebuildRegions(resolvedLayout); } @@ -2304,7 +2306,10 @@ export class HeaderFooterSessionManager { /** * Create a decoration provider for header or footer rendering. */ - createDecorationProvider(kind: 'header' | 'footer', layout: Layout): PageDecorationProvider | undefined { + createDecorationProvider( + kind: 'header' | 'footer', + resolvedLayout: ResolvedLayout, + ): PageDecorationProvider | undefined { const results = kind === 'header' ? this.#headerLayoutResults : this.#footerLayoutResults; const layoutsByRId = kind === 'header' ? this.#headerLayoutsByRId : this.#footerLayoutsByRId; const resolvedResults = kind === 'header' ? this.#resolvedHeaderLayouts : this.#resolvedFooterLayouts; @@ -2325,7 +2330,7 @@ export class HeaderFooterSessionManager { // Build section first page map const sectionFirstPageNumbers = new Map(); - for (const p of layout.pages) { + for (const p of resolvedLayout.pages) { const idx = p.sectionIndex ?? 0; if (!sectionFirstPageNumbers.has(idx)) { sectionFirstPageNumbers.set(idx, p.number); @@ -2391,16 +2396,17 @@ export class HeaderFooterSessionManager { const slotPage = this.#findPageForNumber(rIdLayout.layout.pages, pageNumber); if (slotPage) { const fragments = slotPage.fragments ?? []; - const resolvedLayout = resolvedByRId.get(rIdLayoutKey); + const rIdResolvedLayout = resolvedByRId.get(rIdLayoutKey); const alignedItems = this.resolveAlignedDecorationItems( fragments, slotPage.number, rIdLayout, - resolvedLayout, + rIdResolvedLayout, `rId '${rIdLayoutKey}' page ${pageNumber}`, ); - const pageHeight = page?.height ?? layout.pageSize?.h ?? layoutOptions.pageSize?.h ?? defaultPageSize.h; - const margins = pageMargins ?? layout.pages[0]?.margins ?? layoutOptions.margins ?? defaultMargins; + const pageHeight = + page?.height ?? resolvedLayout.pages[0]?.height ?? layoutOptions.pageSize?.h ?? defaultPageSize.h; + const margins = pageMargins ?? resolvedLayout.pages[0]?.margins ?? layoutOptions.margins ?? defaultMargins; const decorationMargins = kind === 'footer' ? this.#stripFootnoteReserveFromBottomMargin(margins, page ?? null) : margins; const box = this.#computeDecorationBox(kind, decorationMargins, pageHeight); @@ -2460,8 +2466,9 @@ export class HeaderFooterSessionManager { `variant '${headerFooterType}' page ${pageNumber}`, ); - const pageHeight = page?.height ?? layout.pageSize?.h ?? layoutOptions.pageSize?.h ?? defaultPageSize.h; - const margins = pageMargins ?? layout.pages[0]?.margins ?? layoutOptions.margins ?? defaultMargins; + const pageHeight = + page?.height ?? resolvedLayout.pages[0]?.height ?? layoutOptions.pageSize?.h ?? defaultPageSize.h; + const margins = pageMargins ?? resolvedLayout.pages[0]?.margins ?? layoutOptions.margins ?? defaultMargins; const decorationMargins = kind === 'footer' ? this.#stripFootnoteReserveFromBottomMargin(margins, page ?? null) : margins; const box = this.#computeDecorationBox(kind, decorationMargins, pageHeight); diff --git a/packages/super-editor/src/editors/v1/core/presentation-editor/tests/HeaderFooterSessionManager.test.ts b/packages/super-editor/src/editors/v1/core/presentation-editor/tests/HeaderFooterSessionManager.test.ts index 85382e00e2..4c447fceb7 100644 --- a/packages/super-editor/src/editors/v1/core/presentation-editor/tests/HeaderFooterSessionManager.test.ts +++ b/packages/super-editor/src/editors/v1/core/presentation-editor/tests/HeaderFooterSessionManager.test.ts @@ -14,7 +14,15 @@ vi.mock('../../header-footer/HeaderFooterPerRidLayout.js', () => ({ })); import type { Editor } from '../../Editor.js'; -import type { FlowBlock, HeaderFooterLayout, Layout, Measure, ParaFragment } from '@superdoc/contracts'; +import type { + FlowBlock, + HeaderFooterLayout, + Layout, + Measure, + ParaFragment, + ResolvedLayout, + ResolvedPage, +} from '@superdoc/contracts'; import type { HeaderFooterLayoutResult } from '@superdoc/layout-bridge'; import { HeaderFooterSessionManager, @@ -618,9 +626,9 @@ describe('HeaderFooterSessionManager', () => { pageSize: { w: 612, h: 792 }, pages: [{ number: 1, margins: { top: 72, right: 72, bottom: 72, left: 72, header: 36, footer: 36 } } as never], } as unknown as Layout; - const provider = manager.createDecorationProvider('header', layout); + const provider = manager.createDecorationProvider('header', layout as unknown as ResolvedLayout); expect(provider).toBeDefined(); - const payload = provider!(1, layout.pages[0]!.margins, layout.pages[0]); + const payload = provider!(1, layout.pages[0]!.margins, layout.pages[0] as unknown as ResolvedPage); expect(payload).not.toBeNull(); expect(payload!.fragments).toHaveLength(1); expect(payload!.items).toBeDefined(); @@ -678,8 +686,8 @@ describe('HeaderFooterSessionManager', () => { pageSize: { w: 612, h: 792 }, pages: [{ number: 1, margins: { top: 72, right: 72, bottom: 72, left: 72, header: 36, footer: 36 } } as never], } as unknown as Layout; - const provider = manager.createDecorationProvider('header', layout); - const payload = provider!(1, layout.pages[0]!.margins, layout.pages[0]); + const provider = manager.createDecorationProvider('header', layout as unknown as ResolvedLayout); + const payload = provider!(1, layout.pages[0]!.margins, layout.pages[0] as unknown as ResolvedPage); expect(payload).not.toBeNull(); expect(payload!.fragments).toHaveLength(2); @@ -727,8 +735,8 @@ describe('HeaderFooterSessionManager', () => { pageSize: { w: 612, h: 792 }, pages: [{ number: 1, margins: { top: 72, right: 72, bottom: 72, left: 72, header: 36, footer: 36 } } as never], } as unknown as Layout; - const provider = manager.createDecorationProvider('header', layout); - const payload = provider!(1, layout.pages[0]!.margins, layout.pages[0]); + const provider = manager.createDecorationProvider('header', layout as unknown as ResolvedLayout); + const payload = provider!(1, layout.pages[0]!.margins, layout.pages[0] as unknown as ResolvedPage); expect(payload).not.toBeNull(); expect(payload!.fragments[0]!.y).toBe(0); @@ -813,8 +821,8 @@ describe('HeaderFooterSessionManager', () => { [{ sectionIndex: 0 } as never], ); - const provider = manager.createDecorationProvider('header', layout); - const payload = provider!(1, layout.pages[0]!.margins, layout.pages[0]); + const provider = manager.createDecorationProvider('header', layout as unknown as ResolvedLayout); + const payload = provider!(1, layout.pages[0]!.margins, layout.pages[0] as unknown as ResolvedPage); expect(mockLayoutPerRIdHeaderFooters).toHaveBeenCalledTimes(1); expect(payload).not.toBeNull(); @@ -912,8 +920,8 @@ describe('HeaderFooterSessionManager', () => { width: 468, }); - const provider = manager.createDecorationProvider('header', layout); - const payload = provider!(1, layout.pages[0]!.margins, layout.pages[0]); + const provider = manager.createDecorationProvider('header', layout as unknown as ResolvedLayout); + const payload = provider!(1, layout.pages[0]!.margins, layout.pages[0] as unknown as ResolvedPage); expect(payload).not.toBeNull(); expect(payload!.fragments).toHaveLength(2); @@ -922,4 +930,108 @@ describe('HeaderFooterSessionManager', () => { expect(payload!.items!.every((item) => item.blockId === 'p1')).toBe(true); }); }); + + describe('rebuildRegions — ResolvedLayout entry', () => { + function buildManager(): HeaderFooterSessionManager { + const deps: SessionManagerDependencies = { + getLayoutOptions: vi.fn(() => ({})), + getPageElement: vi.fn(() => null), + scrollPageIntoView: vi.fn(), + waitForPageMount: vi.fn(async () => true), + convertPageLocalToOverlayCoords: vi.fn(() => ({ x: 0, y: 0 })), + isViewLocked: vi.fn(() => false), + getBodyPageHeight: vi.fn(() => 800), + notifyInputBridgeTargetChanged: vi.fn(), + scheduleRerender: vi.fn(), + setPendingDocChange: vi.fn(), + getBodyPageCount: vi.fn(() => 1), + }; + + const m = new HeaderFooterSessionManager({ + painterHost, + visibleHost, + selectionOverlay, + editor: createMainEditorStub(), + defaultPageSize: { w: 612, h: 792 }, + defaultMargins: { top: 72, right: 72, bottom: 72, left: 72, header: 36, footer: 36 }, + }); + m.setDependencies(deps); + return m; + } + + function makePage(overrides: Partial & { number: number; height: number }): ResolvedPage { + return { + id: `page-${overrides.number - 1}`, + index: overrides.number - 1, + width: 612, + items: [], + margins: { top: 72, right: 72, bottom: 72, left: 72, header: 36, footer: 36 }, + ...overrides, + } as ResolvedPage; + } + + it('shrinks footer height by footnoteReserved and shifts its offset upward', () => { + manager = buildManager(); + const layout: ResolvedLayout = { + version: 1, + flowMode: 'paginated', + pageGap: 0, + pages: [makePage({ number: 1, height: 792 }), makePage({ number: 2, height: 792, footnoteReserved: 24 })], + }; + + manager.rebuildRegions(layout); + + // Page 1: untouched. height = bottom - footer = 72 - 36 = 36; offset = 792 - 72 = 720. + const baseline = manager.footerRegions.get(0)!; + expect(baseline.height).toBe(36); + expect(baseline.localY).toBe(720); + + // Page 2: bottom shrinks to 72 - 24 = 48. height = 48 - 36 = 12; offset = 792 - 48 = 744. + const reserved = manager.footerRegions.get(1)!; + expect(reserved.height).toBe(12); + expect(reserved.localY).toBe(744); + }); + + it('honors per-page height variation when computing footer offsets', () => { + manager = buildManager(); + const layout: ResolvedLayout = { + version: 1, + flowMode: 'paginated', + pageGap: 0, + pages: [ + makePage({ number: 1, height: 792 }), + makePage({ number: 2, height: 1000 }), + makePage({ number: 3, height: 1400 }), + ], + }; + + manager.rebuildRegions(layout); + + // offset = pageHeight - bottom margin (72) + expect(manager.footerRegions.get(0)!.localY).toBe(792 - 72); + expect(manager.footerRegions.get(1)!.localY).toBe(1000 - 72); + expect(manager.footerRegions.get(2)!.localY).toBe(1400 - 72); + }); + + it('propagates sectionIndex from ResolvedPage onto built regions', () => { + manager = buildManager(); + const layout: ResolvedLayout = { + version: 1, + flowMode: 'paginated', + pageGap: 0, + pages: [ + makePage({ number: 1, height: 792, sectionIndex: 0 }), + makePage({ number: 2, height: 792, sectionIndex: 1 }), + makePage({ number: 3, height: 792, sectionIndex: 1 }), + ], + }; + + manager.rebuildRegions(layout); + + expect(manager.headerRegions.get(0)!.sectionIndex).toBe(0); + expect(manager.headerRegions.get(1)!.sectionIndex).toBe(1); + expect(manager.headerRegions.get(2)!.sectionIndex).toBe(1); + expect(manager.footerRegions.get(2)!.sectionIndex).toBe(1); + }); + }); }); From dba57df6570304c4ffe46019d6a5119a1f207555 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tadeu=20Tupinamb=C3=A1?= Date: Tue, 5 May 2026 14:43:18 -0300 Subject: [PATCH 12/13] [2/3] refactor(painter): collapse legacy API surface (SD-2836) (#3117) * refactor(painter): collapse legacy API surface (SD-2836) Make ResolvedLayout the only painter input contract: * DomPainterInput collapses to `{ resolvedLayout: ResolvedLayout }`. sourceLayout, blocks/measures, headerBlocks/Measures, footerBlocks/Measures all removed. * DomPainterOptions: drop blocks/measures. * DomPainterHandle: drop setData, setResolvedLayout. paint takes only DomPainterInput. Drops the `paint(Layout)` overload across painter, PresentationPainterAdapter, and (transitively) PresentationEditor's paintInput. * createDomPainter shrinks to a thin pass-through. Removes buildLegacyPaintInput, normalizeDomPainterInput, isDomPainterInput, wrapProvider, resolveDecorationItems, normalizeOptionalBlockMeasurePair, assertRequiredBlockMeasurePair, createEmptyResolvedLayout, LegacyDomPainterState, OptionalBlockMeasurePair. * PageDecorationPayload.items becomes required (the synthesis path is gone). Tests: * New `_test-utils.ts` exposes a legacy-shaped `createTestPainter` that test files import in place of `createDomPainter`. The helper builds a real DomPainterInput internally and synthesizes decoration items so existing test bodies don't have to be rewritten. * All 17 painter test files migrated to the helper. * Two source-anchor tests still failing under investigation; remaining work is bounded and tracked. * fix(painter): chain user onPaintSnapshot in test utility (SD-2836) createTestPainter was overwriting the user-supplied onPaintSnapshot callback with its own, so tests that relied on the callback (source-anchor tests) saw an undefined snapshot. Chain the user callback after the internal one. * test(pm-adapter): migrate integration tests to ResolvedLayout (SD-2836) The three integration tests in pm-adapter were calling painter.paint(layout, mount) with raw Layout. They now resolveLayout() first and call painter.paint({ resolvedLayout }, mount). All 1794 pm-adapter tests pass. * test(layout-bridge): migrate perf benchmark to ResolvedLayout (SD-2836) The layout-bridge incremental-pipeline performance benchmark called painter.paint(layout, mount) and painter.setData(...). Both are removed by the API collapse. Migrate to resolveLayout() + DomPainterInput so the benchmark continues to exercise the painter under the new contract. * chore(deps): declare @superdoc/layout-resolved as devDep where tests use it (SD-2836) PR1 added test imports of @superdoc/layout-resolved in pm-adapter/src/integration.test.ts and layout-bridge/test/benchmarks/index.ts without declaring it as a devDependency. Both packages now resolve the import locally via pnpm. * fix(super-editor): honor PageDecorationPayload.items contract (SD-2836) When resolveAlignedDecorationItems cannot align items 1:1 with fragments (returns undefined), HeaderFooterSessionManager now bails out with null instead of emitting a payload with items: undefined, which would violate the now-required PageDecorationPayload.items contract from PR2. normalizeDecorationItems narrowed to ResolvedPaintItem[] -> ResolvedPaintItem[]. Also refresh painter-dom README: drop blocks/measures/setData/paint(layout) examples; document the ResolvedLayout-only paint() and the items-aligned- with-fragments invariant on PageDecorationPayload. * [3/3] test(painter): lock ResolvedLayout-only boundary (SD-2836) (#3118) * test(painter): lock ResolvedLayout-only boundary (SD-2836) * chore(painter): drop unused imports and skipped legacy tests (SD-2836) renderer.ts: drop unused Layout, Page, Measure, FlowBlock, ParagraphBorder type imports left over from the migration. index.test.ts: delete the skipped 'decoration item synthesis' describe block (it was protecting the synthesis path that has been removed). * chore(deps): regenerate lockfile after dropping layout-resolved runtime dep (SD-2836) Moves @superdoc/layout-resolved to devDependencies in the lockfile to match package.json, so CI's --frozen-lockfile install matches. Boundary tests still need it under devDependencies. --- packages/layout-engine/AGENTS.md | 12 +- .../layout-engine/layout-bridge/package.json | 1 + .../layout-bridge/test/benchmarks/index.ts | 24 +- packages/layout-engine/painters/dom/README.md | 21 +- .../layout-engine/painters/dom/package.json | 1 - .../painters/dom/src/_test-utils.ts | 141 ++++++++++ .../painters/dom/src/between-borders.test.ts | 2 +- .../src/clip-path-cache-invalidation.test.ts | 2 +- .../painters/dom/src/contract-shape.test.ts | 41 +++ .../painters/dom/src/index.test.ts | 198 +------------ .../layout-engine/painters/dom/src/index.ts | 264 +----------------- .../painters/dom/src/link-click.test.ts | 2 +- .../src/renderer-column-separators.test.ts | 2 +- .../dom/src/renderer-dispatch.test.ts | 2 +- .../dom/src/renderer-hanging-indent.test.ts | 2 +- .../src/renderer-known-divergences.test.ts | 2 +- .../dom/src/renderer-marker-suffix.test.ts | 2 +- .../dom/src/renderer-marker-textwidth.test.ts | 2 +- .../dom/src/renderer-parity-contracts.test.ts | 2 +- .../src/renderer-shape-regressions.test.ts | 2 +- .../renderer-vector-shape-geometry.test.ts | 2 +- .../painters/dom/src/renderer.ts | 30 +- .../painters/dom/src/source-anchor.test.ts | 2 +- .../dom/src/text-style-rendering.test.ts | 2 +- .../painters/dom/src/virtualization.test.ts | 51 +--- .../layout-engine/pm-adapter/package.json | 1 + .../pm-adapter/src/integration.test.ts | 16 +- .../tests/src/architecture-boundaries.test.ts | 43 +++ .../presentation-editor/PresentationEditor.ts | 1 - .../HeaderFooterSessionManager.ts | 13 +- .../rendering/PresentationPainterAdapter.ts | 2 +- pnpm-lock.yaml | 12 +- 32 files changed, 340 insertions(+), 560 deletions(-) create mode 100644 packages/layout-engine/painters/dom/src/_test-utils.ts create mode 100644 packages/layout-engine/painters/dom/src/contract-shape.test.ts diff --git a/packages/layout-engine/AGENTS.md b/packages/layout-engine/AGENTS.md index 4ec58703c2..cfcc5f29cf 100644 --- a/packages/layout-engine/AGENTS.md +++ b/packages/layout-engine/AGENTS.md @@ -22,8 +22,16 @@ ProseMirror Doc → pm-adapter → FlowBlock[] → layout-engine → Layout[] ## Key Insight: DomPainter is "Dumb" -DomPainter receives pre-computed `Layout` with positioned fragments and renders them. -It does NOT do layout logic - that's in `layout-engine/`. +DomPainter receives a single paint-ready input — `ResolvedLayout` — with +positioned fragments, pre-resolved styles, and `fragment` back-pointers on +every `ResolvedPaintItem` — and renders the result to DOM. It does NOT do +layout logic, measurement, or PM-adapter conversion (that's upstream in +`layout-engine/` / `layout-resolved/` / `pm-adapter/`). + +The painter has zero runtime imports from `@superdoc/pm-adapter`, +`@superdoc/layout-bridge`, or `@superdoc/layout-resolved`. Architecture +boundary tests in `tests/src/architecture-boundaries.test.ts` (Guard D) +enforce this. ## Common Tasks diff --git a/packages/layout-engine/layout-bridge/package.json b/packages/layout-engine/layout-bridge/package.json index 3adebeb0f7..b21462d905 100644 --- a/packages/layout-engine/layout-bridge/package.json +++ b/packages/layout-engine/layout-bridge/package.json @@ -29,6 +29,7 @@ "@superdoc/word-layout": "workspace:*" }, "devDependencies": { + "@superdoc/layout-resolved": "workspace:*", "@superdoc/painter-dom": "workspace:*", "@superdoc/pm-adapter": "workspace:*", "@types/node": "catalog:", diff --git a/packages/layout-engine/layout-bridge/test/benchmarks/index.ts b/packages/layout-engine/layout-bridge/test/benchmarks/index.ts index d3db048972..ec7a812b39 100644 --- a/packages/layout-engine/layout-bridge/test/benchmarks/index.ts +++ b/packages/layout-engine/layout-bridge/test/benchmarks/index.ts @@ -3,6 +3,7 @@ import type { FlowBlock, Layout, ParagraphBlock, ParagraphMeasure, Run } from '@ import type { LayoutOptions } from '@superdoc/layout-engine'; import { measureBlock } from '@superdoc/measuring-dom'; import { createDomPainter } from '@superdoc/painter-dom'; +import { resolveLayout } from '@superdoc/layout-resolved'; import { layoutDocument } from '@superdoc/layout-engine'; import { incrementalLayout, measureCache, resolveMeasurementConstraints } from '../../src/incrementalLayout'; @@ -88,11 +89,19 @@ export async function runBenchmarkScenario(config: BenchmarkConfig): Promise { + const resolvedLayout = resolveLayout({ + layout, + flowMode: 'paginated', + blocks: painterBlocks, + measures: painterMeasures, + }); + painter.paint({ resolvedLayout }, mount); + }; + paintLayout(initial.layout); previousBlocks = doc.blocks; previousLayout = initial.layout; @@ -111,8 +120,9 @@ export async function runBenchmarkScenario(config: BenchmarkConfig): Promise { + const decorationBlocks = kind === 'header' ? headerBlocks : footerBlocks; + const decorationMeasures = kind === 'header' ? headerMeasures : footerMeasures; + const mergedBlocks = [...(currentBlocks ?? []), ...(decorationBlocks ?? [])]; + const mergedMeasures = [...(currentMeasures ?? []), ...(decorationMeasures ?? [])]; + if (mergedBlocks.length !== mergedMeasures.length || mergedBlocks.length === 0) { + return undefined; + } + const fakeLayout: Layout = { pageSize: { w: 400, h: 500 }, pages: [{ number: 1, fragments: [...fragments] }] }; + try { + const resolved = resolveLayout({ + layout: fakeLayout, + flowMode: opts.flowMode ?? 'paginated', + blocks: mergedBlocks, + measures: mergedMeasures, + }); + return resolved.pages[0]?.items; + } catch { + return undefined; + } + }; + + const wrapProvider = ( + provider: PageDecorationProvider | undefined, + kind: 'header' | 'footer', + ): PageDecorationProvider | undefined => { + if (!provider) return undefined; + return (pageNumber, pageMargins, page) => { + const payload = provider(pageNumber, pageMargins, page); + if (!payload) return payload; + if (payload.items) return payload; + const items = resolveDecorationItems(payload.fragments, kind); + return items ? { ...payload, items } : { ...payload, items: [] }; + }; + }; + + const userOnPaintSnapshot = painterOpts.onPaintSnapshot; + const painter = createDomPainter({ + ...painterOpts, + headerProvider: wrapProvider(headerProvider, 'header'), + footerProvider: wrapProvider(footerProvider, 'footer'), + onPaintSnapshot: (snapshot) => { + lastPaintSnapshot = snapshot; + userOnPaintSnapshot?.(snapshot); + }, + }); + + return { + paint(layout: Layout, mount: HTMLElement, mapping?: unknown) { + const effectiveResolved = resolvedLayoutOverridden + ? currentResolved + : resolveLayout({ + layout, + flowMode: opts.flowMode ?? 'paginated', + blocks: currentBlocks, + measures: currentMeasures, + }); + const input: DomPainterInput = { + resolvedLayout: effectiveResolved, + }; + painter.paint(input, mount, mapping as never); + }, + setData( + blocks: FlowBlock[], + measures: Measure[], + hb?: FlowBlock[], + hm?: Measure[], + fb?: FlowBlock[], + fm?: Measure[], + ) { + currentBlocks = blocks; + currentMeasures = measures; + headerBlocks = hb; + headerMeasures = hm; + footerBlocks = fb; + footerMeasures = fm; + }, + setResolvedLayout(rl: ResolvedLayout | null) { + currentResolved = rl ?? emptyResolved; + resolvedLayoutOverridden = true; + }, + setProviders(header?: PageDecorationProvider, footer?: PageDecorationProvider) { + painter.setProviders(wrapProvider(header, 'header'), wrapProvider(footer, 'footer')); + }, + setVirtualizationPins(pageIndices: number[] | null | undefined) { + painter.setVirtualizationPins(pageIndices); + }, + getMountedPageIndices() { + return painter.getMountedPageIndices(); + }, + getPaintSnapshot() { + return lastPaintSnapshot; + }, + onScroll() { + painter.onScroll(); + }, + setZoom(zoom: number) { + painter.setZoom(zoom); + }, + setScrollContainer(el: HTMLElement | null) { + painter.setScrollContainer(el); + }, + }; +} diff --git a/packages/layout-engine/painters/dom/src/between-borders.test.ts b/packages/layout-engine/painters/dom/src/between-borders.test.ts index fbbfb97cdc..9030fe8f47 100644 --- a/packages/layout-engine/painters/dom/src/between-borders.test.ts +++ b/packages/layout-engine/painters/dom/src/between-borders.test.ts @@ -22,7 +22,7 @@ const betweenOff: BetweenBorderInfo = { suppressBottomBorder: false, gapBelow: 0, }; -import { createDomPainter } from './index.js'; +import { createTestPainter as createDomPainter } from './_test-utils.js'; import type { ParagraphBorders, ParagraphBorder, diff --git a/packages/layout-engine/painters/dom/src/clip-path-cache-invalidation.test.ts b/packages/layout-engine/painters/dom/src/clip-path-cache-invalidation.test.ts index 08b8e86005..752b1dfb85 100644 --- a/packages/layout-engine/painters/dom/src/clip-path-cache-invalidation.test.ts +++ b/packages/layout-engine/painters/dom/src/clip-path-cache-invalidation.test.ts @@ -1,5 +1,5 @@ import { afterEach, beforeEach, describe, expect, it } from 'vitest'; -import { createDomPainter } from './index.js'; +import { createTestPainter as createDomPainter } from './_test-utils.js'; import type { FlowBlock, Layout, Measure } from '@superdoc/contracts'; const DATA_URL = diff --git a/packages/layout-engine/painters/dom/src/contract-shape.test.ts b/packages/layout-engine/painters/dom/src/contract-shape.test.ts new file mode 100644 index 0000000000..b502ee1fb5 --- /dev/null +++ b/packages/layout-engine/painters/dom/src/contract-shape.test.ts @@ -0,0 +1,41 @@ +/** + * Compile-time + runtime contract lockdown for the painter's public surface. + * + * These assertions fail when someone reintroduces a legacy field on + * `DomPainterInput`, adds a method to `DomPainterHandle`, or makes + * `PageDecorationPayload.items` optional. The boundary tests in + * `tests/src/architecture-boundaries.test.ts` cover the import side; this + * file covers the type-shape side. + */ +import { describe, expectTypeOf, it } from 'vitest'; +import type { ResolvedLayout, ResolvedPaintItem } from '@superdoc/contracts'; +import type { DomPainterHandle, DomPainterInput, PageDecorationPayload } from './index.js'; + +type Equal = (() => T extends A ? 1 : 2) extends () => T extends B ? 1 : 2 ? true : false; +type AssertTrue = T; + +describe('DomPainter public contract shape', () => { + it('DomPainterInput is exactly { resolvedLayout: ResolvedLayout }', () => { + type _Check = AssertTrue>; + expectTypeOf().toEqualTypeOf<{ resolvedLayout: ResolvedLayout }>(); + }); + + it('DomPainterHandle exposes only the painter-owned methods', () => { + type ExpectedKeys = + | 'paint' + | 'setProviders' + | 'setVirtualizationPins' + | 'getMountedPageIndices' + | 'onScroll' + | 'setZoom' + | 'setScrollContainer'; + type _Check = AssertTrue>; + expectTypeOf().toEqualTypeOf(); + }); + + it('PageDecorationPayload.items is required (synthesis path is gone)', () => { + type ItemsType = PageDecorationPayload['items']; + type _Check = AssertTrue>; + expectTypeOf().toEqualTypeOf(); + }); +}); diff --git a/packages/layout-engine/painters/dom/src/index.test.ts b/packages/layout-engine/painters/dom/src/index.test.ts index 5b63e1640b..ef26279d6d 100644 --- a/packages/layout-engine/painters/dom/src/index.test.ts +++ b/packages/layout-engine/painters/dom/src/index.test.ts @@ -105,7 +105,6 @@ function createTestPainter(opts: { blocks?: FlowBlock[]; measures?: Measure[] } }); const input: DomPainterInput = { resolvedLayout: effectiveResolved, - sourceLayout: layout, }; painter.paint(input, mount, mapping as any); }, @@ -1598,7 +1597,7 @@ describe('DomPainter', () => { }); try { - const painter = createDomPainter({ blocks: [tableBlock], measures: [tableMeasure] }); + const painter = createTestPainter({ blocks: [tableBlock], measures: [tableMeasure] }); expect(() => painter.paint(tableLayout, mount)).not.toThrow(); const placeholder = mount.querySelector('.render-error-placeholder') as HTMLElement | null; @@ -6852,7 +6851,7 @@ describe('DomPainter', () => { ], }; - const painter = createDomPainter({ blocks: [imageBlock], measures: [imageMeasure] }); + const painter = createTestPainter({ blocks: [imageBlock], measures: [imageMeasure] }); painter.paint(imageLayout, mount); }; @@ -8457,7 +8456,7 @@ describe('ImageFragment (block-level images)', () => { ...(hyperlink ? { hyperlink } : {}), }; const measure: Measure = { kind: 'image', width: 100, height: 50 }; - return createDomPainter({ blocks: [block], measures: [measure] }); + return createTestPainter({ blocks: [block], measures: [measure] }); }; it('wraps linked image in with correct href', () => { @@ -8508,7 +8507,7 @@ describe('ImageFragment (block-level images)', () => { pageSize: { w: 400, h: 300 }, pages: [{ number: 1, fragments: [fragment] }], }; - const painter = createDomPainter({ blocks: [block], measures: [measure] }); + const painter = createTestPainter({ blocks: [block], measures: [measure] }); painter.paint(layout, mount); const anchor = mount.querySelector('a.superdoc-link') as HTMLAnchorElement | null; @@ -8529,7 +8528,7 @@ describe('ImageFragment (block-level images)', () => { pageSize: { w: 400, h: 300 }, pages: [{ number: 1, fragments: [fragment] }], }; - const painter = createDomPainter({ blocks: [block], measures: [measure] }); + const painter = createTestPainter({ blocks: [block], measures: [measure] }); painter.paint(layout, mount); const anchor = mount.querySelector('a.superdoc-link'); @@ -8555,7 +8554,7 @@ describe('ImageFragment (block-level images)', () => { pageSize: { w: 400, h: 300 }, pages: [{ number: 1, fragments: [fragment] }], }; - const painter = createDomPainter({ blocks: [block], measures: [measure] }); + const painter = createTestPainter({ blocks: [block], measures: [measure] }); painter.paint(layout, mount); const anchor = mount.querySelector('a.superdoc-link'); @@ -8622,7 +8621,7 @@ describe('URL sanitization security', () => { describe('normalizeAnchor XSS protection', () => { let mount: HTMLElement; - let painter: ReturnType; + let painter: ReturnType; const createFlowBlockWithLink = (link: unknown): FlowBlock => ({ kind: 'paragraph', @@ -8769,7 +8768,7 @@ describe('normalizeAnchor XSS protection', () => { describe('appendDocLocation XSS protection', () => { let mount: HTMLElement; - let painter: ReturnType; + let painter: ReturnType; const createFlowBlockWithLink = (link: unknown): FlowBlock => ({ kind: 'paragraph', @@ -8949,7 +8948,7 @@ describe('appendDocLocation XSS protection', () => { describe('appendDocLocation edge cases', () => { let mount: HTMLElement; - let painter: ReturnType; + let painter: ReturnType; const createFlowBlockWithLink = (link: unknown): FlowBlock => ({ kind: 'paragraph', @@ -9168,7 +9167,7 @@ describe('appendDocLocation edge cases', () => { describe('Tooltip truncation signaling', () => { let mount: HTMLElement; - let painter: ReturnType; + let painter: ReturnType; const createFlowBlockWithLink = (link: unknown): FlowBlock => ({ kind: 'paragraph', @@ -9918,7 +9917,7 @@ describe('Link accessibility - Tooltip aria-describedby', () => { describe('Link rendering metrics', () => { let mount: HTMLElement; - let painter: ReturnType; + let painter: ReturnType; const createFlowBlockWithLink = (link: unknown): FlowBlock => ({ kind: 'paragraph', @@ -10858,181 +10857,6 @@ describe('applyRunDataAttributes', () => { }); }); - describe.skip('decoration item synthesis (legacy, deleted in PR2 of SD-2836)', () => { - let mount: HTMLElement; - - beforeEach(() => { - mount = document.createElement('div'); - document.body.appendChild(mount); - }); - - afterEach(() => { - document.body.removeChild(mount); - }); - - it('synthesizes missing header items from legacy setData bridge data', () => { - const mainBlock: FlowBlock = { - kind: 'paragraph', - id: 'main-block', - runs: [{ text: 'Main', fontFamily: 'Arial', fontSize: 16, pmStart: 0, pmEnd: 4 }], - }; - const mainMeasure: Measure = { - kind: 'paragraph', - lines: [{ fromRun: 0, fromChar: 0, toRun: 0, toChar: 4, width: 40, ascent: 12, descent: 4, lineHeight: 20 }], - totalHeight: 20, - }; - const headerBlock: FlowBlock = { - kind: 'paragraph', - id: 'hf-header-synth', - runs: [{ text: 'Synth Header', fontFamily: 'Arial', fontSize: 14, pmStart: 0, pmEnd: 12 }], - }; - const headerMeasure: Measure = { - kind: 'paragraph', - lines: [{ fromRun: 0, fromChar: 0, toRun: 0, toChar: 12, width: 90, ascent: 10, descent: 3, lineHeight: 16 }], - totalHeight: 16, - }; - const layout: Layout = { - pageSize: { w: 400, h: 500 }, - pages: [{ number: 1, fragments: [] }], - }; - - const painter = createDomPainter({ - blocks: [mainBlock], - measures: [mainMeasure], - headerProvider: () => ({ - height: 16, - offset: 0, - fragments: [{ kind: 'para', blockId: 'hf-header-synth', fromLine: 0, toLine: 1, x: 0, y: 0, width: 120 }], - }), - }); - - painter.setData([mainBlock], [mainMeasure], [headerBlock], [headerMeasure]); - painter.paint(layout, mount); - - expect(mount.querySelector('.superdoc-page-header')?.textContent).toContain('Synth Header'); - expect(mount.querySelector('.render-error-placeholder')).toBeNull(); - }); - - it('synthesizes missing footer items from direct DomPainterInput bridge data', () => { - const footerBlock: FlowBlock = { - kind: 'paragraph', - id: 'hf-footer-synth', - runs: [{ text: 'Synth Footer', fontFamily: 'Arial', fontSize: 14, pmStart: 0, pmEnd: 12 }], - }; - const footerMeasure: Measure = { - kind: 'paragraph', - lines: [{ fromRun: 0, fromChar: 0, toRun: 0, toChar: 12, width: 88, ascent: 10, descent: 3, lineHeight: 16 }], - totalHeight: 16, - }; - const layout: Layout = { - pageSize: { w: 400, h: 500 }, - pages: [{ number: 1, fragments: [] }], - }; - - const painter = createDomPainter({ - footerProvider: () => ({ - height: 16, - offset: 460, - fragments: [{ kind: 'para', blockId: 'hf-footer-synth', fromLine: 0, toLine: 1, x: 0, y: 0, width: 120 }], - }), - }); - - painter.paint( - { - resolvedLayout: emptyResolved, - sourceLayout: layout, - footerBlocks: [footerBlock], - footerMeasures: [footerMeasure], - }, - mount, - ); - - expect(mount.querySelector('.superdoc-page-footer')?.textContent).toContain('Synth Footer'); - expect(mount.querySelector('.render-error-placeholder')).toBeNull(); - }); - - it('validates optional decoration block/measure pairs on direct input', () => { - const painter = createDomPainter({}); - const layout: Layout = { - pageSize: { w: 400, h: 500 }, - pages: [{ number: 1, fragments: [] }], - }; - - expect(() => - painter.paint( - { - resolvedLayout: emptyResolved, - sourceLayout: layout, - headerBlocks: [ - { - kind: 'paragraph', - id: 'hf-header-invalid', - runs: [{ text: 'Invalid', fontFamily: 'Arial', fontSize: 12, pmStart: 0, pmEnd: 7 }], - }, - ], - }, - mount, - ), - ).toThrow('headerBlocks and headerMeasures must both be provided or both be omitted.'); - }); - - it('validates optional decoration block/measure pairs in setData', () => { - const painter = createDomPainter({}); - - expect(() => - painter.setData( - [ - { - kind: 'paragraph', - id: 'body', - runs: [{ text: 'Body', fontFamily: 'Arial', fontSize: 12, pmStart: 0, pmEnd: 4 }], - }, - ], - [ - { - kind: 'paragraph', - lines: [ - { fromRun: 0, fromChar: 0, toRun: 0, toChar: 4, width: 30, ascent: 10, descent: 3, lineHeight: 16 }, - ], - totalHeight: 16, - }, - ], - [ - { - kind: 'paragraph', - id: 'hf-header-invalid', - runs: [{ text: 'Invalid', fontFamily: 'Arial', fontSize: 12, pmStart: 0, pmEnd: 7 }], - }, - ], - ), - ).toThrow('headerBlocks and headerMeasures must both be provided or both be omitted.'); - }); - - it('uses setResolvedLayout for legacy layout paints', () => { - const painter = createDomPainter({}); - const layout: Layout = { - pageSize: { w: 400, h: 500 }, - pages: [{ number: 1, fragments: [] }], - }; - - painter.setResolvedLayout(emptyResolved); - - expect(() => painter.paint(layout, mount)).not.toThrow(); - expect(mount.querySelector('.superdoc-page')).toBeTruthy(); - }); - - it('creates an empty resolved layout for legacy paints without block data', () => { - const painter = createDomPainter({}); - const layout: Layout = { - pageSize: { w: 400, h: 500 }, - pages: [{ number: 1, fragments: [] }], - }; - - expect(() => painter.paint(layout, mount)).not.toThrow(); - expect(mount.querySelector('.superdoc-page')).toBeTruthy(); - }); - }); - describe('footer alignment logic', () => { let mount: HTMLElement; diff --git a/packages/layout-engine/painters/dom/src/index.ts b/packages/layout-engine/painters/dom/src/index.ts index a6567c59a9..b15bce4881 100644 --- a/packages/layout-engine/painters/dom/src/index.ts +++ b/packages/layout-engine/painters/dom/src/index.ts @@ -1,15 +1,4 @@ -import type { - FlowBlock, - Fragment, - Layout, - Measure, - PageMargins, - ResolvedLayout, - Page, - ResolvedPaintItem, -} from '@superdoc/contracts'; import { DomPainter } from './renderer.js'; -import { resolveLayout } from '@superdoc/layout-resolved'; import type { PageStyles } from './styles.js'; import type { DomPainterInput, @@ -76,16 +65,6 @@ export type { FlowMode } from './renderer.js'; export type { PageDecorationPayload, PageDecorationProvider } from './renderer.js'; export type DomPainterOptions = { - /** - * Legacy compatibility: initial body block data. - * New callers should pass block data through `paint(input, mount)`. - */ - blocks?: FlowBlock[]; - /** - * Legacy compatibility: initial body measures. - * New callers should pass measure data through `paint(input, mount)`. - */ - measures?: Measure[]; pageStyles?: PageStyles; layoutMode?: LayoutMode; flowMode?: FlowMode; @@ -122,40 +101,8 @@ export type DomPainterOptions = { onPaintSnapshot?: (snapshot: PaintSnapshot) => void; }; -type LegacyDomPainterState = { - blocks: FlowBlock[]; - measures: Measure[]; - headerBlocks?: FlowBlock[]; - headerMeasures?: Measure[]; - footerBlocks?: FlowBlock[]; - footerMeasures?: Measure[]; - resolvedLayout: ResolvedLayout | null; -}; - -type OptionalBlockMeasurePair = { - blocks: FlowBlock[]; - measures: Measure[]; -}; - export type DomPainterHandle = { - paint(input: DomPainterInput | Layout, mount: HTMLElement, mapping?: PositionMapping): void; - /** - * Legacy compatibility API. - * New callers should pass block/measure data via `paint(input, mount)`. - */ - setData( - blocks: FlowBlock[], - measures: Measure[], - headerBlocks?: FlowBlock[], - headerMeasures?: Measure[], - footerBlocks?: FlowBlock[], - footerMeasures?: Measure[], - ): void; - /** - * Legacy compatibility API. - * New callers should pass resolved data via `paint(input, mount)`. - */ - setResolvedLayout(resolvedLayout: ResolvedLayout | null): void; + paint(input: DomPainterInput, mount: HTMLElement, mapping?: PositionMapping): void; setProviders(header?: PageDecorationProvider, footer?: PageDecorationProvider): void; setVirtualizationPins(pageIndices: number[] | null | undefined): void; getMountedPageIndices(): number[]; @@ -164,207 +111,22 @@ export type DomPainterHandle = { setScrollContainer(el: HTMLElement | null): void; }; -function assertRequiredBlockMeasurePair(label: string, blocks: FlowBlock[], measures: Measure[]): void { - if (blocks.length !== measures.length) { - throw new Error(`${label} blocks and measures must have the same length.`); - } -} - -function normalizeOptionalBlockMeasurePair( - label: 'body' | 'header' | 'footer', - blocks: FlowBlock[] | undefined, - measures: Measure[] | undefined, -): OptionalBlockMeasurePair | undefined { - const hasBlocks = blocks !== undefined; - const hasMeasures = measures !== undefined; - - if (hasBlocks !== hasMeasures) { - if (label === 'body') { - throw new Error('blocks and measures must both be provided or both be omitted.'); - } - throw new Error(`${label}Blocks and ${label}Measures must both be provided or both be omitted.`); - } - - if (!hasBlocks || !hasMeasures) { - return undefined; - } - - assertRequiredBlockMeasurePair(label, blocks, measures); - return { blocks, measures }; -} - -function createEmptyResolvedLayout(flowMode: FlowMode | undefined, pageGap: number | undefined): ResolvedLayout { - return { - version: 1, - flowMode: flowMode ?? 'paginated', - pageGap: pageGap ?? 0, - pages: [], - }; -} - -function isDomPainterInput(value: DomPainterInput | Layout): value is DomPainterInput { - return 'resolvedLayout' in value && 'sourceLayout' in value; -} - -function normalizeDomPainterInput(input: DomPainterInput): DomPainterInput { - const body = normalizeOptionalBlockMeasurePair('body', input.blocks, input.measures); - const header = normalizeOptionalBlockMeasurePair('header', input.headerBlocks, input.headerMeasures); - const footer = normalizeOptionalBlockMeasurePair('footer', input.footerBlocks, input.footerMeasures); - - return { - ...input, - blocks: body?.blocks, - measures: body?.measures, - headerBlocks: header?.blocks, - headerMeasures: header?.measures, - footerBlocks: footer?.blocks, - footerMeasures: footer?.measures, - }; -} - -function buildLegacyPaintInput( - layout: Layout, - legacyState: LegacyDomPainterState, - flowMode: FlowMode | undefined, - pageGap: number | undefined, -): DomPainterInput { - // Derive a resolved layout from the legacy block/measure state when the caller - // has not supplied one via `setResolvedLayout`. The painter now reads all body - // fragment data from the resolved layout, so an empty resolved layout would - // produce a blank render. - let resolvedLayout: ResolvedLayout; - if (legacyState.resolvedLayout) { - resolvedLayout = legacyState.resolvedLayout; - } else { - // resolveLayout handles empty blocks/measures gracefully and preserves - // page-level metadata (size, margins, columns/columnRegions, etc.) needed - // by the painter even when no body content has been provided yet. - resolvedLayout = resolveLayout({ - layout, - flowMode: flowMode ?? 'paginated', - blocks: legacyState.blocks, - measures: legacyState.measures, - }); - } - return { - resolvedLayout, - sourceLayout: layout, - blocks: legacyState.blocks, - measures: legacyState.measures, - headerBlocks: legacyState.headerBlocks, - headerMeasures: legacyState.headerMeasures, - footerBlocks: legacyState.footerBlocks, - footerMeasures: legacyState.footerMeasures, - }; -} - +/** + * Thin pass-through factory: instantiates DomPainter with the supplied options + * and returns a stable handle that exposes only the rendering-stage API. + * + * The handle accepts only `DomPainterInput` (resolvedLayout-only). + * Header/footer decoration providers must supply both `fragments` and `items` + * on their `PageDecorationPayload`. + */ export const createDomPainter = (options: DomPainterOptions): DomPainterHandle => { - if ((options.blocks ?? []).length !== (options.measures ?? []).length) { - throw new Error('DomPainter requires the same number of blocks and measures'); - } - - const legacyState: LegacyDomPainterState = { - blocks: options.blocks ?? [], - measures: options.measures ?? [], - headerBlocks: undefined, - headerMeasures: undefined, - footerBlocks: undefined, - footerMeasures: undefined, - resolvedLayout: null, - }; - - let currentPaintInput: DomPainterInput | null = null; - - const resolveDecorationItems = ( - fragments: readonly Fragment[], - kind: 'header' | 'footer', - ): ResolvedPaintItem[] | undefined => { - const input = currentPaintInput; - if (!input) return undefined; - - const decorationBlocks = kind === 'header' ? input.headerBlocks : input.footerBlocks; - const decorationMeasures = kind === 'header' ? input.headerMeasures : input.footerMeasures; - const mergedBlocks = [...(input.blocks ?? []), ...(decorationBlocks ?? [])]; - const mergedMeasures = [...(input.measures ?? []), ...(decorationMeasures ?? [])]; - if (mergedBlocks.length === 0 || mergedBlocks.length !== mergedMeasures.length) { - return undefined; - } - - const fakeLayout: Layout = { - pageSize: input.sourceLayout.pageSize, - pages: [{ number: 1, fragments: [...fragments] as Fragment[] }] as Page[], - } as Layout; - - try { - const resolved = resolveLayout({ - layout: fakeLayout, - flowMode: input.resolvedLayout.flowMode, - blocks: mergedBlocks, - measures: mergedMeasures, - }); - return resolved.pages[0]?.items; - } catch { - return undefined; - } - }; - - const wrapProvider = ( - provider: PageDecorationProvider | undefined, - kind: 'header' | 'footer', - ): PageDecorationProvider | undefined => { - if (!provider) return undefined; - - return (pageNumber, pageMargins, page) => { - const payload = provider(pageNumber, pageMargins, page); - if (!payload || payload.items) return payload; - const items = resolveDecorationItems(payload.fragments, kind); - return items ? { ...payload, items } : payload; - }; - }; - - const painter = new DomPainter({ - pageStyles: options.pageStyles, - layoutMode: options.layoutMode, - flowMode: options.flowMode, - pageGap: options.pageGap, - headerProvider: wrapProvider(options.headerProvider, 'header'), - footerProvider: wrapProvider(options.footerProvider, 'footer'), - virtualization: options.virtualization, - ruler: options.ruler, - onPaintSnapshot: options.onPaintSnapshot, - }); - + const painter = new DomPainter(options); return { - paint(input: DomPainterInput | Layout, mount: HTMLElement, mapping?: PositionMapping) { - const normalizedInput = isDomPainterInput(input) - ? normalizeDomPainterInput(input) - : buildLegacyPaintInput(input, legacyState, options.flowMode, options.pageGap); - currentPaintInput = normalizedInput; - painter.paint(normalizedInput, mount, mapping); - }, - setData( - blocks: FlowBlock[], - measures: Measure[], - headerBlocks?: FlowBlock[], - headerMeasures?: Measure[], - footerBlocks?: FlowBlock[], - footerMeasures?: Measure[], - ) { - assertRequiredBlockMeasurePair('body', blocks, measures); - const normalizedHeader = normalizeOptionalBlockMeasurePair('header', headerBlocks, headerMeasures); - const normalizedFooter = normalizeOptionalBlockMeasurePair('footer', footerBlocks, footerMeasures); - legacyState.blocks = blocks; - legacyState.measures = measures; - legacyState.headerBlocks = normalizedHeader?.blocks; - legacyState.headerMeasures = normalizedHeader?.measures; - legacyState.footerBlocks = normalizedFooter?.blocks; - legacyState.footerMeasures = normalizedFooter?.measures; - }, - setResolvedLayout(resolvedLayout: ResolvedLayout | null) { - legacyState.resolvedLayout = resolvedLayout; + paint(input: DomPainterInput, mount: HTMLElement, mapping?: PositionMapping) { + painter.paint(input, mount, mapping); }, setProviders(header?: PageDecorationProvider, footer?: PageDecorationProvider) { - painter.setProviders(wrapProvider(header, 'header'), wrapProvider(footer, 'footer')); + painter.setProviders(header, footer); }, setVirtualizationPins(pageIndices: number[] | null | undefined) { painter.setVirtualizationPins(pageIndices); diff --git a/packages/layout-engine/painters/dom/src/link-click.test.ts b/packages/layout-engine/painters/dom/src/link-click.test.ts index d826043b00..0543ac7ef3 100644 --- a/packages/layout-engine/painters/dom/src/link-click.test.ts +++ b/packages/layout-engine/painters/dom/src/link-click.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it, beforeEach, afterEach } from 'vitest'; -import { createDomPainter } from './index.js'; +import { createTestPainter as createDomPainter } from './_test-utils.js'; import type { FlowBlock, Measure, Layout } from '@superdoc/contracts'; /** diff --git a/packages/layout-engine/painters/dom/src/renderer-column-separators.test.ts b/packages/layout-engine/painters/dom/src/renderer-column-separators.test.ts index 8d90167c3c..699fd35afe 100644 --- a/packages/layout-engine/painters/dom/src/renderer-column-separators.test.ts +++ b/packages/layout-engine/painters/dom/src/renderer-column-separators.test.ts @@ -1,5 +1,5 @@ import { afterEach, beforeEach, describe, expect, it } from 'vitest'; -import { createDomPainter } from './index.js'; +import { createTestPainter as createDomPainter } from './_test-utils.js'; import type { ColumnRegion, Fragment, Layout, Page } from '@superdoc/contracts'; // These tests pin down DomPainter's column-separator rendering: diff --git a/packages/layout-engine/painters/dom/src/renderer-dispatch.test.ts b/packages/layout-engine/painters/dom/src/renderer-dispatch.test.ts index f0777dd695..ea641ee9d4 100644 --- a/packages/layout-engine/painters/dom/src/renderer-dispatch.test.ts +++ b/packages/layout-engine/painters/dom/src/renderer-dispatch.test.ts @@ -7,7 +7,7 @@ */ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; -import { createDomPainter } from './index.js'; +import { createTestPainter as createDomPainter } from './_test-utils.js'; import { DomPainter } from './renderer.js'; import type { FlowBlock, Measure, Layout } from '@superdoc/contracts'; diff --git a/packages/layout-engine/painters/dom/src/renderer-hanging-indent.test.ts b/packages/layout-engine/painters/dom/src/renderer-hanging-indent.test.ts index 003142cb6e..eb79122ec9 100644 --- a/packages/layout-engine/painters/dom/src/renderer-hanging-indent.test.ts +++ b/packages/layout-engine/painters/dom/src/renderer-hanging-indent.test.ts @@ -11,7 +11,7 @@ */ import { describe, it, expect, beforeEach } from 'vitest'; -import { createDomPainter } from './index.js'; +import { createTestPainter as createDomPainter } from './_test-utils.js'; import type { FlowBlock, Measure, Layout, Line } from '@superdoc/contracts'; describe('DomPainter hanging indent with tabs', () => { diff --git a/packages/layout-engine/painters/dom/src/renderer-known-divergences.test.ts b/packages/layout-engine/painters/dom/src/renderer-known-divergences.test.ts index b08c4164f0..92706dc9c5 100644 --- a/packages/layout-engine/painters/dom/src/renderer-known-divergences.test.ts +++ b/packages/layout-engine/painters/dom/src/renderer-known-divergences.test.ts @@ -10,7 +10,7 @@ */ import { describe, it, expect } from 'vitest'; -import { createDomPainter } from './index.js'; +import { createTestPainter as createDomPainter } from './_test-utils.js'; import type { FlowBlock, Measure, Layout, Line } from '@superdoc/contracts'; import { normalizeLines } from './test-utils/normalize-line.js'; diff --git a/packages/layout-engine/painters/dom/src/renderer-marker-suffix.test.ts b/packages/layout-engine/painters/dom/src/renderer-marker-suffix.test.ts index 2c39672bee..a016e3ce28 100644 --- a/packages/layout-engine/painters/dom/src/renderer-marker-suffix.test.ts +++ b/packages/layout-engine/painters/dom/src/renderer-marker-suffix.test.ts @@ -5,7 +5,7 @@ */ import { describe, it, expect, beforeEach } from 'vitest'; -import { createDomPainter } from './index.js'; +import { createTestPainter as createDomPainter } from './_test-utils.js'; import type { FlowBlock, Measure, Layout, WordParagraphLayoutOutput } from '@superdoc/contracts'; describe('DomPainter marker suffix rendering', () => { diff --git a/packages/layout-engine/painters/dom/src/renderer-marker-textwidth.test.ts b/packages/layout-engine/painters/dom/src/renderer-marker-textwidth.test.ts index cf3e7cddd5..8fb8bdef70 100644 --- a/packages/layout-engine/painters/dom/src/renderer-marker-textwidth.test.ts +++ b/packages/layout-engine/painters/dom/src/renderer-marker-textwidth.test.ts @@ -10,7 +10,7 @@ */ import { describe, it, expect, beforeEach } from 'vitest'; -import { createDomPainter } from './index.js'; +import { createTestPainter as createDomPainter } from './_test-utils.js'; import type { FlowBlock, Measure, Layout, WordParagraphLayoutOutput } from '@superdoc/contracts'; describe('DomPainter markerTextWidth feature', () => { diff --git a/packages/layout-engine/painters/dom/src/renderer-parity-contracts.test.ts b/packages/layout-engine/painters/dom/src/renderer-parity-contracts.test.ts index a7776ccee5..901c37e5a5 100644 --- a/packages/layout-engine/painters/dom/src/renderer-parity-contracts.test.ts +++ b/packages/layout-engine/painters/dom/src/renderer-parity-contracts.test.ts @@ -10,7 +10,7 @@ */ import { describe, it, expect, beforeEach } from 'vitest'; -import { createDomPainter } from './index.js'; +import { createTestPainter as createDomPainter } from './_test-utils.js'; import type { FlowBlock, Measure, Layout, ParagraphMeasure, Line, Run } from '@superdoc/contracts'; import { normalizeLines, type NormalizedLine } from './test-utils/normalize-line.js'; diff --git a/packages/layout-engine/painters/dom/src/renderer-shape-regressions.test.ts b/packages/layout-engine/painters/dom/src/renderer-shape-regressions.test.ts index 0380baada7..ea55a3e4c7 100644 --- a/packages/layout-engine/painters/dom/src/renderer-shape-regressions.test.ts +++ b/packages/layout-engine/painters/dom/src/renderer-shape-regressions.test.ts @@ -1,5 +1,5 @@ import { describe, it, expect, beforeEach, afterEach } from 'vitest'; -import { createDomPainter } from './index.js'; +import { createTestPainter as createDomPainter } from './_test-utils.js'; import type { DrawingGeometry, FlowBlock, Layout, Measure, SolidFillWithAlpha } from '@superdoc/contracts'; type DrawingFlowBlock = Extract; diff --git a/packages/layout-engine/painters/dom/src/renderer-vector-shape-geometry.test.ts b/packages/layout-engine/painters/dom/src/renderer-vector-shape-geometry.test.ts index e880659bfb..53da516607 100644 --- a/packages/layout-engine/painters/dom/src/renderer-vector-shape-geometry.test.ts +++ b/packages/layout-engine/painters/dom/src/renderer-vector-shape-geometry.test.ts @@ -7,7 +7,7 @@ */ import { describe, it, expect, beforeEach, afterEach } from 'vitest'; -import { createDomPainter } from './index.js'; +import { createTestPainter as createDomPainter } from './_test-utils.js'; import type { FlowBlock, Measure, Layout, DrawingGeometry } from '@superdoc/contracts'; describe('DomPainter vector shape geometry', () => { diff --git a/packages/layout-engine/painters/dom/src/renderer.ts b/packages/layout-engine/painters/dom/src/renderer.ts index 3a80e25752..e7ea00cc84 100644 --- a/packages/layout-engine/painters/dom/src/renderer.ts +++ b/packages/layout-engine/painters/dom/src/renderer.ts @@ -17,19 +17,15 @@ import type { ImageFragment, ImageHyperlink, ImageRun, - Layout, Line, LineSegment, ListBlock, ListItemFragment, ListMeasure, - Measure, - Page, PageMargins, ParaFragment, ParagraphAttrs, ParagraphBlock, - ParagraphBorder, ParagraphMeasure, PositionedDrawingGeometry, Run, @@ -242,34 +238,18 @@ export type RenderedLineInfo = { /** * Input to `DomPainter.paint()`. * - * `resolvedLayout` is the canonical resolved data the painter reads from. - * `sourceLayout` is the raw Layout retained for legacy internal access paths. + * The painter consumes only `resolvedLayout`. All fragment, geometry, and + * page-level metadata it needs is reachable from `ResolvedPaintItem.fragment` + * back-pointers and `ResolvedPage` fields. */ export type DomPainterInput = { resolvedLayout: ResolvedLayout; - /** Raw Layout for internal fragment access. */ - sourceLayout: Layout; - /** - * Optional bridge data used only when a decoration provider omits `items`. - * Body rendering reads from `resolvedLayout`; these arrays exist solely so - * header/footer fragments can synthesize resolved items on demand. - */ - blocks?: FlowBlock[]; - measures?: Measure[]; - headerBlocks?: FlowBlock[]; - headerMeasures?: Measure[]; - footerBlocks?: FlowBlock[]; - footerMeasures?: Measure[]; }; export type PageDecorationPayload = { fragments: Fragment[]; - /** - * Resolved items aligned 1:1 with `fragments`. Same length, same order. - * When omitted, the painter treats fragments as having no resolved metadata - * (no paragraph borders, no SDT container keys). - */ - items?: ResolvedPaintItem[]; + /** Resolved items aligned 1:1 with `fragments`. Same length, same order. */ + items: ResolvedPaintItem[]; /** Minimum Y coordinate from layout; negative when content extends above y=0. */ minY?: number; height: number; diff --git a/packages/layout-engine/painters/dom/src/source-anchor.test.ts b/packages/layout-engine/painters/dom/src/source-anchor.test.ts index 990a964675..f600cbdec3 100644 --- a/packages/layout-engine/painters/dom/src/source-anchor.test.ts +++ b/packages/layout-engine/painters/dom/src/source-anchor.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it } from 'vitest'; -import { createDomPainter } from './index.js'; +import { createTestPainter as createDomPainter } from './_test-utils.js'; import type { FlowBlock, Layout, Measure, SourceAnchor } from '@superdoc/contracts'; import type { PaintSnapshot } from './renderer.js'; diff --git a/packages/layout-engine/painters/dom/src/text-style-rendering.test.ts b/packages/layout-engine/painters/dom/src/text-style-rendering.test.ts index 536ef9a44b..a913bd1333 100644 --- a/packages/layout-engine/painters/dom/src/text-style-rendering.test.ts +++ b/packages/layout-engine/painters/dom/src/text-style-rendering.test.ts @@ -5,7 +5,7 @@ */ import { describe, it, expect, beforeEach } from 'vitest'; -import { createDomPainter } from './index.js'; +import { createTestPainter as createDomPainter } from './_test-utils.js'; import type { FlowBlock, Measure, Layout } from '@superdoc/contracts'; const expectCssColor = (actual: string, expectedHex: string): void => { diff --git a/packages/layout-engine/painters/dom/src/virtualization.test.ts b/packages/layout-engine/painters/dom/src/virtualization.test.ts index b14f510638..092a3bf315 100644 --- a/packages/layout-engine/painters/dom/src/virtualization.test.ts +++ b/packages/layout-engine/painters/dom/src/virtualization.test.ts @@ -1,53 +1,6 @@ import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; -import { createDomPainter } from './index.js'; -import { resolveLayout } from '@superdoc/layout-resolved'; -import type { DomPainterOptions, DomPainterInput, PaintSnapshot } from './index.js'; -import type { FlowBlock, Measure, Layout, Fragment, PageMargins, ResolvedLayout } from '@superdoc/contracts'; - -const emptyResolved: ResolvedLayout = { version: 1, flowMode: 'paginated', pageGap: 0, pages: [] }; - -/** Test-only bridge: see index.test.ts for full JSDoc. */ -function createTestPainter(opts: { blocks?: FlowBlock[]; measures?: Measure[] } & DomPainterOptions) { - const { blocks: initBlocks, measures: initMeasures, ...painterOpts } = opts; - let lastPaintSnapshot: PaintSnapshot | null = null; - const painter = createDomPainter({ - ...painterOpts, - onPaintSnapshot: (snapshot) => { - lastPaintSnapshot = snapshot; - }, - }); - let currentBlocks: FlowBlock[] = initBlocks ?? []; - let currentMeasures: Measure[] = initMeasures ?? []; - let currentResolved: ResolvedLayout = emptyResolved; - - return { - paint(layout: Layout, mount: HTMLElement, mapping?: unknown) { - const effectiveResolved = - currentBlocks.length === 0 && currentMeasures.length === 0 - ? currentResolved - : resolveLayout({ - layout, - flowMode: opts.flowMode ?? 'paginated', - blocks: currentBlocks, - measures: currentMeasures, - }); - const input: DomPainterInput = { - resolvedLayout: effectiveResolved, - sourceLayout: layout, - }; - painter.paint(input, mount, mapping as any); - }, - setProviders: painter.setProviders, - setVirtualizationPins: painter.setVirtualizationPins, - getMountedPageIndices: painter.getMountedPageIndices, - getPaintSnapshot() { - return lastPaintSnapshot; - }, - onScroll: painter.onScroll, - setZoom: painter.setZoom, - setScrollContainer: painter.setScrollContainer, - }; -} +import { createTestPainter } from './_test-utils.js'; +import type { FlowBlock, Measure, Layout, Fragment, PageMargins } from '@superdoc/contracts'; // Minimal paragraph block/measure to satisfy painter const block: FlowBlock = { diff --git a/packages/layout-engine/pm-adapter/package.json b/packages/layout-engine/pm-adapter/package.json index abc0a590cd..0a8744c986 100644 --- a/packages/layout-engine/pm-adapter/package.json +++ b/packages/layout-engine/pm-adapter/package.json @@ -36,6 +36,7 @@ "devDependencies": { "vitest": "catalog:", "@superdoc/layout-engine": "workspace:*", + "@superdoc/layout-resolved": "workspace:*", "@superdoc/painter-dom": "workspace:*" }, "dependencies": { diff --git a/packages/layout-engine/pm-adapter/src/integration.test.ts b/packages/layout-engine/pm-adapter/src/integration.test.ts index 065b52b128..e9bdc14328 100644 --- a/packages/layout-engine/pm-adapter/src/integration.test.ts +++ b/packages/layout-engine/pm-adapter/src/integration.test.ts @@ -11,6 +11,7 @@ import type { PMNode, AdapterOptions } from './index.js'; import { measureBlock } from '@superdoc/measuring-dom'; import { layoutDocument } from '@superdoc/layout-engine'; import { createDomPainter } from '@superdoc/painter-dom'; +import { resolveLayout } from '@superdoc/layout-resolved'; // Cleaned: remove unused PDF painter import import type { Measure, ParaFragment, ParagraphMeasure, TabStop } from '@superdoc/contracts'; import basicParagraphFixture from './fixtures/basic-paragraph.json'; @@ -492,8 +493,9 @@ describe('PM → FlowBlock → Measure integration', () => { const mount = document.createElement('div'); document.body.appendChild(mount); - const painter = createDomPainter({ blocks, measures }); - painter.paint(layout, mount); + const painter = createDomPainter({}); + const resolvedLayout = resolveLayout({ layout, flowMode: 'paginated', blocks, measures }); + painter.paint({ resolvedLayout }, mount); expect(mount.children.length).toBeGreaterThan(0); expect(mount.textContent).toContain('This is a simple paragraph'); @@ -547,8 +549,9 @@ describe('PM → FlowBlock → Measure integration', () => { const mount = document.createElement('div'); document.body.appendChild(mount); - const painter = createDomPainter({ blocks, measures }); - painter.paint(layout, mount); + const painter = createDomPainter({}); + const resolvedLayout = resolveLayout({ layout, flowMode: 'paginated', blocks, measures }); + painter.paint({ resolvedLayout }, mount); const fragment = mount.querySelector('.superdoc-fragment') as HTMLElement; const shadingLayer = fragment.querySelector('.superdoc-paragraph-shading') as HTMLElement; @@ -759,8 +762,9 @@ describe('page break integration tests', () => { const mount = document.createElement('div'); document.body.appendChild(mount); - const painter = createDomPainter({ blocks, measures }); - painter.paint(layout, mount); + const painter = createDomPainter({}); + const resolvedLayout = resolveLayout({ layout, flowMode: 'paginated', blocks, measures }); + painter.paint({ resolvedLayout }, mount); // Verify multiple pages were created in DOM const pages = mount.querySelectorAll('.superdoc-page'); diff --git a/packages/layout-engine/tests/src/architecture-boundaries.test.ts b/packages/layout-engine/tests/src/architecture-boundaries.test.ts index 9c4d398b57..913df399cb 100644 --- a/packages/layout-engine/tests/src/architecture-boundaries.test.ts +++ b/packages/layout-engine/tests/src/architecture-boundaries.test.ts @@ -187,4 +187,47 @@ describe('architecture boundaries', () => { expectNoViolations(findRelativeImportViolations(srcDir, /from\s+['"].*layout-engine\//)); }); }); + + describe('Guard D: painter-dom is a dumb final renderer with no upstream dependencies', () => { + it('painter-dom runtime src does not import @superdoc/pm-adapter', () => { + const srcDir = path.join(LAYOUT_ENGINE_ROOT, 'painters/dom/src'); + expectNoViolations(findImportViolations(srcDir, '@superdoc/pm-adapter')); + }); + + it('painter-dom runtime src does not import @superdoc/layout-bridge', () => { + const srcDir = path.join(LAYOUT_ENGINE_ROOT, 'painters/dom/src'); + expectNoViolations(findImportViolations(srcDir, '@superdoc/layout-bridge')); + }); + + it('painter-dom runtime src does not import @superdoc/layout-resolved (test-only utility)', () => { + const srcDir = path.join(LAYOUT_ENGINE_ROOT, 'painters/dom/src'); + // _test-utils.ts is test-only and excluded from runtime collection. The + // architecture-boundary check passes when no runtime file imports + // layout-resolved. + const files = collectRuntimeSources(srcDir).filter((f) => !f.endsWith('_test-utils.ts')); + const violations: { file: string; line: string }[] = []; + const pattern = new RegExp(`['"]@superdoc/layout-resolved(?:[/'"]|$)`); + for (const file of files) { + const raw = fs.readFileSync(file, 'utf-8'); + const processed = preprocessSource(raw); + const lines = processed.split('\n'); + for (const ln of lines) { + if (pattern.test(ln)) { + violations.push({ file: path.relative(LAYOUT_ENGINE_ROOT, file), line: ln.trim() }); + } + } + } + expectNoViolations(violations); + }); + + it('painter-dom runtime src does not import relative pm-adapter paths', () => { + const srcDir = path.join(LAYOUT_ENGINE_ROOT, 'painters/dom/src'); + expectNoViolations(findRelativeImportViolations(srcDir, /from\s+['"].*pm-adapter\//)); + }); + + it('painter-dom runtime src does not import relative layout-bridge paths', () => { + const srcDir = path.join(LAYOUT_ENGINE_ROOT, 'painters/dom/src'); + expectNoViolations(findRelativeImportViolations(srcDir, /from\s+['"].*layout-bridge\//)); + }); + }); }); diff --git a/packages/super-editor/src/editors/v1/core/presentation-editor/PresentationEditor.ts b/packages/super-editor/src/editors/v1/core/presentation-editor/PresentationEditor.ts index 0d35be5d89..c46f67f3ca 100644 --- a/packages/super-editor/src/editors/v1/core/presentation-editor/PresentationEditor.ts +++ b/packages/super-editor/src/editors/v1/core/presentation-editor/PresentationEditor.ts @@ -6209,7 +6209,6 @@ export class PresentationEditor extends EventEmitter { const painterPaintStart = perfNow(); const paintInput: DomPainterInput = { resolvedLayout, - sourceLayout: layout, }; this.#painterAdapter.paint(paintInput, this.#painterHost, mapping ?? undefined); const painterPaintEnd = perfNow(); diff --git a/packages/super-editor/src/editors/v1/core/presentation-editor/header-footer/HeaderFooterSessionManager.ts b/packages/super-editor/src/editors/v1/core/presentation-editor/header-footer/HeaderFooterSessionManager.ts index 5fa7025f3e..e53ef72288 100644 --- a/packages/super-editor/src/editors/v1/core/presentation-editor/header-footer/HeaderFooterSessionManager.ts +++ b/packages/super-editor/src/editors/v1/core/presentation-editor/header-footer/HeaderFooterSessionManager.ts @@ -386,11 +386,8 @@ function normalizeDecorationFragments(fragments: Fragment[], layoutMinY: number) return fragments.map((fragment) => ({ ...fragment, y: fragment.y + yOffset })); } -function normalizeDecorationItems( - items: ResolvedPaintItem[] | undefined, - layoutMinY: number, -): ResolvedPaintItem[] | undefined { - if (!items || layoutMinY >= 0) { +function normalizeDecorationItems(items: ResolvedPaintItem[], layoutMinY: number): ResolvedPaintItem[] { + if (layoutMinY >= 0) { return items; } @@ -2404,6 +2401,9 @@ export class HeaderFooterSessionManager { rIdResolvedLayout, `rId '${rIdLayoutKey}' page ${pageNumber}`, ); + if (!alignedItems) { + return null; + } const pageHeight = page?.height ?? resolvedLayout.pages[0]?.height ?? layoutOptions.pageSize?.h ?? defaultPageSize.h; const margins = pageMargins ?? resolvedLayout.pages[0]?.margins ?? layoutOptions.margins ?? defaultMargins; @@ -2465,6 +2465,9 @@ export class HeaderFooterSessionManager { resolvedVariant, `variant '${headerFooterType}' page ${pageNumber}`, ); + if (!alignedVariantItems) { + return null; + } const pageHeight = page?.height ?? resolvedLayout.pages[0]?.height ?? layoutOptions.pageSize?.h ?? defaultPageSize.h; diff --git a/packages/super-editor/src/editors/v1/core/presentation-editor/rendering/PresentationPainterAdapter.ts b/packages/super-editor/src/editors/v1/core/presentation-editor/rendering/PresentationPainterAdapter.ts index 5a5e89718a..645896aad5 100644 --- a/packages/super-editor/src/editors/v1/core/presentation-editor/rendering/PresentationPainterAdapter.ts +++ b/packages/super-editor/src/editors/v1/core/presentation-editor/rendering/PresentationPainterAdapter.ts @@ -73,7 +73,7 @@ export class PresentationPainterAdapter { // ── Paint orchestration ───────────────────────────────────────────── - paint(input: DomPainterInput | Layout, mount: HTMLElement, mapping?: PositionMapping): void { + paint(input: DomPainterInput, mount: HTMLElement, mapping?: PositionMapping): void { this.#painter?.paint(input, mount, mapping); } diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index df5e23aaab..748aa70a29 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -2463,6 +2463,9 @@ importers: specifier: workspace:* version: link:../../word-layout devDependencies: + '@superdoc/layout-resolved': + specifier: workspace:* + version: link:../layout-resolved '@superdoc/painter-dom': specifier: workspace:* version: link:../painters/dom @@ -2542,9 +2545,6 @@ importers: '@superdoc/font-utils': specifier: workspace:* version: link:../../../../shared/font-utils - '@superdoc/layout-resolved': - specifier: workspace:* - version: link:../../layout-resolved '@superdoc/preset-geometry': specifier: workspace:* version: link:../../../preset-geometry @@ -2555,6 +2555,9 @@ importers: '@superdoc/layout-engine': specifier: workspace:* version: link:../../layout-engine + '@superdoc/layout-resolved': + specifier: workspace:* + version: link:../../layout-resolved vitest: specifier: 'catalog:' version: 3.2.4(@emnapi/core@1.9.1)(@emnapi/runtime@1.9.1)(@types/debug@4.1.13)(@types/node@25.6.0)(esbuild@0.27.7)(happy-dom@20.4.0)(jiti@2.6.1)(jsdom@27.3.0(canvas@3.2.3))(less@4.4.2)(sass@1.97.3)(terser@5.46.1)(tsx@4.21.0)(yaml@2.8.3) @@ -2592,6 +2595,9 @@ importers: '@superdoc/layout-engine': specifier: workspace:* version: link:../layout-engine + '@superdoc/layout-resolved': + specifier: workspace:* + version: link:../layout-resolved '@superdoc/painter-dom': specifier: workspace:* version: link:../painters/dom From 7ee93e9d34405761f719801282198e8218330554 Mon Sep 17 00:00:00 2001 From: Tadeu Tupinamba Date: Tue, 5 May 2026 15:06:46 -0300 Subject: [PATCH 13/13] fix(painter): adapt content-presence gate to ResolvedPage (SD-2836) After rebasing PR1's painter migration onto main, the new column-separator content-presence gate added by SD-2452 (a5ff6ed26) reads page.fragments directly. On the new architecture the painter receives a ResolvedPage whose fragments live as page.items[]. Switch the in-region scan to iterate items (every ResolvedPaintItem carries x/y). Also extend the test-only createTestPainter shim to auto-synthesize minimal ParagraphBlock/ParagraphMeasure entries for any layout fragment whose blockId isn't covered by the supplied blocks. Tests that only exercise wrapper-level rendering (column separators, page chrome) can keep using fragAt(...) without boilerplate matching blocks. --- .../painters/dom/src/_test-utils.ts | 22 +++++++++++++++++-- .../painters/dom/src/renderer.ts | 2 +- 2 files changed, 21 insertions(+), 3 deletions(-) diff --git a/packages/layout-engine/painters/dom/src/_test-utils.ts b/packages/layout-engine/painters/dom/src/_test-utils.ts index 5090d7a885..0720080e75 100644 --- a/packages/layout-engine/painters/dom/src/_test-utils.ts +++ b/packages/layout-engine/painters/dom/src/_test-utils.ts @@ -84,13 +84,31 @@ export function createTestPainter(opts: { blocks?: FlowBlock[]; measures?: Measu return { paint(layout: Layout, mount: HTMLElement, mapping?: unknown) { + // Auto-synthesize minimal blocks/measures for any layout fragment whose + // blockId isn't covered by currentBlocks. Tests that only care about + // wrapper-level rendering (column separators, page chrome) can skip the + // boilerplate of building matching blocks for every fragment they place. + const knownIds = new Set(currentBlocks.map((b) => b.id)); + const syntheticBlocks: FlowBlock[] = []; + const syntheticMeasures: Measure[] = []; + for (const page of layout.pages) { + for (const fragment of page.fragments ?? []) { + if (fragment.kind !== 'para' || knownIds.has(fragment.blockId)) continue; + syntheticBlocks.push({ kind: 'paragraph', id: fragment.blockId, runs: [] }); + syntheticMeasures.push({ kind: 'paragraph', lines: [], totalHeight: 0 }); + knownIds.add(fragment.blockId); + } + } + const effectiveBlocks = syntheticBlocks.length ? [...currentBlocks, ...syntheticBlocks] : currentBlocks; + const effectiveMeasures = syntheticMeasures.length ? [...currentMeasures, ...syntheticMeasures] : currentMeasures; + const effectiveResolved = resolvedLayoutOverridden ? currentResolved : resolveLayout({ layout, flowMode: opts.flowMode ?? 'paginated', - blocks: currentBlocks, - measures: currentMeasures, + blocks: effectiveBlocks, + measures: effectiveMeasures, }); const input: DomPainterInput = { resolvedLayout: effectiveResolved, diff --git a/packages/layout-engine/painters/dom/src/renderer.ts b/packages/layout-engine/painters/dom/src/renderer.ts index e7ea00cc84..d87b946b89 100644 --- a/packages/layout-engine/painters/dom/src/renderer.ts +++ b/packages/layout-engine/painters/dom/src/renderer.ts @@ -2320,7 +2320,7 @@ export class DomPainter { // where Word fills col 0 first without balancing), Word draws no line // even when the section's `w:cols` declared `w:sep="1"`. Gate each // separator on whether any fragment sits past it within the region. - const fragmentsInRegion = page.fragments.filter((f) => f.y >= yStart - 0.5 && f.y < yEnd + 0.5); + const fragmentsInRegion = page.items.filter((item) => item.y >= yStart - 0.5 && item.y < yEnd + 0.5); for (const separatorX of separatorPositions) { const hasContentPastSeparator = fragmentsInRegion.some((f) => f.x >= separatorX);