add rounding-using-doubles methods on SkScalar and SkRect

Inspired by the excellent repro case for https://crbug.com/364224

patch from issue 265933010

BUG=skia:
R=bungeman@google.com

Author: reed@google.com

Review URL: https://codereview.chromium.org/267003002

git-svn-id: http://skia.googlecode.com/svn/trunk@14566 2bbb7eff-a529-9590-31e7-b0007b416f81

5 files changed, 102 insertions, 1 deletions

diff --git a/include/core/SkRect.h b/include/core/SkRect.h
index 397e4a03ee..fd8cb16020 100644
--- a/include/core/SkRect.h
+++ b/include/core/SkRect.h
@@ -732,6 +732,24 @@ struct SK_API SkRect {
     }
 
     /**
+     *  Variant of round() that explicitly performs the rounding step (i.e. floor(x + 0.5)) using
+     *  double instead of SkScalar (float). It does this by calling SkDScalarRoundToInt(), which
+     *  may be slower than calling SkScalarRountToInt(), but gives slightly more accurate results.
+     *
+     *  e.g.
+     *      SkScalar x = 0.49999997f;
+     *      int ix = SkScalarRoundToInt(x);
+     *      SkASSERT(0 == ix);  // <--- fails
+     *      ix = SkDScalarRoundToInt(x);
+     *      SkASSERT(0 == ix);  // <--- succeeds
+     */
+    void dround(SkIRect* dst) const {
+        SkASSERT(dst);
+        dst->set(SkDScalarRoundToInt(fLeft), SkDScalarRoundToInt(fTop),
+                 SkDScalarRoundToInt(fRight), SkDScalarRoundToInt(fBottom));
+    }
+
+    /**
      *  Set the dst rectangle by rounding "out" this rectangle, choosing the
      *  SkScalarFloor of top and left, and the SkScalarCeil of right and bottom.
      */
diff --git a/include/core/SkScalar.h b/include/core/SkScalar.h
index b9256badb4..b37cf5c998 100644
--- a/include/core/SkScalar.h
+++ b/include/core/SkScalar.h
@@ -83,6 +83,26 @@ static inline bool SkScalarIsFinite(float x) {
 #define SkScalarRoundToInt(x)       sk_float_round2int(x)
 #define SkScalarTruncToInt(x)       static_cast<int>(x)
 
+/**
+ *  Variant of SkScalarRoundToInt, that performs the rounding step (adding 0.5) explicitly using
+ *  double, to avoid possibly losing the low bit(s) of the answer before calling floor().
+ *
+ *  This routine will likely be slower than SkScalarRoundToInt(), and should only be used when the
+ *  extra precision is known to be valuable.
+ *
+ *  In particular, this catches the following case:
+ *      SkScalar x = 0.49999997;
+ *      int ix = SkScalarRoundToInt(x);
+ *      SkASSERT(0 == ix);    // <--- fails
+ *      ix = SkDScalarRoundToInt(x);
+ *      SkASSERT(0 == ix);    // <--- succeeds
+ */
+static inline int SkDScalarRoundToInt(SkScalar x) {
+    double xx = x;
+    xx += 0.5;
+    return (int)floor(xx);
+}
+
 /** Returns the absolute value of the specified SkScalar
 */
 #define SkScalarAbs(x)          sk_float_abs(x)
diff --git a/src/core/SkScan_Path.cpp b/src/core/SkScan_Path.cpp
index 66e9507678..b32d68e734 100644
--- a/src/core/SkScan_Path.cpp
+++ b/src/core/SkScan_Path.cpp
@@ -602,7 +602,11 @@ void SkScan::FillPath(const SkPath& path, const SkRegion& origClip,
         // don't reference "origClip" any more, just use clipPtr
 
     SkIRect ir;
-    path.getBounds().round(&ir);
+    // We deliberately call dround() instead of round(), since we can't afford to generate a
+    // bounds that is tighter than the corresponding SkEdges. The edge code basically converts
+    // the floats to fixed, and then "rounds". If we called round() instead of dround() here,
+    // we could generate the wrong ir for values like 0.4999997.
+    path.getBounds().dround(&ir);
     if (ir.isEmpty()) {
         if (path.isInverseFillType()) {
             blitter->blitRegion(*clipPtr);
diff --git a/tests/PathTest.cpp b/tests/PathTest.cpp
index e1fbff28b6..3de10cb106 100644
--- a/tests/PathTest.cpp
+++ b/tests/PathTest.cpp
@@ -20,6 +20,49 @@
 #include "SkWriter32.h"
 #include "Test.h"
 
+static void make_path_crbug364224(SkPath* path) {
+    path->reset();
+    path->moveTo(3.747501373f, 2.724499941f);
+    path->lineTo(3.747501373f, 3.75f);
+    path->cubicTo(3.747501373f, 3.88774991f, 3.635501385f, 4.0f, 3.497501373f, 4.0f);
+    path->lineTo(0.7475013733f, 4.0f);
+    path->cubicTo(0.6095013618f, 4.0f, 0.4975013733f, 3.88774991f, 0.4975013733f, 3.75f);
+    path->lineTo(0.4975013733f, 1.0f);
+    path->cubicTo(0.4975013733f, 0.8622499704f, 0.6095013618f, 0.75f, 0.7475013733f,0.75f);
+    path->lineTo(3.497501373f, 0.75f);
+    path->cubicTo(3.50275135f, 0.75f, 3.5070014f, 0.7527500391f, 3.513001442f, 0.753000021f);
+    path->lineTo(3.715001345f, 0.5512499809f);
+    path->cubicTo(3.648251295f, 0.5194999576f, 3.575501442f, 0.4999999702f, 3.497501373f, 0.4999999702f);
+    path->lineTo(0.7475013733f, 0.4999999702f);
+    path->cubicTo(0.4715013802f, 0.4999999702f, 0.2475013733f, 0.7239999771f, 0.2475013733f, 1.0f);
+    path->lineTo(0.2475013733f, 3.75f);
+    path->cubicTo(0.2475013733f, 4.026000023f, 0.4715013504f, 4.25f, 0.7475013733f, 4.25f);
+    path->lineTo(3.497501373f, 4.25f);
+    path->cubicTo(3.773501396f, 4.25f, 3.997501373f, 4.026000023f, 3.997501373f, 3.75f);
+    path->lineTo(3.997501373f, 2.474750042f);
+    path->lineTo(3.747501373f, 2.724499941f);
+    path->close();
+}
+
+static void make_path_crbug364224_simplified(SkPath* path) {
+    path->moveTo(3.747501373f, 2.724499941f);
+    path->cubicTo(3.648251295f, 0.5194999576f, 3.575501442f, 0.4999999702f, 3.497501373f, 0.4999999702f);
+    path->close();
+}
+
+static void test_path_crbug364224() {
+    SkPath path;
+    SkPaint paint;
+    SkAutoTUnref<SkSurface> surface(SkSurface::NewRasterPMColor(84, 88));
+    SkCanvas* canvas = surface->getCanvas();
+
+    make_path_crbug364224_simplified(&path);
+    canvas->drawPath(path, paint);
+
+    make_path_crbug364224(&path);
+    canvas->drawPath(path, paint);
+}
+
 static void make_path0(SkPath* path) {
     // from  *  https://code.google.com/p/skia/issues/detail?id=1706
 
@@ -3326,6 +3369,8 @@ public:
 };
 
 DEF_TEST(Paths, reporter) {
+    test_path_crbug364224();
+
     SkTSize<SkScalar>::Make(3,4);
 
     SkPath  p, empty;
diff --git a/tests/ScalarTest.cpp b/tests/ScalarTest.cpp
index 82f61ec5c5..6b7fe6bfa0 100644
--- a/tests/ScalarTest.cpp
+++ b/tests/ScalarTest.cpp
@@ -12,6 +12,19 @@
 #include "SkRect.h"
 #include "Test.h"
 
+static void test_roundtoint(skiatest::Reporter* reporter) {
+    SkScalar x = 0.49999997;
+    int ix = SkScalarRoundToInt(x);
+    // We "should" get 0, since x < 0.5, but we don't due to float addition rounding up the low
+    // bit after adding 0.5.
+    REPORTER_ASSERT(reporter, 1 == ix);
+
+    // This version explicitly performs the +0.5 step using double, which should avoid losing the
+    // low bits.
+    ix = SkDScalarRoundToInt(x);
+    REPORTER_ASSERT(reporter, 0 == ix);
+}
+
 struct PointSet {
     const SkPoint* fPts;
     size_t         fCount;
@@ -187,4 +200,5 @@ static void test_isfinite(skiatest::Reporter* reporter) {
 
 DEF_TEST(Scalar, reporter) {
     test_isfinite(reporter);
+    test_roundtoint(reporter);
 }