On 24 January 2013 19:50, Matt Turner <matts...@gmail.com> wrote: > This uses the existing pack/unpack testing infrasturcture for GLSL ES > 3.0 and adds support for testing pack/unpack 4x8 operations. > > Generate the following test files: > {const,vs,fs}-{pack,unpack}{Snorm,Unorm}{2x16,4x8}.shader_test > {const,vs,fs}-{pack,unpack}Half2x16.shader_test > --- > generated_tests/gen_builtin_packing_tests.py | 298 > ++++++++++++++++++++++---- > 1 files changed, 258 insertions(+), 40 deletions(-) > > diff --git a/generated_tests/gen_builtin_packing_tests.py > b/generated_tests/gen_builtin_packing_tests.py > index 30190d6..653e6fb 100644 > --- a/generated_tests/gen_builtin_packing_tests.py > +++ b/generated_tests/gen_builtin_packing_tests.py > @@ -12,7 +12,7 @@ import sys > from collections import namedtuple > from mako.template import Template > from math import copysign, fabs, fmod, frexp, isinf, isnan, modf > -from numpy import int16, int32, uint16, uint32, float32 > +from numpy import int8, int16, int32, uint8, uint16, uint32, float32 > from textwrap import dedent > > # > ---------------------------------------------------------------------------- > @@ -37,10 +37,13 @@ from textwrap import dedent > # Test evaluation of constant pack2x16 expressions. > const_pack_2x16_template = Template(dedent("""\ >
Can we rename this template to just "const_pack_template", to reflect the fact that it no longer just does 2x16 packing? (A similar comment applies to the other templates) > [require] > - GL ES >= 3.0 > - GLSL ES >= 3.00 > + ${func.requirements} > > [vertex shader] > + #ifndef GL_ES > + #extension GL_ARB_shading_language_packing : require > + #endif > + > const vec4 red = vec4(1, 0, 0, 1); > const vec4 green = vec4(0, 1, 0, 1); > > @@ -55,7 +58,12 @@ const_pack_2x16_template = Template(dedent("""\ > vert_color = green; > > % for io in func.inout_seq: > - actual = ${func.name}(vec2(${io.input[0]}, ${io.input[1]})); > + actual = ${func.name}(vec${func.vector_size}( > + % for index in range(func.vector_size - 1): > + ${io.input[index]}, > + % endfor > + ${io.input[func.vector_size - 1]} > + )); > This could be expressed more simply using the join() function: actual = ${func.name}(vec${func.vector_size}(${', '.join(io.input)})); > > if (true > % for u in sorted(set(io.valid_outputs)): > @@ -92,10 +100,13 @@ const_pack_2x16_template = Template(dedent("""\ > # Test evaluation of constant unpack2x16 expressions. > const_unpack_2x16_template = Template(dedent("""\ > [require] > - GL ES >= 3.0 > - GLSL ES >= 3.00 > + ${func.requirements} > > [vertex shader] > + #ifndef GL_ES > + #extension GL_ARB_shading_language_packing : require > + #endif > + > const vec4 red = vec4(1, 0, 0, 1); > const vec4 green = vec4(0, 1, 0, 1); > > @@ -104,7 +115,7 @@ const_unpack_2x16_template = Template(dedent("""\ > > void main() > { > - ${func.result_precision} vec2 actual; > + ${func.result_precision} vec${func.vector_size} actual; > > gl_Position = vertex; > vert_color = green; > @@ -114,7 +125,11 @@ const_unpack_2x16_template = Template(dedent("""\ > > if (true > % for v in io.valid_outputs: > - && actual != vec2(${v[0]}, ${v[1]}) > + && actual != vec${func.vector_size}( > + % for index in range(func.vector_size - 1): > + ${v[index]}, > + % endfor > + ${v[func.vector_size - 1]}) > join() could be used here too. > % endfor > ) { > vert_color = red; > @@ -147,14 +162,17 @@ const_unpack_2x16_template = Template(dedent("""\ > # Test execution of pack2x16 functions in the vertex shader. > vs_pack_2x16_template = Template(dedent("""\ > [require] > - GL ES >= 3.0 > - GLSL ES >= 3.00 > + ${func.requirements} > > [vertex shader] > + #ifndef GL_ES > + #extension GL_ARB_shading_language_packing : require > + #endif > + > const vec4 red = vec4(1, 0, 0, 1); > const vec4 green = vec4(0, 1, 0, 1); > > - uniform vec2 func_input; > + uniform vec${func.vector_size} func_input; > > % for j in range(func.num_valid_outputs): > uniform ${func.result_precision} uint expect${j}; > (snip) > @@ -688,7 +793,7 @@ def unpack_half_1x16(u16): > # > ---------------------------------------------------------------------------- > > # This table maps GLSL pack/unpack function names to a sequence of inputs > to > -# the respective component-wise function. It contains two types of > mappings: > +# the respective component-wise function. It contains four types of > mappings: > # - name of a pack2x16 function to a sequence of float32 > # - name of a unpack2x16 function to a sequence of uint16 > Did you forget to add bullet points here to describe the two new types of mappings? > full_input_table = dict() > @@ -720,6 +825,8 @@ def make_inputs_for_pack_snorm_2x16(): > full_input_table["packSnorm2x16"] = make_inputs_for_pack_snorm_2x16() > reduced_input_table["packSnorm2x16"] = None > > +full_input_table["packSnorm4x8"] = full_input_table["packSnorm2x16"] > + > # XXX: Perhaps there is a better choice of test inputs? > full_input_table["unpackSnorm2x16"] = tuple(uint16(u) for u in ( > 0, 1, 2, 3, > @@ -729,6 +836,15 @@ full_input_table["unpackSnorm2x16"] = tuple(uint16(u) > for u in ( > 2**16 - 1, # max uint16 > )) > > +# XXX: Perhaps there is a better choice of test inputs? > +full_input_table["unpackSnorm4x8"] = tuple(uint8(u) for u in ( > + 0, 1, 2, 3, > + 2**7 - 1, > + 2**7, > + 2**7 + 1, > + 2**8 - 1, # max uint8 > + )) > + > full_input_table["packUnorm2x16"] = tuple(float32(x) for x in ( > # The domain of packUnorm2x16 is [-inf, +inf]^2. The function clamps > its > # input into the range [0, 1]^2. > @@ -746,8 +862,11 @@ full_input_table["packUnorm2x16"] = tuple(float32(x) > for x in ( > > reduced_input_table["packUnorm2x16"] = None > > +full_input_table["packUnorm4x8"] = full_input_table["packUnorm2x16"] > + > # XXX: Perhaps there is a better choice of test inputs? > full_input_table["unpackUnorm2x16"] = full_input_table["unpackSnorm2x16"] > +full_input_table["unpackUnorm4x8"] = full_input_table["unpackSnorm4x8"] > > def make_inputs_for_pack_half_2x16(): > # The domain of packHalf2x16 is ([-inf, +inf] + {NaN})^2. The function > @@ -989,6 +1108,35 @@ def make_inouts_for_pack_2x16(pack_1x16_func, > > return inout_seq > > +def make_inouts_for_pack_4x8(pack_1x8_func, float32_inputs): > + """Determine valid outputs for a given GLSL pack4x8 function. > + > + :param pack_1x8_func: the component-wise function of the pack4x8 > + function > + :param float32_inputs: a sequence of inputs to pack_1x8_func > + :return: a sequence of InOutTuple > + """ > + inout_seq = [] > + > + func_opt_seq = (FuncOpts(FuncOpts.ROUND_TO_EVEN), > + FuncOpts(FuncOpts.ROUND_TO_NEAREST)) > + > + for y in float32_inputs: > + for x in float32_inputs: > + assert(isinstance(x, float32)) > + > + valid_outputs = [] > + for func_opts in func_opt_seq: > + u32 = pack_4x8(pack_1x8_func, x, y, x, y, func_opts) > + assert(isinstance(u32, uint32)) > + valid_outputs.append(glsl_literal(u32)) > + > + inout_seq.append( > + InOutTuple(input=(glsl_literal(x), glsl_literal(y), > + glsl_literal(x), glsl_literal(y)), > + valid_outputs=valid_outputs)) > + return inout_seq > + > 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. 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. > def make_inouts_for_unpack_2x16(unpack_1x16_func, uint16_inputs): > """Determine expected outputs of a given GLSL unpack2x16 function. > > @@ -1014,27 +1162,69 @@ def make_inouts_for_unpack_2x16(unpack_1x16_func, > uint16_inputs): > > return inout_seq > > +def make_inouts_for_unpack_4x8(unpack_1x8_func, uint8_inputs): > + """Determine expected outputs of a given GLSL unpack4x8 function. > + > + :param unpack_1x8_func: the component-wise function of the unpack4x8 > + function > + :param uint8_inputs: a sequence of inputs to unpack_1x8_func > + :return: a sequence of InOutTuple > + """ > + inout_seq = [] > + > + func_opts = FuncOpts() > + > + for y in uint8_inputs: > + for x in uint8_inputs: > + assert(isinstance(x, uint8)) > + u32 = uint32((y << 24) | (x << 16) | (y << 8) | x) > + > + valid_outputs = [] > + vec4 = unpack_4x8(unpack_1x8_func, u32, func_opts) > + assert(isinstance(vec4[0], float32)) > + assert(isinstance(vec4[1], float32)) > + assert(isinstance(vec4[2], float32)) > + assert(isinstance(vec4[3], float32)) > + valid_outputs.append((glsl_literal(vec4[0]), > + glsl_literal(vec4[1]), > + glsl_literal(vec4[2]), > + glsl_literal(vec4[3]))) > + > + inout_seq.append( > + InOutTuple(input=glsl_literal(u32), > + valid_outputs=valid_outputs)) > + > + return inout_seq > + > The same concern and suggested fix apply here. (snip) 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.
_______________________________________________ Piglit mailing list Piglit@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/piglit