[Piglit] [PATCH 3/4] arb_internalformat_query2: fix TBO testing

[Marek Olšák]

On Tue, Oct 24, 2017 at 8:54 AM, Alejandro Piñeiro <apinheiro at 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 at 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 at igalia.com> wrote:
>>>>> On 21/10/17 14:55, Marek Olšák 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.
>>>>> 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