another rect is path fix

This addresses comment #17 of skbug.com/7792.

The bug overshoots the end and exploits that the
first point tracked by close isn't the first
point in the rectangle.

Fixing this slightly regresses the example
in comment #14; before it was treated as a filled
rect but now it is not; this conservative approach
doesn't cause any other regressions.

bug7792 in pathfill.cpp verifies that all paths
in the bug draw correctly by comparing CPU and GPU.

R=robertphillips@google.com

Bug: 824145,skia:7792
Change-Id: I55bea023d2ad7456c8c3ebd9d1df95fe34e0a0d4
Reviewed-on: https://skia-review.googlesource.com/120996
Commit-Queue: Cary Clark <caryclark@skia.org>
Reviewed-by: Robert Phillips <robertphillips@google.com>

3 files changed, 21 insertions, 5 deletions

diff --git a/gm/pathfill.cpp b/gm/pathfill.cpp
index ff9f0083a2..18b30c9b0b 100644
--- a/gm/pathfill.cpp
+++ b/gm/pathfill.cpp
@@ -433,7 +433,7 @@ DEF_SIMPLE_GM(rotatedcubicpath, canvas, 200, 200) {
 DEF_GM( return new PathFillGM; )
 DEF_GM( return new PathInverseFillGM; )
 
-DEF_SIMPLE_GM(bug7792, canvas, 600, 400) {
+DEF_SIMPLE_GM(bug7792, canvas, 600, 600) {
     // from skbug.com/7792 bug description
     SkPaint p;
     SkPath path;
@@ -498,4 +498,15 @@ DEF_SIMPLE_GM(bug7792, canvas, 600, 400) {
     path.lineTo(75, 150);
     path.moveTo(250, 75);
     canvas->drawPath(path, p);
+    // from skbug.com/7792 comment 17
+    canvas->translate(-200 * 2, 200);
+    path.reset();
+    path.moveTo(75, 10);
+    path.moveTo(75, 75);
+    path.lineTo(150, 75);
+    path.lineTo(150, 150);
+    path.lineTo(75, 150);
+    path.lineTo(75, 10);
+    path.close();
+    canvas->drawPath(path, p);
 }
diff --git a/src/core/SkPath.cpp b/src/core/SkPath.cpp
index 9e242376c0..942b95da4d 100644
--- a/src/core/SkPath.cpp
+++ b/src/core/SkPath.cpp
@@ -467,7 +467,7 @@ bool SkPath::isRectContour(bool allowPartial, int* currVerb, const SkPoint** pts
         switch (verb) {
             case kClose_Verb:
                 savePts = pts;
-                pts = *ptsPtr;
+                pts = firstPt;
                 autoClose = true;
                 insertClose = false;
                 accumulatingRect = false;
diff --git a/tests/PathTest.cpp b/tests/PathTest.cpp
index ae6568f411..4d250283cf 100644
--- a/tests/PathTest.cpp
+++ b/tests/PathTest.cpp
@@ -4906,6 +4906,9 @@ DEF_TEST(Path_isRect, reporter) {
         for (size_t index = 0; index < count; ++index) {
             index < 2 ? path.moveTo(points[index]) : path.lineTo(points[index]);
         }
+        if (close) {
+            path.close();
+        }
         return path;
     };
     auto makePath2 = [](const SkPoint* points, const SkPath::Verb* verbs, size_t count) -> SkPath {
@@ -4961,9 +4964,7 @@ DEF_TEST(Path_isRect, reporter) {
     SkPoint points14[] = { {250, 75}, {250, 75}, {250, 75}, {100, 75},
                            {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));
-    compare.set(&points14[3], 5);
-    REPORTER_ASSERT(reporter, rect == compare);
+    REPORTER_ASSERT(reporter, !path.isRect(&rect, nullptr, nullptr));
     // isolated from skbug.com/7792 comment 15
     SkPath::Verb verbs15[] = { SkPath::kMove_Verb, SkPath::kLine_Verb, SkPath::kLine_Verb,
                                SkPath::kLine_Verb, SkPath::kMove_Verb };
@@ -4972,4 +4973,8 @@ 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
+    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));
 }