Adds local coords to GrEffect system.

Adds local coords to GrEffect system.

Effects can ask the builder for local coords which may or may not be distinct from positions.

GrEffectStage tracks changes to relationship between pos and local coords.

GrGLEffectMatrix and GrSingleTextureEffect can use either pos or textures as intput coords

GrSimpleTextureEffect now allows for an explicit texture coords attribute.
Committed: https://code.google.com/p/skia/source/detail?r=8264

[bsalomon, 2013-03-19 14:07:39]

Take two.

First off, I'm so so sorry at how big this is. A lot of it is basically search
and replace (esp. in src/effects). Very little of it can be done as a separate
CL with out breaking everything.

The basic idea is that instead of passing vertex coords down to each effect in
emitCode() effects query the builder for the coords they care about. There are
two built in set of coords local coords and positions which may or may not be
the same. Effects don't have to use either and can instead use a custom
attribute for their texture coords (e.g. text rendering from the atlas).

Reading these headers should help grok the rest: GrDrawState.h, GrDrawEffect.h,
GrEffect.h, GrGLEffect.h, and GrGLEffectMatrix.h.

[robertphillips, 2013-03-19 15:09:38]

Ouch but lgtm modulo some suggestions.

https://codereview.chromium.org/12531015/diff/13002/include/core/SkTLazy.h
File include/core/SkTLazy.h (right):

https://codereview.chromium.org/12531015/diff/13002/include/core/SkTLazy.h#ne...
include/core/SkTLazy.h:95: 
.. macro (SkNEW_IN_TLAZY) rather ...

https://codereview.chromium.org/12531015/diff/13002/src/effects/SkBlendImageF...
File src/effects/SkBlendImageFilter.cpp (right):

https://codereview.chromium.org/12531015/diff/13002/src/effects/SkBlendImageF...
src/effects/SkBlendImageFilter.cpp:97: GrGLBlendEffect(const
GrBackendEffectFactory& factory,
mv up a line?

https://codereview.chromium.org/12531015/diff/13002/src/gpu/GrContext.cpp
File src/gpu/GrContext.cpp (right):

https://codereview.chromium.org/12531015/diff/13002/src/gpu/GrContext.cpp#new...
src/gpu/GrContext.cpp:871: // localRect and localMatrix. This code path is
preventing some encapsulation in GrDrawState.
matrix -> savedMatrix, origMatrix?

https://codereview.chromium.org/12531015/diff/13002/src/gpu/gl/GrGLEffectMatr...
File src/gpu/gl/GrGLEffectMatrix.h (right):

https://codereview.chromium.org/12531015/diff/13002/src/gpu/gl/GrGLEffectMatr...
src/gpu/gl/GrGLEffectMatrix.h:103: 
kMatrixFlagsMask? I misunderstood kMatrixTypeMask initially.

[jvanverth1, 2013-03-19 15:11:59]

Some initial comments:

https://codereview.chromium.org/12531015/diff/13002/src/gpu/GrDrawState.h
File src/gpu/GrDrawState.h (left):

https://codereview.chromium.org/12531015/diff/13002/src/gpu/GrDrawState.h#old...
src/gpu/GrDrawState.h:1199: s.fCommon.fAttribBindings & kAttribIndexMasks[i]) &&
Can we get rid of kAttribIndexMasks now? It was only created because the
texCoords took up multiple bits.

https://codereview.chromium.org/12531015/diff/13002/src/gpu/GrDrawState.h
File src/gpu/GrDrawState.h (right):

https://codereview.chromium.org/12531015/diff/13002/src/gpu/GrDrawState.h#new...
src/gpu/GrDrawState.h:269: /** explicit local coords are provided (instead of
using pre-view-matrix positions) */
It may not matter, but for consistency's sake it'd be good to have the bit order
here match the index order below. Then the bit for an index is 1 << (index-1),
ignoring position. Or maybe set kPosition_AttribBindingsBit to 0x1, shift
everything else up one bit, and have kDefault_AttribBindings set to
kPosition_AttribBindings bit. That would get rid of the (i ==
kPosition_AttribIndex) check in operator==(), below.

[bsalomon, 2013-03-19 15:22:25]

https://codereview.chromium.org/12531015/diff/13002/src/gpu/GrDrawState.h
File src/gpu/GrDrawState.h (left):

https://codereview.chromium.org/12531015/diff/13002/src/gpu/GrDrawState.h#old...
src/gpu/GrDrawState.h:1199: s.fCommon.fAttribBindings & kAttribIndexMasks[i]) &&
On 2013/03/19 15:11:59, JimVV wrote:
> Can we get rid of kAttribIndexMasks now? It was only created because the
> texCoords took up multiple bits.

Yes, I was planning to do that in a follow-up change.

https://codereview.chromium.org/12531015/diff/13002/src/gpu/GrDrawState.h
File src/gpu/GrDrawState.h (right):

https://codereview.chromium.org/12531015/diff/13002/src/gpu/GrDrawState.h#new...
src/gpu/GrDrawState.h:269: /** explicit local coords are provided (instead of
using pre-view-matrix positions) */
On 2013/03/19 15:11:59, JimVV wrote:
> It may not matter, but for consistency's sake it'd be good to have the bit
order
> here match the index order below. Then the bit for an index is 1 << (index-1),
> ignoring position. Or maybe set kPosition_AttribBindingsBit to 0x1, shift
> everything else up one bit, and have kDefault_AttribBindings set to
> kPosition_AttribBindings bit. That would get rid of the (i ==
> kPosition_AttribIndex) check in operator==(), below.

Can do... but if we're about to get rid of them is it worth it?

[jvanverth1, 2013-03-19 15:36:51]

LGTM + comments

https://codereview.chromium.org/12531015/diff/13002/src/gpu/GrDrawState.h
File src/gpu/GrDrawState.h (right):

https://codereview.chromium.org/12531015/diff/13002/src/gpu/GrDrawState.h#new...
src/gpu/GrDrawState.h:269: /** explicit local coords are provided (instead of
using pre-view-matrix positions) */
On 2013/03/19 15:22:25, bsalomon wrote:
> On 2013/03/19 15:11:59, JimVV wrote:
> > It may not matter, but for consistency's sake it'd be good to have the bit
> order
> > here match the index order below. Then the bit for an index is 1 <<
(index-1),
> > ignoring position. Or maybe set kPosition_AttribBindingsBit to 0x1, shift
> > everything else up one bit, and have kDefault_AttribBindings set to
> > kPosition_AttribBindings bit. That would get rid of the (i ==
> > kPosition_AttribIndex) check in operator==(), below.
> 
> Can do... but if we're about to get rid of them is it worth it?

Well, like I said, it may not matter... :)

https://codereview.chromium.org/12531015/diff/13002/src/gpu/gl/GrGLEffectMatr...
File src/gpu/gl/GrGLEffectMatrix.h (right):

https://codereview.chromium.org/12531015/diff/13002/src/gpu/gl/GrGLEffectMatr...
src/gpu/gl/GrGLEffectMatrix.h:103: 
On 2013/03/19 15:09:38, robertphillips wrote:
> kMatrixFlagsMask? I misunderstood kMatrixTypeMask initially.

Does kGeneral_Key mean (kTrans_Key | kNoPersp_Key)? If not, I think FlagsMask
might also be misleading.

[robertphillips, 2013-03-19 15:41:34]

https://codereview.chromium.org/12531015/diff/13002/src/gpu/gl/GrGLEffectMatr...
File src/gpu/gl/GrGLEffectMatrix.h (right):

https://codereview.chromium.org/12531015/diff/13002/src/gpu/gl/GrGLEffectMatr...
src/gpu/gl/GrGLEffectMatrix.h:103: 
How about kMatrixKeys? Jim is right should kGeneralKey be 0x04?

[bsalomon, 2013-03-19 19:34:42]

https://codereview.chromium.org/12531015/diff/13002/include/core/SkTLazy.h
File include/core/SkTLazy.h (right):

https://codereview.chromium.org/12531015/diff/13002/include/core/SkTLazy.h#ne...
include/core/SkTLazy.h:95: 
On 2013/03/19 15:09:38, robertphillips wrote:
> .. macro (SkNEW_IN_TLAZY) rather ...

Done.

https://codereview.chromium.org/12531015/diff/13002/src/effects/SkBlendImageF...
File src/effects/SkBlendImageFilter.cpp (right):

https://codereview.chromium.org/12531015/diff/13002/src/effects/SkBlendImageF...
src/effects/SkBlendImageFilter.cpp:97: GrGLBlendEffect(const
GrBackendEffectFactory& factory,
On 2013/03/19 15:09:38, robertphillips wrote:
> mv up a line?

Done.

https://codereview.chromium.org/12531015/diff/13002/src/gpu/GrContext.cpp
File src/gpu/GrContext.cpp (right):

https://codereview.chromium.org/12531015/diff/13002/src/gpu/GrContext.cpp#new...
src/gpu/GrContext.cpp:871: // localRect and localMatrix. This code path is
preventing some encapsulation in GrDrawState.
On 2013/03/19 15:09:38, robertphillips wrote:
> matrix -> savedMatrix, origMatrix?

Done. (savedViewMatrix)

https://codereview.chromium.org/12531015/diff/13002/src/gpu/gl/GrGLEffectMatr...
File src/gpu/gl/GrGLEffectMatrix.h (right):

https://codereview.chromium.org/12531015/diff/13002/src/gpu/gl/GrGLEffectMatr...
src/gpu/gl/GrGLEffectMatrix.h:103: 
On 2013/03/19 15:41:35, robertphillips wrote:
> How about kMatrixKeys? Jim is right should kGeneralKey be 0x04?

They aren't meant to be OR'ed together, but rather are an enum.

Check out the latest patch and see if it's any clearer.

https://codereview.chromium.org/12531015/diff/17002/src/gpu/gl/GrGLEffectMatr...
File src/gpu/gl/GrGLEffectMatrix.cpp (right):

https://codereview.chromium.org/12531015/diff/17002/src/gpu/gl/GrGLEffectMatr...
src/gpu/gl/GrGLEffectMatrix.cpp:41: } else if (SkMatrix::kTranslate_Mask &
combinedTypes) {
This was actually a bug. combinedTypes is a bitfield of SkMatrix::TypeMask.

[bsalomon, 2013-03-20 17:33:10]

Committed patchset #4 manually as r8264 (presubmit successful).