[Piglit] [PATCH] program_interface_query: add new subtest for getprogramresourceiv
Martin Peres
martin.peres at linux.intel.com
Wed May 20 03:54:15 PDT 2015
On 20/05/15 11:39, Samuel Iglesias Gonsálvez wrote:
> On 19/05/15 12:02, Martin Peres wrote:
>> On 19/05/15 08:46, Samuel Iglesias Gonsalvez wrote:
>>> Test that uniforms inside arrays of uniform blocks with instance name are
>>> queried properly.
>>>
>>> Tested on NVIDIA's proprietary driver version 340.65
>>>
>>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=90397
>>>
>>> Signed-off-by: Samuel Iglesias Gonsalvez <siglesias at igalia.com>
>>> ---
>>> tests/spec/arb_program_interface_query/common.h | 12 +++++++++---
>>> .../getprogramresourceiv.c | 19
>>> +++++++++++++++++++
>>> .../spec/arb_program_interface_query/resource-query.c | 16
>>> ++++++++++++----
>>> tests/spec/arb_shader_storage_buffer_object/minmax.c | 4 ++--
>>> 4 files changed, 42 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/tests/spec/arb_program_interface_query/common.h
>>> b/tests/spec/arb_program_interface_query/common.h
>>> index 55f0358..4079ac8 100755
>>> --- a/tests/spec/arb_program_interface_query/common.h
>>> +++ b/tests/spec/arb_program_interface_query/common.h
>>> @@ -102,13 +102,19 @@ static const char fs_std[] =
>>> "uniform fs_uniform_block {"
>>> " vec4 fs_color;\n"
>>> " float fs_array[4];\n"
>>> - "};"
>>> + "};\n"
>>> + "uniform fs_array_uniform_block {\n"
>>> + " vec4 fs_color;\n"
>>> + " float fs_array[4];\n"
>>> + "} ubo[4];\n"
>> Hmm, aside from the obvious problem of having ubo != uniform_block,
> I am going to rename that name uniform block in the second version of
> the patch.
sounds nicer indeed!
>
>> I
>> wonder what would happen if you instead had:
>>
>> + "} faub1[2], faub2[2];\n"
>>
> That kind of definition is not allowed in the spec. I think it is only
> one instance name per interface block.
>
> I tried defining "faub2[2]" by repeating the fs_array_uniform_block
> definition but i965 doesn't allowed it, although NVIDIA does.
I hope that no games rely on this...
>
>> I am worried that you would get the following resources listed:
>>
>> + "fs_array_uniform_block[0]",
>> + "fs_array_uniform_block[1]",
>> + "fs_array_uniform_block[0]",
>> + "fs_array_uniform_block[1]",
>>
> In my tests on NVIDIA, I get:
>
> + "fs_array_uniform_block[0]",
> + "fs_array_uniform_block[1]",
>
> But they are not repeated. I guess NVIDIA thinks they are the same
> uniform block but I did not check it further.
Seems like a lovely bug!
>
>> which would be a clear indication that the driver is broken. My view on
>> this is that named uniform blocks should use their named version when
>> being referenced. What do you think?
> I have been checking what the specs say about it.
>
> * In ARB_program_interface_query, GetProgramResourceName() is equivalent
> to GetActiveUniformBlockName().
>
> * Looking at ARB_uniform_buffer_object, GetActiveUniformBlockName() is
> talking about uniform block name.
>
> * Looking at GLSL spec (for example 4.4), when talking about interface
> block (4.3.9 Interface Blocks), its definition is:
>
> layout-qualifieropt interface-qualifier block-name { member-list }
> instance-nameopt ;
>
> But also, the same sections talks about the name:
>
> "For blocks declared as arrays, the array index must also be included
> when accessing members, as in this example
>
> uniform Transform { // API uses “Transform[2]” to refer to instance 2
> mat4 ModelViewMatrix;
> mat4 ModelViewProjectionMatrix;
> vec4 a[]; // array will get implicitly sized
> float Deformation;
> } transforms[4];
>
> [...]
> When using OpenGL API entry points to identify the name of an
> individual block in an array of blocks, the name string must include
> an array index (e.g., Transform[2])."
>
> Notice that it is talking about the block name, not the instance name.
> So the current behaviour is right.
Wow, I am baffled by those crazy rules... Thanks for tracking this down
the spec!
>
>>> "in vec4 fs_input1;\n"
>>> "out vec4 fs_output0;\n"
>>> "out vec4 fs_output1;\n"
>>> "void main() {\n"
>>> - "fs_output0 = fs_color * fs_input1 * fs_array[2];\n"
>>> - "fs_output1 = fs_color * fs_input1 * fs_array[3];\n"
>>> + "fs_output0 = fs_color * fs_input1 * fs_array[2] * \n"
>>> + " ubo[0].fs_array[2] * ubo[2].fs_array[2];\n"
>>> + "fs_output1 = fs_color * fs_input1 * fs_array[3] * \n"
>>> + " ubo[1].fs_array[3] * ubo[3].fs_array[3];\n"
>>> "}";
>>> static const char vs_stor[] =
>>> diff --git
>>> a/tests/spec/arb_program_interface_query/getprogramresourceiv.c
>>> b/tests/spec/arb_program_interface_query/getprogramresourceiv.c
>>> index da9751a..03f2fc6 100755
>>> --- a/tests/spec/arb_program_interface_query/getprogramresourceiv.c
>>> +++ b/tests/spec/arb_program_interface_query/getprogramresourceiv.c
>>> @@ -454,6 +454,25 @@ static const struct subtest_t subtests[] = {
>>> { GL_REFERENCED_BY_COMPUTE_SHADER, 1, { 0 } },
>>> { 0, 0, { 0 } }}
>>> },
>>> + { &prog_std, GL_UNIFORM, "fs_array_uniform_block.fs_array",
>>> "fs_array_uniform_block[0]", {
>>> + { GL_NAME_LENGTH, 1, { 35 } },
>>> + { GL_TYPE, 1, { GL_FLOAT } },
>>> + { GL_ARRAY_SIZE, 1, { 4 } },
>>> + { GL_OFFSET, 1, { 0 } }, /* valid index == anything but -1 */
>>> + { GL_BLOCK_INDEX, 1, { 2 } }, /* compared to
>>> fs_array_uniform_block[0]'s idx */
>>> + { GL_ARRAY_STRIDE, 1, { 0 } }, /* valid index == anything but -1 */
>>> + { GL_MATRIX_STRIDE, 1, { 0 } },
>>> + { GL_IS_ROW_MAJOR, 1, { 0 } },
>>> + { GL_ATOMIC_COUNTER_BUFFER_INDEX, 1, { -1 } }, /* valid index ==
>>> anything but -1 */
>>> + { GL_REFERENCED_BY_VERTEX_SHADER, 1, { 0 } },
>>> + { GL_REFERENCED_BY_TESS_CONTROL_SHADER, 1, { 0 } },
>>> + { GL_REFERENCED_BY_TESS_EVALUATION_SHADER, 1, { 0 } },
>>> + { GL_REFERENCED_BY_GEOMETRY_SHADER, 1, { 0 } },
>>> + { GL_REFERENCED_BY_FRAGMENT_SHADER, 1, { 1 } },
>>> + { GL_REFERENCED_BY_COMPUTE_SHADER, 1, { 0 } },
>>> + { GL_LOCATION, 1, { -1 } }, /* valid index == anything but -1 */
>>> + { 0, 0, { 0 } }}
>>> + },
>>> { &prog_stor, GL_BUFFER_VARIABLE, "gs_buf_var", "gs_buffer_block", {
>>> { GL_NAME_LENGTH, 1, { 11 } },
>>> { GL_TYPE, 1, { GL_FLOAT_VEC4 } },
>>> diff --git a/tests/spec/arb_program_interface_query/resource-query.c
>>> b/tests/spec/arb_program_interface_query/resource-query.c
>>> index 92b8cd8..1db5585 100755
>>> --- a/tests/spec/arb_program_interface_query/resource-query.c
>>> +++ b/tests/spec/arb_program_interface_query/resource-query.c
>>> @@ -189,13 +189,21 @@ PIGLIT_GL_TEST_CONFIG_END
>>> * white space anywhere in the string.
>>> */
>>> static const char *st_r_uniform[] = {"vs_test", "gs_test", "fs_color",
>>> - "fs_array[0]", "sa[0].a[0]", "sa[1].a[0]",
>>> + "fs_array[0]",
>>> + "fs_array_uniform_block.fs_color",
>>> + "fs_array_uniform_block.fs_array[0]",
>>> + "sa[0].a[0]", "sa[1].a[0]",
>>> NULL};
>>> static const char *st_r_tess_uniform[] = {"tcs_test", "tes_test",
>>> NULL};
>>> static const char *st_r_cs_uniform[] = {"cs_test", "tex", NULL};
>>> static const char *st_r_uniform_block[] = {"vs_uniform_block",
>>> "gs_uniform_block",
>>> - "fs_uniform_block", NULL};
>>> + "fs_uniform_block",
>>> + "fs_array_uniform_block[0]",
>>> + "fs_array_uniform_block[1]",
>>> + "fs_array_uniform_block[2]",
>>> + "fs_array_uniform_block[3]",
>>> + NULL};
>>> static const char *st_r_tess_uniform_block[] = {"tcs_uniform_block",
>>> "tes_uniform_block", NULL};
>>> static const char *st_r_cs_uniform_block[] = {"cs_uniform_block",
>>> NULL};
>>> @@ -346,10 +354,10 @@ struct subtest_t {
>>> }
>>> static const struct subtest_t subtests[] = {
>>> - ST( 6, 12, -1, -1, vs_std, NULL, NULL, gs_std, fs_std,
>>> NULL, GL_UNIFORM, "(vs,gs,fs)", st_r_uniform),
>>> + ST( 8, 35, -1, -1, vs_std, NULL, NULL, gs_std, fs_std,
>>> NULL, GL_UNIFORM, "(vs,gs,fs)", st_r_uniform),
>>> ST( 2, 9, -1, -1, NULL, tcs_sub, tes_sub, NULL, NULL,
>>> NULL, GL_UNIFORM, "(tes,tcs)", st_r_tess_uniform),
>>> ST( 2, 8, -1, -1, NULL, NULL, NULL, NULL, NULL,
>>> cs_sub, GL_UNIFORM, "(cs)", st_r_cs_uniform),
>>> - ST( 3, 17, 2, -1, vs_std, NULL, NULL, gs_std, fs_std,
>>> NULL, GL_UNIFORM_BLOCK, "(vs,gs,fs)", st_r_uniform_block),
>>> + ST( 7, 26, 2, -1, vs_std, NULL, NULL, gs_std, fs_std,
>>> NULL, GL_UNIFORM_BLOCK, "(vs,gs,fs)", st_r_uniform_block),
>>> ST( 2, 18, -1, -1, NULL, tcs_sub, tes_sub, NULL, NULL,
>>> NULL, GL_UNIFORM_BLOCK, "(tcs,tes)", st_r_tess_uniform_block),
>>> ST( 1, 17, -1, -1, NULL, NULL, NULL, NULL, NULL,
>>> cs_sub, GL_UNIFORM_BLOCK, "(cs)", st_r_cs_uniform_block),
>>> ST( 2, 10, -1, -1, vs_std, NULL, NULL, NULL, NULL,
>>> NULL, GL_PROGRAM_INPUT, "(vs)", st_r_in_vs),
>>> diff --git a/tests/spec/arb_shader_storage_buffer_object/minmax.c
>>> b/tests/spec/arb_shader_storage_buffer_object/minmax.c
>>> index 71d86fe..281bb99 100644
>>> --- a/tests/spec/arb_shader_storage_buffer_object/minmax.c
>>> +++ b/tests/spec/arb_shader_storage_buffer_object/minmax.c
>>> @@ -31,8 +31,8 @@
>>> PIGLIT_GL_TEST_CONFIG_BEGIN
>>> - config.supports_gl_compat_version = 40;
>>> - config.supports_gl_core_version = 40;
>>> + config.supports_gl_compat_version = 31;
>>> + config.supports_gl_core_version = 31;
>> I think this got included in the patch for no good reason :)
> Oh right! I am going to fix it :-)
>
> Thanks!
You're welcome!
More information about the Piglit
mailing list