On 21 May 2012 14:10, Ian Romanick <span dir="ltr"><<a href="mailto:idr@freedesktop.org" target="_blank">idr@freedesktop.org</a>></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 <<a href="mailto:idr@freedesktop.org" target="_blank">idr@freedesktop.org</a><br></div><div class="im">
<mailto:<a href="mailto:idr@freedesktop.org" target="_blank">idr@freedesktop.org</a>>> wrote:<br>
<br>
From: Ian Romanick <<a href="mailto:ian.d.romanick@intel.com" target="_blank">ian.d.romanick@intel.com</a><br></div>
<mailto:<a href="mailto:ian.d.romanick@intel.com" target="_blank">ian.d.romanick@intel.<u></u>com</a>>><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 <<a href="mailto:ian.d.romanick@intel.com" target="_blank">ian.d.romanick@intel.com</a><br></div>
<mailto:<a href="mailto:ian.d.romanick@intel.com" target="_blank">ian.d.romanick@intel.<u></u>com</a>>></blockquote></blockquote><div><br>Whoops, I failed to notice when I reviewed this patch that it doesn'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'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 "gen-tests" 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>
"Software"),<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 "AS IS", 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>
+ """Get a string representing a number in the specified GLSL type"""<br>
+<br>
+ idx = idx % len(random_numbers)<br>
+ value = random_numbers[idx]<br>
+<br>
+ if type[0] == 'b':<br>
+ if (value * 10) > 5:<br>
+ return "1"<br>
+ else:<br>
+ return "0"<br>
+ elif type[0] == 'i':<br>
+ return str(int(value * 100) - 50)<br>
+ elif type[0] == 'u':<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 ("vs", "fs"):<br>
+ for t in all_templates:<br>
+ template_file_name =<br>
"uniform-initializer-<u></u>templates/{}-initializer{}.<u></u>template".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('spec',<br>
+ 'glsl-{}.{}'.format(major, minor),<br>
+ 'execution',<br>
+ 'uniform-initializer',<br>
+ '{}-{}{}.shader_test'.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 = "{0}({1})".format(type, ", ".join(numbers))<br>
+<br>
+ api_type = type;<br>
+ if type == "bool":<br>
+ api_type = "int"<br>
+ elif type[0] == 'b':<br>
+ api_type = "ivec{}".format(type[-1])<br>
+<br>
+ if type[-1] in ["2", "3", "4"]:<br>
+ name = "".join(["u", type[0], type[-1]])<br>
+ else:<br>
+ name = "".join(["u", 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, "w")<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 ("vs", "fs"):<br>
+ template_file_name =<br>
"uniform-initializer-<u></u>templates/{}-initializer.<u></u>template".format(target)<br>
+ f = open(template_file_name)<br>
+ template = f.read()<br>
+ f.close()<br>
+<br>
+ test_file_name = dirname = os.path.join('spec',<br>
+ 'glsl-{}.{}'.format(major, minor),<br>
+ 'execution',<br>
+ 'uniform-initializer',<br>
+ '{}-{}-array.shader_test'.<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("{0}(<u></u>{1})".format(type, ",<br>
".join(numbers)))<br>
+<br>
+ if type[-1] in ["2", "3", "4"]:<br>
+ name = "".join(["u", type[0], type[-1]])<br>
+ else:<br>
+ name = "".join(["u", type[0]])<br>
+<br>
+ array_type = "".join([format(type), "[2]"])<br>
+ value = "{}({})".format(array_type, ",<br>
".join(constructor_parts))<br>
+ test_vectors.append((array_<u></u>type, name, value))<br>
+<br>
+ f = open(test_file_name, "w")<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 = ("",<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>
"{vs,fs}-initializer-direct.<u></u>template" (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't like uniform-initializer/vs-mat4-<u></u>direct (or similar names that I could think of). I also didn'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 = [("bool", 1), ("bvec2", 2), ("bvec3", 3), ("bvec4", 4)]<br>
+int_types = [("int", 1), ("ivec2", 2), ("ivec3", 3), ("ivec4", 4)]<br>
+float_types = [("float", 1), ("vec2", 2), ("vec3", 3), ("vec4", 4)]<br>
+uint_types = [("uint", 1), ("uvec2", 2), ("uvec3", 3), ("uvec4", 4)]<br>
+mat2_types = [("mat2x2", 4), ("mat2x3", 6), ("mat2x4", 8)]<br>
+mat3_types = [("mat3x2", 6), ("mat3x3", 9), ("mat3x4", 12)]<br>
+mat4_types = [("mat4x2", 8), ("mat4x3", 12), ("mat4x4", 16)]<br>
+<br>
+for (types, base_name) in [(bool_types, "bool"),<br>
+ (int_types, "int"),<br>
+ (float_types, "float"),<br>
+ (mat2_types, "mat2"),<br>
+ (mat3_types, "mat3"),<br>
+ (mat4_types, "mat4")]:<br>
+ generate_tests(types, base_name, 1, 20)<br>
+ generate_array_tests(types, base_name, 1, 20)<br>
+<br>
+generate_tests(uint_types, "uint", 1, 30)<br>
+generate_array_tests(uint_<u></u>types, "uint", 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, "bool", 1, 20),<br>
... (uint_types, "uint", 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 >= ${"{}.{}".format(major, minor)}<br>
+<br>
+[vertex shader]<br>
+#version ${"{}{}".format(major, minor)}<br>
+<br>
+void main()<br>
+{<br>
+ gl_Position = gl_Vertex;<br>
+}<br>
+<br>
+[fragment shader]<br>
+#version ${"{}{}".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>
+ && (${name} == ${value})<br>
+% endfor<br>
+ && (${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 (${' && '.join('{0} == {1}'.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't mind having "&& true" 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} &&<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 >= ${"{}.{}".format(major, minor)}<br>
+<br>
+[vertex shader]<br>
+#version ${"{}{}".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>
+ && (${name} == ${value})<br>
+% endfor<br>
+ && (${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 ${"{}{}".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> <mailto:<a href="mailto:Piglit@lists.freedesktop.org" target="_blank">Piglit@lists.<u></u>freedesktop.org</a>><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 <<a href="mailto:stereotype441@gmail.com" target="_blank">stereotype441@gmail.com</a><br></div>
<mailto:<a href="mailto:stereotype441@gmail.com" target="_blank">stereotype441@gmail.<u></u>com</a>>><br>
</blockquote>
</blockquote></div><br>