Extract most of the mutable state of SkShader into a separate Context object.

src/core/SkComposeShader.cpp

Index: src/core/SkComposeShader.cpp
diff --git a/src/core/SkComposeShader.cpp b/src/core/SkComposeShader.cpp
index b5ea63c24c5ed7e60dbe54d671ef9f59c076953d..e67e6328d65423e3570991ad09c102cdaa987a6d 100644
--- a/src/core/SkComposeShader.cpp
+++ b/src/core/SkComposeShader.cpp
@@ -45,6 +45,10 @@ SkComposeShader::~SkComposeShader() {
     fShaderA->unref();
 }
 
+size_t SkComposeShader::contextSize() const {
+    return sizeof(ComposeShaderContext) + fShaderA->contextSize() + fShaderB->contextSize();
+}
+
 class SkAutoAlphaRestore {
 public:
     SkAutoAlphaRestore(SkPaint* paint, uint8_t newAlpha) {
@@ -73,13 +77,11 @@ void SkComposeShader::flatten(SkWriteBuffer& buffer) const {
     always let them see opaque alpha, and if the paint really
     is translucent, then we apply that after the fact.
 
-    We need to keep the calls to setContext/endContext balanced, since if we
-    return false, our endContext() will not be called.
  */
-bool SkComposeShader::setContext(const SkBitmap& device,
-                                 const SkPaint& paint,
-                                 const SkMatrix& matrix) {
-    if (!this->INHERITED::setContext(device, paint, matrix)) {
+bool SkComposeShader::validContext(const SkBitmap& device,
+                                   const SkPaint& paint,
+                                   const SkMatrix& matrix) const {
+    if (!this->INHERITED::validContext(device, paint, matrix)) {
         return false;
     }
 
@@ -90,37 +92,60 @@ bool SkComposeShader::setContext(const SkBitmap& device,
 
     tmpM.setConcat(matrix, this->getLocalMatrix());
 
-    SkAutoAlphaRestore  restore(const_cast<SkPaint*>(&paint), 0xFF);
+    return fShaderA->validContext(device, paint, tmpM) &&
+           fShaderB->validContext(device, paint, tmpM);
+}
 
-    bool setContextA = fShaderA->setContext(device, paint, tmpM);
-    bool setContextB = fShaderB->setContext(device, paint, tmpM);
-    if (!setContextA || !setContextB) {
-        if (setContextB) {
-            fShaderB->endContext();
-        }
-        else if (setContextA) {
-            fShaderA->endContext();
-        }
-        this->INHERITED::endContext();
-        return false;
+SkShader::Context* SkComposeShader::createContext(const SkBitmap& device, const SkPaint& paint,
+                                                  const SkMatrix& matrix, void* storage) const {
+    // TODO: move this check to SkShader::createContext and the rest to createContextImpl().

[scroggo, 2014/03/24 21:24:46]

The general naming pattern in Skia is to name the virtual version (the one you
name Impl) onCreateContext.

[Dominik Grewe, 2014/03/26 17:22:22]

Oh yes, I forgot. But if we break the contract between validContext and
createContext we may not want to do this anyway.

+    if (!this->validContext(device, paint, matrix)) {
+        return NULL;
     }
-    return true;
+
+    // we preconcat our localMatrix (if any) with the device matrix
+    // before calling our sub-shaders
+
+    SkMatrix tmpM;
+
+    tmpM.setConcat(matrix, this->getLocalMatrix());
+
+    SkAutoAlphaRestore  restore(const_cast<SkPaint*>(&paint), 0xFF);
+
+    char* aStorage = (char*) storage + sizeof(ComposeShaderContext);
+    char* bStorage = aStorage + fShaderA->contextSize();
+
+    SkShader::Context* contextA = fShaderA->createContext(device, paint, tmpM, aStorage);
+    SkShader::Context* contextB = fShaderB->createContext(device, paint, tmpM, bStorage);
+
+    // Both functions must succeed; otherwise validContext should have returned
+    // false.
+    SkASSERT(contextA);
+    SkASSERT(contextB);
+
+    return SkNEW_PLACEMENT_ARGS(storage, ComposeShaderContext,
+                                (*this, device, paint, matrix, contextA, contextB));
 }
 
-void SkComposeShader::endContext() {
-    fShaderB->endContext();
-    fShaderA->endContext();
-    this->INHERITED::endContext();
+SkComposeShader::ComposeShaderContext::ComposeShaderContext(
+        const SkComposeShader& shader, const SkBitmap& device,
+        const SkPaint& paint, const SkMatrix& matrix,
+        SkShader::Context* contextA, SkShader::Context* contextB)
+    : INHERITED(shader, device, paint, matrix)
+    , fShaderContextA(contextA)
+    , fShaderContextB(contextB) {}
+
+SkComposeShader::ComposeShaderContext::~ComposeShaderContext() {
+    fShaderContextA->SkShader::Context::~Context();
+    fShaderContextB->SkShader::Context::~Context();
 }
 
 // larger is better (fewer times we have to loop), but we shouldn't
 // take up too much stack-space (each element is 4 bytes)
 #define TMP_COLOR_COUNT     64
 
-void SkComposeShader::shadeSpan(int x, int y, SkPMColor result[], int count) {
-    SkShader*   shaderA = fShaderA;

[scroggo, 2014/03/24 21:24:46]

I think we grabbed these pointers outside the loop for caching purposes. Should
we do the same for fShaderContextA/B?

[Dominik Grewe, 2014/03/26 17:22:22]

Done.

-    SkShader*   shaderB = fShaderB;
-    SkXfermode* mode = fMode;
+void SkComposeShader::ComposeShaderContext::shadeSpan(int x, int y, SkPMColor result[], int count) {
+    SkXfermode* mode = static_cast<const SkComposeShader&>(fShader).fMode;
     unsigned    scale = SkAlpha255To256(this->getPaintAlpha());
 
     SkPMColor   tmp[TMP_COLOR_COUNT];
@@ -134,8 +159,8 @@ void SkComposeShader::shadeSpan(int x, int y, SkPMColor result[], int count) {
                 n = TMP_COLOR_COUNT;
             }
 
-            shaderA->shadeSpan(x, y, result, n);
-            shaderB->shadeSpan(x, y, tmp, n);
+            fShaderContextA->shadeSpan(x, y, result, n);
+            fShaderContextB->shadeSpan(x, y, tmp, n);
 
             if (256 == scale) {
                 for (int i = 0; i < n; i++) {
@@ -159,8 +184,8 @@ void SkComposeShader::shadeSpan(int x, int y, SkPMColor result[], int count) {
                 n = TMP_COLOR_COUNT;
             }
 
-            shaderA->shadeSpan(x, y, result, n);
-            shaderB->shadeSpan(x, y, tmp, n);
+            fShaderContextA->shadeSpan(x, y, result, n);
+            fShaderContextB->shadeSpan(x, y, tmp, n);
             mode->xfer32(result, tmp, n, NULL);
 
             if (256 == scale) {