Make GrShape compute keys for short paths from path data instead of using the gen id.

Make GrShape compute keys for short paths from path data instead of using the gen id.
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2357643002

Committed: https://skia.googlesource.com/skia/+/67fa4e31d885acf74514527a2cc3ed759c05d3c4

[bsalomon, 2016-09-20 14:00:06]

Description was changed from

==========
Make GrShape compute keys for short paths from path data instead of using the
gen id.
==========

to

==========
Make GrShape compute keys for short paths from path data instead of using the
gen id.
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2357643002
==========

[bsalomon, 2016-09-20 17:13:21]

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

[robertphillips, 2016-09-21 13:37:47]

lgtm

https://codereview.chromium.org/2357643002/diff/20001/src/core/SkPathPriv.h
File src/core/SkPathPriv.h (right):

https://codereview.chromium.org/2357643002/diff/20001/src/core/SkPathPriv.h#n...
src/core/SkPathPriv.h:109: 
point -> pointer ?

https://codereview.chromium.org/2357643002/diff/20001/src/core/SkPathPriv.h#n...
src/core/SkPathPriv.h:119: 
same here ?

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

https://codereview.chromium.org/2357643002/diff/20001/src/gpu/GrShape.cpp#new...
src/gpu/GrShape.cpp:105: uint8_t* pad = reinterpret_cast<uint8_t*>(key)+
verbCnt;
Why not 0x0 ?

https://codereview.chromium.org/2357643002/diff/20001/src/gpu/GrShape.cpp#new...
src/gpu/GrShape.cpp:134: case Type::kPath: {
IIUC, you don't think we should give precedence to the genID for non-volatile
paths (i.e., should this go in the following if block) ?

[bsalomon, 2016-09-21 14:17:35]

https://codereview.chromium.org/2357643002/diff/20001/src/core/SkPathPriv.h
File src/core/SkPathPriv.h (right):

https://codereview.chromium.org/2357643002/diff/20001/src/core/SkPathPriv.h#n...
src/core/SkPathPriv.h:109: 
On 2016/09/21 13:37:46, robertphillips wrote:
> point -> pointer ?

Done.

https://codereview.chromium.org/2357643002/diff/20001/src/core/SkPathPriv.h#n...
src/core/SkPathPriv.h:119: 
On 2016/09/21 13:37:46, robertphillips wrote:
> same here ?

Done.

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

https://codereview.chromium.org/2357643002/diff/20001/src/gpu/GrShape.cpp#new...
src/gpu/GrShape.cpp:105: uint8_t* pad = reinterpret_cast<uint8_t*>(key)+
verbCnt;
On 2016/09/21 13:37:47, robertphillips wrote:
> Why not 0x0 ?

So when I dump the key contents in a debugger I know that its not actual verb
content. Perhaps I should pick something more distinctive. Added a comment.

https://codereview.chromium.org/2357643002/diff/20001/src/gpu/GrShape.cpp#new...
src/gpu/GrShape.cpp:134: case Type::kPath: {
On 2016/09/21 13:37:47, robertphillips wrote:
> IIUC, you don't think we should give precedence to the genID for non-volatile
> paths (i.e., should this go in the following if block) ?

For the use case I'm trying to address the paths are not marked volatile but are
respecified every frame.

[bsalomon, 2016-09-21 14:32:11]

The CQ bit was checked by bsalomon@google.com

[bsalomon, 2016-09-21 14:32:11]

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/2357643002/#ps40001
(title: "Address comments")

[commit-bot: I haz the power, 2016-09-21 14:32:20]

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

[commit-bot: I haz the power, 2016-09-21 14:32:21]

This CL has an open dependency (Issue 2355833002 Patch 20001). Please resolve
the dependency and try again.
If you are sure that there is no real dependency, please use one of the options
listed in https://goo.gl/9Es4OR to land the CL.

[robertphillips, 2016-09-21 14:43:10]

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

https://codereview.chromium.org/2357643002/diff/20001/src/gpu/GrShape.cpp#new...
src/gpu/GrShape.cpp:134: case Type::kPath: {
On 2016/09/21 14:17:35, bsalomon wrote:
> On 2016/09/21 13:37:47, robertphillips wrote:
> > IIUC, you don't think we should give precedence to the genID for
non-volatile
> > paths (i.e., should this go in the following if block) ?
> 
> For the use case I'm trying to address the paths are not marked volatile but
are
> respecified every frame.

:(

[bsalomon, 2016-09-21 14:47:50]

The CQ bit was checked by bsalomon@google.com

[commit-bot: I haz the power, 2016-09-21 14:48:01]

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-21 15:26:59]

Description was changed from

==========
Make GrShape compute keys for short paths from path data instead of using the
gen id.
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2357643002
==========

to

==========
Make GrShape compute keys for short paths from path data instead of using the
gen id.
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2357643002

Committed:
https://skia.googlesource.com/skia/+/67fa4e31d885acf74514527a2cc3ed759c05d3c4
==========

[commit-bot: I haz the power, 2016-09-21 15:27:00]

Committed patchset #3 (id:40001) as
https://skia.googlesource.com/skia/+/67fa4e31d885acf74514527a2cc3ed759c05d3c4