From dc5d6aad8acf91cbf704a1da258b6cec50d7efe2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicol=C3=B2=20Ribaudo?= Date: Thu, 16 Jan 2025 11:26:19 +0100 Subject: [PATCH] Avoid degrading scroll performance due to the detail view When scrolling quickly, the constant re-rendering of the detail view significantly affects rendering performance, causing Firefox to not render even the _background canvas_, which is just a static canvas not being re-drawn by JavaScript. This commit changes the viewer to only render the detail view while scrolling if its rendering hasn't just been cancelled. This means that: - when the user is scrolling slowly, we have enough time to render the detail view before that we need to change its area, so the user always sees the full screen as high resolution. - when the user is scrolling quickly, as soon as we have to cancel a rendering we just give up, and the user will see the lower resolution canvas. When then the user stops scrolling, we render the detail view for the new visible area. --- web/pdf_page_detail_view.js | 22 ++++++++++++++++++++++ web/pdf_rendering_queue.js | 21 +++++++++++++++------ web/pdf_thumbnail_viewer.js | 4 +++- web/pdf_viewer.js | 27 ++++++++++++++++++++++++++- 4 files changed, 66 insertions(+), 8 deletions(-) diff --git a/web/pdf_page_detail_view.js b/web/pdf_page_detail_view.js index 34585d522..72d3eba60 100644 --- a/web/pdf_page_detail_view.js +++ b/web/pdf_page_detail_view.js @@ -24,6 +24,14 @@ import { RenderingStates } from "./ui_utils.js"; class PDFPageDetailView extends BasePDFPageView { #detailArea = null; + /** + * @type {boolean} True when the last rendering attempt of the view was + * cancelled due to a `.reset()` call. This will happen when + * the visible area changes so much during the rendering that + * we need to cancel the rendering and start over. + */ + renderingCancelled = false; + constructor({ pageView }) { super(pageView); @@ -41,9 +49,23 @@ class PDFPageDetailView extends BasePDFPageView { return this.pageView.pdfPage; } + get renderingState() { + return super.renderingState; + } + + set renderingState(value) { + this.renderingCancelled = false; + super.renderingState = value; + } + reset({ keepCanvas = false } = {}) { + const renderingCancelled = + this.renderingCancelled || + this.renderingState === RenderingStates.RUNNING || + this.renderingState === RenderingStates.PAUSED; this.cancelRendering(); this.renderingState = RenderingStates.INITIAL; + this.renderingCancelled = renderingCancelled; if (!keepCanvas) { this._resetCanvas(); diff --git a/web/pdf_rendering_queue.js b/web/pdf_rendering_queue.js index 0020518f9..bbcb51fc3 100644 --- a/web/pdf_rendering_queue.js +++ b/web/pdf_rendering_queue.js @@ -102,15 +102,22 @@ class PDFRenderingQueue { * @param {Array} views * @param {boolean} scrolledDown * @param {boolean} [preRenderExtra] + * @param {boolean} [ignoreDetailViews] */ - getHighestPriority(visible, views, scrolledDown, preRenderExtra = false) { + getHighestPriority( + visible, + views, + scrolledDown, + preRenderExtra = false, + ignoreDetailViews = false + ) { /** * The state has changed. Figure out which page has the highest priority to * render next (if any). * * Priority: * 1. visible pages - * 2. zoomed-in partial views of visible pages + * 2. zoomed-in partial views of visible pages, unless `ignoreDetailViews` * 3. if last scrolled down, the page after the visible pages, or * if last scrolled up, the page before the visible pages */ @@ -127,10 +134,12 @@ class PDFRenderingQueue { } } - for (let i = 0; i < numVisible; i++) { - const { detailView } = visibleViews[i].view; - if (detailView && !this.isViewFinished(detailView)) { - return detailView; + if (!ignoreDetailViews) { + for (let i = 0; i < numVisible; i++) { + const { detailView } = visibleViews[i].view; + if (detailView && !this.isViewFinished(detailView)) { + return detailView; + } } } diff --git a/web/pdf_thumbnail_viewer.js b/web/pdf_thumbnail_viewer.js index 7ec1b9837..b6bdf36f3 100644 --- a/web/pdf_thumbnail_viewer.js +++ b/web/pdf_thumbnail_viewer.js @@ -292,7 +292,9 @@ class PDFThumbnailViewer { const thumbView = this.renderingQueue.getHighestPriority( visibleThumbs, this._thumbnails, - scrollAhead + scrollAhead, + /* preRenderExtra */ false, + /* ignoreDetailViews */ true ); if (thumbView) { this.#ensurePdfPageLoaded(thumbView).then(() => { diff --git a/web/pdf_viewer.js b/web/pdf_viewer.js index 9587c0d20..5cd548577 100644 --- a/web/pdf_viewer.js +++ b/web/pdf_viewer.js @@ -241,6 +241,8 @@ class PDFViewer { #mlManager = null; + #scrollTimeoutId = null; + #switchAnnotationEditorModeAC = null; #switchAnnotationEditorModeTimeoutId = null; @@ -1241,6 +1243,15 @@ class PDFViewer { if (this.pagesCount === 0) { return; } + + if (this.#scrollTimeoutId) { + clearTimeout(this.#scrollTimeoutId); + } + this.#scrollTimeoutId = setTimeout(() => { + this.#scrollTimeoutId = null; + this.update(); + }, 100); + this.update(); } @@ -1851,11 +1862,21 @@ class PDFViewer { this._spreadMode !== SpreadMode.NONE && this._scrollMode !== ScrollMode.HORIZONTAL; + // If we are scrolling and the rendering of a detail view was just + // cancelled, it's because the user is scrolling too quickly and so + // we constantly need to re-render a different area. + // Don't attempt to re-render it: this will be done once the user + // stops scrolling. + const ignoreDetailViews = + this.#scrollTimeoutId !== null && + visiblePages.views.some(page => page.detailView?.renderingCancelled); + const pageView = this.renderingQueue.getHighestPriority( visiblePages, this._pages, scrollAhead, - preRenderExtra + preRenderExtra, + ignoreDetailViews ); if (pageView) { @@ -2449,6 +2470,10 @@ class PDFViewer { clearTimeout(this.#scaleTimeoutId); this.#scaleTimeoutId = null; } + if (this.#scrollTimeoutId !== null) { + clearTimeout(this.#scrollTimeoutId); + this.#scrollTimeoutId = null; + } if (!noUpdate) { this.update(); }