GrCustomStages for Radial, Radial2, Radial2Degenerate Mappings

On old codebase, has the multi-substage design we're planning on not using, not cleaned up - but works!

Comment on any overall issues you here, meanwhile I'll open new CLs to incrementally port this to the current codebase.

[bsalomon, 2012-05-23 19:07:28]

great that you got it working. Some comments, I'll keep looking but I'm ignoring
the fetch vs. mapping aspect.

http://codereview.appspot.com/6214056/diff/4002/src/gpu/effects/GrRadialMappi...
File src/gpu/effects/GrRadialMapping.cpp (right):

http://codereview.appspot.com/6214056/diff/4002/src/gpu/effects/GrRadialMappi...
src/gpu/effects/GrRadialMapping.cpp:70:
(*paramVar)->setTypeModifier(GrGLShaderVar::kUniform_TypeModifier);
I think we should start to flesh out the builder to help with code like this. It
seems like the caller has to specify quite a bit to setup a uniform and can set
them up incorrectly (e.g. not give the correct modifier/precision params or make
the name be non-unique).

http://codereview.appspot.com/6214056/diff/4002/src/gpu/effects/GrRadialMappi...
src/gpu/effects/GrRadialMapping.cpp:482: class GrRadial2DegenerateMappingFactory
: public GrProgramStageFactory {
It'd be great if the caller didn't have to know whether the gradient was
degenerate or not. We might have to think about factories that can produce
different GL-specific subclasses depending up the stage data.

[TomH, 2012-05-23 20:14:00]

On 2012/05/23 19:07:28, bsalomon wrote:
> great that you got it working. Some comments, I'll keep looking but I'm
ignoring
> the fetch vs. mapping aspect.
> 
>
http://codereview.appspot.com/6214056/diff/4002/src/gpu/effects/GrRadialMappi...
> File src/gpu/effects/GrRadialMapping.cpp (right):
> 
>
http://codereview.appspot.com/6214056/diff/4002/src/gpu/effects/GrRadialMappi...
> src/gpu/effects/GrRadialMapping.cpp:70:
> (*paramVar)->setTypeModifier(GrGLShaderVar::kUniform_TypeModifier);
> I think we should start to flesh out the builder to help with code like this.
It
> seems like the caller has to specify quite a bit to setup a uniform and can
set
> them up incorrectly (e.g. not give the correct modifier/precision params or
make
> the name be non-unique).

Will do.

http://codereview.appspot.com/6214056/diff/4002/src/gpu/effects/GrRadialMappi...
> src/gpu/effects/GrRadialMapping.cpp:482: class
GrRadial2DegenerateMappingFactory
> : public GrProgramStageFactory {
> It'd be great if the caller didn't have to know whether the gradient was
> degenerate or not. We might have to think about factories that can produce
> different GL-specific subclasses depending up the stage data.

I'm not sure we even need different subclasses; in this case, the only
difference is a couple of lines in the FS, and we can probably access the data
we need to make that decision when we're emitting the FS.

[robertphillips, 2012-05-23 20:24:22]

LGTM + 2 possibilities

http://codereview.appspot.com/6214056/diff/4002/src/gpu/effects/GrRadialMappi...
File src/gpu/effects/GrRadialMapping.cpp (right):

http://codereview.appspot.com/6214056/diff/4002/src/gpu/effects/GrRadialMappi...
src/gpu/effects/GrRadialMapping.cpp:55: // do nothing
rm comment?

http://codereview.appspot.com/6214056/diff/4002/src/gpu/effects/GrRadialMappi...
src/gpu/effects/GrRadialMapping.cpp:333: , fVSVaryingName (NULL)
init fParamLocation?

[TomH, 2012-05-23 20:25:06]

Robert, this is just proof-of-concept; I ought to have cc'd you on
http://codereview.appspot.com/6239043/.