removing GrDrawEffect

removing GrDrawEffect

BUG=skia:

Committed: https://skia.googlesource.com/skia/+/8ddbe8b9366c8c59c4fb55f01f253de8a0b37d6e

Committed: https://skia.googlesource.com/skia/+/49586bec7383d4ccb81f85f8e2dc4162e2d4f6a8

[joshua.litt, 2014-09-15 19:11:22]

joshualitt@chromium.org changed reviewers:
+ bsalomon@google.com

[bsalomon, 2014-09-15 19:54:26]

https://codereview.chromium.org/571163002/diff/20001/include/gpu/GrEffect.h
File include/gpu/GrEffect.h (right):

https://codereview.chromium.org/571163002/diff/20001/include/gpu/GrEffect.h#n...
include/gpu/GrEffect.h:136: template <typename T> static const T&
CastEffect(const GrEffect& effect) {
How about

template <typename T> const T& cast() const { return static_cast<T>(*this); }

and then do

const MyEffect& me = effect.cast<MyEffect>();

in setData() and other spots?

I find the repeated GrEffect::CastEffect to be verbose.

https://codereview.chromium.org/571163002/diff/20001/src/gpu/gl/GrGLEffect.h
File src/gpu/gl/GrGLEffect.h (right):

https://codereview.chromium.org/571163002/diff/20001/src/gpu/gl/GrGLEffect.h#...
src/gpu/gl/GrGLEffect.h:55: @param effect   A wrapper on the effect that
generated this program stage.
algnment, no longer a wrapper.

https://codereview.chromium.org/571163002/diff/20001/src/gpu/gl/GrGLProgramEf...
File src/gpu/gl/GrGLProgramEffects.cpp (right):

https://codereview.chromium.org/571163002/diff/20001/src/gpu/gl/GrGLProgramEf...
src/gpu/gl/GrGLProgramEffects.cpp:99: const SkMatrix& ccm =
effectStage.getCoordChangeMatrix(useExplicitLocalCoords);
It feels like having this variant of getCoordChangeMatrix isn't helping much.
Why don't we just make the callers only call it if !usesExplicitLocalCoords?

https://codereview.chromium.org/571163002/diff/20001/src/gpu/gl/GrGLProgramEf...
File src/gpu/gl/GrGLProgramEffects.h (right):

https://codereview.chromium.org/571163002/diff/20001/src/gpu/gl/GrGLProgramEf...
src/gpu/gl/GrGLProgramEffects.h:121: static uint32_t GenTransformKey(const
GrEffectStage&, bool);
seems like we should name the bool param since it's meaning isn't obvious.

[joshua.litt, 2014-09-15 23:01:21]

Feedback inc

[bsalomon, 2014-09-16 13:29:31]

lgtm

[joshua.litt, 2014-09-16 13:36:40]

The CQ bit was checked by joshualitt@chromium.org

[commit-bot: I haz the power, 2014-09-16 13:37:19]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patchset/571163002/60001

[commit-bot: I haz the power, 2014-09-16 13:46:36]

Committed patchset #4 (id:60001) as 8ddbe8b9366c8c59c4fb55f01f253de8a0b37d6e

[joshua.litt, 2014-09-16 14:13:45]

A revert of this CL (patchset #4 id:60001) has been created in
https://codereview.chromium.org/577593003/ by joshualitt@chromium.org.

The reason for reverting is: reverting to unblock another revert.

[joshua.litt, 2014-09-16 15:12:58]

The CQ bit was checked by joshualitt@chromium.org

[commit-bot: I haz the power, 2014-09-16 15:13:49]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patchset/571163002/80001

[commit-bot: I haz the power, 2014-09-16 15:21:47]

Committed patchset #5 (id:80001) as 49586bec7383d4ccb81f85f8e2dc4162e2d4f6a8