On Tuesday, June 10, 2014 09:57:51 AM 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.
>
> V3: Improve error message wording; eliminate redundant error generation
> for inputs in VS or outputs in FS.
>
> Signed-off-by: Chris Forbes <chr...@ijw.co.nz>
> ---
> src/glsl/ast_to_hir.cpp | 76
++++++++++++++++++++++++-------------------------
> 1 file changed, 37 insertions(+), 39 deletions(-)
>
> diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp
> index d1c77f1..00a59a5 100644
> --- a/src/glsl/ast_to_hir.cpp
> +++ b/src/glsl/ast_to_hir.cpp
> @@ -2649,6 +2649,36 @@ 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 */
> +
> + /* 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."
> + */
> +
> + /* 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 (qual->flags.q.sample && (!is_varying_var(var, state->stage) ||
uses_deprecated_qualifier)) {
> + _mesa_glsl_error(loc, state,
> + "sample qualifier may only be used on `in` or `out`
"
> + "variables between shader stages");
> + }
> + 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' variables between shader
stages");
> + }
> +
> +
> /* 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
> @@ -3606,45 +3636,6 @@ ast_declarator_list::hir(exec_list *instructions,
> }
>
>
> - /* 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");
> - }
> -
> /* Precision qualifiers exists only in GLSL versions 1.00 and >=
1.30.
> */
> if (this->type->qualifier.precision != ast_precision_none) {
> @@ -5041,6 +5032,13 @@ ast_process_structure_or_interface_block(exec_list
*instructions,
> "with uniform interface blocks");
> }
>
> + 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;
> Looks good to me. Thanks, Chris! Reviewed-by: Kenneth Graunke <kenn...@whitecape.org>
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
