PDF: Now threadsafe!

src/pdf/SkPDFGraphicState.cpp

 /*
  * Copyright 2011 Google Inc.
  *
  * Use of this source code is governed by a BSD-style license that can be
  * found in the LICENSE file.
  */
 
 #include "SkData.h"
 #include "SkLazyPtr.h"
 #include "SkPDFCanon.h"
@@ skipping 95 matching lines @@
     SkASSERT(bXfermodeString != NULL);
 
     return strcmp(aXfermodeString, bXfermodeString) == 0;
 }
 
 bool SkPDFGraphicState::equals(const SkPaint& paint) const {
     return equivalent(paint, fPaint);
 }
 
 SkPDFGraphicState::~SkPDFGraphicState() {
-    SkAutoMutexAcquire lock(SkPDFCanon::GetPaintMutex());
+    SkASSERT((fSMask || fCanon) && !(fSMask && fCanon));

[mtklein, 2015/02/19 23:50:38]

My head asplode.  Can we write this as

if (fCanon) {
   SkASSERT(!fSMask);
   fCanon->removeGraphicState(this);
}

[hal.canary, 2015/02/20 01:22:28]

Not exactly.  Is this more readable?

  SkASSERT((fSMask == NULL) != (fCanon == NULL));

[mtklein, 2015/02/20 14:55:41]

Yes that was actually my first thought too, until I realized fSMask is a bool. 
Then I couldn't figure out any way to write it that wasn't either unclear or
misleading.

Why don't we just drop the fSMask checks altogether here?  Seems like mixing
that in is just sort of confusing things.

if (fCanon) {
    fCanon->removeGraphicState(this);
}

     if (!fSMask) {
-        SkPDFCanon::GetCanon().removeGraphicState(this);
+        fCanon->removeGraphicState(this);
     }
 }
 
 void SkPDFGraphicState::emitObject(SkWStream* stream, SkPDFCatalog* catalog) {
     populateDict();
     SkPDFDict::emitObject(stream, catalog);
 }
 
 // static
 SkPDFGraphicState* SkPDFGraphicState::GetGraphicStateForPaint(
-        const SkPaint& paint) {
-    SkAutoMutexAcquire lock(SkPDFCanon::GetPaintMutex());
-    SkPDFGraphicState* pdfGraphicState =
-            SkPDFCanon::GetCanon().findGraphicState(paint);
+        SkPDFCanon* canon, const SkPaint& paint) {
+    SkASSERT(canon);
+    SkPDFGraphicState* pdfGraphicState = canon->findGraphicState(paint);
     if (pdfGraphicState) {
         return SkRef(pdfGraphicState);
     }
-    pdfGraphicState = new SkPDFGraphicState(paint);
-    SkPDFCanon::GetCanon().addGraphicState(pdfGraphicState);
+    pdfGraphicState = new SkPDFGraphicState(canon, paint);
+    canon->addGraphicState(pdfGraphicState);
     return pdfGraphicState;
 }
 
 namespace {
 SkPDFObject* create_invert_function() {
     // Acrobat crashes if we use a type 0 function, kpdf crashes if we use
     // a type 2 function, so we use a type 4 function.
     SkAutoTUnref<SkPDFArray> domainAndRange(new SkPDFArray);
     domainAndRange->reserve(2);
     domainAndRange->appendInt(0);
@@ skipping 56 matching lines @@
 SK_DECLARE_STATIC_LAZY_PTR(
         SkPDFGraphicState, noSMaskGraphicState,
         SkPDFGraphicState::CreateNoSMaskGraphicState, unref<SkPDFGraphicState>);
 
 // static
 SkPDFGraphicState* SkPDFGraphicState::GetNoSMaskGraphicState() {
     return SkRef(noSMaskGraphicState.get());
 }
 
 SkPDFGraphicState::SkPDFGraphicState()
-    : fPopulated(false),
-      fSMask(false) {
-}
+    : fCanon(NULL), fPopulated(false), fSMask(false) {}
 
-SkPDFGraphicState::SkPDFGraphicState(const SkPaint& paint)
-    : fPaint(paint),
-      fPopulated(false),
-      fSMask(false) {
-}
+SkPDFGraphicState::SkPDFGraphicState(SkPDFCanon* canon, const SkPaint& paint)
+    : fCanon(canon), fPaint(paint), fPopulated(false), fSMask(false) {}
 
 // populateDict and operator== have to stay in sync with each other.
 void SkPDFGraphicState::populateDict() {
     if (!fPopulated) {
         fPopulated = true;
         insertName("Type", "ExtGState");
 
         SkAutoTUnref<SkPDFScalar> alpha(
             new SkPDFScalar(SkScalarDiv(fPaint.getAlpha(), 0xFF)));
         insert("CA", alpha.get());
@@ skipping 23 matching lines @@
             fPaint.getXfermode()->asMode(&xfermode);
         // If we don't support the mode, just use kSrcOver_Mode.
         if (xfermode < 0 || xfermode > SkXfermode::kLastMode ||
             as_blend_mode(xfermode) == NULL) {
             xfermode = SkXfermode::kSrcOver_Mode;
             NOT_IMPLEMENTED("unsupported xfermode", false);
         }
         insertName("BM", as_blend_mode(xfermode));
     }
 }