Fix ClipMaskManager's SW-fallback logic
'useSWOnlyPath' was not correctly toggling between stencil and color draws so there was a mismatch with the behavior in createAlphaClipMask (i.e., we were inadvertently rendering some of the elements in a clip using SW but using stenciling for others - precisely what 'useSWOnlyPath' was intended to prevent).
Review URL: https://codereview.chromium.org/1421533007
diff --git a/include/gpu/GrContext.h b/include/gpu/GrContext.h
index b886560..1e160c6 100644
--- a/include/gpu/GrContext.h
+++ b/include/gpu/GrContext.h
@@ -334,7 +334,7 @@
void getTestTarget(GrTestTarget*);
GrPathRenderer* getPathRenderer(
- const GrPipelineBuilder*,
+ const GrPipelineBuilder&,
const SkMatrix& viewMatrix,
const SkPath& path,
const GrStrokeInfo& stroke,
diff --git a/include/gpu/GrPathRendererChain.h b/include/gpu/GrPathRendererChain.h
index a50160f..98441ce 100644
--- a/include/gpu/GrPathRendererChain.h
+++ b/include/gpu/GrPathRendererChain.h
@@ -56,7 +56,7 @@
whether the path can be rendered with arbitrary stencil rules or not. See comments on
StencilSupport in GrPathRenderer.h. */
GrPathRenderer* getPathRenderer(const GrShaderCaps* shaderCaps,
- const GrPipelineBuilder*,
+ const GrPipelineBuilder&,
const SkMatrix& viewMatrix,
const SkPath& path,
const GrStrokeInfo& stroke,
diff --git a/src/gpu/GrClipMaskManager.cpp b/src/gpu/GrClipMaskManager.cpp
index b57f3cc..b87788b 100644
--- a/src/gpu/GrClipMaskManager.cpp
+++ b/src/gpu/GrClipMaskManager.cpp
@@ -46,24 +46,71 @@
kDevice_GrCoordSet);
}
+// Does the path in 'element' require SW rendering? If so, return true (and,
+// optionally, set 'prOut' to NULL. If not, return false (and, optionally, set
+// 'prOut' to the non-SW path renderer that will do the job).
static bool path_needs_SW_renderer(GrContext* context,
const GrPipelineBuilder& pipelineBuilder,
const SkMatrix& viewMatrix,
- const SkPath& origPath,
- const GrStrokeInfo& stroke,
- bool doAA) {
- // the gpu alpha mask will draw the inverse paths as non-inverse to a temp buffer
- SkTCopyOnFirstWrite<SkPath> path(origPath);
- if (path->isInverseFillType()) {
- path.writable()->toggleInverseFillType();
- }
- // last (false) parameter disallows use of the SW path renderer
- GrPathRendererChain::DrawType type = doAA ?
- GrPathRendererChain::kColorAntiAlias_DrawType :
- GrPathRendererChain::kColor_DrawType;
+ const Element* element,
+ GrPathRenderer** prOut,
+ bool needsStencil) {
+ if (Element::kRect_Type == element->getType()) {
+ // rects can always be drawn directly w/o using the software path
+ // TODO: skip rrects once we're drawing them directly.
+ return false;
+ } else {
+ // We shouldn't get here with an empty clip element.
+ SkASSERT(Element::kEmpty_Type != element->getType());
- return nullptr == context->getPathRenderer(&pipelineBuilder, viewMatrix, *path, stroke,
- false, type);
+ // the gpu alpha mask will draw the inverse paths as non-inverse to a temp buffer
+ SkPath path;
+ element->asPath(&path);
+ if (path.isInverseFillType()) {
+ path.toggleInverseFillType();
+ }
+ GrStrokeInfo stroke(SkStrokeRec::kFill_InitStyle);
+
+ GrPathRendererChain::DrawType type;
+
+ if (needsStencil) {
+ type = element->isAA()
+ ? GrPathRendererChain::kStencilAndColorAntiAlias_DrawType
+ : GrPathRendererChain::kStencilAndColor_DrawType;
+ } else {
+ type = element->isAA()
+ ? GrPathRendererChain::kColorAntiAlias_DrawType
+ : GrPathRendererChain::kColor_DrawType;
+ }
+
+ // the 'false' parameter disallows use of the SW path renderer
+ GrPathRenderer* pr = context->getPathRenderer(pipelineBuilder, viewMatrix, path,
+ stroke, false, type);
+ if (prOut) {
+ *prOut = pr;
+ }
+ return SkToBool(!pr);
+ }
+}
+
+// Determines whether it is possible to draw the element to both the stencil buffer and the
+// alpha mask simultaneously. If so and the element is a path a compatible path renderer is
+// also returned.
+static bool can_stencil_and_draw_element(GrContext* context,
+ GrPipelineBuilder* pipelineBuilder,
+ GrTexture* texture,
+ const SkMatrix& viewMatrix,
+ const SkClipStack::Element* element,
+ GrPathRenderer** pr) {
+ pipelineBuilder->setRenderTarget(texture->asRenderTarget());
+
+ static const bool kNeedsStencil = true;
+ return !path_needs_SW_renderer(context,
+ *pipelineBuilder,
+ viewMatrix,
+ element,
+ pr,
+ kNeedsStencil);
}
GrClipMaskManager::GrClipMaskManager(GrDrawTarget* drawTarget)
@@ -84,24 +131,22 @@
// TODO: generalize this function so that when
// a clip gets complex enough it can just be done in SW regardless
// of whether it would invoke the GrSoftwarePathRenderer.
- GrStrokeInfo stroke(SkStrokeRec::kFill_InitStyle);
// Set the matrix so that rendered clip elements are transformed to mask space from clip
// space.
- SkMatrix translate;
- translate.setTranslate(clipToMaskOffset);
+ const SkMatrix translate = SkMatrix::MakeTrans(clipToMaskOffset.fX, clipToMaskOffset.fY);
for (GrReducedClip::ElementList::Iter iter(elements.headIter()); iter.get(); iter.next()) {
const Element* element = iter.get();
- // rects can always be drawn directly w/o using the software path
- // Skip rrects once we're drawing them directly.
- if (Element::kRect_Type != element->getType()) {
- SkPath path;
- element->asPath(&path);
- if (path_needs_SW_renderer(this->getContext(), pipelineBuilder, translate,
- path, stroke, element->isAA())) {
- return true;
- }
+
+ SkRegion::Op op = element->getOp();
+ bool invert = element->isInverseFilled();
+ bool needsStencil = invert ||
+ SkRegion::kIntersect_Op == op || SkRegion::kReverseDifference_Op == op;
+
+ if (path_needs_SW_renderer(this->getContext(), pipelineBuilder, translate,
+ element, nullptr, needsStencil)) {
+ return true;
}
}
return false;
@@ -412,7 +457,7 @@
GrPathRendererChain::DrawType type;
type = element->isAA() ? GrPathRendererChain::kColorAntiAlias_DrawType :
GrPathRendererChain::kColor_DrawType;
- pr = this->getContext()->getPathRenderer(pipelineBuilder, viewMatrix,
+ pr = this->getContext()->getPathRenderer(*pipelineBuilder, viewMatrix,
path, stroke, false, type);
}
if (nullptr == pr) {
@@ -434,32 +479,6 @@
return true;
}
-bool GrClipMaskManager::canStencilAndDrawElement(GrPipelineBuilder* pipelineBuilder,
- GrTexture* target,
- GrPathRenderer** pr,
- const SkClipStack::Element* element) {
- pipelineBuilder->setRenderTarget(target->asRenderTarget());
-
- if (Element::kRect_Type == element->getType()) {
- return true;
- } else {
- // We shouldn't get here with an empty clip element.
- SkASSERT(Element::kEmpty_Type != element->getType());
- SkPath path;
- element->asPath(&path);
- if (path.isInverseFillType()) {
- path.toggleInverseFillType();
- }
- GrStrokeInfo stroke(SkStrokeRec::kFill_InitStyle);
- GrPathRendererChain::DrawType type = element->isAA() ?
- GrPathRendererChain::kStencilAndColorAntiAlias_DrawType :
- GrPathRendererChain::kStencilAndColor_DrawType;
- *pr = this->getContext()->getPathRenderer(pipelineBuilder, SkMatrix::I(), path,
- stroke, false, type);
- return SkToBool(*pr);
- }
-}
-
void GrClipMaskManager::mergeMask(GrPipelineBuilder* pipelineBuilder,
GrTexture* dstMask,
GrTexture* srcMask,
@@ -555,8 +574,7 @@
// Set the matrix so that rendered clip elements are transformed to mask space from clip
// space.
- SkMatrix translate;
- translate.setTranslate(clipToMaskOffset);
+ const SkMatrix translate = SkMatrix::MakeTrans(clipToMaskOffset.fX, clipToMaskOffset.fY);
// The texture may be larger than necessary, this rect represents the part of the texture
// we populate with a rasterization of the clip.
@@ -586,7 +604,8 @@
pipelineBuilder.setClip(clip);
GrPathRenderer* pr = nullptr;
- bool useTemp = !this->canStencilAndDrawElement(&pipelineBuilder, texture, &pr, element);
+ bool useTemp = !can_stencil_and_draw_element(this->getContext(), &pipelineBuilder,
+ texture, translate, element, &pr);
GrTexture* dst;
// This is the bounds of the clip element in the space of the alpha-mask. The temporary
// mask buffer can be substantially larger than the actually clip stack element. We
@@ -594,6 +613,8 @@
// the accumulator.
SkIRect maskSpaceElementIBounds;
+ SkASSERT(!useTemp);
+
if (useTemp) {
if (invert) {
maskSpaceElementIBounds = maskSpaceIBounds;
@@ -759,7 +780,7 @@
if (fillInverted) {
clipPath.toggleInverseFillType();
}
- pr = this->getContext()->getPathRenderer(&pipelineBuilder,
+ pr = this->getContext()->getPathRenderer(pipelineBuilder,
viewMatrix,
clipPath,
stroke,
diff --git a/src/gpu/GrClipMaskManager.h b/src/gpu/GrClipMaskManager.h
index 3ce3e72..515f4b6 100644
--- a/src/gpu/GrClipMaskManager.h
+++ b/src/gpu/GrClipMaskManager.h
@@ -133,14 +133,6 @@
const SkClipStack::Element*,
GrPathRenderer* pr = nullptr);
- // Determines whether it is possible to draw the element to both the stencil buffer and the
- // alpha mask simultaneously. If so and the element is a path a compatible path renderer is
- // also returned.
- bool canStencilAndDrawElement(GrPipelineBuilder*,
- GrTexture* target,
- GrPathRenderer**,
- const SkClipStack::Element*);
-
void mergeMask(GrPipelineBuilder*,
GrTexture* dstMask,
GrTexture* srcMask,
diff --git a/src/gpu/GrContext.cpp b/src/gpu/GrContext.cpp
index 081bc0a..04e1b8d 100644
--- a/src/gpu/GrContext.cpp
+++ b/src/gpu/GrContext.cpp
@@ -541,7 +541,7 @@
* Due to its expense, the software path renderer has split out so it can
* can be individually allowed/disallowed via the "allowSW" boolean.
*/
-GrPathRenderer* GrContext::getPathRenderer(const GrPipelineBuilder* pipelineBuilder,
+GrPathRenderer* GrContext::getPathRenderer(const GrPipelineBuilder& pipelineBuilder,
const SkMatrix& viewMatrix,
const SkPath& path,
const GrStrokeInfo& stroke,
diff --git a/src/gpu/GrDrawContext.cpp b/src/gpu/GrDrawContext.cpp
index f52d7dc..d64273a 100644
--- a/src/gpu/GrDrawContext.cpp
+++ b/src/gpu/GrDrawContext.cpp
@@ -691,7 +691,7 @@
const GrStrokeInfo* strokeInfoPtr = &strokeInfo;
// Try a 1st time without stroking the path and without allowing the SW renderer
- GrPathRenderer* pr = fDrawingManager->getContext()->getPathRenderer(pipelineBuilder,
+ GrPathRenderer* pr = fDrawingManager->getContext()->getPathRenderer(*pipelineBuilder,
viewMatrix, *pathPtr,
*strokeInfoPtr, false,
type);
@@ -707,7 +707,7 @@
return;
}
strokeInfoPtr = &dashlessStrokeInfo;
- pr = fDrawingManager->getContext()->getPathRenderer(pipelineBuilder, viewMatrix,
+ pr = fDrawingManager->getContext()->getPathRenderer(*pipelineBuilder, viewMatrix,
*pathPtr, *strokeInfoPtr,
false, type);
}
@@ -732,7 +732,7 @@
}
// This time, allow SW renderer
- pr = fDrawingManager->getContext()->getPathRenderer(pipelineBuilder, viewMatrix,
+ pr = fDrawingManager->getContext()->getPathRenderer(*pipelineBuilder, viewMatrix,
*pathPtr, *strokeInfoPtr,
true, type);
}
diff --git a/src/gpu/GrPathRendererChain.cpp b/src/gpu/GrPathRendererChain.cpp
index b1eb320..c846dee 100644
--- a/src/gpu/GrPathRendererChain.cpp
+++ b/src/gpu/GrPathRendererChain.cpp
@@ -40,7 +40,7 @@
}
GrPathRenderer* GrPathRendererChain::getPathRenderer(const GrShaderCaps* shaderCaps,
- const GrPipelineBuilder* pipelineBuilder,
+ const GrPipelineBuilder& pipelineBuilder,
const SkMatrix& viewMatrix,
const SkPath& path,
const GrStrokeInfo& stroke,
@@ -66,15 +66,15 @@
minStencilSupport = GrPathRenderer::kNoSupport_StencilSupport;
}
+ GrPathRenderer::CanDrawPathArgs args;
+ args.fShaderCaps = shaderCaps;
+ args.fPipelineBuilder = &pipelineBuilder;
+ args.fViewMatrix = &viewMatrix;
+ args.fPath = &path;
+ args.fStroke = &stroke;
+ args.fAntiAlias = antiAlias;
for (int i = 0; i < fChain.count(); ++i) {
- GrPathRenderer::CanDrawPathArgs args;
- args.fShaderCaps = shaderCaps;
- args.fPipelineBuilder = pipelineBuilder;
- args.fViewMatrix = &viewMatrix;
- args.fPath = &path;
- args.fStroke = &stroke;
- args.fAntiAlias = antiAlias;
if (fChain[i]->canDrawPath(args)) {
if (GrPathRenderer::kNoSupport_StencilSupport != minStencilSupport) {
GrPathRenderer::StencilSupport support =