[Piglit] [PATCH 3/3] arb_gpu_shader5: interpolateAt* must reject inputs in structs

[Nicolai Hähnle]

On 20.06.2017 13:32, Nicolai Hähnle wrote:
> On 19.06.2017 23:46, Ian Romanick wrote:
>> That's actually a little surprising... do other drivers reject this?  If
>> so, I'll agree with Timothy that a spec quote would be nice.
> 
> So, I guess I was going off of what was in the Mesa source already, plus 
> the spec part of Section 8.13.2 (Interpolation Functions) of GLSL 4.50 
> which says:
> 
>     "For all of the interpolation functions, interpolant must be an
>      input variable or an element of an input variable declared as an
>      array. Component selection operators (e.g., .xy) may be used when
>      specifying interpolant."
> 
> ... and if you go out of your way to explicitly say that it *can* be an 
> element of an array, one kind of expects that it *cannot* be an element 
> of a struct. But maybe that's an artifact of how the spec evolved? 
> Actually, now that I re-read it, our rejection of swizzles also looks 
> incorrect.
> 
> As for compilers I can test:
> 
> glslangValidator: accepts swizzles and struct elements
> amdgpu-pro: accepts struct elements but rejects swizzles; it does accept 
> non-swizzling individual component selection, which seems like a spec 
> misunderstanding given how "component selection" is defined in Section 
> 5.5 (Vector and Scalar Components and Length).
> Mesa: rejects swizzles and struct elements
> 
> Going back to the ARB_gpu_shader5 spec, it explicitly forbids swizzles 
> (there's an additional "not" in the sentence quoted above). So that 
> seems to have changed at some point and we simply missed it. More 
> digging needed...

So, the "not" was removed in GLSL 4.40, but there's no mention of this 
change in the changelog.

On the matter of struct elements: Consider that they cannot have 
flat/centroid/noperspective qualifiers, so there's actually a good 
reason forbidding them.

I've opened a spec bug about this anyway: 
https://gitlab.khronos.org/opengl/GLSL/issues/8

Cheers,
Nicolai


> 
> Honestly, I'd interpret the spec language above in a way that forbids 
> using struct elements. The other compilers I can test disagree, but both 
> of them aren't exactly known for their high quality...
> 
> Cheers,
> Nicolai
> 
>>
>> On 06/16/2017 01:34 PM, Nicolai Hähnle wrote:
>>> From: Nicolai Hähnle <nicolai.haehnle at amd.com>
>>>
>>> ---
>>>   .../fs-interpolateAtCentroid-struct.frag                | 17 
>>> +++++++++++++++++
>>>   .../fs-interpolateAtOffset-struct.frag                  | 17 
>>> +++++++++++++++++
>>>   .../fs-interpolateAtSampler-struct.frag                 | 17 
>>> +++++++++++++++++
>>>   3 files changed, 51 insertions(+)
>>>   create mode 100644 
>>> tests/spec/arb_gpu_shader5/compiler/builtin-functions/fs-interpolateAtCentroid-struct.frag 
>>>
>>>   create mode 100644 
>>> tests/spec/arb_gpu_shader5/compiler/builtin-functions/fs-interpolateAtOffset-struct.frag 
>>>
>>>   create mode 100644 
>>> tests/spec/arb_gpu_shader5/compiler/builtin-functions/fs-interpolateAtSampler-struct.frag 
>>>
>>>
>>> diff --git 
>>> a/tests/spec/arb_gpu_shader5/compiler/builtin-functions/fs-interpolateAtCentroid-struct.frag 
>>> b/tests/spec/arb_gpu_shader5/compiler/builtin-functions/fs-interpolateAtCentroid-struct.frag 
>>>
>>> new file mode 100644
>>> index 0000000..453c2db
>>> --- /dev/null
>>> +++ 
>>> b/tests/spec/arb_gpu_shader5/compiler/builtin-functions/fs-interpolateAtCentroid-struct.frag 
>>>
>>> @@ -0,0 +1,17 @@
>>> +// [config]
>>> +// expect_result: fail
>>> +// glsl_version: 1.50
>>> +// require_extensions: GL_ARB_gpu_shader5
>>> +// [end config]
>>> +
>>> +#version 150
>>> +#extension GL_ARB_gpu_shader5: require
>>> +
>>> +in struct {
>>> +   vec4 a;
>>> +} a;
>>> +
>>> +void main()
>>> +{
>>> +   interpolateAtCentroid(a.a);
>>> +}
>>> diff --git 
>>> a/tests/spec/arb_gpu_shader5/compiler/builtin-functions/fs-interpolateAtOffset-struct.frag 
>>> b/tests/spec/arb_gpu_shader5/compiler/builtin-functions/fs-interpolateAtOffset-struct.frag 
>>>
>>> new file mode 100644
>>> index 0000000..9b7ee7e
>>> --- /dev/null
>>> +++ 
>>> b/tests/spec/arb_gpu_shader5/compiler/builtin-functions/fs-interpolateAtOffset-struct.frag 
>>>
>>> @@ -0,0 +1,17 @@
>>> +// [config]
>>> +// expect_result: fail
>>> +// glsl_version: 1.50
>>> +// require_extensions: GL_ARB_gpu_shader5
>>> +// [end config]
>>> +
>>> +#version 150
>>> +#extension GL_ARB_gpu_shader5: require
>>> +
>>> +in struct {
>>> +   vec4 a;
>>> +} a;
>>> +
>>> +void main()
>>> +{
>>> +   interpolateAtOffset(a.a, vec2(0));
>>> +}
>>> diff --git 
>>> a/tests/spec/arb_gpu_shader5/compiler/builtin-functions/fs-interpolateAtSampler-struct.frag 
>>> b/tests/spec/arb_gpu_shader5/compiler/builtin-functions/fs-interpolateAtSampler-struct.frag 
>>>
>>> new file mode 100644
>>> index 0000000..78a844e
>>> --- /dev/null
>>> +++ 
>>> b/tests/spec/arb_gpu_shader5/compiler/builtin-functions/fs-interpolateAtSampler-struct.frag 
>>>
>>> @@ -0,0 +1,17 @@
>>> +// [config]
>>> +// expect_result: fail
>>> +// glsl_version: 1.50
>>> +// require_extensions: GL_ARB_gpu_shader5
>>> +// [end config]
>>> +
>>> +#version 150
>>> +#extension GL_ARB_gpu_shader5: require
>>> +
>>> +in struct {
>>> +   vec4 a;
>>> +} a;
>>> +
>>> +void main()
>>> +{
>>> +   interpolateAtSample(a.a, 0);
>>> +}
>>>
>>
> 
> 


-- 
Lerne, wie die Welt wirklich ist,
Aber vergiss niemals, wie sie sein sollte.