Revision: 338 http://trac.macosforge.org/projects/xquartz/changeset/338 Author: gstaplin@apple.com Date: 2009-04-28 20:40:04 -0700 (Tue, 28 Apr 2009) Log Message: ----------- tests/simple/simple.mk: Add the query_drawable test and make it build. tests/simple/query_drawable.c: Add a test for glXQueryDrawable, other than the existing pbuffer test. tests/tests.mk: build testbuilds/query_drawable glx_pbuffer.c: Fix a deadlock reported by Ken Thomases. The deadlock occurred in the glXQueryDrawable path when the drawable was a window. The paths for the valid pixmaps and buffers were unaffected. Thank you Ken, for your detailed analysis. Modified Paths: -------------- AppleSGLX/trunk/glx_pbuffer.c AppleSGLX/trunk/tests/simple/simple.mk AppleSGLX/trunk/tests/tests.mk Added Paths: ----------- AppleSGLX/trunk/tests/simple/query_drawable.c Modified: AppleSGLX/trunk/glx_pbuffer.c =================================================================== --- AppleSGLX/trunk/glx_pbuffer.c 2009-04-12 02:24:16 UTC (rev 337) +++ AppleSGLX/trunk/glx_pbuffer.c 2009-04-29 03:40:04 UTC (rev 338) @@ -44,21 +44,6 @@ #include "apple_glx_drawable.h" #include "glx_error.h" - -static pthread_mutex_t queryLock = PTHREAD_MUTEX_INITIALIZER; - -/* - * This is protected by the queryLock. - * Unfortunately XSetErrorHandler requires a lock. - */ -static int errorCount = 0; - -static int -errorHandler(Display *dpy, XErrorEvent *error) { - ++errorCount; - return 0; /* don't exit */ -} - /** * Create a new pbuffer. */ @@ -133,7 +118,6 @@ Window root; int x, y; unsigned int width, height, bd, depth; - int (*old_handler)(Display *, XErrorEvent *); if(apple_glx_pixmap_query(drawable, attribute, value)) return; /*done*/ @@ -142,33 +126,27 @@ return; /*done*/ /* - * This is used because XSetErrorHandler changes global state. - * Ideally we should prevent other threads from calling XSetErrorHandler. - * Another way of handling this might be a private display connection. + * The OpenGL spec states that we should report GLXBadDrawable if + * the drawable is invalid, however doing so would require that we + * use XSetErrorHandler(), which is known to not be thread safe. + * If we use a round-trip call to validate the drawable, there could + * be a race, so instead we just opt in favor of letting the + * XGetGeometry request fail with a GetGeometry request X error + * rather than GLXBadDrawable, in what is hoped to be a rare + * case of an invalid drawable. In practice most and possibly all + * X11 apps using GLX shouldn't notice a difference. */ - pthread_mutex_lock(&queryLock); - - old_handler = XSetErrorHandler(errorHandler); - if(XGetGeometry(dpy, drawable, &root, &x, &y, &width, &height, &bd, &depth)) { switch(attribute) { case GLX_WIDTH: *value = width; - return; + break; case GLX_HEIGHT: *value = height; - return; + break; } - /*FALL THROUGH*/ } - - XSetErrorHandler(old_handler); - - pthread_mutex_unlock(&queryLock); - - __glXSendError(dpy, GLXBadDrawable, drawable, X_GLXGetDrawableAttributes, - false); } Added: AppleSGLX/trunk/tests/simple/query_drawable.c =================================================================== --- AppleSGLX/trunk/tests/simple/query_drawable.c (rev 0) +++ AppleSGLX/trunk/tests/simple/query_drawable.c 2009-04-29 03:40:04 UTC (rev 338) @@ -0,0 +1,45 @@ +#include <stdio.h> +#include <stdlib.h> +#include <X11/Xlib.h> +#include <GL/glx.h> + +int main(int argc, char *argv[]) { + Display *dpy; + int eventbase, errorbase; + int major, minor; + Window root; + unsigned int width, height; + + dpy = XOpenDisplay(NULL); + + if(NULL == dpy) { + fprintf(stderr, "error: unable to open display!\n"); + return EXIT_FAILURE; + } + + if(!glXQueryExtension(dpy, &eventbase, &errorbase)) { + fprintf(stderr, "GLX is not available!\n"); + return EXIT_FAILURE; + } + + printf("GLX eventbase %d errorbase %d\n", eventbase, errorbase); + + if(!glXQueryVersion(dpy, &major, &minor)) { + fprintf(stderr, "GLX version query error!\n"); + return EXIT_FAILURE; + } + + printf("GLX version: %d.%d\n", major, minor); + + root = DefaultRootWindow(dpy); + + glXQueryDrawable(dpy, root, GLX_WIDTH, &width); + + printf("query 1: width %d\n", width); + + glXQueryDrawable(dpy, root, GLX_HEIGHT, &height); + + printf("query 2: height %d\n", height); + + return EXIT_SUCCESS; +} Modified: AppleSGLX/trunk/tests/simple/simple.mk =================================================================== --- AppleSGLX/trunk/tests/simple/simple.mk 2009-04-12 02:24:16 UTC (rev 337) +++ AppleSGLX/trunk/tests/simple/simple.mk 2009-04-29 03:40:04 UTC (rev 338) @@ -13,3 +13,5 @@ $(TEST_BUILD_DIR)/glthreads: tests/simple/glthreads.c $(LIBGL) $(CC) -DPTHREADS -pthread tests/simple/glthreads.c $(INCLUDE) -o $(TEST_BUILD_DIR)/glthreads $(LINK_TEST) +$(TEST_BUILD_DIR)/query_drawable: tests/simple/query_drawable.c $(LIBGL) + $(CC) tests/simple/query_drawable.c $(INCLUDE) -o $@ $(LINK_TEST) Modified: AppleSGLX/trunk/tests/tests.mk =================================================================== --- AppleSGLX/trunk/tests/tests.mk 2009-04-12 02:24:16 UTC (rev 337) +++ AppleSGLX/trunk/tests/tests.mk 2009-04-29 03:40:04 UTC (rev 338) @@ -35,5 +35,6 @@ $(TEST_BUILD_DIR)/triangle_glx_surface \ $(TEST_BUILD_DIR)/triangle_glx_surface-2 \ $(TEST_BUILD_DIR)/triangle_glx_withdraw_remap \ - $(TEST_BUILD_DIR)/triangle_glx_destroy_relation + $(TEST_BUILD_DIR)/triangle_glx_destroy_relation \ + $(TEST_BUILD_DIR)/query_drawable