From fbf1f2ba15678afb82a285aa4a0a29469fbd9c34 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Mon, 3 Mar 2025 17:43:37 +0100 Subject: [PATCH] Remove `ColorSpaceUtils.parseAsync` and simplify the ColorSpace "API-surface" This patch reduces the number of `ColorSpaceUtils` static-methods, and in particular the `parseAsync` method is removed and it's now instead possible to have `parse` optionally return a Promise. This thus removes the need to manually check if a `ColorSpace`-instance is cached, note the changes in the `src/core/evaluator.js` file. --- src/core/colorspace_utils.js | 172 +++++++++++------------------------ src/core/evaluator.js | 88 ++++++++---------- 2 files changed, 90 insertions(+), 170 deletions(-) diff --git a/src/core/colorspace_utils.js b/src/core/colorspace_utils.js index b65df8f79..859c5fb4a 100644 --- a/src/core/colorspace_utils.js +++ b/src/core/colorspace_utils.js @@ -25,112 +25,12 @@ import { LabCS, PatternCS, } from "./colorspace.js"; -import { assert, shadow, warn } from "../shared/util.js"; import { Dict, Name, Ref } from "./primitives.js"; +import { shadow, unreachable, warn } from "../shared/util.js"; import { IccColorSpace } from "./icc_colorspace.js"; import { MissingDataException } from "./core_utils.js"; class ColorSpaceUtils { - /** - * @private - */ - static #cache( - cacheKey, - parsedCS, - { xref, globalColorSpaceCache, localColorSpaceCache } - ) { - if (!globalColorSpaceCache || !localColorSpaceCache) { - throw new Error( - 'ColorSpace.#cache - expected "globalColorSpaceCache"/"localColorSpaceCache" argument.' - ); - } - if (!parsedCS) { - throw new Error('ColorSpace.#cache - expected "parsedCS" argument.'); - } - let csName, csRef; - if (cacheKey instanceof Ref) { - csRef = cacheKey; - - // If parsing succeeded, we know that this call cannot throw. - cacheKey = xref.fetch(cacheKey); - } - if (cacheKey instanceof Name) { - csName = cacheKey.name; - } - if (csName || csRef) { - localColorSpaceCache.set(csName, csRef, parsedCS); - - if (csRef) { - globalColorSpaceCache.set(/* name = */ null, csRef, parsedCS); - } - } - } - - static getCached( - cacheKey, - xref, - globalColorSpaceCache, - localColorSpaceCache - ) { - if (!globalColorSpaceCache || !localColorSpaceCache) { - throw new Error( - 'ColorSpace.getCached - expected "globalColorSpaceCache"/"localColorSpaceCache" argument.' - ); - } - if (cacheKey instanceof Ref) { - const cachedCS = - globalColorSpaceCache.getByRef(cacheKey) || - localColorSpaceCache.getByRef(cacheKey); - if (cachedCS) { - return cachedCS; - } - - try { - cacheKey = xref.fetch(cacheKey); - } catch (ex) { - if (ex instanceof MissingDataException) { - throw ex; - } - // Any errors should be handled during parsing, rather than here. - } - } - if (cacheKey instanceof Name) { - return localColorSpaceCache.getByName(cacheKey.name) || null; - } - return null; - } - - static async parseAsync({ - cs, - xref, - resources = null, - pdfFunctionFactory, - globalColorSpaceCache, - localColorSpaceCache, - }) { - if (typeof PDFJSDev === "undefined" || PDFJSDev.test("TESTING")) { - assert( - !this.getCached(cs, xref, globalColorSpaceCache, localColorSpaceCache), - "Expected `ColorSpace.getCached` to have been manually checked " + - "before calling `ColorSpace.parseAsync`." - ); - } - - const options = { - xref, - resources, - pdfFunctionFactory, - globalColorSpaceCache, - localColorSpaceCache, - }; - const parsedCS = this.#parse(cs, options); - - // Attempt to cache the parsed ColorSpace, by name and/or reference. - this.#cache(cs, parsedCS, options); - - return parsedCS; - } - static parse({ cs, xref, @@ -138,17 +38,16 @@ class ColorSpaceUtils { pdfFunctionFactory, globalColorSpaceCache, localColorSpaceCache, + asyncIfNotCached = false, }) { - const cachedCS = this.getCached( - cs, - xref, - globalColorSpaceCache, - localColorSpaceCache - ); - if (cachedCS) { - return cachedCS; + if ( + (typeof PDFJSDev === "undefined" || PDFJSDev.test("TESTING")) && + (!globalColorSpaceCache || !localColorSpaceCache) + ) { + unreachable( + 'ColorSpaceUtils.parse - expected "globalColorSpaceCache"/"localColorSpaceCache" argument.' + ); } - const options = { xref, resources, @@ -156,12 +55,47 @@ class ColorSpaceUtils { globalColorSpaceCache, localColorSpaceCache, }; - const parsedCS = this.#parse(cs, options); + let csName, csRef, parsedCS; + + // Check if the ColorSpace is cached first, to avoid re-parsing it. + if (cs instanceof Ref) { + csRef = cs; + + const cachedCS = + globalColorSpaceCache.getByRef(csRef) || + localColorSpaceCache.getByRef(csRef); + if (cachedCS) { + return cachedCS; + } + cs = xref.fetch(cs); + } + if (cs instanceof Name) { + csName = cs.name; + + const cachedCS = localColorSpaceCache.getByName(csName); + if (cachedCS) { + return cachedCS; + } + } + + try { + parsedCS = this.#parse(cs, options); + } catch (ex) { + if (asyncIfNotCached && !(ex instanceof MissingDataException)) { + return Promise.reject(ex); + } + throw ex; + } // Attempt to cache the parsed ColorSpace, by name and/or reference. - this.#cache(cs, parsedCS, options); + if (csName || csRef) { + localColorSpaceCache.set(csName, csRef, parsedCS); - return parsedCS; + if (csRef) { + globalColorSpaceCache.set(/* name = */ null, csRef, parsedCS); + } + } + return asyncIfNotCached ? Promise.resolve(parsedCS) : parsedCS; } /** @@ -170,14 +104,16 @@ class ColorSpaceUtils { */ static #subParse(cs, options) { const { globalColorSpaceCache } = options; - let csRef; + + // Check if the ColorSpace is cached first, to avoid re-parsing it. if (cs instanceof Ref) { - const cachedCS = globalColorSpaceCache.getByRef(cs); + csRef = cs; + + const cachedCS = globalColorSpaceCache.getByRef(csRef); if (cachedCS) { return cachedCS; } - csRef = cs; } const parsedCS = this.#parse(cs, options); @@ -189,7 +125,8 @@ class ColorSpaceUtils { } static #parse(cs, options) { - const { xref, resources, pdfFunctionFactory } = options; + const { xref, resources, pdfFunctionFactory, globalColorSpaceCache } = + options; cs = xref.fetchIfRef(cs); if (cs instanceof Name) { @@ -254,7 +191,6 @@ class ColorSpaceUtils { const matrix = params.getArray("Matrix"); return new CalRGBCS(whitePoint, blackPoint, gamma, matrix); case "ICCBased": - const { globalColorSpaceCache } = options; const isRef = cs[1] instanceof Ref; if (isRef) { const cachedCS = globalColorSpaceCache.getByRef(cs[1]); diff --git a/src/core/evaluator.js b/src/core/evaluator.js index a41cd0345..d5224d7ac 100644 --- a/src/core/evaluator.js +++ b/src/core/evaluator.js @@ -68,6 +68,7 @@ import { } from "./image_utils.js"; import { BaseStream } from "./base_stream.js"; import { bidi } from "./bidi.js"; +import { ColorSpace } from "./colorspace.js"; import { ColorSpaceUtils } from "./colorspace_utils.js"; import { DecodeStream } from "./decode_stream.js"; import { FontFlags } from "./fonts_utils.js"; @@ -489,23 +490,13 @@ class PartialEvaluator { groupOptions.isolated = group.get("I") || false; groupOptions.knockout = group.get("K") || false; if (group.has("CS")) { - const cs = group.getRaw("CS"); - - const cachedColorSpace = ColorSpaceUtils.getCached( - cs, - this.xref, - this.globalColorSpaceCache, + const cs = this._getColorSpace( + group.getRaw("CS"), + resources, localColorSpaceCache ); - if (cachedColorSpace) { - colorSpace = cachedColorSpace; - } else { - colorSpace = await this.parseColorSpace({ - cs, - resources, - localColorSpaceCache, - }); - } + colorSpace = + cs instanceof ColorSpace ? cs : await this._handleColorSpace(cs); } } @@ -1462,24 +1453,31 @@ class PartialEvaluator { } } - parseColorSpace({ cs, resources, localColorSpaceCache }) { - return ColorSpaceUtils.parseAsync({ + _getColorSpace(cs, resources, localColorSpaceCache) { + return ColorSpaceUtils.parse({ cs, xref: this.xref, resources, pdfFunctionFactory: this._pdfFunctionFactory, globalColorSpaceCache: this.globalColorSpaceCache, localColorSpaceCache, - }).catch(reason => { - if (reason instanceof AbortException) { + asyncIfNotCached: true, + }); + } + + async _handleColorSpace(csPromise) { + try { + return await csPromise; + } catch (ex) { + if (ex instanceof AbortException) { return null; } if (this.options.ignoreErrors) { - warn(`parseColorSpace - ignoring ColorSpace: "${reason}".`); + warn(`_handleColorSpace - ignoring ColorSpace: "${ex}".`); return null; } - throw reason; - }); + throw ex; + } } parseShading({ @@ -1981,54 +1979,40 @@ class PartialEvaluator { break; case OPS.setFillColorSpace: { - const cachedColorSpace = ColorSpaceUtils.getCached( + const fillCS = self._getColorSpace( args[0], - xref, - self.globalColorSpaceCache, + resources, localColorSpaceCache ); - if (cachedColorSpace) { - stateManager.state.fillColorSpace = cachedColorSpace; + if (fillCS instanceof ColorSpace) { + stateManager.state.fillColorSpace = fillCS; continue; } next( - self - .parseColorSpace({ - cs: args[0], - resources, - localColorSpaceCache, - }) - .then(function (colorSpace) { - stateManager.state.fillColorSpace = - colorSpace || ColorSpaceUtils.singletons.gray; - }) + self._handleColorSpace(fillCS).then(colorSpace => { + stateManager.state.fillColorSpace = + colorSpace || ColorSpaceUtils.singletons.gray; + }) ); return; } case OPS.setStrokeColorSpace: { - const cachedColorSpace = ColorSpaceUtils.getCached( + const strokeCS = self._getColorSpace( args[0], - xref, - self.globalColorSpaceCache, + resources, localColorSpaceCache ); - if (cachedColorSpace) { - stateManager.state.strokeColorSpace = cachedColorSpace; + if (strokeCS instanceof ColorSpace) { + stateManager.state.strokeColorSpace = strokeCS; continue; } next( - self - .parseColorSpace({ - cs: args[0], - resources, - localColorSpaceCache, - }) - .then(function (colorSpace) { - stateManager.state.strokeColorSpace = - colorSpace || ColorSpaceUtils.singletons.gray; - }) + self._handleColorSpace(strokeCS).then(colorSpace => { + stateManager.state.strokeColorSpace = + colorSpace || ColorSpaceUtils.singletons.gray; + }) ); return; }