Add support for transformedLocalCoords to GrDefaultGeoProc

This CL wires up the backend portion necessary for sending transformed coords via vertex attributes.

BUG=skia:

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

[joshualitt, 2015-07-20 17:50:13]

joshualitt@google.com changed reviewers:
+ bsalomon@google.com, joshualitt@google.com

[joshualitt, 2015-07-20 17:50:13]

ptal

[joshualitt, 2015-08-05 14:59:41]

On 2015/07/20 17:50:13, joshualitt wrote:
> ptal

Okay, this is updated, please take a look.

[bsalomon, 2015-08-05 15:17:02]

https://codereview.chromium.org/1243583002/diff/100001/src/gpu/GrDefaultGeoPr...
File src/gpu/GrDefaultGeoProcFactory.cpp (right):

https://codereview.chromium.org/1243583002/diff/100001/src/gpu/GrDefaultGeoPr...
src/gpu/GrDefaultGeoProcFactory.cpp:20: enum GPType {
This looks like a set of flags for a bitfield rather than distinct types

https://codereview.chromium.org/1243583002/diff/100001/src/gpu/GrDefaultGeoPr...
File src/gpu/GrDefaultGeoProcFactory.h (right):

https://codereview.chromium.org/1243583002/diff/100001/src/gpu/GrDefaultGeoPr...
src/gpu/GrDefaultGeoProcFactory.h:108: kHasTransformed_Type,
don't we need a count?

https://codereview.chromium.org/1243583002/diff/100001/src/gpu/GrGeometryProc...
File src/gpu/GrGeometryProcessor.h (right):

https://codereview.chromium.org/1243583002/diff/100001/src/gpu/GrGeometryProc...
src/gpu/GrGeometryProcessor.h:49: bool hasTransformedLocalCoords() const
override { return fHasTransformedLocalCoords; }
should we be using enums rather than a set of bools?

https://codereview.chromium.org/1243583002/diff/100001/src/gpu/gl/GrGLGeometr...
File src/gpu/gl/GrGLGeometryProcessor.h (right):

https://codereview.chromium.org/1243583002/diff/100001/src/gpu/gl/GrGLGeometr...
src/gpu/gl/GrGLGeometryProcessor.h:53: // emit transforms via uniform
// Emit a uniform matrix for each coord transform.
?

https://codereview.chromium.org/1243583002/diff/100001/src/gpu/gl/GrGLGeometr...
src/gpu/gl/GrGLGeometryProcessor.h:61: // caller has emitted transforms via
attributes
// Emit pre-transformed coords as a vertex attribute per coord-transform.
?

[joshualitt, 2015-08-05 15:41:09]

On 2015/08/05 15:17:02, bsalomon wrote:
>
https://codereview.chromium.org/1243583002/diff/100001/src/gpu/GrDefaultGeoPr...
> File src/gpu/GrDefaultGeoProcFactory.cpp (right):
> 
>
https://codereview.chromium.org/1243583002/diff/100001/src/gpu/GrDefaultGeoPr...
> src/gpu/GrDefaultGeoProcFactory.cpp:20: enum GPType {
> This looks like a set of flags for a bitfield rather than distinct types
> 
>
https://codereview.chromium.org/1243583002/diff/100001/src/gpu/GrDefaultGeoPr...
> File src/gpu/GrDefaultGeoProcFactory.h (right):
> 
>
https://codereview.chromium.org/1243583002/diff/100001/src/gpu/GrDefaultGeoPr...
> src/gpu/GrDefaultGeoProcFactory.h:108: kHasTransformed_Type,
> don't we need a count?
> 
>
https://codereview.chromium.org/1243583002/diff/100001/src/gpu/GrGeometryProc...
> File src/gpu/GrGeometryProcessor.h (right):
> 
>
https://codereview.chromium.org/1243583002/diff/100001/src/gpu/GrGeometryProc...
> src/gpu/GrGeometryProcessor.h:49: bool hasTransformedLocalCoords() const
> override { return fHasTransformedLocalCoords; }
> should we be using enums rather than a set of bools?
> 
>
https://codereview.chromium.org/1243583002/diff/100001/src/gpu/gl/GrGLGeometr...
> File src/gpu/gl/GrGLGeometryProcessor.h (right):
> 
>
https://codereview.chromium.org/1243583002/diff/100001/src/gpu/gl/GrGLGeometr...
> src/gpu/gl/GrGLGeometryProcessor.h:53: // emit transforms via uniform
> // Emit a uniform matrix for each coord transform.
> ?
> 
>
https://codereview.chromium.org/1243583002/diff/100001/src/gpu/gl/GrGLGeometr...
> src/gpu/gl/GrGLGeometryProcessor.h:61: // caller has emitted transforms via
> attributes
> // Emit pre-transformed coords as a vertex attribute per coord-transform.
> ?

feedback inc

[bsalomon, 2015-08-05 16:20:20]

https://codereview.chromium.org/1243583002/diff/120001/src/gpu/GrDefaultGeoPr...
File src/gpu/GrDefaultGeoProcFactory.cpp (right):

https://codereview.chromium.org/1243583002/diff/120001/src/gpu/GrDefaultGeoPr...
src/gpu/GrDefaultGeoProcFactory.cpp:21: kPosition_GPFlag = 0x0, // we ALWAYS
have position
Might be nice to line up the = 0x's

https://codereview.chromium.org/1243583002/diff/120001/src/gpu/GrDefaultGeoPr...
src/gpu/GrDefaultGeoProcFactory.cpp:26: kLastGPFlag =
kTransformedLocalCoord_GPFlag
Is this used?

https://codereview.chromium.org/1243583002/diff/120001/src/gpu/GrGeometryProc...
File src/gpu/GrGeometryProcessor.h (right):

https://codereview.chromium.org/1243583002/diff/120001/src/gpu/GrGeometryProc...
src/gpu/GrGeometryProcessor.h:48: bool hasTransformedLocalCoords() const
override {
enum getter here rather than two queries?

https://codereview.chromium.org/1243583002/diff/120001/src/gpu/GrGeometryProc...
src/gpu/GrGeometryProcessor.h:84: kIgnoredOrPosition_LocalCoordsType,
Are there really ignored local local coords? Unused or not required? Maybe
separate values?

[joshualitt, 2015-08-05 16:32:30]

https://codereview.chromium.org/1243583002/diff/120001/src/gpu/GrDefaultGeoPr...
File src/gpu/GrDefaultGeoProcFactory.cpp (right):

https://codereview.chromium.org/1243583002/diff/120001/src/gpu/GrDefaultGeoPr...
src/gpu/GrDefaultGeoProcFactory.cpp:21: kPosition_GPFlag = 0x0, // we ALWAYS
have position
On 2015/08/05 16:20:19, bsalomon wrote:
> Might be nice to line up the = 0x's

Acknowledged.

https://codereview.chromium.org/1243583002/diff/120001/src/gpu/GrDefaultGeoPr...
src/gpu/GrDefaultGeoProcFactory.cpp:26: kLastGPFlag =
kTransformedLocalCoord_GPFlag
On 2015/08/05 16:20:19, bsalomon wrote:
> Is this used?

Acknowledged.

https://codereview.chromium.org/1243583002/diff/120001/src/gpu/GrGeometryProc...
File src/gpu/GrGeometryProcessor.h (right):

https://codereview.chromium.org/1243583002/diff/120001/src/gpu/GrGeometryProc...
src/gpu/GrGeometryProcessor.h:48: bool hasTransformedLocalCoords() const
override {
On 2015/08/05 16:20:19, bsalomon wrote:
> enum getter here rather than two queries?

I'd rather just leave the queries.  All of this will go away when I circle back
to clean up program builder anyways, and I'd rather keep it as contained as
possible.  Unless you have a strong preference.

https://codereview.chromium.org/1243583002/diff/120001/src/gpu/GrGeometryProc...
src/gpu/GrGeometryProcessor.h:84: kIgnoredOrPosition_LocalCoordsType,
On 2015/08/05 16:20:19, bsalomon wrote:
> Are there really ignored local local coords? Unused or not required? Maybe
> separate values?

Acknowledged.

[bsalomon, 2015-08-05 17:13:56]

lgtm

https://codereview.chromium.org/1243583002/diff/140001/src/gpu/GrDefaultGeoPr...
File src/gpu/GrDefaultGeoProcFactory.cpp (right):

https://codereview.chromium.org/1243583002/diff/140001/src/gpu/GrDefaultGeoPr...
src/gpu/GrDefaultGeoProcFactory.cpp:21: kPosition_GPFlag =              0x0, //
we ALWAYS have position
kill it?

[joshualitt, 2015-08-05 17:34:25]

The CQ bit was checked by joshualitt@google.com

[joshualitt, 2015-08-05 17:34:25]

The patchset sent to the CQ was uploaded after l-g-t-m from bsalomon@google.com
Link to the patchset: https://codereview.chromium.org/1243583002/#ps160001
(title: "tweak")

[commit-bot: I haz the power, 2015-08-05 17:34:42]

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

[commit-bot: I haz the power, 2015-08-05 17:41:34]

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

[commit-bot: I haz the power, 2015-08-05 17:41:35]

Try jobs failed on following builders:
  Build-Mac10.8-Clang-x86_64-Release-Trybot on client.skia.compile (JOB_FAILED,
http://build.chromium.org/p/client.skia.compile/builders/Build-Mac10.8-Clang-...)

[joshualitt, 2015-08-05 17:50:59]

The CQ bit was checked by joshualitt@google.com

[joshualitt, 2015-08-05 17:50:59]

The patchset sent to the CQ was uploaded after l-g-t-m from bsalomon@google.com
Link to the patchset: https://codereview.chromium.org/1243583002/#ps180001
(title: "tweaks")

[commit-bot: I haz the power, 2015-08-05 17:51:11]

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

[commit-bot: I haz the power, 2015-08-05 18:05:25]

Committed patchset #10 (id:180001) as
https://skia.googlesource.com/skia/+/b2aa7cb90f4c66125056bac70dbca2093dd5ba41