SkPDF: SkPDFGraphicState Lookup hashtabled

src/pdf/SkPDFCanon.h

Index: src/pdf/SkPDFCanon.h
diff --git a/src/pdf/SkPDFCanon.h b/src/pdf/SkPDFCanon.h
index d0bef27ed6ecb1ae99310fdaaca1f9557a564cb0..730d5f22fa8fbc625c92b083583039ef2cb61231 100644
--- a/src/pdf/SkPDFCanon.h
+++ b/src/pdf/SkPDFCanon.h
@@ -7,12 +7,13 @@
 #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;
 
@@ -56,7 +57,7 @@ public:
     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;
@@ -76,7 +77,22 @@ private:
 
     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;
 };