Don't take a ref on GP in AutoEffectRestore.

BUG=skia:2889
R=joshualitt@chromium.org, robertphillips@google.com

Author: bsalomon@google.com

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

5 files changed, 30 insertions, 13 deletions

diff --git a/include/gpu/GrGpuResource.h b/include/gpu/GrGpuResource.h
index d117c42533..c81e022618 100644
--- a/include/gpu/GrGpuResource.h
+++ b/include/gpu/GrGpuResource.h
@@ -172,8 +172,8 @@ public:
     const GrResourceKey& getScratchKey() const { return fScratchKey; }
 
     /**
-     * Gets an id that is unique for this GrCacheable object. It is static in that it does
-     * not change when the content of the GrCacheable object changes. This will never return
+     * Gets an id that is unique for this GrGpuResource object. It is static in that it does
+     * not change when the content of the GrGpuResource object changes. This will never return
      * 0.
      */
     uint32_t getUniqueID() const { return fUniqueID; }
diff --git a/include/gpu/GrProgramElement.h b/include/gpu/GrProgramElement.h
index 6fdc98daa9..5f88a2a68f 100644
--- a/include/gpu/GrProgramElement.h
+++ b/include/gpu/GrProgramElement.h
@@ -49,6 +49,11 @@ public:
         }
     }
 
+    /**
+     * Gets an id that is unique for this GrProgramElement object. This will never return 0.
+     */
+    uint32_t getUniqueID() const { return fUniqueID; }
+
     void validate() const {
 #ifdef SK_DEBUG
         SkASSERT(fRefCnt >= 0);
@@ -58,7 +63,7 @@ public:
     }
 
 protected:
-    GrProgramElement() : fRefCnt(1), fPendingExecutions(0) {}
+    GrProgramElement() : fRefCnt(1), fPendingExecutions(0), fUniqueID(CreateUniqueID()) {}
 
     /** Subclasses registers their resources using this function. It is assumed the GrProgramResouce
         is and will remain owned by the subclass and this function will retain a raw ptr. Once a
@@ -69,6 +74,8 @@ protected:
     }
 
 private:
+    static uint32_t CreateUniqueID();
+
     void convertRefToPendingExecution() const;
 
     void completedExecution() const;
@@ -76,6 +83,7 @@ private:
     mutable int32_t fRefCnt;
     // Count of deferred executions not yet issued to the 3D API.
     mutable int32_t fPendingExecutions;
+    uint32_t        fUniqueID;
 
     SkSTArray<4, const GrProgramResource*, true> fProgramResources;
 
diff --git a/src/gpu/GrDrawState.cpp b/src/gpu/GrDrawState.cpp
index 6acc202065..f2028eaf94 100644
--- a/src/gpu/GrDrawState.cpp
+++ b/src/gpu/GrDrawState.cpp
@@ -300,12 +300,12 @@ GrDrawState::AutoVertexAttribRestore::AutoVertexAttribRestore(
 void GrDrawState::AutoRestoreEffects::set(GrDrawState* ds) {
     if (NULL != fDrawState) {
         // See the big comment on the class definition about GPs.
-        if (NULL != fOriginalGP) {
-            SkASSERT(fDrawState->getGeometryProcessor()->getEffect() == fOriginalGP);
-            fOriginalGP->unref();
-            fOriginalGP = NULL;
-        } else {
+        if (SK_InvalidUniqueID == fOriginalGPID) {
             fDrawState->fGeometryProcessor.reset(NULL);
+        } else {
+            SkASSERT(fDrawState->getGeometryProcessor()->getEffect()->getUniqueID() ==
+                     fOriginalGPID);
+            fOriginalGPID = SK_InvalidUniqueID;
         }
 
         int m = fDrawState->numColorStages() - fColorEffectCnt;
@@ -322,9 +322,9 @@ void GrDrawState::AutoRestoreEffects::set(GrDrawState* ds) {
     }
     fDrawState = ds;
     if (NULL != ds) {
-        SkASSERT(NULL == fOriginalGP);
+        SkASSERT(SK_InvalidUniqueID == fOriginalGPID);
         if (NULL != ds->getGeometryProcessor()) {
-            fOriginalGP = SkRef(ds->getGeometryProcessor()->getEffect());
+            fOriginalGPID = ds->getGeometryProcessor()->getEffect()->getUniqueID();
         }
         fColorEffectCnt = ds->numColorStages();
         fCoverageEffectCnt = ds->numCoverageStages();
diff --git a/src/gpu/GrDrawState.h b/src/gpu/GrDrawState.h
index 7b5f150f4c..6223f5388b 100644
--- a/src/gpu/GrDrawState.h
+++ b/src/gpu/GrDrawState.h
@@ -256,13 +256,13 @@ public:
     public:
         AutoRestoreEffects() 
             : fDrawState(NULL)
-            , fOriginalGP(NULL)
+            , fOriginalGPID(SK_InvalidUniqueID)
             , fColorEffectCnt(0)
             , fCoverageEffectCnt(0) {}
 
         AutoRestoreEffects(GrDrawState* ds)
             : fDrawState(NULL)
-            , fOriginalGP(NULL)
+            , fOriginalGPID(SK_InvalidUniqueID)
             , fColorEffectCnt(0)
             , fCoverageEffectCnt(0) {
             this->set(ds);
@@ -276,7 +276,7 @@ public:
 
     private:
         GrDrawState*    fDrawState;
-        const GrEffect* fOriginalGP;
+        uint32_t        fOriginalGPID;
         int             fColorEffectCnt;
         int             fCoverageEffectCnt;
     };
diff --git a/src/gpu/GrProgramElement.cpp b/src/gpu/GrProgramElement.cpp
index 70d5339e90..20a957f7c7 100644
--- a/src/gpu/GrProgramElement.cpp
+++ b/src/gpu/GrProgramElement.cpp
@@ -8,6 +8,15 @@
 #include "GrProgramElement.h"
 #include "GrProgramResource.h"
 
+uint32_t GrProgramElement::CreateUniqueID() {
+    static int32_t gUniqueID = SK_InvalidUniqueID;
+    uint32_t id;
+    do {
+        id = static_cast<uint32_t>(sk_atomic_inc(&gUniqueID) + 1);
+    } while (id == SK_InvalidUniqueID);
+    return id;
+}
+
 void GrProgramElement::convertRefToPendingExecution() const {
     // This function makes it so that all the GrProgramResources own a single ref to their
     // underlying GrGpuResource if there are any refs to the GrProgramElement and a single