Correctly discard or load RT when doing copies as draws in Vulkan

This fixes all the copy as draw issues we've had with certain devices and
the cap is no longer needed.

Bug: skia:
Change-Id: Id0b750849c4c920beae2d8cb3eda5f402018f194
Reviewed-on: https://skia-review.googlesource.com/114860
Commit-Queue: Greg Daniel <egdaniel@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>

16 files changed, 51 insertions, 48 deletions

diff --git a/src/gpu/GrContext.cpp b/src/gpu/GrContext.cpp
index 510225e01e..f9c941565c 100644
--- a/src/gpu/GrContext.cpp
+++ b/src/gpu/GrContext.cpp
@@ -963,7 +963,14 @@ sk_sp<GrSurfaceContext> GrContextPriv::makeDeferredSurfaceContext(const GrSurfac
         return nullptr;
     }
 
-    return this->makeWrappedSurfaceContext(std::move(proxy), std::move(colorSpace), props);
+    sk_sp<GrSurfaceContext> sContext = this->makeWrappedSurfaceContext(std::move(proxy),
+                                                                       std::move(colorSpace),
+                                                                       props);
+    if (sContext && sContext->asRenderTargetContext()) {
+        sContext->asRenderTargetContext()->discard();
+    }
+
+    return sContext;
 }
 
 sk_sp<GrTextureContext> GrContextPriv::makeBackendTextureContext(const GrBackendTexture& tex,
diff --git a/src/gpu/GrGpu.cpp b/src/gpu/GrGpu.cpp
index fe42a9391e..18a13de991 100644
--- a/src/gpu/GrGpu.cpp
+++ b/src/gpu/GrGpu.cpp
@@ -185,11 +185,13 @@ GrBuffer* GrGpu::createBuffer(size_t size, GrBufferType intendedType,
 
 bool GrGpu::copySurface(GrSurface* dst, GrSurfaceOrigin dstOrigin,
                         GrSurface* src, GrSurfaceOrigin srcOrigin,
-                        const SkIRect& srcRect, const SkIPoint& dstPoint) {
+                        const SkIRect& srcRect, const SkIPoint& dstPoint,
+                        bool canDiscardOutsideDstRect) {
     GR_CREATE_TRACE_MARKER_CONTEXT("GrGpu", "copySurface", fContext);
     SkASSERT(dst && src);
     this->handleDirtyContext();
-    return this->onCopySurface(dst, dstOrigin, src, srcOrigin, srcRect, dstPoint);
+    return this->onCopySurface(dst, dstOrigin, src, srcOrigin, srcRect, dstPoint,
+                               canDiscardOutsideDstRect);
 }
 
 bool GrGpu::getReadPixelsInfo(GrSurface* srcSurface, GrSurfaceOrigin srcOrigin, int width,
diff --git a/src/gpu/GrGpu.h b/src/gpu/GrGpu.h
index 2aacc49151..16561418aa 100644
--- a/src/gpu/GrGpu.h
+++ b/src/gpu/GrGpu.h
@@ -345,11 +345,13 @@ public:
     // Called to perform a surface to surface copy. Fallbacks to issuing a draw from the src to dst
     // take place at the GrOpList level and this function implement faster copy paths. The rect
     // and point are pre-clipped. The src rect and implied dst rect are guaranteed to be within the
-    // src/dst bounds and non-empty.
+    // src/dst bounds and non-empty. If canDiscardOutsideDstRect is set to true then we don't need
+    // to preserve any data on the dst surface outside of the copy.
     bool copySurface(GrSurface* dst, GrSurfaceOrigin dstOrigin,
                      GrSurface* src, GrSurfaceOrigin srcOrigin,
                      const SkIRect& srcRect,
-                     const SkIPoint& dstPoint);
+                     const SkIPoint& dstPoint,
+                     bool canDiscardOutsideDstRect = false);
 
     // Creates a GrGpuRTCommandBuffer which GrOpLists send draw commands to instead of directly
     // to the Gpu object.
@@ -599,7 +601,8 @@ private:
     // overridden by backend specific derived class to perform the copy surface
     virtual bool onCopySurface(GrSurface* dst, GrSurfaceOrigin dstOrigin,
                                GrSurface* src, GrSurfaceOrigin srcOrigin,
-                               const SkIRect& srcRect, const SkIPoint& dstPoint) = 0;
+                               const SkIRect& srcRect, const SkIPoint& dstPoint,
+                               bool canDiscardOutsideDstRect) = 0;
 
     virtual void onFinishFlush(bool insertedSemaphores) = 0;
 
diff --git a/src/gpu/gl/GrGLGpu.cpp b/src/gpu/gl/GrGLGpu.cpp
index 4e88f7dde2..4e1e64b21e 100644
--- a/src/gpu/gl/GrGLGpu.cpp
+++ b/src/gpu/gl/GrGLGpu.cpp
@@ -3449,7 +3449,8 @@ void GrGLGpu::unbindTextureFBOForPixelOps(GrGLenum fboTarget, GrSurface* surface
 
 bool GrGLGpu::onCopySurface(GrSurface* dst, GrSurfaceOrigin dstOrigin,
                             GrSurface* src, GrSurfaceOrigin srcOrigin,
-                            const SkIRect& srcRect, const SkIPoint& dstPoint) {
+                            const SkIRect& srcRect, const SkIPoint& dstPoint,
+                            bool canDiscardOutsideDstRect) {
     // None of our copy methods can handle a swizzle. TODO: Make copySurfaceAsDraw handle the
     // swizzle.
     if (this->caps()->shaderCaps()->configOutputSwizzle(src->config()) !=
diff --git a/src/gpu/gl/GrGLGpu.h b/src/gpu/gl/GrGLGpu.h
index 7ddd4e752d..c09cac64fc 100644
--- a/src/gpu/gl/GrGLGpu.h
+++ b/src/gpu/gl/GrGLGpu.h
@@ -256,7 +256,8 @@ private:
 
     bool onCopySurface(GrSurface* dst, GrSurfaceOrigin dstOrigin,
                        GrSurface* src, GrSurfaceOrigin srcOrigin,
-                       const SkIRect& srcRect, const SkIPoint& dstPoint) override;
+                       const SkIRect& srcRect, const SkIPoint& dstPoint,
+                       bool canDiscardOutsideDstRect) override;
 
     // binds texture unit in GL
     void setTextureUnit(int unitIdx);
diff --git a/src/gpu/mock/GrMockGpu.h b/src/gpu/mock/GrMockGpu.h
index 8a3dc3045b..5065350746 100644
--- a/src/gpu/mock/GrMockGpu.h
+++ b/src/gpu/mock/GrMockGpu.h
@@ -100,7 +100,7 @@ private:
 
     bool onCopySurface(GrSurface* dst, GrSurfaceOrigin dstOrigin, GrSurface* src,
                        GrSurfaceOrigin srcOrigin, const SkIRect& srcRect,
-                       const SkIPoint& dstPoint) override {
+                       const SkIPoint& dstPoint, bool canDiscardOutsideDstRect) override {
         return true;
     }
 
diff --git a/src/gpu/mtl/GrMtlGpu.h b/src/gpu/mtl/GrMtlGpu.h
index 4729e6e6be..fedce6ee6c 100644
--- a/src/gpu/mtl/GrMtlGpu.h
+++ b/src/gpu/mtl/GrMtlGpu.h
@@ -46,7 +46,8 @@ public:
     bool onCopySurface(GrSurface* dst, GrSurfaceOrigin dstOrigin,
                        GrSurface* src, GrSurfaceOrigin srcOrigin,
                        const SkIRect& srcRect,
-                       const SkIPoint& dstPoint) override { return false; }
+                       const SkIPoint& dstPoint,
+                       bool canDiscardOutsideDstRect) override { return false; }
 
     GrGpuRTCommandBuffer* createCommandBuffer(
                                     GrRenderTarget*, GrSurfaceOrigin,
diff --git a/src/gpu/vk/GrVkCaps.cpp b/src/gpu/vk/GrVkCaps.cpp
index 7e22135097..0d75cbc56e 100644
--- a/src/gpu/vk/GrVkCaps.cpp
+++ b/src/gpu/vk/GrVkCaps.cpp
@@ -19,7 +19,6 @@ GrVkCaps::GrVkCaps(const GrContextOptions& contextOptions, const GrVkInterface*
     : INHERITED(contextOptions) {
     fCanUseGLSLForShaderModule = false;
     fMustDoCopiesFromOrigin = false;
-    fSupportsCopiesAsDraws = true;
     fMustSubmitCommandsBeforeCopyOp = false;
     fMustSleepOnTearDown  = false;
     fNewCBOnPipelineChange = false;
@@ -65,7 +64,7 @@ bool GrVkCaps::initDescForDstCopy(const GrRenderTargetProxy* src, GrSurfaceDesc*
     // render target as well.
     *origin = src->origin();
     desc->fConfig = src->config();
-    if (src->numColorSamples() > 1 || (src->asTextureProxy() && this->supportsCopiesAsDraws())) {
+    if (src->numColorSamples() > 1 || src->asTextureProxy()) {
         desc->fFlags = kRenderTarget_GrSurfaceFlag;
     } else {
         // Just going to use CopyImage here
@@ -116,13 +115,6 @@ void GrVkCaps::applyDriverCorrectnessWorkarounds(const VkPhysicalDevicePropertie
         fMustSubmitCommandsBeforeCopyOp = true;
     }
 
-    if (kQualcomm_VkVendor == properties.vendorID ||
-        kARM_VkVendor == properties.vendorID) {
-        fSupportsCopiesAsDraws = false;
-        // We require copies as draws to support cross context textures.
-        fCrossContextTextureSupport = false;
-    }
-
 #if defined(SK_BUILD_FOR_WIN)
     if (kNvidia_VkVendor == properties.vendorID) {
         fMustSleepOnTearDown = true;
diff --git a/src/gpu/vk/GrVkCaps.h b/src/gpu/vk/GrVkCaps.h
index d8d1d2e997..63bd4e90d4 100644
--- a/src/gpu/vk/GrVkCaps.h
+++ b/src/gpu/vk/GrVkCaps.h
@@ -74,11 +74,6 @@ public:
         return fMustDoCopiesFromOrigin;
     }
 
-    // Check whether we support using draws for copies.
-    bool supportsCopiesAsDraws() const {
-        return fSupportsCopiesAsDraws;
-    }
-
     // On Nvidia there is a current bug where we must the current command buffer before copy
     // operations or else the copy will not happen. This includes copies, blits, resolves, and copy
     // as draws.
@@ -176,8 +171,6 @@ private:
 
     bool fMustDoCopiesFromOrigin;
 
-    bool fSupportsCopiesAsDraws;
-
     bool fMustSubmitCommandsBeforeCopyOp;
 
     bool fMustSleepOnTearDown;
diff --git a/src/gpu/vk/GrVkCopyManager.cpp b/src/gpu/vk/GrVkCopyManager.cpp
index f22d99b9eb..8c54fc5f34 100644
--- a/src/gpu/vk/GrVkCopyManager.cpp
+++ b/src/gpu/vk/GrVkCopyManager.cpp
@@ -142,7 +142,8 @@ bool GrVkCopyManager::createCopyProgram(GrVkGpu* gpu) {
 bool GrVkCopyManager::copySurfaceAsDraw(GrVkGpu* gpu,
                                         GrSurface* dst, GrSurfaceOrigin dstOrigin,
                                         GrSurface* src, GrSurfaceOrigin srcOrigin,
-                                        const SkIRect& srcRect, const SkIPoint& dstPoint) {
+                                        const SkIRect& srcRect, const SkIPoint& dstPoint,
+                                        bool canDiscardOutsideDstRect) {
     // None of our copy methods can handle a swizzle. TODO: Make copySurfaceAsDraw handle the
     // swizzle.
     if (gpu->caps()->shaderCaps()->configOutputSwizzle(src->config()) !=
@@ -150,10 +151,6 @@ bool GrVkCopyManager::copySurfaceAsDraw(GrVkGpu* gpu,
         return false;
     }
 
-    if (!gpu->vkCaps().supportsCopiesAsDraws()) {
-        return false;
-    }
-
     if (gpu->vkCaps().newCBOnPipelineChange()) {
         // We bind a new pipeline here for the copy so we must start a new command buffer.
         gpu->finishFlush(0, nullptr);
@@ -312,13 +309,13 @@ bool GrVkCopyManager::copySurfaceAsDraw(GrVkGpu* gpu,
                            VK_PIPELINE_STAGE_ALL_GRAPHICS_BIT,
                            false);
 
-    GrVkRenderPass::LoadStoreOps vkColorOps(VK_ATTACHMENT_LOAD_OP_DONT_CARE,
-                                            VK_ATTACHMENT_STORE_OP_STORE);
+    VkAttachmentLoadOp loadOp = canDiscardOutsideDstRect ? VK_ATTACHMENT_LOAD_OP_DONT_CARE
+                                                         : VK_ATTACHMENT_LOAD_OP_LOAD;
+    GrVkRenderPass::LoadStoreOps vkColorOps(loadOp, VK_ATTACHMENT_STORE_OP_STORE);
     GrVkRenderPass::LoadStoreOps vkStencilOps(VK_ATTACHMENT_LOAD_OP_LOAD,
                                               VK_ATTACHMENT_STORE_OP_STORE);
     const GrVkRenderPass* renderPass;
-    const GrVkResourceProvider::CompatibleRPHandle& rpHandle =
-        rt->compatibleRenderPassHandle();
+    const GrVkResourceProvider::CompatibleRPHandle& rpHandle = rt->compatibleRenderPassHandle();
     if (rpHandle.isValid()) {
         renderPass = gpu->resourceProvider().findRenderPass(rpHandle,
                                                             vkColorOps,
diff --git a/src/gpu/vk/GrVkCopyManager.h b/src/gpu/vk/GrVkCopyManager.h
index 726faf30d3..f36cc302b4 100644
--- a/src/gpu/vk/GrVkCopyManager.h
+++ b/src/gpu/vk/GrVkCopyManager.h
@@ -30,7 +30,8 @@ public:
     bool copySurfaceAsDraw(GrVkGpu* gpu,
                            GrSurface* dst, GrSurfaceOrigin dstOrigin,
                            GrSurface* src, GrSurfaceOrigin srcOrigin,
-                           const SkIRect& srcRect, const SkIPoint& dstPoint);
+                           const SkIRect& srcRect, const SkIPoint& dstPoint,
+                           bool canDiscardOutsideDstRect);
 
     void destroyResources(GrVkGpu* gpu);
     void abandonResources();
diff --git a/src/gpu/vk/GrVkGpu.cpp b/src/gpu/vk/GrVkGpu.cpp
index 4bd3179de4..b59d85e0bb 100644
--- a/src/gpu/vk/GrVkGpu.cpp
+++ b/src/gpu/vk/GrVkGpu.cpp
@@ -1854,8 +1854,8 @@ void GrVkGpu::copySurfaceAsResolve(GrSurface* dst, GrSurfaceOrigin dstOrigin, Gr
 
 bool GrVkGpu::onCopySurface(GrSurface* dst, GrSurfaceOrigin dstOrigin,
                             GrSurface* src, GrSurfaceOrigin srcOrigin,
-                            const SkIRect& srcRect,
-                            const SkIPoint& dstPoint) {
+                            const SkIRect& srcRect, const SkIPoint& dstPoint,
+                            bool canDiscardOutsideDstRect) {
     if (can_copy_as_resolve(dst, dstOrigin, src, srcOrigin, this)) {
         this->copySurfaceAsResolve(dst, dstOrigin, src, srcOrigin, srcRect, dstPoint);
         return true;
@@ -1865,7 +1865,8 @@ bool GrVkGpu::onCopySurface(GrSurface* dst, GrSurfaceOrigin dstOrigin,
         this->submitCommandBuffer(GrVkGpu::kSkip_SyncQueue);
     }
 
-    if (fCopyManager.copySurfaceAsDraw(this, dst, dstOrigin, src, srcOrigin, srcRect, dstPoint)) {
+    if (fCopyManager.copySurfaceAsDraw(this, dst, dstOrigin, src, srcOrigin, srcRect, dstPoint,
+                                       canDiscardOutsideDstRect)) {
         auto dstRect = srcRect.makeOffset(dstPoint.fX, dstPoint.fY);
         this->didWriteToSurface(dst, dstOrigin, &dstRect);
         return true;
diff --git a/src/gpu/vk/GrVkGpu.h b/src/gpu/vk/GrVkGpu.h
index 6eac5c8086..20df339627 100644
--- a/src/gpu/vk/GrVkGpu.h
+++ b/src/gpu/vk/GrVkGpu.h
@@ -204,7 +204,7 @@ private:
 
     bool onCopySurface(GrSurface* dst, GrSurfaceOrigin dstOrigin, GrSurface* src,
                        GrSurfaceOrigin srcOrigin, const SkIRect& srcRect,
-                       const SkIPoint& dstPoint) override;
+                       const SkIPoint& dstPoint, bool canDiscardOutsideDstRect) override;
 
     void onFinishFlush(bool insertedSemaphores) override;
 
diff --git a/src/gpu/vk/GrVkGpuCommandBuffer.cpp b/src/gpu/vk/GrVkGpuCommandBuffer.cpp
index 4fb6e2e436..c05cb61d38 100644
--- a/src/gpu/vk/GrVkGpuCommandBuffer.cpp
+++ b/src/gpu/vk/GrVkGpuCommandBuffer.cpp
@@ -174,7 +174,7 @@ void GrVkGpuRTCommandBuffer::submit() {
         for (int j = 0; j < cbInfo.fPreCopies.count(); ++j) {
             CopyInfo& copyInfo = cbInfo.fPreCopies[j];
             fGpu->copySurface(fRenderTarget, fOrigin, copyInfo.fSrc, copyInfo.fSrcOrigin,
-                              copyInfo.fSrcRect, copyInfo.fDstPoint);
+                              copyInfo.fSrcRect, copyInfo.fDstPoint, copyInfo.fShouldDiscardDst);
         }
 
         // Make sure we do the following layout changes after all copies, uploads, or any other pre
@@ -476,6 +476,10 @@ void GrVkGpuRTCommandBuffer::copy(GrSurface* src, GrSurfaceOrigin srcOrigin, con
         this->addAdditionalRenderPass();
     }
 
+    fCommandBufferInfos[fCurrentCmdInfo].fPreCopies.emplace_back(
+            src, srcOrigin, srcRect, dstPoint,
+            LoadStoreState::kStartsWithDiscard == cbInfo.fLoadStoreState);
+
     if (LoadStoreState::kLoadAndStore != cbInfo.fLoadStoreState) {
         // Change the render pass to do a load and store so we don't lose the results of our copy
         GrVkRenderPass::LoadStoreOps vkColorOps(VK_ATTACHMENT_LOAD_OP_LOAD,
@@ -503,7 +507,6 @@ void GrVkGpuRTCommandBuffer::copy(GrSurface* src, GrSurfaceOrigin srcOrigin, con
         cbInfo.fLoadStoreState = LoadStoreState::kLoadAndStore;
 
     }
-    fCommandBufferInfos[fCurrentCmdInfo].fPreCopies.emplace_back(src, srcOrigin, srcRect, dstPoint);
 }
 
 ////////////////////////////////////////////////////////////////////////////////
diff --git a/src/gpu/vk/GrVkGpuCommandBuffer.h b/src/gpu/vk/GrVkGpuCommandBuffer.h
index 2e9ac12bd4..f2d48c7ab0 100644
--- a/src/gpu/vk/GrVkGpuCommandBuffer.h
+++ b/src/gpu/vk/GrVkGpuCommandBuffer.h
@@ -143,13 +143,18 @@ private:
 
     struct CopyInfo {
         CopyInfo(GrSurface* src, GrSurfaceOrigin srcOrigin, const SkIRect& srcRect,
-                 const SkIPoint& dstPoint)
-            : fSrc(src), fSrcOrigin(srcOrigin), fSrcRect(srcRect), fDstPoint(dstPoint) {}
+                 const SkIPoint& dstPoint, bool shouldDiscardDst)
+            : fSrc(src)
+            , fSrcOrigin(srcOrigin)
+            , fSrcRect(srcRect)
+            , fDstPoint(dstPoint)
+            , fShouldDiscardDst(shouldDiscardDst) {}
 
         GrSurface*      fSrc;
         GrSurfaceOrigin fSrcOrigin;
         SkIRect         fSrcRect;
         SkIPoint        fDstPoint;
+        bool            fShouldDiscardDst;
     };
 
     enum class LoadStoreState {
diff --git a/tests/VkMakeCopyPipelineTest.cpp b/tests/VkMakeCopyPipelineTest.cpp
index cb3cbe9e30..c8fe6f2a70 100644
--- a/tests/VkMakeCopyPipelineTest.cpp
+++ b/tests/VkMakeCopyPipelineTest.cpp
@@ -178,10 +178,6 @@ DEF_GPUTEST_FOR_VULKAN_CONTEXT(VkMakeCopyPipelineTest, reporter, ctxInfo) {
     GrContext* context = ctxInfo.grContext();
     GrVkGpu* gpu = static_cast<GrVkGpu*>(context->contextPriv().getGpu());
 
-    if (!gpu->vkCaps().supportsCopiesAsDraws()) {
-        return;
-    }
-
     TestVkCopyProgram copyProgram;
     copyProgram.test(gpu, reporter);
 }