[Piglit] [PATCH] arb_internalformat_query2: fix TBO testing (v2)

[Ilia Mirkin]

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. I didn't actually run though, and this code is
pretty hard to follow. 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.

>                                   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