[Mesa-dev] [PATCH V2] glsl: remove unnecessary check

Timothy Arceri t_arceri at yahoo.com.au
Sat Jul 19 03:24:14 PDT 2014


On Sat, 2014-07-19 at 14:20 +1000, Timothy Arceri wrote:
> On Sat, 2014-07-19 at 12:25 +1000, Timothy Arceri wrote:
> > This code does nothing useful as the next recursive call on the array element
> > will override any null values if the element is a record anyway. The code is
> > also not doing what the comment says as its trying to set the record type
> > pointer for only the first element of the array not the first leaf field 
> > of the record.
> 
> It's been a while since I wrote this patch so I decided to double check
> it. For a second I thought I should have maybe left the null assignment
> in the array loop, so that if record_type was already passed it wouldn't
> be passed to each array element but that's what happens to each element
> anyway if record_type is null.
> 
> For example:
> 
> struct S1 { 
>    vec4 v;
>    float f;
> };
> 
> struct S { 
>    S1 s1[3];
>    S1 s2;
> };
> 
> uniform Ubo { 
>    S s;
> };
> 
> 
> Without my patch:
> 
> s.s1[0].v == record_type: S
> s.s1[1].v == record_type: S1
> s.s1[2].v == record_type: S1
> s.s1[0].f == record_type: null
> s.s1[1].f == record_type: null
> s.s1[2].f == record_type: null
> s.s2.v == record_type: S1
> s.s2.f == record_type: null
> 
> With my patch:
> 
> s.s1[0].v == record_type: S
> s.s1[1].v == record_type: S
> s.s1[2].v == record_type: S
> s.s1[0].f == record_type: null
> s.s1[1].f == record_type: null
> s.s1[2].f == record_type: null
> s.s2.v == record_type: S1
> s.s2.v == record_type: null
> 

Ok it looks like that code path is just plain broken either way.

I've opened a bug [1] and attached a piglit patch that triggers the
issue the test probably needs some work to make it a little more
thorough before submitting to the piglit list.

[1] https://bugs.freedesktop.org/show_bug.cgi?id=81528



More information about the mesa-dev mailing list