Make GrShape track the winding direction and starting point for rrect types.

Make GrShape track the winding direction and starting point for rrect types.
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2042813002

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

[bsalomon, 2016-06-06 14:32:42]

Description was changed from

==========
Make GrShape track the winding direction and starting point for rrect types.
==========

to

==========
Make GrShape track the winding direction and starting point for rrect types.
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2042813002
==========

[bsalomon, 2016-06-06 15:02:59]

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

[egdaniel, 2016-06-06 15:50:30]

https://codereview.chromium.org/2042813002/diff/20001/src/gpu/GrShape.h
File src/gpu/GrShape.h (right):

https://codereview.chromium.org/2042813002/diff/20001/src/gpu/GrShape.h#newco...
src/gpu/GrShape.h:266: *dir = SkPath::kCW_Direction;
This line is repeated above

https://codereview.chromium.org/2042813002/diff/20001/src/gpu/GrShape.h#newco...
src/gpu/GrShape.h:273: *dir = SkPath::kCW_Direction;
CCW?

https://codereview.chromium.org/2042813002/diff/20001/src/gpu/GrShape.h#newco...
src/gpu/GrShape.h:277: *dir = SkPath::kCW_Direction;
CCW?

[bsalomon, 2016-06-06 16:28:05]

I made the default starting index always be 0 when there is no path effect.

https://codereview.chromium.org/2042813002/diff/20001/src/gpu/GrShape.h
File src/gpu/GrShape.h (right):

https://codereview.chromium.org/2042813002/diff/20001/src/gpu/GrShape.h#newco...
src/gpu/GrShape.h:266: *dir = SkPath::kCW_Direction;
On 2016/06/06 15:50:29, egdaniel wrote:
> This line is repeated above

Done.

https://codereview.chromium.org/2042813002/diff/20001/src/gpu/GrShape.h#newco...
src/gpu/GrShape.h:273: *dir = SkPath::kCW_Direction;
On 2016/06/06 15:50:29, egdaniel wrote:
> CCW?

Done.

https://codereview.chromium.org/2042813002/diff/20001/src/gpu/GrShape.h#newco...
src/gpu/GrShape.h:277: *dir = SkPath::kCW_Direction;
On 2016/06/06 15:50:29, egdaniel wrote:
> CCW?

Done.

[egdaniel, 2016-06-06 18:34:37]

https://codereview.chromium.org/2042813002/diff/40001/src/gpu/GrShape.cpp
File src/gpu/GrShape.cpp (right):

https://codereview.chromium.org/2042813002/diff/40001/src/gpu/GrShape.cpp#new...
src/gpu/GrShape.cpp:102: *key++ |= fRRectStart;
So obviously fRRectStart will be less than 2^31, but for extra safety is it
worth asserting that here?

https://codereview.chromium.org/2042813002/diff/40001/tests/GrShapeTest.cpp
File tests/GrShapeTest.cpp (right):

https://codereview.chromium.org/2042813002/diff/40001/tests/GrShapeTest.cpp#n...
tests/GrShapeTest.cpp:257: unsigned startA = ~0U, startB = ~0U;
comment that this initialization is for compiler

https://codereview.chromium.org/2042813002/diff/40001/tests/GrShapeTest.cpp#n...
tests/GrShapeTest.cpp:267: REPORTER_ASSERT(r, aIsRRect && bIsRRect);
is this check redundent with allowSameRRectBDSAD?

[bsalomon, 2016-06-06 19:41:29]

https://codereview.chromium.org/2042813002/diff/40001/src/gpu/GrShape.cpp
File src/gpu/GrShape.cpp (right):

https://codereview.chromium.org/2042813002/diff/40001/src/gpu/GrShape.cpp#new...
src/gpu/GrShape.cpp:102: *key++ |= fRRectStart;
On 2016/06/06 18:34:36, egdaniel wrote:
> So obviously fRRectStart will be less than 2^31, but for extra safety is it
> worth asserting that here?

Added SkASSERT(fRRectStart < 8)

https://codereview.chromium.org/2042813002/diff/40001/tests/GrShapeTest.cpp
File tests/GrShapeTest.cpp (right):

https://codereview.chromium.org/2042813002/diff/40001/tests/GrShapeTest.cpp#n...
tests/GrShapeTest.cpp:257: unsigned startA = ~0U, startB = ~0U;
On 2016/06/06 18:34:36, egdaniel wrote:
> comment that this initialization is for compiler

Done.

https://codereview.chromium.org/2042813002/diff/40001/tests/GrShapeTest.cpp#n...
tests/GrShapeTest.cpp:267: REPORTER_ASSERT(r, aIsRRect && bIsRRect);
On 2016/06/06 18:34:36, egdaniel wrote:
> is this check redundent with allowSameRRectBDSAD?

Done.

[egdaniel, 2016-06-06 19:47:13]

lgtm + nits

https://codereview.chromium.org/2042813002/diff/60001/tests/GrShapeTest.cpp
File tests/GrShapeTest.cpp (right):

https://codereview.chromium.org/2042813002/diff/60001/tests/GrShapeTest.cpp#n...
tests/GrShapeTest.cpp:249: // GrShape only respects that input winding direction
and start point for rrect shapes
nit: that->the

https://codereview.chromium.org/2042813002/diff/60001/tests/GrShapeTest.cpp#n...
tests/GrShapeTest.cpp:253: // without the path effect. If *both* have path
effects then they should have both preserve
preserveD

[bsalomon, 2016-06-06 19:50:41]

https://codereview.chromium.org/2042813002/diff/60001/tests/GrShapeTest.cpp
File tests/GrShapeTest.cpp (right):

https://codereview.chromium.org/2042813002/diff/60001/tests/GrShapeTest.cpp#n...
tests/GrShapeTest.cpp:249: // GrShape only respects that input winding direction
and start point for rrect shapes
On 2016/06/06 19:47:13, egdaniel wrote:
> nit: that->the

Done.

https://codereview.chromium.org/2042813002/diff/60001/tests/GrShapeTest.cpp#n...
tests/GrShapeTest.cpp:253: // without the path effect. If *both* have path
effects then they should have both preserve
On 2016/06/06 19:47:13, egdaniel wrote:
> preserveD

Done.

[bsalomon, 2016-06-06 19:50:47]

The CQ bit was checked by bsalomon@google.com to run a CQ dry run

[commit-bot: I haz the power, 2016-06-06 19:50:53]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2042813002/80001

[commit-bot: I haz the power, 2016-06-06 19:56:43]

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

[commit-bot: I haz the power, 2016-06-06 19:56:44]

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

[bsalomon, 2016-06-06 20:04:14]

The CQ bit was checked by bsalomon@google.com

[bsalomon, 2016-06-06 20:04:15]

The patchset sent to the CQ was uploaded after l-g-t-m from egdaniel@google.com
Link to the patchset: https://codereview.chromium.org/2042813002/#ps100001
(title: "Fix MSVS warnings")

[commit-bot: I haz the power, 2016-06-06 20:04:22]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2042813002/100001

[commit-bot: I haz the power, 2016-06-06 20:12:12]

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

[commit-bot: I haz the power, 2016-06-06 20:12:13]

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-06-06 20:45:52]

The CQ bit was checked by bsalomon@google.com

[bsalomon, 2016-06-06 20:45:52]

The patchset sent to the CQ was uploaded after l-g-t-m from egdaniel@google.com
Link to the patchset: https://codereview.chromium.org/2042813002/#ps120001
(title: "Fix more MSVS warnings")

[commit-bot: I haz the power, 2016-06-06 20:46:04]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2042813002/120001

[commit-bot: I haz the power, 2016-06-06 21:01:30]

Description was changed from

==========
Make GrShape track the winding direction and starting point for rrect types.
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2042813002
==========

to

==========
Make GrShape track the winding direction and starting point for rrect types.
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2042813002

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

[commit-bot: I haz the power, 2016-06-06 21:01:32]

Committed patchset #7 (id:120001) as
https://skia.googlesource.com/skia/+/ee295645bd91fcbe1714847c5fe5341759037cc5