change isNestedRect to isNestedFillRect

R=reed@google.com, bsalomon@google.com

Let isNested(Fill)Rect return true if drawn path describes filled
rectangles.

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

8 files changed, 56 insertions, 26 deletions

diff --git a/include/core/SkPath.h b/include/core/SkPath.h
index 7444cc97ca..b5b95e15e7 100644
--- a/include/core/SkPath.h
+++ b/include/core/SkPath.h
@@ -581,7 +581,8 @@ public:
      */
     bool isRect(SkRect* rect, bool* isClosed = NULL, Direction* direction = NULL) const;
 
-    /** Returns true if the path specifies a pair of nested rectangles. If so, and if
+    /** Returns true if the path specifies a pair of nested rectangles, or would draw a
+        pair of nested rectangles when filled. If so, and if
         rect is not null, set rect[0] to the outer rectangle and rect[1] to the inner
         rectangle. If so, and dirs is not null, set dirs[0] to the direction of
         the outer rectangle and dirs[1] to the direction of the inner rectangle. If
@@ -592,7 +593,7 @@ public:
         @param dirs If not null, returns the direction of the rects
         @return true if the path describes a pair of nested rectangles
     */
-    bool isNestedRects(SkRect rect[2], Direction dirs[2] = NULL) const;
+    bool isNestedFillRects(SkRect rect[2], Direction dirs[2] = NULL) const;
 
     /**
      *  Add a closed rectangle contour to the path
diff --git a/src/core/SkMaskFilter.cpp b/src/core/SkMaskFilter.cpp
index 54a22fc900..5e20c151f6 100644
--- a/src/core/SkMaskFilter.cpp
+++ b/src/core/SkMaskFilter.cpp
@@ -208,7 +208,7 @@ static void draw_nine(const SkMask& mask, const SkIRect& outerR, const SkIPoint&
 }
 
 static int countNestedRects(const SkPath& path, SkRect rects[2]) {
-    if (path.isNestedRects(rects)) {
+    if (path.isNestedFillRects(rects)) {
         return 2;
     }
     return path.isRect(&rects[0]);
diff --git a/src/core/SkPath.cpp b/src/core/SkPath.cpp
index 76e70d99cf..19e7048869 100644
--- a/src/core/SkPath.cpp
+++ b/src/core/SkPath.cpp
@@ -396,13 +396,16 @@ bool SkPath::isRectContour(bool allowPartial, int* currVerb, const SkPoint** pts
     int nextDirection = 0;
     bool closedOrMoved = false;
     bool autoClose = false;
+    bool insertClose = false;
     int verbCnt = fPathRef->countVerbs();
     while (*currVerb < verbCnt && (!allowPartial || !autoClose)) {
-        switch (fPathRef->atVerb(*currVerb)) {
+        uint8_t verb = insertClose ? (uint8_t) kClose_Verb : fPathRef->atVerb(*currVerb);
+        switch (verb) {
             case kClose_Verb:
                 savePts = pts;
                 pts = *ptsPtr;
                 autoClose = true;
+                insertClose = false;
             case kLine_Verb: {
                 SkScalar left = last.fX;
                 SkScalar top = last.fY;
@@ -455,6 +458,11 @@ bool SkPath::isRectContour(bool allowPartial, int* currVerb, const SkPoint** pts
             case kCubic_Verb:
                 return false; // quadratic, cubic not allowed
             case kMove_Verb:
+                if (allowPartial && !autoClose && firstDirection) {
+                    insertClose = true;
+                    *currVerb -= 1;  // try move again afterwards
+                    goto addMissingClose;
+                }
                 last = *pts++;
                 closedOrMoved = true;
                 break;
@@ -464,6 +472,8 @@ bool SkPath::isRectContour(bool allowPartial, int* currVerb, const SkPoint** pts
         }
         *currVerb += 1;
         lastDirection = nextDirection;
+addMissingClose:
+        ;
     }
     // Success if 4 corners and first point equals last
     bool result = 4 == corners && (first == last || autoClose);
@@ -518,7 +528,7 @@ bool SkPath::isRect(SkRect* rect, bool* isClosed, Direction* direction) const {
     return true;
 }
 
-bool SkPath::isNestedRects(SkRect rects[2], Direction dirs[2]) const {
+bool SkPath::isNestedFillRects(SkRect rects[2], Direction dirs[2]) const {
     SkDEBUGCODE(this->validate();)
     int currVerb = 0;
     const SkPoint* pts = fPathRef->points();
@@ -529,8 +539,12 @@ bool SkPath::isNestedRects(SkRect rects[2], Direction dirs[2]) const {
     }
     const SkPoint* last = pts;
     SkRect testRects[2];
-    if (isRectContour(false, &currVerb, &pts, NULL, &testDirs[1])) {
+    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 (testRects[0].contains(testRects[1])) {
             if (rects) {
diff --git a/src/gpu/GrContext.cpp b/src/gpu/GrContext.cpp
index b38bc1eea8..40e714b152 100755
--- a/src/gpu/GrContext.cpp
+++ b/src/gpu/GrContext.cpp
@@ -1280,7 +1280,7 @@ static bool is_nested_rects(GrDrawTarget* target,
     }
 
     SkPath::Direction dirs[2];
-    if (!path.isNestedRects(rects, dirs)) {
+    if (!path.isNestedFillRects(rects, dirs)) {
         return false;
     }
 
diff --git a/src/utils/SkLua.cpp b/src/utils/SkLua.cpp
index 84932cd23e..11d9b2938c 100644
--- a/src/utils/SkLua.cpp
+++ b/src/utils/SkLua.cpp
@@ -1495,10 +1495,10 @@ static const char* dir2string(SkPath::Direction dir) {
     return gStr[dir];
 }
 
-static int lpath_isNestedRects(lua_State* L) {
+static int lpath_isNestedFillRects(lua_State* L) {
     SkRect rects[2];
     SkPath::Direction dirs[2];
-    bool pred = get_obj<SkPath>(L, 1)->isNestedRects(rects, dirs);
+    bool pred = get_obj<SkPath>(L, 1)->isNestedFillRects(rects, dirs);
     int ret_count = 1;
     lua_pushboolean(L, pred);
     if (pred) {
@@ -1562,7 +1562,7 @@ static const struct luaL_Reg gSkPath_Methods[] = {
     { "isConvex", lpath_isConvex },
     { "isEmpty", lpath_isEmpty },
     { "isRect", lpath_isRect },
-    { "isNestedRects", lpath_isNestedRects },
+    { "isNestedFillRects", lpath_isNestedFillRects },
     { "countPoints", lpath_countPoints },
     { "reset", lpath_reset },
     { "moveTo", lpath_moveTo },
diff --git a/tests/PathTest.cpp b/tests/PathTest.cpp
index 964b7e130c..b78b343ccc 100644
--- a/tests/PathTest.cpp
+++ b/tests/PathTest.cpp
@@ -1857,7 +1857,7 @@ static void test_isRect(skiatest::Reporter* reporter) {
     REPORTER_ASSERT(reporter, !path1.isRect(NULL));
 }
 
-static void test_isNestedRects(skiatest::Reporter* reporter) {
+static void test_isNestedFillRects(skiatest::Reporter* reporter) {
     // passing tests (all moveTo / lineTo...
     SkPoint r1[] = {{0, 0}, {1, 0}, {1, 1}, {0, 1}}; // CW
     SkPoint r2[] = {{1, 0}, {1, 1}, {0, 1}, {0, 0}};
@@ -1884,7 +1884,7 @@ static void test_isNestedRects(skiatest::Reporter* reporter) {
     SkPoint f7[] = {{0, 0}, {1, 0}, {1, 1}, {0, 2}}; // end overshoots
     SkPoint f8[] = {{0, 0}, {1, 0}, {1, 1}, {1, 0}}; // 'L'
 
-    // failing, no close
+    // success, no close is OK
     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
 
@@ -1919,8 +1919,8 @@ static void test_isNestedRects(skiatest::Reporter* reporter) {
         { f7, SK_ARRAY_COUNT(f7), SkPath::kUnknown_Direction, true, false },
         { f8, SK_ARRAY_COUNT(f8), SkPath::kUnknown_Direction, true, false },
 
-        { c1, SK_ARRAY_COUNT(c1), SkPath::kUnknown_Direction, false, false },
-        { c2, SK_ARRAY_COUNT(c2), SkPath::kUnknown_Direction, false, false },
+        { c1, SK_ARRAY_COUNT(c1), SkPath::kCW_Direction, false, true },
+        { c2, SK_ARRAY_COUNT(c2), SkPath::kCW_Direction, false, true },
     };
 
     const size_t testCount = SK_ARRAY_COUNT(tests);
@@ -1941,7 +1941,8 @@ static void test_isNestedRects(skiatest::Reporter* reporter) {
             if (!rectFirst) {
                 path.addRect(-1, -1, 2, 2, SkPath::kCCW_Direction);
             }
-            REPORTER_ASSERT(reporter, tests[testIndex].fIsNestedRect == path.isNestedRects(NULL));
+            REPORTER_ASSERT(reporter,
+                    tests[testIndex].fIsNestedRect == path.isNestedFillRects(NULL));
             if (tests[testIndex].fIsNestedRect) {
                 SkRect expected[2], computed[2];
                 SkPath::Direction expectedDirs[2], computedDirs[2];
@@ -1955,7 +1956,7 @@ static void test_isNestedRects(skiatest::Reporter* reporter) {
                     expectedDirs[0] = SkPath::kCCW_Direction;
                 }
                 expectedDirs[1] = tests[testIndex].fDirection;
-                REPORTER_ASSERT(reporter, path.isNestedRects(computed, computedDirs));
+                REPORTER_ASSERT(reporter, path.isNestedFillRects(computed, computedDirs));
                 REPORTER_ASSERT(reporter, expected[0] == computed[0]);
                 REPORTER_ASSERT(reporter, expected[1] == computed[1]);
                 REPORTER_ASSERT(reporter, expectedDirs[0] == computedDirs[0]);
@@ -1977,7 +1978,7 @@ static void test_isNestedRects(skiatest::Reporter* reporter) {
         if (!rectFirst) {
             path1.addRect(-1, -1, 2, 2, SkPath::kCCW_Direction);
         }
-        REPORTER_ASSERT(reporter, !path1.isNestedRects(NULL));
+        REPORTER_ASSERT(reporter, !path1.isNestedFillRects(NULL));
 
         // fail, move in the middle
         path1.reset();
@@ -1995,7 +1996,7 @@ static void test_isNestedRects(skiatest::Reporter* reporter) {
         if (!rectFirst) {
             path1.addRect(-1, -1, 2, 2, SkPath::kCCW_Direction);
         }
-        REPORTER_ASSERT(reporter, !path1.isNestedRects(NULL));
+        REPORTER_ASSERT(reporter, !path1.isNestedFillRects(NULL));
 
         // fail, move on the edge
         path1.reset();
@@ -2010,7 +2011,7 @@ static void test_isNestedRects(skiatest::Reporter* reporter) {
         if (!rectFirst) {
             path1.addRect(-1, -1, 2, 2, SkPath::kCCW_Direction);
         }
-        REPORTER_ASSERT(reporter, !path1.isNestedRects(NULL));
+        REPORTER_ASSERT(reporter, !path1.isNestedFillRects(NULL));
 
         // fail, quad
         path1.reset();
@@ -2028,7 +2029,7 @@ static void test_isNestedRects(skiatest::Reporter* reporter) {
         if (!rectFirst) {
             path1.addRect(-1, -1, 2, 2, SkPath::kCCW_Direction);
         }
-        REPORTER_ASSERT(reporter, !path1.isNestedRects(NULL));
+        REPORTER_ASSERT(reporter, !path1.isNestedFillRects(NULL));
 
         // fail, cubic
         path1.reset();
@@ -2046,15 +2047,29 @@ static void test_isNestedRects(skiatest::Reporter* reporter) {
         if (!rectFirst) {
             path1.addRect(-1, -1, 2, 2, SkPath::kCCW_Direction);
         }
-        REPORTER_ASSERT(reporter, !path1.isNestedRects(NULL));
+        REPORTER_ASSERT(reporter, !path1.isNestedFillRects(NULL));
 
         // fail,  not nested
         path1.reset();
         path1.addRect(1, 1, 3, 3, SkPath::kCW_Direction);
         path1.addRect(2, 2, 4, 4, SkPath::kCW_Direction);
-        REPORTER_ASSERT(reporter, !path1.isNestedRects(NULL));
+        REPORTER_ASSERT(reporter, !path1.isNestedFillRects(NULL));
     }
 
+    //  pass, constructed explicitly from manually closed rects specified as moves/lines.
+    SkPath path;
+    path.moveTo(0, 0);
+    path.lineTo(10, 0);
+    path.lineTo(10, 10);
+    path.lineTo(0, 10);
+    path.lineTo(0, 0);
+    path.moveTo(1, 1);
+    path.lineTo(9, 1);
+    path.lineTo(9, 9);
+    path.lineTo(1, 9);
+    path.lineTo(1, 1);
+    REPORTER_ASSERT(reporter, path.isNestedFillRects(NULL));
+
     // pass, stroke rect
     SkPath src, dst;
     src.addRect(1, 1, 7, 7, SkPath::kCW_Direction);
@@ -2062,7 +2077,7 @@ static void test_isNestedRects(skiatest::Reporter* reporter) {
     strokePaint.setStyle(SkPaint::kStroke_Style);
     strokePaint.setStrokeWidth(2);
     strokePaint.getFillPath(src, &dst);
-    REPORTER_ASSERT(reporter, dst.isNestedRects(NULL));
+    REPORTER_ASSERT(reporter, dst.isNestedFillRects(NULL));
 }
 
 static void write_and_read_back(skiatest::Reporter* reporter,
@@ -3719,7 +3734,7 @@ DEF_TEST(Paths, reporter) {
     test_operatorEqual(reporter);
     test_isLine(reporter);
     test_isRect(reporter);
-    test_isNestedRects(reporter);
+    test_isNestedFillRects(reporter);
     test_zero_length_paths(reporter);
     test_direction(reporter);
     test_convexity(reporter);
diff --git a/tests/StrokeTest.cpp b/tests/StrokeTest.cpp
index 80ca317302..b8abbd3a8a 100644
--- a/tests/StrokeTest.cpp
+++ b/tests/StrokeTest.cpp
@@ -74,7 +74,7 @@ static void test_strokerect(skiatest::Reporter* reporter) {
 
         bool isMiter = SkPaint::kMiter_Join == joins[i];
         SkRect nested[2];
-        REPORTER_ASSERT(reporter, fillPath.isNestedRects(nested) == isMiter);
+        REPORTER_ASSERT(reporter, fillPath.isNestedFillRects(nested) == isMiter);
         if (isMiter) {
             SkRect inner(r);
             inner.inset(width/2, width/2);
diff --git a/tools/lua/scrape.lua b/tools/lua/scrape.lua
index 627018800a..4f4adbf3db 100644
--- a/tools/lua/scrape.lua
+++ b/tools/lua/scrape.lua
@@ -59,7 +59,7 @@ function sk_scrape_accumulate(t)
     end
 
     if false and t.verb == "drawPath" then
-        local pred, r1, r2, d1, d2 = t.path:isNestedRects()
+        local pred, r1, r2, d1, d2 = t.path:isNestedFillRects()
         
         if pred then
             print("drawRect_Nested", tostr(r1), tostr(r2), d1, d2)