From 3446f15bf374e761a42695f30e5cc9e93a00dcc1 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Sun, 24 Jul 2022 13:14:58 +0200 Subject: [PATCH] Improve how we disable `PDFThumbnailView.setImage` for documents with Optional Content (PR 12170 follow-up) Rather than always disable `PDFThumbnailView.setImage` as soon as user has changed the visibility of the Optional Content, we can utilize the new method added in the previous patch to improve thumbnail performance. Note in particular how, in the old code, even *resetting* of the Optional Content to its default state wouldn't enable `PDFThumbnailView.setImage` again. While slightly unrelated, this patch also removes the `PDFThumbnailViewer._optionalContentConfigPromise`-property since it's completely unused. --- web/base_viewer.js | 1 + web/pdf_page_view.js | 36 ++++++++++++++++++++++++++++++++++++ web/pdf_thumbnail_view.js | 12 +----------- web/pdf_thumbnail_viewer.js | 14 -------------- 4 files changed, 38 insertions(+), 25 deletions(-) diff --git a/web/base_viewer.js b/web/base_viewer.js index 0ecb510ab..2566dba31 100644 --- a/web/base_viewer.js +++ b/web/base_viewer.js @@ -1824,6 +1824,7 @@ class BaseViewer { return Promise.resolve(null); } if (!this._optionalContentConfigPromise) { + console.error("optionalContentConfigPromise: Not initialized yet."); // Prevent issues if the getter is accessed *before* the `onePageRendered` // promise has resolved; won't (normally) happen in the default viewer. return this.pdfDocument.getOptionalContentConfig(); diff --git a/web/pdf_page_view.js b/web/pdf_page_view.js index bc77234fc..2beca556b 100644 --- a/web/pdf_page_view.js +++ b/web/pdf_page_view.js @@ -99,6 +99,8 @@ const MAX_CANVAS_PIXELS = compatibilityParams.maxCanvasPixels || 16777216; class PDFPageView { #annotationMode = AnnotationMode.ENABLE_FORMS; + #useThumbnailCanvas = true; + /** * @param {PDFPageViewOptions} options */ @@ -174,6 +176,22 @@ class PDFPageView { this.div = div; container?.append(div); + + if (this._isStandalone) { + const { optionalContentConfigPromise } = options; + if (optionalContentConfigPromise) { + // Ensure that the thumbnails always display the *initial* document + // state. + optionalContentConfigPromise.then(optionalContentConfig => { + if ( + optionalContentConfigPromise !== this._optionalContentConfigPromise + ) { + return; + } + this.#useThumbnailCanvas = optionalContentConfig.hasInitialVisibility; + }); + } + } } setPdfPage(pdfPage) { @@ -376,6 +394,16 @@ class PDFPageView { } if (optionalContentConfigPromise instanceof Promise) { this._optionalContentConfigPromise = optionalContentConfigPromise; + + // Ensure that the thumbnails always display the *initial* document state. + optionalContentConfigPromise.then(optionalContentConfig => { + if ( + optionalContentConfigPromise !== this._optionalContentConfigPromise + ) { + return; + } + this.#useThumbnailCanvas = optionalContentConfig.hasInitialVisibility; + }); } const totalRotation = (this.rotation + this.pdfPageRotate) % 360; @@ -1003,6 +1031,14 @@ class PDFPageView { this.div.removeAttribute("data-page-label"); } } + + /** + * For use by the `PDFThumbnailView.setImage`-method. + * @ignore + */ + get thumbnailCanvas() { + return this.#useThumbnailCanvas ? this.canvas : null; + } } export { PDFPageView }; diff --git a/web/pdf_thumbnail_view.js b/web/pdf_thumbnail_view.js index e45c61f7d..2cf3f6c5c 100644 --- a/web/pdf_thumbnail_view.js +++ b/web/pdf_thumbnail_view.js @@ -37,7 +37,6 @@ const THUMBNAIL_WIDTH = 98; // px * The default value is `null`. * @property {IPDFLinkService} linkService - The navigation/linking service. * @property {PDFRenderingQueue} renderingQueue - The rendering queue object. - * @property {function} checkSetImageDisabled * @property {IL10n} l10n - Localization service. * @property {Object} [pageColors] - Overwrites background and foreground colors * with user defined ones in order to improve readability in high contrast @@ -88,7 +87,6 @@ class PDFThumbnailView { optionalContentConfigPromise, linkService, renderingQueue, - checkSetImageDisabled, l10n, pageColors, }) { @@ -109,11 +107,6 @@ class PDFThumbnailView { this.renderTask = null; this.renderingState = RenderingStates.INITIAL; this.resume = null; - this._checkSetImageDisabled = - checkSetImageDisabled || - function () { - return false; - }; const pageWidth = this.viewport.width, pageHeight = this.viewport.height, @@ -356,13 +349,10 @@ class PDFThumbnailView { } setImage(pageView) { - if (this._checkSetImageDisabled()) { - return; - } if (this.renderingState !== RenderingStates.INITIAL) { return; } - const { canvas, pdfPage } = pageView; + const { thumbnailCanvas: canvas, pdfPage } = pageView; if (!canvas) { return; } diff --git a/web/pdf_thumbnail_viewer.js b/web/pdf_thumbnail_viewer.js index 582448253..d54cb043c 100644 --- a/web/pdf_thumbnail_viewer.js +++ b/web/pdf_thumbnail_viewer.js @@ -85,12 +85,6 @@ class PDFThumbnailViewer { this.scroll = watchScroll(this.container, this._scrollUpdated.bind(this)); this._resetView(); - - eventBus._on("optionalcontentconfigchanged", () => { - // Ensure that the thumbnails always render with the *default* optional - // content configuration. - this._setImageDisabled = true; - }); } /** @@ -195,8 +189,6 @@ class PDFThumbnailViewer { this._currentPageNumber = 1; this._pageLabels = null; this._pagesRotation = 0; - this._optionalContentConfigPromise = null; - this._setImageDisabled = false; // Remove the thumbnails from the DOM. this.container.textContent = ""; @@ -220,13 +212,8 @@ class PDFThumbnailViewer { firstPagePromise .then(firstPdfPage => { - this._optionalContentConfigPromise = optionalContentConfigPromise; - const pagesCount = pdfDocument.numPages; const viewport = firstPdfPage.getViewport({ scale: 1 }); - const checkSetImageDisabled = () => { - return this._setImageDisabled; - }; for (let pageNum = 1; pageNum <= pagesCount; ++pageNum) { const thumbnail = new PDFThumbnailView({ @@ -236,7 +223,6 @@ class PDFThumbnailViewer { optionalContentConfigPromise, linkService: this.linkService, renderingQueue: this.renderingQueue, - checkSetImageDisabled, l10n: this.l10n, pageColors: this.pageColors, });