[Piglit] [PATCH 4/4] egl_khr_create_context_profile: Verify that the profile is ignored pre-3.2

[Matt Turner]

On Wed, Oct 10, 2012 at 11:04 AM, Chad Versace
<chad.versace at linux.intel.com> wrote:
> On 10/08/2012 04:57 PM, Matt Turner wrote:
>> ---
>>  tests/all_egl.tests                                |    1 +
>>  .../spec/egl_khr_create_context/CMakeLists.gl.txt  |    1 +
>>  .../spec/egl_khr_create_context/pre-GL3-profile.c  |  115 ++++++++++++++++++++
>
> I think "pre-GL32-profile" would be a better name for this test. The name
> "pre-GL3-profile" is misleading.

Reasonable. Do we also rename the GLX test?

>>  3 files changed, 117 insertions(+), 0 deletions(-)
>>  create mode 100644 tests/egl/spec/egl_khr_create_context/pre-GL3-profile.c
>>
>> diff --git a/tests/all_egl.tests b/tests/all_egl.tests
>> index cd8a596..655c697 100644
>> --- a/tests/all_egl.tests
>> +++ b/tests/all_egl.tests
>> @@ -36,3 +36,4 @@ create_context['invalid flag GL'] = plain_test('egl-create-context-invalid-flag-
>>  create_context['valid forward-compatible flag GL'] = plain_test('egl-create-context-valid-flag-forward-compatible-gl')
>>  create_context['invalid profile'] = plain_test('egl-create-context-invalid-profile')
>>  create_context['3.2 core profile required'] = plain_test('egl-create-context-core-profile')
>> +create_context['pre-GL3 profile'] = plain_test('egl-create-context-pre-GL3-profile')
>> diff --git a/tests/egl/spec/egl_khr_create_context/CMakeLists.gl.txt b/tests/egl/spec/egl_khr_create_context/CMakeLists.gl.txt
>> index 656291c..7759546 100644
>> --- a/tests/egl/spec/egl_khr_create_context/CMakeLists.gl.txt
>> +++ b/tests/egl/spec/egl_khr_create_context/CMakeLists.gl.txt
>> @@ -25,6 +25,7 @@ piglit_add_executable (egl-create-context-invalid-attribute-gl invalid-attribute
>>  piglit_add_executable (egl-create-context-invalid-flag-gles invalid-flag-gles.c common.c)
>>  piglit_add_executable (egl-create-context-invalid-flag-gl invalid-flag-gl.c common.c)
>>  piglit_add_executable (egl-create-context-invalid-profile invalid-profile.c common.c)
>> +piglit_add_executable (egl-create-context-pre-GL3-profile pre-GL3-profile.c common.c)
>>  piglit_add_executable (egl-create-context-valid-flag-forward-compatible-gl valid-flag-forward-compatible-gl.c common.c)
>>  piglit_add_executable (egl-create-context-core-profile core-profile.c common.c)
>>
>> diff --git a/tests/egl/spec/egl_khr_create_context/pre-GL3-profile.c b/tests/egl/spec/egl_khr_create_context/pre-GL3-profile.c
>> new file mode 100644
>> index 0000000..afea2b3
>> --- /dev/null
>> +++ b/tests/egl/spec/egl_khr_create_context/pre-GL3-profile.c
>> @@ -0,0 +1,115 @@
>
> Again, like patch 2/4, the test needs some brief documentation here at the top
> of the file that explains what the test is attempting to verify and how it goes
> about it. I suggest something along these lines:
>
> """
> This test verifies that the context profile bit is ignored for GL versions less
> than 3.2.
>
> The test iterates over several OpenGL versions less than 3.2. For each version,
> the test tries to create a context with no profile specified. If context
> creation succeeds, it then verifies that context creation also succeeds with a
> core profile specified. If context creation succeeds with the core profile
> specified, then the test assumes that the profile bit as correctly ignored
> because there exists no core profile for GL versions less then 3.2.
>
> From the EGL_KHR_create_context spec:
>
>     When the current rendering API is EGL_OPENGL_API, the value of
>     EGL_CONTEXT_OPENGL_PROFILE_MASK_KHR requests an OpenGL context
>     supporting the corresponding profile... If the requested OpenGL
>     version is less than 3.2, EGL_CONTEXT_OPENGL_PROFILE_MASK_KHR is
>     ignored and the functionality of the context is determined solely
>     by the requested version.
> """"

Do you think this adds significantly to the comment in main()?

         *     "When the current rendering API is EGL_OPENGL_API, the value of
         *     EGL_CONTEXT_OPENGL_PROFILE_MASK_KHR requests an OpenGL context
         *     supporting the corresponding profile... If the requested OpenGL
         *     version is less than 3.2, EGL_CONTEXT_OPENGL_PROFILE_MASK_KHR is
         *     ignored and the functionality of the context is determined solely
         *     by the requested version."
         *
         * Try to create a context without any profile specified.  If that
         * works, try to create a context with the core profile specified.
         * That should also work.

I don't really think it does. None of the other tests have top level
comments either. All the tests do have comments explaining what's
going on though, including the tests I'm adding now.

core-profile.c:
         *     "All OpenGL 3.2 and later implementations are required to
         *     implement the core profile, but implementation of the
         *     compatibility profile is optional."
         *
         * If it is possible to create a context with the compatibility
         * profile, then it must also be possible to create a context with the
         * core profile.  Conversely, if it is not possible to create a
         * context with the core profile, it must also not be possible to create
         * a context with the compatibility profile.

invalid-profile.c:
         *     "* If an OpenGL context is requested, the requested version is
         *        greater than [sic] 3.2, and the value for attribute
         *        EGL_CONTEXT_OPENGL_PROFILE_MASK_KHR has no bits set; has any
         *        bits set other than EGL_CONTEXT_OPENGL_CORE_PROFILE_BIT_KHR
         *        and EGL_CONTEXT_OPENGL_COMPATIBILITY_PROFILE_BIT_KHR; has more
         *        than one of these bits set; or if the implementation does not
         *        support the requested profile, then an EGL_BAD_MATCH error is
         *        generated."

I could (re)duplicate this part of the spec at the top of the file and
add "Confirm that creating profiles with no bits set, multiple bits
set, or invalid bits set fails." but I don't think adding that
description helps much. Same goes for the others as well.

>> +#include "piglit-util-gl-common.h"
>> +#include "common.h"
>> +
>> +static bool try_version(int major, int minor)
>> +{
>> +     EGLContext ctx;
>> +
>> +     const EGLint attribs_with_profile[] = {
>> +             EGL_CONTEXT_MAJOR_VERSION_KHR,
>> +             major,
>> +
>> +             EGL_CONTEXT_MINOR_VERSION_KHR,
>> +             minor,
>> +
>> +             EGL_CONTEXT_OPENGL_PROFILE_MASK_KHR,
>> +             EGL_CONTEXT_OPENGL_CORE_PROFILE_BIT_KHR,
>> +
>> +             EGL_NONE
>> +     };
>> +
>> +     const EGLint attribs_without_profile[] = {
>> +             EGL_CONTEXT_MAJOR_VERSION_KHR,
>> +             major,
>> +
>> +             EGL_CONTEXT_MINOR_VERSION_KHR,
>> +             minor,
>> +
>> +             EGL_CONTEXT_OPENGL_PROFILE_MASK_KHR,
>> +             EGL_CONTEXT_OPENGL_CORE_PROFILE_BIT_KHR,
>
> Copy-and-paste error. There should be no profile in this array.

Well, in a way. Just duplicating a bug in the GLX-equivalent really :)