Skip to content

Commit

Permalink
Extend the globally cached image main-thread copying to "complex" ima…
Browse files Browse the repository at this point in the history
…ges 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.
  • Loading branch information
Snuffleupagus committed Apr 20, 2024
1 parent 5ad42c1 commit 93dfeb1
Show file tree
Hide file tree
Showing 4 changed files with 69 additions and 6 deletions.
15 changes: 9 additions & 6 deletions src/core/evaluator.js
Original file line number Diff line number Diff line change
Expand Up @@ -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({
Expand Down Expand Up @@ -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",
Expand Down
1 change: 1 addition & 0 deletions test/pdfs/issue11518.pdf.link
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
https://github.com/mozilla/pdf.js/files/4076920/set-2_lyst7242.pdf
7 changes: 7 additions & 0 deletions test/test_manifest.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
52 changes: 52 additions & 0 deletions test/unit/api_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -4001,6 +4001,58 @@ Caron Broadcasting, Inc., an Ohio corporation (“Lessee”).`)
firstStatsOverall = null;
});

it("caches image resources at the document/page, 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);
}
}
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(
Expand Down

0 comments on commit 93dfeb1

Please sign in to comment.