ccpr: Clip path octo bounds by the scissor
Finds the (octagonal) intersection of the path's bounding octagon and
the scissor, and draws that octagon instead. This allows us to avoid
ever using the scissor when drawing paths to the main canvas. It will
also let us use that same octagon without scissor when resolving the
stencil buffer to coverage in MSAA mode.
Bug: skia:
Change-Id: Ia7fe60343424bc77532fa9919d3fa108337a5d63
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/212840
Commit-Queue: Chris Dalton <csmartdalton@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
diff --git a/src/gpu/ccpr/GrCCDrawPathsOp.cpp b/src/gpu/ccpr/GrCCDrawPathsOp.cpp
index be5290e..19f854f 100644
--- a/src/gpu/ccpr/GrCCDrawPathsOp.cpp
+++ b/src/gpu/ccpr/GrCCDrawPathsOp.cpp
@@ -108,9 +108,15 @@
paint.getColor4f())
, fProcessors(std::move(paint)) { // Paint must be moved after fetching its color above.
SkDEBUGCODE(fBaseInstance = -1);
- // FIXME: intersect with clip bounds to (hopefully) improve batching.
- // (This is nontrivial due to assumptions in generating the octagon cover geometry.)
- this->setBounds(conservativeDevBounds, GrOp::HasAABloat::kYes, GrOp::IsZeroArea::kNo);
+ // If the path is clipped, CCPR will only draw the visible portion. This helps improve batching,
+ // since it eliminates the need for scissor when drawing to the main canvas.
+ // FIXME: We should parse the path right here. It will provide a tighter bounding box for us to
+ // give the opList, as well as enabling threaded parsing when using DDL.
+ SkRect clippedDrawBounds;
+ if (!clippedDrawBounds.intersect(conservativeDevBounds, SkRect::Make(maskDevIBounds))) {
+ clippedDrawBounds.setEmpty();
+ }
+ this->setBounds(clippedDrawBounds, GrOp::HasAABloat::kYes, GrOp::IsZeroArea::kNo);
}
GrCCDrawPathsOp::~GrCCDrawPathsOp() {
diff --git a/src/gpu/ccpr/GrCCPerFlushResources.cpp b/src/gpu/ccpr/GrCCPerFlushResources.cpp
index ae001ce..c941926 100644
--- a/src/gpu/ccpr/GrCCPerFlushResources.cpp
+++ b/src/gpu/ccpr/GrCCPerFlushResources.cpp
@@ -365,26 +365,33 @@
stroke.getJoin(), stroke.getMiter(), stroke.getCap(), strokeDevWidth);
octoBounds->outset(r);
}
- octoBounds->roundOut(devIBounds);
- GrScissorTest scissorTest;
- SkIRect clippedPathIBounds;
- if (!this->placeRenderedPathInAtlas(clipIBounds, *devIBounds, &scissorTest, &clippedPathIBounds,
- devToAtlasOffset)) {
+ GrScissorTest enableScissorInAtlas;
+ if (clipIBounds.contains(octoBounds->bounds())) {
+ enableScissorInAtlas = GrScissorTest::kDisabled;
+ } else if (octoBounds->clip(clipIBounds)) {
+ enableScissorInAtlas = GrScissorTest::kEnabled;
+ } else {
+ // The clip and octo bounds do not intersect. Draw nothing.
SkDEBUGCODE(--fEndPathInstance);
- return nullptr; // Path was degenerate or clipped away.
+ return nullptr;
}
+ octoBounds->roundOut(devIBounds);
+ SkASSERT(clipIBounds.contains(*devIBounds));
+
+ this->placeRenderedPathInAtlas(*devIBounds, enableScissorInAtlas, devToAtlasOffset);
if (stroke.isFillStyle()) {
SkASSERT(0 == strokeDevWidth);
- fFiller.parseDeviceSpaceFill(path, fLocalDevPtsBuffer.begin(), scissorTest,
- clippedPathIBounds, *devToAtlasOffset);
+ fFiller.parseDeviceSpaceFill(path, fLocalDevPtsBuffer.begin(), enableScissorInAtlas,
+ *devIBounds, *devToAtlasOffset);
} else {
// Stroke-and-fill is not yet supported.
SkASSERT(SkStrokeRec::kStroke_Style == stroke.getStyle() || stroke.isHairlineStyle());
SkASSERT(!stroke.isHairlineStyle() || 1 == strokeDevWidth);
- fStroker.parseDeviceSpaceStroke(path, fLocalDevPtsBuffer.begin(), stroke, strokeDevWidth,
- scissorTest, clippedPathIBounds, *devToAtlasOffset);
+ fStroker.parseDeviceSpaceStroke(
+ path, fLocalDevPtsBuffer.begin(), stroke, strokeDevWidth, enableScissorInAtlas,
+ *devIBounds, *devToAtlasOffset);
}
return &fRenderedAtlasStack.current();
}
@@ -398,41 +405,34 @@
return nullptr;
}
- GrScissorTest scissorTest;
+ GrScissorTest enableScissorInAtlas;
SkIRect clippedPathIBounds;
- if (!this->placeRenderedPathInAtlas(clipIBounds, devPathIBounds, &scissorTest,
- &clippedPathIBounds, devToAtlasOffset)) {
+ if (clipIBounds.contains(devPathIBounds)) {
+ clippedPathIBounds = devPathIBounds;
+ enableScissorInAtlas = GrScissorTest::kDisabled;
+ } else if (clippedPathIBounds.intersect(clipIBounds, devPathIBounds)) {
+ enableScissorInAtlas = GrScissorTest::kEnabled;
+ } else {
+ // The clip and path bounds do not intersect. Draw nothing.
return nullptr;
}
- fFiller.parseDeviceSpaceFill(devPath, SkPathPriv::PointData(devPath), scissorTest,
+ this->placeRenderedPathInAtlas(clippedPathIBounds, enableScissorInAtlas, devToAtlasOffset);
+ fFiller.parseDeviceSpaceFill(devPath, SkPathPriv::PointData(devPath), enableScissorInAtlas,
clippedPathIBounds, *devToAtlasOffset);
return &fRenderedAtlasStack.current();
}
-bool GrCCPerFlushResources::placeRenderedPathInAtlas(const SkIRect& clipIBounds,
- const SkIRect& pathIBounds,
- GrScissorTest* scissorTest,
- SkIRect* clippedPathIBounds,
- SkIVector* devToAtlasOffset) {
- if (clipIBounds.contains(pathIBounds)) {
- *clippedPathIBounds = pathIBounds;
- *scissorTest = GrScissorTest::kDisabled;
- } else if (clippedPathIBounds->intersect(clipIBounds, pathIBounds)) {
- *scissorTest = GrScissorTest::kEnabled;
- } else {
- return false;
- }
-
+void GrCCPerFlushResources::placeRenderedPathInAtlas(
+ const SkIRect& clippedPathIBounds, GrScissorTest scissorTest, SkIVector* devToAtlasOffset) {
if (GrCCAtlas* retiredAtlas =
- fRenderedAtlasStack.addRect(*clippedPathIBounds, devToAtlasOffset)) {
+ fRenderedAtlasStack.addRect(clippedPathIBounds, devToAtlasOffset)) {
// We did not fit in the previous coverage count atlas and it was retired. Close the path
// parser's current batch (which does not yet include the path we just parsed). We will
// render this batch into the retired atlas during finalize().
retiredAtlas->setFillBatchID(fFiller.closeCurrentBatch());
retiredAtlas->setStrokeBatchID(fStroker.closeCurrentBatch());
}
- return true;
}
bool GrCCPerFlushResources::finalize(GrOnFlushResourceProvider* onFlushRP,
diff --git a/src/gpu/ccpr/GrCCPerFlushResources.h b/src/gpu/ccpr/GrCCPerFlushResources.h
index 4fcb114..ad5ff82 100644
--- a/src/gpu/ccpr/GrCCPerFlushResources.h
+++ b/src/gpu/ccpr/GrCCPerFlushResources.h
@@ -122,8 +122,7 @@
private:
void recordCopyPathInstance(const GrCCPathCacheEntry&, const SkIVector& newAtlasOffset,
GrCCPathProcessor::DoEvenOddFill, sk_sp<GrTextureProxy> srcProxy);
- bool placeRenderedPathInAtlas(const SkIRect& clipIBounds, const SkIRect& pathIBounds,
- GrScissorTest*, SkIRect* clippedPathIBounds,
+ void placeRenderedPathInAtlas(const SkIRect& clippedPathIBounds, GrScissorTest,
SkIVector* devToAtlasOffset);
const SkAutoSTArray<32, SkPoint> fLocalDevPtsBuffer;
diff --git a/src/gpu/ccpr/GrOctoBounds.cpp b/src/gpu/ccpr/GrOctoBounds.cpp
index 7baa216..a510fd3 100644
--- a/src/gpu/ccpr/GrOctoBounds.cpp
+++ b/src/gpu/ccpr/GrOctoBounds.cpp
@@ -6,8 +6,115 @@
*/
#include "src/gpu/ccpr/GrOctoBounds.h"
+#include <algorithm>
+
+bool GrOctoBounds::clip(const SkIRect& clipRect) {
+ // Intersect dev bounds with the clip rect.
+ float l = std::max(fBounds.left(), (float)clipRect.left());
+ float t = std::max(fBounds.top(), (float)clipRect.top());
+ float r = std::min(fBounds.right(), (float)clipRect.right());
+ float b = std::min(fBounds.bottom(), (float)clipRect.bottom());
+
+ float l45 = fBounds45.left();
+ float t45 = fBounds45.top();
+ float r45 = fBounds45.right();
+ float b45 = fBounds45.bottom();
+
+ // Check if either the bounds or 45-degree bounds are empty. We write this check as the NOT of
+ // non-empty rects, so we will return false if any values are NaN.
+ if (!(l < r && t < b && l45 < r45 && t45 < b45)) {
+ return false;
+ }
+
+ // Tighten dev bounds around the new (octagonal) intersection that results after clipping. This
+ // may be tighter now even than the clipped bounds, depending on the diagonals. Shader code that
+ // emits octagons expects both bounding boxes to circumcribe the inner octagon, and will fail if
+ // they do not.
+ if (l45 > Get_x45(r,b)) {
+ // Slide the bottom upward until it crosses the l45 diagonal at x=r.
+ // y = x + (y0 - x0)
+ // Substitute: l45 = x0 - y0
+ // y = x - l45
+ b = SkScalarPin(r - l45, t, b);
+ } else if (r45 < Get_x45(r,b)) {
+ // Slide the right side leftward until it crosses the r45 diagonal at y=b.
+ // x = y + (x0 - y0)
+ // Substitute: r45 = x0 - y0
+ // x = y + r45
+ r = SkScalarPin(b + r45, l, r);
+ }
+ if (l45 > Get_x45(l,t)) {
+ // Slide the left side rightward until it crosses the l45 diagonal at y=t.
+ // x = y + (x0 - y0)
+ // Substitute: l45 = x0 - y0
+ // x = y + l45
+ l = SkScalarPin(t + l45, l, r);
+ } else if (r45 < Get_x45(l,t)) {
+ // Slide the top downward until it crosses the r45 diagonal at x=l.
+ // y = x + (y0 - x0)
+ // Substitute: r45 = x0 - y0
+ // y = x - r45
+ t = SkScalarPin(l - r45, t, b);
+ }
+ if (t45 > Get_y45(l,b)) {
+ // Slide the left side rightward until it crosses the t45 diagonal at y=b.
+ // x = -y + (x0 + y0)
+ // Substitute: t45 = x0 + y0
+ // x = -y + t45
+ l = SkScalarPin(t45 - b, l, r);
+ } else if (b45 < Get_y45(l,b)) {
+ // Slide the bottom upward until it crosses the b45 diagonal at x=l.
+ // y = -x + (y0 + x0)
+ // Substitute: b45 = x0 + y0
+ // y = -x + b45
+ b = SkScalarPin(b45 - l, t, b);
+ }
+ if (t45 > Get_y45(r,t)) {
+ // Slide the top downward until it crosses the t45 diagonal at x=r.
+ // y = -x + (y0 + x0)
+ // Substitute: t45 = x0 + y0
+ // y = -x + t45
+ t = SkScalarPin(t45 - r, t, b);
+ } else if (b45 < Get_y45(r,t)) {
+ // Slide the right side leftward until it crosses the b45 diagonal at y=t.
+ // x = -y + (x0 + y0)
+ // Substitute: b45 = x0 + y0
+ // x = -y + b45
+ r = SkScalarPin(b45 - t, l, r);
+ }
+
+ // Tighten the 45-degree bounding box. Since the dev bounds are now fully tightened, we only
+ // have to clamp the diagonals to outer corners.
+ // NOTE: This will not cause l,t,r,b to need more insetting. We only ever change a diagonal by
+ // pinning it to a FAR corner, which, by definition, is still outside the other corners.
+ l45 = SkScalarPin(Get_x45(l,b), l45, r45);
+ t45 = SkScalarPin(Get_y45(l,t), t45, b45);
+ r45 = SkScalarPin(Get_x45(r,t), l45, r45);
+ b45 = SkScalarPin(Get_y45(r,b), t45, b45);
+
+ // Make one final check for empty or NaN bounds. If the dev bounds were clipped completely
+ // outside one of the diagonals, they will have been pinned to empty. It's also possible that
+ // some Infs crept in and turned into NaNs.
+ if (!(l < r && t < b && l45 < r45 && t45 < b45)) {
+ return false;
+ }
+
+ fBounds.setLTRB(l, t, r, b);
+ fBounds45.setLTRB(l45, t45, r45, b45);
#ifdef SK_DEBUG
+ // Verify dev bounds are inside the clip rect.
+ SkASSERT(l >= (float)clipRect.left());
+ SkASSERT(t >= (float)clipRect.top());
+ SkASSERT(r <= (float)clipRect.right());
+ SkASSERT(b <= (float)clipRect.bottom());
+ this->validateBoundsAreTight();
+#endif
+
+ return true;
+}
+
+#if defined(SK_DEBUG) || defined(GR_TEST_UTILS)
void GrOctoBounds::validateBoundsAreTight() const {
this->validateBoundsAreTight([](bool cond, const char* file, int line, const char* code) {
SkASSERTF(cond, "%s(%d): assertion failure: \"assert(%s)\"", file, line, code);
diff --git a/src/gpu/ccpr/GrOctoBounds.h b/src/gpu/ccpr/GrOctoBounds.h
index 7630618..d2272ae 100644
--- a/src/gpu/ccpr/GrOctoBounds.h
+++ b/src/gpu/ccpr/GrOctoBounds.h
@@ -26,12 +26,22 @@
*/
class GrOctoBounds {
public:
+ GrOctoBounds() = default;
+ GrOctoBounds(const SkRect& bounds, const SkRect& bounds45) {
+ this->set(bounds, bounds45);
+ }
+
void set(const SkRect& bounds, const SkRect& bounds45) {
fBounds = bounds;
fBounds45 = bounds45;
SkDEBUGCODE(this->validateBoundsAreTight());
}
+ bool operator==(const GrOctoBounds& that) const {
+ return fBounds == that.fBounds && fBounds45 == that.fBounds45;
+ }
+ bool operator!=(const GrOctoBounds& that) const { return !(*this == that); }
+
const SkRect& bounds() const { return fBounds; }
float left() const { return fBounds.left(); }
float top() const { return fBounds.top(); }
@@ -72,6 +82,13 @@
SkDEBUGCODE(this->validateBoundsAreTight());
}
+ // Clips the octo bounds by a clip rect and ensures the resulting bounds are fully tightened.
+ // Returns false if the octagon and clipRect do not intersect at all.
+ //
+ // NOTE: Does not perform a trivial containment test before the clip routine. It is probably a
+ // good idea to not call this method if 'this->bounds()' are fully contained within 'clipRect'.
+ bool SK_WARN_UNUSED_RESULT clip(const SkIRect& clipRect);
+
// The 45-degree bounding box resides in "| 1 -1 | * coords" space.
// | 1 1 |
//
@@ -84,7 +101,7 @@
constexpr static float Get_x(float x45, float y45) { return (x45 + y45) * .5f; }
constexpr static float Get_y(float x45, float y45) { return (y45 - x45) * .5f; }
-#ifdef SK_DEBUG
+#if defined(SK_DEBUG) || defined(GR_TEST_UTILS)
void validateBoundsAreTight() const;
void validateBoundsAreTight(const std::function<void(
bool cond, const char* file, int line, const char* code)>& validateFn) const;