glx: add support for GLX_ARB_create_context_no_error (v2)

Submitted by Adam Jackson on Nov. 29, 2017, 9:20 p.m.

Details

Message ID 20171129212025.21955-1-ajax@redhat.com
State New
Headers show
Series "glx: add support for GLX_ARB_create_context_no_error (v2)" ( rev: 1 ) in Mesa

Not browsing as part of any series.

Commit Message

Adam Jackson Nov. 29, 2017, 9:20 p.m.
From: Grigori Goronzy <greg@chown.ath.cx>

v2: Only reject no-error contexts for too-old GL if we're actually
trying to create a no-error context (Adam Jackson)

Reviewed-by: Adam Jackson <ajax@redhat.com>
---
 src/glx/dri2_glx.c      | 12 ++++++++++++
 src/glx/dri3_glx.c      |  8 ++++++++
 src/glx/dri_common.c    | 52 ++++++++++++++++++++++++++++++++++++++++++++++++-
 src/glx/dri_common.h    |  5 +++++
 src/glx/drisw_glx.c     |  3 +++
 src/glx/glxclient.h     |  6 ++++++
 src/glx/glxextensions.c |  1 +
 src/glx/glxextensions.h |  1 +
 8 files changed, 87 insertions(+), 1 deletion(-)

Patch hide | download patch | download mbox

diff --git a/src/glx/dri2_glx.c b/src/glx/dri2_glx.c
index 0f44635725..148e294202 100644
--- a/src/glx/dri2_glx.c
+++ b/src/glx/dri2_glx.c
@@ -263,6 +263,10 @@  dri2_create_context_attribs(struct glx_screen *base,
                                  &api, &reset, &release, error))
       goto error_exit;
 
+   if (!dri2_check_no_error(flags, shareList, major_ver, error)) {
+      goto error_exit;
+   }
+
    /* Check the renderType value */
    if (!validate_renderType_against_config(config_base, renderType))
        goto error_exit;
@@ -1165,6 +1169,14 @@  dri2BindExtensions(struct dri2_screen *psc, struct glx_display * priv,
          __glXEnableDirectExtension(&psc->base,
                                     "GLX_ARB_create_context_robustness");
 
+      /* DRI2 version 3 is also required because
+       * GLX_ARB_create_context_no_error requires GLX_ARB_create_context.
+       */
+      if (psc->dri2->base.version >= 3
+          && strcmp(extensions[i]->name, __DRI2_NO_ERROR) == 0)
+         __glXEnableDirectExtension(&psc->base,
+                                    "GLX_ARB_create_context_no_error");
+
       /* DRI2 version 3 is also required because GLX_MESA_query_renderer
        * requires GLX_ARB_create_context_profile.
        */
diff --git a/src/glx/dri3_glx.c b/src/glx/dri3_glx.c
index a10306fe32..54220ccca7 100644
--- a/src/glx/dri3_glx.c
+++ b/src/glx/dri3_glx.c
@@ -248,6 +248,10 @@  dri3_create_context_attribs(struct glx_screen *base,
                                  &reset, &release, error))
       goto error_exit;
 
+   if (!dri2_check_no_error(flags, shareList, major_ver, error)) {
+      goto error_exit;
+   }
+
    /* Check the renderType value */
    if (!validate_renderType_against_config(config_base, render_type))
        goto error_exit;
@@ -756,6 +760,10 @@  dri3_bind_extensions(struct dri3_screen *psc, struct glx_display * priv,
          __glXEnableDirectExtension(&psc->base,
                                     "GLX_ARB_create_context_robustness");
 
+      if (strcmp(extensions[i]->name, __DRI2_NO_ERROR) == 0)
+         __glXEnableDirectExtension(&psc->base,
+                                    "GLX_ARB_create_context_no_error");
+
       if (strcmp(extensions[i]->name, __DRI2_RENDERER_QUERY) == 0) {
          psc->rendererQuery = (__DRI2rendererQueryExtension *) extensions[i];
          __glXEnableDirectExtension(&psc->base, "GLX_MESA_query_renderer");
diff --git a/src/glx/dri_common.c b/src/glx/dri_common.c
index 3b82309fa2..4f9beb22b1 100644
--- a/src/glx/dri_common.c
+++ b/src/glx/dri_common.c
@@ -479,6 +479,7 @@  dri2_convert_glx_attribs(unsigned num_attribs, const uint32_t *attribs,
 {
    unsigned i;
    bool got_profile = false;
+   int no_error = 0;
    uint32_t profile;
 
    *major_ver = 1;
@@ -511,6 +512,9 @@  dri2_convert_glx_attribs(unsigned num_attribs, const uint32_t *attribs,
       case GLX_CONTEXT_FLAGS_ARB:
 	 *flags = attribs[i * 2 + 1];
 	 break;
+      case GLX_CONTEXT_OPENGL_NO_ERROR_ARB:
+	 no_error = attribs[i * 2 + 1];
+	 break;
       case GLX_CONTEXT_PROFILE_MASK_ARB:
 	 profile = attribs[i * 2 + 1];
 	 got_profile = true;
@@ -552,6 +556,10 @@  dri2_convert_glx_attribs(unsigned num_attribs, const uint32_t *attribs,
       }
    }
 
+   if (no_error) {
+      *flags |= __DRI_CTX_FLAG_NO_ERROR;
+   }
+
    if (!got_profile) {
       if (*major_ver > 3 || (*major_ver == 3 && *minor_ver >= 2))
 	 *api = __DRI_API_OPENGL_CORE;
@@ -592,7 +600,8 @@  dri2_convert_glx_attribs(unsigned num_attribs, const uint32_t *attribs,
    /* Unknown flag value.
     */
    if (*flags & ~(__DRI_CTX_FLAG_DEBUG | __DRI_CTX_FLAG_FORWARD_COMPATIBLE
-                  | __DRI_CTX_FLAG_ROBUST_BUFFER_ACCESS)) {
+                  | __DRI_CTX_FLAG_ROBUST_BUFFER_ACCESS
+                  | __DRI_CTX_FLAG_NO_ERROR)) {
       *error = __DRI_CTX_ERROR_UNKNOWN_FLAG;
       return false;
    }
@@ -617,4 +626,45 @@  dri2_convert_glx_attribs(unsigned num_attribs, const uint32_t *attribs,
    return true;
 }
 
+_X_HIDDEN bool
+dri2_check_no_error(uint32_t flags, struct glx_context *share_context,
+                    int major, unsigned *error)
+{
+   Bool noError = flags & __DRI_CTX_FLAG_NO_ERROR;
+
+   /* The KHR_no_error specs say:
+    *
+    *    Requires OpenGL ES 2.0 or OpenGL 2.0.
+    */
+   if (noError && major < 2) {
+      *error = __DRI_CTX_ERROR_UNKNOWN_ATTRIBUTE;
+      return false;
+   }
+
+   /* The GLX_ARB_create_context_no_error specs say:
+    *
+    *    BadMatch is generated if the value of GLX_CONTEXT_OPENGL_NO_ERROR_ARB
+    *    used to create <share_context> does not match the value of
+    *    GLX_CONTEXT_OPENGL_NO_ERROR_ARB for the context being created.
+    */
+   if (share_context && !!share_context->noError != !!noError) {
+      *error = __DRI_CTX_ERROR_BAD_FLAG;
+      return false;
+   }
+
+   /* The GLX_ARB_create_context_no_error specs say:
+    *
+    *    BadMatch is generated if the GLX_CONTEXT_OPENGL_NO_ERROR_ARB is TRUE at
+    *    the same time as a debug or robustness context is specified.
+    *
+    */
+   if (noError && ((flags & __DRI_CTX_FLAG_DEBUG) ||
+                   (flags & __DRI_CTX_FLAG_ROBUST_BUFFER_ACCESS))) {
+      *error = __DRI_CTX_ERROR_BAD_FLAG;
+      return false;
+   }
+
+   return true;
+}
+
 #endif /* GLX_DIRECT_RENDERING */
diff --git a/src/glx/dri_common.h b/src/glx/dri_common.h
index 4d97ff82b4..fcf496f3ed 100644
--- a/src/glx/dri_common.h
+++ b/src/glx/dri_common.h
@@ -80,4 +80,9 @@  dri2_convert_glx_attribs(unsigned num_attribs, const uint32_t *attribs,
                          uint32_t *render_type, uint32_t *flags, unsigned *api,
                          int *reset, int *release, unsigned *error);
 
+extern bool
+dri2_check_no_error(uint32_t flags, struct glx_context *share_context,
+                    int major, unsigned *error);
+
+
 #endif /* _DRI_COMMON_H */
diff --git a/src/glx/drisw_glx.c b/src/glx/drisw_glx.c
index df2467a5c2..3cd88b80c1 100644
--- a/src/glx/drisw_glx.c
+++ b/src/glx/drisw_glx.c
@@ -432,6 +432,9 @@  drisw_create_context_attribs(struct glx_screen *base,
                                  &api, &reset, &release, error))
       return NULL;
 
+   if (!dri2_check_no_error(flags, shareList, major_ver, error))
+      return NULL;
+
    /* Check the renderType value */
    if (!validate_renderType_against_config(config_base, renderType)) {
        return NULL;
diff --git a/src/glx/glxclient.h b/src/glx/glxclient.h
index 0d29e5635e..38647b21d5 100644
--- a/src/glx/glxclient.h
+++ b/src/glx/glxclient.h
@@ -437,6 +437,12 @@  struct glx_context
     */
    unsigned long thread_refcount;
 
+   /**
+    * GLX_ARB_create_context_no_error setting for this context.
+    * This needs to be kept here to enforce shared context rules.
+    */
+   Bool noError;
+
    char gl_extension_bits[__GL_EXT_BYTES];
 };
 
diff --git a/src/glx/glxextensions.c b/src/glx/glxextensions.c
index af6ffbf660..0531d7c3f5 100644
--- a/src/glx/glxextensions.c
+++ b/src/glx/glxextensions.c
@@ -134,6 +134,7 @@  struct extension_info
 static const struct extension_info known_glx_extensions[] = {
    { GLX(ARB_context_flush_control),   VER(0,0), Y, N, N, N },
    { GLX(ARB_create_context),          VER(0,0), Y, N, N, N },
+   { GLX(ARB_create_context_no_error), VER(1,4), Y, N, N, N },
    { GLX(ARB_create_context_profile),  VER(0,0), Y, N, N, N },
    { GLX(ARB_create_context_robustness), VER(0,0), Y, N, N, N },
    { GLX(ARB_fbconfig_float),          VER(0,0), Y, Y, N, N },
diff --git a/src/glx/glxextensions.h b/src/glx/glxextensions.h
index d73128bd0e..ba24ec5c85 100644
--- a/src/glx/glxextensions.h
+++ b/src/glx/glxextensions.h
@@ -39,6 +39,7 @@  enum
 {
    ARB_context_flush_control_bit = 0,
    ARB_create_context_bit,
+   ARB_create_context_no_error_bit,
    ARB_create_context_profile_bit,
    ARB_create_context_robustness_bit,
    ARB_fbconfig_float_bit,

Comments

On Wed, 2017-11-29 at 16:20 -0500, Adam Jackson wrote:
> From: Grigori Goronzy <greg@chown.ath.cx>
> 
> v2: Only reject no-error contexts for too-old GL if we're actually
> trying to create a no-error context (Adam Jackson)
> 
> Reviewed-by: Adam Jackson <ajax@redhat.com>

Note that the original series for this also updated st/glx. That patch
looks correct to me as far as it goes, but it does change
XMesaCreateContext's function signature, which seems rude. Someone who
actually cares about the st/glx code should make that decision.

- ajax
On Wed, 2017-11-29 at 16:20 -0500, Adam Jackson wrote:
> From: Grigori Goronzy <greg@chown.ath.cx>
> 
> v2: Only reject no-error contexts for too-old GL if we're actually
> trying to create a no-error context (Adam Jackson)

D'oh, this is still busted, sorry for the noise. We're not saving the
no-error state in the context, so the share-context check will get it
wrong (no-error contexts would never be able to share). v3 to follow
once I figure out why vim isn't honoring editorconfig.

- ajax
On 11/29/2017 02:30 PM, Adam Jackson wrote:
> On Wed, 2017-11-29 at 16:20 -0500, Adam Jackson wrote:
>> From: Grigori Goronzy <greg@chown.ath.cx>
>>
>> v2: Only reject no-error contexts for too-old GL if we're actually
>> trying to create a no-error context (Adam Jackson)
>>
>> Reviewed-by: Adam Jackson <ajax@redhat.com>
>
> Note that the original series for this also updated st/glx. That patch
> looks correct to me as far as it goes, but it does change
> XMesaCreateContext's function signature, which seems rude. Someone who
> actually cares about the st/glx code should make that decision.

I don't know if any apps directly use the XMesa interface (I hope not). 
  The alternatives to adding a new parameter to XMesaCreateContext() is 
to add a new entrypoing, or just "poke" the new state into the 
XMesaContext object from the call site.

-Brian