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

[Anuj Phogat]

On Tue, Aug 27, 2013 at 11:53 AM, Ian Romanick <idr at freedesktop.org> wrote:
> 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
You mean to say GLSL ES 3.00?
> 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..."
>
Yes, samplers are now allowed in GLSL 4.4. They were not in GLSL 4.3.

> 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.
>
Yes, there is no clear statement in GLSL spec which allows:
uniform mediump sampler2D s;

> Which syntax is the test using?
>
test uses:
uniform mediump sampler2D s;

I haven't yet tested if it is accepted by NVIDIA.
>
>> 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:
>>
>