Track if we need stencil on GrRenderTargetContext
There are a few places that have been checking whether the *proxy*
needs stencil, in order to determine if the current render target
context needs stencil. This is problematic since a render target
context can not require stencil itself, but wrap an existing proxy
that already has stencil.
Bug: skia:
Change-Id: I2719dd3a9df15fef3d64f991cda4fadea23266bb
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/223970
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
Commit-Queue: Chris Dalton <csmartdalton@google.com>
diff --git a/src/gpu/GrClipStackClip.cpp b/src/gpu/GrClipStackClip.cpp
index 2acb5a8..7399076 100644
--- a/src/gpu/GrClipStackClip.cpp
+++ b/src/gpu/GrClipStackClip.cpp
@@ -297,8 +297,6 @@
}
}
- renderTargetContext->setNeedsStencil();
-
// This relies on the property that a reduced sub-rect of the last clip will contain all the
// relevant window rectangles that were in the last clip. This subtle requirement will go away
// after clipping is overhauled.
diff --git a/src/gpu/GrRenderTargetContext.cpp b/src/gpu/GrRenderTargetContext.cpp
index 306ce5e..a6fb9a4 100644
--- a/src/gpu/GrRenderTargetContext.cpp
+++ b/src/gpu/GrRenderTargetContext.cpp
@@ -35,6 +35,7 @@
#include "src/gpu/GrPathRenderer.h"
#include "src/gpu/GrRecordingContextPriv.h"
#include "src/gpu/GrRenderTargetContextPriv.h"
+#include "src/gpu/GrRenderTargetProxyPriv.h"
#include "src/gpu/GrResourceProvider.h"
#include "src/gpu/GrStencilAttachment.h"
#include "src/gpu/GrStyle.h"
@@ -302,15 +303,16 @@
}
if (isFull) {
- if (this->getRTOpList()->resetForFullscreenClear() &&
+ GrRenderTargetOpList* opList = this->getRTOpList();
+ if (opList->resetForFullscreenClear(this->canDiscardPreviousOpsOnFullClear()) &&
!this->caps()->performColorClearsAsDraws()) {
// The op list was emptied and native clears are allowed, so just use the load op
- this->getRTOpList()->setColorLoadOp(GrLoadOp::kClear, color);
+ opList->setColorLoadOp(GrLoadOp::kClear, color);
return;
} else {
// Will use an op for the clear, reset the load op to discard since the op will
// blow away the color buffer contents
- this->getRTOpList()->setColorLoadOp(GrLoadOp::kDiscard);
+ opList->setColorLoadOp(GrLoadOp::kDiscard);
}
// Must add an op to the list (either because we couldn't use a load op, or because the
@@ -323,8 +325,8 @@
GrFillRectOp::MakeNonAARect(fContext, std::move(paint), SkMatrix::I(),
rtRect));
} else {
- this->getRTOpList()->addOp(GrClearOp::Make(fContext, SkIRect::MakeEmpty(), color,
- /* fullscreen */ true), *this->caps());
+ opList->addOp(GrClearOp::Make(fContext, SkIRect::MakeEmpty(), color,
+ /* fullscreen */ true), *this->caps());
}
} else {
if (this->caps()->performPartialClearsAsDraws()) {
@@ -397,7 +399,8 @@
}
} else {
// Reset the oplist like in internalClear(), but do not rely on a load op for the clear
- fRenderTargetContext->getRTOpList()->resetForFullscreenClear();
+ fRenderTargetContext->getRTOpList()->resetForFullscreenClear(
+ fRenderTargetContext->canDiscardPreviousOpsOnFullClear());
fRenderTargetContext->getRTOpList()->setColorLoadOp(GrLoadOp::kDiscard);
if (fRenderTargetContext->caps()->performColorClearsAsDraws()) {
@@ -804,6 +807,45 @@
*fRenderTargetContext->caps());
}
+GrRenderTargetOpList::CanDiscardPreviousOps GrRenderTargetContext::canDiscardPreviousOpsOnFullClear(
+ ) const {
+#if GR_TEST_UTILS
+ if (fPreserveOpsOnFullClear_TestingOnly) {
+ return GrRenderTargetOpList::CanDiscardPreviousOps::kNo;
+ }
+#endif
+ // Regardless of how the clear is implemented (native clear or a fullscreen quad), all prior ops
+ // would normally be overwritten. The one exception is if the render target context is marked as
+ // needing a stencil buffer then there may be a prior op that writes to the stencil buffer.
+ // Although the clear will ignore the stencil buffer, following draw ops may not so we can't get
+ // rid of all the preceding ops. Beware! If we ever add any ops that have a side effect beyond
+ // modifying the stencil buffer we will need a more elaborate tracking system (skbug.com/7002).
+ return GrRenderTargetOpList::CanDiscardPreviousOps(!fNeedsStencil);
+}
+
+void GrRenderTargetContext::setNeedsStencil() {
+ // Don't clear stencil until after we've changed fNeedsStencil. This ensures we don't loop
+ // forever in the event that there are driver bugs and we need to clear as a draw.
+ bool needsStencilClear = !fNeedsStencil;
+
+ fNeedsStencil = true;
+ fRenderTargetProxy->setNeedsStencil();
+
+ if (needsStencilClear) {
+ if (this->caps()->performStencilClearsAsDraws()) {
+ // There is a driver bug with clearing stencil. We must use an op to manually clear the
+ // stencil buffer before the op that required 'setNeedsStencil'.
+ this->internalStencilClear(GrFixedClip::Disabled(), /* inside mask */ false);
+ } else {
+ // Setting the clear stencil load op is preferable. On non-tilers, this lets the flush
+ // code note when the instantiated stencil buffer is already clear and skip the clear
+ // altogether. And on tilers, loading the stencil buffer cleared is even faster than
+ // preserving the previous contents.
+ this->getRTOpList()->setStencilLoadOp(GrLoadOp::kClear);
+ }
+ }
+}
+
void GrRenderTargetContextPriv::clearStencilClip(const GrFixedClip& clip, bool insideStencilMask) {
ASSERT_SINGLE_OWNER_PRIV
RETURN_IF_ABANDONED_PRIV
@@ -824,10 +866,6 @@
// Configure the paint to have no impact on the color buffer
GrPaint paint;
paint.setXPFactory(GrDisableColorXPFactory::Get());
-
- // Mark stencil usage here before addDrawOp() so that it doesn't try to re-call
- // internalStencilClear() just because the op has stencil settings.
- this->setNeedsStencil();
this->addDrawOp(clip, GrFillRectOp::MakeNonAARect(fContext, std::move(paint), SkMatrix::I(),
rtRect, ss));
} else {
@@ -865,7 +903,6 @@
return;
}
- fRenderTargetContext->setNeedsStencil();
std::unique_ptr<GrOp> op = GrStencilPathOp::Make(fRenderTargetContext->fContext,
viewMatrix,
@@ -878,6 +915,8 @@
return;
}
op->setClippedBounds(bounds);
+
+ fRenderTargetContext->setNeedsStencil();
fRenderTargetContext->getRTOpList()->addOp(std::move(op), *fRenderTargetContext->caps());
}
@@ -2556,31 +2595,19 @@
op_bounds(&bounds, op.get());
GrAppliedClip appliedClip;
GrDrawOp::FixedFunctionFlags fixedFunctionFlags = op->fixedFunctionFlags();
- if (!clip.apply(fContext, this, fixedFunctionFlags & GrDrawOp::FixedFunctionFlags::kUsesHWAA,
- fixedFunctionFlags & GrDrawOp::FixedFunctionFlags::kUsesStencil, &appliedClip,
- &bounds)) {
+ bool usesHWAA = fixedFunctionFlags & GrDrawOp::FixedFunctionFlags::kUsesHWAA;
+ bool usesStencil = fixedFunctionFlags & GrDrawOp::FixedFunctionFlags::kUsesStencil;
+
+ if (usesStencil) {
+ this->setNeedsStencil();
+ }
+
+ if (!clip.apply(fContext, this, usesHWAA, usesStencil, &appliedClip, &bounds)) {
fContext->priv().opMemoryPool()->release(std::move(op));
return;
}
- if (fixedFunctionFlags & GrDrawOp::FixedFunctionFlags::kUsesStencil ||
- appliedClip.hasStencilClip()) {
- if (this->caps()->performStencilClearsAsDraws()) {
- // Must use an op to perform the clear of the stencil buffer before this op, but only
- // have to clear the first time any draw needs it (this also ensures we don't loop
- // forever when the internal stencil clear adds a draw op that has stencil settings).
- if (!fRenderTargetProxy->needsStencil()) {
- // Send false so that the stencil buffer is fully cleared to 0
- this->internalStencilClear(GrFixedClip::Disabled(), /* inside mask */ false);
- }
- } else {
- // Just make sure the stencil buffer is cleared before the draw op, easy to do it as
- // a load at the start
- this->getRTOpList()->setStencilLoadOp(GrLoadOp::kClear);
- }
-
- this->setNeedsStencil();
- }
+ SkASSERT((!usesStencil && !appliedClip.hasStencilClip()) || fNeedsStencil);
GrClampType clampType = GrPixelConfigClampType(this->colorSpaceInfo().config());
// MIXED SAMPLES TODO: check stencil buffer is MSAA and make sure stencil test is actually doing
diff --git a/src/gpu/GrRenderTargetContext.h b/src/gpu/GrRenderTargetContext.h
index 5fa87e5..2746fc5 100644
--- a/src/gpu/GrRenderTargetContext.h
+++ b/src/gpu/GrRenderTargetContext.h
@@ -15,6 +15,7 @@
#include "include/core/SkSurfaceProps.h"
#include "include/private/GrTypesPriv.h"
#include "src/gpu/GrPaint.h"
+#include "src/gpu/GrRenderTargetOpList.h"
#include "src/gpu/GrRenderTargetProxy.h"
#include "src/gpu/GrSurfaceContext.h"
#include "src/gpu/GrXferProcessor.h"
@@ -31,7 +32,6 @@
class GrOp;
class GrRenderTarget;
class GrRenderTargetContextPriv;
-class GrRenderTargetOpList;
class GrShape;
class GrStyle;
class GrTextureProxy;
@@ -464,8 +464,6 @@
bool wrapsVkSecondaryCB() const { return fRenderTargetProxy->wrapsVkSecondaryCB(); }
GrMipMapped mipMapped() const;
- void setNeedsStencil() { fRenderTargetProxy->setNeedsStencil(); }
-
// This entry point should only be called if the backing GPU object is known to be
// instantiated.
GrRenderTarget* accessRenderTarget() { return fRenderTargetProxy->peekRenderTarget(); }
@@ -491,6 +489,7 @@
#if GR_TEST_UTILS
bool testingOnly_IsInstantiated() const { return fRenderTargetProxy->isInstantiated(); }
+ void testingOnly_SetPreserveOpsOnFullClear() { fPreserveOpsOnFullClear_TestingOnly = true; }
#endif
protected:
@@ -529,6 +528,9 @@
std::unique_ptr<GrFragmentProcessor>,
sk_sp<GrTextureProxy>);
+ GrRenderTargetOpList::CanDiscardPreviousOps canDiscardPreviousOpsOnFullClear() const;
+ void setNeedsStencil();
+
void internalClear(const GrFixedClip&, const SkPMColor4f&, CanClearFullscreen);
void internalStencilClear(const GrFixedClip&, bool insideStencilMask);
@@ -625,6 +627,11 @@
SkSurfaceProps fSurfaceProps;
bool fManagedOpList;
+ bool fNeedsStencil = false;
+#if GR_TEST_UTILS
+ bool fPreserveOpsOnFullClear_TestingOnly = false;
+#endif
+
typedef GrSurfaceContext INHERITED;
};
diff --git a/src/gpu/GrRenderTargetOpList.cpp b/src/gpu/GrRenderTargetOpList.cpp
index 080893e..84eb13b 100644
--- a/src/gpu/GrRenderTargetOpList.cpp
+++ b/src/gpu/GrRenderTargetOpList.cpp
@@ -536,31 +536,25 @@
}
}
-void GrRenderTargetOpList::setStencilLoadOp(GrLoadOp op) {
- fStencilLoadOp = op;
-}
-
void GrRenderTargetOpList::setColorLoadOp(GrLoadOp op, const SkPMColor4f& color) {
fColorLoadOp = op;
fLoadClearColor = color;
}
-bool GrRenderTargetOpList::resetForFullscreenClear() {
+bool GrRenderTargetOpList::resetForFullscreenClear(CanDiscardPreviousOps canDiscardPreviousOps) {
// Mark the color load op as discard (this may be followed by a clearColorOnLoad call to make
// the load op kClear, or it may be followed by an explicit op). In the event of an absClear()
// after a regular clear(), we could end up with a clear load op and a real clear op in the list
// if the load op were not reset here.
fColorLoadOp = GrLoadOp::kDiscard;
- // Regardless of how the clear is implemented (native clear or a fullscreen quad), all prior ops
- // would be overwritten, so discard them entirely. The one exception is if the opList is marked
- // as needing a stencil buffer then there may be a prior op that writes to the stencil buffer.
- // Although the clear will ignore the stencil buffer, following draw ops may not so we can't get
- // rid of all the preceding ops. Beware! If we ever add any ops that have a side effect beyond
- // modifying the stencil buffer we will need a more elaborate tracking system (skbug.com/7002).
- // Additionally, if we previously recorded a wait op, we cannot delete the wait op. Until we
- // track the wait ops separately from normal ops, we have to avoid clearing out any ops.
- if (this->isEmpty() || (!fTarget->asRenderTargetProxy()->needsStencil() && !fHasWaitOp)) {
+ // If we previously recorded a wait op, we cannot delete the wait op. Until we track the wait
+ // ops separately from normal ops, we have to avoid clearing out any ops in this case as well.
+ if (fHasWaitOp) {
+ canDiscardPreviousOps = CanDiscardPreviousOps::kNo;
+ }
+
+ if (CanDiscardPreviousOps::kYes == canDiscardPreviousOps || this->isEmpty()) {
this->deleteOps();
fDeferredProxies.reset();
diff --git a/src/gpu/GrRenderTargetOpList.h b/src/gpu/GrRenderTargetOpList.h
index 3269113..d24f471 100644
--- a/src/gpu/GrRenderTargetOpList.h
+++ b/src/gpu/GrRenderTargetOpList.h
@@ -128,7 +128,7 @@
bool onIsUsed(GrSurfaceProxy*) const override;
// Must only be called if native stencil buffer clearing is enabled
- void setStencilLoadOp(GrLoadOp op);
+ void setStencilLoadOp(GrLoadOp op) { fStencilLoadOp = op; }
// Must only be called if native color buffer clearing is enabled.
void setColorLoadOp(GrLoadOp op, const SkPMColor4f& color);
// Sets the clear color to transparent black
@@ -137,10 +137,15 @@
this->setColorLoadOp(op, kDefaultClearColor);
}
+ enum class CanDiscardPreviousOps : bool {
+ kYes = true,
+ kNo = false
+ };
+
// Perform book-keeping for a fullscreen clear, regardless of how the clear is implemented later
// (i.e. setColorLoadOp(), adding a ClearOp, or adding a GrFillRectOp that covers the device).
// Returns true if the clear can be converted into a load op (barring device caps).
- bool resetForFullscreenClear();
+ bool resetForFullscreenClear(CanDiscardPreviousOps);
void deleteOps();