On 04/11/2014 06:21 PM, Chris Forbes wrote: > We've been allowing `centroid` and `sample` in all kinds of weird places > where they're not valid. > > Insist that `sample` is combined with `in` or `out`; > and that `centroid` is combined with `in`, `out`, or the deprecated > `varying`. > > V2: Validate this in a more sensible place. This does require an extra > case for uniform blocks members and struct members, though, since they > don't go through the normal path. > > Signed-off-by: Chris Forbes <chr...@ijw.co.nz> > --- > src/glsl/ast_to_hir.cpp | 20 ++++++++++++++++++++ > 1 file changed, 20 insertions(+) > > diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp > index 1d15e62..6c7e00a 100644 > --- a/src/glsl/ast_to_hir.cpp > +++ b/src/glsl/ast_to_hir.cpp > @@ -2552,6 +2552,19 @@ apply_type_qualifier_to_variable(const struct > ast_type_qualifier *qual, > const bool uses_deprecated_qualifier = qual->flags.q.attribute > || qual->flags.q.varying; > > + > + /* Validate auxiliary storage qualifiers */
Spec citations would be great if you can find them.
> + if (qual->flags.q.sample && (!is_varying_var(var, state->stage) ||
> uses_deprecated_qualifier)) {
> + _mesa_glsl_error(loc, state,
> + "sample qualifier may only be used with `in' or
> `out'");
> + }
> + if (qual->flags.q.centroid && !is_varying_var(var, state->stage)) {
> + _mesa_glsl_error(loc, state,
> + "centroid qualifier may only be used with `in', "
> + "`out' or `varying'");
> + }
This does a little more than the error message says - won't it prevent using
sample/centroid on, say, vertex shader inputs/fragment shader outputs?
If so, then it seems like you could drop a pile of other code:
/* From section 4.3.4 of the GLSL 1.30 spec:
* "It is an error to use centroid in in a vertex shader."
*
* From section 4.3.4 of the GLSL ES 3.00 spec:
* "It is an error to use centroid in or interpolation qualifiers in
* a vertex shader input."
*/
if (state->is_version(130, 300)
&& this->type->qualifier.flags.q.centroid
&& this->type->qualifier.flags.q.in
&& state->stage == MESA_SHADER_VERTEX) {
_mesa_glsl_error(&loc, state,
"'centroid in' cannot be used in a vertex shader");
}
if (state->stage == MESA_SHADER_VERTEX
&& this->type->qualifier.flags.q.sample
&& this->type->qualifier.flags.q.in) {
_mesa_glsl_error(&loc, state,
"'sample in' cannot be used in a vertex shader");
}
/* Section 4.3.6 of the GLSL 1.30 specification states:
* "It is an error to use centroid out in a fragment shader."
*
* The GL_ARB_shading_language_420pack extension specification states:
* "It is an error to use auxiliary storage qualifiers or interpolation
* qualifiers on an output in a fragment shader."
*/
if (state->stage == MESA_SHADER_FRAGMENT &&
this->type->qualifier.flags.q.out &&
this->type->qualifier.has_auxiliary_storage()) {
_mesa_glsl_error(&loc, state,
"auxiliary storage qualifiers cannot be used on "
"fragment shader outputs");
}
> +
> +
> /* Is the 'layout' keyword used with parameters that allow relaxed
> checking.
> * Many implementations of GL_ARB_fragment_coord_conventions_enable and
> some
> * implementations (only Mesa?) GL_ARB_explicit_attrib_location_enable
> @@ -4924,6 +4937,13 @@ ast_process_structure_or_interface_block(exec_list
> *instructions,
> "with uniform interface blocks");
> }
>
/* From the GLSL 4.20 specification, section 4.3 "Storage Qualifiers":
* "[...] structure members do not use storage qualifiers."
*/
presumably this applies to auxiliary storage qualifiers too...it was the
best I could find. Which, incidentally, makes me wonder...where do we check
for storage qualifiers? Maybe do it in the same place?
> + if ((qual->flags.q.uniform || !is_interface) &&
> + qual->has_auxiliary_storage()) {
> + _mesa_glsl_error(&loc, state,
> + "auxiliary storage qualifiers cannot be used "
> + "in uniform blocks or structures.");
> + }
> +
> if (field_type->is_matrix() ||
> (field_type->is_array() &&
> field_type->fields.array->is_matrix())) {
> fields[i].row_major = block_row_major;
>
signature.asc
Description: OpenPGP digital signature
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
