From 057ae8a15ddd2af639a829d63aca29cbc6b1bb57 Mon Sep 17 00:00:00 2001 From: bsalomon Date: Sun, 24 Jul 2016 05:37:26 -0700 Subject: [PATCH] Fix misdetection of rectangles in SkPath::IsSimpleClosedRect. BUG=chromium:630369 GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2175923002 Review-Url: https://codereview.chromium.org/2175923002 --- src/core/SkPath.cpp | 111 ++++++++++++++++---------------------------- tests/PathTest.cpp | 36 ++++++++++++++ 2 files changed, 77 insertions(+), 70 deletions(-) diff --git a/src/core/SkPath.cpp b/src/core/SkPath.cpp index 50789b42eb..64d3c0d36e 100644 --- a/src/core/SkPath.cpp +++ b/src/core/SkPath.cpp @@ -3292,81 +3292,52 @@ bool SkPathPriv::IsSimpleClosedRect(const SkPath& path, SkRect* rect, SkPath::Di if (rectPts[0] != rectPts[4]) { return false; } - int verticalCnt = 0; - int horizontalCnt = 0; - // dirs are 0 - right, 1 - down, 2 - left, 3 - up. - int firstDir = 0; - int secondDir = 0; - SkRect tempRect; - for (int i = 0; i < 4; ++i) { - int sameCnt = 0; - if (rectPts[i].fX == rectPts[i + 1].fX) { - verticalCnt += 1; - sameCnt = 1; - if (0 == i) { - if (rectPts[1].fY > rectPts[0].fY) { - firstDir = 1; - tempRect.fTop = rectPts[0].fY; - tempRect.fBottom = rectPts[1].fY; - } else { - firstDir = 3; - tempRect.fTop = rectPts[1].fY; - tempRect.fBottom = rectPts[0].fY; - } - } else if (1 == i) { - if (rectPts[2].fY > rectPts[1].fY) { - secondDir = 1; - tempRect.fTop = rectPts[1].fY; - tempRect.fBottom = rectPts[2].fY; - } else { - secondDir = 3; - tempRect.fTop = rectPts[2].fY; - tempRect.fBottom = rectPts[1].fY; - } - } - } - if (rectPts[i].fY == rectPts[i + 1].fY) { - horizontalCnt += 1; - sameCnt += 1; - if (0 == i) { - if (rectPts[1].fX > rectPts[0].fX) { - firstDir = 0; - tempRect.fLeft = rectPts[0].fX; - tempRect.fRight = rectPts[1].fX; - } else { - firstDir = 2; - tempRect.fLeft = rectPts[1].fX; - tempRect.fRight = rectPts[0].fX; - } - } else if (1 == i) { - if (rectPts[2].fX > rectPts[1].fX) { - secondDir = 0; - tempRect.fLeft = rectPts[1].fX; - tempRect.fRight = rectPts[2].fX; - } else { - secondDir = 2; - tempRect.fLeft = rectPts[2].fX; - tempRect.fRight = rectPts[1].fX; - } - } + // Check for two cases of rectangles: pts 0 and 3 form a vertical edge or a horizontal edge ( + // and pts 1 and 2 the opposite vertical or horizontal edge). + bool vec03IsVertical; + if (rectPts[0].fX == rectPts[3].fX && rectPts[1].fX == rectPts[2].fX && + rectPts[0].fY == rectPts[1].fY && rectPts[3].fY == rectPts[2].fY) { + // Make sure it has non-zero width and height + if (rectPts[0].fX == rectPts[1].fX || rectPts[0].fY == rectPts[3].fY) { + return false; } - if (sameCnt != 1) { + vec03IsVertical = true; + } else if (rectPts[0].fY == rectPts[3].fY && rectPts[1].fY == rectPts[2].fY && + rectPts[0].fX == rectPts[1].fX && rectPts[3].fX == rectPts[2].fX) { + // Make sure it has non-zero width and height + if (rectPts[0].fY == rectPts[1].fY || rectPts[0].fX == rectPts[3].fX) { return false; } - } - if (2 != horizontalCnt || 2 != verticalCnt) { + vec03IsVertical = false; + } else { return false; } - // low bit indicates a vertical dir - SkASSERT((firstDir ^ secondDir) & 0b1); - if (((firstDir + 1) & 0b11) == secondDir) { - *direction = SkPath::kCW_Direction; - *start = firstDir; - } else { - SkASSERT(((secondDir + 1) & 0b11) == firstDir); - *direction = SkPath::kCCW_Direction; - *start = secondDir; + // Set sortFlags so that it has the low bit set if pt index 0 is on right edge and second bit + // set if it is on the bottom edge. + unsigned sortFlags = + ((rectPts[0].fX < rectPts[2].fX) ? 0b00 : 0b01) | + ((rectPts[0].fY < rectPts[2].fY) ? 0b00 : 0b10); + switch (sortFlags) { + case 0b00: + rect->set(rectPts[0].fX, rectPts[0].fY, rectPts[2].fX, rectPts[2].fY); + *direction = vec03IsVertical ? SkPath::kCW_Direction : SkPath::kCCW_Direction; + *start = 0; + break; + case 0b01: + rect->set(rectPts[2].fX, rectPts[0].fY, rectPts[0].fX, rectPts[2].fY); + *direction = vec03IsVertical ? SkPath::kCCW_Direction : SkPath::kCW_Direction; + *start = 1; + break; + case 0b10: + rect->set(rectPts[0].fX, rectPts[2].fY, rectPts[2].fX, rectPts[0].fY); + *direction = vec03IsVertical ? SkPath::kCCW_Direction : SkPath::kCW_Direction; + *start = 3; + break; + case 0b11: + rect->set(rectPts[2].fX, rectPts[2].fY, rectPts[0].fX, rectPts[0].fY); + *direction = vec03IsVertical ? SkPath::kCW_Direction : SkPath::kCCW_Direction; + *start = 2; + break; } - *rect = tempRect; return true; } diff --git a/tests/PathTest.cpp b/tests/PathTest.cpp index 8f98e6abaa..cb330c58fe 100644 --- a/tests/PathTest.cpp +++ b/tests/PathTest.cpp @@ -2094,6 +2094,42 @@ static void test_is_simple_closed_rect(skiatest::Reporter* reporter) { check_simple_closed_rect(reporter, path2, testRect, swapDir, kYSwapStarts[start]); } } + // down, up, left, close + path.reset(); + path.moveTo(1, 1); + path.lineTo(1, 2); + path.lineTo(1, 1); + path.lineTo(0, 1); + SkRect rect; + SkPath::Direction dir; + unsigned start; + path.close(); + REPORTER_ASSERT(reporter, !SkPathPriv::IsSimpleClosedRect(path, &rect, &dir, &start)); + // right, left, up, close + path.reset(); + path.moveTo(1, 1); + path.lineTo(2, 1); + path.lineTo(1, 1); + path.lineTo(1, 0); + path.close(); + REPORTER_ASSERT(reporter, !SkPathPriv::IsSimpleClosedRect(path, &rect, &dir, &start)); + // parallelogram with horizontal edges + path.reset(); + path.moveTo(1, 0); + path.lineTo(3, 0); + path.lineTo(2, 1); + path.lineTo(0, 1); + path.close(); + REPORTER_ASSERT(reporter, !SkPathPriv::IsSimpleClosedRect(path, &rect, &dir, &start)); + // parallelogram with vertical edges + path.reset(); + path.moveTo(0, 1); + path.lineTo(0, 3); + path.lineTo(1, 2); + path.lineTo(1, 0); + path.close(); + REPORTER_ASSERT(reporter, !SkPathPriv::IsSimpleClosedRect(path, &rect, &dir, &start)); + } static void test_isNestedFillRects(skiatest::Reporter* reporter) { -- 2.34.1