some fuzzer fixes

should fix three or four of the PathOp asserts
triggered by the fuzzer tool.

R=kjlubick@google.com
Bug: skia:
Change-Id: I470895addf1e922da6a7c41d44d54eca92e68fb6
Reviewed-on: https://skia-review.googlesource.com/113163
Reviewed-by: Kevin Lubick <kjlubick@google.com>
Commit-Queue: Cary Clark <caryclark@skia.org>

11 files changed, 75 insertions, 33 deletions

diff --git a/src/pathops/SkIntersections.cpp b/src/pathops/SkIntersections.cpp
index f17e5dbc38..541cfb0ab6 100644
--- a/src/pathops/SkIntersections.cpp
+++ b/src/pathops/SkIntersections.cpp
@@ -45,23 +45,32 @@ int SkIntersections::insert(double one, double two, const SkDPoint& pt) {
             return -1;
         }
         if (more_roughly_equal(oldOne, one) && more_roughly_equal(oldTwo, two)) {
-            if ((precisely_zero(one) && !precisely_zero(oldOne))
-                    || (precisely_equal(one, 1) && !precisely_equal(oldOne, 1))
-                    || (precisely_zero(two) && !precisely_zero(oldTwo))
-                    || (precisely_equal(two, 1) && !precisely_equal(oldTwo, 1))) {
-                SkASSERT(one >= 0 && one <= 1);
-                SkASSERT(two >= 0 && two <= 1);
-                fT[0][index] = one;
-                fT[1][index] = two;
-                fPt[index] = pt;
+            if ((!precisely_zero(one) || precisely_zero(oldOne))
+                    && (!precisely_equal(one, 1) || precisely_equal(oldOne, 1))
+                    && (!precisely_zero(two) || precisely_zero(oldTwo))
+                    && (!precisely_equal(two, 1) || precisely_equal(oldTwo, 1))) {
+                return -1;
             }
-            return -1;
+            SkASSERT(one >= 0 && one <= 1);
+            SkASSERT(two >= 0 && two <= 1);
+            // remove this and reinsert below in case replacing would make list unsorted
+            int remaining = fUsed - index - 1;
+            memmove(&fPt[index], &fPt[index + 1], sizeof(fPt[0]) * remaining);
+            memmove(&fT[0][index], &fT[0][index + 1], sizeof(fT[0][0]) * remaining);
+            memmove(&fT[1][index], &fT[1][index + 1], sizeof(fT[1][0]) * remaining);
+            int clearMask = ~((1 << index) - 1);
+            fIsCoincident[0] -= (fIsCoincident[0] >> 1) & clearMask;
+            fIsCoincident[1] -= (fIsCoincident[1] >> 1) & clearMask;
+            --fUsed;
+            break;
         }
     #if ONE_OFF_DEBUG
         if (pt.roughlyEqual(fPt[index])) {
             SkDebugf("%s t=%1.9g pts roughly equal\n", __FUNCTION__, one);
         }
     #endif
+    }
+    for (index = 0; index < fUsed; ++index) {
         if (fT[0][index] > one) {
             break;
         }
diff --git a/src/pathops/SkOpCoincidence.cpp b/src/pathops/SkOpCoincidence.cpp
index 2f9d28f9ee..8e5c6c1d4d 100644
--- a/src/pathops/SkOpCoincidence.cpp
+++ b/src/pathops/SkOpCoincidence.cpp
@@ -694,7 +694,7 @@ bool SkOpCoincidence::addOrOverlap(SkOpSegment* coinSeg, SkOpSegment* oppSeg,
     SkASSERT(!cs || !cs->deleted());
     SkASSERT(!os || !os->deleted());
     SkASSERT(!ce || !ce->deleted());
-    SkASSERT(!oe || !oe->deleted());
+    FAIL_IF(oe && oe->deleted());
     const SkOpPtT* csExisting = !cs ? coinSeg->existing(coinTs, nullptr) : nullptr;
     const SkOpPtT* ceExisting = !ce ? coinSeg->existing(coinTe, nullptr) : nullptr;
     FAIL_IF(csExisting && csExisting == ceExisting);
@@ -723,6 +723,7 @@ bool SkOpCoincidence::addOrOverlap(SkOpSegment* coinSeg, SkOpSegment* oppSeg,
         csWritable->span()->addOpp(osWritable->span());
         cs = csWritable;
         os = osWritable->active();
+        FAIL_IF(!os);
         FAIL_IF((ce && ce->deleted()) || (oe && oe->deleted()));
     }
     if (!ce || !oe) {
diff --git a/src/pathops/SkOpEdgeBuilder.cpp b/src/pathops/SkOpEdgeBuilder.cpp
index cda6850701..16cbfc410c 100644
--- a/src/pathops/SkOpEdgeBuilder.cpp
+++ b/src/pathops/SkOpEdgeBuilder.cpp
@@ -323,7 +323,9 @@ bool SkOpEdgeBuilder::walk() {
                         }
                         SkPoint* curve = SkPath::kCubic_Verb == split->fVerb
                                 ? split->fPts : split->fReduced;
-                        SkAssertResult(can_add_curve(split->fVerb, curve));
+                        if (!can_add_curve(split->fVerb, curve)) {
+                            return false;
+                        }
                         fContourBuilder.addCurve(split->fVerb, curve);
                     }
                 }
diff --git a/src/pathops/SkOpSpan.cpp b/src/pathops/SkOpSpan.cpp
index 28989d9511..f5195edb38 100644
--- a/src/pathops/SkOpSpan.cpp
+++ b/src/pathops/SkOpSpan.cpp
@@ -24,8 +24,7 @@ const SkOpPtT* SkOpPtT::active() const {
             return ptT;
         }
     }
-    SkASSERT(0);  // should never return deleted
-    return this;
+    return nullptr; // should never return deleted; caller must abort
 }
 
 bool SkOpPtT::contains(const SkOpPtT* check) const {
@@ -269,12 +268,6 @@ tryNextRemainder:
     fSpanAdds += span->fSpanAdds;
 }
 
-SkOpSpanBase* SkOpSpanBase::active() {
-    SkOpSpanBase* result = fPrev ? fPrev->next() : upCast()->next()->prev();
-    SkASSERT(this == result || fDebugDeleted);
-    return result;
-}
-
 // please keep in sync with debugCheckForCollapsedCoincidence()
 void SkOpSpanBase::checkForCollapsedCoincidence() {
     SkOpCoincidence* coins = this->globalState()->coincidence();
diff --git a/src/pathops/SkOpSpan.h b/src/pathops/SkOpSpan.h
index fea3309598..af1481e920 100644
--- a/src/pathops/SkOpSpan.h
+++ b/src/pathops/SkOpSpan.h
@@ -177,7 +177,6 @@ protected:
 
 class SkOpSpanBase {
 public:
-    SkOpSpanBase* active();
     void addOpp(SkOpSpanBase* opp);
 
     void bumpSpanAdds() {
diff --git a/src/pathops/SkPathOpsCubic.cpp b/src/pathops/SkPathOpsCubic.cpp
index 33229fa8ae..eb2ddb7c7d 100644
--- a/src/pathops/SkPathOpsCubic.cpp
+++ b/src/pathops/SkPathOpsCubic.cpp
@@ -35,8 +35,7 @@ double SkDCubic::binarySearch(double min, double max, double axisIntercept,
     double calcPos = (&cubicAtT.fX)[xAxis];
     double calcDist = calcPos - axisIntercept;
     do {
-        double priorT = t - step;
-        SkOPASSERT(priorT >= min);
+        double priorT = std::max(min, t - step);
         SkDPoint lessPt = ptAtT(priorT);
         if (approximately_equal_half(lessPt.fX, cubicAtT.fX)
                 && approximately_equal_half(lessPt.fY, cubicAtT.fY)) {
diff --git a/src/pathops/SkPathOpsOp.cpp b/src/pathops/SkPathOpsOp.cpp
index 479b9a5c24..ee2b200f62 100644
--- a/src/pathops/SkPathOpsOp.cpp
+++ b/src/pathops/SkPathOpsOp.cpp
@@ -205,8 +205,7 @@ static const bool gOutInverse[kReverseDifference_SkPathOp + 1][2][2] = {
 
 SK_DECLARE_STATIC_MUTEX(debugWorstLoop);
 
-SkOpGlobalState debugWorstState(nullptr, nullptr  SkDEBUGPARAMS(false) SkDEBUGPARAMS(nullptr)
-        SkDEBUGPARAMS(nullptr));
+SkOpGlobalState debugWorstState(nullptr, nullptr  SkDEBUGPARAMS(false) SkDEBUGPARAMS(nullptr));
 
 void ReportPathOpsDebugging() {
     debugWorstState.debugLoopReport();
diff --git a/src/pathops/SkPathOpsTSect.h b/src/pathops/SkPathOpsTSect.h
index fb00d9abc9..833e735da6 100644
--- a/src/pathops/SkPathOpsTSect.h
+++ b/src/pathops/SkPathOpsTSect.h
@@ -272,7 +272,7 @@ private:
     }
 
     bool binarySearchCoin(SkTSect<OppCurve, TCurve>* , double tStart, double tStep, double* t,
-                          double* oppT);
+                          double* oppT, SkTSpan<OppCurve, TCurve>** oppFirst);
     SkTSpan<TCurve, OppCurve>* boundsMax() const;
     bool coincidentCheck(SkTSect<OppCurve, TCurve>* sect2);
     void coincidentForce(SkTSect<OppCurve, TCurve>* sect2, double start1s, double start1e);
@@ -908,7 +908,7 @@ SkTSpan<TCurve, OppCurve>* SkTSect<TCurve, OppCurve>::addOne() {
 
 template<typename TCurve, typename OppCurve>
 bool SkTSect<TCurve, OppCurve>::binarySearchCoin(SkTSect<OppCurve, TCurve>* sect2, double tStart,
-        double tStep, double* resultT, double* oppT) {
+        double tStep, double* resultT, double* oppT, SkTSpan<OppCurve, TCurve>** oppFirst) {
     SkTSpan<TCurve, OppCurve> work;
     double result = work.fStartT = work.fEndT = tStart;
     SkDEBUGCODE(work.fDebugSect = this);
@@ -916,7 +916,7 @@ bool SkTSect<TCurve, OppCurve>::binarySearchCoin(SkTSect<OppCurve, TCurve>* sect
     SkDPoint oppPt;
     bool flip = false;
     bool contained = false;
-    SkDEBUGCODE(bool down = tStep < 0);
+    bool down = tStep < 0;
     const OppCurve& opp = sect2->fCurve;
     do {
         tStep *= 0.5;
@@ -943,7 +943,10 @@ bool SkTSect<TCurve, OppCurve>::binarySearchCoin(SkTSect<OppCurve, TCurve>* sect
                 *oppT = oppTTest;
                 oppPt = work.fCoinStart.perpPt();
                 contained = true;
-                SkASSERT(down ? result > work.fStartT : result < work.fStartT);
+                if (down ? result <= work.fStartT : result >= work.fStartT) {
+                    *oppFirst = nullptr;    // signal caller to fail
+                    return false;
+                }
                 result = work.fStartT;
                 continue;
             }
@@ -1199,7 +1202,7 @@ bool SkTSect<TCurve, OppCurve>::extractCoincident(
     SkTSpan<OppCurve, TCurve>* cutFirst;
     if (prev && prev->fEndT == startT
             && this->binarySearchCoin(sect2, startT, prev->fStartT - startT, &coinStart,
-                                      &oppStartT)
+                                      &oppStartT, &oppFirst)
             && prev->fStartT < coinStart && coinStart < startT
             && (cutFirst = prev->oppT(oppStartT))) {
         oppFirst = cutFirst;
@@ -1218,8 +1221,10 @@ bool SkTSect<TCurve, OppCurve>::extractCoincident(
             }
         }
     } else {
+        if (!oppFirst) {
+            return false;
+        }
         SkDEBUGCODE(coinStart = first->fStartT);
-        FAIL_IF(!oppFirst);
         SkDEBUGCODE(oppStartT = oppMatched ? oppFirst->fStartT : oppFirst->fEndT);
     }
     // FIXME: incomplete : if we're not at the end, find end of coin
diff --git a/tests/PathOpsConicQuadIntersectionTest.cpp b/tests/PathOpsConicQuadIntersectionTest.cpp
index 99411dc98c..bdf1d42d37 100644
--- a/tests/PathOpsConicQuadIntersectionTest.cpp
+++ b/tests/PathOpsConicQuadIntersectionTest.cpp
@@ -15,6 +15,13 @@ static struct conicQuad {
     ConicPts conic;
     QuadPts quad;
 } conicQuadTests[] = {
+   {{{{{0.00000000000000000, -1.8135968446731567 },

+    {0.00000000000000000, -1.0033817291259766 },

+    {-0.0073835160583257675, 0.00000000000000000 }}}, 2.26585215e+11f},

+    {{{0.00000000000000000, -1.0000113248825073 },

+    {-2.4824290449032560e-05, -1.0000115633010864 },

+    {-0.0073835160583257675, 0.00000000000000000 }}}},
+
    {{{{{494.348663,224.583771}, {494.365143,224.633194}, {494.376404,224.684067}}}, 0.998645842f},
     {{{494.30481,224.474213}, {494.334961,224.538284}, {494.355774,224.605927}}}},
 
@@ -69,5 +76,5 @@ DEF_TEST(PathOpsConicQuadIntersection, reporter) {
 }
 
 DEF_TEST(PathOpsConicQuadIntersectionOneOff, reporter) {
-    conicQuadIntersection(reporter, 1);
+    conicQuadIntersection(reporter, 0);
 }
diff --git a/tests/PathOpsCubicConicIntersectionTest.cpp b/tests/PathOpsCubicConicIntersectionTest.cpp
index d7924634b2..ff4dec9864 100644
--- a/tests/PathOpsCubicConicIntersectionTest.cpp
+++ b/tests/PathOpsCubicConicIntersectionTest.cpp
@@ -15,6 +15,20 @@ static struct cubicConic {
     CubicPts cubic;
     ConicPts conic;
 } cubicConicTests[] = {
+#if 0
+// FIXME: this triggers an assert in bool SkTSect::extractCoincident() at
+// SkOPASSERT(oppStartT < oppEndT);
+// Throwing an error here breaks one test, but only in release.
+// More work to be done to figure this out.
+    {{{{2.1883804947719909e-05, 3.6366123822517693e-05 },

+        {2.9145950975362211e-05, 2.9117207304807380e-05 },

+        {2.9113532946212217e-05, 2.9173743314458989e-05 },

+        {0.00000000000000000, 5.8282588724978268e-05 }}},

+    {{{{0.00000000000000000, 5.8282581449020654e-05 },

+        {0.00000000000000000, 5.8282563259126619e-05 },

+        {5.8282588724978268e-05, 0.00000000000000000 }}}, 53684.6563f}},
+#endif

+
     {{{{188.60000610351562, 2041.5999755859375}, {188.60000610351562, 2065.39990234375},
         {208, 2084.800048828125}, {231.80000305175781, 2084.800048828125}}},
     {{{{231.80000305175781, 2084.800048828125}, {188.60000610351562, 2084.800048828125},
@@ -73,5 +87,5 @@ DEF_TEST(PathOpsCubicConicIntersection, reporter) {
 }
 
 DEF_TEST(PathOpsCubicConicIntersectionOneOff, reporter) {
-    cubicConicIntersection(reporter, 1);
+    cubicConicIntersection(reporter, 0);
 }
diff --git a/tests/PathOpsSimplifyFailTest.cpp b/tests/PathOpsSimplifyFailTest.cpp
index b6f210e8ea..58179a2175 100644
--- a/tests/PathOpsSimplifyFailTest.cpp
+++ b/tests/PathOpsSimplifyFailTest.cpp
@@ -215,9 +215,23 @@ path.close();
     testSimplifyFuzz(reporter, path, filename);
 }
 
+static void fuzz_x3(skiatest::Reporter* reporter, const char* filename) {
+    SkPath path;
+path.setFillType(SkPath::kWinding_FillType);

+path.moveTo(SkBits2Float(0x00000000), SkBits2Float(0x00000000));  // 0, 0

+path.cubicTo(SkBits2Float(0x92743420), SkBits2Float(0x74747474), SkBits2Float(0x0f747c74), SkBits2Float(0xff538565), SkBits2Float(0x74744374), SkBits2Float(0x20437474));  // -7.70571e-28f, 7.74708e+31f, 1.20541e-29f, -2.8116e+38f, 7.74102e+31f, 1.65557e-19f

+path.conicTo(SkBits2Float(0x7474926d), SkBits2Float(0x7c747474), SkBits2Float(0x00170f74), SkBits2Float(0x3a7410d7), SkBits2Float(0x3a3a3a3a));  // 7.7508e+31f, 5.07713e+36f, 2.11776e-39f, 0.000931037f, 0.000710401f

+path.quadTo(SkBits2Float(0x203a3a3a), SkBits2Float(0x7459f43a), SkBits2Float(0x74747474), SkBits2Float(0x2043ad6e));  // 1.57741e-19f, 6.90724e+31f, 7.74708e+31f, 1.65745e-19f

+path.conicTo(SkBits2Float(0x7474b374), SkBits2Float(0x74747474), SkBits2Float(0x0f747c74), SkBits2Float(0xff537065), SkBits2Float(0x74744374));  // 7.75488e+31f, 7.74708e+31f, 1.20541e-29f, -2.81051e+38f, 7.74102e+31f

+path.cubicTo(SkBits2Float(0x3a3a3a3a), SkBits2Float(0x3a2c103a), SkBits2Float(0x7474263a), SkBits2Float(0x74976507), SkBits2Float(0x000000ff), SkBits2Float(0x00000000));  // 0.000710401f, 0.00065637f, 7.7374e+31f, 9.59578e+31f, 3.57331e-43f, 0

+    testSimplifyFuzz(reporter, path, filename);
+}
+
+
 #define TEST(test) test(reporter, #test)
 
 DEF_TEST(PathOpsSimplifyFail, reporter) {
+    TEST(fuzz_x3);
     TEST(fuzz763_2);
     TEST(fuzz763_1);
     TEST(fuzz_x2);