handle failing pathop tests

Some tests isolated from GrShapes_arcs do not
fail gracefully, so make sure errors are properly
handled.

TBR=reed@google.com

Bug: skia:
Change-Id: Ia8c9903e64ef755ec11c398df3e5d258ca1f5f8b
Reviewed-on: https://skia-review.googlesource.com/143112
Reviewed-by: Cary Clark <caryclark@skia.org>
Commit-Queue: Cary Clark <caryclark@skia.org>
Auto-Submit: Cary Clark <caryclark@skia.org>

src/pathops/SkOpSegment.cpp

@@ -338,7 +338,7 @@ SkOpSpanBase::Collapsed SkOpSegment::collapsed(double s, double e) const {
     return SkOpSpanBase::Collapsed::kNo;
 }
 
-void SkOpSegment::ComputeOneSum(const SkOpAngle* baseAngle, SkOpAngle* nextAngle,
+bool SkOpSegment::ComputeOneSum(const SkOpAngle* baseAngle, SkOpAngle* nextAngle,
         SkOpAngle::IncludeType includeType) {
     SkOpSegment* baseSegment = baseAngle->segment();
     int sumMiWinding = baseSegment->updateWindingReverse(baseAngle);
@@ -358,17 +358,22 @@ void SkOpSegment::ComputeOneSum(const SkOpAngle* baseAngle, SkOpAngle* nextAngle
         int oppMaxWinding, oppSumWinding;
         nextSegment->setUpWindings(nextAngle->start(), nextAngle->end(), &sumMiWinding,
                 &sumSuWinding, &maxWinding, &sumWinding, &oppMaxWinding, &oppSumWinding);
-        last = nextSegment->markAngle(maxWinding, sumWinding, oppMaxWinding, oppSumWinding,
+        if (!nextSegment->markAngle(maxWinding, sumWinding, oppMaxWinding, oppSumWinding,
-                nextAngle);
+                nextAngle, &last)) {
+            return false;
+        }
     } else {
         nextSegment->setUpWindings(nextAngle->start(), nextAngle->end(), &sumMiWinding,
                 &maxWinding, &sumWinding);
-        last = nextSegment->markAngle(maxWinding, sumWinding, nextAngle);
+        if (!nextSegment->markAngle(maxWinding, sumWinding, nextAngle, &last)) {
+            return false;
+        }
     }
     nextAngle->setLastMarked(last);
+    return true;
 }
 
-void SkOpSegment::ComputeOneSumReverse(SkOpAngle* baseAngle, SkOpAngle* nextAngle,
+bool SkOpSegment::ComputeOneSumReverse(SkOpAngle* baseAngle, SkOpAngle* nextAngle,
         SkOpAngle::IncludeType includeType) {
     SkOpSegment* baseSegment = baseAngle->segment();
     int sumMiWinding = baseSegment->updateWinding(baseAngle);
@@ -388,14 +393,19 @@ void SkOpSegment::ComputeOneSumReverse(SkOpAngle* baseAngle, SkOpAngle* nextAngl
         int oppMaxWinding, oppSumWinding;
         nextSegment->setUpWindings(nextAngle->end(), nextAngle->start(), &sumMiWinding,
                 &sumSuWinding, &maxWinding, &sumWinding, &oppMaxWinding, &oppSumWinding);
-        last = nextSegment->markAngle(maxWinding, sumWinding, oppMaxWinding, oppSumWinding,
+        if (!nextSegment->markAngle(maxWinding, sumWinding, oppMaxWinding, oppSumWinding,
-                nextAngle);
+                nextAngle, &last)) {
+            return false;
+        }
     } else {
         nextSegment->setUpWindings(nextAngle->end(), nextAngle->start(), &sumMiWinding,
                 &maxWinding, &sumWinding);
-        last = nextSegment->markAngle(maxWinding, sumWinding, nextAngle);
+        if (!nextSegment->markAngle(maxWinding, sumWinding, nextAngle, &last)) {
+            return false;
+        }
     }
     nextAngle->setLastMarked(last);
+    return true;
 }
 
 // at this point, the span is already ordered, or unorderable
@@ -910,10 +920,10 @@ bool SkOpSegment::markAndChaseWinding(SkOpSpanBase* start, SkOpSpanBase* end,
             if (this->operand() == other->operand()) {
                 if (spanStart->windSum() != winding || spanStart->oppSum() != oppWinding) {
                     this->globalState()->setWindingFailed();
-                    return false;
+                    return true;  // ... but let it succeed anyway
                 }
             } else {
-                SkASSERT(spanStart->windSum() == oppWinding);
+                FAIL_IF(spanStart->windSum() != oppWinding);
                 SkASSERT(spanStart->oppSum() == winding);
             }
             SkASSERT(!last);
@@ -931,13 +941,15 @@ bool SkOpSegment::markAndChaseWinding(SkOpSpanBase* start, SkOpSpanBase* end,
     return success;
 }
 
-SkOpSpanBase* SkOpSegment::markAngle(int maxWinding, int sumWinding, const SkOpAngle* angle) {
+bool SkOpSegment::markAngle(int maxWinding, int sumWinding, const SkOpAngle* angle,
+                            SkOpSpanBase** result) {
     SkASSERT(angle->segment() == this);
     if (UseInnerWinding(maxWinding, sumWinding)) {
         maxWinding = sumWinding;
     }
-    SkOpSpanBase* last;
+    if (!markAndChaseWinding(angle->start(), angle->end(), maxWinding, result)) {
-    (void) markAndChaseWinding(angle->start(), angle->end(), maxWinding, &last);
+        return false;
+    }
 #if DEBUG_WINDING
     if (last) {
         SkDebugf("%s last seg=%d span=%d", __FUNCTION__,
@@ -949,11 +961,11 @@ SkOpSpanBase* SkOpSegment::markAngle(int maxWinding, int sumWinding, const SkOpA
         SkDebugf("\n");
     }
 #endif
-    return last;
+    return true;
 }
 
-SkOpSpanBase* SkOpSegment::markAngle(int maxWinding, int sumWinding, int oppMaxWinding,
+bool SkOpSegment::markAngle(int maxWinding, int sumWinding, int oppMaxWinding,
-                                   int oppSumWinding, const SkOpAngle* angle) {
+                            int oppSumWinding, const SkOpAngle* angle, SkOpSpanBase** result) {
     SkASSERT(angle->segment() == this);
     if (UseInnerWinding(maxWinding, sumWinding)) {
         maxWinding = sumWinding;
@@ -961,9 +973,10 @@ SkOpSpanBase* SkOpSegment::markAngle(int maxWinding, int sumWinding, int oppMaxW
     if (oppMaxWinding != oppSumWinding && UseInnerWinding(oppMaxWinding, oppSumWinding)) {
         oppMaxWinding = oppSumWinding;
     }
-    SkOpSpanBase* last = nullptr;
     // caller doesn't require that this marks anything
-    (void) markAndChaseWinding(angle->start(), angle->end(), maxWinding, oppMaxWinding, &last);
+    if (!markAndChaseWinding(angle->start(), angle->end(), maxWinding, oppMaxWinding, result)) {
+        return false;
+    }
 #if DEBUG_WINDING
     if (last) {
         SkDebugf("%s last segment=%d span=%d", __FUNCTION__,
@@ -975,7 +988,7 @@ SkOpSpanBase* SkOpSegment::markAngle(int maxWinding, int sumWinding, int oppMaxW
         SkDebugf(" \n");
     }
 #endif
-    return last;
+    return true;
 }
 
 void SkOpSegment::markDone(SkOpSpan* span) {

src/pathops/SkOpSegment.h

@@ -105,9 +105,9 @@ public:
 
     void calcAngles();
     SkOpSpanBase::Collapsed collapsed(double startT, double endT) const;
-    static void ComputeOneSum(const SkOpAngle* baseAngle, SkOpAngle* nextAngle,
+    static bool ComputeOneSum(const SkOpAngle* baseAngle, SkOpAngle* nextAngle,
                               SkOpAngle::IncludeType );
-    static void ComputeOneSumReverse(SkOpAngle* baseAngle, SkOpAngle* nextAngle,
+    static bool ComputeOneSumReverse(SkOpAngle* baseAngle, SkOpAngle* nextAngle,
                                      SkOpAngle::IncludeType );
     int computeSum(SkOpSpanBase* start, SkOpSpanBase* end, SkOpAngle::IncludeType includeType);
 
@@ -279,9 +279,9 @@ public:
             SkOpSpanBase** lastPtr);
     bool markAndChaseWinding(SkOpSpanBase* start, SkOpSpanBase* end, int winding,
             int oppWinding, SkOpSpanBase** lastPtr);
-    SkOpSpanBase* markAngle(int maxWinding, int sumWinding, const SkOpAngle* angle);
+    bool markAngle(int maxWinding, int sumWinding, const SkOpAngle* angle, SkOpSpanBase** result);
-    SkOpSpanBase* markAngle(int maxWinding, int sumWinding, int oppMaxWinding, int oppSumWinding,
+    bool markAngle(int maxWinding, int sumWinding, int oppMaxWinding, int oppSumWinding,
-                         const SkOpAngle* angle);
+                         const SkOpAngle* angle, SkOpSpanBase** result);
     void markDone(SkOpSpan* );
     bool markWinding(SkOpSpan* , int winding);
     bool markWinding(SkOpSpan* , int winding, int oppWinding);

src/pathops/SkPathOpsCommon.cpp

@@ -156,7 +156,8 @@ SkOpSegment* FindChase(SkTDArray<SkOpSpanBase*>* chase, SkOpSpanBase** startPtr,
                 }
                 // OPTIMIZATION: should this also add to the chase?
                 if (sortable) {
-                    (void) segment->markAngle(maxWinding, sumWinding, angle);
+                    // TODO: add error handling
+                    SkAssertResult(segment->markAngle(maxWinding, sumWinding, angle, nullptr));
                 }
             }
         }

src/pathops/SkPathOpsOp.cpp

@@ -12,8 +12,8 @@
 
 #include <utility>
 
-static SkOpSegment* findChaseOp(SkTDArray<SkOpSpanBase*>& chase, SkOpSpanBase** startPtr,
+static bool findChaseOp(SkTDArray<SkOpSpanBase*>& chase, SkOpSpanBase** startPtr,
-        SkOpSpanBase** endPtr) {
+        SkOpSpanBase** endPtr, SkOpSegment** result) {
     while (chase.count()) {
         SkOpSpanBase* span;
         chase.pop(&span);
@@ -30,7 +30,8 @@ static SkOpSegment* findChaseOp(SkTDArray<SkOpSpanBase*>& chase, SkOpSpanBase**
    #else
             *chase.append() = span;
    #endif
-            return last->segment();
+            *result = last->segment();
+            return true;
         }
         if (done) {
             continue;
@@ -39,7 +40,8 @@ static SkOpSegment* findChaseOp(SkTDArray<SkOpSpanBase*>& chase, SkOpSpanBase**
         bool sortable;
         const SkOpAngle* angle = AngleWinding(*startPtr, *endPtr, &winding, &sortable);
         if (!angle) {
-            return nullptr;
+            *result = nullptr;
+            return true;
         }
         if (winding == SK_MinS32) {
             continue;
@@ -50,12 +52,14 @@ static SkOpSegment* findChaseOp(SkTDArray<SkOpSpanBase*>& chase, SkOpSpanBase**
             sumMiWinding = segment->updateWindingReverse(angle);
             if (sumMiWinding == SK_MinS32) {
                 SkASSERT(segment->globalState()->debugSkipAssert());
-                return nullptr;
+                *result = nullptr;
+                return true;
             }
             sumSuWinding = segment->updateOppWindingReverse(angle);
             if (sumSuWinding == SK_MinS32) {
                 SkASSERT(segment->globalState()->debugSkipAssert());
-                return nullptr;
+                *result = nullptr;
+                return true;
             }
             if (segment->operand()) {
                 using std::swap;
@@ -81,8 +85,10 @@ static SkOpSegment* findChaseOp(SkTDArray<SkOpSpanBase*>& chase, SkOpSpanBase**
                 }
                 // OPTIMIZATION: should this also add to the chase?
                 if (sortable) {
-                    (void) segment->markAngle(maxWinding, sumWinding, oppMaxWinding,
+                    if (!segment->markAngle(maxWinding, sumWinding, oppMaxWinding,
-                        oppSumWinding, angle);
+                            oppSumWinding, angle, nullptr)) {
+                        return false;
+                    }
                 }
             }
         }
@@ -92,10 +98,12 @@ static SkOpSegment* findChaseOp(SkTDArray<SkOpSpanBase*>& chase, SkOpSpanBase**
        #else
             *chase.append() = span;
        #endif
-            return first;
+            *result = first;
+            return true;
         }
     }
-    return nullptr;
+    *result = nullptr;
+    return true;
 }
 
 static bool bridgeOp(SkOpContourHead* contourList, const SkPathOp op,
@@ -181,7 +189,9 @@ static bool bridgeOp(SkOpContourHead* contourList, const SkPathOp op,
 #endif
                 }
             }
-            current = findChaseOp(chase, &start, &end);
+            if (!findChaseOp(chase, &start, &end, &current)) {
+                return false;
+            }
             SkPathOpsDebug::ShowActiveSpans(contourList);
             if (!current) {
                 break;
@@ -347,10 +357,12 @@ bool OpDebug(const SkPath& one, const SkPath& two, SkPathOp op, SkPath* result
     contourList->dumpSegments("aligned");
 #endif
     // construct closed contours
+    SkPath original = *result;
     result->reset();
     result->setFillType(fillType);
     SkPathWriter wrapper(*result);
     if (!bridgeOp(contourList, op, xorMask, xorOpMask, &wrapper)) {
+        *result = original;
         return false;
     }
     wrapper.assemble();  // if some edges could not be resolved, assemble remaining

tests/PathOpsExtendedTest.cpp

@@ -520,6 +520,11 @@ bool testSimplifyCheck(skiatest::Reporter* reporter, const SkPath& path, const c
             ExpectSuccess::kYes : ExpectSuccess::kNo, SkipAssert::kNo, ExpectMatch::kNo);
 }
 
+bool testSimplifyFail(skiatest::Reporter* reporter, const SkPath& path, const char* filename) {
+    return inner_simplify(reporter, path, filename,
+            ExpectSuccess::kNo, SkipAssert::kYes, ExpectMatch::kNo);
+}
+
 #if DEBUG_SHOW_TEST_NAME
 static void showName(const SkPath& a, const SkPath& b, const SkPathOp shapeOp) {
     SkDebugf("\n");

tests/PathOpsExtendedTest.h

@@ -38,15 +38,17 @@ extern bool testPathOpCheck(skiatest::Reporter* reporter, const SkPath& a, const
                             const SkPathOp , const char* testName, bool checkFail);
 extern bool testPathOpFail(skiatest::Reporter* reporter, const SkPath& a, const SkPath& b,
                            const SkPathOp, const char* testName);
-extern bool testPathOpFuzz(skiatest::Reporter* reporter, const SkPath& a,
+extern bool testPathOpFuzz(skiatest::Reporter* reporter, const SkPath& a, const SkPath& b,
-                           const SkPath& b, const SkPathOp , const char* testName);
+                           const SkPathOp , const char* testName);
 extern bool testSimplify(SkPath& path, bool useXor, SkPath& out, PathOpsThreadState& state,
                          const char* pathStr);
 extern bool testSimplify(skiatest::Reporter* reporter, const SkPath& path, const char* filename);
 extern bool testSimplifyCheck(skiatest::Reporter* reporter, const SkPath& path,
                               const char* filename, bool checkFail);
+extern bool testSimplifyFail(skiatest::Reporter* reporter, const SkPath& path,
+                             const char* filename);
 extern bool testSimplifyFuzz(skiatest::Reporter* reporter, const SkPath& path,
-                                       const char* filename);
+                             const char* filename);
 
 void initializeTests(skiatest::Reporter* reporter, const char* testName);