Merge pull request #17978 from Snuffleupagus/pr-17428-followup

Extend the globally cached image main-thread copying to "complex" images as well (PR 17428 follow-up)
mozilla · Apr 22, 2024 · 9e80c6d · 9e80c6d
2 parents f6b0413 + 91898e5
commit 9e80c6d
Show file tree

Hide file tree

Showing 4 changed files with 71 additions and 6 deletions.
diff --git a/src/core/evaluator.js b/src/core/evaluator.js
@@ -734,9 +734,9 @@ class PartialEvaluator {
     // Inlining small images into the queue as RGB data
     if (
       isInline &&
+      w + h < SMALL_IMAGE_DIMENSIONS &&
       !dict.has("SMask") &&
-      !dict.has("Mask") &&
-      w + h < SMALL_IMAGE_DIMENSIONS
+      !dict.has("Mask")
     ) {
       try {
         const imageObj = new PDFImage({
@@ -796,10 +796,13 @@ class PartialEvaluator {
     args = [objId, w, h];
     operatorList.addImageOps(OPS.paintImageXObject, args, optionalContent);
 
-    // For large images, at least 500x500 in size, that we'll cache globally
-    // check if the image is still cached locally on the main-thread to avoid
-    // having to re-parse the image (since that can be slow).
-    if (cacheGlobally && w * h > 250000) {
+    // For large (at least 500x500) or more complex images that we'll cache
+    // globally, check if the image is still cached locally on the main-thread
+    // to avoid having to re-parse the image (since that can be slow).
+    if (
+      cacheGlobally &&
+      (w * h > 250000 || dict.has("SMask") || dict.has("Mask"))
+    ) {
       const localLength = await this.handler.sendWithPromise("commonobj", [
         objId,
         "CopyLocalImage",

diff --git a/test/pdfs/issue11518.pdf.link b/test/pdfs/issue11518.pdf.link
@@ -0,0 +1 @@
+https://github.com/mozilla/pdf.js/files/4076920/set-2_lyst7242.pdf
diff --git a/test/test_manifest.json b/test/test_manifest.json
@@ -4336,6 +4336,13 @@
     "link": true,
     "type": "other"
   },
+  {
+    "id": "issue11518",
+    "file": "pdfs/issue11518.pdf",
+    "md5": "4cced24e75c35654d47141cef348301e",
+    "link": true,
+    "type": "other"
+  },
   {
     "id": "issue4722",
     "file": "pdfs/issue4722.pdf",

diff --git a/test/unit/api_spec.js b/test/unit/api_spec.js
@@ -4001,6 +4001,60 @@ Caron Broadcasting, Inc., an Ohio corporation (“Lessee”).`)
       firstStatsOverall = null;
     });
 
+    it("caches image resources at the document/page level, with main-thread copying of complex images (issue 11518)", async function () {
+      if (isNodeJS) {
+        pending("Linked test-cases are not supported in Node.js.");
+      }
+      const { NUM_PAGES_THRESHOLD } = GlobalImageCache;
+
+      const loadingTask = getDocument(
+        buildGetDocumentParams("issue11518.pdf", {
+          pdfBug: true,
+        })
+      );
+      const pdfDoc = await loadingTask.promise;
+      let checkedCopyLocalImage = false,
+        firstStatsOverall = null;
+
+      for (let i = 1; i <= pdfDoc.numPages; i++) {
+        const pdfPage = await pdfDoc.getPage(i);
+        const viewport = pdfPage.getViewport({ scale: 1 });
+
+        const canvasAndCtx = CanvasFactory.create(
+          viewport.width,
+          viewport.height
+        );
+        const renderTask = pdfPage.render({
+          canvasContext: canvasAndCtx.context,
+          viewport,
+        });
+
+        await renderTask.promise;
+        // The canvas is no longer necessary, since we only care about
+        // the stats below.
+        CanvasFactory.destroy(canvasAndCtx);
+
+        const [statsOverall] = pdfPage.stats.times
+          .filter(time => time.name === "Overall")
+          .map(time => time.end - time.start);
+
+        if (i === 1) {
+          firstStatsOverall = statsOverall;
+        } else if (i === NUM_PAGES_THRESHOLD) {
+          checkedCopyLocalImage = true;
+          // Ensure that the images were copied in the main-thread, rather
+          // than being re-parsed in the worker-thread (which is slower).
+          expect(statsOverall).toBeLessThan(firstStatsOverall / 2);
+        } else if (i > NUM_PAGES_THRESHOLD) {
+          break;
+        }
+      }
+      expect(checkedCopyLocalImage).toBeTruthy();
+
+      await loadingTask.destroy();
+      firstStatsOverall = null;
+    });
+
     it("render for printing, with `printAnnotationStorage` set", async function () {
       async function getPrintData(printAnnotationStorage = null) {
         const canvasAndCtx = CanvasFactory.create(