Convert GrAppliedClip interface to builder style
GrAppliedClip was about at its limit for how many "make" functions it
could have. Window rectangles would push it over the edge. This change
makes it so GrDrawTarget supplies the original draw bounds to the
constructor, and then GrClip adds the various required clipping
techniques.
BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2246113002
Review-Url: https://codereview.chromium.org/2246113002
diff --git a/src/gpu/GrClip.cpp b/src/gpu/GrClip.cpp
index 3eccd80..ef9c9cd 100644
--- a/src/gpu/GrClip.cpp
+++ b/src/gpu/GrClip.cpp
@@ -21,10 +21,7 @@
if (fHasStencilClip) {
return false;
}
- if (!fScissorState.enabled()) {
- return true;
- }
- return fScissorState.rect().contains(rect);
+ return !fScissorState.enabled() || GrClip::IsInsideClip(fScissorState.rect(), rect);
}
void GrFixedClip::getConservativeBounds(int width, int height, SkIRect* devResult,
@@ -40,32 +37,25 @@
}
}
-bool GrFixedClip::apply(GrContext*,
- GrDrawContext* drawContext,
- const SkRect* devBounds,
- bool isHWAntiAlias,
- bool hasUserStencilSettings,
- GrAppliedClip* out) const {
- SkASSERT(!fDeviceBounds.isLargest());
+bool GrFixedClip::apply(GrContext*, GrDrawContext* drawContext, bool isHWAntiAlias,
+ bool hasUserStencilSettings, GrAppliedClip* out) const {
if (fScissorState.enabled()) {
SkIRect tightScissor;
if (!tightScissor.intersect(fScissorState.rect(),
SkIRect::MakeWH(drawContext->width(), drawContext->height()))) {
return false;
}
- if (devBounds && IsOutsideClip(tightScissor, *devBounds)) {
+ if (IsOutsideClip(tightScissor, out->clippedDrawBounds())) {
return false;
}
- if (!devBounds || !IsInsideClip(fScissorState.rect(), *devBounds)) {
- if (fHasStencilClip) {
- out->makeScissoredStencil(tightScissor, &fDeviceBounds);
- } else {
- out->makeScissored(tightScissor);
- }
- return true;
+ if (!IsInsideClip(fScissorState.rect(), out->clippedDrawBounds())) {
+ out->addScissor(tightScissor);
}
}
- out->makeStencil(fHasStencilClip, fDeviceBounds);
+ if (fHasStencilClip) {
+ out->addStencilClip();
+ }
+
return true;
}
diff --git a/src/gpu/GrClipStackClip.cpp b/src/gpu/GrClipStackClip.cpp
index e117144..6cb216e 100644
--- a/src/gpu/GrClipStackClip.cpp
+++ b/src/gpu/GrClipStackClip.cpp
@@ -242,18 +242,14 @@
////////////////////////////////////////////////////////////////////////////////
// sort out what kind of clip mask needs to be created: alpha, stencil,
// scissor, or entirely software
-bool GrClipStackClip::apply(GrContext* context,
- GrDrawContext* drawContext,
- const SkRect* origDevBounds,
- bool useHWAA,
- bool hasUserStencilSettings,
- GrAppliedClip* out) const {
+bool GrClipStackClip::apply(GrContext* context, GrDrawContext* drawContext, bool useHWAA,
+ bool hasUserStencilSettings, GrAppliedClip* out) const {
if (!fStack || fStack->isWideOpen()) {
return true;
}
SkRect devBounds = SkRect::MakeIWH(drawContext->width(), drawContext->height());
- if (origDevBounds && !devBounds.intersect(*origDevBounds)) {
+ if (!devBounds.intersect(out->clippedDrawBounds())) {
return false;
}
@@ -263,18 +259,19 @@
SkRect clipSpaceDevBounds = devBounds.makeOffset(clipX, clipY);
const GrReducedClip reducedClip(*fStack, clipSpaceDevBounds);
- if (reducedClip.elements().isEmpty()) {
- if (GrReducedClip::InitialState::kAllOut == reducedClip.initialState()) {
- return false;
- }
- if (!GrClip::IsInsideClip(reducedClip.iBounds(), clipSpaceDevBounds)) {
- SkIRect scissorSpaceIBounds(reducedClip.iBounds());
- scissorSpaceIBounds.offset(-fOrigin);
- out->makeScissored(scissorSpaceIBounds);
- }
- return true;
+ if (reducedClip.hasIBounds() &&
+ !GrClip::IsInsideClip(reducedClip.ibounds(), clipSpaceDevBounds)) {
+ SkIRect scissorSpaceIBounds(reducedClip.ibounds());
+ scissorSpaceIBounds.offset(-fOrigin);
+ out->addScissor(scissorSpaceIBounds);
}
+ if (reducedClip.elements().isEmpty()) {
+ return InitialState::kAllIn == reducedClip.initialState();
+ }
+
+ SkASSERT(reducedClip.hasIBounds());
+
// An element count of 4 was chosen because of the common pattern in Blink of:
// isect RR
// diff RR
@@ -297,13 +294,7 @@
if (reducedClip.requiresAA() &&
get_analytic_clip_processor(reducedClip.elements(), disallowAnalyticAA,
{-clipX, -clipY}, devBounds, &clipFP)) {
- SkIRect scissorSpaceIBounds(reducedClip.iBounds());
- scissorSpaceIBounds.offset(-fOrigin);
- if (GrClip::IsInsideClip(scissorSpaceIBounds, devBounds)) {
- out->makeFPBased(std::move(clipFP), SkRect::Make(scissorSpaceIBounds));
- } else {
- out->makeScissoredFPBased(std::move(clipFP), scissorSpaceIBounds);
- }
+ out->addCoverageFP(std::move(clipFP));
return true;
}
}
@@ -333,10 +324,9 @@
if (result) {
// The mask's top left coord should be pinned to the rounded-out top left corner of
// clipSpace bounds. We determine the mask's position WRT to the render target here.
- SkIRect rtSpaceMaskBounds = reducedClip.iBounds();
+ SkIRect rtSpaceMaskBounds = reducedClip.ibounds();
rtSpaceMaskBounds.offset(-fOrigin);
- out->makeFPBased(create_fp_for_mask(result.get(), rtSpaceMaskBounds),
- SkRect::Make(rtSpaceMaskBounds));
+ out->addCoverageFP(create_fp_for_mask(result.get(), rtSpaceMaskBounds));
return true;
}
// if alpha clip mask creation fails fall through to the non-AA code paths
@@ -345,17 +335,7 @@
// use the stencil clip if we can't represent the clip as a rectangle.
SkIPoint clipSpaceToStencilSpaceOffset = -fOrigin;
CreateStencilClipMask(context, drawContext, reducedClip, clipSpaceToStencilSpaceOffset);
-
- // This must occur after createStencilClipMask. That function may change the scissor. Also, it
- // only guarantees that the stencil mask is correct within the bounds it was passed, so we must
- // use both stencil and scissor test to the bounds for the final draw.
- if (GrClip::IsInsideClip(reducedClip.iBounds(), clipSpaceDevBounds)) {
- out->makeStencil(true, devBounds);
- } else {
- SkIRect scissorSpaceIBounds(reducedClip.iBounds());
- scissorSpaceIBounds.offset(clipSpaceToStencilSpaceOffset);
- out->makeScissoredStencil(scissorSpaceIBounds);
- }
+ out->addStencilClip();
return true;
}
@@ -438,7 +418,7 @@
const SkVector& clipToMaskOffset) {
GrResourceProvider* resourceProvider = context->resourceProvider();
GrUniqueKey key;
- GetClipMaskKey(reducedClip.genID(), reducedClip.iBounds(), &key);
+ GetClipMaskKey(reducedClip.genID(), reducedClip.ibounds(), &key);
if (GrTexture* texture = resourceProvider->findAndRefTextureByUniqueKey(key)) {
return sk_sp<GrTexture>(texture);
}
@@ -463,9 +443,7 @@
// The scratch texture that we are drawing into can be substantially larger than the mask. Only
// clear the part that we care about.
- dc->clear(&maskSpaceIBounds,
- GrReducedClip::InitialState::kAllIn == reducedClip.initialState() ? -1 : 0,
- true);
+ dc->clear(&maskSpaceIBounds, InitialState::kAllIn == reducedClip.initialState() ? -1 : 0, true);
// Set the matrix so that rendered clip elements are transformed to mask space from clip
// space.
@@ -513,7 +491,7 @@
if (!dc->drawContextPriv().drawAndStencilRect(clip, &kDrawOutsideElement,
op, !invert, false,
translate,
- SkRect::Make(reducedClip.iBounds()))) {
+ SkRect::Make(reducedClip.ibounds()))) {
return nullptr;
}
} else {
@@ -548,9 +526,9 @@
}
// TODO: these need to be swapped over to using a StencilAttachmentProxy
- if (stencilAttachment->mustRenderClip(reducedClip.genID(), reducedClip.iBounds(),
+ if (stencilAttachment->mustRenderClip(reducedClip.genID(), reducedClip.ibounds(),
clipSpaceToStencilOffset)) {
- stencilAttachment->setLastClip(reducedClip.genID(), reducedClip.iBounds(),
+ stencilAttachment->setLastClip(reducedClip.genID(), reducedClip.ibounds(),
clipSpaceToStencilOffset);
// Set the matrix so that rendered clip elements are transformed from clip to stencil space.
SkVector translate = {
@@ -561,11 +539,11 @@
viewMatrix.setTranslate(translate);
// We set the current clip to the bounds so that our recursive draws are scissored to them.
- SkIRect stencilSpaceIBounds(reducedClip.iBounds());
+ SkIRect stencilSpaceIBounds(reducedClip.ibounds());
stencilSpaceIBounds.offset(clipSpaceToStencilOffset);
GrFixedClip clip(stencilSpaceIBounds);
- bool insideClip = GrReducedClip::InitialState::kAllIn == reducedClip.initialState();
+ bool insideClip = InitialState::kAllIn == reducedClip.initialState();
drawContext->drawContextPriv().clearStencilClip(stencilSpaceIBounds, insideClip);
// walk through each clip element and perform its set op
@@ -671,22 +649,19 @@
}
}
+ // Just enable stencil clip. The passes choose whether or not they will actually use it.
+ clip.enableStencilClip();
+
// now we modify the clip bit by rendering either the clip
// element directly or a bounding rect of the entire clip.
for (GrUserStencilSettings const* const* pass = stencilPasses; *pass; ++pass) {
-
if (drawDirectToClip) {
if (Element::kRect_Type == element->getType()) {
- clip.enableStencilClip(element->getRect().makeOffset(translate.fX,
- translate.fY));
drawContext->drawContextPriv().stencilRect(clip, *pass, useHWAA, viewMatrix,
element->getRect());
} else {
GrShape shape(clipPath, GrStyle::SimpleFill());
GrPaint paint;
- SkRect bounds = clipPath.getBounds();
- bounds.offset(translate.fX, translate.fY);
- clip.enableStencilClip(bounds);
paint.setXPFactory(GrDisableColorXPFactory::Make());
paint.setAntiAlias(element->isAA());
GrPathRenderer::DrawPathArgs args;
@@ -704,11 +679,8 @@
} else {
// The view matrix is setup to do clip space -> stencil space translation, so
// draw rect in clip space.
- SkRect bounds = SkRect::Make(reducedClip.iBounds());
- bounds.offset(translate.fX, translate.fY);
- clip.enableStencilClip(bounds);
drawContext->drawContextPriv().stencilRect(clip, *pass, false, viewMatrix,
- SkRect::Make(reducedClip.iBounds()));
+ SkRect::Make(reducedClip.ibounds()));
}
}
}
@@ -721,7 +693,7 @@
const GrReducedClip& reducedClip,
const SkVector& clipToMaskOffset) {
GrUniqueKey key;
- GetClipMaskKey(reducedClip.genID(), reducedClip.iBounds(), &key);
+ GetClipMaskKey(reducedClip.genID(), reducedClip.ibounds(), &key);
if (GrTexture* texture = texProvider->findAndRefTextureByUniqueKey(key)) {
return sk_sp<GrTexture>(texture);
}
@@ -738,7 +710,7 @@
translate.setTranslate(clipToMaskOffset);
helper.init(maskSpaceIBounds, &translate);
- helper.clear(GrReducedClip::InitialState::kAllIn == reducedClip.initialState() ? 0xFF : 0x00);
+ helper.clear(InitialState::kAllIn == reducedClip.initialState() ? 0xFF : 0x00);
for (ElementList::Iter iter(reducedClip.elements()); iter.get(); iter.next()) {
const Element* element = iter.get();
@@ -750,7 +722,7 @@
// but leave the pixels inside the geometry alone. For reverse difference we invert all
// the pixels before clearing the ones outside the geometry.
if (SkRegion::kReverseDifference_Op == op) {
- SkRect temp = SkRect::Make(reducedClip.iBounds());
+ SkRect temp = SkRect::Make(reducedClip.ibounds());
// invert the entire scene
helper.drawRect(temp, SkRegion::kXOR_Op, false, 0xFF);
}
diff --git a/src/gpu/GrClipStackClip.h b/src/gpu/GrClipStackClip.h
index aaa2f90..98675e6 100644
--- a/src/gpu/GrClipStackClip.h
+++ b/src/gpu/GrClipStackClip.h
@@ -33,11 +33,7 @@
bool quickContains(const SkRect&) const final;
void getConservativeBounds(int width, int height, SkIRect* devResult,
bool* isIntersectionOfRects) const final;
- bool apply(GrContext*,
- GrDrawContext*,
- const SkRect* devBounds,
- bool useHWAA,
- bool hasUserStencilSettings,
+ bool apply(GrContext*, GrDrawContext*, bool useHWAA, bool hasUserStencilSettings,
GrAppliedClip* out) const final;
private:
diff --git a/src/gpu/GrDrawTarget.cpp b/src/gpu/GrDrawTarget.cpp
index e143f90..b4142eb 100644
--- a/src/gpu/GrDrawTarget.cpp
+++ b/src/gpu/GrDrawTarget.cpp
@@ -321,51 +321,38 @@
}
}
-static inline bool intersect(SkRect* out, const SkRect& a, const SkRect& b) {
- SkASSERT(a.fLeft <= a.fRight && a.fTop <= a.fBottom);
- SkASSERT(b.fLeft <= b.fRight && b.fTop <= b.fBottom);
- out->fLeft = SkTMax(a.fLeft, b.fLeft);
- out->fTop = SkTMax(a.fTop, b.fTop);
- out->fRight = SkTMin(a.fRight, b.fRight);
- out->fBottom = SkTMin(a.fBottom, b.fBottom);
- return (out->fLeft <= out->fRight && out->fTop <= out->fBottom);
-}
-
void GrDrawTarget::drawBatch(const GrPipelineBuilder& pipelineBuilder,
GrDrawContext* drawContext,
const GrClip& clip,
GrDrawBatch* batch) {
// Setup clip
- GrAppliedClip appliedClip;
SkRect bounds;
batch_bounds(&bounds, batch);
- if (!clip.apply(fContext, drawContext, &bounds,
- pipelineBuilder.isHWAntialias(), pipelineBuilder.hasUserStencilSettings(),
- &appliedClip)) {
+ GrAppliedClip appliedClip(bounds);
+ if (!clip.apply(fContext, drawContext, pipelineBuilder.isHWAntialias(),
+ pipelineBuilder.hasUserStencilSettings(), &appliedClip)) {
return;
}
// TODO: this is the only remaining usage of the AutoRestoreFragmentProcessorState - remove it
GrPipelineBuilder::AutoRestoreFragmentProcessorState arfps;
- if (appliedClip.getClipCoverageFragmentProcessor()) {
+ if (appliedClip.clipCoverageFragmentProcessor()) {
arfps.set(&pipelineBuilder);
- arfps.addCoverageFragmentProcessor(sk_ref_sp(appliedClip.getClipCoverageFragmentProcessor()));
+ arfps.addCoverageFragmentProcessor(sk_ref_sp(appliedClip.clipCoverageFragmentProcessor()));
}
- GrPipeline::CreateArgs args;
- args.fPipelineBuilder = &pipelineBuilder;
- args.fDrawContext = drawContext;
- args.fCaps = this->caps();
- args.fScissor = &appliedClip.scissorState();
- args.fHasStencilClip = appliedClip.hasStencilClip();
if (pipelineBuilder.hasUserStencilSettings() || appliedClip.hasStencilClip()) {
if (!fResourceProvider->attachStencilAttachment(drawContext->accessRenderTarget())) {
SkDebugf("ERROR creating stencil attachment. Draw skipped.\n");
return;
}
}
+
+ GrPipeline::CreateArgs args;
+ args.fPipelineBuilder = &pipelineBuilder;
+ args.fDrawContext = drawContext;
+ args.fCaps = this->caps();
batch->getPipelineOptimizations(&args.fOpts);
- GrScissorState finalScissor;
if (args.fOpts.fOverrides.fUsePLSDstRead || fClipBatchToBounds) {
GrGLIRect viewport;
viewport.fLeft = 0;
@@ -381,14 +368,9 @@
viewport.fWidth);
ibounds.fBottom = SkTPin(SkScalarCeilToInt(batch->bounds().fBottom), viewport.fBottom,
viewport.fHeight);
- if (appliedClip.scissorState().enabled()) {
- const SkIRect& scissorRect = appliedClip.scissorState().rect();
- if (!ibounds.intersect(scissorRect)) {
- return;
- }
+ if (!appliedClip.addScissor(ibounds)) {
+ return;
}
- finalScissor.set(ibounds);
- args.fScissor = &finalScissor;
}
args.fOpts.fColorPOI.completeCalculations(
sk_sp_address_as_pointer_address(pipelineBuilder.fColorFragmentProcessors.begin()),
@@ -396,6 +378,8 @@
args.fOpts.fCoveragePOI.completeCalculations(
sk_sp_address_as_pointer_address(pipelineBuilder.fCoverageFragmentProcessors.begin()),
pipelineBuilder.numCoverageFragmentProcessors());
+ args.fScissor = &appliedClip.scissorState();
+ args.fHasStencilClip = appliedClip.hasStencilClip();
if (!this->setupDstReadIfNecessary(pipelineBuilder, drawContext->accessRenderTarget(),
clip, args.fOpts,
&args.fDstTexture, batch->bounds())) {
@@ -410,9 +394,7 @@
SkASSERT(fRenderTarget);
batch->pipeline()->addDependenciesTo(fRenderTarget);
#endif
- SkRect clippedBounds;
- SkAssertResult(intersect(&clippedBounds, bounds, appliedClip.deviceBounds()));
- this->recordBatch(batch, clippedBounds);
+ this->recordBatch(batch, appliedClip.clippedDrawBounds());
}
void GrDrawTarget::stencilPath(GrDrawContext* drawContext,
@@ -424,16 +406,20 @@
SkASSERT(path);
SkASSERT(this->caps()->shaderCaps()->pathRenderingSupport());
+ // FIXME: Use path bounds instead of this WAR once
+ // https://bugs.chromium.org/p/skia/issues/detail?id=5640 is resolved.
+ SkRect bounds = SkRect::MakeIWH(drawContext->width(), drawContext->height());
+
// Setup clip
- GrAppliedClip appliedClip;
- if (!clip.apply(fContext, drawContext, nullptr, useHWAA, true, &appliedClip)) {
+ GrAppliedClip appliedClip(bounds);
+ if (!clip.apply(fContext, drawContext, useHWAA, true, &appliedClip)) {
return;
}
// TODO: respect fClipBatchToBounds if we ever start computing bounds here.
// Coverage AA does not make sense when rendering to the stencil buffer. The caller should never
// attempt this in a situation that would require coverage AA.
- SkASSERT(!appliedClip.getClipCoverageFragmentProcessor());
+ SkASSERT(!appliedClip.clipCoverageFragmentProcessor());
GrStencilAttachment* stencilAttachment = fResourceProvider->attachStencilAttachment(
drawContext->accessRenderTarget());
@@ -450,7 +436,7 @@
appliedClip.scissorState(),
drawContext->accessRenderTarget(),
path);
- this->recordBatch(batch, appliedClip.deviceBounds());
+ this->recordBatch(batch, appliedClip.clippedDrawBounds());
batch->unref();
}
diff --git a/src/gpu/GrReducedClip.cpp b/src/gpu/GrReducedClip.cpp
index f7bac4a..132936c 100644
--- a/src/gpu/GrReducedClip.cpp
+++ b/src/gpu/GrReducedClip.cpp
@@ -343,9 +343,7 @@
// generation id. When we make early returns, we do not know what was the generation
// id that lead to the state. Make a conservative guess.
fGenID = stack.getTopmostGenID();
-
- // TODO: instead devise a way of telling the caller to disregard some or all of the clip bounds.
- fIBounds = GrClip::GetPixelIBounds(queryBounds);
+ fHasIBounds = false;
if (stack.isWideOpen()) {
fInitialState = InitialState::kAllIn;
@@ -372,6 +370,7 @@
// The clip is a non-aa rect. This is the one spot where we can actually implement the
// clip (using fIBounds) rather than just telling the caller what it should be.
stackBounds.round(&fIBounds);
+ fHasIBounds = true;
fInitialState = fIBounds.isEmpty() ? InitialState::kAllOut : InitialState::kAllIn;
return;
}
@@ -380,8 +379,11 @@
return;
}
- SkAssertResult(fIBounds.intersect(GrClip::GetPixelIBounds(stackBounds)));
+ SkRect tightBounds;
+ SkAssertResult(tightBounds.intersect(stackBounds, queryBounds));
+ fIBounds = GrClip::GetPixelIBounds(tightBounds);
SkASSERT(!fIBounds.isEmpty()); // Empty should have been blocked by IsOutsideClip above.
+ fHasIBounds = true;
// Implement the clip with an AA rect element.
fElements.addToHead(stackBounds, SkRegion::kReplace_Op, true/*doAA*/);
@@ -396,10 +398,11 @@
// Tighten the query by introducing a new clip at the stack's pixel boundaries. (This new
// clip will be enforced by the scissor through fIBounds.)
SkAssertResult(tighterQuery.intersect(GrClip::GetPixelBounds(stackBounds)));
- fIBounds = GrClip::GetPixelIBounds(tighterQuery);
}
+ fIBounds = GrClip::GetPixelIBounds(tighterQuery);
SkASSERT(!fIBounds.isEmpty()); // Empty should have been blocked by IsOutsideClip above.
+ fHasIBounds = true;
// Now that we have determined the bounds to use and filtered out the trivial cases, call the
// helper that actually walks the stack.
diff --git a/src/gpu/GrReducedClip.h b/src/gpu/GrReducedClip.h
index 07e0669..d7b7ea8 100644
--- a/src/gpu/GrReducedClip.h
+++ b/src/gpu/GrReducedClip.h
@@ -25,14 +25,20 @@
int32_t genID() const { return fGenID; }
/**
- * Bounding box within which the reduced clip is valid. The caller must not draw any pixels
- * outside this box.
+ * If hasIBounds() is true, this is the bounding box within which the reduced clip is valid, and
+ * the caller must not modify any pixels outside this box. Undefined if hasIBounds() is false.
*/
- const SkIRect& iBounds() const { return fIBounds; }
- int left() const { return this->iBounds().left(); }
- int top() const { return this->iBounds().top(); }
- int width() const { return this->iBounds().width(); }
- int height() const { return this->iBounds().height(); }
+ const SkIRect& ibounds() const { SkASSERT(fHasIBounds); return fIBounds; }
+ int left() const { return this->ibounds().left(); }
+ int top() const { return this->ibounds().top(); }
+ int width() const { return this->ibounds().width(); }
+ int height() const { return this->ibounds().height(); }
+
+ /**
+ * Indicates whether ibounds() are defined. They will always be defined if the elements() are
+ * nonempty.
+ */
+ bool hasIBounds() const { return fHasIBounds; }
typedef SkTLList<SkClipStack::Element, 16> ElementList;
@@ -51,14 +57,12 @@
kAllOut
};
- /**
- * The initial state of the clip within iBounds().
- */
InitialState initialState() const { return fInitialState; }
private:
int32_t fGenID;
SkIRect fIBounds;
+ bool fHasIBounds;
ElementList fElements;
bool fRequiresAA;
InitialState fInitialState;