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

[kalyan kondapally]

On Wed, Sep 24, 2014 at 11:56 AM, Matt Turner <mattst88 at gmail.com> wrote:
> 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

> > +   /* 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>


This would ignore the case when record "A" is anonymous but record "B" is not.

Br,
Kalyan