Fix key computation for GrPaths

Fix key computation for GrPaths

Improve tests to ensure paths are receiving valid keys
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2342873002

Committed: https://skia.googlesource.com/skia/+/7bffcd2673015f04fd3e20785746be38dd81b566

[bsalomon, 2016-09-15 14:43:21]

Description was changed from

==========
Fix key computation for GrPaths

Improve tests to ensure paths are receiving valid keys
==========

to

==========
Fix key computation for GrPaths

Improve tests to ensure paths are receiving valid keys
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2342873002
==========

[bsalomon, 2016-09-15 14:53:49]

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

[bsalomon, 2016-09-15 14:53:50]

This still has some issue I need to debug, but I think we ought to be using
GrShape's key mechanism for GrPath. The paths returned by GrShape.asPath() are
often volatile by design and shouldn't be used for keys.

https://codereview.chromium.org/2342873002/diff/20001/src/gpu/GrPath.cpp
File src/gpu/GrPath.cpp (left):

https://codereview.chromium.org/2342873002/diff/20001/src/gpu/GrPath.cpp#oldc...
src/gpu/GrPath.cpp:30: inline static bool compute_key_for_line_path(const
SkPath& path, const GrStyle& style,
GrShape already computes geometric keys for simple line, rect, etc paths.

[bsalomon, 2016-09-15 15:21:35]

This is now working. The new tests pass and all GMs draw the same as before in
with the nvprdit4 config.

[csmartdalton, 2016-09-15 17:21:27]

lgtm with a thought for an additional cleanup opportunity this patch gives us

https://codereview.chromium.org/2342873002/diff/100001/src/gpu/GrPath.cpp
File src/gpu/GrPath.cpp (right):

https://codereview.chromium.org/2342873002/diff/100001/src/gpu/GrPath.cpp#new...
src/gpu/GrPath.cpp:135: if (styleCnt < 0 || geoCnt < 0) {
Is this an indirect way of determining whether a GrShape is volatile?

https://codereview.chromium.org/2342873002/diff/100001/src/gpu/batches/GrSten...
File src/gpu/batches/GrStencilAndCoverPathRenderer.cpp (right):

https://codereview.chromium.org/2342873002/diff/100001/src/gpu/batches/GrSten...
src/gpu/batches/GrStencilAndCoverPathRenderer.cpp:100: SkAutoTUnref<GrPath>
p(get_gr_path(fResourceProvider, *args.fShape));
Nice.

I think we can drop the above "SkPath" and "asPath()" completely if below we use
p->inverseFilled() instead.

Then "p" could cleaned up to just be "path".

[bsalomon, 2016-09-15 17:41:35]

https://codereview.chromium.org/2342873002/diff/100001/src/gpu/GrPath.cpp
File src/gpu/GrPath.cpp (right):

https://codereview.chromium.org/2342873002/diff/100001/src/gpu/GrPath.cpp#new...
src/gpu/GrPath.cpp:135: if (styleCnt < 0 || geoCnt < 0) {
On 2016/09/15 17:21:27, csmartdalton wrote:
> Is this an indirect way of determining whether a GrShape is volatile?

This basically is doing that. If the GrShape was made from a volatile path then
GrShape::unstyledKeySize() will return -1 unless the path was recognized as a
simpler shape (e.g. a rrect).

https://codereview.chromium.org/2342873002/diff/100001/src/gpu/batches/GrSten...
File src/gpu/batches/GrStencilAndCoverPathRenderer.cpp (right):

https://codereview.chromium.org/2342873002/diff/100001/src/gpu/batches/GrSten...
src/gpu/batches/GrStencilAndCoverPathRenderer.cpp:100: SkAutoTUnref<GrPath>
p(get_gr_path(fResourceProvider, *args.fShape));
On 2016/09/15 17:21:27, csmartdalton wrote:
> Nice.
> 
> I think we can drop the above "SkPath" and "asPath()" completely if below we
use
> p->inverseFilled() instead.
> 
> Then "p" could cleaned up to just be "path".

I think you're right, will try in another CL.

[bsalomon, 2016-09-15 17:42:26]

The CQ bit was checked by bsalomon@google.com

[commit-bot: I haz the power, 2016-09-15 17:42:34]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-15 18:07:02]

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

[commit-bot: I haz the power, 2016-09-15 18:07:04]

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

[bsalomon, 2016-09-15 18:19:33]

The CQ bit was checked by bsalomon@google.com

[bsalomon, 2016-09-15 18:19:33]

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

[commit-bot: I haz the power, 2016-09-15 18:19:46]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-15 18:59:41]

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

[commit-bot: I haz the power, 2016-09-15 18:59:43]

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

[bsalomon, 2016-09-15 19:34:20]

The CQ bit was checked by bsalomon@google.com

[bsalomon, 2016-09-15 19:34:21]

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

[commit-bot: I haz the power, 2016-09-15 19:34:27]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-15 20:55:35]

Description was changed from

==========
Fix key computation for GrPaths

Improve tests to ensure paths are receiving valid keys
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2342873002
==========

to

==========
Fix key computation for GrPaths

Improve tests to ensure paths are receiving valid keys
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2342873002

Committed:
https://skia.googlesource.com/skia/+/7bffcd2673015f04fd3e20785746be38dd81b566
==========

[commit-bot: I haz the power, 2016-09-15 20:55:36]

Committed patchset #8 (id:140001) as
https://skia.googlesource.com/skia/+/7bffcd2673015f04fd3e20785746be38dd81b566