Ganesh shaders: plumbing to use GrCustomShader nodes if provided

The previous CL had the base classes; this CL turns them on.

One member function that Brian asked me to make protected last time around had to revert to public because now we're actually calling it.

[TomH, 2012-04-19 13:54:26]

Step 2: hook up the pipes.

[robertphillips, 2012-04-19 15:14:04]

One nit + several clarifying questions

http://codereview.appspot.com/6074043/diff/2001/include/gpu/GrSamplerState.h
File include/gpu/GrSamplerState.h (right):

http://codereview.appspot.com/6074043/diff/2001/include/gpu/GrSamplerState.h#...
include/gpu/GrSamplerState.h:242: void setCustomStage(GrCustomStage* stage) {
fCustomStage = stage; }
So this class doesn't need a ref count?

http://codereview.appspot.com/6074043/diff/2001/src/gpu/gl/GrGLProgram.cpp
File src/gpu/gl/GrGLProgram.cpp (right):

http://codereview.appspot.com/6074043/diff/2001/src/gpu/gl/GrGLProgram.cpp#ne...
src/gpu/gl/GrGLProgram.cpp:741: for (int s = 0; s < GrDrawState::kNumStages;
s++) {
pre-increment

http://codereview.appspot.com/6074043/diff/2001/src/gpu/gl/GrGLProgram.h
File src/gpu/gl/GrGLProgram.h (right):

http://codereview.appspot.com/6074043/diff/2001/src/gpu/gl/GrGLProgram.h#newc...
src/gpu/gl/GrGLProgram.h:369: fCustomStages[index] = stage;
This guy doesn't own it either?

http://codereview.appspot.com/6074043/diff/2001/src/gpu/gl/GrGLProgram.h#newc...
src/gpu/gl/GrGLProgram.h:375: GrCustomStage*
fCustomStages[GrDrawState::kNumStages];
Would initializing this to all NULLs in the constructor buy us any additional
safety?

[TomH, 2012-04-19 16:44:39]

Note from verbal review: try to not store GrCustomStage* on GrGLProgram; pass it
in when required.

[TomH, 2012-04-19 20:54:26]

This version (a bit rough) removes the GrCustomStage* array from GrGLProgram;
it's assembled on the stack and passed through function arguments when needed.
Worked better than I expected.

Still doesn't have a solution for lifetimes; I'll get to that in the morning.

http://codereview.appspot.com/6074043/diff/2001/src/gpu/gl/GrGLProgram.cpp
File src/gpu/gl/GrGLProgram.cpp (right):

http://codereview.appspot.com/6074043/diff/2001/src/gpu/gl/GrGLProgram.cpp#ne...
src/gpu/gl/GrGLProgram.cpp:741: for (int s = 0; s < GrDrawState::kNumStages;
s++) {
On 2012/04/19 15:14:04, robertphillips wrote:
> pre-increment

Eww! Not in the coding standards, but I guess it's what other misguided souls
have done in this file.
Done.

[bsalomon, 2012-04-19 21:06:33]

If you want to defer finalizing the lifetime until there is an actual caller
that's ok with me. My guesses: GrCustomStage is refcounted, GrGLProgramStage is
solely used by the GrGLProgram and it can delete it unconditionally in its
destructor. I suppose some custom stage that has no uniform data to track could
have a factory that always returns a singleton GrGLProgramStage, but it'd be OK
to disallow that.

http://codereview.appspot.com/6074043/diff/2001/src/gpu/gl/GrGLProgram.cpp
File src/gpu/gl/GrGLProgram.cpp (right):

http://codereview.appspot.com/6074043/diff/2001/src/gpu/gl/GrGLProgram.cpp#ne...
src/gpu/gl/GrGLProgram.cpp:741: for (int s = 0; s < GrDrawState::kNumStages;
s++) {
On 2012/04/19 20:54:27, TomH wrote:
> On 2012/04/19 15:14:04, robertphillips wrote:
> > pre-increment
> 
> Eww! Not in the coding standards, but I guess it's what other misguided souls
> have done in this file.
> Done.

I think the world has converged on pre- as the "normal" way since the advent of
operator overloading (and the horrible things that can happen with
post-increment).

[robertphillips, 2012-04-20 12:43:43]

LGTM

[TomH, 2012-04-20 15:33:30]

This version has lifetime management as sketched by Brian, although we won't
thoroughly test it until we start passing these objects around.

[bsalomon, 2012-04-20 17:51:15]

SampleState needs to ref / unref.

http://codereview.appspot.com/6074043/diff/15001/src/gpu/gl/GrGLProgram.cpp
File src/gpu/gl/GrGLProgram.cpp (right):

http://codereview.appspot.com/6074043/diff/15001/src/gpu/gl/GrGLProgram.cpp#n...
src/gpu/gl/GrGLProgram.cpp:630: delete fCustomStage[i];
delete allows NULL

[TomH, 2012-04-20 18:06:06]

Also per discussion with Brian move GrCustomStage.h from src/gpu/ to
include/gpu/, to allow for future extension by third parties.

[TomH, 2012-04-20 18:36:36]

Committed in r3742.

Won't actually work: we need to implement RTTI for GrCustomStage so that we can
properly compare GrSamplerState.