[Piglit] [PATCH] glsl-1.30: Test interpolation qualifiers on built-in variables

Paul Berry stereotype441 at gmail.com
Tue Sep 3 08:28:51 PDT 2013


On 29 August 2013 09:05, Ian Romanick <idr at freedesktop.org> wrote:

> From: Ian Romanick <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.


>
> 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>
> 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.

All of my other comments (below) are minor, so with this fixed, the patch
is:

Reviewed-by: Paul Berry <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"?


> +
> +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"?


> +    % 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.


> +            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
> http://lists.freedesktop.org/mailman/listinfo/piglit
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/piglit/attachments/20130903/52859c0b/attachment-0001.html>


More information about the Piglit mailing list