On Wed, Sep 27, 2017 at 9:29 AM, Ian Romanick <i...@freedesktop.org> wrote: > NAK. > > You mention varyings in the commit message, but then you quote spec text > about uniforms.
Sorry, the part about varyings was a bad copy paste from the old workaround patch on the bugzilla. The problem here is about uniforms. > This bugged me, so I actually went and looked at the > GLSL ES 1.00 specification. > > Section 4.3.5 (Varying) of the GLSL ES 1.00 specification says: > > The precision of varying variables does not need to match. > > SO... for GLSL ES 1.00 we should not even emit a warning for varyings > with mismatched precision. The above spec quotation should appear in > the patch that does this. We should also add a piglit test and, > frankly, a CTS test! We've been rejecting completely valid shaders for > over a year now, and the CTS should have caught that. > > If there are actually shaders in the wild that expect mismatched > precision on uniforms in GLSL ES 1.00 shaders, we should deal with that > in a separate patch. Given the commit message corrected to refer to "uniforms" instead, would it remove your NAK? Best regards, Tomasz > > On 09/26/2017 01:35 AM, Tomasz Figa wrote: >> Commit 259fc505454ea6a67aeacf6cdebf1398d9947759 added linker error for >> mismatching uniform precision, as required by GLES 3.0 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, this poses a serious >> application compatibility issue. >> >> Starting from GLSL ES 3.0, declarations with conflicting precision >> qualifiers are explicitly prohibited. However GLSL ES 1.00 does not >> clearly specify the behavior, except that >> >> "Uniforms are defined to behave as if they are using the same storage in >> the vertex and fragment processors and may be implemented this way. >> If uniforms are used in both the vertex and fragment shaders, developers >> should be warned if the precisions are different. Conversion of >> precision should never be implicit." >> >> The word "used" is not clear in this context and might refer to >> 1) declared (same as GLES 3.x) >> 2) referred after post-processing, or >> 3) linked after all optimizations are done. >> >> Looking at existing applications, 2) or 3) seems to be widely adopted. >> To avoid compatibility issues, turn the error into a warning if GLSL ES >> version is lower than 3.0 and the data is dead in at least one of the >> shaders. >> >> 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..ff2e6b7a0d3 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 || >> !var->get_interface_type()) && >> existing->data.precision != var->data.precision) { >> - linker_error(prog, "declarations for %s `%s` have " >> - "mismatching precision qualifiers\n", >> - mode_string(var), var->name); >> - return; >> + if ((existing->data.used && var->data.used) || >> prog->data->Version >= 300) { >> + linker_error(prog, "declarations for %s `%s` have " >> + "mismatching precision qualifiers\n", >> + mode_string(var), var->name); >> + return; >> + } else { >> + linker_warning(prog, "declarations for %s `%s` have " >> + "mismatching precision qualifiers\n", >> + mode_string(var), var->name); >> + } >> } >> } else >> variables->add_variable(var); >> > _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev