On 04/14/2015 08:36 AM, Emil Velikov wrote:
> On 14 April 2015 at 13:52, Jose Fonseca <jfons...@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 <jfons...@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 <bri...@vmware.com>
>>>> Reviewed-by: Emil Velikov <emil.l.veli...@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? :)
>> {
>> __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.
> I'm leaning that we need Jose's patch above (apart from updating the
> xserver), although perhaps we should handle __DRI_API_GLES and drisw
> as well ?
>
> Now if we can find a brave soul to rebuild xserver, as I cannot really
> mock around with mine atm that would be great :-) A small side note
> piglit's glx_arb_create_context tests might need an update (there
> might be a regression or two depending on how lucky we are).
>
>
> Cheers,
> Emil
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
