[Piglit] [PATCH] arb_internalformat_query2: fix TBO testing (v2)
Marek Olšák
maraeo at gmail.com
Tue Oct 24 09:02:46 UTC 2017
Version 3 is on the list now.
Marek
On Tue, Oct 24, 2017 at 9:01 AM, Alejandro Piñeiro <apinheiro at igalia.com> wrote:
> 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