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

src/core/SkShader.cpp

Index: src/core/SkShader.cpp
diff --git a/src/core/SkShader.cpp b/src/core/SkShader.cpp
index b06e0c26ff6391e3125d21d847746b05d76c7aa0..62e5343c3d9731955a6b17bc8b4bbbfbc42ca8ad 100644
--- a/src/core/SkShader.cpp
+++ b/src/core/SkShader.cpp
@@ -15,7 +15,6 @@
 
 SkShader::SkShader() {
     fLocalMatrix.reset();
-    SkDEBUGCODE(fInSetContext = false;)
 }
 
 SkShader::SkShader(SkReadBuffer& buffer)
@@ -25,12 +24,9 @@ SkShader::SkShader(SkReadBuffer& buffer)
     } else {
         fLocalMatrix.reset();
     }
-
-    SkDEBUGCODE(fInSetContext = false;)
 }
 
 SkShader::~SkShader() {
-    SkASSERT(!fInSetContext);
 }
 
 void SkShader::flatten(SkWriteBuffer& buffer) const {
@@ -42,39 +38,48 @@ void SkShader::flatten(SkWriteBuffer& buffer) const {
     }
 }
 
-bool SkShader::setContext(const SkBitmap& device,
-                          const SkPaint& paint,
-                          const SkMatrix& matrix) {
-    SkASSERT(!this->setContextHasBeenCalled());
-
+bool SkShader::validContext(const SkBitmap& device,
+                                     const SkPaint& paint,

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

nit: spacing

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

Done.

+                                     const SkMatrix& matrix) const {
     const SkMatrix* m = &matrix;
     SkMatrix        total;
 
-    fPaintAlpha = paint.getAlpha();
     if (this->hasLocalMatrix()) {
         total.setConcat(matrix, this->getLocalMatrix());
         m = &total;
     }
-    if (m->invert(&fTotalInverse)) {
-        fTotalInverseClass = (uint8_t)ComputeMatrixClass(fTotalInverse);
-        SkDEBUGCODE(fInSetContext = true;)
+    if (m->invert(NULL)) {

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

This can be replaced with:

return m->invert(NULL);

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

Done.

         return true;
     }
     return false;
 }
 
-void SkShader::endContext() {
-    SkASSERT(fInSetContext);
-    SkDEBUGCODE(fInSetContext = false;)
+SkShader::Context::Context(const SkShader& shader, const SkBitmap& device,
+                           const SkPaint& paint, const SkMatrix& matrix)
+    : fShader(shader) {

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

nit: Easier to follow if you move the brace to its own line after an initializer
list, to distinguish the body of the constructor from the list.

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

Done.

+    SkASSERT(fShader.validContext(device, paint, matrix));
+
+    const SkMatrix* m = &matrix;
+    SkMatrix        total;
+
+    fPaintAlpha = paint.getAlpha();
+    if (fShader.hasLocalMatrix()) {

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

Worth it to refer to the local variable so we don't need to read from memory?

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

Done.

+        total.setConcat(matrix, fShader.getLocalMatrix());

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

Ditto

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

Done.

+        m = &total;
+    }
+    SkAssertResult(m->invert(&fTotalInverse));
+    fTotalInverseClass = (uint8_t)ComputeMatrixClass(fTotalInverse);
 }
 
-SkShader::ShadeProc SkShader::asAShadeProc(void** ctx) {
+SkShader::Context::~Context() {}
+
+SkShader::Context::ShadeProc SkShader::Context::asAShadeProc(void** ctx) {
     return NULL;
 }
 
 #include "SkColorPriv.h"
 
-void SkShader::shadeSpan16(int x, int y, uint16_t span16[], int count) {
+void SkShader::Context::shadeSpan16(int x, int y, uint16_t span16[], int count) {
     SkASSERT(span16);
     SkASSERT(count > 0);
     SkASSERT(this->canCallShadeSpan16());
@@ -92,7 +97,7 @@ void SkShader::shadeSpan16(int x, int y, uint16_t span16[], int count) {
     #define SkU32BitShiftToByteOffset(shift)    ((shift) >> 3)
 #endif
 
-void SkShader::shadeSpanAlpha(int x, int y, uint8_t alpha[], int count) {
+void SkShader::Context::shadeSpanAlpha(int x, int y, uint8_t alpha[], int count) {
     SkASSERT(count > 0);
 
     SkPMColor   colors[kTempColorCount];
@@ -146,7 +151,8 @@ void SkShader::shadeSpanAlpha(int x, int y, uint8_t alpha[], int count) {
 #endif
 }
 
-SkShader::MatrixClass SkShader::ComputeMatrixClass(const SkMatrix& mat) {
+SkShader::Context::MatrixClass
+SkShader::Context::ComputeMatrixClass(const SkMatrix& mat) {
     MatrixClass mc = kLinear_MatrixClass;
 
     if (mat.hasPerspective()) {
@@ -161,8 +167,7 @@ SkShader::MatrixClass SkShader::ComputeMatrixClass(const SkMatrix& mat) {
 
 //////////////////////////////////////////////////////////////////////////////
 
-SkShader::BitmapType SkShader::asABitmap(SkBitmap*, SkMatrix*,
-                                         TileMode*) const {
+SkShader::BitmapType SkShader::asABitmap(SkBitmap*, SkMatrix*, TileMode*) const {
     return kNone_BitmapType;
 }
 
@@ -193,19 +198,16 @@ void SkShader::toString(SkString* str) const {
 #include "SkColorShader.h"
 #include "SkUtils.h"
 
-SkColorShader::SkColorShader() {
-    fFlags = 0;
-    fInheritColor = true;
+SkColorShader::SkColorShader()
+    : fColor()
+    , fInheritColor(true) {
 }
 
-SkColorShader::SkColorShader(SkColor c) {
-    fFlags = 0;
-    fColor = c;
-    fInheritColor = false;
+SkColorShader::SkColorShader(SkColor c)
+    : fColor(c)
+    , fInheritColor(false) {
 }
 
-SkColorShader::~SkColorShader() {}
-
 bool SkColorShader::isOpaque() const {
     if (fInheritColor) {
         return true; // using paint's alpha
@@ -213,14 +215,11 @@ bool SkColorShader::isOpaque() const {
     return SkColorGetA(fColor) == 255;
 }
 
-SkColorShader::SkColorShader(SkReadBuffer& b) : INHERITED(b) {
-    fFlags = 0; // computed in setContext
-
-    fInheritColor = b.readBool();
-    if (fInheritColor) {
-        return;
+SkFlattenable* SkColorShader::CreateProc(SkReadBuffer& b) {
+    if (b.readBool()) {
+        return SkNEW(SkColorShader);
     }
-    fColor = b.readColor();
+    return SkNEW_ARGS(SkColorShader, (b.readColor()));
 }
 
 void SkColorShader::flatten(SkWriteBuffer& buffer) const {
@@ -232,32 +231,42 @@ void SkColorShader::flatten(SkWriteBuffer& buffer) const {
     buffer.writeColor(fColor);
 }
 
-uint32_t SkColorShader::getFlags() {
+uint32_t SkColorShader::ColorShaderContext::getFlags() const {
     return fFlags;
 }
 
-uint8_t SkColorShader::getSpan16Alpha() const {
+uint8_t SkColorShader::ColorShaderContext::getSpan16Alpha() const {
     return SkGetPackedA32(fPMColor);
 }
 
-bool SkColorShader::setContext(const SkBitmap& device, const SkPaint& paint,
-                               const SkMatrix& matrix) {
-    if (!this->INHERITED::setContext(device, paint, matrix)) {
-        return false;
+SkShader::Context* SkColorShader::createContext(const SkBitmap& device, const SkPaint& paint,
+                                                const SkMatrix& matrix, void* storage) const {
+    if (!this->validContext(device, paint, matrix)) {
+        return NULL;
     }
 
+    return SkNEW_PLACEMENT_ARGS(storage, ColorShaderContext, (*this, device, paint, matrix));
+}
+
+SkColorShader::ColorShaderContext::ColorShaderContext(const SkColorShader& shader,
+                                                      const SkBitmap& device,
+                                                      const SkPaint& paint,
+                                                      const SkMatrix& matrix)
+    : INHERITED(shader, device, paint, matrix) {

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

nit: brace on its own line.

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

Done.

     unsigned a;
 
-    if (fInheritColor) {
-        fColor = paint.getColor();
-        a = SkColorGetA(fColor);
+    SkColor color;
+    if (shader.fInheritColor) {
+        color = paint.getColor();
+        a = SkColorGetA(color);
     } else {
-        a = SkAlphaMul(SkColorGetA(fColor), SkAlpha255To256(paint.getAlpha()));
+        color = shader.fColor;
+        a = SkAlphaMul(SkColorGetA(color), SkAlpha255To256(paint.getAlpha()));
     }
 
-    unsigned r = SkColorGetR(fColor);
-    unsigned g = SkColorGetG(fColor);
-    unsigned b = SkColorGetB(fColor);
+    unsigned r = SkColorGetR(color);
+    unsigned g = SkColorGetG(color);
+    unsigned b = SkColorGetB(color);
 
     // we want this before we apply any alpha
     fColor16 = SkPack888ToRGB16(r, g, b);
@@ -276,19 +285,17 @@ bool SkColorShader::setContext(const SkBitmap& device, const SkPaint& paint,
             fFlags |= kHasSpan16_Flag;
         }
     }
-
-    return true;
 }
 
-void SkColorShader::shadeSpan(int x, int y, SkPMColor span[], int count) {
+void SkColorShader::ColorShaderContext::shadeSpan(int x, int y, SkPMColor span[], int count) {
     sk_memset32(span, fPMColor, count);
 }
 
-void SkColorShader::shadeSpan16(int x, int y, uint16_t span[], int count) {
+void SkColorShader::ColorShaderContext::shadeSpan16(int x, int y, uint16_t span[], int count) {
     sk_memset16(span, fColor16, count);
 }
 
-void SkColorShader::shadeSpanAlpha(int x, int y, uint8_t alpha[], int count) {
+void SkColorShader::ColorShaderContext::shadeSpanAlpha(int x, int y, uint8_t alpha[], int count) {
     memset(alpha, SkGetPackedA32(fPMColor), count);
 }
 
@@ -330,24 +337,6 @@ void SkColorShader::toString(SkString* str) const {
 
 #include "SkEmptyShader.h"
 
-uint32_t SkEmptyShader::getFlags() { return 0; }
-uint8_t SkEmptyShader::getSpan16Alpha() const { return 0; }
-
-bool SkEmptyShader::setContext(const SkBitmap&, const SkPaint&,
-                               const SkMatrix&) { return false; }
-
-void SkEmptyShader::shadeSpan(int x, int y, SkPMColor span[], int count) {
-    SkDEBUGFAIL("should never get called, since setContext() returned false");
-}
-
-void SkEmptyShader::shadeSpan16(int x, int y, uint16_t span[], int count) {
-    SkDEBUGFAIL("should never get called, since setContext() returned false");
-}
-
-void SkEmptyShader::shadeSpanAlpha(int x, int y, uint8_t alpha[], int count) {
-    SkDEBUGFAIL("should never get called, since setContext() returned false");
-}
-
 #ifdef SK_DEVELOPER
 void SkEmptyShader::toString(SkString* str) const {
     str->append("SkEmptyShader: (");