From bc8523ac2947d0bacc6b13a6105a037b8d9c5dcd Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Thu, 25 May 2023 14:29:30 +0200 Subject: [PATCH] Introduce a "thumbnailrendered" event to simplify cleanup after thumbnail rendering (PR 12613 follow-up) The way that the cleanup was implemented in PR 12613 has always bothered me slightly, since the `isPageCached`-method that I introduced there always felt quite out-of-place in the `IPDFLinkService`-implementations. By introducing a new "thumbnailrendered" event, similar to the existing "pagerendered" one, we're able to move the cleanup handling into the `PDFViewer`-class instead. --- web/app.js | 1 + web/interfaces.js | 5 ----- web/pdf_link_service.js | 14 -------------- web/pdf_thumbnail_view.js | 15 +++++++++------ web/pdf_thumbnail_viewer.js | 13 ++++++++++++- web/pdf_viewer.js | 30 +++++++++--------------------- 6 files changed, 31 insertions(+), 47 deletions(-) diff --git a/web/app.js b/web/app.js index 26714e5167a32..0acf47fd5dfbf 100644 --- a/web/app.js +++ b/web/app.js @@ -533,6 +533,7 @@ const PDFViewerApplication = { if (appConfig.sidebar?.thumbnailView) { this.pdfThumbnailViewer = new PDFThumbnailViewer({ container: appConfig.sidebar.thumbnailView, + eventBus, renderingQueue: pdfRenderingQueue, linkService: pdfLinkService, l10n: this.l10n, diff --git a/web/interfaces.js b/web/interfaces.js index 24ad4b64763fd..abf0390403765 100644 --- a/web/interfaces.js +++ b/web/interfaces.js @@ -112,11 +112,6 @@ class IPDFLinkService { * @param {Object} pageRef - reference to the page. */ cachePageRef(pageNum, pageRef) {} - - /** - * @param {number} pageNumber - */ - isPageCached(pageNumber) {} } /** diff --git a/web/pdf_link_service.js b/web/pdf_link_service.js index 8c2740a123361..340a071e8c7cd 100644 --- a/web/pdf_link_service.js +++ b/web/pdf_link_service.js @@ -569,13 +569,6 @@ class PDFLinkService { return this.#pagesRefCache.get(refStr) || null; } - /** - * @param {number} pageNumber - */ - isPageCached(pageNumber) { - return this.pdfViewer.isPageCached(pageNumber); - } - static #isValidExplicitDestination(dest) { if (!Array.isArray(dest)) { return false; @@ -737,13 +730,6 @@ class SimpleLinkService { * @param {Object} pageRef - reference to the page. */ cachePageRef(pageNum, pageRef) {} - - /** - * @param {number} pageNumber - */ - isPageCached(pageNumber) { - return true; - } } export { LinkTarget, PDFLinkService, SimpleLinkService }; diff --git a/web/pdf_thumbnail_view.js b/web/pdf_thumbnail_view.js index e44802df1abca..9e2d7017b745a 100644 --- a/web/pdf_thumbnail_view.js +++ b/web/pdf_thumbnail_view.js @@ -13,6 +13,7 @@ * limitations under the License. */ +/** @typedef {import("./event_utils").EventBus} EventBus */ /** @typedef {import("./interfaces").IL10n} IL10n */ /** @typedef {import("./interfaces").IPDFLinkService} IPDFLinkService */ /** @typedef {import("./interfaces").IRenderableView} IRenderableView */ @@ -29,6 +30,7 @@ const THUMBNAIL_WIDTH = 98; // px /** * @typedef {Object} PDFThumbnailViewOptions * @property {HTMLDivElement} container - The viewer element. + * @property {EventBus} eventBus - The application event bus. * @property {number} id - The thumbnail's unique ID (normally its number). * @property {PageViewport} defaultViewport - The page viewport. * @property {Promise} [optionalContentConfigPromise] - @@ -81,6 +83,7 @@ class PDFThumbnailView { */ constructor({ container, + eventBus, id, defaultViewport, optionalContentConfigPromise, @@ -100,6 +103,7 @@ class PDFThumbnailView { this._optionalContentConfigPromise = optionalContentConfigPromise || null; this.pageColors = pageColors || null; + this.eventBus = eventBus; this.linkService = linkService; this.renderingQueue = renderingQueue; @@ -314,12 +318,11 @@ class PDFThumbnailView { canvas.width = 0; canvas.height = 0; - // Only trigger cleanup, once rendering has finished, when the current - // pageView is *not* cached on the `BaseViewer`-instance. - const pageCached = this.linkService.isPageCached(this.id); - if (!pageCached) { - this.pdfPage?.cleanup(); - } + this.eventBus.dispatch("thumbnailrendered", { + source: this, + pageNumber: this.id, + pdfPage: this.pdfPage, + }); }); return resultPromise; diff --git a/web/pdf_thumbnail_viewer.js b/web/pdf_thumbnail_viewer.js index 10f757ad854bc..92be8bbe63401 100644 --- a/web/pdf_thumbnail_viewer.js +++ b/web/pdf_thumbnail_viewer.js @@ -14,6 +14,7 @@ */ /** @typedef {import("../src/display/api").PDFDocumentProxy} PDFDocumentProxy */ +/** @typedef {import("./event_utils").EventBus} EventBus */ /** @typedef {import("./interfaces").IL10n} IL10n */ /** @typedef {import("./interfaces").IPDFLinkService} IPDFLinkService */ // eslint-disable-next-line max-len @@ -35,6 +36,7 @@ const THUMBNAIL_SELECTED_CLASS = "selected"; * @typedef {Object} PDFThumbnailViewerOptions * @property {HTMLDivElement} container - The container for the thumbnail * elements. + * @property {EventBus} eventBus - The application event bus. * @property {IPDFLinkService} linkService - The navigation/linking service. * @property {PDFRenderingQueue} renderingQueue - The rendering queue object. * @property {IL10n} l10n - Localization service. @@ -50,8 +52,16 @@ class PDFThumbnailViewer { /** * @param {PDFThumbnailViewerOptions} options */ - constructor({ container, linkService, renderingQueue, l10n, pageColors }) { + constructor({ + container, + eventBus, + linkService, + renderingQueue, + l10n, + pageColors, + }) { this.container = container; + this.eventBus = eventBus; this.linkService = linkService; this.renderingQueue = renderingQueue; this.l10n = l10n; @@ -209,6 +219,7 @@ class PDFThumbnailViewer { for (let pageNum = 1; pageNum <= pagesCount; ++pageNum) { const thumbnail = new PDFThumbnailView({ container: this.container, + eventBus: this.eventBus, id: pageNum, defaultViewport: viewport.clone(), optionalContentConfigPromise, diff --git a/web/pdf_viewer.js b/web/pdf_viewer.js index 499d5b5da80b8..63cc21f724f8b 100644 --- a/web/pdf_viewer.js +++ b/web/pdf_viewer.js @@ -322,6 +322,15 @@ class PDFViewer { } this.#updateContainerHeightCss(); + + // Trigger API-cleanup, once thumbnail rendering has finished, + // if the relevant pageView is *not* cached in the buffer. + this.eventBus._on("thumbnailrendered", ({ pageNumber, pdfPage }) => { + const pageView = this._pages[pageNumber - 1]; + if (!this.#buffer.has(pageView)) { + pdfPage?.cleanup(); + } + }); } get pagesCount() { @@ -1645,27 +1654,6 @@ class PDFViewer { }); } - /** - * @param {number} pageNumber - */ - isPageCached(pageNumber) { - if (!this.pdfDocument) { - return false; - } - if ( - !( - Number.isInteger(pageNumber) && - pageNumber > 0 && - pageNumber <= this.pagesCount - ) - ) { - console.error(`isPageCached: "${pageNumber}" is not a valid page.`); - return false; - } - const pageView = this._pages[pageNumber - 1]; - return this.#buffer.has(pageView); - } - cleanup() { for (const pageView of this._pages) { if (pageView.renderingState !== RenderingStates.FINISHED) {