From 97180af7f5e577a451367aa0c15cfaff1b6076f5 Mon Sep 17 00:00:00 2001 From: Brian Salomon Date: Tue, 14 Mar 2017 13:42:58 -0400 Subject: [PATCH] Compute clipped draw bounds outside GrAppliedClip. We will be storing GrAppliedClips alongside ops. The op already stores the clipped bounds. If GrAppliedClip has draw bounds then as ops combine the GrAppliedClip's bounds should be merged to be consistent. However, we won't actually ever use those bounds again so it would be wasteful to merge them. Change-Id: I4ef3010dc04761e256120a2e0e074bc3c6ff6ca1 Reviewed-on: https://skia-review.googlesource.com/9642 Commit-Queue: Brian Salomon Reviewed-by: Chris Dalton --- gm/windowrectangles.cpp | 6 ++++-- src/gpu/GrAppliedClip.h | 20 +++++--------------- src/gpu/GrClip.h | 14 ++++++++++++-- src/gpu/GrClipStackClip.cpp | 10 +++++----- src/gpu/GrClipStackClip.h | 2 +- src/gpu/GrFixedClip.cpp | 10 +++++----- src/gpu/GrFixedClip.h | 3 ++- src/gpu/GrReducedClip.cpp | 5 +++-- src/gpu/GrRenderTargetContext.cpp | 12 ++++++------ tests/GpuSampleLocationsTest.cpp | 2 +- tests/GrPorterDuffTest.cpp | 2 +- 11 files changed, 45 insertions(+), 41 deletions(-) diff --git a/gm/windowrectangles.cpp b/gm/windowrectangles.cpp index b24febd71c..202836f18a 100644 --- a/gm/windowrectangles.cpp +++ b/gm/windowrectangles.cpp @@ -155,7 +155,8 @@ public: SkIRect::MakeWH(w, h), {x, y}); } private: - bool apply(GrContext*, GrRenderTargetContext*, bool, bool, GrAppliedClip* out) const override { + bool apply(GrContext*, GrRenderTargetContext*, bool, bool, GrAppliedClip* out, + SkRect* bounds) const override { out->addCoverageFP(fFP); return true; } @@ -167,7 +168,8 @@ private: */ class StencilOnlyClip final : public MaskOnlyClipBase { private: - bool apply(GrContext*, GrRenderTargetContext*, bool, bool, GrAppliedClip* out) const override { + bool apply(GrContext*, GrRenderTargetContext*, bool, bool, GrAppliedClip* out, + SkRect* bounds) const override { out->addStencilClip(); return true; } diff --git a/src/gpu/GrAppliedClip.h b/src/gpu/GrAppliedClip.h index 4786f6434f..57314ec7d4 100644 --- a/src/gpu/GrAppliedClip.h +++ b/src/gpu/GrAppliedClip.h @@ -18,11 +18,6 @@ */ class GrAppliedClip : public SkNoncopyable { public: - GrAppliedClip(const SkRect& drawBounds) - : fHasStencilClip(false) - , fClippedDrawBounds(drawBounds) { - } - const GrScissorState& scissorState() const { return fScissorState; } const GrWindowRectsState& windowRectsState() const { return fWindowRectsState; } GrFragmentProcessor* clipCoverageFragmentProcessor() const { return fClipCoverageFP.get(); } @@ -30,9 +25,11 @@ public: /** * Intersects the applied clip with the provided rect. Returns false if the draw became empty. + * 'clippedDrawBounds' will be intersected with 'irect'. This returns false if the clip becomes + * empty or the draw no longer intersects the clip. In either case the draw can be skipped. */ - bool addScissor(const SkIRect& irect) { - return fScissorState.intersect(irect) && fClippedDrawBounds.intersect(SkRect::Make(irect)); + bool addScissor(const SkIRect& irect, SkRect* clippedDrawBounds) { + return fScissorState.intersect(irect) && clippedDrawBounds->intersect(SkRect::Make(irect)); } void addWindowRectangles(const GrWindowRectsState& windowState) { @@ -55,18 +52,11 @@ public: fHasStencilClip = true; } - /** - * Returns the device bounds of the draw after clip has been applied. TODO: Ideally this would - * consider the combined effect of all clipping techniques in play (scissor, stencil, fp, etc.). - */ - const SkRect& clippedDrawBounds() const { return fClippedDrawBounds; } - private: GrScissorState fScissorState; GrWindowRectsState fWindowRectsState; sk_sp fClipCoverageFP; - bool fHasStencilClip; - SkRect fClippedDrawBounds; + bool fHasStencilClip = false; typedef SkNoncopyable INHERITED; }; diff --git a/src/gpu/GrClip.h b/src/gpu/GrClip.h index eb4eab2d0c..c44653baad 100644 --- a/src/gpu/GrClip.h +++ b/src/gpu/GrClip.h @@ -28,8 +28,17 @@ public: } virtual void getConservativeBounds(int width, int height, SkIRect* devResult, bool* isIntersectionOfRects = nullptr) const = 0; + /** + * 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. 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 bool apply(GrContext*, GrRenderTargetContext*, bool useHWAA, - bool hasUserStencilSettings, GrAppliedClip* out) const = 0; + bool hasUserStencilSettings, GrAppliedClip* result, + SkRect* bounds) const = 0; virtual ~GrClip() {} @@ -134,7 +143,8 @@ private: *isIntersectionOfRects = true; } } - bool apply(GrContext*, GrRenderTargetContext*, bool, bool, GrAppliedClip*) const final { + bool apply(GrContext*, GrRenderTargetContext*, bool, bool, GrAppliedClip*, + 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 d44373f957..e324770f62 100644 --- a/src/gpu/GrClipStackClip.cpp +++ b/src/gpu/GrClipStackClip.cpp @@ -247,14 +247,14 @@ static bool get_analytic_clip_processor(const ElementList& elements, // sort out what kind of clip mask needs to be created: alpha, stencil, // scissor, or entirely software bool GrClipStackClip::apply(GrContext* context, GrRenderTargetContext* renderTargetContext, - bool useHWAA, bool hasUserStencilSettings, GrAppliedClip* out) const { + bool useHWAA, bool hasUserStencilSettings, GrAppliedClip* out, + SkRect* bounds) const { if (!fStack || fStack->isWideOpen()) { return true; } - SkRect devBounds = SkRect::MakeIWH(renderTargetContext->width(), - renderTargetContext->height()); - if (!devBounds.intersect(out->clippedDrawBounds())) { + SkRect devBounds = SkRect::MakeIWH(renderTargetContext->width(), renderTargetContext->height()); + if (!devBounds.intersect(*bounds)) { return false; } @@ -262,7 +262,7 @@ bool GrClipStackClip::apply(GrContext* context, GrRenderTargetContext* renderTar renderTargetContext->priv().maxWindowRectangles()); if (reducedClip.hasIBounds() && !GrClip::IsInsideClip(reducedClip.ibounds(), devBounds)) { - out->addScissor(reducedClip.ibounds()); + out->addScissor(reducedClip.ibounds(), bounds); } if (!reducedClip.windowRectangles().empty()) { diff --git a/src/gpu/GrClipStackClip.h b/src/gpu/GrClipStackClip.h index a1365a7713..8058c3b8a4 100644 --- a/src/gpu/GrClipStackClip.h +++ b/src/gpu/GrClipStackClip.h @@ -29,7 +29,7 @@ public: void getConservativeBounds(int width, int height, SkIRect* devResult, bool* isIntersectionOfRects) const final; bool apply(GrContext*, GrRenderTargetContext*, bool useHWAA, bool hasUserStencilSettings, - GrAppliedClip* out) const final; + GrAppliedClip* out, SkRect* bounds) const final; bool isRRect(const SkRect& rtBounds, SkRRect* rr, GrAA* aa) const override; diff --git a/src/gpu/GrFixedClip.cpp b/src/gpu/GrFixedClip.cpp index d584ee119e..e551f9b966 100644 --- a/src/gpu/GrFixedClip.cpp +++ b/src/gpu/GrFixedClip.cpp @@ -45,18 +45,18 @@ bool GrFixedClip::isRRect(const SkRect& rtBounds, SkRRect* rr, GrAA* aa) const { return false; }; -bool GrFixedClip::apply(GrContext*, GrRenderTargetContext* rtc, - bool, bool, GrAppliedClip* out) const { +bool GrFixedClip::apply(GrContext*, GrRenderTargetContext* rtc, bool, bool, GrAppliedClip* out, + SkRect* bounds) const { if (fScissorState.enabled()) { SkIRect tightScissor = SkIRect::MakeWH(rtc->width(), rtc->height()); if (!tightScissor.intersect(fScissorState.rect())) { return false; } - if (IsOutsideClip(tightScissor, out->clippedDrawBounds())) { + if (IsOutsideClip(tightScissor, *bounds)) { return false; } - if (!IsInsideClip(fScissorState.rect(), out->clippedDrawBounds())) { - out->addScissor(tightScissor); + if (!IsInsideClip(fScissorState.rect(), *bounds)) { + out->addScissor(tightScissor, bounds); } } diff --git a/src/gpu/GrFixedClip.h b/src/gpu/GrFixedClip.h index 0f517525a0..744bb27a81 100644 --- a/src/gpu/GrFixedClip.h +++ b/src/gpu/GrFixedClip.h @@ -42,7 +42,8 @@ public: bool quickContains(const SkRect&) const override; void getConservativeBounds(int w, int h, SkIRect* devResult, bool* iior) const override; bool isRRect(const SkRect& rtBounds, SkRRect* rr, GrAA*) const override; - bool apply(GrContext*, GrRenderTargetContext*, bool, bool, GrAppliedClip* out) const override; + bool apply(GrContext*, GrRenderTargetContext*, bool, bool, GrAppliedClip*, + SkRect*) const override; static const GrFixedClip& Disabled(); diff --git a/src/gpu/GrReducedClip.cpp b/src/gpu/GrReducedClip.cpp index d322c6b6ca..4b7e69bc02 100644 --- a/src/gpu/GrReducedClip.cpp +++ b/src/gpu/GrReducedClip.cpp @@ -674,8 +674,9 @@ private: return false; } bool apply(GrContext* context, GrRenderTargetContext* renderTargetContext, bool useHWAA, - bool hasUserStencilSettings, GrAppliedClip* out) const override { - if (!fFixedClip.apply(context, renderTargetContext, useHWAA, hasUserStencilSettings, out)) { + bool hasUserStencilSettings, GrAppliedClip* out, SkRect* bounds) const override { + if (!fFixedClip.apply(context, renderTargetContext, useHWAA, hasUserStencilSettings, out, + bounds)) { return false; } out->addStencilClip(); diff --git a/src/gpu/GrRenderTargetContext.cpp b/src/gpu/GrRenderTargetContext.cpp index 7a6abe0e73..8c231429fe 100644 --- a/src/gpu/GrRenderTargetContext.cpp +++ b/src/gpu/GrRenderTargetContext.cpp @@ -712,9 +712,9 @@ void GrRenderTargetContextPriv::stencilPath(const GrClip& clip, SkRect bounds = SkRect::MakeIWH(fRenderTargetContext->width(), fRenderTargetContext->height()); // Setup clip - GrAppliedClip appliedClip(bounds); + GrAppliedClip appliedClip; if (!clip.apply(fRenderTargetContext->fContext, fRenderTargetContext, useHWAA, true, - &appliedClip)) { + &appliedClip, &bounds)) { return; } @@ -741,7 +741,7 @@ void GrRenderTargetContextPriv::stencilPath(const GrClip& clip, appliedClip.scissorState(), fRenderTargetContext->accessRenderTarget(), path); - op->setClippedBounds(appliedClip.clippedDrawBounds()); + op->setClippedBounds(bounds); fRenderTargetContext->getOpList()->recordOp(std::move(op), fRenderTargetContext); } @@ -1692,9 +1692,9 @@ uint32_t GrRenderTargetContext::addDrawOp(const GrPipelineBuilder& pipelineBuild // Setup clip SkRect bounds; op_bounds(&bounds, op.get()); - GrAppliedClip appliedClip(bounds); + GrAppliedClip appliedClip; if (!clip.apply(fContext, this, pipelineBuilder.isHWAntialias(), - pipelineBuilder.hasUserStencilSettings(), &appliedClip)) { + pipelineBuilder.hasUserStencilSettings(), &appliedClip, &bounds)) { return SK_InvalidUniqueID; } @@ -1731,7 +1731,7 @@ uint32_t GrRenderTargetContext::addDrawOp(const GrPipelineBuilder& pipelineBuild } op->initPipeline(args); // TODO: We need to add pipeline dependencies on textures, etc before recording this op. - op->setClippedBounds(appliedClip.clippedDrawBounds()); + op->setClippedBounds(bounds); return this->getOpList()->addOp(std::move(op), this); } diff --git a/tests/GpuSampleLocationsTest.cpp b/tests/GpuSampleLocationsTest.cpp index e3764a5977..8808b1d8cb 100644 --- a/tests/GpuSampleLocationsTest.cpp +++ b/tests/GpuSampleLocationsTest.cpp @@ -96,7 +96,7 @@ static void construct_dummy_pipeline(GrRenderTargetContext* dc, GrPipeline* pipe GrScissorState dummyScissor; GrWindowRectsState dummyWindows; - GrAppliedClip dummyAppliedClip(SkRect::MakeLargest()); + GrAppliedClip dummyAppliedClip; GrProcessorSet::FragmentProcessorAnalysis analysis; GrPipeline::InitArgs args; dummyBuilder.getPipelineInitArgs(&args); diff --git a/tests/GrPorterDuffTest.cpp b/tests/GrPorterDuffTest.cpp index b09192e3bf..ddca03a497 100644 --- a/tests/GrPorterDuffTest.cpp +++ b/tests/GrPorterDuffTest.cpp @@ -968,7 +968,7 @@ static void test_lcd_coverage_fallback_case(skiatest::Reporter* reporter, const } testLCDCoverageOp; GrProcessorSet::FragmentProcessorAnalysis analysis; - GrAppliedClip clip(SkRect::MakeLargest()); + GrAppliedClip clip; testLCDCoverageOp.analyzeProcessors(&analysis, GrProcessorSet(GrPaint()), &clip, caps); SkASSERT(analysis.hasKnownOutputColor()); -- 2.34.1