On Wed, 2018-10-31 at 12:01 -0400, Ilia Mirkin wrote: > I had to do a double (or triple) take on this logic as well. Part of > the subtlety is that the fallback only applies for ES when there's a > match but no exact match. Probably good to mention this.
Yeah, that makes sense. I thought I mentioneded this in the commit message, but perhaps you want that to be more explicit than the "In GLES, "-introduction? How about I simply add something like "This fallback should only affect GLES." at the end of the commit message? > On Wed, Oct 31, 2018 at 11:13 AM Erik Faye-Lund > <erik.faye-l...@collabora.com> wrote: > > > > On Wed, 2018-10-31 at 15:46 +0200, Tapani Pälli wrote: > > > > > > On 10/30/18 7:11 PM, Erik Faye-Lund wrote: > > > > In GLES, we currently either need an exact match with a local > > > > function, > > > > or an exact match with a builtin. > > > > > > > > However, if we add support for implicit conversions for GLES > > > > shaders, > > > > we also need to fall back to a non-exact match in the case > > > > where > > > > there > > > > were no builtin match either. > > > > > > > > Luckily, we already have a variable ready with this, so let's > > > > just > > > > return it if the builtin-search failed. > > > > > > > > Signed-off-by: Erik Faye-Lund <erik.faye-l...@collabora.com> > > > > --- > > > > src/compiler/glsl/ast_function.cpp | 2 +- > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > diff --git a/src/compiler/glsl/ast_function.cpp > > > > b/src/compiler/glsl/ast_function.cpp > > > > index 1fa3f7561ae..50fd8bc5f5d 100644 > > > > --- a/src/compiler/glsl/ast_function.cpp > > > > +++ b/src/compiler/glsl/ast_function.cpp > > > > @@ -667,7 +667,7 @@ match_function_by_name(const char *name, > > > > /* Local shader has no exact candidates; check the built- > > > > ins. > > > > */ > > > > _mesa_glsl_initialize_builtin_functions(); > > > > sig = _mesa_glsl_find_builtin_function(state, name, > > > > actual_parameters); > > > > - return sig; > > > > + return sig ? sig : local_sig; > > > > > > I think this would deserve a comment about 'local_sig', it did > > > not > > > seem > > > obvious ... maybe something like: > > > > > > /* Variable local_sig contains the result from > > > choose_best_inexact_overload(). */ > > > > > > > Yeah, good point. I've changed it to this, which feels like it > > explains > > the point without spoon-feeding the reader too much... How does > > that > > sound? > > > > ---8<--- > > diff --git a/src/compiler/glsl/ast_function.cpp > > b/src/compiler/glsl/ast_function.cpp > > index 1fa3f7561ae..6aa30400060 100644 > > --- a/src/compiler/glsl/ast_function.cpp > > +++ b/src/compiler/glsl/ast_function.cpp > > @@ -667,7 +667,11 @@ match_function_by_name(const char *name, > > /* Local shader has no exact candidates; check the built-ins. > > */ > > _mesa_glsl_initialize_builtin_functions(); > > sig = _mesa_glsl_find_builtin_function(state, name, > > actual_parameters); > > - return sig; > > + > > + /* if _mesa_glsl_find_builtin_function failed, fall back to the > > result > > + * of choose_best_inexact_overload() instead > > + */ > > + return sig ? sig : local_sig; > > } > > > > static ir_function_signature * > > ---8<--- > > > > > > > > } > > > > > > > > static ir_function_signature * > > > > > > > > > > _______________________________________________ > > > 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 _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
