From 24b7fb20ef97cdd967b20b2dce3bf4015e60fff3 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Mon, 18 Oct 2021 12:13:54 +0200 Subject: [PATCH 1/3] Improve pre-rendering at the start/end of the document This is a very old "issue", which has existed since essentially forever, and it affects all of the available scrollModes. However, in the recently added Page-mode it's particularily noticeable since we use a *simulated* scroll direction there. When deciding what page(s) to pre-render, we only consider the current scroll direction. This works well in most cases, but can break down at the start/end of the document by trying to pre-render a page *outside* of the existing ones. To improve this, we'll thus *force* the scroll direction at the start/end of the document. *Steps to reproduce:* 0. Open the viewer, e.g. https://mozilla.github.io/pdf.js/web/viewer.html 1. Enable vertical scrolling. 2. Press the End key. 3. Open the devtools and, using the DOM Inspector, notice how page 13 is *not* being pre-rendered. --- web/base_viewer.js | 12 +++++++----- web/pdf_thumbnail_viewer.js | 12 +++++++++++- 2 files changed, 18 insertions(+), 6 deletions(-) diff --git a/web/base_viewer.js b/web/base_viewer.js index fbacf361c..fdfb7857e 100644 --- a/web/base_viewer.js +++ b/web/base_viewer.js @@ -1361,10 +1361,12 @@ class BaseViewer { return promise; } - /** - * @private - */ - get _scrollAhead() { + #getScrollAhead(views) { + if (views.first.id === 1) { + return true; + } else if (views.last.id === this.pagesCount) { + return false; + } switch (this._scrollMode) { case ScrollMode.PAGE: return this._scrollModePageState.scrollDown; @@ -1376,7 +1378,7 @@ class BaseViewer { forceRendering(currentlyVisiblePages) { const visiblePages = currentlyVisiblePages || this._getVisiblePages(); - const scrollAhead = this._scrollAhead; + const scrollAhead = this.#getScrollAhead(visiblePages); const preRenderExtra = this._spreadMode !== SpreadMode.NONE && this._scrollMode !== ScrollMode.HORIZONTAL; diff --git a/web/pdf_thumbnail_viewer.js b/web/pdf_thumbnail_viewer.js index 36d2e6bd6..350b1a820 100644 --- a/web/pdf_thumbnail_viewer.js +++ b/web/pdf_thumbnail_viewer.js @@ -295,12 +295,22 @@ class PDFThumbnailViewer { return promise; } + #getScrollAhead(views) { + if (views.first.id === 1) { + return true; + } else if (views.last.id === this._thumbnails.length) { + return false; + } + return this.scroll.down; + } + forceRendering() { const visibleThumbs = this._getVisibleThumbs(); + const scrollAhead = this.#getScrollAhead(visibleThumbs); const thumbView = this.renderingQueue.getHighestPriority( visibleThumbs, this._thumbnails, - this.scroll.down + scrollAhead ); if (thumbView) { this._ensurePdfPageLoaded(thumbView).then(() => { From 91692a20d152cbb7300c984a2735f0c5d8a4c333 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Mon, 18 Oct 2021 13:41:10 +0200 Subject: [PATCH 2/3] Skip the first/last visible pages pre-rendering page layouts with "holes" (PR 14131 follow-up) This was a stupid oversight on my part, since the first/last visible pages have obviously already been rendered at the point when we're checking for any potential "holes" in the page layout. While this will obviously not have any measurable effect on performance, we should nonetheless avoid doing completely unnecessary checks here. --- web/pdf_rendering_queue.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/web/pdf_rendering_queue.js b/web/pdf_rendering_queue.js index d08a2e80f..3444a1bec 100644 --- a/web/pdf_rendering_queue.js +++ b/web/pdf_rendering_queue.js @@ -132,7 +132,7 @@ class PDFRenderingQueue { // All the visible views have rendered; try to handle any "holes" in the // page layout (can happen e.g. with spreadModes at higher zoom levels). if (lastId - firstId > 1) { - for (let i = 0, ii = lastId - firstId; i <= ii; i++) { + for (let i = 1, ii = lastId - firstId; i < ii; i++) { const holeId = scrolledDown ? firstId + i : lastId - i, holeView = views[holeId - 1]; if (!this.isViewFinished(holeView)) { From 3dc738c4c8681840fb7f16ecf2c405237e4c1949 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Sat, 23 Oct 2021 19:59:14 +0200 Subject: [PATCH 3/3] Slightly simplify the `isThumbnailViewEnabled` check in `PDFRenderingQueue.renderHighestPriority` --- web/pdf_rendering_queue.js | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/web/pdf_rendering_queue.js b/web/pdf_rendering_queue.js index 3444a1bec..06f7016de 100644 --- a/web/pdf_rendering_queue.js +++ b/web/pdf_rendering_queue.js @@ -82,10 +82,11 @@ class PDFRenderingQueue { return; } // No pages needed rendering, so check thumbnails. - if (this.pdfThumbnailViewer && this.isThumbnailViewEnabled) { - if (this.pdfThumbnailViewer.forceRendering()) { - return; - } + if ( + this.isThumbnailViewEnabled && + this.pdfThumbnailViewer?.forceRendering() + ) { + return; } if (this.printing) {