Make sure we use clear load op on GrOpsTask as often as possible.

With this change we are also removing the code in the Gr*OpsRenderPass which
will dynamically change the load op for given clear calls. The load is is
set at creation time.

Change-Id: I19f0f37bb38f790b11052953106e0492e6f9fc87
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/237425
Commit-Queue: Greg Daniel <egdaniel@google.com>
Reviewed-by: Michael Ludwig <michaelludwig@google.com>

src/core/SkGpuBlurUtils.cpp

@@ -386,7 +386,7 @@ static sk_sp<GrTextureProxy> decimate(GrRecordingContext* context,
             // X convolution from reading garbage.
             SkIRect clearRect = SkIRect::MakeXYWH(contentRect->fRight, contentRect->fTop,
                                                   radiusX, contentRect->height());
-            dstRenderTargetContext->priv().absClear(&clearRect, SK_PMColor4fTRANSPARENT);
+            dstRenderTargetContext->priv().absClear(&clearRect);
         }
     } else {
         if (scaleFactorY > 1) {
@@ -394,7 +394,7 @@ static sk_sp<GrTextureProxy> decimate(GrRecordingContext* context,
             // convolution from reading garbage.
             SkIRect clearRect = SkIRect::MakeXYWH(contentRect->fLeft, contentRect->fBottom,
                                                   contentRect->width(), radiusY);
-            dstRenderTargetContext->priv().absClear(&clearRect, SK_PMColor4fTRANSPARENT);
+            dstRenderTargetContext->priv().absClear(&clearRect);
         }
     }
 
@@ -421,9 +421,9 @@ static std::unique_ptr<GrRenderTargetContext> reexpand(
     // TODO: it seems like we should actually be clamping here rather than darkening
     // the bottom right edges.
     SkIRect clearRect = SkIRect::MakeXYWH(srcRect.fLeft, srcRect.fBottom, srcRect.width() + 1, 1);
-    srcRenderTargetContext->priv().absClear(&clearRect, SK_PMColor4fTRANSPARENT);
+    srcRenderTargetContext->priv().absClear(&clearRect);
     clearRect = SkIRect::MakeXYWH(srcRect.fRight, srcRect.fTop, 1, srcRect.height());
-    srcRenderTargetContext->priv().absClear(&clearRect, SK_PMColor4fTRANSPARENT);
+    srcRenderTargetContext->priv().absClear(&clearRect);
 
     sk_sp<GrTextureProxy> srcProxy = srcRenderTargetContext->asTextureProxyRef();
     if (!srcProxy) {
@@ -554,7 +554,7 @@ std::unique_ptr<GrRenderTargetContext> GaussianBlur(GrRecordingContext* context,
             // convolution from reading garbage.
             SkIRect clearRect = SkIRect::MakeXYWH(srcRect.fLeft, srcRect.fBottom,
                                                   srcRect.width(), radiusY);
-            dstRenderTargetContext->priv().absClear(&clearRect, SK_PMColor4fTRANSPARENT);
+            dstRenderTargetContext->priv().absClear(&clearRect);
         }
 
         srcProxy = dstRenderTargetContext->asTextureProxyRef();

src/gpu/GrBlurUtils.cpp

@@ -181,7 +181,7 @@ static sk_sp<GrTextureProxy> create_mask_GPU(GrRecordingContext* context,
         return nullptr;
     }
 
-    rtContext->priv().absClear(nullptr, SK_PMColor4fTRANSPARENT);
+    rtContext->priv().absClear(nullptr);
 
     GrPaint maskPaint;
     maskPaint.setCoverageSetOpXPFactory(SkRegion::kReplace_Op);

src/gpu/GrRenderTargetContext.cpp

@@ -342,7 +342,7 @@ void GrRenderTargetContext::internalClear(const GrFixedClip& clip,
     }
 }
 
-void GrRenderTargetContextPriv::absClear(const SkIRect* clearRect, const SkPMColor4f& color) {
+void GrRenderTargetContextPriv::absClear(const SkIRect* clearRect) {
     ASSERT_SINGLE_OWNER_PRIV
     RETURN_IF_ABANDONED_PRIV
     SkDEBUGCODE(fRenderTargetContext->validate();)
@@ -364,6 +364,8 @@ void GrRenderTargetContextPriv::absClear(const SkIRect* clearRect, const SkPMCol
         }
     }
 
+    static const SkPMColor4f kColor = SK_PMColor4fTRANSPARENT;
+
     // TODO: in a post-MDB world this should be handled at the OpsTask level.
     // This makes sure to always add an op to the list, instead of marking the clear as a load op.
     // This code follows very similar logic to internalClear() below, but critical differences are
@@ -371,7 +373,7 @@ void GrRenderTargetContextPriv::absClear(const SkIRect* clearRect, const SkPMCol
     if (clearRect) {
         if (fRenderTargetContext->caps()->performPartialClearsAsDraws()) {
             GrPaint paint;
-            clear_to_grpaint(color, &paint);
+            clear_to_grpaint(kColor, &paint);
 
             // Use the disabled clip; the rect geometry already matches the clear rectangle and
             // if it were added to a scissor, that would be intersected with the logical surface
@@ -385,31 +387,35 @@ void GrRenderTargetContextPriv::absClear(const SkIRect* clearRect, const SkPMCol
             // proxy. The surface proxy variant would intersect the clip rect with its logical
             // bounds, which is not desired in this special case.
             fRenderTargetContext->addOp(GrClearOp::Make(
-                    fRenderTargetContext->fContext, rtRect, color, /* fullscreen */ false));
+                    fRenderTargetContext->fContext, rtRect, kColor, /* fullscreen */ false));
         }
     } else {
-        // Reset the oplist like in internalClear(), but do not rely on a load op for the clear
+        if (fRenderTargetContext->getOpsTask()->resetForFullscreenClear(
-        fRenderTargetContext->getOpsTask()->resetForFullscreenClear(
+                fRenderTargetContext->canDiscardPreviousOpsOnFullClear()) &&
-                fRenderTargetContext->canDiscardPreviousOpsOnFullClear());
+            !fRenderTargetContext->caps()->performColorClearsAsDraws()) {
+            fRenderTargetContext->getOpsTask()->setColorLoadOp(GrLoadOp::kClear, kColor);
+        } else {
             fRenderTargetContext->getOpsTask()->setColorLoadOp(GrLoadOp::kDiscard);
 
             if (fRenderTargetContext->caps()->performColorClearsAsDraws()) {
                 // This draws a quad covering the worst case dimensions instead of just the logical
                 // width and height like in internalClear().
                 GrPaint paint;
-            clear_to_grpaint(color, &paint);
+                clear_to_grpaint(kColor, &paint);
                 fRenderTargetContext->addDrawOp(
                         GrFixedClip::Disabled(),
-                    GrFillRectOp::MakeNonAARect(fRenderTargetContext->fContext, std::move(paint),
+                        GrFillRectOp::MakeNonAARect(fRenderTargetContext->fContext,
-                                                SkMatrix::I(), SkRect::Make(rtRect)));
+                                                    std::move(paint), SkMatrix::I(),
+                                                    SkRect::Make(rtRect)));
             } else {
                 // Nothing special about this path in absClear compared to internalClear()
                 fRenderTargetContext->addOp(GrClearOp::Make(
-                    fRenderTargetContext->fContext, SkIRect::MakeEmpty(), color,
+                        fRenderTargetContext->fContext, SkIRect::MakeEmpty(), kColor,
                         /* fullscreen */ true));
             }
         }
     }
+}
 
 void GrRenderTargetContext::drawPaint(const GrClip& clip,
                                       GrPaint&& paint,

src/gpu/GrRenderTargetContextPriv.h

@@ -57,10 +57,11 @@ public:
      * upsampling. The "absClear" entry point ignores the content bounds but does use the
      * worst case (instantiated) bounds.
      *
+     * This call will always clear to transparent black.
+     *
      * @param rect      if (!null) the rect to clear, otherwise it is a full screen clear
-     * @param color     the color to clear to
      */
-    void absClear(const SkIRect* rect, const SkPMColor4f& color);
+    void absClear(const SkIRect* rect);
 
     // While this can take a general clip, since GrReducedClip relies on this function, it must take
     // care to only provide hard clips or we could get stuck in a loop. The general clip is needed

src/gpu/mtl/GrMtlGpu.mm

@@ -144,7 +144,7 @@ GrOpsRenderPass* GrMtlGpu::getOpsRenderPass(
             GrRenderTarget* renderTarget, GrSurfaceOrigin origin, const SkRect& bounds,
             const GrOpsRenderPass::LoadAndStoreInfo& colorInfo,
             const GrOpsRenderPass::StencilLoadAndStoreInfo& stencilInfo) {
-    return new GrMtlOpsRenderPass(this, renderTarget, origin, bounds, colorInfo, stencilInfo);
+    return new GrMtlOpsRenderPass(this, renderTarget, origin, colorInfo, stencilInfo);
 }
 
 void GrMtlGpu::submit(GrOpsRenderPass* renderPass) {

src/gpu/mtl/GrMtlOpsRenderPass.h

@@ -23,7 +23,6 @@ class GrMtlRenderTarget;
 class GrMtlOpsRenderPass : public GrOpsRenderPass, private GrMesh::SendToGpuImpl {
 public:
     GrMtlOpsRenderPass(GrMtlGpu* gpu, GrRenderTarget* rt, GrSurfaceOrigin origin,
-                       const SkRect& bounds,
                        const GrOpsRenderPass::LoadAndStoreInfo& colorInfo,
                        const GrOpsRenderPass::StencilLoadAndStoreInfo& stencilInfo);
 
@@ -94,10 +93,6 @@ private:
     void precreateCmdEncoder();
 
     GrMtlGpu*                   fGpu;
-    // GrRenderTargetProxy bounds
-#ifdef SK_DEBUG
-    SkRect                      fRTBounds;
-#endif
 
     id<MTLRenderCommandEncoder> fActiveRenderCmdEncoder;
     MTLRenderPassDescriptor*    fRenderPassDesc;

src/gpu/mtl/GrMtlOpsRenderPass.mm

@@ -22,14 +22,11 @@
 #endif
 
 GrMtlOpsRenderPass::GrMtlOpsRenderPass(
-        GrMtlGpu* gpu, GrRenderTarget* rt, GrSurfaceOrigin origin, const SkRect& bounds,
+        GrMtlGpu* gpu, GrRenderTarget* rt, GrSurfaceOrigin origin,
         const GrOpsRenderPass::LoadAndStoreInfo& colorInfo,
         const GrOpsRenderPass::StencilLoadAndStoreInfo& stencilInfo)
         : INHERITED(rt, origin)
         , fGpu(gpu)
-#ifdef SK_DEBUG
-        , fRTBounds(bounds)
-#endif
         {
     this->setupRenderPass(colorInfo, stencilInfo);
 }
@@ -196,14 +193,10 @@ void GrMtlOpsRenderPass::onDraw(const GrPrimitiveProcessor& primProc,
 }
 
 void GrMtlOpsRenderPass::onClear(const GrFixedClip& clip, const SkPMColor4f& color) {
-    // if we end up here from absClear, the clear bounds may be bigger than the RT proxy bounds -
+    // We should never end up here since all clears should either be done as draws or load ops in
-    // but in that case, scissor should be enabled, so this check should still succeed
+    // metal. If we hit this assert then we missed a chance to set a load op on the
-    SkASSERT(!clip.scissorEnabled() || clip.scissorRect().contains(fRTBounds));
+    // GrRenderTargetContext level.
-    fRenderPassDesc.colorAttachments[0].clearColor = MTLClearColorMake(color.fR, color.fG, color.fB,
+    SkASSERT(false);
-                                                                       color.fA);
-    fRenderPassDesc.colorAttachments[0].loadAction = MTLLoadActionClear;
-    this->precreateCmdEncoder();
-    fRenderPassDesc.colorAttachments[0].loadAction = MTLLoadActionLoad;
 }
 
 void GrMtlOpsRenderPass::onClearStencilClip(const GrFixedClip& clip, bool insideStencilMask) {

src/gpu/vk/GrVkOpsRenderPass.cpp

@@ -376,8 +376,6 @@ void GrVkOpsRenderPass::onClearStencilClip(const GrFixedClip& clip, bool insideS
 }
 
 void GrVkOpsRenderPass::onClear(const GrFixedClip& clip, const SkPMColor4f& color) {
-    GrVkRenderTarget* vkRT = static_cast<GrVkRenderTarget*>(fRenderTarget);
-
     // parent class should never let us get here with no RT
     SkASSERT(!clip.hasWindowRectangles());
 
@@ -385,36 +383,11 @@ void GrVkOpsRenderPass::onClear(const GrFixedClip& clip, const SkPMColor4f& colo
 
     VkClearColorValue vkColor = {{color.fR, color.fG, color.fB, color.fA}};
 
-    if (cbInfo.fIsEmpty && !clip.scissorEnabled()) {
+    // If we end up in a situation where we are calling clear without a scissior then in general it
-        // Change the render pass to do a clear load
+    // means we missed an opportunity higher up the stack to set the load op to be a clear. However,
-        GrVkRenderPass::LoadStoreOps vkColorOps(VK_ATTACHMENT_LOAD_OP_CLEAR,
+    // there are situations where higher up we couldn't discard the previous ops and set a clear
-                                                VK_ATTACHMENT_STORE_OP_STORE);
+    // load op (e.g. if we needed to execute a wait op). Thus we also have the empty check here.
-        // Preserve the stencil buffer's load & store settings
+    SkASSERT(!cbInfo.fIsEmpty || clip.scissorEnabled());
-        GrVkRenderPass::LoadStoreOps vkStencilOps(fVkStencilLoadOp, fVkStencilStoreOp);
-
-        const GrVkRenderPass* oldRP = cbInfo.fRenderPass;
-
-        const GrVkResourceProvider::CompatibleRPHandle& rpHandle =
-            vkRT->compatibleRenderPassHandle();
-        if (rpHandle.isValid()) {
-            cbInfo.fRenderPass = fGpu->resourceProvider().findRenderPass(rpHandle,
-                                                                         vkColorOps,
-                                                                         vkStencilOps);
-        } else {
-            cbInfo.fRenderPass = fGpu->resourceProvider().findRenderPass(*vkRT,
-                                                                         vkColorOps,
-                                                                         vkStencilOps);
-        }
-
-        SkASSERT(cbInfo.fRenderPass->isCompatible(*oldRP));
-        oldRP->unref(fGpu);
-
-        cbInfo.fColorClearValue.color = {{color.fR, color.fG, color.fB, color.fA}};
-        cbInfo.fLoadStoreState = LoadStoreState::kStartsWithClear;
-        // Update command buffer bounds
-        cbInfo.fBounds.join(fRenderTarget->getBoundsRect());
-        return;
-    }
 
     // We always do a sub rect clear with clearAttachments since we are inside a render pass
     VkClearRect clearRect;