diff options
-rw-r--r-- | src/core/SkRRect.cpp | 95 | ||||
-rw-r--r-- | tests/DrawPathTest.cpp | 35 |
2 files changed, 41 insertions, 89 deletions
diff --git a/src/core/SkRRect.cpp b/src/core/SkRRect.cpp index c8b3a6ba4c..ad62e5bbae 100644 --- a/src/core/SkRRect.cpp +++ b/src/core/SkRRect.cpp @@ -5,7 +5,6 @@ * found in the LICENSE file. */ -#include <cmath> #include "SkRRect.h" #include "SkMatrix.h" @@ -110,6 +109,28 @@ 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. @@ -120,48 +141,6 @@ static double compute_min_scale(double rad1, double rad2, double limit, double c return curMin; } -// 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). -static void adjust_radii(double limit, double scale, float* a, float* b) { - SkASSERT(scale < 1.0 && scale > 0.0); - // 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 * scale; - *minRadius = newMinRadius; - // 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 is larger than the same value as a double, 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 (newMaxRadius > limit - newMinRadius) { - newMaxRadius = nexttowardf(newMaxRadius, limit - newMinRadius); - } - // This handles the case where both sets of radii are larger than a side by differing - // scale factors. The one that needs the larger scale factor (the radii with less - // overlap) will produce radii that are short enough just using the smaller scale factor - // from the side where the radii overlap is larger. - *maxRadius = SkMinScalar(scale * *maxRadius, newMaxRadius); - } else { - *a *= scale; - *b *= scale; - } - SkASSERT(*a >= 0.0f && *b >= 0.0f); - SkASSERT((*a + *b) <= limit); -} - void SkRRect::setRectRadii(const SkRect& rect, const SkVector radii[4]) { fRect = rect; fRect.sort(); @@ -211,21 +190,29 @@ 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; - // 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); + 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); if (scale < 1.0) { - adjust_radii(width, scale, &fRadii[0].fX, &fRadii[1].fX); - adjust_radii(height, scale, &fRadii[1].fY, &fRadii[2].fY); - adjust_radii(width, scale, &fRadii[2].fX, &fRadii[3].fX); - adjust_radii(height, scale, &fRadii[3].fY, &fRadii[0].fY); + for (int i = 0; i < 4; ++i) { + fRadii[i].fX *= scale; + fRadii[i].fY *= scale; + } } + // 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(); diff --git a/tests/DrawPathTest.cpp b/tests/DrawPathTest.cpp index e9aa4499d9..364a297123 100644 --- a/tests/DrawPathTest.cpp +++ b/tests/DrawPathTest.cpp @@ -313,39 +313,6 @@ 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(); @@ -358,8 +325,6 @@ 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(); } |