Revert of Fix ClipMaskManager's SW-fallback logic (patchset #4 id:60001 of https...

Reason for revert:
Logic may be incorrect

Original issue's description:
> 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).
>
> Committed: https://skia.googlesource.com/skia/+/5c3ea4cd3921e8904d4f201bcdedfd5b8a726542

TBR=bsalomon@google.com
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true

Review URL: https://codereview.chromium.org/1426443008

diff --git a/include/gpu/GrContext.h b/include/gpu/GrContext.h
index 1e160c6..b886560 100644 (file)
--- a/include/gpu/GrContext.h
+++ b/include/gpu/GrContext.h
@@ -334,7 +334,7 @@ public:
     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 98441ce..a50160f 100644 (file)
--- a/include/gpu/GrPathRendererChain.h
+++ b/include/gpu/GrPathRendererChain.h
@@ -56,7 +56,7 @@ public:
         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 b87788b..b57f3cc 100644 (file)
--- a/src/gpu/GrClipMaskManager.cpp
+++ b/src/gpu/GrClipMaskManager.cpp
@@ -46,71 +46,24 @@ static const GrFragmentProcessor* create_fp_for_mask(GrTexture* result, const Sk
                                          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 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());
-
-        // 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);
+                                   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;
 
-// 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);
+    return nullptr == context->getPathRenderer(&pipelineBuilder, viewMatrix, *path, stroke,
+                                               false, type);
 }
 
 GrClipMaskManager::GrClipMaskManager(GrDrawTarget* drawTarget)
@@ -131,22 +84,24 @@ bool GrClipMaskManager::useSWOnlyPath(const GrPipelineBuilder& pipelineBuilder,
     // 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.
-    const SkMatrix translate = SkMatrix::MakeTrans(clipToMaskOffset.fX, clipToMaskOffset.fY);
+    SkMatrix translate;
+    translate.setTranslate(clipToMaskOffset);
 
     for (GrReducedClip::ElementList::Iter iter(elements.headIter()); iter.get(); iter.next()) {
         const Element* element = iter.get();
-
-        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;
+        // 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;
+            }
         }
     }
     return false;
@@ -457,7 +412,7 @@ bool GrClipMaskManager::drawElement(GrPipelineBuilder* pipelineBuilder,
                 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) {
@@ -479,6 +434,32 @@ bool GrClipMaskManager::drawElement(GrPipelineBuilder* pipelineBuilder,
     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,
@@ -574,7 +555,8 @@ GrTexture* GrClipMaskManager::createAlphaClipMask(int32_t elementsGenID,
 
     // Set the matrix so that rendered clip elements are transformed to mask space from clip
     // space.
-    const SkMatrix translate = SkMatrix::MakeTrans(clipToMaskOffset.fX, clipToMaskOffset.fY);
+    SkMatrix translate;
+    translate.setTranslate(clipToMaskOffset);
 
     // The texture may be larger than necessary, this rect represents the part of the texture
     // we populate with a rasterization of the clip.
@@ -604,8 +586,7 @@ GrTexture* GrClipMaskManager::createAlphaClipMask(int32_t elementsGenID,
 
             pipelineBuilder.setClip(clip);
             GrPathRenderer* pr = nullptr;
-            bool useTemp = !can_stencil_and_draw_element(this->getContext(), &pipelineBuilder,
-                                                         texture, translate, element, &pr);
+            bool useTemp = !this->canStencilAndDrawElement(&pipelineBuilder, texture, &pr, element);
             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
@@ -613,8 +594,6 @@ GrTexture* GrClipMaskManager::createAlphaClipMask(int32_t elementsGenID,
             // the accumulator.
             SkIRect maskSpaceElementIBounds;
 
-            SkASSERT(!useTemp);
-
             if (useTemp) {
                 if (invert) {
                     maskSpaceElementIBounds = maskSpaceIBounds;
@@ -780,7 +759,7 @@ bool GrClipMaskManager::createStencilClipMask(GrRenderTarget* rt,
                 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 515f4b6..3ce3e72 100644 (file)
--- a/src/gpu/GrClipMaskManager.h
+++ b/src/gpu/GrClipMaskManager.h
@@ -133,6 +133,14 @@ private:
                      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 04e1b8d..081bc0a 100644 (file)
--- a/src/gpu/GrContext.cpp
+++ b/src/gpu/GrContext.cpp
@@ -541,7 +541,7 @@ void GrContext::flushSurfaceWrites(GrSurface* surface) {
  * 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 d64273a..f52d7dc 100644 (file)
--- a/src/gpu/GrDrawContext.cpp
+++ b/src/gpu/GrDrawContext.cpp
@@ -691,7 +691,7 @@ void GrDrawContext::internalDrawPath(GrPipelineBuilder* pipelineBuilder,
     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 @@ void GrDrawContext::internalDrawPath(GrPipelineBuilder* pipelineBuilder,
             return;
         }
         strokeInfoPtr = &dashlessStrokeInfo;
-        pr = fDrawingManager->getContext()->getPathRenderer(*pipelineBuilder, viewMatrix,
+        pr = fDrawingManager->getContext()->getPathRenderer(pipelineBuilder, viewMatrix,
                                                             *pathPtr, *strokeInfoPtr,
                                                             false, type);
     }
@@ -732,7 +732,7 @@ void GrDrawContext::internalDrawPath(GrPipelineBuilder* pipelineBuilder,
         }
 
         // 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 c846dee..b1eb320 100644 (file)
--- a/src/gpu/GrPathRendererChain.cpp
+++ b/src/gpu/GrPathRendererChain.cpp
@@ -40,7 +40,7 @@ GrPathRenderer* GrPathRendererChain::addPathRenderer(GrPathRenderer* pr) {
 }
 
 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 @@ GrPathRenderer* GrPathRendererChain::getPathRenderer(const GrShaderCaps* shaderC
         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 =