Hi,
On 26.09.2017 11:33, Tomasz Figa wrote:
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.
Btw. If you're also interested about performance, there's a difference
between GLES 3.x and earlier GLES version semantics that affects
performance.
According to the GLES 3.0 spec, cubemap sampling is required to always
be seamless (sampling from multiple surfaces near the edges), but in
earlier GLES versions, this isn't required (on GL side, it's application
decision, so there it doesn't matter).
For anything using 1x1 / 2x2 cubemaps, this can have clear performance.
Do many Android applications use them?
This is relevant as Mesa uses GLES 3 semantics also for GLES 2:
------------- src/mesa/main/texstate.c -------------
/* Appendix F.2 of the OpenGL ES 3.0 spec says:
*
* "OpenGL ES 3.0 requires that all cube map filtering be
* seamless. OpenGL ES 2.0 specified that a single cube map face be
* selected and used for filtering."
*
* Unfortunatley, a call to _mesa_is_gles3 below will only work if
* the driver has already computed and set ctx->Version, however drivers
* seem to call _mesa_initialize_context (which calls this) early
* in the CreateContext hook and _mesa_compute_version much later (since
* it needs information about available extensions). So, we will
* enable seamless cubemaps by default since GLES2. This should work
* for most implementations and drivers that don't support seamless
* cubemaps for GLES2 can still disable it.
*/
ctx->Texture.CubeMapSeamless = ctx->API == API_OPENGLES2;
----------------------------------------------------
- Eero
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
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
