Skip to content

Commit 41722f5

Browse files
fix: prevent text overlap when multi-column sections span pages (SD-1869) (#2676)
* fix: use actual font size for tab runs instead of hardcoded 12pt default The tab measurement code hardcoded `maxFontSize: 12` for tab-created lines and tab runs, causing incorrect line height calculations when the surrounding text used a different font size. This also adds `fontSize: '0'` to line styles to eliminate the CSS strut that caused baseline misalignment between normal-flow and absolutely-positioned (tab-aligned) children. * fix: prevent text overlap when multi-column section spills across pages When a multi-column section's content spans multiple pages and columns have unequal heights, the next section's content was positioned at the shorter column's cursor Y instead of below all column content. This caused visible text overlap on page 2. Add maxCursorY to PageState to track the deepest Y reached across all columns. startMidPageRegion now uses this value so the next section begins below all column content. * fix: eliminate CSS strut misalignment between tab-segmented and normal lines Lines with tab stops use absolute positioning for text segments, which bypasses the CSS strut. Normal-flow lines inherit the browser's default 16px font-size, creating a strut that shifts text down ~1px via baseline alignment. This made tab-indented first lines appear shifted up relative to continuation lines. Zero the line container's font-size to remove the strut, and restore explicit font-size on the three child elements that inherit rather than set their own: empty-run caret placeholder (lineHeight), math run wrapper (run height), and field annotation wrapper (16px fallback). * test: add regression test for maxCursorY overlap fix (SD-1869) Verifies that mid-page column transitions start below the tallest column when columns have unequal heights. Uses a 3-col → 2-col transition to exercise the maxCursorY tracking without triggering the new-page guard (columnIndexBefore >= newColumns.count). * fix: use browser default font size for math run fallback run.height is a layout heuristic that can reach 80–100px for tall expressions (fractions, equation arrays). Using it as the wrapper's font-size made the plain-text fallback render at that size. Swap to BROWSER_DEFAULT_FONT_SIZE (16px) — MathML has its own scaling, so the value only affects the textContent fallback path. --------- Co-authored-by: Caio Pizzol <caio@harbourshare.com>
1 parent 4ac72bf commit 41722f5

15 files changed

Lines changed: 368 additions & 12 deletions

File tree

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

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2013,6 +2013,78 @@ describe('layoutDocument', () => {
20132013
expect(p3Fragment).toBeDefined();
20142014
expect(p3Fragment?.width).toBe(210); // Half width = two columns
20152015
});
2016+
2017+
it('starts new region below tallest column when columns have unequal heights', () => {
2018+
// Regression test for SD-1869: when a multi-column section has unequal column
2019+
// heights, the next region must start below the TALLEST column, not the last
2020+
// column's cursor. Without the maxCursorY fix, the new region would start at
2021+
// the shorter column's bottom, overlapping the taller one.
2022+
//
2023+
// Uses a 3-col → 2-col transition because the layout engine forces a new page
2024+
// when reducing to fewer columns than the current column index (guard at
2025+
// columnIndexBefore >= newColumns.count). With 3→2, content in col1
2026+
// (columnIndex=1) stays on the same page (1 < 2).
2027+
const toThreeColumns: FlowBlock = {
2028+
kind: 'sectionBreak',
2029+
id: 'sb-to-3col',
2030+
type: 'continuous',
2031+
columns: { count: 3, gap: 24 },
2032+
margins: {},
2033+
};
2034+
const toTwoColumns: FlowBlock = {
2035+
kind: 'sectionBreak',
2036+
id: 'sb-to-2col',
2037+
type: 'continuous',
2038+
columns: { count: 2, gap: 48 },
2039+
margins: {},
2040+
};
2041+
2042+
const blocks: FlowBlock[] = [
2043+
{ kind: 'paragraph', id: 'p1', runs: [] }, // single column preamble
2044+
toThreeColumns,
2045+
{ kind: 'paragraph', id: 'p-cols', runs: [] }, // 3 lines → col0 gets 2, col1 gets 1
2046+
toTwoColumns,
2047+
{ kind: 'paragraph', id: 'p-after', runs: [] }, // must start below tallest column
2048+
];
2049+
2050+
// p-cols: 3 lines of 250px each (750px total)
2051+
// Available column height = 720 (page bottom) - 112 (region top) = 608px
2052+
// Column 0 fits lines 0+1 (500px), line 2 overflows to column 1
2053+
// Column 0 bottom = 112 + 500 = 612
2054+
// Column 1 bottom = 112 + 250 = 362
2055+
const measures: Measure[] = [
2056+
makeMeasure([40]), // p1
2057+
{ kind: 'sectionBreak' },
2058+
makeMeasure([250, 250, 250]), // p-cols: 3 lines, 2 in col0 + 1 in col1
2059+
{ kind: 'sectionBreak' },
2060+
makeMeasure([40]), // p-after
2061+
];
2062+
2063+
const options: LayoutOptions = {
2064+
pageSize: { w: 612, h: 792 },
2065+
margins: { top: 72, right: 72, bottom: 72, left: 72 },
2066+
};
2067+
2068+
const layout = layoutDocument(blocks, measures, options);
2069+
2070+
// Everything should fit on one page
2071+
expect(layout.pages.length).toBe(1);
2072+
2073+
// p1 at y=72, height=40 → region for 3-col section starts at y=112
2074+
const regionTop = 72 + 40; // 112
2075+
2076+
// Column 0: 2 lines × 250px = 500px → bottom at 112 + 500 = 612
2077+
// Column 1: 1 line × 250px = 250px → bottom at 112 + 250 = 362
2078+
const tallestColumnBottom = regionTop + 500; // 612
2079+
2080+
const page = layout.pages[0];
2081+
const pAfter = page.fragments.find((f) => f.blockId === 'p-after');
2082+
expect(pAfter).toBeDefined();
2083+
2084+
// KEY ASSERTION: p-after must start at or below the tallest column's bottom (612)
2085+
// Without the fix, it would start at 362 (column 1's bottom), overlapping column 0
2086+
expect(pAfter!.y).toBeGreaterThanOrEqual(tallestColumnBottom);
2087+
});
20162088
});
20172089

20182090
describe('columnBreak with multi-column pages', () => {

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

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1451,9 +1451,16 @@ export function layoutDocument(blocks: FlowBlock[], measures: Measure[], options
14511451

14521452
// Start a new mid-page region with different column configuration
14531453
const startMidPageRegion = (state: PageState, newColumns: ColumnLayout): void => {
1454-
// Record the boundary at current Y position
1454+
// Use the maximum Y reached across all columns so the new region starts
1455+
// below ALL column content, not just the current column's cursor position.
1456+
// This prevents overlap when a multi-column section's columns have unequal heights.
1457+
const regionStartY = Math.max(state.cursorY, state.maxCursorY);
1458+
state.cursorY = regionStartY;
1459+
state.maxCursorY = regionStartY;
1460+
1461+
// Record the boundary at the resolved Y position
14551462
const boundary: ConstraintBoundary = {
1456-
y: state.cursorY,
1463+
y: regionStartY,
14571464
columns: newColumns,
14581465
};
14591466
state.constraintBoundaries.push(boundary);
@@ -1465,7 +1472,7 @@ export function layoutDocument(blocks: FlowBlock[], measures: Measure[], options
14651472
layoutLog(`[Layout] *** COLUMNS CHANGED MID-PAGE ***`);
14661473
layoutLog(` OLD activeColumns: ${JSON.stringify(activeColumns)}`);
14671474
layoutLog(` NEW activeColumns: ${JSON.stringify(newColumns)}`);
1468-
layoutLog(` Current page: ${state.page.number}, cursorY: ${state.cursorY}`);
1475+
layoutLog(` Current page: ${state.page.number}, cursorY: ${state.cursorY}, maxCursorY: ${state.maxCursorY}`);
14691476

14701477
// Update activeColumns so subsequent pages use this column configuration
14711478
activeColumns = cloneColumnLayout(newColumns);
@@ -1479,9 +1486,6 @@ export function layoutDocument(blocks: FlowBlock[], measures: Measure[], options
14791486
{ left: activeLeftMargin, right: activeRightMargin },
14801487
activePageSize.w,
14811488
);
1482-
1483-
// Note: We do NOT reset cursorY - content continues from current position
1484-
// This creates the mid-page region effect
14851489
};
14861490

14871491
// Collect anchored drawings mapped to their anchor paragraphs
@@ -2129,6 +2133,7 @@ export function layoutDocument(blocks: FlowBlock[], measures: Measure[], options
21292133
}
21302134
}
21312135
state.cursorY = tableBottomY;
2136+
state.maxCursorY = Math.max(state.maxCursorY, state.cursorY);
21322137
}
21332138
continue;
21342139
}

packages/layout-engine/layout-engine/src/layout-drawing.test.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,7 @@ describe('layoutDrawingBlock', () => {
6262
constraintBoundaries: [];
6363
activeConstraintIndex: number;
6464
trailingSpacing: number;
65+
maxCursorY: number;
6566
};
6667

6768
const createMockPageState = (overrides: Record<string, unknown> = {}): MockPageState =>
@@ -76,6 +77,7 @@ describe('layoutDrawingBlock', () => {
7677
constraintBoundaries: [],
7778
activeConstraintIndex: -1,
7879
trailingSpacing: 0,
80+
maxCursorY: 100,
7981
...overrides,
8082
}) as MockPageState;
8183

packages/layout-engine/layout-engine/src/layout-drawing.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -140,4 +140,5 @@ export function layoutDrawingBlock({
140140

141141
state.page.fragments.push(fragment);
142142
state.cursorY += requiredHeight;
143+
state.maxCursorY = Math.max(state.maxCursorY, state.cursorY);
143144
}

packages/layout-engine/layout-engine/src/layout-image.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,4 +86,5 @@ export function layoutImageBlock({
8686

8787
state.page.fragments.push(fragment);
8888
state.cursorY += requiredHeight;
89+
state.maxCursorY = Math.max(state.maxCursorY, state.cursorY);
8990
}

packages/layout-engine/layout-engine/src/layout-paragraph.test.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@ const makePageState = (): PageState => ({
6060
trailingSpacing: 0,
6161
lastParagraphStyleId: undefined,
6262
lastParagraphContextualSpacing: false,
63+
maxCursorY: 50,
6364
});
6465

6566
/**
@@ -1271,6 +1272,7 @@ describe('layoutParagraphBlock - keepLines', () => {
12711272
currentState = {
12721273
...state,
12731274
cursorY: 50, // Reset to top of new page
1275+
maxCursorY: 50,
12741276
page: { number: state.page.number + 1, fragments: [] },
12751277
trailingSpacing: 0,
12761278
};
@@ -1420,6 +1422,7 @@ describe('layoutParagraphBlock - keepLines', () => {
14201422
const advanceColumn = mock((state: PageState) => ({
14211423
...state,
14221424
cursorY: 50,
1425+
maxCursorY: 50,
14231426
trailingSpacing: 0,
14241427
page: { number: 2, fragments: [] },
14251428
}));

packages/layout-engine/layout-engine/src/layout-paragraph.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -789,6 +789,7 @@ export function layoutParagraphBlock(ctx: ParagraphLayoutContext, anchors?: Para
789789

790790
if (neededSpacingBefore > 0) {
791791
state.cursorY += neededSpacingBefore;
792+
state.maxCursorY = Math.max(state.maxCursorY, state.cursorY);
792793
if (spacingDebugEnabled) {
793794
spacingDebugLog('spacingBefore applied', {
794795
blockId: block.id,
@@ -907,6 +908,7 @@ export function layoutParagraphBlock(ctx: ParagraphLayoutContext, anchors?: Para
907908
state.page.fragments.push(fragment);
908909

909910
state.cursorY += borderExpansion.top + fragmentHeight + borderExpansion.bottom;
911+
state.maxCursorY = Math.max(state.maxCursorY, state.cursorY);
910912
lastState = state;
911913
fromLine = slice.toLine;
912914
}
@@ -929,6 +931,7 @@ export function layoutParagraphBlock(ctx: ParagraphLayoutContext, anchors?: Para
929931
appliedSpacingAfter = 0;
930932
} else {
931933
targetState.cursorY += spacingAfter;
934+
targetState.maxCursorY = Math.max(targetState.maxCursorY, targetState.cursorY);
932935
}
933936
targetState.trailingSpacing = appliedSpacingAfter;
934937
if (spacingDebugEnabled) {

packages/layout-engine/layout-engine/src/layout-table.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1239,6 +1239,7 @@ function layoutMonolithicTable(context: TableLayoutContext): void {
12391239
applyTableFragmentPmRange(fragment, context.block, context.measure);
12401240
state.page.fragments.push(fragment);
12411241
state.cursorY += height;
1242+
state.maxCursorY = Math.max(state.maxCursorY, state.cursorY);
12421243
}
12431244

12441245
/**
@@ -1389,6 +1390,7 @@ export function layoutTableBlock({
13891390
applyTableFragmentPmRange(fragment, block, measure);
13901391
state.page.fragments.push(fragment);
13911392
state.cursorY += height;
1393+
state.maxCursorY = Math.max(state.maxCursorY, state.cursorY);
13921394
return;
13931395
}
13941396

@@ -1554,6 +1556,7 @@ export function layoutTableBlock({
15541556
applyTableFragmentPmRange(fragment, block, measure);
15551557
state.page.fragments.push(fragment);
15561558
state.cursorY += fragmentHeight;
1559+
state.maxCursorY = Math.max(state.maxCursorY, state.cursorY);
15571560
}
15581561

15591562
const rowComplete = !hasRemainingLinesAfterContinuation;
@@ -1668,6 +1671,7 @@ export function layoutTableBlock({
16681671
applyTableFragmentPmRange(fragment, block, measure);
16691672
state.page.fragments.push(fragment);
16701673
state.cursorY += fragmentHeight;
1674+
state.maxCursorY = Math.max(state.maxCursorY, state.cursorY);
16711675
pendingPartialRow = forcedPartialRow;
16721676
samePagePartialContinuation = true;
16731677
isTableContinuation = true;
@@ -1717,6 +1721,7 @@ export function layoutTableBlock({
17171721
applyTableFragmentPmRange(fragment, block, measure);
17181722
state.page.fragments.push(fragment);
17191723
state.cursorY += fragmentHeight;
1724+
state.maxCursorY = Math.max(state.maxCursorY, state.cursorY);
17201725

17211726
// Handle partial row tracking
17221727
if (partialRow && !partialRow.isLastPart) {

packages/layout-engine/layout-engine/src/paginator.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,10 @@ export type PageState = {
2020
lastParagraphContextualSpacing: boolean;
2121
/** Border hash of the last paragraph for between-border group detection. */
2222
lastParagraphBorderHash?: string;
23+
/** Tracks the maximum cursorY reached across all columns on this page.
24+
* Used when starting a mid-page region so the new section begins below
25+
* all column content, not just the current column's cursor. */
26+
maxCursorY: number;
2327
};
2428

2529
export type PaginatorOptions = {
@@ -107,6 +111,7 @@ export function createPaginator(opts: PaginatorOptions) {
107111
trailingSpacing: 0,
108112
lastParagraphStyleId: undefined,
109113
lastParagraphContextualSpacing: false,
114+
maxCursorY: topMargin,
110115
};
111116
states.push(state);
112117
pages.push(state.page);
@@ -123,6 +128,8 @@ export function createPaginator(opts: PaginatorOptions) {
123128
const advanceColumn = (state: PageState): PageState => {
124129
const activeCols = getActiveColumnsForState(state);
125130
if (state.columnIndex < activeCols.count - 1) {
131+
// Snapshot max Y before resetting cursor for the next column
132+
state.maxCursorY = Math.max(state.maxCursorY, state.cursorY);
126133
state.columnIndex += 1;
127134
if (state.activeConstraintIndex >= 0 && state.constraintBoundaries[state.activeConstraintIndex]) {
128135
state.cursorY = state.constraintBoundaries[state.activeConstraintIndex].y;

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

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1881,6 +1881,91 @@ describe('measureBlock', () => {
18811881
}
18821882
}
18831883
});
1884+
1885+
it('uses surrounding text font size for tab line height, not hardcoded 12', async () => {
1886+
// Regression: tab runs previously hardcoded maxFontSize=12, producing
1887+
// wrong line heights when the surrounding text used a larger font.
1888+
const largeFontBlock: FlowBlock = {
1889+
kind: 'paragraph',
1890+
id: 'tab-font-size-large',
1891+
runs: [
1892+
{ text: 'Hello', fontFamily: 'Arial', fontSize: 24 },
1893+
{ kind: 'tab', text: '\t', pmStart: 5, pmEnd: 6 },
1894+
{ text: 'World', fontFamily: 'Arial', fontSize: 24 },
1895+
],
1896+
attrs: {},
1897+
};
1898+
1899+
const smallFontBlock: FlowBlock = {
1900+
kind: 'paragraph',
1901+
id: 'tab-font-size-small',
1902+
runs: [
1903+
{ text: 'Hello', fontFamily: 'Arial', fontSize: 10 },
1904+
{ kind: 'tab', text: '\t', pmStart: 5, pmEnd: 6 },
1905+
{ text: 'World', fontFamily: 'Arial', fontSize: 10 },
1906+
],
1907+
attrs: {},
1908+
};
1909+
1910+
const largeMeasure = expectParagraphMeasure(await measureBlock(largeFontBlock, 1000));
1911+
const smallMeasure = expectParagraphMeasure(await measureBlock(smallFontBlock, 1000));
1912+
1913+
expect(largeMeasure.lines).toHaveLength(1);
1914+
expect(smallMeasure.lines).toHaveLength(1);
1915+
1916+
// The large-font paragraph must have a taller line than the small-font one.
1917+
// With the old hardcoded 12, both could collapse to similar heights.
1918+
expect(largeMeasure.lines[0].lineHeight).toBeGreaterThan(smallMeasure.lines[0].lineHeight);
1919+
});
1920+
1921+
it('uses fallback font size when tab is the first run (no preceding text)', async () => {
1922+
// When a tab starts a paragraph, lastFontSize should fall back to the
1923+
// first text run's font size, not a hardcoded default.
1924+
const block: FlowBlock = {
1925+
kind: 'paragraph',
1926+
id: 'tab-first-run',
1927+
runs: [
1928+
{ kind: 'tab', text: '\t', pmStart: 0, pmEnd: 1 },
1929+
{ text: 'After tab', fontFamily: 'Arial', fontSize: 20 },
1930+
],
1931+
attrs: {},
1932+
};
1933+
1934+
const refBlock: FlowBlock = {
1935+
kind: 'paragraph',
1936+
id: 'no-tab-ref',
1937+
runs: [{ text: 'After tab', fontFamily: 'Arial', fontSize: 20 }],
1938+
attrs: {},
1939+
};
1940+
1941+
const measure = expectParagraphMeasure(await measureBlock(block, 1000));
1942+
const refMeasure = expectParagraphMeasure(await measureBlock(refBlock, 1000));
1943+
1944+
expect(measure.lines).toHaveLength(1);
1945+
// Line height should match or exceed the reference (same font size drives both)
1946+
expect(measure.lines[0].lineHeight).toBeGreaterThanOrEqual(refMeasure.lines[0].lineHeight);
1947+
});
1948+
1949+
it('tab-only line inherits font size from following text run', async () => {
1950+
// A line that contains only a tab should derive its height from the
1951+
// paragraph's font size context, not from a hardcoded 12pt.
1952+
const block: FlowBlock = {
1953+
kind: 'paragraph',
1954+
id: 'tab-only-line',
1955+
runs: [{ kind: 'tab', text: '\t', pmStart: 0, pmEnd: 1 }],
1956+
attrs: {
1957+
// paragraph-level font size hint via a nearby run
1958+
},
1959+
};
1960+
1961+
const measure = expectParagraphMeasure(await measureBlock(block, 1000));
1962+
1963+
expect(measure.lines).toHaveLength(1);
1964+
// With the fallback font size (default 12 when no runs present),
1965+
// the line should still have a reasonable height
1966+
expect(measure.lines[0].lineHeight).toBeGreaterThan(0);
1967+
expect(measure.lines[0].maxFontSize).toBeGreaterThan(0);
1968+
});
18841969
});
18851970

18861971
describe('space-only runs', () => {

0 commit comments

Comments
 (0)