Bug 90397

Yep, the problem is that their stage reference bitmask is empty. This is a debug print of active resources for your shader. They are considered active but not referenced. Will have to see how to tackle this.

--- 8< ---

PROGRAM RESOURCE LIST
IN   piglit_vertex [x_______]
OUT  gl_Position [x_______]
UNI  ubo_std140.v [________]
UNI  ubo_std140.s[0].b [________]
UNI  ubo_std140.s[0].c [________]
UNI  ubo_std140.s[1].b [________]
UNI  ubo_std140.s[1].c [________]
UBO  ubo_std140 [________]

Created attachment 115690 [details] [review]
patch to fix the issue

Does this fix the issue for you?

(In reply to Tapani Pälli from comment #2)
> Created attachment 115690 [details] [review] [review]
> patch to fix the issue
> 
> Does this fix the issue for you?

Yes, it does :-D

Thanks!

Created attachment 115737 [details] [review]
patch to GetProgramResourceIndex() to fix arrays of UBOs behavior

I found another issue when modifying arb_program_interface_query-getprogramresourceiv to test an uniform inside an array of UBOs.

I modified a shader which is defined inside common.h. Because of that, I updated other tests' code to work with that change. In particular, resource-query.c.

Shader code:

	#version 150

	uniform fs_uniform_block {
		vec4 fs_color;
		float fs_array[4];
	};

	uniform fs_array_uniform_block {
		vec4 fs_color;
		float fs_array[4];
	} ubo[4];

	in vec4 fs_input1;
	out vec4 fs_output0;
	out vec4 fs_output1;

	void main() {
	   fs_output0 = fs_color * fs_input1 * fs_array[2] *
		        ubo[0].fs_array[2] * ubo[2].fs_array[2];
	   fs_output1 = fs_color * fs_input1 * fs_array[3] *
		        ubo[1].fs_array[3] * ubo[3].fs_array[3];
	}


I found that glGetProgramResourceName() is returning the name "fs_array_uniform_block[0]", "fs_array_uniform_block[1]", "fs_array_uniform_block[2]", and "fs_array_uniform_block[3]" when asked with the corresponding block index as those blocks are active resources in the shader.

resource-query test checks again that those names corresponds to active resources by comparing the former index against the one returned by glGetProgramResourceIndex(name).

Unfortunately, Mesa is returning GL_INVALID_INDEX to fs_array_uniform_block[{1,2,3}] (other than zero),  while NVIDIA passes the test for that case.

Reading GL_ARB_program_interface_query, I found the following:

   "Note that if an
    interface enumerates a single active resource list entry for an array
    variable (e.g., "a[0]"), a <name> identifying any array element other than
    the first (e.g., "a[1]") is not considered to match."

I understand that this is referring to uniform/buffer variables but not uniform/shader-storage blocks, but I might be wrong.

I would like to have your opinion regarding if it is valid or not spec-wise. What do you think?

P.S: Attached to this, it's the patch that fixes this case.

(In reply to Samuel Iglesias from comment #4)
> I understand that this is referring to uniform/buffer variables but not
> uniform/shader-storage blocks, but I might be wrong.
> 
> I would like to have your opinion regarding if it is valid or not spec-wise.
> What do you think?
> 
> P.S: Attached to this, it's the patch that fixes this case.

I'm somewhat surprised if this rule is not be common for all resources. The text in OpenGL Core 4.5 says (right below function declaration):

"If name exactly matches the name string of one of the active resources for
programInterface, the index of the matched resource is returned. Additionally, if
name would exactly match the name string of an active resource if "[0]" were
appended to name, the index of the matched resource is returned. Otherwise, name
is considered not to be the name of an active resource, and INVALID_INDEX is
returned.

Note that if an interface enumerates a single active resource list entry for an array variable (e.g., "a[0]"), a name identifying any array element other than the first (e.g., "a[1]") is not considered to match."

Just to make sure I understand correctly, when querying the index for fs_array_uniform_block[{1,2,3}] you are giving GL_UNIFORM as the interface?

(In reply to Tapani Pälli from comment #5)
> (In reply to Samuel Iglesias from comment #4)
> > I understand that this is referring to uniform/buffer variables but not
> > uniform/shader-storage blocks, but I might be wrong.
> > 
> > I would like to have your opinion regarding if it is valid or not spec-wise.
> > What do you think?
> > 
> > P.S: Attached to this, it's the patch that fixes this case.
> 
> I'm somewhat surprised if this rule is not be common for all resources. The
> text in OpenGL Core 4.5 says (right below function declaration):
> 
> "If name exactly matches the name string of one of the active resources for
> programInterface, the index of the matched resource is returned.
> Additionally, if
> name would exactly match the name string of an active resource if "[0]" were
> appended to name, the index of the matched resource is returned. Otherwise,
> name
> is considered not to be the name of an active resource, and INVALID_INDEX is
> returned.
> 
> Note that if an interface enumerates a single active resource list entry for
> an array variable (e.g., "a[0]"), a name identifying any array element other
> than the first (e.g., "a[1]") is not considered to match."
> 
> Just to make sure I understand correctly, when querying the index for
> fs_array_uniform_block[{1,2,3}] you are giving GL_UNIFORM as the interface?

Nope, the interface type is GL_UNIFORM_BLOCK.

Let me recapitulate, simplifying what I wrote:

* I am querying the index for a <programInterface> equals to GL_UNIFORM_BLOCK and for the following names -> fs_array_uniform_block[{1,2,3}].

* The OpenGL Core 4.5 spec snippet you pasted says the same than the ARB_program_interface_query extension spec.

* As I said, NVIDIA doesn't give any error. Mesa returns INVALID_INDEX because of valid_program_resource_index_name() function as, for each name, the array index included in the name is not zero (in the example, they are 1, 2 and 3).

* I have doubts about what to do in this case, spec-wise.

I hope this is clearer now :-)

Sam

(In reply to Samuel Iglesias from comment #6)
> (In reply to Tapani Pälli from comment #5)
> > (In reply to Samuel Iglesias from comment #4)
> > > I understand that this is referring to uniform/buffer variables but not
> > > uniform/shader-storage blocks, but I might be wrong.
> > > 
> > > I would like to have your opinion regarding if it is valid or not spec-wise.
> > > What do you think?
> > > 
> > > P.S: Attached to this, it's the patch that fixes this case.
> > 
> > I'm somewhat surprised if this rule is not be common for all resources. The
> > text in OpenGL Core 4.5 says (right below function declaration):
> > 
> > "If name exactly matches the name string of one of the active resources for
> > programInterface, the index of the matched resource is returned.
> > Additionally, if
> > name would exactly match the name string of an active resource if "[0]" were
> > appended to name, the index of the matched resource is returned. Otherwise,
> > name
> > is considered not to be the name of an active resource, and INVALID_INDEX is
> > returned.
> > 
> > Note that if an interface enumerates a single active resource list entry for
> > an array variable (e.g., "a[0]"), a name identifying any array element other
> > than the first (e.g., "a[1]") is not considered to match."
> > 
> > Just to make sure I understand correctly, when querying the index for
> > fs_array_uniform_block[{1,2,3}] you are giving GL_UNIFORM as the interface?
> 
> Nope, the interface type is GL_UNIFORM_BLOCK.
> 
> Let me recapitulate, simplifying what I wrote:
> 
> * I am querying the index for a <programInterface> equals to
> GL_UNIFORM_BLOCK and for the following names ->
> fs_array_uniform_block[{1,2,3}].
> 
> * The OpenGL Core 4.5 spec snippet you pasted says the same than the
> ARB_program_interface_query extension spec.

Right, I just wanted to highlight this again as this explicitly states the rule and does not state that it would be valid only for some resource types. IMO it covers them all.
 
> * As I said, NVIDIA doesn't give any error. Mesa returns INVALID_INDEX
> because of valid_program_resource_index_name() function as, for each name,
> the array index included in the name is not zero (in the example, they are
> 1, 2 and 3).

Could you attach or link to the modified Piglit test case? I'm wondering a bit if glGetProgramResourceName returns the name with array index and if so, why.
 
> * I have doubts about what to do in this case, spec-wise.
> 
> I hope this is clearer now :-)

Yes!

(In reply to Tapani Pälli from comment #7)
> (In reply to Samuel Iglesias from comment #6)
> > (In reply to Tapani Pälli from comment #5)
> > > (In reply to Samuel Iglesias from comment #4)
> > > > I understand that this is referring to uniform/buffer variables but not
> > > > uniform/shader-storage blocks, but I might be wrong.
> > > > 
> > > > I would like to have your opinion regarding if it is valid or not spec-wise.
> > > > What do you think?
> > > > 
> > > > P.S: Attached to this, it's the patch that fixes this case.
> > > 
> > > I'm somewhat surprised if this rule is not be common for all resources. The
> > > text in OpenGL Core 4.5 says (right below function declaration):
> > > 
> > > "If name exactly matches the name string of one of the active resources for
> > > programInterface, the index of the matched resource is returned.
> > > Additionally, if
> > > name would exactly match the name string of an active resource if "[0]" were
> > > appended to name, the index of the matched resource is returned. Otherwise,
> > > name
> > > is considered not to be the name of an active resource, and INVALID_INDEX is
> > > returned.
> > > 
> > > Note that if an interface enumerates a single active resource list entry for
> > > an array variable (e.g., "a[0]"), a name identifying any array element other
> > > than the first (e.g., "a[1]") is not considered to match."
> > > 
> > > Just to make sure I understand correctly, when querying the index for
> > > fs_array_uniform_block[{1,2,3}] you are giving GL_UNIFORM as the interface?
> > 
> > Nope, the interface type is GL_UNIFORM_BLOCK.
> > 
> > Let me recapitulate, simplifying what I wrote:
> > 
> > * I am querying the index for a <programInterface> equals to
> > GL_UNIFORM_BLOCK and for the following names ->
> > fs_array_uniform_block[{1,2,3}].
> > 
> > * The OpenGL Core 4.5 spec snippet you pasted says the same than the
> > ARB_program_interface_query extension spec.
> 
> Right, I just wanted to highlight this again as this explicitly states the
> rule and does not state that it would be valid only for some resource types.
> IMO it covers them all.
>  
> > * As I said, NVIDIA doesn't give any error. Mesa returns INVALID_INDEX
> > because of valid_program_resource_index_name() function as, for each name,
> > the array index included in the name is not zero (in the example, they are
> > 1, 2 and 3).
> 
> Could you attach or link to the modified Piglit test case? I'm wondering a
> bit if glGetProgramResourceName returns the name with array index and if so,
> why.
>

I published a branch with the modified test, please see last commit:

git clone -b arb_uniform_buffer_object-patches https://github.com/samuelig/piglit.git

File: tests/spec/arb_program_interface_query/resource-query.c

Thanks for the test. I do agree with the patch, UBO arrays should be treated differently.

Reviewed-by: Tapani Pälli <tapani.palli@intel.com>

(I'm resolving this bug as the patch has been pushed to master and ubo array is a separate issue)

(In reply to Tapani Pälli from comment #9)
> Thanks for the test. I do agree with the patch, UBO arrays should be treated
> differently.
> 
> Reviewed-by: Tapani Pälli <tapani.palli@intel.com>
> 
> (I'm resolving this bug as the patch has been pushed to master and ubo array
> is a separate issue)

OK, thanks.