Besides fixing the mentioned dEQP crashes, this patch also generally
fixes instance arrays with UBOs. The problem we have now is that each
element in the UBO instance array is a separate UBO mapped to a specific
binding point (and thus, a separate buffer), but we kill the instances
that are not being referenced in the shader code, so if we have
something like this:
layout(std140, binding=2) uniform Fragments {
vec4 v0;
vec4 v1;
} inst[3];
And then the shader code only references inst[1], for example:
vec4 tfOutput0 = inst[1].v0;
That UBO read for inst[1].v0 can fail as a consequence of the fact that
we we are killing UBOs for inst[0] and inst[2] and we shouldn't.
I hit this while developing SSBO, which is the same thing, and this
patch fixes the problem.
Iago
On Wed, 2015-03-11 at 10:01 +0100, Eduardo Lima Mitev wrote:
> From: Antia Puentes <[email protected]>
>
> Commmit 1ca25ab (glsl: Do not eliminate 'shared' or 'std140'
> blocks or block members) considers active 'shared' and 'std140'
> uniform blocks and uniform block arrays but did not include the
> block array elements. It was possible to have an active uniform
> block array without any elements marked as used, making the
> assertion ((b->num_array_elements > 0) == b->type->is_array())
> in link_uniform_blocks fail.
>
> Fixes the following 5 dEQP tests:
>
> * dEQP-GLES3.functional.ubo.random.nested_structs_instance_arrays.18
> * dEQP-GLES3.functional.ubo.random.nested_structs_instance_arrays.24
> * dEQP-GLES3.functional.ubo.random.nested_structs_arrays_instance_arrays.19
> * dEQP-GLES3.functional.ubo.random.all_per_block_buffers.49
> * dEQP-GLES3.functional.ubo.random.all_shared_buffer.36
> ---
> src/glsl/link_uniform_block_active_visitor.cpp | 23 +++++++++++++++++++++++
> 1 file changed, 23 insertions(+)
>
> diff --git a/src/glsl/link_uniform_block_active_visitor.cpp
> b/src/glsl/link_uniform_block_active_visitor.cpp
> index 292cde3..8379750 100644
> --- a/src/glsl/link_uniform_block_active_visitor.cpp
> +++ b/src/glsl/link_uniform_block_active_visitor.cpp
> @@ -105,6 +105,22 @@ link_uniform_block_active_visitor::visit(ir_variable
> *var)
> assert(b->num_array_elements == 0);
> assert(b->array_elements == NULL);
> assert(b->type != NULL);
> + assert(!b->type->is_array() || b->has_instance_name);
> +
> + /* For uniform block arrays declared with a shared or std140 layout
> + * qualifier, mark all its instances as used.
> + */
> + if (b->type->is_array() && b->type->length > 0) {
> + b->num_array_elements = b->type->length;
> + b->array_elements = reralloc(this->mem_ctx,
> + b->array_elements,
> + unsigned,
> + b->num_array_elements);
> +
> + for (unsigned i = 0; i < b->num_array_elements; i++) {
> + b->array_elements[i] = i;
> + }
> + }
>
> return visit_continue;
> }
> @@ -146,6 +162,13 @@
> link_uniform_block_active_visitor::visit_enter(ir_dereference_array *ir)
> assert((b->num_array_elements == 0) == (b->array_elements == NULL));
> assert(b->type != NULL);
>
> + /* If the block array was declared with a shared or std140 layout
> qualifier,
> + * all its instances have been already marked as used (see
> + * link_uniform_block_active_visitor::visit(ir_variable *) function).
> + */
> + if (var->type->interface_packing == GLSL_INTERFACE_PACKING_PACKED)
> + return visit_continue;
> +
> ir_constant *c = ir->array_index->as_constant();
>
> if (c) {
_______________________________________________
mesa-dev mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
