[Mesa-dev] [PATCH] glsl/linker: fix bug when checking precision qualifier

[Samuel Iglesias Gonsálvez]

On 27/02/18 13:37, Tapani Pälli wrote:
>
>
> On 01/30/2018 09:50 AM, Samuel Iglesias Gonsálvez wrote:
>> According to GLSL ES 3.2 spec, see table in 9.2.1 "Linked Shaders"
>> section, the precision qualifier should match for uniform variables.
>> This also applies to previous GLSL ES 3.x specs.
>>
>> This 'if' checks the condition for uniform variables, while for UBOs
>> it is checked in link_interface_blocks.cpp.
>>
>> Fixes: b50b82b8a553
>> ("glsl/es31: precision qualifier doesn't need to match in shader
>> interface block members")
>>
>> Signed-off-by: Samuel Iglesias Gonsálvez <siglesias at igalia.com>
>> ---
>>   src/compiler/glsl/linker.cpp | 12 ++++--------
>>   1 file changed, 4 insertions(+), 8 deletions(-)
>>
>> diff --git a/src/compiler/glsl/linker.cpp b/src/compiler/glsl/linker.cpp
>> index ce101935b01..050b2906f6b 100644
>> --- a/src/compiler/glsl/linker.cpp
>> +++ b/src/compiler/glsl/linker.cpp
>> @@ -1134,15 +1134,11 @@ cross_validate_globals(struct
>> gl_shader_program *prog,
>>                 return;
>>            }
>> -         /* Only in GLSL ES 3.10, the precision qualifier should not
>> match
>> -          * between block members defined in matched block names
>> within a
>> -          * shader interface.
>> -          *
>> -          * In GLSL ES 3.00 and ES 3.20, precision qualifier for
>> each block
>> -          * member should match.
>> +
>> +         /* Check the precision qualifier matches for uniform
>> variables. For UBOs,
>> +          * it is checked in link_interface_blocks.cpp.
>
> I think this change is correct. However this comment started to bother
> me, there was commit fc6d55952d08ea03d133c1178871b0d4d289a0cf that
> says "glsl/es: precision qualifier doesn't need to match in UBOs". Do
> we have precision qualifier matching for UBOs?
>

On GLSL ES they don't need to match, only on desktop GLSL. My intention
here was to indicate where such check is done for UBO inside Mesa for
desktop GLSL, not implying it is needed for GLSL ES at all. Sorry for
the confusion.

I can change the comment as:

"Check the precision qualifier matches for uniform variables on GLSL ES"

... and forget about the rest, if you agree.

Sam

>
>>             */
>> -         if (prog->IsES && (prog->data->Version != 310 ||
>> -                            !var->get_interface_type()) &&
>> +         if (prog->IsES && !var->get_interface_type() &&
>>                existing->data.precision != var->data.precision) {
>>               if ((existing->data.used && var->data.used) ||
>> prog->data->Version >= 300) {
>>                  linker_error(prog, "declarations for %s `%s` have "
>>
>

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