[Mesa-dev] [PATCH 1/3] glsl: Compile error if fs defines conflicting qualifiers for gl_FragCoord

[Ian Romanick]

On 02/18/2014 03:36 PM, Anuj Phogat wrote:
> On Tue, Feb 18, 2014 at 11:01 AM, Ian Romanick <idr at freedesktop.org> wrote:
>> On 02/10/2014 05:29 PM, Anuj Phogat wrote:
>>> GLSL 1.50 spec says:
>>>    "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."
>>>
>>> This patch makes the glsl compiler to generate an error if we have a
>>> fragment shader defined with conflicting layout qualifier declarations
>>> for gl_FragCoord. For example:
>>>
>>> layout(origin_upper_left, pixel_center_integer) in vec4 gl_FragCoord;
>>> layout(pixel_center_integer) in vec4 gl_FragCoord;
>>> void main()
>>> {
>>>    gl_FragColor = gl_FragCoord.xyzz;
>>> }
>>>
>>> Cc: <mesa-stable at lists.freedesktop.org>
>>> Signed-off-by: Anuj Phogat <anuj.phogat at gmail.com>
>>> ---
>>>  src/glsl/ast_to_hir.cpp         | 39 +++++++++++++++++++++++++++++++++++++++
>>>  src/glsl/glsl_parser_extras.cpp |  3 +++
>>>  src/glsl/glsl_parser_extras.h   | 10 ++++++++++
>>>  3 files changed, 52 insertions(+)
>>>
>>> diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp
>>> index c89a26b..7d7d89b 100644
>>> --- a/src/glsl/ast_to_hir.cpp
>>> +++ b/src/glsl/ast_to_hir.cpp
>>> @@ -2374,6 +2374,45 @@ apply_type_qualifier_to_variable(const struct ast_type_qualifier *qual,
>>>                      qual_string);
>>>     }
>>>
>>> +   /* Make sure all gl_FragCoord redeclarations specify the same layout
>>> +    * qualifier type.
>>> +    */
>>> +   bool conflicting_pixel_center_integer =
>>> +      state->fs_pixel_center_integer &&
>>> +      !qual->flags.q.pixel_center_integer;
>>> +
>>> +   bool conflicting_origin_upper_left =
>>> +      state->fs_origin_upper_left &&
>>> +      !qual->flags.q.origin_upper_left;
>>
>> I don't think this catches all the cases.  What about
>>
>> layout(origin_upper_left                      ) in vec4 gl_FragCoord;
>> layout(origin_upper_left, pixel_center_integer) in vec4 gl_FragCoord;
>>
> Nice catch. I'll update my patch to include this case. What do you think
> about following two cases?
> case 1:
> in vec4 gl_FragCoord;
> layout(origin_upper_left, pixel_center_integer) in vec4 gl_FragCoord;
> 
> AMD produces no compilation error. This patch matches the behavior.
> 
> case 2:
> layout(origin_upper_left, pixel_center_integer) in vec4 gl_FragCoord;
> in vec4 gl_FragCoord;
> 
> AMD produces compilation error. This patch matches the behavior.

I don't think that's right.  I think they should both produce an error.
 The spec says, "All redeclarations of gl_FragCoord in all fragment
shaders in a single program must have the same set of qualifiers."  I
can't see any reason to give an error for case 2 but not for case 1.

We should also check NVIDIA.