[Piglit] [PATCH v2] arb_internalformat_query2: minmax GL_ALPHA8 deprecated on 3.1+

[Alejandro Piñeiro]

Sorry for the late answer. I didn't have time to check what Ilia is
asking below at that time, and then I forgot all about this until some
query2 fixes were sent to the list yesterday.

Answering now.

On 01/11/17 16:06, Ilia Mirkin wrote:
> On Wed, Nov 1, 2017 at 4:15 AM, Alejandro Piñeiro <apinheiro at igalia.com> wrote:
>> When ARB_compatibility is missing or the context is Core.
>>
>> v2: check for ARB_compatibility or core (Marek)
>> ---
>>  tests/spec/arb_internalformat_query2/minmax.c | 18 ++++++++++++++++--
>>  1 file changed, 16 insertions(+), 2 deletions(-)
>>
>> diff --git a/tests/spec/arb_internalformat_query2/minmax.c b/tests/spec/arb_internalformat_query2/minmax.c
>> index a2cf059928..2571e3b219 100644
>> --- a/tests/spec/arb_internalformat_query2/minmax.c
>> +++ b/tests/spec/arb_internalformat_query2/minmax.c
>> @@ -46,6 +46,10 @@ PIGLIT_GL_TEST_CONFIG_END
>>
>>  /* These are all the formats that are required to be color-renderable
>>   * by the OpenGL 3.0 spec.
>> + *
>> + * But note that GL_ALPHA8 was removed on 3.1 and beyond on core, or
>> + * if ARB_compatibility is missing, so we need to take that into
>> + * account.
>>   */
>>  static const GLenum valid_formats[] = {
>>          GL_RGBA32F,
>> @@ -283,6 +287,7 @@ piglit_init(int argc, char **argv)
>>          const bool tms_supported =
>>                  piglit_is_extension_supported("GL_ARB_texture_multisample");
>>          GLint max_samples;
>> +        GLint valid_formats_size = ARRAY_SIZE(valid_formats);
>>
>>          piglit_require_extension("GL_ARB_framebuffer_object");
>>          piglit_require_extension("GL_ARB_internalformat_query2");
>> @@ -293,8 +298,17 @@ piglit_init(int argc, char **argv)
>>                  piglit_require_extension("GL_ARB_texture_float");
>>          }
>>
>> +        /* GL_ALPHA8 was removed on OpenGL 3.1 core, or if
>> +         * ARB_compatibility is missing, so on that case we skip that
>> +         * format
>> +         */
>> +        if (piglit_get_gl_version() >= 31 &&
>> +            (piglit_is_core_profile ||
>> +             !piglit_is_extension_supported("GL_ARB_compatibility")))
>> +                valid_formats_size = valid_formats_size - 1;
> Isn't all this equivalent to just
>
> if (piglit_is_core_profile)
>   valid_formats_size--;
>
> Why all the other checks? If GL_ARB_compatibility is there, then
> piglit_is_core_profile should certainly not be set. There are no core
> profiles prior to GL 3.1.
>
> Or does the piglit infra not report a GL 3.1 context without
> GL_ARB_compatibility as piglit_is_core_profile? If so, I'd call that
> counterintuitive.

Just checked and yes. It seems that you can have a GL 3.1 version,
without GL_ARB_compatibility, and get piglit_is_core_profile as false.
At  least is what I found using
MESA_GL_VERSION_OVERRIDE/MESA_EXTENSION_OVERRIDE, that it is the easier
way to test all this, but at the same time it is a kind of "fake"
environment.

So I think that for now, it would be better to keep the slightly
over-checked condition. It is also easier to read (imho).

I have just rebased and updated also patch2 with your review. As patch3
is not needed anymore, I will resend the series.

Again, sorry for not replying you before. And thanks for the feedback.

>
>   -ilia
>
>> +
>>          glGetIntegerv(GL_MAX_SAMPLES, &max_samples);
>> -        for (i = 0; i < ARRAY_SIZE(valid_formats); i++) {
>> +        for (i = 0; i < valid_formats_size; i++) {
>>                  pass = try(GL_RENDERBUFFER,
>>                             valid_formats[i],
>>                             max_samples,
>> @@ -345,7 +359,7 @@ piglit_init(int argc, char **argv)
>>                                  max_samples_name = "GL_MAX_COLOR_TEXTURE_SAMPLES";
>>                          }
>>
>> -                        for (j = 0; j < ARRAY_SIZE(valid_formats); j++) {
>> +                        for (j = 0; j < valid_formats_size; j++) {
>>                                  pass = try(valid_targets_with_tms[i],
>>                                             valid_formats[j],
>>                                             max_samples,
>> --
>> 2.11.0
>>
>> _______________________________________________
>> Piglit mailing list
>> Piglit at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/piglit