Merge pull request #13904 from Snuffleupagus/fix-LocalTilingPatternCache

Re-factor the LocalTilingPatternCache to cache by Ref rather than Name (PR 12458 follow-up, issue 13780)
mozilla · Aug 21, 2021 · 1b3382f · 1b3382f
2 parents 50dffda + 5f25fea
commit 1b3382f
Show file tree

Hide file tree

Showing 2 changed files with 22 additions and 31 deletions.
diff --git a/src/core/evaluator.js b/src/core/evaluator.js
@@ -795,7 +795,6 @@ class PartialEvaluator {
     patternDict,
     operatorList,
     task,
-    cacheKey,
     localTilingPatternCache
   ) {
     // Create an IR of the pattern code.
@@ -825,8 +824,8 @@ class PartialEvaluator {
         operatorList.addDependencies(tilingOpList.dependencies);
         operatorList.addOp(fn, tilingPatternIR);
 
-        if (cacheKey) {
-          localTilingPatternCache.set(cacheKey, patternDict.objId, {
+        if (patternDict.objId) {
+          localTilingPatternCache.set(/* name = */ null, patternDict.objId, {
             operatorListIR,
             dict: patternDict,
           });
@@ -1356,9 +1355,11 @@ class PartialEvaluator {
     const patternName = args.pop();
     // SCN/scn applies patterns along with normal colors
     if (patternName instanceof Name) {
-      const name = patternName.name;
+      const rawPattern = patterns.getRaw(patternName.name);
 
-      const localTilingPattern = localTilingPatternCache.getByName(name);
+      const localTilingPattern =
+        rawPattern instanceof Ref &&
+        localTilingPatternCache.getByRef(rawPattern);
       if (localTilingPattern) {
         try {
           const color = cs.base ? cs.base.getRgb(args, 0) : null;
@@ -1373,11 +1374,8 @@ class PartialEvaluator {
           // Handle any errors during normal TilingPattern parsing.
         }
       }
-      // TODO: Attempt to lookup cached TilingPatterns by reference as well,
-      //       if and only if there are PDF documents where doing so would
-      //       significantly improve performance.
 
-      const pattern = patterns.get(name);
+      const pattern = this.xref.fetchIfRef(rawPattern);
       if (pattern) {
         const dict = isStream(pattern) ? pattern.dict : pattern;
         const typeNum = dict.get("PatternType");
@@ -1392,7 +1390,6 @@ class PartialEvaluator {
             dict,
             operatorList,
             task,
-            /* cacheKey = */ name,
             localTilingPatternCache
           );
         } else if (typeNum === PatternType.SHADING) {

diff --git a/src/core/image_utils.js b/src/core/image_utils.js
@@ -21,14 +21,19 @@ class BaseLocalCache {
     if (this.constructor === BaseLocalCache) {
       unreachable("Cannot initialize BaseLocalCache.");
     }
-    if (!options || !options.onlyRefs) {
+    this._onlyRefs = (options && options.onlyRefs) === true;
+
+    if (!this._onlyRefs) {
       this._nameRefMap = new Map();
       this._imageMap = new Map();
     }
     this._imageCache = new RefSetCache();
   }
 
   getByName(name) {
+    if (this._onlyRefs) {
+      unreachable("Should not call `getByName` method.");
+    }
     const ref = this._nameRefMap.get(name);
     if (ref) {
       return this.getByRef(ref);
@@ -97,10 +102,6 @@ class LocalFunctionCache extends BaseLocalCache {
     super({ onlyRefs: true });
   }
 
-  getByName(name) {
-    unreachable("Should not call `getByName` method.");
-  }
-
   set(name = null, ref, data) {
     if (!ref) {
       throw new Error('LocalFunctionCache.set - expected "ref" argument.');
@@ -134,25 +135,18 @@ class LocalGStateCache extends BaseLocalCache {
 }
 
 class LocalTilingPatternCache extends BaseLocalCache {
-  set(name, ref = null, data) {
-    if (typeof name !== "string") {
-      throw new Error(
-        'LocalTilingPatternCache.set - expected "name" argument.'
-      );
-    }
-    if (ref) {
-      if (this._imageCache.has(ref)) {
-        return;
-      }
-      this._nameRefMap.set(name, ref);
-      this._imageCache.put(ref, data);
-      return;
+  constructor(options) {
+    super({ onlyRefs: true });
+  }
+
+  set(name = null, ref, data) {
+    if (!ref) {
+      throw new Error('LocalTilingPatternCache.set - expected "ref" argument.');
     }
-    // name
-    if (this._imageMap.has(name)) {
+    if (this._imageCache.has(ref)) {
       return;
     }
-    this._imageMap.set(name, data);
+    this._imageCache.put(ref, data);
   }
 }