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 <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? :)

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
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to