Allow resources' unique keys to be changed.
Review URL: https://codereview.chromium.org/938943002
diff --git a/src/effects/SkBlurMaskFilter.cpp b/src/effects/SkBlurMaskFilter.cpp
index c16cc2d..ae4f756 100644
--- a/src/effects/SkBlurMaskFilter.cpp
+++ b/src/effects/SkBlurMaskFilter.cpp
@@ -777,7 +777,7 @@
if (NULL == *blurProfileTexture) {
return false;
}
- SkAssertResult(context->addResourceToCache(key, *blurProfileTexture));
+ context->addResourceToCache(key, *blurProfileTexture);
}
return true;
@@ -962,7 +962,7 @@
texDesc.fConfig = kAlpha_8_GrPixelConfig;
blurNinePatchTexture = context->createTexture(texDesc, true, blurred_mask.fImage, 0);
- SkAssertResult(context->addResourceToCache(key, blurNinePatchTexture));
+ context->addResourceToCache(key, blurNinePatchTexture);
SkMask::FreeImage(blurred_mask.fImage);
}
diff --git a/src/effects/SkColorCubeFilter.cpp b/src/effects/SkColorCubeFilter.cpp
index 552d000..f71f471 100644
--- a/src/effects/SkColorCubeFilter.cpp
+++ b/src/effects/SkColorCubeFilter.cpp
@@ -354,7 +354,7 @@
if (!textureCube) {
textureCube.reset(context->createTexture(desc, true, fCubeData->data(), 0));
if (textureCube) {
- SkAssertResult(context->addResourceToCache(key, textureCube));
+ context->addResourceToCache(key, textureCube);
}
}
diff --git a/src/gpu/GrContext.cpp b/src/gpu/GrContext.cpp
index ef80b5b..a4679a9 100755
--- a/src/gpu/GrContext.cpp
+++ b/src/gpu/GrContext.cpp
@@ -1582,12 +1582,12 @@
fResourceCache->setLimits(maxTextures, maxTextureBytes);
}
-bool GrContext::addResourceToCache(const GrUniqueKey& key, GrGpuResource* resource) {
+void GrContext::addResourceToCache(const GrUniqueKey& key, GrGpuResource* resource) {
ASSERT_OWNED_RESOURCE(resource);
- if (!resource || resource->wasDestroyed()) {
- return false;
+ if (!resource) {
+ return;
}
- return resource->resourcePriv().setUniqueKey(key);
+ resource->resourcePriv().setUniqueKey(key);
}
bool GrContext::isResourceInCache(const GrUniqueKey& key) const {
diff --git a/src/gpu/GrGpuResource.cpp b/src/gpu/GrGpuResource.cpp
index 0c2c9a1..a2fc7b3 100644
--- a/src/gpu/GrGpuResource.cpp
+++ b/src/gpu/GrGpuResource.cpp
@@ -86,31 +86,23 @@
void GrGpuResource::removeUniqueKey() {
SkASSERT(fUniqueKey.isValid());
- get_resource_cache(fGpu)->resourceAccess().willRemoveUniqueKey(this);
- fUniqueKey.reset();
+ get_resource_cache(fGpu)->resourceAccess().removeUniqueKey(this);
}
-bool GrGpuResource::setUniqueKey(const GrUniqueKey& key) {
- // Currently this can only be called once and can't be called when the resource is scratch.
+void GrGpuResource::setUniqueKey(const GrUniqueKey& key) {
SkASSERT(this->internalHasRef());
SkASSERT(key.isValid());
// Wrapped and uncached resources can never have a unique key.
if (!this->resourcePriv().isBudgeted()) {
- return false;
+ return;
}
- if (fUniqueKey.isValid() || this->wasDestroyed()) {
- return false;
+ if (this->wasDestroyed()) {
+ return;
}
- fUniqueKey = key;
-
- if (!get_resource_cache(fGpu)->resourceAccess().didSetUniqueKey(this)) {
- fUniqueKey.reset();
- return false;
- }
- return true;
+ get_resource_cache(fGpu)->resourceAccess().changeUniqueKey(this, key);
}
void GrGpuResource::notifyIsPurgeable() const {
diff --git a/src/gpu/GrGpuResourceCacheAccess.h b/src/gpu/GrGpuResourceCacheAccess.h
index df0ca34..4f38fc6 100644
--- a/src/gpu/GrGpuResourceCacheAccess.h
+++ b/src/gpu/GrGpuResourceCacheAccess.h
@@ -55,6 +55,12 @@
}
}
+ /** Called by the cache to assign a new unique key. */
+ void setUniqueKey(const GrUniqueKey& key) { fResource->fUniqueKey = key; }
+
+ /** Called by the cache to make the unique key invalid. */
+ void removeUniqueKey() { fResource->fUniqueKey.reset(); }
+
uint32_t timestamp() const { return fResource->fTimestamp; }
void setTimestamp(uint32_t ts) { fResource->fTimestamp = ts; }
diff --git a/src/gpu/GrGpuResourcePriv.h b/src/gpu/GrGpuResourcePriv.h
index 92c53da..5324dcc 100644
--- a/src/gpu/GrGpuResourcePriv.h
+++ b/src/gpu/GrGpuResourcePriv.h
@@ -19,17 +19,15 @@
public:
/**
* Sets a unique key for the resource. If the resource was previously cached as scratch it will
- * be converted to a uniquely-keyed resource. Currently this may only be called once per
- * resource. It fails if there is already a resource with the same unique key. TODO: make this
- * supplant the resource that currently is using the unique key, allow resources' unique keys
- * to change, and allow removal of a unique key to convert a resource back to scratch.
+ * be converted to a uniquely-keyed resource. If the key is invalid then this is equivalent to
+ * removeUniqueKey(). If another resource is using the key then its unique key is removed and
+ * this resource takes over the key.
*/
- bool setUniqueKey(const GrUniqueKey& key) {
- return fResource->setUniqueKey(key);
- }
+ void setUniqueKey(const GrUniqueKey& key) { fResource->setUniqueKey(key); }
- /** Removes the unique key from a resource */
- void removeUniqueKey() { return fResource->removeUniqueKey(); }
+ /** Removes the unique key from a resource. If the resource has a scratch key, it may be
+ preserved for recycling as scratch. */
+ void removeUniqueKey() { fResource->removeUniqueKey(); }
/**
* If the resource is uncached make it cached. Has no effect on resources that are wrapped or
diff --git a/src/gpu/GrResourceCache.cpp b/src/gpu/GrResourceCache.cpp
index 8658744..a2fde2f 100644
--- a/src/gpu/GrResourceCache.cpp
+++ b/src/gpu/GrResourceCache.cpp
@@ -237,26 +237,50 @@
fScratchMap.remove(resource->resourcePriv().getScratchKey(), resource);
}
-void GrResourceCache::willRemoveUniqueKey(const GrGpuResource* resource) {
+void GrResourceCache::removeUniqueKey(GrGpuResource* resource) {
// Someone has a ref to this resource in order to invalidate it. When the ref count reaches
// zero we will get a notifyPurgable() and figure out what to do with it.
- SkASSERT(resource->getUniqueKey().isValid());
- fUniqueHash.remove(resource->getUniqueKey());
+ if (resource->getUniqueKey().isValid()) {
+ SkASSERT(resource == fUniqueHash.find(resource->getUniqueKey()));
+ fUniqueHash.remove(resource->getUniqueKey());
+ }
+ resource->cacheAccess().removeUniqueKey();
+ this->validate();
}
-bool GrResourceCache::didSetUniqueKey(GrGpuResource* resource) {
+void GrResourceCache::changeUniqueKey(GrGpuResource* resource, const GrUniqueKey& newKey) {
SkASSERT(resource);
SkASSERT(this->isInCache(resource));
- SkASSERT(resource->getUniqueKey().isValid());
- GrGpuResource* res = fUniqueHash.find(resource->getUniqueKey());
- if (NULL != res) {
- return false;
+ // Remove the entry for this resource if it already has a unique key.
+ if (resource->getUniqueKey().isValid()) {
+ SkASSERT(resource == fUniqueHash.find(resource->getUniqueKey()));
+ fUniqueHash.remove(resource->getUniqueKey());
+ SkASSERT(NULL == fUniqueHash.find(resource->getUniqueKey()));
}
- fUniqueHash.add(resource);
+ // If another resource has the new key, remove its key then install the key on this resource.
+ if (newKey.isValid()) {
+ if (GrGpuResource* old = fUniqueHash.find(newKey)) {
+ // If the old resource using the key is purgeable and is unreachable, then remove it.
+ if (!old->resourcePriv().getScratchKey().isValid() && old->isPurgeable()) {
+ // release may call validate() which will assert that resource is in fUniqueHash
+ // if it has a valid key. So in debug reset the key here before we assign it.
+ SkDEBUGCODE(resource->cacheAccess().removeUniqueKey();)
+ old->cacheAccess().release();
+ } else {
+ fUniqueHash.remove(newKey);
+ old->cacheAccess().removeUniqueKey();
+ }
+ }
+ SkASSERT(NULL == fUniqueHash.find(newKey));
+ resource->cacheAccess().setUniqueKey(newKey);
+ fUniqueHash.add(resource);
+ } else {
+ resource->cacheAccess().removeUniqueKey();
+ }
+
this->validate();
- return true;
}
void GrResourceCache::refAndMakeResourceMRU(GrGpuResource* resource) {
diff --git a/src/gpu/GrResourceCache.h b/src/gpu/GrResourceCache.h
index a908140..3e5df38 100644
--- a/src/gpu/GrResourceCache.h
+++ b/src/gpu/GrResourceCache.h
@@ -32,9 +32,9 @@
* between two temporary surfaces). The scratch key is set at resource creation time and
* should never change. Resources need not have a scratch key.
* 2) A unique key. This key's meaning is specific to the domain that created the key. Only one
- * resource may have a given unique key. The unique key can be set after resource creation
- * creation. Currently it may only be set once and cannot be cleared. This restriction will
- * be removed.
+ * resource may have a given unique key. The unique key can be set, cleared, or changed
+ * anytime after resource creation.
+ *
* A unique key always takes precedence over a scratch key when a resource has both types of keys.
* If a resource has neither key type then it will be deleted as soon as the last reference to it
* is dropped.
@@ -179,9 +179,9 @@
void removeResource(GrGpuResource*);
void notifyPurgeable(GrGpuResource*);
void didChangeGpuMemorySize(const GrGpuResource*, size_t oldSize);
- bool didSetUniqueKey(GrGpuResource*);
+ void changeUniqueKey(GrGpuResource*, const GrUniqueKey&);
+ void removeUniqueKey(GrGpuResource*);
void willRemoveScratchKey(const GrGpuResource*);
- void willRemoveUniqueKey(const GrGpuResource*);
void didChangeBudgetStatus(GrGpuResource*);
void refAndMakeResourceMRU(GrGpuResource*);
/// @}
@@ -297,20 +297,16 @@
}
/**
- * Called by GrGpuResources when their unique keys change.
- *
- * This currently returns a bool and fails when an existing resource has a key that collides
- * with the new key. In the future it will null out the unique key for the existing resource.
- * The failure is a temporary measure which will be fixed soon.
+ * Called by GrGpuResources to change their unique keys.
*/
- bool didSetUniqueKey(GrGpuResource* resource) { return fCache->didSetUniqueKey(resource); }
+ void changeUniqueKey(GrGpuResource* resource, const GrUniqueKey& newKey) {
+ fCache->changeUniqueKey(resource, newKey);
+ }
/**
- * Called by a GrGpuResource when it removes its unique key.
+ * Called by a GrGpuResource to remove its unique key.
*/
- void willRemoveUniqueKey(GrGpuResource* resource) {
- return fCache->willRemoveUniqueKey(resource);
- }
+ void removeUniqueKey(GrGpuResource* resource) { fCache->removeUniqueKey(resource); }
/**
* Called by a GrGpuResource when it removes its scratch key.
diff --git a/src/gpu/SkGr.cpp b/src/gpu/SkGr.cpp
index e353d41..e13c2c7 100644
--- a/src/gpu/SkGr.cpp
+++ b/src/gpu/SkGr.cpp
@@ -180,7 +180,7 @@
if (result && optionalKey.isValid()) {
BitmapInvalidator* listener = SkNEW_ARGS(BitmapInvalidator, (optionalKey));
pixelRefForInvalidationNotification->addGenIDChangeListener(listener);
- SkAssertResult(ctx->addResourceToCache(optionalKey, result));
+ ctx->addResourceToCache(optionalKey, result);
}
return result;
}
diff --git a/src/gpu/effects/GrTextureStripAtlas.cpp b/src/gpu/effects/GrTextureStripAtlas.cpp
index 7376419..51aaf53 100644
--- a/src/gpu/effects/GrTextureStripAtlas.cpp
+++ b/src/gpu/effects/GrTextureStripAtlas.cpp
@@ -204,7 +204,7 @@
fTexture = fDesc.fContext->findAndRefCachedTexture(key);
if (NULL == fTexture) {
fTexture = fDesc.fContext->createTexture(texDesc, true, NULL, 0);
- SkAssertResult(fDesc.fContext->addResourceToCache(key, fTexture));
+ fDesc.fContext->addResourceToCache(key, fTexture);
// This is a new texture, so all of our cache info is now invalid
this->initLRU();
fKeyTable.rewind();