From a18613d12098f3998e34f66bfb6389693f5ad91f Mon Sep 17 00:00:00 2001 From: Michael Ludwig Date: Tue, 22 Dec 2020 08:36:34 -0500 Subject: [PATCH] Add halfWidth/halfHeight functions to SkRectPriv These effectively compute width/2 or height/2, but switch operations around so that it's less likely to overflow on finite rects that would have overflows in width or height. Bug: skia:1160678 Change-Id: Ic93ca0c1d12598163b3dd48a5e8ba0ac7903301f Reviewed-on: https://skia-review.googlesource.com/c/skia/+/344968 Commit-Queue: Mike Reed Auto-Submit: Michael Ludwig Reviewed-by: Mike Reed --- include/core/SkRRect.h | 22 +--------------------- src/core/SkRRect.cpp | 27 +++++++++++++++++++++++++-- src/core/SkRectPriv.h | 9 +++++++++ tests/RectTest.cpp | 15 +++++++++++---- 4 files changed, 46 insertions(+), 27 deletions(-) diff --git a/include/core/SkRRect.h b/include/core/SkRRect.h index b0ee03b079..099385168f 100644 --- a/include/core/SkRRect.h +++ b/include/core/SkRRect.h @@ -185,27 +185,7 @@ public: @param oval bounds of oval */ - void setOval(const SkRect& oval) { - if (!this->initializeRect(oval)) { - return; - } - - SkScalar xRad = SkScalarHalf(fRect.width()); - SkScalar yRad = SkScalarHalf(fRect.height()); - - if (xRad == 0.0f || yRad == 0.0f) { - // All the corners will be square - memset(fRadii, 0, sizeof(fRadii)); - fType = kRect_Type; - } else { - for (int i = 0; i < 4; ++i) { - fRadii[i].set(xRad, yRad); - } - fType = kOval_Type; - } - - SkASSERT(this->isValid()); - } + void setOval(const SkRect& oval); /** Sets to rounded rectangle with the same radii for all four corners. If rect is empty, sets to kEmpty_Type. diff --git a/src/core/SkRRect.cpp b/src/core/SkRRect.cpp index 69d6766305..f2f6ab3d9a 100644 --- a/src/core/SkRRect.cpp +++ b/src/core/SkRRect.cpp @@ -10,6 +10,7 @@ #include "include/private/SkMalloc.h" #include "src/core/SkBuffer.h" #include "src/core/SkRRectPriv.h" +#include "src/core/SkRectPriv.h" #include "src/core/SkScaleToSides.h" #include @@ -17,6 +18,28 @@ /////////////////////////////////////////////////////////////////////////////// +void SkRRect::setOval(const SkRect& oval) { + if (!this->initializeRect(oval)) { + return; + } + + SkScalar xRad = SkRectPriv::HalfWidth(fRect); + SkScalar yRad = SkRectPriv::HalfHeight(fRect); + + if (xRad == 0.0f || yRad == 0.0f) { + // All the corners will be square + memset(fRadii, 0, sizeof(fRadii)); + fType = kRect_Type; + } else { + for (int i = 0; i < 4; ++i) { + fRadii[i].set(xRad, yRad); + } + fType = kOval_Type; + } + + SkASSERT(this->isValid()); +} + void SkRRect::setRectXY(const SkRect& rect, SkScalar xRad, SkScalar yRad) { if (!this->initializeRect(rect)) { return; @@ -659,8 +682,8 @@ bool SkRRect::isValid() const { } for (int i = 0; i < 4; ++i) { - if (!SkScalarNearlyEqual(fRadii[i].fX, SkScalarHalf(fRect.width())) || - !SkScalarNearlyEqual(fRadii[i].fY, SkScalarHalf(fRect.height()))) { + if (!SkScalarNearlyEqual(fRadii[i].fX, SkRectPriv::HalfWidth(fRect)) || + !SkScalarNearlyEqual(fRadii[i].fY, SkRectPriv::HalfHeight(fRect))) { return false; } } diff --git a/src/core/SkRectPriv.h b/src/core/SkRectPriv.h index 0f13b6303f..7220490c5c 100644 --- a/src/core/SkRectPriv.h +++ b/src/core/SkRectPriv.h @@ -59,6 +59,15 @@ public: SkTFitsIn(r.fRight) && SkTFitsIn(r.fBottom); } + // Returns r.width()/2 but divides first to avoid width() overflowing. + static SkScalar HalfWidth(const SkRect& r) { + return SkScalarHalf(r.fRight) - SkScalarHalf(r.fLeft); + } + // Returns r.height()/2 but divides first to avoid height() overflowing. + static SkScalar HalfHeight(const SkRect& r) { + return SkScalarHalf(r.fBottom) - SkScalarHalf(r.fTop); + } + // Evaluate A-B. If the difference shape cannot be represented as a rectangle then false is // returned and 'out' is set to the largest rectangle contained in said shape. If true is // returned then A-B is representable as a rectangle, which is stored in 'out'. diff --git a/tests/RectTest.cpp b/tests/RectTest.cpp index 1ab93d1c7d..8be796c2b1 100644 --- a/tests/RectTest.cpp +++ b/tests/RectTest.cpp @@ -148,16 +148,23 @@ static float make_big_value(skiatest::Reporter* reporter) { return reporter ? SK_ScalarMax * 0.75f : 0; } -DEF_TEST(Rect_center, reporter) { - // ensure we can compute center even when the width/height might overflow +DEF_TEST(Rect_whOverflow, reporter) { const SkScalar big = make_big_value(reporter); const SkRect r = { -big, -big, big, big }; REPORTER_ASSERT(reporter, r.isFinite()); - REPORTER_ASSERT(reporter, SkScalarIsFinite(r.centerX())); - REPORTER_ASSERT(reporter, SkScalarIsFinite(r.centerY())); REPORTER_ASSERT(reporter, !SkScalarIsFinite(r.width())); REPORTER_ASSERT(reporter, !SkScalarIsFinite(r.height())); + + // ensure we can compute center even when the width/height might overflow + REPORTER_ASSERT(reporter, SkScalarIsFinite(r.centerX())); + REPORTER_ASSERT(reporter, SkScalarIsFinite(r.centerY())); + + + // ensure we can compute halfWidth and halfHeight even when width/height might overflow, + // i.e. for use computing the radii filling a rectangle. + REPORTER_ASSERT(reporter, SkScalarIsFinite(SkRectPriv::HalfWidth(r))); + REPORTER_ASSERT(reporter, SkScalarIsFinite(SkRectPriv::HalfHeight(r))); } DEF_TEST(Rect_subtract, reporter) {