Hi Ian, An update on my testing inline.
On Tue, Sep 26, 2017 at 7:13 AM, Tomasz Figa <tf...@chromium.org> wrote: > On Tue, Sep 26, 2017 at 6:21 AM, Tomasz Figa <tf...@chromium.org> wrote: >> Hi Ian, >> >> On Tue, Sep 26, 2017 at 5:59 AM, Ian Romanick <i...@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. > > I'm looking through Khronos CTS github repository and it seems like a > related test > (dEQP-GLES3.functional.shaders.linkage.uniform.block.differing_precision) > was removed from GLES 3 suite and instead few others added to GLES 3.1 > suite: > > https://github.com/KhronosGroup/VK-GL-CTS/commit/b8d0d0a842280c2594703ecce985563aebe1cde8#diff-72bd3f02df978e071f2a53ec2654ddd5 > > Moreover I can see a test being included in GLES 3.2 suite > (KHR-GLES32.shaders.negative.unused_uniform_precision_matching) that > prohibits precision mismatch even for unused uniforms, but it is not > included in any suites for earlier GLES versions. Just ran the whole suite for GLES 3.0 conformance run with i965 driver on Android and there is only 1 failure that seems to be unrelated to the workaround (fails both with and without the change): KHR-GLES3.shaders.uniform_block.common.precision_matching However, I just realized that the applications affected are all using GLSL ES 1.00. So, if we turn the condition introduced by my patch into "(existing->data.used && var->data.used) || prog->data->Version >= 300", we should be safe both on compatibility and conformance side. >> >> 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 <tf...@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. I just tested it now and it doesn't fix the problem. However, I misunderstood the code before. The shaders affected are not using uniform blocks, so "!var->get_interface_type()" hits and leads to the error. Best regards, Tomasz _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
