Make GrPathRenderer only support fills for path stenciling

Make GrPathRenderer only support fills for path stenciling

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&issue=1950893002

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

[bsalomon, 2016-05-04 03:50:24]

Description was changed from

==========
Start on making GrPathRenderer only support fills for path stenciling


path renderer interface

BUG=skia:
==========

to

==========
Start on making GrPathRenderer only support fills for path stenciling


path renderer interface

BUG=skia:
GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...
==========

[bsalomon, 2016-05-04 16:21:59]

Description was changed from

==========
Start on making GrPathRenderer only support fills for path stenciling


path renderer interface

BUG=skia:
GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...
==========

to

==========
Make GrPathRenderer only support fills for path stenciling

BUG=skia:
GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...
==========

[bsalomon, 2016-05-04 16:26:30]

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

[bsalomon, 2016-05-04 16:26:30]

This is more prep for replacing GrStrokeInfo with GrStyle. The path renderer
implementations don't properly consider the GrStrokeInfo in all cases. In
practice path stenciling is only used for clips and clips are always simple
fills. This formalizes that and stops passing a GrStrokeInfo to
getStencilSupport.

I tried to do a deeper change to further separate path drawing from path
stenciling in the GrPathRenderer(Chain) interfaces but it got quite complicated
and GrShape will eventually force a larger refactor of this that would obviate
an intermediate improvment.

[bsalomon, 2016-05-04 16:26:41]

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

[commit-bot: I haz the power, 2016-05-04 16:26:43]

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

[commit-bot: I haz the power, 2016-05-04 16:37:51]

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

[commit-bot: I haz the power, 2016-05-04 16:37:52]

Dry run: This issue passed the CQ dry run.

[bsalomon, 2016-05-04 20:09:34]

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

[bsalomon, 2016-05-04 20:09:35]

+Rob

[egdaniel, 2016-05-04 20:30:12]

https://codereview.chromium.org/1950893002/diff/40001/src/gpu/GrPathRendererC...
File src/gpu/GrPathRendererChain.cpp (right):

https://codereview.chromium.org/1950893002/diff/40001/src/gpu/GrPathRendererC...
src/gpu/GrPathRendererChain.cpp:82: if (!args.fStroke->isFillStyle() ||
args.fStroke->isDashed()) {
Is this something we should never get to? Should we assert here?

[bsalomon, 2016-05-04 20:35:27]

https://codereview.chromium.org/1950893002/diff/40001/src/gpu/GrPathRendererC...
File src/gpu/GrPathRendererChain.cpp (right):

https://codereview.chromium.org/1950893002/diff/40001/src/gpu/GrPathRendererC...
src/gpu/GrPathRendererChain.cpp:82: if (!args.fStroke->isFillStyle() ||
args.fStroke->isDashed()) {
On 2016/05/04 20:30:12, egdaniel wrote:
> Is this something we should never get to? Should we assert here?

Our current use cases should never get here. I couldn't decide whether to check
or assert. I'm hoping this all goes away as GrShape gets deployed.

[egdaniel, 2016-05-04 20:37:28]

lgtm. I'll leave it up to your discretion on whether there should be an assert
or not.

[robertphillips, 2016-05-04 20:44:19]

https://codereview.chromium.org/1950893002/diff/40001/src/gpu/GrPathRenderer.h
File src/gpu/GrPathRenderer.h (left):

https://codereview.chromium.org/1950893002/diff/40001/src/gpu/GrPathRenderer....
src/gpu/GrPathRenderer.h:283: drawArgs.fPath = args.fPath;
Don't we still need to fill in the DrawPathArgs fStroke param?

[bsalomon, 2016-05-04 20:59:08]

https://codereview.chromium.org/1950893002/diff/40001/src/gpu/GrPathRenderer.h
File src/gpu/GrPathRenderer.h (left):

https://codereview.chromium.org/1950893002/diff/40001/src/gpu/GrPathRenderer....
src/gpu/GrPathRenderer.h:283: drawArgs.fPath = args.fPath;
On 2016/05/04 20:44:19, robertphillips wrote:
> Don't we still need to fill in the DrawPathArgs fStroke param?

Yes, you're right. This changed worked for me locally and sure enough we never
hit this code path on my system.

Set it to GrStrokeInfo::FillInfo()

[robertphillips, 2016-05-04 21:11:18]

lgtm

[bsalomon, 2016-05-05 14:36:11]

The CQ bit was checked by bsalomon@google.com

[bsalomon, 2016-05-05 14:36:11]

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/1950893002/#ps40002
(title: "Set draw params to fill in default impl of GrPR::onStencilPath")

[commit-bot: I haz the power, 2016-05-05 14:36:17]

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

[commit-bot: I haz the power, 2016-05-05 14:40:24]

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

[commit-bot: I haz the power, 2016-05-05 14:40:26]

Try jobs failed on following builders:
  Build-Ubuntu-GCC-Arm7-Debug-Android-Trybot on client.skia.compile (JOB_FAILED,
http://build.chromium.org/p/client.skia.compile/builders/Build-Ubuntu-GCC-Arm...)
  Build-Ubuntu-GCC-x86_64-Release-CMake-Trybot on client.skia.compile
(JOB_FAILED,
http://build.chromium.org/p/client.skia.compile/builders/Build-Ubuntu-GCC-x86...)

[bsalomon, 2016-05-05 14:42:58]

The CQ bit was checked by bsalomon@google.com

[commit-bot: I haz the power, 2016-05-05 14:43:05]

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

[commit-bot: I haz the power, 2016-05-05 14:45:34]

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

[commit-bot: I haz the power, 2016-05-05 14:45:35]

Try jobs failed on following builders:
  Build-Ubuntu-GCC-Arm7-Debug-Android-Trybot on client.skia.compile (JOB_FAILED,
http://build.chromium.org/p/client.skia.compile/builders/Build-Ubuntu-GCC-Arm...)

[bsalomon, 2016-05-05 16:17:58]

The CQ bit was checked by bsalomon@google.com

[commit-bot: I haz the power, 2016-05-05 16:18:05]

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

[commit-bot: I haz the power, 2016-05-05 16:26:24]

Description was changed from

==========
Make GrPathRenderer only support fills for path stenciling

BUG=skia:
GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...
==========

to

==========
Make GrPathRenderer only support fills for path stenciling

BUG=skia:
GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...

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

[commit-bot: I haz the power, 2016-05-05 16:26:25]

Committed patchset #4 (id:40002) as
https://skia.googlesource.com/skia/+/d6f25bf0ef353da39859fb4173d2cf60d52277e4