Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
72 changes: 72 additions & 0 deletions packages/layout-engine/layout-engine/src/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2013,6 +2013,78 @@ describe('layoutDocument', () => {
expect(p3Fragment).toBeDefined();
expect(p3Fragment?.width).toBe(210); // Half width = two columns
});

it('starts new region below tallest column when columns have unequal heights', () => {
// Regression test for SD-1869: when a multi-column section has unequal column
// heights, the next region must start below the TALLEST column, not the last
// column's cursor. Without the maxCursorY fix, the new region would start at
// the shorter column's bottom, overlapping the taller one.
//
// Uses a 3-col → 2-col transition because the layout engine forces a new page
// when reducing to fewer columns than the current column index (guard at
// columnIndexBefore >= newColumns.count). With 3→2, content in col1
// (columnIndex=1) stays on the same page (1 < 2).
const toThreeColumns: FlowBlock = {
kind: 'sectionBreak',
id: 'sb-to-3col',
type: 'continuous',
columns: { count: 3, gap: 24 },
margins: {},
};
const toTwoColumns: FlowBlock = {
kind: 'sectionBreak',
id: 'sb-to-2col',
type: 'continuous',
columns: { count: 2, gap: 48 },
margins: {},
};

const blocks: FlowBlock[] = [
{ kind: 'paragraph', id: 'p1', runs: [] }, // single column preamble
toThreeColumns,
{ kind: 'paragraph', id: 'p-cols', runs: [] }, // 3 lines → col0 gets 2, col1 gets 1
toTwoColumns,
{ kind: 'paragraph', id: 'p-after', runs: [] }, // must start below tallest column
];

// p-cols: 3 lines of 250px each (750px total)
// Available column height = 720 (page bottom) - 112 (region top) = 608px
// Column 0 fits lines 0+1 (500px), line 2 overflows to column 1
// Column 0 bottom = 112 + 500 = 612
// Column 1 bottom = 112 + 250 = 362
const measures: Measure[] = [
makeMeasure([40]), // p1
{ kind: 'sectionBreak' },
makeMeasure([250, 250, 250]), // p-cols: 3 lines, 2 in col0 + 1 in col1
{ kind: 'sectionBreak' },
makeMeasure([40]), // p-after
];

const options: LayoutOptions = {
pageSize: { w: 612, h: 792 },
margins: { top: 72, right: 72, bottom: 72, left: 72 },
};

const layout = layoutDocument(blocks, measures, options);

// Everything should fit on one page
expect(layout.pages.length).toBe(1);

// p1 at y=72, height=40 → region for 3-col section starts at y=112
const regionTop = 72 + 40; // 112

// Column 0: 2 lines × 250px = 500px → bottom at 112 + 500 = 612
// Column 1: 1 line × 250px = 250px → bottom at 112 + 250 = 362
const tallestColumnBottom = regionTop + 500; // 612

const page = layout.pages[0];
const pAfter = page.fragments.find((f) => f.blockId === 'p-after');
expect(pAfter).toBeDefined();

// KEY ASSERTION: p-after must start at or below the tallest column's bottom (612)
// Without the fix, it would start at 362 (column 1's bottom), overlapping column 0
expect(pAfter!.y).toBeGreaterThanOrEqual(tallestColumnBottom);
});
});

describe('columnBreak with multi-column pages', () => {
Expand Down
17 changes: 11 additions & 6 deletions packages/layout-engine/layout-engine/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1451,9 +1451,16 @@ export function layoutDocument(blocks: FlowBlock[], measures: Measure[], options

// Start a new mid-page region with different column configuration
const startMidPageRegion = (state: PageState, newColumns: ColumnLayout): void => {
// Record the boundary at current Y position
// Use the maximum Y reached across all columns so the new region starts
// below ALL column content, not just the current column's cursor position.
// This prevents overlap when a multi-column section's columns have unequal heights.
const regionStartY = Math.max(state.cursorY, state.maxCursorY);
state.cursorY = regionStartY;
state.maxCursorY = regionStartY;

// Record the boundary at the resolved Y position
const boundary: ConstraintBoundary = {
y: state.cursorY,
y: regionStartY,
columns: newColumns,
};
state.constraintBoundaries.push(boundary);
Expand All @@ -1465,7 +1472,7 @@ export function layoutDocument(blocks: FlowBlock[], measures: Measure[], options
layoutLog(`[Layout] *** COLUMNS CHANGED MID-PAGE ***`);
layoutLog(` OLD activeColumns: ${JSON.stringify(activeColumns)}`);
layoutLog(` NEW activeColumns: ${JSON.stringify(newColumns)}`);
layoutLog(` Current page: ${state.page.number}, cursorY: ${state.cursorY}`);
layoutLog(` Current page: ${state.page.number}, cursorY: ${state.cursorY}, maxCursorY: ${state.maxCursorY}`);

// Update activeColumns so subsequent pages use this column configuration
activeColumns = cloneColumnLayout(newColumns);
Expand All @@ -1479,9 +1486,6 @@ export function layoutDocument(blocks: FlowBlock[], measures: Measure[], options
{ left: activeLeftMargin, right: activeRightMargin },
activePageSize.w,
);

// Note: We do NOT reset cursorY - content continues from current position
// This creates the mid-page region effect
};

// Collect anchored drawings mapped to their anchor paragraphs
Expand Down Expand Up @@ -2129,6 +2133,7 @@ export function layoutDocument(blocks: FlowBlock[], measures: Measure[], options
}
}
state.cursorY = tableBottomY;
state.maxCursorY = Math.max(state.maxCursorY, state.cursorY);
}
continue;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ describe('layoutDrawingBlock', () => {
constraintBoundaries: [];
activeConstraintIndex: number;
trailingSpacing: number;
maxCursorY: number;
};

const createMockPageState = (overrides: Record<string, unknown> = {}): MockPageState =>
Expand All @@ -76,6 +77,7 @@ describe('layoutDrawingBlock', () => {
constraintBoundaries: [],
activeConstraintIndex: -1,
trailingSpacing: 0,
maxCursorY: 100,
...overrides,
}) as MockPageState;

Expand Down
1 change: 1 addition & 0 deletions packages/layout-engine/layout-engine/src/layout-drawing.ts
Original file line number Diff line number Diff line change
Expand Up @@ -140,4 +140,5 @@ export function layoutDrawingBlock({

state.page.fragments.push(fragment);
state.cursorY += requiredHeight;
state.maxCursorY = Math.max(state.maxCursorY, state.cursorY);
}
1 change: 1 addition & 0 deletions packages/layout-engine/layout-engine/src/layout-image.ts
Original file line number Diff line number Diff line change
Expand Up @@ -86,4 +86,5 @@ export function layoutImageBlock({

state.page.fragments.push(fragment);
state.cursorY += requiredHeight;
state.maxCursorY = Math.max(state.maxCursorY, state.cursorY);
}
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ const makePageState = (): PageState => ({
trailingSpacing: 0,
lastParagraphStyleId: undefined,
lastParagraphContextualSpacing: false,
maxCursorY: 50,
});

/**
Expand Down Expand Up @@ -1271,6 +1272,7 @@ describe('layoutParagraphBlock - keepLines', () => {
currentState = {
...state,
cursorY: 50, // Reset to top of new page
maxCursorY: 50,
page: { number: state.page.number + 1, fragments: [] },
trailingSpacing: 0,
};
Expand Down Expand Up @@ -1420,6 +1422,7 @@ describe('layoutParagraphBlock - keepLines', () => {
const advanceColumn = mock((state: PageState) => ({
...state,
cursorY: 50,
maxCursorY: 50,
trailingSpacing: 0,
page: { number: 2, fragments: [] },
}));
Expand Down
3 changes: 3 additions & 0 deletions packages/layout-engine/layout-engine/src/layout-paragraph.ts
Original file line number Diff line number Diff line change
Expand Up @@ -789,6 +789,7 @@ export function layoutParagraphBlock(ctx: ParagraphLayoutContext, anchors?: Para

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

state.cursorY += borderExpansion.top + fragmentHeight + borderExpansion.bottom;
state.maxCursorY = Math.max(state.maxCursorY, state.cursorY);
lastState = state;
fromLine = slice.toLine;
}
Expand All @@ -929,6 +931,7 @@ export function layoutParagraphBlock(ctx: ParagraphLayoutContext, anchors?: Para
appliedSpacingAfter = 0;
} else {
targetState.cursorY += spacingAfter;
targetState.maxCursorY = Math.max(targetState.maxCursorY, targetState.cursorY);
}
targetState.trailingSpacing = appliedSpacingAfter;
if (spacingDebugEnabled) {
Expand Down
5 changes: 5 additions & 0 deletions packages/layout-engine/layout-engine/src/layout-table.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1239,6 +1239,7 @@ function layoutMonolithicTable(context: TableLayoutContext): void {
applyTableFragmentPmRange(fragment, context.block, context.measure);
state.page.fragments.push(fragment);
state.cursorY += height;
state.maxCursorY = Math.max(state.maxCursorY, state.cursorY);
}

/**
Expand Down Expand Up @@ -1389,6 +1390,7 @@ export function layoutTableBlock({
applyTableFragmentPmRange(fragment, block, measure);
state.page.fragments.push(fragment);
state.cursorY += height;
state.maxCursorY = Math.max(state.maxCursorY, state.cursorY);
return;
}

Expand Down Expand Up @@ -1554,6 +1556,7 @@ export function layoutTableBlock({
applyTableFragmentPmRange(fragment, block, measure);
state.page.fragments.push(fragment);
state.cursorY += fragmentHeight;
state.maxCursorY = Math.max(state.maxCursorY, state.cursorY);
}

const rowComplete = !hasRemainingLinesAfterContinuation;
Expand Down Expand Up @@ -1668,6 +1671,7 @@ export function layoutTableBlock({
applyTableFragmentPmRange(fragment, block, measure);
state.page.fragments.push(fragment);
state.cursorY += fragmentHeight;
state.maxCursorY = Math.max(state.maxCursorY, state.cursorY);
pendingPartialRow = forcedPartialRow;
samePagePartialContinuation = true;
isTableContinuation = true;
Expand Down Expand Up @@ -1717,6 +1721,7 @@ export function layoutTableBlock({
applyTableFragmentPmRange(fragment, block, measure);
state.page.fragments.push(fragment);
state.cursorY += fragmentHeight;
state.maxCursorY = Math.max(state.maxCursorY, state.cursorY);

// Handle partial row tracking
if (partialRow && !partialRow.isLastPart) {
Expand Down
7 changes: 7 additions & 0 deletions packages/layout-engine/layout-engine/src/paginator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,10 @@ export type PageState = {
lastParagraphContextualSpacing: boolean;
/** Border hash of the last paragraph for between-border group detection. */
lastParagraphBorderHash?: string;
/** Tracks the maximum cursorY reached across all columns on this page.
* Used when starting a mid-page region so the new section begins below
* all column content, not just the current column's cursor. */
maxCursorY: number;
};

export type PaginatorOptions = {
Expand Down Expand Up @@ -107,6 +111,7 @@ export function createPaginator(opts: PaginatorOptions) {
trailingSpacing: 0,
lastParagraphStyleId: undefined,
lastParagraphContextualSpacing: false,
maxCursorY: topMargin,
};
states.push(state);
pages.push(state.page);
Expand All @@ -123,6 +128,8 @@ export function createPaginator(opts: PaginatorOptions) {
const advanceColumn = (state: PageState): PageState => {
const activeCols = getActiveColumnsForState(state);
if (state.columnIndex < activeCols.count - 1) {
// Snapshot max Y before resetting cursor for the next column
state.maxCursorY = Math.max(state.maxCursorY, state.cursorY);
state.columnIndex += 1;
if (state.activeConstraintIndex >= 0 && state.constraintBoundaries[state.activeConstraintIndex]) {
state.cursorY = state.constraintBoundaries[state.activeConstraintIndex].y;
Expand Down
85 changes: 85 additions & 0 deletions packages/layout-engine/measuring/dom/src/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1881,6 +1881,91 @@ describe('measureBlock', () => {
}
}
});

it('uses surrounding text font size for tab line height, not hardcoded 12', async () => {
// Regression: tab runs previously hardcoded maxFontSize=12, producing
// wrong line heights when the surrounding text used a larger font.
const largeFontBlock: FlowBlock = {
kind: 'paragraph',
id: 'tab-font-size-large',
runs: [
{ text: 'Hello', fontFamily: 'Arial', fontSize: 24 },
{ kind: 'tab', text: '\t', pmStart: 5, pmEnd: 6 },
{ text: 'World', fontFamily: 'Arial', fontSize: 24 },
],
attrs: {},
};

const smallFontBlock: FlowBlock = {
kind: 'paragraph',
id: 'tab-font-size-small',
runs: [
{ text: 'Hello', fontFamily: 'Arial', fontSize: 10 },
{ kind: 'tab', text: '\t', pmStart: 5, pmEnd: 6 },
{ text: 'World', fontFamily: 'Arial', fontSize: 10 },
],
attrs: {},
};

const largeMeasure = expectParagraphMeasure(await measureBlock(largeFontBlock, 1000));
const smallMeasure = expectParagraphMeasure(await measureBlock(smallFontBlock, 1000));

expect(largeMeasure.lines).toHaveLength(1);
expect(smallMeasure.lines).toHaveLength(1);

// The large-font paragraph must have a taller line than the small-font one.
// With the old hardcoded 12, both could collapse to similar heights.
expect(largeMeasure.lines[0].lineHeight).toBeGreaterThan(smallMeasure.lines[0].lineHeight);
});

it('uses fallback font size when tab is the first run (no preceding text)', async () => {
// When a tab starts a paragraph, lastFontSize should fall back to the
// first text run's font size, not a hardcoded default.
const block: FlowBlock = {
kind: 'paragraph',
id: 'tab-first-run',
runs: [
{ kind: 'tab', text: '\t', pmStart: 0, pmEnd: 1 },
{ text: 'After tab', fontFamily: 'Arial', fontSize: 20 },
],
attrs: {},
};

const refBlock: FlowBlock = {
kind: 'paragraph',
id: 'no-tab-ref',
runs: [{ text: 'After tab', fontFamily: 'Arial', fontSize: 20 }],
attrs: {},
};

const measure = expectParagraphMeasure(await measureBlock(block, 1000));
const refMeasure = expectParagraphMeasure(await measureBlock(refBlock, 1000));

expect(measure.lines).toHaveLength(1);
// Line height should match or exceed the reference (same font size drives both)
expect(measure.lines[0].lineHeight).toBeGreaterThanOrEqual(refMeasure.lines[0].lineHeight);
});

it('tab-only line inherits font size from following text run', async () => {
// A line that contains only a tab should derive its height from the
// paragraph's font size context, not from a hardcoded 12pt.
const block: FlowBlock = {
kind: 'paragraph',
id: 'tab-only-line',
runs: [{ kind: 'tab', text: '\t', pmStart: 0, pmEnd: 1 }],
attrs: {
// paragraph-level font size hint via a nearby run
},
};

const measure = expectParagraphMeasure(await measureBlock(block, 1000));

expect(measure.lines).toHaveLength(1);
// With the fallback font size (default 12 when no runs present),
// the line should still have a reasonable height
expect(measure.lines[0].lineHeight).toBeGreaterThan(0);
expect(measure.lines[0].maxFontSize).toBeGreaterThan(0);
});
});

describe('space-only runs', () => {
Expand Down
5 changes: 3 additions & 2 deletions packages/layout-engine/measuring/dom/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1411,7 +1411,8 @@ async function measureParagraphBlock(block: ParagraphBlock, maxWidth: number): P
toRun: runIndex,
toChar: 1,
width: 0,
maxFontSize: 12, // Default font size for tabs
maxFontSize: lastFontSize,
maxFontInfo: hasSeenTextRun ? undefined : fallbackFontInfo,
maxWidth: getEffectiveWidth(lines.length === 0 ? initialAvailableWidth : bodyContentWidth),
segments: [],
spaceCount: 0,
Expand All @@ -1432,7 +1433,7 @@ async function measureParagraphBlock(block: ParagraphBlock, maxWidth: number): P
// Persist measured tab width on the TabRun for downstream consumers/tests
(run as TabRun & { width?: number }).width = tabAdvance;

currentLine.maxFontSize = Math.max(currentLine.maxFontSize, 12);
currentLine.maxFontSize = Math.max(currentLine.maxFontSize, lastFontSize);
currentLine.toRun = runIndex;
currentLine.toChar = 1; // tab is a single character
let currentLeader: LeaderDecoration | null = null;
Expand Down
Loading
Loading