[Mesa-dev] [PATCH RFC 3/3] glsl: Rework builtin_variables.cpp to reduce code duplication.

[Paul Berry]

On 8 July 2013 10:40, Paul Berry <stereotype441 at gmail.com> wrote:

> Previously, we had a separate function for setting up the built-in
> variables for each combination of shader stage and GLSL version
> (e.g. generate_110_vs_variables to generate the built-in variables for
> GLSL 1.10 vertex shaders).  The functions called each other in ad-hoc
> ways, leading to unexpected inconsistencies (for example,
> generate_120_fs_variables was called for GLSL versions 1.20 and above,
> but generate_130_fs_variables was called only for GLSL version 1.30).
> In addition, it led to a lot of code duplication, since many varyings
> had to be duplicated in both the FS and VS code paths.  With the
> advent of geometry shaders (and later, tessellation control and
> tessellation evaluation shaders), this code duplication was going to
> get a lot worse.
>
> So this patch reworks things so that instead of having a separate
> function for each shader type and GLSL version, we have a function for
> constants, one for uniforms, one for varyings, and one for the special
> variables that are specific to each shader type.
>
> In addition, we use a class, builtin_variable_generator, to keep track
> of the instruction exec_list, the GLSL parse state, commonly-used
> types, and a few other variables, so that we don't have to pass them
> around as function arguments.  This makes the code a lot more compact.
>
> Where it was feasible to do so without introducing compilation errors,
> I've also gone ahead and introduced the variables needed for
> {ARB,EXT}_geometry_shader4 style geometry shaders.  This patch takes
> care of everything except the GS variable gl_VerticesIn, the FS
> variable gl_PrimitiveID, and GLSL 1.50 style geometry shader inputs
> (using the gl_in interface block).  Those remaining features will be
> added later.
>
> I've also made a slight nomenclature change: previously we used the
> word "deprecated" to refer to variables which are marked in GLSL 1.40
> as requiring the ARB_compatibility extension, and are marked in GLSL
> 1.50 onward as requiring the compatibilty profile.  This was
> misleading, since not all deprecated variables require the
> compatibility profile (for example gl_FragData and gl_FragColor, which
> have been deprecated since GLSL 1.30, but do not require the
> compatibility profile until GLSL 4.20).  We now consistently use the
> word "compatibility" to refer to these variables.
>
> This patch doesn't introduce any functional changes (since geometry
> shaders haven't been enabled yet).
> ---
>  src/glsl/builtin_variables.cpp | 1124
> +++++++++++++---------------------------
>  1 file changed, 363 insertions(+), 761 deletions(-)
>

Ian wanted to know what the impact of this patch was on executable size.
With I release build, I measured 5742592 bytes before (for libdricore) and
5737376 bytes after.  So it's a 0.09% decrease in size.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20130708/5a3f4566/attachment.html>