Let them eat GrEffectRef.

Changes the remaining existing code that operates on naked GrEffects to GrEffectRef.
Review URL: https://codereview.appspot.com/7124058

git-svn-id: http://skia.googlecode.com/svn/trunk@7321 2bbb7eff-a529-9590-31e7-b0007b416f81

4 files changed, 55 insertions, 26 deletions

diff --git a/include/gpu/GrBackendEffectFactory.h b/include/gpu/GrBackendEffectFactory.h
index 437bcdd6f2..a2913870b4 100644
--- a/include/gpu/GrBackendEffectFactory.h
+++ b/include/gpu/GrBackendEffectFactory.h
@@ -23,7 +23,7 @@
     of GrGLEffect.
  */
 
-class GrEffect;
+class GrEffectRef;
 class GrEffectStage;
 class GrGLEffect;
 class GrGLCaps;
@@ -44,7 +44,7 @@ public:
     };
 
     virtual EffectKey glEffectKey(const GrEffectStage&, const GrGLCaps&) const = 0;
-    virtual GrGLEffect* createGLInstance(const GrEffect&) const = 0;
+    virtual GrGLEffect* createGLInstance(const GrEffectRef&) const = 0;
 
     bool operator ==(const GrBackendEffectFactory& b) const {
         return fEffectClassID == b.fEffectClassID;
diff --git a/include/gpu/GrEffect.h b/include/gpu/GrEffect.h
index 507bd5736b..0e6aab027e 100644
--- a/include/gpu/GrEffect.h
+++ b/include/gpu/GrEffect.h
@@ -34,6 +34,9 @@ public:
     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);
 
@@ -58,8 +61,11 @@ private:
     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 outside of a GrEffectRef, we privately
+    inherit from GrRefCnt to help prevent accidental direct ref'ing/unref'ing of effects.
   */
-class GrEffect : public GrRefCnt {
+class GrEffect : private GrRefCnt {
 public:
     SK_DECLARE_INST_COUNT(GrEffect)
 
@@ -113,16 +119,16 @@ public:
         computed by the GrBackendEffectFactory:
             effectA.getFactory().glEffectKey(effectA) == effectB.getFactory().glEffectKey(effectB).
      */
-    bool isEqual(const GrEffect& other) const {
-        if (&this->getFactory() != &other.getFactory()) {
+    bool isEqual(const GrEffectRef& other) const {
+        if (&this->getFactory() != &other->getFactory()) {
             return false;
         }
         bool result = this->onIsEqual(other);
 #if GR_DEBUG
         if (result) {
-            GrAssert(this->numTextures() == other.numTextures());
+            GrAssert(this->numTextures() == other->numTextures());
             for (int i = 0; i < this->numTextures(); ++i) {
-                GrAssert(*fTextureAccesses[i] == *other.fTextureAccesses[i]);
+                GrAssert(*fTextureAccesses[i] == *other->fTextureAccesses[i]);
             }
         }
 #endif
@@ -154,6 +160,11 @@ public:
     void* operator new(size_t size);
     void operator delete(void* target);
 
+    /** These use non-standard names because GrEffects should only be ref'ed an unref'ed deep in
+        the bowels. Rendering code should use GrEffectRef. */
+    void addRef() { this->ref(); }
+    void subRef() { this->unref(); }
+
 protected:
     /**
      * Subclasses call this from their constructor to register GrTextureAcceses. The effect subclass
@@ -164,7 +175,8 @@ protected:
 
     GrEffect() : fEffectRef(NULL) {};
 
-    /** This should be called by GrEffect subclass factories */
+    /** This should be called by GrEffect subclass factories. See the comment on AutoEffectUnref for
+        an example factory function. */
     static GrEffectRef* CreateEffectRef(GrEffect* effect) {
         if (NULL == effect->fEffectRef) {
             effect->fEffectRef = SkNEW_ARGS(GrEffectRef, (effect));
@@ -175,16 +187,41 @@ protected:
         return effect->fEffectRef;
     }
 
+    /** Helper used in subclass factory functions to unref the effect after it has been wrapped in a
+        GrEffectRef. E.g.:
+
+        class EffectSubclass : public GrEffect {
+        public:
+            GrEffectRef* Create(ParamType1 param1, ParamType2 param2, ...) {
+                AutoEffectUnref effect(SkNEW_ARGS(EffectSubclass, (param1, param2, ...)));
+                return CreateEffectRef(effect);
+            }
+     */
+    class AutoEffectUnref {
+    public:
+        AutoEffectUnref(GrEffect* effect) : fEffect(effect) { }
+        ~AutoEffectUnref() { fEffect->subRef(); }
+        operator GrEffect*() { return fEffect; }
+    private:
+        GrEffect* fEffect;
+    };
+
+    /** Helper for getting the GrEffect out of a GrEffectRef and down-casting to a GrEffect subclass
+      */
+    template <typename T>
+    static const T& CastEffect(const GrEffectRef& effectRef) {
+        GrAssert(NULL != effectRef.get());
+        return *static_cast<const T*>(effectRef.get());
+    }
+
 private:
 
     /** Subclass implements this to support isEqual(). It will only be called if it is known that
-        the two effects are of the same subclass (i.e. they return the same object
-        from getFactory()).*/
-    virtual bool onIsEqual(const GrEffect& other) const = 0;
+        the two effects are of the same subclass (i.e. they return the same object from
+        getFactory()).*/
+    virtual bool onIsEqual(const GrEffectRef& other) const = 0;
 
-    void EffectRefDestroyed() {
-        fEffectRef = NULL;
-    }
+    void EffectRefDestroyed() { fEffectRef = NULL; }
 
     friend class GrEffectRef; // to call GrEffectRef destroyed
 
diff --git a/include/gpu/GrEffectStage.h b/include/gpu/GrEffectStage.h
index 0060152815..c09cd450a7 100644
--- a/include/gpu/GrEffectStage.h
+++ b/include/gpu/GrEffectStage.h
@@ -39,7 +39,7 @@ public:
             return false;
         }
 
-        if (!this->getEffect()->isEqual(*other.getEffect())) {
+        if (!(*this->getEffect())->isEqual(*other.getEffect())) {
             return false;
         }
 
@@ -112,14 +112,7 @@ public:
         return EffectRef;
     }
 
-    // TODO: Push GrEffectRef deeper and make this getter return it rather than GrEffect.
-    const GrEffect* getEffect() const {
-        if (NULL != fEffectRef) {
-            return fEffectRef->get();
-        } else {
-            return NULL;
-        }
-    }
+    const GrEffectRef* getEffect() const { return fEffectRef; }
 
 private:
     SkMatrix            fCoordChangeMatrix;
@@ -129,4 +122,3 @@ private:
 };
 
 #endif
-
diff --git a/include/gpu/GrTBackendEffectFactory.h b/include/gpu/GrTBackendEffectFactory.h
index 52dbc64dd8..7ea7e39ebc 100644
--- a/include/gpu/GrTBackendEffectFactory.h
+++ b/include/gpu/GrTBackendEffectFactory.h
@@ -34,7 +34,7 @@ public:
                                   const GrGLCaps& caps) const SK_OVERRIDE {
         GrAssert(kIllegalEffectClassID != fEffectClassID);
         EffectKey effectKey = GLEffect::GenKey(stage, caps);
-        EffectKey textureKey = GLEffect::GenTextureKey(*stage.getEffect(), caps);
+        EffectKey textureKey = GLEffect::GenTextureKey(stage.getEffect(), caps);
 #if GR_DEBUG
         static const EffectKey kIllegalIDMask = (uint16_t) (~((1U << kEffectKeyBits) - 1));
         GrAssert(!(kIllegalIDMask & effectKey));
@@ -48,7 +48,7 @@ public:
     /** Returns a new instance of the appropriate *GL* implementation class
         for the given GrEffect; caller is responsible for deleting
         the object. */
-    virtual GLEffect* createGLInstance(const GrEffect& effect) const SK_OVERRIDE {
+    virtual GLEffect* createGLInstance(const GrEffectRef& effect) const SK_OVERRIDE {
         return SkNEW_ARGS(GLEffect, (*this, effect));
     }