On Wednesday, February 18, 2015 12:00:39 PM Matt Turner wrote:
> On Mon, Dec 1, 2014 at 5:04 AM, Eduardo Lima Mitev <el...@igalia.com> wrote:
> > From: Samuel Iglesias Gonsalvez <sigles...@igalia.com>
> >
> > Create a new search function to look for matching built-in functions by name
> > and use it for built-in function redefinition or overload in GLSL ES 3.00.
> >
> > GLSL ES 3.0 spec, chapter 6.1 "Function Definitions", page 71
> >
> > "A shader cannot redefine or overload built-in functions."
> >
> > In case of GLSL ES 1.0 spec, chapter 6.1 "Function Definitions", page 54
> >
> > "Function names can be overloaded. This allows the same function name to be
> > used for multiple functions, as long as the argument list types differ. If
> > functions’ names and argument types match, then their return type and
> > parameter qualifiers must also match. Function signature matching is based
> > on
> > parameter type only, no qualifiers are used. Overloading is used heavily in
> > the
> > built-in functions. When overloaded functions (or indeed any functions) are
> > resolved, an exact match for the function's signature is sought. This
> > includes
> > exact match of array size as well. No promotion or demotion of the return
> > type
> > or input argument types is done. All expected combination of inputs and
> > outputs must be defined as separate functions."
> >
> > So this check is specific to GLSL ES 3.00.
> >
> > This patch fixes the following dEQP tests:
> >
> > dEQP-GLES3.functional.shaders.functions.invalid.overload_builtin_function_vertex
> > dEQP-GLES3.functional.shaders.functions.invalid.overload_builtin_function_fragment
> > dEQP-GLES3.functional.shaders.functions.invalid.redefine_builtin_function_vertex
> > dEQP-GLES3.functional.shaders.functions.invalid.redefine_builtin_function_fragment
> >
> > No piglit regressions.
> >
> > Signed-off-by: Samuel Iglesias Gonsalvez <sigles...@igalia.com>
> > ---
> > src/glsl/ast_to_hir.cpp | 22 ++++++++++++++++++++++
> > src/glsl/builtin_functions.cpp | 11 +++++++++++
> > src/glsl/ir.h | 4 ++++
> > 3 files changed, 37 insertions(+)
> >
> > diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp
> > index fe1e129..b7074bc 100644
> > --- a/src/glsl/ast_to_hir.cpp
> > +++ b/src/glsl/ast_to_hir.cpp
> > @@ -4167,6 +4167,28 @@ ast_function::hir(exec_list *instructions,
> > return NULL;
> > }
> > }
> > + } else {
> > + /* From GLSL ES 3.0 spec, chapter 6.1 "Function Definitions",
> > page 71:
> > + *
> > + * "A shader cannot redefine or overload built-in functions."
> > + *
> > + * While in GLSL ES 1.0 spec, chapter 6.1 "Function Definitions",
> > page
> > + * 54, this is allowed:
> > + *
> > + * "Function names can be overloaded. [...] Overloading is used
> > heavily
> > + * in the built-in functions."
>
> I don't think that quote is really explicitly saying that you can
> overload built-in functions. It's just saying that built-in functions
> contain many overloads.
>
> It doesn't, however, prohibit the user from adding overloads of their own.
>
> I'd probably replace the spec citation with just a statement that the
> GLSL ES 1.0 spec doesn't prohibit overloading built-in functions,
> since it really doesn't say anything explicitly about the topic.
The GLSL ES 1.0 specification actually very explicitly allows
overloading of built-in functions:
From the GLSL ES 1.0 specification, section 4.2.6:
"User defined functions may overload built-in functions."
and chapter 8:
"User code can overload the built-in functions but cannot redefine
them."
This is indeed different than GLSL ES 3.00, which prohibits both,
and different than desktop GLSL, which has entirely different rules.
>
> > + *
> > + */
> > + if (state->es_shader && state->language_version >= 300) {
> > + /* Local shader has no exact candidates; check the built-ins.
> > */
> > + _mesa_glsl_initialize_builtin_functions();
> > + if (_mesa_glsl_find_builtin_function_by_name(state, name)) {
> > + YYLTYPE loc = this->get_location();
> > + _mesa_glsl_error(& loc, state,
> > + "A shader cannot redefine or overload
> > built-in "
> > + "function `%s' in GLSL ES 3.00", name);
> > + }
> > + }
This code looks good, but there's one subtle issue with where you've
placed it. Namely, it allows this to compile:
#version 300 es
precision highp float;
out vec4 color;
void main()
{
color = vec4(sin(0.5));
}
float sin(float x);
When handling that prototype, it will take the existing
exact_matching_signature != NULL path (since your code is the "else" to
that block), which sees it as a harmless redundant prototype, and allows
it.
I would advise simply moving this code out a level and up:
if (state->es_shader && state->language_version >= 300) {
/* Do your code here */
return NULL;
}
if (state->es_shader || f->has_user_signature()) {
/* Do the old code here */
}
Please submit an updated patch and CC me. Thanks!
signature.asc
Description: This is a digitally signed message part.
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
