Make copySurface work for texture dsts, return a bool, & add unit test.

GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&issue=1684313002

Committed: https://skia.googlesource.com/skia/+/7ea5e28065e5eb797e95f5d81c1a65cf3209d741

Review URL: https://codereview.chromium.org/1684313002

8 files changed, 56 insertions, 39 deletions

diff --git a/src/gpu/GrContext.cpp b/src/gpu/GrContext.cpp
index 187a3ca37a..296dfb9e70 100644
--- a/src/gpu/GrContext.cpp
+++ b/src/gpu/GrContext.cpp
@@ -18,6 +18,7 @@
 #include "SkConfig8888.h"
 #include "SkGrPriv.h"
 
+#include "batches/GrCopySurfaceBatch.h"
 #include "effects/GrConfigConversionEffect.h"
 #include "text/GrTextBlobCache.h"
 
@@ -509,34 +510,42 @@ void GrContext::prepareSurfaceForExternalIO(GrSurface* surface) {
     }
 }
 
-void GrContext::copySurface(GrSurface* dst, GrSurface* src, const SkIRect& srcRect,
-                            const SkIPoint& dstPoint, uint32_t pixelOpsFlags) {
+bool GrContext::copySurface(GrSurface* dst, GrSurface* src, const SkIRect& srcRect,
+                            const SkIPoint& dstPoint) {
     ASSERT_SINGLE_OWNER
-    RETURN_IF_ABANDONED
+    RETURN_FALSE_IF_ABANDONED
     GR_AUDIT_TRAIL_AUTO_FRAME(&fAuditTrail, "GrContext::copySurface");
 
     if (!src || !dst) {
-        return;
+        return false;
     }
     ASSERT_OWNED_RESOURCE(src);
     ASSERT_OWNED_RESOURCE(dst);
 
-    // Since we're going to the draw target and not GPU, no need to check kNoFlush
-    // here.
     if (!dst->asRenderTarget()) {
-        return;
+        SkIRect clippedSrcRect;
+        SkIPoint clippedDstPoint;
+        if (!GrCopySurfaceBatch::ClipSrcRectAndDstPoint(dst, src, srcRect, dstPoint,
+                                                        &clippedSrcRect, &clippedDstPoint)) {
+            return false;
+        }
+        // If we don't have an RT for the dst then we won't have a GrDrawContext to insert the
+        // the copy surface into. In the future we plan to have a more limited Context type
+        // (GrCopyContext?) that has the subset of GrDrawContext operations that should be
+        // allowed on textures that aren't render targets.
+        // For now we just flush any writes to the src and issue an immediate copy to the dst.
+        src->flushWrites();
+        return fGpu->copySurface(dst, src, clippedSrcRect, clippedDstPoint);
     }
-
     SkAutoTUnref<GrDrawContext> drawContext(this->drawContext(dst->asRenderTarget()));
     if (!drawContext) {
-        return;
+        return false;
     }
 
-    drawContext->copySurface(src, srcRect, dstPoint);
-
-    if (kFlushWrites_PixelOp & pixelOpsFlags) {
-        this->flush();
+    if (!drawContext->copySurface(src, srcRect, dstPoint)) {
+        return false;
     }
+    return true;
 }
 
 void GrContext::flushSurfaceWrites(GrSurface* surface) {
diff --git a/src/gpu/GrDrawContext.cpp b/src/gpu/GrDrawContext.cpp
index f37c48c9f3..d971f58ece 100644
--- a/src/gpu/GrDrawContext.cpp
+++ b/src/gpu/GrDrawContext.cpp
@@ -97,13 +97,13 @@ GrDrawTarget* GrDrawContext::getDrawTarget() {
     return fDrawTarget;
 }
 
-void GrDrawContext::copySurface(GrSurface* src, const SkIRect& srcRect, const SkIPoint& dstPoint) {
+bool GrDrawContext::copySurface(GrSurface* src, const SkIRect& srcRect, const SkIPoint& dstPoint) {
     ASSERT_SINGLE_OWNER
-    RETURN_IF_ABANDONED
+    RETURN_FALSE_IF_ABANDONED
     SkDEBUGCODE(this->validate();)
     GR_AUDIT_TRAIL_AUTO_FRAME(fAuditTrail, "GrDrawContext::copySurface");
 
-    this->getDrawTarget()->copySurface(fRenderTarget, src, srcRect, dstPoint);
+    return this->getDrawTarget()->copySurface(fRenderTarget, src, srcRect, dstPoint);
 }
 
 void GrDrawContext::drawText(const GrClip& clip, const GrPaint& grPaint,
diff --git a/src/gpu/GrDrawTarget.cpp b/src/gpu/GrDrawTarget.cpp
index b9dc794526..9f15c11508 100644
--- a/src/gpu/GrDrawTarget.cpp
+++ b/src/gpu/GrDrawTarget.cpp
@@ -406,19 +406,21 @@ void GrDrawTarget::discard(GrRenderTarget* renderTarget) {
 
 ////////////////////////////////////////////////////////////////////////////////
 
-void GrDrawTarget::copySurface(GrSurface* dst,
+bool GrDrawTarget::copySurface(GrSurface* dst,
                                GrSurface* src,
                                const SkIRect& srcRect,
                                const SkIPoint& dstPoint) {
     GrBatch* batch = GrCopySurfaceBatch::Create(dst, src, srcRect, dstPoint);
-    if (batch) {
+    if (!batch) {
+        return false;
+    }
 #ifdef ENABLE_MDB
-        this->addDependency(src);
+    this->addDependency(src);
 #endif
 
-        this->recordBatch(batch);
-        batch->unref();
-    }
+    this->recordBatch(batch);
+    batch->unref();
+    return true;
 }
 
 template <class Left, class Right> static bool intersect(const Left& a, const Right& b) {
diff --git a/src/gpu/GrDrawTarget.h b/src/gpu/GrDrawTarget.h
index 55c11da667..a850efd842 100644
--- a/src/gpu/GrDrawTarget.h
+++ b/src/gpu/GrDrawTarget.h
@@ -144,7 +144,7 @@ public:
      * depending on the type of surface, configs, etc, and the backend-specific
      * limitations.
      */
-    void copySurface(GrSurface* dst,
+    bool copySurface(GrSurface* dst,
                      GrSurface* src,
                      const SkIRect& srcRect,
                      const SkIPoint& dstPoint);
diff --git a/src/gpu/SkGrPixelRef.cpp b/src/gpu/SkGrPixelRef.cpp
index 3876f17411..e48cbf5d15 100644
--- a/src/gpu/SkGrPixelRef.cpp
+++ b/src/gpu/SkGrPixelRef.cpp
@@ -84,10 +84,10 @@ static SkGrPixelRef* copy_to_new_texture_pixelref(GrTexture* texture, SkColorTyp
     }
 
     // Blink is relying on the above copy being sent to GL immediately in the case when the source
-    // is a WebGL canvas backing store. We could have a TODO to remove this flush flag, but we have
+    // is a WebGL canvas backing store. We could have a TODO to remove this flush, but we have
     // a larger TODO to remove SkGrPixelRef entirely.
-    context->copySurface(dst->asRenderTarget(), texture, srcRect, SkIPoint::Make(0,0),
-                         GrContext::kFlushWrites_PixelOp);
+    context->copySurface(dst, texture, srcRect, SkIPoint::Make(0,0));
+    context->flushSurfaceWrites(dst);
 
     SkImageInfo info = SkImageInfo::Make(desc.fWidth, desc.fHeight, dstCT, kPremul_SkAlphaType,
                                          dstPT);
diff --git a/src/gpu/batches/GrCopySurfaceBatch.cpp b/src/gpu/batches/GrCopySurfaceBatch.cpp
index 098f7c7704..a59ed38f51 100644
--- a/src/gpu/batches/GrCopySurfaceBatch.cpp
+++ b/src/gpu/batches/GrCopySurfaceBatch.cpp
@@ -9,12 +9,12 @@
 #include "GrCopySurfaceBatch.h"
 
 // returns true if the read/written rect intersects the src/dst and false if not.
-static bool clip_srcrect_and_dstpoint(const GrSurface* dst,
-                                      const GrSurface* src,
-                                      const SkIRect& srcRect,
-                                      const SkIPoint& dstPoint,
-                                      SkIRect* clippedSrcRect,
-                                      SkIPoint* clippedDstPoint) {
+bool GrCopySurfaceBatch::ClipSrcRectAndDstPoint(const GrSurface* dst,
+                                                const GrSurface* src,
+                                                const SkIRect& srcRect,
+                                                const SkIPoint& dstPoint,
+                                                SkIRect* clippedSrcRect,
+                                                SkIPoint* clippedDstPoint) {
     *clippedSrcRect = srcRect;
     *clippedDstPoint = dstPoint;
 
@@ -67,12 +67,7 @@ GrBatch* GrCopySurfaceBatch::Create(GrSurface* dst, GrSurface* src, const SkIRec
     SkIRect clippedSrcRect;
     SkIPoint clippedDstPoint;
     // If the rect is outside the src or dst then we've already succeeded.
-    if (!clip_srcrect_and_dstpoint(dst,
-                                    src,
-                                    srcRect,
-                                    dstPoint,
-                                    &clippedSrcRect,
-                                    &clippedDstPoint)) {
+    if (!ClipSrcRectAndDstPoint(dst, src, srcRect, dstPoint, &clippedSrcRect, &clippedDstPoint)) {
         return nullptr;
     }
     return new GrCopySurfaceBatch(dst, src, clippedSrcRect, clippedDstPoint);
diff --git a/src/gpu/batches/GrCopySurfaceBatch.h b/src/gpu/batches/GrCopySurfaceBatch.h
index 7bf8d8d8c2..3926643f8a 100644
--- a/src/gpu/batches/GrCopySurfaceBatch.h
+++ b/src/gpu/batches/GrCopySurfaceBatch.h
@@ -17,6 +17,16 @@ class GrCopySurfaceBatch final : public GrBatch {
 public:
     DEFINE_BATCH_CLASS_ID
 
+    /** This should not really be exposed as Create() will apply this clipping, but there is
+     *  currently a workaround in GrContext::copySurface() for non-render target dsts that relies
+     *  on it. */
+    static bool ClipSrcRectAndDstPoint(const GrSurface* dst,
+                                       const GrSurface* src,
+                                       const SkIRect& srcRect,
+                                       const SkIPoint& dstPoint,
+                                       SkIRect* clippedSrcRect,
+                                       SkIPoint* clippedDstPoint);
+
     static GrBatch* Create(GrSurface* dst, GrSurface* src, const SkIRect& srcRect,
                            const SkIPoint& dstPoint);
 
diff --git a/src/image/SkImage_Gpu.cpp b/src/image/SkImage_Gpu.cpp
index 04939e36f1..c502fc0092 100644
--- a/src/image/SkImage_Gpu.cpp
+++ b/src/image/SkImage_Gpu.cpp
@@ -326,7 +326,8 @@ GrTexture* GrDeepCopyTexture(GrTexture* src, bool budgeted) {
 
     const SkIRect srcR = SkIRect::MakeWH(desc.fWidth, desc.fHeight);
     const SkIPoint dstP = SkIPoint::Make(0, 0);
-    ctx->copySurface(dst, src, srcR, dstP, GrContext::kFlushWrites_PixelOp);
+    ctx->copySurface(dst, src, srcR, dstP);
+    ctx->flushSurfaceWrites(dst);
     return dst;
 }