Bug 83508

This is going to be fixed by Antía's patch:

http://lists.freedesktop.org/archives/mesa-dev/2015-March/079102.html

We are planning to send an updated version of this patch in the coming days.

(In reply to Samuel Iglesias from comment #1)
> This is going to be fixed by Antía's patch:
> 
> http://lists.freedesktop.org/archives/mesa-dev/2015-March/079102.html
> 
> We are planning to send an updated version of this patch in the coming days.

That will work around the issue for 'shared' and 'std140' but if a shader has a ubo with a 'packed' layout and is unused in the shader it will still hit this same problem right?

The problem is the linking code always expects there to be a least one active use of the ubo array in the shader, I attempted to fix this but it REALLY expects there to be a use so its not quite as simple as you would expect.

I may give it another go when I have some time as I'm playing around with this code for AoA support currently.

(In reply to Timothy Arceri from comment #2)
> (In reply to Samuel Iglesias from comment #1)
> > This is going to be fixed by Antía's patch:
> > 
> > http://lists.freedesktop.org/archives/mesa-dev/2015-March/079102.html
> > 
> > We are planning to send an updated version of this patch in the coming days.

Hi! Timothy,

I have a limited knowledge about the linking code but I did take a look at how the link_uniform_block_active_visitor works.

> That will work around the issue for 'shared' and 'std140' but if a shader
> has a ubo with a 'packed' layout and is unused in the shader it will still
> hit this same problem right?

I think it will not. The 'shared' and 'std140' ubo arrays are always marked as active, regardless of they being referenced or not in the code. However, for 'packed' ubos I can see that they are marked as active by the link_uniform_block_active_visitor *only if* there is a reference to them. By marking them as active, I mean that they will be added by the visitor to the 'block_hash' hashtable  declared in the "link_uniform_blocks" method.

> The problem is the linking code always expects there to be a least one
> active use of the ubo array in the shader, I attempted to fix this but it
> REALLY expects there to be a use so its not quite as simple as you would
> expect.

Could you point out where that is expected?. Thanks.
My understanding is that the linking process would only care about what we have inside that 'block_hash' active uniform blocks hashtable and, in the case of 'packed' ubos, they only be there if they have been referenced.

> I may give it another go when I have some time as I'm playing around with
> this code for AoA support currently.

(In reply to Antia Puentes from comment #3)
> (In reply to Timothy Arceri from comment #2)
> > (In reply to Samuel Iglesias from comment #1)
> > > This is going to be fixed by Antía's patch:
> > > 
> > > http://lists.freedesktop.org/archives/mesa-dev/2015-March/079102.html
> > > 
> > > We are planning to send an updated version of this patch in the coming days.
> 
> Hi! Timothy,
> 
> I have a limited knowledge about the linking code but I did take a look at
> how the link_uniform_block_active_visitor works.
> 
> > That will work around the issue for 'shared' and 'std140' but if a shader
> > has a ubo with a 'packed' layout and is unused in the shader it will still
> > hit this same problem right?
> 
> I think it will not. The 'shared' and 'std140' ubo arrays are always marked
> as active, regardless of they being referenced or not in the code. However,
> for 'packed' ubos I can see that they are marked as active by the
> link_uniform_block_active_visitor *only if* there is a reference to them.

ok, so this was the problem. The code was broken for arrays, I've sent a patch to the mailing list [1] and Cc'd yourself and Ian. Also sent piglit test [2]

[1] http://lists.freedesktop.org/archives/mesa-dev/2015-September/095070.html
[2] http://lists.freedesktop.org/archives/piglit/2015-September/017247.html

Should be fixed by:

commit e92c35a8724efd36a35ac9106e5977c5ec2cb332
Author: Antia Puentes <apuentes@igalia.com>
Date:   Wed Jul 29 16:01:24 2015 +0200

    glsl: Mark as active all elements of shared/std140 block arrays
    
    Commit 1ca25ab (glsl: Do not eliminate 'shared' or 'std140' blocks
    or block members) considered as active 'shared' and 'std140' uniform
    blocks and uniform block arrays, but did not include the block array
    elements. Because of that, 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
    
    Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=83508
    Tested-by: Tapani Pälli <tapani.palli@intel.com>
    Reviewed-by: Timothy Arceri <t_arceri@yahoo.com.au>
    Reviewed-by: Kristian Høgsberg <krh@bitplanet.net>