Fix radii calculation code to handle large radii.

BUG=472147
GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&issue=1569403002

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

src/core/SkRRect.cpp

@@ -5,8 +5,10 @@
  * found in the LICENSE file.
  */
 
+#include <cmath>
 #include "SkRRect.h"
 #include "SkMatrix.h"
+#include "SkScaleToSides.h"
 
 ///////////////////////////////////////////////////////////////////////////////
 
@@ -109,28 +111,6 @@ void SkRRect::setNinePatch(const SkRect& rect, SkScalar leftRad, SkScalar topRad
     SkDEBUGCODE(this->validate();)
 }
 
-/*
- *  TODO: clean this guy up and possibly add to SkScalar.h
- */
-static inline SkScalar SkScalarDecULP(SkScalar value) {
-#if SK_SCALAR_IS_FLOAT
-        return SkBits2Float(SkFloat2Bits(value) - 1);
-#else
-    #error "need impl for doubles"
-#endif
-}
-
- /**
- *  We need all combinations of predicates to be true to have a "safe" radius value.
- */
-static SkScalar clamp_radius_check_predicates(SkScalar rad, SkScalar min, SkScalar max) {
-    SkASSERT(min < max);
-    if (rad > max - min || min + rad > max || max - rad < min) {
-        rad = SkScalarDecULP(rad);
-    }
-    return rad;
-}
-
 // These parameters intentionally double. Apropos crbug.com/463920, if one of the
 // radii is huge while the other is small, single precision math can completely
 // miss the fact that a scale is required.
@@ -190,29 +170,21 @@ void SkRRect::setRectRadii(const SkRect& rect, const SkVector radii[4]) {
     // If f < 1, then all corner radii are reduced by multiplying them by f."
     double scale = 1.0;
 
-    scale = compute_min_scale(fRadii[0].fX, fRadii[1].fX, fRect.width(),  scale);
-    scale = compute_min_scale(fRadii[1].fY, fRadii[2].fY, fRect.height(), scale);
-    scale = compute_min_scale(fRadii[2].fX, fRadii[3].fX, fRect.width(),  scale);
-    scale = compute_min_scale(fRadii[3].fY, fRadii[0].fY, fRect.height(), scale);
+    // The sides of the rectangle may be larger than a float.
+    double width = (double)fRect.fRight - (double)fRect.fLeft;
+    double height = (double)fRect.fBottom - (double)fRect.fTop;
+    scale = compute_min_scale(fRadii[0].fX, fRadii[1].fX, width,  scale);
+    scale = compute_min_scale(fRadii[1].fY, fRadii[2].fY, height, scale);
+    scale = compute_min_scale(fRadii[2].fX, fRadii[3].fX, width,  scale);
+    scale = compute_min_scale(fRadii[3].fY, fRadii[0].fY, height, scale);
 
     if (scale < 1.0) {
-        for (int i = 0; i < 4; ++i) {
-            fRadii[i].fX *= scale;
-            fRadii[i].fY *= scale;
-        }
+        ScaleToSides::AdjustRadii(width,  scale, &fRadii[0].fX, &fRadii[1].fX);
+        ScaleToSides::AdjustRadii(height, scale, &fRadii[1].fY, &fRadii[2].fY);
+        ScaleToSides::AdjustRadii(width,  scale, &fRadii[2].fX, &fRadii[3].fX);
+        ScaleToSides::AdjustRadii(height, scale, &fRadii[3].fY, &fRadii[0].fY);
     }
 
-    // https://bug.skia.org/3239 -- its possible that we can hit the following inconsistency:
-    //     rad == bounds.bottom - bounds.top
-    //     bounds.bottom - radius < bounds.top
-    //     YIKES
-    // We need to detect and "fix" this now, otherwise we can have the following wackiness:
-    //     path.addRRect(rrect);
-    //     rrect.rect() != path.getBounds()
-    for (int i = 0; i < 4; ++i) {
-        fRadii[i].fX = clamp_radius_check_predicates(fRadii[i].fX, fRect.fLeft, fRect.fRight);
-        fRadii[i].fY = clamp_radius_check_predicates(fRadii[i].fY, fRect.fTop, fRect.fBottom);
-    }
     // At this point we're either oval, simple, or complex (not empty or rect).
     this->computeType();
 

src/core/SkScaleToSides.h

@@ -0,0 +1,61 @@
+/*
+ * Copyright 2016 Google Inc.
+ *
+ * Use of this source code is governed by a BSD-style license that can be
+ * found in the LICENSE file.
+ */
+
+#ifndef SkScaleToSides_DEFINED
+#define SkScaleToSides_DEFINED
+
+#include <cmath>
+#include "SkScalar.h"
+#include "SkTypes.h"
+
+class ScaleToSides {
+public:
+    // This code assumes that a and b fit in in a float, and therefore the resulting smaller value
+    // of a and b will fit in a float. The side of the rectangle may be larger than a float.
+    // Scale must be less than or equal to the ratio limit / (*a + *b).
+    // This code assumes that NaN and Inf are never passed in.
+    static void AdjustRadii(double limit, double scale, SkScalar* a, SkScalar* b) {
+        SkASSERTF(scale < 1.0 && scale > 0.0, "scale: %g", scale);
+
+        *a = (float)((double)*a * scale);
+        *b = (float)((double)*b * scale);
+
+        // This check is conservative. (double)*a + (double)*b >= (double)(*a + *b)
+        if ((double)*a + (double)*b > limit) {
+            float* minRadius = a;
+            float* maxRadius = b;
+
+            // Force minRadius to be the smaller of the two.
+            if (*minRadius > *maxRadius) {
+                SkTSwap(minRadius, maxRadius);
+            }
+
+            // newMinRadius must be float in order to give the actual value of the radius.
+            // The newMinRadius will always be smaller than limit. The largest that minRadius can be
+            // is 1/2 the ratio of minRadius : (minRadius + maxRadius), therefore in the resulting
+            // division, minRadius can be no larger than 1/2 limit + ULP.
+            float newMinRadius = *minRadius;
+
+            // Because newMaxRadius is the result of a double to float conversion, it can be larger
+            // than limit, but only by one ULP.
+            float newMaxRadius = (float)(limit - newMinRadius);
+
+            // If newMaxRadius forces the total over the limit, then it needs to be
+            // reduced by one ULP to be less than limit - newMinRadius.
+            // Note: nexttowardf is a c99 call and should be std::nexttoward, but this is not
+            // implemented in the ARM compiler.
+            if ((double)newMaxRadius + (double)newMinRadius > limit) {
+                newMaxRadius = nexttowardf(newMaxRadius, 0.0);
+            }
+            *maxRadius = newMaxRadius;
+        }
+
+        SkASSERTF(*a >= 0.0f && *b >= 0.0f, "a: %g, b: %g", *a, *b);
+        SkASSERTF((*a + *b) <= limit, "limit: %g, a: %g, b: %g", limit, *a, *b);
+    }
+};
+#endif // ScaleToSides_DEFINED

tests/DrawPathTest.cpp

@@ -313,6 +313,39 @@ static void test_crbug_165432(skiatest::Reporter* reporter) {
     REPORTER_ASSERT(reporter, filteredPath.isEmpty());
 }
 
+// http://crbug.com/472147
+// This is a simplified version from the bug. RRect radii not properly scaled.
+static void test_crbug_472147_simple(skiatest::Reporter* reporter) {
+    SkAutoTUnref<SkSurface> surface(SkSurface::NewRasterN32Premul(1000, 1000));
+    SkCanvas* canvas = surface->getCanvas();
+    SkPaint p;
+    SkRect r = SkRect::MakeLTRB(-246.0f, 33.0f, 848.0f, 33554464.0f);
+    SkVector radii[4] = {
+        { 13.0f, 8.0f }, { 170.0f, 2.0 }, { 256.0f, 33554430.0f }, { 120.0f, 5.0f }
+    };
+    SkRRect rr;
+    rr.setRectRadii(r, radii);
+    canvas->drawRRect(rr, p);
+}
+
+// http://crbug.com/472147
+// RRect radii not properly scaled.
+static void test_crbug_472147_actual(skiatest::Reporter* reporter) {
+    SkAutoTUnref<SkSurface> surface(SkSurface::NewRasterN32Premul(1000, 1000));
+    SkCanvas* canvas = surface->getCanvas();
+    SkPaint p;
+    SkRect r = SkRect::MakeLTRB(-246.0f, 33.0f, 848.0f, 33554464.0f);
+    SkVector radii[4] = {
+        { 13.0f, 8.0f }, { 170.0f, 2.0 }, { 256.0f, 33554430.0f }, { 120.0f, 5.0f }
+    };
+    SkRRect rr;
+    rr.setRectRadii(r, radii);
+    canvas->clipRRect(rr, SkRegion::kIntersect_Op, false);
+
+    SkRect r2 = SkRect::MakeLTRB(0, 33, 1102, 33554464);
+    canvas->drawRect(r2, p);
+}
+
 DEF_TEST(DrawPath, reporter) {
     test_giantaa();
     test_bug533();
@@ -325,6 +358,8 @@ DEF_TEST(DrawPath, reporter) {
     if (false) test_crbug131181();
     test_infinite_dash(reporter);
     test_crbug_165432(reporter);
+    test_crbug_472147_simple(reporter);
+    test_crbug_472147_actual(reporter);
     test_big_aa_rect(reporter);
     test_halfway();
 }

tests/ScaleToSidesTest.cpp

@@ -0,0 +1,49 @@
+/*
+ * Copyright 2016 Google Inc.
+ *
+ * Use of this source code is governed by a BSD-style license that can be
+ * found in the LICENSE file.
+ */
+
+#include "SkScaleToSides.h"
+
+#include <cfloat>
+#include "Test.h"
+
+DEF_TEST(ScaleToSides, reporter) {
+    float interestingValues[] = {
+        0.0f,
+        0.5f,
+        1.0f,
+        2.0f,
+        3.0f,
+        33.0f,
+        33554430.0f,
+        33554431.0f,
+        33554464.0f,
+        333333332.0f,
+        333333333.0f,
+        333333334.0f,
+        FLT_MAX,
+        FLT_EPSILON,
+        FLT_MIN
+    };
+
+    int numInterestingValues = (int)SK_ARRAY_COUNT(interestingValues);
+
+    for (int i = 0; i < numInterestingValues; i++) {
+        for (int j = 0; j < numInterestingValues; j++) {
+            for (int k = 0; k < numInterestingValues; k++) {
+                float radius1 = interestingValues[i];
+                float radius2 = interestingValues[j];
+                float width = interestingValues[k];
+                if (width > 0.0f) {
+                    double scale = (double)width / ((double)radius1 + (double)radius2);
+                    if (scale < 1.0) {
+                        ScaleToSides::AdjustRadii(width, scale, &radius1, &radius2);
+                    }
+                }
+            }
+        }
+    }
+}