path is rect bug number nine

This variation exploits a sequence which uses a zero
length line to note that lines have been recorded, but
no rectangle edge has been encountered.

R=robertphillips@google.com

Docs-Preview: https://skia.org/?cl=121282
Bug: 824145,skia:7792
Change-Id: I652e9482b2867c3d7da30d5f5df2aecbfd0d716d
Reviewed-on: https://skia-review.googlesource.com/121282
Reviewed-by: Robert Phillips <robertphillips@google.com>
Commit-Queue: Cary Clark <caryclark@skia.org>

3 files changed, 38 insertions, 16 deletions

diff --git a/gm/pathfill.cpp b/gm/pathfill.cpp
index 74cafcb1c4..1bf396c513 100644
--- a/gm/pathfill.cpp
+++ b/gm/pathfill.cpp
@@ -443,7 +443,7 @@ DEF_SIMPLE_GM(bug7792, canvas, 600, 600) {
     path.lineTo(150, 150);
     path.lineTo(75, 150);
     canvas->drawPath(path, p);
-    // from skbug.com/7792 comment 3
+    // from skbug.com/7792#c3
     canvas->translate(200, 0);
     path.reset();
     path.moveTo(75, 50);
@@ -454,7 +454,7 @@ DEF_SIMPLE_GM(bug7792, canvas, 600, 600) {
     path.lineTo(75, 50);
     path.close();
     canvas->drawPath(path, p);
-    // from skbug.com/7792 comment 9
+    // from skbug.com/7792#c9
     canvas->translate(200, 0);
     path.reset();
     path.moveTo(10, 10);
@@ -464,7 +464,7 @@ DEF_SIMPLE_GM(bug7792, canvas, 600, 600) {
     path.lineTo(75, 150);
     path.close();
     canvas->drawPath(path, p);
-    // from skbug.com/7792 comment 11
+    // from skbug.com/7792#c11
     canvas->translate(-200 * 2, 200);
     path.reset();
     path.moveTo(75, 150);
@@ -474,7 +474,7 @@ DEF_SIMPLE_GM(bug7792, canvas, 600, 600) {
     path.lineTo(75, 150);
     path.moveTo(75, 150);
     canvas->drawPath(path, p);
-    // from skbug.com/7792 comment 14
+    // from skbug.com/7792#c14
     canvas->translate(200, 0);
     path.reset();
     path.moveTo(250, 75);
@@ -489,7 +489,7 @@ DEF_SIMPLE_GM(bug7792, canvas, 600, 600) {
     path.lineTo(0, 0);
     path.close();
     canvas->drawPath(path, p);
-    // from skbug.com/7792 comment 15
+    // from skbug.com/7792#c15
     canvas->translate(200, 0);
     path.reset();
     path.moveTo(75, 75);
@@ -498,7 +498,7 @@ DEF_SIMPLE_GM(bug7792, canvas, 600, 600) {
     path.lineTo(75, 150);
     path.moveTo(250, 75);
     canvas->drawPath(path, p);
-    // from skbug.com/7792 comment 17
+    // from skbug.com/7792#c17
     canvas->translate(-200 * 2, 200);
     path.reset();
     path.moveTo(75, 10);
@@ -509,7 +509,7 @@ DEF_SIMPLE_GM(bug7792, canvas, 600, 600) {
     path.lineTo(75, 10);
     path.close();
     canvas->drawPath(path, p);
-    // from skbug.com/7792 comment 19
+    // from skbug.com/7792#c19
     canvas->translate(200, 0);
     path.reset();
     path.moveTo(75, 75);
@@ -524,4 +524,16 @@ DEF_SIMPLE_GM(bug7792, canvas, 600, 600) {
     path.lineTo(30, 10);
     path.lineTo(10, 30);
     canvas->drawPath(path, p);
+    // from skbug.com/7792#c23
+    canvas->translate(200, 0);
+    path.reset();
+    path.moveTo(75, 75);
+    path.lineTo(75, 75);
+    path.moveTo(75, 75);
+    path.lineTo(75, 75);
+    path.lineTo(150, 75);
+    path.lineTo(150, 150);
+    path.lineTo(75, 150);
+    path.close();
+    canvas->drawPath(path, p);
 }
diff --git a/src/core/SkPath.cpp b/src/core/SkPath.cpp
index beb46eb7ee..2fd5201066 100644
--- a/src/core/SkPath.cpp
+++ b/src/core/SkPath.cpp
@@ -482,10 +482,10 @@ bool SkPath::isRectContour(bool allowPartial, int* currVerb, const SkPoint** pts
                 if (lineDelta.fX && lineDelta.fY) {
                     return false; // diagonal
                 }
-                addedLine = true;
                 if (lineStart == lineEnd) {
                     break; // single point on side OK
                 }
+                addedLine = true;
                 nextDirection = rect_make_dir(lineDelta.fX, lineDelta.fY);
                 if (0 == corners) {
                     firstDirection = nextDirection;
diff --git a/tests/PathTest.cpp b/tests/PathTest.cpp
index b96b93ccfe..8ac6e02e17 100644
--- a/tests/PathTest.cpp
+++ b/tests/PathTest.cpp
@@ -4930,7 +4930,7 @@ DEF_TEST(Path_isRect, reporter) {
         }
         return path;
     };
-    // isolated from skbug.com/7792 bug description
+    // isolated from skbug.com/7792 (bug description)
     SkRect rect;
     SkPoint points[] = { {10, 10}, {75, 75}, {150, 75}, {150, 150}, {75, 150} };
     SkPath path = makePath(points, SK_ARRAY_COUNT(points), false);
@@ -4938,17 +4938,17 @@ DEF_TEST(Path_isRect, reporter) {
     SkRect compare;
     compare.set(&points[1], SK_ARRAY_COUNT(points) - 1);
     REPORTER_ASSERT(reporter, rect == compare);
-    // isolated from skbug.com/7792 comment 3
+    // isolated from skbug.com/7792#c3
     SkPoint points3[] = { {75, 50}, {100, 75}, {150, 75}, {150, 150}, {75, 150}, {75, 50} };
     path = makePath(points3, SK_ARRAY_COUNT(points3), true);
     REPORTER_ASSERT(reporter, !path.isRect(&rect, nullptr, nullptr));
-    // isolated from skbug.com/7792 comment 9
+    // isolated from skbug.com/7792#c9
     SkPoint points9[] = { {10, 10}, {75, 75}, {150, 75}, {150, 150}, {75, 150} };
     path = makePath(points9, SK_ARRAY_COUNT(points9), true);
     REPORTER_ASSERT(reporter, path.isRect(&rect, nullptr, nullptr));
     compare.set(&points9[1], SK_ARRAY_COUNT(points9) - 1);
     REPORTER_ASSERT(reporter, rect == compare);
-    // isolated from skbug.com/7792 comment 11
+    // isolated from skbug.com/7792#c11
     SkPath::Verb verbs11[] = { SkPath::kMove_Verb, SkPath::kLine_Verb, SkPath::kLine_Verb,
                                SkPath::kLine_Verb, SkPath::kLine_Verb, SkPath::kMove_Verb };
     SkPoint points11[] = { {75, 150}, {75, 75}, {150, 75}, {150, 150}, {75, 150}, {75, 150} };
@@ -4956,7 +4956,7 @@ DEF_TEST(Path_isRect, reporter) {
     REPORTER_ASSERT(reporter, path.isRect(&rect, nullptr, nullptr));
     compare.set(&points11[0], SK_ARRAY_COUNT(points11));
     REPORTER_ASSERT(reporter, rect == compare);
-    // isolated from skbug.com/7792 comment 14
+    // isolated from skbug.com/7792#c14
     SkPath::Verb verbs14[] = { SkPath::kMove_Verb, SkPath::kMove_Verb, SkPath::kMove_Verb,
                                SkPath::kMove_Verb, SkPath::kLine_Verb, SkPath::kLine_Verb,
                                SkPath::kLine_Verb, SkPath::kLine_Verb, SkPath::kClose_Verb,
@@ -4965,7 +4965,7 @@ DEF_TEST(Path_isRect, reporter) {
                            {150, 75}, {150, 150}, {75, 150}, {75, 75}, {0, 0} };
     path = makePath2(points14, verbs14, SK_ARRAY_COUNT(verbs14));
     REPORTER_ASSERT(reporter, !path.isRect(&rect, nullptr, nullptr));
-    // isolated from skbug.com/7792 comment 15
+    // isolated from skbug.com/7792#c15
     SkPath::Verb verbs15[] = { SkPath::kMove_Verb, SkPath::kLine_Verb, SkPath::kLine_Verb,
                                SkPath::kLine_Verb, SkPath::kMove_Verb };
     SkPoint points15[] = { {75, 75}, {150, 75}, {150, 150}, {75, 150}, {250, 75} };
@@ -4973,11 +4973,11 @@ DEF_TEST(Path_isRect, reporter) {
     REPORTER_ASSERT(reporter, path.isRect(&rect, nullptr, nullptr));
     compare.set(&points15[0], SK_ARRAY_COUNT(points15) - 1);
     REPORTER_ASSERT(reporter, rect == compare);
-    // isolated from skbug.com/7792 comment 17
+    // isolated from skbug.com/7792#c17
     SkPoint points17[] = { {75, 10}, {75, 75}, {150, 75}, {150, 150}, {75, 150}, {75, 10} };
     path = makePath(points17, SK_ARRAY_COUNT(points17), true);
     REPORTER_ASSERT(reporter, !path.isRect(&rect, nullptr, nullptr));
-    // isolated from skbug.com/7792 comment 19
+    // isolated from skbug.com/7792#c19
     SkPath::Verb verbs19[] = { SkPath::kMove_Verb, SkPath::kLine_Verb, SkPath::kLine_Verb,
                                SkPath::kLine_Verb, SkPath::kLine_Verb, SkPath::kLine_Verb,
                                SkPath::kLine_Verb, SkPath::kClose_Verb, SkPath::kMove_Verb,
@@ -4986,4 +4986,14 @@ DEF_TEST(Path_isRect, reporter) {
                            {75, 150}, {10, 10}, {30, 10}, {10, 30} };
     path = makePath2(points19, verbs19, SK_ARRAY_COUNT(verbs19));
     REPORTER_ASSERT(reporter, !path.isRect(&rect, nullptr, nullptr));
+    // isolated from skbug.com/7792#c23
+    SkPath::Verb verbs23[] = { SkPath::kMove_Verb, SkPath::kLine_Verb, SkPath::kMove_Verb,
+                               SkPath::kLine_Verb, SkPath::kLine_Verb, SkPath::kLine_Verb,
+                               SkPath::kLine_Verb, SkPath::kClose_Verb };
+    SkPoint points23[] = { {75, 75}, {75, 75}, {75, 75}, {75, 75}, {150, 75}, {150, 150},
+                           {75, 150} };
+    path = makePath2(points23, verbs23, SK_ARRAY_COUNT(verbs23));
+    REPORTER_ASSERT(reporter, path.isRect(&rect, nullptr, nullptr));
+    compare.set(&points23[0], SK_ARRAY_COUNT(points23));
+    REPORTER_ASSERT(reporter, rect == compare);
 }