On Wednesday, February 18, 2015 12:00:39 PM Matt Turner wrote: > On Mon, Dec 1, 2014 at 5:04 AM, Eduardo Lima Mitev <[email protected]> wrote: > > From: Samuel Iglesias Gonsalvez <[email protected]> > > > > 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 <[email protected]> > > --- > > 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 [email protected] http://lists.freedesktop.org/mailman/listinfo/mesa-dev
