[Piglit] [PATCH 2/2] glsl-1.50: Add copiler tests to verify gl_FragCoord layout qualifiers

Ian Romanick idr at freedesktop.org
Tue Feb 18 15:31:31 PST 2014


On 02/18/2014 03:17 PM, Anuj Phogat wrote:
> On Thu, Feb 13, 2014 at 5:52 PM, Anuj Phogat <anuj.phogat at gmail.com> wrote:
>>
>> On Tue, Feb 11, 2014 at 11:07 AM, Anuj Phogat <anuj.phogat at gmail.com> wrote:
>>>
>>>
>>>
>>> On Mon, Feb 10, 2014 at 6:42 PM, Ian Romanick <idr at freedesktop.org> wrote:
>>>>
>>>> I think you mean "compiler" in the subject. :)
>>>
>>> yeah. "copiler" is still too futuristic :)
>>>>
>>>>
>>>> On 02/10/2014 06:15 PM, Anuj Phogat wrote:
>>>>> Signed-off-by: Anuj Phogat <anuj.phogat at gmail.com>
>>>>> ---
>>>>>  .../layout-qualifiers-conflicting-case-1.frag      | 37
>>>>> ++++++++++++++++++++++
>>>>>  .../layout-qualifiers-conflicting-case-2.frag      | 37
>>>>> ++++++++++++++++++++++
>>>>>  .../layout-qualifiers-matching.frag                | 37
>>>>> ++++++++++++++++++++++
>>>>>  3 files changed, 111 insertions(+)
>>>>>  create mode 100644
>>>>> tests/spec/glsl-1.50/compiler/fragment_coord_conventions/layout-qualifiers-conflicting-case-1.frag
>>>>>  create mode 100644
>>>>> tests/spec/glsl-1.50/compiler/fragment_coord_conventions/layout-qualifiers-conflicting-case-2.frag
>>>>>  create mode 100644
>>>>> tests/spec/glsl-1.50/compiler/fragment_coord_conventions/layout-qualifiers-matching.frag
>>>>>
>>>>> diff --git
>>>>> a/tests/spec/glsl-1.50/compiler/fragment_coord_conventions/layout-qualifiers-conflicting-case-1.frag
>>>>> b/tests/spec/glsl-1.50/compiler/fragment_coord_conventions/layout-qualifiers-conflicting-case-1.frag
>>>>> new file mode 100644
>>>>> index 0000000..d574e4f
>>>>> --- /dev/null
>>>>> +++
>>>>> b/tests/spec/glsl-1.50/compiler/fragment_coord_conventions/layout-qualifiers-conflicting-case-1.frag
>>>>> @@ -0,0 +1,37 @@
>>>>> +/* [config]
>>>>> + * expect_result: fail
>>>>> + * glsl_version: 1.50
>>>>> + * check_link: false
>>>>> + * [end config]
>>>>> + */
>>>>> +
>>>>
>>>> I would just combine these comments.
>>>
>>> ok
>>>>
>>>>
>>>>> +/* Section 4.3.8.1 (Input Layout Qualifiers) of the GLSL 1.50 spec
>>>>> says:
>>>>> + *
>>>>> + *     "Fragment shaders can have an input layout only for redeclaring
>>>>> the
>>>>> + *     built-in variable gl_FragCoord (see section 7.2 Fragment Shader
>>>>> + *     Special Variables). The layout qualifier identifiers for
>>>>> + *     gl_FragCoord are
>>>>> + *
>>>>> + *     layout-qualifier-id:
>>>>> + *         origin_upper_left
>>>>> + *         pixel_center_integer"
>>>>> + *
>>>>> + *     "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."
>>>>
>>>> Do later versions of the spec give any guidance about when these errors
>>>> should be generated?  Somewhere in the 4.x series we started being
>>>> specific.  Before that, it was ambiguous.  Some implementations
>>>> generated the error at compile time, while others generated the error at
>>>> link time.
>>>
>>> No additional details in later versions of spec. They all use same text
>>> which
>>> I quoted in above comment. It looks more sensible to generate compile error
>>> for these tests. Do you want me to find out what NVIDIA and AMD does?
>>>
>> Both these tests pass on NVIDIA's proprietary drivers.
>>
>> Interesting thing is that all fragcoord-layout-qualifiers-conflicting* tests
>> added in '[PATCH 1/2] glsl-1.50: Add shader tests to verify gl_FragCoord
>> layout qualifiers' fails on NVIDIA. No link error for conflicting qualifiers in
>> different shaders. This behavior is clearly not following spec.
>>
> Both the tests pass on AMD as well.

If you add that information (with driver versions, like commit 9e34509)
to the commit message, the series is

Reviewed-by: Ian Romanick <ian.d.romanick at intel.com>

>>>>> + *
>>>>> + * Tests the conflicting redeclarations of gl_FragCoord within same
>>>>> fragment
>>>>> + * shader.
>>>>> + */
>>>>> +
>>>>> +#version 150
>>>>> +
>>>>> +layout(origin_upper_left, pixel_center_integer) in vec4 gl_FragCoord;
>>>>> +layout(origin_upper_left) in vec4 gl_FragCoord;
>>>>> +
>>>>> +void main()
>>>>> +{
>>>>> +     gl_FragColor = gl_FragCoord.xyzz;
>>>>> +}
>>>>> diff --git
>>>>> a/tests/spec/glsl-1.50/compiler/fragment_coord_conventions/layout-qualifiers-conflicting-case-2.frag
>>>>> b/tests/spec/glsl-1.50/compiler/fragment_coord_conventions/layout-qualifiers-conflicting-case-2.frag
>>>>> new file mode 100644
>>>>> index 0000000..eb8976c
>>>>> --- /dev/null
>>>>> +++
>>>>> b/tests/spec/glsl-1.50/compiler/fragment_coord_conventions/layout-qualifiers-conflicting-case-2.frag
>>>>> @@ -0,0 +1,37 @@
>>>>> +/* [config]
>>>>> + * expect_result: fail
>>>>> + * glsl_version: 1.50
>>>>> + * check_link: false
>>>>> + * [end config]
>>>>> + */
>>>>> +
>>>>> +/* Section 4.3.8.1 (Input Layout Qualifiers) of the GLSL 1.50 spec
>>>>> says:
>>>>> + *
>>>>> + *     "Fragment shaders can have an input layout only for redeclaring
>>>>> the
>>>>> + *     built-in variable gl_FragCoord (see section 7.2 Fragment Shader
>>>>> + *     Special Variables). The layout qualifier identifiers for
>>>>> + *     gl_FragCoord are
>>>>> + *
>>>>> + *     layout-qualifier-id:
>>>>> + *         origin_upper_left
>>>>> + *         pixel_center_integer"
>>>>> + *
>>>>> + *     "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."
>>>>> + *
>>>>> + * Tests the conflicting redeclarations in a fragment shader cause the
>>>>> compile
>>>>> + * failure even if gl_FragCoord is not used.
>>>>> + */
>>>>> +
>>>>> +#version 150
>>>>> +
>>>>> +layout(origin_upper_left, pixel_center_integer) in vec4 gl_FragCoord;
>>>>> +layout(origin_upper_left) in vec4 gl_FragCoord;
>>>>> +
>>>>> +void main()
>>>>> +{
>>>>> +     gl_FragColor = vec4(1.0);
>>>>> +}
>>>>> diff --git
>>>>> a/tests/spec/glsl-1.50/compiler/fragment_coord_conventions/layout-qualifiers-matching.frag
>>>>> b/tests/spec/glsl-1.50/compiler/fragment_coord_conventions/layout-qualifiers-matching.frag
>>>>> new file mode 100644
>>>>> index 0000000..63942cc
>>>>> --- /dev/null
>>>>> +++
>>>>> b/tests/spec/glsl-1.50/compiler/fragment_coord_conventions/layout-qualifiers-matching.frag
>>>>> @@ -0,0 +1,37 @@
>>>>> +/* [config]
>>>>> + * expect_result: pass
>>>>> + * glsl_version: 1.50
>>>>> + * check_link: false
>>>>> + * [end config]
>>>>> + */
>>>>> +
>>>>> +/* Section 4.3.8.1 (Input Layout Qualifiers) of the GLSL 1.50 spec
>>>>> says:
>>>>> + *
>>>>> + *     "Fragment shaders can have an input layout only for redeclaring
>>>>> the
>>>>> + *     built-in variable gl_FragCoord (see section 7.2 Fragment Shader
>>>>> + *     Special Variables). The layout qualifier identifiers for
>>>>> + *     gl_FragCoord are
>>>>> + *
>>>>> + *     layout-qualifier-id:
>>>>> + *         origin_upper_left
>>>>> + *         pixel_center_integer"
>>>>> + *
>>>>> + *     "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."
>>>>> + *
>>>>> + * Tests the matching redeclarations of gl_FragCoord within same
>>>>> fragment
>>>>> + * shader.
>>>>> + */
>>>>> +
>>>>> +#version 150
>>>>> +
>>>>> +layout(origin_upper_left, pixel_center_integer) in vec4 gl_FragCoord;
>>>>> +layout(origin_upper_left, pixel_center_integer) in vec4 gl_FragCoord;
>>>>> +
>>>>> +void main()
>>>>> +{
>>>>> +     gl_FragColor = gl_FragCoord.xyzz;
>>>>> +}



More information about the Piglit mailing list