[PATCH] Fix for issues with asynchronous/multithreaded openGL

Paul Olav Tvete paul.tvete at nokia.com
Wed Nov 16 07:55:33 PST 2011


Two problems arise with asynchronous GL drivers that allow issuing GL
commands for the next frame before the GPU is done with the current
frame:

1. The client code never knows when it is necessary to call
wl_display_iterate.

Solution: Write to a pipe when there is data to be sent. The client can
then select() on the pipe's fd.

2. The GL library might call  wl_surface_attach/wl_surface_damage while
the main thread is doing wl_display_iterate.

The problem is also visible if makeCurrent and swapBuffers is called
from another thread then the thread calling wl_display_iterate.
Solution:

Protect wl_display_iterate and wl_proxy_marshal with a mutex

Also add a wl_display_thread which returnes the thread for what thread
the wl_display was created on. This makes it possible to figure out if
wl_display_iterate can be safely called.

Reviewed-By: Jørgen Lind<jorgen.lind at nokia.com>
---
 src/Makefile.am      |    2 +
 src/wayland-client.c |   94 ++++++++++++++++++++++++++++++++++++++++++++++---
 src/wayland-client.h |    5 +++
 3 files changed, 95 insertions(+), 6 deletions(-)

diff --git a/src/Makefile.am b/src/Makefile.am
index f356b54..9aab9de 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -24,6 +24,8 @@ libwayland_server_la_SOURCES =			\
 	event-loop.c
 
 libwayland_client_la_LIBADD = $(FFI_LIBS) libwayland-util.la -lrt
+libwayland_client_la_LDFLAGS = -pthread
+libwayland_client_la_CFLAGS = -pthread
 libwayland_client_la_SOURCES =			\
 	wayland-protocol.c			\
 	wayland-client.c
diff --git a/src/wayland-client.c b/src/wayland-client.c
index 7814379..611ef3a 100644
--- a/src/wayland-client.c
+++ b/src/wayland-client.c
@@ -34,6 +34,7 @@
 #include <assert.h>
 #include <fcntl.h>
 #include <sys/poll.h>
+#include <pthread.h>
 
 #include "wayland-util.h"
 #include "wayland-client.h"
@@ -62,6 +63,9 @@ struct wl_display {
 	struct wl_proxy proxy;
 	struct wl_connection *connection;
 	int fd;
+	int write_notification_pipe[2];
+	pthread_t thread_id;
+	pthread_mutex_t marshalling_mutex;
 	uint32_t mask;
 	struct wl_map objects;
 	struct wl_list global_listener_list;
@@ -186,12 +190,22 @@ wl_proxy_add_listener(struct wl_proxy *proxy,
 	return 0;
 }
 
+static void
+display_signal_flush_needed(struct wl_display *display)
+{
+	int success;
+	const char data = '!';
+	if (display->write_notification_pipe[1])
+		success = write(display->write_notification_pipe[1], &data, 1);
+}
+
 WL_EXPORT void
 wl_proxy_marshal(struct wl_proxy *proxy, uint32_t opcode, ...)
 {
 	struct wl_closure *closure;
 	va_list ap;
 
+	pthread_mutex_lock(&proxy->display->marshalling_mutex);
 	va_start(ap, opcode);
 	closure = wl_connection_vmarshal(proxy->display->connection,
 					 &proxy->object, opcode, ap,
@@ -209,6 +223,11 @@ wl_proxy_marshal(struct wl_proxy *proxy, uint32_t opcode, ...)
 		wl_closure_print(closure, &proxy->object, true);
 
 	wl_closure_destroy(closure);
+
+	if (proxy->display->thread_id != pthread_self())
+		display_signal_flush_needed(proxy->display);
+
+	pthread_mutex_unlock(&proxy->display->marshalling_mutex);
 }
 
 /* Can't do this, there may be more than one instance of an
@@ -344,6 +363,7 @@ wl_display_connect(const char *name)
 	const char *debug;
 	char *connection, *end;
 	int flags;
+	int success;
 
 	debug = getenv("WAYLAND_DEBUG");
 	if (debug)
@@ -393,6 +413,20 @@ wl_display_connect(const char *name)
 		return NULL;
 	}
 
+	display->thread_id = pthread_self();
+	pthread_mutexattr_t mutex_attr;
+	success = pthread_mutexattr_init(&mutex_attr);
+	success += pthread_mutexattr_settype(&mutex_attr,PTHREAD_MUTEX_RECURSIVE);
+	success += pthread_mutex_init(&display->marshalling_mutex, &mutex_attr);
+	success += pthread_mutexattr_destroy(&mutex_attr);
+
+	if (success)
+		fprintf(stderr, "Threading setup was unsuccessfull\n");
+
+	success = pipe2(display->write_notification_pipe, O_CLOEXEC | O_NONBLOCK);
+	if (success)
+		fprintf(stderr, "Failed to initialize write notification pipe\n");
+
 	return display;
 }
 
@@ -427,6 +461,18 @@ wl_display_get_fd(struct wl_display *display,
 	return display->fd;
 }
 
+WL_EXPORT int
+wl_display_get_write_notification_fd(struct wl_display *display)
+{
+	return display->write_notification_pipe[0];
+}
+
+WL_EXPORT pthread_t
+wl_display_thread(struct wl_display *display)
+{
+	return display->thread_id;
+}
+
 static void
 sync_callback(void *data, struct wl_callback *callback, uint32_t time)
 {
@@ -440,18 +486,46 @@ static const struct wl_callback_listener sync_listener = {
 	sync_callback
 };
 
+static void
+threaded_sync_callback(void *data, struct wl_callback *callback, uint32_t time)
+{
+   fprintf(stderr, "threaded_sync_callback\n");
+   pthread_cond_t *wait_condition = data;
+
+   pthread_cond_broadcast(wait_condition);
+   wl_callback_destroy(callback);
+}
+
+static const struct wl_callback_listener threaded_sync_listener = {
+	threaded_sync_callback
+};
+
 WL_EXPORT void
 wl_display_roundtrip(struct wl_display *display)
 {
 	struct wl_callback *callback;
 	int done;
+	pthread_cond_t wait_cond;
+	pthread_mutex_t wait_mutex;
 
-	done = 0;
 	callback = wl_display_sync(display);
-	wl_callback_add_listener(callback, &sync_listener, &done);
-	wl_display_flush(display);
-	while (!done)
-		wl_display_iterate(display, WL_DISPLAY_READABLE);
+
+	if (wl_display_thread(display) == pthread_self()) {
+		done = 0;
+		wl_callback_add_listener(callback, &sync_listener, &done);
+		wl_display_flush(display);
+		while (!done)
+			wl_display_iterate(display, WL_DISPLAY_READABLE);
+	} else {
+		pthread_mutex_init(&wait_mutex,NULL);
+		pthread_cond_init(&wait_cond, NULL);
+		pthread_mutex_lock(&wait_mutex);
+
+		wl_callback_add_listener(callback, &threaded_sync_listener, &wait_cond);
+		pthread_cond_wait(&wait_cond,&wait_mutex);
+		pthread_cond_destroy(&wait_cond);
+		pthread_mutex_destroy(&wait_mutex);
+	}
 }
 
 static void
@@ -494,9 +568,17 @@ handle_event(struct wl_display *display,
 WL_EXPORT void
 wl_display_iterate(struct wl_display *display, uint32_t mask)
 {
+	if (pthread_self() != display->thread_id)
+		fprintf(stderr,
+			"wl_display_iterate called from another thread than "
+			"the thead that created wl_display. This will result "
+			"in undefined behavior\n");
 	uint32_t p[2], object, opcode, size;
 	int len;
-
+	if ((mask & WL_DISPLAY_WRITABLE) && display->write_notification_pipe[0]) {
+		char buf[80];
+		int n = read(display->write_notification_pipe[0], buf, 80);
+	}
 	mask &= display->mask;
 	if (mask == 0) {
 		fprintf(stderr,
diff --git a/src/wayland-client.h b/src/wayland-client.h
index efeee4a..c9a616f 100644
--- a/src/wayland-client.h
+++ b/src/wayland-client.h
@@ -25,6 +25,8 @@
 
 #include "wayland-util.h"
 
+#include <pthread.h>
+
 #ifdef  __cplusplus
 extern "C" {
 #endif
@@ -94,6 +96,9 @@ uint32_t
 wl_display_get_global(struct wl_display *display,
 		      const char *interface, uint32_t version);
 
+int wl_display_get_write_notification_fd(struct wl_display *display);
+pthread_t wl_display_thread(struct wl_display *display);
+
 #ifdef  __cplusplus
 }
 #endif
-- 
1.7.5.4


--VS++wcV0S1rZb1Fb
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename="0001-Wayland-Use-the-thread-affinity-for-the-display.patch"



More information about the wayland-devel mailing list