From 91898e5923280c5997dba8622dd1368a5513c3dd Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Sat, 20 Apr 2024 09:55:18 +0200 Subject: [PATCH] Extend the globally cached image main-thread copying to "complex" images as well (PR 17428 follow-up) In PR 17428 this functionality was limited to "larger" images, to not affect performance negatively. However it turns out that it's also beneficial to consider more "complex" images, regardless of their size, that contain /SMask or /Mask data; see issue 11518. --- src/core/evaluator.js | 15 ++++++---- test/pdfs/issue11518.pdf.link | 1 + test/test_manifest.json | 7 +++++ test/unit/api_spec.js | 54 +++++++++++++++++++++++++++++++++++ 4 files changed, 71 insertions(+), 6 deletions(-) create mode 100644 test/pdfs/issue11518.pdf.link diff --git a/src/core/evaluator.js b/src/core/evaluator.js index 5132cc830..d758f363e 100644 --- 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 new file mode 100644 index 000000000..2633605cd --- /dev/null +++ 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 index ab043fba8..6de9d98e4 100644 --- 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 index 0d79c6494..5be6c949b 100644 --- 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(