Altered GrDrawState to always ref texture and render target

http://codereview.appspot.com/6251049/

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

diff --git a/include/gpu/GrContext.h b/include/gpu/GrContext.h
index 71263d4..122baf1 100644 (file)
--- a/include/gpu/GrContext.h
+++ b/include/gpu/GrContext.h
@@ -772,7 +772,6 @@ private:
     static int PaintStageVertexLayoutBits(
                                     const GrPaint& paint,
                                     const bool hasTexCoords[GrPaint::kTotalStages]);
-    
 };
 
 /**
@@ -834,8 +833,13 @@ public:
     }
     
     ~GrAutoScratchTexture() {
+        this->reset();
+    }
+
+    void reset() {
         if (NULL != fContext) {
             fContext->unlockTexture(fEntry);
+            fEntry.reset();
         }
     }
 
@@ -843,10 +847,8 @@ public:
                    const GrTextureDesc& desc,
                    GrContext::ScratchTexMatch match =
                         GrContext::kApprox_ScratchTexMatch) {
-        if (NULL != fContext) {
-            fContext->unlockTexture(fEntry);
-            fEntry.reset();
-        }
+        this->reset();
+
         fContext = context;
         if (NULL != fContext) {
             fEntry = fContext->lockScratchTexture(desc, match);

diff --git a/src/gpu/GrDrawState.h b/src/gpu/GrDrawState.h
index e77b63a..5feccb5 100644 (file)
--- a/src/gpu/GrDrawState.h
+++ b/src/gpu/GrDrawState.h
@@ -15,11 +15,10 @@
 #include "GrSamplerState.h"
 #include "GrStencil.h"
 #include "GrTexture.h"
+#include "GrRenderTarget.h"
 
 #include "SkXfermode.h"
 
-class GrRenderTarget;
-class GrTexture;
 
 class GrDrawState : public GrRefCnt {
 
@@ -54,18 +53,39 @@ public:
     typedef uint32_t StageMask;
     GR_STATIC_ASSERT(sizeof(StageMask)*8 >= GrDrawState::kNumStages);
 
-    GrDrawState() {
+    GrDrawState() 
+        : fRenderTarget(NULL) {
+
+        for (int i = 0; i < kNumStages; ++i) {
+            fTextures[i] = NULL;
+        }
+
         this->reset();
     }
 
-    GrDrawState(const GrDrawState& state) {
+    GrDrawState(const GrDrawState& state) 
+        : fRenderTarget(NULL) {
+
+        for (int i = 0; i < kNumStages; ++i) {
+            fTextures[i] = NULL;
+        }
+
         *this = state;
     }
 
+    virtual ~GrDrawState() {
+        this->releaseTextures();
+        GrSafeSetNull(fRenderTarget);
+    }
+
     /**
      * Resets to the default state. Sampler states will not be modified.
      */ 
     void reset() {
+
+        this->releaseTextures();
+        GrSafeSetNull(fRenderTarget);
+
         // make sure any pad is zero for memcmp
         // all GrDrawState members should default to something valid by the
         // the memset except those initialized individually below. There should
@@ -86,14 +106,13 @@ public:
         fSrcBlend = kOne_GrBlendCoeff;
         fDstBlend = kZero_GrBlendCoeff;
         fViewMatrix.reset();
-        fBehaviorBits = 0;
 
         // ensure values that will be memcmp'ed in == but not memset in reset()
         // are tightly packed
         GrAssert(this->memsetSize() +  sizeof(fColor) + sizeof(fCoverage) +
                  sizeof(fFirstCoverageStage) + sizeof(fColorFilterMode) +
-                 sizeof(fSrcBlend) + sizeof(fDstBlend) ==
-                 this->podSize());
+                 sizeof(fSrcBlend) + sizeof(fDstBlend) + sizeof(fTextures) +
+                 sizeof(fRenderTarget) == this->podSize());
     }
 
     ///////////////////////////////////////////////////////////////////////////
@@ -175,18 +194,7 @@ public:
     void setTexture(int stage, GrTexture* texture) {
         GrAssert((unsigned)stage < kNumStages);
 
-        if (isBehaviorEnabled(kTexturesNeedRef_BehaviorBit)) {
-            // If we don't clear out the current texture before unreffing
-            // it we can get into an infinite loop as the GrGLTexture's
-            // onRelease method recursively calls setTexture
-            GrTexture* temp = fTextures[stage];
-            fTextures[stage] = NULL;
-
-            SkSafeRef(texture);
-            SkSafeUnref(temp);
-        }
-
-        fTextures[stage] = texture;
+        GrSafeAssign(fTextures[stage], texture);
     }
 
     /**
@@ -210,14 +218,14 @@ public:
      */
     void releaseTextures() {
         for (int i = 0; i < kNumStages; ++i) {
-            this->setTexture(i, NULL);
+            GrSafeSetNull(fTextures[i]);
         }
     }
 
     class AutoTextureRelease : public ::GrNoncopyable {
     public:
         AutoTextureRelease(GrDrawState* ds) : fDrawState(ds) {}
-        ~AutoTextureRelease() { 
+        ~AutoTextureRelease() {
             if (NULL != fDrawState) {
                 fDrawState->releaseTextures();
             }
@@ -483,7 +491,9 @@ public:
      *
      * @param target  The render target to set.
      */
-    void setRenderTarget(GrRenderTarget* target) { fRenderTarget = target; }
+    void setRenderTarget(GrRenderTarget* target) { 
+        GrSafeAssign(fRenderTarget, target);
+    }
 
     /**
      * Retrieves the currently set rendertarget.
@@ -501,16 +511,26 @@ public:
             fSavedTarget = NULL;
             this->set(ds, newTarget);
         }
-        ~AutoRenderTargetRestore() { this->set(NULL, NULL); }
-        void set(GrDrawState* ds, GrRenderTarget* newTarget) {
+        ~AutoRenderTargetRestore() { this->restore(); }
+
+        void restore() {
             if (NULL != fDrawState) {
                 fDrawState->setRenderTarget(fSavedTarget);
+                fDrawState = NULL;
             }
+            GrSafeSetNull(fSavedTarget);
+        }
+
+        void set(GrDrawState* ds, GrRenderTarget* newTarget) {
+            this->restore();
+
             if (NULL != ds) {
+                GrAssert(NULL == fSavedTarget);
                 fSavedTarget = ds->getRenderTarget();
+                SkSafeRef(fSavedTarget);
                 ds->setRenderTarget(newTarget);
+                fDrawState = ds;
             }
-            fDrawState = ds;
         }
     private:
         GrDrawState* fDrawState;
@@ -693,28 +713,6 @@ public:
         return 0 != (stateBit & fFlagBits);
     }
 
-    /**
-     *  Flags that do not affect rendering. 
-     */
-    enum GrBehaviorBits {
-        /**
-         * Calls to setTexture will ref/unref the texture
-         */
-        kTexturesNeedRef_BehaviorBit = 0x01,
-    };
-
-    void enableBehavior(uint32_t behaviorBits) {
-        fBehaviorBits |= behaviorBits;
-    }
-
-    void disableBehavior(uint32_t behaviorBits) {
-        fBehaviorBits &= ~(behaviorBits);
-    }
-
-    bool isBehaviorEnabled(uint32_t behaviorBits) const {
-        return 0 != (behaviorBits & fBehaviorBits);
-    }
-
     /// @}
 
     ///////////////////////////////////////////////////////////////////////////
@@ -760,14 +758,6 @@ public:
             return false;
         }
 
-        // kTexturesNeedRef is an internal flag for altering the draw state's 
-        // behavior rather than a property that will impact drawing - ignore it
-        // here
-        if ((fBehaviorBits & ~kTexturesNeedRef_BehaviorBit) != 
-            (s.fBehaviorBits & ~kTexturesNeedRef_BehaviorBit)) {
-            return false;
-        }
-
         for (int i = 0; i < kNumStages; i++) {
             if (fTextures[i] &&
                 this->fSamplerStates[i] != s.fSamplerStates[i]) {
@@ -792,13 +782,16 @@ public:
         memcpy(this->podStart(), s.podStart(), this->podSize());
 
         fViewMatrix = s.fViewMatrix;
-        fBehaviorBits = s.fBehaviorBits;
 
         for (int i = 0; i < kNumStages; i++) {
+            SkSafeRef(fTextures[i]);            // already copied by memcpy
             if (s.fTextures[i]) {
                 this->fSamplerStates[i] = s.fSamplerStates[i];
             }
         }
+
+        SkSafeRef(fRenderTarget);               // already copied by memcpy
+
         if (kColorMatrix_StateBit & s.fFlagBits) {
             memcpy(this->fColorMatrix, s.fColorMatrix, sizeof(fColorMatrix));
         }
@@ -832,23 +825,25 @@ private:
         GrColor             fBlendConstant;
         GrColor             fPodStartMarker;
     };
-    GrTexture*          fTextures[kNumStages];
     GrColor             fColorFilterColor;
-    uint32_t            fFlagBits;
     DrawFace            fDrawFace; 
     VertexEdgeType      fVertexEdgeType;
     GrStencilSettings   fStencilSettings;
     union {
-        GrRenderTarget* fRenderTarget;
-        GrRenderTarget* fMemsetEndMarker;
+        uint32_t        fFlagBits;
+        uint32_t        fMemsetEndMarker;
     };
     // @}
 
+    int                 fFirstCoverageStage;
+
     // @{ Initialized to values other than zero, but memcmp'ed in operator==
     // and memcpy'ed in operator=.
+    GrTexture*          fTextures[kNumStages];
+    GrRenderTarget*     fRenderTarget;
+
     GrColor             fColor;
     GrColor             fCoverage;
-    int                 fFirstCoverageStage;
     SkXfermode::Mode    fColorFilterMode;
     GrBlendCoeff        fSrcBlend;
     union {
@@ -857,7 +852,6 @@ private:
     };
     // @}
 
-    uint32_t            fBehaviorBits;
     GrMatrix            fViewMatrix;
 
     // This field must be last; it will not be copied or compared

diff --git a/src/gpu/GrInOrderDrawBuffer.cpp b/src/gpu/GrInOrderDrawBuffer.cpp
index 9d8f887..f70b2ee 100644 (file)
--- a/src/gpu/GrInOrderDrawBuffer.cpp
+++ b/src/gpu/GrInOrderDrawBuffer.cpp
@@ -469,17 +469,6 @@ void GrInOrderDrawBuffer::reset() {
     GrAssert(1 == fGeoPoolStateStack.count());
     this->resetVertexSource();
     this->resetIndexSource();
-    uint32_t numStates = fStates.count();
-    for (uint32_t i = 0; i < numStates; ++i) {
-        for (int s = 0; s < GrDrawState::kNumStages; ++s) {
-            GrSafeUnref(fStates[i].getTexture(s));
-        }
-        GrSafeUnref(fStates[i].getRenderTarget());
-
-        // GrInOrderDrawBuffer is no longer managing the refs/unrefs 
-        // for the stored GrDrawStates
-        fStates[i].disableBehavior(GrDrawState::kTexturesNeedRef_BehaviorBit);
-    }
     int numDraws = fDraws.count();
     for (int d = 0; d < numDraws; ++d) {
         // we always have a VB, but not always an IB
@@ -793,16 +782,7 @@ bool GrInOrderDrawBuffer::needsNewState() const {
 }
 
 void GrInOrderDrawBuffer::pushState() {
-    const GrDrawState& drawState = this->getDrawState();
-    for (int s = 0; s < GrDrawState::kNumStages; ++s) {
-        GrSafeRef(drawState.getTexture(s));
-    }
-    GrSafeRef(drawState.getRenderTarget());
     fStates.push_back(this->getDrawState());
-
-    // Any textures that are added to the stored state need to be
-    // reffed so the unref in reset doesn't inappropriately free them
-    fStates.back().enableBehavior(GrDrawState::kTexturesNeedRef_BehaviorBit);
  }
 
 bool GrInOrderDrawBuffer::needsNewClip() const {

diff --git a/src/gpu/SkGpuDevice.cpp b/src/gpu/SkGpuDevice.cpp
index 3ce70d1..46b06d8 100644 (file)
--- a/src/gpu/SkGpuDevice.cpp
+++ b/src/gpu/SkGpuDevice.cpp
@@ -257,6 +257,10 @@ SkGpuDevice::~SkGpuDevice() {
         delete fDrawProcs;
     }
 
+    // The SkGpuDevice gives the context the render target (e.g., in gainFocus)
+    // This call gives the context a chance to relinquish it 
+    fContext->setRenderTarget(NULL);
+
     SkSafeUnref(fTexture);
     SkSafeUnref(fRenderTarget);
     if (fCache.texture()) {

diff --git a/src/gpu/gl/GrGpuGL.cpp b/src/gpu/gl/GrGpuGL.cpp
index 76a7a63..5b03500 100644 (file)
--- a/src/gpu/gl/GrGpuGL.cpp
+++ b/src/gpu/gl/GrGpuGL.cpp
@@ -2193,10 +2193,6 @@ void GrGpuGL::notifyIndexBufferDelete(const GrGLIndexBuffer* buffer) {
 
 void GrGpuGL::notifyRenderTargetDelete(GrRenderTarget* renderTarget) {
     GrAssert(NULL != renderTarget);
-    GrDrawState* drawState = this->drawState();
-    if (drawState->getRenderTarget() == renderTarget) {
-        drawState->setRenderTarget(NULL);
-    }
     if (fHWBoundRenderTarget == renderTarget) {
         fHWBoundRenderTarget = NULL;
     }
@@ -2204,10 +2200,6 @@ void GrGpuGL::notifyRenderTargetDelete(GrRenderTarget* renderTarget) {
 
 void GrGpuGL::notifyTextureDelete(GrGLTexture* texture) {
     for (int s = 0; s < GrDrawState::kNumStages; ++s) {
-        GrDrawState* drawState = this->drawState();
-        if (drawState->getTexture(s) == texture) {
-            this->drawState()->setTexture(s, NULL);
-        }
         if (fHWBoundTextures[s] == texture) {
             // deleting bound texture does implied bind to 0
             fHWBoundTextures[s] = NULL;