From b7eef925acd69bd8b60cd04b9ba6aca5f581bc39 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Sat, 15 Mar 2025 13:19:34 +0100 Subject: [PATCH 1/3] Shorten the `PDFThumbnailView.prototype.#getReducedImageDims` method (PR 19635 follow-up) This method is slightly more verbose than necessary, hence we can shorten the code a little bit. --- web/pdf_thumbnail_view.js | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/web/pdf_thumbnail_view.js b/web/pdf_thumbnail_view.js index 59e171bb3..c512aca5f 100644 --- a/web/pdf_thumbnail_view.js +++ b/web/pdf_thumbnail_view.js @@ -366,8 +366,8 @@ class PDFThumbnailView { } #getReducedImageDims(canvas) { - let reducedWidth = canvas.width << MAX_NUM_SCALING_STEPS, - reducedHeight = canvas.height << MAX_NUM_SCALING_STEPS; + const width = canvas.width << MAX_NUM_SCALING_STEPS, + height = canvas.height << MAX_NUM_SCALING_STEPS; const outputScale = new OutputScale(); // Here we're not actually "rendering" to the canvas and the `OutputScale` @@ -375,15 +375,12 @@ class PDFThumbnailView { outputScale.sx = outputScale.sy = 1; outputScale.limitCanvas( - reducedWidth, - reducedHeight, + width, + height, this.maxCanvasPixels, this.maxCanvasDim ); - reducedWidth = (reducedWidth * outputScale.sx) | 0; - reducedHeight = (reducedHeight * outputScale.sy) | 0; - - return [reducedWidth, reducedHeight]; + return [(width * outputScale.sx) | 0, (height * outputScale.sy) | 0]; } #reduceImage(img) { From 7ee061bcf117aae8730b59d72cd5d99af920bea9 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Sat, 15 Mar 2025 13:24:52 +0100 Subject: [PATCH 2/3] Add a helper function, in `web/pdf_thumbnail_view.js`, for "zeroing" a canvas This removes a tiny bit of code duplication. --- web/pdf_thumbnail_view.js | 25 +++++++++++-------------- 1 file changed, 11 insertions(+), 14 deletions(-) diff --git a/web/pdf_thumbnail_view.js b/web/pdf_thumbnail_view.js index c512aca5f..f73774a9f 100644 --- a/web/pdf_thumbnail_view.js +++ b/web/pdf_thumbnail_view.js @@ -31,6 +31,13 @@ const DRAW_UPSCALE_FACTOR = 2; // See comment in `PDFThumbnailView.draw` below. const MAX_NUM_SCALING_STEPS = 3; const THUMBNAIL_WIDTH = 98; // px +function zeroCanvas(c) { + // Zeroing the width and height causes Firefox to release graphics + // resources immediately, which can greatly reduce memory consumption. + c.width = 0; + c.height = 0; +} + /** * @typedef {Object} PDFThumbnailViewOptions * @property {HTMLDivElement} container - The viewer element. @@ -74,12 +81,8 @@ class TempImageFactory { } static destroyCanvas() { - const tempCanvas = this.#tempCanvas; - if (tempCanvas) { - // Zeroing the width and height causes Firefox to release graphics - // resources immediately, which can greatly reduce memory consumption. - tempCanvas.width = 0; - tempCanvas.height = 0; + if (this.#tempCanvas) { + zeroCanvas(this.#tempCanvas); } this.#tempCanvas = null; } @@ -255,10 +258,7 @@ class PDFThumbnailView { this.div.setAttribute("data-loaded", true); this._placeholderImg.replaceWith(image); - // Zeroing the width and height causes Firefox to release graphics - // resources immediately, which can greatly reduce memory consumption. - reducedCanvas.width = 0; - reducedCanvas.height = 0; + zeroCanvas(reducedCanvas); } async #finishRenderTask(renderTask, canvas, error = null) { @@ -331,10 +331,7 @@ class PDFThumbnailView { error => this.#finishRenderTask(renderTask, canvas, error) ); resultPromise.finally(() => { - // Zeroing the width and height causes Firefox to release graphics - // resources immediately, which can greatly reduce memory consumption. - canvas.width = 0; - canvas.height = 0; + zeroCanvas(canvas); this.eventBus.dispatch("thumbnailrendered", { source: this, From 6548c9f1f61dd4f85e9081409de2db279dd7ce68 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Sat, 15 Mar 2025 13:57:15 +0100 Subject: [PATCH 3/3] Inline the `PDFThumbnailView.prototype.#finishRenderTask` helper method Given that the `draw` method is already asynchronous we can easily inline this old helper method, which shortens the code and improves consistency in the code-base (note the `BasePDFPageView`-implementation). --- web/pdf_thumbnail_view.js | 65 +++++++++++++++++++-------------------- 1 file changed, 31 insertions(+), 34 deletions(-) diff --git a/web/pdf_thumbnail_view.js b/web/pdf_thumbnail_view.js index f73774a9f..0115c5756 100644 --- a/web/pdf_thumbnail_view.js +++ b/web/pdf_thumbnail_view.js @@ -261,31 +261,12 @@ class PDFThumbnailView { zeroCanvas(reducedCanvas); } - async #finishRenderTask(renderTask, canvas, error = null) { - // The renderTask may have been replaced by a new one, so only remove - // the reference to the renderTask if it matches the one that is - // triggering this callback. - if (renderTask === this.renderTask) { - this.renderTask = null; - } - - if (error instanceof RenderingCancelledException) { - return; - } - this.renderingState = RenderingStates.FINISHED; - this.#convertCanvasToImage(canvas); - - if (error) { - throw error; - } - } - async draw() { if (this.renderingState !== RenderingStates.INITIAL) { console.error("Must be in new state before drawing"); - return undefined; + return; } - const { pdfPage } = this; + const { pageColors, pdfPage } = this; if (!pdfPage) { this.renderingState = RenderingStates.FINISHED; @@ -321,26 +302,42 @@ class PDFThumbnailView { transform, viewport: drawViewport, optionalContentConfigPromise: this._optionalContentConfigPromise, - pageColors: this.pageColors, + pageColors, }; const renderTask = (this.renderTask = pdfPage.render(renderContext)); renderTask.onContinue = renderContinueCallback; - const resultPromise = renderTask.promise.then( - () => this.#finishRenderTask(renderTask, canvas), - error => this.#finishRenderTask(renderTask, canvas, error) - ); - resultPromise.finally(() => { - zeroCanvas(canvas); + let error = null; + try { + await renderTask.promise; + } catch (e) { + if (e instanceof RenderingCancelledException) { + zeroCanvas(canvas); + return; + } + error = e; + } finally { + // The renderTask may have been replaced by a new one, so only remove + // the reference to the renderTask if it matches the one that is + // triggering this callback. + if (renderTask === this.renderTask) { + this.renderTask = null; + } + } + this.renderingState = RenderingStates.FINISHED; - this.eventBus.dispatch("thumbnailrendered", { - source: this, - pageNumber: this.id, - pdfPage: this.pdfPage, - }); + this.#convertCanvasToImage(canvas); + zeroCanvas(canvas); + + this.eventBus.dispatch("thumbnailrendered", { + source: this, + pageNumber: this.id, + pdfPage, }); - return resultPromise; + if (error) { + throw error; + } } setImage(pageView) {