On 08/05/2015 02:20 AM, Michel Dänzer wrote:
> From: Michel Dänzer <[email protected]>
> 
> Fixes a bunch of random (mostly tessellation related) piglit failures
> since commit 7ac946e5 ("glsl: Add constuctors for the common cases of
> glsl_struct_field").

I was going to look at this yesterday, but I didn't get to it. :(

That said, this is not the patch that I would have anticipated.  I
thought these paths should have used the default constructor, but I
think your change is correct.  It took a bit of research to convince
myself.  He's my thinking (and some of this should end up in the commit
message)...

There is apparently a subtle difference in C++ between

    F f;

and

    F f();

The former will use the default constructor.  If there is no default
constructor specified, the compiler provides one that simply invokes the
default constructor for each field.  For built-in basic types, the
default constructor does nothing.  The later will, according to
http://stackoverflow.com/questions/2417065/does-the-default-constructor-initialize-built-in-types)
perform value-initialization of the type.  For built-in types this means
initializing to zero.

The per_vertex_accumulator constructor is:

    per_vertex_accumulator::per_vertex_accumulator()
       : fields(),
         num_fields(0)
    {
    }

This is the second form of constructor, so the glsl_struct_field
objects were previously zero initialized.  With the addition of an empty
default constructor in commit 7ac946e5, per_vertex_accumulator::fields
receive no initialization.

> Signed-off-by: Michel Dänzer <[email protected]>

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=91544
Reviewed-by: Ian Romanick <[email protected]>

> ---
>  src/glsl/builtin_variables.cpp | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/src/glsl/builtin_variables.cpp b/src/glsl/builtin_variables.cpp
> index 0ff3a3f..b8ad2c9 100644
> --- a/src/glsl/builtin_variables.cpp
> +++ b/src/glsl/builtin_variables.cpp
> @@ -322,6 +322,7 @@ per_vertex_accumulator::add_field(int slot, const 
> glsl_type *type,
>     this->fields[this->num_fields].interpolation = INTERP_QUALIFIER_NONE;
>     this->fields[this->num_fields].centroid = 0;
>     this->fields[this->num_fields].sample = 0;
> +   this->fields[this->num_fields].patch = 0;
>     this->num_fields++;
>  }
>  
> @@ -1159,6 +1160,7 @@ builtin_variable_generator::generate_varyings()
>           var->data.interpolation = fields[i].interpolation;
>           var->data.centroid = fields[i].centroid;
>           var->data.sample = fields[i].sample;
> +         var->data.patch = fields[i].patch;  
>           var->init_interface_type(per_vertex_out_type);
>        }
>     }
> 

_______________________________________________
mesa-dev mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to