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>
diff --git a/src/gpu/GrBlurUtils.cpp b/src/gpu/GrBlurUtils.cpp
index e84f6b8..c8fdc0b 100644
--- a/src/gpu/GrBlurUtils.cpp
+++ b/src/gpu/GrBlurUtils.cpp
@@ -181,7 +181,7 @@
return nullptr;
}
- rtContext->priv().absClear(nullptr, SK_PMColor4fTRANSPARENT);
+ rtContext->priv().absClear(nullptr);
GrPaint maskPaint;
maskPaint.setCoverageSetOpXPFactory(SkRegion::kReplace_Op);
diff --git a/src/gpu/GrRenderTargetContext.cpp b/src/gpu/GrRenderTargetContext.cpp
index 862b79c..26813d6 100644
--- a/src/gpu/GrRenderTargetContext.cpp
+++ b/src/gpu/GrRenderTargetContext.cpp
@@ -342,7 +342,7 @@
}
}
-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 @@
}
}
+ 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 @@
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,28 +387,32 @@
// 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
- fRenderTargetContext->getOpsTask()->resetForFullscreenClear(
- fRenderTargetContext->canDiscardPreviousOpsOnFullClear());
- 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);
- fRenderTargetContext->addDrawOp(
- GrFixedClip::Disabled(),
- GrFillRectOp::MakeNonAARect(fRenderTargetContext->fContext, std::move(paint),
- SkMatrix::I(), SkRect::Make(rtRect)));
+ if (fRenderTargetContext->getOpsTask()->resetForFullscreenClear(
+ fRenderTargetContext->canDiscardPreviousOpsOnFullClear()) &&
+ !fRenderTargetContext->caps()->performColorClearsAsDraws()) {
+ fRenderTargetContext->getOpsTask()->setColorLoadOp(GrLoadOp::kClear, kColor);
} else {
- // Nothing special about this path in absClear compared to internalClear()
- fRenderTargetContext->addOp(GrClearOp::Make(
- fRenderTargetContext->fContext, SkIRect::MakeEmpty(), color,
- /* fullscreen */ true));
+ 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(kColor, &paint);
+ fRenderTargetContext->addDrawOp(
+ GrFixedClip::Disabled(),
+ GrFillRectOp::MakeNonAARect(fRenderTargetContext->fContext,
+ 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(), kColor,
+ /* fullscreen */ true));
+ }
}
}
}
diff --git a/src/gpu/GrRenderTargetContextPriv.h b/src/gpu/GrRenderTargetContextPriv.h
index 1440ec2..b29011d 100644
--- a/src/gpu/GrRenderTargetContextPriv.h
+++ b/src/gpu/GrRenderTargetContextPriv.h
@@ -57,10 +57,11 @@
* 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
diff --git a/src/gpu/mtl/GrMtlGpu.mm b/src/gpu/mtl/GrMtlGpu.mm
index aba9e67..9d9d121 100644
--- a/src/gpu/mtl/GrMtlGpu.mm
+++ b/src/gpu/mtl/GrMtlGpu.mm
@@ -144,7 +144,7 @@
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) {
diff --git a/src/gpu/mtl/GrMtlOpsRenderPass.h b/src/gpu/mtl/GrMtlOpsRenderPass.h
index 5736db3..5e6f221 100644
--- a/src/gpu/mtl/GrMtlOpsRenderPass.h
+++ b/src/gpu/mtl/GrMtlOpsRenderPass.h
@@ -23,7 +23,6 @@
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 @@
void precreateCmdEncoder();
GrMtlGpu* fGpu;
- // GrRenderTargetProxy bounds
-#ifdef SK_DEBUG
- SkRect fRTBounds;
-#endif
id<MTLRenderCommandEncoder> fActiveRenderCmdEncoder;
MTLRenderPassDescriptor* fRenderPassDesc;
diff --git a/src/gpu/mtl/GrMtlOpsRenderPass.mm b/src/gpu/mtl/GrMtlOpsRenderPass.mm
index f9754fe..ddcfe96 100644
--- a/src/gpu/mtl/GrMtlOpsRenderPass.mm
+++ b/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::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 -
- // but in that case, scissor should be enabled, so this check should still succeed
- SkASSERT(!clip.scissorEnabled() || clip.scissorRect().contains(fRTBounds));
- fRenderPassDesc.colorAttachments[0].clearColor = MTLClearColorMake(color.fR, color.fG, color.fB,
- color.fA);
- fRenderPassDesc.colorAttachments[0].loadAction = MTLLoadActionClear;
- this->precreateCmdEncoder();
- fRenderPassDesc.colorAttachments[0].loadAction = MTLLoadActionLoad;
+ // We should never end up here since all clears should either be done as draws or load ops in
+ // metal. If we hit this assert then we missed a chance to set a load op on the
+ // GrRenderTargetContext level.
+ SkASSERT(false);
}
void GrMtlOpsRenderPass::onClearStencilClip(const GrFixedClip& clip, bool insideStencilMask) {
diff --git a/src/gpu/vk/GrVkOpsRenderPass.cpp b/src/gpu/vk/GrVkOpsRenderPass.cpp
index 6862436..ba72061 100644
--- a/src/gpu/vk/GrVkOpsRenderPass.cpp
+++ b/src/gpu/vk/GrVkOpsRenderPass.cpp
@@ -376,8 +376,6 @@
}
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 @@
VkClearColorValue vkColor = {{color.fR, color.fG, color.fB, color.fA}};
- if (cbInfo.fIsEmpty && !clip.scissorEnabled()) {
- // Change the render pass to do a clear load
- GrVkRenderPass::LoadStoreOps vkColorOps(VK_ATTACHMENT_LOAD_OP_CLEAR,
- VK_ATTACHMENT_STORE_OP_STORE);
- // Preserve the stencil buffer's load & store settings
- 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;
- }
+ // If we end up in a situation where we are calling clear without a scissior then in general it
+ // means we missed an opportunity higher up the stack to set the load op to be a clear. However,
+ // there are situations where higher up we couldn't discard the previous ops and set a clear
+ // load op (e.g. if we needed to execute a wait op). Thus we also have the empty check here.
+ SkASSERT(!cbInfo.fIsEmpty || clip.scissorEnabled());
// We always do a sub rect clear with clearAttachments since we are inside a render pass
VkClearRect clearRect;