Reland "Simplify GrRTC::clean APIs"
This reverts commit 4730f29993783fc9e96e45098d7e708fe08f1f26.
Reason for revert: Fix WIP
Original change's description:
> Revert "Simplify GrRTC::clean APIs"
>
> This reverts commit 6cbd7c2e57af9c499f7e99feb215b890fdd3a10a.
>
> Reason for revert: mac/generated files failures
>
> Original change's description:
> > Simplify GrRTC::clean APIs
> >
> > The CanClearFullscreen enum type is removed. Most usages of clear() had
> > kYes because a null scissor rect was provided, or had kNo because the
> > scissor was really critical to the behavior. A few places did provide a
> > scissor and kYes (e.g. for initializing the target).
> >
> > To simplify this, the public GrRTC has two variants of clear(). One with
> > only a color (for fullscreen clears), and one with a rect for partial
> > clears. The private API also adds a clearAtLeast() function that replaces
> > the several cases where we'd have a scissor but could expand to fullscreen.
> >
> > I find the current control flow in internalClear() to be hard to
> > follow (albeit I was the one to make it that way...), but later CLs
> > will improve it.
> >
> > Bug: skia:10205
> > Change-Id: I87cf8d688c58fbe58ee854fbc4ffe22482d969c6
> > Reviewed-on: https://skia-review.googlesource.com/c/skia/+/290256
> > Reviewed-by: Brian Salomon <bsalomon@google.com>
> > Commit-Queue: Michael Ludwig <michaelludwig@google.com>
>
> TBR=bsalomon@google.com,csmartdalton@google.com,michaelludwig@google.com
>
> Change-Id: I7131df6f5323f4f9c120cbcfd9bc57e627e2eb65
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Bug: skia:10205
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/291842
> Reviewed-by: Michael Ludwig <michaelludwig@google.com>
> Commit-Queue: Michael Ludwig <michaelludwig@google.com>
# Not skipping CQ checks because this is a reland.
Bug: skia:10205
Change-Id: Id5db153d7c2500279cca8478818b66f67a53e143
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/291844
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
diff --git a/src/gpu/GrRenderTargetContext.cpp b/src/gpu/GrRenderTargetContext.cpp
index cc53910..24755d1 100644
--- a/src/gpu/GrRenderTargetContext.cpp
+++ b/src/gpu/GrRenderTargetContext.cpp
@@ -479,32 +479,6 @@
this->getOpsTask()->discard();
}
-void GrRenderTargetContext::clear(const SkIRect* rect,
- const SkPMColor4f& color,
- CanClearFullscreen canClearFullscreen) {
- ASSERT_SINGLE_OWNER
- RETURN_IF_ABANDONED
- SkDEBUGCODE(this->validate();)
- GR_CREATE_TRACE_MARKER_CONTEXT("GrRenderTargetContext", "clear", fContext);
-
- AutoCheckFlush acf(this->drawingManager());
- this->internalClear(rect ? GrFixedClip(*rect) : GrFixedClip::Disabled(), color,
- canClearFullscreen);
-}
-
-void GrRenderTargetContextPriv::clear(const GrFixedClip& clip,
- const SkPMColor4f& color,
- CanClearFullscreen canClearFullscreen) {
- ASSERT_SINGLE_OWNER_PRIV
- RETURN_IF_ABANDONED_PRIV
- SkDEBUGCODE(fRenderTargetContext->validate();)
- GR_CREATE_TRACE_MARKER_CONTEXT("GrRenderTargetContextPriv", "clear",
- fRenderTargetContext->fContext);
-
- AutoCheckFlush acf(fRenderTargetContext->drawingManager());
- fRenderTargetContext->internalClear(clip, color, canClearFullscreen);
-}
-
static void clear_to_grpaint(const SkPMColor4f& color, GrPaint* paint) {
paint->setColor4f(color);
if (color.isOpaque()) {
@@ -517,19 +491,39 @@
}
}
-void GrRenderTargetContext::internalClear(const GrFixedClip& clip,
+// NOTE: We currently pass the premul color unmodified to the gpu, since we assume the GrRTC has a
+// premul alpha type. If we ever support different alpha type render targets, this function should
+// transform the color as appropriate.
+void GrRenderTargetContext::internalClear(const SkIRect* scissor,
const SkPMColor4f& color,
- CanClearFullscreen canClearFullscreen) {
- bool isFull = false;
- if (!clip.hasWindowRectangles()) {
- // TODO: wrt the shouldInitializeTextures path, it would be more performant to
- // only clear the entire target if we knew it had not been cleared before. As
- // is this could end up doing a lot of redundant clears.
- isFull = !clip.scissorEnabled() ||
- (CanClearFullscreen::kYes == canClearFullscreen &&
- (this->caps()->preferFullscreenClears() || this->caps()->shouldInitializeTextures())) ||
- clip.scissorRect().contains(SkIRect::MakeWH(this->width(), this->height()));
+ bool upgradePartialToFull) {
+ ASSERT_SINGLE_OWNER
+ RETURN_IF_ABANDONED
+ SkDEBUGCODE(this->validate();)
+ GR_CREATE_TRACE_MARKER_CONTEXT("GrRenderTargetContext", "clear", fContext);
+
+ // The clear will be fullscreen if no scissor is provided, or if the scissor is larger than
+ // the logical bounds of the render target, or if the special flag was provided that allows
+ // partial clears to upgrade to full (because it's a scratch resource and the caller knows
+ // anything outside the scissor doesn't matter, but if full screen clears aren't free, then
+ // the scissor is still provided so that fewer pixels are written to).
+ // TODO: wrt the shouldInitializeTextures path, it would be more performant to
+ // only clear the entire target if we knew it had not been cleared before. As
+ // is this could end up doing a lot of redundant clears.
+ GrScissorState scissorState;
+ if (scissor) {
+ // TODO(michaelludwig) - This will get simpler when GrScissorState knows the device dims
+ scissorState.set(*scissor);
+ if (!scissorState.intersect(SkIRect::MakeWH(this->width(), this->height()))) {
+ // The clear is offscreen, so skip it (normally this would be handled by addDrawOp,
+ // except clear ops are not draw ops).
+ return;
+ }
}
+ bool isFull = !scissorState.enabled() ||
+ scissorState.rect().contains(SkIRect::MakeWH(this->width(), this->height())) ||
+ (upgradePartialToFull && (this->caps()->preferFullscreenClears() ||
+ this->caps()->shouldInitializeTextures()));
if (isFull) {
GrOpsTask* opsTask = this->getOpsTask();
@@ -554,20 +548,19 @@
GrFillRectOp::MakeNonAARect(fContext, std::move(paint), SkMatrix::I(),
rtRect));
} else {
- this->addOp(GrClearOp::Make(
- fContext, SkIRect::MakeEmpty(), color, /* fullscreen */ true));
+ this->addOp(GrClearOp::Make(fContext, GrScissorState(), color, this->asSurfaceProxy()));
}
} else {
- if (this->caps()->performPartialClearsAsDraws() || clip.hasWindowRectangles()) {
+ if (this->caps()->performPartialClearsAsDraws()) {
// performPartialClearsAsDraws() also returns true if any clear has to be a draw.
GrPaint paint;
clear_to_grpaint(color, &paint);
- this->addDrawOp(clip,
+ this->addDrawOp(GrFixedClip::Disabled(),
GrFillRectOp::MakeNonAARect(fContext, std::move(paint), SkMatrix::I(),
- SkRect::Make(clip.scissorRect())));
+ SkRect::Make(scissorState.rect())));
} else {
- std::unique_ptr<GrOp> op(GrClearOp::Make(fContext, clip.scissorState(), color,
+ std::unique_ptr<GrOp> op(GrClearOp::Make(fContext, scissorState, color,
this->asSurfaceProxy()));
// This version of the clear op factory can return null if the clip doesn't intersect
// with the surface proxy's boundary
@@ -717,14 +710,14 @@
drawBounds = quad->fDevice.bounds();
if (drawBounds.contains(rtRect)) {
// Fullscreen clear
- this->clear(nullptr, *constColor, CanClearFullscreen::kYes);
+ this->clear(*constColor);
return QuadOptimization::kSubmitted;
} else if (GrClip::IsPixelAligned(drawBounds) &&
drawBounds.width() > 256 && drawBounds.height() > 256) {
// Scissor + clear (round shouldn't do anything since we are pixel aligned)
SkIRect scissorRect;
drawBounds.round(&scissorRect);
- this->clear(&scissorRect, *constColor, CanClearFullscreen::kNo);
+ this->clear(scissorRect, *constColor);
return QuadOptimization::kSubmitted;
}
}
@@ -956,7 +949,7 @@
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);
+ this->internalStencilClear(nullptr, /* inside mask */ false);
} else {
this->getOpsTask()->setInitialStencilContent(
GrOpsTask::StencilContent::kUserBitsCleared);
@@ -964,38 +957,30 @@
}
}
-void GrRenderTargetContextPriv::clearStencilClip(const GrFixedClip& clip, bool insideStencilMask) {
- ASSERT_SINGLE_OWNER_PRIV
- RETURN_IF_ABANDONED_PRIV
- SkDEBUGCODE(fRenderTargetContext->validate();)
- GR_CREATE_TRACE_MARKER_CONTEXT("GrRenderTargetContextPriv", "clearStencilClip",
- fRenderTargetContext->fContext);
-
- AutoCheckFlush acf(fRenderTargetContext->drawingManager());
-
- fRenderTargetContext->internalStencilClear(clip, insideStencilMask);
-}
-
-void GrRenderTargetContext::internalStencilClear(const GrFixedClip& clip, bool insideStencilMask) {
+void GrRenderTargetContext::internalStencilClear(const SkIRect* scissor, bool insideStencilMask) {
this->setNeedsStencil(/* useMixedSamplesIfNotMSAA = */ false);
bool clearWithDraw = this->caps()->performStencilClearsAsDraws() ||
- (clip.scissorEnabled() && this->caps()->performPartialClearsAsDraws());
- // TODO(michaelludwig): internalStencilClear will eventually just take a GrScissorState so
- // we won't need to check window rectangles here.
- if (clearWithDraw || clip.hasWindowRectangles()) {
+ (scissor && this->caps()->performPartialClearsAsDraws());
+ if (clearWithDraw) {
const GrUserStencilSettings* ss = GrStencilSettings::SetClipBitSettings(insideStencilMask);
- SkRect rect = clip.scissorEnabled() ? SkRect::Make(clip.scissorRect())
- : SkRect::MakeWH(this->width(), this->height());
+ SkRect rect = scissor ? SkRect::Make(*scissor)
+ : SkRect::MakeWH(this->width(), this->height());
// Configure the paint to have no impact on the color buffer
GrPaint paint;
paint.setXPFactory(GrDisableColorXPFactory::Get());
- this->addDrawOp(clip, GrFillRectOp::MakeNonAARect(fContext, std::move(paint), SkMatrix::I(),
- rect, ss));
+ this->addDrawOp(GrFixedClip::Disabled(),
+ GrFillRectOp::MakeNonAARect(fContext, std::move(paint), SkMatrix::I(),
+ rect, ss));
} else {
+ GrScissorState scissorState;
+ if (scissor) {
+ scissorState.set(*scissor);
+ }
+
std::unique_ptr<GrOp> op(GrClearStencilClipOp::Make(
- fContext, clip.scissorState(), insideStencilMask, this->asRenderTargetProxy()));
+ fContext, scissorState, insideStencilMask, this->asRenderTargetProxy()));
if (!op) {
return;
}