perf: incremental layout pipeline for 30x faster keystroke response#246
Closed
sicko7947 wants to merge 3 commits intoeigenpal:mainfrom
Closed
perf: incremental layout pipeline for 30x faster keystroke response#246sicko7947 wants to merge 3 commits intoeigenpal:mainfrom
sicko7947 wants to merge 3 commits intoeigenpal:mainfrom
Conversation
The editor re-processed the entire document on every keystroke, causing 200-500ms input lag on 20+ page documents. This adds an incremental pipeline that only re-converts, re-measures, and re-paginates from the edited paragraph forward, with early exit when page breaks stabilize. ## Changes **Incremental block conversion (Phase 1)** - New `IncrementalBlockCache` tracks previous doc state and uses ProseMirror's structural sharing (node identity comparison) to detect which top-level nodes changed — O(1) per node - Extracted `convertTopLevelNode()` from `toFlowBlocks()` for per-node incremental conversion - List counter snapshots at each node boundary for correct numbering after partial re-conversion - Forward propagation of list counter changes until stabilization **Incremental measurement** - `measureBlocksIncremental()` reuses cached measures for clean blocks before the dirty range, re-measures only from dirtyFrom forward - Full floating zone pre-scan still runs (fast, zones can shift) **Layout engine resume + early exit (Phase 2)** - Paginator `snapshot()`/`createPaginatorFromSnapshot()` API for capturing and restoring layout state at page boundaries - `layoutDocument()` accepts `resumeFrom` option to skip blocks before the dirty range and start from a saved paginator snapshot - Early exit: after 2 consecutive blocks past the dirty range converge with the previous layout state, remaining pages are spliced from the previous run — avoids re-paginating the entire tail - `applyContextualSpacingRange()` for partial spacing application **CSS containment (Phase 3)** - `content-visibility: auto` + `contain-intrinsic-size` on page shells so the browser skips layout/paint for off-screen pages **Pipeline integration** - PagedEditor detects doc changes via PM node identity comparison (works for both transaction-driven and direct relayout calls) - Deferred cache mutation: `updateBlocks()` returns results without mutating the cache; `applyIncrementalResult()` is called only after successful paint to prevent split-state on stale aborts - Per-step performance diagnostics via `console.debug` ## Measured Results (287 blocks, 24 pages) | Step | Full pipeline | Incremental | Speedup | |------|-------------|-------------|---------| | Block conversion | 1.3ms | 0.0ms | ∞ | | Measurement | 13.7ms | 0.5ms | **27x** | | Layout | 0.5ms | 0.3ms (resumed) | 1.7x | | Paint | 12.7ms | 12.7ms | 1x | | **Total** | **~28ms** | **~13ms** | **2x** | Steps 1-3 combined: **15ms → 0.8ms (19x faster)** On larger documents (500+ blocks), the savings scale linearly since unchanged blocks are completely skipped. ## Test Coverage - 36 new unit tests (incrementalBlockCache, paginator-snapshot, layout-resume) — all passing - 368/368 total unit tests passing - Demo-docx E2E suite passing - Typecheck clean across all 4 packages Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
@sicko7947 is attempting to deploy a commit to the EigenPal Team on Vercel. A member of the Team first needs to authorize it. |
Internal planning document, not needed in the upstream repo. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR introduces an incremental layout pipeline for the paged editor so that edits only re-convert, re-measure, and re-layout the affected portion of the document, with optional layout resume from paginator snapshots to reduce per-keystroke latency on large documents.
Changes:
- Added an incremental block cache with dirty-range detection and partial top-level node conversion.
- Added paginator snapshot/restore + layout resume/early-exit plumbing (types + engine + tests).
- Added paint optimizations via CSS
content-visibilityon page shells and wired incremental pipeline intoPagedEditor.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| PERFORMANCE_ROADMAP.md | New performance roadmap documenting the lag root cause and planned/implemented optimization tiers. |
| packages/react/src/paged-editor/PagedEditor.tsx | Integrates incremental conversion/measurement/layout resume and adds step timing logs. |
| packages/core/src/layout-painter/renderPage.ts | Adds page shell CSS containment (content-visibility, contain-intrinsic-size). |
| packages/core/src/layout-engine/types.ts | Adds snapshot/resume/convergence-related types and layout metadata fields. |
| packages/core/src/layout-engine/paginator.ts | Implements paginator snapshotting and restore via createPaginatorFromSnapshot. |
| packages/core/src/layout-engine/paginator-snapshot.test.ts | Adds unit tests validating snapshot deep-clone and restore correctness. |
| packages/core/src/layout-engine/layout-resume.test.ts | Adds unit tests covering resume behavior, convergence, and early-exit. |
| packages/core/src/layout-engine/index.ts | Adds resumeFrom support, per-block paginator state capture, snapshot capture, and early-exit logic. |
| packages/core/src/layout-bridge/toFlowBlocks.ts | Extracts convertTopLevelNode() to support incremental top-level conversion. |
| packages/core/src/layout-bridge/incrementalBlockCache.ts | New incremental cache implementation (dirty detection, block splicing, list counter propagation). |
| packages/core/src/layout-bridge/incrementalBlockCache.test.ts | Adds unit tests for dirty detection, incremental updates, and position reindexing helpers. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+296
to
+307
| // Adjust nodeToBlockRange for nodes after dirty range | ||
| const blockDelta = newBlocks.length - (effectiveDirtyTo - dirtyFrom); | ||
| if (blockDelta !== 0) { | ||
| for (const [nodeIdx, [start, end]] of newNodeToBlockRange) { | ||
| if (start >= effectiveDirtyTo) { | ||
| newNodeToBlockRange.set(nodeIdx, [start + blockDelta, end + blockDelta]); | ||
| } | ||
| } | ||
| // Reindex pmStart/pmEnd on blocks after the splice point | ||
| if (result.length > dirtyFrom + newBlocks.length) { | ||
| reindexPositions(result, dirtyFrom + newBlocks.length, blockDelta === 0 ? 0 : 0); | ||
| } |
There was a problem hiding this comment.
updateBlocks() splices in newly converted blocks but then calls reindexPositions(..., delta=0). This leaves pmStart/pmEnd (and nested run/table/textBox positions) incorrect for all reused blocks after the dirty range whenever the edit inserts/deletes content before them, breaking click-to-position, selection mapping, and hit-testing. Compute the ProseMirror position delta between old and new docs at the splice boundary (or use transaction.mapping when available) and shift all subsequent blocks by that delta; also ensure nodeToBlockRange updates stay consistent with any forward-extension logic.
Comment on lines
+372
to
+396
| for (let nodeIdx = 0; nodeIdx < doc.childCount; nodeIdx++) { | ||
| const node = doc.child(nodeIdx); | ||
| const nodeStart = getNodeStartPos(doc, nodeIdx); | ||
| const nodeEnd = nodeStart + node.nodeSize; | ||
| const rangeStart = blockIdx; | ||
|
|
||
| // Walk blocks that belong to this node | ||
| while (blockIdx < blocks.length) { | ||
| const block = blocks[blockIdx]; | ||
|
|
||
| // SectionBreak has no pmStart — it's always emitted right after its paragraph | ||
| if (block.kind === 'sectionBreak') { | ||
| cache.sectionBreakIndices.push(blockIdx); | ||
| blockIdx++; | ||
| continue; | ||
| } | ||
|
|
||
| const bStart = 'pmStart' in block ? (block as { pmStart: number }).pmStart : -1; | ||
| if (bStart < nodeStart) break; | ||
| if (bStart >= nodeEnd) break; | ||
| blockIdx++; | ||
| } | ||
|
|
||
| cache.nodeToBlockRange.set(nodeIdx, [rangeStart, blockIdx]); | ||
| } |
There was a problem hiding this comment.
rebuildIndices() computes nodeStart via getNodeStartPos() inside a loop over nodeIdx, making this O(n^2) in top-level node count and running on every saveBlockState() after paint. For large documents this can eat into the per-keystroke win. Track the running pos as you iterate doc children (like toFlowBlocks does) to compute nodeStart/nodeEnd in O(n).
Comment on lines
+1007
to
+1022
| if (numId == null || numId === 0) { | ||
| result.push(block); | ||
| break; | ||
| } | ||
| const level = pmAttrs.numPr.ilvl ?? 0; | ||
| const counters = listCounters.get(numId) ?? new Array(9).fill(0); | ||
|
|
||
| counters[level] = (counters[level] ?? 0) + 1; | ||
| for (let i = level + 1; i < counters.length; i += 1) { | ||
| counters[i] = 0; | ||
| } | ||
|
|
||
| listCounters.set(numId, counters); | ||
|
|
||
| const marker = pmAttrs.listIsBullet ? '•' : formatNumberedMarker(counters, level); | ||
| block.attrs = { ...block.attrs, listMarker: marker }; |
There was a problem hiding this comment.
convertTopLevelNode(): when numPr exists with numId null/0, the break exits the switch early. That prevents emitting a SectionBreakBlock for paragraphs that also carry section properties, which can desync section handling and incremental dirty-range propagation. Instead of breaking out of the switch, skip list-marker assignment and continue with the normal paragraph + optional sectionBreak emission.
Suggested change
| if (numId == null || numId === 0) { | |
| result.push(block); | |
| break; | |
| } | |
| const level = pmAttrs.numPr.ilvl ?? 0; | |
| const counters = listCounters.get(numId) ?? new Array(9).fill(0); | |
| counters[level] = (counters[level] ?? 0) + 1; | |
| for (let i = level + 1; i < counters.length; i += 1) { | |
| counters[i] = 0; | |
| } | |
| listCounters.set(numId, counters); | |
| const marker = pmAttrs.listIsBullet ? '•' : formatNumberedMarker(counters, level); | |
| block.attrs = { ...block.attrs, listMarker: marker }; | |
| if (numId != null && numId !== 0) { | |
| const level = pmAttrs.numPr.ilvl ?? 0; | |
| const counters = listCounters.get(numId) ?? new Array(9).fill(0); | |
| counters[level] = (counters[level] ?? 0) + 1; | |
| for (let i = level + 1; i < counters.length; i += 1) { | |
| counters[i] = 0; | |
| } | |
| listCounters.set(numId, counters); | |
| const marker = pmAttrs.listIsBullet ? '•' : formatNumberedMarker(counters, level); | |
| block.attrs = { ...block.attrs, listMarker: marker }; | |
| } |
Comment on lines
+418
to
+420
| const earlyExitBlock = | ||
| convergentCount >= CONVERGENCE_THRESHOLD ? statesAtBlock.length - 1 : undefined; | ||
|
|
There was a problem hiding this comment.
layoutDocument(): earlyExitBlock is computed as statesAtBlock.length - 1 when convergence triggers, but that will typically report the final block index (especially after you splice remaining states from prevStates) rather than the actual block where early-exit happened. Track the early-exit index (e.g., earlyExitAt) and return that value so callers/debugging can accurately identify where the layout stopped.
Comment on lines
+2095
to
+2104
| newLayout = layoutDocument(newBlocks, newMeasures, { | ||
| ...layoutOpts, | ||
| resumeFrom: { | ||
| resumeFromBlock: snapshotBlock, | ||
| paginatorSnapshot: snapshot, | ||
| dirtyTo: Math.min(incrementalDirtyFrom + 10, newBlocks.length), | ||
| prevStatesAtBlock: | ||
| cache.statesAtBlock.length > 0 ? cache.statesAtBlock : undefined, | ||
| prevPages: layout?.pages, | ||
| }, |
There was a problem hiding this comment.
runLayoutPipeline(): resumeFrom.dirtyTo is currently set to Math.min(incrementalDirtyFrom + 10, newBlocks.length) instead of the dirty range end from computeDirtyRange/updateBlocks. Starting convergence checks before the true dirty range is fully processed can early-exit and splice old pages while still in the changed region, producing incorrect layout. Pass the actual dirtyTo block index (and account for any list-counter extension) to resumeFrom.dirtyTo.
Comment on lines
1873
to
2230
| // Signal layout is starting | ||
| syncCoordinator.onLayoutStart(); | ||
|
|
||
| try { | ||
| // Step 1: Convert PM doc to flow blocks | ||
| // Try incremental update first, fall back to full conversion | ||
| let stepStart = performance.now(); | ||
| const pageContentHeight = pageSize.h - margins.top - margins.bottom; | ||
| const newBlocks = toFlowBlocks(state.doc, { theme: _theme, pageContentHeight }); | ||
| const toFlowOpts = { theme: _theme, pageContentHeight }; | ||
| const cache = incrementalCacheRef.current; | ||
| let newBlocks: FlowBlock[]; | ||
| let incrementalDirtyFrom = -1; // -1 = full pipeline, >=0 = incremental from this block | ||
| let pendingIncrementalResult: IncrementalUpdateResult | null = null; | ||
|
|
||
| // Try incremental path when we have a previous doc to compare against. | ||
| // Uses PM node identity comparison (not transaction steps) so it works | ||
| // for both transaction-driven and direct runLayoutPipeline calls. | ||
| if (cache.prevDoc && cache.prevDoc !== state.doc) { | ||
| const dirtyRange = computeDirtyRange(cache, state.doc, transaction); | ||
| if (dirtyRange) { | ||
| // updateBlocks returns a result WITHOUT mutating the cache. | ||
| // We apply it only after the pipeline commits (saveBlockState). | ||
| pendingIncrementalResult = updateBlocks( | ||
| cache, | ||
| state.doc, | ||
| dirtyRange.dirtyFrom, | ||
| dirtyRange.dirtyTo, | ||
| toFlowOpts | ||
| ); | ||
| newBlocks = pendingIncrementalResult.blocks; | ||
| incrementalDirtyFrom = dirtyRange.dirtyFrom; | ||
| } else { | ||
| // Dirty range too large or section break hit — full conversion | ||
| newBlocks = toFlowBlocks(state.doc, toFlowOpts); | ||
| } | ||
| } else { | ||
| newBlocks = toFlowBlocks(state.doc, toFlowOpts); | ||
| } | ||
|
|
||
| const usedIncremental = incrementalDirtyFrom >= 0; | ||
| let stepTime = performance.now() - stepStart; | ||
| // Always log step timing for performance diagnostics | ||
| console.debug( | ||
| `[PagedEditor] Step 1 (${usedIncremental ? `incremental from ${incrementalDirtyFrom}` : 'full'}) → ${stepTime.toFixed(1)}ms (${newBlocks.length} blocks)` | ||
| ); | ||
| if (stepTime > 500) { | ||
| console.warn( | ||
| `[PagedEditor] toFlowBlocks took ${Math.round(stepTime)}ms (${newBlocks.length} blocks)` | ||
| `[PagedEditor] ${usedIncremental ? 'incremental' : 'toFlowBlocks'} took ${Math.round(stepTime)}ms (${newBlocks.length} blocks)` | ||
| ); | ||
| } | ||
| setBlocks(newBlocks); | ||
|
|
||
| // Step 2: Measure all blocks. | ||
| // Must use full measureBlocks() because measurements depend on | ||
| // inter-block context (floating zones, cumulative Y). Individual | ||
| // block measurements cannot be cached by PM node identity since | ||
| // floating tables/images create exclusion zones that affect | ||
| // neighboring paragraphs' line widths. | ||
| // Step 2: Measure blocks. | ||
| // Incremental path reuses cached measures for clean blocks before dirtyFrom. | ||
| // Full path measures all blocks from scratch. Both paths do full floating | ||
| // zone extraction (it's fast and zones could shift). | ||
| stepStart = performance.now(); | ||
| // Compute per-block widths accounting for section breaks with different column configs | ||
| const blockWidths = computePerBlockWidths(newBlocks, contentWidth, columns); | ||
| const newMeasures = measureBlocks(newBlocks, blockWidths); | ||
| let newMeasures: Measure[]; | ||
| if (usedIncremental && cache.measures.length > 0) { | ||
| newMeasures = measureBlocksIncremental( | ||
| newBlocks, | ||
| blockWidths, | ||
| cache.measures, | ||
| incrementalDirtyFrom | ||
| ); | ||
| } else { | ||
| newMeasures = measureBlocks(newBlocks, blockWidths); | ||
| } | ||
| stepTime = performance.now() - stepStart; | ||
| console.debug( | ||
| `[PagedEditor] Step 2 (${usedIncremental ? `measureIncremental from ${incrementalDirtyFrom}` : 'full'}) → ${stepTime.toFixed(1)}ms (${newBlocks.length} blocks)` | ||
| ); | ||
| if (stepTime > 1000) { | ||
| console.warn( | ||
| `[PagedEditor] measureBlocks took ${Math.round(stepTime)}ms (${newBlocks.length} blocks)` | ||
| `[PagedEditor] ${usedIncremental ? 'measureBlocksIncremental' : 'measureBlocks'} took ${Math.round(stepTime)}ms (${newBlocks.length} blocks)` | ||
| ); | ||
| } | ||
| setMeasures(newMeasures); | ||
|
|
||
| // Step 2.5: Collect footnote references from blocks | ||
| const footnoteRefs = collectFootnoteRefs(newBlocks); | ||
| const hasFootnotes = footnoteRefs.length > 0 && document?.package?.footnotes; | ||
|
|
||
| // Step 2.75: Prepare header/footer content for rendering (needed before layout | ||
| // to compute effective margins when header content exceeds available space) | ||
| const hfMetricsHeader = { section: 'header' as const, pageSize, margins }; | ||
| const hfMetricsFooter = { section: 'footer' as const, pageSize, margins }; | ||
| const headerContentForRender = convertHeaderFooterToContent( | ||
| headerContent, | ||
| contentWidth, | ||
| hfMetricsHeader | ||
| ); | ||
| const footerContentForRender = convertHeaderFooterToContent( | ||
| footerContent, | ||
| contentWidth, | ||
| hfMetricsFooter | ||
| ); | ||
| const hasTitlePg = sectionProperties?.titlePg === true; | ||
| const firstPageHeaderForRender = hasTitlePg | ||
| ? convertHeaderFooterToContent(firstPageHeaderContent, contentWidth, hfMetricsHeader) | ||
| : undefined; | ||
| const firstPageFooterForRender = hasTitlePg | ||
| ? convertHeaderFooterToContent(firstPageFooterContent, contentWidth, hfMetricsFooter) | ||
| : undefined; | ||
|
|
||
| // Adjust margins if header/footer content exceeds available space | ||
| // (Word and Google Docs push body content down when header grows) | ||
| // Use the tallest header/footer across all variants for margin computation | ||
| const headerDistance = margins.header ?? 48; | ||
| const footerDistance = margins.footer ?? 48; | ||
| const availableHeaderSpace = margins.top - headerDistance; | ||
| const availableFooterSpace = margins.bottom - footerDistance; | ||
| const hfHeight = (hf: HeaderFooterContent | undefined) => | ||
| hf ? (hf.visualBottom ?? hf.height) : 0; | ||
| const hfFooterHeight = (hf: HeaderFooterContent | undefined) => | ||
| hf ? Math.max((hf.visualBottom ?? hf.height) - (hf.visualTop ?? 0), hf.height) : 0; | ||
| const headerContentHeight = Math.max( | ||
| hfHeight(headerContentForRender), | ||
| hfHeight(firstPageHeaderForRender) | ||
| ); | ||
| const footerContentHeight = Math.max( | ||
| hfFooterHeight(footerContentForRender), | ||
| hfFooterHeight(firstPageFooterForRender) | ||
| ); | ||
|
|
||
| let effectiveMargins = margins; | ||
| if ( | ||
| headerContentHeight > availableHeaderSpace || | ||
| footerContentHeight > availableFooterSpace | ||
| ) { | ||
| effectiveMargins = { ...margins }; | ||
| if (headerContentHeight > availableHeaderSpace) { | ||
| effectiveMargins.top = Math.max(margins.top, headerDistance + headerContentHeight); | ||
| } | ||
| if (footerContentHeight > availableFooterSpace) { | ||
| effectiveMargins.bottom = Math.max( | ||
| margins.bottom, | ||
| footerDistance + footerContentHeight | ||
| ); | ||
| } | ||
| } | ||
|
|
||
| // Step 3: Layout blocks onto pages (two-pass if footnotes exist) | ||
| stepStart = performance.now(); | ||
| let newLayout: Layout; | ||
| let pageFootnoteMap = new Map<number, number[]>(); | ||
| let footnoteContentMap = new Map<number, { displayNumber: number; height: number }>(); | ||
|
|
||
| // Common layout options for all passes | ||
| const bodyBreakType = sectionProperties?.sectionStart as | ||
| | 'continuous' | ||
| | 'nextPage' | ||
| | 'evenPage' | ||
| | 'oddPage' | ||
| | undefined; | ||
| const layoutOpts = { | ||
| pageSize, | ||
| margins: effectiveMargins, | ||
| columns, | ||
| bodyBreakType, | ||
| pageGap, | ||
| }; | ||
|
|
||
| if (hasFootnotes) { | ||
| // Pass 1: Layout without footnote space to determine page assignments | ||
| const pass1Layout = layoutDocument(newBlocks, newMeasures, layoutOpts); | ||
|
|
||
| // Map footnote refs to pages | ||
| pageFootnoteMap = mapFootnotesToPages(pass1Layout.pages, footnoteRefs); | ||
|
|
||
| // Build footnote content and measure heights | ||
| footnoteContentMap = buildFootnoteContentMap( | ||
| document!.package.footnotes!, | ||
| footnoteRefs, | ||
| contentWidth | ||
| ); | ||
|
|
||
| // Calculate per-page reserved heights | ||
| const footnoteReservedHeights = calculateFootnoteReservedHeights( | ||
| pageFootnoteMap, | ||
| footnoteContentMap | ||
| ); | ||
|
|
||
| // Pass 2: Layout with reserved heights | ||
| if (footnoteReservedHeights.size > 0) { | ||
| newLayout = layoutDocument(newBlocks, newMeasures, { | ||
| ...layoutOpts, | ||
| footnoteReservedHeights, | ||
| }); | ||
|
|
||
| // Re-map footnotes to pages (assignments may have shifted) | ||
| pageFootnoteMap = mapFootnotesToPages(newLayout.pages, footnoteRefs); | ||
|
|
||
| // Store footnoteIds on each page for rendering | ||
| for (const [pageNum, fnIds] of pageFootnoteMap) { | ||
| const page = newLayout.pages.find((p) => p.number === pageNum); | ||
| if (page) { | ||
| page.footnoteIds = fnIds; | ||
| } | ||
| } | ||
| } else { | ||
| newLayout = pass1Layout; | ||
| } | ||
| } else { | ||
| // No footnotes — single pass | ||
| newLayout = layoutDocument(newBlocks, newMeasures, layoutOpts); | ||
| // No footnotes — single pass. | ||
| // Use resumed layout when incremental path succeeded and we have a snapshot. | ||
| if (usedIncremental && cache.paginatorSnapshotAtBlock.size > 0) { | ||
| // Find the closest snapshot at or before dirtyFrom | ||
| let snapshotBlock = -1; | ||
| for (const blockIdx of cache.paginatorSnapshotAtBlock.keys()) { | ||
| if (blockIdx <= incrementalDirtyFrom && blockIdx > snapshotBlock) { | ||
| snapshotBlock = blockIdx; | ||
| } | ||
| } | ||
| const snapshot = | ||
| snapshotBlock >= 0 ? cache.paginatorSnapshotAtBlock.get(snapshotBlock) : undefined; | ||
|
|
||
| if (snapshot && snapshotBlock > 0) { | ||
| console.debug( | ||
| `[PagedEditor] Step 3 using RESUME from block ${snapshotBlock} (dirty: ${incrementalDirtyFrom})` | ||
| ); | ||
| newLayout = layoutDocument(newBlocks, newMeasures, { | ||
| ...layoutOpts, | ||
| resumeFrom: { | ||
| resumeFromBlock: snapshotBlock, | ||
| paginatorSnapshot: snapshot, | ||
| dirtyTo: Math.min(incrementalDirtyFrom + 10, newBlocks.length), | ||
| prevStatesAtBlock: | ||
| cache.statesAtBlock.length > 0 ? cache.statesAtBlock : undefined, | ||
| prevPages: layout?.pages, | ||
| }, | ||
| }); | ||
| } else { | ||
| newLayout = layoutDocument(newBlocks, newMeasures, layoutOpts); | ||
| } | ||
| } else { | ||
| newLayout = layoutDocument(newBlocks, newMeasures, layoutOpts); | ||
| } | ||
| } | ||
|
|
||
| stepTime = performance.now() - stepStart; | ||
| console.debug( | ||
| `[PagedEditor] Step 3 (layout) → ${stepTime.toFixed(1)}ms (${newLayout.pages.length} pages)` | ||
| ); | ||
| if (stepTime > 500) { | ||
| console.warn( | ||
| `[PagedEditor] layoutDocument took ${Math.round(stepTime)}ms (${newLayout.pages.length} pages)` | ||
| ); | ||
| } | ||
| setLayout(newLayout); | ||
|
|
||
| // No yield before paint — layout→paint must be atomic to avoid visual flash | ||
|
|
||
| // Step 4: Paint to DOM | ||
| if (pagesContainerRef.current && painterRef.current) { | ||
| stepStart = performance.now(); | ||
|
|
||
| // Build block lookup | ||
| const blockLookup: BlockLookup = new Map(); | ||
| for (let i = 0; i < newBlocks.length; i++) { | ||
| const block = newBlocks[i]; | ||
| const measure = newMeasures[i]; | ||
| if (block && measure) { | ||
| blockLookup.set(String(block.id), { block, measure }); | ||
| } | ||
| } | ||
| painterRef.current.setBlockLookup(blockLookup); | ||
|
|
||
| // Build per-page footnote render items | ||
| const footnotesByPage = hasFootnotes | ||
| ? buildFootnoteRenderItems(pageFootnoteMap, footnoteContentMap, document) | ||
| : undefined; | ||
|
|
||
| // Render pages to container | ||
| renderPages(newLayout.pages, pagesContainerRef.current, { | ||
| pageGap, | ||
| showShadow: true, | ||
| pageBackground: '#fff', | ||
| blockLookup, | ||
| headerContent: headerContentForRender, | ||
| footerContent: footerContentForRender, | ||
| firstPageHeaderContent: firstPageHeaderForRender, | ||
| firstPageFooterContent: firstPageFooterForRender, | ||
| titlePg: hasTitlePg, | ||
| headerDistance: sectionProperties?.headerDistance | ||
| ? twipsToPixels(sectionProperties.headerDistance) | ||
| : undefined, | ||
| footerDistance: sectionProperties?.footerDistance | ||
| ? twipsToPixels(sectionProperties.footerDistance) | ||
| : undefined, | ||
| pageBorders: sectionProperties?.pageBorders, | ||
| theme: _theme, | ||
| footnotesByPage: footnotesByPage?.size ? footnotesByPage : undefined, | ||
| resolvedCommentIds, | ||
| } as RenderPageOptions & { | ||
| pageGap?: number; | ||
| blockLookup?: BlockLookup; | ||
| footnotesByPage?: Map<number, FootnoteRenderItem[]>; | ||
| }); | ||
|
|
||
| stepTime = performance.now() - stepStart; | ||
| console.debug(`[PagedEditor] Step 4 (paint) → ${stepTime.toFixed(1)}ms`); | ||
| if (stepTime > 500) { | ||
| console.warn(`[PagedEditor] renderPages took ${Math.round(stepTime)}ms`); | ||
| } | ||
|
|
||
| // Create and expose RenderedDomContext after DOM is painted | ||
| if (onRenderedDomContextReady) { | ||
| const domContext = createRenderedDomContext(pagesContainerRef.current, zoom); | ||
| onRenderedDomContextReady(domContext); | ||
| } | ||
| } | ||
|
|
||
| // Compute anchor Y positions for comments sidebar (works without DOM queries). | ||
| // Only runs when the sidebar callback is registered. | ||
| if (onAnchorPositionsChange) { | ||
| const positions = computeAnchorPositions( | ||
| hiddenPMRef.current?.getView() ?? null, | ||
| newLayout, | ||
| newBlocks, | ||
| newMeasures, | ||
| pageGap | ||
| ); | ||
| onAnchorPositionsChange(positions); | ||
| } | ||
|
|
||
| // Save cache state for next incremental update (only after successful paint). | ||
| // Apply pending incremental result first (deferred from updateBlocks to avoid | ||
| // split-state corruption on stale abort). | ||
| if (pendingIncrementalResult) { | ||
| applyIncrementalResult(cache, pendingIncrementalResult); | ||
| } | ||
| saveBlockState(cache, state.doc, newBlocks, newMeasures); | ||
|
|
||
| // Save layout statesAtBlock for convergence detection in future resumed layouts | ||
| if (newLayout.statesAtBlock) { | ||
| cache.statesAtBlock = newLayout.statesAtBlock; | ||
| } | ||
|
|
||
| // Save paginator snapshots at page boundaries for future resume | ||
| if (newLayout.paginatorSnapshots) { | ||
| cache.paginatorSnapshotAtBlock = newLayout.paginatorSnapshots; | ||
| } | ||
|
|
||
| // Signal layout is complete — only after we actually painted | ||
| syncCoordinator.onLayoutComplete(currentEpoch); | ||
|
|
||
| const totalTime = performance.now() - pipelineStart; | ||
| if (totalTime > 2000) { | ||
| console.warn( | ||
| `[PagedEditor] Layout pipeline took ${Math.round(totalTime)}ms total ` + | ||
| `(${newBlocks.length} blocks, ${newMeasures.length} measures)` | ||
| ); | ||
| } | ||
| } catch (error) { | ||
| console.error('[PagedEditor] Layout pipeline error:', error); | ||
| } |
There was a problem hiding this comment.
runLayoutPipeline(): syncCoordinator.onLayoutStart() is called unconditionally, but onLayoutComplete() is only called on the success path. If an exception occurs anywhere in the pipeline, LayoutSelectionGate will remain in layoutUpdating=true and can block selection rendering indefinitely. Move the completion signaling into a finally block (or add an explicit abort/reset) so the gate can recover from errors.
Comment on lines
+1914
to
+1917
| // Always log step timing for performance diagnostics | ||
| console.debug( | ||
| `[PagedEditor] Step 1 (${usedIncremental ? `incremental from ${incrementalDirtyFrom}` : 'full'}) → ${stepTime.toFixed(1)}ms (${newBlocks.length} blocks)` | ||
| ); |
There was a problem hiding this comment.
The new per-step console.debug calls will trigger the repo’s eslint no-console rule (only warn/error are allowed) and can spam logs in production. Consider gating behind a debug flag (e.g. env var) or removing debug logs and keeping only the existing slow-path console.warn thresholds.
Suggested change
| // Always log step timing for performance diagnostics | |
| console.debug( | |
| `[PagedEditor] Step 1 (${usedIncremental ? `incremental from ${incrementalDirtyFrom}` : 'full'}) → ${stepTime.toFixed(1)}ms (${newBlocks.length} blocks)` | |
| ); |
Comment on lines
+1107
to
+1112
| if ('totalHeight' in cached) { | ||
| const block = blocks[i]; | ||
| if (!(block.kind === 'table' && (block as TableBlock).floating)) { | ||
| cumulativeY += (cached as { totalHeight: number }).totalHeight; | ||
| } | ||
| } |
There was a problem hiding this comment.
measureBlocksIncremental(): cumulativeY is advanced only for cached/measured results that have totalHeight. ImageMeasure and TextBoxMeasure use height (no totalHeight), so their vertical contribution is skipped, making paragraphYOffset incorrect for subsequent paragraphs when floating zones are active. Update cumulativeY for all flow-affecting block kinds (e.g. add image/textBox height, and any other blocks that advance the cursor) to keep floating-zone overlap calculations consistent with full measurement.
yash-giantanalytics
added a commit
to giantanalyticsai/docx-editor
that referenced
this pull request
Apr 5, 2026
Adds incremental layout that reuses cached measurements for unchanged blocks instead of re-measuring the entire document on every keystroke. Uses ProseMirror structural sharing to detect dirty ranges, resumes pagination from cached snapshots, and applies CSS containment for browser-level optimization. ~30x faster measurement step on 20+ page documents (13.7ms down to 0.5ms).
yash-giantanalytics
added a commit
to giantanalyticsai/docx-editor
that referenced
this pull request
Apr 5, 2026
Lock file was stale after merging upstream PRs eigenpal#201, eigenpal#243, eigenpal#246, eigenpal#247. Regenerated to include new dependencies (e.g. @happy-dom/global-registrator).
2 tasks
jedrazb
requested changes
Apr 12, 2026
Contributor
There was a problem hiding this comment.
Thanks for the PR @sicko7947. Two blockers before merge, rest can be follow-ups:
incrementalBlockCache.ts:306stale PM positions on reused blocks. See inline comment.PagedEditor.tsx:2219onLayoutCompleteno longer fires on exception. See inline comment.
Also worth rebasing on main
| } | ||
| // Reindex pmStart/pmEnd on blocks after the splice point | ||
| if (result.length > dirtyFrom + newBlocks.length) { | ||
| reindexPositions(result, dirtyFrom + newBlocks.length, blockDelta === 0 ? 0 : 0); |
Contributor
There was a problem hiding this comment.
Delta is always 0 here (dead ternary), and the outer guard only runs when block count changes. Typing one char inside a paragraph keeps block count the same but shifts all subsequent PM positions by 1, so reused blocks from oldBlocks.slice(effectiveDirtyTo) carry stale pmStart/pmEnd. That breaks click-to-position, selection mapping, and hit-testing after pretty much any edit.
Fix: compute the real PM position delta (newDoc.nodeSize vs prevDoc.nodeSize at the splice boundary, or use transaction.mapping when available) and call reindexPositions unconditionally whenever delta != 0, regardless of block count change.
Contributor
Author
There was a problem hiding this comment.
Fixed in 9af4ddb.
Separated block-index adjustment (still guarded by blockDelta !== 0) from PM position reindexing. The PM delta is now computed by comparing newDoc vs prevDoc cumulative node sizes at the splice boundary. reindexPositions runs unconditionally whenever pmDelta !== 0, so even single-char edits that don't change block count will correctly shift pmStart/pmEnd on all reused blocks.
| } | ||
|
|
||
| // Signal layout is complete — only after we actually painted | ||
| syncCoordinator.onLayoutComplete(currentEpoch); |
Contributor
There was a problem hiding this comment.
This used to live outside the try/catch on main. Moving it inside means any throw in the pipeline leaves LayoutSelectionGate stuck in layoutUpdating=true and selection rendering blocks indefinitely. Wrap in finally so it always fires.
Contributor
Author
There was a problem hiding this comment.
Fixed in 9af4ddb. Moved to a finally block so it always fires, even when the pipeline throws.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
1. incrementalBlockCache: compute real PM position delta from doc size difference at splice boundary instead of using dead blockDelta ternary. Reindex unconditionally when pmDelta != 0, fixing click-to-position and selection mapping after any edit. 2. PagedEditor: move onLayoutComplete to finally block so LayoutSelectionGate always unblocks, even on pipeline exceptions. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
6 tasks
Contributor
|
I ran performance test on this end-to-end against
The "30×" is one sub-step, the PR's own table sums to 2× total pipeline, and even that doesn't land end-to-end because paint/DOM commit dominate on this workload, not the phases this PR touches. Given ~3,000 LOC im closing this PR. Please lemme know if I missed something |
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.
Summary
content-visibility: autoon page shells for browser paint optimizationProblem
The editor becomes unusable on real-world documents (20-80 pages). Profiling on a 23-page document showed:
Architecture
The layout pipeline has 4 steps: block conversion → measurement → pagination → paint. Previously, all 4 steps processed the entire document. Now:
Step 1 — Incremental block conversion: Uses ProseMirror's structural sharing (
newDoc.child(i) === prevDoc.child(i)) to detect which top-level nodes changed. Only re-converts the dirty range via the newconvertTopLevelNode()function. Handles list counter propagation past the dirty range.Step 2 — Incremental measurement: Reuses cached measures for clean blocks before the dirty range. Re-measures only from
dirtyFromforward. Full floating zone pre-scan still runs (fast, zones can shift).Step 3 — Layout resume + early exit: Paginator snapshots are captured at page boundaries during each layout run. On incremental updates,
layoutDocument()resumes from the closest snapshot before the dirty block. After 2 consecutive blocks past the dirty range converge with the previous layout state, remaining pages are spliced from the previous run.Step 4 — CSS containment:
content-visibility: auto+contain-intrinsic-sizeon page shells lets the browser skip layout/paint for off-screen pages.Measured Results (287 blocks, 24 pages)
Steps 1-3 combined: 15ms → 0.8ms (19x faster). On larger documents (500+ blocks) the savings scale linearly.
Key Design Decisions
PM node identity as primary dirty detection — works for both transaction-driven and direct relayout calls (resize, font load, header changes). No dependency on transaction steps.
Deferred cache mutation —
updateBlocks()returns results without mutating the cache.applyIncrementalResult()is called only after successful paint, preventing split-state corruption on stale aborts during rapid typing.Conservative fallback — falls back to full pipeline when: no previous doc cached, >50% of nodes changed, section break in dirty range.
Deep fragment cloning in snapshots — paginator
snapshot()deep-clones fragment objects (not just arrays) to prevent shared-reference mutations.Files Changed
packages/core/src/layout-bridge/incrementalBlockCache.tspackages/core/src/layout-bridge/toFlowBlocks.tsconvertTopLevelNode()packages/core/src/layout-engine/index.tsresumeFromoption, early exit,applyContextualSpacingRange()packages/core/src/layout-engine/paginator.tssnapshot(),createPaginatorFromSnapshot()packages/core/src/layout-engine/types.tsPaginatorSnapshot,ResumeOptions,PaginatorStateAtBlockpackages/core/src/layout-painter/renderPage.tspackages/react/src/paged-editor/PagedEditor.tsxTest plan
🤖 Generated with Claude Code