<div dir="ltr">On 29 August 2013 09:05, Ian Romanick <<a href="mailto:idr@freedesktop.org" target="_blank">idr@freedesktop.org</a>> wrote: <div class="gmail_extra"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> From: Ian Romanick <<a href="mailto:ian.d.romanick@intel.com">ian.d.romanick@intel.com</a>> 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. </blockquote><div> </div>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. <div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> 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 <<a href="mailto:ian.d.romanick@intel.com">ian.d.romanick@intel.com</a>> Bugzilla: <a href="https://bugs.freedesktop.org/show_bug.cgi?id=47755" target="_blank">https://bugs.freedesktop.org/show_bug.cgi?id=47755</a> --- 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' +} </blockquote><div> </div><div>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: </div><div>interpolation_modes = [ </div><div> 'flat', </div><div> 'noperspective', </div><div> 'smooth', </div><div> 'default' ] </div><div>Same with vertex_shader_variables and vertex_shader_variables_front_only. </div><div>All of my other comments (below) are minor, so with this fixed, the patch is: </div><div>Reviewed-by: Paul Berry <<a href="mailto:stereotype441@gmail.com">stereotype441@gmail.com</a>> </div> <div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> + +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' +} </blockquote><div> </div><div>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"? </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> + +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}; </blockquote><div> </div><div>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"? </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> + % 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] </blockquote><div> </div><div>Similar comment here: "var_back" is a misleading name. </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> + 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 <a href="mailto:Piglit@lists.freedesktop.org">Piglit@lists.freedesktop.org</a> <a href="http://lists.freedesktop.org/mailman/listinfo/piglit" target="_blank">http://lists.freedesktop.org/mailman/listinfo/piglit</a> </blockquote></div> </div></div>