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

7 files changed, 18 insertions, 18 deletions

diff --git a/src/gpu/GrGpuResource.cpp b/src/gpu/GrGpuResource.cpp
index 98cea1c9a8..7c053b31d7 100644
--- a/src/gpu/GrGpuResource.cpp
+++ b/src/gpu/GrGpuResource.cpp
@@ -169,7 +169,7 @@ void GrGpuResource::setScratchKey(const GrScratchKey& scratchKey) {
     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 70fe08503e..4b47a1c932 100644
--- a/src/gpu/GrGpuResourceCacheAccess.h
+++ b/src/gpu/GrGpuResourceCacheAccess.h
@@ -31,11 +31,6 @@ private:
     }
 
     /**
-     * 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 62dc85059a..5da1c94878 100644
--- a/src/gpu/GrGpuResourcePriv.h
+++ b/src/gpu/GrGpuResourcePriv.h
@@ -50,7 +50,12 @@ public:
         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 005b6c4ccd..087e762592 100644
--- a/src/gpu/GrResourceCache.cpp
+++ b/src/gpu/GrResourceCache.cpp
@@ -147,7 +147,7 @@ void GrResourceCache::insertResource(GrGpuResource* resource) {
 #endif
     }
     if (resource->resourcePriv().getScratchKey().isValid()) {
-        SkASSERT(!resource->cacheAccess().isExternal());
+        SkASSERT(!resource->resourcePriv().isExternal());
         fScratchMap.insert(resource->resourcePriv().getScratchKey(), resource);
     }
 
@@ -377,7 +377,7 @@ void GrResourceCache::notifyCntReachedZero(GrGpuResource* resource, uint32_t fla
 
     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 @@ void GrResourceCache::validate() const {
                 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 8321289dff..fb13dde88e 100644
--- a/src/gpu/GrResourceCache.h
+++ b/src/gpu/GrResourceCache.h
@@ -211,7 +211,7 @@ public:
             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 5345ceee20..173a0d81d7 100644
--- a/src/image/SkSurface_Gpu.cpp
+++ b/src/image/SkSurface_Gpu.cpp
@@ -82,8 +82,10 @@ SkImage* SkSurface_Gpu::onNewImageSnapshot(SkBudgeted budgeted, ForceCopyMode fo
     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 7449f211b5..8166bba9bd 100644
--- a/tests/SurfaceTest.cpp
+++ b/tests/SurfaceTest.cpp
@@ -439,9 +439,7 @@ DEF_GPUTEST_FOR_RENDERING_CONTEXTS(UniqueImageSnapshot_Gpu, reporter, context) {
         {
             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();