[Piglit] [PATCH 1/6] glsl-1.20 / glsl-1.30: Add tests for uniforms with initializers

Ian Romanick idr at freedesktop.org
Mon May 21 14:10:56 PDT 2012


On 05/21/2012 10:34 AM, Paul Berry wrote:
> On 16 May 2012 14:23, 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>>
>
>     These first tests simply set the value of an uniform with an
>     initializer in the shader source and verifies that the uniform has
>     that value when the shader is executed.
>
>     Signed-off-by: Ian Romanick <ian.d.romanick at intel.com
>     <mailto:ian.d.romanick at intel.com>>
>     ---
>       generated_tests/CMakeLists.txt                     |    9 +-
>       generated_tests/gen_uniform_initializer_tests.py   |  192
>     ++++++++++++++++++++
>       .../fs-initializer.template                        |   34 ++++
>       .../vs-initializer.template                        |   38 ++++
>       4 files changed, 272 insertions(+), 1 deletions(-)
>       create mode 100644 generated_tests/gen_uniform_initializer_tests.py
>       create mode 100644
>     generated_tests/uniform-initializer-templates/fs-initializer.template
>       create mode 100644
>     generated_tests/uniform-initializer-templates/vs-initializer.template
>
>     diff --git a/generated_tests/CMakeLists.txt
>     b/generated_tests/CMakeLists.txt
>     index 1b52775..84d648b 100644
>     --- a/generated_tests/CMakeLists.txt
>     +++ b/generated_tests/CMakeLists.txt
>     @@ -33,6 +33,12 @@ piglit_make_generated_tests(
>       piglit_make_generated_tests(
>             non-lvalue_tests.list
>             gen_non-lvalue_tests.py)
>     +piglit_make_generated_tests(
>     +       uniform-initializer_tests.list
>     +       gen_uniform_initializer_tests.py
>     +       uniform-initializer-templates/fs-initializer.template
>     +       uniform-initializer-templates/vs-initializer.template
>     +       )
>
>       # Add a "gen-tests" target that can be used to generate all the
>       # tests without doing any other compilation.
>     @@ -40,4 +46,5 @@ add_custom_target(gen-tests ALL
>             DEPENDS builtin_uniform_tests.list
>                     constant_array_size_tests.list
>                     interpolation_tests.list
>     -               non-lvalue_tests.list)
>     +               non-lvalue_tests.list
>     +               uniform-initializer_tests.list)
>     diff --git a/generated_tests/gen_uniform_initializer_tests.py
>     b/generated_tests/gen_uniform_initializer_tests.py
>     new file mode 100644
>     index 0000000..87d7068
>     --- /dev/null
>     +++ b/generated_tests/gen_uniform_initializer_tests.py
>     @@ -0,0 +1,192 @@
>     +# coding=utf-8
>     +#
>     +# Copyright © 2012 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 os
>     +import os.path
>     +from mako.template import Template
>     +
>     +def get_value(type, idx):
>     + """Get a string representing a number in the specified GLSL type"""
>     +
>     +    idx = idx % len(random_numbers)
>     +    value = random_numbers[idx]
>     +
>     +    if type[0] == 'b':
>     +        if (value * 10) > 5:
>     +            return "1"
>     +        else:
>     +            return "0"
>     +    elif type[0] == 'i':
>     +        return str(int(value * 100) - 50)
>     +    elif type[0] == 'u':
>     +        return str(int(value * 50))
>     +    else:
>     +        return str((value * 20.0) - 10.0)
>     +
>     +
>     +def generate_tests(type_list, base_name, major, minor):
>     +    for target in ("vs", "fs"):
>     +        for t in all_templates:
>     +            template_file_name =
>     "uniform-initializer-templates/{}-initializer{}.template".format(target,
>     t)
>     +            f = open(template_file_name)
>     +            template = f.read()
>     +            f.close()
>     +
>     +            test_file_name = dirname = os.path.join('spec',
>     + 'glsl-{}.{}'.format(major, minor),
>     + 'execution',
>     + 'uniform-initializer',
>     + '{}-{}{}.shader_test'.format(target, base_name, t))
>     +            print test_file_name
>     +
>     +            dirname = os.path.dirname(test_file_name)
>     +            if not os.path.exists(dirname):
>     +                os.makedirs(dirname)
>     +
>     +            # Generate the test vectors.  This is a list of tuples.
>       Each
>     +            # tuple is a type name paired with a value.  The value is
>     +            # formatted as a GLSL constructor.
>     +            #
>     +            # A set of types and values is also generated that can
>     be set via
>     +            # the OpenGL API.  Some of the tests use this information.
>     +            test_vectors = []
>     +            api_vectors = []
>     +            j = 0
>     +            for (type, num_values) in type_list:
>     +                numbers = []
>     +                alt_numbers = []
>     +                for i in range(num_values):
>     +                    numbers.append(get_value(type, i + j))
>     +                    alt_numbers.append(get_value(type, i + j + 7))
>     +
>     +                value = "{0}({1})".format(type, ", ".join(numbers))
>     +
>     +                api_type = type;
>     +                if type == "bool":
>     +                    api_type = "int"
>     +                elif type[0] == 'b':
>     +                    api_type = "ivec{}".format(type[-1])
>     +
>     +                if type[-1] in ["2", "3", "4"]:
>     +                    name = "".join(["u", type[0], type[-1]])
>     +                else:
>     +                    name = "".join(["u", type[0]])
>     +
>     +                test_vectors.append((type, name, value))
>     +                api_vectors.append((api_type, name, alt_numbers))
>     +                j = j + 1
>     +
>     +            f = open(test_file_name, "w")
>     +            f.write(Template(template).render(type_list = test_vectors,
>     +                                              api_types = api_vectors,
>     +                                              major = major,
>     +                                              minor = minor))
>     +            f.close()
>     +
>     +
>     +def generate_array_tests(type_list, base_name, major, minor):
>     +    for target in ("vs", "fs"):
>     +        template_file_name =
>     "uniform-initializer-templates/{}-initializer.template".format(target)
>     +        f = open(template_file_name)
>     +        template = f.read()
>     +        f.close()
>     +
>     +        test_file_name = dirname = os.path.join('spec',
>     + 'glsl-{}.{}'.format(major, minor),
>     + 'execution',
>     + 'uniform-initializer',
>     + '{}-{}-array.shader_test'.format(target, base_name))
>     +        print test_file_name
>     +
>     +       dirname = os.path.dirname(test_file_name)
>     +       if not os.path.exists(dirname):
>     +           os.makedirs(dirname)
>     +
>     +        test_vectors = []
>     +        for (type, num_values) in type_list:
>     +
>     +            constructor_parts = []
>     +            for element in range(2):
>     +                numbers = []
>     +                for i in range(num_values):
>     +                    numbers.append(get_value(type, i))
>     +
>     +                constructor_parts.append("{0}({1})".format(type, ",
>     ".join(numbers)))
>     +
>     +            if type[-1] in ["2", "3", "4"]:
>     +                name = "".join(["u", type[0], type[-1]])
>     +            else:
>     +                name = "".join(["u", type[0]])
>     +
>     +            array_type = "".join([format(type), "[2]"])
>     +            value = "{}({})".format(array_type, ",
>     ".join(constructor_parts))
>     +            test_vectors.append((array_type, name, value))
>     +
>     +        f = open(test_file_name, "w")
>     +        f.write(Template(template).render(type_list = test_vectors,
>     +                                          major = major,
>     +                                          minor = minor))
>     +        f.close()
>     +
>     +# These are a set of pseudo random values used by the number sequence
>     +# generator.  See get_value above.
>     +random_numbers = (0.78685, 0.89828, 0.36590, 0.92504, 0.48998, 0.27989,
>     +                  0.08693, 0.48144, 0.87644, 0.18080, 0.95147, 0.18892,
>     +                  0.45851, 0.76423, 0.78659, 0.97998, 0.24352, 0.60922,
>     +                  0.45241, 0.33045, 0.27233, 0.92331, 0.63593, 0.67826,
>     +                  0.12195, 0.24853, 0.35977, 0.41759, 0.79119, 0.54281,
>     +                  0.04089, 0.03877, 0.58445, 0.43017, 0.58635, 0.48151,
>     +                  0.58778, 0.37033, 0.47464, 0.17470, 0.18308, 0.49466,
>     +                  0.45838, 0.30337, 0.71273, 0.45083, 0.88339, 0.47350,
>     +                  0.86539, 0.48355, 0.92923, 0.79107, 0.77266, 0.71677,
>     +                  0.79860, 0.95149, 0.05604, 0.16863, 0.14072, 0.29028,
>     +                  0.57637, 0.13572, 0.36011, 0.65431, 0.38951, 0.73245,
>     +                  0.69497, 0.76041, 0.31016, 0.48708, 0.96677, 0.58732,
>     +                  0.33741, 0.73691, 0.24445, 0.35686, 0.72645, 0.65438,
>     +                  0.00824, 0.00923, 0.87650, 0.43315, 0.67256, 0.66939,
>     +                  0.87706, 0.73880, 0.96248, 0.24148, 0.24126, 0.24673,
>     +                  0.18999, 0.10330, 0.78826, 0.23209, 0.59548, 0.23134,
>     +                  0.72414, 0.88036, 0.54498, 0.32668, 0.02967, 0.12643)
>     +
>     +all_templates = ("",
>     +                 )
>
>
> I stumbled on your use of the empty string to specify a template with no
> suffix.  You might consider renaming the suffix-less templates to
> "{vs,fs}-initializer-direct.template" (or something similar) just to
> avoid confusion.

The problem was that I wanted the tests to show up with sensible names 
in the piglit report.  I didn't like uniform-initializer/vs-mat4-direct 
(or similar names that I could think of).  I also didn't want 
uniform-initializer/vs-mat4-initializer.  Having the empty quotes was 
annoying to me, but it felt less annoying than the alternatives.

>     +
>     +bool_types =  [("bool", 1), ("bvec2", 2), ("bvec3", 3), ("bvec4", 4)]
>     +int_types =   [("int", 1), ("ivec2", 2), ("ivec3", 3), ("ivec4", 4)]
>     +float_types = [("float", 1), ("vec2", 2), ("vec3", 3), ("vec4", 4)]
>     +uint_types =  [("uint", 1), ("uvec2", 2), ("uvec3", 3), ("uvec4", 4)]
>     +mat2_types =  [("mat2x2", 4), ("mat2x3", 6), ("mat2x4", 8)]
>     +mat3_types =  [("mat3x2", 6), ("mat3x3", 9), ("mat3x4", 12)]
>     +mat4_types =  [("mat4x2", 8), ("mat4x3", 12), ("mat4x4", 16)]
>     +
>     +for (types, base_name) in [(bool_types, "bool"),
>     +                           (int_types, "int"),
>     +                           (float_types, "float"),
>     +                           (mat2_types, "mat2"),
>     +                           (mat3_types, "mat3"),
>     +                           (mat4_types, "mat4")]:
>     +    generate_tests(types, base_name, 1, 20)
>     +    generate_array_tests(types, base_name, 1, 20)
>     +
>     +generate_tests(uint_types, "uint", 1, 30)
>     +generate_array_tests(uint_types, "uint", 1, 30)
>
>
> Breaking out the uint types as a special case seems weird.  How about:

I separated the types by GLSL version.  I expect in the future there 
will also be double and dmat types (GL_ARB_gpu_shader_fp64 / GLSL 4.00), 
and it seemed reasonable to separate those out as well.

It was also less total typing. :)

> for (types, base_name, major, minor) in [(bool_types, "bool", 1, 20),
> ... (uint_types, "uint", 1, 30)]:
>      generate_tests(types, base_name, major, minor)
>      generate_array_tests(types, base_name, major, minor)
>
>     diff --git
>     a/generated_tests/uniform-initializer-templates/fs-initializer.template
>     b/generated_tests/uniform-initializer-templates/fs-initializer.template
>     new file mode 100644
>     index 0000000..9d78e36
>     --- /dev/null
>     +++
>     b/generated_tests/uniform-initializer-templates/fs-initializer.template
>     @@ -0,0 +1,34 @@
>     +[require]
>     +GLSL >= ${"{}.{}".format(major, minor)}
>     +
>     +[vertex shader]
>     +#version ${"{}{}".format(major, minor)}
>     +
>     +void main()
>     +{
>     +  gl_Position = gl_Vertex;
>     +}
>     +
>     +[fragment shader]
>     +#version ${"{}{}".format(major, minor)}
>     +
>     +% for (type, name, value) in type_list:
>     +uniform ${type} ${name} = ${value};
>     +% endfor
>     +
>     +void main()
>     +{
>     +  if ((${type_list[0][1]} == ${type_list[0][2]})
>     +% for (type, name, value) in type_list[1:-1]:
>     + && (${name} == ${value})
>     +% endfor
>     + && (${type_list[-1][1]} == ${type_list[-1][2]})) {
>
>
> This is a lot of work to go to to ensure that the if statement will be
> formatted nicely, and it will generate an unexpectedly redundant if test
> if len(type_list) == 1.
>
> You might consider this instead:
>
>    if (${' && '.join('{0} == {1}'.format(name, value) for type, name,
> value in type_list)}) {

I like that a lot!  I really hated this code when I wrote, and I 
originally had something like your suggestion below.

> Or, alternatively, if you don't mind having "&& true" at the end of the
> if test, this might be a little less obscure:
>
>    if (
> % for type, name, value in type_list:
>        ${name} == ${value} &&
> % endfor
>        true) {
>
>
>     +    gl_FragColor = vec4(0, 1, 0, 1);
>     +  } else {
>     +    gl_FragColor = vec4(1, 0, 0, 1);
>     +  }
>     +}
>     +
>     +[test]
>     +draw rect -1 -1 2 2
>     +probe all rgb 0 1 0
>     diff --git
>     a/generated_tests/uniform-initializer-templates/vs-initializer.template
>     b/generated_tests/uniform-initializer-templates/vs-initializer.template
>     new file mode 100644
>     index 0000000..dd01cde
>     --- /dev/null
>     +++
>     b/generated_tests/uniform-initializer-templates/vs-initializer.template
>     @@ -0,0 +1,38 @@
>     +[require]
>     +GLSL >= ${"{}.{}".format(major, minor)}
>     +
>     +[vertex shader]
>     +#version ${"{}{}".format(major, minor)}
>     +varying vec4 color;
>     +
>     +% for (type, name, value) in type_list:
>     +uniform ${type} ${name} = ${value};
>     +% endfor
>     +
>     +void main()
>     +{
>     +  if ((${type_list[0][1]} == ${type_list[0][2]})
>     +% for (type, name, value) in type_list[1:-1]:
>     + && (${name} == ${value})
>     +% endfor
>     + && (${type_list[-1][1]} == ${type_list[-1][2]})) {
>     +    color = vec4(0, 1, 0, 1);
>     +  } else {
>     +    color = vec4(1, 0, 0, 1);
>     +  }
>     +
>     +  gl_Position = gl_Vertex;
>     +}
>     +
>     +[fragment shader]
>     +#version ${"{}{}".format(major, minor)}
>     +varying vec4 color;
>     +
>     +void main()
>     +{
>     +  gl_FragColor = color;
>     +}
>     +
>     +[test]
>     +draw rect -1 -1 2 2
>     +probe all rgb 0 1 0
>     --
>     1.7.6.5
>
>     _______________________________________________
>     Piglit mailing list
>     Piglit at lists.freedesktop.org <mailto:Piglit at lists.freedesktop.org>
>     http://lists.freedesktop.org/mailman/listinfo/piglit
>
>
> All of my comments are minor nit-picks, so regardless of whether you
> take my suggestions:
>
> Reviewed-by: Paul Berry <stereotype441 at gmail.com
> <mailto:stereotype441 at gmail.com>>


More information about the Piglit mailing list