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

src/core/SkBlitter.cpp

 
 /*
  * Copyright 2006 The Android Open Source Project
  *
  * Use of this source code is governed by a BSD-style license that can be
  * found in the LICENSE file.
  */
 
 
 #include "SkBlitter.h"
 #include "SkAntiRun.h"
 #include "SkColor.h"
 #include "SkColorFilter.h"
 #include "SkCoreBlitters.h"
 #include "SkFilterShader.h"
 #include "SkReadBuffer.h"
 #include "SkWriteBuffer.h"
 #include "SkMask.h"
 #include "SkMaskFilter.h"
 #include "SkString.h"
 #include "SkTLazy.h"
 #include "SkUtils.h"
 #include "SkXfermode.h"
 
 SkBlitter::~SkBlitter() {}
 
 bool SkBlitter::isNullBlitter() const { return false; }
 
+bool SkBlitter::resetShaderContext(const SkBitmap& device, const SkPaint& paint,
+                                   const SkMatrix& matrix) {
+    return true;
+}
+
+SkShader::Context* SkBlitter::getShaderContext() const {
+    return NULL;
+}
+
 const SkBitmap* SkBlitter::justAnOpaqueColor(uint32_t* value) {
     return NULL;
 }
 
 void SkBlitter::blitH(int x, int y, int width) {
     SkDEBUGFAIL("unimplemented");
 }
 
 void SkBlitter::blitAntiH(int x, int y, const SkAlpha antialias[],
                           const int16_t runs[]) {
@@ skipping 522 matching lines @@
 
 ///////////////////////////////////////////////////////////////////////////////
 
 #include "SkColorShader.h"
 #include "SkColorPriv.h"
 
 class Sk3DShader : public SkShader {
 public:
     Sk3DShader(SkShader* proxy) : fProxy(proxy) {
         SkSafeRef(proxy);
-        fMask = NULL;
     }
 
     virtual ~Sk3DShader() {
         SkSafeUnref(fProxy);
     }
 
-    void setMask(const SkMask* mask) { fMask = mask; }
+    virtual size_t contextSize() const SK_OVERRIDE {
+        size_t size = sizeof(Sk3DShaderContext);
+        if (fProxy) {
+            size += fProxy->contextSize();
+        }
+        return size;
+    }
 
-    virtual bool setContext(const SkBitmap& device, const SkPaint& paint,
-                            const SkMatrix& matrix) SK_OVERRIDE {
-        if (!this->INHERITED::setContext(device, paint, matrix)) {
+    virtual bool validContext(const SkBitmap& device, const SkPaint& paint,
+                              const SkMatrix& matrix, SkMatrix* totalInverse = NULL) const
+            SK_OVERRIDE
+    {
+        if (!this->INHERITED::validContext(device, paint, matrix, totalInverse)) {
             return false;
         }
         if (fProxy) {
-            if (!fProxy->setContext(device, paint, matrix)) {
-                // must keep our set/end context calls balanced
-                this->INHERITED::endContext();
-                return false;
-            }
-        } else {
-            fPMColor = SkPreMultiplyColor(paint.getColor());
+            return fProxy->validContext(device, paint, matrix);
         }
         return true;
     }
 
-    virtual void endContext() SK_OVERRIDE {
+    virtual SkShader::Context* createContext(const SkBitmap& device,
+                                             const SkPaint& paint,
+                                             const SkMatrix& matrix,
+                                             void* storage) const SK_OVERRIDE
+    {
+        if (!this->validContext(device, paint, matrix)) {
+            return NULL;
+        }
+
+        SkShader::Context* proxyContext;
         if (fProxy) {
-            fProxy->endContext();
+            char* proxyContextStorage = (char*) storage + sizeof(Sk3DShaderContext);
+            proxyContext = fProxy->createContext(device, paint, matrix, proxyContextStorage);
+            SkASSERT(proxyContext);
+        } else {
+            proxyContext = NULL;
         }
-        this->INHERITED::endContext();
+        return SkNEW_PLACEMENT_ARGS(storage, Sk3DShaderContext, (*this, device, paint, matrix,
+                                                                 proxyContext));
     }
 
-    virtual void shadeSpan(int x, int y, SkPMColor span[], int count) SK_OVERRIDE {
-        if (fProxy) {
-            fProxy->shadeSpan(x, y, span, count);
+    class Sk3DShaderContext : public SkShader::Context {
+    public:
+        // Takes ownership of proxyContext and calls its destructor.

[scroggo, 2014/04/03 15:35:54]

Once again, I think this is not pure ownership.

[Dominik Grewe, 2014/04/04 10:59:41]

Done.

+        Sk3DShaderContext(const Sk3DShader& shader, const SkBitmap& device, const SkPaint& paint,
+                          const SkMatrix& matrix, SkShader::Context* proxyContext)
+            : INHERITED(shader, device, paint, matrix)
+            , fMask(NULL)
+            , fProxyContext(proxyContext)
+        {
+            if (!fProxyContext) {
+                fPMColor = SkPreMultiplyColor(paint.getColor());
+            }
         }
 
-        if (fMask == NULL) {
-            if (fProxy == NULL) {
-                sk_memset32(span, fPMColor, count);
+        virtual ~Sk3DShaderContext() {
+            if (fProxyContext) {
+                fProxyContext->SkShader::Context::~Context();
             }
-            return;
         }
 
-        SkASSERT(fMask->fBounds.contains(x, y));
-        SkASSERT(fMask->fBounds.contains(x + count - 1, y));
+        void setMask(const SkMask* mask) { fMask = mask; }
 
-        size_t          size = fMask->computeImageSize();
-        const uint8_t*  alpha = fMask->getAddr8(x, y);
-        const uint8_t*  mulp = alpha + size;
-        const uint8_t*  addp = mulp + size;
+        virtual void shadeSpan(int x, int y, SkPMColor span[], int count) SK_OVERRIDE {
+            if (fProxyContext) {
+                fProxyContext->shadeSpan(x, y, span, count);
+            }
 
-        if (fProxy) {
-            for (int i = 0; i < count; i++) {
-                if (alpha[i]) {
-                    SkPMColor c = span[i];
-                    if (c) {
-                        unsigned a = SkGetPackedA32(c);
-                        unsigned r = SkGetPackedR32(c);
-                        unsigned g = SkGetPackedG32(c);
-                        unsigned b = SkGetPackedB32(c);
+            if (fMask == NULL) {
+                if (fProxyContext == NULL) {
+                    sk_memset32(span, fPMColor, count);
+                }
+                return;
+            }
 
+            SkASSERT(fMask->fBounds.contains(x, y));
+            SkASSERT(fMask->fBounds.contains(x + count - 1, y));
+
+            size_t          size = fMask->computeImageSize();
+            const uint8_t*  alpha = fMask->getAddr8(x, y);
+            const uint8_t*  mulp = alpha + size;
+            const uint8_t*  addp = mulp + size;
+
+            if (fProxyContext) {
+                for (int i = 0; i < count; i++) {
+                    if (alpha[i]) {
+                        SkPMColor c = span[i];
+                        if (c) {
+                            unsigned a = SkGetPackedA32(c);
+                            unsigned r = SkGetPackedR32(c);
+                            unsigned g = SkGetPackedG32(c);
+                            unsigned b = SkGetPackedB32(c);
+
+                            unsigned mul = SkAlpha255To256(mulp[i]);
+                            unsigned add = addp[i];
+
+                            r = SkFastMin32(SkAlphaMul(r, mul) + add, a);
+                            g = SkFastMin32(SkAlphaMul(g, mul) + add, a);
+                            b = SkFastMin32(SkAlphaMul(b, mul) + add, a);
+
+                            span[i] = SkPackARGB32(a, r, g, b);
+                        }
+                    } else {
+                        span[i] = 0;
+                    }
+                }
+            } else {    // color
+                unsigned a = SkGetPackedA32(fPMColor);
+                unsigned r = SkGetPackedR32(fPMColor);
+                unsigned g = SkGetPackedG32(fPMColor);
+                unsigned b = SkGetPackedB32(fPMColor);
+                for (int i = 0; i < count; i++) {
+                    if (alpha[i]) {
                         unsigned mul = SkAlpha255To256(mulp[i]);
                         unsigned add = addp[i];
 
-                        r = SkFastMin32(SkAlphaMul(r, mul) + add, a);
-                        g = SkFastMin32(SkAlphaMul(g, mul) + add, a);
-                        b = SkFastMin32(SkAlphaMul(b, mul) + add, a);
-
-                        span[i] = SkPackARGB32(a, r, g, b);
+                        span[i] = SkPackARGB32( a,
+                                        SkFastMin32(SkAlphaMul(r, mul) + add, a),
+                                        SkFastMin32(SkAlphaMul(g, mul) + add, a),
+                                        SkFastMin32(SkAlphaMul(b, mul) + add, a));
+                    } else {
+                        span[i] = 0;
                     }
-                } else {
-                    span[i] = 0;
-                }
-            }
-        } else {    // color
-            unsigned a = SkGetPackedA32(fPMColor);
-            unsigned r = SkGetPackedR32(fPMColor);
-            unsigned g = SkGetPackedG32(fPMColor);
-            unsigned b = SkGetPackedB32(fPMColor);
-            for (int i = 0; i < count; i++) {
-                if (alpha[i]) {
-                    unsigned mul = SkAlpha255To256(mulp[i]);
-                    unsigned add = addp[i];
-
-                    span[i] = SkPackARGB32( a,
-                                    SkFastMin32(SkAlphaMul(r, mul) + add, a),
-                                    SkFastMin32(SkAlphaMul(g, mul) + add, a),
-                                    SkFastMin32(SkAlphaMul(b, mul) + add, a));
-                } else {
-                    span[i] = 0;
                 }
             }
         }
-    }
+    private:
+        // Unowned.
+        const SkMask*       fMask;
+        // Memory is unowned, but we need to call the destructor.
+        SkShader::Context*  fProxyContext;
+        SkPMColor           fPMColor;
+
+        typedef SkShader::Context INHERITED;
+    };
 
 #ifndef SK_IGNORE_TO_STRING
     virtual void toString(SkString* str) const SK_OVERRIDE {
         str->append("Sk3DShader: (");
 
         if (NULL != fProxy) {
             str->append("Proxy: ");
             fProxy->toString(str);
         }
 
         this->INHERITED::toString(str);
 
         str->append(")");
     }
 #endif
 
     SK_DECLARE_PUBLIC_FLATTENABLE_DESERIALIZATION_PROCS(Sk3DShader)
 
 protected:
     Sk3DShader(SkReadBuffer& buffer) : INHERITED(buffer) {
         fProxy = buffer.readShader();
-        fPMColor = buffer.readColor();
-        fMask = NULL;
+        // Leaving this here until we bump the picture version, though this
+        // shader should never be recorded.
+        buffer.readColor();
     }
 
     virtual void flatten(SkWriteBuffer& buffer) const SK_OVERRIDE {
         this->INHERITED::flatten(buffer);
         buffer.writeFlattenable(fProxy);
-        buffer.writeColor(fPMColor);
+        // Leaving this here until we bump the picture version, though this
+        // shader should never be recorded.
+        buffer.writeColor(SkColor());
     }
 
 private:
     SkShader*       fProxy;
-    SkPMColor       fPMColor;
-    const SkMask*   fMask;
 
     typedef SkShader INHERITED;
 };
 
 class Sk3DBlitter : public SkBlitter {
 public:
-    Sk3DBlitter(SkBlitter* proxy, Sk3DShader* shader)
+    Sk3DBlitter(SkBlitter* proxy, Sk3DShader::Sk3DShaderContext* shaderContext)
         : fProxy(proxy)
-        , f3DShader(SkRef(shader))
+        , f3DShaderContext(shaderContext)
     {}
 
     virtual void blitH(int x, int y, int width) {
         fProxy->blitH(x, y, width);
     }
 
     virtual void blitAntiH(int x, int y, const SkAlpha antialias[],
                            const int16_t runs[]) {
         fProxy->blitAntiH(x, y, antialias, runs);
     }
 
     virtual void blitV(int x, int y, int height, SkAlpha alpha) {
         fProxy->blitV(x, y, height, alpha);
     }
 
     virtual void blitRect(int x, int y, int width, int height) {
         fProxy->blitRect(x, y, width, height);
     }
 
     virtual void blitMask(const SkMask& mask, const SkIRect& clip) {
         if (mask.fFormat == SkMask::k3D_Format) {
-            f3DShader->setMask(&mask);
+            f3DShaderContext->setMask(&mask);
 
             ((SkMask*)&mask)->fFormat = SkMask::kA8_Format;
             fProxy->blitMask(mask, clip);
             ((SkMask*)&mask)->fFormat = SkMask::k3D_Format;
 
-            f3DShader->setMask(NULL);
+            f3DShaderContext->setMask(NULL);
         } else {
             fProxy->blitMask(mask, clip);
         }
     }
 
 private:
-    // fProxy is unowned. It will be deleted by SkSmallAllocator.
-    SkBlitter*               fProxy;
-    SkAutoTUnref<Sk3DShader> f3DShader;
+    // Both pointers are unowned. They will be deleted by SkSmallAllocator.
+    SkBlitter*                     fProxy;
+    Sk3DShader::Sk3DShaderContext* f3DShaderContext;
 };
 
 ///////////////////////////////////////////////////////////////////////////////
 
 #include "SkCoreBlitters.h"
 
 static bool just_solid_color(const SkPaint& paint) {
     if (paint.getAlpha() == 0xFF && paint.getColorFilter() == NULL) {
         SkShader* shader = paint.getShader();
-        if (NULL == shader ||
-            (shader->getFlags() & SkShader::kOpaqueAlpha_Flag)) {
+        // FIXME: This is ONLY called BEFORE setContext, when the flags are not
+        // supposed to be meaningful. Do we need flags on the shader as well as
+        // the impl?
+        if (NULL == shader /*||
+            (shader->getFlags() & SkShader::kOpaqueAlpha_Flag)*/) {

[scroggo, 2014/04/03 15:35:54]

Here are the shaders that potentially return kOpaqueAlpha_Flag:

SkBitmapProcShader (after setContext)
SkColorShader (after setContext)
SkTransparentShader - this one is interesting because if it had been called
before setContext, its results would have been undefined! It calls into fDevice
to determine whether to return the flag, but fDevice is not set to NULL in the
constructor, so it could be garbage. We probably never ran into this because
SkTransparentShader is only used by our sample code. (This code also only gets
called if there is an xfermode, lessening the likelihood it might be called.)
SkGradientShader (after setContext)
SkTwoPointConicalGradient *clears* the flag (in setContext)

All of the shaders only set the flag after setContext (further, they could not
know a meaningful value without knowing the paint passed to setContext). So by
removing the check for kOpaqueAlpha_Flag, you're not actually changing the
behavior any. So I think it's okay to remove the comment and make the check just
if (NULL == shader).

[Dominik Grewe, 2014/04/04 10:59:41]

That just shows that getFlags shouldn't be called before setContext, which
cannot happen with this patch anyway.
Done.

             return true;
         }
     }
     return false;
 }
 
 /** By analyzing the paint (with an xfermode), we may decide we can take
     special action. This enum lists our possible actions
  */
 enum XferInterp {
@@ skipping 117 matching lines @@
 
     if (cf) {
         SkASSERT(shader);
         shader = SkNEW_ARGS(SkFilterShader, (shader, cf));
         paint.writable()->setShader(shader)->unref();
         // blitters should ignore the presence/absence of a filter, since
         // if there is one, the shader will take care of it.
     }
 
     /*
-     *  We need to have balanced calls to the shader:
-     *      setContext
-     *      endContext
-     *  We make the first call here, in case it fails we can abort the draw.
-     *  The endContext() call is made by the blitter (assuming setContext did
-     *  not fail) in its destructor.
+     *  We create a SkShader::Context object, and store it on the blitter.
      */
-    if (shader && !shader->setContext(device, *paint, matrix)) {
-        blitter = allocator->createT<SkNullBlitter>();
-        return blitter;
+    SkShader::Context* shaderContext;
+    if (shader) {
+        // Try to create the ShaderContext
+        void* storage = allocator->reserveT<SkShader::Context>(shader->contextSize());
+        shaderContext = shader->createContext(device, *paint, matrix, storage);
+        if (!shaderContext) {
+            allocator->freeLast();
+            blitter = allocator->createT<SkNullBlitter>();
+            return blitter;
+        }
+        SkASSERT(shaderContext);
+        SkASSERT((void*) shaderContext == storage);
+    } else {
+        shaderContext = NULL;
     }
 
 
     switch (device.colorType()) {
         case kAlpha_8_SkColorType:
             if (drawCoverage) {
                 SkASSERT(NULL == shader);
                 SkASSERT(NULL == paint->getXfermode());
                 blitter = allocator->createT<SkA8_Coverage_Blitter>(device, *paint);
             } else if (shader) {
-                blitter = allocator->createT<SkA8_Shader_Blitter>(device, *paint);
+                blitter = allocator->createT<SkA8_Shader_Blitter>(device, *paint, shaderContext);
             } else {
                 blitter = allocator->createT<SkA8_Blitter>(device, *paint);
             }
             break;
 
         case kRGB_565_SkColorType:
-            blitter = SkBlitter_ChooseD565(device, *paint, allocator);
+            blitter = SkBlitter_ChooseD565(device, *paint, shaderContext, allocator);
             break;
 
         case kPMColor_SkColorType:
             if (shader) {
-                blitter = allocator->createT<SkARGB32_Shader_Blitter>(device, *paint);
+                blitter = allocator->createT<SkARGB32_Shader_Blitter>(device, *paint, shaderContext);

[scroggo, 2014/04/03 15:35:54]

nit: 100 chars.

[Dominik Grewe, 2014/04/04 10:59:41]

Done.

             } else if (paint->getColor() == SK_ColorBLACK) {
                 blitter = allocator->createT<SkARGB32_Black_Blitter>(device, *paint);
             } else if (paint->getAlpha() == 0xFF) {
                 blitter = allocator->createT<SkARGB32_Opaque_Blitter>(device, *paint);
             } else {
                 blitter = allocator->createT<SkARGB32_Blitter>(device, *paint);
             }
             break;
 
         default:
             SkDEBUGFAIL("unsupported device config");
             blitter = allocator->createT<SkNullBlitter>();
             break;
     }
 
     if (shader3D) {
         SkBlitter* innerBlitter = blitter;
         // innerBlitter was allocated by allocator, which will delete it.
-        blitter = allocator->createT<Sk3DBlitter>(innerBlitter, shader3D);
+        // We know shaderContext is of type Sk3DShaderContext because it belongs to shader3D.
+        blitter = allocator->createT<Sk3DBlitter>(innerBlitter,
+                static_cast<Sk3DShader::Sk3DShaderContext*>(shaderContext));
     }
     return blitter;
 }
 
 ///////////////////////////////////////////////////////////////////////////////
 
 const uint16_t gMask_0F0F = 0xF0F;
 const uint32_t gMask_00FF00FF = 0xFF00FF;
 
 ///////////////////////////////////////////////////////////////////////////////
 
-SkShaderBlitter::SkShaderBlitter(const SkBitmap& device, const SkPaint& paint)
-        : INHERITED(device) {
-    fShader = paint.getShader();
+SkShaderBlitter::SkShaderBlitter(const SkBitmap& device, const SkPaint& paint,
+                                 SkShader::Context* shaderContext)
+        : INHERITED(device), fShader(paint.getShader()), fShaderContext(shaderContext) {

[scroggo, 2014/04/03 15:35:54]

nit: fShader and fShaderContext should be on their own lines.

[Dominik Grewe, 2014/04/04 10:59:41]

Done.

     SkASSERT(fShader);
-    SkASSERT(fShader->setContextHasBeenCalled());
+    SkASSERT(fShaderContext);
 
     fShader->ref();
-    fShaderFlags = fShader->getFlags();
+    fShaderFlags = fShaderContext->getFlags();
 }
 
 SkShaderBlitter::~SkShaderBlitter() {
-    SkASSERT(fShader->setContextHasBeenCalled());
-    fShader->endContext();
     fShader->unref();
 }
+
+bool SkShaderBlitter::resetShaderContext(const SkBitmap& device, const SkPaint& paint,
+                                         const SkMatrix& matrix) {
+    if (!fShader->validContext(device, paint, matrix)) {
+        return false;
+    }
+
+    // Only destroy the old context if we have a new one. We need to ensure to have a
+    // live context in fShaderContext because the storage is owned by an SkSmallAllocator
+    // outside of this class.
+    // The new context will be of the same size is the old one because we use the same
+    // shader to create it. It is therefore safe to re-use the storage.
+    fShaderContext->SkShader::Context::~Context();
+    fShaderContext = fShader->createContext(device, paint, matrix, (void*)fShaderContext);
+    SkASSERT(fShaderContext);
+
+    return true;
+}