path ops -- handle non-finite numbers

src/pathops/SkOpEdgeBuilder.cpp

Index: src/pathops/SkOpEdgeBuilder.cpp
===================================================================
--- src/pathops/SkOpEdgeBuilder.cpp	(revision 8862)
+++ src/pathops/SkOpEdgeBuilder.cpp	(working copy)
@@ -28,8 +28,10 @@
     preFetch();
 }
 
-void SkOpEdgeBuilder::finish() {
-    walk();
+bool SkOpEdgeBuilder::finish() {
+    if (!walk()) {
+        return false;
+    }
     complete();
     if (fCurrentContour && !fCurrentContour->segments().count()) {
         fContours.pop_back();
@@ -51,6 +53,7 @@
                 &fReducePts[rIndex]);
     }
     fExtra.reset();  // we're done with this
+    return true;
 }
 
 // Note that copying the points here avoids copying the resulting path later.
@@ -74,14 +77,73 @@
     return fPathVerbs.count() - 1;
 }
 
-void SkOpEdgeBuilder::walk() {
+static bool is_finite_point(const SkPoint& pt) {
+    // repeat logic in SkRect::isFinite()
+    float accum = 0;
+    accum *= pt.fX;
+    accum *= pt.fY;
+
+    // accum is either NaN or it is finite (zero).
+    SkASSERT(0 == accum || !(accum == accum));
+
+    // value==value will be true iff value is not NaN
+    // TODO: is it faster to say !accum or accum==accum?
+    return accum == accum;
+}
+
+static bool is_finite_line(const SkPoint pts[2]) {
+    // repeat logic in SkRect::isFinite()
+    float accum = 0;
+    accum *= pts[0].fX;
+    accum *= pts[0].fY;
+    accum *= pts[1].fX;
+    accum *= pts[1].fY;
+
+    // accum is either NaN or it is finite (zero).
+    SkASSERT(0 == accum || !(accum == accum));
+
+    // value==value will be true iff value is not NaN
+    // TODO: is it faster to say !accum or accum==accum?
+    return accum == accum;
+}
+
+static bool is_finite_quad(const SkPoint pts[3]) {
+    // repeat logic in SkRect::isFinite()
+    float accum = 0;
+    accum *= pts[0].fX;
+    accum *= pts[0].fY;
+    accum *= pts[1].fX;
+    accum *= pts[1].fY;
+    accum *= pts[2].fX;
+    accum *= pts[2].fY;
+
+    // accum is either NaN or it is finite (zero).
+    SkASSERT(0 == accum || !(accum == accum));
+
+    // value==value will be true iff value is not NaN
+    // TODO: is it faster to say !accum or accum==accum?
+    return accum == accum;
+}
+
+bool SkOpEdgeBuilder::close() {
+    if (fFinalCurveStart && fFinalCurveEnd && *fFinalCurveStart != *fFinalCurveEnd) {
+        *fReducePts.append() = *fFinalCurveStart;
+        *fReducePts.append() = *fFinalCurveEnd;
+        const SkPoint* lineStart = fReducePts.end() - 2;
+        *fExtra.append() = fCurrentContour->addLine(lineStart);
+    }
+    complete();
+    return true;
+}
+
+bool SkOpEdgeBuilder::walk() {
     SkPath::Verb reducedVerb;
     uint8_t* verbPtr = fPathVerbs.begin();
     uint8_t* endOfFirstHalf = &verbPtr[fSecondHalf];
     const SkPoint* pointsPtr = fPathPts.begin();
-    const SkPoint* finalCurveStart = NULL;
-    const SkPoint* finalCurveEnd = NULL;
     SkPath::Verb verb;
+    fFinalCurveStart = NULL;
+    fFinalCurveEnd = NULL;
     while ((verb = (SkPath::Verb) *verbPtr) != SkPath::kDone_Verb) {
         if (verbPtr == endOfFirstHalf) {
             fOperand = true;
@@ -89,64 +151,88 @@
         verbPtr++;
         switch (verb) {
             case SkPath::kMove_Verb:
-                complete();
+                if (fCurrentContour) {
+                    if (fAllowOpenContours) {
+                        complete();
+                    } else if (!close()) {
+                        return false;
+                    }
+                }
                 if (!fCurrentContour) {
                     fCurrentContour = fContours.push_back_n(1);
                     fCurrentContour->setOperand(fOperand);
                     fCurrentContour->setXor(fXorMask[fOperand] == kEvenOdd_PathOpsMask);
                     *fExtra.append() = -1;  // start new contour
                 }
-                finalCurveEnd = pointsPtr++;
+                if (!is_finite_point(pointsPtr[0])) {

[reed1, 2013/04/26 17:49:14]

Would it be faster to just ask for the path's bounds at the very beginning,
since that can be cached, and should report if the path was finite?

[caryclark, 2013/04/26 19:47:30]

Yes indeed. Done and uploaded.

On 2013/04/26 17:49:14, reed1 wrote:

+                    return false;
+                }
+                fFinalCurveEnd = pointsPtr++;
                 continue;
-            case SkPath::kLine_Verb:
+            case SkPath::kLine_Verb: {
+                const SkPoint& lineEnd = pointsPtr[0];
+                if (!is_finite_point(lineEnd)) {
+                    return false;
+                }
+                const SkPoint& lineStart = pointsPtr[-1];
                 // skip degenerate points
-                if (pointsPtr[-1].fX != pointsPtr[0].fX || pointsPtr[-1].fY != pointsPtr[0].fY) {
-                    fCurrentContour->addLine(&pointsPtr[-1]);
+                if (lineStart.fX != lineEnd.fX || lineStart.fY != lineEnd.fY) {
+                    fCurrentContour->addLine(&lineStart);
                 }
-                break;
-            case SkPath::kQuad_Verb:
-                reducedVerb = SkReduceOrder::Quad(&pointsPtr[-1], &fReducePts);
+                } break;
+            case SkPath::kQuad_Verb: {
+                if (!is_finite_line(pointsPtr)) {
+                    return false;
+                }
+                const SkPoint* quadStart = &pointsPtr[-1];
+                reducedVerb = SkReduceOrder::Quad(quadStart, &fReducePts);
                 if (reducedVerb == 0) {
                     break;  // skip degenerate points
                 }
                 if (reducedVerb == 1) {
-                    *fExtra.append() =
-                            fCurrentContour->addLine(fReducePts.end() - 2);
+                    const SkPoint* lineStart = fReducePts.end() - 2;
+                   *fExtra.append() = fCurrentContour->addLine(lineStart);
                     break;
                 }
-                fCurrentContour->addQuad(&pointsPtr[-1]);
-                break;
-            case SkPath::kCubic_Verb:
-                reducedVerb = SkReduceOrder::Cubic(&pointsPtr[-1], &fReducePts);
+                fCurrentContour->addQuad(quadStart);
+                } break;
+            case SkPath::kCubic_Verb: {
+                if (!is_finite_quad(pointsPtr)) {
+                    return false;
+                }
+                const SkPoint* cubicStart = &pointsPtr[-1];
+                reducedVerb = SkReduceOrder::Cubic(cubicStart, &fReducePts);
                 if (reducedVerb == 0) {
                     break;  // skip degenerate points
                 }
                 if (reducedVerb == 1) {
-                    *fExtra.append() = fCurrentContour->addLine(fReducePts.end() - 2);
+                    const SkPoint* lineStart = fReducePts.end() - 2;
+                    *fExtra.append() = fCurrentContour->addLine(lineStart);
                     break;
                 }
                 if (reducedVerb == 2) {
-                    *fExtra.append() = fCurrentContour->addQuad(fReducePts.end() - 3);
+                    const SkPoint* quadStart = fReducePts.end() - 3;
+                    *fExtra.append() = fCurrentContour->addQuad(quadStart);
                     break;
                 }
-                fCurrentContour->addCubic(&pointsPtr[-1]);
-                break;
+                fCurrentContour->addCubic(cubicStart);
+                } break;
             case SkPath::kClose_Verb:
                 SkASSERT(fCurrentContour);
-                if (finalCurveStart && finalCurveEnd
-                        && *finalCurveStart != *finalCurveEnd) {
-                    *fReducePts.append() = *finalCurveStart;
-                    *fReducePts.append() = *finalCurveEnd;
-                    *fExtra.append() = fCurrentContour->addLine(fReducePts.end() - 2);
+                if (!close()) {
+                    return false;
                 }
-                complete();
                 continue;
             default:
                 SkDEBUGFAIL("bad verb");
-                return;
+                return false;
         }
-        finalCurveStart = &pointsPtr[verb - 1];
+        fFinalCurveStart = &pointsPtr[verb - 1];
         pointsPtr += verb;
         SkASSERT(fCurrentContour);
     }
+   if (fCurrentContour && !fAllowOpenContours && !close()) {
+       return false;
+   }
+   return true;
 }