Revert of GrContext::copyTexture->GrContext::copySurface. Add a flush writes pixel ops flag. (patchset #3 id:40001 of https://codereview.chromium.org/622663002/)

Reason for revert:
Breaking GMs on some bots

Original issue's description:
> GrContext::copyTexture->GrContext::copySurface.
>
> Add a flush writes pixel ops flag.
>
> Add an explicit flush writes for GrSurface.
>
> BUG=skia:2977
>
> Committed: https://skia.googlesource.com/skia/+/cf99b00980b6c9c557e71abf1a7c9f9b21217262

TBR=robertphillips@google.com
NOTREECHECKS=true
NOTRY=true
BUG=skia:2977

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

7 files changed, 51 insertions, 102 deletions

diff --git a/include/gpu/GrContext.h b/include/gpu/GrContext.h
index 4500936b82..366fba993e 100644
--- a/include/gpu/GrContext.h
+++ b/include/gpu/GrContext.h
@@ -597,15 +597,12 @@ public:
     * These flags can be used with the read/write pixels functions below.
     */
     enum PixelOpsFlags {
-        /** The GrContext will not be flushed before the surface read or write. This means that
-            the read or write may occur before previous draws have executed. */
+        /** The GrContext will not be flushed. This means that the read or write may occur before
+            previous draws have executed. */
         kDontFlush_PixelOpsFlag = 0x1,
-        /** Any surface writes should be flushed to the backend 3D API after the surface operation
-            is complete */
-        kFlushWrites_PixelOp = 0x2,
         /** The src for write or dst read is unpremultiplied. This is only respected if both the
             config src and dst configs are an RGBA/BGRA 8888 format. */
-        kUnpremul_PixelOpsFlag  = 0x4,
+        kUnpremul_PixelOpsFlag  = 0x2,
     };
 
     /**
@@ -698,36 +695,15 @@ public:
                             uint32_t pixelOpsFlags = 0);
 
     /**
-     * Copies a rectangle of texels from src to dst.
+     * Copies a rectangle of texels from src to dst. The size of dst is the size of the rectangle
+     * copied and topLeft is the position of the rect in src. The rectangle is clipped to src's
      * bounds.
-     * @param dst           the surface to copy to.
-     * @param src           the surface to copy from.
-     * @param srcRect       the rectangle of the src that should be copied.
-     * @param dstPoint      the translation applied when writing the srcRect's pixels to the dst.
-     * @param pixelOpsFlags see PixelOpsFlags enum above. (kUnpremul_PixelOpsFlag is not allowed).
+     * @param src           the texture to copy from.
+     * @param dst           the render target to copy to.
+     * @param topLeft       the point in src that will be copied to the top-left of dst. If NULL,
+     *                      (0, 0) will be used.
      */
-    void copySurface(GrSurface* dst,
-                     GrSurface* src,
-                     const SkIRect& srcRect,
-                     const SkIPoint& dstPoint,
-                     uint32_t pixelOpsFlags = 0);
-
-    /** Helper that copies the whole surface but fails when the two surfaces are not identically
-        sized. */
-    bool copySurface(GrSurface* dst, GrSurface* src) {
-        if (NULL == dst || NULL == src || dst->width() != src->width() ||
-            dst->height() != src->height()) {
-            return false;
-        }
-        this->copySurface(dst, src, SkIRect::MakeWH(dst->width(), dst->height()),
-                          SkIPoint::Make(0,0));
-        return true;
-    }
-
-    /**
-     * After this returns any pending writes to the surface will have been issued to the backend 3D API.
-     */
-    void flushSurfaceWrites(GrSurface* surface);
+    void copyTexture(GrTexture* src, GrRenderTarget* dst, const SkIPoint* topLeft = NULL);
 
     /**
      * Resolves a render target that has MSAA. The intermediate MSAA buffer is
diff --git a/include/gpu/GrSurface.h b/include/gpu/GrSurface.h
index 0bbf8d9c12..23311a2f52 100644
--- a/include/gpu/GrSurface.h
+++ b/include/gpu/GrSurface.h
@@ -112,11 +112,6 @@ public:
                              size_t rowBytes = 0,
                              uint32_t pixelOpsFlags = 0) = 0;
 
-    /**
-     * After this returns any pending writes to the surface will be issued to the backend 3D API.
-     */
-    void flushWrites();
-
     /** Access methods that are only to be used within Skia code. */
     inline GrSurfacePriv surfacePriv();
     inline const GrSurfacePriv surfacePriv() const;
diff --git a/samplecode/SampleApp.cpp b/samplecode/SampleApp.cpp
index 67e8dd3d2e..6ca67bcbcd 100644
--- a/samplecode/SampleApp.cpp
+++ b/samplecode/SampleApp.cpp
@@ -297,8 +297,7 @@ public:
                                              SkImageInfo2GrPixelConfig(bm.colorType(),
                                                                        bm.alphaType()),
                                              bm.getPixels(),
-                                             bm.rowBytes(),
-                                             GrContext::kFlushWrites_PixelOp);
+                                             bm.rowBytes());
             }
         }
 #endif
diff --git a/src/gpu/GrContext.cpp b/src/gpu/GrContext.cpp
index d0f3cc5671..46a5576b11 100755
--- a/src/gpu/GrContext.cpp
+++ b/src/gpu/GrContext.cpp
@@ -1374,8 +1374,6 @@ bool GrContext::writeTexturePixels(GrTexture* texture,
 
     return fGpu->writeTexturePixels(texture, left, top, width, height,
                                     config, buffer, rowBytes);
-
-    // No need to check the kFlushWrites flag here since we issued the write directly to fGpu.
 }
 
 bool GrContext::readTexturePixels(GrTexture* texture,
@@ -1405,8 +1403,7 @@ bool GrContext::readTexturePixels(GrTexture* texture,
         ast.set(this, desc, kExact_ScratchTexMatch);
         GrTexture* dst = ast.texture();
         if (dst && (target = dst->asRenderTarget())) {
-            this->copySurface(target, texture, SkIRect::MakeXYWH(top, left, width, height),
-                              SkIPoint::Make(0,0));
+            this->copyTexture(texture, target, NULL);
             return this->readRenderTargetPixels(target,
                                                 left, top, width, height,
                                                 config, buffer, rowBytes,
@@ -1594,26 +1591,29 @@ void GrContext::discardRenderTarget(GrRenderTarget* renderTarget) {
     target->discard(renderTarget);
 }
 
-void GrContext::copySurface(GrSurface* dst, GrSurface* src, const SkIRect& srcRect,
-                            const SkIPoint& dstPoint, uint32_t pixelOpsFlags) {
+void GrContext::copyTexture(GrTexture* src, GrRenderTarget* dst, const SkIPoint* topLeft) {
     if (NULL == src || NULL == dst) {
         return;
     }
     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.
+    SkIRect srcRect = SkIRect::MakeWH(dst->width(), dst->height());
+    if (topLeft) {
+        srcRect.offset(*topLeft);
+    }
+    SkIRect srcBounds = SkIRect::MakeWH(src->width(), src->height());
+    if (!srcRect.intersect(srcBounds)) {
+        return;
+    }
 
     GrDrawTarget* target = this->prepareToDraw(NULL, BUFFERED_DRAW, NULL, NULL);
     if (NULL == target) {
         return;
     }
+    SkIPoint dstPoint;
+    dstPoint.setZero();
     target->copySurface(dst, src, srcRect, dstPoint);
-
-    if (kFlushWrites_PixelOp & pixelOpsFlags) {
-        this->flush();
-    }
 }
 
 bool GrContext::writeRenderTargetPixels(GrRenderTarget* renderTarget,
@@ -1621,7 +1621,7 @@ bool GrContext::writeRenderTargetPixels(GrRenderTarget* renderTarget,
                                         GrPixelConfig srcConfig,
                                         const void* buffer,
                                         size_t rowBytes,
-                                        uint32_t pixelOpsFlags) {
+                                        uint32_t flags) {
     ASSERT_OWNED_RESOURCE(renderTarget);
 
     if (NULL == renderTarget) {
@@ -1646,11 +1646,11 @@ bool GrContext::writeRenderTargetPixels(GrRenderTarget* renderTarget,
     // At least some drivers on the Mac get confused when glTexImage2D is called on a texture
     // attached to an FBO. The FBO still sees the old image. TODO: determine what OS versions and/or
     // HW is affected.
-    if (renderTarget->asTexture() && !(kUnpremul_PixelOpsFlag & pixelOpsFlags) &&
+    if (renderTarget->asTexture() && !(kUnpremul_PixelOpsFlag & flags) &&
         fGpu->canWriteTexturePixels(renderTarget->asTexture(), srcConfig)) {
         return this->writeTexturePixels(renderTarget->asTexture(),
                                         left, top, width, height,
-                                        srcConfig, buffer, rowBytes, pixelOpsFlags);
+                                        srcConfig, buffer, rowBytes, flags);
     }
 #endif
 
@@ -1683,7 +1683,7 @@ bool GrContext::writeRenderTargetPixels(GrRenderTarget* renderTarget,
     // allocate a tmp buffer and sw convert the pixels to premul
     SkAutoSTMalloc<128 * 128, uint32_t> tmpPixels(0);
 
-    if (kUnpremul_PixelOpsFlag & pixelOpsFlags) {
+    if (kUnpremul_PixelOpsFlag & flags) {
         if (!GrPixelConfigIs8888(srcConfig)) {
             return false;
         }
@@ -1724,42 +1724,25 @@ bool GrContext::writeRenderTargetPixels(GrRenderTarget* renderTarget,
     if (!this->writeTexturePixels(texture,
                                   0, 0, width, height,
                                   writeConfig, buffer, rowBytes,
-                                  pixelOpsFlags & ~kUnpremul_PixelOpsFlag)) {
+                                  flags & ~kUnpremul_PixelOpsFlag)) {
         return false;
     }
 
     SkMatrix matrix;
     matrix.setTranslate(SkIntToScalar(left), SkIntToScalar(top));
 
-
     // This function can be called in the midst of drawing another object (e.g., when uploading a
     // SW-rasterized clip while issuing a draw). So we push the current geometry state before
     // drawing a rect to the render target.
-    // The bracket ensures we pop the stack if we wind up flushing below.
-    {
-        GrDrawTarget* drawTarget = this->prepareToDraw(NULL, kYes_BufferedDraw, NULL, NULL);
-        GrDrawTarget::AutoGeometryAndStatePush agasp(drawTarget, GrDrawTarget::kReset_ASRInit,
-                                                     &matrix);
-        GrDrawState* drawState = drawTarget->drawState();
-        drawState->addColorProcessor(fp);
-        drawState->setRenderTarget(renderTarget);
-        drawState->disableState(GrDrawState::kClip_StateBit);
-        drawTarget->drawSimpleRect(SkRect::MakeWH(SkIntToScalar(width), SkIntToScalar(height)));
-    }
-
-    if (kFlushWrites_PixelOp & pixelOpsFlags) {
-        this->flush();
-    }
-
+    GrDrawTarget* drawTarget = this->prepareToDraw(NULL, kYes_BufferedDraw, NULL, NULL);
+    GrDrawTarget::AutoGeometryAndStatePush agasp(drawTarget, GrDrawTarget::kReset_ASRInit, &matrix);
+    GrDrawState* drawState = drawTarget->drawState();
+    drawState->addColorProcessor(fp);
+    drawState->setRenderTarget(renderTarget);
+    drawState->disableState(GrDrawState::kClip_StateBit);
+    drawTarget->drawSimpleRect(SkRect::MakeWH(SkIntToScalar(width), SkIntToScalar(height)));
     return true;
 }
-
-void GrContext::flushSurfaceWrites(GrSurface* surface) {
-    if (surface->surfacePriv().hasPendingWrite()) {
-        this->flush();
-    }
-}
-
 ////////////////////////////////////////////////////////////////////////////////
 
 GrDrawTarget* GrContext::prepareToDraw(const GrPaint* paint,
diff --git a/src/gpu/GrSurface.cpp b/src/gpu/GrSurface.cpp
index d067f07d0d..37fd73f4f4 100644
--- a/src/gpu/GrSurface.cpp
+++ b/src/gpu/GrSurface.cpp
@@ -48,12 +48,6 @@ bool GrSurface::savePixels(const char* filename) {
     return true;
 }
 
-void GrSurface::flushWrites() {
-    if (!this->wasDestroyed()) {
-        this->getContext()->flushSurfaceWrites(this);
-    }
-}
-
 bool GrSurface::hasPendingRead() const {
     const GrTexture* thisTex = this->asTexture();
     if (thisTex && thisTex->internalHasPendingRead()) {
diff --git a/src/gpu/SkGrPixelRef.cpp b/src/gpu/SkGrPixelRef.cpp
index 0fe219f6f0..fcf22e350b 100644
--- a/src/gpu/SkGrPixelRef.cpp
+++ b/src/gpu/SkGrPixelRef.cpp
@@ -64,18 +64,19 @@ static SkGrPixelRef* copy_to_new_texture_pixelref(GrTexture* texture, SkColorTyp
     }
     GrTextureDesc desc;
 
-    SkIRect srcRect;
-
-    if (!subset) {
-        desc.fWidth  = texture->width();
-        desc.fHeight = texture->height();
-        srcRect = SkIRect::MakeWH(texture->width(), texture->height());
-    } else {
+    SkIPoint pointStorage;
+    SkIPoint* topLeft;
+    if (subset != NULL) {
         SkASSERT(SkIRect::MakeWH(texture->width(), texture->height()).contains(*subset));
         // Create a new texture that is the size of subset.
         desc.fWidth = subset->width();
         desc.fHeight = subset->height();
-        srcRect = *subset;
+        pointStorage.set(subset->x(), subset->y());
+        topLeft = &pointStorage;
+    } else {
+        desc.fWidth  = texture->width();
+        desc.fHeight = texture->height();
+        topLeft = NULL;
     }
     desc.fFlags = kRenderTarget_GrTextureFlagBit | kNoStencil_GrTextureFlagBit;
     desc.fConfig = SkImageInfo2GrPixelConfig(dstCT, kPremul_SkAlphaType);
@@ -85,12 +86,13 @@ static SkGrPixelRef* copy_to_new_texture_pixelref(GrTexture* texture, SkColorTyp
         return NULL;
     }
 
+    context->copyTexture(texture, dst->asRenderTarget(), topLeft);
+
     // 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
-    // a larger TODO to remove SkGrPixelRef entirely.
-    context->copySurface(texture, dst->asRenderTarget(), srcRect, SkIPoint::Make(0,0),
-                         GrContext::kFlushWrites_PixelOp);
-  
+    // 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->flush();
+
     SkImageInfo info = SkImageInfo::Make(desc.fWidth, desc.fHeight, dstCT, kPremul_SkAlphaType);
     SkGrPixelRef* pixelRef = SkNEW_ARGS(SkGrPixelRef, (info, dst));
     SkSafeUnref(dst);
diff --git a/src/image/SkSurface_Gpu.cpp b/src/image/SkSurface_Gpu.cpp
index fb087ea24b..024c151cea 100644
--- a/src/image/SkSurface_Gpu.cpp
+++ b/src/image/SkSurface_Gpu.cpp
@@ -89,7 +89,7 @@ void SkSurface_Gpu::onCopyOnWrite(ContentChangeMode mode) {
             fDevice->createCompatibleDevice(fDevice->imageInfo()));
         SkAutoTUnref<SkGpuDevice> aurd(newDevice);
         if (kRetain_ContentChangeMode == mode) {
-            fDevice->context()->copySurface(newDevice->accessRenderTarget(), rt->asTexture());
+            fDevice->context()->copyTexture(rt->asTexture(), newDevice->accessRenderTarget());
         }
         SkASSERT(this->getCachedCanvas());
         SkASSERT(this->getCachedCanvas()->getDevice() == fDevice);