[Mesa-dev] [PATCH] glx: Allow to create any OpenGL ES version.
Jose Fonseca
jfonseca at vmware.com
Wed Apr 15 13:03:20 PDT 2015
On 14/04/15 17:22, Ian Romanick wrote:
> On 04/14/2015 08:36 AM, Emil Velikov wrote:
>> On 14 April 2015 at 13:52, Jose Fonseca <jfonseca at vmware.com> wrote:
>>> On 13/04/15 22:59, Ian Romanick wrote:
>>>>
>>>> On 04/10/2015 03:36 PM, Jose Fonseca wrote:
>>>>>
>>>>> From: José Fonseca <jfonseca at vmware.com>
>>>>>
>>>>> The latest version of GLX_EXT_create_context_es2_profile states:
>>>>>
>>>>> "If the version requested is a valid and supported OpenGL-ES version,
>>>>> and the GLX_CONTEXT_ES_PROFILE_BIT_EXT bit is set in the
>>>>> GLX_CONTEXT_PROFILE_MASK_ARB attribute (see below), then the context
>>>>> returned will implement the OpenGL ES version requested."
>>>>>
>>>>> We must also export EXT_create_context_es_profile too, as
>>>>> EXT_create_context_es2_profile specification is crystal clear:
>>>>>
>>>>> "NOTE: implementations of this extension must export BOTH extension
>>>>> strings, for backwards compatibility with applications written
>>>>> against version 1 of this extension."
>>>>>
>>>>> Totally untested. (Just happened to noticed this while implementing
>>>>> GLX_EXT_create_context_es2_profile for st/xlib.)
>>>>>
>>>>> Reviewed-by: Brian Paul <brianp at vmware.com>
>>>>> Reviewed-by: Emil Velikov <emil.l.velikov at gmail.com>
>>>>>
>>>>> v2: Replicate the drisw_glx.c to dri2_glx.c and dri3_glx.c as suggested
>>>>> by Emil Velikov.
>>>>> ---
>>>>> src/glx/dri2_glx.c | 5 ++++-
>>>>> src/glx/dri3_glx.c | 5 ++++-
>>>>> src/glx/dri_common.c | 32 ++++++++++++++++----------------
>>>>> src/glx/drisw_glx.c | 2 ++
>>>>> 4 files changed, 26 insertions(+), 18 deletions(-)
>>>>>
>>>>> diff --git a/src/glx/dri2_glx.c b/src/glx/dri2_glx.c
>>>>> index 462d560..8192c54 100644
>>>>> --- a/src/glx/dri2_glx.c
>>>>> +++ b/src/glx/dri2_glx.c
>>>>> @@ -1102,9 +1102,12 @@ dri2BindExtensions(struct dri2_screen *psc, struct
>>>>> glx_display * priv,
>>>>> __glXEnableDirectExtension(&psc->base, "GLX_ARB_create_context");
>>>>> __glXEnableDirectExtension(&psc->base,
>>>>> "GLX_ARB_create_context_profile");
>>>>>
>>>>> - if ((mask & (1 << __DRI_API_GLES2)) != 0)
>>>>> + if ((mask & (1 << __DRI_API_GLES2)) != 0) {
>>>>> + __glXEnableDirectExtension(&psc->base,
>>>>> + "GLX_EXT_create_context_es_profile");
>>>>> __glXEnableDirectExtension(&psc->base,
>>>>>
>>>>> "GLX_EXT_create_context_es2_profile");
>>>>> + }
>>>>> }
>>>>>
>>>>> for (i = 0; extensions[i]; i++) {
>>>>> diff --git a/src/glx/dri3_glx.c b/src/glx/dri3_glx.c
>>>>> index 1ddc723..6973ad1 100644
>>>>> --- a/src/glx/dri3_glx.c
>>>>> +++ b/src/glx/dri3_glx.c
>>>>> @@ -1825,9 +1825,12 @@ dri3_bind_extensions(struct dri3_screen *psc,
>>>>> struct glx_display * priv,
>>>>> __glXEnableDirectExtension(&psc->base, "GLX_ARB_create_context");
>>>>> __glXEnableDirectExtension(&psc->base,
>>>>> "GLX_ARB_create_context_profile");
>>>>>
>>>>> - if ((mask & (1 << __DRI_API_GLES2)) != 0)
>>>>> + if ((mask & (1 << __DRI_API_GLES2)) != 0) {
>>>>> + __glXEnableDirectExtension(&psc->base,
>>>>> + "GLX_EXT_create_context_es_profile");
>>>>> __glXEnableDirectExtension(&psc->base,
>>>>> "GLX_EXT_create_context_es2_profile");
>>>>> + }
>>>>>
>>>>> for (i = 0; extensions[i]; i++) {
>>>>> /* when on a different gpu than the server, the server pixmaps
>>>>> diff --git a/src/glx/dri_common.c b/src/glx/dri_common.c
>>>>> index 63c8de3..541abbb 100644
>>>>> --- a/src/glx/dri_common.c
>>>>> +++ b/src/glx/dri_common.c
>>>>> @@ -544,9 +544,22 @@ dri2_convert_glx_attribs(unsigned num_attribs, const
>>>>> uint32_t *attribs,
>>>>> case GLX_CONTEXT_COMPATIBILITY_PROFILE_BIT_ARB:
>>>>> *api = __DRI_API_OPENGL;
>>>>> break;
>>>>> - case GLX_CONTEXT_ES2_PROFILE_BIT_EXT:
>>>>> - *api = __DRI_API_GLES2;
>>>>> - break;
>>>>> + case GLX_CONTEXT_ES_PROFILE_BIT_EXT:
>>>>> + switch (*major_ver) {
>>>>> + case 3:
>>>>> + *api = __DRI_API_GLES3;
>>>>> + break;
>>>>> + case 2:
>>>>> + *api = __DRI_API_GLES2;
>>>>> + break;
>>>>> + case 1:
>>>>> + *api = __DRI_API_GLES;
>>>>> + break;
>>>>> + default:
>>>>> + *error = __DRI_CTX_ERROR_BAD_API;
>>>>> + return false;
>>>>> + }
>>>>> + break;
>>>>> default:
>>>>> *error = __DRI_CTX_ERROR_BAD_API;
>>>>> return false;
>>>>> @@ -577,19 +590,6 @@ dri2_convert_glx_attribs(unsigned num_attribs, const
>>>>> uint32_t *attribs,
>>>>> return false;
>>>>> }
>>>>>
>>>>> - /* The GLX_EXT_create_context_es2_profile spec says:
>>>>> - *
>>>>> - * "... If the version requested is 2.0, and the
>>>>> - * GLX_CONTEXT_ES2_PROFILE_BIT_EXT bit is set in the
>>>>> - * GLX_CONTEXT_PROFILE_MASK_ARB attribute (see below), then the
>>>>> context
>>>>> - * returned will implement OpenGL ES 2.0. This is the only way in
>>>>> which
>>>>> - * an implementation may request an OpenGL ES 2.0 context."
>>>>> - */
>>>>> - if (*api == __DRI_API_GLES2 && (*major_ver != 2 || *minor_ver != 0))
>>>>> {
>>>>> - *error = __DRI_CTX_ERROR_BAD_API;
>>>>> - return false;
>>>>> - }
>>>>
>>>>
>>>> I guess this text was removed from the extension spec, and now we rely
>>>> on some other layer detecting invalid versions (like 2.1)?
>>>
>>>
>>> Yes, the spec replaced that with
>>>
>>> "... If the version requested is a valid and supported OpenGL-ES
>>> version,
>>> and the GLX_CONTEXT_ES_PROFILE_BIT_EXT bit is set in the
>>> GLX_CONTEXT_PROFILE_MASK_ARB attribute (see below), then the context
>>> returned will implement the OpenGL ES version requested."
>>>
>>> That is, GLX_EXT_create_context_es_profile effectively allows to create any
>>> GLES version from 1.x to 3.x.
>>>
>>> And we never validated the minor version on src/glx/dri_common.c.
>>>
>>>>
>>>> This patch combined with Chad's patch seems like it should work... I'm a
>>>> little confused why Waffle doesn't like it. :(
>>>
>>>
>>> I tried to repro, but my main development machine has native NVIDIA drivers
>>> (no DRI), and somehow swrast refuses to load, even with gles2.
>>>
>>>
>>> Maybe this is what's missing:
>>>
>>> diff --git a/src/glx/dri2_glx.c b/src/glx/dri2_glx.c
>>> index 8192c54..192525a 100644
>>> --- a/src/glx/dri2_glx.c
>>> +++ b/src/glx/dri2_glx.c
>>> @@ -1102,7 +1102,7 @@ dri2BindExtensions(struct dri2_screen *psc, struct
>>> glx_display * priv,
>>> __glXEnableDirectExtension(&psc->base, "GLX_ARB_create_context");
>>> __glXEnableDirectExtension(&psc->base,
>>> "GLX_ARB_create_context_profile");
>>>
>>> - if ((mask & (1 << __DRI_API_GLES2)) != 0) {
>>> + if ((mask & ((1 << __DRI_API_GLES2) | (1 << __DRI_API_GLES3))) != 0)
>
> Since ES3 is a superset of ES2, this shouldn't be necessary. I can't
> imagine a driver only setting __DRI_API_GLES3... and the common code may
> not even make that possible. We could, however, enable
> GLX_EXT_create_context_es2_profile if only __DRI_API_GLES is set. I
> don't really feel like testing any drivers that only support ES 1.x, do
> you? :)
I see. No, I'm fine requiring GLES 2 :)
>>> {
>>> __glXEnableDirectExtension(&psc->base,
>>> "GLX_EXT_create_context_es_profile");
>>> __glXEnableDirectExtension(&psc->base,
>>> diff --git a/src/glx/dri3_glx.c b/src/glx/dri3_glx.c
>>> index 6973ad1..a8ef8b5 100644
>>> --- a/src/glx/dri3_glx.c
>>> +++ b/src/glx/dri3_glx.c
>>> @@ -1825,7 +1825,7 @@ dri3_bind_extensions(struct dri3_screen *psc, struct
>>> glx_display * priv,
>>> __glXEnableDirectExtension(&psc->base, "GLX_ARB_create_context");
>>> __glXEnableDirectExtension(&psc->base,
>>> "GLX_ARB_create_context_profile");
>>>
>>> - if ((mask & (1 << __DRI_API_GLES2)) != 0) {
>>> + if ((mask & ((1 << __DRI_API_GLES2) | (1 << __DRI_API_GLES3)) != 0) {
>>> __glXEnableDirectExtension(&psc->base,
>>> "GLX_EXT_create_context_es_profile");
>>> __glXEnableDirectExtension(&psc->base,
>>>
>>>
>> After a bit of looking around it seems that we due to
>> glXCreateContextAttribsARB's
>> xcb_glx_create_context_attribs_arb_checked().
>> So I've skimmed through the xserver's glx/createcontext.c which seems
>> to explicitly error out if the version is different from 2.0
>>
>> case GLX_CONTEXT_ES2_PROFILE_BIT_EXT:
>> ...
>> if (major_version != 2 || minor_version != 0)
>> return __glXError(GLXBadProfileARB);
>
> Ah... that is the problem. I forgot that there was actually server
> support necessary for the extension. I think we should do the following:
>
> 1. I believe we can remove the hack that Chad mentioned if we also
> change the availability of GLX_EXT_create_context_es2_profile on server
> support. I'll submit a patch for that.
>
> 2. Modify Chad's patch to use a separate bit for
> GLX_EXT_create_context_es_profile, and configure it the same way as
> GLX_EXT_create_context_es2_profile (i.e., require server support).
>
> 3. Add support for GLX_EXT_create_context_es_profile to the server. I
> can write a compile-tested patch for that, but I won't have an
> opportunity to really test it in the near future.
>
> 4. Update the GLX_EXT_create_context_es2_profile to be aware of
> GLX_EXT_create_context_es_profile.
Sounds good plan.
I don't have means to test this neither. If Emil/Chad can test, feel
free to take my patch and do whatever's necessary with it. I don't
really care about authorship -- I already have my 25 commits! :D --, but
seriously, this patch by now has more work from others than myself.
Jose
More information about the mesa-dev
mailing list