[Piglit] [PATCH] program_interface_query: add new subtest for getprogramresourceiv
Samuel Iglesias Gonsálvez
siglesias at igalia.com
Wed May 20 01:39:25 PDT 2015
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.
> 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 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.
>
> 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.
>> "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!
Sam
More information about the Piglit
mailing list