[3/4] arb_internalformat_query2: fix TBO testing

Submitted by Marek Olšák on Oct. 21, 2017, 12:55 p.m.

Details

Message ID 1508590505-4715-3-git-send-email-maraeo@gmail.com
State New
Headers show
Series "Series without cover letter" ( rev: 1 ) in Piglit

Not browsing as part of any series.

Commit Message

Marek Olšák Oct. 21, 2017, 12:55 p.m.
From: Marek Olšák <marek.olsak@amd.com>

This was never tested and it uses functions that are illegal with TBOs,
like glGetTexParameteriv and glTexImage2DMultisample.
---
 tests/spec/arb_internalformat_query2/common.h                         | 4 +++-
 tests/spec/arb_internalformat_query2/format-components.c              | 2 +-
 tests/spec/arb_internalformat_query2/generic-pname-checks.c           | 2 +-
 .../spec/arb_internalformat_query2/image-format-compatibility-type.c  | 2 +-
 4 files changed, 6 insertions(+), 4 deletions(-)

Patch hide | download patch | download mbox

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,
 };
 
+#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/format-components.c b/tests/spec/arb_internalformat_query2/format-components.c
index a484e01..983091f 100644
--- a/tests/spec/arb_internalformat_query2/format-components.c
+++ b/tests/spec/arb_internalformat_query2/format-components.c
@@ -244,21 +244,21 @@  check_format_components(void)
         test_data *data = test_data_new(0, 1);
         unsigned i;
         int testing64;
 
         for (i = 0; i < ARRAY_SIZE(pnames); i++) {
                 bool pass = true;
 
                 for (testing64 = 0; testing64 <= 1; testing64++) {
                         test_data_set_testing64(data, testing64);
 
-                        pass = try(valid_targets, ARRAY_SIZE(valid_targets),
+                        pass = try(valid_targets, NUM_TARGETS_EXCEPT_TBO,
                                    valid_internalformats, ARRAY_SIZE(valid_internalformats),
                                    pnames[i], data)
                                 && pass;
                 }
 
                 piglit_report_subtest_result(pass ? PIGLIT_PASS : PIGLIT_FAIL,
                                              "%s", piglit_get_gl_enum_name(pnames[i]));
 
                 check_pass = check_pass && pass;
         }
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,
                                          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,
                                  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);

Comments

e.g. the format-components test doesn't seem like it should be doing
anything illegal. GL_TEXTURE_BUFFER is fine to pass into
glGetInternalformat*. IMHO it'd be better to fix the tests and/or mesa
than just disable TBO's in the tests.

On Sat, Oct 21, 2017 at 8:55 AM, Marek Olšák <maraeo@gmail.com> wrote:
> From: Marek Olšák <marek.olsak@amd.com>
>
> This was never tested and it uses functions that are illegal with TBOs,
> like glGetTexParameteriv and glTexImage2DMultisample.
> ---
>  tests/spec/arb_internalformat_query2/common.h                         | 4 +++-
>  tests/spec/arb_internalformat_query2/format-components.c              | 2 +-
>  tests/spec/arb_internalformat_query2/generic-pname-checks.c           | 2 +-
>  .../spec/arb_internalformat_query2/image-format-compatibility-type.c  | 2 +-
>  4 files changed, 6 insertions(+), 4 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,
>  };
>
> +#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/format-components.c b/tests/spec/arb_internalformat_query2/format-components.c
> index a484e01..983091f 100644
> --- a/tests/spec/arb_internalformat_query2/format-components.c
> +++ b/tests/spec/arb_internalformat_query2/format-components.c
> @@ -244,21 +244,21 @@ check_format_components(void)
>          test_data *data = test_data_new(0, 1);
>          unsigned i;
>          int testing64;
>
>          for (i = 0; i < ARRAY_SIZE(pnames); i++) {
>                  bool pass = true;
>
>                  for (testing64 = 0; testing64 <= 1; testing64++) {
>                          test_data_set_testing64(data, testing64);
>
> -                        pass = try(valid_targets, ARRAY_SIZE(valid_targets),
> +                        pass = try(valid_targets, NUM_TARGETS_EXCEPT_TBO,
>                                     valid_internalformats, ARRAY_SIZE(valid_internalformats),
>                                     pnames[i], data)
>                                  && pass;
>                  }
>
>                  piglit_report_subtest_result(pass ? PIGLIT_PASS : PIGLIT_FAIL,
>                                               "%s", piglit_get_gl_enum_name(pnames[i]));
>
>                  check_pass = check_pass && pass;
>          }
> 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,
>                                           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,
>                                   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@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/piglit
On 21/10/17 14:55, Marek Olšák wrote:
> From: Marek Olšák <marek.olsak@amd.com>
>
> This was never tested and it uses functions that are illegal with TBOs,
> like glGetTexParameteriv and glTexImage2DMultisample.

Hi, I didn't check all the patch in detail, but as Illia is saying, it
is correct to pass TEXTURE_BUFFER to GetInternalFormat*. I also disagree
that this case it is not tested, or at least for the case I checked. See
below, I have two extra comments.

> ---
>  tests/spec/arb_internalformat_query2/common.h                         | 4 +++-
>  tests/spec/arb_internalformat_query2/format-components.c              | 2 +-
>  tests/spec/arb_internalformat_query2/generic-pname-checks.c           | 2 +-
>  .../spec/arb_internalformat_query2/image-format-compatibility-type.c  | 2 +-
>  4 files changed, 6 insertions(+), 4 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,
>  };
>  
> +#define NUM_TARGETS_EXCEPT_TBO (ARRAY_SIZE(valid_targets) - 1)
> +

Not sure if this is really needed, as we have texture_targets, defined
on common.h too, that is valid_targets except TEXTURE_BUFFER and
RENDERBUFFER. The latter also need some special handling.

>  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/format-components.c b/tests/spec/arb_internalformat_query2/format-components.c
> index a484e01..983091f 100644
> --- a/tests/spec/arb_internalformat_query2/format-components.c
> +++ b/tests/spec/arb_internalformat_query2/format-components.c
> @@ -244,21 +244,21 @@ check_format_components(void)
>          test_data *data = test_data_new(0, 1);
>          unsigned i;
>          int testing64;
>  
>          for (i = 0; i < ARRAY_SIZE(pnames); i++) {
>                  bool pass = true;
>  
>                  for (testing64 = 0; testing64 <= 1; testing64++) {
>                          test_data_set_testing64(data, testing64);
>  
> -                        pass = try(valid_targets, ARRAY_SIZE(valid_targets),
> +                        pass = try(valid_targets, NUM_TARGETS_EXCEPT_TBO,
>                                     valid_internalformats, ARRAY_SIZE(valid_internalformats),
>                                     pnames[i], data)
>                                  && pass;
>                  }
>  
>                  piglit_report_subtest_result(pass ? PIGLIT_PASS : PIGLIT_FAIL,
>                                               "%s", piglit_get_gl_enum_name(pnames[i]));
>  
>                  check_pass = check_pass && pass;
>          }
> 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,
>                                           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,

Your point is that TEXTURE_BUFFER shouldn't be tested here. But it is
tested in the sense that for this specific pname, querying should return
GL_NONE.

C&P the comment at try_local:
/* From the spec:
 *
 * "- IMAGE_FORMAT_COMPATIBILITY_TYPE: The matching criteria use for
 *   the resource when used as an image textures is returned in
 *   <params>. This is equivalent to calling GetTexParameter with
 *   <value> set to IMAGE_FORMAT_COMPATIBILITY_TYPE. Possible values
 *   are IMAGE_FORMAT_COMPATIBILITY_BY_SIZE or
 *   IMAGE_FORMAT_COMPATIBILITY_BY_CLASS.  If the resource is not
 *   supported for image textures, or if image textures are not
 *   supported, NONE is returned."
 *
 * So try_local is equivalent to try_basic, except that instead of
 * checking against a list of possible value, we test against the
 * value returned by GetTexParameter, or against GL_NONE if not
 * supported of if it is not a texture.
 */
 
So in this case, TBO is tested (or should be, unless a bug on the test),
but the test is just to confirm that we get a GL_NONE. On the code is
checked against texture_targets. As it doesn't belong there, it is not
used  glGetTexParameteriv  and just checks if GL_NONE is returned.

I still need to check the other cases though.

>                                   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);
On Sun, Oct 22, 2017 at 6:33 AM, Alejandro Piñeiro <apinheiro@igalia.com> wrote:
> On 21/10/17 14:55, Marek Olšák wrote:
>> From: Marek Olšák <marek.olsak@amd.com>
>>
>> This was never tested and it uses functions that are illegal with TBOs,
>> like glGetTexParameteriv and glTexImage2DMultisample.
>
> Hi, I didn't check all the patch in detail, but as Illia is saying, it
> is correct to pass TEXTURE_BUFFER to GetInternalFormat*. I also disagree
> that this case it is not tested, or at least for the case I checked. See
> below, I have two extra comments.

It's untested because these are all run in compat contexts, which in
turn don't have GL_TEXTURE_BUFFER support. [At least untested against
current mesa.]

  -ilia
On 22/10/17 17:21, Ilia Mirkin wrote:
> On Sun, Oct 22, 2017 at 6:33 AM, Alejandro Piñeiro <apinheiro@igalia.com> wrote:
>> On 21/10/17 14:55, Marek Olšák wrote:
>>> From: Marek Olšák <marek.olsak@amd.com>
>>>
>>> This was never tested and it uses functions that are illegal with TBOs,
>>> like glGetTexParameteriv and glTexImage2DMultisample.
>> Hi, I didn't check all the patch in detail, but as Illia is saying, it
>> is correct to pass TEXTURE_BUFFER to GetInternalFormat*. I also disagree
>> that this case it is not tested, or at least for the case I checked. See
>> below, I have two extra comments.
> It's untested because these are all run in compat contexts, which in
> turn don't have GL_TEXTURE_BUFFER support. [At least untested against
> current mesa.]

Ah ok. In any case, somehow is still testing it, as in compat mode it
should return NONE, as you say, because on that case it doesn't support
TEXTURE_BUFFER (dependencies are checked).

In any case, as I said (you mentioned it before) the solution is not
test TBO, but as you said, fix the piglit tests to run in non compat
modes. Probably improve more than fix, in order to test both compat and
non compat modes.

BR
On Sun, Oct 22, 2017 at 7:55 PM, Alejandro Piñeiro <apinheiro@igalia.com> wrote:
> On 22/10/17 17:21, Ilia Mirkin wrote:
>> On Sun, Oct 22, 2017 at 6:33 AM, Alejandro Piñeiro <apinheiro@igalia.com> wrote:
>>> On 21/10/17 14:55, Marek Olšák wrote:
>>>> From: Marek Olšák <marek.olsak@amd.com>
>>>>
>>>> This was never tested and it uses functions that are illegal with TBOs,
>>>> like glGetTexParameteriv and glTexImage2DMultisample.
>>> Hi, I didn't check all the patch in detail, but as Illia is saying, it
>>> is correct to pass TEXTURE_BUFFER to GetInternalFormat*. I also disagree
>>> that this case it is not tested, or at least for the case I checked. See
>>> below, I have two extra comments.
>> It's untested because these are all run in compat contexts, which in
>> turn don't have GL_TEXTURE_BUFFER support. [At least untested against
>> current mesa.]
>
> Ah ok. In any case, somehow is still testing it, as in compat mode it
> should return NONE, as you say, because on that case it doesn't support
> TEXTURE_BUFFER (dependencies are checked).

I think GL_INVALID_ENUM is returned by Mesa, because GL_TEXTURE_BUFFER
is not a valid enum in OpenGL 3.0 + ARB_internalformat_query2.

>
> In any case, as I said (you mentioned it before) the solution is not
> test TBO, but as you said, fix the piglit tests to run in non compat
> modes. Probably improve more than fix, in order to test both compat and
> non compat modes.

My primary goal is adjust the tests such they don't fail with OpenGL
3.1. Of course, people can improve/fix the tests if they want.

Marek
On Sun, Oct 22, 2017 at 12:33 PM, Alejandro Piñeiro
<apinheiro@igalia.com> wrote:
> On 21/10/17 14:55, Marek Olšák wrote:
>> From: Marek Olšák <marek.olsak@amd.com>
>>
>> This was never tested and it uses functions that are illegal with TBOs,
>> like glGetTexParameteriv and glTexImage2DMultisample.
>
> Hi, I didn't check all the patch in detail, but as Illia is saying, it
> is correct to pass TEXTURE_BUFFER to GetInternalFormat*. I also disagree
> that this case it is not tested, or at least for the case I checked. See
> below, I have two extra comments.
>
>> ---
>>  tests/spec/arb_internalformat_query2/common.h                         | 4 +++-
>>  tests/spec/arb_internalformat_query2/format-components.c              | 2 +-
>>  tests/spec/arb_internalformat_query2/generic-pname-checks.c           | 2 +-
>>  .../spec/arb_internalformat_query2/image-format-compatibility-type.c  | 2 +-
>>  4 files changed, 6 insertions(+), 4 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,
>>  };
>>
>> +#define NUM_TARGETS_EXCEPT_TBO (ARRAY_SIZE(valid_targets) - 1)
>> +
>
> Not sure if this is really needed, as we have texture_targets, defined
> on common.h too, that is valid_targets except TEXTURE_BUFFER and
> RENDERBUFFER. The latter also need some special handling.
>
>>  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/format-components.c b/tests/spec/arb_internalformat_query2/format-components.c
>> index a484e01..983091f 100644
>> --- a/tests/spec/arb_internalformat_query2/format-components.c
>> +++ b/tests/spec/arb_internalformat_query2/format-components.c
>> @@ -244,21 +244,21 @@ check_format_components(void)
>>          test_data *data = test_data_new(0, 1);
>>          unsigned i;
>>          int testing64;
>>
>>          for (i = 0; i < ARRAY_SIZE(pnames); i++) {
>>                  bool pass = true;
>>
>>                  for (testing64 = 0; testing64 <= 1; testing64++) {
>>                          test_data_set_testing64(data, testing64);
>>
>> -                        pass = try(valid_targets, ARRAY_SIZE(valid_targets),
>> +                        pass = try(valid_targets, NUM_TARGETS_EXCEPT_TBO,
>>                                     valid_internalformats, ARRAY_SIZE(valid_internalformats),
>>                                     pnames[i], data)
>>                                  && pass;
>>                  }
>>
>>                  piglit_report_subtest_result(pass ? PIGLIT_PASS : PIGLIT_FAIL,
>>                                               "%s", piglit_get_gl_enum_name(pnames[i]));
>>
>>                  check_pass = check_pass && pass;
>>          }
>> 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,
>>                                           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,
>
> Your point is that TEXTURE_BUFFER shouldn't be tested here. But it is
> tested in the sense that for this specific pname, querying should return
> GL_NONE.
>
> C&P the comment at try_local:
> /* From the spec:
>  *
>  * "- IMAGE_FORMAT_COMPATIBILITY_TYPE: The matching criteria use for
>  *   the resource when used as an image textures is returned in
>  *   <params>. This is equivalent to calling GetTexParameter with
>  *   <value> set to IMAGE_FORMAT_COMPATIBILITY_TYPE. Possible values
>  *   are IMAGE_FORMAT_COMPATIBILITY_BY_SIZE or
>  *   IMAGE_FORMAT_COMPATIBILITY_BY_CLASS.  If the resource is not
>  *   supported for image textures, or if image textures are not
>  *   supported, NONE is returned."
>  *
>  * So try_local is equivalent to try_basic, except that instead of
>  * checking against a list of possible value, we test against the
>  * value returned by GetTexParameter, or against GL_NONE if not
>  * supported of if it is not a texture.
>  */
>
> So in this case, TBO is tested (or should be, unless a bug on the test),
> but the test is just to confirm that we get a GL_NONE. On the code is
> checked against texture_targets. As it doesn't belong there, it is not
> used  glGetTexParameteriv  and just checks if GL_NONE is returned.

This test does use glGetTexParameter(GL_TEXTURE_BUFFER).

Marek
I just sent patch version 2 separately.

Marek

On Tue, Oct 24, 2017 at 3:15 AM, Marek Olšák <maraeo@gmail.com> wrote:
> On Sun, Oct 22, 2017 at 12:33 PM, Alejandro Piñeiro
> <apinheiro@igalia.com> wrote:
>> On 21/10/17 14:55, Marek Olšák wrote:
>>> From: Marek Olšák <marek.olsak@amd.com>
>>>
>>> This was never tested and it uses functions that are illegal with TBOs,
>>> like glGetTexParameteriv and glTexImage2DMultisample.
>>
>> Hi, I didn't check all the patch in detail, but as Illia is saying, it
>> is correct to pass TEXTURE_BUFFER to GetInternalFormat*. I also disagree
>> that this case it is not tested, or at least for the case I checked. See
>> below, I have two extra comments.
>>
>>> ---
>>>  tests/spec/arb_internalformat_query2/common.h                         | 4 +++-
>>>  tests/spec/arb_internalformat_query2/format-components.c              | 2 +-
>>>  tests/spec/arb_internalformat_query2/generic-pname-checks.c           | 2 +-
>>>  .../spec/arb_internalformat_query2/image-format-compatibility-type.c  | 2 +-
>>>  4 files changed, 6 insertions(+), 4 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,
>>>  };
>>>
>>> +#define NUM_TARGETS_EXCEPT_TBO (ARRAY_SIZE(valid_targets) - 1)
>>> +
>>
>> Not sure if this is really needed, as we have texture_targets, defined
>> on common.h too, that is valid_targets except TEXTURE_BUFFER and
>> RENDERBUFFER. The latter also need some special handling.
>>
>>>  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/format-components.c b/tests/spec/arb_internalformat_query2/format-components.c
>>> index a484e01..983091f 100644
>>> --- a/tests/spec/arb_internalformat_query2/format-components.c
>>> +++ b/tests/spec/arb_internalformat_query2/format-components.c
>>> @@ -244,21 +244,21 @@ check_format_components(void)
>>>          test_data *data = test_data_new(0, 1);
>>>          unsigned i;
>>>          int testing64;
>>>
>>>          for (i = 0; i < ARRAY_SIZE(pnames); i++) {
>>>                  bool pass = true;
>>>
>>>                  for (testing64 = 0; testing64 <= 1; testing64++) {
>>>                          test_data_set_testing64(data, testing64);
>>>
>>> -                        pass = try(valid_targets, ARRAY_SIZE(valid_targets),
>>> +                        pass = try(valid_targets, NUM_TARGETS_EXCEPT_TBO,
>>>                                     valid_internalformats, ARRAY_SIZE(valid_internalformats),
>>>                                     pnames[i], data)
>>>                                  && pass;
>>>                  }
>>>
>>>                  piglit_report_subtest_result(pass ? PIGLIT_PASS : PIGLIT_FAIL,
>>>                                               "%s", piglit_get_gl_enum_name(pnames[i]));
>>>
>>>                  check_pass = check_pass && pass;
>>>          }
>>> 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,
>>>                                           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,
>>
>> Your point is that TEXTURE_BUFFER shouldn't be tested here. But it is
>> tested in the sense that for this specific pname, querying should return
>> GL_NONE.
>>
>> C&P the comment at try_local:
>> /* From the spec:
>>  *
>>  * "- IMAGE_FORMAT_COMPATIBILITY_TYPE: The matching criteria use for
>>  *   the resource when used as an image textures is returned in
>>  *   <params>. This is equivalent to calling GetTexParameter with
>>  *   <value> set to IMAGE_FORMAT_COMPATIBILITY_TYPE. Possible values
>>  *   are IMAGE_FORMAT_COMPATIBILITY_BY_SIZE or
>>  *   IMAGE_FORMAT_COMPATIBILITY_BY_CLASS.  If the resource is not
>>  *   supported for image textures, or if image textures are not
>>  *   supported, NONE is returned."
>>  *
>>  * So try_local is equivalent to try_basic, except that instead of
>>  * checking against a list of possible value, we test against the
>>  * value returned by GetTexParameter, or against GL_NONE if not
>>  * supported of if it is not a texture.
>>  */
>>
>> So in this case, TBO is tested (or should be, unless a bug on the test),
>> but the test is just to confirm that we get a GL_NONE. On the code is
>> checked against texture_targets. As it doesn't belong there, it is not
>> used  glGetTexParameteriv  and just checks if GL_NONE is returned.
>
> This test does use glGetTexParameter(GL_TEXTURE_BUFFER).
>
> Marek
On 24/10/17 01:38, Marek Olšák wrote:
> On Sun, Oct 22, 2017 at 7:55 PM, Alejandro Piñeiro <apinheiro@igalia.com> wrote:
>> On 22/10/17 17:21, Ilia Mirkin wrote:
>>> On Sun, Oct 22, 2017 at 6:33 AM, Alejandro Piñeiro <apinheiro@igalia.com> wrote:
>>>> On 21/10/17 14:55, Marek Olšák wrote:
>>>>> From: Marek Olšák <marek.olsak@amd.com>
>>>>>
>>>>> This was never tested and it uses functions that are illegal with TBOs,
>>>>> like glGetTexParameteriv and glTexImage2DMultisample.
>>>> Hi, I didn't check all the patch in detail, but as Illia is saying, it
>>>> is correct to pass TEXTURE_BUFFER to GetInternalFormat*. I also disagree
>>>> that this case it is not tested, or at least for the case I checked. See
>>>> below, I have two extra comments.
>>> It's untested because these are all run in compat contexts, which in
>>> turn don't have GL_TEXTURE_BUFFER support. [At least untested against
>>> current mesa.]
>> Ah ok. In any case, somehow is still testing it, as in compat mode it
>> should return NONE, as you say, because on that case it doesn't support
>> TEXTURE_BUFFER (dependencies are checked).
> I think GL_INVALID_ENUM is returned by Mesa, because GL_TEXTURE_BUFFER
> is not a valid enum in OpenGL 3.0 + ARB_internalformat_query2.

It shouldn't return GL_INVALID_ENUM. And if it does, it is a bug on mesa.

GL_TEXTURE_BUFFER is a valid enum in OpenGL 3.0 +
ARB_internalformat_query2. In fact GL_TEXTURE_BUFFER is a valid enum in
OpenGL 2.0 + ARB_internalformat_query2. From the spec:

https://www.khronos.org/registry/OpenGL/extensions/ARB/ARB_internalformat_query2.txt


"Dependencies OpenGL 2.0 or OpenGL ES 2.0 is required."

<skip>

"

New Tokens

    Accepted by the <target> parameter of GetInternalformativ
    and GetInternalformati64v:

        TEXTURE_1D                                      0x0DE0
        TEXTURE_1D_ARRAY                                0x8C18
        TEXTURE_2D                                      0x0DE1
        TEXTURE_2D_ARRAY                                0x8C1A
        TEXTURE_3D                                      0x806F
        TEXTURE_CUBE_MAP                                0x8513
        TEXTURE_CUBE_MAP_ARRAY                          0x9009
        TEXTURE_RECTANGLE                               0x84F5
        TEXTURE_BUFFER                                  0x8C2A
        RENDERBUFFER                                    0x8D41
        TEXTURE_2D_MULTISAMPLE                          0x9100
        TEXTURE_2D_MULTISAMPLE_ARRAY                    0x9102
"

So if the enums are not available because of the specific extensions,
they are added by this extension itself.

<skip>

"Dependencies on ARB_texture_buffer_object

    If ARB_texture_buffer_object or equivalent functionality is not supported,
    queries for the TEXTURE_BUFFER <target> parameter will return the
    appropriate unsupported response. Ignore all references to TexBuffer."


So even without ARB_texture_buffer_object extension, you can query TBO
info, but you get a unsupported response.

>
>> In any case, as I said (you mentioned it before) the solution is not
>> test TBO, but as you said, fix the piglit tests to run in non compat
>> modes. Probably improve more than fix, in order to test both compat and
>> non compat modes.
> My primary goal is adjust the tests such they don't fail with OpenGL
> 3.1. Of course, people can improve/fix the tests if they want.

Ok, makes sense. But I still think that skipping TBO is not the way to
go. If they are failing is for other reason. It is possible to reproduce
those test failures with a intel gpu somehow (envvars or whatever) so I
could take a look to it?

BR
On Tue, Oct 24, 2017 at 8:54 AM, Alejandro Piñeiro <apinheiro@igalia.com> wrote:
> On 24/10/17 01:38, Marek Olšák wrote:
>> On Sun, Oct 22, 2017 at 7:55 PM, Alejandro Piñeiro <apinheiro@igalia.com> wrote:
>>> On 22/10/17 17:21, Ilia Mirkin wrote:
>>>> On Sun, Oct 22, 2017 at 6:33 AM, Alejandro Piñeiro <apinheiro@igalia.com> wrote:
>>>>> On 21/10/17 14:55, Marek Olšák wrote:
>>>>>> From: Marek Olšák <marek.olsak@amd.com>
>>>>>>
>>>>>> This was never tested and it uses functions that are illegal with TBOs,
>>>>>> like glGetTexParameteriv and glTexImage2DMultisample.
>>>>> Hi, I didn't check all the patch in detail, but as Illia is saying, it
>>>>> is correct to pass TEXTURE_BUFFER to GetInternalFormat*. I also disagree
>>>>> that this case it is not tested, or at least for the case I checked. See
>>>>> below, I have two extra comments.
>>>> It's untested because these are all run in compat contexts, which in
>>>> turn don't have GL_TEXTURE_BUFFER support. [At least untested against
>>>> current mesa.]
>>> Ah ok. In any case, somehow is still testing it, as in compat mode it
>>> should return NONE, as you say, because on that case it doesn't support
>>> TEXTURE_BUFFER (dependencies are checked).
>> I think GL_INVALID_ENUM is returned by Mesa, because GL_TEXTURE_BUFFER
>> is not a valid enum in OpenGL 3.0 + ARB_internalformat_query2.
>
> It shouldn't return GL_INVALID_ENUM. And if it does, it is a bug on mesa.
>
> GL_TEXTURE_BUFFER is a valid enum in OpenGL 3.0 +
> ARB_internalformat_query2. In fact GL_TEXTURE_BUFFER is a valid enum in
> OpenGL 2.0 + ARB_internalformat_query2. From the spec:
>
> https://www.khronos.org/registry/OpenGL/extensions/ARB/ARB_internalformat_query2.txt
>
>
> "Dependencies OpenGL 2.0 or OpenGL ES 2.0 is required."
>
> <skip>
>
> "
>
> New Tokens
>
>     Accepted by the <target> parameter of GetInternalformativ
>     and GetInternalformati64v:
>
>         TEXTURE_1D                                      0x0DE0
>         TEXTURE_1D_ARRAY                                0x8C18
>         TEXTURE_2D                                      0x0DE1
>         TEXTURE_2D_ARRAY                                0x8C1A
>         TEXTURE_3D                                      0x806F
>         TEXTURE_CUBE_MAP                                0x8513
>         TEXTURE_CUBE_MAP_ARRAY                          0x9009
>         TEXTURE_RECTANGLE                               0x84F5
>         TEXTURE_BUFFER                                  0x8C2A
>         RENDERBUFFER                                    0x8D41
>         TEXTURE_2D_MULTISAMPLE                          0x9100
>         TEXTURE_2D_MULTISAMPLE_ARRAY                    0x9102
> "
>
> So if the enums are not available because of the specific extensions,
> they are added by this extension itself.
>
> <skip>
>
> "Dependencies on ARB_texture_buffer_object
>
>     If ARB_texture_buffer_object or equivalent functionality is not supported,
>     queries for the TEXTURE_BUFFER <target> parameter will return the
>     appropriate unsupported response. Ignore all references to TexBuffer."
>
>
> So even without ARB_texture_buffer_object extension, you can query TBO
> info, but you get a unsupported response.
>
>>
>>> In any case, as I said (you mentioned it before) the solution is not
>>> test TBO, but as you said, fix the piglit tests to run in non compat
>>> modes. Probably improve more than fix, in order to test both compat and
>>> non compat modes.
>> My primary goal is adjust the tests such they don't fail with OpenGL
>> 3.1. Of course, people can improve/fix the tests if they want.
>
> Ok, makes sense. But I still think that skipping TBO is not the way to
> go. If they are failing is for other reason. It is possible to reproduce
> those test failures with a intel gpu somehow (envvars or whatever) so I
> could take a look to it?

You need these 4 commits:
https://cgit.freedesktop.org/~mareko/mesa/log/?h=gl31

And the 3rd one should be changed to enable TBOs in i965 with a compat profile.

Marek