[Piglit] [PATCH] ARB_shading_language_packing: Generate tests for builtin packing functions

Paul Berry stereotype441 at gmail.com
Fri Jan 25 08:22:00 PST 2013


On 24 January 2013 19:50, Matt Turner <mattst88 at 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.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/piglit/attachments/20130125/c2da56d3/attachment-0001.html>


More information about the Piglit mailing list