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
If this change is not desirable then leaving record_type = NULL; in the
array loop will restore the previous behaviour. However the removal of
of setting the record_type seems valid either way.
>
> Signed-off-by: Timothy Arceri <t_arc...@yahoo.com.au>
> ---
>
> V2: Make commit message lines shorter than 80 characters
>
> Resending this patch. Its a required cleanup for arrays of arrays.
>
> The code was added as part of commit 684316512c5570483365d36849250a008b6fcd84
> Ian maybe you can comment on whether I'm following correctly?
>
> src/glsl/link_uniforms.cpp | 8 --------
> 1 file changed, 8 deletions(-)
>
> diff --git a/src/glsl/link_uniforms.cpp b/src/glsl/link_uniforms.cpp
> index ba66053..9566124 100644
> --- a/src/glsl/link_uniforms.cpp
> +++ b/src/glsl/link_uniforms.cpp
> @@ -187,9 +187,6 @@ program_resource_visitor::recursion(const glsl_type *t,
> char **name,
> }
> } else if (t->is_array() && (t->fields.array->is_record()
> || t->fields.array->is_interface())) {
> - if (record_type == NULL && t->fields.array->is_record())
> - record_type = t->fields.array;
> -
> for (unsigned i = 0; i < t->length; i++) {
> size_t new_length = name_length;
>
> @@ -198,11 +195,6 @@ program_resource_visitor::recursion(const glsl_type *t,
> char **name,
>
> recursion(t->fields.array, name, new_length, row_major,
> record_type);
> -
> - /* Only the first leaf-field of the record gets called with the
> - * record type pointer.
> - */
> - record_type = NULL;
> }
> } else {
> this->visit_field(t, *name, row_major, record_type);
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
