[Piglit] [PATCH] arb_internalformat_query2: fix TBO testing (v2)
Alejandro Piñeiro
apinheiro at igalia.com
Tue Oct 24 07:01:17 UTC 2017
On 24/10/17 03:39, Ilia Mirkin wrote:
> On Mon, Oct 23, 2017 at 9:15 PM, Marek Olšák <maraeo at gmail.com> wrote:
>> From: Marek Olšák <marek.olsak at amd.com>
>>
>> This was never tested and it uses functions that are illegal with TBOs,
>> like glGetTexParameteriv and glTexImage2DMultisample.
>>
>> v2: enable GL_xxx_COMPONENTS testing
>> ---
>> tests/spec/arb_internalformat_query2/common.h | 4 +++-
>> tests/spec/arb_internalformat_query2/generic-pname-checks.c | 2 +-
>> .../spec/arb_internalformat_query2/image-format-compatibility-type.c | 2 +-
>> 3 files changed, 5 insertions(+), 3 deletions(-)
>>
>> diff --git a/tests/spec/arb_internalformat_query2/common.h b/tests/spec/arb_internalformat_query2/common.h
>> index df68581..0e5e11f 100644
>> --- a/tests/spec/arb_internalformat_query2/common.h
>> +++ b/tests/spec/arb_internalformat_query2/common.h
>> @@ -25,26 +25,28 @@
>>
>> static const GLenum valid_targets[] = {
>> GL_TEXTURE_1D,
>> GL_TEXTURE_1D_ARRAY,
>> GL_TEXTURE_2D,
>> GL_TEXTURE_2D_ARRAY,
>> GL_TEXTURE_3D,
>> GL_TEXTURE_CUBE_MAP,
>> GL_TEXTURE_CUBE_MAP_ARRAY,
>> GL_TEXTURE_RECTANGLE,
>> - GL_TEXTURE_BUFFER,
>> GL_RENDERBUFFER,
>> GL_TEXTURE_2D_MULTISAMPLE,
>> GL_TEXTURE_2D_MULTISAMPLE_ARRAY,
>> + GL_TEXTURE_BUFFER,
> tab
>
>> };
>>
>> +#define NUM_TARGETS_EXCEPT_TBO (ARRAY_SIZE(valid_targets) - 1)
>> +
>> static const GLenum invalid_targets[] = {
>> GL_FRAMEBUFFER,
>> GL_COLOR_ATTACHMENT0,
>> GL_COLOR_ATTACHMENT1,
>> GL_COLOR_ATTACHMENT2,
>> GL_COLOR_ATTACHMENT3,
>> GL_COLOR_ATTACHMENT4,
>> GL_COLOR_ATTACHMENT5,
>> GL_COLOR_ATTACHMENT6,
>> GL_COLOR_ATTACHMENT7,
>> diff --git a/tests/spec/arb_internalformat_query2/generic-pname-checks.c b/tests/spec/arb_internalformat_query2/generic-pname-checks.c
>> index e521fac..feb953c 100644
>> --- a/tests/spec/arb_internalformat_query2/generic-pname-checks.c
>> +++ b/tests/spec/arb_internalformat_query2/generic-pname-checks.c
>> @@ -255,21 +255,21 @@ check_basic(const GLenum *pnames, unsigned num_pnames,
>> test_data *data = test_data_new(0, 1);
>> unsigned i;
>> int testing64;
>>
>> for (i = 0; i < num_pnames; i++) {
>> bool pass = true;
>>
>> for (testing64 = 0; testing64 <= 1; testing64++) {
>> test_data_set_testing64(data, testing64);
>>
>> - pass = try_basic(valid_targets, ARRAY_SIZE(valid_targets),
>> + pass = try_basic(valid_targets, NUM_TARGETS_EXCEPT_TBO,
> I looked and looked and don't see what this is doing illegally for
> GL_TEXTURE_BUFFER.
Ditto.
> I didn't actually run though, and this code is
> pretty hard to follow.
Yeah sorry. In my defense, arb_internalformat_query2 is a really big
extension to test.
> Can you point out what this test does that's
> illegal for TBO's?
>
>> valid_internalformats, ARRAY_SIZE(valid_internalformats),
>> pnames[i],
>> possible_values, num_possible_values,
>> data)
>> && pass;
>> }
>>
>> piglit_report_subtest_result(pass ? PIGLIT_PASS : PIGLIT_FAIL,
>> "%s", piglit_get_gl_enum_name(pnames[i]));
>>
>> diff --git a/tests/spec/arb_internalformat_query2/image-format-compatibility-type.c b/tests/spec/arb_internalformat_query2/image-format-compatibility-type.c
>> index 28bf292..af46c60 100644
>> --- a/tests/spec/arb_internalformat_query2/image-format-compatibility-type.c
>> +++ b/tests/spec/arb_internalformat_query2/image-format-compatibility-type.c
>> @@ -138,21 +138,21 @@ try_local(const GLenum *targets, unsigned num_targets,
>> static bool
>> check_format_compatibility_type(void)
>> {
>> bool pass = true;
>> test_data *data = test_data_new(0, 1);
>> int testing64;
>>
>> for (testing64 = 0; testing64 <= 1; testing64++) {
>> test_data_set_testing64(data, testing64);
>>
>> - pass = try_local(valid_targets, ARRAY_SIZE(valid_targets),
>> + pass = try_local(valid_targets, NUM_TARGETS_EXCEPT_TBO,
> Yeah, this definitely calls glGetTexParameteriv in get_tex_parameter_value.
Yes, true, I have just confirmed it. C&P the code from try_local:
is_texture = value_on_set((const
GLint*)texture_targets, ARRAY_SIZE(texture_targets),
(GLint) targets[i]);
if (is_texture && supported) {
param =
get_tex_parameter_value(targets[i], internalformats[j]);
error_test = error_test &&
piglit_check_gl_error(GL_NO_ERROR);
value_test =
test_data_value_at_index(data, 0) == param;
} else {
value_test =
test_data_is_unsupported_response(data, pname);
}
The idea of that code is filter out targets that are not compatible with
GetTexParameteriv. But TEXTURE_BUFFER is on the texture_targets set.
So the bug is in this code. I think that the correct solution is fix
this, and not skip TBO testing.
I will try to send an email today. I think that the straightforward
solution would be add a new set of targets, with those compatible with
GetTexParameter.
>
>> valid_internalformats, ARRAY_SIZE(valid_internalformats),
>> GL_IMAGE_FORMAT_COMPATIBILITY_TYPE,
>> data)
>> && pass;
>> }
>>
>> piglit_report_subtest_result(pass ? PIGLIT_PASS : PIGLIT_FAIL,
>> "%s", piglit_get_gl_enum_name(GL_IMAGE_FORMAT_COMPATIBILITY_TYPE));
>>
>> test_data_clear(&data);
>> --
>> 2.7.4
>>
>> _______________________________________________
>> Piglit mailing list
>> Piglit at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/piglit
> _______________________________________________
> Piglit mailing list
> Piglit at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/piglit
More information about the Piglit
mailing list