[Mesa-dev] [PATCH 1/2] glsl: don't allow non-flat integral types in varying structs/arrays.

[Anuj Phogat]

On Tue, Feb 12, 2013 at 8:12 AM, Paul Berry <stereotype441 at gmail.com> wrote:
> In the GLSL 1.30 spec, section 4.3.6 ("Outputs") says:
>
>     "If a vertex output is a signed or unsigned integer or integer
>     vector, then it must be qualified with the interpolation qualifier
>     flat."
>
> The GLSL ES 3.00 spec further clarifies, in section 4.3.6 ("Output
> Variables"):
>
>     "Vertex shader outputs that are, *or contain*, signed or unsigned
>     integers or integer vectors must be qualified with the
>     interpolation qualifier flat."
>
> (Emphasis mine.)
>
> The language in the GLSL ES 3.00 spec is clearly correct and should be
> applied to all shading language versions, since varyings that contain
> ints can't be interpolated, regardless of which shading language
> version is in use.
>
> (Note that in GLSL 1.50 the restriction is changed to apply to
> fragment shader inputs rather than vertex shader outputs, to
> accommodate the fact that in the presence of geometry shaders, vertex
> shader outputs are not necessarily interpolated.  That will be
> addressed by a future patch).
>
> NOTE: This is a candidate for stable branches.
> ---
>  src/glsl/ast_to_hir.cpp | 12 ++++++------
>  src/glsl/glsl_types.cpp | 18 ++++++++++++++++++
>  src/glsl/glsl_types.h   |  6 ++++++
>  3 files changed, 30 insertions(+), 6 deletions(-)
>
> diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp
> index 49093d8..092834b 100644
> --- a/src/glsl/ast_to_hir.cpp
> +++ b/src/glsl/ast_to_hir.cpp
> @@ -2829,9 +2829,9 @@ ast_declarator_list::hir(exec_list *instructions,
>         *    flat."
>         *
>         * From section 4.3.4 of the GLSL 3.00 ES spec:
> -       *    "Fragment shader inputs that are signed or unsigned integers or
> -       *    integer vectors must be qualified with the interpolation qualifier
> -       *    flat."
> +       *    "Fragment shader inputs that are, or contain, signed or unsigned
> +       *    integers or integer vectors must be qualified with the
> +       *    interpolation qualifier flat."
>         *
>         * Since vertex outputs and fragment inputs must have matching
>         * qualifiers, these two requirements are equivalent.
> @@ -2839,12 +2839,12 @@ ast_declarator_list::hir(exec_list *instructions,
>        if (state->is_version(130, 300)
>            && state->target == vertex_shader
>            && state->current_function == NULL
> -          && var->type->is_integer()
> +          && var->type->contains_integer()
>            && var->mode == ir_var_shader_out
>            && var->interpolation != INTERP_QUALIFIER_FLAT) {
>
> -         _mesa_glsl_error(&loc, state, "If a vertex output is an integer, "
> -                          "then it must be qualified with 'flat'");
> +         _mesa_glsl_error(&loc, state, "If a vertex output is (or contains) "
> +                          "an integer, then it must be qualified with 'flat'");
>        }
>
>
> diff --git a/src/glsl/glsl_types.cpp b/src/glsl/glsl_types.cpp
> index 3b066d0..82aeb84 100644
> --- a/src/glsl/glsl_types.cpp
> +++ b/src/glsl/glsl_types.cpp
> @@ -158,6 +158,24 @@ glsl_type::contains_sampler() const
>     }
>  }
>
> +
> +bool
> +glsl_type::contains_integer() const
> +{
> +   if (this->is_array()) {
> +      return this->fields.array->contains_integer();
> +   } else if (this->is_record()) {
> +      for (unsigned int i = 0; i < this->length; i++) {
> +        if (this->fields.structure[i].type->contains_integer())
> +           return true;
> +      }
> +      return false;
> +   } else {
> +      return this->is_integer();
> +   }
> +}
> +
> +
>  gl_texture_index
>  glsl_type::sampler_index() const
>  {
> diff --git a/src/glsl/glsl_types.h b/src/glsl/glsl_types.h
> index b0db2bf..8cfd8dd 100644
> --- a/src/glsl/glsl_types.h
> +++ b/src/glsl/glsl_types.h
> @@ -360,6 +360,12 @@ struct glsl_type {
>     }
>
>     /**
> +    * Query whether or not type is an integral type, or for struct and array
> +    * types, contains an integral type.
> +    */
> +   bool contains_integer() const;
> +
> +   /**
>      * Query whether or not a type is a float type
>      */
>     bool is_float() const
> --
> 1.8.1.3
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Both patches are Reviewed-by: Anuj Phogat <anuj.phogat at gmail.com>