SkPDFCanon

src/pdf/SkPDFGraphicState.h

Index: src/pdf/SkPDFGraphicState.h
diff --git a/src/pdf/SkPDFGraphicState.h b/src/pdf/SkPDFGraphicState.h
index 9182efeffb02080285518b579d2b7d685f13aceb..8761f99c781e95dd6273b0157655a75e9f332faf 100644
--- a/src/pdf/SkPDFGraphicState.h
+++ b/src/pdf/SkPDFGraphicState.h
@@ -73,30 +73,14 @@ public:
      */
     static SkPDFGraphicState* GetNoSMaskGraphicState();
 
+    bool equals(const SkPaint&) const;

[mtklein, 2015/01/20 21:59:52]

If you're going to inline this, do it here?

bool equals(const SkPaint& paint) const {
  return SkPDFGraphicState::PDFPaintEquality(paint, fPaint);
}

[hal.canary, 2015/01/21 17:07:51]

Done.

+
 private:
     const SkPaint fPaint;
     SkTDArray<SkPDFObject*> fResources;
     bool fPopulated;
     bool fSMask;
 
-    class GSCanonicalEntry {
-    public:
-        SkPDFGraphicState* fGraphicState;
-        const SkPaint* fPaint;
-
-        bool operator==(const GSCanonicalEntry& b) const;
-        explicit GSCanonicalEntry(SkPDFGraphicState* gs)
-            : fGraphicState(gs),
-              fPaint(&gs->fPaint) {}
-        explicit GSCanonicalEntry(const SkPaint* paint)
-            : fGraphicState(NULL),
-              fPaint(paint) {}
-    };
-
-    // This should be made a hash table if performance is a problem.
-    static SkTDArray<GSCanonicalEntry>& CanonicalPaints();
-    static SkBaseMutex& CanonicalPaintsMutex();
-
     SkPDFGraphicState();
     explicit SkPDFGraphicState(const SkPaint& paint);
 
@@ -104,8 +88,111 @@ private:
 
     static SkPDFObject* GetInvertFunction();
 
-    static int Find(const SkPaint& paint);
+    static bool PDFPaintEquality(const SkPaint&, const SkPaint&);
+    static const char* PDFBlendModeFromXfermode(SkXfermode::Mode);
+
     typedef SkPDFDict INHERITED;
 };
 
+inline const char* SkPDFGraphicState::PDFBlendModeFromXfermode(

[mtklein, 2015/01/20 21:59:51]

Same deal.  Seems like these two giant methods don't really belong.  Let's put
them in the .cpp file that calls them.

[hal.canary, 2015/01/21 17:07:51]

Done.

+        SkXfermode::Mode mode) {
+    switch (mode) {
+        case SkXfermode::kSrcOver_Mode:
+            return "Normal";
+        case SkXfermode::kMultiply_Mode:
+            return "Multiply";
+        case SkXfermode::kScreen_Mode:
+            return "Screen";
+        case SkXfermode::kOverlay_Mode:
+            return "Overlay";
+        case SkXfermode::kDarken_Mode:
+            return "Darken";
+        case SkXfermode::kLighten_Mode:
+            return "Lighten";
+        case SkXfermode::kColorDodge_Mode:
+            return "ColorDodge";
+        case SkXfermode::kColorBurn_Mode:
+            return "ColorBurn";
+        case SkXfermode::kHardLight_Mode:
+            return "HardLight";
+        case SkXfermode::kSoftLight_Mode:
+            return "SoftLight";
+        case SkXfermode::kDifference_Mode:
+            return "Difference";
+        case SkXfermode::kExclusion_Mode:
+            return "Exclusion";
+        case SkXfermode::kHue_Mode:
+            return "Hue";
+        case SkXfermode::kSaturation_Mode:
+            return "Saturation";
+        case SkXfermode::kColor_Mode:
+            return "Color";
+        case SkXfermode::kLuminosity_Mode:
+            return "Luminosity";
+
+        // These are handled in SkPDFDevice::setUpContentEntry.
+        case SkXfermode::kClear_Mode:
+        case SkXfermode::kSrc_Mode:
+        case SkXfermode::kDst_Mode:
+        case SkXfermode::kDstOver_Mode:
+        case SkXfermode::kSrcIn_Mode:
+        case SkXfermode::kDstIn_Mode:
+        case SkXfermode::kSrcOut_Mode:
+        case SkXfermode::kDstOut_Mode:
+        case SkXfermode::kSrcATop_Mode:
+        case SkXfermode::kDstATop_Mode:
+        case SkXfermode::kModulate_Mode:
+            return "Normal";
+
+        // TODO(vandebo): Figure out if we can support more of these modes.
+        case SkXfermode::kXor_Mode:
+        case SkXfermode::kPlus_Mode:
+            return NULL;
+    }
+    return NULL;
+}
+
+inline bool SkPDFGraphicState::equals(const SkPaint& paint) const {
+    return SkPDFGraphicState::PDFPaintEquality(paint, fPaint);
+}
+
+inline bool SkPDFGraphicState::PDFPaintEquality(const SkPaint& a,

[mtklein, 2015/01/20 21:59:52]

I keep glazing over reading these names.. consider something like

SkPDFGraphicState::PDFPaintEquality  --> SkPDFGraphicState::Equivalent
SkPDFGraphicState::PDFBlendModeFromXfermode --> SkPDFGraphicState::AsBlendMode()

?

[hal.canary, 2015/01/21 17:07:51]

Done.

+                                                const SkPaint& b) {
+    // We're only interested in some fields of the SkPaint, so we have
+    // a custom equality function.
+    if (SkColorGetA(a.getColor()) != SkColorGetA(b.getColor()) ||
+        a.getStrokeCap() != b.getStrokeCap() ||
+        a.getStrokeJoin() != b.getStrokeJoin() ||
+        a.getStrokeWidth() != b.getStrokeWidth() ||
+        a.getStrokeMiter() != b.getStrokeMiter()) {
+        return false;
+    }
+
+    SkXfermode::Mode aXfermodeName = SkXfermode::kSrcOver_Mode;
+    SkXfermode* aXfermode = a.getXfermode();
+    if (aXfermode) {
+        aXfermode->asMode(&aXfermodeName);
+    }
+    if (aXfermodeName < 0 || aXfermodeName > SkXfermode::kLastMode ||
+        PDFBlendModeFromXfermode(aXfermodeName) == NULL) {
+        aXfermodeName = SkXfermode::kSrcOver_Mode;
+    }
+    const char* aXfermodeString = PDFBlendModeFromXfermode(aXfermodeName);
+    SkASSERT(aXfermodeString != NULL);
+
+    SkXfermode::Mode bXfermodeName = SkXfermode::kSrcOver_Mode;
+    SkXfermode* bXfermode = b.getXfermode();
+    if (bXfermode) {
+        bXfermode->asMode(&bXfermodeName);
+    }
+    if (bXfermodeName < 0 || bXfermodeName > SkXfermode::kLastMode ||
+        PDFBlendModeFromXfermode(bXfermodeName) == NULL) {
+        bXfermodeName = SkXfermode::kSrcOver_Mode;
+    }
+    const char* bXfermodeString = PDFBlendModeFromXfermode(bXfermodeName);
+    SkASSERT(bXfermodeString != NULL);
+
+    return strcmp(aXfermodeString, bXfermodeString) == 0;
+}
+
 #endif