commit to xfermode objects being immutable

BUG=
R=djsollen@google.com, mtklein@google.com

Author: reed@google.com

Review URL: https://codereview.chromium.org/25968004

git-svn-id: http://skia.googlecode.com/svn/trunk@11614 2bbb7eff-a529-9590-31e7-b0007b416f81

diff --git a/bench/XfermodeBench.cpp b/bench/XfermodeBench.cpp
index e9b3b95..29e8598 100644 (file)
--- a/bench/XfermodeBench.cpp
+++ b/bench/XfermodeBench.cpp
@@ -62,6 +62,28 @@ private:
     typedef SkBenchmark INHERITED;
 };
 
+class XferCreateBench : public SkBenchmark {
+public:
+    XferCreateBench() {
+        fIsRendering = false;
+    }
+    
+protected:
+    virtual const char* onGetName() SK_OVERRIDE { return "xfermode_create"; }
+
+    virtual void onDraw(SkCanvas* canvas) SK_OVERRIDE {
+        for (int outer = 0; outer < this->getLoops() * 10; ++outer) {
+            for (int i = 0; i <= SkXfermode::kLastMode; ++i) {
+                SkXfermode* xfer = SkXfermode::Create(SkXfermode::Mode(i));
+                SkSafeUnref(xfer);
+            }
+        }
+    }
+    
+private:
+    typedef SkBenchmark INHERITED;
+};
+
 //////////////////////////////////////////////////////////////////////////////
 
 #define CONCAT_I(x, y) x ## y
@@ -106,3 +128,5 @@ BENCH(SkXfermode::kLuminosity_Mode)
 BENCH(SkLumaMaskXfermode::Create(SkXfermode::kSrcIn_Mode), "SrcInLuma")
 BENCH(SkLumaMaskXfermode::Create(SkXfermode::kDstIn_Mode), "DstInLuma")
 BENCH(SkLumaMaskXfermode::Create(SkXfermode::kSrcOver_Mode), "SrcOverLuma")
+
+DEF_BENCH(return new XferCreateBench;)

diff --git a/include/core/SkXfermode.h b/include/core/SkXfermode.h
index ef00f71..04f3bfe 100644 (file)
--- a/include/core/SkXfermode.h
+++ b/include/core/SkXfermode.h
@@ -237,6 +237,10 @@ private:
     enum {
         kModeCount = kLastMode + 1
     };
+
+    friend class SkGraphics;
+    static void Term();
+
     typedef SkFlattenable INHERITED;
 };
 

diff --git a/src/core/SkGraphics.cpp b/src/core/SkGraphics.cpp
index 8e7c7cd..453c30d 100644 (file)
--- a/src/core/SkGraphics.cpp
+++ b/src/core/SkGraphics.cpp
@@ -129,6 +129,7 @@ void SkGraphics::Init() {
 void SkGraphics::Term() {
     PurgeFontCache();
     SkPaint::Term();
+    SkXfermode::Term();
 }
 
 ///////////////////////////////////////////////////////////////////////////////

diff --git a/src/core/SkPicturePlayback.cpp b/src/core/SkPicturePlayback.cpp
index 13128aa..04c90ce 100644 (file)
--- a/src/core/SkPicturePlayback.cpp
+++ b/src/core/SkPicturePlayback.cpp
@@ -152,11 +152,11 @@ static bool needs_deep_copy(const SkPaint& paint) {
      *
      *  getTypeface();
      *  getAnnotation();
+     *  getXfermode();
      */
 
     return paint.getPathEffect() ||
            paint.getShader() ||
-           paint.getXfermode() ||
            paint.getMaskFilter() ||
            paint.getColorFilter() ||
            paint.getRasterizer() ||

diff --git a/src/core/SkXfermode.cpp b/src/core/SkXfermode.cpp
index e50e4f5..6631495 100644 (file)
--- a/src/core/SkXfermode.cpp
+++ b/src/core/SkXfermode.cpp
@@ -1688,26 +1688,66 @@ void SkDstOutXfermode::toString(SkString* str) const {
 
 ///////////////////////////////////////////////////////////////////////////////
 
+SK_DECLARE_STATIC_MUTEX(gCachedXfermodesMutex);
+static SkXfermode* gCachedXfermodes[SkXfermode::kLastMode + 1];
+
+void SkXfermode::Term() {
+    SkAutoMutexAcquire ac(gCachedXfermodesMutex);
+
+    for (size_t i = 0; i < SK_ARRAY_COUNT(gCachedXfermodes); ++i) {
+        SkSafeUnref(gCachedXfermodes[i]);
+        gCachedXfermodes[i] = NULL;
+    }
+}
+
 SkXfermode* SkXfermode::Create(Mode mode) {
     SkASSERT(SK_ARRAY_COUNT(gProcCoeffs) == kModeCount);
-    SkASSERT((unsigned)mode < kModeCount);
+    SkASSERT(SK_ARRAY_COUNT(gCachedXfermodes) == kModeCount);
 
-    const ProcCoeff& rec = gProcCoeffs[mode];
+    if ((unsigned)mode >= kModeCount) {
+        // report error
+        return NULL;
+    }
 
-    switch (mode) {
-        case kClear_Mode:
-            return SkNEW_ARGS(SkClearXfermode, (rec));
-        case kSrc_Mode:
-            return SkNEW_ARGS(SkSrcXfermode, (rec));
-        case kSrcOver_Mode:
-            return NULL;
-        case kDstIn_Mode:
-            return SkNEW_ARGS(SkDstInXfermode, (rec));
-        case kDstOut_Mode:
-            return SkNEW_ARGS(SkDstOutXfermode, (rec));
-        default:
-            return SkNEW_ARGS(SkProcCoeffXfermode, (rec, mode));
+    // Skia's "defaut" mode is srcover. NULL in SkPaint is interpreted as srcover
+    // so we can just return NULL from the factory.
+    if (kSrcOver_Mode == mode) {
+        return NULL;
+    }
+
+    // guard our access to gCachedXfermodes, since we may write into it
+    SkAutoMutexAcquire ac(gCachedXfermodesMutex);
+
+    SkXfermode* xfer = gCachedXfermodes[mode];
+    if (NULL == xfer) {
+        const ProcCoeff& rec = gProcCoeffs[mode];
+        // All modes can in theory be represented by the ProcCoeff rec, since
+        // it contains function ptrs. However, a few modes are both simple and
+        // commonly used, so we call those out for their own subclasses here.
+        switch (mode) {
+            case kClear_Mode:
+                xfer = SkNEW_ARGS(SkClearXfermode, (rec));
+                break;
+            case kSrc_Mode:
+                xfer = SkNEW_ARGS(SkSrcXfermode, (rec));
+                break;
+            case kSrcOver_Mode:
+                SkASSERT(false);    // should not land here
+                break;
+            case kDstIn_Mode:
+                xfer = SkNEW_ARGS(SkDstInXfermode, (rec));
+                break;
+            case kDstOut_Mode:
+                xfer = SkNEW_ARGS(SkDstOutXfermode, (rec));
+                break;
+            default:
+                // no special-case, just rely in the rec and its function-ptrs
+                xfer = SkNEW_ARGS(SkProcCoeffXfermode, (rec, mode));
+                break;
+        }
+        gCachedXfermodes[mode] = xfer;
     }
+    return SkSafeRef(xfer);
 }
 
 SkXfermodeProc SkXfermode::GetProc(Mode mode) {