Reland "Improve scissor state tracking in GrRTC"
This reverts commit 4926b07217f07e8f5ff1dba15d23bab960ffded3.
Reason for revert: fix wip
Original change's description:
> Revert "Improve scissor state tracking in GrRTC"
>
> This reverts commit 3b923a880bc0855772daffd95b5728795f515d5f.
>
> Reason for revert: GrAppliedHardClip isn't tracking scissor state properly
>
> Original change's description:
> > Improve scissor state tracking in GrRTC
> >
> > At a low level, this changes GrScissorState from a rect+bool to a rect+size.
> > The scissor test is considered enablebd if the rect does not fill the
> > device bounds rect specified by the size. This has a number of benefits:
> >
> > 1. We can always access the scissor rect and know that it will be
> > restricted to the render target dimensions.
> > 2. It helps consolidate code that previously had to test the scissor rect
> > and render target bounds separately.
> > 3. The clear operations can now match the proper backing store dimensions
> > of the render target.
> > 4. It makes it easier to reason about scissors applying to the logical
> > dimensions of the render target vs. its backing store dimensions.
> >
> > Originally, I was going to have the extra scissor guards for the logical
> > dimensions be added in a separate CL (with the cleanup for
> > attemptQuadOptimization). However, it became difficult to ensure correct
> > behavior respecting the vulkan render pass bounds without applying this
> > new logic at the same time.
> >
> > So now, with this CL, GrAppliedClips are sized to the backing store
> > dimensions of the render target. GrOpsTasks also clip bounds to the
> > backing store dimensions instead of the logical dimensions (which seems
> > more correct since that's where the auto-clipping happens). Then when
> > we convert a GrClip to a GrAppliedClip, the GrRTC automatically enforces
> > the logical dimensions scissor if we have stencil settings (to ensure
> > the padded pixels don't get corrupted). It also may remove the scissor
> > if the draw was just a color buffer update.
> >
> > Change-Id: I75671c9cc921f4696b1dd5231e02486090aa4282
> > Reviewed-on: https://skia-review.googlesource.com/c/skia/+/290654
> > Commit-Queue: Michael Ludwig <michaelludwig@google.com>
> > Reviewed-by: Brian Salomon <bsalomon@google.com>
>
> TBR=bsalomon@google.com,csmartdalton@google.com,michaelludwig@google.com
>
> Change-Id: Ie98d084158e3a537604ab0fecee69bde3e744d1b
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/294340
> Reviewed-by: Michael Ludwig <michaelludwig@google.com>
> Commit-Queue: Michael Ludwig <michaelludwig@google.com>
TBR=bsalomon@google.com,csmartdalton@google.com,michaelludwig@google.com
# Not skipping CQ checks because this is a reland.
Change-Id: I2116e52146890ee4b7ea007f3c3d5c3e532e4bdd
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/294257
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
diff --git a/gm/clockwise.cpp b/gm/clockwise.cpp
index 480252b..d19690a 100644
--- a/gm/clockwise.cpp
+++ b/gm/clockwise.cpp
@@ -188,7 +188,7 @@
SkArenaAlloc* arena = context->priv().recordTimeAllocator();
// This is equivalent to a GrOpFlushState::detachAppliedClip
- GrAppliedClip appliedClip = clip ? std::move(*clip) : GrAppliedClip();
+ GrAppliedClip appliedClip = clip ? std::move(*clip) : GrAppliedClip::Disabled();
fProgramInfo = this->createProgramInfo(context->priv().caps(), arena, writeView,
std::move(appliedClip), dstProxyView);
diff --git a/gm/fwidth_squircle.cpp b/gm/fwidth_squircle.cpp
index 0386a02..33fd566 100644
--- a/gm/fwidth_squircle.cpp
+++ b/gm/fwidth_squircle.cpp
@@ -197,7 +197,7 @@
SkArenaAlloc* arena = context->priv().recordTimeAllocator();
// This is equivalent to a GrOpFlushState::detachAppliedClip
- GrAppliedClip appliedClip = clip ? std::move(*clip) : GrAppliedClip();
+ GrAppliedClip appliedClip = clip ? std::move(*clip) : GrAppliedClip::Disabled();
fProgramInfo = this->createProgramInfo(context->priv().caps(), arena, writeView,
std::move(appliedClip), dstProxyView);
diff --git a/gm/samplelocations.cpp b/gm/samplelocations.cpp
index f0aa981..108fa3d 100644
--- a/gm/samplelocations.cpp
+++ b/gm/samplelocations.cpp
@@ -274,7 +274,7 @@
SkArenaAlloc* arena = context->priv().recordTimeAllocator();
// This is equivalent to a GrOpFlushState::detachAppliedClip
- GrAppliedClip appliedClip = clip ? std::move(*clip) : GrAppliedClip();
+ GrAppliedClip appliedClip = clip ? std::move(*clip) : GrAppliedClip::Disabled();
fProgramInfo = this->createProgramInfo(context->priv().caps(), arena, writeView,
std::move(appliedClip), dstProxyView);
diff --git a/gm/windowrectangles.cpp b/gm/windowrectangles.cpp
index 932b83f..f7c3f7b 100644
--- a/gm/windowrectangles.cpp
+++ b/gm/windowrectangles.cpp
@@ -193,13 +193,12 @@
/**
* Makes a clip object that enforces the stencil clip bit. Used to visualize the stencil mask.
*/
-static const GrStencilClip* make_stencil_only_clip() {
- static const GrStencilClip kClip(SkClipStack::kEmptyGenID);
- return &kClip;
-}
+static GrStencilClip make_stencil_only_clip(GrRenderTargetContext* rtc) {
+ return GrStencilClip(rtc->dimensions(), SkClipStack::kEmptyGenID);
+};
DrawResult WindowRectanglesMaskGM::onCoverClipStack(const SkClipStack& stack, SkCanvas* canvas,
- SkString* errorMsg) {
+ SkString* errorMsg) {
GrContext* ctx = canvas->getGrContext();
GrRenderTargetContext* rtc = canvas->internal_private_accessTopLayerRenderTargetContext();
if (!ctx || !rtc) {
@@ -242,9 +241,10 @@
maskRTC->clear(SK_PMColor4fWHITE);
GrPaint stencilPaint;
stencilPaint.setCoverageSetOpXPFactory(SkRegion::kDifference_Op, false);
- maskRTC->priv().stencilRect(make_stencil_only_clip(), &GrUserStencilSettings::kUnused,
+ GrStencilClip stencilClip = make_stencil_only_clip(maskRTC.get());
+ maskRTC->priv().stencilRect(&stencilClip, &GrUserStencilSettings::kUnused,
std::move(stencilPaint), GrAA::kNo, SkMatrix::I(),
- SkRect::MakeIWH(maskRTC->width(), maskRTC->height()));
+ SkRect::Make(maskRTC->dimensions()));
reducedClip.drawAlphaClipMask(maskRTC.get());
int x = kCoverRect.x() - kDeviceRect.x(),
@@ -253,8 +253,8 @@
// Now visualize the alpha mask by drawing a rect over the area where it is defined. The regions
// inside window rectangles or outside the scissor should still have the initial checkerboard
// intact. (This verifies we didn't spend any time modifying those pixels in the mask.)
- AlphaOnlyClip clip(maskRTC->readSurfaceView(), x, y);
- rtc->drawRect(&clip, std::move(paint), GrAA::kYes, SkMatrix::I(),
+ AlphaOnlyClip alphaClip(maskRTC->readSurfaceView(), x, y);
+ rtc->drawRect(&alphaClip, std::move(paint), GrAA::kYes, SkMatrix::I(),
SkRect::Make(SkIRect::MakeXYWH(x, y, maskRTC->width(), maskRTC->height())));
}
@@ -275,7 +275,8 @@
// Now visualize the stencil mask by covering the entire render target. The regions inside
// window rectangles or outside the scissor should still have the initial checkerboard intact.
// (This verifies we didn't spend any time modifying those pixels in the mask.)
- rtc->drawPaint(make_stencil_only_clip(), std::move(paint), SkMatrix::I());
+ GrStencilClip clip = make_stencil_only_clip(rtc);
+ rtc->drawPaint(&clip, std::move(paint), SkMatrix::I());
}
void WindowRectanglesMaskGM::stencilCheckerboard(GrRenderTargetContext* rtc, bool flip) {
diff --git a/src/gpu/GrAppliedClip.h b/src/gpu/GrAppliedClip.h
index 594a627..75e614a 100644
--- a/src/gpu/GrAppliedClip.h
+++ b/src/gpu/GrAppliedClip.h
@@ -22,11 +22,19 @@
class GrAppliedHardClip {
public:
static const GrAppliedHardClip& Disabled() {
- static GrAppliedHardClip kDisabled;
+ // The size doesn't really matter here since it's returned as const& so an actual scissor
+ // will never be set on it, and applied clips are not used to query or bounds test like
+ // the GrClip is.
+ static const GrAppliedHardClip kDisabled({1 << 29, 1 << 29});
return kDisabled;
}
- GrAppliedHardClip() = default;
+ GrAppliedHardClip(const SkISize& rtDims) : fScissorState(rtDims) {}
+ GrAppliedHardClip(const SkISize& logicalRTDims, const SkISize& backingStoreDims)
+ : fScissorState(backingStoreDims) {
+ fScissorState.set(SkIRect::MakeSize(logicalRTDims));
+ }
+
GrAppliedHardClip(GrAppliedHardClip&& that) = default;
GrAppliedHardClip(const GrAppliedHardClip&) = delete;
@@ -81,7 +89,14 @@
*/
class GrAppliedClip {
public:
- GrAppliedClip() = default;
+ static GrAppliedClip Disabled() {
+ return GrAppliedClip({1 << 29, 1 << 29});
+ }
+
+ GrAppliedClip(const SkISize& rtDims) : fHardClip(rtDims) {}
+ GrAppliedClip(const SkISize& logicalRTDims, const SkISize& backingStoreDims)
+ : fHardClip(logicalRTDims, backingStoreDims) {}
+
GrAppliedClip(GrAppliedClip&& that) = default;
GrAppliedClip(const GrAppliedClip&) = delete;
diff --git a/src/gpu/GrBlurUtils.cpp b/src/gpu/GrBlurUtils.cpp
index e348f4b..7068b77 100644
--- a/src/gpu/GrBlurUtils.cpp
+++ b/src/gpu/GrBlurUtils.cpp
@@ -200,8 +200,7 @@
maskPaint.setCoverageSetOpXPFactory(SkRegion::kReplace_Op);
// setup new clip
- const SkIRect clipRect = SkIRect::MakeWH(maskRect.width(), maskRect.height());
- GrFixedClip clip(clipRect);
+ GrFixedClip clip(rtContext->dimensions(), SkIRect::MakeWH(maskRect.width(), maskRect.height()));
// Draw the mask into maskTexture with the path's integerized top-left at the origin using
// maskPaint.
diff --git a/src/gpu/GrFixedClip.h b/src/gpu/GrFixedClip.h
index 017c3ff..422b11d 100644
--- a/src/gpu/GrFixedClip.h
+++ b/src/gpu/GrFixedClip.h
@@ -17,17 +17,21 @@
*/
class GrFixedClip final : public GrHardClip {
public:
- GrFixedClip() = default;
- explicit GrFixedClip(const SkIRect& scissorRect) : fScissorState(scissorRect) {}
+ explicit GrFixedClip(const SkISize& rtDims) : fScissorState(rtDims) {}
+ GrFixedClip(const SkISize& rtDims, const SkIRect& scissorRect)
+ : GrFixedClip(rtDims) {
+ SkAssertResult(fScissorState.set(scissorRect));
+ }
const GrScissorState& scissorState() const { return fScissorState; }
bool scissorEnabled() const { return fScissorState.enabled(); }
- const SkIRect& scissorRect() const { SkASSERT(scissorEnabled()); return fScissorState.rect(); }
+ // Returns the scissor rect or rt bounds if the scissor test is not enabled.
+ const SkIRect& scissorRect() const { return fScissorState.rect(); }
void disableScissor() { fScissorState.setDisabled(); }
- void setScissor(const SkIRect& irect) {
- fScissorState.set(irect);
+ bool SK_WARN_UNUSED_RESULT setScissor(const SkIRect& irect) {
+ return fScissorState.set(irect);
}
bool SK_WARN_UNUSED_RESULT intersect(const SkIRect& irect) {
return fScissorState.intersect(irect);
diff --git a/src/gpu/GrOpFlushState.cpp b/src/gpu/GrOpFlushState.cpp
index dfaa9d6..8bc88f0 100644
--- a/src/gpu/GrOpFlushState.cpp
+++ b/src/gpu/GrOpFlushState.cpp
@@ -194,7 +194,7 @@
}
GrAppliedClip GrOpFlushState::detachAppliedClip() {
- return fOpArgs->appliedClip() ? std::move(*fOpArgs->appliedClip()) : GrAppliedClip();
+ return fOpArgs->appliedClip() ? std::move(*fOpArgs->appliedClip()) : GrAppliedClip::Disabled();
}
GrStrikeCache* GrOpFlushState::strikeCache() const {
diff --git a/src/gpu/GrOpsTask.cpp b/src/gpu/GrOpsTask.cpp
index 4537242..a5426bc 100644
--- a/src/gpu/GrOpsTask.cpp
+++ b/src/gpu/GrOpsTask.cpp
@@ -613,7 +613,7 @@
if (GrLoadOp::kClear == fColorLoadOp) {
GrSurfaceProxy* proxy = fTargetView.proxy();
SkASSERT(proxy);
- fTotalBounds = proxy->getBoundsRect();
+ fTotalBounds = proxy->backingStoreBoundsRect();
}
}
@@ -894,7 +894,9 @@
});
if (!this->isNoOp()) {
GrSurfaceProxy* proxy = fTargetView.proxy();
- SkRect clippedContentBounds = proxy->getBoundsRect();
+ // Use the entire backing store bounds since the GPU doesn't clip automatically to the
+ // logical dimensions.
+ SkRect clippedContentBounds = proxy->backingStoreBoundsRect();
// TODO: If we can fix up GLPrograms test to always intersect the fTargetView proxy bounds
// then we can simply assert here that the bounds intersect.
if (clippedContentBounds.intersect(fTotalBounds)) {
diff --git a/src/gpu/GrReducedClip.cpp b/src/gpu/GrReducedClip.cpp
index 0e01895..313f1ff 100644
--- a/src/gpu/GrReducedClip.cpp
+++ b/src/gpu/GrReducedClip.cpp
@@ -742,7 +742,7 @@
bool GrReducedClip::drawAlphaClipMask(GrRenderTargetContext* rtc) const {
// The texture may be larger than necessary, this rect represents the part of the texture
// we populate with a rasterization of the clip.
- GrFixedClip clip(SkIRect::MakeWH(fScissor.width(), fScissor.height()));
+ GrFixedClip clip(rtc->dimensions(), SkIRect::MakeWH(fScissor.width(), fScissor.height()));
if (!fWindowRects.empty()) {
clip.setWindowRectangles(fWindowRects.makeOffset(-fScissor.left(), -fScissor.top()),
diff --git a/src/gpu/GrRenderTargetContext.cpp b/src/gpu/GrRenderTargetContext.cpp
index cdbc65d..0918fb5d 100644
--- a/src/gpu/GrRenderTargetContext.cpp
+++ b/src/gpu/GrRenderTargetContext.cpp
@@ -500,30 +500,35 @@
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;
+ // There are three ways clears are handled: load ops, native clears, and draws. Load ops are
+ // only for fullscreen clears; native clears can be fullscreen or with scissors if the backend
+ // supports then. Drawing an axis-aligned rect is the fallback path.
+ GrScissorState scissorState(this->asSurfaceProxy()->backingStoreDimensions());
+ if (scissor && !scissorState.set(*scissor)) {
+ // The clear is offscreen, so skip it (normally this would be handled by addDrawOp,
+ // except clear ops are not draw ops).
+ return;
+ }
+
+ // If we have a scissor but it's okay to clear beyond it for performance reasons, then disable
+ // the test. We only do this when the clear would be handled by a load op or natively.
+ if (scissorState.enabled() && !this->caps()->performColorClearsAsDraws()) {
+ if (upgradePartialToFull && (this->caps()->preferFullscreenClears() ||
+ this->caps()->shouldInitializeTextures())) {
+ // 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.
+ scissorState.setDisabled();
+ } else {
+ // Unlike with stencil clears, we also allow clears up to the logical dimensions of the
+ // render target to overflow into any approx-fit padding of the backing store dimensions
+ scissorState.relaxTest(this->dimensions());
}
}
- bool isFull = !scissorState.enabled() ||
- scissorState.rect().contains(SkIRect::MakeWH(this->width(), this->height())) ||
- (upgradePartialToFull && (this->caps()->preferFullscreenClears() ||
- this->caps()->shouldInitializeTextures()));
- if (isFull) {
+ if (!scissorState.enabled()) {
+ // This is a fullscreen clear, so could be handled as a load op. Regardless, we can also
+ // discard all prior ops in the current task since the color buffer will be overwritten.
GrOpsTask* opsTask = this->getOpsTask();
if (opsTask->resetForFullscreenClear(this->canDiscardPreviousOpsOnFullClear()) &&
!this->caps()->performColorClearsAsDraws()) {
@@ -535,38 +540,20 @@
// blow away the color buffer contents
opsTask->setColorLoadOp(GrLoadOp::kDiscard);
}
+ }
- // Must add an op to the list (either because we couldn't use a load op, or because the
- // clear load op isn't supported)
- if (this->caps()->performColorClearsAsDraws()) {
- SkRect rtRect = SkRect::MakeWH(this->width(), this->height());
- GrPaint paint;
- clear_to_grpaint(color, &paint);
- this->addDrawOp(nullptr,
- GrFillRectOp::MakeNonAARect(fContext, std::move(paint), SkMatrix::I(),
- rtRect));
- } else {
- this->addOp(GrClearOp::Make(fContext, GrScissorState(), color, this->asSurfaceProxy()));
- }
+ // At this point we are either a partial clear or a fullscreen clear that couldn't be applied
+ // as a load op.
+ bool clearAsDraw = this->caps()->performColorClearsAsDraws() ||
+ (scissorState.enabled() && this->caps()->performPartialClearsAsDraws());
+ if (clearAsDraw) {
+ GrPaint paint;
+ clear_to_grpaint(color, &paint);
+ this->addDrawOp(nullptr,
+ GrFillRectOp::MakeNonAARect(fContext, std::move(paint), SkMatrix::I(),
+ SkRect::Make(scissorState.rect())));
} else {
- 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(nullptr,
- GrFillRectOp::MakeNonAARect(fContext, std::move(paint), SkMatrix::I(),
- SkRect::Make(scissorState.rect())));
- } else {
- 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
- if (!op) {
- return;
- }
- this->addOp(std::move(op));
- }
+ this->addOp(GrClearOp::Make(fContext, scissorState, color));
}
}
@@ -641,16 +628,9 @@
// better to just keep the old flags instead of introducing mixed edge flags.
GrQuadAAFlags oldFlags = quad->fEdgeFlags;
- SkRect rtRect;
- if (stencilSettings) {
- // Must use size at which the rendertarget will ultimately be allocated so that stencil
- // buffer updates on approximately sized render targets don't get corrupted.
- rtRect = this->asSurfaceProxy()->backingStoreBoundsRect();
- } else {
- // Use the logical size of the render target, which allows for "fullscreen" clears even if
- // the render target has an approximate backing fit
- rtRect = SkRect::MakeWH(this->width(), this->height());
- }
+ // Use the logical size of the render target, which allows for "fullscreen" clears even if
+ // the render target has an approximate backing fit
+ SkRect rtRect = this->asSurfaceProxy()->getBoundsRect();
SkRect drawBounds = quad->fDevice.bounds();
if (constColor) {
@@ -961,30 +941,25 @@
void GrRenderTargetContext::internalStencilClear(const SkIRect* scissor, bool insideStencilMask) {
this->setNeedsStencil(/* useMixedSamplesIfNotMSAA = */ false);
+ GrScissorState scissorState(this->asSurfaceProxy()->backingStoreDimensions());
+ if (scissor && !scissorState.set(*scissor)) {
+ // The requested clear region is off screen, so nothing to do.
+ return;
+ }
+
bool clearWithDraw = this->caps()->performStencilClearsAsDraws() ||
- (scissor && this->caps()->performPartialClearsAsDraws());
+ (scissorState.enabled() && this->caps()->performPartialClearsAsDraws());
if (clearWithDraw) {
const GrUserStencilSettings* ss = GrStencilSettings::SetClipBitSettings(insideStencilMask);
- 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(nullptr, GrFillRectOp::MakeNonAARect(fContext, std::move(paint),
- SkMatrix::I(), rect, ss));
+ this->addDrawOp(nullptr,
+ GrFillRectOp::MakeNonAARect(fContext, std::move(paint), SkMatrix::I(),
+ SkRect::Make(scissorState.rect()), ss));
} else {
- GrScissorState scissorState;
- if (scissor) {
- scissorState.set(*scissor);
- }
-
- std::unique_ptr<GrOp> op(GrClearStencilClipOp::Make(
- fContext, scissorState, insideStencilMask, this->asRenderTargetProxy()));
- if (!op) {
- return;
- }
- this->addOp(std::move(op));
+ this->addOp(GrClearStencilClipOp::Make(fContext, scissorState, insideStencilMask));
}
}
@@ -1009,7 +984,9 @@
// Setup clip and reject offscreen paths; we do this explicitly instead of relying on addDrawOp
// because GrStencilPathOp is not a draw op as its state depends directly on the choices made
// during this clip application.
- GrAppliedHardClip appliedClip;
+ GrAppliedHardClip appliedClip(fRenderTargetContext->dimensions(),
+ fRenderTargetContext->asSurfaceProxy()->backingStoreDimensions());
+
if (clip && !clip->apply(fRenderTargetContext->width(), fRenderTargetContext->height(),
&appliedClip, &bounds)) {
return;
@@ -2496,7 +2473,7 @@
// Setup clip
SkRect bounds;
op_bounds(&bounds, op.get());
- GrAppliedClip appliedClip;
+ GrAppliedClip appliedClip(this->dimensions(), this->asSurfaceProxy()->backingStoreDimensions());
GrDrawOp::FixedFunctionFlags fixedFunctionFlags = op->fixedFunctionFlags();
bool usesHWAA = fixedFunctionFlags & GrDrawOp::FixedFunctionFlags::kUsesHWAA;
bool usesUserStencilBits = fixedFunctionFlags & GrDrawOp::FixedFunctionFlags::kUsesStencil;
diff --git a/src/gpu/GrRenderTargetProxy.h b/src/gpu/GrRenderTargetProxy.h
index de21acc..31193ae 100644
--- a/src/gpu/GrRenderTargetProxy.h
+++ b/src/gpu/GrRenderTargetProxy.h
@@ -60,7 +60,7 @@
bool wrapsVkSecondaryCB() const { return fWrapsVkSecondaryCB == WrapsVkSecondaryCB::kYes; }
void markMSAADirty(const SkIRect& dirtyRect, GrSurfaceOrigin origin) {
- SkASSERT(SkIRect::MakeSize(this->dimensions()).contains(dirtyRect));
+ SkASSERT(SkIRect::MakeSize(this->backingStoreDimensions()).contains(dirtyRect));
SkASSERT(this->requiresManualMSAAResolve());
auto nativeRect = GrNativeRect::MakeRelativeTo(
origin, this->backingStoreDimensions().height(), dirtyRect);
diff --git a/src/gpu/GrScissorState.h b/src/gpu/GrScissorState.h
index dac735b..cc1cea2 100644
--- a/src/gpu/GrScissorState.h
+++ b/src/gpu/GrScissorState.h
@@ -10,33 +10,74 @@
#include "include/core/SkRect.h"
+/**
+ * The scissor state is stored as the scissor rectangle and the backing store bounds of the render
+ * target that the scissor will apply to. If the render target is approximate fit and the padded
+ * content should not be modified, the clip should apply the render target context's logical bounds
+ * as part of the scissor (e.g. when stenciling). This puts the onus on the render target context
+ * to intentionally discard the scissor at its logical bounds when drawing into the padded content
+ * is acceptable (e.g. color-only updates).
+ */
class GrScissorState {
public:
- GrScissorState() : fEnabled(false) {}
- GrScissorState(const SkIRect& rect) : fEnabled(true), fRect(rect) {}
- void setDisabled() { fEnabled = false; }
- void set(const SkIRect& rect) { fRect = rect; fEnabled = true; }
+ // The disabled scissor state for a render target of the given size.
+ explicit GrScissorState(const SkISize& rtDims)
+ : fRTSize(rtDims)
+ , fRect(SkIRect::MakeSize(rtDims)) {}
+
+ void setDisabled() { fRect = SkIRect::MakeSize(fRTSize); }
+ bool set(const SkIRect& rect) {
+ this->setDisabled();
+ return this->intersect(rect);
+ }
+
bool SK_WARN_UNUSED_RESULT intersect(const SkIRect& rect) {
- if (!fEnabled) {
- this->set(rect);
+ if (!fRect.intersect(rect)) {
+ fRect.setEmpty();
+ return false;
+ } else {
return true;
}
- return fRect.intersect(rect);
}
+
+ // If the scissor was configured for the backing store dimensions and it's acceptable to
+ // draw outside the logical dimensions of the target, this will discard the scissor test if
+ // the test wouldn't modify the logical dimensions.
+ bool relaxTest(const SkISize& logicalDimensions) {
+ SkASSERT(logicalDimensions.fWidth <= fRTSize.fWidth &&
+ logicalDimensions.fHeight <= fRTSize.fHeight);
+ if (fRect.fLeft == 0 && fRect.fTop == 0 && fRect.fRight >= logicalDimensions.fWidth &&
+ fRect.fBottom >= logicalDimensions.fHeight) {
+ this->setDisabled();
+ return true;
+ } else {
+ return false;
+ }
+ }
+
bool operator==(const GrScissorState& other) const {
- return fEnabled == other.fEnabled &&
- (false == fEnabled || fRect == other.fRect);
+ return fRTSize == other.fRTSize && fRect == other.fRect;
}
bool operator!=(const GrScissorState& other) const { return !(*this == other); }
- bool enabled() const { return fEnabled; }
+ bool enabled() const {
+ SkASSERT(fRect.isEmpty() || SkIRect::MakeSize(fRTSize).contains(fRect));
+ // This is equivalent to a strict contains check on SkIRect::MakeSize(rtSize) w/o creating
+ // the render target bounding rectangle.
+ return fRect.fLeft > 0 || fRect.fTop > 0 ||
+ fRect.fRight < fRTSize.fWidth || fRect.fBottom < fRTSize.fHeight;
+ }
+
+ // Will always be equal to or contained in the rt bounds, or empty if scissor rectangles were
+ // added that did not intersect with the render target or prior scissor.
const SkIRect& rect() const {
- SkASSERT(fEnabled);
+ SkASSERT(fRect.isEmpty() || SkIRect::MakeSize(fRTSize).contains(fRect));
return fRect;
}
private:
- bool fEnabled;
+ // The scissor is considered enabled if the rectangle does not cover the render target
+ SkISize fRTSize;
SkIRect fRect;
};
diff --git a/src/gpu/GrStencilClip.h b/src/gpu/GrStencilClip.h
index 854ba1c..5c9f4a5 100644
--- a/src/gpu/GrStencilClip.h
+++ b/src/gpu/GrStencilClip.h
@@ -16,12 +16,14 @@
*/
class GrStencilClip final : public GrHardClip {
public:
- GrStencilClip(uint32_t stencilStackID = SK_InvalidGenID) : fStencilStackID(stencilStackID) {}
+ explicit GrStencilClip(const SkISize& rtDims, uint32_t stencilStackID = SK_InvalidGenID)
+ : fFixedClip(rtDims)
+ , fStencilStackID(stencilStackID) {}
- explicit GrStencilClip(const SkIRect& scissorRect, uint32_t stencilStackID = SK_InvalidGenID)
- : fFixedClip(scissorRect)
- , fStencilStackID(stencilStackID) {
- }
+ GrStencilClip(const SkISize& rtDims, const SkIRect& scissorRect,
+ uint32_t stencilStackID = SK_InvalidGenID)
+ : fFixedClip(rtDims, scissorRect)
+ , fStencilStackID(stencilStackID) {}
const GrFixedClip& fixedClip() const { return fFixedClip; }
GrFixedClip& fixedClip() { return fFixedClip; }
diff --git a/src/gpu/GrStencilMaskHelper.cpp b/src/gpu/GrStencilMaskHelper.cpp
index c4bd9b9..f261e26 100644
--- a/src/gpu/GrStencilMaskHelper.cpp
+++ b/src/gpu/GrStencilMaskHelper.cpp
@@ -334,7 +334,8 @@
}
fClip.setStencilClip(genID);
- fClip.fixedClip().setScissor(bounds);
+ // Should have caught bounds not intersecting the render target much earlier in clip application
+ SkAssertResult(fClip.fixedClip().setScissor(bounds));
if (!windowRects.empty()) {
fClip.fixedClip().setWindowRectangles(
windowRects, GrWindowRectsState::Mode::kExclusive);
diff --git a/src/gpu/GrStencilMaskHelper.h b/src/gpu/GrStencilMaskHelper.h
index 547bee2..f6cd2b9 100644
--- a/src/gpu/GrStencilMaskHelper.h
+++ b/src/gpu/GrStencilMaskHelper.h
@@ -39,7 +39,8 @@
// of which must outlive the helper.
GrStencilMaskHelper(GrRecordingContext* context, GrRenderTargetContext* rtc)
: fContext(context)
- , fRTC(rtc) {}
+ , fRTC(rtc)
+ , fClip(rtc->dimensions()) {}
// Returns true if the stencil mask must be redrawn
bool init(const SkIRect& maskBounds, uint32_t genID,
diff --git a/src/gpu/ccpr/GrCCPerFlushResources.cpp b/src/gpu/ccpr/GrCCPerFlushResources.cpp
index dde97ae..d13a52a 100644
--- a/src/gpu/ccpr/GrCCPerFlushResources.cpp
+++ b/src/gpu/ccpr/GrCCPerFlushResources.cpp
@@ -96,8 +96,10 @@
GrCCPathProcessor pathProc(coverageMode, fSrcProxy->peekTexture(), swizzle,
GrCCAtlas::kTextureOrigin);
- GrPipeline pipeline(GrScissorTest::kDisabled, SkBlendMode::kSrc,
- flushState->drawOpArgs().writeSwizzle());
+ bool hasScissor = flushState->appliedClip() &&
+ flushState->appliedClip()->scissorState().enabled();
+ GrPipeline pipeline(hasScissor ? GrScissorTest::kEnabled : GrScissorTest::kDisabled,
+ SkBlendMode::kSrc, flushState->drawOpArgs().writeSwizzle());
pathProc.drawPaths(flushState, pipeline, *fSrcProxy, *fResources, fBaseInstance,
fEndInstance, this->bounds());
diff --git a/src/gpu/d3d/GrD3DOpsRenderPass.cpp b/src/gpu/d3d/GrD3DOpsRenderPass.cpp
index 0d3357f..7b22fc1 100644
--- a/src/gpu/d3d/GrD3DOpsRenderPass.cpp
+++ b/src/gpu/d3d/GrD3DOpsRenderPass.cpp
@@ -51,7 +51,8 @@
// TODO: set stencil too
if (GrLoadOp::kClear == fColorLoadOp) {
- fGpu->currentCommandList()->clearRenderTargetView(d3dRT, fClearColor, GrScissorState());
+ fGpu->currentCommandList()->clearRenderTargetView(
+ d3dRT, fClearColor, GrScissorState(fRenderTarget->dimensions()));
}
}
diff --git a/src/gpu/gl/GrGLGpu.cpp b/src/gpu/gl/GrGLGpu.cpp
index a63db97..bf9c1f8 100644
--- a/src/gpu/gl/GrGLGpu.cpp
+++ b/src/gpu/gl/GrGLGpu.cpp
@@ -2260,8 +2260,9 @@
// Apple's extension uses the scissor as the blit bounds.
// Passing in kTopLeft_GrSurfaceOrigin will make sure no transformation of the rect
// happens inside flushScissor since resolveRect is already in native device coordinates.
- this->flushScissor(GrScissorState(resolveRect), rt->width(), rt->height(),
- kTopLeft_GrSurfaceOrigin);
+ GrScissorState scissor(rt->dimensions());
+ SkAssertResult(scissor.set(resolveRect));
+ this->flushScissor(scissor, rt->width(), rt->height(), kTopLeft_GrSurfaceOrigin);
this->disableWindowRectangles();
GL_CALL(ResolveMultisampleFramebuffer());
} else {
diff --git a/src/gpu/ops/GrAAHairLinePathRenderer.cpp b/src/gpu/ops/GrAAHairLinePathRenderer.cpp
index 8a0a1b8..9dae347 100644
--- a/src/gpu/ops/GrAAHairLinePathRenderer.cpp
+++ b/src/gpu/ops/GrAAHairLinePathRenderer.cpp
@@ -1117,7 +1117,7 @@
const GrCaps* caps = context->priv().caps();
// This is equivalent to a GrOpFlushState::detachAppliedClip
- GrAppliedClip appliedClip = clip ? std::move(*clip) : GrAppliedClip();
+ GrAppliedClip appliedClip = clip ? std::move(*clip) : GrAppliedClip::Disabled();
// Conservatively predict which programs will be required
fCharacterization = this->predictPrograms(caps);
diff --git a/src/gpu/ops/GrClearOp.cpp b/src/gpu/ops/GrClearOp.cpp
index 753f75d..e954083 100644
--- a/src/gpu/ops/GrClearOp.cpp
+++ b/src/gpu/ops/GrClearOp.cpp
@@ -16,24 +16,16 @@
std::unique_ptr<GrClearOp> GrClearOp::Make(GrRecordingContext* context,
const GrScissorState& scissor,
- const SkPMColor4f& color,
- const GrSurfaceProxy* dstProxy) {
- const SkIRect rect = SkIRect::MakeSize(dstProxy->dimensions());
- if (scissor.enabled() && !SkIRect::Intersects(scissor.rect(), rect)) {
- return nullptr;
- }
-
+ const SkPMColor4f& color) {
GrOpMemoryPool* pool = context->priv().opMemoryPool();
- return pool->allocate<GrClearOp>(scissor, color, dstProxy);
+ return pool->allocate<GrClearOp>(scissor, color);
}
-GrClearOp::GrClearOp(const GrScissorState& scissor, const SkPMColor4f& color,
- const GrSurfaceProxy* proxy)
+GrClearOp::GrClearOp(const GrScissorState& scissor, const SkPMColor4f& color)
: INHERITED(ClassID())
, fScissor(scissor)
, fColor(color) {
- this->setBounds(scissor.enabled() ? SkRect::Make(scissor.rect()) : proxy->getBoundsRect(),
- HasAABloat::kNo, IsHairline::kNo);
+ this->setBounds(SkRect::Make(scissor.rect()), HasAABloat::kNo, IsHairline::kNo);
}
void GrClearOp::onExecute(GrOpFlushState* state, const SkRect& chainBounds) {
diff --git a/src/gpu/ops/GrClearOp.h b/src/gpu/ops/GrClearOp.h
index 770a7de..ad50c7d 100644
--- a/src/gpu/ops/GrClearOp.h
+++ b/src/gpu/ops/GrClearOp.h
@@ -18,10 +18,10 @@
public:
DEFINE_OP_CLASS_ID
+ // A fullscreen or scissored clear, depending on the clip and proxy dimensions
static std::unique_ptr<GrClearOp> Make(GrRecordingContext* context,
const GrScissorState& scissor,
- const SkPMColor4f& color,
- const GrSurfaceProxy* dstProxy);
+ const SkPMColor4f& color);
const char* name() const override { return "Clear"; }
@@ -41,13 +41,10 @@
}
#endif
- const SkPMColor4f& color() const { return fColor; }
- void setColor(const SkPMColor4f& color) { fColor = color; }
-
private:
friend class GrOpMemoryPool; // for ctors
- GrClearOp(const GrScissorState& scissor, const SkPMColor4f& color, const GrSurfaceProxy* proxy);
+ GrClearOp(const GrScissorState& scissor, const SkPMColor4f& color);
CombineResult onCombineIfPossible(GrOp* t, GrRecordingContext::Arenas*,
const GrCaps& caps) override {
diff --git a/src/gpu/ops/GrClearStencilClipOp.cpp b/src/gpu/ops/GrClearStencilClipOp.cpp
index dc26471..4fc0da5 100644
--- a/src/gpu/ops/GrClearStencilClipOp.cpp
+++ b/src/gpu/ops/GrClearStencilClipOp.cpp
@@ -15,11 +15,10 @@
std::unique_ptr<GrOp> GrClearStencilClipOp::Make(GrRecordingContext* context,
const GrScissorState& scissor,
- bool insideStencilMask,
- GrRenderTargetProxy* proxy) {
+ bool insideStencilMask) {
GrOpMemoryPool* pool = context->priv().opMemoryPool();
- return pool->allocate<GrClearStencilClipOp>(scissor, insideStencilMask, proxy);
+ return pool->allocate<GrClearStencilClipOp>(scissor, insideStencilMask);
}
void GrClearStencilClipOp::onExecute(GrOpFlushState* state, const SkRect& chainBounds) {
diff --git a/src/gpu/ops/GrClearStencilClipOp.h b/src/gpu/ops/GrClearStencilClipOp.h
index f185cde..69d457e 100644
--- a/src/gpu/ops/GrClearStencilClipOp.h
+++ b/src/gpu/ops/GrClearStencilClipOp.h
@@ -21,8 +21,7 @@
static std::unique_ptr<GrOp> Make(GrRecordingContext* context,
const GrScissorState& scissor,
- bool insideStencilMask,
- GrRenderTargetProxy* proxy);
+ bool insideStencilMask);
const char* name() const override { return "ClearStencilClip"; }
@@ -44,14 +43,11 @@
private:
friend class GrOpMemoryPool; // for ctor
- GrClearStencilClipOp(const GrScissorState& scissor, bool insideStencilMask,
- GrRenderTargetProxy* proxy)
+ GrClearStencilClipOp(const GrScissorState& scissor, bool insideStencilMask)
: INHERITED(ClassID())
, fScissor(scissor)
, fInsideStencilMask(insideStencilMask) {
- const SkRect& bounds =
- fScissor.enabled() ? SkRect::Make(fScissor.rect()) : proxy->getBoundsRect();
- this->setBounds(bounds, HasAABloat::kNo, IsHairline::kNo);
+ this->setBounds(SkRect::Make(scissor.rect()), HasAABloat::kNo, IsHairline::kNo);
}
void onPrePrepare(GrRecordingContext*,
diff --git a/src/gpu/ops/GrFillRectOp.cpp b/src/gpu/ops/GrFillRectOp.cpp
index 645a0f7..5d20400 100644
--- a/src/gpu/ops/GrFillRectOp.cpp
+++ b/src/gpu/ops/GrFillRectOp.cpp
@@ -250,7 +250,7 @@
SkArenaAlloc* arena = context->priv().recordTimeAllocator();
// This is equivalent to a GrOpFlushState::detachAppliedClip
- GrAppliedClip appliedClip = clip ? std::move(*clip) : GrAppliedClip();
+ GrAppliedClip appliedClip = clip ? std::move(*clip) : GrAppliedClip::Disabled();
this->createProgramInfo(context->priv().caps(), arena, writeView,
std::move(appliedClip), dstProxyView);
diff --git a/src/gpu/ops/GrMeshDrawOp.cpp b/src/gpu/ops/GrMeshDrawOp.cpp
index f5a8c04..809620b 100644
--- a/src/gpu/ops/GrMeshDrawOp.cpp
+++ b/src/gpu/ops/GrMeshDrawOp.cpp
@@ -33,7 +33,7 @@
SkArenaAlloc* arena = context->priv().recordTimeAllocator();
// This is equivalent to a GrOpFlushState::detachAppliedClip
- GrAppliedClip appliedClip = clip ? std::move(*clip) : GrAppliedClip();
+ GrAppliedClip appliedClip = clip ? std::move(*clip) : GrAppliedClip::Disabled();
this->createProgramInfo(context->priv().caps(), arena, writeView,
std::move(appliedClip), dstProxyView);
diff --git a/src/gpu/ops/GrStencilAndCoverPathRenderer.cpp b/src/gpu/ops/GrStencilAndCoverPathRenderer.cpp
index 61f6a11..fb6e45b 100644
--- a/src/gpu/ops/GrStencilAndCoverPathRenderer.cpp
+++ b/src/gpu/ops/GrStencilAndCoverPathRenderer.cpp
@@ -125,15 +125,16 @@
args.fRenderTargetContext->height()); // Inverse fill.
// fake inverse with a stencil and cover
- GrAppliedClip appliedClip;
+ GrAppliedClip appliedClip(args.fRenderTargetContext->dimensions());
if (args.fClip && !args.fClip->apply(
args.fContext, args.fRenderTargetContext, doStencilMSAA, true, &appliedClip,
&devBounds)) {
return true;
}
- GrStencilClip stencilClip(appliedClip.stencilStackID());
+ GrStencilClip stencilClip(args.fRenderTargetContext->dimensions(),
+ appliedClip.stencilStackID());
if (appliedClip.scissorState().enabled()) {
- stencilClip.fixedClip().setScissor(appliedClip.scissorState().rect());
+ SkAssertResult(stencilClip.fixedClip().setScissor(appliedClip.scissorState().rect()));
}
if (appliedClip.windowRectsState().enabled()) {
stencilClip.fixedClip().setWindowRectangles(appliedClip.windowRectsState().windows(),
diff --git a/tests/OnFlushCallbackTest.cpp b/tests/OnFlushCallbackTest.cpp
index e817b4a..b7157c0 100644
--- a/tests/OnFlushCallbackTest.cpp
+++ b/tests/OnFlushCallbackTest.cpp
@@ -84,7 +84,7 @@
GrProcessorAnalysisColor gpColor;
gpColor.setToUnknown();
// We ignore the clip so pass this rather than the GrAppliedClip param.
- static GrAppliedClip kNoClip;
+ static GrAppliedClip kNoClip = GrAppliedClip::Disabled();
return fHelper.finalizeProcessors(caps, &kNoClip, hasMixedSampledCoverage, clampType,
GrProcessorAnalysisCoverage::kNone, &gpColor);
}
