From 2a679ae8f5b80a337f67783dbc0a447a9f4312c7 Mon Sep 17 00:00:00 2001 From: robertphillips Date: Fri, 13 Mar 2015 09:53:01 -0700 Subject: [PATCH] Fix bug with very large round rects with large radii BUG=463920 Review URL: https://codereview.chromium.org/998903003 --- src/core/SkRRect.cpp | 34 ++++++++++++++++------------------ tests/RoundRectTest.cpp | 31 ++++++++++++++++++++++++------- 2 files changed, 40 insertions(+), 25 deletions(-) diff --git a/src/core/SkRRect.cpp b/src/core/SkRRect.cpp index d8b4a20d88..e54256009a 100644 --- a/src/core/SkRRect.cpp +++ b/src/core/SkRRect.cpp @@ -128,6 +128,16 @@ static SkScalar clamp_radius_check_predicates(SkScalar rad, SkScalar min, SkScal 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. +static double compute_min_scale(double rad1, double rad2, double limit, double curMin) { + if ((rad1 + rad2) > limit) { + return SkTMin(curMin, limit / (rad1 + rad2)); + } + return curMin; +} + void SkRRect::setRectRadii(const SkRect& rect, const SkVector radii[4]) { if (rect.isEmpty() || !rect.isFinite()) { this->setEmpty(); @@ -173,26 +183,14 @@ void SkRRect::setRectRadii(const SkRect& rect, const SkVector radii[4]) { // and Ltop = Lbottom = the width of the box, // and Lleft = Lright = the height of the box. // If f < 1, then all corner radii are reduced by multiplying them by f." - SkScalar scale = SK_Scalar1; + double scale = 1.0; - if (fRadii[0].fX + fRadii[1].fX > rect.width()) { - scale = SkMinScalar(scale, - SkScalarDiv(rect.width(), fRadii[0].fX + fRadii[1].fX)); - } - if (fRadii[1].fY + fRadii[2].fY > rect.height()) { - scale = SkMinScalar(scale, - SkScalarDiv(rect.height(), fRadii[1].fY + fRadii[2].fY)); - } - if (fRadii[2].fX + fRadii[3].fX > rect.width()) { - scale = SkMinScalar(scale, - SkScalarDiv(rect.width(), fRadii[2].fX + fRadii[3].fX)); - } - if (fRadii[3].fY + fRadii[0].fY > rect.height()) { - scale = SkMinScalar(scale, - SkScalarDiv(rect.height(), fRadii[3].fY + fRadii[0].fY)); - } + scale = compute_min_scale(fRadii[0].fX, fRadii[1].fX, rect.width(), scale); + scale = compute_min_scale(fRadii[1].fY, fRadii[2].fY, rect.height(), scale); + scale = compute_min_scale(fRadii[2].fX, fRadii[3].fX, rect.width(), scale); + scale = compute_min_scale(fRadii[3].fY, fRadii[0].fY, rect.height(), scale); - if (scale < SK_Scalar1) { + if (scale < 1.0) { for (int i = 0; i < 4; ++i) { fRadii[i].fX *= scale; fRadii[i].fY *= scale; diff --git a/tests/RoundRectTest.cpp b/tests/RoundRectTest.cpp index 9920d1013b..8d8f76fbf0 100644 --- a/tests/RoundRectTest.cpp +++ b/tests/RoundRectTest.cpp @@ -9,12 +9,29 @@ #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_tricky_radii(skiatest::Reporter* reporter) { + { + // crbug.com/458522 + 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); + } + + { + // crbug.com//463920 + SkRect r = SkRect::MakeLTRB(0, 0, 1009, 33554432.0); + SkVector radii[4] = { + { 13.0f, 8.0f }, { 170.0f, 2.0 }, { 256.0f, 33554432.0 }, { 110.0f, 5.0f } + }; + SkRRect rr; + rr.setRectRadii(r, radii); + + REPORTER_ASSERT(reporter, (double) rr.radii(SkRRect::kUpperRight_Corner).fY + + (double) rr.radii(SkRRect::kLowerRight_Corner).fY <= + rr.height()); + } } static void test_empty_crbug_458524(skiatest::Reporter* reporter) { @@ -660,6 +677,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_tricky_radii(reporter); test_empty_crbug_458524(reporter); } -- 2.34.1