SkPDF: SkPDFGraphicState Lookup hashtabled

src/pdf/SkPDFCanon.h

 /*
  * Copyright 2015 Google Inc.
  *
  * Use of this source code is governed by a BSD-style license that can be
  * found in the LICENSE file.
  */
 #ifndef SkPDFCanon_DEFINED
 #define SkPDFCanon_DEFINED
 
+#include "SkPDFGraphicState.h"
 #include "SkPDFShader.h"
 #include "SkTDArray.h"
+#include "SkTHash.h"
 
 class SkBitmap;
 class SkPDFFont;
-class SkPDFGraphicState;
 class SkPDFBitmap;
 class SkPaint;
 
 /**
  *  The SkPDFCanon canonicalizes objects across PDF pages(SkPDFDevices).
  *
  *  The PDF backend works correctly if:
  *  -  There is no more than one SkPDFCanon for each thread.
  *  -  Every SkPDFDevice is given a pointer to a SkPDFCanon on creation.
  *  -  All SkPDFDevices in a document share the same SkPDFCanon.
@@ skipping 23 matching lines @@
 
     SkPDFFunctionShader* findFunctionShader(const SkPDFShader::State&) const;
     void addFunctionShader(SkPDFFunctionShader*);
 
     SkPDFAlphaFunctionShader* findAlphaShader(const SkPDFShader::State&) const;
     void addAlphaShader(SkPDFAlphaFunctionShader*);
 
     SkPDFImageShader* findImageShader(const SkPDFShader::State&) const;
     void addImageShader(SkPDFImageShader*);
 
-    SkPDFGraphicState* findGraphicState(const SkPaint&) const;
+    SkPDFGraphicState* findGraphicState(SkPDFGraphicState&) const;

[mtklein, 2015/04/01 20:05:39]

It's unusual for us to have non-const refs as function arguments.  If we mutate
the arg, let's pass as a pointer... if not, const&.

     void addGraphicState(SkPDFGraphicState*);
 
     SkPDFBitmap* findBitmap(const SkBitmap&) const;
     void addBitmap(SkPDFBitmap*);
 
 private:
     struct FontRec {
         SkPDFFont* fFont;
         uint32_t fFontID;
         uint16_t fGlyphID;
     };
     SkTDArray<FontRec> fFontRecords;
 
     SkTDArray<SkPDFFunctionShader*> fFunctionShaderRecords;
 
     SkTDArray<SkPDFAlphaFunctionShader*> fAlphaShaderRecords;
 
     SkTDArray<SkPDFImageShader*> fImageShaderRecords;
 
-    SkTDArray<SkPDFGraphicState*> fGraphicStateRecords;
+    template <typename T>

[mtklein, 2015/04/01 20:05:39]

This doesn't really need to be templated, does it?

+    struct Wrap {

[mtklein, 2015/04/01 20:05:39]

// Wrap lets us store pointers in the hash set, but compare them by value.

+        explicit Wrap(T* ptr = NULL) : fPtr(ptr) {}

[mtklein, 2015/04/01 20:05:39]

Do we do anything with these pointers that isn't const?  Can we take and store
const T* ?

+        T* fPtr;
+        bool operator==(const Wrap<T>& rhs) const {
+            SkASSERT(fPtr);
+            SkASSERT(rhs.fPtr);
+            return *fPtr == *rhs.fPtr;
+        }
+        static uint32_t Hash(const Wrap<T>& w) {
+            SkASSERT(w.fPtr);
+            return SkGoodHash(w.fPtr->hash());

[mtklein, 2015/04/01 20:05:39]

Seems like overkill to rehash a hash?

+        }
+    };
+    SkTHashSet<Wrap<SkPDFGraphicState>, Wrap<SkPDFGraphicState>::Hash>
+            fGraphicStateRecords;
 
     SkTDArray<SkPDFBitmap*> fBitmapRecords;
 };
 #endif  // SkPDFCanon_DEFINED