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/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;
};