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>
diff --git a/src/core/SkPath.cpp b/src/core/SkPath.cpp
index 422c2b9..ff0152b 100644
--- a/src/core/SkPath.cpp
+++ b/src/core/SkPath.cpp
@@ -1127,14 +1127,10 @@
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 63fd8cc..7cdac3b 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::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 @@
}
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 @@
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 @@
// 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 @@
}
// 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 @@
}
dst->scaleRadii();
+ dst->isValid();
return true;
}
@@ -438,10 +444,24 @@
///////////////////////////////////////////////////////////////////////////////
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::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 95804e6..bdec7b3 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 91cd3c8..8c56054 100644
--- a/src/gpu/GrShape.cpp
+++ b/src/gpu/GrShape.cpp
@@ -539,6 +539,7 @@
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 871578f..c6430ab 100644
--- a/src/gpu/effects/GrRRectEffect.cpp
+++ b/src/gpu/effects/GrRRectEffect.cpp
@@ -132,9 +132,7 @@
class GLCircularRRectEffect : public GrGLSLFragmentProcessor {
public:
- GLCircularRRectEffect() {
- fPrevRRect.setEmpty();
- }
+ GLCircularRRectEffect() = default;
virtual void emitCode(EmitArgs&) override;
@@ -483,9 +481,7 @@
class GLEllipticalRRectEffect : public GrGLSLFragmentProcessor {
public:
- GLEllipticalRRectEffect() {
- fPrevRRect.setEmpty();
- }
+ GLEllipticalRRectEffect() = default;
void emitCode(EmitArgs&) override;