[Mesa-dev] [PATCH] glsl: Structures must have same name to be considered same type.

[Matt Turner]

On Mon, Sep 22, 2014 at 5:11 AM, Tapani Pälli <tapani.palli at intel.com> wrote:
> From: Kalyan Kondapally <kalyan.kondapally at intel.com>
>
> According to GLSL(4.2) and GLSL-ES (1.0, 3.0) spec, Structures must
> have the same name to be considered same type. We currently ignore
> the name check while checking if two records are same. This patch
> fixes this.
>
> Patch fixes failing tests in WebGL conformance test
> 'shaders-with-uniform-structs' when running Chrome on OpenGL ES.
>
> v2: Do not force name comparison with unnamed types (Tapani)
>
> Signed-off-by: Kalyan Kondapally <kalyan.kondapally at intel.com>
> Signed-off-by: Tapani Pälli <tapani.palli at intel.com>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=83934
> ---
>  src/glsl/glsl_types.cpp | 14 ++++++++++++++
>  src/glsl/glsl_types.h   |  8 ++++++++
>  2 files changed, 22 insertions(+)
>
> diff --git a/src/glsl/glsl_types.cpp b/src/glsl/glsl_types.cpp
> index 66e9b13..4f8bb62 100644
> --- a/src/glsl/glsl_types.cpp
> +++ b/src/glsl/glsl_types.cpp
> @@ -490,6 +490,20 @@ glsl_type::record_compare(const glsl_type *b) const
>     if (this->interface_packing != b->interface_packing)
>        return false;
>
> +   /* From the GLSL 4.20 specification (Sec 4.2):
> +    *
> +    *     "Structures must have the same name, sequence of type names, and
> +    *     type definitions, and field names to be considered the same type."
> +    *
> +    * GLSL ES behaves the same (Ver 1.00 Sec 4.2.4, Ver 3.00 Sec 4.2.5).
> +    *
> +    * Note that we cannot force type name check when comparing unnamed
> +    * structure types, these have a unique name assigned during parsing.
> +    */
> +   if (!(this->is_anonymous() && b->is_anonymous()))

This would be clearer as

    if (!this->is_anonymous() && !b->is_anonymous())

but you don't even need to check both of them. If
this->is_anonymous(), it doesn't matter whether b->is_anonymous().

That is to say, isn't

    if (!this->is_anonymous())
        if (strcmp(this->name, b->name) != 0)
            return false;

sufficient?

Assuming that logic is right,

Reviewed-by: Matt Turner <mattst88 at gmail.com>

> +      if (strcmp(this->name, b->name) != 0)
> +         return false;
> +
>     for (unsigned i = 0; i < this->length; i++) {
>        if (this->fields.structure[i].type != b->fields.structure[i].type)
>          return false;
> diff --git a/src/glsl/glsl_types.h b/src/glsl/glsl_types.h
> index d545533..9693c80 100644
> --- a/src/glsl/glsl_types.h
> +++ b/src/glsl/glsl_types.h
> @@ -486,6 +486,14 @@ struct glsl_type {
>     }
>
>     /**
> +    * Query if a type is unnamed/anonymous (named by the parser)
> +    */
> +   bool is_anonymous() const
> +   {
> +      return !strncmp(name, "#anon", 5);
> +   }
> +
> +   /**
>      * Get the type stripped of any arrays
>      *
>      * \return
> --
> 1.9.3
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev