From d8f4f42b03a3b144c0b191354c59fd1a0ec75f75 Mon Sep 17 00:00:00 2001 From: Michael Ludwig Date: Wed, 24 Jun 2020 18:58:18 +0000 Subject: [PATCH] Revert "Simplify GrClip API" This reverts commit 9716414e93f7b279e547595fab08b68235c4b2be. Reason for revert: clipRRect elision seems to trigger precision issues on Nexus5x, see https://chrome-gpu-gold.skia.org/search?fdiffmax=-1&fref=false&frgbamax=255&frgbamin=0&head=true&include=false&issue=2264837&limit=50&master=false&match=name&metric=combined&neg=false&offset=0&pos=false&query=source_type%3Dchrome-gpu&sort=desc&unt=true Original change's description: > Simplify GrClip API > > Removes quickContains(SkRect), quickContains(SkRRect), and isRRect(). > Replaces these three functions with preApply() that conservatively > determines the clip effect up to a single rrect intersection. The major > motivation for this is the new GrClipStack implementation. preApply() > and apply() will be able to reuse much more code compared to separating > the preApply functionality across the older three functions that were > removed. Additionally, preApply is able to convey more information for > less work, since it can usually determine being skipped or unclipped while > determining if the clip is a single rrect. > > As part of using this API, the attemptQuadOptimiziation and the equivalent > rrect optimization are overhauled. Hopefully legibility is improved, and > the rrect case is now applied outside of the android framework (but with > tighter AA requirements). > > Bug: skia:10205 > Change-Id: I33249dd75a28a611495f87b211cb7ec74ebb7ba4 > Reviewed-on: https://skia-review.googlesource.com/c/skia/+/298506 > Reviewed-by: Brian Salomon > Reviewed-by: Chris Dalton > Commit-Queue: Michael Ludwig TBR=bsalomon@google.com,csmartdalton@google.com,michaelludwig@google.com Change-Id: I850cbf92eea9cf5f2db5528a93251f02dbd6fee2 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: skia:10205 Reviewed-on: https://skia-review.googlesource.com/c/skia/+/298753 Reviewed-by: Michael Ludwig Commit-Queue: Michael Ludwig --- gm/windowrectangles.cpp | 18 +- src/gpu/GrClip.h | 70 ++--- src/gpu/GrClipStackClip.cpp | 56 ++-- src/gpu/GrClipStackClip.h | 9 +- src/gpu/GrFixedClip.cpp | 44 ++- src/gpu/GrFixedClip.h | 7 +- src/gpu/GrRenderTargetContext.cpp | 291 ++++++++++-------- src/gpu/GrStencilClip.h | 28 +- src/gpu/ops/GrStencilAndCoverPathRenderer.cpp | 4 +- src/gpu/text/GrTextBlob.cpp | 26 +- tests/GrCCPRTest.cpp | 9 +- tests/LazyProxyTest.cpp | 10 +- 12 files changed, 284 insertions(+), 288 deletions(-) diff --git a/gm/windowrectangles.cpp b/gm/windowrectangles.cpp index e47309e7fb..1e38bf0d9a 100644 --- a/gm/windowrectangles.cpp +++ b/gm/windowrectangles.cpp @@ -157,10 +157,19 @@ private: void stencilCheckerboard(GrRenderTargetContext*, bool flip); }; +/** + * Base class for GrClips that visualize a clip mask. + */ +class MaskOnlyClipBase : public GrClip { +private: + bool quickContains(const SkRect&) const final { return false; } + bool isRRect(SkRRect* rr, GrAA*) const final { return false; } +}; + /** * This class clips a cover by an alpha mask. We use it to visualize the alpha clip mask. */ -class AlphaOnlyClip final : public GrClip { +class AlphaOnlyClip final : public MaskOnlyClipBase { public: AlphaOnlyClip(GrSurfaceProxyView mask, int x, int y) : fMask(std::move(mask)), fX(x), fY(y) {} @@ -168,8 +177,9 @@ private: SkIRect getConservativeBounds() const final { return SkIRect::MakeXYWH(fX, fY, fMask.width(), fMask.height()); } - Effect apply(GrRecordingContext* ctx, GrRenderTargetContext*, bool, bool, - GrAppliedClip* out, SkRect* bounds) const override { + + bool apply(GrRecordingContext* ctx, GrRenderTargetContext*, bool, bool, GrAppliedClip* out, + SkRect* bounds) const override { GrSamplerState samplerState(GrSamplerState::WrapMode::kClampToBorder, GrSamplerState::Filter::kNearest); auto m = SkMatrix::Translate(-fX, -fY); @@ -179,7 +189,7 @@ private: domain, *ctx->priv().caps()); fp = GrDeviceSpaceEffect::Make(std::move(fp)); out->addCoverageFP(std::move(fp)); - return Effect::kClipped; + return true; } GrSurfaceProxyView fMask; int fX; diff --git a/src/gpu/GrClip.h b/src/gpu/GrClip.h index f9008ae4b2..ee6935d1d2 100644 --- a/src/gpu/GrClip.h +++ b/src/gpu/GrClip.h @@ -21,33 +21,12 @@ class GrContext; */ class GrClip { public: - enum class Effect { - // The clip conservatively modifies the draw's coverage but doesn't eliminate the draw - kClipped, - // The clip definitely does not modify the draw's coverage and the draw can be performed - // without clipping (beyond the automatic device bounds clip). - kUnclipped, - // The clip definitely eliminates all of the draw's coverage and the draw can be skipped - kClippedOut - }; - - struct PreClipResult { - Effect fEffect; - SkRRect fRRect; // Ignore if 'isRRect' is false - GrAA fAA; // Ignore if 'isRRect' is false - bool fIsRRect; - - PreClipResult(Effect effect) : fEffect(effect), fIsRRect(false) {} - PreClipResult(SkRect rect, GrAA aa) : PreClipResult(SkRRect::MakeRect(rect), aa) {} - PreClipResult(SkRRect rrect, GrAA aa) - : fEffect(Effect::kClipped) - , fRRect(rrect) - , fAA(aa) - , fIsRRect(true) {} - }; - virtual ~GrClip() {} + virtual bool quickContains(const SkRect&) const = 0; + virtual bool quickContains(const SkRRect& rrect) const { + return this->quickContains(rrect.getBounds()); + } /** * 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 @@ -59,34 +38,25 @@ public: * 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 * the draw will enable HW AA or uses the stencil buffer. On input 'bounds' is a conservative - * bounds of the draw that is to be clipped. If kClipped or kUnclipped is returned, the 'bounds' - * will have been updated to be contained within the clip bounds (or the device's, for wide-open - * clips). If kNoDraw is returned, 'bounds' and the applied clip are in an undetermined state - * and should be ignored (and the draw should be skipped). + * bounds of the draw that is to be clipped. After return 'bounds' has been intersected with a + * conservative bounds of the clip. A return value of false indicates that the draw can be + * skipped as it is fully clipped out. */ - virtual Effect apply(GrRecordingContext*, GrRenderTargetContext*, bool useHWAA, - bool hasUserStencilSettings, GrAppliedClip*, SkRect* bounds) const = 0; + virtual bool apply(GrRecordingContext*, GrRenderTargetContext*, bool useHWAA, + bool hasUserStencilSettings, GrAppliedClip*, SkRect* bounds) const = 0; /** - * Perform preliminary, conservative analysis on the draw bounds as if it were provided to - * apply(). The results of this are returned the PreClipResults struct, where 'result.fEffect' - * corresponds to what 'apply' would return. If this value is kUnclipped or kNoDraw, then it - * can be assumed that apply() would also always result in the same Effect. + * This method quickly and conservatively determines whether the entire clip is equivalent to + * intersection with a rrect. Moreover, the returned rrect need not be contained by the render + * target bounds. We assume all draws will be implicitly clipped by the render target bounds. * - * If kClipped is returned, apply() may further refine the effect to kUnclipped or kNoDraw, - * with one exception. When 'result.fIsRRect' is true, preApply() reports the single round rect - * and anti-aliased state that would act as an intersection on the draw geometry. If no further - * action is taken to modify the draw, apply() will represent this round rect in the applied - * clip. + * @param rrect If return is true rrect will contain the rrect equivalent to the clip within + * rtBounds. + * @param aa If return is true aa will indicate whether the rrect clip is antialiased. + * @return true if the clip is equivalent to a single rrect, false otherwise. * - * When set, 'result.fRRect' will intersect with the render target bounds but may extend - * beyond it. If the render target bounds are the only clip effect on the draw, this is reported - * as kUnclipped and not as a degenerate rrect that matches the bounds. */ - virtual PreClipResult preApply(const SkRect& drawBounds) const { - bool outside = !drawBounds.intersects(SkRect::Make(this->getConservativeBounds())); - return outside ? Effect::kClippedOut : Effect::kClipped; - } + virtual bool isRRect(SkRRect* rrect, GrAA* aa) const = 0; /** * This is the maximum distance that a draw may extend beyond a clip's boundary and still count @@ -177,11 +147,11 @@ public: * return 'bounds' has been intersected with a conservative bounds of the clip. A return value * of false indicates that the draw can be skipped as it is fully clipped out. */ - virtual Effect apply(GrAppliedHardClip* out, SkRect* bounds) const = 0; + virtual bool apply(GrAppliedHardClip* out, SkRect* bounds) const = 0; private: - Effect apply(GrRecordingContext*, GrRenderTargetContext* rtc, bool useHWAA, - bool hasUserStencilSettings, GrAppliedClip* out, SkRect* bounds) const final { + bool apply(GrRecordingContext*, GrRenderTargetContext* rtc, bool useHWAA, + bool hasUserStencilSettings, GrAppliedClip* out, SkRect* bounds) const final { return this->apply(&out->hardClip(), bounds); } }; diff --git a/src/gpu/GrClipStackClip.cpp b/src/gpu/GrClipStackClip.cpp index 9654f085ac..6553238934 100644 --- a/src/gpu/GrClipStackClip.cpp +++ b/src/gpu/GrClipStackClip.cpp @@ -33,22 +33,32 @@ typedef GrReducedClip::ElementList ElementList; const char GrClipStackClip::kMaskTestTag[] = "clip_mask"; -GrClip::PreClipResult GrClipStackClip::preApply(const SkRect& drawBounds) const { - SkIRect deviceRect = SkIRect::MakeSize(fDeviceSize); - SkRect rect = SkRect::Make(deviceRect); - if (!rect.intersect(drawBounds) || (fStack && fStack->isEmpty(deviceRect))) { - return Effect::kClippedOut; - } else if (!fStack || fStack->isWideOpen()) { - return Effect::kUnclipped; +bool GrClipStackClip::quickContains(const SkRect& rect) const { + if (!fStack || fStack->isWideOpen()) { + return true; + } + return fStack->quickContains(rect); +} + +bool GrClipStackClip::quickContains(const SkRRect& rrect) const { + if (!fStack || fStack->isWideOpen()) { + return true; + } + return fStack->quickContains(rrect); +} + +bool GrClipStackClip::isRRect(SkRRect* rr, GrAA* aa) const { + if (!fStack) { + return false; } - PreClipResult result(Effect::kClipped); + SkRect rtBounds = SkRect::MakeIWH(fDeviceSize.fWidth, fDeviceSize.fHeight); bool isAA; - if (fStack->isRRect(rect, &result.fRRect, &isAA)) { - result.fIsRRect = true; - result.fAA = GrAA(isAA); + if (fStack->isRRect(rtBounds, rr, &isAA)) { + *aa = GrAA(isAA); + return true; } - return result; + return false; } SkIRect GrClipStackClip::getConservativeBounds() const { @@ -186,19 +196,18 @@ bool GrClipStackClip::UseSWOnlyPath(GrRecordingContext* context, //////////////////////////////////////////////////////////////////////////////// // sort out what kind of clip mask needs to be created: alpha, stencil, // scissor, or entirely software -GrClip::Effect GrClipStackClip::apply(GrRecordingContext* context, - GrRenderTargetContext* renderTargetContext, - bool useHWAA, bool hasUserStencilSettings, - GrAppliedClip* out, SkRect* bounds) const { +bool GrClipStackClip::apply(GrRecordingContext* context, GrRenderTargetContext* renderTargetContext, + bool useHWAA, bool hasUserStencilSettings, GrAppliedClip* out, + SkRect* bounds) const { SkASSERT(renderTargetContext->width() == fDeviceSize.fWidth && renderTargetContext->height() == fDeviceSize.fHeight); SkRect devBounds = SkRect::MakeIWH(fDeviceSize.fWidth, fDeviceSize.fHeight); if (!devBounds.intersect(*bounds)) { - return Effect::kClippedOut; + return false; } if (!fStack || fStack->isWideOpen()) { - return Effect::kUnclipped; + return true; } // An default count of 4 was chosen because of the common pattern in Blink of: @@ -223,27 +232,23 @@ GrClip::Effect GrClipStackClip::apply(GrRecordingContext* context, maxWindowRectangles, maxAnalyticFPs, ccpr ? maxAnalyticFPs : 0); if (InitialState::kAllOut == reducedClip.initialState() && reducedClip.maskElements().isEmpty()) { - return Effect::kClippedOut; + return false; } - Effect effect = Effect::kUnclipped; if (reducedClip.hasScissor() && !GrClip::IsInsideClip(reducedClip.scissor(), devBounds)) { out->hardClip().addScissor(reducedClip.scissor(), bounds); - effect = Effect::kClipped; } if (!reducedClip.windowRectangles().empty()) { out->hardClip().addWindowRectangles(reducedClip.windowRectangles(), GrWindowRectsState::Mode::kExclusive); - effect = Effect::kClipped; } if (!reducedClip.maskElements().isEmpty()) { if (!this->applyClipMask(context, renderTargetContext, reducedClip, hasUserStencilSettings, out)) { - return Effect::kClippedOut; + return false; } - effect = Effect::kClipped; } // The opsTask ID must not be looked up until AFTER producing the clip mask (if any). That step @@ -252,10 +257,9 @@ GrClip::Effect GrClipStackClip::apply(GrRecordingContext* context, if (auto clipFPs = reducedClip.finishAndDetachAnalyticFPs(context, *fMatrixProvider, ccpr, opsTaskID)) { out->addCoverageFP(std::move(clipFPs)); - effect = Effect::kClipped; } - return effect; + return true; } bool GrClipStackClip::applyClipMask(GrRecordingContext* context, diff --git a/src/gpu/GrClipStackClip.h b/src/gpu/GrClipStackClip.h index f1d47964ea..ffbd0017cc 100644 --- a/src/gpu/GrClipStackClip.h +++ b/src/gpu/GrClipStackClip.h @@ -27,10 +27,13 @@ public: , fStack(stack) , fMatrixProvider(matrixProvider) {} + bool quickContains(const SkRect&) const final; + bool quickContains(const SkRRect&) const final; SkIRect getConservativeBounds() const final; - Effect apply(GrRecordingContext*, GrRenderTargetContext*, bool useHWAA, - bool hasUserStencilSettings, GrAppliedClip* out, SkRect* bounds) const final; - PreClipResult preApply(const SkRect& drawBounds) const final; + bool apply(GrRecordingContext*, GrRenderTargetContext*, bool useHWAA, + bool hasUserStencilSettings, GrAppliedClip* out, SkRect* bounds) const final; + + bool isRRect(SkRRect* rr, GrAA* aa) const override; sk_sp testingOnly_createClipMask(GrContext*) const; static const char kMaskTestTag[]; diff --git a/src/gpu/GrFixedClip.cpp b/src/gpu/GrFixedClip.cpp index 928b6809d8..2f75bc9a4b 100644 --- a/src/gpu/GrFixedClip.cpp +++ b/src/gpu/GrFixedClip.cpp @@ -10,46 +10,42 @@ #include "src/gpu/GrAppliedClip.h" #include "src/gpu/GrRenderTargetContext.h" +bool GrFixedClip::quickContains(const SkRect& rect) const { + if (fWindowRectsState.enabled()) { + return false; + } + + return !fScissorState.enabled() || GrClip::IsInsideClip(fScissorState.rect(), rect); +} + SkIRect GrFixedClip::getConservativeBounds() const { return fScissorState.rect(); } -GrClip::PreClipResult GrFixedClip::preApply(const SkRect& drawBounds) const { - if (IsOutsideClip(fScissorState.rect(), drawBounds)) { - return Effect::kClippedOut; - } - +bool GrFixedClip::isRRect(SkRRect* rr, GrAA* aa) const { if (fWindowRectsState.enabled()) { - return Effect::kClipped; + return false; } + // Whether or not the scissor test is enabled, the remaining clip is a rectangle described + // by scissorState.rect() (either the scissor or the rt bounds). + rr->setRect(SkRect::Make(fScissorState.rect())); + *aa = GrAA::kNo; + return true; +}; - if (!fScissorState.enabled() || IsInsideClip(fScissorState.rect(), drawBounds)) { - // Either no scissor or the scissor doesn't clip the draw - return Effect::kUnclipped; - } - // Report the scissor as a degenerate round rect - return {SkRect::Make(fScissorState.rect()), GrAA::kNo}; -} - -GrClip::Effect GrFixedClip::apply(GrAppliedHardClip* out, SkRect* bounds) const { +bool GrFixedClip::apply(GrAppliedHardClip* out, SkRect* bounds) const { if (IsOutsideClip(fScissorState.rect(), *bounds)) { - return Effect::kClippedOut; + return false; } - - Effect effect = Effect::kUnclipped; - if (fScissorState.enabled() && !IsInsideClip(fScissorState.rect(), *bounds)) { + if (!IsInsideClip(fScissorState.rect(), *bounds)) { SkIRect tightScissor = bounds->roundOut(); SkAssertResult(tightScissor.intersect(fScissorState.rect())); out->addScissor(tightScissor, bounds); - effect = Effect::kClipped; } if (fWindowRectsState.enabled()) { out->addWindowRectangles(fWindowRectsState); - // We could iterate each window rectangle to check for intersection, but be conservative - // and report that it's clipped - effect = Effect::kClipped; } - return effect; + return true; } diff --git a/src/gpu/GrFixedClip.h b/src/gpu/GrFixedClip.h index 383c388bf2..5abc3102c2 100644 --- a/src/gpu/GrFixedClip.h +++ b/src/gpu/GrFixedClip.h @@ -46,9 +46,10 @@ public: fWindowRectsState.set(windows, mode); } - SkIRect getConservativeBounds() const final; - Effect apply(GrAppliedHardClip*, SkRect*) const final; - PreClipResult preApply(const SkRect& drawBounds) const final; + bool quickContains(const SkRect&) const override; + SkIRect getConservativeBounds() const override; + bool isRRect(SkRRect* rr, GrAA*) const override; + bool apply(GrAppliedHardClip*, SkRect*) const override; private: GrScissorState fScissorState; diff --git a/src/gpu/GrRenderTargetContext.cpp b/src/gpu/GrRenderTargetContext.cpp index 3b8d039d89..c4e3a1341f 100644 --- a/src/gpu/GrRenderTargetContext.cpp +++ b/src/gpu/GrRenderTargetContext.cpp @@ -680,6 +680,25 @@ enum class GrRenderTargetContext::QuadOptimization { kCropped }; +static bool make_vertex_finite(float* value) { + if (SkScalarIsNaN(*value)) { + return false; + } + + if (!SkScalarIsFinite(*value)) { + // +/- infinity at this point. Don't use exactly SK_ScalarMax so that we have some precision + // left when calculating crops. + static constexpr float kNearInfinity = SK_ScalarMax / 4.f; + *value = *value < 0.f ? -kNearInfinity : kNearInfinity; + } + + return true; +} + +static SkIRect get_clip_bounds(const GrRenderTargetContext* rtc, const GrClip* clip) { + return clip ? clip->getConservativeBounds() : SkIRect::MakeWH(rtc->width(), rtc->height()); +} + GrRenderTargetContext::QuadOptimization GrRenderTargetContext::attemptQuadOptimization( const GrClip* clip, const SkPMColor4f* constColor, const GrUserStencilSettings* stencilSettings, GrAA* aa, DrawQuad* quad) { @@ -700,108 +719,132 @@ GrRenderTargetContext::QuadOptimization GrRenderTargetContext::attemptQuadOptimi SkRect rtRect = this->asSurfaceProxy()->getBoundsRect(); SkRect drawBounds = quad->fDevice.bounds(); - if (!quad->fDevice.isFinite() || drawBounds.isEmpty() || - GrClip::IsOutsideClip(rtRect, drawBounds)) { - return QuadOptimization::kDiscarded; - } - auto conservativeCrop = [&]() { - static constexpr int kLargeDrawLimit = 15000; - // Crop the quad to the render target. This doesn't change the visual results of drawing but - // is meant to help numerical stability for excessively large draws. - if (drawBounds.width() > kLargeDrawLimit || drawBounds.height() > kLargeDrawLimit) { - GrQuadUtils::CropToRect(rtRect, *aa, quad, /* compute local */ !constColor); - SkASSERT(quad->fEdgeFlags == oldFlags); - } - }; - - bool simpleColor = !stencilSettings && constColor; - GrClip::PreClipResult result = clip ? clip->preApply(drawBounds) - : GrClip::PreClipResult(GrClip::Effect::kUnclipped); - switch(result.fEffect) { - case GrClip::Effect::kClippedOut: - return QuadOptimization::kDiscarded; - case GrClip::Effect::kUnclipped: - if (!simpleColor) { - conservativeCrop(); - return QuadOptimization::kClipApplied; - } else { - // Update result to store the render target bounds in order and then fall - // through to attempt the draw->native clear optimization - result = GrClip::PreClipResult(SkRRect::MakeRect(rtRect), *aa); - } - break; - case GrClip::Effect::kClipped: - if (!result.fIsRRect || (stencilSettings && result.fAA != *aa) || - (!result.fRRect.isRect() && !simpleColor)) { - // The clip and draw state are too complicated to try and reduce - conservativeCrop(); - return QuadOptimization::kCropped; - } // Else fall through to attempt to combine the draw and clip geometry together - break; - default: - SkUNREACHABLE; - } - - // If we reached here, we know we're an axis-aligned clip that is either a rect or a round rect, - // so we can potentially combine it with the draw geometry so that no clipping is needed. - SkASSERT(result.fEffect == GrClip::Effect::kClipped && result.fIsRRect); - if (result.fRRect.isRect()) { - // No rounded corners, so we might be able to become a native clear or we might be able to - // modify geometry and edge flags to represent intersected shape of clip and draw. - if (GrQuadUtils::CropToRect(result.fRRect.rect(), result.fAA, quad, - /*compute local*/ !constColor)) { - if (simpleColor && quad->fDevice.quadType() == GrQuad::Type::kAxisAligned) { - // Clear optimization is possible - drawBounds = quad->fDevice.bounds(); - if (drawBounds.contains(rtRect)) { - // Fullscreen clear - this->clear(*constColor); - return QuadOptimization::kSubmitted; - } else if (GrClip::IsPixelAligned(drawBounds) && - drawBounds.width() > 256 && drawBounds.height() > 256) { - // Scissor + clear (round shouldn't do anything since we are pixel aligned) - SkIRect scissorRect; - drawBounds.round(&scissorRect); - this->clear(scissorRect, *constColor); - return QuadOptimization::kSubmitted; + if (constColor) { + // If the device quad is not finite, coerce into a finite quad. This is acceptable since it + // will be cropped to the finite 'clip' or render target and there is no local space mapping + if (!quad->fDevice.isFinite()) { + for (int i = 0; i < 4; ++i) { + if (!make_vertex_finite(quad->fDevice.xs() + i) || + !make_vertex_finite(quad->fDevice.ys() + i) || + !make_vertex_finite(quad->fDevice.ws() + i)) { + // Discard if we see a nan + return QuadOptimization::kDiscarded; } } - - // else the draw and clip were combined so just update the AA to reflect combination - if (*aa == GrAA::kNo && result.fAA == GrAA::kYes && - quad->fEdgeFlags != GrQuadAAFlags::kNone) { - // The clip was anti-aliased and now the draw needs to be upgraded to AA to - // properly reflect the smooth edge of the clip. - *aa = GrAA::kYes; - } - // We intentionally do not downgrade AA here because we don't know if we need to - // preserve MSAA (see GrQuadAAFlags docs). But later in the pipeline, the ops can - // use GrResolveAATypeForQuad() to turn off coverage AA when all flags are off. - // deviceQuad is exactly the intersection of original quad and clip, so it can be - // drawn with no clip (submitted by caller) - return QuadOptimization::kClipApplied; + SkASSERT(quad->fDevice.isFinite()); } } else { - // Rounded corners and constant filled color (limit ourselves to solid colors because - // there is no way to use custom local coordinates with drawRRect). - SkASSERT(simpleColor); - if (GrQuadUtils::CropToRect(result.fRRect.getBounds(), result.fAA, quad, - /* compute local */ false) && - quad->fDevice.quadType() == GrQuad::Type::kAxisAligned && - quad->fDevice.bounds().contains(result.fRRect.getBounds())) { - // Since the cropped quad became a rectangle which covered the bounds of the rrect, - // we can draw the rrect directly and ignore the edge flags - GrPaint paint; - clear_to_grpaint(*constColor, &paint); - this->drawRRect(nullptr, std::move(paint), result.fAA, SkMatrix::I(), result.fRRect, - GrStyle::SimpleFill()); - return QuadOptimization::kSubmitted; + // CropToRect requires the quads to be finite. If they are not finite and we have local + // coordinates, the mapping from local space to device space is poorly defined so drop it + if (!quad->fDevice.isFinite()) { + return QuadOptimization::kDiscarded; } } - // The quads have been updated to better fit the clip bounds, but can't get rid of - // the clip entirely + // If the quad is entirely off screen, it doesn't matter what the clip does + if (!rtRect.intersects(drawBounds)) { + return QuadOptimization::kDiscarded; + } + + // Check if clip can be represented as a rounded rect (initialize as if clip fully contained + // the render target). + SkRRect clipRRect = SkRRect::MakeRect(rtRect); + // We initialize clipAA to *aa when there are stencil settings so that we don't artificially + // encounter mixed-aa edges (not allowed for stencil), but we want to start as non-AA for + // regular draws so that if we fully cover the render target, that can stop being anti-aliased. + GrAA clipAA = stencilSettings ? *aa : GrAA::kNo; + bool axisAlignedClip = true; + if (clip && !clip->quickContains(rtRect)) { + if (!clip->isRRect(&clipRRect, &clipAA)) { + axisAlignedClip = false; + } + } + + // If the clip rrect is valid (i.e. axis-aligned), we can potentially combine it with the + // draw geometry so that no clip is needed when drawing. + if (axisAlignedClip && (!stencilSettings || clipAA == *aa)) { + // Tighten clip bounds (if clipRRect.isRect() is true, clipBounds now holds the intersection + // of the render target and the clip rect) + SkRect clipBounds = rtRect; + if (!clipBounds.intersect(clipRRect.rect()) || !clipBounds.intersects(drawBounds)) { + return QuadOptimization::kDiscarded; + } + + if (clipRRect.isRect()) { + // No rounded corners, so the kClear and kExplicitClip optimizations are possible + if (GrQuadUtils::CropToRect(clipBounds, clipAA, quad, /*compute local*/ !constColor)) { + if (!stencilSettings && constColor && + quad->fDevice.quadType() == GrQuad::Type::kAxisAligned) { + // Clear optimization is possible + drawBounds = quad->fDevice.bounds(); + if (drawBounds.contains(rtRect)) { + // Fullscreen clear + this->clear(*constColor); + return QuadOptimization::kSubmitted; + } else if (GrClip::IsPixelAligned(drawBounds) && + drawBounds.width() > 256 && drawBounds.height() > 256) { + // Scissor + clear (round shouldn't do anything since we are pixel aligned) + SkIRect scissorRect; + drawBounds.round(&scissorRect); + this->clear(scissorRect, *constColor); + return QuadOptimization::kSubmitted; + } + } + + // Update overall AA setting. + if (*aa == GrAA::kNo && clipAA == GrAA::kYes && + quad->fEdgeFlags != GrQuadAAFlags::kNone) { + // The clip was anti-aliased and now the draw needs to be upgraded to AA to + // properly reflect the smooth edge of the clip. + *aa = GrAA::kYes; + } + // We intentionally do not downgrade AA here because we don't know if we need to + // preserve MSAA (see GrQuadAAFlags docs). But later in the pipeline, the ops can + // use GrResolveAATypeForQuad() to turn off coverage AA when all flags are off. + + // deviceQuad is exactly the intersection of original quad and clip, so it can be + // drawn with no clip (submitted by caller) + return QuadOptimization::kClipApplied; + } else { + // The quads have been updated to better fit the clip bounds, but can't get rid of + // the clip entirely + quad->fEdgeFlags = oldFlags; + return QuadOptimization::kCropped; + } + } else if (!stencilSettings && constColor) { + // Rounded corners and constant filled color (limit ourselves to solid colors because + // there is no way to use custom local coordinates with drawRRect). + if (GrQuadUtils::CropToRect(clipBounds, clipAA, quad, /* compute local */ false) && + quad->fDevice.quadType() == GrQuad::Type::kAxisAligned && + quad->fDevice.bounds().contains(clipBounds)) { + // Since the cropped quad became a rectangle which covered the bounds of the rrect, + // we can draw the rrect directly and ignore the edge flags + GrPaint paint; + clear_to_grpaint(*constColor, &paint); + this->drawRRect(nullptr, std::move(paint), clipAA, SkMatrix::I(), + clipRRect, GrStyle::SimpleFill()); + return QuadOptimization::kSubmitted; + } else { + // The quad has been updated to better fit clip bounds, but can't remove the clip + quad->fEdgeFlags = oldFlags; + return QuadOptimization::kCropped; + } + } + } + + // Crop the quad to the conservative bounds of the clip. + SkRect clipBounds = SkRect::Make(get_clip_bounds(this, clip)); + + // One final check for discarding, since we may have gone here directly due to a complex clip + if (!clipBounds.intersects(drawBounds)) { + return QuadOptimization::kDiscarded; + } + + // Even if this were to return true, the crop rect does not exactly match the clip, so can not + // report explicit-clip. Since these edges aren't visible, don't update the final edge flags. + GrQuadUtils::CropToRect(clipBounds, clipAA, quad, /* compute local */ !constColor); quad->fEdgeFlags = oldFlags; + return QuadOptimization::kCropped; } @@ -1030,7 +1073,7 @@ void GrRenderTargetContextPriv::stencilPath(const GrHardClip* clip, GrAppliedHardClip appliedClip(fRenderTargetContext->dimensions(), fRenderTargetContext->asSurfaceProxy()->backingStoreDimensions()); - if (clip && GrClip::Effect::kClippedOut == clip->apply(&appliedClip, &bounds)) { + if (clip && !clip->apply(&appliedClip, &bounds)) { return; } // else see FIXME above; we'd normally want to check path bounds with render target bounds, @@ -1131,49 +1174,26 @@ void GrRenderTargetContext::drawRRect(const GrClip* origClip, SkDEBUGCODE(this->validate();) GR_CREATE_TRACE_MARKER_CONTEXT("GrRenderTargetContext", "drawRRect", fContext); - SkASSERT(!style.pathEffect()); // this should've been devolved to a path in SkGpuDevice - const SkStrokeRec& stroke = style.strokeRec(); if (stroke.getStyle() == SkStrokeRec::kFill_Style && rrect.isEmpty()) { return; } const GrClip* clip = origClip; - // It is not uncommon to clip to a round rect and then draw that same round rect. Since our - // lower level clip code works from op bounds, which are SkRects, it doesn't detect that the - // clip can be ignored. The following test attempts to mitigate the stencil clip cost but only - // works for axis-aligned round rects. This also only works for filled rrects since the stroke - // width outsets beyond the rrect itself. +#ifdef SK_BUILD_FOR_ANDROID_FRAMEWORK + // The Android framework frequently clips rrects to themselves where the clip is non-aa and the + // draw is aa. Since our lower level clip code works from op bounds, which are SkRects, it + // doesn't detect that the clip can be ignored (modulo antialiasing). The following test + // attempts to mitigate the stencil clip cost but will only help when the entire clip stack + // can be ignored. We'd prefer to fix this in the framework by removing the clips calls. This + // only works for filled rrects since the stroke width outsets beyond the rrect itself. SkRRect devRRect; if (clip && stroke.getStyle() == SkStrokeRec::kFill_Style && - rrect.transform(viewMatrix, &devRRect)) { - GrClip::PreClipResult result = clip->preApply(devRRect.getBounds()); - switch(result.fEffect) { - case GrClip::Effect::kClippedOut: - return; - case GrClip::Effect::kUnclipped: - clip = nullptr; - break; - case GrClip::Effect::kClipped: - // Currently there's no general-purpose rrect-to-rrect contains function, and if we - // got here, we know the devRRect's bounds aren't fully contained by the clip. - // Testing for equality between the two is a reasonable stop-gap for now. - if (result.fIsRRect && result.fRRect == devRRect) { - // NOTE: On the android framework, we allow this optimization even when the clip - // is non-AA and the draw is AA. -#ifdef SK_BUILD_FOR_ANDROID_FRAMEWORK - if (result.fAA == aa || (result.fAA == GrAA::kNo && aa == GrAA::kYes)) { -#else - if (result.fAA == aa) { -#endif - clip = nullptr; - } - } - break; - default: - SkUNREACHABLE; - } + rrect.transform(viewMatrix, &devRRect) && clip->quickContains(devRRect)) { + clip = nullptr; } +#endif + SkASSERT(!style.pathEffect()); // this should've been devolved to a path in SkGpuDevice AutoCheckFlush acf(this->drawingManager()); @@ -2322,10 +2342,6 @@ void GrRenderTargetContext::drawShape(const GrClip* clip, /* attempt fallback */ false); } -static SkIRect get_clip_bounds(const GrRenderTargetContext* rtc, const GrClip* clip) { - return clip ? clip->getConservativeBounds() : SkIRect::MakeWH(rtc->width(), rtc->height()); -} - bool GrRenderTargetContextPriv::drawAndStencilPath(const GrHardClip* clip, const GrUserStencilSettings* ss, SkRegion::Op op, @@ -2562,11 +2578,14 @@ void GrRenderTargetContext::addDrawOp(const GrClip* clip, std::unique_ptrapply(fContext, this, usesHWAA, usesUserStencilBits, - &appliedClip, &bounds) == GrClip::Effect::kClippedOut; + if (!clip->apply(fContext, this, usesHWAA, usesUserStencilBits, &appliedClip, &bounds)) { + skipDraw = true; + } } else { // No clipping, so just clip the bounds against the logical render target dimensions - skipDraw = !bounds.intersect(this->asSurfaceProxy()->getBoundsRect()); + if (!bounds.intersect(this->asSurfaceProxy()->getBoundsRect())) { + skipDraw = true; + } } if (skipDraw) { diff --git a/src/gpu/GrStencilClip.h b/src/gpu/GrStencilClip.h index a55c463de2..4ab8a82e08 100644 --- a/src/gpu/GrStencilClip.h +++ b/src/gpu/GrStencilClip.h @@ -32,29 +32,23 @@ public: bool hasStencilClip() const { return SK_InvalidGenID != fStencilStackID; } void setStencilClip(uint32_t stencilStackID) { fStencilStackID = stencilStackID; } - SkIRect getConservativeBounds() const final { + bool quickContains(const SkRect& rect) const override { + return !this->hasStencilClip() && fFixedClip.quickContains(rect); + } + SkIRect getConservativeBounds() const override { return fFixedClip.getConservativeBounds(); } - - Effect apply(GrAppliedHardClip* out, SkRect* bounds) const final { - Effect effect = fFixedClip.apply(out, bounds); - if (effect == Effect::kClippedOut) { - // Stencil won't bring back coverage - return Effect::kClippedOut; + bool isRRect(SkRRect* rr, GrAA* aa) const override { + return !this->hasStencilClip() && fFixedClip.isRRect(rr, aa); + } + bool apply(GrAppliedHardClip* out, SkRect* bounds) const override { + if (!fFixedClip.apply(out, bounds)) { + return false; } if (this->hasStencilClip()) { out->addStencilClip(fStencilStackID); - effect = Effect::kClipped; - } - return effect; - } - - PreClipResult preApply(const SkRect& drawBounds) const final { - if (this->hasStencilClip()) { - return this->INHERITED::preApply(drawBounds); - } else { - return fFixedClip.preApply(drawBounds); } + return true; } private: diff --git a/src/gpu/ops/GrStencilAndCoverPathRenderer.cpp b/src/gpu/ops/GrStencilAndCoverPathRenderer.cpp index 5ea3738c8a..fcff78cf6f 100644 --- a/src/gpu/ops/GrStencilAndCoverPathRenderer.cpp +++ b/src/gpu/ops/GrStencilAndCoverPathRenderer.cpp @@ -125,9 +125,9 @@ bool GrStencilAndCoverPathRenderer::onDrawPath(const DrawPathArgs& args) { // fake inverse with a stencil and cover GrAppliedClip appliedClip(args.fRenderTargetContext->dimensions()); - if (args.fClip && args.fClip->apply( + if (args.fClip && !args.fClip->apply( args.fContext, args.fRenderTargetContext, doStencilMSAA, true, &appliedClip, - &devBounds) == GrClip::Effect::kClippedOut) { + &devBounds)) { return true; } GrStencilClip stencilClip(args.fRenderTargetContext->dimensions(), diff --git a/src/gpu/text/GrTextBlob.cpp b/src/gpu/text/GrTextBlob.cpp index 2d70bf36b8..565d605f99 100644 --- a/src/gpu/text/GrTextBlob.cpp +++ b/src/gpu/text/GrTextBlob.cpp @@ -441,29 +441,25 @@ void GrTextBlob::SubRun::insertSubRunOpsIntoTarget(GrTextTarget* target, bool skipClip = false; SkIRect clipRect = SkIRect::MakeEmpty(); SkRect rtBounds = SkRect::MakeWH(target->width(), target->height()); + SkRRect clipRRect = SkRRect::MakeRect(rtBounds); + GrAA aa; // We can clip geometrically if we're not using SDFs or transformed glyphs, // and we have an axis-aligned rectangular non-AA clip - if (!this->drawAsDistanceFields() && !this->needsTransform()) { + if (!this->drawAsDistanceFields() && + !this->needsTransform() && + (!clip || (clip->isRRect(&clipRRect, &aa) && + clipRRect.isRect() && GrAA::kNo == aa))) { // We only need to do clipping work if the subrun isn't contained by the clip - skipClip = true; SkRect subRunBounds = this->deviceRect(deviceMatrix.localToDevice(), drawOrigin); - if (!clip && !rtBounds.intersects(subRunBounds)) { + if (!clipRRect.getBounds().contains(subRunBounds)) { // If the subrun is completely outside, don't add an op for it - return; - } else if (clip) { - GrClip::PreClipResult result = clip->preApply(subRunBounds); - if (result.fEffect == GrClip::Effect::kClipped) { - if (result.fIsRRect && result.fRRect.isRect() && result.fAA == GrAA::kNo) { - // Embed non-AA axis-aligned clip into the draw - result.fRRect.getBounds().round(&clipRect); - } else { - // Can't actually skip the regular clipping - skipClip = false; - } - } else if (result.fEffect == GrClip::Effect::kClippedOut) { + if (!clipRRect.getBounds().intersects(subRunBounds)) { return; + } else { + clipRRect.getBounds().round(&clipRect); } } + skipClip = true; } auto op = this->makeOp(deviceMatrix, drawOrigin, clipRect, paint, props, target); diff --git a/tests/GrCCPRTest.cpp b/tests/GrCCPRTest.cpp index 7561981f28..4f1b6d8172 100644 --- a/tests/GrCCPRTest.cpp +++ b/tests/GrCCPRTest.cpp @@ -40,15 +40,16 @@ public: private: SkIRect getConservativeBounds() const final { return fPath.getBounds().roundOut(); } - Effect apply(GrRecordingContext* context, GrRenderTargetContext* rtc, bool useHWAA, - bool hasUserStencilSettings, GrAppliedClip* out, - SkRect* bounds) const override { + bool apply(GrRecordingContext* context, GrRenderTargetContext* rtc, bool useHWAA, + bool hasUserStencilSettings, GrAppliedClip* out, SkRect* bounds) const override { out->addCoverageFP(fCCPR->makeClipProcessor(/*inputFP=*/nullptr, rtc->priv().testingOnly_getOpsTaskID(), fPath, SkIRect::MakeWH(rtc->width(), rtc->height()), *context->priv().caps())); - return Effect::kClipped; + return true; } + bool quickContains(const SkRect&) const final { return false; } + bool isRRect(SkRRect* rr, GrAA*) const final { return false; } GrCoverageCountingPathRenderer* const fCCPR; const SkPath fPath; diff --git a/tests/LazyProxyTest.cpp b/tests/LazyProxyTest.cpp index 77965a4b34..9bb4bffa54 100644 --- a/tests/LazyProxyTest.cpp +++ b/tests/LazyProxyTest.cpp @@ -178,13 +178,15 @@ public: SkIRect getConservativeBounds() const final { return SkIRect::MakeSize(fAtlas->dimensions()); } - Effect apply(GrRecordingContext* context, GrRenderTargetContext*, bool useHWAA, - bool hasUserStencilSettings, GrAppliedClip* out, - SkRect* bounds) const override { + + bool apply(GrRecordingContext* context, GrRenderTargetContext*, bool useHWAA, + bool hasUserStencilSettings, GrAppliedClip* out, SkRect* bounds) const override { GrProxyProvider* proxyProvider = context->priv().proxyProvider(); out->addCoverageFP(std::make_unique(context, proxyProvider, fTest, fAtlas)); - return Effect::kClipped; + return true; } + bool quickContains(const SkRect&) const final { return false; } + bool isRRect(SkRRect* rr, GrAA*) const final { return false; } LazyProxyTest* const fTest; GrTextureProxy* fAtlas;