change isRect to return true for 3-sided rectangular paths

BUG=skia:
R=caryclark@google.com, yunchao.he@intel.com

Author: reed@google.com

Review URL: https://codereview.chromium.org/139483002

git-svn-id: http://skia.googlecode.com/svn/trunk@13092 2bbb7eff-a529-9590-31e7-b0007b416f81

diff --git a/src/core/SkPath.cpp b/src/core/SkPath.cpp
index af8b1aa..2f8eab2 100644 (file)
--- a/src/core/SkPath.cpp
+++ b/src/core/SkPath.cpp
@@ -359,10 +359,13 @@ The test fails if:
   There's more than four changes of direction.
   There's a discontinuity on the line (e.g., a move in the middle)
   The line reverses direction.
-  The rectangle doesn't complete a cycle.
   The path contains a quadratic or cubic.
   The path contains fewer than four points.
-  The final point isn't equal to the first point.
+ *The rectangle doesn't complete a cycle.
+ *The final point isn't equal to the first point.
+
+  *These last two conditions we relax if we have a 3-edge path that would
+   form a rectangle if it were closed (as we do when we fill a path)
 
 It's OK if the path has:
   Several colinear line segments composing a rectangle side.
@@ -374,7 +377,18 @@ must travel in opposite directions.
 FIXME: Allow colinear quads and cubics to be treated like lines.
 FIXME: If the API passes fill-only, return true if the filled stroke
        is a rectangle, though the caller failed to close the path.
+
+ first,last,next direction state-machine:
+    0x1 is set if the segment is horizontal
+    0x2 is set if the segment is moving to the right or down
+ thus:
+    two directions are opposites iff (dirA ^ dirB) == 0x2
+    two directions are perpendicular iff (dirA ^ dirB) == 0x1
+ 
  */
+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 {
     int corners = 0;
@@ -407,8 +421,7 @@ bool SkPath::isRectContour(bool allowPartial, int* currVerb, const SkPoint** pts
                 if (left == right && top == bottom) {
                     break; // single point on side OK
                 }
-                nextDirection = (left != right) << 0 |
-                    (left < right || top < bottom) << 1;
+                nextDirection = rect_make_dir(right - left, bottom - top);
                 if (0 == corners) {
                     firstDirection = nextDirection;
                     first = last;
@@ -460,6 +473,25 @@ bool SkPath::isRectContour(bool allowPartial, int* currVerb, const SkPoint** pts
     }
     // Success if 4 corners and first point equals last
     bool result = 4 == corners && (first == last || autoClose);
+    if (!result) {
+        // check if we are just an incomplete rectangle, in which case we can
+        // return true, but not claim to be closed.
+        // e.g.
+        //    3 sided rectangle
+        //    4 sided but the last edge is not long enough to reach the start
+        //
+        SkScalar closeX = first.x() - last.x();
+        SkScalar closeY = first.y() - last.y();
+        if (closeX && closeY) {
+            return false;   // we're diagonal, abort (can we ever reach this?)
+        }
+        int closeDirection = rect_make_dir(closeX, closeY);
+        // make sure the close-segment doesn't double-back on itself
+        if (3 == corners || (4 == corners && closeDirection == lastDirection)) {
+            result = true;
+            autoClose = false;  // we are not closed
+        }
+    }
     if (savePts) {
         *ptsPtr = savePts;
     }

diff --git a/tests/PathTest.cpp b/tests/PathTest.cpp
index 44de1aa..33d8da4 100644 (file)
--- a/tests/PathTest.cpp
+++ b/tests/PathTest.cpp
@@ -1438,16 +1438,8 @@ static void test_isRect_open_close(skiatest::Reporter* reporter) {
     bool isClosed;
 
     path.moveTo(0, 0); path.lineTo(1, 0); path.lineTo(1, 1); path.lineTo(0, 1);
-
-    if (false) {
-        // I think these should pass, but isRect() doesn't behave
-        // this way... yet
-        REPORTER_ASSERT(reporter, path.isRect(NULL, NULL));
-        REPORTER_ASSERT(reporter, path.isRect(&isClosed, NULL));
-        REPORTER_ASSERT(reporter, !isClosed);
-    }
-
     path.close();
+
     REPORTER_ASSERT(reporter, path.isRect(NULL, NULL));
     REPORTER_ASSERT(reporter, path.isRect(&isClosed, NULL));
     REPORTER_ASSERT(reporter, isClosed);
@@ -1488,9 +1480,16 @@ static void test_isRect(skiatest::Reporter* reporter) {
     SkPoint fa[] = {{1, 0}, {8, 0}, {8, 8}, {0, 8}, {0, -1}, {1, -1}}; // non colinear gap
     SkPoint fb[] = {{1, 0}, {8, 0}, {8, 8}, {0, 8}, {0, 1}}; // falls short
 
-    // failing, no close
-    SkPoint c1[] = {{0, 0}, {1, 0}, {1, 1}, {0, 1}}; // close doesn't match
-    SkPoint c2[] = {{0, 0}, {1, 0}, {1, 2}, {0, 2}, {0, 1}}; // ditto
+    // no close, but we should detect them as fillably the same as a rect
+    SkPoint c1[] = {{0, 0}, {1, 0}, {1, 1}, {0, 1}};
+    SkPoint c2[] = {{0, 0}, {1, 0}, {1, 2}, {0, 2}, {0, 1}};
+    SkPoint c3[] = {{0, 0}, {1, 0}, {1, 2}, {0, 2}, {0, 1}, {0, 0}}; // hit the start
+
+    // like c2, but we double-back on ourselves
+    SkPoint d1[] = {{0, 0}, {1, 0}, {1, 2}, {0, 2}, {0, 1}, {0, 2}};
+    // like c2, but we overshoot the start point
+    SkPoint d2[] = {{0, 0}, {1, 0}, {1, 2}, {0, 2}, {0, -1}};
+    SkPoint d3[] = {{0, 0}, {1, 0}, {1, 2}, {0, 2}, {0, -1}, {0, 0}};
 
     struct IsRectTest {
         SkPoint *fPoints;
@@ -1526,8 +1525,13 @@ static void test_isRect(skiatest::Reporter* reporter) {
         { fa, SK_ARRAY_COUNT(fa), true, false },
         { fb, SK_ARRAY_COUNT(fb), true, false },
 
-        { c1, SK_ARRAY_COUNT(c1), false, false },
-        { c2, SK_ARRAY_COUNT(c2), false, false },
+        { c1, SK_ARRAY_COUNT(c1), false, true },
+        { c2, SK_ARRAY_COUNT(c2), false, true },
+        { c3, SK_ARRAY_COUNT(c3), false, true },
+
+        { d1, SK_ARRAY_COUNT(d1), false, false },
+        { d2, SK_ARRAY_COUNT(d2), false, false },
+        { d3, SK_ARRAY_COUNT(d3), false, false },
     };
 
     const size_t testCount = SK_ARRAY_COUNT(tests);
@@ -3246,7 +3250,7 @@ public:
     }
 };
 
-DEF_TEST(Path, reporter) {
+DEF_TEST(Paths, reporter) {
     SkTSize<SkScalar>::Make(3,4);
 
     SkPath  p, empty;