<div dir="ltr">On 24 January 2013 19:50, Matt Turner <span dir="ltr"><<a href="mailto:mattst88@gmail.com" target="_blank">mattst88@gmail.com</a>></span> wrote:<br><div class="gmail_extra"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
This uses the existing pack/unpack testing infrasturcture for GLSL ES<br>
3.0 and adds support for testing pack/unpack 4x8 operations.<br>
<br>
Generate the following test files:<br>
        {const,vs,fs}-{pack,unpack}{Snorm,Unorm}{2x16,4x8}.shader_test<br>
        {const,vs,fs}-{pack,unpack}Half2x16.shader_test<br>
---<br>
 generated_tests/gen_builtin_packing_tests.py |  298 ++++++++++++++++++++++----<br>
 1 files changed, 258 insertions(+), 40 deletions(-)<br>
<br>
diff --git a/generated_tests/gen_builtin_packing_tests.py b/generated_tests/gen_builtin_packing_tests.py<br>
index 30190d6..653e6fb 100644<br>
--- a/generated_tests/gen_builtin_packing_tests.py<br>
+++ b/generated_tests/gen_builtin_packing_tests.py<br>
@@ -12,7 +12,7 @@ import sys<br>
 from collections import namedtuple<br>
 from mako.template import Template<br>
 from math import copysign, fabs, fmod, frexp, isinf, isnan, modf<br>
-from numpy import int16, int32, uint16, uint32, float32<br>
+from numpy import int8, int16, int32, uint8, uint16, uint32, float32<br>
 from textwrap import dedent<br>
<br>
 # ----------------------------------------------------------------------------<br>
@@ -37,10 +37,13 @@ from textwrap import dedent<br>
 # Test evaluation of constant pack2x16 expressions.<br>
 const_pack_2x16_template = Template(dedent("""\<br></blockquote><div><br></div><div>Can we rename this template to just "const_pack_template", to reflect the fact that it no longer just does 2x16 packing?<br>
<br></div><div>(A similar comment applies to the other templates)<br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
     [require]<br>
-    GL ES >= 3.0<br>
-    GLSL ES >= 3.00<br>
+    ${func.requirements}<br>
<br>
     [vertex shader]<br>
+    #ifndef GL_ES<br>
+    #extension GL_ARB_shading_language_packing : require<br>
+    #endif<br>
+<br>
     const vec4 red = vec4(1, 0, 0, 1);<br>
     const vec4 green = vec4(0, 1, 0, 1);<br>
<br>
@@ -55,7 +58,12 @@ const_pack_2x16_template = Template(dedent("""\<br>
         vert_color = green;<br>
<br>
         % for io in func.inout_seq:<br>
-        actual = ${<a href="http://func.name" target="_blank">func.name</a>}(vec2(${io.input[0]}, ${io.input[1]}));<br>
+        actual = ${<a href="http://func.name" target="_blank">func.name</a>}(vec${func.vector_size}(<br>
+        % for index in range(func.vector_size - 1):<br>
+            ${io.input[index]},<br>
+        % endfor<br>
+            ${io.input[func.vector_size - 1]}<br>
+        ));<br></blockquote><div><br></div><div>This could be expressed more simply using the join() function:<br><br></div><div>actual = ${<a href="http://func.name">func.name</a>}(vec${func.vector_size}(${', '.join(io.input)}));<br>
</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
         if (true<br>
             % for u in sorted(set(io.valid_outputs)):<br>
@@ -92,10 +100,13 @@ const_pack_2x16_template = Template(dedent("""\<br>
 # Test evaluation of constant unpack2x16 expressions.<br>
 const_unpack_2x16_template = Template(dedent("""\<br>
     [require]<br>
-    GL ES >= 3.0<br>
-    GLSL ES >= 3.00<br>
+    ${func.requirements}<br>
<br>
     [vertex shader]<br>
+    #ifndef GL_ES<br>
+    #extension GL_ARB_shading_language_packing : require<br>
+    #endif<br>
+<br>
     const vec4 red = vec4(1, 0, 0, 1);<br>
     const vec4 green = vec4(0, 1, 0, 1);<br>
<br>
@@ -104,7 +115,7 @@ const_unpack_2x16_template = Template(dedent("""\<br>
<br>
     void main()<br>
     {<br>
-        ${func.result_precision} vec2 actual;<br>
+        ${func.result_precision} vec${func.vector_size} actual;<br>
<br>
         gl_Position = vertex;<br>
         vert_color = green;<br>
@@ -114,7 +125,11 @@ const_unpack_2x16_template = Template(dedent("""\<br>
<br>
         if (true<br>
             % for v in io.valid_outputs:<br>
-            && actual != vec2(${v[0]}, ${v[1]})<br>
+            && actual != vec${func.vector_size}(<br>
+            % for index in range(func.vector_size - 1):<br>
+                ${v[index]},<br>
+            % endfor<br>
+                ${v[func.vector_size - 1]})<br></blockquote><div><br></div><div>join() could be used here too.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">

             % endfor<br>
            ) {<br>
             vert_color = red;<br>
@@ -147,14 +162,17 @@ const_unpack_2x16_template = Template(dedent("""\<br>
 # Test execution of pack2x16 functions in the vertex shader.<br>
 vs_pack_2x16_template = Template(dedent("""\<br>
     [require]<br>
-    GL ES >= 3.0<br>
-    GLSL ES >= 3.00<br>
+    ${func.requirements}<br>
<br>
     [vertex shader]<br>
+    #ifndef GL_ES<br>
+    #extension GL_ARB_shading_language_packing : require<br>
+    #endif<br>
+<br>
     const vec4 red = vec4(1, 0, 0, 1);<br>
     const vec4 green = vec4(0, 1, 0, 1);<br>
<br>
-    uniform vec2 func_input;<br>
+    uniform vec${func.vector_size} func_input;<br>
<br>
     % for j in range(func.num_valid_outputs):<br>
     uniform ${func.result_precision} uint expect${j};<br></blockquote><div><br></div><div>(snip)<br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">

@@ -688,7 +793,7 @@ def unpack_half_1x16(u16):<br>
 # ----------------------------------------------------------------------------<br>
<br>
 # This table maps GLSL pack/unpack function names to a sequence of inputs to<br>
-# the respective component-wise function. It contains two types of mappings:<br>
+# the respective component-wise function. It contains four types of mappings:<br>
 #    - name of a pack2x16 function to a sequence of float32<br>
 #    - name of a unpack2x16 function to a sequence of uint16<br></blockquote><div><br></div><div>Did you forget to add bullet points here to describe the two new types of mappings?<br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">

 full_input_table = dict()<br>
@@ -720,6 +825,8 @@ def make_inputs_for_pack_snorm_2x16():<br>
 full_input_table["packSnorm2x16"] = make_inputs_for_pack_snorm_2x16()<br>
 reduced_input_table["packSnorm2x16"] = None<br>
<br>
+full_input_table["packSnorm4x8"] = full_input_table["packSnorm2x16"]<br>
+<br>
 # XXX: Perhaps there is a better choice of test inputs?<br>
 full_input_table["unpackSnorm2x16"] = tuple(uint16(u) for u in (<br>
     0, 1, 2, 3,<br>
@@ -729,6 +836,15 @@ full_input_table["unpackSnorm2x16"] = tuple(uint16(u) for u in (<br>
     2**16 - 1, # max uint16<br>
     ))<br>
<br>
+# XXX: Perhaps there is a better choice of test inputs?<br>
+full_input_table["unpackSnorm4x8"] = tuple(uint8(u) for u in (<br>
+    0, 1, 2, 3,<br>
+    2**7 - 1,<br>
+    2**7,<br>
+    2**7 + 1,<br>
+    2**8 - 1, # max uint8<br>
+    ))<br>
+<br>
 full_input_table["packUnorm2x16"] = tuple(float32(x) for x in (<br>
     # The domain of packUnorm2x16 is [-inf, +inf]^2. The function clamps its<br>
     # input into the range [0, 1]^2.<br>
@@ -746,8 +862,11 @@ full_input_table["packUnorm2x16"] = tuple(float32(x) for x in (<br>
<br>
 reduced_input_table["packUnorm2x16"] = None<br>
<br>
+full_input_table["packUnorm4x8"] = full_input_table["packUnorm2x16"]<br>
+<br>
 # XXX: Perhaps there is a better choice of test inputs?<br>
 full_input_table["unpackUnorm2x16"] = full_input_table["unpackSnorm2x16"]<br>
+full_input_table["unpackUnorm4x8"] = full_input_table["unpackSnorm4x8"]<br>
<br>
 def make_inputs_for_pack_half_2x16():<br>
     # The domain of packHalf2x16 is ([-inf, +inf] + {NaN})^2. The function<br>
@@ -989,6 +1108,35 @@ def make_inouts_for_pack_2x16(pack_1x16_func,<br>
<br>
     return inout_seq<br>
<br>
+def make_inouts_for_pack_4x8(pack_1x8_func, float32_inputs):<br>
+    """Determine valid outputs for a given GLSL pack4x8 function.<br>
+<br>
+    :param pack_1x8_func: the component-wise function of the pack4x8<br>
+        function<br>
+    :param float32_inputs: a sequence of inputs to pack_1x8_func<br>
+    :return: a sequence of InOutTuple<br>
+    """<br>
+    inout_seq = []<br>
+<br>
+    func_opt_seq = (FuncOpts(FuncOpts.ROUND_TO_EVEN),<br>
+                    FuncOpts(FuncOpts.ROUND_TO_NEAREST))<br>
+<br>
+    for y in float32_inputs:<br>
+        for x in float32_inputs:<br>
+            assert(isinstance(x, float32))<br>
+<br>
+            valid_outputs = []<br>
+            for func_opts in func_opt_seq:<br>
+                u32 = pack_4x8(pack_1x8_func, x, y, x, y, func_opts)<br>
+                assert(isinstance(u32, uint32))<br>
+                valid_outputs.append(glsl_literal(u32))<br>
+<br>
+            inout_seq.append(<br>
+                InOutTuple(input=(glsl_literal(x), glsl_literal(y),<br>
+                                  glsl_literal(x), glsl_literal(y)),<br>
+                           valid_outputs=valid_outputs))<br>
+    return inout_seq<br>
+<br></blockquote><div><br></div><div>I'm not convinced this is an adequate test, since it always generates test vectors where .x=.z and .y=.w.  If, for instance, the implementation had a bug that caused the .x and .z components of the vector to be swapped, that bug would go uncaught.<br>
<br></div><div>Suggestion: during each iteration of the loop, in addition to constructing the test vector (x, y, x, y), construct the test vector (x, x, y, y).  I think that should give adequate coverage, and it only increases the number of test vectors by a factor of 2.<br>
</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
 def make_inouts_for_unpack_2x16(unpack_1x16_func, uint16_inputs):<br>
     """Determine expected outputs of a given GLSL unpack2x16 function.<br>
<br>
@@ -1014,27 +1162,69 @@ def make_inouts_for_unpack_2x16(unpack_1x16_func, uint16_inputs):<br>
<br>
     return inout_seq<br>
<br>
+def make_inouts_for_unpack_4x8(unpack_1x8_func, uint8_inputs):<br>
+    """Determine expected outputs of a given GLSL unpack4x8 function.<br>
+<br>
+    :param unpack_1x8_func: the component-wise function of the unpack4x8<br>
+        function<br>
+    :param uint8_inputs: a sequence of inputs to unpack_1x8_func<br>
+    :return: a sequence of InOutTuple<br>
+    """<br>
+    inout_seq = []<br>
+<br>
+    func_opts = FuncOpts()<br>
+<br>
+    for y in uint8_inputs:<br>
+        for x in uint8_inputs:<br>
+            assert(isinstance(x, uint8))<br>
+            u32 = uint32((y << 24) | (x << 16) | (y << 8) | x)<br>
+<br>
+            valid_outputs = []<br>
+            vec4 = unpack_4x8(unpack_1x8_func, u32, func_opts)<br>
+            assert(isinstance(vec4[0], float32))<br>
+            assert(isinstance(vec4[1], float32))<br>
+            assert(isinstance(vec4[2], float32))<br>
+            assert(isinstance(vec4[3], float32))<br>
+            valid_outputs.append((glsl_literal(vec4[0]),<br>
+                                  glsl_literal(vec4[1]),<br>
+                                  glsl_literal(vec4[2]),<br>
+                                  glsl_literal(vec4[3])))<br>
+<br>
+            inout_seq.append(<br>
+                InOutTuple(input=glsl_literal(u32),<br>
+                           valid_outputs=valid_outputs))<br>
+<br>
+    return inout_seq<br>
+<br></blockquote><div><br></div><div>The same concern and suggested fix apply here.<br><br></div><div>(snip)<br></div><br></div><div class="gmail_quote">That's all my comments on this patch.  I still haven't spotted the bug you're running into.  Hopefully the simulator will provide some clues.<br>
</div></div></div>