From a766ca9af12e1175cfb01f4b516802da9197ba78 Mon Sep 17 00:00:00 2001 From: Mike Reed Date: Tue, 9 Jan 2018 11:31:53 -0500 Subject: use 64bit math to compute is a rect is empty Will work next to try to make isEmpty() private Bug: skia:7470 Bug:799715 Change-Id: I7b43028ecd86dca68e0c67225712516d2f2f88a2 Reviewed-on: https://skia-review.googlesource.com/92620 Commit-Queue: Mike Reed Reviewed-by: Brian Salomon --- docs/SkIRect_Reference.bmh | 48 +++++++++++++++++++++++++++++++++++++++++++++- include/core/SkRect.h | 23 ++++++++++++++-------- include/core/SkRegion.h | 4 +++- src/core/SkAAClip.cpp | 2 +- src/core/SkRectPriv.h | 8 -------- src/core/SkRegion.cpp | 11 +++-------- 6 files changed, 69 insertions(+), 27 deletions(-) diff --git a/docs/SkIRect_Reference.bmh b/docs/SkIRect_Reference.bmh index 7995ebe63b..48dcbd57d7 100644 --- a/docs/SkIRect_Reference.bmh +++ b/docs/SkIRect_Reference.bmh @@ -49,6 +49,7 @@ its top, it is considered empty. # contains() # Returns true if points are equal or inside. ## # containsNoEmptyCheck # Returns true if points are equal or inside. Skips empty check. ## # height() # Returns span in y. ## +# height64() # Returns span in y as int64_t. ## # inset() # Moves the sides symmetrically about the center. ## # intersect # Sets to shared area; returns true if not empty. ## # intersectNoEmptyCheck # Sets to shared area; returns true if not empty. Skips empty check. ## @@ -73,6 +74,7 @@ its top, it is considered empty. # sort() # Orders sides from smaller to larger. ## # top() # Returns smaller bounds in y, if sorted. ## # width() # Returns span in x. ## +# width64() # Returns span in y as int64_t. ## # x() # Returns bounds left. ## # y() # Returns bounds top. ## #Table ## @@ -417,7 +419,29 @@ large width: -5 ## ## -#SeeAlso height() SkRect::width() +#SeeAlso height() width64() height64() SkRect::width() + +## + +# ------------------------------------------------------------------------------ + +#Method int64_t width64() const + +Returns span on the x-axis. This does not check if IRect is sorted, so the +result may be negative. This is safer than calling width() since width() might +overflow in its calculation. + +#Return fRight minus fLeft cast to int64_t ## + +#Example +SkIRect large = { -2147483647, 1, 2147483644, 2 }; +SkDebugf("width: %d wdith64: %lld\n", large.width(), large.width64()); +#StdOut +width: -5 width64: 4294967291 +## +## + +#SeeAlso width() height() height64() SkRect::width() ## @@ -447,6 +471,28 @@ large height: -5 # ------------------------------------------------------------------------------ +#Method int64_t height64() const + +Returns span on the y-axis. This does not check if IRect is sorted, so the +result may be negative. This is safer than calling height() since height() might +overflow in its calculation. + +#Return fBottom minus fTop cast to int64_t ## + +#Example +SkIRect large = { 1, -2147483647, 2, 2147483644 }; +SkDebugf("height: %d height64: %lld\n", large.height(), large.height64()); +#StdOut +height: -5 height64: 4294967291 +## +## + +#SeeAlso width() height() width64() SkRect::height() + +## + +# ------------------------------------------------------------------------------ + #Method SkISize size() const Returns spans on the x-axis and y-axis. This does not check if IRect is sorted, diff --git a/include/core/SkRect.h b/include/core/SkRect.h index a9d73be235..eae2e49e4b 100644 --- a/include/core/SkRect.h +++ b/include/core/SkRect.h @@ -194,14 +194,21 @@ struct SK_API SkIRect { */ int32_t centerY() const { return (fBottom + fTop) >> 1; } - /** Returns true if fLeft is equal to or greater than fRight, or if fTop is equal - to or greater than fBottom. Call sort() to reverse rectangles with negative - width() or height(). - - @return true if width() or height() are zero or negative - */ - bool isEmpty() const { return fLeft >= fRight || fTop >= fBottom; } - + int64_t width64() const { return (int64_t)fRight - (int64_t)fLeft; } + int64_t height64() const { return (int64_t)fBottom - (int64_t)fTop; } + + /** Returns true if the rectangle's dimensions are non-positive or if either its width or hieght + * exceeds int32_t. + */ + bool isEmpty() const { + int64_t w = this->width64(); + int64_t h = this->height64(); + if (w <= 0 || h <= 0) { + return true; + } + // Return true if either exceeds int32_t + return !sk_64_isS32(w | h); + } /** Returns true if all members in a: fLeft, fTop, fRight, and fBottom; are identical to corresponding members in b. diff --git a/include/core/SkRegion.h b/include/core/SkRegion.h index 32be090764..84ab670cc3 100644 --- a/include/core/SkRegion.h +++ b/include/core/SkRegion.h @@ -117,7 +117,9 @@ public: * If left < right and top < bottom, set this region to that rectangle and * return true, otherwise set this region to empty and return false. */ - bool setRect(int32_t left, int32_t top, int32_t right, int32_t bottom); + bool setRect(int32_t left, int32_t top, int32_t right, int32_t bottom) { + return this->setRect({ left, top, right, bottom }); + } /** * Set this region to the union of an array of rects. This is generally diff --git a/src/core/SkAAClip.cpp b/src/core/SkAAClip.cpp index c0707c5b0c..b22b8ebebb 100644 --- a/src/core/SkAAClip.cpp +++ b/src/core/SkAAClip.cpp @@ -704,7 +704,7 @@ bool SkAAClip::setEmpty() { } bool SkAAClip::setRect(const SkIRect& bounds) { - if (!SkRectPriv::PositiveDimensions(bounds)) { + if (bounds.isEmpty()) { return this->setEmpty(); } diff --git a/src/core/SkRectPriv.h b/src/core/SkRectPriv.h index 01d184dce6..8b663ab3dd 100644 --- a/src/core/SkRectPriv.h +++ b/src/core/SkRectPriv.h @@ -12,14 +12,6 @@ class SkRectPriv { public: - // Returns true iff width and height are positive. Catches inverted, empty, and overflowing - // (way too big) rects. This is used by clients that want a non-empty rect that they can also - // actually use its computed width/height. - // - static bool PositiveDimensions(const SkIRect& r) { - return r.width() > 0 && r.height() > 0; - } - static SkRect MakeLargestS32() { const int32_t ihalf = SK_MaxS32 >> 1; const SkScalar half = SkIntToScalar(ihalf); diff --git a/src/core/SkRegion.cpp b/src/core/SkRegion.cpp index ba185ae38c..8c402e9250 100644 --- a/src/core/SkRegion.cpp +++ b/src/core/SkRegion.cpp @@ -140,21 +140,16 @@ bool SkRegion::setEmpty() { return false; } -bool SkRegion::setRect(int32_t left, int32_t top, - int32_t right, int32_t bottom) { - if (left >= right || top >= bottom) { +bool SkRegion::setRect(const SkIRect& r) { + if (r.isEmpty()) { return this->setEmpty(); } this->freeRuns(); - fBounds.set(left, top, right, bottom); + fBounds = r; fRunHead = SkRegion_gRectRunHeadPtr; return true; } -bool SkRegion::setRect(const SkIRect& r) { - return this->setRect(r.fLeft, r.fTop, r.fRight, r.fBottom); -} - bool SkRegion::setRegion(const SkRegion& src) { if (this != &src) { this->freeRuns(); -- cgit v1.2.3