[Mesa-dev] [PATCH v2 2/3] glsl: allow built-in variables to be explicitly declared

[Timothy Arceri]

Ian can I get your thoughts on this series?

On 12/05/18 14:49, Timothy Arceri wrote:
> Mesa seems to be the only implementation that doesn't allow builtins
> to be explicitly declared. The GLSL 1.30 spec seems to imply that
> buitins may be explicitly declared.
> 
> This this allows the game "Full Bore" the be playable (when using
> MESA_GL_VERSION_OVERRIDE=3.3COMPAT). It will also allow us to
> remove the allow_glsl_builtin_variable_redeclaration dri override.
> 
>  From the GLSL 1.30 spec Section 7.2 (Fragment Shader Special
> Variables):
> 
>      "Both gl_FragColor and gl_FragData are deprecated; the preferred
>      usage is to explicitly declare these outputs in the fragment
>      shader using the out storage qualifier."
> 
> To avoid some GLSL ES tests failing we add a check to make sure
> precision matches on the redeclared builtin.
> ---
>   src/compiler/glsl/ast_to_hir.cpp | 32 ++++++++++++++++++++++----------
>   1 file changed, 22 insertions(+), 10 deletions(-)
> 
> diff --git a/src/compiler/glsl/ast_to_hir.cpp b/src/compiler/glsl/ast_to_hir.cpp
> index a7a9ac80769..54d0816a986 100644
> --- a/src/compiler/glsl/ast_to_hir.cpp
> +++ b/src/compiler/glsl/ast_to_hir.cpp
> @@ -4390,14 +4390,8 @@ get_variable_being_redeclared(ir_variable **var_ptr, YYLTYPE loc,
>         earlier->data.precision = var->data.precision;
>         earlier->data.memory_coherent = var->data.memory_coherent;
>   
> -   } else if (earlier->data.how_declared == ir_var_declared_implicitly &&
> -              state->allow_builtin_variable_redeclaration) {
> -      /* Allow verbatim redeclarations of built-in variables. Not explicitly
> -       * valid, but some applications do it.
> -       */
> -      if (earlier->data.mode != var->data.mode &&
> -          !(earlier->data.mode == ir_var_system_value &&
> -            var->data.mode == ir_var_shader_in)) {
> +   } else if (allow_all_redeclarations) {
> +      if (earlier->data.mode != var->data.mode) {
>            _mesa_glsl_error(&loc, state,
>                             "redeclaration of `%s' with incorrect qualifiers",
>                             var->name);
> @@ -4406,8 +4400,22 @@ get_variable_being_redeclared(ir_variable **var_ptr, YYLTYPE loc,
>                             "redeclaration of `%s' has incorrect type",
>                             var->name);
>         }
> -   } else if (allow_all_redeclarations) {
> -      if (earlier->data.mode != var->data.mode) {
> +   } else if (earlier->data.how_declared == ir_var_declared_implicitly) {
> +      /* Allow verbatim redeclarations of built-in variables. The GLSL 1.30
> +       * spec seems to imply that buitins may be explicitly declared.
> +       *
> +       * From the GLSL 1.30 spec Section 7.2 (Fragment Shader Special
> +       * Variables):
> +       *
> +       *    "Both gl_FragColor and gl_FragData are deprecated; the preferred
> +       *    usage is to explicitly declare these outputs in the fragment
> +       *    shader using the out storage qualifier."
> +       */
> +      enum ir_variable_mode builtin_mode =
> +         glsl_external_mode((ir_variable_mode) earlier->data.mode,
> +                            state->stage, earlier->data.location);
> +
> +      if (builtin_mode != var->data.mode) {
>            _mesa_glsl_error(&loc, state,
>                             "redeclaration of `%s' with incorrect qualifiers",
>                             var->name);
> @@ -4415,6 +4423,10 @@ get_variable_being_redeclared(ir_variable **var_ptr, YYLTYPE loc,
>            _mesa_glsl_error(&loc, state,
>                             "redeclaration of `%s' has incorrect type",
>                             var->name);
> +      } else if (earlier->data.precision != var->data.precision) {
> +         _mesa_glsl_error(&loc, state,
> +                          "redeclaration of `%s' has incorrect precision",
> +                          var->name);
>         }
>      } else {
>         _mesa_glsl_error(&loc, state, "`%s' redeclared", var->name);
>