Merge pull request #20462 from calixteman/improve_thumbnails

Slightly reduce the memory used by thumbnails

Author: timvandermeij

test/integration/thumbnail_view_spec.mjs

@@ -21,13 +21,13 @@ describe("PDF Thumbnail View", () => {
           await page.waitForSelector(thumbSelector, { visible: true });
 
           await page.waitForSelector(
-            '#thumbnailView .thumbnail[data-loaded="true"]'
+            "#thumbnailView .thumbnail:not(.missingThumbnailImage)"
           );
 
           const src = await page.$eval(thumbSelector, el => el.src);
           expect(src)
             .withContext(`In ${browserName}`)
-            .toMatch(/^data:image\//);
+            .toMatch(/^blob:http:/);
         })
       );
     });

web/pdf_thumbnail_view.js

@@ -23,21 +23,18 @@
 // eslint-disable-next-line max-len
 /** @typedef {import("./pdf_rendering_queue").PDFRenderingQueue} PDFRenderingQueue */
 
-import { OutputScale, RenderingCancelledException } from "pdfjs-lib";
+import {
+  FeatureTest,
+  OutputScale,
+  RenderingCancelledException,
+} from "pdfjs-lib";
 import { AppOptions } from "./app_options.js";
 import { RenderingStates } from "./ui_utils.js";
 
 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.
@@ -61,12 +58,15 @@ function zeroCanvas(c) {
  */
 
 class TempImageFactory {
-  static #tempCanvas = null;
-
   static getCanvas(width, height) {
-    const tempCanvas = (this.#tempCanvas ||= document.createElement("canvas"));
-    tempCanvas.width = width;
-    tempCanvas.height = height;
+    let tempCanvas;
+    if (FeatureTest.isOffscreenCanvasSupported) {
+      tempCanvas = new OffscreenCanvas(width, height);
+    } else {
+      tempCanvas = document.createElement("canvas");
+      tempCanvas.width = width;
+      tempCanvas.height = height;
+    }
 
     // Since this is a temporary canvas, we need to fill it with a white
     // background ourselves. `#getPageDrawContext` uses CSS rules for this.
@@ -75,14 +75,7 @@ class TempImageFactory {
     ctx.fillStyle = "rgb(255, 255, 255)";
     ctx.fillRect(0, 0, width, height);
     ctx.restore();
-    return [tempCanvas, tempCanvas.getContext("2d")];
-  }
-
-  static destroyCanvas() {
-    if (this.#tempCanvas) {
-      zeroCanvas(this.#tempCanvas);
-    }
-    this.#tempCanvas = null;
+    return [tempCanvas, ctx];
   }
 }
 
@@ -126,27 +119,24 @@ class PDFThumbnailView {
     this.renderingState = RenderingStates.INITIAL;
     this.resume = null;
 
-    const anchor = document.createElement("a");
-    anchor.href = linkService.getAnchorUrl("#page=" + id);
+    const anchor = (this.anchor = document.createElement("a"));
+    anchor.href = linkService.getAnchorUrl(`#page=${id}`);
     anchor.setAttribute("data-l10n-id", "pdfjs-thumb-page-title");
     anchor.setAttribute("data-l10n-args", this.#pageL10nArgs);
-    anchor.onclick = function () {
+    anchor.onclick = () => {
       linkService.goToPage(id);
       return false;
     };
-    this.anchor = anchor;
 
-    const div = document.createElement("div");
-    div.className = "thumbnail";
+    const div = (this.div = document.createElement("div"));
+    div.classList.add("thumbnail", "missingThumbnailImage");
     div.setAttribute("data-page-number", this.id);
-    this.div = div;
     this.#updateDims();
 
-    const img = document.createElement("div");
-    img.className = "thumbnailImage";
-    this._placeholderImg = img;
+    const image = (this.image = document.createElement("img"));
+    image.className = "thumbnailImage";
 
-    div.append(img);
+    div.append(image);
     anchor.append(div);
     container.append(anchor);
   }
@@ -155,13 +145,11 @@ class PDFThumbnailView {
     const { width, height } = this.viewport;
     const ratio = width / height;
 
-    this.canvasWidth = THUMBNAIL_WIDTH;
-    this.canvasHeight = (this.canvasWidth / ratio) | 0;
-    this.scale = this.canvasWidth / width;
+    const canvasWidth = (this.canvasWidth = THUMBNAIL_WIDTH);
+    const canvasHeight = (this.canvasHeight = (canvasWidth / ratio) | 0);
+    this.scale = canvasWidth / width;
 
-    const { style } = this.div;
-    style.setProperty("--thumbnail-width", `${this.canvasWidth}px`);
-    style.setProperty("--thumbnail-height", `${this.canvasHeight}px`);
+    this.div.style.height = `${canvasHeight}px`;
   }
 
   setPdfPage(pdfPage) {
@@ -175,14 +163,16 @@ class PDFThumbnailView {
   reset() {
     this.cancelRendering();
     this.renderingState = RenderingStates.INITIAL;
-
-    this.div.removeAttribute("data-loaded");
-    this.image?.replaceWith(this._placeholderImg);
     this.#updateDims();
 
-    if (this.image) {
-      this.image.removeAttribute("src");
-      delete this.image;
+    const { image } = this;
+    const url = image.src;
+    if (url) {
+      URL.revokeObjectURL(url);
+      image.removeAttribute("data-l10n-id");
+      image.removeAttribute("data-l10n-args");
+      image.src = "";
+      this.div.classList.add("missingThumbnailImage");
     }
   }
 
@@ -213,7 +203,6 @@ class PDFThumbnailView {
   #getPageDrawContext(upscaleFactor = 1) {
     // Keep the no-thumbnail outline visible, i.e. `data-loaded === false`,
     // until rendering/image conversion is complete, to avoid display issues.
-    const canvas = document.createElement("canvas");
     const outputScale = new OutputScale();
     const width = upscaleFactor * this.canvasWidth,
       height = upscaleFactor * this.canvasHeight;
@@ -224,6 +213,9 @@ class PDFThumbnailView {
       this.maxCanvasPixels,
       this.maxCanvasDim
     );
+    // Because of: https://bugzilla.mozilla.org/show_bug.cgi?id=2003060
+    // we need use a HTMLCanvasElement here.
+    const canvas = document.createElement("canvas");
     canvas.width = (width * outputScale.sx) | 0;
     canvas.height = (height * outputScale.sy) | 0;
 
@@ -234,23 +226,23 @@ class PDFThumbnailView {
     return { canvas, transform };
   }
 
-  #convertCanvasToImage(canvas) {
+  async #convertCanvasToImage(canvas) {
     if (this.renderingState !== RenderingStates.FINISHED) {
       throw new Error("#convertCanvasToImage: Rendering has not finished.");
     }
     const reducedCanvas = this.#reduceImage(canvas);
-
-    const image = document.createElement("img");
-    image.className = "thumbnailImage";
+    const { image } = this;
+    const { promise, resolve } = Promise.withResolvers();
+    reducedCanvas.toBlob(resolve);
+    const blob = await promise;
+    image.src = URL.createObjectURL(blob);
     image.setAttribute("data-l10n-id", "pdfjs-thumb-page-canvas");
     image.setAttribute("data-l10n-args", this.#pageL10nArgs);
-    image.src = reducedCanvas.toDataURL();
-    this.image = image;
-
-    this.div.setAttribute("data-loaded", true);
-    this._placeholderImg.replaceWith(image);
-
-    zeroCanvas(reducedCanvas);
+    this.div.classList.remove("missingThumbnailImage");
+    if (!FeatureTest.isOffscreenCanvasSupported) {
+      // Clean up the canvas element since it is no longer needed.
+      reducedCanvas.width = reducedCanvas.height = 0;
+    }
   }
 
   async draw() {
@@ -303,7 +295,6 @@ class PDFThumbnailView {
       await renderTask.promise;
     } catch (e) {
       if (e instanceof RenderingCancelledException) {
-        zeroCanvas(canvas);
         return;
       }
       error = e;
@@ -317,8 +308,7 @@ class PDFThumbnailView {
     }
     this.renderingState = RenderingStates.FINISHED;
 
-    this.#convertCanvasToImage(canvas);
-    zeroCanvas(canvas);
+    await this.#convertCanvasToImage(canvas);
 
     this.eventBus.dispatch("thumbnailrendered", {
       source: this,
@@ -449,14 +439,9 @@ class PDFThumbnailView {
    */
   setPageLabel(label) {
     this.pageLabel = typeof label === "string" ? label : null;
-
     this.anchor.setAttribute("data-l10n-args", this.#pageL10nArgs);
-
-    if (this.renderingState !== RenderingStates.FINISHED) {
-      return;
-    }
-    this.image?.setAttribute("data-l10n-args", this.#pageL10nArgs);
+    this.image.setAttribute("data-l10n-args", this.#pageL10nArgs);
   }
 }
 
-export { PDFThumbnailView, TempImageFactory };
+export { PDFThumbnailView };

web/pdf_thumbnail_viewer.js

@@ -27,7 +27,7 @@ import {
   scrollIntoView,
   watchScroll,
 } from "./ui_utils.js";
-import { PDFThumbnailView, TempImageFactory } from "./pdf_thumbnail_view.js";
+import { PDFThumbnailView } from "./pdf_thumbnail_view.js";
 
 const THUMBNAIL_SCROLL_MARGIN = -19;
 const THUMBNAIL_SELECTED_CLASS = "selected";
@@ -174,7 +174,6 @@ class PDFThumbnailViewer {
         thumbnail.reset();
       }
     }
-    TempImageFactory.destroyCanvas();
   }
 
   #resetView() {
@@ -209,10 +208,11 @@ class PDFThumbnailViewer {
       .then(firstPdfPage => {
         const pagesCount = pdfDocument.numPages;
         const viewport = firstPdfPage.getViewport({ scale: 1 });
+        const fragment = document.createDocumentFragment();
 
         for (let pageNum = 1; pageNum <= pagesCount; ++pageNum) {
           const thumbnail = new PDFThumbnailView({
-            container: this.container,
+            container: fragment,
             eventBus: this.eventBus,
             id: pageNum,
             defaultViewport: viewport.clone(),
@@ -234,6 +234,7 @@ class PDFThumbnailViewer {
         // Ensure that the current thumbnail is always highlighted on load.
         const thumbnailView = this._thumbnails[this._currentPageNumber - 1];
         thumbnailView.div.classList.add(THUMBNAIL_SELECTED_CLASS);
+        this.container.append(fragment);
       })
       .catch(reason => {
         console.error("Unable to initialize thumbnail viewer", reason);

web/viewer.css

@@ -717,61 +717,63 @@ body {
 }
 
 #thumbnailView {
+  --thumbnail-width: 98px;
+
+  display: flex;
+  flex-wrap: wrap;
   width: calc(100% - 60px);
   padding: 10px 30px 0;
-}
 
-#thumbnailView > a:is(:active, :focus) {
-  outline: 0;
-}
+  > a {
+    width: auto;
+    height: auto;
 
-.thumbnail {
-  /* Define these variables here, and not in :root, since the individual
-     thumbnails may have different sizes. */
-  --thumbnail-width: 0;
-  --thumbnail-height: 0;
+    > .thumbnail {
+      width: var(--thumbnail-width);
+      margin: 0 10px 5px;
+      padding: 1px;
+      border: 7px solid transparent;
+      border-radius: 2px;
 
-  float: var(--inline-start);
-  width: var(--thumbnail-width);
-  height: var(--thumbnail-height);
-  margin: 0 10px 5px;
-  padding: 1px;
-  border: 7px solid transparent;
-  border-radius: 2px;
-}
+      &.selected {
+        border-color: var(--thumbnail-selected-color) !important;
 
-#thumbnailView > a:last-of-type > .thumbnail {
-  margin-bottom: 10px;
-}
+        > .thumbnailImage {
+          opacity: 1 !important;
+        }
+      }
 
-a:focus > .thumbnail,
-.thumbnail:hover {
-  border-color: var(--thumbnail-hover-color);
-}
+      &.missingThumbnailImage {
+        border: 1px dashed rgb(132 132 132);
+        padding: 7px;
+        > .thumbnailImage {
+          display: none;
+        }
+      }
 
-.thumbnail.selected {
-  border-color: var(--thumbnail-selected-color) !important;
-}
+      > .thumbnailImage {
+        width: 100%;
+        opacity: 0.9;
+      }
+    }
 
-.thumbnailImage {
-  width: var(--thumbnail-width);
-  height: var(--thumbnail-height);
-  opacity: 0.9;
-}
+    &:is(:active, :focus) {
+      outline: 0;
+    }
 
-a:focus > .thumbnail > .thumbnailImage,
-.thumbnail:hover > .thumbnailImage {
-  opacity: 0.95;
-}
+    &:last-of-type > .thumbnail {
+      margin-bottom: 10px;
+    }
 
-.thumbnail.selected > .thumbnailImage {
-  opacity: 1 !important;
-}
+    &:focus > .thumbnail,
+    .thumbnail:hover {
+      border-color: var(--thumbnail-hover-color);
 
-.thumbnail:not([data-loaded]) > .thumbnailImage {
-  width: calc(var(--thumbnail-width) - 2px);
-  height: calc(var(--thumbnail-height) - 2px);
-  border: 1px dashed rgb(132 132 132);
+      > .thumbnailImage {
+        opacity: 0.95;
+      }
+    }
+  }
 }
 
 .treeWithDeepNesting > .treeItem,