[prev in list] [next in list] [prev in thread] [next in thread]
List: mesa3d-dev
Subject: Re: [Mesa-dev] [PATCH RFC 3/3] glsl: Rework builtin_variables.cpp to reduce code duplication.
From: Paul Berry <stereotype441 () gmail ! com>
Date: 2013-07-08 21:32:22
Message-ID: CA+yLL66VS1LNnzyPGGXVR_p1aUE8okJM6-Wbao0G43S8Cama0g () mail ! gmail ! com
[Download RAW message or body]
[Attachment #2 (multipart/alternative)]
On 8 July 2013 10:40, Paul Berry <stereotype441@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.
[Attachment #5 (text/html)]
<div dir="ltr">On 8 July 2013 10:40, Paul Berry <span dir="ltr"><<a \
href="mailto:stereotype441@gmail.com" \
target="_blank">stereotype441@gmail.com</a>></span> wrote:<br><div \
class="gmail_extra"><div class="gmail_quote"> <blockquote class="gmail_quote" \
style="margin:0px 0px 0px 0.8ex;border-left:1px solid \
rgb(204,204,204);padding-left:1ex">Previously, we had a separate function for setting \
up the built-in<br> variables for each combination of shader stage and GLSL \
version<br> (e.g. generate_110_vs_variables to generate the built-in variables \
for<br> GLSL 1.10 vertex shaders). The functions called each other in ad-hoc<br>
ways, leading to unexpected inconsistencies (for example,<br>
generate_120_fs_variables was called for GLSL versions 1.20 and above,<br>
but generate_130_fs_variables was called only for GLSL version 1.30).<br>
In addition, it led to a lot of code duplication, since many varyings<br>
had to be duplicated in both the FS and VS code paths. With the<br>
advent of geometry shaders (and later, tessellation control and<br>
tessellation evaluation shaders), this code duplication was going to<br>
get a lot worse.<br>
<br>
So this patch reworks things so that instead of having a separate<br>
function for each shader type and GLSL version, we have a function for<br>
constants, one for uniforms, one for varyings, and one for the special<br>
variables that are specific to each shader type.<br>
<br>
In addition, we use a class, builtin_variable_generator, to keep track<br>
of the instruction exec_list, the GLSL parse state, commonly-used<br>
types, and a few other variables, so that we don't have to pass them<br>
around as function arguments. This makes the code a lot more compact.<br>
<br>
Where it was feasible to do so without introducing compilation errors,<br>
I've also gone ahead and introduced the variables needed for<br>
{ARB,EXT}_geometry_shader4 style geometry shaders. This patch takes<br>
care of everything except the GS variable gl_VerticesIn, the FS<br>
variable gl_PrimitiveID, and GLSL 1.50 style geometry shader inputs<br>
(using the gl_in interface block). Those remaining features will be<br>
added later.<br>
<br>
I've also made a slight nomenclature change: previously we used the<br>
word "deprecated" to refer to variables which are marked in GLSL 1.40<br>
as requiring the ARB_compatibility extension, and are marked in GLSL<br>
1.50 onward as requiring the compatibilty profile. This was<br>
misleading, since not all deprecated variables require the<br>
compatibility profile (for example gl_FragData and gl_FragColor, which<br>
have been deprecated since GLSL 1.30, but do not require the<br>
compatibility profile until GLSL 4.20). We now consistently use the<br>
word "compatibility" to refer to these variables.<br>
<br>
This patch doesn't introduce any functional changes (since geometry<br>
shaders haven't been enabled yet).<br>
---<br>
src/glsl/builtin_variables.cpp | 1124 +++++++++++++---------------------------<br>
1 file changed, 363 insertions(+), 761 \
deletions(-)<br></blockquote><div><br></div><div>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.<br> </div></div></div></div>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic