Skip to content

Commit 506bde3

Browse files
authored
[1/3] refactor(painter): consume only ResolvedLayout (SD-2836) (#3116)
* 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. * 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. * 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. * 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. * 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. * 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. * 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. * 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. * 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. * 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. * 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.
1 parent 0132a97 commit 506bde3

25 files changed

Lines changed: 732 additions & 397 deletions

File tree

packages/layout-engine/contracts/src/index.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2065,4 +2065,8 @@ export type {
20652065
} from './resolved-layout.js';
20662066
export { isResolvedTableItem, isResolvedImageItem, isResolvedDrawingItem } from './resolved-layout.js';
20672067

2068+
// Pure transformations on inline-run shapes (used by pm-adapter, layout-bridge,
2069+
// and painter-dom). Located in contracts to avoid reverse stage dependencies.
2070+
export { expandRunsForInlineNewlines, sliceRunsForLine } from './run-helpers.js';
2071+
20682072
export * as Engines from './engines/index.js';

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

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
11
import type {
2+
ColumnLayout,
3+
ColumnRegion,
24
DrawingBlock,
35
FlowMode,
46
Fragment,
@@ -66,6 +68,10 @@ export type ResolvedPage = {
6668
};
6769
/** Page orientation. */
6870
orientation?: 'portrait' | 'landscape';
71+
/** Column layout configuration for this page (reflects page-start config). */
72+
columns?: ColumnLayout;
73+
/** Vertical column regions when continuous section breaks change column layout mid-page. */
74+
columnRegions?: ColumnRegion[];
6975
};
7076

7177
/** Union of all resolved paint item kinds. */
@@ -111,6 +117,10 @@ export type ResolvedFragmentItem = {
111117
zIndex?: number;
112118
/** Source fragment kind — used by the painter for wrapper style decisions. */
113119
fragmentKind: Fragment['kind'];
120+
/** Source fragment back-pointer. Lets the painter iterate resolved items
121+
* and pass the underlying fragment to render helpers without indexing
122+
* back into the legacy `page.fragments` array. */
123+
fragment: Fragment;
114124
/** Block ID. Written to data-block-id. */
115125
blockId: string;
116126
/**
@@ -257,6 +267,8 @@ export type ResolvedTableItem = {
257267
* promote this to a permanent API surface.
258268
*/
259269
fragmentIndex: number;
270+
/** Source TableFragment back-pointer (see ResolvedFragmentItem.fragment). */
271+
fragment: Fragment;
260272
/** ProseMirror start position for click-to-position mapping. */
261273
pmStart?: number;
262274
/** ProseMirror end position for click-to-position mapping. */
@@ -318,6 +330,8 @@ export type ResolvedImageItem = {
318330
* promote this to a permanent API surface.
319331
*/
320332
fragmentIndex: number;
333+
/** Source ImageFragment back-pointer (see ResolvedFragmentItem.fragment). */
334+
fragment: Fragment;
321335
/** ProseMirror start position for click-to-position mapping. */
322336
pmStart?: number;
323337
/** ProseMirror end position for click-to-position mapping. */
@@ -371,6 +385,8 @@ export type ResolvedDrawingItem = {
371385
* promote this to a permanent API surface.
372386
*/
373387
fragmentIndex: number;
388+
/** Source DrawingFragment back-pointer (see ResolvedFragmentItem.fragment). */
389+
fragment: Fragment;
374390
/** ProseMirror start position for click-to-position mapping. */
375391
pmStart?: number;
376392
/** ProseMirror end position for click-to-position mapping. */
Lines changed: 139 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,139 @@
1+
import { describe, it, expect } from 'vitest';
2+
import type { FlowBlock, Line, ParagraphBlock, Run, TabRun, TextRun, TrackedChangeMeta } from './index.js';
3+
import { expandRunsForInlineNewlines, sliceRunsForLine } from './run-helpers.js';
4+
5+
describe('expandRunsForInlineNewlines', () => {
6+
const makeRun = (text: string, pmStart = 0): TextRun => ({
7+
text,
8+
fontFamily: 'Arial',
9+
fontSize: 12,
10+
pmStart,
11+
pmEnd: pmStart + text.length,
12+
});
13+
14+
it('returns runs without inline newlines unchanged', () => {
15+
const runs: Run[] = [makeRun('hello')];
16+
expect(expandRunsForInlineNewlines(runs)).toEqual(runs);
17+
});
18+
19+
it('splits a text run at a single inline newline', () => {
20+
const result = expandRunsForInlineNewlines([makeRun('foo\nbar')]);
21+
expect(result).toHaveLength(3);
22+
expect(result[0]).toMatchObject({ text: 'foo', pmStart: 0, pmEnd: 3 });
23+
expect(result[1]).toMatchObject({ kind: 'break', pmStart: 3, pmEnd: 4 });
24+
expect(result[2]).toMatchObject({ text: 'bar', pmStart: 4, pmEnd: 7 });
25+
});
26+
27+
it('keeps the break and advances the cursor for a leading newline', () => {
28+
const result = expandRunsForInlineNewlines([makeRun('\nfoo')]);
29+
expect(result).toHaveLength(2);
30+
expect(result[0]).toMatchObject({ kind: 'break', pmStart: 0, pmEnd: 1 });
31+
expect(result[1]).toMatchObject({ text: 'foo', pmStart: 1, pmEnd: 4 });
32+
});
33+
34+
it('keeps both breaks when a run contains consecutive inline newlines', () => {
35+
const result = expandRunsForInlineNewlines([makeRun('a\n\nb')]);
36+
expect(result).toHaveLength(4);
37+
expect(result[0]).toMatchObject({ text: 'a', pmStart: 0, pmEnd: 1 });
38+
expect(result[1]).toMatchObject({ kind: 'break', pmStart: 1, pmEnd: 2 });
39+
expect(result[2]).toMatchObject({ kind: 'break', pmStart: 2, pmEnd: 3 });
40+
expect(result[3]).toMatchObject({ text: 'b', pmStart: 3, pmEnd: 4 });
41+
});
42+
43+
it('does not emit an empty trailing text run for a trailing newline', () => {
44+
const result = expandRunsForInlineNewlines([makeRun('foo\n')]);
45+
expect(result).toHaveLength(2);
46+
expect(result[0]).toMatchObject({ text: 'foo', pmStart: 0, pmEnd: 3 });
47+
expect(result[1]).toMatchObject({ kind: 'break', pmStart: 3, pmEnd: 4 });
48+
});
49+
50+
it('propagates trackedChange metadata onto emitted break runs', () => {
51+
const trackedChange: TrackedChangeMeta = {
52+
id: 'change-1',
53+
kind: 'insert',
54+
author: 'alice',
55+
date: '2024-01-01T00:00:00Z',
56+
};
57+
const run: TextRun = { ...makeRun('foo\nbar'), trackedChange };
58+
const result = expandRunsForInlineNewlines([run]);
59+
expect(result[1]).toMatchObject({ kind: 'break', trackedChange });
60+
});
61+
});
62+
63+
describe('sliceRunsForLine', () => {
64+
const makeTextRun = (text: string, pmStart = 0): TextRun => ({
65+
text,
66+
fontFamily: 'Arial',
67+
fontSize: 12,
68+
pmStart,
69+
pmEnd: pmStart + text.length,
70+
});
71+
72+
const makeParagraph = (runs: Run[]): ParagraphBlock => ({
73+
kind: 'paragraph',
74+
id: 'p-1',
75+
runs,
76+
});
77+
78+
const makeLine = (overrides: Partial<Line> = {}): Line => ({
79+
fromRun: 0,
80+
fromChar: 0,
81+
toRun: 0,
82+
toChar: 0,
83+
width: 0,
84+
ascent: 12,
85+
descent: 4,
86+
lineHeight: 16,
87+
...overrides,
88+
});
89+
90+
it('returns an empty array for non-paragraph blocks', () => {
91+
const block: FlowBlock = {
92+
kind: 'image',
93+
id: 'i-1',
94+
attrs: { src: 'about:blank', alt: '' },
95+
} as unknown as FlowBlock;
96+
expect(sliceRunsForLine(block, makeLine())).toEqual([]);
97+
});
98+
99+
it('passes tab runs through unchanged', () => {
100+
const tab: TabRun = { kind: 'tab', text: '\t', pmStart: 0, pmEnd: 1 };
101+
const block = makeParagraph([tab]);
102+
const line = makeLine({ toRun: 0, fromChar: 0, toChar: 1 });
103+
expect(sliceRunsForLine(block, line)).toEqual([tab]);
104+
});
105+
106+
it('passes line-break runs through unchanged', () => {
107+
const lineBreak: Run = { kind: 'lineBreak', pmStart: 0, pmEnd: 1 } as Run;
108+
const block = makeParagraph([lineBreak]);
109+
const line = makeLine({ toRun: 0, fromChar: 0, toChar: 1 });
110+
expect(sliceRunsForLine(block, line)).toEqual([lineBreak]);
111+
});
112+
113+
it('slices text on the first/last run and adjusts pmStart/pmEnd', () => {
114+
const run = makeTextRun('hello world', 100);
115+
const block = makeParagraph([run]);
116+
const line = makeLine({ fromRun: 0, fromChar: 0, toRun: 0, toChar: 5 });
117+
const result = sliceRunsForLine(block, line);
118+
expect(result).toHaveLength(1);
119+
expect(result[0]).toMatchObject({ text: 'hello', pmStart: 100, pmEnd: 105 });
120+
});
121+
122+
it('passes middle text runs through unchanged when the line spans multiple runs', () => {
123+
const first = makeTextRun('foo', 0);
124+
const middle = makeTextRun('bar', 3);
125+
const last = makeTextRun('baz', 6);
126+
const block = makeParagraph([first, middle, last]);
127+
const line = makeLine({ fromRun: 0, fromChar: 0, toRun: 2, toChar: 3 });
128+
const result = sliceRunsForLine(block, line);
129+
expect(result).toHaveLength(3);
130+
expect(result[1]).toBe(middle);
131+
});
132+
133+
it('drops empty slices when the requested range produces no characters', () => {
134+
const run = makeTextRun('abc', 0);
135+
const block = makeParagraph([run]);
136+
const line = makeLine({ fromRun: 0, fromChar: 2, toRun: 0, toChar: 2 });
137+
expect(sliceRunsForLine(block, line)).toEqual([]);
138+
});
139+
});
Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,109 @@
1+
/**
2+
* Pure transformations on inline-run shapes.
3+
*
4+
* These helpers operate on `Run[]` shapes defined in this contracts package.
5+
* They have no upstream dependencies (no pm-adapter, no layout-bridge, no
6+
* style-engine), so any stage can consume them without creating a reverse
7+
* dependency back into a downstream package.
8+
*/
9+
10+
import type { FlowBlock, Line, Run, TextRun } from './index.js';
11+
12+
/**
13+
* Expands text runs that contain inline newlines into multiple runs.
14+
*
15+
* @param {Run[]} runs - The runs to expand
16+
* @returns {Run[]} The expanded runs
17+
*/
18+
export function expandRunsForInlineNewlines(runs: Run[]): Run[] {
19+
const result: Run[] = [];
20+
for (const run of runs) {
21+
const textRun = run as TextRun;
22+
if ('text' in run && typeof textRun.text === 'string' && textRun.text.includes('\n')) {
23+
const segments = textRun.text.split('\n');
24+
let cursor = textRun.pmStart ?? 0;
25+
segments.forEach((segment, idx) => {
26+
if (segment.length > 0) {
27+
result.push({ ...textRun, text: segment, pmStart: cursor, pmEnd: cursor + segment.length });
28+
cursor += segment.length;
29+
}
30+
if (idx !== segments.length - 1) {
31+
result.push({
32+
kind: 'break',
33+
breakType: 'line',
34+
pmStart: cursor,
35+
pmEnd: cursor + 1,
36+
sdt: textRun.sdt,
37+
trackedChange: textRun.trackedChange,
38+
});
39+
cursor += 1;
40+
}
41+
});
42+
} else {
43+
result.push(run);
44+
}
45+
}
46+
return result;
47+
}
48+
49+
/**
50+
* Extracts the subset of runs that appear in a specific line.
51+
* Handles partial runs that span multiple lines.
52+
*
53+
* @param block - The paragraph block containing the runs
54+
* @param line - The line to extract runs for
55+
* @returns Array of runs present in the line
56+
*/
57+
export function sliceRunsForLine(block: FlowBlock, line: Line): Run[] {
58+
const result: Run[] = [];
59+
if (block.kind !== 'paragraph') return result;
60+
61+
for (let runIndex = line.fromRun; runIndex <= line.toRun; runIndex += 1) {
62+
const run = block.runs[runIndex];
63+
if (!run) continue;
64+
65+
if (run.kind === 'tab') {
66+
result.push(run);
67+
continue;
68+
}
69+
70+
// Images, line breaks, breaks, field annotations, and math runs are atomic
71+
// units. They occupy a single character of the run sequence and are passed
72+
// through to the result without slicing.
73+
if (
74+
'src' in run ||
75+
run.kind === 'lineBreak' ||
76+
run.kind === 'break' ||
77+
run.kind === 'fieldAnnotation' ||
78+
run.kind === 'math'
79+
) {
80+
result.push(run);
81+
continue;
82+
}
83+
84+
const text = run.text ?? '';
85+
const isFirstRun = runIndex === line.fromRun;
86+
const isLastRun = runIndex === line.toRun;
87+
88+
if (isFirstRun || isLastRun) {
89+
const start = isFirstRun ? line.fromChar : 0;
90+
const end = isLastRun ? line.toChar : text.length;
91+
const slice = text.slice(start, end);
92+
if (!slice) continue;
93+
const pmStart =
94+
run.pmStart != null ? run.pmStart + start : run.pmEnd != null ? run.pmEnd - (text.length - start) : undefined;
95+
const pmEnd =
96+
run.pmStart != null ? run.pmStart + end : run.pmEnd != null ? run.pmEnd - (text.length - end) : undefined;
97+
result.push({
98+
...run,
99+
text: slice,
100+
pmStart,
101+
pmEnd,
102+
});
103+
} else {
104+
result.push(run);
105+
}
106+
}
107+
108+
return result;
109+
}

packages/layout-engine/layout-bridge/src/index.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,8 @@ export type { HeaderFooterLayoutResult, IncrementalLayoutResult } from './increm
7474
export { computeDisplayPageNumber } from '@superdoc/layout-engine';
7575
export type { DisplayPageInfo, HeaderFooterConstraints } from '@superdoc/layout-engine';
7676
export { remeasureParagraph } from './remeasure';
77-
export { measureCharacterX, sliceRunsForLine } from './text-measurement';
77+
export { measureCharacterX } from './text-measurement';
78+
export { sliceRunsForLine } from '@superdoc/contracts';
7879
export { clickToPositionDom, findPageElement } from './dom-mapping';
7980
export { isListItem, getWordLayoutConfig, calculateTextStartIndent, extractParagraphIndent } from './list-indent-utils';
8081
export type { TextIndentCalculationParams } from './list-indent-utils';

0 commit comments

Comments
 (0)