Hey
On Fri, Aug 3, 2018 at 7:44 PM Adam Jackson <a...@redhat.com> wrote:
>
> GLXCreate{,New}Context, like most X resource creation requests, does not
> emit a reply and therefore is emitted into the X stream asynchronously.
> However, unlike most resource creation requests, the GLXContext we
> return is a handle to library state instead of an XID. So if context
> creation fails for any reason - say, the server doesn't support indirect
> contexts - then we will fail in strange places for strange reasons.
>
> We could make every GLX entrypoint robust against half-created contexts,
> or we could just verify that context creation worked. Reuse the
> __glXIsDirect code to do this, as a cheap way of verifying that the
> XID is real.
>
> glXCreateContextAttribsARB solves this by using the _checked version of
> the xcb command, so effectively this change makes the classic context
> creation paths as robust as CreateContextAttribs.
>
> Signed-off-by: Adam Jackson <a...@redhat.com>
> ---
> src/glx/glxcmds.c | 92 +++++++++++++++++++++++++++--------------------
> 1 file changed, 54 insertions(+), 38 deletions(-)
>
> diff --git a/src/glx/glxcmds.c b/src/glx/glxcmds.c
> index 4db0228eaba..3789f55d038 100644
> --- a/src/glx/glxcmds.c
> +++ b/src/glx/glxcmds.c
> @@ -272,6 +272,43 @@ glx_context_init(struct glx_context *gc,
> return True;
> }
>
> +/**
> + * Determine if a context uses direct rendering.
> + *
> + * \param dpy Display where the context was created.
> + * \param contextID ID of the context to be tested.
> + * \param error Out parameter, set to True on error if not NULL
> + *
> + * \returns \c True if the context is direct rendering or not.
> + */
> +static Bool
> +__glXIsDirect(Display * dpy, GLXContextID contextID, int *error)
Nitpicking, maybe a Bool would be more explicit here (even if it's the
same), it's set to “None” and later set to “True”.
> +{
> + CARD8 opcode;
> + xcb_connection_t *c;
> + xcb_generic_error_t *err;
> + xcb_glx_is_direct_reply_t *reply;
> + Bool is_direct;
> +
> + opcode = __glXSetupForCommand(dpy);
> + if (!opcode) {
> + return False;
> + }
> +
> + c = XGetXCBConnection(dpy);
> + reply = xcb_glx_is_direct_reply(c, xcb_glx_is_direct(c, contextID), &err);
> + is_direct = (reply != NULL && reply->is_direct) ? True : False;
> +
> + if (err != NULL) {
> + *error = True;
glXIsDirect() passes “NULL” as “error”, but it's set unconditionally here.
> + __glXSendErrorForXcb(dpy, err);
> + free(err);
> + }
> +
> + free(reply);
> +
> + return is_direct;
> +}
>
> /**
> * Create a new context.
> @@ -376,6 +413,21 @@ CreateContext(Display *dpy, int generic_id, struct
> glx_config *config,
> gc->share_xid = shareList ? shareList->xid : None;
> gc->imported = GL_FALSE;
>
> + /* Unlike most X resource creation requests, we're about to return a
> handle
> + * with client-side state, not just an XID. To simplify error handling
> + * elsewhere in libGL, force a round-trip here to ensure the CreateContext
> + * request above succeeded.
> + */
> + {
> + int error = None;
> + int isDirect = __glXIsDirect(dpy, gc->xid, &error);
> +
> + if (error != None || isDirect != gc->isDirect) {
> + gc->vtable->destroy(gc);
> + gc = NULL;
> + }
> + }
> +
> return (GLXContext) gc;
> }
>
> @@ -612,42 +664,6 @@ glXCopyContext(Display * dpy, GLXContext source_user,
> }
>
>
> -/**
> - * Determine if a context uses direct rendering.
> - *
> - * \param dpy Display where the context was created.
> - * \param contextID ID of the context to be tested.
> - *
> - * \returns \c True if the context is direct rendering or not.
> - */
> -static Bool
> -__glXIsDirect(Display * dpy, GLXContextID contextID)
> -{
> - CARD8 opcode;
> - xcb_connection_t *c;
> - xcb_generic_error_t *err;
> - xcb_glx_is_direct_reply_t *reply;
> - Bool is_direct;
> -
> - opcode = __glXSetupForCommand(dpy);
> - if (!opcode) {
> - return False;
> - }
> -
> - c = XGetXCBConnection(dpy);
> - reply = xcb_glx_is_direct_reply(c, xcb_glx_is_direct(c, contextID), &err);
> - is_direct = (reply != NULL && reply->is_direct) ? True : False;
> -
> - if (err != NULL) {
> - __glXSendErrorForXcb(dpy, err);
> - free(err);
> - }
> -
> - free(reply);
> -
> - return is_direct;
> -}
> -
> /**
> * \todo
> * Shouldn't this function \b always return \c False when
> @@ -668,7 +684,7 @@ glXIsDirect(Display * dpy, GLXContext gc_user)
> #ifdef GLX_USE_APPLEGL /* TODO: indirect on darwin */
> return False;
> #else
> - return __glXIsDirect(dpy, gc->xid);
> + return __glXIsDirect(dpy, gc->xid, NULL);
> #endif
> }
>
> @@ -1428,7 +1444,7 @@ glXImportContextEXT(Display *dpy, GLXContextID
> contextID)
> return NULL;
> }
>
> - if (__glXIsDirect(dpy, contextID))
> + if (__glXIsDirect(dpy, contextID, NULL))
> return NULL;
>
> opcode = __glXSetupForCommand(dpy);
> --
> 2.17.0
OIther than the two points above, it works in my test case and avoids
the [xcb] “Unknown sequence number while processing queue” when trying
to catch the XError is IsDirect().
Cheers,
Olivier
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
