fix path to rect when missing close verb

R=brianosman@google.com,robertphillips@google.com
TBR=reed@google.com
Bug: 824145,skia:7792
Change-Id: I24f121cfa7d437c95b94bd917d3c4888a10c519e
Reviewed-on: https://skia-review.googlesource.com/119569
Commit-Queue: Cary Clark <caryclark@skia.org>
Reviewed-by: Brian Osman <brianosman@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>

3 files changed, 25 insertions, 27 deletions

diff --git a/include/core/SkPath.h b/include/core/SkPath.h
index 2aab341505..895cf1ff75 100644
--- a/include/core/SkPath.h
+++ b/include/core/SkPath.h
@@ -1674,7 +1674,7 @@ private:
     SkDEBUGCODE(void validateRef() const { fPathRef->validate(); } )
 
     bool isRectContour(bool allowPartial, int* currVerb, const SkPoint** pts,
-                       bool* isClosed, Direction* direction) const;
+                       bool* isClosed, Direction* direction, SkRect* rect) const;
 
     // called by stroker to see if all points (in the last contour) are equal and worthy of a cap
     bool isZeroLengthSincePoint(int startPtIndex) const;
diff --git a/src/core/SkPath.cpp b/src/core/SkPath.cpp
index d57e95ad20..57192e8f9e 100644
--- a/src/core/SkPath.cpp
+++ b/src/core/SkPath.cpp
@@ -445,9 +445,10 @@ static int rect_make_dir(SkScalar dx, SkScalar dy) {
     return ((0 != dx) << 0) | ((dx > 0 || dy > 0) << 1);
 }
 bool SkPath::isRectContour(bool allowPartial, int* currVerb, const SkPoint** ptsPtr,
-        bool* isClosed, Direction* direction) const {
+        bool* isClosed, Direction* direction, SkRect* rect) const {
     int corners = 0;
     SkPoint first, last;
+    const SkPoint* firstPt = nullptr;
     const SkPoint* pts = *ptsPtr;
     const SkPoint* savePts = nullptr;
     first.set(0, 0);
@@ -524,6 +525,7 @@ bool SkPath::isRectContour(bool allowPartial, int* currVerb, const SkPoint** pts
                     *currVerb -= 1;  // try move again afterwards
                     goto addMissingClose;
                 }
+                firstPt = pts;
                 last = *pts++;
                 closedOrMoved = true;
                 break;
@@ -560,6 +562,10 @@ addMissingClose:
     if (savePts) {
         *ptsPtr = savePts;
     }
+    if (result && rect) {
+        ptrdiff_t count = (savePts ? savePts : pts) - firstPt;
+        rect->set(firstPt, (int) count);
+    }
     if (result && isClosed) {
         *isClosed = autoClose;
     }
@@ -573,40 +579,19 @@ bool SkPath::isRect(SkRect* rect, bool* isClosed, Direction* direction) const {
     SkDEBUGCODE(this->validate();)
     int currVerb = 0;
     const SkPoint* pts = fPathRef->points();
-    const SkPoint* first = pts;
-    if (!this->isRectContour(false, &currVerb, &pts, isClosed, direction)) {
-        return false;
-    }
-    if (rect) {
-        int32_t num = SkToS32(pts - first);
-        if (num) {
-            rect->set(first, num);
-        } else {
-            // 'pts' isn't updated for open rects
-            *rect = this->getBounds();
-        }
-    }
-    return true;
+    return this->isRectContour(false, &currVerb, &pts, isClosed, direction, rect);
 }
 
 bool SkPath::isNestedFillRects(SkRect rects[2], Direction dirs[2]) const {
     SkDEBUGCODE(this->validate();)
     int currVerb = 0;
     const SkPoint* pts = fPathRef->points();
-    const SkPoint* first = pts;
     Direction testDirs[2];
-    if (!isRectContour(true, &currVerb, &pts, nullptr, &testDirs[0])) {
+    SkRect testRects[2];
+    if (!isRectContour(true, &currVerb, &pts, nullptr, &testDirs[0], &testRects[0])) {
         return false;
     }
-    const SkPoint* last = pts;
-    SkRect testRects[2];
-    bool isClosed;
-    if (isRectContour(false, &currVerb, &pts, &isClosed, &testDirs[1])) {
-        testRects[0].set(first, SkToS32(last - first));
-        if (!isClosed) {
-            pts = fPathRef->points() + fPathRef->countPoints();
-        }
-        testRects[1].set(last, SkToS32(pts - last));
+    if (isRectContour(false, &currVerb, &pts, nullptr, &testDirs[1], &testRects[1])) {
         if (testRects[0].contains(testRects[1])) {
             if (rects) {
                 rects[0] = testRects[0];
diff --git a/tests/PathTest.cpp b/tests/PathTest.cpp
index a87b371b5e..c3c023d6a6 100644
--- a/tests/PathTest.cpp
+++ b/tests/PathTest.cpp
@@ -4899,3 +4899,16 @@ DEF_TEST(ClipPath_nonfinite, reporter) {
     REPORTER_ASSERT(reporter, !canvas->isClipEmpty());
 }
 
+// skbug.com/7792
+DEF_TEST(Path_isRect, reporter) {
+    SkPath path;
+    SkPoint points[] = { {10, 10}, {75, 75}, {150, 75}, {150, 150}, {75, 150} };
+    for (size_t index = 0; index < SK_ARRAY_COUNT(points); ++index) {
+        index < 2 ? path.moveTo(points[index]) : path.lineTo(points[index]);
+    }
+    SkRect rect;
+    REPORTER_ASSERT(reporter, path.isRect(&rect, nullptr, nullptr));
+    SkRect compare;
+    compare.set(&points[1], SK_ARRAY_COUNT(points) - 1);
+    REPORTER_ASSERT(reporter, rect == compare);
+}