Make SkPath::isOval() and SkPath::isRRect return the orientation and starting index.

Make SkPath::isOval() and SkPath::isRRect return the orientation and starting index.

These are tracked in SkPathRef.

Unit tests are updated to test that the returned values are correct.
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2012233002

Committed: https://skia.googlesource.com/skia/+/78d58d1084f0390c1c0f9123ac6e48efcd226f39

[bsalomon, 2016-05-26 14:11:49]

Description was changed from

==========
Make SkPath::isOval() and SkPath::isRRect return the orientation and starting
index.

These are tracked in SkPathRef.

Unit tests are updated to test that the returned values are correct.
==========

to

==========
Make SkPath::isOval() and SkPath::isRRect return the orientation and starting
index.

These are tracked in SkPathRef.

Unit tests are updated to test that the returned values are correct.
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2012233002
==========

[bsalomon, 2016-05-26 14:17:36]

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

[robertphillips, 2016-05-26 15:57:58]

https://codereview.chromium.org/2012233002/diff/20001/include/core/SkPath.h
File include/core/SkPath.h (right):

https://codereview.chromium.org/2012233002/diff/20001/include/core/SkPath.h#n...
include/core/SkPath.h:175: *                  if the height and width are the
same.
isCCW -> dir ?

https://codereview.chromium.org/2012233002/diff/20001/include/core/SkPath.h#n...
include/core/SkPath.h:196: * @param rrect  Returns the bounding rect and radii
of this round rect.
isCCW -> dir ?

https://codereview.chromium.org/2012233002/diff/20001/include/core/SkPath.h#n...
include/core/SkPath.h:215: 
Not for this CL but, would it be worthwhile to push the dir vs. isCCW change
down to the pathRef ?

https://codereview.chromium.org/2012233002/diff/20001/include/core/SkPathRef.h
File include/core/SkPathRef.h (right):

https://codereview.chromium.org/2012233002/diff/20001/include/core/SkPathRef....
include/core/SkPathRef.h:337: // The next two values don't matter unless fIsOval
or fIsRRect are true.
0xAB -> false ?

https://codereview.chromium.org/2012233002/diff/20001/include/core/SkPathRef....
include/core/SkPathRef.h:537: SkBool8  fIsRRect;
// Both the circle and rrect special cases have a notion of direction & starting
point. The next two variables store that information for either.

? - not sure if this would actually be beneficial

https://codereview.chromium.org/2012233002/diff/20001/src/core/SkPathRef.cpp
File src/core/SkPathRef.cpp (right):

https://codereview.chromium.org/2012233002/diff/20001/src/core/SkPathRef.cpp#...
src/core/SkPathRef.cpp:64: // Degenerate rrect indices to oval indices and
remember the remainder.
// The oval indices mark the cardinal directions while the rrect indices
interleave the corners -- or something

?

https://codereview.chromium.org/2012233002/diff/20001/tests/PathTest.cpp
File tests/PathTest.cpp (right):

https://codereview.chromium.org/2012233002/diff/20001/tests/PathTest.cpp#newc...
tests/PathTest.cpp:2900: 
Don't we need an expectedStart too ?

https://codereview.chromium.org/2012233002/diff/20001/tests/PathTest.cpp#newc...
tests/PathTest.cpp:3055: // Try different starting points for the contour.
Do we not have a kNumCircleStarts ?

https://codereview.chromium.org/2012233002/diff/20001/tests/RRectInPathTest.cpp
File tests/RRectInPathTest.cpp (right):

https://codereview.chromium.org/2012233002/diff/20001/tests/RRectInPathTest.c...
tests/RRectInPathTest.cpp:200: for (auto dir : {SkPath::kCW_Direction,
SkPath::kCCW_Direction}) {
We don't have a kNumRRectStarts ?

[bsalomon, 2016-05-26 18:25:44]

https://codereview.chromium.org/2012233002/diff/20001/include/core/SkPath.h
File include/core/SkPath.h (right):

https://codereview.chromium.org/2012233002/diff/20001/include/core/SkPath.h#n...
include/core/SkPath.h:175: *                  if the height and width are the
same.
On 2016/05/26 15:57:58, robertphillips wrote:
> isCCW -> dir ?

Done.

https://codereview.chromium.org/2012233002/diff/20001/include/core/SkPath.h#n...
include/core/SkPath.h:196: * @param rrect  Returns the bounding rect and radii
of this round rect.
On 2016/05/26 15:57:58, robertphillips wrote:
> isCCW -> dir ?

Done.

https://codereview.chromium.org/2012233002/diff/20001/include/core/SkPath.h#n...
include/core/SkPath.h:215: 
On 2016/05/26 15:57:58, robertphillips wrote:
> Not for this CL but, would it be worthwhile to push the dir vs. isCCW change
> down to the pathRef ?

Right now SkPathRef can't see SkPath::Direction. I think we'd have to pull
Direction out of SkPath (which I'm all for).

https://codereview.chromium.org/2012233002/diff/20001/include/core/SkPathRef.h
File include/core/SkPathRef.h (right):

https://codereview.chromium.org/2012233002/diff/20001/include/core/SkPathRef....
include/core/SkPathRef.h:337: // The next two values don't matter unless fIsOval
or fIsRRect are true.
On 2016/05/26 15:57:58, robertphillips wrote:
> 0xAB -> false ?

Done.

https://codereview.chromium.org/2012233002/diff/20001/include/core/SkPathRef....
include/core/SkPathRef.h:537: SkBool8  fIsRRect;
On 2016/05/26 15:57:58, robertphillips wrote:
> // Both the circle and rrect special cases have a notion of direction &
starting
> point. The next two variables store that information for either.
> 
> ? - not sure if this would actually be beneficial

Done

https://codereview.chromium.org/2012233002/diff/20001/src/core/SkPathRef.cpp
File src/core/SkPathRef.cpp (right):

https://codereview.chromium.org/2012233002/diff/20001/src/core/SkPathRef.cpp#...
src/core/SkPathRef.cpp:64: // Degenerate rrect indices to oval indices and
remember the remainder.
On 2016/05/26 15:57:58, robertphillips wrote:
> // The oval indices mark the cardinal directions while the rrect indices
> interleave the corners -- or something
> 
> ?

Done (sort of)

https://codereview.chromium.org/2012233002/diff/20001/tests/PathTest.cpp
File tests/PathTest.cpp (right):

https://codereview.chromium.org/2012233002/diff/20001/tests/PathTest.cpp#newc...
tests/PathTest.cpp:2900: 
On 2016/05/26 15:57:58, robertphillips wrote:
> Don't we need an expectedStart too ?

I don't think so. Below we create a path with the extracted params and ensure
that all the points and verbs agree. If the returned start was wrong then it
would create a path with points in a different order than the original.

https://codereview.chromium.org/2012233002/diff/20001/tests/PathTest.cpp#newc...
tests/PathTest.cpp:3055: // Try different starting points for the contour.
On 2016/05/26 15:57:58, robertphillips wrote:
> Do we not have a kNumCircleStarts ?

We don't

https://codereview.chromium.org/2012233002/diff/20001/tests/RRectInPathTest.cpp
File tests/RRectInPathTest.cpp (right):

https://codereview.chromium.org/2012233002/diff/20001/tests/RRectInPathTest.c...
tests/RRectInPathTest.cpp:200: for (auto dir : {SkPath::kCW_Direction,
SkPath::kCCW_Direction}) {
On 2016/05/26 15:57:58, robertphillips wrote:
> We don't have a kNumRRectStarts ?

We don't

[robertphillips, 2016-05-26 18:31:00]

lgtm

[bsalomon, 2016-05-26 18:36:40]

The CQ bit was checked by bsalomon@google.com

[commit-bot: I haz the power, 2016-05-26 18:36:52]

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

[commit-bot: I haz the power, 2016-05-26 18:41:22]

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

[commit-bot: I haz the power, 2016-05-26 18:41:23]

Try jobs failed on following builders:
  Build-Win-MSVC-x86-Debug-Trybot on client.skia.compile (JOB_FAILED,
http://build.chromium.org/p/client.skia.compile/builders/Build-Win-MSVC-x86-D...)

[bsalomon, 2016-05-26 19:21:58]

The CQ bit was checked by bsalomon@google.com

[bsalomon, 2016-05-26 19:21:58]

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/2012233002/#ps60001
(title: "Fix compiler warnings")

[commit-bot: I haz the power, 2016-05-26 19:22:02]

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

[commit-bot: I haz the power, 2016-05-26 19:27:34]

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

[commit-bot: I haz the power, 2016-05-26 19:27:35]

Try jobs failed on following builders:
  Test-Ubuntu-GCC-GCE-CPU-AVX2-x86_64-Release-Shared-Trybot on client.skia
(JOB_FAILED,
http://build.chromium.org/p/client.skia/builders/Test-Ubuntu-GCC-GCE-CPU-AVX2...)

[bsalomon, 2016-05-26 20:23:30]

The CQ bit was checked by bsalomon@google.com

[bsalomon, 2016-05-26 20:23:31]

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/2012233002/#ps30008
(title: "more warning fixes")

[commit-bot: I haz the power, 2016-05-26 20:23:38]

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

[commit-bot: I haz the power, 2016-05-26 20:28:48]

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

[commit-bot: I haz the power, 2016-05-26 20:28:49]

Try jobs failed on following builders:
  Test-Ubuntu-GCC-GCE-CPU-AVX2-x86_64-Release-Shared-Trybot on client.skia
(JOB_FAILED,
http://build.chromium.org/p/client.skia/builders/Test-Ubuntu-GCC-GCE-CPU-AVX2...)
  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-05-26 20:42:28]

The CQ bit was checked by bsalomon@google.com

[bsalomon, 2016-05-26 20:42:28]

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/2012233002/#ps90001
(title: "another warning")

[commit-bot: I haz the power, 2016-05-26 20:42:37]

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

[commit-bot: I haz the power, 2016-05-26 20:48:46]

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

[commit-bot: I haz the power, 2016-05-26 20:48:48]

Try jobs failed on following builders:
  Test-Ubuntu-GCC-GCE-CPU-AVX2-x86_64-Debug-Trybot on client.skia (JOB_FAILED,
http://build.chromium.org/p/client.skia/builders/Test-Ubuntu-GCC-GCE-CPU-AVX2...)

[bsalomon, 2016-05-26 20:56:37]

The CQ bit was checked by bsalomon@google.com

[commit-bot: I haz the power, 2016-05-26 20:56:47]

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

[commit-bot: I haz the power, 2016-05-26 20:59:38]

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

[commit-bot: I haz the power, 2016-05-26 20:59:39]

Try jobs failed on following builders:
  Test-Ubuntu-GCC-GCE-CPU-AVX2-x86_64-Debug-Trybot on client.skia (JOB_FAILED,
http://build.chromium.org/p/client.skia/builders/Test-Ubuntu-GCC-GCE-CPU-AVX2...)

[bsalomon, 2016-05-27 15:58:42]

The CQ bit was checked by bsalomon@google.com

[bsalomon, 2016-05-27 15:58:42]

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/2012233002/#ps110001
(title: "record mod'ed start indices")

[commit-bot: I haz the power, 2016-05-27 15:58:46]

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

[commit-bot: I haz the power, 2016-05-27 16:17:07]

Description was changed from

==========
Make SkPath::isOval() and SkPath::isRRect return the orientation and starting
index.

These are tracked in SkPathRef.

Unit tests are updated to test that the returned values are correct.
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2012233002
==========

to

==========
Make SkPath::isOval() and SkPath::isRRect return the orientation and starting
index.

These are tracked in SkPathRef.

Unit tests are updated to test that the returned values are correct.
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2012233002

Committed:
https://skia.googlesource.com/skia/+/78d58d1084f0390c1c0f9123ac6e48efcd226f39
==========

[commit-bot: I haz the power, 2016-05-27 16:17:08]

Committed patchset #7 (id:110001) as
https://skia.googlesource.com/skia/+/78d58d1084f0390c1c0f9123ac6e48efcd226f39