commit 1942c05e9c51b462355626155c1bdc3fc43161cf
author robertphillips@google.com <robertphillips@google.com@2bbb7eff-a529-9590-31e7-b0007b416f81> Thu May 03 17:58:27 2012 +0000
committer robertphillips@google.com <robertphillips@google.com@2bbb7eff-a529-9590-31e7-b0007b416f81> Thu May 03 17:58:27 2012 +0000
tree b0a24c9ff844387fe546bb1c50b58164fcf5ead0
parent 58b381841cb8d234d09c85af43720cbff00e6212

Fixed texture ref/unref bug in GrInOrderDrawBuffer/GrDrawState

http://codereview.appspot.com/6186043/



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

src/gpu/GrDrawState.h

diff --git a/src/gpu/GrDrawState.h b/src/gpu/GrDrawState.h
index fee22bf..6313408 100644
--- a/src/gpu/GrDrawState.h
+++ b/src/gpu/GrDrawState.h
@@ -169,6 +169,18 @@
      */
     void setTexture(int stage, GrTexture* texture) {
         GrAssert((unsigned)stage < kNumStages);
+
+        if (isStateFlagEnabled(kTexturesNeedRef_StateBit)) {
+            // 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;
     }
 
@@ -657,6 +669,10 @@
          * ignored.
          */
         kColorMatrix_StateBit   = 0x20,
+        /**
+         * Calls to setTexture will ref/unref the texture
+         */
+        kTexturesNeedRef_StateBit = 0x40,
 
         // Users of the class may add additional bits to the vector
         kDummyStateBit,

src/gpu/GrInOrderDrawBuffer.cpp

diff --git a/src/gpu/GrInOrderDrawBuffer.cpp b/src/gpu/GrInOrderDrawBuffer.cpp
index e6b3ca8..de49e8c 100644
--- a/src/gpu/GrInOrderDrawBuffer.cpp
+++ b/src/gpu/GrInOrderDrawBuffer.cpp
@@ -466,6 +466,10 @@
             GrSafeUnref(fStates[i].getTexture(s));
         }
         GrSafeUnref(fStates[i].getRenderTarget());
+
+        // GrInOrderDrawBuffer is no longer managing the refs/unrefs 
+        // for the stored GrDrawStates
+        fStates[i].disableState(GrDrawState::kTexturesNeedRef_StateBit);
     }
     int numDraws = fDraws.count();
     for (int d = 0; d < numDraws; ++d) {
@@ -782,6 +786,10 @@
     }
     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().enableState(GrDrawState::kTexturesNeedRef_StateBit);
  }
 
 bool GrInOrderDrawBuffer::needsNewClip() const {

src/gpu/gl/GrGpuGL.cpp

diff --git a/src/gpu/gl/GrGpuGL.cpp b/src/gpu/gl/GrGpuGL.cpp
index 3b5fffe..a06b008 100644
--- a/src/gpu/gl/GrGpuGL.cpp
+++ b/src/gpu/gl/GrGpuGL.cpp
@@ -2102,7 +2102,7 @@
             // true for now, but maybe not with GrEffect.
             GrAssert(NULL != nextTexture);
             // if we created a rt/tex and rendered to it without using a
-            // texture and now we're texuring from the rt it will still be
+            // texture and now we're texturing from the rt it will still be
             // the last bound texture, but it needs resolving. So keep this
             // out of the "last != next" check.
             GrGLRenderTarget* texRT = 
@@ -2239,6 +2239,8 @@
     // relies on detecting when the kModifyStencilClip_StateBit state has
     // changed since the last draw.
     fHWDrawState.copyStateFlags(*drawState);
+    // only GrInOrderDrawBuffer ever needs to ref/unref the textures
+    fHWDrawState.disableState(GrDrawState::kTexturesNeedRef_StateBit);
     return true;
 }