more path is rect bugs

More edge cases found; clean up the logic a bit
to make more clear where the rectangle points
start and stop.

R=robertphillips@google.com,brianosman@google.com
Bug: 824145,skia:7792
Change-Id: Ie24dfd1519f30875f44ffac68e20d777490b00b9
Reviewed-on: https://skia-review.googlesource.com/120422
Commit-Queue: Cary Clark <caryclark@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>

1 files changed, 32 insertions, 19 deletions

diff --git a/src/core/SkPath.cpp b/src/core/SkPath.cpp
index 284a6a716c..9e242376c0 100644
--- a/src/core/SkPath.cpp
+++ b/src/core/SkPath.cpp
@@ -447,18 +447,20 @@ static int rect_make_dir(SkScalar dx, SkScalar dy) {
 bool SkPath::isRectContour(bool allowPartial, int* currVerb, const SkPoint** ptsPtr,
         bool* isClosed, Direction* direction, SkRect* rect) const {
     int corners = 0;
-    SkPoint first, last;
-    const SkPoint* firstPt = nullptr;
+    SkPoint previous;  // used to construct line from previous point
+    const SkPoint* firstPt = nullptr; // first point in the rect (last of first moves)
+    const SkPoint* lastPt = nullptr;  // last point in the rect (last of lines or first if closed)
     const SkPoint* pts = *ptsPtr;
-    const SkPoint* savePts = nullptr;
-    first.set(0, 0);
-    last.set(0, 0);
+    const SkPoint* savePts = nullptr; // used to allow caller to iterate through a pair of rects
+    previous.set(0, 0);
     int firstDirection = 0;
     int lastDirection = 0;
     int nextDirection = 0;
     bool closedOrMoved = false;
+    bool addedLine = false;
     bool autoClose = false;
     bool insertClose = false;
+    bool accumulatingRect = false;
     int verbCnt = fPathRef->countVerbs();
     while (*currVerb < verbCnt && (!allowPartial || !autoClose)) {
         uint8_t verb = insertClose ? (uint8_t) kClose_Verb : fPathRef->atVerb(*currVerb);
@@ -468,23 +470,27 @@ bool SkPath::isRectContour(bool allowPartial, int* currVerb, const SkPoint** pts
                 pts = *ptsPtr;
                 autoClose = true;
                 insertClose = false;
+                accumulatingRect = false;
             case kLine_Verb: {
-                SkScalar left = last.fX;
-                SkScalar top = last.fY;
+                SkScalar left = previous.fX;
+                SkScalar top = previous.fY;
                 SkScalar right = pts->fX;
                 SkScalar bottom = pts->fY;
+                if (accumulatingRect) {
+                    lastPt = pts;
+                }
                 ++pts;
                 if (left != right && top != bottom) {
                     return false; // diagonal
                 }
+                addedLine = true;
                 if (left == right && top == bottom) {
                     break; // single point on side OK
                 }
                 nextDirection = rect_make_dir(right - left, bottom - top);
                 if (0 == corners) {
                     firstDirection = nextDirection;
-                    first = last;
-                    last = pts[-1];
+                    previous = pts[-1];
                     corners = 1;
                     closedOrMoved = false;
                     break;
@@ -501,7 +507,7 @@ bool SkPath::isRectContour(bool allowPartial, int* currVerb, const SkPoint** pts
                         return false; // too many direction changes
                     }
                 }
-                last = pts[-1];
+                previous = pts[-1];
                 if (lastDirection == nextDirection) {
                     break; // colinear segment
                 }
@@ -525,8 +531,13 @@ bool SkPath::isRectContour(bool allowPartial, int* currVerb, const SkPoint** pts
                     *currVerb -= 1;  // try move again afterwards
                     goto addMissingClose;
                 }
-                firstPt = pts;
-                last = *pts++;
+                if (!addedLine) {
+                    firstPt = pts;
+                    accumulatingRect = true;
+                } else {
+                    accumulatingRect = false;
+                }
+                previous = *pts++;
                 closedOrMoved = true;
                 break;
             default:
@@ -539,10 +550,12 @@ addMissingClose:
         ;
     }
     // Success if 4 corners and first point equals last
-    SkScalar closeX = first.x() - last.x();
-    SkScalar closeY = first.y() - last.y();
+    if (corners < 3 || corners > 4) {
+        return false;
+    }
+    SkPoint closeXY = *firstPt - *lastPt;
     // If autoClose, check if close generates diagonal
-    bool result = 4 == corners && (first == last || (autoClose && (!closeX || !closeY)));
+    bool result = 4 == corners && (closeXY.isZero() || (autoClose && (!closeXY.fX || !closeXY.fY)));
     if (!result) {
         // check if we are just an incomplete rectangle, in which case we can
         // return true, but not claim to be closed.
@@ -550,12 +563,12 @@ addMissingClose:
         //    3 sided rectangle
         //    4 sided but the last edge is not long enough to reach the start
         //
-        if (closeX && closeY) {
+        if (closeXY.fX && closeXY.fY) {
             return false;   // we're diagonal, abort (can we ever reach this?)
         }
-        int closeDirection = rect_make_dir(closeX, closeY);
+        int closeDirection = rect_make_dir(closeXY.fX, closeXY.fY);
         // make sure the close-segment doesn't double-back on itself
-        if (3 == corners || (4 == corners && closeDirection == lastDirection)) {
+        if (3 == corners || closeDirection == lastDirection) {
             result = true;
             autoClose = false;  // we are not closed
         }
@@ -564,7 +577,7 @@ addMissingClose:
         *ptsPtr = savePts;
     }
     if (result && rect) {
-        ptrdiff_t count = (savePts ? savePts : pts) - firstPt;
+        ptrdiff_t count = lastPt - firstPt + 1;
         rect->set(firstPt, (int) count);
     }
     if (result && isClosed) {