From 2ce2f9f7ef513b0050b3e74473b7c91fa4064534 Mon Sep 17 00:00:00 2001 From: Tadeu Tupinamba Date: Tue, 21 Apr 2026 09:13:56 -0300 Subject: [PATCH 1/2] fix(layout-bridge): keep footnote fragments within the reserved band (SD-1680) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When multiple footnotes competed for space on a single page, footnote fragments could render past the bottom page margin — sometimes 150+ pixels into the footer area or off the page entirely. Caused by three interacting issues in `computeFootnoteLayoutPlan` + the multi-pass reserve loop: 1. Placement used `maxReserve` (~the whole page) as the height ceiling, so the plan happily placed more footnote content than the body layout had actually reserved at the bottom. 2. The multi-pass stabilization loop could oscillate when reducing the reserve shifted refs between pages, and exit with a `reserves` vector that didn't match what the plan placed — leaving body layout and footnote plan inconsistent. 3. The post-loop "reservesDiffer → relayout" branch could then reintroduce the mismatch by laying out with one reserve vector while the final plan operated on another. Fixes: - `computeFootnoteLayoutPlan`: when `baseReserve > 0`, cap `placeFootnote`'s `availableHeight` to `baseReserve` (the band actually reserved by the body layout), not `maxReserve`. Excess footnotes are pushed to the next page via the existing `pendingByColumn` mechanism. Initial pass (`baseReserve === 0`) still uses `maxReserve` to seed a meaningful reserve for subsequent passes. - Multi-pass loop: merge reserves element-wise with `max(prev, next)` each pass. Reserves are monotonically non-decreasing, so the loop is guaranteed to converge (bounded by `maxReserve` per page) and the old `[247] ↔ [394]` oscillation can't recur. - Post-loop: drop the `reservesDiffer → relayout` branch. With the monotonic loop already converged and placement bounded by `reserves`, `finalPlan`'s slices always fit within the body's reserved band. The extra relayout was the mechanism that reintroduced inconsistency. Visual validation (in-browser) on the 3 fixtures from the Linear issue: - reporter-test.docx: max overflow 156px → 0px - harvey-nvca.docx: overflow present → 0px - torke-carlsbad.docx: overflow present → 0px Tests: - 2 new tests in footnoteBandOverflow.test.ts asserting the invariant "every footnote fragment's bottom-Y ≤ page's physical bottom limit" for both multi-footnote pressure and oversized-footnote cases. - 1168 layout-bridge tests pass, 604 layout-engine tests pass, 1737 pm-adapter tests pass, 11385 super-editor tests pass. Follow-up (Step 3, separate PR): footnote-aware body pagination — consult footnote heights when deciding whether paragraph P fits on page N, rather than reactively shrinking body via reserves. This would eliminate the multi-pass loop entirely. --- .../layout-bridge/src/incrementalLayout.ts | 84 +++---- .../test/footnoteBandOverflow.test.ts | 224 ++++++++++++++++++ 2 files changed, 262 insertions(+), 46 deletions(-) create mode 100644 packages/layout-engine/layout-bridge/test/footnoteBandOverflow.test.ts diff --git a/packages/layout-engine/layout-bridge/src/incrementalLayout.ts b/packages/layout-engine/layout-bridge/src/incrementalLayout.ts index cafffe5c72..823dbaf0ca 100644 --- a/packages/layout-engine/layout-bridge/src/incrementalLayout.ts +++ b/packages/layout-engine/layout-bridge/src/incrementalLayout.ts @@ -1326,6 +1326,13 @@ export async function incrementalLayout( for (let pageIndex = 0; pageIndex < pageCount; pageIndex += 1) { const baseReserve = Number.isFinite(baseReserves?.[pageIndex]) ? Math.max(0, baseReserves[pageIndex]) : 0; const maxReserve = computeMaxFootnoteReserve(layoutForPages, pageIndex, baseReserve); + // SD-1680: cap placement to the band actually reserved by the body layout. + // When baseReserve > 0, the body was laid out assuming a specific footnote band + // of that height — placing more footnote content than that would overflow the + // reserved area and render past the bottom margin. On the initial pass + // (baseReserve === 0), fall back to maxReserve so we can seed a meaningful + // reserve for subsequent passes. + const placementCeiling = baseReserve > 0 ? baseReserve : maxReserve; const columns = pageColumns.get(pageIndex); const columnCount = Math.max(1, Math.floor(columns?.count ?? 1)); @@ -1366,7 +1373,7 @@ export async function incrementalLayout( : 0; const overhead = isFirstSlice ? separatorBefore + separatorHeight + safeTopPadding : 0; const gapBefore = !isFirstSlice ? safeGap : 0; - const availableHeight = Math.max(0, maxReserve - usedHeight - overhead - gapBefore); + const availableHeight = Math.max(0, placementCeiling - usedHeight - overhead - gapBefore); const { slice, remainingRanges } = fitFootnoteContent( id, ranges, @@ -1375,7 +1382,7 @@ export async function incrementalLayout( columnIndex, isContinuation, measuresById, - isFirstSlice && maxReserve > 0, + isFirstSlice && placementCeiling > 0, ); if (slice.ranges.length === 0) { @@ -1755,37 +1762,38 @@ export async function incrementalLayout( // Relayout with footnote reserves and iterate until reserves and page count stabilize, // so each page gets the correct reserve (avoids "too much" on one page and "not enough" on another). + // + // SD-1680: reserves merge monotonically element-wise (max of prior and new) across + // passes. Combined with the placement cap in `computeFootnoteLayoutPlan`, this makes + // the iteration guaranteed to converge. Rationale: placement is bounded by baseReserve, + // so the plan's new reserves are always <= baseReserve per page. Taking element-wise + // max means reserves only grow (or stay), eliminating the old oscillation pattern + // ([247] ↔ [394] as body refs moved between pages) that left body layout and plan + // inconsistent. Over-reserving a few pixels on a page where footnote refs moved off + // is harmless; under-reserving causes footnotes to render past the page edge. if (reserves.some((h) => h > 0)) { let reservesStabilized = false; - const seenReserveKeys = new Set([reserves.join(',')]); for (let pass = 0; pass < MAX_FOOTNOTE_LAYOUT_PASSES; pass += 1) { layout = relayout(reserves); ({ columns: pageColumns, idsByColumn } = resolveFootnoteAssignments(layout)); ({ measuresById } = await measureFootnoteBlocks(collectFootnoteIdsByColumn(idsByColumn))); plan = computeFootnoteLayoutPlan(layout, idsByColumn, measuresById, reserves, pageColumns); const nextReserves = plan.reserves; - const reservesStable = - nextReserves.length === reserves.length && - nextReserves.every((h, i) => (reserves[i] ?? 0) === h) && - reserves.every((h, i) => (nextReserves[i] ?? 0) === h); - if (reservesStable) { - reserves = nextReserves; - reservesStabilized = true; - break; + const mergedLength = Math.max(reserves.length, nextReserves.length); + const merged = new Array(mergedLength); + let changed = false; + for (let i = 0; i < mergedLength; i += 1) { + const prev = reserves[i] ?? 0; + const next = nextReserves[i] ?? 0; + merged[i] = Math.max(prev, next); + if (merged[i] !== prev) changed = true; } - // Detect oscillation: if we've produced a reserve vector we already tried, - // the loop will never converge. Break early to avoid wasted relayout passes. - const nextKey = nextReserves.join(','); - if (seenReserveKeys.has(nextKey)) { + if (!changed) { + reserves = merged; + reservesStabilized = true; break; } - seenReserveKeys.add(nextKey); - // Only update reserves when we will do another layout pass; otherwise layout - // would be built with the previous reserves while reserves would be nextReserves, - // and the plan/injection phase could place footnotes in the wrong band. - if (pass < MAX_FOOTNOTE_LAYOUT_PASSES - 1) { - reserves = nextReserves; - } + reserves = merged; } if (!reservesStabilized) { console.warn( @@ -1793,38 +1801,22 @@ export async function incrementalLayout( ); } - let { columns: finalPageColumns, idsByColumn: finalIdsByColumn } = resolveFootnoteAssignments(layout); - let { blocks: finalBlocks, measuresById: finalMeasuresById } = await measureFootnoteBlocks( + const { columns: finalPageColumns, idsByColumn: finalIdsByColumn } = resolveFootnoteAssignments(layout); + const { blocks: finalBlocks, measuresById: finalMeasuresById } = await measureFootnoteBlocks( collectFootnoteIdsByColumn(finalIdsByColumn), ); - let finalPlan = computeFootnoteLayoutPlan( + const finalPlan = computeFootnoteLayoutPlan( layout, finalIdsByColumn, finalMeasuresById, reserves, finalPageColumns, ); - const finalReserves = finalPlan.reserves; - let reservesAppliedToLayout = reserves; - const reservesDiffer = - finalReserves.length !== reserves.length || - finalReserves.some((h, i) => (reserves[i] ?? 0) !== h) || - reserves.some((h, i) => (finalReserves[i] ?? 0) !== h); - if (reservesDiffer) { - layout = relayout(finalReserves); - reservesAppliedToLayout = finalReserves; - ({ columns: finalPageColumns, idsByColumn: finalIdsByColumn } = resolveFootnoteAssignments(layout)); - ({ blocks: finalBlocks, measuresById: finalMeasuresById } = await measureFootnoteBlocks( - collectFootnoteIdsByColumn(finalIdsByColumn), - )); - finalPlan = computeFootnoteLayoutPlan( - layout, - finalIdsByColumn, - finalMeasuresById, - reservesAppliedToLayout, - finalPageColumns, - ); - } + // SD-1680: with monotonic reserves + bounded placement, finalPlan's slices are + // always bounded by `reserves`. Skipping the post-loop relayout avoids + // introducing a fresh inconsistency (where relayouting with smaller reserves + // would grow body, shift refs, and break the body/footnote agreement again). + const reservesAppliedToLayout = reserves; const blockById = new Map(); finalBlocks.forEach((block) => { blockById.set(block.id, block); diff --git a/packages/layout-engine/layout-bridge/test/footnoteBandOverflow.test.ts b/packages/layout-engine/layout-bridge/test/footnoteBandOverflow.test.ts new file mode 100644 index 0000000000..d70db31293 --- /dev/null +++ b/packages/layout-engine/layout-bridge/test/footnoteBandOverflow.test.ts @@ -0,0 +1,224 @@ +/** + * SD-1680: Footnotes render past the bottom page margin when multiple footnotes + * need more total height than the reserved band allows. Verifies that the plan + * output is internally consistent with the body layout's bottom margin — i.e., + * no footnote fragment's bottom-Y exceeds the top of the physical bottom margin. + */ + +import { describe, it, expect, vi } from 'vitest'; +import type { FlowBlock, Measure, Fragment } from '@superdoc/contracts'; +import { incrementalLayout } from '../src/incrementalLayout'; + +const makeParagraph = (id: string, text: string, pmStart: number): FlowBlock => ({ + kind: 'paragraph', + id, + runs: [{ text, fontFamily: 'Arial', fontSize: 12, pmStart, pmEnd: pmStart + text.length }], +}); + +const makeSingleLineMeasure = (lineHeight: number, textLength: number): Measure => ({ + kind: 'paragraph', + lines: [ + { + fromRun: 0, + fromChar: 0, + toRun: 0, + toChar: textLength, + width: 200, + ascent: lineHeight * 0.8, + descent: lineHeight * 0.2, + lineHeight, + }, + ], + totalHeight: lineHeight, +}); + +const makeMultiLineMeasure = (lineHeight: number, lineCount: number): Measure => { + const lines = Array.from({ length: lineCount }, (_, i) => ({ + fromRun: 0, + fromChar: i, + toRun: 0, + toChar: i + 1, + width: 200, + ascent: lineHeight * 0.8, + descent: lineHeight * 0.2, + lineHeight, + })); + return { kind: 'paragraph', lines, totalHeight: lineCount * lineHeight }; +}; + +const PAGE_PHYSICAL_BOTTOM_MARGIN = 72; +const BODY_LINE_HEIGHT = 20; +const FOOTNOTE_LINE_HEIGHT = 12; + +describe('SD-1680: Footnote band must not overflow the reserved area', () => { + /** + * Scenario: multiple medium-size footnotes compete for a single page's band. + * Page body fills to have all 4 refs on page 1. Each footnote is long enough that + * the total needed reserve exceeds what a single pass' body layout anticipated. + * Before the fix, footnotes stacked past the reserved band and rendered past the + * page's physical bottom margin. After the fix, excess footnotes must be + * pushed to a later page (or split) and the band must stay within its reserve. + */ + it('keeps every footnote fragment within the band on multi-footnote pages', async () => { + // Realistic-ish scenario: ~20 body paragraphs across 2-3 pages, with 2 footnote + // refs per page. Each footnote is medium-sized (4 lines). Ample body slack so + // the layout converges within MAX_FOOTNOTE_LAYOUT_PASSES. + const BODY_PARAS = 20; + const FOOTNOTE_LINES = 4; + const FOOTNOTE_COUNT = 4; + + let pos = 0; + const bodyBlocks: FlowBlock[] = []; + const refs: Array<{ id: string; pos: number }> = []; + const blocksById = new Map(); + + for (let i = 0; i < BODY_PARAS; i += 1) { + const text = `Body paragraph ${i + 1}.`; + const para = makeParagraph(`body-${i}`, text, pos); + bodyBlocks.push(para); + + // Spread refs so 2 land on each page + if (refs.length < FOOTNOTE_COUNT && i % 3 === 1) { + const refId = String(refs.length + 1); + refs.push({ id: refId, pos: pos + 2 }); + const fnBlock = makeParagraph( + `footnote-${refId}-0-paragraph`, + `Footnote ${refId} content spanning multiple lines.`, + 0, + ); + blocksById.set(refId, [fnBlock]); + } + + pos += text.length + 1; + } + + const measureBlock = vi.fn(async (block: FlowBlock) => { + if (block.id.startsWith('footnote-')) { + return makeMultiLineMeasure(FOOTNOTE_LINE_HEIGHT, FOOTNOTE_LINES); + } + return makeSingleLineMeasure(BODY_LINE_HEIGHT, 20); + }); + + // Page holds ~10 body paragraphs (200px). With footnote reserve, body shrinks + // enough that refs can move between pages, but everything must stay bounded. + const contentHeight = 10 * BODY_LINE_HEIGHT; // 200px + const pageHeight = contentHeight + 72 + PAGE_PHYSICAL_BOTTOM_MARGIN; + + const result = await incrementalLayout( + [], + null, + bodyBlocks, + { + pageSize: { w: 612, h: pageHeight }, + margins: { top: 72, right: 72, bottom: PAGE_PHYSICAL_BOTTOM_MARGIN, left: 72 }, + footnotes: { + refs, + blocksById, + topPadding: 4, + dividerHeight: 2, + }, + }, + measureBlock, + ); + + const { layout } = result; + const pageH = pageHeight; + + // INVARIANT: on every page, every footnote fragment's bottom edge must be ≤ the + // top of the physical bottom margin (pageH - PAGE_PHYSICAL_BOTTOM_MARGIN). + // Anything below that point has rendered past the reserved band into the + // footer area — the core SD-1680 bug. + const overflows: string[] = []; + for (const page of layout.pages) { + const pageBottomLimit = (page.size?.h ?? pageH) - PAGE_PHYSICAL_BOTTOM_MARGIN; + const footnoteFragments = page.fragments.filter( + (f: Fragment) => + typeof f.blockId === 'string' && f.blockId.startsWith('footnote-') && !f.blockId.includes('separator'), + ); + for (const f of footnoteFragments) { + // For paragraph fragments, height is computed from fromLine/toLine * lineHeight. + // The footnote measure's totalHeight is FOOTNOTE_LINES * FOOTNOTE_LINE_HEIGHT. + let fragHeight = 0; + if (f.kind === 'para' && typeof f.toLine === 'number' && typeof f.fromLine === 'number') { + fragHeight = (f.toLine - f.fromLine) * FOOTNOTE_LINE_HEIGHT; + } else if (typeof (f as { height?: number }).height === 'number') { + fragHeight = (f as { height: number }).height; + } + const fragBottom = (f.y ?? 0) + fragHeight; + if (fragBottom > pageBottomLimit + 1) { + overflows.push( + `page ${page.number}: fragment ${f.blockId} bottom=${fragBottom.toFixed(1)} exceeds limit ${pageBottomLimit.toFixed(1)} by ${(fragBottom - pageBottomLimit).toFixed(1)}px`, + ); + } + } + } + + expect(overflows).toEqual([]); + }); + + /** + * A single huge footnote (taller than the page's max reserve) must be split + * across pages with a continuation on the next page — never rendered as one + * monolithic block that extends off the page. + */ + it('splits an oversized footnote across pages rather than overflowing', async () => { + const BIG_FOOTNOTE_LINES = 30; // way bigger than a single page's band + const refPos = 5; + + const bodyBlocks: FlowBlock[] = [makeParagraph('body-0', 'Body before footnote ref.', 0)]; + const footnoteBlock = makeParagraph( + 'footnote-1-0-paragraph', + 'A very long footnote that should not fit on a single page band.', + 0, + ); + + const measureBlock = vi.fn(async (block: FlowBlock) => { + if (block.id.startsWith('footnote-')) { + return makeMultiLineMeasure(FOOTNOTE_LINE_HEIGHT, BIG_FOOTNOTE_LINES); + } + return makeSingleLineMeasure(BODY_LINE_HEIGHT, 26); + }); + + const contentHeight = 200; + const pageHeight = contentHeight + 72 + PAGE_PHYSICAL_BOTTOM_MARGIN; + + const result = await incrementalLayout( + [], + null, + bodyBlocks, + { + pageSize: { w: 612, h: pageHeight }, + margins: { top: 72, right: 72, bottom: PAGE_PHYSICAL_BOTTOM_MARGIN, left: 72 }, + footnotes: { + refs: [{ id: '1', pos: refPos }], + blocksById: new Map([['1', [footnoteBlock]]]), + topPadding: 4, + dividerHeight: 2, + }, + }, + measureBlock, + ); + + const { layout } = result; + const pageBottomLimit = pageHeight - PAGE_PHYSICAL_BOTTOM_MARGIN; + + // No single fragment may extend past the page's bottom limit. + for (const page of layout.pages) { + const pageBottomThisPage = (page.size?.h ?? pageHeight) - PAGE_PHYSICAL_BOTTOM_MARGIN; + const footnoteFragments = page.fragments.filter( + (f: Fragment) => + typeof f.blockId === 'string' && f.blockId.startsWith('footnote-') && !f.blockId.includes('separator'), + ); + for (const f of footnoteFragments) { + let fragHeight = 0; + if (f.kind === 'para' && typeof f.toLine === 'number' && typeof f.fromLine === 'number') { + fragHeight = (f.toLine - f.fromLine) * FOOTNOTE_LINE_HEIGHT; + } else if (typeof (f as { height?: number }).height === 'number') { + fragHeight = (f as { height: number }).height; + } + const fragBottom = (f.y ?? 0) + fragHeight; + expect(fragBottom).toBeLessThanOrEqual(pageBottomThisPage + 1); + } + } + }); +}); From 70d4c85b13e741b8452554aa4cf6bed8c6f708e0 Mon Sep 17 00:00:00 2001 From: Tadeu Tupinamba Date: Tue, 21 Apr 2026 10:01:06 -0300 Subject: [PATCH 2/2] fix(layout-bridge): drive footnote reserve from per-page demand (SD-1680) Cap footnote placement at each page's measured demand (capped at maxReserve) rather than the body's prior-pass reserve, and merge element-wise max across oscillating reserve vectors before the post-loop relayout. Ensures every page's reserve is large enough to hold its placed footnotes so fragments cannot render past the bottom margin, and iterates the post-loop relayout until the layout's reserve per page matches the final plan's demand. --- .../layout-bridge/src/incrementalLayout.ts | 142 +++++++++++++----- 1 file changed, 107 insertions(+), 35 deletions(-) diff --git a/packages/layout-engine/layout-bridge/src/incrementalLayout.ts b/packages/layout-engine/layout-bridge/src/incrementalLayout.ts index 823dbaf0ca..79a4423c87 100644 --- a/packages/layout-engine/layout-bridge/src/incrementalLayout.ts +++ b/packages/layout-engine/layout-bridge/src/incrementalLayout.ts @@ -1326,16 +1326,37 @@ export async function incrementalLayout( for (let pageIndex = 0; pageIndex < pageCount; pageIndex += 1) { const baseReserve = Number.isFinite(baseReserves?.[pageIndex]) ? Math.max(0, baseReserves[pageIndex]) : 0; const maxReserve = computeMaxFootnoteReserve(layoutForPages, pageIndex, baseReserve); - // SD-1680: cap placement to the band actually reserved by the body layout. - // When baseReserve > 0, the body was laid out assuming a specific footnote band - // of that height — placing more footnote content than that would overflow the - // reserved area and render past the bottom margin. On the initial pass - // (baseReserve === 0), fall back to maxReserve so we can seed a meaningful - // reserve for subsequent passes. - const placementCeiling = baseReserve > 0 ? baseReserve : maxReserve; const columns = pageColumns.get(pageIndex); const columnCount = Math.max(1, Math.floor(columns?.count ?? 1)); + // SD-1680: cap placement to the footnote demand on this page (capped by maxReserve). + // Demand = sum of measured heights of all footnote refs anchored here, plus the + // separator/padding/gap overhead they would incur when stacked. Capping placement + // at `min(demand, maxReserve)` (rather than `baseReserve`) decouples the plan's + // placement from the body's prior-pass reserve: the plan reports how much band + // the footnotes actually need, the body grows its reserve to match on the next + // pass, and placement never exceeds maxReserve so footnotes cannot render past + // the page's bottom margin. + let demand = 0; + for (let columnIndex = 0; columnIndex < columnCount; columnIndex += 1) { + const ids = idsByColumn.get(pageIndex)?.get(columnIndex) ?? []; + let columnDemand = 0; + ids.forEach((id, idx) => { + const ranges = rangesByFootnoteId.get(id) ?? []; + let rangesHeight = 0; + ranges.forEach((range) => { + const spacingAfter = 'spacingAfter' in range ? (range.spacingAfter ?? 0) : 0; + rangesHeight += range.height + spacingAfter; + }); + columnDemand += rangesHeight + (idx > 0 ? safeGap : 0); + }); + if (columnDemand > 0) { + columnDemand += safeSeparatorSpacingBefore + safeDividerHeight + safeTopPadding; + } + if (columnDemand > demand) demand = columnDemand; + } + const placementCeiling = demand > 0 ? Math.min(Math.ceil(demand), maxReserve) : maxReserve; + const pendingForPage = new Map>(); pendingByColumn.forEach((entries, columnIndex) => { const targetIndex = columnIndex < columnCount ? columnIndex : Math.max(0, columnCount - 1); @@ -1762,38 +1783,57 @@ export async function incrementalLayout( // Relayout with footnote reserves and iterate until reserves and page count stabilize, // so each page gets the correct reserve (avoids "too much" on one page and "not enough" on another). - // - // SD-1680: reserves merge monotonically element-wise (max of prior and new) across - // passes. Combined with the placement cap in `computeFootnoteLayoutPlan`, this makes - // the iteration guaranteed to converge. Rationale: placement is bounded by baseReserve, - // so the plan's new reserves are always <= baseReserve per page. Taking element-wise - // max means reserves only grow (or stay), eliminating the old oscillation pattern - // ([247] ↔ [394] as body refs moved between pages) that left body layout and plan - // inconsistent. Over-reserving a few pixels on a page where footnote refs moved off - // is harmless; under-reserving causes footnotes to render past the page edge. if (reserves.some((h) => h > 0)) { let reservesStabilized = false; + const seenReserveVectors: number[][] = [reserves.slice()]; for (let pass = 0; pass < MAX_FOOTNOTE_LAYOUT_PASSES; pass += 1) { layout = relayout(reserves); ({ columns: pageColumns, idsByColumn } = resolveFootnoteAssignments(layout)); ({ measuresById } = await measureFootnoteBlocks(collectFootnoteIdsByColumn(idsByColumn))); plan = computeFootnoteLayoutPlan(layout, idsByColumn, measuresById, reserves, pageColumns); const nextReserves = plan.reserves; - const mergedLength = Math.max(reserves.length, nextReserves.length); - const merged = new Array(mergedLength); - let changed = false; - for (let i = 0; i < mergedLength; i += 1) { - const prev = reserves[i] ?? 0; - const next = nextReserves[i] ?? 0; - merged[i] = Math.max(prev, next); - if (merged[i] !== prev) changed = true; + const reservesStable = + nextReserves.length === reserves.length && + nextReserves.every((h, i) => (reserves[i] ?? 0) === h) && + reserves.every((h, i) => (nextReserves[i] ?? 0) === h); + if (reservesStable) { + reserves = nextReserves; + reservesStabilized = true; + break; } - if (!changed) { + // SD-1680: when reserves oscillate (typically between a state where all footnotes + // fit and a state where body packs tighter with some footnotes pushed off the + // page), prefer the element-wise max across all seen states. This matches Word's + // bias toward keeping footnotes on their ref's page rather than tight body + // packing, and avoids overflow from the body reserving less than the plan places. + const nextKey = nextReserves.join(','); + const seen = seenReserveVectors.some((v) => v.join(',') === nextKey); + if (seen) { + const allVectors = [...seenReserveVectors, nextReserves]; + const mergedLength = Math.max(...allVectors.map((v) => v.length)); + const merged = new Array(mergedLength).fill(0); + for (const vec of allVectors) { + for (let i = 0; i < mergedLength; i += 1) { + if ((vec[i] ?? 0) > merged[i]) merged[i] = vec[i]; + } + } reserves = merged; - reservesStabilized = true; + // Relayout with merged reserves so post-loop sees a layout consistent with the + // reserves we're about to apply — otherwise pages may collapse to the layout + // built with the smaller oscillating reserve. + layout = relayout(reserves); + ({ columns: pageColumns, idsByColumn } = resolveFootnoteAssignments(layout)); + ({ measuresById } = await measureFootnoteBlocks(collectFootnoteIdsByColumn(idsByColumn))); + plan = computeFootnoteLayoutPlan(layout, idsByColumn, measuresById, reserves, pageColumns); break; } - reserves = merged; + seenReserveVectors.push(nextReserves.slice()); + // Only update reserves when we will do another layout pass; otherwise layout + // would be built with the previous reserves while reserves would be nextReserves, + // and the plan/injection phase could place footnotes in the wrong band. + if (pass < MAX_FOOTNOTE_LAYOUT_PASSES - 1) { + reserves = nextReserves; + } } if (!reservesStabilized) { console.warn( @@ -1801,22 +1841,54 @@ export async function incrementalLayout( ); } - const { columns: finalPageColumns, idsByColumn: finalIdsByColumn } = resolveFootnoteAssignments(layout); - const { blocks: finalBlocks, measuresById: finalMeasuresById } = await measureFootnoteBlocks( + let { columns: finalPageColumns, idsByColumn: finalIdsByColumn } = resolveFootnoteAssignments(layout); + let { blocks: finalBlocks, measuresById: finalMeasuresById } = await measureFootnoteBlocks( collectFootnoteIdsByColumn(finalIdsByColumn), ); - const finalPlan = computeFootnoteLayoutPlan( + let finalPlan = computeFootnoteLayoutPlan( layout, finalIdsByColumn, finalMeasuresById, reserves, finalPageColumns, ); - // SD-1680: with monotonic reserves + bounded placement, finalPlan's slices are - // always bounded by `reserves`. Skipping the post-loop relayout avoids - // introducing a fresh inconsistency (where relayouting with smaller reserves - // would grow body, shift refs, and break the body/footnote agreement again). - const reservesAppliedToLayout = reserves; + let reservesAppliedToLayout = reserves; + // SD-1680: the post-loop can still mismatch the body reserve and plan placement when + // relayouting with finalPlan.reserves shifts footnote refs between pages (the newly + // relaxed page now holds refs the old reserves didn't account for). Iterate a few + // times, each step taking the element-wise max of current reserves and the new plan's + // reserves, so the final layout's reservation on every page is at least as large as + // the demand from the final ref assignment. This guarantees placements stay inside + // the band and cannot render past the page's bottom margin. + const MAX_POST_PASSES = 3; + for (let postPass = 0; postPass < MAX_POST_PASSES; postPass += 1) { + const target = reservesAppliedToLayout.slice(); + const planReserves = finalPlan.reserves; + const len = Math.max(target.length, planReserves.length); + let needsRelayout = false; + for (let i = 0; i < len; i += 1) { + const applied = target[i] ?? 0; + const needed = planReserves[i] ?? 0; + if (needed > applied) { + target[i] = needed; + needsRelayout = true; + } + } + if (!needsRelayout) break; + layout = relayout(target); + reservesAppliedToLayout = target; + ({ columns: finalPageColumns, idsByColumn: finalIdsByColumn } = resolveFootnoteAssignments(layout)); + ({ blocks: finalBlocks, measuresById: finalMeasuresById } = await measureFootnoteBlocks( + collectFootnoteIdsByColumn(finalIdsByColumn), + )); + finalPlan = computeFootnoteLayoutPlan( + layout, + finalIdsByColumn, + finalMeasuresById, + reservesAppliedToLayout, + finalPageColumns, + ); + } const blockById = new Map(); finalBlocks.forEach((block) => { blockById.set(block.id, block);