From 0a241ce808511ceb1c72d6f2473b01b455ac5101 Mon Sep 17 00:00:00 2001 From: Brian Salomon Date: Fri, 15 Dec 2017 11:31:05 -0500 Subject: Don't canonicalize empty SkRRects. They stroke differently. Make insetting greater than width or height collapse to a point/line. SkPath::addRRect() doesn't ignore an empty SkRRect. Change-Id: I933a3419a6d75be534f1d8328faa715772045f67 Reviewed-on: https://skia-review.googlesource.com/85680 Reviewed-by: Mike Reed Commit-Queue: Brian Salomon --- src/core/SkPath.cpp | 6 +-- src/core/SkRRect.cpp | 78 ++++++++++++++++++++++++--------------- src/core/SkScopeExit.h | 1 + src/gpu/GrShape.cpp | 1 + src/gpu/effects/GrRRectEffect.cpp | 8 +--- 5 files changed, 54 insertions(+), 40 deletions(-) (limited to 'src') diff --git a/src/core/SkPath.cpp b/src/core/SkPath.cpp index 422c2b911d..ff0152bb4a 100644 --- a/src/core/SkPath.cpp +++ b/src/core/SkPath.cpp @@ -1127,14 +1127,10 @@ void SkPath::addRRect(const SkRRect& rrect, Direction dir) { void SkPath::addRRect(const SkRRect &rrect, Direction dir, unsigned startIndex) { assert_known_direction(dir); - if (rrect.isEmpty()) { - return; - } - bool isRRect = hasOnlyMoveTos(); const SkRect& bounds = rrect.getBounds(); - if (rrect.isRect()) { + if (rrect.isRect() || rrect.isEmpty()) { // degenerate(rect) => radii points are collapsing this->addRect(bounds, dir, (startIndex + 1) / 2); } else if (rrect.isOval()) { diff --git a/src/core/SkRRect.cpp b/src/core/SkRRect.cpp index 63fd8ccfcd..7cdac3b512 100644 --- a/src/core/SkRRect.cpp +++ b/src/core/SkRRect.cpp @@ -7,6 +7,7 @@ #include #include "SkRRect.h" +#include "SkScopeExit.h" #include "SkBuffer.h" #include "SkMatrix.h" #include "SkScaleToSides.h" @@ -14,11 +15,7 @@ /////////////////////////////////////////////////////////////////////////////// void SkRRect::setRectXY(const SkRect& rect, SkScalar xRad, SkScalar yRad) { - fRect = rect; - fRect.sort(); - - if (fRect.isEmpty() || !fRect.isFinite()) { - this->setEmpty(); + if (!this->initializeRect(rect)) { return; } @@ -52,11 +49,7 @@ void SkRRect::setRectXY(const SkRect& rect, SkScalar xRad, SkScalar yRad) { void SkRRect::setNinePatch(const SkRect& rect, SkScalar leftRad, SkScalar topRad, SkScalar rightRad, SkScalar bottomRad) { - fRect = rect; - fRect.sort(); - - if (fRect.isEmpty() || !fRect.isFinite()) { - this->setEmpty(); + if (!this->initializeRect(rect)) { return; } @@ -123,11 +116,7 @@ static double compute_min_scale(double rad1, double rad2, double limit, double c } void SkRRect::setRectRadii(const SkRect& rect, const SkVector radii[4]) { - fRect = rect; - fRect.sort(); - - if (fRect.isEmpty() || !fRect.isFinite()) { - this->setEmpty(); + if (!this->initializeRect(rect)) { return; } @@ -162,6 +151,21 @@ void SkRRect::setRectRadii(const SkRect& rect, const SkVector radii[4]) { this->scaleRadii(); } +bool SkRRect::initializeRect(const SkRect& rect) { + // Check this before sorting because sorting can hide nans. + if (!rect.isFinite()) { + *this = SkRRect(); + return false; + } + fRect = rect.makeSorted(); + if (fRect.isEmpty()) { + memset(fRadii, 0, sizeof(fRadii)); + fType = kEmpty_Type; + return false; + } + return true; +} + void SkRRect::scaleRadii() { // Proportionally scale down all radii to fit. Find the minimum ratio @@ -289,13 +293,13 @@ static bool radii_are_nine_patch(const SkVector radii[4]) { // There is a simplified version of this method in setRectXY void SkRRect::computeType() { - struct Validator { - Validator(const SkRRect* r) : fR(r) {} - ~Validator() { SkASSERT(fR->isValid()); } - const SkRRect* fR; - } autoValidate(this); + SK_AT_SCOPE_EXIT(SkASSERT(this->isValid())); if (fRect.isEmpty()) { + SkASSERT(fRect.isSorted()); + for (size_t i = 0; i < SK_ARRAY_COUNT(fRadii); ++i) { + SkASSERT((fRadii[i] == SkVector{0, 0})); + } fType = kEmpty_Type; return; } @@ -369,10 +373,11 @@ bool SkRRect::transform(const SkMatrix& matrix, SkRRect* dst) const { } // 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; + // some dimension of the rect, so we need to check for that. Note that matrix must be + // scale and translate and mapRect() produces a sorted rect. So an empty rect indicates + // loss of precision. + if (!newRect.isFinite() || newRect.isEmpty()) { + return false; } // At this point, this is guaranteed to succeed, so we can modify dst. @@ -431,6 +436,7 @@ bool SkRRect::transform(const SkMatrix& matrix, SkRRect* dst) const { } dst->scaleRadii(); + dst->isValid(); return true; } @@ -438,10 +444,24 @@ bool SkRRect::transform(const SkMatrix& matrix, SkRRect* dst) const { /////////////////////////////////////////////////////////////////////////////// void SkRRect::inset(SkScalar dx, SkScalar dy, SkRRect* dst) const { - const SkRect r = fRect.makeInset(dx, dy); - - if (r.isEmpty()) { - dst->setEmpty(); + SkRect r = fRect.makeInset(dx, dy); + bool degenerate = false; + if (r.fRight <= r.fLeft) { + degenerate = true; + r.fLeft = r.fRight = SkScalarAve(r.fLeft, r.fRight); + } + if (r.fBottom <= r.fTop) { + degenerate = true; + r.fTop = r.fBottom = SkScalarAve(r.fTop, r.fBottom); + } + if (degenerate) { + dst->fRect = r; + memset(dst->fRadii, 0, sizeof(dst->fRadii)); + dst->fType = kEmpty_Type; + return; + } + if (!r.isFinite()) { + *dst = SkRRect(); return; } @@ -608,7 +628,7 @@ bool SkRRect::isValid() const { } bool SkRRect::AreRectAndRadiiValid(const SkRect& rect, const SkVector radii[4]) { - if (!rect.isFinite()) { + if (!rect.isFinite() || !rect.isSorted()) { return false; } for (int i = 0; i < 4; ++i) { diff --git a/src/core/SkScopeExit.h b/src/core/SkScopeExit.h index 95804e637a..bdec7b34f0 100644 --- a/src/core/SkScopeExit.h +++ b/src/core/SkScopeExit.h @@ -9,6 +9,7 @@ #define SkScopeExit_DEFINED #include "SkTypes.h" +#include /** SkScopeExit calls a std:::function in its destructor. */ class SkScopeExit { diff --git a/src/gpu/GrShape.cpp b/src/gpu/GrShape.cpp index 91cd3c8eb1..8c56054135 100644 --- a/src/gpu/GrShape.cpp +++ b/src/gpu/GrShape.cpp @@ -539,6 +539,7 @@ void GrShape::attemptToSimplifyPath() { void GrShape::attemptToSimplifyRRect() { SkASSERT(Type::kRRect == fType); SkASSERT(!fInheritedKey.count()); + // TODO: This isn't valid for strokes. if (fRRectData.fRRect.isEmpty()) { // Dashing ignores the inverseness currently. skbug.com/5421 fType = fRRectData.fInverted && !fStyle.isDashed() ? Type::kInvertedEmpty : Type::kEmpty; diff --git a/src/gpu/effects/GrRRectEffect.cpp b/src/gpu/effects/GrRRectEffect.cpp index 871578f924..c6430abd15 100644 --- a/src/gpu/effects/GrRRectEffect.cpp +++ b/src/gpu/effects/GrRRectEffect.cpp @@ -132,9 +132,7 @@ std::unique_ptr CircularRRectEffect::TestCreate(GrProcessor class GLCircularRRectEffect : public GrGLSLFragmentProcessor { public: - GLCircularRRectEffect() { - fPrevRRect.setEmpty(); - } + GLCircularRRectEffect() = default; virtual void emitCode(EmitArgs&) override; @@ -483,9 +481,7 @@ std::unique_ptr EllipticalRRectEffect::TestCreate(GrProcess class GLEllipticalRRectEffect : public GrGLSLFragmentProcessor { public: - GLEllipticalRRectEffect() { - fPrevRRect.setEmpty(); - } + GLEllipticalRRectEffect() = default; void emitCode(EmitArgs&) override; -- cgit v1.2.3