From c91065d028472688ce15e635a29abe5256ff89ef Mon Sep 17 00:00:00 2001 From: fs Date: Thu, 17 Dec 2015 09:03:27 -0800 Subject: [PATCH] Use the unswapped end point y for early out case in winding_line The x-coordinates are not swapped, so using the swapped y will result in a comparison with the wrong (end) point. BUG=skia:4265 GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&issue=1533873002 Review URL: https://codereview.chromium.org/1533873002 --- src/core/SkPath.cpp | 9 +++++---- tests/PathTest.cpp | 10 ++++++++++ 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/src/core/SkPath.cpp b/src/core/SkPath.cpp index d32fb4c..d7e047e 100644 --- a/src/core/SkPath.cpp +++ b/src/core/SkPath.cpp @@ -2844,7 +2844,7 @@ static int winding_line(const SkPoint pts[], SkScalar x, SkScalar y, int* onCurv if (y < y0 || y > y1) { return 0; } - if (y == y1) { + if (y == pts[1].fY) { if (y0 == y1 && between(x0, x, x1) && x != x1) { // check if on horizontal line *onCurveCount += 1; } @@ -2853,9 +2853,10 @@ static int winding_line(const SkPoint pts[], SkScalar x, SkScalar y, int* onCurv SkScalar cross = SkScalarMul(x1 - x0, y - pts[0].fY) - SkScalarMul(dy, x - x0); if (!cross) { - if (x != x1 || y != pts[1].fY) { // don't test end points since they're also start points - *onCurveCount += 1; // zero cross means the point is on the line - } + // zero cross means the point is on the line, and since the case where + // y of the query point is at the end point is handled above, we can be + // sure that we're on the line (excluding the end point) here + *onCurveCount += 1; dir = 0; } else if (SkScalarSignAsInt(cross) == dir) { dir = 0; diff --git a/tests/PathTest.cpp b/tests/PathTest.cpp index 464943b..aa2ffdb 100644 --- a/tests/PathTest.cpp +++ b/tests/PathTest.cpp @@ -3542,6 +3542,16 @@ static void test_contains(skiatest::Reporter* reporter) { REPORTER_ASSERT(reporter, !p.contains(7, 7)); p.reset(); p.moveTo(4, 4); + p.lineTo(8, 4); + p.lineTo(8, 8); + p.lineTo(4, 8); + // test on vertices + REPORTER_ASSERT(reporter, p.contains(4, 4)); + REPORTER_ASSERT(reporter, p.contains(8, 4)); + REPORTER_ASSERT(reporter, p.contains(8, 8)); + REPORTER_ASSERT(reporter, p.contains(4, 8)); + p.reset(); + p.moveTo(4, 4); p.lineTo(6, 8); p.lineTo(2, 8); // test on edge -- 2.7.4