fix more tricky-float cases in SkRRect validate

BUG=458524,458522

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

diff --git a/src/core/SkRRect.cpp b/src/core/SkRRect.cpp
index d3dce98ad5f103ddade09c219c4e09a117d2f884..788bf1d9a693c98084249ed08ad9ed9e32a9801a 100644 (file)
--- a/src/core/SkRRect.cpp
+++ b/src/core/SkRRect.cpp
@@ -117,17 +117,12 @@ static inline SkScalar SkScalarDecULP(SkScalar value) {
 #endif
 }
 
-static SkScalar clamp_radius_add(SkScalar rad, SkScalar min, SkScalar max) {
-    SkASSERT(rad <= max - min);
-    if (min + rad > max) {
-        rad = SkScalarDecULP(rad);
-    }
-    return rad;
-}
-
-static SkScalar clamp_radius_sub(SkScalar rad, SkScalar min, SkScalar max) {
-    SkASSERT(rad <= max - min);
-    if (max - rad < min) {
+ /**
+ *  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;
@@ -211,15 +206,10 @@ void SkRRect::setRectRadii(const SkRect& rect, const SkVector radii[4]) {
     // We need to detect and "fix" this now, otherwise we can have the following wackiness:
     //     path.addRRect(rrect);
     //     rrect.rect() != path.getBounds()
-    fRadii[0].fX = clamp_radius_add(fRadii[0].fX, rect.fLeft, rect.fRight);
-    fRadii[0].fY = clamp_radius_add(fRadii[0].fY, rect.fTop, rect.fBottom);
-    fRadii[1].fX = clamp_radius_sub(fRadii[1].fX, rect.fLeft, rect.fRight);
-    fRadii[1].fY = clamp_radius_add(fRadii[1].fY, rect.fTop, rect.fBottom);
-    fRadii[2].fX = clamp_radius_sub(fRadii[2].fX, rect.fLeft, rect.fRight);
-    fRadii[2].fY = clamp_radius_sub(fRadii[2].fY, rect.fTop, rect.fBottom);
-    fRadii[3].fX = clamp_radius_add(fRadii[3].fX, rect.fLeft, rect.fRight);
-    fRadii[3].fY = clamp_radius_sub(fRadii[3].fY, rect.fTop, rect.fBottom);
-
+    for (int i = 0; i < 4; ++i) {
+        fRadii[i].fX = clamp_radius_check_predicates(fRadii[i].fX, rect.fLeft, rect.fRight);
+        fRadii[i].fY = clamp_radius_check_predicates(fRadii[i].fY, rect.fTop, rect.fBottom);
+    }
     // At this point we're either oval, simple, or complex (not empty or rect).
     this->computeType();
 
@@ -397,6 +387,13 @@ bool SkRRect::transform(const SkMatrix& matrix, SkRRect* dst) const {
         return false;
     }
 
+    // The matrix may have scaled us to zero (or due to float madness, we now have collapsed
+    // some dimension of the rect, so we need to check for that.
+    if (newRect.isEmpty()) {
+        dst->setEmpty();
+        return true;
+    }
+
     // At this point, this is guaranteed to succeed, so we can modify dst.
     dst->fRect = newRect;
 
@@ -528,6 +525,16 @@ void SkRRect::dump(bool asHex) const {
 ///////////////////////////////////////////////////////////////////////////////
 
 #ifdef SK_DEBUG
+/**
+ *  We need all combinations of predicates to be true to have a "safe" radius value.
+ */
+static void validate_radius_check_predicates(SkScalar rad, SkScalar min, SkScalar max) {
+    SkASSERT(min <= max);
+    SkASSERT(rad <= max - min);
+    SkASSERT(min + rad <= max);
+    SkASSERT(max - rad >= min);
+}
+
 void SkRRect::validate() const {
     bool allRadiiZero = (0 == fRadii[0].fX && 0 == fRadii[0].fY);
     bool allCornersSquare = (0 == fRadii[0].fX || 0 == fRadii[0].fY);
@@ -581,6 +588,11 @@ void SkRRect::validate() const {
             SkASSERT(!patchesOfNine);
             break;
     }
+
+    for (int i = 0; i < 4; ++i) {
+        validate_radius_check_predicates(fRadii[i].fX, fRect.fLeft, fRect.fRight);
+        validate_radius_check_predicates(fRadii[i].fY, fRect.fTop, fRect.fBottom);
+    }
 }
 #endif // SK_DEBUG
 

diff --git a/tests/RoundRectTest.cpp b/tests/RoundRectTest.cpp
index b67718a6b1fa491463bc7a335980f797edcf9373..cff3e8f35ad5bc498390c2ddd992f97db2314549 100644 (file)
--- a/tests/RoundRectTest.cpp
+++ b/tests/RoundRectTest.cpp
@@ -9,6 +9,27 @@
 #include "SkRRect.h"
 #include "Test.h"
 
+static void test_tricky_radii_crbug_458522(skiatest::Reporter* reporter) {
+    SkRRect rr;
+    const SkRect bounds = { 3709, 3709, 3709 + 7402, 3709 + 29825 };
+    const SkScalar rad = 12814;
+    const SkVector vec[] = { { rad, rad }, { 0, rad }, { rad, rad }, { 0, rad } };
+    rr.setRectRadii(bounds, vec);
+}
+
+static void test_empty_crbug_458524(skiatest::Reporter* reporter) {
+    SkRRect rr;
+    const SkRect bounds = { 3709, 3709, 3709 + 7402, 3709 + 29825 };
+    const SkScalar rad = 40;
+    rr.setRectXY(bounds, rad, rad);
+
+    SkRRect other;
+    SkMatrix matrix;
+    matrix.setScale(0, 1);
+    rr.transform(matrix, &other);
+    REPORTER_ASSERT(reporter, SkRRect::kEmpty_Type == other.getType());
+}
+
 static const SkScalar kWidth = 100.0f;
 static const SkScalar kHeight = 100.0f;
 
@@ -621,4 +642,6 @@ DEF_TEST(RoundRect, reporter) {
     test_round_rect_contains_rect(reporter);
     test_round_rect_transform(reporter);
     test_issue_2696(reporter);
+    test_tricky_radii_crbug_458522(reporter);
+    test_empty_crbug_458524(reporter);
 }