From a292112154f803feb9f5cc002bbfab559f7cb633 Mon Sep 17 00:00:00 2001 From: "bsalomon@google.com" Date: Tue, 28 Aug 2012 12:34:17 +0000 Subject: [PATCH] Remove SkGpuDevice::fTexture, use new pixel ref class name Review URL: https://codereview.appspot.com/6474068/ git-svn-id: http://skia.googlecode.com/svn/trunk@5307 2bbb7eff-a529-9590-31e7-b0007b416f81 --- include/core/SkTDLinkedList.h | 3 ++ include/gpu/GrResource.h | 2 + include/gpu/SkGpuDevice.h | 5 +- src/gpu/GrContext.cpp | 4 +- src/gpu/GrResourceCache.cpp | 96 ++++++++++++++++++++--------------- src/gpu/GrResourceCache.h | 14 +++-- src/gpu/GrStencilBuffer.cpp | 4 +- src/gpu/SkGpuDevice.cpp | 46 ++++++++--------- 8 files changed, 94 insertions(+), 80 deletions(-) diff --git a/include/core/SkTDLinkedList.h b/include/core/SkTDLinkedList.h index 7afb5b1b8..88bf96d34 100644 --- a/include/core/SkTDLinkedList.h +++ b/include/core/SkTDLinkedList.h @@ -95,6 +95,9 @@ public: return NULL == fHead && NULL == fTail; } + T* head() { return fHead; } + T* tail() { return fTail; } + class Iter { public: enum IterStart { diff --git a/include/gpu/GrResource.h b/include/gpu/GrResource.h index 0f4b03d85..6b85729b5 100644 --- a/include/gpu/GrResource.h +++ b/include/gpu/GrResource.h @@ -76,6 +76,8 @@ protected: virtual void onRelease() = 0; virtual void onAbandon() = 0; + bool isInCache() const { return NULL != fCacheEntry; } + private: friend class GrGpu; // GrGpu manages list of resources. diff --git a/include/gpu/SkGpuDevice.h b/include/gpu/SkGpuDevice.h index c2fc186a9..f04be622f 100644 --- a/include/gpu/SkGpuDevice.h +++ b/include/gpu/SkGpuDevice.h @@ -132,9 +132,8 @@ private: GrClipData fClipData; // state for our offscreen render-target - // TODO: remove 'fCached' and let fTexture automatically return to the cache - bool fCached; // is fTexture in the cache - GrTexture* fTexture; + // TODO: remove 'fCached' and automatically return to the cache + bool fCached; // is fRenderTarget->asTexture() in the cache GrRenderTarget* fRenderTarget; bool fNeedClear; bool fNeedPrepareRenderTarget; diff --git a/src/gpu/GrContext.cpp b/src/gpu/GrContext.cpp index 2b5014d6a..a13394cd6 100644 --- a/src/gpu/GrContext.cpp +++ b/src/gpu/GrContext.cpp @@ -136,7 +136,7 @@ void GrContext::contextDestroyed() { fAARectRenderer->reset(); - fTextureCache->removeAll(); + fTextureCache->purgeAllUnlocked(); fFontCache->freeAll(); fGpu->markContextDirty(); } @@ -152,7 +152,7 @@ void GrContext::freeGpuResources() { fAARectRenderer->reset(); - fTextureCache->removeAll(); + fTextureCache->purgeAllUnlocked(); fFontCache->freeAll(); // a path renderer may be holding onto resources GrSafeSetNull(fPathRendererChain); diff --git a/src/gpu/GrResourceCache.cpp b/src/gpu/GrResourceCache.cpp index 60e20a77c..777470b00 100644 --- a/src/gpu/GrResourceCache.cpp +++ b/src/gpu/GrResourceCache.cpp @@ -35,6 +35,36 @@ void GrResourceEntry::validate() const { /////////////////////////////////////////////////////////////////////////////// +class GrResourceCache::Key { + typedef GrResourceEntry T; + + const GrResourceKey& fKey; +public: + Key(const GrResourceKey& key) : fKey(key) {} + + uint32_t getHash() const { return fKey.hashIndex(); } + + static bool LT(const T& entry, const Key& key) { + return entry.key() < key.fKey; + } + static bool EQ(const T& entry, const Key& key) { + return entry.key() == key.fKey; + } +#if GR_DEBUG + static uint32_t GetHash(const T& entry) { + return entry.key().hashIndex(); + } + static bool LT(const T& a, const T& b) { + return a.key() < b.key(); + } + static bool EQ(const T& a, const T& b) { + return a.key() == b.key(); + } +#endif +}; + +/////////////////////////////////////////////////////////////////////////////// + GrResourceCache::GrResourceCache(int maxCount, size_t maxBytes) : fMaxCount(maxCount), fMaxBytes(maxBytes) { @@ -58,7 +88,20 @@ GrResourceCache::GrResourceCache(int maxCount, size_t maxBytes) : GrResourceCache::~GrResourceCache() { GrAutoResourceCacheValidate atcv(this); - this->removeAll(); + EntryList::Iter iter; + + // Unlike the removeAll, here we really remove everything, including locked resources. + while (GrResourceEntry* entry = fList.head()) { + GrAutoResourceCacheValidate atcv(this); + + // remove from our cache + fCache.remove(entry->fKey, entry); + + // remove from our llist + this->internalDetach(entry, false); + + delete entry; + } } void GrResourceCache::getLimits(int* maxResources, size_t* maxResourceBytes) const{ @@ -141,34 +184,6 @@ void GrResourceCache::attachToHead(GrResourceEntry* entry, } } -class GrResourceCache::Key { - typedef GrResourceEntry T; - - const GrResourceKey& fKey; -public: - Key(const GrResourceKey& key) : fKey(key) {} - - uint32_t getHash() const { return fKey.hashIndex(); } - - static bool LT(const T& entry, const Key& key) { - return entry.key() < key.fKey; - } - static bool EQ(const T& entry, const Key& key) { - return entry.key() == key.fKey; - } -#if GR_DEBUG - static uint32_t GetHash(const T& entry) { - return entry.key().hashIndex(); - } - static bool LT(const T& a, const T& b) { - return a.key() < b.key(); - } - static bool EQ(const T& a, const T& b) { - return a.key() == b.key(); - } -#endif -}; - GrResource* GrResourceCache::findAndLock(const GrResourceKey& key, LockType type) { GrAutoResourceCacheValidate atcv(this); @@ -310,14 +325,13 @@ void GrResourceCache::purgeAsNeeded() { fPurging = true; bool withinBudget = false; do { - SkTDLinkedList::Iter iter; + EntryList::Iter iter; // Note: the following code relies on the fact that the // doubly linked list doesn't invalidate its data/pointers // outside of the specific area where a deletion occurs (e.g., // in internalDetach) - GrResourceEntry* entry = iter.init(fList, - SkTDLinkedList::Iter::kTail_IterStart); + GrResourceEntry* entry = iter.init(fList, EntryList::Iter::kTail_IterStart); while (entry && fUnlockedEntryCount) { GrAutoResourceCacheValidate atcv(this); @@ -350,7 +364,7 @@ void GrResourceCache::purgeAsNeeded() { } } -void GrResourceCache::removeAll() { +void GrResourceCache::purgeAllUnlocked() { GrAutoResourceCacheValidate atcv(this); // we can have one GrResource holding a lock on another @@ -384,14 +398,13 @@ void GrResourceCache::removeAll() { /////////////////////////////////////////////////////////////////////////////// #if GR_DEBUG -size_t GrResourceCache::countBytes(const SkTDLinkedList& list) { +size_t GrResourceCache::countBytes(const EntryList& list) { size_t bytes = 0; - SkTDLinkedList::Iter iter; + EntryList::Iter iter; - const GrResourceEntry* entry = iter.init( - const_cast&>(list), - SkTDLinkedList::Iter::kTail_IterStart); + const GrResourceEntry* entry = iter.init(const_cast(list), + EntryList::Iter::kTail_IterStart); for ( ; NULL != entry; entry = iter.prev()) { bytes += entry->resource()->sizeInBytes(); @@ -417,11 +430,10 @@ void GrResourceCache::validate() const { int count = 0; int unlockCount = 0; - SkTDLinkedList::Iter iter; + EntryList::Iter iter; - const GrResourceEntry* entry = iter.init( - const_cast&>(fList), - SkTDLinkedList::Iter::kHead_IterStart); + const GrResourceEntry* entry = iter.init(const_cast(fList), + EntryList::Iter::kHead_IterStart); for ( ; NULL != entry; entry = iter.next()) { entry->validate(); diff --git a/src/gpu/GrResourceCache.h b/src/gpu/GrResourceCache.h index 162e1ef04..5741e47dc 100644 --- a/src/gpu/GrResourceCache.h +++ b/src/gpu/GrResourceCache.h @@ -264,12 +264,15 @@ public: void makeNonExclusive(GrResourceEntry* entry); /** - * When done with an entry, call unlock(entry) on it, which returns it to - * a purgable state. + * When done with an entry, call unlock(entry) on it, which returns it to + * a purgable state. */ void unlock(GrResourceEntry*); - void removeAll(); + /** + * Removes every resource in the cache that isn't locked. + */ + void purgeAllUnlocked(); #if GR_DEBUG void validate() const; @@ -289,11 +292,12 @@ private: GrTHashTable fCache; // manage the dlink list - SkTDLinkedList fList; + typedef SkTDLinkedList EntryList; + EntryList fList; #if GR_DEBUG // These objects cannot be returned by a search - SkTDLinkedList fExclusiveList; + EntryList fExclusiveList; #endif // our budget, used in purgeAsNeeded() diff --git a/src/gpu/GrStencilBuffer.cpp b/src/gpu/GrStencilBuffer.cpp index bf14f4864..7bd6f3228 100644 --- a/src/gpu/GrStencilBuffer.cpp +++ b/src/gpu/GrStencilBuffer.cpp @@ -41,7 +41,6 @@ void GrStencilBuffer::onRelease() { this->unlockInCache(); // we shouldn't be deleted here because some RT still has a ref on us. } - fHoldingLock = false; } void GrStencilBuffer::onAbandon() { @@ -50,12 +49,13 @@ void GrStencilBuffer::onAbandon() { } void GrStencilBuffer::unlockInCache() { - if (fHoldingLock) { + if (fHoldingLock && this->isInCache()) { GrGpu* gpu = this->getGpu(); if (NULL != gpu) { GrAssert(NULL != gpu->getContext()); gpu->getContext()->unlockStencilBuffer(this); } + fHoldingLock = false; } } diff --git a/src/gpu/SkGpuDevice.cpp b/src/gpu/SkGpuDevice.cpp index 556608659..8d43f704e 100644 --- a/src/gpu/SkGpuDevice.cpp +++ b/src/gpu/SkGpuDevice.cpp @@ -189,28 +189,23 @@ void SkGpuDevice::initFromRenderTarget(GrContext* context, fContext->ref(); fCached = false; - fTexture = NULL; fRenderTarget = NULL; fNeedClear = false; GrAssert(NULL != renderTarget); fRenderTarget = renderTarget; fRenderTarget->ref(); - // if this RT is also a texture, hold a ref on it - fTexture = fRenderTarget->asTexture(); - SkSafeRef(fTexture); - // Create a pixel ref for the underlying SkBitmap. We prefer a texture pixel - // ref to a render target pixel reft. The pixel ref may get ref'ed outside - // the device via accessBitmap. This external ref may outlive the device. - // Since textures own their render targets (but not vice-versa) we - // are ensuring that both objects will live as long as the pixel ref. - SkPixelRef* pr; - if (fTexture) { - pr = SkNEW_ARGS(SkGrTexturePixelRef, (fTexture)); - } else { - pr = SkNEW_ARGS(SkGrRenderTargetPixelRef, (fRenderTarget)); + // Hold onto to the texture in the pixel ref (if there is one) because the texture holds a ref + // on the RT but not vice-versa. + // TODO: Remove this trickery once we figure out how to make SkGrPixelRef do this without + // busting chrome (for a currently unknown reason). + GrSurface* surface = fRenderTarget->asTexture(); + if (NULL == surface) { + surface = fRenderTarget; } + SkPixelRef* pr = SkNEW_ARGS(SkGrPixelRef, (surface)); + this->setPixelRef(pr, 0)->unref(); } @@ -227,7 +222,6 @@ SkGpuDevice::SkGpuDevice(GrContext* context, fContext->ref(); fCached = false; - fTexture = NULL; fRenderTarget = NULL; fNeedClear = false; @@ -243,16 +237,16 @@ SkGpuDevice::SkGpuDevice(GrContext* context, desc.fHeight = height; desc.fConfig = SkBitmapConfig2GrPixelConfig(bm.config()); - fTexture = fContext->createUncachedTexture(desc, NULL, 0); + SkAutoTUnref texture(fContext->createUncachedTexture(desc, NULL, 0)); - if (NULL != fTexture) { - fRenderTarget = fTexture->asRenderTarget(); + if (NULL != texture) { + fRenderTarget = texture->asRenderTarget(); fRenderTarget->ref(); GrAssert(NULL != fRenderTarget); // wrap the bitmap with a pixelref to expose our texture - SkGrTexturePixelRef* pr = SkNEW_ARGS(SkGrTexturePixelRef, (fTexture)); + SkGrPixelRef* pr = SkNEW_ARGS(SkGrPixelRef, (texture)); this->setPixelRef(pr, 0)->unref(); } else { GrPrintf("--- failed to create gpu-offscreen [%d %d]\n", @@ -270,12 +264,11 @@ SkGpuDevice::~SkGpuDevice() { // This call gives the context a chance to relinquish it fContext->setRenderTarget(NULL); - SkSafeUnref(fTexture); - SkSafeUnref(fRenderTarget); - if (NULL != fTexture && fCached) { - GrAssert(fRenderTarget == fTexture->asRenderTarget()); - fContext->unlockTexture(fTexture); + GrTexture* texture = fRenderTarget->asTexture(); + if (NULL != texture && fCached) { + fContext->unlockTexture(texture); } + SkSafeUnref(fRenderTarget); fContext->unref(); } @@ -485,9 +478,10 @@ SkGpuRenderTarget* SkGpuDevice::accessRenderTarget() { } bool SkGpuDevice::bindDeviceAsTexture(GrPaint* paint) { - if (NULL != fTexture) { + GrTexture* texture = fRenderTarget->asTexture(); + if (NULL != texture) { paint->textureSampler(kBitmapTextureIdx)->setCustomStage( - SkNEW_ARGS(GrSingleTextureEffect, (fTexture)))->unref(); + SkNEW_ARGS(GrSingleTextureEffect, (texture)))->unref(); return true; } return false;