Experiment: always use doubles for setLength

Should allow more stroke cases to succeed, and simplifies the impl.

Bug: oss-fuzz:6811
Change-Id: I53390f57372938ba6d732726486a567c26ff121f
Reviewed-on: https://skia-review.googlesource.com/c/131140
Reviewed-by: Florin Malita <fmalita@chromium.org>
Commit-Queue: Mike Reed <reed@google.com>

src/core/SkPoint.cpp

@@ -5,30 +5,9 @@
  * found in the LICENSE file.
  */
 
-
 #include "SkMathPriv.h"
 #include "SkPointPriv.h"
 
-#if 0
-void SkIPoint::rotateCW(SkIPoint* dst) const {
-    SkASSERT(dst);
-
-    // use a tmp in case this == dst
-    int32_t tmp = fX;
-    dst->fX = -fY;
-    dst->fY = tmp;
-}
-
-void SkIPoint::rotateCCW(SkIPoint* dst) const {
-    SkASSERT(dst);
-
-    // use a tmp in case this == dst
-    int32_t tmp = fX;
-    dst->fX = fY;
-    dst->fY = -tmp;
-}
-#endif
-
 ///////////////////////////////////////////////////////////////////////////////
 
 void SkPoint::scale(SkScalar scale, SkPoint* dst) const {
@@ -48,22 +27,17 @@ bool SkPoint::setLength(SkScalar length) {
     return this->setLength(fX, fY, length);
 }
 
-// Returns the square of the Euclidian distance to (dx,dy).
+#ifdef SK_SUPPORT_LEGACY_SETLENGTH
 static inline float getLengthSquared(float dx, float dy) {
     return dx * dx + dy * dy;
 }
 
-// Calculates the square of the Euclidian distance to (dx,dy) and stores it in
-// *lengthSquared.  Returns true if the distance is judged to be "nearly zero".
-//
-// This logic is encapsulated in a helper method to make it explicit that we
-// always perform this check in the same manner, to avoid inconsistencies
-// (see http://code.google.com/p/skia/issues/detail?id=560 ).
 static inline bool is_length_nearly_zero(float dx, float dy,
                                          float *lengthSquared) {
     *lengthSquared = getLengthSquared(dx, dy);
     return *lengthSquared <= (SK_ScalarNearlyZero * SK_ScalarNearlyZero);
 }
+#endif
 
 /*
  *  We have to worry about 2 tricky conditions:
@@ -78,6 +52,7 @@ template <bool use_rsqrt> bool set_point_length(SkPoint* pt, float x, float y, f
     SkASSERT(!use_rsqrt || (orig_length == nullptr));
 
     float mag = 0;
+#ifdef SK_SUPPORT_LEGACY_SETLENGTH
     float mag2;
     if (is_length_nearly_zero(x, y, &mag2)) {
         pt->set(0, 0);
@@ -94,16 +69,24 @@ template <bool use_rsqrt> bool set_point_length(SkPoint* pt, float x, float y, f
         }
         x *= scale;
         y *= scale;
-    } else {
+    } else
+#endif
+    {
         // our mag2 step overflowed to infinity, so use doubles instead.
         // much slower, but needed when x or y are very large, other wise we
         // divide by inf. and return (0,0) vector.
         double xx = x;
         double yy = y;
         double dmag = sqrt(xx * xx + yy * yy);
-        double dscale = length / dmag;
-        x *= dscale;
-        y *= dscale;
+        double dscale;
+        if (dmag) {
+            dscale = length / dmag;
+            x *= dscale;
+            y *= dscale;
+        } else {
+            SkASSERT(x == 0);
+            SkASSERT(y == 0);
+        }
         // check if we're not finite, or we're zero-length
         if (!sk_float_isfinite(x) || !sk_float_isfinite(y) || (x == 0 && y == 0)) {
             pt->set(0, 0);

src/core/SkPointPriv.h

@@ -29,8 +29,12 @@ public:
         if (!SkScalarsAreFinite(dx, dy)) {
             return false;
         }
+#ifdef SK_SUPPORT_LEGACY_SETLENGTH
         // Simple enough (and performance critical sometimes) so we inline it.
         return (dx*dx + dy*dy) > (SK_ScalarNearlyZero * SK_ScalarNearlyZero);
+#else
+        return dx || dy;
+#endif
     }
 
     static SkScalar DistanceToLineBetweenSqd(const SkPoint& pt, const SkPoint& a,

src/core/SkStroke.cpp

@@ -764,6 +764,7 @@ void SkPathStroker::quadTo(const SkPoint& pt1, const SkPoint& pt2) {
 // compute the perpendicular point and its tangent.
 void SkPathStroker::setRayPts(const SkPoint& tPt, SkVector* dxy, SkPoint* onPt,
         SkPoint* tangent) const {
+#ifdef SK_SUPPORT_LEGACY_SETLENGTH
     SkPoint oldDxy = *dxy;
     if (!dxy->setLength(fRadius)) {  // consider moving double logic into SkPoint::setLength
         double xx = oldDxy.fX;
@@ -772,6 +773,11 @@ void SkPathStroker::setRayPts(const SkPoint& tPt, SkVector* dxy, SkPoint* onPt,
         dxy->fX = SkDoubleToScalar(xx * dscale);
         dxy->fY = SkDoubleToScalar(yy * dscale);
     }
+#else
+    if (!dxy->setLength(fRadius)) {
+        dxy->set(fRadius, 0);
+    }
+#endif
     SkScalar axisFlip = SkIntToScalar(fStrokeType);  // go opposite ways for outer, inner
     onPt->fX = tPt.fX + axisFlip * dxy->fY;
     onPt->fY = tPt.fY - axisFlip * dxy->fX;

tests/PointTest.cpp

@@ -121,19 +121,6 @@ static void test_overflow(skiatest::Reporter* reporter) {
     REPORTER_ASSERT(reporter, SkScalarNearlyEqual(length, SK_Scalar1));
 }
 
-// test that we handle very small values correctly. i.e. that we can
-// report failure if we try to normalize them.
-static void test_underflow(skiatest::Reporter* reporter) {
-    SkPoint pt = { 1.0e-37f, 1.0e-37f };
-    const SkPoint empty = { 0, 0 };
-
-    REPORTER_ASSERT(reporter, 0 == SkPoint::Normalize(&pt));
-    REPORTER_ASSERT(reporter, pt == empty);
-
-    REPORTER_ASSERT(reporter, !pt.setLength(SK_Scalar1));
-    REPORTER_ASSERT(reporter, pt == empty);
-}
-
 DEF_TEST(Point, reporter) {
     test_casts(reporter);
 
@@ -150,7 +137,6 @@ DEF_TEST(Point, reporter) {
         test_length(reporter, gRec[i].fX, gRec[i].fY, gRec[i].fLength);
     }
 
-    test_underflow(reporter);
     test_overflow(reporter);
     test_normalize_cannormalize_consistent(reporter);
 }