On 21 May 2012 14:10, Ian Romanick <span dir="ltr">&lt;<a href="mailto:idr@freedesktop.org" target="_blank">idr@freedesktop.org</a>&gt;</span> wrote:<br><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="im">On 05/21/2012 10:34 AM, Paul Berry wrote:<br>
</div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="im">
On 16 May 2012 14:23, Ian Romanick &lt;<a href="mailto:idr@freedesktop.org" target="_blank">idr@freedesktop.org</a><br></div><div class="im">
&lt;mailto:<a href="mailto:idr@freedesktop.org" target="_blank">idr@freedesktop.org</a>&gt;&gt; wrote:<br>
<br>
    From: Ian Romanick &lt;<a href="mailto:ian.d.romanick@intel.com" target="_blank">ian.d.romanick@intel.com</a><br></div>
    &lt;mailto:<a href="mailto:ian.d.romanick@intel.com" target="_blank">ian.d.romanick@intel.<u></u>com</a>&gt;&gt;<div class="im"><br>
<br>
    These first tests simply set the value of an uniform with an<br>
    initializer in the shader source and verifies that the uniform has<br>
    that value when the shader is executed.<br>
<br>
    Signed-off-by: Ian Romanick &lt;<a href="mailto:ian.d.romanick@intel.com" target="_blank">ian.d.romanick@intel.com</a><br></div>
    &lt;mailto:<a href="mailto:ian.d.romanick@intel.com" target="_blank">ian.d.romanick@intel.<u></u>com</a>&gt;&gt;</blockquote></blockquote><div><br>Whoops, I failed to notice when I reviewed this patch that it doesn&#39;t work properly with out-of-tree builds, since the Python script attempts to load template files relative to the cwd (which is in the build tree, not the source tree).  I&#39;ll commit a fix shortly.<br>
 </div><blockquote class="gmail_quote" style="margin:0pt 0pt 0pt 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><blockquote class="gmail_quote" style="margin:0pt 0pt 0pt 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<div><div class="h5"><br>
    ---<br>
      generated_tests/CMakeLists.txt                     |    9 +-<br>
      generated_tests/gen_uniform_<u></u>initializer_tests.py   |  192<br>
    ++++++++++++++++++++<br>
      .../fs-initializer.template                        |   34 ++++<br>
      .../vs-initializer.template                        |   38 ++++<br>
      4 files changed, 272 insertions(+), 1 deletions(-)<br>
      create mode 100644 generated_tests/gen_uniform_<u></u>initializer_tests.py<br>
      create mode 100644<br>
    generated_tests/uniform-<u></u>initializer-templates/fs-<u></u>initializer.template<br>
      create mode 100644<br>
    generated_tests/uniform-<u></u>initializer-templates/vs-<u></u>initializer.template<br>
<br>
    diff --git a/generated_tests/CMakeLists.<u></u>txt<br>
    b/generated_tests/CMakeLists.<u></u>txt<br>
    index 1b52775..84d648b 100644<br>
    --- a/generated_tests/CMakeLists.<u></u>txt<br>
    +++ b/generated_tests/CMakeLists.<u></u>txt<br>
    @@ -33,6 +33,12 @@ piglit_make_generated_tests(<br>
      piglit_make_generated_tests(<br>
            non-lvalue_tests.list<br>
            gen_non-lvalue_tests.py)<br>
    +piglit_make_generated_tests(<br>
    +       uniform-initializer_tests.list<br>
    +       gen_uniform_initializer_tests.<u></u>py<br>
    +       uniform-initializer-templates/<u></u>fs-initializer.template<br>
    +       uniform-initializer-templates/<u></u>vs-initializer.template<br>
    +       )<br>
<br>
      # Add a &quot;gen-tests&quot; target that can be used to generate all the<br>
      # tests without doing any other compilation.<br>
    @@ -40,4 +46,5 @@ add_custom_target(gen-tests ALL<br>
            DEPENDS builtin_uniform_tests.list<br>
                    constant_array_size_tests.list<br>
                    interpolation_tests.list<br>
    -               non-lvalue_tests.list)<br>
    +               non-lvalue_tests.list<br>
    +               uniform-initializer_tests.<u></u>list)<br>
    diff --git a/generated_tests/gen_uniform_<u></u>initializer_tests.py<br>
    b/generated_tests/gen_uniform_<u></u>initializer_tests.py<br>
    new file mode 100644<br>
    index 0000000..87d7068<br>
    --- /dev/null<br>
    +++ b/generated_tests/gen_uniform_<u></u>initializer_tests.py<br>
    @@ -0,0 +1,192 @@<br>
    +# coding=utf-8<br>
    +#<br>
    +# Copyright © 2012 Intel Corporation<br>
    +#<br>
    +# Permission is hereby granted, free of charge, to any person<br>
    obtaining a<br>
    +# copy of this software and associated documentation files (the<br>
    &quot;Software&quot;),<br>
    +# to deal in the Software without restriction, including without<br>
    limitation<br>
    +# the rights to use, copy, modify, merge, publish, distribute,<br>
    sublicense,<br>
    +# and/or sell copies of the Software, and to permit persons to whom the<br>
    +# Software is furnished to do so, subject to the following conditions:<br>
    +#<br>
    +# The above copyright notice and this permission notice (including<br>
    the next<br>
    +# paragraph) shall be included in all copies or substantial<br>
    portions of the<br>
    +# Software.<br>
    +#<br>
    +# THE SOFTWARE IS PROVIDED &quot;AS IS&quot;, WITHOUT WARRANTY OF ANY KIND,<br>
    EXPRESS OR<br>
    +# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF<br>
    MERCHANTABILITY,<br>
    +# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO<br>
    EVENT SHALL<br>
    +# THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES<br>
    OR OTHER<br>
    +# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,<br>
    ARISING<br>
    +# FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER<br>
    +# DEALINGS IN THE SOFTWARE.<br>
    +<br>
    +import os<br>
    +import os.path<br>
    +from mako.template import Template<br>
    +<br>
    +def get_value(type, idx):<br>
    + &quot;&quot;&quot;Get a string representing a number in the specified GLSL type&quot;&quot;&quot;<br>
    +<br>
    +    idx = idx % len(random_numbers)<br>
    +    value = random_numbers[idx]<br>
    +<br>
    +    if type[0] == &#39;b&#39;:<br>
    +        if (value * 10) &gt; 5:<br>
    +            return &quot;1&quot;<br>
    +        else:<br>
    +            return &quot;0&quot;<br>
    +    elif type[0] == &#39;i&#39;:<br>
    +        return str(int(value * 100) - 50)<br>
    +    elif type[0] == &#39;u&#39;:<br>
    +        return str(int(value * 50))<br>
    +    else:<br>
    +        return str((value * 20.0) - 10.0)<br>
    +<br>
    +<br>
    +def generate_tests(type_list, base_name, major, minor):<br>
    +    for target in (&quot;vs&quot;, &quot;fs&quot;):<br>
    +        for t in all_templates:<br>
    +            template_file_name =<br>
    &quot;uniform-initializer-<u></u>templates/{}-initializer{}.<u></u>template&quot;.format(target,<br>
    t)<br>
    +            f = open(template_file_name)<br>
    +            template = f.read()<br>
    +            f.close()<br>
    +<br>
    +            test_file_name = dirname = os.path.join(&#39;spec&#39;,<br>
    + &#39;glsl-{}.{}&#39;.format(major, minor),<br>
    + &#39;execution&#39;,<br>
    + &#39;uniform-initializer&#39;,<br>
    + &#39;{}-{}{}.shader_test&#39;.format(<u></u>target, base_name, t))<br>
    +            print test_file_name<br>
    +<br>
    +            dirname = os.path.dirname(test_file_<u></u>name)<br>
    +            if not os.path.exists(dirname):<br>
    +                os.makedirs(dirname)<br>
    +<br>
    +            # Generate the test vectors.  This is a list of tuples.<br>
      Each<br>
    +            # tuple is a type name paired with a value.  The value is<br>
    +            # formatted as a GLSL constructor.<br>
    +            #<br>
    +            # A set of types and values is also generated that can<br>
    be set via<br>
    +            # the OpenGL API.  Some of the tests use this information.<br>
    +            test_vectors = []<br>
    +            api_vectors = []<br>
    +            j = 0<br>
    +            for (type, num_values) in type_list:<br>
    +                numbers = []<br>
    +                alt_numbers = []<br>
    +                for i in range(num_values):<br>
    +                    numbers.append(get_value(type, i + j))<br>
    +                    alt_numbers.append(get_value(<u></u>type, i + j + 7))<br>
    +<br>
    +                value = &quot;{0}({1})&quot;.format(type, &quot;, &quot;.join(numbers))<br>
    +<br>
    +                api_type = type;<br>
    +                if type == &quot;bool&quot;:<br>
    +                    api_type = &quot;int&quot;<br>
    +                elif type[0] == &#39;b&#39;:<br>
    +                    api_type = &quot;ivec{}&quot;.format(type[-1])<br>
    +<br>
    +                if type[-1] in [&quot;2&quot;, &quot;3&quot;, &quot;4&quot;]:<br>
    +                    name = &quot;&quot;.join([&quot;u&quot;, type[0], type[-1]])<br>
    +                else:<br>
    +                    name = &quot;&quot;.join([&quot;u&quot;, type[0]])<br>
    +<br>
    +                test_vectors.append((type, name, value))<br>
    +                api_vectors.append((api_type, name, alt_numbers))<br>
    +                j = j + 1<br>
    +<br>
    +            f = open(test_file_name, &quot;w&quot;)<br>
    +            f.write(Template(template).<u></u>render(type_list = test_vectors,<br>
    +                                              api_types = api_vectors,<br>
    +                                              major = major,<br>
    +                                              minor = minor))<br>
    +            f.close()<br>
    +<br>
    +<br>
    +def generate_array_tests(type_<u></u>list, base_name, major, minor):<br>
    +    for target in (&quot;vs&quot;, &quot;fs&quot;):<br>
    +        template_file_name =<br>
    &quot;uniform-initializer-<u></u>templates/{}-initializer.<u></u>template&quot;.format(target)<br>
    +        f = open(template_file_name)<br>
    +        template = f.read()<br>
    +        f.close()<br>
    +<br>
    +        test_file_name = dirname = os.path.join(&#39;spec&#39;,<br>
    + &#39;glsl-{}.{}&#39;.format(major, minor),<br>
    + &#39;execution&#39;,<br>
    + &#39;uniform-initializer&#39;,<br>
    + &#39;{}-{}-array.shader_test&#39;.<u></u>format(target, base_name))<br>
    +        print test_file_name<br>
    +<br>
    +       dirname = os.path.dirname(test_file_<u></u>name)<br>
    +       if not os.path.exists(dirname):<br>
    +           os.makedirs(dirname)<br>
    +<br>
    +        test_vectors = []<br>
    +        for (type, num_values) in type_list:<br>
    +<br>
    +            constructor_parts = []<br>
    +            for element in range(2):<br>
    +                numbers = []<br>
    +                for i in range(num_values):<br>
    +                    numbers.append(get_value(type, i))<br>
    +<br>
    +                constructor_parts.append(&quot;{0}(<u></u>{1})&quot;.format(type, &quot;,<br>
    &quot;.join(numbers)))<br>
    +<br>
    +            if type[-1] in [&quot;2&quot;, &quot;3&quot;, &quot;4&quot;]:<br>
    +                name = &quot;&quot;.join([&quot;u&quot;, type[0], type[-1]])<br>
    +            else:<br>
    +                name = &quot;&quot;.join([&quot;u&quot;, type[0]])<br>
    +<br>
    +            array_type = &quot;&quot;.join([format(type), &quot;[2]&quot;])<br>
    +            value = &quot;{}({})&quot;.format(array_type, &quot;,<br>
    &quot;.join(constructor_parts))<br>
    +            test_vectors.append((array_<u></u>type, name, value))<br>
    +<br>
    +        f = open(test_file_name, &quot;w&quot;)<br>
    +        f.write(Template(template).<u></u>render(type_list = test_vectors,<br>
    +                                          major = major,<br>
    +                                          minor = minor))<br>
    +        f.close()<br>
    +<br>
    +# These are a set of pseudo random values used by the number sequence<br>
    +# generator.  See get_value above.<br>
    +random_numbers = (0.78685, 0.89828, 0.36590, 0.92504, 0.48998, 0.27989,<br>
    +                  0.08693, 0.48144, 0.87644, 0.18080, 0.95147, 0.18892,<br>
    +                  0.45851, 0.76423, 0.78659, 0.97998, 0.24352, 0.60922,<br>
    +                  0.45241, 0.33045, 0.27233, 0.92331, 0.63593, 0.67826,<br>
    +                  0.12195, 0.24853, 0.35977, 0.41759, 0.79119, 0.54281,<br>
    +                  0.04089, 0.03877, 0.58445, 0.43017, 0.58635, 0.48151,<br>
    +                  0.58778, 0.37033, 0.47464, 0.17470, 0.18308, 0.49466,<br>
    +                  0.45838, 0.30337, 0.71273, 0.45083, 0.88339, 0.47350,<br>
    +                  0.86539, 0.48355, 0.92923, 0.79107, 0.77266, 0.71677,<br>
    +                  0.79860, 0.95149, 0.05604, 0.16863, 0.14072, 0.29028,<br>
    +                  0.57637, 0.13572, 0.36011, 0.65431, 0.38951, 0.73245,<br>
    +                  0.69497, 0.76041, 0.31016, 0.48708, 0.96677, 0.58732,<br>
    +                  0.33741, 0.73691, 0.24445, 0.35686, 0.72645, 0.65438,<br>
    +                  0.00824, 0.00923, 0.87650, 0.43315, 0.67256, 0.66939,<br>
    +                  0.87706, 0.73880, 0.96248, 0.24148, 0.24126, 0.24673,<br>
    +                  0.18999, 0.10330, 0.78826, 0.23209, 0.59548, 0.23134,<br>
    +                  0.72414, 0.88036, 0.54498, 0.32668, 0.02967, 0.12643)<br>
    +<br>
    +all_templates = (&quot;&quot;,<br>
    +                 )<br>
<br>
<br>
I stumbled on your use of the empty string to specify a template with no<br>
suffix.  You might consider renaming the suffix-less templates to<br>
&quot;{vs,fs}-initializer-direct.<u></u>template&quot; (or something similar) just to<br>
avoid confusion.<br>
</div></div></blockquote>
<br>
The problem was that I wanted the tests to show up with sensible names in the piglit report.  I didn&#39;t like uniform-initializer/vs-mat4-<u></u>direct (or similar names that I could think of).  I also didn&#39;t want uniform-initializer/vs-mat4-<u></u>initializer.  Having the empty quotes was annoying to me, but it felt less annoying than the alternatives.<div class="im">
<br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
    +<br>
    +bool_types =  [(&quot;bool&quot;, 1), (&quot;bvec2&quot;, 2), (&quot;bvec3&quot;, 3), (&quot;bvec4&quot;, 4)]<br>
    +int_types =   [(&quot;int&quot;, 1), (&quot;ivec2&quot;, 2), (&quot;ivec3&quot;, 3), (&quot;ivec4&quot;, 4)]<br>
    +float_types = [(&quot;float&quot;, 1), (&quot;vec2&quot;, 2), (&quot;vec3&quot;, 3), (&quot;vec4&quot;, 4)]<br>
    +uint_types =  [(&quot;uint&quot;, 1), (&quot;uvec2&quot;, 2), (&quot;uvec3&quot;, 3), (&quot;uvec4&quot;, 4)]<br>
    +mat2_types =  [(&quot;mat2x2&quot;, 4), (&quot;mat2x3&quot;, 6), (&quot;mat2x4&quot;, 8)]<br>
    +mat3_types =  [(&quot;mat3x2&quot;, 6), (&quot;mat3x3&quot;, 9), (&quot;mat3x4&quot;, 12)]<br>
    +mat4_types =  [(&quot;mat4x2&quot;, 8), (&quot;mat4x3&quot;, 12), (&quot;mat4x4&quot;, 16)]<br>
    +<br>
    +for (types, base_name) in [(bool_types, &quot;bool&quot;),<br>
    +                           (int_types, &quot;int&quot;),<br>
    +                           (float_types, &quot;float&quot;),<br>
    +                           (mat2_types, &quot;mat2&quot;),<br>
    +                           (mat3_types, &quot;mat3&quot;),<br>
    +                           (mat4_types, &quot;mat4&quot;)]:<br>
    +    generate_tests(types, base_name, 1, 20)<br>
    +    generate_array_tests(types, base_name, 1, 20)<br>
    +<br>
    +generate_tests(uint_types, &quot;uint&quot;, 1, 30)<br>
    +generate_array_tests(uint_<u></u>types, &quot;uint&quot;, 1, 30)<br>
<br>
<br>
Breaking out the uint types as a special case seems weird.  How about:<br>
</blockquote>
<br></div>
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.<br>
<br>
It was also less total typing. :)<div><div class="h5"><br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
for (types, base_name, major, minor) in [(bool_types, &quot;bool&quot;, 1, 20),<br>
... (uint_types, &quot;uint&quot;, 1, 30)]:<br>
     generate_tests(types, base_name, major, minor)<br>
     generate_array_tests(types, base_name, major, minor)<br>
<br>
    diff --git<br>
    a/generated_tests/uniform-<u></u>initializer-templates/fs-<u></u>initializer.template<br>
    b/generated_tests/uniform-<u></u>initializer-templates/fs-<u></u>initializer.template<br>
    new file mode 100644<br>
    index 0000000..9d78e36<br>
    --- /dev/null<br>
    +++<br>
    b/generated_tests/uniform-<u></u>initializer-templates/fs-<u></u>initializer.template<br>
    @@ -0,0 +1,34 @@<br>
    +[require]<br>
    +GLSL &gt;= ${&quot;{}.{}&quot;.format(major, minor)}<br>
    +<br>
    +[vertex shader]<br>
    +#version ${&quot;{}{}&quot;.format(major, minor)}<br>
    +<br>
    +void main()<br>
    +{<br>
    +  gl_Position = gl_Vertex;<br>
    +}<br>
    +<br>
    +[fragment shader]<br>
    +#version ${&quot;{}{}&quot;.format(major, minor)}<br>
    +<br>
    +% for (type, name, value) in type_list:<br>
    +uniform ${type} ${name} = ${value};<br>
    +% endfor<br>
    +<br>
    +void main()<br>
    +{<br>
    +  if ((${type_list[0][1]} == ${type_list[0][2]})<br>
    +% for (type, name, value) in type_list[1:-1]:<br>
    + &amp;&amp; (${name} == ${value})<br>
    +% endfor<br>
    + &amp;&amp; (${type_list[-1][1]} == ${type_list[-1][2]})) {<br>
<br>
<br>
This is a lot of work to go to to ensure that the if statement will be<br>
formatted nicely, and it will generate an unexpectedly redundant if test<br>
if len(type_list) == 1.<br>
<br>
You might consider this instead:<br>
<br>
   if (${&#39; &amp;&amp; &#39;.join(&#39;{0} == {1}&#39;.format(name, value) for type, name,<br>
value in type_list)}) {<br>
</blockquote>
<br></div></div>
I like that a lot!  I really hated this code when I wrote, and I originally had something like your suggestion below.<br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><div class="h5">
Or, alternatively, if you don&#39;t mind having &quot;&amp;&amp; true&quot; at the end of the<br>
if test, this might be a little less obscure:<br>
<br>
   if (<br>
% for type, name, value in type_list:<br>
       ${name} == ${value} &amp;&amp;<br>
% endfor<br>
       true) {<br>
<br>
<br>
    +    gl_FragColor = vec4(0, 1, 0, 1);<br>
    +  } else {<br>
    +    gl_FragColor = vec4(1, 0, 0, 1);<br>
    +  }<br>
    +}<br>
    +<br>
    +[test]<br>
    +draw rect -1 -1 2 2<br>
    +probe all rgb 0 1 0<br>
    diff --git<br>
    a/generated_tests/uniform-<u></u>initializer-templates/vs-<u></u>initializer.template<br>
    b/generated_tests/uniform-<u></u>initializer-templates/vs-<u></u>initializer.template<br>
    new file mode 100644<br>
    index 0000000..dd01cde<br>
    --- /dev/null<br>
    +++<br>
    b/generated_tests/uniform-<u></u>initializer-templates/vs-<u></u>initializer.template<br>
    @@ -0,0 +1,38 @@<br>
    +[require]<br>
    +GLSL &gt;= ${&quot;{}.{}&quot;.format(major, minor)}<br>
    +<br>
    +[vertex shader]<br>
    +#version ${&quot;{}{}&quot;.format(major, minor)}<br>
    +varying vec4 color;<br>
    +<br>
    +% for (type, name, value) in type_list:<br>
    +uniform ${type} ${name} = ${value};<br>
    +% endfor<br>
    +<br>
    +void main()<br>
    +{<br>
    +  if ((${type_list[0][1]} == ${type_list[0][2]})<br>
    +% for (type, name, value) in type_list[1:-1]:<br>
    + &amp;&amp; (${name} == ${value})<br>
    +% endfor<br>
    + &amp;&amp; (${type_list[-1][1]} == ${type_list[-1][2]})) {<br>
    +    color = vec4(0, 1, 0, 1);<br>
    +  } else {<br>
    +    color = vec4(1, 0, 0, 1);<br>
    +  }<br>
    +<br>
    +  gl_Position = gl_Vertex;<br>
    +}<br>
    +<br>
    +[fragment shader]<br>
    +#version ${&quot;{}{}&quot;.format(major, minor)}<br>
    +varying vec4 color;<br>
    +<br>
    +void main()<br>
    +{<br>
    +  gl_FragColor = color;<br>
    +}<br>
    +<br>
    +[test]<br>
    +draw rect -1 -1 2 2<br>
    +probe all rgb 0 1 0<br>
    --<br>
    1.7.6.5<br>
<br>
    ______________________________<u></u>_________________<br>
    Piglit mailing list<br></div></div>
    <a href="mailto:Piglit@lists.freedesktop.org" target="_blank">Piglit@lists.freedesktop.org</a> &lt;mailto:<a href="mailto:Piglit@lists.freedesktop.org" target="_blank">Piglit@lists.<u></u>freedesktop.org</a>&gt;<div class="im">
<br>
    <a href="http://lists.freedesktop.org/mailman/listinfo/piglit" target="_blank">http://lists.freedesktop.org/<u></u>mailman/listinfo/piglit</a><br>
<br>
<br>
All of my comments are minor nit-picks, so regardless of whether you<br>
take my suggestions:<br>
<br>
Reviewed-by: Paul Berry &lt;<a href="mailto:stereotype441@gmail.com" target="_blank">stereotype441@gmail.com</a><br></div>
&lt;mailto:<a href="mailto:stereotype441@gmail.com" target="_blank">stereotype441@gmail.<u></u>com</a>&gt;&gt;<br>
</blockquote>
</blockquote></div><br>