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>

gm/clockwise.cpp

@@ -188,7 +188,7 @@ private:
         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);

gm/fwidth_squircle.cpp

@@ -197,7 +197,7 @@ private:
         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);

gm/samplelocations.cpp

@@ -274,7 +274,7 @@ private:
         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);

gm/windowrectangles.cpp

@@ -193,13 +193,12 @@ private:
 /**
  * Makes a clip object that enforces the stencil clip bit. Used to visualize the stencil mask.
  */
-static const GrStencilClip* make_stencil_only_clip() {
+static GrStencilClip make_stencil_only_clip(GrRenderTargetContext* rtc) {
-    static const GrStencilClip kClip(SkClipStack::kEmptyGenID);
+    return GrStencilClip(rtc->dimensions(), SkClipStack::kEmptyGenID);
-    return &kClip;
+};
-}
 
 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 @@ void WindowRectanglesMaskGM::visualizeAlphaMask(GrContext* ctx, GrRenderTargetCo
     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 @@ void WindowRectanglesMaskGM::visualizeAlphaMask(GrContext* ctx, GrRenderTargetCo
     // 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);
+    AlphaOnlyClip alphaClip(maskRTC->readSurfaceView(), x, y);
-    rtc->drawRect(&clip, std::move(paint), GrAA::kYes, SkMatrix::I(),
+    rtc->drawRect(&alphaClip, std::move(paint), GrAA::kYes, SkMatrix::I(),
                   SkRect::Make(SkIRect::MakeXYWH(x, y, maskRTC->width(), maskRTC->height())));
 }
 
@@ -275,7 +275,8 @@ void WindowRectanglesMaskGM::visualizeStencilMask(GrContext* ctx, GrRenderTarget
     // 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) {

src/gpu/GrAppliedClip.h

@@ -21,12 +21,12 @@
  */
 class GrAppliedHardClip {
 public:
-    static const GrAppliedHardClip& Disabled() {
+    GrAppliedHardClip(const SkISize& rtDims) : fScissorState(rtDims) {}
-        static GrAppliedHardClip kDisabled;
+    GrAppliedHardClip(const SkISize& logicalRTDims, const SkISize& backingStoreDims)
-        return kDisabled;
+            : fScissorState(backingStoreDims) {
+        fScissorState.set(SkIRect::MakeSize(logicalRTDims));
     }
 
-    GrAppliedHardClip() = default;
     GrAppliedHardClip(GrAppliedHardClip&& that) = default;
     GrAppliedHardClip(const GrAppliedHardClip&) = delete;
 
@@ -81,7 +81,17 @@ private:
  */
 class GrAppliedClip {
 public:
-    GrAppliedClip() = default;
+    static GrAppliedClip Disabled() {
+        // 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.
+        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;
 

src/gpu/GrBlurUtils.cpp

@@ -200,8 +200,7 @@ static std::unique_ptr<GrRenderTargetContext> create_mask_GPU(GrRecordingContext
     maskPaint.setCoverageSetOpXPFactory(SkRegion::kReplace_Op);
 
     // setup new clip
-    const SkIRect clipRect = SkIRect::MakeWH(maskRect.width(), maskRect.height());
+    GrFixedClip clip(rtContext->dimensions(), SkIRect::MakeWH(maskRect.width(), maskRect.height()));
-    GrFixedClip clip(clipRect);
 
     // Draw the mask into maskTexture with the path's integerized top-left at the origin using
     // maskPaint.

src/gpu/GrFixedClip.h

@@ -17,17 +17,21 @@
  */
 class GrFixedClip final : public GrHardClip {
 public:
-    GrFixedClip() = default;
+    explicit GrFixedClip(const SkISize& rtDims) : fScissorState(rtDims) {}
-    explicit GrFixedClip(const SkIRect& scissorRect) : fScissorState(scissorRect) {}
+    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) {
+    bool SK_WARN_UNUSED_RESULT setScissor(const SkIRect& irect) {
-        fScissorState.set(irect);
+        return fScissorState.set(irect);
     }
     bool SK_WARN_UNUSED_RESULT intersect(const SkIRect& irect) {
         return fScissorState.intersect(irect);

src/gpu/GrOpFlushState.cpp

@@ -194,7 +194,7 @@ void GrOpFlushState::putBackVertices(int vertices, size_t vertexStride) {
 }
 
 GrAppliedClip GrOpFlushState::detachAppliedClip() {
-    return fOpArgs->appliedClip() ? std::move(*fOpArgs->appliedClip()) : GrAppliedClip();
+    return fOpArgs->appliedClip() ? std::move(*fOpArgs->appliedClip()) : GrAppliedClip::Disabled();
 }
 
 GrStrikeCache* GrOpFlushState::strikeCache() const {

src/gpu/GrOpFlushState.h

@@ -145,7 +145,7 @@ public:
     const GrAppliedClip* appliedClip() const final { return this->drawOpArgs().appliedClip(); }
     const GrAppliedHardClip& appliedHardClip() const {
         return (fOpArgs->appliedClip()) ?
-                fOpArgs->appliedClip()->hardClip() : GrAppliedHardClip::Disabled();
+                fOpArgs->appliedClip()->hardClip() : GrAppliedClip::Disabled().hardClip();
     }
     GrAppliedClip detachAppliedClip() final;
     const GrXferProcessor::DstProxyView& dstProxyView() const final {

src/gpu/GrOpsTask.cpp

@@ -613,7 +613,7 @@ void GrOpsTask::setColorLoadOp(GrLoadOp op, const SkPMColor4f& color) {
     if (GrLoadOp::kClear == fColorLoadOp) {
         GrSurfaceProxy* proxy = fTargetView.proxy();
         SkASSERT(proxy);
-        fTotalBounds = proxy->getBoundsRect();
+        fTotalBounds = proxy->backingStoreBoundsRect();
     }
 }
 
@@ -894,7 +894,9 @@ GrRenderTask::ExpectedOutcome GrOpsTask::onMakeClosed(
     });
     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)) {

src/gpu/GrReducedClip.cpp

@@ -742,7 +742,7 @@ static void draw_element(GrRenderTargetContext* rtc,
 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()),

src/gpu/GrRenderTargetContext.cpp

@@ -500,30 +500,35 @@ void GrRenderTargetContext::internalClear(const SkIRect* scissor,
     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
+    // There are three ways clears are handled: load ops, native clears, and draws. Load ops are
-    // the logical bounds of the render target, or if the special flag was provided that allows
+    // only for fullscreen clears; native clears can be fullscreen or with scissors if the backend
-    // partial clears to upgrade to full (because it's a scratch resource and the caller knows
+    // supports then. Drawing an axis-aligned rect is the fallback path.
-    // anything outside the scissor doesn't matter, but if full screen clears aren't free, then
+    GrScissorState scissorState(this->asSurfaceProxy()->backingStoreDimensions());
-    // the scissor is still provided so that fewer pixels are written to).
+    if (scissor && !scissorState.set(*scissor)) {
-    // TODO: wrt the shouldInitializeTextures path, it would be more performant to
+        // The clear is offscreen, so skip it (normally this would be handled by addDrawOp,
-    // only clear the entire target if we knew it had not been cleared before. As
+        // except clear ops are not draw ops).
-    // is this could end up doing a lot of redundant clears.
+        return;
-    GrScissorState scissorState;
+    }
-    if (scissor) {
+
-        // TODO(michaelludwig) - This will get simpler when GrScissorState knows the device dims
+    // If we have a scissor but it's okay to clear beyond it for performance reasons, then disable
-        scissorState.set(*scissor);
+    // the test. We only do this when the clear would be handled by a load op or natively.
-        if (!scissorState.intersect(SkIRect::MakeWH(this->width(), this->height()))) {
+    if (scissorState.enabled() && !this->caps()->performColorClearsAsDraws()) {
-            // The clear is offscreen, so skip it (normally this would be handled by addDrawOp,
+        if (upgradePartialToFull && (this->caps()->preferFullscreenClears() ||
-            // except clear ops are not draw ops).
+                                     this->caps()->shouldInitializeTextures())) {
-            return;
+            // 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 @@ void GrRenderTargetContext::internalClear(const SkIRect* scissor,
             // 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
+    // At this point we are either a partial clear or a fullscreen clear that couldn't be applied
-        // clear load op isn't supported)
+    // as a load op.
-        if (this->caps()->performColorClearsAsDraws()) {
+    bool clearAsDraw = this->caps()->performColorClearsAsDraws() ||
-            SkRect rtRect = SkRect::MakeWH(this->width(), this->height());
+                       (scissorState.enabled() && this->caps()->performPartialClearsAsDraws());
-            GrPaint paint;
+    if (clearAsDraw) {
-            clear_to_grpaint(color, &paint);
+        GrPaint paint;
-            this->addDrawOp(nullptr,
+        clear_to_grpaint(color, &paint);
-                            GrFillRectOp::MakeNonAARect(fContext, std::move(paint), SkMatrix::I(),
+        this->addDrawOp(nullptr,
-                                                        rtRect));
+                        GrFillRectOp::MakeNonAARect(fContext, std::move(paint), SkMatrix::I(),
-        } else {
+                                                    SkRect::Make(scissorState.rect())));
-            this->addOp(GrClearOp::Make(fContext, GrScissorState(), color, this->asSurfaceProxy()));
-        }
     } else {
-        if (this->caps()->performPartialClearsAsDraws()) {
+        this->addOp(GrClearOp::Make(fContext, scissorState, color));
-            // 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));
-        }
     }
 }
 
@@ -641,16 +628,9 @@ GrRenderTargetContext::QuadOptimization GrRenderTargetContext::attemptQuadOptimi
     // better to just keep the old flags instead of introducing mixed edge flags.
     GrQuadAAFlags oldFlags = quad->fEdgeFlags;
 
-    SkRect rtRect;
+    // Use the logical size of the render target, which allows for "fullscreen" clears even if
-    if (stencilSettings) {
+    // the render target has an approximate backing fit
-        // Must use size at which the rendertarget will ultimately be allocated so that stencil
+    SkRect rtRect = this->asSurfaceProxy()->getBoundsRect();
-        // 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());
-    }
 
     SkRect drawBounds = quad->fDevice.bounds();
     if (constColor) {
@@ -961,30 +941,25 @@ void GrRenderTargetContext::setNeedsStencil(bool useMixedSamplesIfNotMSAA) {
 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),
+        this->addDrawOp(nullptr,
-                                                             SkMatrix::I(), rect, ss));
+                        GrFillRectOp::MakeNonAARect(fContext, std::move(paint), SkMatrix::I(),
+                                                    SkRect::Make(scissorState.rect()), ss));
     } else {
-        GrScissorState scissorState;
+        this->addOp(GrClearStencilClipOp::Make(fContext, scissorState, insideStencilMask));
-        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));
     }
 }
 
@@ -1009,7 +984,9 @@ void GrRenderTargetContextPriv::stencilPath(const GrHardClip* clip,
     // 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 @@ void GrRenderTargetContext::addDrawOp(const GrClip* clip, std::unique_ptr<GrDraw
     // 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;

src/gpu/GrRenderTargetProxy.h

@@ -60,7 +60,7 @@ public:
     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);

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) {}
+    // The disabled scissor state for a render target of the given size.
-    GrScissorState(const SkIRect& rect) : fEnabled(true), fRect(rect) {}
+    explicit GrScissorState(const SkISize& rtDims)
-    void setDisabled() { fEnabled = false; }
+            : fRTSize(rtDims)
-    void set(const SkIRect& rect) { fRect = rect; fEnabled = true; }
+            , 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) {
+        if (!fRect.intersect(rect)) {
-            this->set(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 &&
+        return fRTSize == other.fRTSize && fRect == other.fRect;
-                (false == fEnabled || 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;
 };
 

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)
+    GrStencilClip(const SkISize& rtDims, const SkIRect& scissorRect,
-        : fFixedClip(scissorRect)
+                  uint32_t stencilStackID = SK_InvalidGenID)
-        , fStencilStackID(stencilStackID) {
+            : fFixedClip(rtDims, scissorRect)
-    }
+            , fStencilStackID(stencilStackID) {}
 
     const GrFixedClip& fixedClip() const { return fFixedClip; }
     GrFixedClip& fixedClip() { return fFixedClip; }

src/gpu/GrStencilMaskHelper.cpp

@@ -334,7 +334,8 @@ bool GrStencilMaskHelper::init(const SkIRect& bounds, uint32_t genID,
     }
 
     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);

src/gpu/GrStencilMaskHelper.h

@@ -39,7 +39,8 @@ public:
     // 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,

src/gpu/ccpr/GrCCPerFlushResources.cpp

@@ -96,8 +96,10 @@ public:
         GrCCPathProcessor pathProc(coverageMode, fSrcProxy->peekTexture(), swizzle,
                                    GrCCAtlas::kTextureOrigin);
 
-        GrPipeline pipeline(GrScissorTest::kDisabled, SkBlendMode::kSrc,
+        bool hasScissor = flushState->appliedClip() &&
-                            flushState->drawOpArgs().writeSwizzle());
+                          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());

src/gpu/d3d/GrD3DOpsRenderPass.cpp

@@ -51,7 +51,8 @@ void GrD3DOpsRenderPass::onBegin() {
     // TODO: set stencil too
 
     if (GrLoadOp::kClear == fColorLoadOp) {
-        fGpu->currentCommandList()->clearRenderTargetView(d3dRT, fClearColor, GrScissorState());
+        fGpu->currentCommandList()->clearRenderTargetView(
+                d3dRT, fClearColor, GrScissorState(fRenderTarget->dimensions()));
     }
 }
 

src/gpu/gl/GrGLGpu.cpp

@@ -2260,8 +2260,9 @@ void GrGLGpu::onResolveRenderTarget(GrRenderTarget* target, const SkIRect& resol
         // 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(),
+        GrScissorState scissor(rt->dimensions());
-                           kTopLeft_GrSurfaceOrigin);
+        SkAssertResult(scissor.set(resolveRect));
+        this->flushScissor(scissor, rt->width(), rt->height(), kTopLeft_GrSurfaceOrigin);
         this->disableWindowRectangles();
         GL_CALL(ResolveMultisampleFramebuffer());
     } else {

src/gpu/ops/GrAAHairLinePathRenderer.cpp

@@ -1117,7 +1117,7 @@ void AAHairlineOp::onPrePrepareDraws(GrRecordingContext* context,
     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);

src/gpu/ops/GrClearOp.cpp

@@ -16,24 +16,16 @@
 
 std::unique_ptr<GrClearOp> GrClearOp::Make(GrRecordingContext* context,
                                            const GrScissorState& scissor,
-                                           const SkPMColor4f& color,
+                                           const SkPMColor4f& color) {
-                                           const GrSurfaceProxy* dstProxy) {
-    const SkIRect rect = SkIRect::MakeSize(dstProxy->dimensions());
-    if (scissor.enabled() && !SkIRect::Intersects(scissor.rect(), rect)) {
-        return nullptr;
-    }
-
     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,
+GrClearOp::GrClearOp(const GrScissorState& scissor, const SkPMColor4f& color)
-                     const GrSurfaceProxy* proxy)
         : INHERITED(ClassID())
         , fScissor(scissor)
         , fColor(color) {
-    this->setBounds(scissor.enabled() ? SkRect::Make(scissor.rect()) : proxy->getBoundsRect(),
+    this->setBounds(SkRect::Make(scissor.rect()), HasAABloat::kNo, IsHairline::kNo);
-                    HasAABloat::kNo, IsHairline::kNo);
 }
 
 void GrClearOp::onExecute(GrOpFlushState* state, const SkRect& chainBounds) {

src/gpu/ops/GrClearOp.h

@@ -18,10 +18,10 @@ class GrClearOp final : public GrOp {
 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 SkPMColor4f& color);
-                                           const GrSurfaceProxy* dstProxy);
 
     const char* name() const override { return "Clear"; }
 
@@ -41,13 +41,10 @@ public:
     }
 #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 {

src/gpu/ops/GrClearStencilClipOp.cpp

@@ -15,11 +15,10 @@
 
 std::unique_ptr<GrOp> GrClearStencilClipOp::Make(GrRecordingContext* context,
                                                  const GrScissorState& scissor,
-                                                 bool insideStencilMask,
+                                                 bool insideStencilMask) {
-                                                 GrRenderTargetProxy* proxy) {
     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) {

src/gpu/ops/GrClearStencilClipOp.h

@@ -21,8 +21,7 @@ public:
 
     static std::unique_ptr<GrOp> Make(GrRecordingContext* context,
                                       const GrScissorState& scissor,
-                                      bool insideStencilMask,
+                                      bool insideStencilMask);
-                                      GrRenderTargetProxy* proxy);
 
     const char* name() const override { return "ClearStencilClip"; }
 
@@ -44,14 +43,11 @@ public:
 private:
     friend class GrOpMemoryPool; // for ctor
 
-    GrClearStencilClipOp(const GrScissorState& scissor, bool insideStencilMask,
+    GrClearStencilClipOp(const GrScissorState& scissor, bool insideStencilMask)
-                         GrRenderTargetProxy* proxy)
             : INHERITED(ClassID())
             , fScissor(scissor)
             , fInsideStencilMask(insideStencilMask) {
-        const SkRect& bounds =
+        this->setBounds(SkRect::Make(scissor.rect()), HasAABloat::kNo, IsHairline::kNo);
-                fScissor.enabled() ? SkRect::Make(fScissor.rect()) : proxy->getBoundsRect();
-        this->setBounds(bounds, HasAABloat::kNo, IsHairline::kNo);
     }
 
     void onPrePrepare(GrRecordingContext*,

src/gpu/ops/GrFillRectOp.cpp

@@ -250,7 +250,7 @@ private:
         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);

src/gpu/ops/GrMeshDrawOp.cpp

@@ -33,7 +33,7 @@ void GrMeshDrawOp::onPrePrepareDraws(GrRecordingContext* context,
     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);

src/gpu/ops/GrStencilAndCoverPathRenderer.cpp

@@ -125,15 +125,16 @@ bool GrStencilAndCoverPathRenderer::onDrawPath(const DrawPathArgs& args) {
                                            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(),

tests/OnFlushCallbackTest.cpp

@@ -84,7 +84,7 @@ public:
         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);
     }