From 4db592c4085afed2be27a208d778f9ee13e671ab Mon Sep 17 00:00:00 2001 From: "reed@google.com" Date: Wed, 30 Oct 2013 17:39:43 +0000 Subject: [PATCH] remove contains(x,y) for rects and rrects ... not well defined, and unused BUG= R=robertphillips@google.com Review URL: https://codereview.chromium.org/51953003 git-svn-id: http://skia.googlecode.com/svn/trunk@12022 2bbb7eff-a529-9590-31e7-b0007b416f81 --- include/core/SkRRect.h | 24 ------------ include/core/SkRect.h | 29 +-------------- src/core/SkPath.cpp | 7 +++- src/core/SkRRect.cpp | 21 ----------- tests/RoundRectTest.cpp | 99 ------------------------------------------------- 5 files changed, 6 insertions(+), 174 deletions(-) diff --git a/include/core/SkRRect.h b/include/core/SkRRect.h index 32d6285..402e6c6 100644 --- a/include/core/SkRRect.h +++ b/include/core/SkRRect.h @@ -199,30 +199,6 @@ public: } /** - * Returns true if (p.fX,p.fY) is inside the RR, and the RR - * is not empty. - * - * Contains treats the left and top differently from the right and bottom. - * The left and top coordinates of the RR are themselves considered - * to be inside, while the right and bottom are not. All the points on the - * edges of the corners are considered to be inside. - */ - bool contains(const SkPoint& p) const { - return contains(p.fX, p.fY); - } - - /** - * Returns true if (x,y) is inside the RR, and the RR - * is not empty. - * - * Contains treats the left and top differently from the right and bottom. - * The left and top coordinates of the RR are themselves considered - * to be inside, while the right and bottom are not. All the points on the - * edges of the corners are considered to be inside. - */ - bool contains(SkScalar x, SkScalar y) const; - - /** * Call inset on the bounds, and adjust the radii to reflect what happens * in stroking: If the corner is sharp (no curvature), leave it alone, * otherwise we grow/shrink the radii by the amount of the inset. If a diff --git a/include/core/SkRect.h b/include/core/SkRect.h index c615603..d9ac3a6 100644 --- a/include/core/SkRect.h +++ b/include/core/SkRect.h @@ -718,38 +718,11 @@ struct SK_API SkRect { } /** - * Returns true if (p.fX,p.fY) is inside the rectangle, and the rectangle - * is not empty. - * - * Contains treats the left and top differently from the right and bottom. - * The left and top coordinates of the rectangle are themselves considered - * to be inside, while the right and bottom are not. Thus for the rectangle - * {0, 0, 5, 10}, (0,0) is contained, but (0,10), (5,0) and (5,10) are not. - */ - bool contains(const SkPoint& p) const { - return !this->isEmpty() && - fLeft <= p.fX && p.fX < fRight && fTop <= p.fY && p.fY < fBottom; - } - - /** - * Returns true if (x,y) is inside the rectangle, and the rectangle - * is not empty. - * - * Contains treats the left and top differently from the right and bottom. - * The left and top coordinates of the rectangle are themselves considered - * to be inside, while the right and bottom are not. Thus for the rectangle - * {0, 0, 5, 10}, (0,0) is contained, but (0,10), (5,0) and (5,10) are not. - */ - bool contains(SkScalar x, SkScalar y) const { - return !this->isEmpty() && - fLeft <= x && x < fRight && fTop <= y && y < fBottom; - } - - /** * Return true if this rectangle contains r, and if both rectangles are * not empty. */ bool contains(const SkRect& r) const { + // todo: can we eliminate the this->isEmpty check? return !r.isEmpty() && !this->isEmpty() && fLeft <= r.fLeft && fTop <= r.fTop && fRight >= r.fRight && fBottom >= r.fBottom; diff --git a/src/core/SkPath.cpp b/src/core/SkPath.cpp index 9df6285..8f79fbe 100644 --- a/src/core/SkPath.cpp +++ b/src/core/SkPath.cpp @@ -2961,14 +2961,17 @@ static int winding_line(const SkPoint pts[], SkScalar x, SkScalar y) { return dir; } +static bool contains_inclusive(const SkRect& r, SkScalar x, SkScalar y) { + return r.fLeft <= x && x <= r.fRight && r.fTop <= y && y <= r.fBottom; +} + bool SkPath::contains(SkScalar x, SkScalar y) const { bool isInverse = this->isInverseFillType(); if (this->isEmpty()) { return isInverse; } - const SkRect& bounds = this->getBounds(); - if (!bounds.contains(x, y)) { + if (!contains_inclusive(this->getBounds(), x, y)) { return isInverse; } diff --git a/src/core/SkRRect.cpp b/src/core/SkRRect.cpp index 75af106..e3d11cb 100644 --- a/src/core/SkRRect.cpp +++ b/src/core/SkRRect.cpp @@ -116,27 +116,6 @@ void SkRRect::setRectRadii(const SkRect& rect, const SkVector radii[4]) { SkDEBUGCODE(this->validate();) } -bool SkRRect::contains(SkScalar x, SkScalar y) const { - SkDEBUGCODE(this->validate();) - - if (kEmpty_Type == this->type()) { - return false; - } - - if (!fRect.contains(x, y)) { - return false; - } - - if (kRect_Type == this->type()) { - // the 'fRect' test above was sufficient - return true; - } - - // We know the point is inside the RR's bounds. The only way it can - // be out is if it outside one of the corners - return checkCornerContainment(x, y); -} - // This method determines if a point known to be inside the RRect's bounds is // inside all the corners. bool SkRRect::checkCornerContainment(SkScalar x, SkScalar y) const { diff --git a/tests/RoundRectTest.cpp b/tests/RoundRectTest.cpp index 93f5e7d..ec94c33 100644 --- a/tests/RoundRectTest.cpp +++ b/tests/RoundRectTest.cpp @@ -120,25 +120,6 @@ static void test_round_rect_basic(skiatest::Reporter* reporter) { // Test out the cases when the RR degenerates to a rect static void test_round_rect_rects(skiatest::Reporter* reporter) { SkRect r; - static const SkPoint pts[] = { - // Upper Left - { -SK_Scalar1, -SK_Scalar1 }, // out - { SK_Scalar1, SK_Scalar1 }, // in - // Upper Right - { SkIntToScalar(101), -SK_Scalar1}, // out - { SkIntToScalar(99), SK_Scalar1 }, // in - // Lower Right - { SkIntToScalar(101), SkIntToScalar(101) }, // out - { SkIntToScalar(99), SkIntToScalar(99) }, // in - // Lower Left - { -SK_Scalar1, SkIntToScalar(101) }, // out - { SK_Scalar1, SkIntToScalar(99) }, // in - // Middle - { SkIntToScalar(50), SkIntToScalar(50) } // in - }; - static const bool isIn[] = { false, true, false, true, false, true, false, true, true }; - - SkASSERT(SK_ARRAY_COUNT(pts) == SK_ARRAY_COUNT(isIn)); //---- SkRRect empty; @@ -157,9 +138,6 @@ static void test_round_rect_rects(skiatest::Reporter* reporter) { REPORTER_ASSERT(reporter, SkRRect::kRect_Type == rr1.type()); r = rr1.rect(); REPORTER_ASSERT(reporter, rect == r); - for (size_t i = 0; i < SK_ARRAY_COUNT(pts); ++i) { - REPORTER_ASSERT(reporter, isIn[i] == rr1.contains(pts[i].fX, pts[i].fY)); - } //---- SkPoint radii[4] = { { 0, 0 }, { 0, 0 }, { 0, 0 }, { 0, 0 } }; @@ -170,9 +148,6 @@ static void test_round_rect_rects(skiatest::Reporter* reporter) { REPORTER_ASSERT(reporter, SkRRect::kRect_Type == rr2.type()); r = rr2.rect(); REPORTER_ASSERT(reporter, rect == r); - for (size_t i = 0; i < SK_ARRAY_COUNT(pts); ++i) { - REPORTER_ASSERT(reporter, isIn[i] == rr2.contains(pts[i].fX, pts[i].fY)); - } //---- SkPoint radii2[4] = { { 0, 0 }, { 20, 20 }, { 50, 50 }, { 20, 50 } }; @@ -184,29 +159,6 @@ static void test_round_rect_rects(skiatest::Reporter* reporter) { // Test out the cases when the RR degenerates to an oval static void test_round_rect_ovals(skiatest::Reporter* reporter) { - static const SkScalar kEps = 0.1f; - static const SkScalar kWidthTol = SkScalarHalf(kWidth) * (SK_Scalar1 - SK_ScalarRoot2Over2); - static const SkScalar kHeightTol = SkScalarHalf(kHeight) * (SK_Scalar1 - SK_ScalarRoot2Over2); - static const SkPoint pts[] = { - // Upper Left - { kWidthTol - kEps, kHeightTol - kEps }, // out - { kWidthTol + kEps, kHeightTol + kEps }, // in - // Upper Right - { kWidth + kEps - kWidthTol, kHeightTol - kEps }, // out - { kWidth - kEps - kWidthTol, kHeightTol + kEps }, // in - // Lower Right - { kWidth + kEps - kWidthTol, kHeight + kEps - kHeightTol }, // out - { kWidth - kEps - kWidthTol, kHeight - kEps - kHeightTol }, // in - // Lower Left - { kWidthTol - kEps, kHeight + kEps - kHeightTol }, //out - { kWidthTol + kEps, kHeight - kEps - kHeightTol }, // in - // Middle - { SkIntToScalar(50), SkIntToScalar(50) } // in - }; - static const bool isIn[] = { false, true, false, true, false, true, false, true, true }; - - SkASSERT(SK_ARRAY_COUNT(pts) == SK_ARRAY_COUNT(isIn)); - //---- SkRect oval; SkRect rect = SkRect::MakeLTRB(0, 0, kWidth, kHeight); @@ -216,75 +168,24 @@ static void test_round_rect_ovals(skiatest::Reporter* reporter) { REPORTER_ASSERT(reporter, SkRRect::kOval_Type == rr1.type()); oval = rr1.rect(); REPORTER_ASSERT(reporter, oval == rect); - for (size_t i = 0; i < SK_ARRAY_COUNT(pts); ++i) { - REPORTER_ASSERT(reporter, isIn[i] == rr1.contains(pts[i].fX, pts[i].fY)); - } } // Test out the non-degenerate RR cases static void test_round_rect_general(skiatest::Reporter* reporter) { - static const SkScalar kEps = 0.1f; - static const SkScalar kDist20 = 20 * (SK_Scalar1 - SK_ScalarRoot2Over2); - static const SkPoint pts[] = { - // Upper Left - { kDist20 - kEps, kDist20 - kEps }, // out - { kDist20 + kEps, kDist20 + kEps }, // in - // Upper Right - { kWidth + kEps - kDist20, kDist20 - kEps }, // out - { kWidth - kEps - kDist20, kDist20 + kEps }, // in - // Lower Right - { kWidth + kEps - kDist20, kHeight + kEps - kDist20 }, // out - { kWidth - kEps - kDist20, kHeight - kEps - kDist20 }, // in - // Lower Left - { kDist20 - kEps, kHeight + kEps - kDist20 }, //out - { kDist20 + kEps, kHeight - kEps - kDist20 }, // in - // Middle - { SkIntToScalar(50), SkIntToScalar(50) } // in - }; - static const bool isIn[] = { false, true, false, true, false, true, false, true, true }; - - SkASSERT(SK_ARRAY_COUNT(pts) == SK_ARRAY_COUNT(isIn)); - //---- SkRect rect = SkRect::MakeLTRB(0, 0, kWidth, kHeight); SkRRect rr1; rr1.setRectXY(rect, 20, 20); REPORTER_ASSERT(reporter, SkRRect::kSimple_Type == rr1.type()); - for (size_t i = 0; i < SK_ARRAY_COUNT(pts); ++i) { - REPORTER_ASSERT(reporter, isIn[i] == rr1.contains(pts[i].fX, pts[i].fY)); - } //---- - static const SkScalar kDist50 = 50*(SK_Scalar1 - SK_ScalarRoot2Over2); - static const SkPoint pts2[] = { - // Upper Left - { -SK_Scalar1, -SK_Scalar1 }, // out - { SK_Scalar1, SK_Scalar1 }, // in - // Upper Right - { kWidth + kEps - kDist20, kDist20 - kEps }, // out - { kWidth - kEps - kDist20, kDist20 + kEps }, // in - // Lower Right - { kWidth + kEps - kDist50, kHeight + kEps - kDist50 }, // out - { kWidth - kEps - kDist50, kHeight - kEps - kDist50 }, // in - // Lower Left - { kDist20 - kEps, kHeight + kEps - kDist50 }, // out - { kDist20 + kEps, kHeight - kEps - kDist50 }, // in - // Middle - { SkIntToScalar(50), SkIntToScalar(50) } // in - }; - - SkASSERT(SK_ARRAY_COUNT(pts2) == SK_ARRAY_COUNT(isIn)); - SkPoint radii[4] = { { 0, 0 }, { 20, 20 }, { 50, 50 }, { 20, 50 } }; SkRRect rr2; rr2.setRectRadii(rect, radii); REPORTER_ASSERT(reporter, SkRRect::kComplex_Type == rr2.type()); - for (size_t i = 0; i < SK_ARRAY_COUNT(pts); ++i) { - REPORTER_ASSERT(reporter, isIn[i] == rr2.contains(pts2[i].fX, pts2[i].fY)); - } } // Test out questionable-parameter handling -- 2.7.4