[6/6] glx: Implement GLX_EXT_no_config_context (v3)

Submitted by Adam Jackson on Nov. 14, 2017, 8:13 p.m.

Details

Message ID 20171114201306.28276-7-ajax@redhat.com
State New
Headers show
Series "glx: Implement GLX_EXT_no_config_context" ( rev: 1 ) in Mesa

Not browsing as part of any series.

Commit Message

Adam Jackson Nov. 14, 2017, 8:13 p.m.
This more or less ports EGL_KHR_no_config_context to GLX.

v2: Enable the extension only for those backends that support it.
v3: Fix glvnd path and dri2_convert_glx_attribs()

Khronos: https://github.com/KhronosGroup/OpenGL-Registry/pull/102
Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
Signed-off-by: Adam Jackson <ajax@redhat.com>
---
 src/glx/create_context.c          | 41 ++++++++++++++++++++++++++-------------
 src/glx/dri2_glx.c                |  1 +
 src/glx/dri3_glx.c                |  1 +
 src/glx/dri_common.c              |  4 ++++
 src/glx/drisw_glx.c               |  1 +
 src/glx/g_glxglvnddispatchfuncs.c | 14 ++++++++++++-
 src/glx/glxcmds.c                 |  2 +-
 src/glx/glxextensions.c           |  1 +
 src/glx/glxextensions.h           |  1 +
 9 files changed, 50 insertions(+), 16 deletions(-)

Patch hide | download patch | download mbox

diff --git a/src/glx/create_context.c b/src/glx/create_context.c
index 38e949ab4c..eab6511ad8 100644
--- a/src/glx/create_context.c
+++ b/src/glx/create_context.c
@@ -47,21 +47,11 @@  glXCreateContextAttribsARB(Display *dpy, GLXFBConfig config,
    xcb_generic_error_t *err;
    xcb_void_cookie_t cookie;
    unsigned dummy_err = 0;
+   int screen = -1;
 
-
-   if (dpy == NULL || cfg == NULL)
-      return NULL;
-
-   /* This means that either the caller passed the wrong display pointer or
-    * one of the internal GLX data structures (probably the fbconfig) has an
-    * error.  There is nothing sensible to do, so return an error.
-    */
-   psc = GetGLXScreenConfigs(dpy, cfg->screen);
-   if (psc == NULL)
+   if (dpy == NULL)
       return NULL;
 
-   assert(cfg->screen == psc->scr);
-
    /* Count the number of attributes specified by the application.  All
     * attributes appear in pairs, except the terminating None.
     */
@@ -70,6 +60,29 @@  glXCreateContextAttribsARB(Display *dpy, GLXFBConfig config,
 	 /* empty */ ;
    }
 
+   if (cfg) {
+      screen = cfg->screen;
+   } else {
+      int i;
+      for (i = 0; i < num_attribs; i++) {
+         if (attrib_list[i * 2] == GLX_SCREEN)
+            screen = attrib_list[i * 2 + 1];
+      }
+   }
+
+   /* This means that either the caller passed the wrong display pointer or
+    * one of the internal GLX data structures (probably the fbconfig) has an
+    * error.  There is nothing sensible to do, so return an error.
+    */
+   psc = GetGLXScreenConfigs(dpy, screen);
+   if (psc == NULL)
+      return NULL;
+
+   assert(screen == psc->scr);
+
+   if (!cfg && !__glXExtensionBitIsEnabled(psc, EXT_no_config_context_bit))
+      return NULL;
+
    if (direct && psc->vtable->create_context_attribs) {
       /* GLX drops the error returned by the driver.  The expectation is that
        * an error will also be returned by the server.  The server's error
@@ -104,8 +117,8 @@  glXCreateContextAttribsARB(Display *dpy, GLXFBConfig config,
    cookie =
       xcb_glx_create_context_attribs_arb_checked(c,
 						 gc->xid,
-						 cfg->fbconfigID,
-						 cfg->screen,
+						 cfg ? cfg->fbconfigID : 0,
+						 screen,
 						 gc->share_xid,
 						 gc->isDirect,
 						 num_attribs,
diff --git a/src/glx/dri2_glx.c b/src/glx/dri2_glx.c
index 0f44635725..eeec4f0d60 100644
--- a/src/glx/dri2_glx.c
+++ b/src/glx/dri2_glx.c
@@ -1129,6 +1129,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");
+      __glXEnableDirectExtension(&psc->base, "GLX_EXT_no_config_context");
 
       if ((mask & ((1 << __DRI_API_GLES) |
                    (1 << __DRI_API_GLES2) |
diff --git a/src/glx/dri3_glx.c b/src/glx/dri3_glx.c
index a10306fe32..ac2e106a7e 100644
--- a/src/glx/dri3_glx.c
+++ b/src/glx/dri3_glx.c
@@ -720,6 +720,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");
+   __glXEnableDirectExtension(&psc->base, "GLX_EXT_no_config_context");
 
    if ((mask & ((1 << __DRI_API_GLES) |
                 (1 << __DRI_API_GLES2) |
diff --git a/src/glx/dri_common.c b/src/glx/dri_common.c
index 0d0b2d997b..2e0ecaaf8d 100644
--- a/src/glx/dri_common.c
+++ b/src/glx/dri_common.c
@@ -562,6 +562,10 @@  dri2_convert_glx_attribs(unsigned num_attribs, const uint32_t *attribs,
             return false;
          }
          break;
+      case GLX_SCREEN:
+         /* Implies GLX_EXT_no_config_context */
+         *render_type = GLX_DONT_CARE;
+         break;
       default:
 	 /* If an unknown attribute is received, fail.
 	  */
diff --git a/src/glx/drisw_glx.c b/src/glx/drisw_glx.c
index df2467a5c2..e33ac4f2a9 100644
--- a/src/glx/drisw_glx.c
+++ b/src/glx/drisw_glx.c
@@ -628,6 +628,7 @@  driswBindExtensions(struct drisw_screen *psc, const __DRIextension **extensions)
    if (psc->swrast->base.version >= 3) {
       __glXEnableDirectExtension(&psc->base, "GLX_ARB_create_context");
       __glXEnableDirectExtension(&psc->base, "GLX_ARB_create_context_profile");
+      __glXEnableDirectExtension(&psc->base, "GLX_EXT_no_config_context");
 
       /* DRISW version >= 2 implies support for OpenGL ES.
        */
diff --git a/src/glx/g_glxglvnddispatchfuncs.c b/src/glx/g_glxglvnddispatchfuncs.c
index 56d894eda7..04f6d8263a 100644
--- a/src/glx/g_glxglvnddispatchfuncs.c
+++ b/src/glx/g_glxglvnddispatchfuncs.c
@@ -164,7 +164,19 @@  static GLXContext dispatch_CreateContextAttribsARB(Display *dpy,
     __GLXvendorInfo *dd;
     GLXContext ret;
 
-    dd = GetDispatchFromFBConfig(dpy, config);
+    if (config) {
+       dd = GetDispatchFromFBConfig(dpy, config);
+    } else if (attrib_list) {
+       int i, screen;
+
+       for (i = 0; attrib_list[i * 2] != None; i++) {
+          if (attrib_list[i * 2] == GLX_SCREEN) {
+             screen = attrib_list[i * 2 + 1];
+             dd = GetDispatchFromDrawable(dpy, RootWindow(dpy, screen));
+             break;
+          }
+       }
+    }
     if (dd == NULL)
         return None;
 
diff --git a/src/glx/glxcmds.c b/src/glx/glxcmds.c
index eee45d962d..bc5333588b 100644
--- a/src/glx/glxcmds.c
+++ b/src/glx/glxcmds.c
@@ -237,7 +237,7 @@  validate_renderType_against_config(const struct glx_config *config,
 {
    /* GLX_EXT_no_config_context supports any render type */
    if (!config)
-      return True;
+      return renderType == GLX_DONT_CARE;
 
    switch (renderType) {
       case GLX_RGBA_TYPE:
diff --git a/src/glx/glxextensions.c b/src/glx/glxextensions.c
index af6ffbf660..4853ad534e 100644
--- a/src/glx/glxextensions.c
+++ b/src/glx/glxextensions.c
@@ -146,6 +146,7 @@  static const struct extension_info known_glx_extensions[] = {
    { GLX(EXT_fbconfig_packed_float),   VER(0,0), Y, Y, N, N },
    { GLX(EXT_framebuffer_sRGB),        VER(0,0), Y, Y, N, N },
    { GLX(EXT_import_context),          VER(0,0), Y, Y, N, N },
+   { GLX(EXT_no_config_context),       VER(0,0), Y, N, N, N },
    { GLX(EXT_texture_from_pixmap),     VER(0,0), Y, N, N, N },
    { GLX(EXT_visual_info),             VER(0,0), Y, Y, N, N },
    { GLX(EXT_visual_rating),           VER(0,0), Y, Y, N, N },
diff --git a/src/glx/glxextensions.h b/src/glx/glxextensions.h
index d73128bd0e..07cd3af0ff 100644
--- a/src/glx/glxextensions.h
+++ b/src/glx/glxextensions.h
@@ -50,6 +50,7 @@  enum
    EXT_fbconfig_packed_float_bit,
    EXT_framebuffer_sRGB_bit,
    EXT_import_context_bit,
+   EXT_no_config_context_bit,
    EXT_texture_from_pixmap_bit,
    EXT_visual_info_bit,
    EXT_visual_rating_bit,

Comments

On 11/14/2017 12:13 PM, Adam Jackson wrote:
> This more or less ports EGL_KHR_no_config_context to GLX.
> 
> v2: Enable the extension only for those backends that support it.
> v3: Fix glvnd path and dri2_convert_glx_attribs()
> 
> Khronos: https://github.com/KhronosGroup/OpenGL-Registry/pull/102
> Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
> Signed-off-by: Adam Jackson <ajax@redhat.com>
> ---
>  src/glx/create_context.c          | 41 ++++++++++++++++++++++++++-------------
>  src/glx/dri2_glx.c                |  1 +
>  src/glx/dri3_glx.c                |  1 +
>  src/glx/dri_common.c              |  4 ++++
>  src/glx/drisw_glx.c               |  1 +
>  src/glx/g_glxglvnddispatchfuncs.c | 14 ++++++++++++-
>  src/glx/glxcmds.c                 |  2 +-
>  src/glx/glxextensions.c           |  1 +
>  src/glx/glxextensions.h           |  1 +
>  9 files changed, 50 insertions(+), 16 deletions(-)
> 
> diff --git a/src/glx/create_context.c b/src/glx/create_context.c
> index 38e949ab4c..eab6511ad8 100644
> --- a/src/glx/create_context.c
> +++ b/src/glx/create_context.c
> @@ -47,21 +47,11 @@ glXCreateContextAttribsARB(Display *dpy, GLXFBConfig config,
>     xcb_generic_error_t *err;
>     xcb_void_cookie_t cookie;
>     unsigned dummy_err = 0;
> +   int screen = -1;
>  
> -
> -   if (dpy == NULL || cfg == NULL)
> -      return NULL;
> -
> -   /* This means that either the caller passed the wrong display pointer or
> -    * one of the internal GLX data structures (probably the fbconfig) has an
> -    * error.  There is nothing sensible to do, so return an error.
> -    */
> -   psc = GetGLXScreenConfigs(dpy, cfg->screen);
> -   if (psc == NULL)
> +   if (dpy == NULL)
>        return NULL;
>  
> -   assert(cfg->screen == psc->scr);
> -
>     /* Count the number of attributes specified by the application.  All
>      * attributes appear in pairs, except the terminating None.
>      */
> @@ -70,6 +60,29 @@ glXCreateContextAttribsARB(Display *dpy, GLXFBConfig config,
>  	 /* empty */ ;
>     }
>  
> +   if (cfg) {
> +      screen = cfg->screen;
> +   } else {
> +      int i;

unsigned to prevent dumb GCC warnings.  Also, I'm pretty sure this code
can use C99 declarations in the for-loop.

Aside from that, this patch is

Reviewed-by: Ian Romanick <ian.d.romanick@intel.com>

> +      for (i = 0; i < num_attribs; i++) {
> +         if (attrib_list[i * 2] == GLX_SCREEN)
> +            screen = attrib_list[i * 2 + 1];
> +      }
> +   }
> +
> +   /* This means that either the caller passed the wrong display pointer or
> +    * one of the internal GLX data structures (probably the fbconfig) has an
> +    * error.  There is nothing sensible to do, so return an error.
> +    */
> +   psc = GetGLXScreenConfigs(dpy, screen);
> +   if (psc == NULL)
> +      return NULL;
> +
> +   assert(screen == psc->scr);
> +
> +   if (!cfg && !__glXExtensionBitIsEnabled(psc, EXT_no_config_context_bit))
> +      return NULL;
> +
>     if (direct && psc->vtable->create_context_attribs) {
>        /* GLX drops the error returned by the driver.  The expectation is that
>         * an error will also be returned by the server.  The server's error
> @@ -104,8 +117,8 @@ glXCreateContextAttribsARB(Display *dpy, GLXFBConfig config,
>     cookie =
>        xcb_glx_create_context_attribs_arb_checked(c,
>  						 gc->xid,
> -						 cfg->fbconfigID,
> -						 cfg->screen,
> +						 cfg ? cfg->fbconfigID : 0,
> +						 screen,
>  						 gc->share_xid,
>  						 gc->isDirect,
>  						 num_attribs,
> diff --git a/src/glx/dri2_glx.c b/src/glx/dri2_glx.c
> index 0f44635725..eeec4f0d60 100644
> --- a/src/glx/dri2_glx.c
> +++ b/src/glx/dri2_glx.c
> @@ -1129,6 +1129,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");
> +      __glXEnableDirectExtension(&psc->base, "GLX_EXT_no_config_context");
>  
>        if ((mask & ((1 << __DRI_API_GLES) |
>                     (1 << __DRI_API_GLES2) |
> diff --git a/src/glx/dri3_glx.c b/src/glx/dri3_glx.c
> index a10306fe32..ac2e106a7e 100644
> --- a/src/glx/dri3_glx.c
> +++ b/src/glx/dri3_glx.c
> @@ -720,6 +720,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");
> +   __glXEnableDirectExtension(&psc->base, "GLX_EXT_no_config_context");
>  
>     if ((mask & ((1 << __DRI_API_GLES) |
>                  (1 << __DRI_API_GLES2) |
> diff --git a/src/glx/dri_common.c b/src/glx/dri_common.c
> index 0d0b2d997b..2e0ecaaf8d 100644
> --- a/src/glx/dri_common.c
> +++ b/src/glx/dri_common.c
> @@ -562,6 +562,10 @@ dri2_convert_glx_attribs(unsigned num_attribs, const uint32_t *attribs,
>              return false;
>           }
>           break;
> +      case GLX_SCREEN:
> +         /* Implies GLX_EXT_no_config_context */
> +         *render_type = GLX_DONT_CARE;
> +         break;
>        default:
>  	 /* If an unknown attribute is received, fail.
>  	  */
> diff --git a/src/glx/drisw_glx.c b/src/glx/drisw_glx.c
> index df2467a5c2..e33ac4f2a9 100644
> --- a/src/glx/drisw_glx.c
> +++ b/src/glx/drisw_glx.c
> @@ -628,6 +628,7 @@ driswBindExtensions(struct drisw_screen *psc, const __DRIextension **extensions)
>     if (psc->swrast->base.version >= 3) {
>        __glXEnableDirectExtension(&psc->base, "GLX_ARB_create_context");
>        __glXEnableDirectExtension(&psc->base, "GLX_ARB_create_context_profile");
> +      __glXEnableDirectExtension(&psc->base, "GLX_EXT_no_config_context");
>  
>        /* DRISW version >= 2 implies support for OpenGL ES.
>         */
> diff --git a/src/glx/g_glxglvnddispatchfuncs.c b/src/glx/g_glxglvnddispatchfuncs.c
> index 56d894eda7..04f6d8263a 100644
> --- a/src/glx/g_glxglvnddispatchfuncs.c
> +++ b/src/glx/g_glxglvnddispatchfuncs.c
> @@ -164,7 +164,19 @@ static GLXContext dispatch_CreateContextAttribsARB(Display *dpy,
>      __GLXvendorInfo *dd;
>      GLXContext ret;
>  
> -    dd = GetDispatchFromFBConfig(dpy, config);
> +    if (config) {
> +       dd = GetDispatchFromFBConfig(dpy, config);
> +    } else if (attrib_list) {
> +       int i, screen;
> +
> +       for (i = 0; attrib_list[i * 2] != None; i++) {
> +          if (attrib_list[i * 2] == GLX_SCREEN) {
> +             screen = attrib_list[i * 2 + 1];
> +             dd = GetDispatchFromDrawable(dpy, RootWindow(dpy, screen));
> +             break;
> +          }
> +       }
> +    }
>      if (dd == NULL)
>          return None;
>  
> diff --git a/src/glx/glxcmds.c b/src/glx/glxcmds.c
> index eee45d962d..bc5333588b 100644
> --- a/src/glx/glxcmds.c
> +++ b/src/glx/glxcmds.c
> @@ -237,7 +237,7 @@ validate_renderType_against_config(const struct glx_config *config,
>  {
>     /* GLX_EXT_no_config_context supports any render type */
>     if (!config)
> -      return True;
> +      return renderType == GLX_DONT_CARE;
>  
>     switch (renderType) {
>        case GLX_RGBA_TYPE:
> diff --git a/src/glx/glxextensions.c b/src/glx/glxextensions.c
> index af6ffbf660..4853ad534e 100644
> --- a/src/glx/glxextensions.c
> +++ b/src/glx/glxextensions.c
> @@ -146,6 +146,7 @@ static const struct extension_info known_glx_extensions[] = {
>     { GLX(EXT_fbconfig_packed_float),   VER(0,0), Y, Y, N, N },
>     { GLX(EXT_framebuffer_sRGB),        VER(0,0), Y, Y, N, N },
>     { GLX(EXT_import_context),          VER(0,0), Y, Y, N, N },
> +   { GLX(EXT_no_config_context),       VER(0,0), Y, N, N, N },
>     { GLX(EXT_texture_from_pixmap),     VER(0,0), Y, N, N, N },
>     { GLX(EXT_visual_info),             VER(0,0), Y, Y, N, N },
>     { GLX(EXT_visual_rating),           VER(0,0), Y, Y, N, N },
> diff --git a/src/glx/glxextensions.h b/src/glx/glxextensions.h
> index d73128bd0e..07cd3af0ff 100644
> --- a/src/glx/glxextensions.h
> +++ b/src/glx/glxextensions.h
> @@ -50,6 +50,7 @@ enum
>     EXT_fbconfig_packed_float_bit,
>     EXT_framebuffer_sRGB_bit,
>     EXT_import_context_bit,
> +   EXT_no_config_context_bit,
>     EXT_texture_from_pixmap_bit,
>     EXT_visual_info_bit,
>     EXT_visual_rating_bit,
>
On 14 November 2017 at 20:13, Adam Jackson <ajax@redhat.com> wrote:

> @@ -562,6 +562,10 @@ dri2_convert_glx_attribs(unsigned num_attribs, const uint32_t *attribs,
>              return false;
>           }
>           break;
> +      case GLX_SCREEN:
> +         /* Implies GLX_EXT_no_config_context */
> +         *render_type = GLX_DONT_CARE;
> +         break;
We should fall-through (and fail) when GLX_SCREEN is set but the
extension is missing.

>        default:
>          /* If an unknown attribute is received, fail.
>           */

Emil
On Thu, 2017-11-30 at 16:10 +0000, Emil Velikov wrote:
> On 14 November 2017 at 20:13, Adam Jackson <ajax@redhat.com> wrote:
> 
> > @@ -562,6 +562,10 @@ dri2_convert_glx_attribs(unsigned num_attribs,
> > const uint32_t *attribs,
> >              return false;
> >           }
> >           break;
> > +      case GLX_SCREEN:
> > +         /* Implies GLX_EXT_no_config_context */
> > +         *render_type = GLX_DONT_CARE;
> > +         break;
> 
> We should fall-through (and fail) when GLX_SCREEN is set but the
> extension is missing.

Nah. This function does not make reference to the screen we're trying
to create a context for. And, for direct contexts (that call this
conversion helper) this extension is now always present anyway.

- ajax
On 5 December 2017 at 20:03, Adam Jackson <ajax@redhat.com> wrote:
> On Thu, 2017-11-30 at 16:10 +0000, Emil Velikov wrote:
>> On 14 November 2017 at 20:13, Adam Jackson <ajax@redhat.com> wrote:
>>
>> > @@ -562,6 +562,10 @@ dri2_convert_glx_attribs(unsigned num_attribs,
>> > const uint32_t *attribs,
>> >              return false;
>> >           }
>> >           break;
>> > +      case GLX_SCREEN:
>> > +         /* Implies GLX_EXT_no_config_context */
>> > +         *render_type = GLX_DONT_CARE;
>> > +         break;
>>
>> We should fall-through (and fail) when GLX_SCREEN is set but the
>> extension is missing.
>
> Nah. This function does not make reference to the screen we're trying
> to create a context for. And, for direct contexts (that call this
> conversion helper) this extension is now always present anyway.
>
Hmm you're right - having adding check is pain in the rear and an overkill.

-Emil