Hi Ian, Kenneth, On Wed, Sep 27, 2017 at 2:57 AM, Tomasz Figa <tf...@chromium.org> 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 uniforms that are declared but not used > further in shader code. The problem affects a big number of Android > games, including ones built on top of one of the common 2D graphics > engines and other GLES implementations accept this, which 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. > > v3: > - Add a comment explaining the behavior. > - Fix bad copy/paste in commit message (s/varyings/uniforms).
Would you be able to take a look? Ian, I believe your previous NAK was due to the confusing erroneous copy/paste from the freedesktop bug I made in commit message. Looking at last comment from Kenneth there, we were going to go with my patch, but things remained quiet after that. Thanks, Tomasz > v2: > - Change the behavior only for GLSL ES 1.00 shaders. > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97532 > Signed-off-by: Tomasz Figa <tf...@chromium.org> > --- > src/compiler/glsl/linker.cpp | 32 ++++++++++++++++++++++++++++---- > 1 file changed, 28 insertions(+), 4 deletions(-) > > diff --git a/src/compiler/glsl/linker.cpp b/src/compiler/glsl/linker.cpp > index f352c5385ca..693c5ae3664 100644 > --- a/src/compiler/glsl/linker.cpp > +++ b/src/compiler/glsl/linker.cpp > @@ -1121,10 +1121,34 @@ 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; > + /* > + * GLSL ES 3.00 is the first version that explicitly requires > + * that uniform declarations have matching precision qualifiers. > + * > + * The only relevant part of GLSL ES 1.00 spec, > + * > + * "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." > + * > + * leaves too wide field for intepretation. However, judging by > + * applications and implementations existing in the wild, it > seems > + * to be widely assumed that declarations alone are not enough to > + * fail the link. > + * > + * Thus, in case of GLSL ES < 3.00, trigger an error only if the > + * uniform is actually referenced in the code of both shaders. > + */ > + 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); > -- > 2.14.1.992.g2c7b836f3a-goog > _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
