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

Chad Versace chad.versace at linux.intel.com
Tue Dec 20 15:31:14 PST 2011

Hash: SHA1

On 12/20/2011 02:50 PM, Ian Romanick wrote:
> On 12/20/2011 01:44 PM, Chad Versace wrote:
>> 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.

Ah! It all makes sense now, but only after reading this spec snippet:
    If GLX_ARB_create_context_profile is not supported, then the
    GLX_CONTEXT_PROFILE_MASK_ARB attribute [is] not defined, and
    specifying the attribute in <attribList> attribute will generate

It makes sense why the spec writers put both GLX_ARB_create_context and GLX_ARB_create_context_profile in
the same spec document. But that choice made some parts of the spec not as clear as I'd like.

Please put that spec quote in the test, and this is
Reviewed-by: Chad Versace <chad.versace at linux.intel.com>

- ----
Chad Versace
chad.versace at linux.intel.com
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/


More information about the Piglit mailing list