On 31 January 2013 22:51, Matt Turner <matts...@gmail.com> wrote:
> The unpack builtins are implemented using floating-point division, which
> is implemented by reciprocal+multiply.
This should probably say something like "Which is implemented by
reciprocal+multiply on Mesa/i965". Other drivers don't necessarily use
reciprocal+multiply to do division.
> As it so happens, 1.0/255.0 falls
> almost exactly between two representable 32-bit floats and leads to bad
> rounding. None of the other divisors (127, 32767, 65535) have this
> property.
>
> So allow the result of unpackUnorm4x8 to be within some small epsilon
> (0.00001) of the actual value. Some values like 0.0 and 1.0 should be
> calculated exactly, so handle them specially.
> ---
> generated_tests/gen_builtin_packing_tests.py | 20 ++++++++++++++++++++
> 1 file changed, 20 insertions(+)
>
> diff --git a/generated_tests/gen_builtin_packing_tests.py
> b/generated_tests/gen_builtin_packing_tests.py
> index 7c1074e..0f7510f 100644
> --- a/generated_tests/gen_builtin_packing_tests.py
> +++ b/generated_tests/gen_builtin_packing_tests.py
> @@ -231,6 +231,8 @@ vs_unpack_template = Template(dedent("""\
>
> uniform highp uint func_input;
>
> + uniform bool exact;
> +
> % for j in range(func.num_valid_outputs):
> uniform ${func.result_precision} ${func.vector_type} expect${j};
> % endfor
> @@ -246,6 +248,9 @@ vs_unpack_template = Template(dedent("""\
>
> if (false
> % for j in range(func.num_valid_outputs):
> + % if not func.exact:
> + || (!exact && distance(actual, expect${j}) < 0.00001)
> + %endif
> || actual == expect${j}
> % endfor
>
I had a little trouble following the boolean logic here. Would something
like this be clearer?
% for j in range(func.num_valid_outputs):
|| (exact ? actual == expect${j}
: distance(actual, expect${j}) < 0.00001)
% endfor
(I don't consider it a blocking issue, though. The code you've written is
correct, it just took me a while to puzzle out.)
Note: since my suggested change would cause the shader to always reference
the "exact" uniform, you'd have to change the code below to emit "uniform"
commands even when func.exact is true.
> ) {
> @@ -274,6 +279,9 @@ vs_unpack_template = Template(dedent("""\
> [test]
> % for io in func.inout_seq:
> uniform uint func_input ${io.input}
> + % if not func.exact:
> + uniform int exact ${int(int(io.input[:-1]) in (0x0, 0xffffffff))}
> + % endif
> % for j in range(func.num_valid_outputs):
> uniform ${func.vector_type} expect${j} ${"
> ".join(io.valid_outputs[j])}
> % endfor
> @@ -370,6 +378,8 @@ fs_unpack_template = Template(dedent("""\
>
> uniform highp uint func_input;
>
> + uniform bool exact;
> +
> % for i in range(func.num_valid_outputs):
> uniform ${func.result_precision} ${func.vector_type} expect${i};
> % endfor
> @@ -382,6 +392,9 @@ fs_unpack_template = Template(dedent("""\
>
> if (false
> % for i in range(func.num_valid_outputs):
> + % if not func.exact:
> + || (!exact && distance(actual, expect${i}) < 0.00001)
> + %endif
> || actual == expect${i}
> % endfor
> ) {
> @@ -401,6 +414,9 @@ fs_unpack_template = Template(dedent("""\
> [test]
> % for io in func.inout_seq:
> uniform uint func_input ${io.input}
> + % if not func.exact:
> + uniform int exact ${int(int(io.input[:-1]) in (0x0, 0xffffffff))}:
> + % endif
> % for i in range(func.num_valid_outputs):
> uniform ${func.vector_type} expect${i} ${"
> ".join(io.valid_outputs[i])}
> % endfor
> @@ -1271,6 +1287,9 @@ class FuncInfo:
>
> - requirements: A set of API/extension requirments to be listed in the
> .shader_test's [requires] section.
> +
> + - exact: Whether the generated results must be exact (e.g., 0.0 and
> 1.0
> + should always be converted exactly).
> """
>
> def __init__(self, name, requirements):
> @@ -1279,6 +1298,7 @@ class FuncInfo:
> self.inout_seq = inout_table[name]
> self.num_valid_outputs = len(self.inout_seq[0].valid_outputs)
> self.requirements = requirements
> + self.exact = not name.endswith("unpackUnorm4x8")
>
I'm not comfortable with only applying this change to unpackUnorm4x8.
Although we've only seen the failure in unpackUnorm4x8, the reason for that
is highly specific to Mesa/i965. Other implementations might conceivably
have different sources of rounding errors, and IMHO we shouldn't make an
exception that's so specific to i965.
I believe the only unpack function we should require to be exact is
unpackHalf2x16.
>
> if name.endswith("2x16"):
> self.dimension = "2x16"
> --
> 1.7.12.4
>
> _______________________________________________
> Piglit mailing list
> Piglit@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/piglit
>
_______________________________________________
Piglit mailing list
Piglit@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/piglit
