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

[Chad Versace]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

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

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
    BadValue.

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
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iQIcBAEBAgAGBQJO8RrBAAoJEAIvNt057x8i8cYP/1fJcirC4T8s+g3CP41woPD4
UH4SrPKE9snnoOgxF0/94R8eRWL58ui/7FHOEHPr6/0jq+YFCP3d5alJOOUnDgOi
qeG7Vl3WfpCG3edngipGn9LokWdQ7mAM6gGeuXORkm8N/+rVT9fYGgmv4iN6Erla
ypxN9lhvZnoW623Dp+8NLNyql5hKNSp1ok0G+YL1JraHgM0Yr/+rZ9ezM6aCTULU
cEpyE6SElKsQkLupCZvx3ZY6K5tdjwMyYSPDPeE6hZM0ryYgkf8pC1O0NdG5TtFN
9vVXmVtJS7AdkrvYngA4x3uOAj/XPKMDfmWttZn4FXdBHCdxhu+VKavv78RutyED
huvbSdLxmbMEgEEHXs/9hnLbn5/WKBISo0xoGNb8Ngz7sB7OjVLAwCblpZ1V6Rbo
t9XUi+nxC1IUlfVJhnofGBDu/fqo8dt390PA0z9TfMcucmMSXZsIhE71r9I/an2Y
TdBoLvEWwKaTMzt/1Zh9XzvDvI5yLqr1qjuH0rDsfKwQqMi8jxcAblHGYgJk/ZDy
2CDj3i/R0V37nYd4EYBuxnyeuy6TGdJkU92+Nu1dnlRGnMLv4UFwe6Z9zhYQUyvU
NkLXfr4d85VKCBVAL4dFScnnh8tNYaLfCq4HMf7TmH/XGg8dhG0cM9Bnn98TUKII
zxJ/Ix3W7v4LhK/u1+yj
=nTf2
-----END PGP SIGNATURE-----