fix for conic fuzz

src/core/SkGeometry.cpp

Index: src/core/SkGeometry.cpp
diff --git a/src/core/SkGeometry.cpp b/src/core/SkGeometry.cpp
index a3ebfb38a892740aed025a08e31653f9a5585e5c..562169743a85f116f3704cd295f9740c9cdccbb8 100644
--- a/src/core/SkGeometry.cpp
+++ b/src/core/SkGeometry.cpp
@@ -1153,6 +1153,11 @@ int SkConic::computeQuadPOW2(SkScalar tol) const {
     return pow2;
 }
 
+// returns true if (a <= b <= c) || (a >= b >= c)
+static bool between(SkScalar a, SkScalar b, SkScalar c) {

[reed1, 2016/09/20 17:33:25]

This is super cool (assuming it works). Seems to work if the intermediate
multiply is inf, since inf is signed (should we test this?)

[caryclark, 2016/09/21 14:37:48]

It works. I've been testing it for years in pathops. I added a comment to
document this.

+    return (a - b) * (c - b) <= 0;
+}
+
 static SkPoint* subdivide(const SkConic& src, SkPoint pts[], int level) {
     SkASSERT(level >= 0);
 
@@ -1162,6 +1167,17 @@ static SkPoint* subdivide(const SkConic& src, SkPoint pts[], int level) {
     } else {
         SkConic dst[2];
         src.chop(dst);
+        if (between(src.fPts[0].fY, src.fPts[1].fY, src.fPts[2].fY)) {

[reed1, 2016/09/20 17:33:25]

// Need to ensure that after a chop of a monotonic in Y src, that the resulting
2 dsts are each also monotonic in Y.
// (why does we care about this?)

[caryclark, 2016/09/21 14:37:48]

Added comments.

+            if (!between(dst[0].fPts[0].fY, dst[0].fPts[1].fY, dst[0].fPts[2].fY)) {
+                dst[0].fPts[1].fY = dst[0].fPts[0].fY;
+            }
+            if (!between(dst[0].fPts[1].fY, dst[1].fPts[0].fY, dst[1].fPts[1].fY)) {

[reed1, 2016/09/20 17:33:25]

Each of these 3 seem to be the same...

ensure_monotonic(SkScalar& a, SkScalar& b, SkScalar& c) {
    if (!between(a, b, c)) {
        b = a;
    }
}

How do we know which direction to push the middle value? This code always pushes
it towards a (the first value). Can it ever be that we want to nudge it towards
c? I presume we're accounting for tiny errors in the calculation, and that b
(when its not mono) is nearly equal to a or c ?

[caryclark, 2016/09/21 14:37:48]

The intention of moving it to one side was for consistency and with the thought
that these quads have all become lines at this point anyway, so moving the
control to equal the first point is no different from moving the control to
equal the last point.

On closer examination, it is more robust to treat aligning the control points
differently from aligning endpoints. Now, the end point is moved to the middle,
and each control point is, for consistency, moved to the end. I added comments
and asserts to verify that all five points are ordered.

+                dst[1].fPts[0].fY = dst[0].fPts[1].fY;
+            }
+            if (!between(dst[1].fPts[0].fY, dst[1].fPts[1].fY, dst[1].fPts[2].fY)) {
+                dst[1].fPts[1].fY = dst[1].fPts[0].fY;
+            }
+        }
         --level;
         pts = subdivide(dst[0], pts, level);
         return subdivide(dst[1], pts, level);