Simplified stencil buffer caching

https://codereview.appspot.com/6503073/



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

include/gpu/GrContext.h

@@ -762,13 +762,12 @@ public:
      * Stencil buffers add themselves to the cache using
      * addAndLockStencilBuffer. When a SB's RT-attachment count
      * reaches zero the SB unlocks itself using unlockStencilBuffer and is
-     * eligible for purging. findStencilBuffer is called to check the cache for
+     * eligible for purging. findAndLockStencilBuffer is called to check the
-     * a SB that matching an RT's criteria. If a match is found that has been
+     * cache for a SB that matches an RT's criteria. 
-     * unlocked (its attachment count has reached 0) then it will be relocked.
      */
     void addAndLockStencilBuffer(GrStencilBuffer* sb);
     void unlockStencilBuffer(GrStencilBuffer* sb);
-    GrStencilBuffer* findStencilBuffer(int width, int height, int sampleCnt);
+    GrStencilBuffer* findAndLockStencilBuffer(int width, int height, int sampleCnt);
 
     GrPathRenderer* getPathRenderer(const SkPath& path,
                                     GrPathFill fill,

include/gpu/GrRenderTarget.h

@@ -168,6 +168,10 @@ protected:
         fTexture = NULL;
     }
 
+    // override of GrResource
+    virtual void onAbandon() SK_OVERRIDE;
+    virtual void onRelease() SK_OVERRIDE;
+
 private:
     GrStencilBuffer*  fStencilBuffer;
     GrTexture*        fTexture; // not ref'ed

src/gpu/GrContext.cpp

@@ -216,8 +216,7 @@ GrTexture* GrContext::findAndLockTexture(const GrTextureDesc& desc,
                                          const GrCacheData& cacheData,
                                          const GrTextureParams* params) {
     GrResourceKey resourceKey = GrTexture::ComputeKey(fGpu, params, desc, cacheData, false);
-    GrResource* resource = fTextureCache->findAndLock(resourceKey,
+    GrResource* resource = fTextureCache->findAndLock(resourceKey);
-                                                      GrResourceCache::kNested_LockType);
     return static_cast<GrTexture*>(resource);
 }
 
@@ -237,19 +236,25 @@ void GrContext::addAndLockStencilBuffer(GrStencilBuffer* sb) {
     fTextureCache->createAndLock(resourceKey, sb);
 }
 
-GrStencilBuffer* GrContext::findStencilBuffer(int width, int height,
+GrStencilBuffer* GrContext::findAndLockStencilBuffer(int width, int height,
                                               int sampleCnt) {
     GrResourceKey resourceKey = GrStencilBuffer::ComputeKey(width,
                                                             height,
                                                             sampleCnt);
-    GrResource* resource = fTextureCache->findAndLock(resourceKey,
+    GrResource* resource = fTextureCache->findAndLock(resourceKey);
-                                            GrResourceCache::kSingle_LockType);
     return static_cast<GrStencilBuffer*>(resource);
 }
 
 void GrContext::unlockStencilBuffer(GrStencilBuffer* sb) {
     ASSERT_OWNED_RESOURCE(sb);
-    GrAssert(NULL != sb->getCacheEntry());
+
+    if (NULL == sb->getCacheEntry()) {
+        // This can happen when the GrResourceCache is being deleted. If
+        // a stencil buffer was evicted before its reffing render targets,
+        // the render targets will attempt to unlock the stencil buffer
+        // when they are deleted.
+        return;
+    }
 
     fTextureCache->unlock(sb->getCacheEntry());
 }
@@ -415,8 +420,7 @@ GrTexture* GrContext::lockScratchTexture(const GrTextureDesc& inDesc,
 
     do {
         GrResourceKey key = GrTexture::ComputeKey(fGpu, NULL, desc, cacheData, true);
-        resource = fTextureCache->findAndLock(key,
+        resource = fTextureCache->findAndLock(key);
-                                              GrResourceCache::kNested_LockType);
         // if we miss, relax the fit of the flags...
         // then try doubling width... then height.
         if (NULL != resource || kExact_ScratchTexMatch == match) {

src/gpu/GrGpu.cpp

@@ -144,7 +144,7 @@ GrTexture* GrGpu::createTexture(const GrTextureDesc& desc,
 bool GrGpu::attachStencilBufferToRenderTarget(GrRenderTarget* rt) {
     GrAssert(NULL == rt->getStencilBuffer());
     GrStencilBuffer* sb =
-        this->getContext()->findStencilBuffer(rt->width(),
+        this->getContext()->findAndLockStencilBuffer(rt->width(),
                                                      rt->height(),
                                                      rt->numSamples());
     if (NULL != sb) {
@@ -157,9 +157,6 @@ bool GrGpu::attachStencilBufferToRenderTarget(GrRenderTarget* rt) {
     }
     if (this->createStencilBufferForRenderTarget(rt,
                                                  rt->width(), rt->height())) {
-        rt->getStencilBuffer()->ref();
-        rt->getStencilBuffer()->transferToCacheAndLock();
-
         // Right now we're clearing the stencil buffer here after it is
         // attached to an RT for the first time. When we start matching
         // stencil buffers with smaller color targets this will no longer

src/gpu/GrRenderTarget.cpp

@@ -96,12 +96,24 @@ void GrRenderTarget::overrideResolveRect(const GrIRect rect) {
 
 void GrRenderTarget::setStencilBuffer(GrStencilBuffer* stencilBuffer) {
     if (NULL != fStencilBuffer) {
-        fStencilBuffer->wasDetachedFromRenderTarget(this);
+        GrContext* context = this->getContext();
+        if (NULL != context) {
+            context->unlockStencilBuffer(fStencilBuffer);
+        }
         fStencilBuffer->unref();
     }
+
     fStencilBuffer = stencilBuffer;
+
     if (NULL != fStencilBuffer) {
-        fStencilBuffer->wasAttachedToRenderTarget(this);
         fStencilBuffer->ref();
     }
 }
+
+void GrRenderTarget::onRelease() {
+    this->setStencilBuffer(NULL);
+}
+
+void GrRenderTarget::onAbandon() {
+    this->setStencilBuffer(NULL);
+}

src/gpu/GrResourceCache.cpp

@@ -195,7 +195,7 @@ GrResource* GrResourceCache::find(const GrResourceKey& key) {
     return entry->fResource;
 }
 
-GrResource* GrResourceCache::findAndLock(const GrResourceKey& key, LockType type) {
+GrResource* GrResourceCache::findAndLock(const GrResourceKey& key) {
     GrAutoResourceCacheValidate atcv(this);
 
     GrResourceEntry* entry = fCache.find(key);
@@ -206,9 +206,7 @@ GrResource* GrResourceCache::findAndLock(const GrResourceKey& key, LockType type
     this->internalDetach(entry, false);
     this->attachToHead(entry, false);
 
-    if (kNested_LockType == type || !entry->isLocked()) {
     this->lock(entry);
-    }
 
     return entry->fResource;
 }

src/gpu/GrResourceCache.h

@@ -231,14 +231,6 @@ public:
      */
     size_t getCachedResourceBytes() const { return fEntryBytes; }
 
-    /**
-     * Controls whether locks should be nestable or not.
-     */
-    enum LockType {
-        kNested_LockType,
-        kSingle_LockType,
-    };
-
     /**
      *  Search for an entry with the same Key. If found, return it.
      *  If not found, return null.
@@ -249,7 +241,7 @@ public:
      *  Search for an entry with the same Key. If found, "lock" it and return it.
      *  If not found, return null.
      */
-    GrResource* findAndLock(const GrResourceKey&, LockType style);
+    GrResource* findAndLock(const GrResourceKey&);
 
     /**
      *  Create a new cache entry, based on the provided key and resource, and

src/gpu/GrStencilBuffer.cpp

@@ -14,49 +14,10 @@
 
 GR_DEFINE_RESOURCE_CACHE_TYPE(GrStencilBuffer)
 
-void GrStencilBuffer::wasDetachedFromRenderTarget(const GrRenderTarget* rt) {
-    GrAssert(fRTAttachmentCnt > 0);
-    if (0 == --fRTAttachmentCnt) {
-        this->unlockInCache();
-        // At this point we could be deleted!
-    }
-}
-
 void GrStencilBuffer::transferToCacheAndLock() {
     GrAssert(NULL == this->getCacheEntry());
-    GrAssert(!fHoldingLock);
 
     this->getGpu()->getContext()->addAndLockStencilBuffer(this);
-    fHoldingLock = true;
-}
-
-void GrStencilBuffer::onRelease() {
-    // When the GrGpu rips through its list of resources and releases
-    // them it may release an SB before it releases its attached RTs.
-    // In that case when GrStencilBuffer sees its last detach it no
-    // long has a gpu ptr (gets nulled in GrResource::release()) and can't
-    // access the cache to unlock itself. So if we're being released and still
-    // have attachments go ahead and unlock now.
-    if (fRTAttachmentCnt) {
-        this->unlockInCache();
-        // we shouldn't be deleted here because some RT still has a ref on us.
-    }
-}
-
-void GrStencilBuffer::onAbandon() {
-    // we can use the same behavior as release.
-    this->onRelease();
-}
-
-void GrStencilBuffer::unlockInCache() {
-    if (fHoldingLock && this->isInCache()) {
-        GrGpu* gpu = this->getGpu();
-        if (NULL != gpu) {
-            GrAssert(NULL != gpu->getContext());
-            gpu->getContext()->unlockStencilBuffer(this);
-        }
-        fHoldingLock = false;
-    }
 }
 
 namespace {

src/gpu/GrStencilBuffer.h

@@ -23,9 +23,7 @@ public:
     GR_DECLARE_RESOURCE_CACHE_TYPE()
 
     virtual ~GrStencilBuffer() {
-        // currently each rt that has attached this sb keeps a ref
         // TODO: allow SB to be purged and detach itself from rts
-        GrAssert(0 == fRTAttachmentCnt);
     }
 
     int width() const { return fWidth; }
@@ -65,12 +63,6 @@ public:
     // a ref to the the cache which will unref when purged.
     void transferToCacheAndLock();
 
-    void wasAttachedToRenderTarget(const GrRenderTarget* rt) {
-        ++fRTAttachmentCnt;
-    }
-
-    void wasDetachedFromRenderTarget(const GrRenderTarget* rt);
-
     static GrResourceKey ComputeKey(int width, int height, int sampleCnt);
 
 protected:
@@ -83,22 +75,11 @@ protected:
         , fLastClipStack()
         , fLastClipData()
         , fLastClipWidth(-1)
-        , fLastClipHeight(-1)
+        , fLastClipHeight(-1) {
-        , fHoldingLock(false)
-        , fRTAttachmentCnt(0) {
     }
 
-    // GrResource overrides
-
-    // subclass override must call INHERITED::onRelease
-    virtual void onRelease();
-    // subclass override must call INHERITED::onAbandon
-    virtual void onAbandon();
-
 private:
 
-    void unlockInCache();
-
     int fWidth;
     int fHeight;
     int fBits;
@@ -109,9 +90,6 @@ private:
     int         fLastClipWidth;
     int         fLastClipHeight;
 
-    bool             fHoldingLock;
-    int              fRTAttachmentCnt;
-
     typedef GrResource INHERITED;
 };
 

src/gpu/gl/GrGLRenderTarget.cpp

@@ -92,7 +92,7 @@ void GrGLRenderTarget::onRelease() {
     fMSColorRenderbufferID  = 0;
     GrSafeUnref(fTexIDObj);
     fTexIDObj = NULL;
-    this->setStencilBuffer(NULL);
+    INHERITED::onRelease();
 }
 
 void GrGLRenderTarget::onAbandon() {
@@ -103,6 +103,6 @@ void GrGLRenderTarget::onAbandon() {
         fTexIDObj->abandon();
         fTexIDObj = NULL;
     }
-    this->setStencilBuffer(NULL);
+    INHERITED::onAbandon();
 }
 

src/gpu/gl/GrGLRenderTarget.h

@@ -80,8 +80,8 @@ public:
 
 protected:
     // override of GrResource
-    virtual void onAbandon();
+    virtual void onAbandon() SK_OVERRIDE;
-    virtual void onRelease();
+    virtual void onRelease() SK_OVERRIDE;
 
 private:
     GrGLuint      fRTFBOID;

src/gpu/gl/GrGLStencilBuffer.cpp

@@ -28,12 +28,10 @@ void GrGLStencilBuffer::onRelease() {
         GR_GL_CALL(gl, DeleteRenderbuffers(1, &fRenderbufferID));
         fRenderbufferID = 0;
     }
-    INHERITED::onRelease();
 }
 
 void GrGLStencilBuffer::onAbandon() {
     fRenderbufferID = 0;
-    INHERITED::onAbandon();
 }
 
 

src/gpu/gl/GrGLStencilBuffer.h

@@ -34,7 +34,7 @@ public:
 
     virtual ~GrGLStencilBuffer();
 
-    virtual size_t sizeInBytes() const;
+    virtual size_t sizeInBytes() const SK_OVERRIDE;
 
     GrGLuint renderbufferID() const {
         return fRenderbufferID;
@@ -43,9 +43,9 @@ public:
     const Format& format() const { return fFormat; }
 
 protected:
-    virtual void onRelease();
+    // overrides of GrResource
-
+    virtual void onRelease() SK_OVERRIDE;
-    virtual void onAbandon();
+    virtual void onAbandon() SK_OVERRIDE;
 
 private:
     Format fFormat;

src/gpu/gl/GrGpuGL.cpp

@@ -1110,8 +1110,10 @@ bool GrGpuGL::createStencilBufferForRenderTarget(GrRenderTarget* rt,
                              samples, format));
             if (this->attachStencilBufferToRenderTarget(sb, rt)) {
                 fLastSuccessfulStencilFmtIdx = sIdx;
+                // This code transfers the creation ref to the
+                // cache and then adds a ref for the render target
+                sb->transferToCacheAndLock();
                 rt->setStencilBuffer(sb);
-                sb->unref();
                 return true;
            }
            sb->abandon(); // otherwise we lose sbID