From c002d5619e614d30c2570746356a2d8dda2228f6 Mon Sep 17 00:00:00 2001 From: Michael Ludwig Date: Wed, 13 May 2020 14:17:57 -0400 Subject: [PATCH] Simplify GrClip::getConservativeBounds() signature It turns out no one was using the intersection of rect functionality on GrClip, and this helps simplify what the new clip stack needs to define. Bug: skia:10205 Change-Id: If85a0c744dd68a8ad2f380b54a539ac74850e4ac Reviewed-on: https://skia-review.googlesource.com/c/skia/+/289440 Commit-Queue: Michael Ludwig Reviewed-by: Brian Salomon --- gm/windowrectangles.cpp | 6 --- src/gpu/GrBlurUtils.cpp | 5 +-- src/gpu/GrClip.h | 18 ++++----- src/gpu/GrClipStackClip.cpp | 18 ++++----- src/gpu/GrClipStackClip.h | 3 +- src/gpu/GrFixedClip.cpp | 12 +++--- src/gpu/GrFixedClip.h | 2 +- src/gpu/GrRenderTargetContext.cpp | 16 +++----- src/gpu/GrSoftwarePathRenderer.cpp | 5 +-- src/gpu/GrStencilClip.h | 4 +- src/gpu/SkGpuDevice_drawTexture.cpp | 39 +++++++++---------- .../ccpr/GrCoverageCountingPathRenderer.cpp | 3 +- src/gpu/ops/GrAAHairLinePathRenderer.cpp | 6 +-- src/gpu/ops/GrTriangulatingPathRenderer.cpp | 6 +-- tests/ClipBoundsTest.cpp | 6 +-- tests/GrCCPRTest.cpp | 7 +--- tests/LazyProxyTest.cpp | 6 --- 17 files changed, 60 insertions(+), 102 deletions(-) diff --git a/gm/windowrectangles.cpp b/gm/windowrectangles.cpp index 846af6bd03..d7714ff0e3 100644 --- a/gm/windowrectangles.cpp +++ b/gm/windowrectangles.cpp @@ -162,12 +162,6 @@ class MaskOnlyClipBase : public GrClip { private: bool quickContains(const SkRect&) const final { return false; } bool isRRect(const SkRect& rtBounds, SkRRect* rr, GrAA*) const final { return false; } - void getConservativeBounds(int width, int height, SkIRect* rect, bool* iior) const final { - rect->setWH(width, height); - if (iior) { - *iior = false; - } - } }; /** diff --git a/src/gpu/GrBlurUtils.cpp b/src/gpu/GrBlurUtils.cpp index abd37e453a..66c145da00 100644 --- a/src/gpu/GrBlurUtils.cpp +++ b/src/gpu/GrBlurUtils.cpp @@ -245,9 +245,8 @@ static bool get_shape_and_clip_bounds(GrRenderTargetContext* renderTargetContext SkIRect* unclippedDevShapeBounds, SkIRect* devClipBounds) { // compute bounds as intersection of rt size, clip, and path - clip.getConservativeBounds(renderTargetContext->width(), - renderTargetContext->height(), - devClipBounds); + *devClipBounds = clip.getConservativeBounds(renderTargetContext->width(), + renderTargetContext->height()); if (!get_unclipped_shape_dev_bounds(shape, matrix, unclippedDevShapeBounds)) { *unclippedDevShapeBounds = SkIRect::MakeEmpty(); diff --git a/src/gpu/GrClip.h b/src/gpu/GrClip.h index cc73c641fd..d85338b375 100644 --- a/src/gpu/GrClip.h +++ b/src/gpu/GrClip.h @@ -25,8 +25,15 @@ public: virtual bool quickContains(const SkRRect& rrect) const { return this->quickContains(rrect.getBounds()); } - virtual void getConservativeBounds(int width, int height, SkIRect* devResult, - bool* isIntersectionOfRects = nullptr) const = 0; + /** + * Compute a conservative pixel bounds restricted to the given render target dimensions. + * The returned bounds represent the limits of pixels that can be drawn; anything outside of the + * bounds will be entirely clipped out. + */ + virtual SkIRect getConservativeBounds(int width, int height) const { + return SkIRect::MakeWH(width, height); + } + /** * This computes a GrAppliedClip from the clip which in turn can be used to build a GrPipeline. * To determine the appropriate clipping implementation the GrClip subclass must know whether @@ -160,13 +167,6 @@ class GrNoClip final : public GrHardClip { private: bool quickContains(const SkRect&) const final { return true; } bool quickContains(const SkRRect&) const final { return true; } - void getConservativeBounds(int width, int height, SkIRect* devResult, - bool* isIntersectionOfRects) const final { - devResult->setXYWH(0, 0, width, height); - if (isIntersectionOfRects) { - *isIntersectionOfRects = true; - } - } bool apply(int rtWidth, int rtHeight, GrAppliedHardClip*, SkRect*) const final { return true; } bool isRRect(const SkRect&, SkRRect*, GrAA*) const override { return false; } }; diff --git a/src/gpu/GrClipStackClip.cpp b/src/gpu/GrClipStackClip.cpp index 4fa6246584..37121fd858 100644 --- a/src/gpu/GrClipStackClip.cpp +++ b/src/gpu/GrClipStackClip.cpp @@ -62,18 +62,14 @@ bool GrClipStackClip::isRRect(const SkRect& origRTBounds, SkRRect* rr, GrAA* aa) return false; } -void GrClipStackClip::getConservativeBounds(int width, int height, SkIRect* devResult, - bool* isIntersectionOfRects) const { - if (!fStack) { - devResult->setXYWH(0, 0, width, height); - if (isIntersectionOfRects) { - *isIntersectionOfRects = true; - } - return; +SkIRect GrClipStackClip::getConservativeBounds(int width, int height) const { + if (fStack) { + SkRect devBounds; + fStack->getConservativeBounds(0, 0, width, height, &devBounds); + return devBounds.roundOut(); + } else { + return this->GrClip::getConservativeBounds(width, height); } - SkRect devBounds; - fStack->getConservativeBounds(0, 0, width, height, &devBounds, isIntersectionOfRects); - devBounds.roundOut(devResult); } //////////////////////////////////////////////////////////////////////////////// diff --git a/src/gpu/GrClipStackClip.h b/src/gpu/GrClipStackClip.h index 40ca7d6f22..90ca8676fc 100644 --- a/src/gpu/GrClipStackClip.h +++ b/src/gpu/GrClipStackClip.h @@ -26,8 +26,7 @@ public: bool quickContains(const SkRect&) const final; bool quickContains(const SkRRect&) const final; - void getConservativeBounds(int width, int height, SkIRect* devResult, - bool* isIntersectionOfRects) const final; + SkIRect getConservativeBounds(int width, int height) const final; bool apply(GrRecordingContext*, GrRenderTargetContext*, bool useHWAA, bool hasUserStencilSettings, GrAppliedClip* out, SkRect* bounds) const final; diff --git a/src/gpu/GrFixedClip.cpp b/src/gpu/GrFixedClip.cpp index 2977d9d72b..51f2f9d7bd 100644 --- a/src/gpu/GrFixedClip.cpp +++ b/src/gpu/GrFixedClip.cpp @@ -17,16 +17,14 @@ bool GrFixedClip::quickContains(const SkRect& rect) const { return !fScissorState.enabled() || GrClip::IsInsideClip(fScissorState.rect(), rect); } -void GrFixedClip::getConservativeBounds(int w, int h, SkIRect* devResult, bool* iior) const { - devResult->setXYWH(0, 0, w, h); +SkIRect GrFixedClip::getConservativeBounds(int w, int h) const { + SkIRect devResult = this->GrClip::getConservativeBounds(w, h); if (fScissorState.enabled()) { - if (!devResult->intersect(fScissorState.rect())) { - devResult->setEmpty(); + if (!devResult.intersect(fScissorState.rect())) { + devResult.setEmpty(); } } - if (iior) { - *iior = true; - } + return devResult; } bool GrFixedClip::isRRect(const SkRect& rtBounds, SkRRect* rr, GrAA* aa) const { diff --git a/src/gpu/GrFixedClip.h b/src/gpu/GrFixedClip.h index 087617eacd..1f175f83be 100644 --- a/src/gpu/GrFixedClip.h +++ b/src/gpu/GrFixedClip.h @@ -43,7 +43,7 @@ public: } bool quickContains(const SkRect&) const override; - void getConservativeBounds(int w, int h, SkIRect* devResult, bool* iior) const override; + SkIRect getConservativeBounds(int w, int h) const override; bool isRRect(const SkRect& rtBounds, SkRRect* rr, GrAA*) const override; bool apply(int rtWidth, int rtHeight, GrAppliedHardClip*, SkRect*) const override; diff --git a/src/gpu/GrRenderTargetContext.cpp b/src/gpu/GrRenderTargetContext.cpp index c0a38354d8..7a53e76afb 100644 --- a/src/gpu/GrRenderTargetContext.cpp +++ b/src/gpu/GrRenderTargetContext.cpp @@ -771,9 +771,7 @@ GrRenderTargetContext::QuadOptimization GrRenderTargetContext::attemptQuadOptimi } // Crop the quad to the conservative bounds of the clip. - SkIRect clipDevBounds; - clip.getConservativeBounds(rtRect.width(), rtRect.height(), &clipDevBounds); - SkRect clipBounds = SkRect::Make(clipDevBounds); + SkRect clipBounds = SkRect::Make(clip.getConservativeBounds(rtRect.width(), rtRect.height())); // One final check for discarding, since we may have gone here directly due to a complex clip if (!clipBounds.intersects(drawBounds)) { @@ -2304,9 +2302,8 @@ bool GrRenderTargetContextPriv::drawAndStencilPath(const GrHardClip& clip, GrAAType aaType = fRenderTargetContext->chooseAAType(aa); bool hasUserStencilSettings = !ss->isUnused(); - SkIRect clipConservativeBounds; - clip.getConservativeBounds(fRenderTargetContext->width(), fRenderTargetContext->height(), - &clipConservativeBounds, nullptr); + SkIRect clipConservativeBounds = clip.getConservativeBounds(fRenderTargetContext->width(), + fRenderTargetContext->height()); GrPaint paint; paint.setCoverageSetOpXPFactory(op, invert); @@ -2379,8 +2376,7 @@ void GrRenderTargetContext::drawShapeUsingPathRenderer(const GrClip& clip, return; } - SkIRect clipConservativeBounds; - clip.getConservativeBounds(this->width(), this->height(), &clipConservativeBounds, nullptr); + SkIRect clipConservativeBounds = clip.getConservativeBounds(this->width(), this->height()); GrStyledShape tempShape; GrAAType aaType = this->chooseAAType(aa); @@ -2567,9 +2563,7 @@ bool GrRenderTargetContext::setupDstProxyView(const GrClip& clip, const GrOp& op SkIRect copyRect = SkIRect::MakeSize(this->asSurfaceProxy()->dimensions()); - SkIRect clippedRect; - clip.getConservativeBounds( - this->width(), this->height(), &clippedRect); + SkIRect clippedRect = clip.getConservativeBounds(this->width(), this->height()); SkRect opBounds = op.bounds(); // If the op has aa bloating or is a infinitely thin geometry (hairline) outset the bounds by // 0.5 pixels. diff --git a/src/gpu/GrSoftwarePathRenderer.cpp b/src/gpu/GrSoftwarePathRenderer.cpp index 76b9b78442..6646caeadb 100644 --- a/src/gpu/GrSoftwarePathRenderer.cpp +++ b/src/gpu/GrSoftwarePathRenderer.cpp @@ -75,9 +75,8 @@ bool GrSoftwarePathRenderer::GetShapeAndClipBounds(GrRenderTargetContext* render SkIRect* clippedDevShapeBounds, SkIRect* devClipBounds) { // compute bounds as intersection of rt size, clip, and path - clip.getConservativeBounds(renderTargetContext->width(), - renderTargetContext->height(), - devClipBounds); + *devClipBounds = clip.getConservativeBounds(renderTargetContext->width(), + renderTargetContext->height()); if (!get_unclipped_shape_dev_bounds(shape, matrix, unclippedDevShapeBounds)) { *unclippedDevShapeBounds = SkIRect::MakeEmpty(); diff --git a/src/gpu/GrStencilClip.h b/src/gpu/GrStencilClip.h index e70098d544..854ba1ce51 100644 --- a/src/gpu/GrStencilClip.h +++ b/src/gpu/GrStencilClip.h @@ -33,8 +33,8 @@ public: bool quickContains(const SkRect& rect) const override { return !this->hasStencilClip() && fFixedClip.quickContains(rect); } - void getConservativeBounds(int width, int height, SkIRect* bounds, bool* iior) const override { - fFixedClip.getConservativeBounds(width, height, bounds, iior); + SkIRect getConservativeBounds(int width, int height) const override { + return fFixedClip.getConservativeBounds(width, height); } bool isRRect(const SkRect& rtBounds, SkRRect* rr, GrAA* aa) const override { return !this->hasStencilClip() && fFixedClip.isRRect(rtBounds, rr, aa); diff --git a/src/gpu/SkGpuDevice_drawTexture.cpp b/src/gpu/SkGpuDevice_drawTexture.cpp index a1de1e63ae..3534b6cd84 100644 --- a/src/gpu/SkGpuDevice_drawTexture.cpp +++ b/src/gpu/SkGpuDevice_drawTexture.cpp @@ -124,32 +124,31 @@ static int determine_tile_size(const SkIRect& src, int maxTileSize) { // Given a bitmap, an optional src rect, and a context with a clip and matrix determine what // pixels from the bitmap are necessary. -static void determine_clipped_src_rect(int width, int height, - const GrClip& clip, - const SkMatrix& viewMatrix, - const SkMatrix& srcToDstRect, - const SkISize& imageDimensions, - const SkRect* srcRectPtr, - SkIRect* clippedSrcIRect) { - clip.getConservativeBounds(width, height, clippedSrcIRect, nullptr); +static SkIRect determine_clipped_src_rect(int width, int height, + const GrClip& clip, + const SkMatrix& viewMatrix, + const SkMatrix& srcToDstRect, + const SkISize& imageDimensions, + const SkRect* srcRectPtr) { + SkIRect clippedSrcIRect = clip.getConservativeBounds(width, height); SkMatrix inv = SkMatrix::Concat(viewMatrix, srcToDstRect); if (!inv.invert(&inv)) { - clippedSrcIRect->setEmpty(); - return; + return SkIRect::MakeEmpty(); } - SkRect clippedSrcRect = SkRect::Make(*clippedSrcIRect); + SkRect clippedSrcRect = SkRect::Make(clippedSrcIRect); inv.mapRect(&clippedSrcRect); if (srcRectPtr) { if (!clippedSrcRect.intersect(*srcRectPtr)) { - clippedSrcIRect->setEmpty(); - return; + return SkIRect::MakeEmpty(); } } - clippedSrcRect.roundOut(clippedSrcIRect); + clippedSrcRect.roundOut(&clippedSrcIRect); SkIRect bmpBounds = SkIRect::MakeSize(imageDimensions); - if (!clippedSrcIRect->intersect(bmpBounds)) { - clippedSrcIRect->setEmpty(); + if (!clippedSrcIRect.intersect(bmpBounds)) { + return SkIRect::MakeEmpty(); } + + return clippedSrcIRect; } // tileSize and clippedSubset are valid if true is returned @@ -166,8 +165,8 @@ static bool should_tile_image_id(GrContext* context, SkIRect* clippedSubset) { // if it's larger than the max tile size, then we have no choice but tiling. if (imageSize.width() > maxTileSize || imageSize.height() > maxTileSize) { - determine_clipped_src_rect(rtSize.width(), rtSize.height(), clip, ctm, srcToDst, - imageSize, src, clippedSubset); + *clippedSubset = determine_clipped_src_rect(rtSize.width(), rtSize.height(), clip, ctm, + srcToDst, imageSize, src); *tileSize = determine_tile_size(*clippedSubset, maxTileSize); return true; } @@ -199,8 +198,8 @@ static bool should_tile_image_id(GrContext* context, // Figure out how much of the src we will need based on the src rect and clipping. Reject if // tiling memory savings would be < 50%. - determine_clipped_src_rect(rtSize.width(), rtSize.height(), clip, ctm, srcToDst, imageSize, src, - clippedSubset); + *clippedSubset = determine_clipped_src_rect(rtSize.width(), rtSize.height(), clip, ctm, + srcToDst, imageSize, src); *tileSize = kBmpSmallTileSize; // already know whole bitmap fits in one max sized tile. size_t usedTileBytes = get_tile_count(*clippedSubset, kBmpSmallTileSize) * kBmpSmallTileSize * kBmpSmallTileSize * diff --git a/src/gpu/ccpr/GrCoverageCountingPathRenderer.cpp b/src/gpu/ccpr/GrCoverageCountingPathRenderer.cpp index 320b743b16..bc56c5de6e 100644 --- a/src/gpu/ccpr/GrCoverageCountingPathRenderer.cpp +++ b/src/gpu/ccpr/GrCoverageCountingPathRenderer.cpp @@ -167,9 +167,8 @@ GrPathRenderer::CanDrawPath GrCoverageCountingPathRenderer::onCanDrawPath( bool GrCoverageCountingPathRenderer::onDrawPath(const DrawPathArgs& args) { SkASSERT(!fFlushing); - SkIRect clipIBounds; GrRenderTargetContext* rtc = args.fRenderTargetContext; - args.fClip->getConservativeBounds(rtc->width(), rtc->height(), &clipIBounds, nullptr); + SkIRect clipIBounds = args.fClip->getConservativeBounds(rtc->width(), rtc->height()); auto op = GrCCDrawPathsOp::Make(args.fContext, clipIBounds, *args.fViewMatrix, *args.fShape, std::move(args.fPaint)); diff --git a/src/gpu/ops/GrAAHairLinePathRenderer.cpp b/src/gpu/ops/GrAAHairLinePathRenderer.cpp index db6a9ee119..537d0f1c62 100644 --- a/src/gpu/ops/GrAAHairLinePathRenderer.cpp +++ b/src/gpu/ops/GrAAHairLinePathRenderer.cpp @@ -1272,10 +1272,8 @@ bool GrAAHairLinePathRenderer::onDrawPath(const DrawPathArgs& args) { "GrAAHairlinePathRenderer::onDrawPath"); SkASSERT(args.fRenderTargetContext->numSamples() <= 1); - SkIRect devClipBounds; - args.fClip->getConservativeBounds(args.fRenderTargetContext->width(), - args.fRenderTargetContext->height(), - &devClipBounds); + SkIRect devClipBounds = args.fClip->getConservativeBounds(args.fRenderTargetContext->width(), + args.fRenderTargetContext->height()); SkPath path; args.fShape->asPath(&path); std::unique_ptr op = diff --git a/src/gpu/ops/GrTriangulatingPathRenderer.cpp b/src/gpu/ops/GrTriangulatingPathRenderer.cpp index 4a00ada57f..67f829827a 100644 --- a/src/gpu/ops/GrTriangulatingPathRenderer.cpp +++ b/src/gpu/ops/GrTriangulatingPathRenderer.cpp @@ -419,10 +419,8 @@ private: bool GrTriangulatingPathRenderer::onDrawPath(const DrawPathArgs& args) { GR_AUDIT_TRAIL_AUTO_FRAME(args.fRenderTargetContext->auditTrail(), "GrTriangulatingPathRenderer::onDrawPath"); - SkIRect clipBoundsI; - args.fClip->getConservativeBounds(args.fRenderTargetContext->width(), - args.fRenderTargetContext->height(), - &clipBoundsI); + SkIRect clipBoundsI = args.fClip->getConservativeBounds(args.fRenderTargetContext->width(), + args.fRenderTargetContext->height()); std::unique_ptr op = TriangulatingPathOp::Make( args.fContext, std::move(args.fPaint), *args.fShape, *args.fViewMatrix, clipBoundsI, args.fAAType, args.fUserStencilSettings); diff --git a/tests/ClipBoundsTest.cpp b/tests/ClipBoundsTest.cpp index 699da3c7ae..2987947a4a 100644 --- a/tests/ClipBoundsTest.cpp +++ b/tests/ClipBoundsTest.cpp @@ -43,12 +43,8 @@ DEF_GPUTEST_FOR_RENDERING_CONTEXTS(GrClipBounds, reporter, ctxInfo) { // wrap the SkClipStack in a GrClip GrClipStackClip clipData(&stack); - SkIRect devGrClipBound; - clipData.getConservativeBounds(kXSize, kYSize, - &devGrClipBound, - &isIntersectionOfRects); + SkIRect devGrClipBound = clipData.getConservativeBounds(kXSize, kYSize); // make sure that GrClip is behaving itself REPORTER_ASSERT(reporter, intScreen == devGrClipBound); - REPORTER_ASSERT(reporter, isIntersectionOfRects); } diff --git a/tests/GrCCPRTest.cpp b/tests/GrCCPRTest.cpp index 5b0f7ce838..c86af824bc 100644 --- a/tests/GrCCPRTest.cpp +++ b/tests/GrCCPRTest.cpp @@ -48,12 +48,7 @@ private: } bool quickContains(const SkRect&) const final { return false; } bool isRRect(const SkRect& rtBounds, SkRRect* rr, GrAA*) const final { return false; } - void getConservativeBounds(int width, int height, SkIRect* rect, bool* iior) const final { - rect->setWH(width, height); - if (iior) { - *iior = false; - } - } + GrCoverageCountingPathRenderer* const fCCPR; const SkPath fPath; }; diff --git a/tests/LazyProxyTest.cpp b/tests/LazyProxyTest.cpp index a48a6f0e6b..8aeab035b4 100644 --- a/tests/LazyProxyTest.cpp +++ b/tests/LazyProxyTest.cpp @@ -185,12 +185,6 @@ public: } bool quickContains(const SkRect&) const final { return false; } bool isRRect(const SkRect& rtBounds, SkRRect* rr, GrAA*) const final { return false; } - void getConservativeBounds(int width, int height, SkIRect* rect, bool* iior) const final { - rect->setLTRB(0, 0, width, height); - if (iior) { - *iior = false; - } - } LazyProxyTest* const fTest; GrTextureProxy* fAtlas;