When a surface is backed by an external render target force a copy on image snap
BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&issue=1694943002
Review URL: https://codereview.chromium.org/1694943002
diff --git a/src/gpu/GrGpuResource.cpp b/src/gpu/GrGpuResource.cpp
index 98cea1c..7c053b3 100644
--- a/src/gpu/GrGpuResource.cpp
+++ b/src/gpu/GrGpuResource.cpp
@@ -169,7 +169,7 @@
SkASSERT(!fScratchKey.isValid());
SkASSERT(scratchKey.isValid());
// Wrapped resources can never have a scratch key.
- if (this->cacheAccess().isExternal()) {
+ if (this->resourcePriv().isExternal()) {
return;
}
fScratchKey = scratchKey;
diff --git a/src/gpu/GrGpuResourceCacheAccess.h b/src/gpu/GrGpuResourceCacheAccess.h
index 70fe085..4b47a1c 100644
--- a/src/gpu/GrGpuResourceCacheAccess.h
+++ b/src/gpu/GrGpuResourceCacheAccess.h
@@ -31,11 +31,6 @@
}
/**
- * Is the resource object wrapping an externally allocated GPU resource?
- */
- bool isExternal() const { return fResource->isExternal(); }
-
- /**
* Is the resource object wrapping an externally allocated GPU resource that Skia has not taken
* ownership of.
*/
diff --git a/src/gpu/GrGpuResourcePriv.h b/src/gpu/GrGpuResourcePriv.h
index 62dc850..5da1c94 100644
--- a/src/gpu/GrGpuResourcePriv.h
+++ b/src/gpu/GrGpuResourcePriv.h
@@ -50,7 +50,12 @@
return SkBudgeted(ret);
}
- /**
+ /**
+ * Is the resource object wrapping an externally allocated GPU resource?
+ */
+ bool isExternal() const { return fResource->isExternal(); }
+
+ /**
* If this resource can be used as a scratch resource this returns a valid scratch key.
* Otherwise it returns a key for which isNullScratch is true. The resource may currently be
* used as a uniquely keyed resource rather than scratch. Check isScratch().
diff --git a/src/gpu/GrResourceCache.cpp b/src/gpu/GrResourceCache.cpp
index 005b6c4..087e762 100644
--- a/src/gpu/GrResourceCache.cpp
+++ b/src/gpu/GrResourceCache.cpp
@@ -147,7 +147,7 @@
#endif
}
if (resource->resourcePriv().getScratchKey().isValid()) {
- SkASSERT(!resource->cacheAccess().isExternal());
+ SkASSERT(!resource->resourcePriv().isExternal());
fScratchMap.insert(resource->resourcePriv().getScratchKey(), resource);
}
@@ -377,7 +377,7 @@
if (SkBudgeted::kNo == resource->resourcePriv().isBudgeted()) {
// Check whether this resource could still be used as a scratch resource.
- if (!resource->cacheAccess().isExternal() &&
+ if (!resource->resourcePriv().isExternal() &&
resource->resourcePriv().getScratchKey().isValid()) {
// We won't purge an existing resource to make room for this one.
if (fBudgetedCount < fMaxCount &&
@@ -662,19 +662,19 @@
SkASSERT(!resource->getUniqueKey().isValid());
++fScratch;
SkASSERT(fScratchMap->countForKey(resource->resourcePriv().getScratchKey()));
- SkASSERT(!resource->cacheAccess().isExternal());
+ SkASSERT(!resource->resourcePriv().isExternal());
} else if (resource->resourcePriv().getScratchKey().isValid()) {
SkASSERT(SkBudgeted::kNo == resource->resourcePriv().isBudgeted() ||
resource->getUniqueKey().isValid());
++fCouldBeScratch;
SkASSERT(fScratchMap->countForKey(resource->resourcePriv().getScratchKey()));
- SkASSERT(!resource->cacheAccess().isExternal());
+ SkASSERT(!resource->resourcePriv().isExternal());
}
const GrUniqueKey& uniqueKey = resource->getUniqueKey();
if (uniqueKey.isValid()) {
++fContent;
SkASSERT(fUniqueHash->find(uniqueKey) == resource);
- SkASSERT(!resource->cacheAccess().isExternal());
+ SkASSERT(!resource->resourcePriv().isExternal());
SkASSERT(SkBudgeted::kYes == resource->resourcePriv().isBudgeted());
}
diff --git a/src/gpu/GrResourceCache.h b/src/gpu/GrResourceCache.h
index 8321289..fb13dde 100644
--- a/src/gpu/GrResourceCache.h
+++ b/src/gpu/GrResourceCache.h
@@ -211,7 +211,7 @@
if (resource->cacheAccess().isScratch()) {
++fScratch;
}
- if (resource->cacheAccess().isExternal()) {
+ if (resource->resourcePriv().isExternal()) {
++fExternal;
}
if (resource->cacheAccess().isBorrowed()) {
diff --git a/src/image/SkSurface_Gpu.cpp b/src/image/SkSurface_Gpu.cpp
index 5345cee..173a0d8 100644
--- a/src/image/SkSurface_Gpu.cpp
+++ b/src/image/SkSurface_Gpu.cpp
@@ -82,8 +82,10 @@
SkASSERT(rt);
GrTexture* tex = rt->asTexture();
SkAutoTUnref<GrTexture> copy;
- // TODO: Force a copy when the rt is an external resource.
- if (kYes_ForceCopyMode == forceCopyMode || !tex) {
+ // If the original render target is a buffer originally created by the client, then we don't
+ // want to ever retarget the SkSurface at another buffer we create. Force a copy now to avoid
+ // copy-on-write.
+ if (kYes_ForceCopyMode == forceCopyMode || !tex || rt->resourcePriv().isExternal()) {
GrSurfaceDesc desc = fDevice->accessRenderTarget()->desc();
GrContext* ctx = fDevice->context();
desc.fFlags = desc.fFlags & ~kRenderTarget_GrSurfaceFlag;
diff --git a/tests/SurfaceTest.cpp b/tests/SurfaceTest.cpp
index 7449f21..8166bba 100644
--- a/tests/SurfaceTest.cpp
+++ b/tests/SurfaceTest.cpp
@@ -439,9 +439,7 @@
{
SkAutoTUnref<SkSurface> surface(
SkSurface::NewRenderTargetDirect(texture->asRenderTarget()));
- // We should be able to pass true here, but disallowing copy on write for direct GPU
- // surfaces is not yet implemented.
- test_unique_image_snap(reporter, surface, false, imageBackingStore,
+ test_unique_image_snap(reporter, surface, true, imageBackingStore,
surfaceBackingStore);
}
texture->unref();