From fe7c427e3d1c2c98bce7a3fa0ae6b5864527f169 Mon Sep 17 00:00:00 2001
From: robertphillips <robertphillips@google.com>
Date: Mon, 29 Dec 2014 11:36:39 -0800
Subject: [PATCH] Fix bound returned by SkPath::isRect when the path contains a
 trailing moveTo

Oddly enough this was fixed in:

https://codereview.chromium.org/16950021/ (add rect-output parameter to isRect, allowing us to return the correct bounds even if a rectagular path has a trailing moveTo)

but was reverted here:

https://skia.googlesource.com/skia/+/8fd160350ca5f57fbb1b2e03383c5778414a9b48

since it appeared to be crashing Chrome's trybots. I think it just fell through the cracks after that.

If this sticks I will land a follow on patch for the stroke issue reported in the original bug (crbug.com/247770).

BUG=247770,445368

Review URL: https://codereview.chromium.org/834483002
---
 src/core/SkPath.cpp | 17 +++++++++++++----
 tests/PathTest.cpp  | 14 ++++++++++++++
 2 files changed, 27 insertions(+), 4 deletions(-)

diff --git a/src/core/SkPath.cpp b/src/core/SkPath.cpp
index cee93d352e..f8aa8521da 100644
--- a/src/core/SkPath.cpp
+++ b/src/core/SkPath.cpp
@@ -538,11 +538,20 @@ bool SkPath::isRect(SkRect* rect) const {
     SkDEBUGCODE(this->validate();)
     int currVerb = 0;
     const SkPoint* pts = fPathRef->points();
-    bool result = isRectContour(false, &currVerb, &pts, NULL, NULL);
-    if (result && rect) {
-        *rect = getBounds();
+    const SkPoint* first = pts;
+    bool isClosed;
+    if (!this->isRectContour(false, &currVerb, &pts, &isClosed, NULL)) {
+        return false;
     }
-    return result;
+    if (rect) {
+        if (isClosed) {
+            rect->set(first, SkToS32(pts - first));
+        } else {
+            // 'pts' isn't updated for open rects
+            *rect = this->getBounds();
+        }
+    }
+    return true;
 }
 
 bool SkPath::isRect(bool* isClosed, Direction* direction) const {
diff --git a/tests/PathTest.cpp b/tests/PathTest.cpp
index 2ec54d79fa..960cdc3b16 100644
--- a/tests/PathTest.cpp
+++ b/tests/PathTest.cpp
@@ -3712,6 +3712,20 @@ DEF_TEST(Paths, reporter) {
     p.addRect(bounds);
     REPORTER_ASSERT(reporter, !p.isRect(NULL));
 
+    // Test an edge case w.r.t. the bound returned by isRect (i.e., the
+    // path has a trailing moveTo. Please see crbug.com\445368)
+    {
+        SkRect r;
+        p.reset();
+        p.addRect(bounds);
+        REPORTER_ASSERT(reporter, p.isRect(&r));
+        REPORTER_ASSERT(reporter, r == bounds);
+        // add a moveTo outside of our bounds
+        p.moveTo(bounds.fLeft + 10, bounds.fBottom + 10);
+        REPORTER_ASSERT(reporter, p.isRect(&r));
+        REPORTER_ASSERT(reporter, r == bounds);
+    }
+
     test_operatorEqual(reporter);
     test_isLine(reporter);
     test_isRect(reporter);
-- 
2.34.1