Return tuple from GrConvexPolyEffect::Make denoting success or failure.

johnstiles-google · Skia Commit-Bot · commit f08a82b52dbf · 2020-06-16T21:08:33.000Z
Change-Id: I1c9cd865c3fd37b5d4c911790713d9ca2283aeee Reviewed-on: https://skia-review.googlesource.com/c/skia/+/296724 Reviewed-by: Brian Osman <brianosman@google.com> Commit-Queue: John Stiles <johnstiles@google.com>
diff --git a/gm/convexpolyeffect.cpp b/gm/convexpolyeffect.cpp
@@ -126,9 +126,8 @@ class ConvexPolyEffect : public GpuGM {
                 path->transform(m, &p);
 
                 GrClipEdgeType edgeType = (GrClipEdgeType) et;
-                std::unique_ptr<GrFragmentProcessor> fp =
-                    GrConvexPolyEffect::Make(/*inputFP=*/nullptr, edgeType, p);
-                if (!fp) {
+                auto [success, fp] = GrConvexPolyEffect::Make(/*inputFP=*/nullptr, edgeType, p);
+                if (!success) {
                     continue;
                 }
 
@@ -166,9 +165,8 @@ class ConvexPolyEffect : public GpuGM {
             for (int et = 0; et < kGrClipEdgeTypeCnt; ++et) {
                 SkRect rect = iter.get()->makeOffset(x, y);
                 GrClipEdgeType edgeType = (GrClipEdgeType) et;
-                std::unique_ptr<GrFragmentProcessor> fp =
-                    GrConvexPolyEffect::Make(/*inputFP=*/nullptr, edgeType, rect);
-                if (!fp) {
+                auto [success, fp] = GrConvexPolyEffect::Make(/*inputFP=*/nullptr, edgeType, rect);
+                if (!success) {
                     continue;
                 }
 
diff --git a/src/gpu/GrFragmentProcessor.h b/src/gpu/GrFragmentProcessor.h
@@ -8,6 +8,8 @@
 #ifndef GrFragmentProcessor_DEFINED
 #define GrFragmentProcessor_DEFINED
 
+#include <tuple>
+
 #include "src/gpu/GrCoordTransform.h"
 #include "src/gpu/GrProcessor.h"
 #include "src/gpu/ops/GrOp.h"
@@ -209,6 +211,20 @@ class GrFragmentProcessor : public GrProcessor {
 
     void visitProxies(const GrOp::VisitProxyFunc& func);
 
+    /**
+     * Some fragment processors' Make() methods have preconditions that might not be satisfied by
+     * the calling code. Those FPs can return a `MakeResult` from their Make() methods. If creation
+     * succeeds, the new fragment processor is created and `success` is true. If a precondition is
+     * not met, `success` is set to false and the input FP is returned unchanged.
+     */
+    using MakeResult = std::tuple<bool /*success*/, std::unique_ptr<GrFragmentProcessor>>;
+    static MakeResult MakeFailure(std::unique_ptr<GrFragmentProcessor> fp) {
+        return {false, std::move(fp)};
+    }
+    static MakeResult MakeSuccess(std::unique_ptr<GrFragmentProcessor> fp) {
+        return {true, std::move(fp)};
+    }
+
     // A pre-order traversal iterator over a hierarchy of FPs. It can also iterate over all the FP
     // hierarchies rooted in a GrPaint, GrProcessorSet, or GrPipeline. For these collections it
     // iterates the tree rooted at each color FP and then each coverage FP.
diff --git a/src/gpu/GrReducedClip.cpp b/src/gpu/GrReducedClip.cpp
@@ -689,18 +689,18 @@ GrReducedClip::ClipResult GrReducedClip::addAnalyticFP(const SkPath& deviceSpace
 
     if (fAnalyticFPs.empty()) {
         // Create our first analytic effect in the stack.
-        auto fp = GrConvexPolyEffect::Make(/*inputFP=*/nullptr, GetClipEdgeType(invert, aa),
-                                           deviceSpacePath);
-        if (fp != nullptr) {
+        auto [success, fp] = GrConvexPolyEffect::Make(/*inputFP=*/nullptr,
+                                                      GetClipEdgeType(invert, aa), deviceSpacePath);
+        if (success) {
             fAnalyticFPs.push_back(std::move(fp));
             return ClipResult::kClipped;
         }
     } else {
         // Combine this analytic effect with the previous effect in the stack.
-        auto fp = GrConvexPolyEffect::Make(&fAnalyticFPs.back(), GetClipEdgeType(invert, aa),
-                                           deviceSpacePath);
-        if (fp != nullptr) {
-            fAnalyticFPs.back() = std::move(fp);
+        auto [success, fp] = GrConvexPolyEffect::Make(std::move(fAnalyticFPs.back()),
+                                                      GetClipEdgeType(invert, aa), deviceSpacePath);
+        fAnalyticFPs.back() = std::move(fp);
+        if (success) {
             return ClipResult::kClipped;
         }
     }
diff --git a/src/gpu/effects/GrConvexPolyEffect.cpp b/src/gpu/effects/GrConvexPolyEffect.cpp
@@ -94,15 +94,14 @@ void GrGLConvexPolyEffect::GenKey(const GrProcessor& processor, const GrShaderCa
 
 //////////////////////////////////////////////////////////////////////////////
 
-std::unique_ptr<GrFragmentProcessor> GrConvexPolyEffect::Make(
-        std::unique_ptr<GrFragmentProcessor>* inputFP,
+GrFragmentProcessor::MakeResult GrConvexPolyEffect::Make(
+        std::unique_ptr<GrFragmentProcessor> inputFP,
         GrClipEdgeType type, const SkPath& path) {
     if (GrClipEdgeType::kHairlineAA == type) {
-        return nullptr;
+        return MakeFailure(std::move(inputFP));
     }
-    if (path.getSegmentMasks() != SkPath::kLine_SegmentMask ||
-        !path.isConvex()) {
-        return nullptr;
+    if (path.getSegmentMasks() != SkPath::kLine_SegmentMask || !path.isConvex()) {
+        return MakeFailure(std::move(inputFP));
     }
 
     SkPathPriv::FirstDirection dir;
@@ -111,17 +110,17 @@ std::unique_ptr<GrFragmentProcessor> GrConvexPolyEffect::Make(
     // skip the draw or omit the clip element.
     if (!SkPathPriv::CheapComputeFirstDirection(path, &dir)) {
         if (GrProcessorEdgeTypeIsInverseFill(type)) {
-            return GrConstColorProcessor::Make(inputFP ? std::move(*inputFP) : nullptr,
-                                               SK_PMColor4fWHITE,
-                                               GrConstColorProcessor::InputMode::kModulateRGBA);
+            return MakeSuccess(
+                    GrConstColorProcessor::Make(std::move(inputFP), SK_PMColor4fWHITE,
+                                                GrConstColorProcessor::InputMode::kModulateRGBA));
         }
         // This could use kIgnore instead of kModulateRGBA but it would trigger a debug print
         // about a coverage processor not being compatible with the alpha-as-coverage optimization.
         // We don't really care about this unlikely case so we just use kModulateRGBA to suppress
         // the print.
-        return GrConstColorProcessor::Make(inputFP ? std::move(*inputFP) : nullptr,
-                                           SK_PMColor4fTRANSPARENT,
-                                           GrConstColorProcessor::InputMode::kModulateRGBA);
+        return MakeSuccess(
+                GrConstColorProcessor::Make(std::move(inputFP), SK_PMColor4fTRANSPARENT,
+                                            GrConstColorProcessor::InputMode::kModulateRGBA));
     }
 
     SkScalar        edges[3 * kMaxEdges];
@@ -143,7 +142,7 @@ std::unique_ptr<GrFragmentProcessor> GrConvexPolyEffect::Make(
                 break;
             case SkPath::kLine_Verb: {
                 if (n >= kMaxEdges) {
-                    return nullptr;
+                    return MakeFailure(std::move(inputFP));
                 }
                 if (pts[0] != pts[1]) {
                     SkVector v = pts[1] - pts[0];
@@ -161,23 +160,24 @@ std::unique_ptr<GrFragmentProcessor> GrConvexPolyEffect::Make(
                 break;
             }
             default:
-                return nullptr;
+                return MakeFailure(std::move(inputFP));
         }
     }
 
     if (path.isInverseFillType()) {
         type = GrInvertProcessorEdgeType(type);
     }
-    return Make(inputFP, type, n, edges);
+    return GrConvexPolyEffect::Make(std::move(inputFP), type, n, edges);
 }
 
-std::unique_ptr<GrFragmentProcessor> GrConvexPolyEffect::Make(
-        std::unique_ptr<GrFragmentProcessor>* inputFP,
+GrFragmentProcessor::MakeResult GrConvexPolyEffect::Make(
+        std::unique_ptr<GrFragmentProcessor> inputFP,
         GrClipEdgeType edgeType, const SkRect& rect) {
-    if (GrClipEdgeType::kHairlineAA == edgeType){
-        return nullptr;
+    if (GrClipEdgeType::kHairlineAA == edgeType) {
+        return MakeFailure(std::move(inputFP));
     }
-    return GrAARectEffect::Make(inputFP ? std::move(*inputFP) : nullptr, edgeType, rect);
+
+    return MakeSuccess(GrAARectEffect::Make(std::move(inputFP), edgeType, rect));
 }
 
 GrConvexPolyEffect::~GrConvexPolyEffect() {}
@@ -245,7 +245,11 @@ std::unique_ptr<GrFragmentProcessor> GrConvexPolyEffect::TestCreate(GrProcessorT
     do {
         GrClipEdgeType edgeType = static_cast<GrClipEdgeType>(
                 d->fRandom->nextULessThan(kGrClipEdgeTypeCnt));
-        fp = GrConvexPolyEffect::Make(/*inputFP=*/nullptr, edgeType, count, edges);
+        auto [success, convexPolyFP] = GrConvexPolyEffect::Make(/*inputFP=*/nullptr, edgeType,
+                                                                count, edges);
+        if (success) {
+            fp = std::move(convexPolyFP);
+        }
     } while (nullptr == fp);
     return fp;
 }
diff --git a/src/gpu/effects/GrConvexPolyEffect.h b/src/gpu/effects/GrConvexPolyEffect.h
@@ -35,32 +35,27 @@ class GrConvexPolyEffect : public GrFragmentProcessor {
      * them in src space. There are a number of ways this could be accomplished but we'd probably
      * have to modify the effect/shaderbuilder interface to make it possible (e.g. give access
      * to the view matrix or untransformed positions in the fragment shader).
-     *
-     * The input fragment processor is passed as a pointer because it is only absorbed if creation
-     * of the convex-poly effect is successful. If Make returns nullptr, the inputFP is left as-is.
      */
-    static std::unique_ptr<GrFragmentProcessor> Make(std::unique_ptr<GrFragmentProcessor>* inputFP,
-                                                     GrClipEdgeType edgeType,
-                                                     int n, const SkScalar edges[]) {
+    static MakeResult Make(std::unique_ptr<GrFragmentProcessor> inputFP,
+                           GrClipEdgeType edgeType, int n, const SkScalar edges[]) {
         if (n <= 0 || n > kMaxEdges || GrClipEdgeType::kHairlineAA == edgeType) {
-            return nullptr;
+            return MakeFailure(std::move(inputFP));
         }
-        return std::unique_ptr<GrFragmentProcessor>(
-            new GrConvexPolyEffect(inputFP ? std::move(*inputFP) : nullptr, edgeType, n, edges));
+
+        return MakeSuccess(std::unique_ptr<GrFragmentProcessor>(
+                    new GrConvexPolyEffect(std::move(inputFP), edgeType, n, edges)));
     }
 
     /**
      * Creates an effect that clips against the path. If the path is not a convex polygon, is
-     * inverse filled, or has too many edges, this will return nullptr.
+     * inverse filled, or has too many edges, creation will fail.
      */
-    static std::unique_ptr<GrFragmentProcessor> Make(std::unique_ptr<GrFragmentProcessor>*,
-                                                     GrClipEdgeType, const SkPath&);
+    static MakeResult Make(std::unique_ptr<GrFragmentProcessor>, GrClipEdgeType, const SkPath&);
 
     /**
      * Creates an effect that fills inside the rect with AA edges..
      */
-    static std::unique_ptr<GrFragmentProcessor> Make(std::unique_ptr<GrFragmentProcessor>*,
-                                                     GrClipEdgeType, const SkRect&);
+    static MakeResult Make(std::unique_ptr<GrFragmentProcessor>, GrClipEdgeType, const SkRect&);
 
     ~GrConvexPolyEffect() override;
 
diff --git a/src/gpu/effects/GrRRectEffect.cpp b/src/gpu/effects/GrRRectEffect.cpp
@@ -726,7 +726,9 @@ std::unique_ptr<GrFragmentProcessor> GrRRectEffect::Make(GrClipEdgeType edgeType
                                                          const SkRRect& rrect,
                                                          const GrShaderCaps& caps) {
     if (rrect.isRect()) {
-        return GrConvexPolyEffect::Make(/*inputFP=*/nullptr, edgeType, rrect.getBounds());
+        auto [success, fp] = GrConvexPolyEffect::Make(/*inputFP=*/nullptr,
+                                                      edgeType, rrect.getBounds());
+        return std::move(fp);
     }
 
     if (rrect.isOval()) {
@@ -738,7 +740,9 @@ std::unique_ptr<GrFragmentProcessor> GrRRectEffect::Make(GrClipEdgeType edgeType
             SkRRectPriv::GetSimpleRadii(rrect).fY < kRadiusMin) {
             // In this case the corners are extremely close to rectangular and we collapse the
             // clip to a rectangular clip.
-            return GrConvexPolyEffect::Make(/*inputFP=*/nullptr, edgeType, rrect.getBounds());
+            auto [success, fp] =  GrConvexPolyEffect::Make(/*inputFP=*/nullptr,
+                                                           edgeType, rrect.getBounds());
+            return std::move(fp);
         }
         if (SkRRectPriv::GetSimpleRadii(rrect).fX == SkRRectPriv::GetSimpleRadii(rrect).fY) {
             return CircularRRectEffect::Make(/*inputFP=*/nullptr, edgeType,
@@ -803,8 +807,11 @@ std::unique_ptr<GrFragmentProcessor> GrRRectEffect::Make(GrClipEdgeType edgeType
                 }
                 return CircularRRectEffect::Make(/*inputFP=*/nullptr, edgeType, cornerFlags, *rr);
             }
-            case CircularRRectEffect::kNone_CornerFlags:
-                return GrConvexPolyEffect::Make(/*inputFP=*/nullptr, edgeType, rrect.getBounds());
+            case CircularRRectEffect::kNone_CornerFlags: {
+                auto [success, fp] =  GrConvexPolyEffect::Make(/*inputFP=*/nullptr,
+                                                               edgeType, rrect.getBounds());
+                return std::move(fp);
+            }
             default: {
                 if (squashedRadii) {
                     // If we got here then we squashed some but not all the radii to zero. (If all
