Skip to content

Commit 328641c

Browse files
committed
perf: stop idle reader re-render loop from visibility reset
viewportPages() reset every visible page's `visible` ratio to 0 and then restored it, producing two reactive writes per page on each call. For translated (compacted) pages, pageLayout reads `visible`, so the pageLayout watcher re-scheduled the viewport, which re-ran viewportPages() on the next frame. The reader re-rendered all mounted pages and rebuilt emphasis overlays at display refresh rate while idle (~170% CPU in the packaged app). - Only reset pages that actually left the viewport. - Skip replacing compaction results on a cache hit when the cached entry is already current. - Add a regression test asserting that an unchanged viewport triggers no reactive updates. Idle CPU on the same translated document drops from ~170% to ~6%.
1 parent e8e3df2 commit 328641c

3 files changed

Lines changed: 71 additions & 2 deletions

File tree

‎src/features/reader/paragraph-compaction.mjs‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -309,7 +309,10 @@ export function createParagraphCompaction({
309309
':' +
310310
p.blocks.map((b) => b.translation).join('\n');
311311
if (cache.has(key)) {
312-
if (cache.get(key)) results.value = new Map(results.value).set(p.number, cache.get(key));
312+
const cached = cache.get(key);
313+
// An unchanged result must not replace the map and re-render every page.
314+
if (cached && results.value.get(p.number) !== cached)
315+
results.value = new Map(results.value).set(p.number, cached);
313316
continue;
314317
}
315318
try {

‎src/features/reader/reader-viewport.mjs‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,8 +38,13 @@ export function createReaderViewport({ renderState, session, motion, view, activ
3838
},
3939
view.active.value,
4040
);
41+
// Reset only pages that left the viewport. Clearing a still-visible page and
42+
// restoring its ratio triggers pageLayout, which schedules this lookup again
43+
// on the next frame and keeps the whole reader re-rendering while idle.
44+
const stillVisible = new Set(window.visible.map((item) => item.number));
4145
for (const n of renderState.visiblePages)
42-
if (session.pages.value[n - 1]) session.pages.value[n - 1].visible = 0;
46+
if (!stillVisible.has(n) && session.pages.value[n - 1])
47+
session.pages.value[n - 1].visible = 0;
4348
renderState.visiblePages.clear();
4449
let best;
4550
for (const item of window.visible) {
Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
import assert from 'node:assert/strict';
2+
import test from 'node:test';
3+
import { effect, reactive, ref, stop } from 'vue';
4+
import { createReaderViewport } from '../../src/features/reader/reader-viewport.mjs';
5+
6+
function readerViewport(bounds) {
7+
const pages = ref(reactive([1, 2, 3].map((number) => ({ number, visible: 0 }))));
8+
const layout = {
9+
columns: 1,
10+
rows: [0, 1, 2].map((i) => ({ offset: i * 110, extent: 100, start: i, end: i + 1 })),
11+
frames: [0, 1, 2].map((i) => ({ number: i + 1, x: 0, y: i * 110, width: 100, height: 100 })),
12+
};
13+
const rect = (r) => ({ getBoundingClientRect: () => r });
14+
const renderState = {
15+
layoutElement: { value: rect({ left: 0, top: 0 }) },
16+
renderMetrics: { viewportLookups: 0, geometryReads: 0 },
17+
visiblePages: new Set(),
18+
renderWindow: ref(new Set()),
19+
canvasEls: new Map(),
20+
};
21+
const viewport = createReaderViewport({
22+
renderState,
23+
session: { pages, restoringView: { value: false } },
24+
motion: { pinching: { value: false }, referenceReturn: { value: null } },
25+
view: { reader: { value: rect(bounds) }, pageLayout: { value: layout }, active: ref(1) },
26+
activity: { foreground: { value: true } },
27+
actions: { canvasRendering: { releaseCanvas() {} } },
28+
});
29+
return { pages, viewport };
30+
}
31+
32+
test('repeating an unchanged viewport lookup does not touch page visibility', () => {
33+
const { pages, viewport } = readerViewport({ left: 0, right: 100, top: 50, bottom: 150 });
34+
viewport.viewportPages();
35+
let runs = 0;
36+
const runner = effect(() => {
37+
for (const page of pages.value) void page.visible;
38+
runs++;
39+
});
40+
runs = 0;
41+
viewport.viewportPages();
42+
stop(runner);
43+
// A reset-then-restore would re-run pageLayout and schedule another lookup.
44+
assert.equal(runs, 0);
45+
assert.deepEqual(
46+
pages.value.map((page) => page.visible),
47+
[0.5, 0.4, 0],
48+
);
49+
});
50+
51+
test('pages that scroll out of the viewport are cleared', () => {
52+
const bounds = { left: 0, right: 100, top: 50, bottom: 150 };
53+
const { pages, viewport } = readerViewport(bounds);
54+
viewport.viewportPages();
55+
Object.assign(bounds, { top: 220, bottom: 320 });
56+
viewport.viewportPages();
57+
assert.deepEqual(
58+
pages.value.map((page) => page.visible),
59+
[0, 0, 1],
60+
);
61+
});

0 commit comments

Comments
 (0)