diff options
| -rw-r--r-- | include/core/SkPathRef.h | 4 | ||||
| -rw-r--r-- | src/core/SkPath.cpp | 1 | ||||
| -rw-r--r-- | src/core/SkRRect.cpp | 46 | ||||
| -rw-r--r-- | tests/PathTest.cpp | 36 |
4 files changed, 83 insertions, 4 deletions
diff --git a/include/core/SkPathRef.h b/include/core/SkPathRef.h index 78c6cd5af7..ded8e3fb64 100644 --- a/include/core/SkPathRef.h +++ b/include/core/SkPathRef.h @@ -253,6 +253,8 @@ public: */ uint32_t genID() const; + SkDEBUGCODE(void validate() const;) + private: enum SerializationOffsets { kIsFinite_SerializationShift = 25, // requires 1 bit @@ -415,8 +417,6 @@ private: return reinterpret_cast<intptr_t>(fVerbs) - reinterpret_cast<intptr_t>(fPoints); } - SkDEBUGCODE(void validate() const;) - /** * Called the first time someone calls CreateEmpty to actually create the singleton. */ diff --git a/src/core/SkPath.cpp b/src/core/SkPath.cpp index f8280bd6d2..6dc056e81f 100644 --- a/src/core/SkPath.cpp +++ b/src/core/SkPath.cpp @@ -1118,6 +1118,7 @@ void SkPath::addRRect(const SkRRect& rrect, Direction dir) { } this->close(); } + SkDEBUGCODE(fPathRef->validate();) } bool SkPath::hasOnlyMoveTos() const { diff --git a/src/core/SkRRect.cpp b/src/core/SkRRect.cpp index d3a571fe99..58e7de7910 100644 --- a/src/core/SkRRect.cpp +++ b/src/core/SkRRect.cpp @@ -97,6 +97,32 @@ void SkRRect::setNinePatch(const SkRect& rect, SkScalar leftRad, SkScalar topRad SkDEBUGCODE(this->validate();) } +/* + * TODO: clean this guy up and possibly add to SkScalar.h + */ +static inline SkScalar SkScalarDecULP(SkScalar value) { +#if SK_SCALAR_IS_FLOAT + return SkBits2Float(SkFloat2Bits(value) - 1); +#else + #error "need impl for doubles" +#endif +} + +static SkScalar clamp_radius_add(SkScalar rad, SkScalar min, SkScalar max) { + SkASSERT(rad <= max - min); + if (min + rad > max) { + rad = SkScalarDecULP(rad); + } + return rad; +} + +static SkScalar clamp_radius_sub(SkScalar rad, SkScalar min, SkScalar max) { + SkASSERT(rad <= max - min); + if (max - rad < min) { + rad = SkScalarDecULP(rad); + } + return rad; +} void SkRRect::setRectRadii(const SkRect& rect, const SkVector radii[4]) { if (rect.isEmpty()) { @@ -159,11 +185,27 @@ void SkRRect::setRectRadii(const SkRect& rect, const SkVector radii[4]) { if (scale < SK_Scalar1) { for (int i = 0; i < 4; ++i) { - fRadii[i].fX = SkScalarMul(fRadii[i].fX, scale); - fRadii[i].fY = SkScalarMul(fRadii[i].fY, scale); + fRadii[i].fX *= scale; + fRadii[i].fY *= scale; } } + // skbug.com/3239 -- its possible that we can hit the following inconsistency: + // rad == bounds.bottom - bounds.top + // bounds.bottom - radius < bounds.top + // YIKES + // We need to detect and "fix" this now, otherwise we can have the following wackiness: + // path.addRRect(rrect); + // rrect.rect() != path.getBounds() + fRadii[0].fX = clamp_radius_add(fRadii[0].fX, rect.fLeft, rect.fRight); + fRadii[0].fY = clamp_radius_add(fRadii[0].fY, rect.fTop, rect.fBottom); + fRadii[1].fX = clamp_radius_sub(fRadii[1].fX, rect.fLeft, rect.fRight); + fRadii[1].fY = clamp_radius_add(fRadii[1].fY, rect.fTop, rect.fBottom); + fRadii[2].fX = clamp_radius_sub(fRadii[2].fX, rect.fLeft, rect.fRight); + fRadii[2].fY = clamp_radius_sub(fRadii[2].fY, rect.fTop, rect.fBottom); + fRadii[3].fX = clamp_radius_add(fRadii[3].fX, rect.fLeft, rect.fRight); + fRadii[3].fY = clamp_radius_sub(fRadii[3].fY, rect.fTop, rect.fBottom); + // At this point we're either oval, simple, or complex (not empty or rect) // but we lazily resolve the type to avoid the work if the information // isn't required. diff --git a/tests/PathTest.cpp b/tests/PathTest.cpp index 1a74682e54..4bd4538f1a 100644 --- a/tests/PathTest.cpp +++ b/tests/PathTest.cpp @@ -21,6 +21,41 @@ #include "SkWriter32.h" #include "Test.h" +static void set_radii(SkVector radii[4], int index, float rad) { + sk_bzero(radii, sizeof(SkVector) * 4); + radii[index].set(rad, rad); +} + +static void test_add_rrect(skiatest::Reporter* reporter, const SkRect& bounds, + const SkVector radii[4]) { + SkRRect rrect; + rrect.setRectRadii(bounds, radii); + REPORTER_ASSERT(reporter, bounds == rrect.rect()); + + SkPath path; + // this line should not assert in the debug build (from validate) + path.addRRect(rrect); + REPORTER_ASSERT(reporter, bounds == path.getBounds()); +} + +static void test_skbug_3239(skiatest::Reporter* reporter) { + const float min = SkBits2Float(0xcb7f16c8); /* -16717512.000000 */ + const float max = SkBits2Float(0x4b7f1c1d); /* 16718877.000000 */ + const float big = SkBits2Float(0x4b7f1bd7); /* 16718807.000000 */ + + const float rad = 33436320; + + const SkRect rectx = SkRect::MakeLTRB(min, min, max, big); + const SkRect recty = SkRect::MakeLTRB(min, min, big, max); + + SkVector radii[4]; + for (int i = 0; i < 4; ++i) { + set_radii(radii, i, rad); + test_add_rrect(reporter, rectx, radii); + test_add_rrect(reporter, recty, radii); + } +} + static void make_path_crbug364224(SkPath* path) { path->reset(); path->moveTo(3.747501373f, 2.724499941f); @@ -3729,4 +3764,5 @@ DEF_TEST(Paths, reporter) { PathRefTest_Private::TestPathRef(reporter); test_dump(reporter); test_path_crbugskia2820(reporter); + test_skbug_3239(reporter); } |