[Piglit] [PATCH 07/12] glx_arb_create_context: Verify that the invalid attributes are rejected

[Ian Romanick]

On 12/20/2011 01:44 PM, Chad Versace wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> On 12/20/2011 01:01 PM, Ian Romanick wrote:
>> On 12/20/2011 12:06 PM, Chad Versace wrote:
>>> On 12/14/2011 10:47 AM, Ian Romanick wrote:
>>>> From: Ian Romanick<ian.d.romanick at intel.com>
>>>>
>>>> NVIDIA's closed-source driver fails this test because it generates the
>>>> wrong X error (BadMatch instead of BadValue).  It correctly does not
>>>> create the context.
>>>>
>>>> AMD's closed-source driver fails this test becuase it creates contexts
>>>> with invalid attributes.
>>>>
>>>> Signed-off-by: Ian Romanick<ian.d.romanick at intel.com>
>>>> ---
>>>>    tests/all.tests                                    |    1 +
>>>>    .../spec/glx_arb_create_context/CMakeLists.gl.txt  |    1 +
>>>>    .../glx_arb_create_context/invalid-attribute.c     |   87 ++++++++++++++++++++
>>>>    3 files changed, 89 insertions(+), 0 deletions(-)
>>>>    create mode 100644 tests/spec/glx_arb_create_context/invalid-attribute.c
>>>>
>>>
>>> I think that the probing for GLX_CONTEXT_PROFILE_MASK_ARB needs to be more thorough and
>>> check that bits {(1<<n)|n=3..31} are invalid.
>>>
>>> Perhaps change the signature of try_attribute() to try_attribute(int attr, int value) and
>>> loop over those invalid bits?
>>
>> The check here is that it's invalid to ask for *any* profile when the GL version is less than 3.2.  I wanted to check that GLX_CONTEXT_PROFILE_MASK_ARB is rejected because of the attribute and not because of its value.  Should I add a comment to that effect?
>
> I was led to think that the attribute value did matter because try_attribute(GLX_CONTEXT_PROFILE_MASK_ARB)
> special-cases the attribute value to non-zero. A comment would have helped.
>
> But, since the actual intention is to "check that GLX_CONTEXT_PROFILE_MASK_ARB is rejected because of the
> attribute and not because of its value" for GL<  3.2, then I think the test is invalid. If I'm
> interpreting the spec correctly, the presence of GLX_CONTEXT_PROFILE_MASK_ARB is ignored if the
> requested GL version is<  3.2 and context creation proceeds regardless of the attribute's value.
> - From the spec:
>      If the requested OpenGL version is less than 3.2,
>      GLX_CONTEXT_PROFILE_MASK_ARB is ignored and the functionality of the
>      context is determined solely by the requested version.

You're right, but that's not what the test was actually testing.  I 
mis-read and mid-remembered.  The test checks that 
GLX_ARB_create_context_profile is *NOT* supported before trying to use 
that attribute.  In that case it should generate an error because the 
attribute is not supported.  A valid profile value is used for the 
reasons previously mentioned.

>> Once we add support for GLX_ARB_create_context_profile, I think we should have a test like the existing flags test.  At that time, I'm inclined to have it as a separate test.
>
> After reviewing the spec quote above, I think that the check for GLX_CONTEXT_PROFILE_MASK_ARB should be completely removed
> this test.
>
> - ----
> Chad Versace
> chad.versace at linux.intel.com