commit 9b536523cae9787f6b24ed9234c50cdf2710d67e
author bsalomon <bsalomon@google.com> Fri Sep 05 09:18:51 2014 -0700
committer Commit bot <commit-bot@chromium.org> Fri Sep 05 09:18:51 2014 -0700
tree b7161e0b550240595338f680852f3b935db3480a
parent a1ae66d252edf6da932caed1fe43d11216e56c0e

Change behavior of GrDS::AutoEffectRestort wrt GPs.

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

Author: bsalomon@google.com

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

src/gpu/GrDrawState.cpp

diff --git a/src/gpu/GrDrawState.cpp b/src/gpu/GrDrawState.cpp
index 236d48f..6acc202 100644
--- a/src/gpu/GrDrawState.cpp
+++ b/src/gpu/GrDrawState.cpp
@@ -299,7 +299,14 @@
 
 void GrDrawState::AutoRestoreEffects::set(GrDrawState* ds) {
     if (NULL != fDrawState) {
-        fDrawState->fGeometryProcessor.reset(fGeometryProcessor.detach());
+        // See the big comment on the class definition about GPs.
+        if (NULL != fOriginalGP) {
+            SkASSERT(fDrawState->getGeometryProcessor()->getEffect() == fOriginalGP);
+            fOriginalGP->unref();
+            fOriginalGP = NULL;
+        } else {
+            fDrawState->fGeometryProcessor.reset(NULL);
+        }
 
         int m = fDrawState->numColorStages() - fColorEffectCnt;
         SkASSERT(m >= 0);
@@ -315,10 +322,9 @@
     }
     fDrawState = ds;
     if (NULL != ds) {
-        if (ds->hasGeometryProcessor()) {
-            fGeometryProcessor.reset(SkNEW_ARGS(GrEffectStage, (*ds->getGeometryProcessor())));
-        } else {
-            fGeometryProcessor.reset(NULL);
+        SkASSERT(NULL == fOriginalGP);
+        if (NULL != ds->getGeometryProcessor()) {
+            fOriginalGP = SkRef(ds->getGeometryProcessor()->getEffect());
         }
         fColorEffectCnt = ds->numColorStages();
         fCoverageEffectCnt = ds->numCoverageStages();

src/gpu/GrDrawState.h

diff --git a/src/gpu/GrDrawState.h b/src/gpu/GrDrawState.h
index a1b07de..7b5f150 100644
--- a/src/gpu/GrDrawState.h
+++ b/src/gpu/GrDrawState.h
@@ -237,14 +237,34 @@
     }
 
     /**
-     * When this object is destroyed it will remove any effects from the draw state that were added
-     * after its constructor.
+     * When this object is destroyed it will remove any color/coverage effects from the draw state
+     * that were added after its constructor.
+     *
+     * This class has strange behavior around geometry processor. If there is a GP on the draw state
+     * it will assert that the GP is not modified until after the destructor of the ARE. If the
+     * draw state has a NULL GP when the ARE is constructed then it will reset it to null in the
+     * destructor.
+     *
+     * TODO: We'd prefer for the ARE to just save and restore the GP. However, this would add
+     * significant complexity to the multi-ref architecture for deferred drawing. Once GrDrawState
+     * and GrOptDrawState are fully separated then GrDrawState will never be in the deferred
+     * execution state and GrOptDrawState always will be (and will be immutable and therefore
+     * unable to have an ARE). At this point we can restore sanity and have the ARE save and restore
+     * the GP.
      */
     class AutoRestoreEffects : public ::SkNoncopyable {
     public:
-        AutoRestoreEffects() : fDrawState(NULL), fColorEffectCnt(0), fCoverageEffectCnt(0) {}
+        AutoRestoreEffects() 
+            : fDrawState(NULL)
+            , fOriginalGP(NULL)
+            , fColorEffectCnt(0)
+            , fCoverageEffectCnt(0) {}
 
-        AutoRestoreEffects(GrDrawState* ds) : fDrawState(NULL), fColorEffectCnt(0), fCoverageEffectCnt(0) {
+        AutoRestoreEffects(GrDrawState* ds)
+            : fDrawState(NULL)
+            , fOriginalGP(NULL)
+            , fColorEffectCnt(0)
+            , fCoverageEffectCnt(0) {
             this->set(ds);
         }
 
@@ -255,10 +275,10 @@
         bool isSet() const { return NULL != fDrawState; }
 
     private:
-        GrDrawState* fDrawState;
-        SkAutoTDelete<GrEffectStage> fGeometryProcessor;
-        int fColorEffectCnt;
-        int fCoverageEffectCnt;
+        GrDrawState*    fDrawState;
+        const GrEffect* fOriginalGP;
+        int             fColorEffectCnt;
+        int             fCoverageEffectCnt;
     };
 
     /// @}