Skip to content

Commit 4b52441

Browse files
committed
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.
1 parent cd490c5 commit 4b52441

3 files changed

Lines changed: 44 additions & 31 deletions

File tree

packages/layout-engine/painters/dom/src/between-borders.test.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,7 @@ const buildResolvedItems = (fragments: readonly Fragment[], blocks: TestBlockLis
102102
width: fragment.width,
103103
height: 'height' in fragment && typeof fragment.height === 'number' ? fragment.height : 0,
104104
fragmentKind: fragment.kind,
105+
fragment,
105106
blockId: fragment.blockId,
106107
fragmentIndex: index,
107108
paragraphBorders: borders,
@@ -113,7 +114,7 @@ const buildResolvedItems = (fragments: readonly Fragment[], blocks: TestBlockLis
113114

114115
/** Test helper: run computeBetweenBorderFlags given fragments and the underlying blocks. */
115116
const runFlags = (fragments: readonly Fragment[], blocks: TestBlockList) =>
116-
computeBetweenBorderFlags(fragments, buildResolvedItems(fragments, blocks));
117+
computeBetweenBorderFlags(buildResolvedItems(fragments, blocks));
117118

118119
const paraFragment = (blockId: string, overrides?: Partial<ParaFragment>): ParaFragment => ({
119120
kind: 'para',

packages/layout-engine/painters/dom/src/features/paragraph-borders/group-analysis.ts

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
* @ooxml w:pPr/w:pBdr/w:between — between border for grouped paragraphs
99
* @spec ECMA-376 §17.3.1.24 (pBdr)
1010
*/
11-
import type { Fragment, ListItemFragment, ResolvedPaintItem, ResolvedFragmentItem } from '@superdoc/contracts';
11+
import type { ListItemFragment, ResolvedPaintItem, ResolvedFragmentItem } from '@superdoc/contracts';
1212
import { hashParagraphBorders } from '../../paragraph-hash-utils.js';
1313

1414
/**
@@ -69,23 +69,25 @@ function isResolvedFragmentWithBorders(
6969
* Middle fragments in a chain of 3+ get both flags.
7070
*/
7171
export const computeBetweenBorderFlags = (
72-
fragments: readonly Fragment[],
7372
resolvedItems: readonly ResolvedPaintItem[],
7473
): Map<number, BetweenBorderInfo> => {
7574
// Phase 1: determine which consecutive pairs form between-border groups
7675
const pairFlags = new Set<number>();
7776
const noBetweenPairs = new Set<number>();
7877

79-
for (let i = 0; i < fragments.length - 1; i += 1) {
80-
const frag = fragments[i];
78+
for (let i = 0; i < resolvedItems.length - 1; i += 1) {
79+
const resolvedCur = resolvedItems[i];
80+
if (resolvedCur.kind !== 'fragment') continue;
81+
const frag = resolvedCur.fragment;
8182
if (frag.kind !== 'para' && frag.kind !== 'list-item') continue;
8283
if (frag.continuesOnNext) continue;
8384

84-
const resolvedCur = resolvedItems[i];
8585
if (!isResolvedFragmentWithBorders(resolvedCur)) continue;
8686
const borders = resolvedCur.paragraphBorders;
8787

88-
const next = fragments[i + 1];
88+
const resolvedNext = resolvedItems[i + 1];
89+
if (resolvedNext.kind !== 'fragment') continue;
90+
const next = resolvedNext.fragment;
8991
if (next.kind !== 'para' && next.kind !== 'list-item') continue;
9092
if (next.continuesFromPrev) continue;
9193
if (next.blockId === frag.blockId && next.kind === 'para') continue;
@@ -97,7 +99,6 @@ export const computeBetweenBorderFlags = (
9799
)
98100
continue;
99101

100-
const resolvedNext = resolvedItems[i + 1];
101102
if (!isResolvedFragmentWithBorders(resolvedNext)) continue;
102103
const nextBorders = resolvedNext.paragraphBorders;
103104

@@ -129,10 +130,12 @@ export const computeBetweenBorderFlags = (
129130
const result = new Map<number, BetweenBorderInfo>();
130131

131132
for (const i of pairFlags) {
132-
const frag = fragments[i];
133-
const next = fragments[i + 1];
134133
const resolvedCur = resolvedItems[i];
135-
const fragHeight = resolvedCur && 'height' in resolvedCur && resolvedCur.height != null ? resolvedCur.height : 0;
134+
const resolvedNext = resolvedItems[i + 1];
135+
if (resolvedCur.kind !== 'fragment' || resolvedNext.kind !== 'fragment') continue;
136+
const frag = resolvedCur.fragment;
137+
const next = resolvedNext.fragment;
138+
const fragHeight = 'height' in resolvedCur && resolvedCur.height != null ? resolvedCur.height : 0;
136139
const gapBelow = Math.max(0, next.y - (frag.y + fragHeight));
137140
const isNoBetween = noBetweenPairs.has(i);
138141

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

Lines changed: 29 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -2225,8 +2225,8 @@ export class DomPainter {
22252225
};
22262226

22272227
const resolvedItems = resolvedPage?.items ?? [];
2228-
const sdtBoundaries = computeSdtBoundaries(page.fragments, resolvedItems, this.sdtLabelsRendered);
2229-
const betweenBorderFlags = computeBetweenBorderFlags(page.fragments, resolvedItems);
2228+
const sdtBoundaries = computeSdtBoundaries(resolvedItems, this.sdtLabelsRendered);
2229+
const betweenBorderFlags = computeBetweenBorderFlags(resolvedItems);
22302230

22312231
resolvedItems.forEach((resolvedItem, index) => {
22322232
if (resolvedItem.kind !== 'fragment') return;
@@ -2581,7 +2581,7 @@ export class DomPainter {
25812581

25822582
// Compute between-border flags for header/footer paragraph fragments
25832583
const decorationItems = data.items ?? [];
2584-
const betweenBorderFlags = computeBetweenBorderFlags(data.fragments, decorationItems);
2584+
const betweenBorderFlags = computeBetweenBorderFlags(decorationItems);
25852585

25862586
// Separate behindDoc fragments from normal fragments.
25872587
// Prefer explicit fragment.behindDoc when present. Keep zIndex===0 as a
@@ -2774,8 +2774,8 @@ export class DomPainter {
27742774
const existing = new Map(state.fragments.map((frag) => [frag.key, frag]));
27752775
const nextFragments: FragmentDomState[] = [];
27762776
const resolvedItems = resolvedPage?.items ?? [];
2777-
const sdtBoundaries = computeSdtBoundaries(page.fragments, resolvedItems, this.sdtLabelsRendered);
2778-
const betweenBorderFlags = computeBetweenBorderFlags(page.fragments, resolvedItems);
2777+
const sdtBoundaries = computeSdtBoundaries(resolvedItems, this.sdtLabelsRendered);
2778+
const betweenBorderFlags = computeBetweenBorderFlags(resolvedItems);
27792779

27802780
const contextBase: FragmentRenderContext = {
27812781
pageNumber: page.number,
@@ -2944,8 +2944,8 @@ export class DomPainter {
29442944
};
29452945

29462946
const resolvedItems = resolvedPage?.items ?? [];
2947-
const sdtBoundaries = computeSdtBoundaries(page.fragments, resolvedItems, this.sdtLabelsRendered);
2948-
const betweenBorderFlags = computeBetweenBorderFlags(page.fragments, resolvedItems);
2947+
const sdtBoundaries = computeSdtBoundaries(resolvedItems, this.sdtLabelsRendered);
2948+
const betweenBorderFlags = computeBetweenBorderFlags(resolvedItems);
29492949
const fragmentStates: FragmentDomState[] = resolvedItems.flatMap((resolvedItem, index) => {
29502950
if (resolvedItem.kind !== 'fragment') return [];
29512951
const fragment = resolvedItem.fragment;
@@ -7255,52 +7255,61 @@ export class DomPainter {
72557255
}
72567256

72577257
const computeSdtBoundaries = (
7258-
fragments: readonly Fragment[],
72597258
resolvedItems: readonly ResolvedPaintItem[],
72607259
sdtLabelsRendered: Set<string>,
72617260
): Map<number, SdtBoundaryOptions> => {
72627261
const boundaries = new Map<number, SdtBoundaryOptions>();
7263-
const containerKeys: (string | null)[] = fragments.map((_frag, idx) => {
7264-
const item = resolvedItems[idx];
7262+
const containerKeys: (string | null)[] = resolvedItems.map((item) => {
72657263
if (item && 'sdtContainerKey' in item) {
72667264
const key = (item as { sdtContainerKey?: string | null }).sdtContainerKey;
72677265
return key ?? null;
72687266
}
72697267
return null;
72707268
});
72717269

7270+
const fragmentOf = (idx: number): Fragment | null => {
7271+
const item = resolvedItems[idx];
7272+
return item && item.kind === 'fragment' ? item.fragment : null;
7273+
};
7274+
72727275
let i = 0;
7273-
while (i < fragments.length) {
7276+
while (i < resolvedItems.length) {
72747277
const currentKey = containerKeys[i];
7275-
if (!currentKey) {
7278+
const startFrag = fragmentOf(i);
7279+
if (!currentKey || !startFrag) {
72767280
i += 1;
72777281
continue;
72787282
}
72797283

7280-
let groupRight = fragments[i].x + fragments[i].width;
7284+
let groupRight = startFrag.x + startFrag.width;
72817285
let j = i;
72827286

7283-
while (j + 1 < fragments.length && containerKeys[j + 1] === currentKey) {
7287+
while (j + 1 < resolvedItems.length && containerKeys[j + 1] === currentKey) {
72847288
j += 1;
7285-
const fragmentRight = fragments[j].x + fragments[j].width;
7289+
const nextFrag = fragmentOf(j);
7290+
if (!nextFrag) break;
7291+
const fragmentRight = nextFrag.x + nextFrag.width;
72867292
if (fragmentRight > groupRight) {
72877293
groupRight = fragmentRight;
72887294
}
72897295
}
72907296

72917297
for (let k = i; k <= j; k += 1) {
7292-
const fragment = fragments[k];
7298+
const fragment = fragmentOf(k);
7299+
if (!fragment) continue;
72937300
const isStart = k === i;
72947301
const isEnd = k === j;
72957302

72967303
let paddingBottomOverride: number | undefined;
72977304
if (!isEnd) {
7298-
const nextFragment = fragments[k + 1];
7305+
const nextFragment = fragmentOf(k + 1);
72997306
const currentHeight = (resolvedItems[k] as { height?: number } | undefined)?.height ?? 0;
73007307
const currentBottom = fragment.y + currentHeight;
7301-
const gapToNext = nextFragment.y - currentBottom;
7302-
if (gapToNext > 0) {
7303-
paddingBottomOverride = gapToNext;
7308+
if (nextFragment) {
7309+
const gapToNext = nextFragment.y - currentBottom;
7310+
if (gapToNext > 0) {
7311+
paddingBottomOverride = gapToNext;
7312+
}
73047313
}
73057314
}
73067315

0 commit comments

Comments
 (0)