Skip to content

Commit 84af4c0

Browse files
authored
fix(virtualization): correct scroll mapping and viewport sizing at non-100% zoom (#2171)
* fix(virtualization): correct scroll mapping and viewport sizing at non-100% zoom Fix blank pages and excess scroll space when zoomed out in long documents. Three interrelated bugs caused the virtualization system to break at zoom != 100%: 1. updateVirtualWindow() used getBoundingClientRect().top (screen-space, affected by CSS transform: scale) but compared against virtualOffsets (layout-space, unscaled). Divide by zoom factor to fix. 2. PresentationEditor used 24px page gap for viewport height calculation while DomPainter virtualization defaulted to 72px. Normalize the virtualization gap to match the effective page gap. 3. CSS transform: scale() does not change layout box dimensions. At zoom < 1, painterHost's unscaled CSS box overflowed viewportHost, inflating the scroll range. Use explicit height + overflow: hidden on viewportHost to clip the CSS box. * fix(zoom): ensure zoom controls work on blank documents Blank documents created by the store lacked an `id`, causing the PresentationEditor instance to not register in the static `#instances` map. When `setGlobalZoom()` iterated the map it found nothing, so zoom CSS transforms were never applied. Two fixes: - Add `id: uuidv4()` to the blank document config in superdoc-store - Always register PresentationEditor instances with a fallback key when `documentId` is not provided, ensuring `setGlobalZoom` works regardless of document configuration * fix(virtualization): address PR review — registryKey, overflow clipping, zoom test - Replace options.documentId mutation with #registryKey private field - Replace overflow:hidden with negative margin-bottom on painterHost to avoid clipping collaboration cursor labels - Add zoom × virtualization interaction test for non-scrollable container * test(virtualization): tighten zoom assertions and add behavior test Tighten the zoom × virtualization unit test to assert exact page indices [7, 8, 9] instead of loose range checks that pass on buggy behavior. Add a Playwright behavior test that generates a long document, sets 75% zoom, scrolls to mid-document, and verifies content is visible (not blank). This guards against regressions when virtualization or zoom code changes.
1 parent c7efa85 commit 84af4c0

5 files changed

Lines changed: 221 additions & 14 deletions

File tree

packages/layout-engine/painters/dom/src/index.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,7 @@ export const createDomPainter = (
122122
getActiveComment?: () => string | null;
123123
getPaintSnapshot?: () => PaintSnapshot | null;
124124
onScroll?: () => void;
125+
setZoom?: (zoom: number) => void;
125126
} => {
126127
const painter = new DomPainter(options.blocks, options.measures, {
127128
pageStyles: options.pageStyles,
@@ -167,5 +168,9 @@ export const createDomPainter = (
167168
onScroll() {
168169
painter.onScroll();
169170
},
171+
// Notify painter of CSS transform scale so virtualization maps scroll correctly
172+
setZoom(zoom: number) {
173+
painter.setZoom(zoom);
174+
},
170175
};
171176
};

packages/layout-engine/painters/dom/src/renderer.ts

Lines changed: 30 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1026,6 +1026,8 @@ export class DomPainter {
10261026
private onScrollHandler: ((e: Event) => void) | null = null;
10271027
private onWindowScrollHandler: ((e: Event) => void) | null = null;
10281028
private onResizeHandler: ((e: Event) => void) | null = null;
1029+
/** CSS zoom/scale factor applied to the mount element via transform: scale(). Defaults to 1 (no zoom). */
1030+
private zoomFactor = 1;
10291031
private sdtHover = new SdtGroupedHover();
10301032
/** The currently active/selected comment ID for highlighting */
10311033
private activeCommentId: string | null = null;
@@ -1083,6 +1085,26 @@ export class DomPainter {
10831085
}
10841086
}
10851087

1088+
/**
1089+
* Sets the CSS zoom/scale factor applied to the mount element.
1090+
*
1091+
* When the mount element has `transform: scale(zoom)`, getBoundingClientRect()
1092+
* returns screen-space coordinates (scaled), but internal layout offsets are in
1093+
* unscaled layout space. This factor is used to convert between the two spaces
1094+
* during virtualization window calculations.
1095+
*
1096+
* @param zoom - The zoom/scale factor (e.g., 0.75 for 75% zoom). Defaults to 1.
1097+
*/
1098+
public setZoom(zoom: number): void {
1099+
const next = typeof zoom === 'number' && Number.isFinite(zoom) && zoom > 0 ? zoom : 1;
1100+
if (next !== this.zoomFactor) {
1101+
this.zoomFactor = next;
1102+
if (this.virtualEnabled && this.mount) {
1103+
this.updateVirtualWindow();
1104+
}
1105+
}
1106+
}
1107+
10861108
/**
10871109
* Sets the active comment ID for highlighting.
10881110
* When set, only the active comment's range is highlighted.
@@ -1612,16 +1634,21 @@ export class DomPainter {
16121634
return;
16131635
}
16141636

1615-
// Map scrollTop -> anchor page index via prefix sums
1637+
// Map scrollTop -> anchor page index via prefix sums.
1638+
// virtualOffsets are in layout (unscaled) space, so scrollY must also be in layout space.
1639+
// When the mount has transform: scale(zoom), getBoundingClientRect() returns
1640+
// screen-space values that must be divided by zoom to get layout-space coordinates.
16161641
const paddingTop = this.getMountPaddingTopPx();
1642+
const zoom = this.zoomFactor;
16171643
let scrollY: number;
16181644
const isContainerScrollable = this.mount.scrollHeight > this.mount.clientHeight + 1;
16191645
if (isContainerScrollable) {
16201646
scrollY = Math.max(0, this.mount.scrollTop - paddingTop);
16211647
} else {
16221648
const rect = this.mount.getBoundingClientRect();
1623-
// Translate viewport scroll to content-space scroll offset
1624-
scrollY = Math.max(0, -rect.top - paddingTop);
1649+
// rect.top is in screen space (affected by CSS transform: scale).
1650+
// Divide by zoom to convert to layout space for comparison with virtualOffsets.
1651+
scrollY = Math.max(0, -rect.top / zoom - paddingTop);
16251652
}
16261653

16271654
// Binary search for anchor index such that topOfIndex(i) <= scrollY < topOfIndex(i+1)

packages/layout-engine/painters/dom/src/virtualization.test.ts

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -369,6 +369,57 @@ describe('DomPainter virtualization (vertical)', () => {
369369
expect(footerEl).toBeTruthy();
370370
});
371371

372+
it('corrects scroll position for zoom factor in non-scrollable container', () => {
373+
// When the mount element has transform: scale(zoom), getBoundingClientRect() returns
374+
// screen-space coordinates. The zoom factor divides rect.top to convert back to layout space.
375+
// Without this correction, the virtual window drifts at non-100% zoom levels.
376+
const zoom = 0.75;
377+
const pageH = 500;
378+
const gap = 72;
379+
const pageCount = 20;
380+
381+
const painter = createDomPainter({
382+
blocks: [block],
383+
measures: [measure],
384+
virtualization: { enabled: true, window: 3, overscan: 0, gap, paddingTop: 0 },
385+
});
386+
387+
const layout = makeLayout(pageCount);
388+
painter.paint(layout, mount);
389+
painter.setZoom!(zoom);
390+
391+
// Simulate non-scrollable container: scrollHeight <= clientHeight so it uses getBoundingClientRect path
392+
Object.defineProperty(mount, 'scrollHeight', { value: 100, configurable: true });
393+
Object.defineProperty(mount, 'clientHeight', { value: 600, configurable: true });
394+
395+
// Simulate being scrolled to layout-space position ~5000px.
396+
// In screen space (after zoom), rect.top = -5000 * zoom = -3750.
397+
const layoutScrollY = 5000;
398+
const screenTop = -layoutScrollY * zoom; // -3750
399+
mount.getBoundingClientRect = () =>
400+
({
401+
top: screenTop,
402+
left: 0,
403+
right: 400,
404+
bottom: 600 + screenTop,
405+
width: 400,
406+
height: 600,
407+
x: 0,
408+
y: screenTop,
409+
toJSON() {},
410+
}) as DOMRect;
411+
412+
painter.onScroll!();
413+
414+
// At layoutScrollY=5000, the anchor page is index 8 (topOfIndex(8)=4576 <= 5000, topOfIndex(9)=5148 > 5000).
415+
// With window=3, overscan=0, the window is centered around the anchor: pages [7, 8, 9].
416+
// Without the zoom correction, scrollY would be 3750 (screen-space), giving anchor=6 and pages [5, 6, 7].
417+
const pages = mount.querySelectorAll('.superdoc-page');
418+
const indices = Array.from(pages).map((p) => Number((p as HTMLElement).dataset.pageIndex));
419+
420+
expect(indices).toEqual([7, 8, 9]);
421+
});
422+
372423
it('renders drawing fragments inside virtualized windows', () => {
373424
const painter = createDomPainter({
374425
blocks: [drawingBlock],

packages/super-editor/src/core/presentation-editor/PresentationEditor.ts

Lines changed: 47 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -263,6 +263,8 @@ export class PresentationEditor extends EventEmitter {
263263
}
264264

265265
#options: PresentationEditorOptions;
266+
/** Key used to register this instance in the static registry. Separate from options.documentId to avoid mutating caller's object. */
267+
#registryKey: string | null = null;
266268
#editor: Editor;
267269
#visibleHost: HTMLElement;
268270
#viewportHost: HTMLElement;
@@ -612,10 +614,11 @@ export class PresentationEditor extends EventEmitter {
612614
this.#setupInputBridge();
613615
this.#syncTrackedChangesPreferences();
614616

615-
// Register this instance in the static registry
616-
if (options.documentId) {
617-
PresentationEditor.#instances.set(options.documentId, this);
618-
}
617+
// Register this instance in the static registry.
618+
// Use a separate field to avoid mutating the caller's options object and to keep
619+
// the registry key consistent with the overlay ID set earlier (line ~453).
620+
this.#registryKey = options.documentId || `__anonymous_${Date.now()}_${Math.random().toString(36).slice(2)}`;
621+
PresentationEditor.#instances.set(this.#registryKey, this);
619622

620623
this.#pendingDocChange = true;
621624
this.#scheduleRerender();
@@ -2313,6 +2316,8 @@ export class PresentationEditor extends EventEmitter {
23132316
}
23142317
this.#layoutOptions.zoom = zoom;
23152318
this.#applyZoom();
2319+
// Notify DomPainter so virtualization accounts for the CSS transform scale
2320+
this.#domPainter?.setZoom?.(zoom);
23162321
this.emit('zoomChange', { zoom });
23172322
this.#scheduleSelectionUpdate();
23182323
// Trigger cursor updates on zoom changes
@@ -2398,8 +2403,9 @@ export class PresentationEditor extends EventEmitter {
23982403
}
23992404

24002405
// Unregister from static registry
2401-
if (this.#options?.documentId) {
2402-
PresentationEditor.#instances.delete(this.#options.documentId);
2406+
if (this.#registryKey) {
2407+
PresentationEditor.#instances.delete(this.#registryKey);
2408+
this.#registryKey = null;
24032409
}
24042410

24052411
// Clean up header/footer session manager
@@ -3551,17 +3557,30 @@ export class PresentationEditor extends EventEmitter {
35513557

35523558
#ensurePainter(blocks: FlowBlock[], measures: Measure[]) {
35533559
if (!this.#domPainter) {
3560+
// Ensure the virtualization gap matches the effective page gap so that
3561+
// DomPainter's spacer/offset math stays consistent with #applyZoom() height calculations.
3562+
const virtualization = this.#layoutOptions.virtualization;
3563+
const effectiveGap = this.#getEffectivePageGap();
3564+
const normalizedVirtualization = virtualization?.enabled
3565+
? { ...virtualization, gap: virtualization.gap ?? effectiveGap }
3566+
: virtualization;
3567+
35543568
this.#domPainter = createDomPainter({
35553569
blocks,
35563570
measures,
35573571
layoutMode: this.#layoutOptions.layoutMode ?? 'vertical',
3558-
virtualization: this.#layoutOptions.virtualization,
3572+
virtualization: normalizedVirtualization,
35593573
pageStyles: this.#layoutOptions.pageStyles,
35603574
headerProvider: this.#headerFooterSession?.headerDecorationProvider,
35613575
footerProvider: this.#headerFooterSession?.footerDecorationProvider,
35623576
ruler: this.#layoutOptions.ruler,
3563-
pageGap: this.#layoutState.layout?.pageGap ?? this.#getEffectivePageGap(),
3577+
pageGap: this.#layoutState.layout?.pageGap ?? effectiveGap,
35643578
});
3579+
// Pass the current zoom so virtualization accounts for the CSS transform scale
3580+
const currentZoom = this.#layoutOptions.zoom ?? 1;
3581+
if (currentZoom !== 1) {
3582+
this.#domPainter.setZoom(currentZoom);
3583+
}
35653584
}
35663585
return this.#domPainter;
35673586
}
@@ -5037,10 +5056,16 @@ export class PresentationEditor extends EventEmitter {
50375056
this.#viewportHost.style.width = `${scaledWidth}px`;
50385057
this.#viewportHost.style.minWidth = `${scaledWidth}px`;
50395058
this.#viewportHost.style.minHeight = `${scaledHeight}px`;
5059+
this.#viewportHost.style.height = '';
5060+
this.#viewportHost.style.overflow = '';
50405061
this.#viewportHost.style.transform = '';
50415062

50425063
this.#painterHost.style.width = `${totalWidth}px`;
50435064
this.#painterHost.style.minHeight = `${maxHeight}px`;
5065+
// Negative margin compensates for the CSS box overflow from transform: scale().
5066+
// At zoom < 1 the unscaled CSS box is larger than the visual; this pulls the
5067+
// bottom edge up to match, without clipping overlays (e.g., cursor labels).
5068+
this.#painterHost.style.marginBottom = zoom !== 1 ? `${maxHeight * zoom - maxHeight}px` : '';
50445069
this.#painterHost.style.transformOrigin = 'top left';
50455070
this.#painterHost.style.transform = zoom === 1 ? '' : `scale(${zoom})`;
50465071

@@ -5059,19 +5084,30 @@ export class PresentationEditor extends EventEmitter {
50595084
//
50605085
// This ensures the scroll container sees the correct scaled content size while
50615086
// the transform provides visual scaling.
5087+
//
5088+
// CSS transform: scale() does NOT change the element's CSS box dimensions.
5089+
// At zoom < 1, painterHost's CSS box stays at the full unscaled height while its
5090+
// visual size is smaller. A negative margin-bottom on painterHost compensates for
5091+
// the difference, so the scroll container sees the correct scaled size without
5092+
// clipping overlays (e.g., collaboration cursor labels that extend above their caret).
50625093
const scaledWidth = maxWidth * zoom;
50635094
const scaledHeight = totalHeight * zoom;
50645095

5065-
// Set viewport to scaled dimensions for scroll container
50665096
this.#viewportHost.style.width = `${scaledWidth}px`;
50675097
this.#viewportHost.style.minWidth = `${scaledWidth}px`;
50685098
this.#viewportHost.style.minHeight = `${scaledHeight}px`;
5099+
this.#viewportHost.style.height = '';
5100+
this.#viewportHost.style.overflow = '';
50695101
this.#viewportHost.style.transform = '';
50705102

5071-
// Set painterHost to UNSCALED dimensions and apply transform
5072-
// This way: 816px * scale(1.5) = 1224px visual = matches viewport
5103+
// Set painterHost to UNSCALED dimensions and apply transform.
5104+
// Negative margin compensates for the CSS box overflow from transform: scale().
5105+
// At zoom < 1: totalHeight=74304 with scale(0.75) → visual 55728px but CSS box stays 74304px.
5106+
// marginBottom = totalHeight * zoom - totalHeight = 74304 * 0.75 - 74304 = -18576px
5107+
// This shrinks the layout contribution to match the visual size.
50735108
this.#painterHost.style.width = `${maxWidth}px`;
50745109
this.#painterHost.style.minHeight = `${totalHeight}px`;
5110+
this.#painterHost.style.marginBottom = zoom !== 1 ? `${totalHeight * zoom - totalHeight}px` : '';
50755111
this.#painterHost.style.transformOrigin = 'top left';
50765112
this.#painterHost.style.transform = zoom === 1 ? '' : `scale(${zoom})`;
50775113

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
1+
import { test, expect } from '../../fixtures/superdoc.js';
2+
3+
test.use({ config: { toolbar: 'none' } });
4+
5+
test('content is visible at 75% zoom when scrolled to mid-document', async ({ superdoc }) => {
6+
// Generate a long document (~60 paragraphs) to span many pages and trigger virtualization.
7+
await superdoc.page.evaluate(() => {
8+
const editor = (window as any).editor;
9+
const { state } = editor;
10+
const { schema } = state;
11+
12+
const paragraphs: any[] = [];
13+
for (let i = 0; i < 60; i++) {
14+
const text = schema.text(
15+
`Paragraph number ${i + 1}. ` +
16+
'This line contains enough text to be clearly visible when rendered ' +
17+
'in the paginated layout engine viewport.',
18+
);
19+
const run = schema.nodes.run.create(null, text);
20+
paragraphs.push(schema.nodes.paragraph.create(null, run));
21+
}
22+
23+
const doc = schema.nodes.doc.create(null, paragraphs);
24+
const tr = state.tr.replaceWith(0, state.doc.content.size, doc.content);
25+
editor.view.dispatch(tr);
26+
});
27+
28+
await superdoc.waitForStable(2000);
29+
30+
// Verify multiple pages exist before proceeding.
31+
const initialPageCount = await superdoc.page.locator('.superdoc-page[data-page-index]').count();
32+
expect(initialPageCount).toBeGreaterThanOrEqual(3);
33+
34+
// Set zoom to 75%.
35+
await superdoc.page.evaluate(() => {
36+
(window as any).superdoc.setZoom(75);
37+
});
38+
await superdoc.waitForStable(1000);
39+
40+
// Scroll to mid-document.
41+
await superdoc.page.evaluate(() => {
42+
// Walk from the editor element up to find the scrollable ancestor.
43+
const editor = document.querySelector('.superdoc-viewport') ?? document.querySelector('#editor');
44+
let scrollable: HTMLElement | null = null;
45+
let el: HTMLElement | null = editor as HTMLElement;
46+
while (el && el !== document.documentElement) {
47+
if (el.scrollHeight > el.clientHeight + 10) {
48+
scrollable = el;
49+
break;
50+
}
51+
el = el.parentElement;
52+
}
53+
if (!scrollable) {
54+
scrollable = document.documentElement;
55+
}
56+
scrollable.scrollTop = Math.floor(scrollable.scrollHeight / 2);
57+
});
58+
59+
await superdoc.waitForStable(1000);
60+
61+
// Pages should be mounted in the DOM.
62+
const visiblePages = superdoc.page.locator('.superdoc-page[data-page-index]');
63+
await expect(visiblePages.first()).toBeAttached({ timeout: 5000 });
64+
65+
// Mounted pages must contain visible content lines (not blank).
66+
const linesInView = superdoc.page.locator('.superdoc-page .superdoc-line');
67+
await expect(linesInView.first()).toBeAttached({ timeout: 5000 });
68+
const lineCount = await linesInView.count();
69+
expect(lineCount).toBeGreaterThan(0);
70+
71+
// At least one line must have non-empty text.
72+
const hasVisibleText = await superdoc.page.evaluate(() => {
73+
const lines = document.querySelectorAll('.superdoc-page .superdoc-line');
74+
for (const line of lines) {
75+
if ((line.textContent ?? '').trim().length > 0) return true;
76+
}
77+
return false;
78+
});
79+
expect(hasVisibleText).toBe(true);
80+
81+
// Mounted pages should include mid-document pages, not just the first page.
82+
const pageIndices = await superdoc.page.evaluate(() => {
83+
const pages = document.querySelectorAll('.superdoc-page[data-page-index]');
84+
return Array.from(pages).map((p) => Number((p as HTMLElement).dataset.pageIndex));
85+
});
86+
const maxPageIndex = Math.max(...pageIndices);
87+
expect(maxPageIndex).toBeGreaterThanOrEqual(1);
88+
});

0 commit comments

Comments
 (0)