Skip to content

Commit 55bf52e

Browse files
authored
[8/16] refactor(painter): remove body blocks/measures from DomPainterInput (#2820)
* refactor(layout): lift page metadata into ResolvedPage * refactor(layout): lift fragment metadata into resolved paint items Add pmStart, pmEnd, continuesFromPrev, continuesOnNext, markerWidth, and metadata fields to resolved paint item types. Populate them in the resolvers and update the painter to prefer resolved item data over legacy Fragment reads with fallbacks. * refactor(layout): pre-compute SDT container keys in resolved layout * refactor(layout): pre-compute paragraph border data in resolved layout * refactor(layout): move change detection into resolved layout stage * refactor(layout): lift paragraph and list-item block/measure into resolved items * refactor(painter): extract block/measure resolution helper * refactor(painter): remove body blocks/measures from DomPainterInput Body block and measure data now flows exclusively through the resolved layout. The painter only builds a blockLookup from header/footer data, which is the last remaining fallback surface for fragments that do not yet have a resolved path. Complex-transaction rebuild detection now walks the resolved layout items directly instead of iterating the body blockLookup. The legacy createDomPainter wrapper derives a resolved layout from its legacyState blocks/measures on the fly so the benchmark path and direct createDomPainter(options).paint(Layout) callers keep working without setResolvedLayout. * fix: dompainter body input contract on first paint
1 parent 04aae44 commit 55bf52e

10 files changed

Lines changed: 198 additions & 43 deletions

File tree

packages/layout-engine/layout-resolved/src/resolveLayout.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -189,6 +189,7 @@ function computeBlockVersion(
189189
cache.set(blockId, version);
190190
return version;
191191
}
192+
192193
function resolveFragmentItem(
193194
fragment: Fragment,
194195
fragmentIndex: number,
@@ -250,12 +251,14 @@ function resolveFragmentItem(
250251
item.measure = entry.measure as ListMeasure;
251252
}
252253
}
254+
253255
// Pre-compute paragraph border data for between-border grouping
254256
const borders = resolveFragmentParagraphBorders(fragment, blockMap);
255257
if (borders) {
256258
item.paragraphBorders = borders;
257259
item.paragraphBorderHash = hashParagraphBorders(borders);
258260
}
261+
259262
if (fragment.kind === 'para') {
260263
const para = fragment as ParaFragment;
261264
if (para.pmStart != null) item.pmStart = para.pmStart;

packages/layout-engine/painters/dom/package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
"@superdoc/contracts": "workspace:*",
2222
"@superdoc/dom-contract": "workspace:*",
2323
"@superdoc/font-utils": "workspace:*",
24+
"@superdoc/layout-resolved": "workspace:*",
2425
"@superdoc/preset-geometry": "workspace:*",
2526
"@superdoc/url-validation": "workspace:*"
2627
},

packages/layout-engine/painters/dom/src/index.test.ts

Lines changed: 49 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { describe, expect, it, beforeEach, afterEach, vi } from 'vitest';
22
import { createDomPainter, sanitizeUrl, linkMetrics, applyRunDataAttributes } from './index.js';
33
import { DomPainter } from './renderer.js';
4+
import { resolveLayout } from '@superdoc/layout-resolved';
45
import type { DomPainterOptions, DomPainterInput, PaintSnapshot } from './index.js';
56
import { resolveListMarkerGeometry } from '../../../../../shared/common/list-marker-utils.js';
67
import type {
@@ -42,17 +43,38 @@ function createTestPainter(opts: { blocks?: FlowBlock[]; measures?: Measure[] }
4243
let footerBlocks: FlowBlock[] | undefined;
4344
let footerMeasures: Measure[] | undefined;
4445

46+
let resolvedLayoutOverridden = false;
47+
4548
return {
4649
paint(layout: Layout, mount: HTMLElement, mapping?: unknown) {
50+
const effectiveResolved = resolvedLayoutOverridden
51+
? currentResolved
52+
: resolveLayout({
53+
layout,
54+
flowMode: opts.flowMode ?? 'paginated',
55+
blocks: currentBlocks,
56+
measures: currentMeasures,
57+
});
58+
// Tests historically pass header/footer blocks via the main `blocks` array and
59+
// rely on the blockLookup containing them. Merge body blocks into headerBlocks
60+
// so header/footer fragments from providers can resolve their block data.
61+
const mergedHeaderBlocks =
62+
headerBlocks || currentBlocks.length > 0 ? [...currentBlocks, ...(headerBlocks ?? [])] : undefined;
63+
const mergedHeaderMeasures =
64+
headerMeasures || currentMeasures.length > 0 ? [...currentMeasures, ...(headerMeasures ?? [])] : undefined;
65+
const mergedFooterBlocks =
66+
footerBlocks || currentBlocks.length > 0 ? [...currentBlocks, ...(footerBlocks ?? [])] : undefined;
67+
const mergedFooterMeasures =
68+
footerMeasures || currentMeasures.length > 0 ? [...currentMeasures, ...(footerMeasures ?? [])] : undefined;
4769
const input: DomPainterInput = {
48-
resolvedLayout: currentResolved,
70+
resolvedLayout: effectiveResolved,
4971
sourceLayout: layout,
5072
blocks: currentBlocks,
5173
measures: currentMeasures,
52-
headerBlocks,
53-
headerMeasures,
54-
footerBlocks,
55-
footerMeasures,
74+
headerBlocks: mergedHeaderBlocks,
75+
headerMeasures: mergedHeaderMeasures,
76+
footerBlocks: mergedFooterBlocks,
77+
footerMeasures: mergedFooterMeasures,
5678
};
5779
painter.paint(input, mount, mapping as any);
5880
},
@@ -73,6 +95,7 @@ function createTestPainter(opts: { blocks?: FlowBlock[]; measures?: Measure[] }
7395
},
7496
setResolvedLayout(rl: ResolvedLayout | null) {
7597
currentResolved = rl ?? emptyResolved;
98+
resolvedLayoutOverridden = true;
7699
},
77100
setProviders: painter.setProviders,
78101
setVirtualizationPins: painter.setVirtualizationPins,
@@ -1357,7 +1380,10 @@ describe('DomPainter', () => {
13571380
expect(lines[1].style.wordSpacing).toBe('');
13581381
});
13591382

1360-
it('renders an error placeholder when a legacy table fragment is missing its lookup entry', () => {
1383+
it('surfaces a missing-block error from resolveLayout when a table fragment references an unknown block', () => {
1384+
// Previous behavior: painter rendered a placeholder for missing lookup entries.
1385+
// New behavior: resolveLayout validates block/measure integrity upstream and throws
1386+
// before the painter runs. Missing-block bugs are now caught at the resolved stage.
13611387
const missingTableLayout: Layout = {
13621388
pageSize: { w: 300, h: 300 },
13631389
pages: [
@@ -1379,19 +1405,8 @@ describe('DomPainter', () => {
13791405
],
13801406
};
13811407

1382-
const consoleErrorSpy = vi.spyOn(console, 'error').mockImplementation(() => {
1383-
// Intentionally empty - suppress expected error logging during this regression test.
1384-
});
1385-
13861408
const painter = createTestPainter({ blocks: [], measures: [] });
1387-
expect(() => painter.paint(missingTableLayout, mount)).not.toThrow();
1388-
1389-
const placeholder = mount.querySelector('.render-error-placeholder') as HTMLElement | null;
1390-
expect(placeholder).toBeTruthy();
1391-
expect(placeholder?.textContent).toContain('[Render Error: missing-table]');
1392-
expect(consoleErrorSpy).toHaveBeenCalled();
1393-
1394-
consoleErrorSpy.mockRestore();
1409+
expect(() => painter.paint(missingTableLayout, mount)).toThrow(/Missing block\/measure/);
13951410
});
13961411

13971412
it('renders an error placeholder when table-cell line rendering throws', () => {
@@ -1680,8 +1695,23 @@ describe('DomPainter', () => {
16801695
});
16811696

16821697
it('throws if blocks and measures length mismatch', () => {
1698+
// Block/measure integrity is now validated at the resolve-layout stage.
16831699
const painter = createTestPainter({ blocks: [block], measures: [] });
1684-
expect(() => painter.paint(layout, mount)).toThrow(/same number of blocks/);
1700+
expect(() => painter.paint(layout, mount)).toThrow();
1701+
});
1702+
1703+
it('rejects resolved-layout-only paint input until body lookups are removed', () => {
1704+
const painter = createDomPainter({});
1705+
1706+
expect(() =>
1707+
painter.paint(
1708+
{
1709+
resolvedLayout: emptyResolved,
1710+
sourceLayout: layout,
1711+
} as DomPainterInput,
1712+
mount,
1713+
),
1714+
).toThrow('DomPainterInput requires body blocks and measures');
16851715
});
16861716

16871717
it('renders placeholder content for empty lines', () => {

packages/layout-engine/painters/dom/src/index.ts

Lines changed: 67 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import type { FlowBlock, Fragment, Layout, Measure, Page, PageMargins, ResolvedLayout } from '@superdoc/contracts';
22
import { DomPainter } from './renderer.js';
3+
import { resolveLayout } from '@superdoc/layout-resolved';
34
import type { PageStyles } from './styles.js';
45
import type { DomPainterInput, PaintSnapshot, PositionMapping, RulerOptions, FlowMode } from './renderer.js';
56

@@ -144,6 +145,11 @@ type BlockMeasurePair = {
144145
measures: Measure[];
145146
};
146147

148+
type DomPainterInputCandidate = Partial<DomPainterInput> & {
149+
resolvedLayout?: ResolvedLayout;
150+
sourceLayout?: Layout;
151+
};
152+
147153
export type DomPainterHandle = {
148154
paint(input: DomPainterInput | Layout, mount: HTMLElement, mapping?: PositionMapping): void;
149155
/**
@@ -177,6 +183,19 @@ function assertRequiredBlockMeasurePair(label: string, blocks: FlowBlock[], meas
177183
}
178184
}
179185

186+
function normalizeRequiredBlockMeasurePair(
187+
label: 'body',
188+
blocks: FlowBlock[] | undefined,
189+
measures: Measure[] | undefined,
190+
): BlockMeasurePair {
191+
if (!Array.isArray(blocks) || !Array.isArray(measures)) {
192+
throw new Error('DomPainterInput requires body blocks and measures; resolved-layout-only input is not supported.');
193+
}
194+
195+
assertRequiredBlockMeasurePair(label, blocks, measures);
196+
return { blocks, measures };
197+
}
198+
180199
function normalizeOptionalBlockMeasurePair(
181200
label: 'header' | 'footer',
182201
blocks: FlowBlock[] | undefined,
@@ -193,6 +212,10 @@ function normalizeOptionalBlockMeasurePair(
193212
return undefined;
194213
}
195214

215+
if (!Array.isArray(blocks) || !Array.isArray(measures)) {
216+
throw new Error(`${label}Blocks and ${label}Measures must be arrays when provided.`);
217+
}
218+
196219
assertRequiredBlockMeasurePair(label, blocks, measures);
197220
return { blocks, measures };
198221
}
@@ -206,8 +229,29 @@ function createEmptyResolvedLayout(flowMode: FlowMode | undefined, pageGap: numb
206229
};
207230
}
208231

209-
function isDomPainterInput(value: DomPainterInput | Layout): value is DomPainterInput {
210-
return 'resolvedLayout' in value && 'sourceLayout' in value && 'blocks' in value && 'measures' in value;
232+
function isLegacyLayoutInput(value: DomPainterInput | Layout): value is Layout {
233+
return 'pages' in value;
234+
}
235+
236+
function normalizeDomPainterInput(input: DomPainterInputCandidate): DomPainterInput {
237+
if (!input.resolvedLayout || !input.sourceLayout) {
238+
throw new Error('DomPainterInput requires resolvedLayout and sourceLayout.');
239+
}
240+
241+
const body = normalizeRequiredBlockMeasurePair('body', input.blocks, input.measures);
242+
const header = normalizeOptionalBlockMeasurePair('header', input.headerBlocks, input.headerMeasures);
243+
const footer = normalizeOptionalBlockMeasurePair('footer', input.footerBlocks, input.footerMeasures);
244+
245+
return {
246+
resolvedLayout: input.resolvedLayout,
247+
sourceLayout: input.sourceLayout,
248+
blocks: body.blocks,
249+
measures: body.measures,
250+
headerBlocks: header?.blocks,
251+
headerMeasures: header?.measures,
252+
footerBlocks: footer?.blocks,
253+
footerMeasures: footer?.measures,
254+
};
211255
}
212256

213257
function buildLegacyPaintInput(
@@ -216,8 +260,25 @@ function buildLegacyPaintInput(
216260
flowMode: FlowMode | undefined,
217261
pageGap: number | undefined,
218262
): DomPainterInput {
263+
// Derive a resolved layout from the legacy block/measure state when the caller
264+
// has not supplied one via `setResolvedLayout`. The painter now reads all body
265+
// fragment data from the resolved layout, so an empty resolved layout would
266+
// produce a blank render.
267+
let resolvedLayout: ResolvedLayout;
268+
if (legacyState.resolvedLayout) {
269+
resolvedLayout = legacyState.resolvedLayout;
270+
} else if (legacyState.blocks.length === 0 && legacyState.measures.length === 0) {
271+
resolvedLayout = createEmptyResolvedLayout(flowMode, pageGap);
272+
} else {
273+
resolvedLayout = resolveLayout({
274+
layout,
275+
flowMode: flowMode ?? 'paginated',
276+
blocks: legacyState.blocks,
277+
measures: legacyState.measures,
278+
});
279+
}
219280
return {
220-
resolvedLayout: legacyState.resolvedLayout ?? createEmptyResolvedLayout(flowMode, pageGap),
281+
resolvedLayout,
221282
sourceLayout: layout,
222283
blocks: legacyState.blocks,
223284
measures: legacyState.measures,
@@ -253,9 +314,9 @@ export const createDomPainter = (options: DomPainterOptions): DomPainterHandle =
253314

254315
return {
255316
paint(input: DomPainterInput | Layout, mount: HTMLElement, mapping?: PositionMapping) {
256-
const normalizedInput = isDomPainterInput(input)
257-
? input
258-
: buildLegacyPaintInput(input, legacyState, options.flowMode, options.pageGap);
317+
const normalizedInput = isLegacyLayoutInput(input)
318+
? buildLegacyPaintInput(input, legacyState, options.flowMode, options.pageGap)
319+
: normalizeDomPainterInput(input);
259320
painter.paint(normalizedInput, mount, mapping);
260321
},
261322
setData(

packages/layout-engine/painters/dom/src/renderer.ts

Lines changed: 20 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -250,17 +250,20 @@ export type RenderedLineInfo = {
250250
* Input to `DomPainter.paint()`.
251251
*
252252
* `resolvedLayout` is the canonical resolved data. The remaining fields are
253-
* bridge data carried for internal rendering of non-paragraph fragments
254-
* (tables, images, drawings) that have not yet been migrated to resolved items.
253+
* still required bridge data until the painter can render solely from resolved
254+
* items for lookups, change tracking, and non-paragraph fragment rendering.
255255
*/
256256
export type DomPainterInput = {
257257
resolvedLayout: ResolvedLayout;
258-
/** Raw Layout for internal fragment access (bridgewill be removed once all fragment types are resolved). */
258+
/** Raw Layout for internal fragment access (bridge, will be removed once render loops iterate resolved items). */
259259
sourceLayout: Layout;
260+
/** Main document blocks/measures used for lookups and version tracking. */
260261
blocks: FlowBlock[];
261262
measures: Measure[];
263+
/** Header block data (still needed for decoration rendering, no resolved path yet). */
262264
headerBlocks?: FlowBlock[];
263265
headerMeasures?: Measure[];
266+
/** Footer block data (still needed for decoration rendering, no resolved path yet). */
264267
footerBlocks?: FlowBlock[];
265268
footerMeasures?: Measure[];
266269
};
@@ -1640,7 +1643,7 @@ export class DomPainter {
16401643
});
16411644
}
16421645

1643-
// Track changed blocks
1646+
// Track changed blocks (decoration only now, body change detection uses resolved version)
16441647
const changed = new Set<string>();
16451648
nextLookup.forEach((entry, id) => {
16461649
const previous = this.blockLookup.get(id);
@@ -1674,7 +1677,13 @@ export class DomPainter {
16741677
// Complex transactions (paste, multi-step replace, etc.) fall back to full rebuild.
16751678
const isSimpleTransaction = mapping && mapping.maps.length === 1;
16761679
if (mapping && !isSimpleTransaction) {
1677-
// Complex transaction - force all fragments to rebuild (safe fallback)
1680+
// Complex transaction, force all body fragments to rebuild (safe fallback).
1681+
for (const page of input.resolvedLayout.pages) {
1682+
for (const item of page.items) {
1683+
if ('blockId' in item) this.changedBlocks.add(item.blockId);
1684+
}
1685+
}
1686+
// Also mark all header/footer blocks as changed.
16781687
this.blockLookup.forEach((_, id) => this.changedBlocks.add(id));
16791688
this.currentMapping = null;
16801689
} else {
@@ -2426,6 +2435,7 @@ export class DomPainter {
24262435

24272436
return separatorPositions;
24282437
}
2438+
24292439
private renderDecorationsForPage(
24302440
pageEl: HTMLElement,
24312441
page: Page,
@@ -5059,6 +5069,11 @@ export class DomPainter {
50595069
// Inner cell fragments still use legacy applyFragmentFrame via deps closure.
50605070
if (resolvedItem) {
50615071
this.applyResolvedFragmentFrame(el, resolvedItem, fragment, context.section);
5072+
// Re-apply the SDT group width override after the resolved frame, so block-SDT
5073+
// containers can stretch table fragments to match sibling paragraph widths.
5074+
if (sdtBoundary?.widthOverride != null) {
5075+
el.style.width = `${sdtBoundary.widthOverride}px`;
5076+
}
50625077
}
50635078

50645079
return el;

packages/layout-engine/painters/dom/src/virtualization.test.ts

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest';
22
import { createDomPainter } from './index.js';
3+
import { resolveLayout } from '@superdoc/layout-resolved';
34
import type { DomPainterOptions, DomPainterInput, PaintSnapshot } from './index.js';
45
import type { FlowBlock, Measure, Layout, Fragment, PageMargins, ResolvedLayout } from '@superdoc/contracts';
56

@@ -21,11 +22,24 @@ function createTestPainter(opts: { blocks?: FlowBlock[]; measures?: Measure[] }
2122

2223
return {
2324
paint(layout: Layout, mount: HTMLElement, mapping?: unknown) {
25+
const effectiveResolved =
26+
currentBlocks.length === 0 && currentMeasures.length === 0
27+
? currentResolved
28+
: resolveLayout({
29+
layout,
30+
flowMode: opts.flowMode ?? 'paginated',
31+
blocks: currentBlocks,
32+
measures: currentMeasures,
33+
});
2434
const input: DomPainterInput = {
25-
resolvedLayout: currentResolved,
35+
resolvedLayout: effectiveResolved,
2636
sourceLayout: layout,
2737
blocks: currentBlocks,
2838
measures: currentMeasures,
39+
headerBlocks: undefined,
40+
headerMeasures: undefined,
41+
footerBlocks: undefined,
42+
footerMeasures: undefined,
2943
};
3044
painter.paint(input, mount, mapping as any);
3145
},

packages/layout-engine/painters/dom/tsconfig.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
"references": [
1313
{ "path": "../../contracts/tsconfig.json" },
1414
{ "path": "../../dom-contract/tsconfig.json" },
15+
{ "path": "../../layout-resolved/tsconfig.json" },
1516
{ "path": "../../measuring/dom/tsconfig.json" },
1617
{ "path": "../../../../shared/common/tsconfig.json" }
1718
]

0 commit comments

Comments
 (0)