[Piglit] [PATCH] glsl-1.30: Test interpolation qualifiers on built-in variables
Ian Romanick
idr at freedesktop.org
Wed Sep 4 08:51:20 PDT 2013
On 09/03/2013 08:28 AM, Paul Berry wrote:
> On 29 August 2013 09:05, Ian Romanick <idr at freedesktop.org
> <mailto:idr at freedesktop.org>> wrote:
>
> From: Ian Romanick <ian.d.romanick at intel.com
> <mailto:ian.d.romanick at intel.com>>
>
> Section 4.3.7 (Interpolation) of the GLSL 1.30 spec says:
>
> "If gl_Color is redeclared with an interpolation qualifier, then
> gl_FrontColor and gl_BackColor (if they are written to) must also be
> redeclared with the same interpolation qualifier, and vice versa. If
> gl_SecondaryColor is redeclared with an interpolation qualifier,
> then gl_FrontSecondaryColor and gl_BackSecondaryColor (if they are
> written to) must also be redeclared with the same interpolation
> qualifier, and vice versa. This qualifier matching on predeclared
> variables is only required for variables that are statically used
> within the shaders in a program."
>
> This adds five sets of tests from a single generator script:
>
> - Set an interpolation qualifier, possibly the default, for one of
> gl_FrontColor, gl_BackColor, gl_FrontSecondaryColor, or
> gl_BackSecondaryColor in the vertex shader, and set a different
> interpolation qualifier, possibly the default, for the matching
> gl_Color or gl_SecondaryColor in the fragment shader. This should
> fail to link.
>
> - Set a non-default interpolation qualifier for one of
> gl_FrontColor, gl_BackColor, gl_FrontSecondaryColor, or
> gl_BackSecondaryColor in the vertex shader, and have no
> redeclaration in the fragment shader. In the fragment shader,
> neither gl_Color nor gl_SecondaryColor is used. This should
> successfully link.
>
> - Set a non-default interpolation qualifier for one of gl_Color or
> gl_SecondaryColor in the fragment shader, and have no
> redeclaration in the vertex shader. In the vertex shader, none of
> the built-in color variables are used. This should successfully
> link.
>
> - Set an interpolation qualifier, possibly the default, for one of
> gl_FrontColor, gl_BackColor, gl_FrontSecondaryColor, or
> gl_BackSecondaryColor in the vertex shader, and set a different
> interpolation qualifier, possibly the default, for the matching
> gl_Color or gl_SecondaryColor in the fragment shader. Neither
> stage uses any of the color variables. This should successfully
> to link.
>
> - Set conflicting interpolation qualifiers for gl_FrontColor /
> gl_BackColor or gl_FrontSecondaryColor / gl_BackSecondaryColor in
> the vertex shader, and set a matching interpolation qualifier on
> gl_Color or gl_SecondaryColor in the fragment shader for one of
> the vertex shader built-in varaibles. This should fail to link.
>
>
> Do we care about testing variants where the variable is statically
> accessed in the VS but not in the FS (or vice versa)? It's not entirely
> clear to me what the spec says about these situations.
The spec does say:
"If gl_Color is redeclared with an interpolation qualifier, then
gl_FrontColor and gl_BackColor (if they are written to) must
also be redeclared with the same interpolation qualifier..."
So, I'm inferring that if there is no static write to either
gl_FrontColor or gl_BackColor in the vertex shader, then the vertex
shader does not need a redeclaration no matter what the fragment shader
does.
> On Mesa, all of the tests that expect linking failures fail. Basically,
> Mesa's linker accepts everything.
>
> NVIDIA (304.64 on GTX 260) fails the same set of tests.
>
> Signed-off-by: Ian Romanick <ian.d.romanick at intel.com
> <mailto:ian.d.romanick at intel.com>>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=47755
> ---
>
> I haven't run this on AMD's driver yet. If someone would be so kind
> as to:
>
> 1. Apply this patch.
>
> 2. make
>
> 3. Run the following bit of shell:
>
> for i in
> generated_tests/spec/glsl-1.30/linker/interpolation-qualifiers/*.shader_test
> do
> echo $i
> bin/shader_runner $i -auto -fbo
> done > output.txt
>
> 4. Email output.txt to me.
>
> Thanks!
>
> generated_tests/CMakeLists.txt | 6 +-
> .../interpolation-qualifier-built-in-variable.py | 390
> +++++++++++++++++++++
> 2 files changed, 395 insertions(+), 1 deletion(-)
> create mode 100644
> generated_tests/interpolation-qualifier-built-in-variable.py
>
> diff --git a/generated_tests/CMakeLists.txt
> b/generated_tests/CMakeLists.txt
> index a0ba4b1..a4d85dc 100644
> --- a/generated_tests/CMakeLists.txt
> +++ b/generated_tests/CMakeLists.txt
> @@ -60,6 +60,9 @@ piglit_make_generated_tests(
> piglit_make_generated_tests(
> cl_store_tests.list
> generate-cl-store-tests.py)
> +piglit_make_generated_tests(
> + interpolation-qualifier-built-in-variable.list
> + interpolation-qualifier-built-in-variable.py)
>
> # Add a "gen-tests" target that can be used to generate all the
> # tests without doing any other compilation.
> @@ -73,4 +76,5 @@ add_custom_target(gen-tests ALL
> non-lvalue_tests.list
> texture_query_lod_tests.list
> shader_bit_encoding_tests.list
> - uniform-initializer_tests.list)
> + uniform-initializer_tests.list
> + interpolation-qualifier-built-in-variable.list)
> diff --git
> a/generated_tests/interpolation-qualifier-built-in-variable.py
> b/generated_tests/interpolation-qualifier-built-in-variable.py
> new file mode 100644
> index 0000000..d03ed4d
> --- /dev/null
> +++ b/generated_tests/interpolation-qualifier-built-in-variable.py
> @@ -0,0 +1,390 @@
> +# coding=utf-8
> +#
> +# Copyright © 2013 Intel Corporation
> +#
> +# Permission is hereby granted, free of charge, to any person
> obtaining a
> +# copy of this software and associated documentation files (the
> "Software"),
> +# to deal in the Software without restriction, including without
> limitation
> +# the rights to use, copy, modify, merge, publish, distribute,
> sublicense,
> +# and/or sell copies of the Software, and to permit persons to whom the
> +# Software is furnished to do so, subject to the following conditions:
> +#
> +# The above copyright notice and this permission notice (including
> the next
> +# paragraph) shall be included in all copies or substantial
> portions of the
> +# Software.
> +#
> +# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> EXPRESS OR
> +# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> MERCHANTABILITY,
> +# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO
> EVENT SHALL
> +# THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES
> OR OTHER
> +# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
> ARISING
> +# FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
> +# DEALINGS IN THE SOFTWARE.
> +
> +import struct
> +import os
> +import os.path
> +from mako.template import Template
> +from textwrap import dedent
> +
> +interpolation_modes = {
> + 'flat',
> + 'noperspective',
> + 'smooth',
> + 'default'
> +}
>
>
> This uses Python's "set literal" syntax, which was introduced in Python
> 2.7. Piglit only requires 2.6, so I think we should change this to a list:
>
> interpolation_modes = [
> 'flat',
> 'noperspective',
> 'smooth',
> 'default'
> ]
>
> Same with vertex_shader_variables and vertex_shader_variables_front_only.
Ah... okay. I'll fix that.
> All of my other comments (below) are minor, so with this fixed, the
> patch is:
>
> Reviewed-by: Paul Berry <stereotype441 at gmail.com
> <mailto:stereotype441 at gmail.com>>
>
>
> +
> +vertex_shader_variables = {
> + 'gl_FrontColor',
> + 'gl_BackColor',
> + 'gl_FrontSecondaryColor',
> + 'gl_BackSecondaryColor'
> +}
> +
> +vertex_shader_variables_front_only = {
> + 'gl_FrontColor',
> + 'gl_FrontSecondaryColor',
> +}
> +
> +vertex_shader_variables_other_side = {
> + 'gl_FrontColor': 'gl_BackColor',
> + 'gl_BackColor': 'gl_FrontColor',
> + 'gl_FrontSecondaryColor': 'gl_BackSecondaryColor',
> + 'gl_BackSecondaryColor': 'gl_FrontSecondaryColor'
> +}
> +
> +fragment_shader_variables = {
> + 'gl_FrontColor': 'gl_Color',
> + 'gl_BackColor': 'gl_Color',
> + 'gl_FrontSecondaryColor': 'gl_SecondaryColor',
> + 'gl_BackSecondaryColor': 'gl_SecondaryColor'
> +}
>
>
> Minor quibble: vertex_shader_variables_other_side and
> fragment_shader_variables sound like names of lists, when they're in
> fact mappings from one variable name to another. Maybe rename to
> something like "other_side_map" and "vs_to_fs_map"?
That's fair.
> +
> +template = Template(dedent("""\
> + # Section 4.3.7 (Interpolation) of the GLSL 1.30 spec says:
> + #
> + # "If gl_Color is redeclared with an interpolation
> qualifier, then
> + # gl_FrontColor and gl_BackColor (if they are written to) must
> + # also be redeclared with the same interpolation qualifier, and
> + # vice versa. If gl_SecondaryColor is redeclared with an
> + # interpolation qualifier, then gl_FrontSecondaryColor and
> + # gl_BackSecondaryColor (if they are written to) must also be
> + # redeclared with the same interpolation qualifier, and vice
> + # versa. This qualifier matching on predeclared variables
> is only
> + # required for variables that are statically used within the
> + # shaders in a program."
> + #
> + # Even though some of the other rules for interpolation qualifier
> + # matching changed in 4.x specifications, this rule has
> remained the
> + # same.
> + [require]
> + GLSL >= 1.30
> +
> + [vertex shader]
> + % if vs_mode != 'default':
> + ${vs_mode} out vec4 ${vs_variable};
> + % endif
> + void main() { gl_Position = vec4(0); ${vs_variable} = vec4(0); }
> +
> + [fragment shader]
> + % if fs_mode != 'default':
> + ${fs_mode} in vec4 ${fs_variable};
> + % endif
> + out vec4 c;
> + void main() { c = ${fs_variable}; }
> +
> + [test]
> + link error
> + """))
> +
> +for fs_mode in interpolation_modes:
> + for vs_mode in interpolation_modes:
> + if vs_mode == fs_mode:
> + continue
> +
> + for var in vertex_shader_variables:
> + filename = os.path.join('spec',
> + 'glsl-1.30',
> + 'linker',
> + 'interpolation-qualifiers',
> +
> '{0}-{1}-{2}-{3}.shader_test'.format(vs_mode,
> +
> var,
> +
> fs_mode,
> +
> fragment_shader_variables[var]))
> + print filename
> +
> + dirname = os.path.dirname(filename)
> + if not os.path.exists(dirname):
> + os.makedirs(dirname)
> +
> + f = open(filename, 'w')
> + f.write(template.render(vs_mode=vs_mode,
> + vs_variable=var,
> + fs_mode=fs_mode,
> +
> fs_variable=fragment_shader_variables[var]))
> + f.close()
> +
> +template = Template(dedent("""\
> + # Section 4.3.7 (Interpolation) of the GLSL 1.30 spec says:
> + #
> + # "If gl_Color is redeclared with an interpolation
> qualifier, then
> + # gl_FrontColor and gl_BackColor (if they are written to) must
> + # also be redeclared with the same interpolation qualifier, and
> + # vice versa. If gl_SecondaryColor is redeclared with an
> + # interpolation qualifier, then gl_FrontSecondaryColor and
> + # gl_BackSecondaryColor (if they are written to) must also be
> + # redeclared with the same interpolation qualifier, and vice
> + # versa. This qualifier matching on predeclared variables
> is only
> + # required for variables that are statically used within the
> + # shaders in a program."
> + #
> + # Even though some of the other rules for interpolation qualifier
> + # matching changed in 4.x specifications, this rule has
> remained the
> + # same.
> + #
> + # We interpret the sentence "variables that are statically used
> within the
> + # shaders in a program" to mean static use of the variable in a
> shader stage
> + # invokes the redeclaration requirement for that stage only.
> This is based on
> + # the additional text "..gl_FrontColor and gl_BackColor (if
> they are written
> + # to) must also be redeclared with the same interpolation
> qualifier..."
> + [require]
> + GLSL >= 1.30
> +
> + [vertex shader]
> + ${vs_mode} out vec4 ${vs_variable};
> + void main() { gl_Position = vec4(0); ${vs_variable} = vec4(0); }
> +
> + [fragment shader]
> + out vec4 c;
> + void main() { c = vec4(0); }
> +
> + [test]
> + link success
> + """))
> +
> +for vs_mode in interpolation_modes:
> + if vs_mode == 'default':
> + continue
> +
> + for var in vertex_shader_variables:
> + filename = os.path.join('spec',
> + 'glsl-1.30',
> + 'linker',
> + 'interpolation-qualifiers',
> +
> '{0}-{1}-unused-{2}.shader_test'.format(vs_mode,
> +
> var,
> +
> fragment_shader_variables[var]))
> + print filename
> +
> + dirname = os.path.dirname(filename)
> + if not os.path.exists(dirname):
> + os.makedirs(dirname)
> +
> + f = open(filename, 'w')
> + f.write(template.render(vs_mode=vs_mode,
> + vs_variable=var))
> + f.close()
> +
> +template = Template(dedent("""\
> + # Section 4.3.7 (Interpolation) of the GLSL 1.30 spec says:
> + #
> + # "If gl_Color is redeclared with an interpolation
> qualifier, then
> + # gl_FrontColor and gl_BackColor (if they are written to) must
> + # also be redeclared with the same interpolation qualifier, and
> + # vice versa. If gl_SecondaryColor is redeclared with an
> + # interpolation qualifier, then gl_FrontSecondaryColor and
> + # gl_BackSecondaryColor (if they are written to) must also be
> + # redeclared with the same interpolation qualifier, and vice
> + # versa. This qualifier matching on predeclared variables
> is only
> + # required for variables that are statically used within the
> + # shaders in a program."
> + #
> + # Even though some of the other rules for interpolation qualifier
> + # matching changed in 4.x specifications, this rule has
> remained the
> + # same.
> + #
> + # We interpret the sentence "variables that are statically used
> within the
> + # shaders in a program" to mean static use of the variable in a
> shader stage
> + # invokes the redeclaration requirement for that stage only.
> This is based on
> + # the additional text "..gl_FrontColor and gl_BackColor (if
> they are written
> + # to) must also be redeclared with the same interpolation
> qualifier..."
> + [require]
> + GLSL >= 1.30
> +
> + [vertex shader]
> + void main() { gl_Position = vec4(0); }
> +
> + [fragment shader]
> + ${fs_mode} in vec4 ${fs_variable};
> + out vec4 c;
> + void main() { c = ${fs_variable}; }
> +
> + [test]
> + link success
> + """))
> +
> +for fs_mode in interpolation_modes:
> + if fs_mode == 'default':
> + continue
> +
> + for var in vertex_shader_variables_front_only:
> + filename = os.path.join('spec',
> + 'glsl-1.30',
> + 'linker',
> + 'interpolation-qualifiers',
> +
> 'unused-{0}-{1}-{2}.shader_test'.format(var,
> +
> fs_mode,
> +
> fragment_shader_variables[var]))
> + print filename
> +
> + dirname = os.path.dirname(filename)
> + if not os.path.exists(dirname):
> + os.makedirs(dirname)
> +
> + f = open(filename, 'w')
> + f.write(template.render(vs_mode=vs_mode,
> + vs_variable=var,
> + fs_mode=fs_mode,
> +
> fs_variable=fragment_shader_variables[var]))
> + f.close()
> +
> +
> +template = Template(dedent("""\
> + # Section 4.3.7 (Interpolation) of the GLSL 1.30 spec says:
> + #
> + # "If gl_Color is redeclared with an interpolation
> qualifier, then
> + # gl_FrontColor and gl_BackColor (if they are written to) must
> + # also be redeclared with the same interpolation qualifier, and
> + # vice versa. If gl_SecondaryColor is redeclared with an
> + # interpolation qualifier, then gl_FrontSecondaryColor and
> + # gl_BackSecondaryColor (if they are written to) must also be
> + # redeclared with the same interpolation qualifier, and vice
> + # versa. This qualifier matching on predeclared variables
> is only
> + # required for variables that are statically used within the
> + # shaders in a program."
> + #
> + # Even though some of the other rules for interpolation qualifier
> + # matching changed in 4.x specifications, this rule has
> remained the
> + # same.
> + #
> + # We interpret the sentence "variables that are statically used
> within the
> + # shaders in a program" to mean static use of the variable in
> any shader in
> + # the program invokes the redeclaration requirement. Since
> neither shader
> + # accesses any built-in variables, linking should succeed no
> matter what the
> + # interpolation qualifiers say.
> + [require]
> + GLSL >= 1.30
> +
> + [vertex shader]
> + % if vs_mode != 'default':
> + ${vs_mode} out vec4 ${vs_variable};
> + % endif
> + void main() { gl_Position = vec4(0); }
> +
> + [fragment shader]
> + % if fs_mode != 'default':
> + ${fs_mode} in vec4 ${fs_variable};
> + % endif
> + out vec4 c;
> + void main() { c = vec4(0); }
> +
> + [test]
> + link success
> + """))
> +
> +for fs_mode in interpolation_modes:
> + for vs_mode in interpolation_modes:
> + if vs_mode == fs_mode:
> + continue
> +
> + for var in vertex_shader_variables:
> + filename = os.path.join('spec',
> + 'glsl-1.30',
> + 'linker',
> + 'interpolation-qualifiers',
> +
> 'unused-{0}-{1}-unused-{2}-{3}.shader_test'.format(vs_mode,
> +
> var,
> +
> fs_mode,
> +
> fragment_shader_variables[var]))
> + print filename
> +
> + dirname = os.path.dirname(filename)
> + if not os.path.exists(dirname):
> + os.makedirs(dirname)
> +
> + f = open(filename, 'w')
> + f.write(template.render(vs_mode=vs_mode,
> + vs_variable=var,
> + fs_mode=fs_mode,
> +
> fs_variable=fragment_shader_variables[var]))
> + f.close()
> +
> +
> +template = Template(dedent("""\
> + # Section 4.3.7 (Interpolation) of the GLSL 1.30 spec says:
> + #
> + # "If gl_Color is redeclared with an interpolation
> qualifier, then
> + # gl_FrontColor and gl_BackColor (if they are written to) must
> + # also be redeclared with the same interpolation qualifier, and
> + # vice versa. If gl_SecondaryColor is redeclared with an
> + # interpolation qualifier, then gl_FrontSecondaryColor and
> + # gl_BackSecondaryColor (if they are written to) must also be
> + # redeclared with the same interpolation qualifier, and vice
> + # versa. This qualifier matching on predeclared variables
> is only
> + # required for variables that are statically used within the
> + # shaders in a program."
> + #
> + # Even though some of the other rules for interpolation qualifier
> + # matching changed in 4.x specifications, this rule has
> remained the
> + # same.
> + [require]
> + GLSL >= 1.30
> +
> + [vertex shader]
> + % if fs_mode != 'default':
> + ${fs_mode} out vec4 ${front_variable};
> + % endif
> + % if vs_mode != 'default':
> + ${vs_mode} out vec4 ${back_variable};
>
>
> It seems a little misleading to call these "front_variable" and
> "back_variable" when in fact sometimes the former is the back variable
> and the latter is the front variable. How about just "vs_variable_1"
> and "vs_variable_2"?
Yeah... how do this_side_variable and other_side_variable sound?
> + % endif
> + void main() { gl_Position = vec4(0); ${front_variable} =
> vec4(0); ${back_variable} = vec4(1); }
> +
> + [fragment shader]
> + % if fs_mode != 'default':
> + ${fs_mode} in vec4 ${fs_variable};
> + % endif
> + out vec4 c;
> + void main() { c = ${fs_variable}; }
> +
> + [test]
> + link error
> + """))
> +
> +for fs_mode in interpolation_modes:
> + for vs_mode in interpolation_modes:
> + if vs_mode == fs_mode:
> + continue
> +
> + for var in vertex_shader_variables:
> + var_back = vertex_shader_variables_other_side[var]
>
>
> Similar comment here: "var_back" is a misleading name.
var => this_side, var_back => other_side?
> + filename = os.path.join('spec',
> + 'glsl-1.30',
> + 'linker',
> + 'interpolation-qualifiers',
> +
> '{0}-{1}-{2}-{3}.shader_test'.format(vs_mode,
> +
> var,
> +
> fs_mode,
> +
> var_back))
> + print filename
> +
> + dirname = os.path.dirname(filename)
> + if not os.path.exists(dirname):
> + os.makedirs(dirname)
> +
> + f = open(filename, 'w')
> + f.write(template.render(vs_mode=vs_mode,
> + front_variable=var,
> + back_variable=var_back,
> + fs_mode=fs_mode,
> +
> fs_variable=fragment_shader_variables[var]))
> + f.close()
> --
> 1.8.1.4
>
> _______________________________________________
> Piglit mailing list
> Piglit at lists.freedesktop.org <mailto:Piglit at lists.freedesktop.org>
> http://lists.freedesktop.org/mailman/listinfo/piglit
More information about the Piglit
mailing list