[RFC weston 0/2] Adding a more flexible shader generation

[Ander Conselvan de Oliveira]

On 11/14/2012 05:41 PM, John Kåre Alsaker wrote:
> These two patches adds a flexible shader generation which is based on concatenating strings instead of duplicating them into a lot of literals. This becomes more useful as I add shader variants for converting to/from sRGB gamma and it also allows me to easily get rid of the alpha uniform used for fading surfaces.
>
> I have not tested the YUV shaders since that doesn't appear to be too easy without an Intel GPU. I also have a more vectorized variant of the YUV->RGB conversion I'd like to test.
>
> I've split this into two patches to make the introduction of this a bit clearer. I seemed to rewrite all of gl-shaders.c anyway with only 2 functions being recognisable. So I was wondering if I should just merge these patches.
>
> The first commit which really utilizes this is the one which adds gamma correct rendering. Should I wait until cleaning up that before submitting this?

I think it it would be a good idea to send it if that would show why do 
you want to make this change.

A few thoughts:

  - Are the shader constructor functions and the shader_builder really 
necessary? It seems the only thing the constructors do is call 
shader_build_add(). If the you could use a struct with a few string 
fields (one for global, one for main, etc) maybe the shader source 
wouldn't be burried so deep in the constructor calls.

Besides, if you only have a list of string you can avoid the memcpy in 
shader_build_add() and let glShaderSource do the concatenation.

Anyway, I just think this makes the shader sources much less readable.

  - What about performance implications?

The whole create all permutations is scary. If the number of shaders
start growing it would be good to compile only the shaders that are 
being used.

Also, before the shader would be selected for the surface on attach, but 
now you call gl_use_shader() on draw surface. Is this really necessary? 
I ask because this function is a for loop on the number of attributes 
and this makes me think of how scalable this is.

Cheers,
Ander



> John Kåre Alsaker (2):
>    gl-renderer: Move shader functions into it's own file.
>    gl-renderer: Add flexible shader generation.
>
>   src/Makefile.am   |   2 +
>   src/gl-internal.h | 171 ++++++++++++++++
>   src/gl-renderer.c | 428 ++++----------------------------------
>   src/gl-shaders.c  | 602 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>   4 files changed, 819 insertions(+), 384 deletions(-)
>   create mode 100644 src/gl-internal.h
>   create mode 100644 src/gl-shaders.c
>