Repository navigation
fix(pagination): lay the NVCA charter out on LibreOffice's 48 pages (#1151) - #1181
Merged
Merged
Conversation
…hs (#1151) The converter dropped the spacing after of every paragraph but the last in a run of paragraphs from one numbered list, an OpenXmlPowerTools heuristic Word does not share. Word removes spacing between paragraphs only through w:contextualSpacing, and only between paragraphs of the same style. Group by contextual spacing alone, and honour w:contextualSpacing w:val="0".
…ht (#1151) Chromium rounds a line-height: normal line to a whole CSS pixel. The export has resolved each such line height to the font's natural height since #850; the paginated view and editor did not, so 11pt Times New Roman advanced 12.75pt a line instead of 12.65pt and 10pt notes 12pt instead of 11.50pt. The paginator now runs the same pass on its staging tree before it measures anything.
The paginator kept a citing paragraph on its page and deferred a note that could not start there, so deferred notes accumulated into pages holding only notes: the NVCA model charter started 24 notes after their citation page and laid out 50 pages against LibreOffice's 48. When a note cannot start beside its citation, the citing line now moves to the next page: the paragraph is split above that line, or moved whole when it cannot be split. The paginated editor, which splits no paragraph, keeps deferring. To make the split possible, paragraphs that cite notes, paragraphs written as h1-h6 and paragraphs that open with a list number may now be split, and so may notes that open with a tab after their number; those leading boxes stay whole with the first piece. A paragraph is split for being too tall only when its own lines do not fit; notes that do not fit are split instead. The charter now takes 48 pages in the paginated view. Existing expectations moved: the paginated editor ratchet is 52 (was 51, from the spacing restored two commits earlier), a citing paragraph is no longer listed among those kept intact, and the pagination bundle budget is 23 KiB (20.5 KiB measured).
Two decisions looked at the size of a page's note area where the space left in it is what matters, once a carried continuation fills the area: - A citing line moved to the next page only when that page's note area would be larger than this one's. A full area as large as any page's kept the line and deferred its note. The test now compares the space still free here. - A block that cites no note was refused for the notes already on the page when they filled the capped area, so the page kept its notes and lost its body text. The cap now judges only the notes a block adds. This happened before #1151 too. Stacked on the paginated footnote change of #1130, the first case deferred four notes of the NVCA model charter; there are none now.
…locks put (#1151) Review findings on the note-placement change: - A note-aware paragraph split could end in the middle of the citing line. The citation pre-check measured the raised number's box, which is shorter than the line it sits on, so heads holding the citation failed one by one and the largest passing head ended just before the citation. A note-aware split now breaks only where a visual line starts: the fit is monotone, so the largest fitting head is cut back to the end of its last full line. - Only a paragraph that keeps its lines together is moved whole. A table that cites a note keeps its place and the note is deferred, as before, and so does a paragraph that a keep-with-next block before it keeps with. - A note number written as a plain footnote-number or endnote-number run, without the reference-run marker, counts as a leading box of its paragraph.
…ocks' line heights (#1151) applyUnroundedNormalLineHeights interleaved computed-style reads with font probes, and each probe made the next read recompute the tree's styles; it also scanned every character of every element. Styles are now read first, Latin-only text is detected with one test, and fallback characters are probed one at a time and cached (a line reaches as far above and below its baseline as the furthest of its fonts). The NVCA charter's pass drops from 75-155ms to about 57ms with identical results. The paginated editor re-renders an edited block outside the paginator, so that block went back to Chromium's whole-pixel pitch. wireBlock now runs the same pass on blocks it wires in paginated mode.
…inated view (#1151) With unrounded line heights, Chromium's whole-pixel baseline rounding fell differently and moved some lines of the paginated view by a pixel, which the LibreOffice browser-page benchmark flagged (the footnote case's separator and note line, landscape-section, running-content). The export has corrected glyph placement to Word's baseline since #942 with alignBaselinesToWord. The paginator now runs the same pass at the end of pagination when alignBaselinesToWord is set: paginateHtml and the React viewer set it, direct engine callers and the paginated editor do not (typed text would part from moved boxes), and the export keeps running it after its clipping check. Glyphs move by relative positioning only; no line box or page break changes.
With the 6pt spacing between paginated notes from #1130 in place, the charter takes 49 pages in the paginated view (main: 51; LibreOffice: 48) and 54 in the paginated editor (main: 53). The editor ratchet and the page counts quoted in the CHANGELOG and corner cases now give these measured values.
Built with buildRecord from the summary of visual-parity run 38988464367, which measured this branch's tip on the pinned reference environment from a clean tree. Seven cases improve; wrapped-image-square, inline-image and tracked-deletion drop slightly and are accepted.
…ew lays it out (#1151) The TOC geometry spec takes the base of an auto line-spacing multiple from a line-height: normal probe at the entry's own size, which Chromium rounds to a whole pixel. Since #1151 the paginated view lays lines out at the font's unrounded natural height, as the export already did. Where Calibri falls back to an Arial-metric font (CI), rounding loses most of a pixel: 16px against 16.87px at 11pt, and Word's single-spaced Arial pitch is the unrounded height (1.15em; corner cases #850/#908). The probe is now laid out at 1000px and scaled to the entry's size, so it measures the font's own line box.
#1151) The line-height pass's speed-up measured each beyond-Latin character beside the Latin probe on its own and combined their extents. Chromium picks fallback fonts per run of text, so a character probed alone can fall back to a different, taller font: a CJK line exported 19.25pt where its own line box is 15.75pt (export-line-pitch.spec.ts). The characters are probed together again, as before. The other speed-ups stay (styles read before any probe, one test for Latin-only text): about 53ms on the NVCA charter, with every resolved value identical to the original pass.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #1151
Why
TestFiles/NVCA-Model-COI.docx(the NVCA model charter) paginates to 51 pages in the paginated view onmain(which now includes #1152's 6pt spacing between notes). LibreOffice 25.8.7.3 lays it out on 48. The issue had located only part of the gap. A page-by-page comparison against LibreOffice found five contributors that interact with each other. With all of them fixed, the paginated view lays the charter out on 49 pages, against LibreOffice's 48 (it was exactly 48 before #1152 landed). No footnote now starts on a later page than its citation; onmain23 do. No page holds only footnotes; onmainpages 6 and 7 do. The remaining page has not been located: the layouts first diverge around pages 18–21, where Docxodus carries about five lines of a note continuation onto the next page that LibreOffice finishes. This is recorded as open indocs/ooxml_corner_cases.md.What was wrong and how it works now
line-height: normalto whole pixels. That gave 12.75pt against LibreOffice's 12.65pt per body line, and 12.0 against 11.5pt per note line. The export already corrected this withapplyUnroundedNormalLineHeights. The paginator now runs the same pass on its staging tree, and the paginated editor runs it on every block it re-renders after an edit, so the view, the editor and the export share one line pitch. The pass now reads all styles before it probes fonts and caches per-character fallback probes. On the charter it takes about 57ms instead of 67–155ms, with identical results.h1–h6, most of the charter's clauses), paragraphs that cite notes, paragraphs with list markers, and notes whose first paragraph opens with a tab after the number could not be split. Now they can. Their leading box (list marker, note number or empty tab box) stays whole on the first piece.maintoo. Room is now judged by the space still free.w:contextualSpacing, which these paragraphs don't set, and LibreOffice keeps the gap (24.7pt between FIRST and SECOND; this PR gives 24.64pt;maingave 12.5pt). The converter's grouping (ApplyContextualSpacing) now groups only byw:contextualSpacingand style, and treatsw:val="0"as off. On its own this fix adds two pages, pages LibreOffice lays out too.Validation
NumberedParagraphSpacingTests.pagination-note-citation-placement.spec.ts(13 tests), pluspagination-line-pitch.spec.tsandeditor-paginated-line-pitch.spec.ts.npm run typecheckis clean. The .NET suite passes.main.editor-paginated-charter.spec.tsPAGES53 → 54, as measured on the combined code. The editor never splits paragraphs; the extra page is not attributed.pagination-paragraph-fragments.spec.ts: a paragraph that cites a footnote, which may now split on purpose.pagination.bundle.jssize budget 20 → 23 KiB. It is now 20.5 KiB, up from 19.45 KiB.npm/tests/visual-parity/ratchet.json) was rebuilt with the repo's ownbuildRecordfrom the summary of CI run https://github.com/JSv4/Docxodus/actions/runs/38988464367. That run measured this branch's tip on the pinned reference environment from a clean tree.mainwas up to 36px off.paginate()takes about 1.2–1.4s against about 0.59s onmainin the paginated view, and about 0.6–0.7s against 0.5s in editor mode (noisy machine). The extra work is the additional splitting, about 10 more measurements per page, and it grows linearly with document length.Overlap with open PR #1152
#1152 changes paginated footnotes too. The two apply together with conflicts only in
CHANGELOG.md,docs/ooxml_corner_cases.mdand the charter spec'sPAGESline. #1152 adds 6pt after each note, so whichever merges second should re-measure the charter's page count rather than pick either number. This PR already handles #1152's note number sitting inside the first paragraph.Adversarial review
One round of hostile review ran before opening. It confirmed that no content is lost or duplicated, that no hangs or blank pages appear, that the memoization is sound, and that the export path stays idempotent. It found:
A related keep-with-next stranding with a pending note continuation predates this change and is pinned by an existing spec. It is filed as #1180, with #1178 (the 60% note-area cap against LibreOffice's ~94%) and #1179 (a whole note placed after a continued one).
Final CI round. After the rebase, CI's Playwright job showed two
toc-line-geometrytests expecting the base line box Chromium rounds to a whole pixel. With CI's fonts, that rounding is 0.9px. The paginated view now lays lines at the font's unrounded height, which is Word's measured single spacing, so the spec's probe now measures the unrounded box (test-only change; TOC geometry unchanged). Reproducing CI's fonts locally also turned up a real regression from the line-pass speed-up: it probed non-Latin characters one at a time, and Chromium chose a taller fallback font, so exported CJK lines came out too tall. It probes them together again; the CJK export spec fails without the fix. All 17 checks pass, and the visual-parity ratchet holds for all 21 cases: https://github.com/JSv4/Docxodus/actions/runs/38994865996