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 <reed@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>

5 files changed, 54 insertions, 40 deletions

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 <cmath>
 #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 <functional>
 
 /** SkScopeExit calls a std:::function<void()> 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<GrFragmentProcessor> 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<GrFragmentProcessor> EllipticalRRectEffect::TestCreate(GrProcess
 
 class GLEllipticalRRectEffect : public GrGLSLFragmentProcessor {
 public:
-    GLEllipticalRRectEffect() {
-        fPrevRRect.setEmpty();
-    }
+    GLEllipticalRRectEffect() = default;
 
     void emitCode(EmitArgs&) override;