[Mesa-dev] [PATCH] glsl: Trigger precision mismatch error only if both symbols are used

[Tomasz Figa]

Hi Ian,

On Tue, Sep 26, 2017 at 5:59 AM, Ian Romanick <idr at freedesktop.org> wrote:
> On 09/25/2017 02:53 AM, Tomasz Figa wrote:
>> Commit 259fc505454ea6a67aeacf6cdebf1398d9947759 added linker error for
>> mismatching uniform precision, as required by GLES specification and
>> conformance test-suite.
>>
>> Several Android applications, including Forge of Empires, have shaders
>> which violate this rule, on a dead varying that will be eliminated.
>> The problem affects a big number of applications using Cocos2D engine
>> and other GLES implementations accept this, so work around the problem
>> by erroring out only if both symbols are actually referenced in the
>> code, which is the only case when the mismatch can cause incorrect
>> behavior.
>
> I think this change will cause failures in the CTS... but, there's a
> Khronos bug about this issue, and I think some of the rules are going to
> get relaxed.  I'd like to wait until that gets sorted before we start
> changing how our linker applies the rules.

Are we 100% sure about the failures? There are multiple GLES
implementations that claim to be compliant, yet they allow such
behavior.

Note that this is affecting a huge number of Android applications,
including a commonly used 2D game engine and we're having a really big
problem right now, because even if the engine gets fixed, it's very
unlikely that all the developers update their engine.

>
>> Based on Kenneth Graunke's patch from Bugzilla, reworked from a drirc
>> option that completely bypasses the check into an incoditional check
>
>                                                     unconditional
>
>> that triggers either an error or warning, respectively if both
>> declarations are further referenced by the code or not.
>>
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97532
>> Signed-off-by: Tomasz Figa <tfiga at chromium.org>
>> ---
>>  src/compiler/glsl/linker.cpp | 14 ++++++++++----
>>  1 file changed, 10 insertions(+), 4 deletions(-)
>>
>> diff --git a/src/compiler/glsl/linker.cpp b/src/compiler/glsl/linker.cpp
>> index f352c5385ca..1c534ea1a3b 100644
>> --- a/src/compiler/glsl/linker.cpp
>> +++ b/src/compiler/glsl/linker.cpp
>> @@ -1121,10 +1121,16 @@ cross_validate_globals(struct gl_shader_program *prog,
>>           if (prog->IsES && (prog->data->Version != 310 ||
>>
>
> Does just removing the "prog->data->Version != 310" clause also fix the
> problem?

Will check later today, but I suspect it would, since the applications
are not GLES 3.1.

Best regards,
Tomasz