From 33dfc4dd5fc263bc9e4d5d73261a32c61d529853 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Sat, 1 Mar 2025 15:28:11 +0100 Subject: [PATCH 1/2] Also cache "sub" ColorSpaces globally (PR 19583 follow-up) Some ColorSpaces can reference other ColorSpaces, and since it's fairly common for many pages to use the same ColorSpaces (see e.g. `issue17061.pdf`) this can help reduce a lot of unnecessary re-parsing especially for e.g. `ICCBased` ColorSpaces. --- src/core/colorspace.js | 97 +++++++++++++++++++++++++++++++++++++----- 1 file changed, 87 insertions(+), 10 deletions(-) diff --git a/src/core/colorspace.js b/src/core/colorspace.js index 6bd879b619ce4..0c47ec1cf87b8 100644 --- a/src/core/colorspace.js +++ b/src/core/colorspace.js @@ -389,7 +389,13 @@ class ColorSpace { "before calling `ColorSpace.parseAsync`." ); } - const parsedCS = this.#parse(cs, xref, resources, pdfFunctionFactory); + const parsedCS = this.#parse( + cs, + xref, + resources, + pdfFunctionFactory, + globalColorSpaceCache + ); // Attempt to cache the parsed ColorSpace, by name and/or reference. this.#cache( @@ -420,7 +426,13 @@ class ColorSpace { if (cachedCS) { return cachedCS; } - const parsedCS = this.#parse(cs, xref, resources, pdfFunctionFactory); + const parsedCS = this.#parse( + cs, + xref, + resources, + pdfFunctionFactory, + globalColorSpaceCache + ); // Attempt to cache the parsed ColorSpace, by name and/or reference. this.#cache( @@ -434,7 +446,47 @@ class ColorSpace { return parsedCS; } - static #parse(cs, xref, resources = null, pdfFunctionFactory) { + /** + * NOTE: This method should *only* be invoked from `this.#parse`, + * when parsing "sub" ColorSpaces. + */ + static #subParse( + cs, + xref, + resources, + pdfFunctionFactory, + globalColorSpaceCache + ) { + let csRef; + if (cs instanceof Ref) { + const cachedCS = globalColorSpaceCache.getByRef(cs); + if (cachedCS) { + return cachedCS; + } + csRef = cs; + } + const parsedCS = this.#parse( + cs, + xref, + resources, + pdfFunctionFactory, + globalColorSpaceCache + ); + + // Only cache the parsed ColorSpace globally, by reference. + if (csRef) { + globalColorSpaceCache.set(/* name = */ null, csRef, parsedCS); + } + return parsedCS; + } + + static #parse( + cs, + xref, + resources = null, + pdfFunctionFactory, + globalColorSpaceCache + ) { cs = xref.fetchIfRef(cs); if (cs instanceof Name) { switch (cs.name) { @@ -462,7 +514,8 @@ class ColorSpace { resourcesCS, xref, resources, - pdfFunctionFactory + pdfFunctionFactory, + globalColorSpaceCache ); } cs = resourcesCS; @@ -506,9 +559,15 @@ class ColorSpace { const stream = xref.fetchIfRef(cs[1]); const dict = stream.dict; numComps = dict.get("N"); - const alt = dict.get("Alternate"); - if (alt) { - const altCS = this.#parse(alt, xref, resources, pdfFunctionFactory); + const altRaw = dict.getRaw("Alternate"); + if (altRaw) { + const altCS = this.#subParse( + altRaw, + xref, + resources, + pdfFunctionFactory, + globalColorSpaceCache + ); // Ensure that the number of components are correct, // and also (indirectly) that it is not a PatternCS. if (altCS.numComps === numComps) { @@ -527,12 +586,24 @@ class ColorSpace { case "Pattern": baseCS = cs[1] || null; if (baseCS) { - baseCS = this.#parse(baseCS, xref, resources, pdfFunctionFactory); + baseCS = this.#subParse( + baseCS, + xref, + resources, + pdfFunctionFactory, + globalColorSpaceCache + ); } return new PatternCS(baseCS); case "I": case "Indexed": - baseCS = this.#parse(cs[1], xref, resources, pdfFunctionFactory); + baseCS = this.#subParse( + cs[1], + xref, + resources, + pdfFunctionFactory, + globalColorSpaceCache + ); const hiVal = Math.max(0, Math.min(xref.fetchIfRef(cs[2]), 255)); const lookup = xref.fetchIfRef(cs[3]); return new IndexedCS(baseCS, hiVal, lookup); @@ -540,7 +611,13 @@ class ColorSpace { case "DeviceN": const name = xref.fetchIfRef(cs[1]); numComps = Array.isArray(name) ? name.length : 1; - baseCS = this.#parse(cs[2], xref, resources, pdfFunctionFactory); + baseCS = this.#subParse( + cs[2], + xref, + resources, + pdfFunctionFactory, + globalColorSpaceCache + ); const tintFn = pdfFunctionFactory.create(cs[3]); return new AlternateCS(numComps, baseCS, tintFn); case "Lab": From ba640e5cdc24a4b4b819e6a97eeca3e3cd47b576 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Mon, 3 Mar 2025 11:08:05 +0100 Subject: [PATCH 2/2] Shorten various static `ColorSpace` method signatures By passing around common parameters, i.e. everything that's not the ColorSpace-data, in an object we can shorten this code a little bit. --- src/core/colorspace.js | 110 +++++++++++------------------------------ 1 file changed, 28 insertions(+), 82 deletions(-) diff --git a/src/core/colorspace.js b/src/core/colorspace.js index 0c47ec1cf87b8..496dcdd0030c0 100644 --- a/src/core/colorspace.js +++ b/src/core/colorspace.js @@ -308,10 +308,8 @@ class ColorSpace { static #cache( cacheKey, - xref, - globalColorSpaceCache, - localColorSpaceCache, - parsedCS + parsedCS, + { xref, globalColorSpaceCache, localColorSpaceCache } ) { if (!globalColorSpaceCache || !localColorSpaceCache) { throw new Error( @@ -389,22 +387,18 @@ class ColorSpace { "before calling `ColorSpace.parseAsync`." ); } - const parsedCS = this.#parse( - cs, + + const options = { xref, resources, pdfFunctionFactory, - globalColorSpaceCache - ); - - // Attempt to cache the parsed ColorSpace, by name and/or reference. - this.#cache( - cs, - xref, globalColorSpaceCache, localColorSpaceCache, - parsedCS - ); + }; + const parsedCS = this.#parse(cs, options); + + // Attempt to cache the parsed ColorSpace, by name and/or reference. + this.#cache(cs, parsedCS, options); return parsedCS; } @@ -426,22 +420,18 @@ class ColorSpace { if (cachedCS) { return cachedCS; } - const parsedCS = this.#parse( - cs, + + const options = { xref, resources, pdfFunctionFactory, - globalColorSpaceCache - ); - - // Attempt to cache the parsed ColorSpace, by name and/or reference. - this.#cache( - cs, - xref, globalColorSpaceCache, localColorSpaceCache, - parsedCS - ); + }; + const parsedCS = this.#parse(cs, options); + + // Attempt to cache the parsed ColorSpace, by name and/or reference. + this.#cache(cs, parsedCS, options); return parsedCS; } @@ -450,13 +440,9 @@ class ColorSpace { * NOTE: This method should *only* be invoked from `this.#parse`, * when parsing "sub" ColorSpaces. */ - static #subParse( - cs, - xref, - resources, - pdfFunctionFactory, - globalColorSpaceCache - ) { + static #subParse(cs, options) { + const { globalColorSpaceCache } = options; + let csRef; if (cs instanceof Ref) { const cachedCS = globalColorSpaceCache.getByRef(cs); @@ -465,13 +451,7 @@ class ColorSpace { } csRef = cs; } - const parsedCS = this.#parse( - cs, - xref, - resources, - pdfFunctionFactory, - globalColorSpaceCache - ); + const parsedCS = this.#parse(cs, options); // Only cache the parsed ColorSpace globally, by reference. if (csRef) { @@ -480,13 +460,9 @@ class ColorSpace { return parsedCS; } - static #parse( - cs, - xref, - resources = null, - pdfFunctionFactory, - globalColorSpaceCache - ) { + static #parse(cs, options) { + const { xref, resources, pdfFunctionFactory } = options; + cs = xref.fetchIfRef(cs); if (cs instanceof Name) { switch (cs.name) { @@ -510,13 +486,7 @@ class ColorSpace { const resourcesCS = colorSpaces.get(cs.name); if (resourcesCS) { if (resourcesCS instanceof Name) { - return this.#parse( - resourcesCS, - xref, - resources, - pdfFunctionFactory, - globalColorSpaceCache - ); + return this.#parse(resourcesCS, options); } cs = resourcesCS; break; @@ -561,13 +531,7 @@ class ColorSpace { numComps = dict.get("N"); const altRaw = dict.getRaw("Alternate"); if (altRaw) { - const altCS = this.#subParse( - altRaw, - xref, - resources, - pdfFunctionFactory, - globalColorSpaceCache - ); + const altCS = this.#subParse(altRaw, options); // Ensure that the number of components are correct, // and also (indirectly) that it is not a PatternCS. if (altCS.numComps === numComps) { @@ -586,24 +550,12 @@ class ColorSpace { case "Pattern": baseCS = cs[1] || null; if (baseCS) { - baseCS = this.#subParse( - baseCS, - xref, - resources, - pdfFunctionFactory, - globalColorSpaceCache - ); + baseCS = this.#subParse(baseCS, options); } return new PatternCS(baseCS); case "I": case "Indexed": - baseCS = this.#subParse( - cs[1], - xref, - resources, - pdfFunctionFactory, - globalColorSpaceCache - ); + baseCS = this.#subParse(cs[1], options); const hiVal = Math.max(0, Math.min(xref.fetchIfRef(cs[2]), 255)); const lookup = xref.fetchIfRef(cs[3]); return new IndexedCS(baseCS, hiVal, lookup); @@ -611,13 +563,7 @@ class ColorSpace { case "DeviceN": const name = xref.fetchIfRef(cs[1]); numComps = Array.isArray(name) ? name.length : 1; - baseCS = this.#subParse( - cs[2], - xref, - resources, - pdfFunctionFactory, - globalColorSpaceCache - ); + baseCS = this.#subParse(cs[2], options); const tintFn = pdfFunctionFactory.create(cs[3]); return new AlternateCS(numComps, baseCS, tintFn); case "Lab":