From 35f3c505417b708147de1e82acba608dbfa73125 Mon Sep 17 00:00:00 2001 From: Steffen Becker Date: Wed, 12 Aug 2026 20:57:49 +0000 Subject: [PATCH] fix: a column break lands on the line it was measured to Three measurement bugs in columnsFlow, each worth a page on its own: rects became lines by their top, so a raised run counted as a line of its own (192 for 111); the split probed `left + 1`, which is a line's *end* right-to-left, so it kept a word too many and the flow then shaved one line per pass; and one median advance under-measured a paragraph whose lines differ in height (2834px real against 2453 estimated), by enough to budget 74 lines into room for 64. The chain used to flip between one fragment and two forever; it now settles after one split. --- docs/architecture/pagination.md | 2 +- src/lib/editor/extensions/columnsFlow.ts | 68 ++++++++++++++++++------ tests/unit/columns-lines.test.ts | 64 ++++++++++++++++++++++ 3 files changed, 117 insertions(+), 17 deletions(-) create mode 100644 tests/unit/columns-lines.test.ts diff --git a/docs/architecture/pagination.md b/docs/architecture/pagination.md index 05a9de8..3755570 100644 --- a/docs/architecture/pagination.md +++ b/docs/architecture/pagination.md @@ -88,4 +88,4 @@ A break *between rows* instead closes the table on both sides of the gap: collap ## Multi-column sections (`columns.ts` + `columnsFlow.ts`) -- **`columns.ts`** (`Columns`) + **`columnsFlow.ts`** (`ColumnsFlow`) — multi-column (newspaper) layout with **cross-page flow** and **Word's sequential fill order**: column 1 fills to the page bottom, then column 2, then the next page — via per-fragment node decorations carrying `height:…;column-fill:auto` (full page slot for fragments with a continuation; content height + `OPEN_HEIGHT_SLACK_PX` for the open last fragment of a document-final chain, so typing grows column 1 first without one-frame flashes into column 2). A chain followed by real content keeps its last fragment undecorated → CSS `column-fill: balance` (Word balances the end of a continuous section). Because the box height is the plugin's own decoration, overflow/join decisions use content-based estimates (`contentHeightPx` = block rects + collapsed margins), not the rendered height. A section is a block region (`content: '(paragraph|heading|bulletList|orderedList)+'`) rendered as CSS multi-column (`column-count`/`column-gap` inline via `renderHTML`; no NodeView, no visual affordance — it must read as plain flowing text, so NOT isolating/selectable: selection, Backspace, and joins cross its boundary). Attrs: `count` (2–3) and `gapCm` (default `DEFAULT_COLUMN_GAP_CM` 0.5, clamp 0–5). Own group like textBox (Document is `'(block | textBox | columns)+'`). **Chain model:** the document may hold a section as several *adjacent equal-attr fragments* — the user-visible section is the maximal run (`columnsChain`); commands, export, and import all treat a chain as one section. **Cross-page flow (`columnsFlow.ts`):** a rAF layout pass (mirroring pageBreaks' geometry via the exported `readVerticalMargins`) keeps the fragmentation in sync with the page grid — an overflowing fragment is **split** at a block boundary so the maximal prefix fills its page (balanced-height estimate = Σ block `getClientRects` heights / count), and a fragment with room **joins** the adjacent continuation back (the split pass then re-splits at the right point). One op per pass, `addToHistory: false` (layout-only; undo of user edits just re-flows), composition-guarded, pass-budgeted (`MAX_FLOW_PASSES`, reset per external change). **pageBreaks.ts cooperation:** a `.columns-node` leaf carries `columnsFragment` (block count + first-block fit estimate); on bottom overflow pageBreaks *waits for the flow* when the fragment is splittable and its first block fits the remaining space, pushes it whole when mid-page (even taller-than-page), and at a page top does nothing (`columns-line-split-pending` — pushing a page-tall fragment just moves the problem down forever). Both sides use the shared `COLUMNS_FIT_MARGIN_PX` for the first-block fit test — differing thresholds make split/push/join cycle at the boundary; the flow's join `HYSTERESIS_PX` must exceed the estimate error for the same reason. **Page boundaries cut mid-paragraph at line boundaries** (`splitParagraphInBlock`): the flow measures a paragraph's rendered lines (document-order rects; a top jump = new line), converts the page's leftover content budget into a line count, finds the boundary text position via `posAtCoords`, and does a depth-2 `tr.split` whose second part carries the layout-internal **`joinPrev`** paragraph attr (a global attr minted by the Columns extension; never rendered/parsed, `keepOnSplit: false`). Used in two places: a paragraph taller than the whole page slot (`force` — at least one line stays), and the **boundary block** after `k` whole blocks fit (leftover room ≥ `MIN_TAIL_LINES` — so the last column of a page is filled instead of pushing the next paragraph out whole); the join side mirrors it by pulling a multi-line paragraph head at ~two line rows of room (partial pull; the re-split hands the excess back). A live per-fragment analysis is exposed via `getColumnsFlowDebug` and included in the dev Debug dump (`columnsFlow` key). pageBreaks skips the empty trailing paragraph after a document-final chain (`trailing-empty-after-columns`) so a sequential-fill box reaching the page bottom doesn't mint a phantom page. Joins reverse it: a fragment join uses depth 2 when the continuation head is a joinPrev paragraph (needing only ~one line row of space), and a stray joinPrev paragraph *inside* a fragment merges into its predecessor immediately. `mergeJoinedBlocks` (columns.ts) restores whole paragraphs on unwrap/absorb; `mergeJoinedParagraphsJson` (export/odt.ts, shared with docx.ts) does the same before serializing, so files always hold the original single paragraph. Commands: `setColumns(n)` — inside a section 1 unwraps the whole chain / 2–3 update it; outside with a selection, 2–3 wrap the covered top-level blocks (absorbing existing sections; fail on tables/boxes/TOCs); with a **bare cursor, the whole document** gets columns (Word behavior; wrappable runs around tables become one section each) — and `setColumnGap(g)` (chain-wide); helper `findColumns`. UI: a Columns dropdown in `ToolbarExpanded.svelte` (1/2/3 buttons + gap input, tick-derived active state). `trailingNode.ts` appends a paragraph after a trailing section. **ODF**: exported via the textBox-style sentinel hoist (`replaceColumns`/`applyColumns`, U+E009) as `` + a minted section style with `` — adjacent equal-attr fragments **coalesce into one section**; import resolves via `StyleResolver.sectionColumns` (gap also derived from per-`style:column` indents), wraps runs of allowed blocks (`pushColumnRuns`), moves tables/boxes out with a warning (columns nodes from inner sections pass through silently), clamps >3 columns (the flow re-fragments after load); Word-style **page-layout columns** (`` in `style:page-layout-properties`, no `text:section`) are read via `StyleResolver.pageColumns` and wrap the whole body the same way. **DOCX**: full support — export splits the body into multiple `docx`-lib sections (`bodyGroups`; continuous breaks, `w:cols` with gap in twips, page geometry + fresh header/footer refs per sectPr, fragments coalesced); import groups body children on mid-body `pPr>sectPr` paragraphs (`splitBodySections`, empty markers dropped) and wraps groups whose sectPr has `w:cols w:num>1` — covering whole-document multi-column files via the body-final sectPr. A section whose own `w:type` is a page-starting break (nextPage/oddPage/evenPage, or absent = the default; `sectionStartsNewPage`, not `continuous`/`nextColumn`) tags its first block with `breakBefore:'page'` so the section starts on a new page like Word. +- **`columns.ts`** (`Columns`) + **`columnsFlow.ts`** (`ColumnsFlow`) — multi-column (newspaper) layout with **cross-page flow** and **Word's sequential fill order**: column 1 fills to the page bottom, then column 2, then the next page — via per-fragment node decorations carrying `height:…;column-fill:auto` (full page slot for fragments with a continuation; content height + `OPEN_HEIGHT_SLACK_PX` for the open last fragment of a document-final chain, so typing grows column 1 first without one-frame flashes into column 2). A chain followed by real content keeps its last fragment undecorated → CSS `column-fill: balance` (Word balances the end of a continuous section). Because the box height is the plugin's own decoration, overflow/join decisions use content-based estimates (`contentHeightPx` = block rects + collapsed margins), not the rendered height. A section is a block region (`content: '(paragraph|heading|bulletList|orderedList)+'`) rendered as CSS multi-column (`column-count`/`column-gap` inline via `renderHTML`; no NodeView, no visual affordance — it must read as plain flowing text, so NOT isolating/selectable: selection, Backspace, and joins cross its boundary). Attrs: `count` (2–3) and `gapCm` (default `DEFAULT_COLUMN_GAP_CM` 0.5, clamp 0–5). Own group like textBox (Document is `'(block | textBox | columns)+'`). **Chain model:** the document may hold a section as several *adjacent equal-attr fragments* — the user-visible section is the maximal run (`columnsChain`); commands, export, and import all treat a chain as one section. **Cross-page flow (`columnsFlow.ts`):** a rAF layout pass (mirroring pageBreaks' geometry via the exported `readVerticalMargins`) keeps the fragmentation in sync with the page grid — an overflowing fragment is **split** at a block boundary so the maximal prefix fills its page (balanced-height estimate = Σ block `getClientRects` heights / count), and a fragment with room **joins** the adjacent continuation back (the split pass then re-splits at the right point). One op per pass, `addToHistory: false` (layout-only; undo of user edits just re-flows), composition-guarded, pass-budgeted (`MAX_FLOW_PASSES`, reset per external change). **pageBreaks.ts cooperation:** a `.columns-node` leaf carries `columnsFragment` (block count + first-block fit estimate); on bottom overflow pageBreaks *waits for the flow* when the fragment is splittable and its first block fits the remaining space, pushes it whole when mid-page (even taller-than-page), and at a page top does nothing (`columns-line-split-pending` — pushing a page-tall fragment just moves the problem down forever). Both sides use the shared `COLUMNS_FIT_MARGIN_PX` for the first-block fit test — differing thresholds make split/push/join cycle at the boundary; the flow's join `HYSTERESIS_PX` must exceed the estimate error for the same reason. **Page boundaries cut mid-paragraph at line boundaries** (`splitParagraphInBlock`): the flow measures a paragraph's rendered lines (`groupLines` — document-order rects banded by *overlap*: a rect joins the line it overlaps by half the taller of the two, so a raised run like a verse number is no line of its own and no outsized rect swallows the line below it), converts the page's leftover content budget into a line count by summing the **real** top-to-top advances (`linesWithin` — a line carrying a raised run is taller than the rest, so one median advance for all of them runs ~15% short; the median stands in only where the tops reset at a column break), finds the boundary text position by probing **both** edges of the line's first rect and keeping the earlier position (`lineStartPos` — on a right-to-left line the logical start is the right edge, and `left + 1` alone keeps a word too many, which costs one split pass per line), and does a depth-2 `tr.split` whose second part carries the layout-internal **`joinPrev`** paragraph attr (a global attr minted by the Columns extension; never rendered/parsed, `keepOnSplit: false`). Used in two places: a paragraph taller than the whole page slot (`force` — at least one line stays), and the **boundary block** after `k` whole blocks fit (leftover room ≥ `MIN_TAIL_LINES` — so the last column of a page is filled instead of pushing the next paragraph out whole); the join side mirrors it by pulling a multi-line paragraph head at ~two line rows of room (partial pull; the re-split hands the excess back). A live per-fragment analysis is exposed via `getColumnsFlowDebug` and included in the dev Debug dump (`columnsFlow` key). pageBreaks skips the empty trailing paragraph after a document-final chain (`trailing-empty-after-columns`) so a sequential-fill box reaching the page bottom doesn't mint a phantom page. Joins reverse it: a fragment join uses depth 2 when the continuation head is a joinPrev paragraph (needing only ~one line row of space), and a stray joinPrev paragraph *inside* a fragment merges into its predecessor immediately. `mergeJoinedBlocks` (columns.ts) restores whole paragraphs on unwrap/absorb; `mergeJoinedParagraphsJson` (export/odt.ts, shared with docx.ts) does the same before serializing, so files always hold the original single paragraph. Commands: `setColumns(n)` — inside a section 1 unwraps the whole chain / 2–3 update it; outside with a selection, 2–3 wrap the covered top-level blocks (absorbing existing sections; fail on tables/boxes/TOCs); with a **bare cursor, the whole document** gets columns (Word behavior; wrappable runs around tables become one section each) — and `setColumnGap(g)` (chain-wide); helper `findColumns`. UI: a Columns dropdown in `ToolbarExpanded.svelte` (1/2/3 buttons + gap input, tick-derived active state). `trailingNode.ts` appends a paragraph after a trailing section. **ODF**: exported via the textBox-style sentinel hoist (`replaceColumns`/`applyColumns`, U+E009) as `` + a minted section style with `` — adjacent equal-attr fragments **coalesce into one section**; import resolves via `StyleResolver.sectionColumns` (gap also derived from per-`style:column` indents), wraps runs of allowed blocks (`pushColumnRuns`), moves tables/boxes out with a warning (columns nodes from inner sections pass through silently), clamps >3 columns (the flow re-fragments after load); Word-style **page-layout columns** (`` in `style:page-layout-properties`, no `text:section`) are read via `StyleResolver.pageColumns` and wrap the whole body the same way. **DOCX**: full support — export splits the body into multiple `docx`-lib sections (`bodyGroups`; continuous breaks, `w:cols` with gap in twips, page geometry + fresh header/footer refs per sectPr, fragments coalesced); import groups body children on mid-body `pPr>sectPr` paragraphs (`splitBodySections`, empty markers dropped) and wraps groups whose sectPr has `w:cols w:num>1` — covering whole-document multi-column files via the body-final sectPr. A section whose own `w:type` is a page-starting break (nextPage/oddPage/evenPage, or absent = the default; `sectionStartsNewPage`, not `continuous`/`nextColumn`) tags its first block with `breakBefore:'page'` so the section starts on a new page like Word. diff --git a/src/lib/editor/extensions/columnsFlow.ts b/src/lib/editor/extensions/columnsFlow.ts index 1802836..75cb27e 100644 --- a/src/lib/editor/extensions/columnsFlow.ts +++ b/src/lib/editor/extensions/columnsFlow.ts @@ -34,6 +34,7 @@ const MIN_TAIL_LINES = 2; const MAX_FLOW_PASSES = 48; type Fragment = { pos: number; node: PMNode }; +export type LineRect = { left: number; right: number; top: number; height: number }; // Live per-fragment view of the flow's decisions, for the dev Debug dump. export type ColumnsFlowDebug = { @@ -86,27 +87,53 @@ function contentHeightPx(children: Element[], scale: number): number { return sum; } -// One rect per rendered text line of `el`, in document order (multicol renders -// column 1's lines before column 2's, so a new line = a top jump in either -// direction). Viewport coordinates. -function lineRects(el: Element): { left: number; top: number; height: number }[] { +// The first rect of each rendered line, in document order — column 1's lines +// before column 2's. A rect joins the line it overlaps by half the taller of the +// two, so a raised run (a verse number, a superscript) is no line of its own. +export function groupLines(rects: LineRect[]): LineRect[] { + const lines: LineRect[] = []; + let band: { top: number; bottom: number } | null = null; + for (const r of rects) { + const bottom = r.top + r.height; + if (band) { + const overlap = Math.min(band.bottom, bottom) - Math.max(band.top, r.top); + if (overlap >= Math.max(band.bottom - band.top, r.height) / 2) continue; + } + band = { top: r.top, bottom }; + lines.push(r); + } + return lines; +} + +// The rendered lines of `el`, in viewport coordinates. +function lineRects(el: Element): LineRect[] { const walker = document.createTreeWalker(el, NodeFilter.SHOW_TEXT); - const lines: { left: number; top: number; height: number }[] = []; - let lastTop = -Infinity; + const rects: LineRect[] = []; let textNode: Node | null; while ((textNode = walker.nextNode())) { if (!textNode.textContent) continue; const range = document.createRange(); range.selectNodeContents(textNode); for (const r of Array.from(range.getClientRects())) { - if (r.width <= 0 || r.height <= 0) continue; - if (Math.abs(r.top - lastTop) > 2) { - lines.push({ left: r.left, top: r.top, height: r.height }); - lastTop = r.top; + if (r.width > 0 && r.height > 0) { + rects.push({ left: r.left, right: r.right, top: r.top, height: r.height }); } } } - return lines; + return groupLines(rects); +} + +// How many of `lines` a flow budget buys, measured top to top: a line carrying a +// raised run is taller than the rest, so one median advance for all of them is +// well short. `advance` stands in where the tops reset at a column break. +export function linesWithin(lines: LineRect[], budgetPx: number, advance: number, scale: number): number { + let used = 0; + for (let n = 1; n < lines.length; n++) { + const gap = (lines[n].top - lines[n - 1].top) / scale; + used += gap > 2 && gap < advance * 3 ? gap : advance; + if (used > budgetPx) return n - 1; + } + return lines.length; } // Median top-to-top line advance (viewport px); 0 when there are too few lines. @@ -197,6 +224,17 @@ export const ColumnsFlow = Extension.create({ return open; } + // First document position on a line. Its first rect in document order runs + // from whichever edge is logically first — the right one on a line of Hebrew + // or Arabic — so both edges are probed and the earlier position wins. + function lineStartPos(line: LineRect): number | null { + const top = line.top + line.height / 2; + const a = editorView.posAtCoords({ left: line.left + 1, top }); + const b = editorView.posAtCoords({ left: line.right - 1, top }); + const pos = Math.min(a?.pos ?? Infinity, b?.pos ?? Infinity); + return Number.isFinite(pos) ? pos : null; + } + // Split the paragraph at `blockIndex` at the line consuming `budgetPx` (a // mid-paragraph page break); the second part is marked joinPrev so joins/export // restore the original. `force` takes at least one line for an over-tall paragraph. @@ -211,16 +249,14 @@ export const ColumnsFlow = Extension.create({ const lines = lineRects(blockEl); if (lines.length < 2) return false; const advance = (lineAdvance(lines) || DEFAULT_LINE_PX * scale) / scale; - let budgetLines = Math.floor(budgetPx / advance); + let budgetLines = linesWithin(lines, budgetPx, advance, scale); if (budgetLines < MIN_TAIL_LINES) { if (!force) return false; budgetLines = 1; } const prefixLines = Math.min(Math.max(budgetLines, 1), lines.length - 1); - const target = lines[prefixLines]; - const coords = editorView.posAtCoords({ left: target.left + 1, top: target.top + target.height / 2 }); - if (!coords) return false; - const pos = coords.pos; + const pos = lineStartPos(lines[prefixLines]); + if (pos === null) return false; let blockStart = frag.pos + 1; for (let j = 0; j < blockIndex; j++) blockStart += frag.node.child(j).nodeSize; if (pos <= blockStart + 1 || pos >= blockStart + block.nodeSize - 1) return false; diff --git a/tests/unit/columns-lines.test.ts b/tests/unit/columns-lines.test.ts new file mode 100644 index 0000000..a654781 --- /dev/null +++ b/tests/unit/columns-lines.test.ts @@ -0,0 +1,64 @@ +import { describe, expect, it } from 'vitest'; +import { groupLines, linesWithin, type LineRect } from '../../src/lib/editor/extensions/columnsFlow'; + +const HEIGHT = 24; +const ADVANCE = 22.1; +const RAISE = 8.5; + +// A two-column Hebrew page as measured: 24px runs on a 22.1px advance, a verse +// number 8.5px above the line it opens (which makes that line 8.5px taller), and +// the tops resetting at the column break. +function pageRects(lines: number, raisedEvery: number, breakAt = Infinity): LineRect[] { + const out: LineRect[] = []; + let top = 180; + for (let i = 0; i < lines; i++) { + if (i === breakAt) top = 180; + const raised = i % raisedEvery === 0; + if (raised) out.push({ left: 600, right: 620, top: top - RAISE, height: HEIGHT }); + for (let r = 0; r < 6; r++) out.push({ left: 340 + r * 40, right: 375 + r * 40, top, height: HEIGHT }); + top += ADVANCE + (raised ? RAISE : 0); + } + return out; +} + +describe('groupLines', () => { + it('counts a raised run with its line, not as one of its own', () => { + expect(groupLines(pageRects(40, 3)).length).toBe(40); + }); + + it('starts a line where the tops reset at a column break', () => { + expect(groupLines(pageRects(40, 3, 20)).length).toBe(40); + }); + + it('lets no outsized rect swallow the line below it', () => { + const tall: LineRect[] = [ + { left: 0, right: 10, top: 100, height: 53 }, + { left: 0, right: 10, top: 122, height: 24 }, + { left: 0, right: 10, top: 144, height: 24 }, + ]; + expect(groupLines(tall).length).toBe(3); + }); +}); + +describe('linesWithin', () => { + const lines = groupLines(pageRects(40, 3)); + + it('measures the budget against the real advances', () => { + const tenLines = lines[10].top - lines[0].top + 1; + expect(linesWithin(lines, tenLines, ADVANCE, 1)).toBe(10); + // What a single median advance for every line would have claimed instead. + expect(Math.floor(tenLines / ADVANCE)).toBeGreaterThan(10); + }); + + it('stands the median advance in at a column break', () => { + const broken = groupLines(pageRects(40, 3, 20)); + const budget = ADVANCE * 25 + RAISE * 9; + expect(linesWithin(broken, budget, ADVANCE, 1)).toBeGreaterThan(20); + }); + + it('scales viewport tops back to document px', () => { + const tenLines = lines[10].top - lines[0].top + 1; + const zoomed = lines.map((l) => ({ ...l, top: l.top * 1.5 })); + expect(linesWithin(zoomed, tenLines, ADVANCE, 1.5)).toBe(10); + }); +});