[Piglit] [PATCH] tests/spec/glsl-1.50/execution/fragcoord-layout-qualifiers-conflicting-case-7.shader_test conflict with spec

[Anuj Phogat]

On Thu, Mar 5, 2015 at 10:48 AM, Anuj Phogat <anuj.phogat at gmail.com> wrote:
> On Wed, Mar 4, 2015 at 8:33 PM, Guo, Johney <Weijun.Guo at amd.com> wrote:
>> The conformance test expected link error, Am I wrong ?
>>
>> In the file of conform\cts\gl_cts\GTF\GL3Tests\fragment_coord_conventions\fragment_coord_conventions_32_multishader_error_1.test
>>
>>     <shader name="redeclaration_with_no_layout_qualifiers_and_no_redeclaration_but_use" expecterror="true">
>>     <version api="GL">150 core</version>
>>     <version api="GLES30">300 es</version>
>>     <vertshader>fragment_coord_conventions_32_default.vert</vertshader>
>>     <fragshader>fragment_coord_conventions_32_redeclaration_with_no_layout_qualifiers.frag</fragshader>
>>     <fragshader>fragment_coord_conventions_32_no_redeclaration_but_use.frag</fragshader>
>>     </shader>
>>
>>
> You're right. It got changed after I added the test. See Khronos Bug#12957.
> Seems like people agreed on this change. In that case your patch is:
> Reviewed-by: Anuj Phogat <anuj.phogat at gmail.com>
>
Pushed upstream with minor changes: commit 0433467

>> -----Original Message-----
>> From: Anuj Phogat [mailto:anuj.phogat at gmail.com]
>> Sent: Thursday, March 5, 2015 11:22 AM
>> To: Matt Turner
>> Cc: Guo, Johney; piglit at lists.freedesktop.org; Jordan Justen; Chris Forbes; Lin, Qun; Ian Romanick
>> Subject: Re: [Piglit] [PATCH] tests/spec/glsl-1.50/execution/fragcoord-layout-qualifiers-conflicting-case-7.shader_test conflict with spec
>>
>> On Wed, Mar 4, 2015 at 6:25 PM, Matt Turner <mattst88 at gmail.com> wrote:
>>> On Tue, Mar 3, 2015 at 6:43 PM, Guo, Johney <Weijun.Guo at amd.com> wrote:
>>>>
>>>> According to spec, this test should expected to error  instead of success.
>> I had a discussion with Ian when I added this test to piglit and khronos conformance. We concluded that spec restriction shouldn't apply to a case when you redeclare without any layout qualifiers because it matches the default declaration of gl_FragCoord. NVIDIA proprietary linux driver also allows the test to link. Making this case fail to link might break some application making this harmless mistake.
>>
>>
>>>> The behavior is different with Khronos conformance test GL44-CTS.gtf32.GL3Tests.fragment_coord_conventions.fragment_coord_conventions_32_multishader_error_1   ( subcase redeclaration_with_no_layout_qualifiers_and_no_redeclaration_but_use).
>> Conformance test behavior matches the piglit test behavior.
>>
>>>>
>>>> *     "If gl_FragCoord is redeclared in any fragment shader in a program,
>>>>  *      it must be redeclared in all the fragment shaders in that program
>>>>  *      that have a static use gl_FragCoord. All redeclarations of
>>>>  *      gl_FragCoord in all fragment shaders in a single program must have
>>>>  *      the same set of qualifiers."
>>>>
>>>> Please check it.
>>>> ---
>>>>
>>>> diff --git
>>>> a/tests/spec/glsl-1.50/execution/fragcoord-layout-qualifiers-conflict
>>>> ing-case-7.shader_test
>>>> b/tests/spec/glsl-1.50/execution/fragcoord-layout-qualifiers-conflict
>>>> ing-case-7.shader_test
>>>> index 13605b4..201c75d 100644
>>>> ---
>>>> a/tests/spec/glsl-1.50/execution/fragcoord-layout-qualifiers-conflict
>>>> ing-case-7.shader_test
>>>> +++ b/tests/spec/glsl-1.50/execution/fragcoord-layout-qualifiers-conf
>>>> +++ licting-case-7.shader_test
>>>> @@ -48,4 +48,4 @@ void alpha()
>>>>
>>>>
>>>>  [test]
>>>> -link success
>>>> +link error
>>>
>>> If you make this change, you should update the comment above the
>>> [require] section as well.
>>>
>>> I think I'd have to agree with your interpretation. It seems like "If
>>> gl_FragCoord is redeclared in any fragment shader in a program, it
>>> must be redeclared in all the fragment shaders in that program that
>>> have a static use [of] gl_FragCoord." is pretty clear that this case
>>> should fail to link.
>>>
>>> Anuj, Jordan, Chris?