[Mesa-dev] [PATCH] glsl: Allow precision qualifiers for sampler types

[Ian Romanick]

On 08/27/2013 10:45 AM, Anuj Phogat wrote:
> GLSL 1.30 doesn't allow precision qualifiers on sampler types,
> but in GLSL ES, sampler types are also allowed. This seems like
> an oversight (since the intention of including these in GLSL 1.30
> is to allow compatibility with ES shaders).
>
> Currently, Mesa allows "default" precision qualifiers to be set for
> sampler types in GLSL (commit d5948f2). This patch makes it follow
> GLSL ES rules and also allow declaring sampler variables with a
> precision qualifier in GLSL.

I think our current behavior is incorrect even in the ES case.  GLSL ES 
3.30 and desktop GLSL 4.40 say the following in section 4.5.3 (Precision 
Qualifiers):

     "Any floating point or any integer declaration can have the type
     preceded by one of these precision qualifiers..."

The also both say the following in section 4.5.4 (Default Precision 
Qualifiers):

     "The precision statement...can be used to establish a default
     precision qualifier. The type field can be either int or float
     or any of the sampler types..."

So I believe

precision mediump sampler2D;

should be legal in all versions, but

uniform mediump sampler2D s;

should not.

Which syntax is the test using?

> This fixes a shader compilation error in Khronos OpenGL conformance
> test "depth_texture_mipmap".
>
> Signed-off-by: Anuj Phogat <anuj.phogat at gmail.com
> ---
>   src/glsl/ast_to_hir.cpp | 14 +++++++++-----
>   1 file changed, 9 insertions(+), 5 deletions(-)
>
> diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp
> index 192130a..b3d6d8c 100644
> --- a/src/glsl/ast_to_hir.cpp
> +++ b/src/glsl/ast_to_hir.cpp
> @@ -3131,8 +3131,8 @@ ast_declarator_list::hir(exec_list *instructions,
>            state->check_precision_qualifiers_allowed(&loc);
>         }
>
> -
> -      /* Precision qualifiers only apply to floating point and integer types.
> +      /* Precision qualifiers apply to floating point, integer and sampler
> +       * types.
>          *
>          * From section 4.5.2 of the GLSL 1.30 spec:
>          *    "Any floating point or any integer declaration can have the type
> @@ -3144,20 +3144,24 @@ ast_declarator_list::hir(exec_list *instructions,
>          *
>          * From page 87 of the GLSL ES spec:
>          *    "RESOLUTION: Allow sampler types to take a precision qualifier."
> +       *
> +       * GLSL 1.30 doesn't allow precision qualifiers on sampler types, but
> +       * this seems like an oversight (since the intention of including these
> +       * in GLSL 1.30 is to allow compatibility with ES shaders). So we allow
> +       * int, float, and all sampler types regardless of GLSL version.
>          */
>         if (this->type->qualifier.precision != ast_precision_none
>             && !var->type->is_float()
>             && !var->type->is_integer()
>             && !var->type->is_record()
> -          && !(var->type->is_sampler() && state->es_shader)
> +          && !(var->type->is_sampler())
>             && !(var->type->is_array()
>                  && (var->type->fields.array->is_float()
>                      || var->type->fields.array->is_integer()))) {
>
>            _mesa_glsl_error(&loc, state,
>                             "precision qualifiers apply only to floating point"
> -                          "%s types", state->es_shader ? ", integer, and sampler"
> -						       : "and integer");
> +                          ", integer and sampler types");
>         }
>
>         /* From page 17 (page 23 of the PDF) of the GLSL 1.20 spec:
>