[Piglit] [PATCH] arb_gpu_shader_fp64: add glsl algebraic tests

[Ilia Mirkin]

On Mon, Feb 16, 2015 at 12:57 AM, Tapani Pälli <tapani.palli at intel.com> wrote:
>
> On 02/13/2015 04:17 PM, Ilia Mirkin wrote:
>>
>> On Fri, Feb 13, 2015 at 7:32 AM, Tapani Pälli <tapani.palli at intel.com>
>> wrote:
>>> diff --git
>>> a/tests/spec/arb_gpu_shader_fp64/execution/built-in-functions/glsl-algebraic-add-double.shader_test
>>> b/tests/spec/arb_gpu_shader_fp64/execution/built-in-functions/glsl-algebraic-add-double.shader_test
>>> new file mode 100644
>>> index 0000000..19d3af4
>>> --- /dev/null
>>> +++
>>> b/tests/spec/arb_gpu_shader_fp64/execution/built-in-functions/glsl-algebraic-add-double.shader_test
>>> @@ -0,0 +1,29 @@
>>> +[require]
>>> +GLSL >= 1.50
>>> +GL_ARB_gpu_shader_fp64
>>> +
>>> +[vertex shader passthrough]
>>> +
>>> +[fragment shader]
>>> +#version 150
>>> +#extension GL_ARB_gpu_shader_fp64 : enable
>>> +uniform dvec4 a;
>>> +uniform dvec4 b;
>>> +uniform dvec4 expected;
>>> +const double tolerance = 1.0000000000000002e-05;
>>
>>
>> That should probably be LF.
>
>
> Yes, that's missing.
>
>>> +void main()
>>> +{
>>> +       dvec4 result = dvec4(a + b);
>>
>>
>> Why the dvec4? Also, what does this test add that the existing
>> generated tests don't have? (e.g. fs-op-add-dvec4-dvec4)
>
>
> I wanted to have the distance comparison happen with actual double values
> resulted from calculation instead of converting first to float and compare
> that.
>
> To be honest, I'm not sure if these tests add anything to the generated
> tests. What I see is that the generated tests seem quite complex and
> debugging is much easier with simple tests, most bugs in the Intel driver
> backend fp64 implementation were found with even smaller tests than these.
> I'm ok with not having these in the tree, I just corrected them to use the
> distance always, the previous tests I sent were too strict and would not
> pass on Intel.
>
>
>
>>> +
>>> +       if (distance(expected, result) < tolerance)
>>> +               gl_FragColor = vec4(0.0, 1.0, 0.0, 1.0);
>>> +       else
>>> +               gl_FragColor = vec4(1.0, 0.0, 0.0, 1.0);
>>> +}

Perhaps I'm missing how this is different from the generated test
fs-op-add-dvec4-dvec4:

uniform dvec4 arg0;
uniform dvec4 arg1;
uniform double tolerance;
uniform dvec4 expected;

void main()
{
  dvec4 result = (arg0 + arg1);
  gl_FragColor = distance(result, expected) <= tolerance ? vec4(0.0,
1.0, 0.0, 1.0) : vec4(1.0, 0.0, 0.0, 1.0);
}

All the comparisons are done with doubles as best I can see

>>> a/tests/spec/arb_gpu_shader_fp64/execution/built-in-functions/glsl-algebraic-csel-double.shader_test
>>> b/tests/spec/arb_gpu_shader_fp64/execution/built-in-functions/glsl-algebraic-csel-double.shader_test
>>> new file mode 100644
>>> index 0000000..e8bf9f8
>>> --- /dev/null
>>> +++
>>> b/tests/spec/arb_gpu_shader_fp64/execution/built-in-functions/glsl-algebraic-csel-double.shader_test
>>> @@ -0,0 +1,20 @@
>>> +[require]
>>> +GLSL >= 1.50
>>> +GL_ARB_gpu_shader_fp64
>>> +
>>> +[vertex shader passthrough]
>>> +
>>> +[fragment shader]
>>> +#version 150
>>> +#extension GL_ARB_gpu_shader_fp64 : enable
>>> +void main()
>>> +{
>>> +       const double small = 1.0lf;
>>> +       const double big = 2.0lf;
>>> +       double val = (big > small) ? 1.0lf : 0.0lf;
>>> +       gl_FragColor = vec4(val, 0.0, val, 1.0);
>>> +}
>>> +
>>> +[test]
>>> +draw rect -1 -1 2 2
>>> +probe all rgb 1.0 0 1.0
>>
>>
>> Usually we do green for success... how about
>>
>> vec4(0, val, 0, 1)
>
>
> Ok that's fine, I don't quite agree on the 'default green' idea though. I
> think ideally Piglit should use more randomized (but hardcoded) colors and
> patterns of those instead of complete solid colors (and always the same)
> which has the possibility to hide implementation issues.

Yeah, I've actually run into issues with that, where the green from a
previous test was left over. Very annoying to debug. But these tests
probably should stay with the flow...

  -ilia