GPU implementation of lighting filters

This patch implements the diffuse and specular lighting filters in Ganesh.  There are three light types for each:  distant, point and spot, whose code generation lives in a GrGLLight class hierarchy.  This similar to the CPU implementation, where each light type provides a function to compute the vector from the surface plane to the light (surfaceToLight) and to compute the light colour (emitLightColour).  Instead of templated member functions, as in the CPU implementation, these are virtual functions to emit the light-specific GLSL code.

All of the code for the GPU path lives in the same file as that for the CPU path, SkLightingImageFilter.cpp.  In order to provide Ganesh a hook to access it, SkImageFilter now has a asNewCustomStage() virtual, which allows an image filter to return a GrCustomStage representing that filter.

Note that this patch does not handle the border conditions correctly (the [top|bottom][Left|Right]Normal() functions in the CPU implementation).  That will come in a future patch.

[Stephen White, 2012-07-10 14:55:59]

Fix for SK_SCALAR_IS_FIXED build

[bsalomon, 2012-07-10 15:34:23]

It seems like a problem that the GrGLLight instances are singeltons given that
the GrGLProgramStage are long-lived and  have pointers to a GrGLight which is
responsible for remembering the uniform locations.

[TomH, 2012-07-10 15:47:59]

I think we're going to have to do a lot of work to be happy with this, including
some serious revision to the GrCustomStage infrastructure - if this is the best
way to do things, then my framework stinks. Here are some early notes.

http://codereview.appspot.com/6345081/diff/5002/include/core/SkImageFilter.h
File include/core/SkImageFilter.h (right):

http://codereview.appspot.com/6345081/diff/5002/include/core/SkImageFilter.h#...
include/core/SkImageFilter.h:103: 
Rather than polluting this class with all the asA*() functions, why don't we
push through an API to get the GrCustomStage?

http://codereview.appspot.com/6345081/diff/5002/include/core/SkImageFilter.h#...
include/core/SkImageFilter.h:137: class SK_API SkPoint3 {
It isn't clear to me from your description why this can't be a forward
declaration here?

http://codereview.appspot.com/6345081/diff/5002/src/gpu/SkGpuDevice.cpp
File src/gpu/SkGpuDevice.cpp (right):

http://codereview.appspot.com/6345081/diff/5002/src/gpu/SkGpuDevice.cpp#newco...
src/gpu/SkGpuDevice.cpp:1467: &surfaceScale, &kd)) {
So much boilerplate code that reaches into an object, extracts a bunch of
parameters, and then passes all of them unchanged to another object smells bad
to me - any reason (if we keep this approach) we couldn't just pass the Sk
object to the Gr class and let *it* extract its parameters?

[Stephen White, 2012-07-10 15:51:48]

On 2012/07/10 15:34:23, bsalomon wrote:
> It seems like a problem that the GrGLLight instances are singeltons given that
> the GrGLProgramStage are long-lived and  have pointers to a GrGLight which is
> responsible for remembering the uniform locations.

You're right.  I think I can safely make the GrGLLight instance simply owned by
the GrGLLightingEffect.

[bsalomon, 2012-07-10 16:21:31]

Let's make src/effects able to see into include/gpu (and include/gpu/gl). We
should then be able to eliminate GrLight, using SkLight instead, by moving the
new GR code into src/effects.

Also, we should go ahead with adding virtual asCustomStage() to SkImageFilter
for image filters that have single-pass gpu implementations. Then rather than
having GrContex::lightDiffuse/lightSpecular we can have a function (prolly in
SkGpuDevice) that applies an arbitrary custom stage / image filter with the
result of the asCustomStage function using GrContext::drawRect.

[Stephen White, 2012-07-10 17:28:10]

> I think we're going to have to do a lot of work to be happy with this,
including
> some serious revision to the GrCustomStage infrastructure - if this is the
best
> way to do things, then my framework stinks. Here are some early notes.

No, I don't think it's your framework.  The boilerplate make-work is mostly due
to the asAFoo() functions, which hopefully can go away with an asCustomStage()
virtual.

http://codereview.appspot.com/6345081/diff/5002/include/core/SkImageFilter.h
File include/core/SkImageFilter.h (right):

http://codereview.appspot.com/6345081/diff/5002/include/core/SkImageFilter.h#...
include/core/SkImageFilter.h:103: 
On 2012/07/10 15:47:59, TomH wrote:
> Rather than polluting this class with all the asA*() functions, why don't we
> push through an API to get the GrCustomStage?

That would be great.  If everyone's on board, I'll just go ahead with that.

http://codereview.appspot.com/6345081/diff/5002/include/core/SkImageFilter.h#...
include/core/SkImageFilter.h:137: class SK_API SkPoint3 {
On 2012/07/10 15:47:59, TomH wrote:
> It isn't clear to me from your description why this can't be a forward
> declaration here?

It isn't necessary for the header itself, but it has to be in a place that's
accessible from SkGpuDevice.cpp (and that currently doesn't include
include/effects).

If we move all the Ganesh-specific code into SkLightingImageFilter.cpp, I think
this delcaration can go back to SkLightingImageFilter.h.  Although come to think
of it, that will also require putting include/effects in the GPU side's search
path, so maybe we should just go ahead and do that anyway.

http://codereview.appspot.com/6345081/diff/5002/src/gpu/SkGpuDevice.cpp
File src/gpu/SkGpuDevice.cpp (right):

http://codereview.appspot.com/6345081/diff/5002/src/gpu/SkGpuDevice.cpp#newco...
src/gpu/SkGpuDevice.cpp:1467: &surfaceScale, &kd)) {
On 2012/07/10 15:47:59, TomH wrote:
> So much boilerplate code that reaches into an object, extracts a bunch of
> parameters, and then passes all of them unchanged to another object smells bad
> to me - any reason (if we keep this approach) we couldn't just pass the Sk
> object to the Gr class and let *it* extract its parameters?

I'd prefer to just move all this stuff into src/effects, add the
asCustomEffect() virtual, since that seems like the best long-term solution.

[Stephen White, 2012-07-10 17:29:42]

Interim version, to fix the GrGLLight singleton problem

[TomH, 2012-07-10 17:31:16]

On 2012/07/10 16:21:31, bsalomon wrote:
> Also, we should go ahead with adding virtual asCustomStage() to SkImageFilter
> for image filters that have single-pass gpu implementations. Then rather than
> having GrContex::lightDiffuse/lightSpecular we can have a function (prolly in
> SkGpuDevice) that applies an arbitrary custom stage / image filter with the
> result of the asCustomStage function using GrContext::drawRect.

First version of the generic part of this under development at
http://codereview.appspot.com/6352092.

[TomH, 2012-07-10 18:20:16]

On 2012/07/10 17:31:16, TomH wrote:
> First version of the generic part of this under development at
> http://codereview.appspot.com/6352092.

Steven's got his own version under development; abandoning mine.

[Stephen White, 2012-07-10 19:26:06]

Implemented asCustomStage(), moved all effect code to SkLightImageFilter

[bsalomon, 2012-07-10 19:47:49]

The asACustomStage is a lot better!

http://codereview.appspot.com/6345081/diff/6004/include/core/SkImageFilter.h
File include/core/SkImageFilter.h (right):

http://codereview.appspot.com/6345081/diff/6004/include/core/SkImageFilter.h#...
include/core/SkImageFilter.h:83: *  not.  If stage is non-NULL, a new
GrCustomStage instance is stored
Comment could be ever so slightly confusing. It's up to the caller to unref it.
Maybe say the caller assumes a ref on the stage if it is non-NULL.

http://codereview.appspot.com/6345081/diff/6004/src/gpu/GrContext.cpp
File src/gpu/GrContext.cpp (right):

http://codereview.appspot.com/6345081/diff/6004/src/gpu/GrContext.cpp#newcode...
src/gpu/GrContext.cpp:1944: void GrContext::applyCustomStage(GrTexture*
srcTexture,
Is it possible that this function could be written as a client of GrContext (say
as static function in SkGpuDevice.cpp) instead of as a member of GrContext? I'm
thinking set up a GrPaint with the custom stage and call GrContext::drawRect. A
small reason would be to declutter GrContext a bit, but also to allow GrContext
to decide whether to draw directly to the GrGpu or instead to the
GrInOrderDrawBuffer.

[Stephen White, 2012-07-10 20:53:05]

http://codereview.appspot.com/6345081/diff/6004/include/core/SkImageFilter.h
File include/core/SkImageFilter.h (right):

http://codereview.appspot.com/6345081/diff/6004/include/core/SkImageFilter.h#...
include/core/SkImageFilter.h:83: *  not.  If stage is non-NULL, a new
GrCustomStage instance is stored
On 2012/07/10 19:47:49, bsalomon wrote:
> Comment could be ever so slightly confusing. It's up to the caller to unref
it.
> Maybe say the caller assumes a ref on the stage if it is non-NULL.

Done.

http://codereview.appspot.com/6345081/diff/6004/src/gpu/GrContext.cpp
File src/gpu/GrContext.cpp (right):

http://codereview.appspot.com/6345081/diff/6004/src/gpu/GrContext.cpp#newcode...
src/gpu/GrContext.cpp:1944: void GrContext::applyCustomStage(GrTexture*
srcTexture,
On 2012/07/10 19:47:49, bsalomon wrote:
> Is it possible that this function could be written as a client of GrContext
(say
> as static function in SkGpuDevice.cpp) instead of as a member of GrContext?
I'm
> thinking set up a GrPaint with the custom stage and call GrContext::drawRect.
A
> small reason would be to declutter GrContext a bit, but also to allow
GrContext
> to decide whether to draw directly to the GrGpu or instead to the
> GrInOrderDrawBuffer.

Done.

[Stephen White, 2012-07-10 20:54:13]

Move applyCustomStage into SkGpuDevice.cpp; improve comments

[bsalomon, 2012-07-10 21:10:42]

On 2012/07/10 20:54:13, Stephen White wrote:
> Move applyCustomStage into SkGpuDevice.cpp; improve comments

One last request (as far as I'm concerned anyway): We should add the new stages
to the unit test (GrGpuGL_unittest.cpp)

[Stephen White, 2012-07-10 21:40:15]

Add test cases to GrGpuGL_unittest.cpp

[bsalomon, 2012-07-10 21:45:23]

On 2012/07/10 21:40:15, Stephen White wrote:
> Add test cases to GrGpuGL_unittest.cpp

LGTM if the test passes.

[reed1, 2012-07-11 13:57:02]

I'm fine, modulo trying to make the name clearer that it is allocating a new
thing that the caller must deal with.

http://codereview.appspot.com/6345081/diff/7008/include/core/SkImageFilter.h
File include/core/SkImageFilter.h (right):

http://codereview.appspot.com/6345081/diff/7008/include/core/SkImageFilter.h#...
include/core/SkImageFilter.h:88: */
Its is rare for us to have a "getter" that implies that the receiver is now also
an owner. Can the name somehow reflect that?

asCustomStateRef(...) ?

[Stephen White, 2012-07-11 15:23:07]

On 2012/07/11 13:57:02, reed1 wrote:
> I'm fine, modulo trying to make the name clearer that it is allocating a new
> thing that the caller must deal with.
> 
> http://codereview.appspot.com/6345081/diff/7008/include/core/SkImageFilter.h
> File include/core/SkImageFilter.h (right):
> 
>
http://codereview.appspot.com/6345081/diff/7008/include/core/SkImageFilter.h#...
> include/core/SkImageFilter.h:88: */
> Its is rare for us to have a "getter" that implies that the receiver is now
also
> an owner. Can the name somehow reflect that?
> 
> asCustomStateRef(...) ?

Not sure that's clearer, since CustomStageRef isn't a class per se.  The Create
functions in the API also return a new object which the caller owns; how about
createCustomStage()?

[reed1, 2012-07-11 15:27:44]

On 2012/07/11 15:23:07, Stephen White wrote:
> On 2012/07/11 13:57:02, reed1 wrote:
> > I'm fine, modulo trying to make the name clearer that it is allocating a new
> > thing that the caller must deal with.
> > 
> > http://codereview.appspot.com/6345081/diff/7008/include/core/SkImageFilter.h
> > File include/core/SkImageFilter.h (right):
> > 
> >
>
http://codereview.appspot.com/6345081/diff/7008/include/core/SkImageFilter.h#...
> > include/core/SkImageFilter.h:88: */
> > Its is rare for us to have a "getter" that implies that the receiver is now
> also
> > an owner. Can the name somehow reflect that?
> > 
> > asCustomStateRef(...) ?
> 
> Not sure that's clearer, since CustomStageRef isn't a class per se.  The
Create
> functions in the API also return a new object which the caller owns; how about
> createCustomStage()?

time to vote?

A) asNewCustomStage()
B) asCreateCustomStage()
C) newCustomStage()
D) createCustomStage()

[Stephen White, 2012-07-11 15:29:31]

On 2012/07/11 15:27:44, reed1 wrote:
> On 2012/07/11 15:23:07, Stephen White wrote:
> > On 2012/07/11 13:57:02, reed1 wrote:
> > > I'm fine, modulo trying to make the name clearer that it is allocating a
new
> > > thing that the caller must deal with.
> > > 
> > >
http://codereview.appspot.com/6345081/diff/7008/include/core/SkImageFilter.h
> > > File include/core/SkImageFilter.h (right):
> > > 
> > >
> >
>
http://codereview.appspot.com/6345081/diff/7008/include/core/SkImageFilter.h#...
> > > include/core/SkImageFilter.h:88: */
> > > Its is rare for us to have a "getter" that implies that the receiver is
now
> > also
> > > an owner. Can the name somehow reflect that?
> > > 
> > > asCustomStateRef(...) ?
> > 
> > Not sure that's clearer, since CustomStageRef isn't a class per se.  The
> Create
> > functions in the API also return a new object which the caller owns; how
about
> > createCustomStage()?
> 
> time to vote?
> 
> A) asNewCustomStage()
> B) asCreateCustomStage()
> C) newCustomStage()
> D) createCustomStage()

E) createAsCustomStage() ?

[Stephen White, 2012-07-11 15:30:18]

On 2012/07/11 15:29:31, Stephen White wrote:
> On 2012/07/11 15:27:44, reed1 wrote:
> > On 2012/07/11 15:23:07, Stephen White wrote:
> > > On 2012/07/11 13:57:02, reed1 wrote:
> > > > I'm fine, modulo trying to make the name clearer that it is allocating a
> new
> > > > thing that the caller must deal with.
> > > > 
> > > >
> http://codereview.appspot.com/6345081/diff/7008/include/core/SkImageFilter.h
> > > > File include/core/SkImageFilter.h (right):
> > > > 
> > > >
> > >
> >
>
http://codereview.appspot.com/6345081/diff/7008/include/core/SkImageFilter.h#...
> > > > include/core/SkImageFilter.h:88: */
> > > > Its is rare for us to have a "getter" that implies that the receiver is
> now
> > > also
> > > > an owner. Can the name somehow reflect that?
> > > > 
> > > > asCustomStateRef(...) ?
> > > 
> > > Not sure that's clearer, since CustomStageRef isn't a class per se.  The
> > Create
> > > functions in the API also return a new object which the caller owns; how
> about
> > > createCustomStage()?
> > 
> > time to vote?
> > 
> > A) asNewCustomStage()
> > B) asCreateCustomStage()
> > C) newCustomStage()
> > D) createCustomStage()
> 
> E) createAsCustomStage() ?

I think (A) gets my vote.

[bsalomon, 2012-07-11 15:36:57]

On 2012/07/11 15:30:18, Stephen White wrote:
> On 2012/07/11 15:29:31, Stephen White wrote:
> > On 2012/07/11 15:27:44, reed1 wrote:
> > > On 2012/07/11 15:23:07, Stephen White wrote:
> > > > On 2012/07/11 13:57:02, reed1 wrote:
> > > > > I'm fine, modulo trying to make the name clearer that it is allocating
a
> > new
> > > > > thing that the caller must deal with.
> > > > > 
> > > > >
> > http://codereview.appspot.com/6345081/diff/7008/include/core/SkImageFilter.h
> > > > > File include/core/SkImageFilter.h (right):
> > > > > 
> > > > >
> > > >
> > >
> >
>
http://codereview.appspot.com/6345081/diff/7008/include/core/SkImageFilter.h#...
> > > > > include/core/SkImageFilter.h:88: */
> > > > > Its is rare for us to have a "getter" that implies that the receiver
is
> > now
> > > > also
> > > > > an owner. Can the name somehow reflect that?
> > > > > 
> > > > > asCustomStateRef(...) ?
> > > > 
> > > > Not sure that's clearer, since CustomStageRef isn't a class per se.  The
> > > Create
> > > > functions in the API also return a new object which the caller owns; how
> > about
> > > > createCustomStage()?
> > > 
> > > time to vote?
> > > 
> > > A) asNewCustomStage()
> > > B) asCreateCustomStage()
> > > C) newCustomStage()
> > > D) createCustomStage()
> > 
> > E) createAsCustomStage() ?
> 
> I think (A) gets my vote.

I vote(E) but can live with (A), (C), or (D). (B) is too awkward.

[bsalomon, 2012-07-11 15:37:16]

On 2012/07/11 15:36:57, bsalomon wrote:
> On 2012/07/11 15:30:18, Stephen White wrote:
> > On 2012/07/11 15:29:31, Stephen White wrote:
> > > On 2012/07/11 15:27:44, reed1 wrote:
> > > > On 2012/07/11 15:23:07, Stephen White wrote:
> > > > > On 2012/07/11 13:57:02, reed1 wrote:
> > > > > > I'm fine, modulo trying to make the name clearer that it is
allocating
> a
> > > new
> > > > > > thing that the caller must deal with.
> > > > > > 
> > > > > >
> > >
http://codereview.appspot.com/6345081/diff/7008/include/core/SkImageFilter.h
> > > > > > File include/core/SkImageFilter.h (right):
> > > > > > 
> > > > > >
> > > > >
> > > >
> > >
> >
>
http://codereview.appspot.com/6345081/diff/7008/include/core/SkImageFilter.h#...
> > > > > > include/core/SkImageFilter.h:88: */
> > > > > > Its is rare for us to have a "getter" that implies that the receiver
> is
> > > now
> > > > > also
> > > > > > an owner. Can the name somehow reflect that?
> > > > > > 
> > > > > > asCustomStateRef(...) ?
> > > > > 
> > > > > Not sure that's clearer, since CustomStageRef isn't a class per se. 
The
> > > > Create
> > > > > functions in the API also return a new object which the caller owns;
how
> > > about
> > > > > createCustomStage()?
> > > > 
> > > > time to vote?
> > > > 
> > > > A) asNewCustomStage()
> > > > B) asCreateCustomStage()
> > > > C) newCustomStage()
> > > > D) createCustomStage()
> > > 
> > > E) createAsCustomStage() ?
> > 
> > I think (A) gets my vote.
> 
> I vote(E) but can live with (A), (C), or (D). (B) is too awkward.
I will literally die if we choose (B).

[Stephen White, 2012-07-11 15:44:12]

On 2012/07/11 15:37:16, bsalomon wrote:
> On 2012/07/11 15:36:57, bsalomon wrote:
> > On 2012/07/11 15:30:18, Stephen White wrote:
> > > On 2012/07/11 15:29:31, Stephen White wrote:
> > > > On 2012/07/11 15:27:44, reed1 wrote:
> > > > > On 2012/07/11 15:23:07, Stephen White wrote:
> > > > > > On 2012/07/11 13:57:02, reed1 wrote:
> > > > > > > I'm fine, modulo trying to make the name clearer that it is
> allocating
> > a
> > > > new
> > > > > > > thing that the caller must deal with.
> > > > > > > 
> > > > > > >
> > > >
> http://codereview.appspot.com/6345081/diff/7008/include/core/SkImageFilter.h
> > > > > > > File include/core/SkImageFilter.h (right):
> > > > > > > 
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>
http://codereview.appspot.com/6345081/diff/7008/include/core/SkImageFilter.h#...
> > > > > > > include/core/SkImageFilter.h:88: */
> > > > > > > Its is rare for us to have a "getter" that implies that the
receiver
> > is
> > > > now
> > > > > > also
> > > > > > > an owner. Can the name somehow reflect that?
> > > > > > > 
> > > > > > > asCustomStateRef(...) ?
> > > > > > 
> > > > > > Not sure that's clearer, since CustomStageRef isn't a class per se. 
> The
> > > > > Create
> > > > > > functions in the API also return a new object which the caller owns;
> how
> > > > about
> > > > > > createCustomStage()?
> > > > > 
> > > > > time to vote?
> > > > > 
> > > > > A) asNewCustomStage()
> > > > > B) asCreateCustomStage()
> > > > > C) newCustomStage()
> > > > > D) createCustomStage()
> > > > 
> > > > E) createAsCustomStage() ?
> > > 
> > > I think (A) gets my vote.
> > 
> > I vote(E) but can live with (A), (C), or (D). (B) is too awkward.
> I will literally die if we choose (B).

I wouldn't want Brian's death on my hands, so (E) it is.  Going once..

[reed1, 2012-07-11 15:44:15]

On 2012/07/11 15:37:16, bsalomon wrote:
> On 2012/07/11 15:36:57, bsalomon wrote:
> > On 2012/07/11 15:30:18, Stephen White wrote:
> > > On 2012/07/11 15:29:31, Stephen White wrote:
> > > > On 2012/07/11 15:27:44, reed1 wrote:
> > > > > On 2012/07/11 15:23:07, Stephen White wrote:
> > > > > > On 2012/07/11 13:57:02, reed1 wrote:
> > > > > > > I'm fine, modulo trying to make the name clearer that it is
> allocating
> > a
> > > > new
> > > > > > > thing that the caller must deal with.
> > > > > > > 
> > > > > > >
> > > >
> http://codereview.appspot.com/6345081/diff/7008/include/core/SkImageFilter.h
> > > > > > > File include/core/SkImageFilter.h (right):
> > > > > > > 
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>
http://codereview.appspot.com/6345081/diff/7008/include/core/SkImageFilter.h#...
> > > > > > > include/core/SkImageFilter.h:88: */
> > > > > > > Its is rare for us to have a "getter" that implies that the
receiver
> > is
> > > > now
> > > > > > also
> > > > > > > an owner. Can the name somehow reflect that?
> > > > > > > 
> > > > > > > asCustomStateRef(...) ?
> > > > > > 
> > > > > > Not sure that's clearer, since CustomStageRef isn't a class per se. 
> The
> > > > > Create
> > > > > > functions in the API also return a new object which the caller owns;
> how
> > > > about
> > > > > > createCustomStage()?
> > > > > 
> > > > > time to vote?
> > > > > 
> > > > > A) asNewCustomStage()
> > > > > B) asCreateCustomStage()
> > > > > C) newCustomStage()
> > > > > D) createCustomStage()
> > > > 
> > > > E) createAsCustomStage() ?
> > > 
> > > I think (A) gets my vote.
> > 
> > I vote(E) but can live with (A), (C), or (D). (B) is too awkward.
> I will literally die if we choose (B).

I like E or A

[Stephen White, 2012-07-11 15:53:13]

On 2012/07/11 15:44:15, reed1 wrote:
> On 2012/07/11 15:37:16, bsalomon wrote:
> > On 2012/07/11 15:36:57, bsalomon wrote:
> > > On 2012/07/11 15:30:18, Stephen White wrote:
> > > > On 2012/07/11 15:29:31, Stephen White wrote:
> > > > > On 2012/07/11 15:27:44, reed1 wrote:
> > > > > > On 2012/07/11 15:23:07, Stephen White wrote:
> > > > > > > On 2012/07/11 13:57:02, reed1 wrote:
> > > > > > > > I'm fine, modulo trying to make the name clearer that it is
> > allocating
> > > a
> > > > > new
> > > > > > > > thing that the caller must deal with.
> > > > > > > > 
> > > > > > > >
> > > > >
> > http://codereview.appspot.com/6345081/diff/7008/include/core/SkImageFilter.h
> > > > > > > > File include/core/SkImageFilter.h (right):
> > > > > > > > 
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>
http://codereview.appspot.com/6345081/diff/7008/include/core/SkImageFilter.h#...
> > > > > > > > include/core/SkImageFilter.h:88: */
> > > > > > > > Its is rare for us to have a "getter" that implies that the
> receiver
> > > is
> > > > > now
> > > > > > > also
> > > > > > > > an owner. Can the name somehow reflect that?
> > > > > > > > 
> > > > > > > > asCustomStateRef(...) ?
> > > > > > > 
> > > > > > > Not sure that's clearer, since CustomStageRef isn't a class per
se. 
> > The
> > > > > > Create
> > > > > > > functions in the API also return a new object which the caller
owns;
> > how
> > > > > about
> > > > > > > createCustomStage()?
> > > > > > 
> > > > > > time to vote?
> > > > > > 
> > > > > > A) asNewCustomStage()
> > > > > > B) asCreateCustomStage()
> > > > > > C) newCustomStage()
> > > > > > D) createCustomStage()
> > > > > 
> > > > > E) createAsCustomStage() ?
> > > > 
> > > > I think (A) gets my vote.
> > > 
> > > I vote(E) but can live with (A), (C), or (D). (B) is too awkward.
> > I will literally die if we choose (B).
> 
> I like E or A

After seeing it in code, I think I like (A) better, so I'm going with that. 
Thanks for playing paint-the-bikeshed!  Don't forget to have your pets spayed or
neutered.

[Stephen White, 2012-07-11 15:56:40]

asNewCustomStage(), GrStringBuilder -> SkString, _VariableLifetime ->
_ShaderType

[bsalomon, 2012-07-11 15:58:49]

On 2012/07/11 15:56:40, Stephen White wrote:
> asNewCustomStage(), GrStringBuilder -> SkString, _VariableLifetime ->
> _ShaderType

LGTM

[Stephen White, 2012-07-12 20:24:42]

Closed with r4535.