experimental GS

[bsalomon, 2011-09-28 18:01:06]

Adds a "null" geometry shader. Debug-only, used in GLShaders unit test.

[Stephen White, 2011-09-28 18:58:29]

Looks fine (some nits which you can fix or ignore at your leisure).  Leaving for
junov for the version number stuff.

http://codereview.appspot.com/5129047/diff/4003/gpu/include/GrGLInterface.h
File gpu/include/GrGLInterface.h (right):

http://codereview.appspot.com/5129047/diff/4003/gpu/include/GrGLInterface.h#n...
gpu/include/GrGLInterface.h:31: #define GR_GLSL_VER(major, minor) (((int)(major)
<< 16) | ((int)(minor)))
nit:  c-style casts (although I guess skia has a lot of those).

http://codereview.appspot.com/5129047/diff/4003/gpu/src/GrGLInterface.cpp
File gpu/src/GrGLInterface.cpp (right):

http://codereview.appspot.com/5129047/diff/4003/gpu/src/GrGLInterface.cpp#new...
gpu/src/GrGLInterface.cpp:47: GrAssert(versionString);
Maybe should do something more than an assert here.  If the driver is really
b0rked and returns a null ptr, should probably handle that gracefully -- random
searching says that sscanf behaviour on a null string is undefined.  Same for
GrGLGetVersionFromString() above (not new code tho).

http://codereview.appspot.com/5129047/diff/4003/gpu/src/GrGLProgram.cpp
File gpu/src/GrGLProgram.cpp (right):

http://codereview.appspot.com/5129047/diff/4003/gpu/src/GrGLProgram.cpp#newco...
gpu/src/GrGLProgram.cpp:387: void append_varying(GrGLShaderVar::Type type,
Nit:  function could be static.

[junov1, 2011-09-28 19:11:45]

http://codereview.appspot.com/5129047/diff/4003/gpu/src/GrGLProgram.cpp
File gpu/src/GrGLProgram.cpp (right):

http://codereview.appspot.com/5129047/diff/4003/gpu/src/GrGLProgram.cpp#newco...
gpu/src/GrGLProgram.cpp:548: #if GR_DEBUG
Why GR_DEBUG?  could be GR_EXPERIMENTAL_GEO_SHADER or something like that

[bsalomon, 2011-09-28 21:03:08]

http://codereview.appspot.com/5129047/diff/4003/gpu/include/GrGLInterface.h
File gpu/include/GrGLInterface.h (right):

http://codereview.appspot.com/5129047/diff/4003/gpu/include/GrGLInterface.h#n...
gpu/include/GrGLInterface.h:31: #define GR_GLSL_VER(major, minor) (((int)(major)
<< 16) | ((int)(minor)))
On 2011/09/28 18:58:29, Stephen White wrote:
> nit:  c-style casts (although I guess skia has a lot of those).

Done.

http://codereview.appspot.com/5129047/diff/4003/gpu/src/GrGLInterface.cpp
File gpu/src/GrGLInterface.cpp (right):

http://codereview.appspot.com/5129047/diff/4003/gpu/src/GrGLInterface.cpp#new...
gpu/src/GrGLInterface.cpp:47: GrAssert(versionString);
On 2011/09/28 18:58:29, Stephen White wrote:
> Maybe should do something more than an assert here.  If the driver is really
> b0rked and returns a null ptr, should probably handle that gracefully --
random
> searching says that sscanf behaviour on a null string is undefined.  Same for
> GrGLGetVersionFromString() above (not new code tho).

Done.

http://codereview.appspot.com/5129047/diff/4003/gpu/src/GrGLProgram.cpp
File gpu/src/GrGLProgram.cpp (right):

http://codereview.appspot.com/5129047/diff/4003/gpu/src/GrGLProgram.cpp#newco...
gpu/src/GrGLProgram.cpp:387: void append_varying(GrGLShaderVar::Type type,
On 2011/09/28 18:58:29, Stephen White wrote:
> Nit:  function could be static.

Does static add value in an anon namespace?

http://codereview.appspot.com/5129047/diff/4003/gpu/src/GrGLProgram.cpp#newco...
gpu/src/GrGLProgram.cpp:548: #if GR_DEBUG
On 2011/09/28 19:11:45, junov1 wrote:
> Why GR_DEBUG?  could be GR_EXPERIMENTAL_GEO_SHADER or something like that

The idea is to make the bots run it. But it is a bit unclear. I made a new
#define that resolves to GR_DEBUG.

[Stephen White, 2011-09-28 22:08:37]

On 2011/09/28 21:03:08, bsalomon wrote:
> > Nit:  function could be static.
> 
> Does static add value in an anon namespace?

Whoops, missed the namespace.  No, it doesn't.
> 
>
http://codereview.appspot.com/5129047/diff/4003/gpu/src/GrGLProgram.cpp#newco...
> gpu/src/GrGLProgram.cpp:548: #if GR_DEBUG
> On 2011/09/28 19:11:45, junov1 wrote:
> > Why GR_DEBUG?  could be GR_EXPERIMENTAL_GEO_SHADER or something like that
> 
> The idea is to make the bots run it. But it is a bit unclear. I made a new
> #define that resolves to GR_DEBUG.

[junov1, 2011-09-29 14:06:01]

LGTM

[bsalomon, 2011-10-03 13:24:15]

Checked in at r2365