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

[Tapani Pälli]

On 02/16/2015 08:14 AM, Ilia Mirkin wrote:
> 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

OK that test looks agreeably simple and almost the same as what I wrote 
:) so maybe not worth having these after all. I'll see if I can find 
anything missing!

>>>> 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
>