[Mesa-dev] [PATCH 1/2] glsl: Fix redeclaration rules for gl_FragCoord.

[Kenneth Graunke]

On 04/12/2014 10:30 PM, Chris Forbes wrote:
> The ARB_fragment_coord_conventions spec, section 4.3.x (Input Layout
> Qualifiers) says:
> 
>   "All redeclarations of gl_FragCoord in all fragment shaders in a
>   single program must have the same set of qualifiers. Within any
>   shader, the first redeclarations of gl_FragCoord must appear before
>   any use of gl_FragCoord."
> 
> Fixes piglit's tests:
>    arb_fragment_coord_conventions/compiler/redeclaration-after-use
>    arb_fragment_coord_conventions/compiler/redeclaration-around-use
>    glsl-1.50/compiler/fragment_coord_conventions/layout-qualifiers-conflicting-case-1
>    glsl-1.50/compiler/fragment_coord_conventions/layout-qualifiers-conflicting-case-2
> 
> Signed-off-by: Chris Forbes <chrisf at ijw.co.nz>
> ---
>  src/glsl/ast_to_hir.cpp | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp
> index 9b9d511..d1eb3e2 100644
> --- a/src/glsl/ast_to_hir.cpp
> +++ b/src/glsl/ast_to_hir.cpp
> @@ -2715,9 +2715,29 @@ get_variable_being_redeclared(ir_variable *var, YYLTYPE loc,
>        /* Allow redeclaration of gl_FragCoord for ARB_fcc layout
>         * qualifiers.
>         */
> +
> +      if (earlier->data.how_declared != ir_var_declared_implicitly
> +          && (earlier->data.origin_upper_left != var->data.origin_upper_left
> +             || earlier->data.pixel_center_integer != var->data.pixel_center_integer)) {
> +         _mesa_glsl_error(&loc, state,
> +                          "Inconsistent redeclarations of gl_FragCoord");
> +      }
> +
>        earlier->data.origin_upper_left = var->data.origin_upper_left;
>        earlier->data.pixel_center_integer = var->data.pixel_center_integer;
>  
> +      if (earlier->data.used &&
> +          earlier->data.how_declared == ir_var_declared_implicitly) {
> +         /* ARB_fragment_coord_conventions spec Section 4.3.x.1
> +          * (Input Layout Qualifier) says:
> +          *
> +          * "Within any shader, the first redeclarations of gl_FragCoord
> +          * must appear before any use of gl_FragCoord."
> +          */
> +         _mesa_glsl_error(&loc, state,
> +                          "First redeclaration of gl_FragCoord must appear before any use");
> +      }
> +
>        /* According to section 4.3.7 of the GLSL 1.30 spec,
>         * the following built-in varaibles can be redeclared with an
>         * interpolation qualifier:
> 

It seems like this will catch multiple, inconsistent redeclarations
within a single fragment shader...but the spec text quoted indicates
that redeclarations in all fragment shaders being linked together into a
single program need to be consistent.

I don't see any code to handle that in linker*.cpp, so presumably we
ought to add that as well.  Regardless, this is a good improvement!

Both patches are:
Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>

Thanks, Chris.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: OpenPGP digital signature
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20140416/a6a0ad27/attachment.sig>