Add SkPathPriv::IsSimpleClosedRect

Add SkPathPriv::IsSimpleClosedRect

This function looks for "simple" rect paths. Simple here means begins and ends at a corner and is closed (either manually or with a close verb). Unlike SkPath::isRect this returns the starting point index (using the same start indexing scheme as SkPath::addRect).

GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2017313002

Committed: https://skia.googlesource.com/skia/+/edc743a57657107b873ed2fc2efeeff1b1efcd23

[bsalomon, 2016-05-28 12:51:55]

Description was changed from

==========
start on getSimpleClosedRect
==========

to

==========
start on getSimpleClosedRect
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2017313002
==========

[bsalomon, 2016-05-31 19:38:50]

Description was changed from

==========
start on getSimpleClosedRect
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2017313002
==========

to

==========
Add SkPathPriv::IsSimpleClosedRect

This function looks for "simple" rect paths. Simple here means begins and ends
at a corner and is closed (either manually or with a close verb). Unlike
SkPath::isRect this returns the starting point index (using the same start
indexing scheme as SkPath::addRect).

GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2017313002
==========

[bsalomon, 2016-05-31 19:40:47]

bsalomon@google.com changed reviewers:
+ robertphillips@google.com

[bsalomon, 2016-05-31 19:40:47]

I started going down the route of modifying SkPath::isRect. However, that
function is pretty complicated and the start index is somewhat non-orthogonal to
the other params. I thought it simpler to add a new checker that has a more
limited sense of what a rect path is.

[robertphillips, 2016-05-31 21:48:36]

lgtm

https://codereview.chromium.org/2017313002/diff/40001/src/core/SkPath.cpp
File src/core/SkPath.cpp (right):

https://codereview.chromium.org/2017313002/diff/40001/src/core/SkPath.cpp#new...
src/core/SkPath.cpp:3250: unsigned* start) {
Would it be worth having a quick out for is there were any quads, cubics or
conics in the segment mask?

https://codereview.chromium.org/2017313002/diff/40001/src/core/SkPath.cpp#new...
src/core/SkPath.cpp:3281: }
Don't we just want to say "if (rectPtCnt < 5) { return false; }" ? If the path
just had a moveTo and a lineTo wouldn't we be accessing uninitialized memory?

https://codereview.chromium.org/2017313002/diff/40001/tests/PathTest.cpp
File tests/PathTest.cpp (right):

https://codereview.chromium.org/2017313002/diff/40001/tests/PathTest.cpp#newc...
tests/PathTest.cpp:2037: check_simple_closed_rect(reporter, path2, testRect,
dir, start);
An -> A ?

[bsalomon, 2016-06-01 15:57:24]

The CQ bit was checked by bsalomon@google.com

[bsalomon, 2016-06-01 15:57:25]

The patchset sent to the CQ was uploaded after l-g-t-m from
robertphillips@google.com
Link to the patchset: https://codereview.chromium.org/2017313002/#ps60001
(title: "Address comments")

[commit-bot: I haz the power, 2016-06-01 15:57:33]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2017313002/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/2017313002/60001

[commit-bot: I haz the power, 2016-06-01 16:01:28]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-06-01 16:01:29]

Try jobs failed on following builders:
  Test-Ubuntu-GCC-ShuttleA-GPU-GTX660-x86_64-Release-Trybot on client.skia
(JOB_FAILED,
http://build.chromium.org/p/client.skia/builders/Test-Ubuntu-GCC-ShuttleA-GPU...)

[bsalomon, 2016-06-01 16:24:40]

The CQ bit was checked by bsalomon@google.com

[bsalomon, 2016-06-01 16:24:40]

The patchset sent to the CQ was uploaded after l-g-t-m from
robertphillips@google.com
Link to the patchset: https://codereview.chromium.org/2017313002/#ps80001
(title: "fix gcc warning")

[commit-bot: I haz the power, 2016-06-01 16:24:48]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2017313002/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/2017313002/80001

[commit-bot: I haz the power, 2016-06-01 16:42:33]

Description was changed from

==========
Add SkPathPriv::IsSimpleClosedRect

This function looks for "simple" rect paths. Simple here means begins and ends
at a corner and is closed (either manually or with a close verb). Unlike
SkPath::isRect this returns the starting point index (using the same start
indexing scheme as SkPath::addRect).

GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2017313002
==========

to

==========
Add SkPathPriv::IsSimpleClosedRect

This function looks for "simple" rect paths. Simple here means begins and ends
at a corner and is closed (either manually or with a close verb). Unlike
SkPath::isRect this returns the starting point index (using the same start
indexing scheme as SkPath::addRect).

GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2017313002

Committed:
https://skia.googlesource.com/skia/+/edc743a57657107b873ed2fc2efeeff1b1efcd23
==========

[commit-bot: I haz the power, 2016-06-01 16:42:34]

Committed patchset #5 (id:80001) as
https://skia.googlesource.com/skia/+/edc743a57657107b873ed2fc2efeeff1b1efcd23

[bsalomon, 2016-06-03 14:49:06]

https://codereview.chromium.org/2017313002/diff/40001/src/core/SkPath.cpp
File src/core/SkPath.cpp (right):

https://codereview.chromium.org/2017313002/diff/40001/src/core/SkPath.cpp#new...
src/core/SkPath.cpp:3250: unsigned* start) {
On 2016/05/31 21:48:36, robertphillips wrote:
> Would it be worth having a quick out for is there were any quads, cubics or
> conics in the segment mask?

Done.

https://codereview.chromium.org/2017313002/diff/40001/src/core/SkPath.cpp#new...
src/core/SkPath.cpp:3281: }
On 2016/05/31 21:48:36, robertphillips wrote:
> Don't we just want to say "if (rectPtCnt < 5) { return false; }" ? If the path
> just had a moveTo and a lineTo wouldn't we be accessing uninitialized memory?

Done, nice catch.

https://codereview.chromium.org/2017313002/diff/40001/tests/PathTest.cpp
File tests/PathTest.cpp (right):

https://codereview.chromium.org/2017313002/diff/40001/tests/PathTest.cpp#newc...
tests/PathTest.cpp:2037: check_simple_closed_rect(reporter, path2, testRect,
dir, start);
On 2016/05/31 21:48:36, robertphillips wrote:
> An -> A ?

Done.