Initial refactor of shaderbuilder

Initial refactor of shaderbuilder to prepare for geometry shaders


gitignore for eclipse

BUG=skia:

Committed: https://skia.googlesource.com/skia/+/30ba436f04e61d4505fb854d5fc56079636e0788

[joshua.litt, 2014-08-20 00:15:47]

Hey Brian,

This is still really messy and preliminary, but if you could take a look and
give some feedback on the overall design, that'd be great.

[bsalomon, 2014-08-21 14:09:40]

On 2014/08/20 at 00:15:47, joshualitt wrote:
> Hey Brian,
> 
> This is still really messy and preliminary, but if you could take a look and
give some feedback on the overall design, that'd be great.

Hey Joshua, this is a huge CL. Can you give some pointers as to what the overall
changes are and what files to start with?

[joshua.litt, 2014-08-21 16:02:06]

On 2014/08/21 14:09:40, bsalomon wrote:
> On 2014/08/20 at 00:15:47, joshualitt wrote:
> > Hey Brian,
> > 
> > This is still really messy and preliminary, but if you could take a look and
> give some feedback on the overall design, that'd be great.
> 
> Hey Joshua, this is a huge CL. Can you give some pointers as to what the
overall
> changes are and what files to start with?

I would focus on the builders folder, most of the other stuff is just renaming
shader builder and returning the appropriate builder in emitcode.

So builders/* is really the heart of this change

[joshua.litt, 2014-08-21 16:14:11]

On 2014/08/21 16:02:06, joshua.litt wrote:
> On 2014/08/21 14:09:40, bsalomon wrote:
> > On 2014/08/20 at 00:15:47, joshualitt wrote:
> > > Hey Brian,
> > > 
> > > This is still really messy and preliminary, but if you could take a look
and
> > give some feedback on the overall design, that'd be great.
> > 
> > Hey Joshua, this is a huge CL. Can you give some pointers as to what the
> overall
> > changes are and what files to start with?
> 
> I would focus on the builders folder, most of the other stuff is just renaming
> shader builder and returning the appropriate builder in emitcode.
> 
> So builders/* is really the heart of this change

Oh and so basically all I really did was refactor GrGLShaderBuilder into:
1) GrGLProgramBuilder - the main interface for building shaders.  Returns sub
builders, and manages varyings / uniforms.
2) GrGLShaderBuilder - base class for all GrGL*ShaderBuilders, owns inputs /
outputs, code, can append textures, functions, extensions, etc.
3) *GrGLVertexShaderBuilder - subclass of shader builder, in addition to shader
builder it also manages attributes
4) *GrGLGeometryShaderBuilder - subclass of shader builder, currently a dummy
5) GrGLFragmentShaderBuilder - subclass of shader builder, manages fragment
position and other fragment related things

* indicates a shaderbuilder which lives off of FullProgramBuilder, a subclass of
ProgramBuilder for the full pipeline.

The different shaderbuilders also manage their own setup and teardown currently

[bsalomon, 2014-08-21 17:18:00]

Thanks for the explanation! It helped a lot. I've just looked through the
builders dir. Seems like a good direction.

https://codereview.chromium.org/491673002/diff/100001/src/gpu/gl/builders/GrG...
File src/gpu/gl/builders/GrGLShaderBuilder.h (right):

https://codereview.chromium.org/491673002/diff/100001/src/gpu/gl/builders/GrG...
src/gpu/gl/builders/GrGLShaderBuilder.h:30: base class for all shaders
How about "... all shader builders"?

https://codereview.chromium.org/491673002/diff/100001/src/gpu/gl/builders/GrG...
src/gpu/gl/builders/GrGLShaderBuilder.h:43: * lookups in any kind of shader. 
However, For the most part, usage of these features should be
Would it work anywhere else? Maybe we should say "currently use of textures is
only supported in fragment shaders"?

If it does just work from other shaders, then great.

https://codereview.chromium.org/491673002/diff/100001/src/gpu/gl/builders/GrG...
src/gpu/gl/builders/GrGLShaderBuilder.h:149: class GrGLFullShaderBuilder :
public GrGLShaderBuilder {
Why do we need this subclass?

https://codereview.chromium.org/491673002/diff/100001/src/gpu/gl/builders/GrG...
File src/gpu/gl/builders/GrGLShaderStringBuilder.h (right):

https://codereview.chromium.org/491673002/diff/100001/src/gpu/gl/builders/GrG...
src/gpu/gl/builders/GrGLShaderStringBuilder.h:15: namespace
GrGLShaderStringBuilder {
Is the namespace helping here? If you aren't planning to add more functions
maybe just have a function in the top level called GrCompileAndAttachShader?

[joshua.litt, 2014-08-21 19:43:55]

Rebase + feedback incorporated

[bsalomon, 2014-08-21 20:21:01]

lgtm with a couple nits

https://codereview.chromium.org/491673002/diff/120001/gyp/gpu.gypi
File gyp/gpu.gypi (right):

https://codereview.chromium.org/491673002/diff/120001/gyp/gpu.gypi#newcode253
gyp/gpu.gypi:253: # Sk files
Did these get duplicated?

https://codereview.chromium.org/491673002/diff/120001/src/gpu/gl/builders/GrG...
File src/gpu/gl/builders/GrGLFragmentShaderBuilder.cpp (right):

https://codereview.chromium.org/491673002/diff/120001/src/gpu/gl/builders/GrG...
src/gpu/gl/builders/GrGLFragmentShaderBuilder.cpp:13:
////////////////////////////////////////////////////////////////////////////////////////////////////
Having these local, static, private blocks is at odds with the rest of the code.

https://codereview.chromium.org/491673002/diff/120001/src/gpu/gl/builders/GrG...
File src/gpu/gl/builders/GrGLShaderBuilder.h (right):

https://codereview.chromium.org/491673002/diff/120001/src/gpu/gl/builders/GrG...
src/gpu/gl/builders/GrGLShaderBuilder.h:164: GrGLFullProgramBuilder*
fullProgram() { return fFullProgram; }
How about getFullProgramBuilder()?  It's more a builder than a program...

[joshua.litt, 2014-08-22 03:10:29]

The CQ bit was checked by joshualitt@chromium.org

[commit-bot: I haz the power, 2014-08-22 03:10:52]

CQ is trying da patch. Follow status at

https://skia-tree-status.appspot.com/cq/joshualitt@chromium.org/491673002/160001

[commit-bot: I haz the power, 2014-08-22 03:18:52]

Committed patchset #9 (160001) as 30ba436f04e61d4505fb854d5fc56079636e0788