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

src/core/SkBitmapProcShader.h

 
 /*
  * 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.
  */
 
 
 #ifndef SkBitmapProcShader_DEFINED
 #define SkBitmapProcShader_DEFINED
 
 #include "SkShader.h"
 #include "SkBitmapProcState.h"
 #include "SkSmallAllocator.h"
 
 class SkBitmapProcShader : public SkShader {
 public:
     SkBitmapProcShader(const SkBitmap& src, TileMode tx, TileMode ty);
 
     // overrides from SkShader
     virtual bool isOpaque() const SK_OVERRIDE;
-    virtual bool setContext(const SkBitmap&, const SkPaint&, const SkMatrix&) SK_OVERRIDE;
-    virtual void endContext() SK_OVERRIDE;
-    virtual uint32_t getFlags() SK_OVERRIDE { return fFlags; }
-    virtual void shadeSpan(int x, int y, SkPMColor dstC[], int count) SK_OVERRIDE;
-    virtual ShadeProc asAShadeProc(void** ctx) SK_OVERRIDE;
-    virtual void shadeSpan16(int x, int y, uint16_t dstC[], int count) SK_OVERRIDE;
     virtual BitmapType asABitmap(SkBitmap*, SkMatrix*, TileMode*) const SK_OVERRIDE;
 
+    virtual bool validContext(const SkBitmap& device,
+                              const SkPaint& paint,
+                              const SkMatrix& matrix,
+                              SkMatrix* totalInverse = NULL) const SK_OVERRIDE;
+    virtual SkShader::Context* createContext(const SkBitmap&, const SkPaint&,
+                                             const SkMatrix&, void* storage) const SK_OVERRIDE;
+    virtual size_t contextSize() const SK_OVERRIDE;
+
     static bool CanDo(const SkBitmap&, TileMode tx, TileMode ty);
 
     SK_DEVELOPER_TO_STRING()
     SK_DECLARE_PUBLIC_FLATTENABLE_DESERIALIZATION_PROCS(SkBitmapProcShader)
 
 #if SK_SUPPORT_GPU
     GrEffectRef* asNewEffect(GrContext*, const SkPaint&) const SK_OVERRIDE;
 #endif
 
+    class BitmapProcShaderContext : public SkShader::Context {
+    public:
+        // The context takes ownership of the state.

[scroggo, 2014/04/01 18:02:41]

This could use some elaboration - it will call the state's destructor, but it
does not free the memory.

Should this constructor perhaps take a function pointer for destruction so we
don't force this ownership behavior?

[Dominik Grewe, 2014/04/02 18:00:48]

I've extended the comment for now. If we passed a function pointer for
destruction, would NULL mean, the caller will destroy it?

[scroggo, 2014/04/02 20:16:52]

I think so. We might have some examples of such a calling pattern elsewhere...

[Dominik Grewe, 2014/04/03 12:12:59]

On the other hand, we shouldn't create any context objects outside of
createContext. We could try to enforce that. I first thought of making the
classes protected, but that doesn't work because we need to access specific
context classes in certain cases (so they have to be public).
Another option would be to make the constructor protected and make the
corresponding shader class a friend of the context class. Wdyt?

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

I think that makes a lot of sense.

+        BitmapProcShaderContext(const SkBitmapProcShader& shader,
+                                const SkBitmap& device,
+                                const SkPaint& paint,
+                                const SkMatrix& matrix,
+                                SkBitmapProcState* state);
+        virtual ~BitmapProcShaderContext();
+
+        virtual void shadeSpan(int x, int y, SkPMColor dstC[], int count) SK_OVERRIDE;
+        virtual ShadeProc asAShadeProc(void** ctx) SK_OVERRIDE;
+        virtual void shadeSpan16(int x, int y, uint16_t dstC[], int count) SK_OVERRIDE;
+
+        virtual uint32_t getFlags() const SK_OVERRIDE { return fFlags; }
+    private:
+        SkBitmapProcState* fState;
+        uint32_t fFlags;
+
+        typedef SkShader::Context INHERITED;
+    };
+
 protected:
     SkBitmapProcShader(SkReadBuffer& );
     virtual void flatten(SkWriteBuffer&) const SK_OVERRIDE;
 
     SkBitmap          fRawBitmap;   // experimental for RLE encoding
-    SkBitmapProcState fState;
-    uint32_t          fFlags;
+    uint8_t fTileModeX, fTileModeY;
 
 private:
+    bool validInternal(const SkBitmap& device, const SkPaint& paint,
+                       const SkMatrix& matrix, SkMatrix* totalInverse,
+                       SkBitmapProcState* state) const;
+
     typedef SkShader INHERITED;
 };
 
-// Commonly used allocator. It currently is only used to allocate up to 2 objects. The total
-// bytes requested is calculated using one of our large shaders plus the size of an Sk3DBlitter
-// in SkDraw.cpp
-typedef SkSmallAllocator<2, sizeof(SkBitmapProcShader) + sizeof(void*) * 2> SkTBlitterAllocator;
+// Commonly used allocator. It currently is only used to allocate up to 3 objects. The total
+// bytes requested is calculated using one of our large shaders, its context (context size
+// is multiplied by 3 because contexts can reference other contexts) plus the size of
+// an Sk3DBlitter in SkDraw.cpp
+typedef SkSmallAllocator<3, sizeof(SkBitmapProcShader) +

[scroggo, 2014/04/01 18:02:41]

Won't this need to be 4 to account for the SkBitmapProcState?

[scroggo, 2014/04/02 16:03:14]

My mistake; you put the SkBitmapProcState in with the BitmapProcShaderContext.
On the other hand, if you did separate it (in which case this would need to be
4), you could eliminate the contract of BitmapProcShaderContext: that it must
call SkBitmapProcState's destructor (but not free its memory).

[Dominik Grewe, 2014/04/02 18:00:48]

We don't have access to the SkSmallAllocator when we create SkBitmapProcState
(in createContext()), that's why I put it together with the context.

[scroggo, 2014/04/02 20:16:52]

Ah, of course. Nvm.

+                            3 * sizeof(SkBitmapProcShader::BitmapProcShaderContext) +
+                            sizeof(SkBitmapProcState) +
+                            sizeof(void*) * 2> SkTBlitterAllocator;
 
 // If alloc is non-NULL, it will be used to allocate the returned SkShader, and MUST outlive
 // the SkShader.
 SkShader* CreateBitmapShader(const SkBitmap& src, SkShader::TileMode, SkShader::TileMode,
                              SkTBlitterAllocator* alloc);
 
 #endif