Revert of Make GrDrawState and GrPaint take GrEffect* instead of GrEffectRef*. (https://codereview.chromium.org/377503004/)

Reason for revert:
broke linux builders

Original issue's description:
> Make GrDrawState and GrPaint take GrEffect* instead of GrEffectRef*.
>
> Make Sk-effect virtuals produce GrEffect* rather than GrEffectRef*
>
> Make GrEffectRef a typedef for GrEffect.
>
> Committed: https://skia.googlesource.com/skia/+/2011fe9cdfa63b83489a146cea6a724cede352c8

R=robertphillips@google.com, bsalomon@google.com
TBR=bsalomon@google.com, robertphillips@google.com
NOTREECHECKS=true
NOTRY=true

Author: reed@google.com

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

5 files changed, 171 insertions, 60 deletions

diff --git a/include/gpu/GrBackendEffectFactory.h b/include/gpu/GrBackendEffectFactory.h
index af8d5c76c5..0fc981cbe9 100644
--- a/include/gpu/GrBackendEffectFactory.h
+++ b/include/gpu/GrBackendEffectFactory.h
@@ -23,6 +23,7 @@
     of GrGLEffect.
  */
 
+class GrEffectRef;
 class GrGLEffect;
 class GrGLCaps;
 class GrDrawEffect;
diff --git a/include/gpu/GrEffect.h b/include/gpu/GrEffect.h
index 978ad3a6a6..5fed5329fb 100644
--- a/include/gpu/GrEffect.h
+++ b/include/gpu/GrEffect.h
@@ -21,17 +21,66 @@ class GrEffect;
 class GrVertexEffect;
 class SkString;
 
+/**
+ * A Wrapper class for GrEffect. Its ref-count will track owners that may use effects to enqueue
+ * new draw operations separately from ownership within a deferred drawing queue. When the
+ * GrEffectRef ref count reaches zero the scratch GrResources owned by the effect can be recycled
+ * in service of later draws. However, the deferred draw queue may still own direct references to
+ * the underlying GrEffect.
+ *
+ * GrEffectRefs created by new are placed in a per-thread managed pool. The pool is destroyed when
+ * the thread ends. Therefore, all dynamically allocated GrEffectRefs must be unreffed before thread
+ * termination.
+ */
+class GrEffectRef : public SkRefCnt {
+public:
+    SK_DECLARE_INST_COUNT(GrEffectRef);
+    virtual ~GrEffectRef();
+
+    GrEffect* get() { return fEffect; }
+    const GrEffect* get() const { return fEffect; }
+
+    const GrEffect* operator-> () { return fEffect; }
+    const GrEffect* operator-> () const { return fEffect; }
+
+    void* operator new(size_t size);
+    void operator delete(void* target);
+
+    void* operator new(size_t size, void* placement) {
+        return ::operator new(size, placement);
+    }
+    void operator delete(void* target, void* placement) {
+        ::operator delete(target, placement);
+    }
+
+private:
+    friend class GrEffect; // to construct these
+
+    explicit GrEffectRef(GrEffect* effect);
+
+    GrEffect* fEffect;
+
+    typedef SkRefCnt INHERITED;
+};
+
 /** Provides custom vertex shader, fragment shader, uniform data for a particular stage of the
     Ganesh shading pipeline.
     Subclasses must have a function that produces a human-readable name:
         static const char* Name();
     GrEffect objects *must* be immutable: after being constructed, their fields may not change.
 
-    Dynamically allocated GrEffects are managed by a per-thread memory pool. The ref count of an
-    effect must reach 0 before the thread terminates and the pool is destroyed. To create a static
-    effect use the macro GR_CREATE_STATIC_EFFECT declared below.
+    GrEffect subclass objects should be created by factory functions that return GrEffectRef.
+    There is no public way to wrap a GrEffect in a GrEffectRef. Thus, a factory should be a static
+    member function of a GrEffect subclass.
+
+    Because almost no code should ever handle a GrEffect directly outside of a GrEffectRef, we
+    privately inherit from SkRefCnt to help prevent accidental direct ref'ing/unref'ing of effects.
+
+    Dynamically allocated GrEffects and their corresponding GrEffectRefs are managed by a per-thread
+    memory pool. The ref count of an effect must reach 0 before the thread terminates and the pool
+    is destroyed. To create a static effect use the macro GR_CREATE_STATIC_EFFECT declared below.
   */
-class GrEffect : public SkRefCnt {
+class GrEffect : private SkRefCnt {
 public:
     SK_DECLARE_INST_COUNT(GrEffect)
 
@@ -75,17 +124,8 @@ public:
         computed by the GrBackendEffectFactory:
             effectA.getFactory().glEffectKey(effectA) == effectB.getFactory().glEffectKey(effectB).
      */
-    bool isEqual(const GrEffect& other) const {
-        if (&this->getFactory() != &other.getFactory()) {
-            return false;
-        }
-        bool result = this->onIsEqual(other);
-#ifdef SK_DEBUG
-        if (result) {
-            this->assertEquality(other);
-        }
-#endif
-        return result;
+    bool isEqual(const GrEffectRef& other) const {
+        return this->isEqual(*other.get());
     }
 
     /** Human-meaningful string to identify this effect; may be embedded
@@ -145,6 +185,24 @@ public:
         ::operator delete(target, placement);
     }
 
+    /** These functions are used when recording effects into a deferred drawing queue. The inc call
+        keeps the effect alive outside of GrEffectRef while allowing any resources owned by the
+        effect to be returned to the cache for reuse. The dec call must balance the inc call. */
+    void incDeferredRefCounts() const {
+        this->ref();
+        int count = fTextureAccesses.count();
+        for (int t = 0; t < count; ++t) {
+            fTextureAccesses[t]->getTexture()->incDeferredRefCount();
+        }
+    }
+    void decDeferredRefCounts() const {
+        int count = fTextureAccesses.count();
+        for (int t = 0; t < count; ++t) {
+            fTextureAccesses[t]->getTexture()->decDeferredRefCount();
+        }
+        this->unref();
+    }
+
 protected:
     /**
      * Subclasses call this from their constructor to register coordinate transformations. The
@@ -167,18 +225,32 @@ protected:
         : fWillReadDstColor(false)
         , fWillReadFragmentPosition(false)
         , fWillUseInputColor(true)
-        , fHasVertexCode(false) {}
+        , fHasVertexCode(false)
+        , fEffectRef(NULL) {}
 
     /** This should be called by GrEffect subclass factories. See the comment on AutoEffectUnref for
         an example factory function. */
-    static GrEffect* CreateEffectRef(GrEffect* effect) {
-        return SkRef(effect);
+    static GrEffectRef* CreateEffectRef(GrEffect* effect) {
+        if (NULL == effect->fEffectRef) {
+            effect->fEffectRef = SkNEW_ARGS(GrEffectRef, (effect));
+        } else {
+            effect->fEffectRef->ref();
+        }
+        return effect->fEffectRef;
     }
 
-    static const GrEffect* CreateEffectRef(const GrEffect* effect) {
+    static const GrEffectRef* CreateEffectRef(const GrEffect* effect) {
         return CreateEffectRef(const_cast<GrEffect*>(effect));
     }
 
+    /** Used by GR_CREATE_STATIC_EFFECT below */
+    static GrEffectRef* CreateStaticEffectRef(void* refStorage, GrEffect* effect) {
+        SkASSERT(NULL == effect->fEffectRef);
+        effect->fEffectRef = SkNEW_PLACEMENT_ARGS(refStorage, GrEffectRef, (effect));
+        return effect->fEffectRef;
+    }
+
+
     /** Helper used in subclass factory functions to unref the effect after it has been wrapped in a
         GrEffectRef. E.g.:
 
@@ -189,7 +261,14 @@ protected:
                 return CreateEffectRef(effect);
             }
      */
-    typedef SkAutoTUnref<GrEffect> AutoEffectUnref;
+    class AutoEffectUnref {
+    public:
+        AutoEffectUnref(GrEffect* effect) : fEffect(effect) { }
+        ~AutoEffectUnref() { fEffect->unref(); }
+        operator GrEffect*() { return fEffect; }
+    private:
+        GrEffect* fEffect;
+    };
 
     /** Helper for getting the GrEffect out of a GrEffectRef and down-casting to a GrEffect subclass
       */
@@ -220,6 +299,19 @@ protected:
     void setWillNotUseInputColor() { fWillUseInputColor = false; }
 
 private:
+    bool isEqual(const GrEffect& other) const {
+        if (&this->getFactory() != &other.getFactory()) {
+            return false;
+        }
+        bool result = this->onIsEqual(other);
+#ifdef SK_DEBUG
+        if (result) {
+            this->assertEquality(other);
+        }
+#endif
+        return result;
+    }
+
     SkDEBUGCODE(void assertEquality(const GrEffect& other) const;)
 
     /** Subclass implements this to support isEqual(). It will only be called if it is known that
@@ -227,6 +319,12 @@ private:
         getFactory()).*/
     virtual bool onIsEqual(const GrEffect& other) const = 0;
 
+    void EffectRefDestroyed() { fEffectRef = NULL; }
+
+    friend class GrEffectRef;    // to call EffectRefDestroyed()
+    friend class GrEffectStage;  // to rewrap GrEffect in GrEffectRef when restoring an effect-stage
+                                 // from deferred state, to call isEqual on naked GrEffects, and
+                                 // to inc/dec deferred ref counts.
     friend class GrVertexEffect; // to set fHasVertexCode and build fVertexAttribTypes.
 
     SkSTArray<4, const GrCoordTransform*, true>  fCoordTransforms;
@@ -236,20 +334,32 @@ private:
     bool                                         fWillReadFragmentPosition;
     bool                                         fWillUseInputColor;
     bool                                         fHasVertexCode;
+    GrEffectRef*                                 fEffectRef;
 
     typedef SkRefCnt INHERITED;
 };
 
-typedef GrEffect GrEffectRef;
+inline GrEffectRef::GrEffectRef(GrEffect* effect) {
+    SkASSERT(NULL != effect);
+    effect->ref();
+    fEffect = effect;
+}
 
 /**
  * This creates an effect outside of the effect memory pool. The effect's destructor will be called
- * at global destruction time. NAME will be the name of the created GrEffect.
+ * at global destruction time. NAME will be the name of the created GrEffectRef.
  */
 #define GR_CREATE_STATIC_EFFECT(NAME, EFFECT_CLASS, ARGS)                                         \
-static SkAlignedSStorage<sizeof(EFFECT_CLASS)> g_##NAME##_Storage;                                \
-static GrEffect* NAME SkNEW_PLACEMENT_ARGS(g_##NAME##_Storage.get(), EFFECT_CLASS, ARGS);         \
-static SkAutoTDestroy<GrEffect> NAME##_ad(NAME);
+enum {                                                                                            \
+    k_##NAME##_EffectRefOffset = GR_CT_ALIGN_UP(sizeof(EFFECT_CLASS), 8),                         \
+    k_##NAME##_StorageSize = k_##NAME##_EffectRefOffset + sizeof(GrEffectRef)                     \
+};                                                                                                \
+static SkAlignedSStorage<k_##NAME##_StorageSize> g_##NAME##_Storage;                              \
+static void* NAME##_RefLocation = (char*)g_##NAME##_Storage.get() + k_##NAME##_EffectRefOffset;   \
+static GrEffect* NAME##_Effect SkNEW_PLACEMENT_ARGS(g_##NAME##_Storage.get(), EFFECT_CLASS, ARGS);\
+static SkAutoTDestroy<GrEffect> NAME##_ad(NAME##_Effect);                                         \
+static GrEffectRef* NAME(GrEffect::CreateStaticEffectRef(NAME##_RefLocation, NAME##_Effect));     \
+static SkAutoTDestroy<GrEffectRef> NAME##_Ref_ad(NAME)
 
 
 #endif
diff --git a/include/gpu/GrEffectStage.h b/include/gpu/GrEffectStage.h
index fb0620d6e6..84ccbbf091 100644
--- a/include/gpu/GrEffectStage.h
+++ b/include/gpu/GrEffectStage.h
@@ -20,8 +20,8 @@
 
 class GrEffectStage {
 public:
-    explicit GrEffectStage(const GrEffect* effect, int attrIndex0 = -1, int attrIndex1 = -1)
-    : fEffect(SkRef(effect)) {
+    explicit GrEffectStage(const GrEffectRef* effectRef, int attrIndex0 = -1, int attrIndex1 = -1)
+    : fEffectRef(SkRef(effectRef)) {
         fCoordChangeMatrixSet = false;
         fVertexAttribIndices[0] = attrIndex0;
         fVertexAttribIndices[1] = attrIndex1;
@@ -36,14 +36,14 @@ public:
         if (other.fCoordChangeMatrixSet) {
             fCoordChangeMatrix = other.fCoordChangeMatrix;
         }
-        fEffect.reset(SkRef(other.fEffect.get()));
+        fEffectRef.reset(SkRef(other.fEffectRef.get()));
         memcpy(fVertexAttribIndices, other.fVertexAttribIndices, sizeof(fVertexAttribIndices));
         return *this;
     }
 
     bool operator== (const GrEffectStage& other) const {
-        SkASSERT(NULL != fEffect.get());
-        SkASSERT(NULL != other.fEffect.get());
+        SkASSERT(NULL != fEffectRef.get());
+        SkASSERT(NULL != other.fEffectRef.get());
 
         if (!this->getEffect()->isEqual(*other.getEffect())) {
             return false;
@@ -81,7 +81,7 @@ public:
     private:
         bool fCoordChangeMatrixSet;
         SkMatrix fCoordChangeMatrix;
-        SkDEBUGCODE(mutable SkAutoTUnref<const GrEffect> fEffect;)
+        SkDEBUGCODE(mutable SkAutoTUnref<const GrEffectRef> fEffectRef;)
 
         friend class GrEffectStage;
     };
@@ -97,9 +97,9 @@ public:
         if (fCoordChangeMatrixSet) {
             savedCoordChange->fCoordChangeMatrix = fCoordChangeMatrix;
         }
-        SkASSERT(NULL == savedCoordChange->fEffect.get());
-        SkDEBUGCODE(SkRef(fEffect.get());)
-        SkDEBUGCODE(savedCoordChange->fEffect.reset(fEffect.get());)
+        SkASSERT(NULL == savedCoordChange->fEffectRef.get());
+        SkDEBUGCODE(SkRef(fEffectRef.get());)
+        SkDEBUGCODE(savedCoordChange->fEffectRef.reset(fEffectRef.get());)
     }
 
     /**
@@ -110,8 +110,8 @@ public:
         if (fCoordChangeMatrixSet) {
             fCoordChangeMatrix = savedCoordChange.fCoordChangeMatrix;
         }
-        SkASSERT(savedCoordChange.fEffect.get() == fEffect);
-        SkDEBUGCODE(savedCoordChange.fEffect.reset(NULL);)
+        SkASSERT(savedCoordChange.fEffectRef.get() == fEffectRef);
+        SkDEBUGCODE(savedCoordChange.fEffectRef.reset(NULL);)
     }
 
     /**
@@ -126,15 +126,15 @@ public:
         }
     }
 
-    const GrEffect* getEffect() const { return fEffect.get(); }
+    const GrEffect* getEffect() const { return fEffectRef.get()->get(); }
 
     const int* getVertexAttribIndices() const { return fVertexAttribIndices; }
-    int getVertexAttribIndexCount() const { return fEffect->numVertexAttribs(); }
+    int getVertexAttribIndexCount() const { return fEffectRef->get()->numVertexAttribs(); }
 
 private:
     bool                                fCoordChangeMatrixSet;
     SkMatrix                            fCoordChangeMatrix;
-    SkAutoTUnref<const GrEffect>        fEffect;
+    SkAutoTUnref<const GrEffectRef>     fEffectRef;
     int                                 fVertexAttribIndices[2];
 };
 
diff --git a/include/gpu/GrEffectUnitTest.h b/include/gpu/GrEffectUnitTest.h
index ffc64f2b82..f71ab54e7e 100644
--- a/include/gpu/GrEffectUnitTest.h
+++ b/include/gpu/GrEffectUnitTest.h
@@ -32,26 +32,26 @@ const SkMatrix& TestMatrix(SkRandom*);
 #if SK_ALLOW_STATIC_GLOBAL_INITIALIZERS
 
 class GrContext;
-class GrEffect;
+class GrEffectRef;
 class GrTexture;
 
 class GrEffectTestFactory : SkNoncopyable {
 public:
 
-    typedef GrEffect* (*CreateProc)(SkRandom*,
-                                    GrContext*,
-                                    const GrDrawTargetCaps& caps,
-                                    GrTexture* dummyTextures[]);
+    typedef GrEffectRef* (*CreateProc)(SkRandom*,
+                                       GrContext*,
+                                       const GrDrawTargetCaps& caps,
+                                       GrTexture* dummyTextures[]);
 
     GrEffectTestFactory(CreateProc createProc) {
         fCreateProc = createProc;
         GetFactories()->push_back(this);
     }
 
-    static GrEffect* CreateStage(SkRandom* random,
-                                 GrContext* context,
-                                 const GrDrawTargetCaps& caps,
-                                 GrTexture* dummyTextures[]) {
+    static GrEffectRef* CreateStage(SkRandom* random,
+                                    GrContext* context,
+                                    const GrDrawTargetCaps& caps,
+                                    GrTexture* dummyTextures[]) {
         uint32_t idx = random->nextRangeU(0, GetFactories()->count() - 1);
         GrEffectTestFactory* factory = (*GetFactories())[idx];
         return factory->fCreateProc(random, context, caps, dummyTextures);
@@ -67,10 +67,10 @@ private:
  */
 #define GR_DECLARE_EFFECT_TEST                                                      \
     static GrEffectTestFactory gTestFactory;                                        \
-    static GrEffect* TestCreate(SkRandom*,                                          \
-                                GrContext*,                                         \
-                                const GrDrawTargetCaps&,                            \
-                                GrTexture* dummyTextures[2])
+    static GrEffectRef* TestCreate(SkRandom*,                                       \
+                                   GrContext*,                                      \
+                                   const GrDrawTargetCaps&,                         \
+                                   GrTexture* dummyTextures[2])
 
 /** GrEffect subclasses should insert this macro in their implementation file. They must then
  *  also implement this static function:
@@ -91,10 +91,10 @@ private:
 // The unit test relies on static initializers. Just declare the TestCreate function so that
 // its definitions will compile.
 #define GR_DECLARE_EFFECT_TEST                                                      \
-    static GrEffect* TestCreate(SkRandom*,                                          \
-                                GrContext*,                                         \
-                                const GrDrawTargetCaps&,                            \
-                                GrTexture* dummyTextures[2])
+    static GrEffectRef* TestCreate(SkRandom*,                                       \
+                                   GrContext*,                                      \
+                                   const GrDrawTargetCaps&,                         \
+                                   GrTexture* dummyTextures[2])
 #define GR_DEFINE_EFFECT_TEST(X)
 
 #endif // !SK_ALLOW_STATIC_GLOBAL_INITIALIZERS
diff --git a/include/gpu/GrPaint.h b/include/gpu/GrPaint.h
index 20dec35b5b..db9b8cc2a6 100644
--- a/include/gpu/GrPaint.h
+++ b/include/gpu/GrPaint.h
@@ -87,9 +87,9 @@ public:
     /**
      * Appends an additional color effect to the color computation.
      */
-    const GrEffect* addColorEffect(const GrEffect* effect, int attr0 = -1, int attr1 = -1) {
+    const GrEffectRef* addColorEffect(const GrEffectRef* effect, int attr0 = -1, int attr1 = -1) {
         SkASSERT(NULL != effect);
-        if (!effect->willUseInputColor()) {
+        if (!(*effect)->willUseInputColor()) {
             fColorStages.reset();
         }
         SkNEW_APPEND_TO_TARRAY(&fColorStages, GrEffectStage, (effect, attr0, attr1));
@@ -99,9 +99,9 @@ public:
     /**
      * Appends an additional coverage effect to the coverage computation.
      */
-    const GrEffect* addCoverageEffect(const GrEffect* effect, int attr0 = -1, int attr1 = -1) {
+    const GrEffectRef* addCoverageEffect(const GrEffectRef* effect, int attr0 = -1, int attr1 = -1) {
         SkASSERT(NULL != effect);
-        if (!effect->willUseInputColor()) {
+        if (!(*effect)->willUseInputColor()) {
             fCoverageStages.reset();
         }
         SkNEW_APPEND_TO_TARRAY(&fCoverageStages, GrEffectStage, (effect, attr0, attr1));