Mesa (master): glx: Lift sending the MakeCurrent request to top-level code
GitLab Mirror
gitlab-mirror at kemper.freedesktop.org
Tue Apr 13 23:31:32 UTC 2021
Module: Mesa
Branch: master
Commit: 80b67a3b444f31462890a8e390650fa77c4d2010
URL: http://cgit.freedesktop.org/mesa/mesa/commit/?id=80b67a3b444f31462890a8e390650fa77c4d2010
Author: Adam Jackson <ajax at redhat.com>
Date: Tue Nov 14 15:13:04 2017 -0500
glx: Lift sending the MakeCurrent request to top-level code
Somewhat terrifyingly, we never sent this for direct contexts, which
means the server never knew the context/drawable bindings. To handle
this sanely, pull the request code up out of the indirect backend, and
rewrite the context switch path to call it as appropriate. This
attempts to preserve the existing behavior of not calling unbind() on
the context if its refcount would not drop to zero.
Of course, you can't just do this indiscriminately, because this is GLX
and extant X servers have bugs and everything is terrible. To wit:
- For 1.20.x prior to 1.20.6, you can bind a direct context once, but
the second time you try to modify the context's binding you will get
GLXBadContextTag. This includes unbinding the context. And "deleting"
the context will leak memory, because it will still appear to be
current.
- For 1.19 and earlier, glXMakeCurrent(dpy, None, ctx) should be legal
for GL 3.0+ contexts, but the server will throw BadMatch.
To guard against this, we only send the request for indirect contexts
unless the server is known good, and only mention one context at a time
in such a request; if switching between contexts, we first unbind the
old, and then bind the new. Note that the second VendorRelease() version
is to catch XFree86 4.x and Xorg [67].x, which almost certainly have the
above bugs. Other servers might report different version numbers here,
but we can't do direct rendering against them, so this should be safe.
Fixes: mesa/mesa#4418
Acked-By: Mike Blumenkrantz <michael.blumenkrantz at gmail.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/9992>
---
src/broadcom/ci/piglit-v3d-rpi4-fails.txt | 2 -
src/broadcom/ci/piglit-vc4-rpi3-fails.txt | 1 -
src/broadcom/ci/piglit-vc4-rpi3-flakes.txt | 1 -
.../freedreno/ci/piglit-freedreno-a530-fails.txt | 2 -
.../freedreno/ci/piglit-freedreno-a630-fails.txt | 2 -
.../drivers/llvmpipe/ci/llvmpipe-quick_gl.txt | 2 -
src/gallium/drivers/softpipe/ci/softpipe-quick.txt | 2 -
.../drivers/zink/ci/piglit-zink-lvp-fails.txt | 1 -
src/glx/glxcurrent.c | 214 ++++++++++++++++-----
src/glx/indirect_glx.c | 139 +++----------
10 files changed, 186 insertions(+), 180 deletions(-)
diff --git a/src/broadcom/ci/piglit-v3d-rpi4-fails.txt b/src/broadcom/ci/piglit-v3d-rpi4-fails.txt
index 7fce21085b7..b0d74801f4d 100644
--- a/src/broadcom/ci/piglit-v3d-rpi4-fails.txt
+++ b/src/broadcom/ci/piglit-v3d-rpi4-fails.txt
@@ -1,5 +1,3 @@
-glx at glx-make-current,Crash
-glx at glx-multi-window-single-context,Fail
glx at glx-multithread-buffer,Fail
glx at glx-query-drawable-glx_fbconfig_id-window,Fail
glx at glx-swap-pixmap-bad,Fail
diff --git a/src/broadcom/ci/piglit-vc4-rpi3-fails.txt b/src/broadcom/ci/piglit-vc4-rpi3-fails.txt
index ee6b54d0214..6ba0bebdfe9 100644
--- a/src/broadcom/ci/piglit-vc4-rpi3-fails.txt
+++ b/src/broadcom/ci/piglit-vc4-rpi3-fails.txt
@@ -4,7 +4,6 @@ spec at egl_ext_device_base@conformance,Crash
spec at egl_khr_wait_sync@conformance,Crash
glx at glx-copy-sub-buffer samples=2,Crash
glx at glx-copy-sub-buffer samples=4,Crash
-glx at glx-make-current,Crash
glx at glx-multithread-buffer,Fail
glx at glx-multithread-texture,Timeout
glx at glx-query-drawable-glx_fbconfig_id-window,Fail
diff --git a/src/broadcom/ci/piglit-vc4-rpi3-flakes.txt b/src/broadcom/ci/piglit-vc4-rpi3-flakes.txt
index 9882c3f3966..73abeff3955 100644
--- a/src/broadcom/ci/piglit-vc4-rpi3-flakes.txt
+++ b/src/broadcom/ci/piglit-vc4-rpi3-flakes.txt
@@ -1,5 +1,4 @@
shaders at glsl-vs-loop
spec at arb_framebuffer_srgb@blit renderbuffer srgb single_sampled enabled clear
-glx at glx-multi-window-single-context
glx at glx_arb_sync_control@timing
spec at egl_chromium_sync_control@conformance
diff --git a/src/gallium/drivers/freedreno/ci/piglit-freedreno-a530-fails.txt b/src/gallium/drivers/freedreno/ci/piglit-freedreno-a530-fails.txt
index d2829a126ea..d3d2de567d8 100644
--- a/src/gallium/drivers/freedreno/ci/piglit-freedreno-a530-fails.txt
+++ b/src/gallium/drivers/freedreno/ci/piglit-freedreno-a530-fails.txt
@@ -1,7 +1,5 @@
fast_color_clear at fcc-read-after-clear copy rb,Fail
fast_color_clear at fcc-read-after-clear copy tex,Fail
-glx at glx-make-current,Crash
-glx at glx-multi-window-single-context,Fail
glx at glx-query-drawable-glx_fbconfig_id-window,Fail
glx at glx-swap-pixmap-bad,Fail
glx at glx-visuals-depth -pixmap,Crash
diff --git a/src/gallium/drivers/freedreno/ci/piglit-freedreno-a630-fails.txt b/src/gallium/drivers/freedreno/ci/piglit-freedreno-a630-fails.txt
index a337805aac5..9b6686bab86 100644
--- a/src/gallium/drivers/freedreno/ci/piglit-freedreno-a630-fails.txt
+++ b/src/gallium/drivers/freedreno/ci/piglit-freedreno-a630-fails.txt
@@ -1,8 +1,6 @@
glx at glx_arb_sync_control@timing -fullscreen -msc-delta 1,Fail
glx at glx_arb_sync_control@timing -waitformsc -msc-delta 2,Fail
glx at glx-copy-sub-buffer samples=2,Fail
-glx at glx-make-current,Crash
-glx at glx-multi-window-single-context,Fail
glx at glx-query-drawable-glx_fbconfig_id-window,Fail
glx at glx-swap-pixmap-bad,Fail
glx at glx-visuals-depth -pixmap,Crash
diff --git a/src/gallium/drivers/llvmpipe/ci/llvmpipe-quick_gl.txt b/src/gallium/drivers/llvmpipe/ci/llvmpipe-quick_gl.txt
index c0e76e6d869..d2273d2cc93 100644
--- a/src/gallium/drivers/llvmpipe/ci/llvmpipe-quick_gl.txt
+++ b/src/gallium/drivers/llvmpipe/ci/llvmpipe-quick_gl.txt
@@ -9,8 +9,6 @@ glx/glx-copy-sub-buffer samples=32: skip
glx/glx-copy-sub-buffer samples=4: skip
glx/glx-copy-sub-buffer samples=6: skip
glx/glx-copy-sub-buffer samples=8: skip
-glx/glx-make-current: fail
-glx/glx-multi-window-single-context: fail
glx/glx-multithread-makecurrent-1: skip
glx/glx-multithread-makecurrent-2: skip
glx/glx-multithread-makecurrent-3: skip
diff --git a/src/gallium/drivers/softpipe/ci/softpipe-quick.txt b/src/gallium/drivers/softpipe/ci/softpipe-quick.txt
index 45de3631706..5db94e98d88 100644
--- a/src/gallium/drivers/softpipe/ci/softpipe-quick.txt
+++ b/src/gallium/drivers/softpipe/ci/softpipe-quick.txt
@@ -10,9 +10,7 @@ glx/glx-copy-sub-buffer samples=32: skip
glx/glx-copy-sub-buffer samples=4: skip
glx/glx-copy-sub-buffer samples=6: skip
glx/glx-copy-sub-buffer samples=8: skip
-glx/glx-make-current: fail
glx/glx-multi-context-front: fail
-glx/glx-multi-window-single-context: fail
glx/glx-multithread-makecurrent-1: skip
glx/glx-multithread-makecurrent-2: skip
glx/glx-multithread-makecurrent-3: skip
diff --git a/src/gallium/drivers/zink/ci/piglit-zink-lvp-fails.txt b/src/gallium/drivers/zink/ci/piglit-zink-lvp-fails.txt
index 35bdaf5cbc5..88f2bc5531a 100644
--- a/src/gallium/drivers/zink/ci/piglit-zink-lvp-fails.txt
+++ b/src/gallium/drivers/zink/ci/piglit-zink-lvp-fails.txt
@@ -1,6 +1,5 @@
glx at extension string sanity,Fail
glx at glx-copy-sub-buffer,Fail
-glx at glx-multi-window-single-context,Fail
glx at glx-multithread-buffer,Fail
glx at glx-multithread-texture,Fail
glx at glx-query-drawable-glx_fbconfig_id-window,Fail
diff --git a/src/glx/glxcurrent.c b/src/glx/glxcurrent.c
index a388df7a78d..7172009c34c 100644
--- a/src/glx/glxcurrent.c
+++ b/src/glx/glxcurrent.c
@@ -165,17 +165,100 @@ glXGetCurrentDrawable(void)
return gc->currentDrawable;
}
-/**
- * Make a particular context current.
- *
- * \note This is in this file so that it can access dummyContext.
- */
+static Bool
+SendMakeCurrentRequest(Display * dpy, GLXContextID gc_id,
+ GLXContextTag gc_tag, GLXDrawable draw,
+ GLXDrawable read, GLXContextTag *out_tag)
+{
+ xGLXMakeCurrentReply reply;
+ Bool ret;
+ int opcode = __glXSetupForCommand(dpy);
+
+ LockDisplay(dpy);
+
+ if (draw == read) {
+ xGLXMakeCurrentReq *req;
+
+ GetReq(GLXMakeCurrent, req);
+ req->reqType = opcode;
+ req->glxCode = X_GLXMakeCurrent;
+ req->drawable = draw;
+ req->context = gc_id;
+ req->oldContextTag = gc_tag;
+ }
+ else {
+ struct glx_display *priv = __glXInitialize(dpy);
+
+ if ((priv->majorVersion > 1) || (priv->minorVersion >= 3)) {
+ xGLXMakeContextCurrentReq *req;
+
+ GetReq(GLXMakeContextCurrent, req);
+ req->reqType = opcode;
+ req->glxCode = X_GLXMakeContextCurrent;
+ req->drawable = draw;
+ req->readdrawable = read;
+ req->context = gc_id;
+ req->oldContextTag = gc_tag;
+ }
+ else {
+ xGLXVendorPrivateWithReplyReq *vpreq;
+ xGLXMakeCurrentReadSGIReq *req;
+
+ GetReqExtra(GLXVendorPrivateWithReply,
+ sz_xGLXMakeCurrentReadSGIReq -
+ sz_xGLXVendorPrivateWithReplyReq, vpreq);
+ req = (xGLXMakeCurrentReadSGIReq *) vpreq;
+ req->reqType = opcode;
+ req->glxCode = X_GLXVendorPrivateWithReply;
+ req->vendorCode = X_GLXvop_MakeCurrentReadSGI;
+ req->drawable = draw;
+ req->readable = read;
+ req->context = gc_id;
+ req->oldContextTag = gc_tag;
+ }
+ }
+
+ ret = _XReply(dpy, (xReply *) &reply, 0, False);
+
+
+ if (ret == 1)
+ *out_tag = reply.contextTag;
+
+ UnlockDisplay(dpy);
+ SyncHandle();
+
+ return ret;
+}
+
+static void
+SetGC(struct glx_context *gc, Display *dpy, GLXDrawable draw, GLXDrawable read)
+{
+ gc->currentDpy = dpy;
+ gc->currentDrawable = draw;
+ gc->currentReadable = read;
+}
+
+static Bool
+should_send(Display *dpy, struct glx_context *gc)
+{
+ /* always send for indirect contexts */
+ if (!gc->isDirect)
+ return 1;
+
+ /* don't send for broken servers. */
+ if (VendorRelease(dpy) < 12006000 || VendorRelease(dpy) >= 40000000)
+ return 0;
+
+ return 1;
+}
+
static Bool
MakeContextCurrent(Display * dpy, GLXDrawable draw,
GLXDrawable read, GLXContext gc_user)
{
struct glx_context *gc = (struct glx_context *) gc_user;
struct glx_context *oldGC = __glXGetCurrentContext();
+ Bool ret = GL_FALSE;
/* Make sure that the new context has a nonzero ID. In the request,
* a zero context ID is used only to mean that we bind to no current
@@ -186,66 +269,97 @@ MakeContextCurrent(Display * dpy, GLXDrawable draw,
}
_glapi_check_multithread();
-
__glXLock();
- if (oldGC == gc &&
- gc->currentDrawable == draw && gc->currentReadable == read) {
- __glXUnlock();
- return True;
- }
- /* can't have only one be 0 */
- if (!!draw != !!read) {
- __glXUnlock();
- __glXSendError(dpy, BadMatch, None, X_GLXMakeContextCurrent, True);
- return False;
+ if (oldGC == gc &&
+ gc->currentDrawable == draw &&
+ gc->currentReadable == read) {
+ /* Same context and drawables: no op, just return */
+ ret = GL_TRUE;
}
- if (oldGC != &dummyContext) {
- if (--oldGC->thread_refcount == 0) {
- oldGC->vtable->unbind(oldGC, gc);
- oldGC->currentDpy = 0;
+ else if (oldGC == gc) {
+ /* Same context and new drawables: update drawable bindings */
+ if (should_send(dpy, gc)) {
+ if (!SendMakeCurrentRequest(dpy, gc->xid, gc->currentContextTag,
+ draw, read, &gc->currentContextTag)) {
+ goto out;
+ }
}
- }
- if (gc) {
- /* Attempt to bind the context. We do this before mucking with
- * gc and __glXSetCurrentContext to properly handle our state in
- * case of an error.
- *
- * If an error occurs, set the Null context since we've already
- * blown away our old context. The caller is responsible for
- * figuring out how to handle setting a valid context.
- */
- if (gc->vtable->bind(gc, oldGC, draw, read) != Success) {
+ if (gc->vtable->bind(gc, gc, draw, read) != Success) {
__glXSetCurrentContextNull();
- __glXUnlock();
- __glXSendError(dpy, GLXBadContext, None, X_GLXMakeContextCurrent,
- False);
- return GL_FALSE;
+ goto out;
}
+ }
- if (gc->thread_refcount == 0) {
- gc->currentDpy = dpy;
- gc->currentDrawable = draw;
- gc->currentReadable = read;
+ else {
+ /* Different contexts: release the old, bind the new */
+ GLXContextTag oldTag = oldGC->currentContextTag;
+
+ if (oldGC != &dummyContext) {
+
+ if (--oldGC->thread_refcount == 0) {
+ if (oldGC->xid != None &&
+ should_send(dpy, oldGC) &&
+ !SendMakeCurrentRequest(dpy, None, oldTag, None, None,
+ &oldGC->currentContextTag)) {
+ goto out;
+ }
+
+ oldGC->vtable->unbind(oldGC, gc);
+
+ if (oldGC->xid == None) {
+ /* destroyed context, free it */
+ oldGC->vtable->destroy(oldGC);
+ oldTag = 0;
+ } else {
+ SetGC(oldGC, NULL, None, None);
+ oldTag = oldGC->currentContextTag;
+ }
+ }
}
- gc->thread_refcount++;
- __glXSetCurrentContext(gc);
- } else {
__glXSetCurrentContextNull();
- }
- if (oldGC->thread_refcount == 0 && oldGC != &dummyContext && oldGC->xid == None) {
- /* We are switching away from a context that was
- * previously destroyed, so we need to free the memory
- * for the old handle. */
- oldGC->vtable->destroy(oldGC);
+ if (gc) {
+ /*
+ * MESA_multithread_makecurrent makes this complicated. We need to
+ * send the request if the new context is
+ *
+ * a) indirect (may be current to another client), or
+ * b) (direct and) newly being made current, or
+ * c) (direct and) being bound to new drawables
+ */
+ Bool new_drawables = gc->currentReadable != read ||
+ gc->currentDrawable != draw;
+
+ if (should_send(dpy, gc)) {
+ if (!gc->isDirect || !gc->thread_refcount || new_drawables) {
+ if (!SendMakeCurrentRequest(dpy, gc->xid, oldTag, draw, read,
+ &gc->currentContextTag)) {
+ goto out;
+ }
+ }
+ }
+
+ if (gc->vtable->bind(gc, oldGC, draw, read) != Success) {
+ __glXSendError(dpy, GLXBadContext, None, X_GLXMakeContextCurrent,
+ False);
+ goto out;
+ }
+
+ if (gc->thread_refcount == 0) {
+ SetGC(gc, dpy, draw, read);
+ }
+ gc->thread_refcount++;
+ __glXSetCurrentContext(gc);
+ }
}
+ ret = GL_TRUE;
+out:
__glXUnlock();
-
- return GL_TRUE;
+ return ret;
}
diff --git a/src/glx/indirect_glx.c b/src/glx/indirect_glx.c
index fb60cc0ff5b..d6466225561 100644
--- a/src/glx/indirect_glx.c
+++ b/src/glx/indirect_glx.c
@@ -61,135 +61,40 @@ indirect_destroy_context(struct glx_context *gc)
free((char *) gc);
}
-static Bool
-SendMakeCurrentRequest(Display * dpy, GLXContextID gc_id,
- GLXContextTag gc_tag, GLXDrawable draw,
- GLXDrawable read, GLXContextTag *out_tag)
-{
- xGLXMakeCurrentReply reply;
- Bool ret;
- int opcode = __glXSetupForCommand(dpy);
-
- LockDisplay(dpy);
-
- if (draw == read) {
- xGLXMakeCurrentReq *req;
-
- GetReq(GLXMakeCurrent, req);
- req->reqType = opcode;
- req->glxCode = X_GLXMakeCurrent;
- req->drawable = draw;
- req->context = gc_id;
- req->oldContextTag = gc_tag;
- }
- else {
- struct glx_display *priv = __glXInitialize(dpy);
-
- /* If the server can support the GLX 1.3 version, we should
- * perfer that. Not only that, some servers support GLX 1.3 but
- * not the SGI extension.
- */
-
- if ((priv->majorVersion > 1) || (priv->minorVersion >= 3)) {
- xGLXMakeContextCurrentReq *req;
-
- GetReq(GLXMakeContextCurrent, req);
- req->reqType = opcode;
- req->glxCode = X_GLXMakeContextCurrent;
- req->drawable = draw;
- req->readdrawable = read;
- req->context = gc_id;
- req->oldContextTag = gc_tag;
- }
- else {
- xGLXVendorPrivateWithReplyReq *vpreq;
- xGLXMakeCurrentReadSGIReq *req;
-
- GetReqExtra(GLXVendorPrivateWithReply,
- sz_xGLXMakeCurrentReadSGIReq -
- sz_xGLXVendorPrivateWithReplyReq, vpreq);
- req = (xGLXMakeCurrentReadSGIReq *) vpreq;
- req->reqType = opcode;
- req->glxCode = X_GLXVendorPrivateWithReply;
- req->vendorCode = X_GLXvop_MakeCurrentReadSGI;
- req->drawable = draw;
- req->readable = read;
- req->context = gc_id;
- req->oldContextTag = gc_tag;
- }
- }
-
- ret = _XReply(dpy, (xReply *) &reply, 0, False);
-
- if (out_tag)
- *out_tag = reply.contextTag;
-
- UnlockDisplay(dpy);
- SyncHandle();
-
- return ret;
-}
-
static int
indirect_bind_context(struct glx_context *gc, struct glx_context *old,
GLXDrawable draw, GLXDrawable read)
{
- GLXContextTag tag;
- Display *dpy = gc->psc->dpy;
- Bool sent;
-
- if (old != &dummyContext && !old->isDirect && old->psc->dpy == dpy) {
- tag = old->currentContextTag;
- old->currentContextTag = 0;
- } else {
- tag = 0;
+ __GLXattribute *state = gc->client_state_private;
+
+ if (!IndirectAPI)
+ IndirectAPI = __glXNewIndirectAPI();
+ _glapi_set_dispatch(IndirectAPI);
+
+ /* The indirect vertex array state must to be initialised after we
+ * have setup the context, as it needs to query server attributes.
+ *
+ * At the point this is called gc->currentDpy is not initialized
+ * nor is the thread's current context actually set. Hence the
+ * cleverness before the GetString calls.
+ */
+ if (state && state->array_state == NULL) {
+ gc->currentDpy = gc->psc->dpy;
+ __glXSetCurrentContext(gc);
+ __indirect_glGetString(GL_EXTENSIONS);
+ __indirect_glGetString(GL_VERSION);
+ __glXInitVertexArrayState(gc);
}
- sent = SendMakeCurrentRequest(dpy, gc->xid, tag, draw, read,
- &gc->currentContextTag);
+ if (state != NULL && state->array_state != NULL)
+ return Success;
- if (sent) {
- if (!IndirectAPI)
- IndirectAPI = __glXNewIndirectAPI();
- _glapi_set_dispatch(IndirectAPI);
-
- /* The indirect vertex array state must to be initialised after we
- * have setup the context, as it needs to query server attributes.
- *
- * At the point this is called gc->currentDpy is not initialized
- * nor is the thread's current context actually set. Hence the
- * cleverness before the GetString calls.
- */
- __GLXattribute *state = gc->client_state_private;
- if (state && state->array_state == NULL) {
- gc->currentDpy = gc->psc->dpy;
- __glXSetCurrentContext(gc);
- __indirect_glGetString(GL_EXTENSIONS);
- __indirect_glGetString(GL_VERSION);
- __glXInitVertexArrayState(gc);
- }
- }
-
- return !sent;
+ return BadAlloc;
}
static void
indirect_unbind_context(struct glx_context *gc, struct glx_context *new)
{
- Display *dpy = gc->psc->dpy;
-
- if (gc == new)
- return;
-
- /* We are either switching to no context, away from an indirect
- * context to a direct context or from one dpy to another and have
- * to send a request to the dpy to unbind the previous context.
- */
- if (!new || new->isDirect || new->psc->dpy != dpy) {
- SendMakeCurrentRequest(dpy, None, gc->currentContextTag, None, None,
- NULL);
- gc->currentContextTag = 0;
- }
}
static void
More information about the mesa-commit
mailing list