gl: Do less work when acquiring and releasing devices

After acquiring a GL device and the same GL context, surface, and
display combination is already active outside of Cairo, do not ask EGL
or GLX to change the current context as that may cause a flush on some
drivers. Also do not unset the context when releasing the device for the
same reason.

src/cairo-egl-context.c

@@ -49,6 +49,10 @@ typedef struct _cairo_egl_context {
     EGLContext context;
 
     EGLSurface dummy_surface;
+
+    EGLDisplay previous_display;
+    EGLContext previous_context;
+    EGLSurface previous_surface;
 } cairo_egl_context_t;
 
 typedef struct _cairo_egl_surface {
@@ -58,19 +62,54 @@ typedef struct _cairo_egl_surface {
 } cairo_egl_surface_t;
 
 
+static cairo_bool_t
+_context_acquisition_changed_egl_state (cairo_egl_context_t *ctx,
+					EGLSurface current_surface)
+{
+    return !(ctx->previous_display == ctx->display &&
+	     ctx->previous_surface == current_surface &&
+	     ctx->previous_context == ctx->context);
+}
+
+static EGLSurface
+_egl_get_current_surface (cairo_egl_context_t *ctx)
+{
+    if (ctx->base.current_target == NULL ||
+        _cairo_gl_surface_is_texture (ctx->base.current_target)) {
+	return  ctx->dummy_surface;
+    }
+
+    return ((cairo_egl_surface_t *) ctx->base.current_target)->egl;
+}
+
+static void
+_egl_query_current_state (cairo_egl_context_t *ctx)
+{
+    ctx->previous_display = eglGetCurrentDisplay ();
+    ctx->previous_surface = eglGetCurrentSurface (EGL_DRAW);
+    ctx->previous_context = eglGetCurrentContext ();
+
+    /* If any of the values were none, assume they are all none. Not all
+       drivers seem well behaved when it comes to using these values across
+       multiple threads. */
+    if (ctx->previous_surface == EGL_NO_SURFACE
+	|| ctx->previous_display == EGL_NO_DISPLAY
+	|| ctx->previous_context == EGL_NO_CONTEXT) {
+	ctx->previous_surface = EGL_NO_SURFACE;
+	ctx->previous_display = EGL_NO_DISPLAY;
+	ctx->previous_context = EGL_NO_CONTEXT;
+    }
+}
+
 static void
 _egl_acquire (void *abstract_ctx)
 {
     cairo_egl_context_t *ctx = abstract_ctx;
-    EGLSurface current_surface;
+    EGLSurface current_surface = _egl_get_current_surface (ctx);
 
-    if (ctx->base.current_target == NULL ||
-        _cairo_gl_surface_is_texture (ctx->base.current_target)) {
-        current_surface = ctx->dummy_surface;
-    } else {
-        cairo_egl_surface_t *surface = (cairo_egl_surface_t *) ctx->base.current_target;
-        current_surface = surface->egl ;
-    }
+    _egl_query_current_state (ctx);
+    if (!_context_acquisition_changed_egl_state (ctx, current_surface))
+	return;
 
     eglMakeCurrent (ctx->display,
 		    current_surface, current_surface, ctx->context);
@@ -80,8 +119,11 @@ static void
 _egl_release (void *abstract_ctx)
 {
     cairo_egl_context_t *ctx = abstract_ctx;
-    if (!ctx->base.thread_aware)
+    if (!ctx->base.thread_aware ||
+	!_context_acquisition_changed_egl_state (ctx,
+						 _egl_get_current_surface (ctx))) {
 	return;
+    }
 
     eglMakeCurrent (ctx->display,
 		    EGL_NO_SURFACE, EGL_NO_SURFACE, EGL_NO_CONTEXT);
@@ -161,6 +203,10 @@ cairo_egl_device_create (EGLDisplay dpy, EGLContext egl)
     ctx->base.swap_buffers = _egl_swap_buffers;
     ctx->base.destroy = _egl_destroy;
 
+    /* We are about the change the current state of EGL, so we should
+     * query the pre-existing surface now instead of later. */
+    _egl_query_current_state (ctx);
+
     if (!_egl_make_current_surfaceless (ctx)) {
 	/* Fall back to dummy surface, meh. */
 	EGLint config_attribs[] = {

src/cairo-glx-context.c

@@ -53,6 +53,10 @@ typedef struct _cairo_glx_context {
     Window dummy_window;
     GLXContext context;
 
+    Display *previous_display;
+    GLXDrawable previous_drawable;
+    GLXContext previous_context;
+
     cairo_bool_t has_multithread_makecurrent;
 } cairo_glx_context_t;
 
@@ -62,19 +66,54 @@ typedef struct _cairo_glx_surface {
     Window win;
 } cairo_glx_surface_t;
 
+static cairo_bool_t
+_context_acquisition_changed_glx_state (cairo_glx_context_t *ctx,
+					GLXDrawable current_drawable)
+{
+    return !(ctx->previous_display == ctx->display &&
+	     ctx->previous_drawable == current_drawable &&
+	     ctx->previous_context == ctx->context);
+}
+
+static GLXDrawable
+_glx_get_current_drawable (cairo_glx_context_t *ctx)
+{
+    if (ctx->base.current_target == NULL ||
+	_cairo_gl_surface_is_texture (ctx->base.current_target)) {
+	return ctx->dummy_window;
+    }
+
+    return ((cairo_glx_surface_t *) ctx->base.current_target)->win;
+}
+
+static void
+_glx_query_current_state (cairo_glx_context_t * ctx)
+{
+    ctx->previous_drawable = glXGetCurrentDrawable ();
+    ctx->previous_display = glXGetCurrentDisplay ();
+    ctx->previous_context = glXGetCurrentContext ();
+
+    /* If any of the values were none, assume they are all none. Not all
+       drivers seem well behaved when it comes to using these values across
+       multiple threads. */
+    if (ctx->previous_drawable == None
+	|| ctx->previous_display == None
+	|| ctx->previous_context == None) {
+	ctx->previous_drawable = None;
+	ctx->previous_display = None;
+	ctx->previous_context = None;
+    }
+}
+
 static void
 _glx_acquire (void *abstract_ctx)
 {
     cairo_glx_context_t *ctx = abstract_ctx;
-    GLXDrawable current_drawable;
+    GLXDrawable current_drawable = _glx_get_current_drawable (ctx);
 
-    if (ctx->base.current_target == NULL ||
-        _cairo_gl_surface_is_texture (ctx->base.current_target)) {
-        current_drawable = ctx->dummy_window;
-    } else {
-        cairo_glx_surface_t *surface = (cairo_glx_surface_t *) ctx->base.current_target;
-        current_drawable = surface->win;
-    }
+    _glx_query_current_state (ctx);
+    if (!_context_acquisition_changed_glx_state (ctx, current_drawable))
+	return;
 
     glXMakeCurrent (ctx->display, current_drawable, ctx->context);
 }
@@ -94,8 +133,11 @@ _glx_release (void *abstract_ctx)
 {
     cairo_glx_context_t *ctx = abstract_ctx;
 
-    if (ctx->has_multithread_makecurrent || !ctx->base.thread_aware)
+    if (ctx->has_multithread_makecurrent || !ctx->base.thread_aware ||
+	!_context_acquisition_changed_glx_state (ctx,
+						_glx_get_current_drawable (ctx))) {
 	return;
+    }
 
     glXMakeCurrent (ctx->display, None, None);
 }
@@ -118,11 +160,11 @@ _glx_destroy (void *abstract_ctx)
     if (ctx->dummy_window != None)
 	XDestroyWindow (ctx->display, ctx->dummy_window);
 
-    glXMakeCurrent (ctx->display, 0, 0);
+    glXMakeCurrent (ctx->display, None, None);
 }
 
 static cairo_status_t
-_glx_dummy_ctx (Display *dpy, GLXContext gl_ctx, Window *dummy)
+_glx_dummy_window (Display *dpy, GLXContext gl_ctx, Window *dummy)
 {
     int attr[3] = { GLX_FBCONFIG_ID, 0, None };
     GLXFBConfig *config;
@@ -181,14 +223,20 @@ cairo_glx_device_create (Display *dpy, GLXContext gl_ctx)
     Window dummy = None;
     const char *glx_extensions;
 
-    status = _glx_dummy_ctx (dpy, gl_ctx, &dummy);
-    if (unlikely (status))
-	return _cairo_gl_context_create_in_error (status);
-
     ctx = calloc (1, sizeof (cairo_glx_context_t));
     if (unlikely (ctx == NULL))
 	return _cairo_gl_context_create_in_error (CAIRO_STATUS_NO_MEMORY);
 
+    /* glx_dummy_window will call glXMakeCurrent, so we need to
+     *  query the current state of the context now. */
+    _glx_query_current_state (ctx);
+
+    status = _glx_dummy_window (dpy, gl_ctx, &dummy);
+    if (unlikely (status)) {
+	free (ctx);
+	return _cairo_gl_context_create_in_error (status);
+    }
+
     ctx->display = dpy;
     ctx->dummy_window = dummy;
     ctx->context = gl_ctx;

test/Makefile.sources

@@ -391,6 +391,7 @@ ft_font_test_sources = \
 	ft-text-antialias-none.c
 
 gl_surface_test_sources = \
+	gl-device-release.c \
 	gl-surface-source.c
 
 quartz_surface_test_sources = quartz-surface-source.c

test/gl-device-release.c

@@ -0,0 +1,182 @@
+/*
+ * Copyright © 2012 Igalia S.L.
+ * Copyright © 2009 Eric Anholt
+ * Copyright © 2009 Chris Wilson
+ * Copyright © 2005 Red Hat, Inc
+ *
+ * Permission to use, copy, modify, distribute, and sell this software
+ * and its documentation for any purpose is hereby granted without
+ * fee, provided that the above copyright notice appear in all copies
+ * and that both that copyright notice and this permission notice
+ * appear in supporting documentation, and that the name of
+ * Chris Wilson not be used in advertising or publicity pertaining to
+ * distribution of the software without specific, written prior
+ * permission. Chris Wilson makes no representations about the
+ * suitability of this software for any purpose.  It is provided "as
+ * is" without express or implied warranty.
+ *
+ * IGALIA S.L. DISCLAIMS ALL WARRANTIES WITH REGARD TO THIS
+ * SOFTWARE, INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND
+ * FITNESS, IN NO EVENT SHALL CHRIS WILSON BE LIABLE FOR ANY SPECIAL,
+ * INDIRECT OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER
+ * RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION
+ * OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR
+ * IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+ *
+ * Author: Martin Robinson <mrobinson@igalia.com>
+ */
+
+#include "cairo-test.h"
+#include <cairo-gl.h>
+#include <assert.h>
+
+static Window
+create_test_window (Display *display,
+		    GLXContext glx_context,
+		    XVisualInfo *visual_info)
+{
+    Colormap colormap;
+    XSetWindowAttributes window_attributes;
+    Window window = None;
+
+    colormap = XCreateColormap (display,
+			    RootWindow (display, visual_info->screen),
+			    visual_info->visual,
+			    AllocNone);
+    window_attributes.colormap = colormap;
+    window_attributes.border_pixel = 0;
+    window = XCreateWindow (display, RootWindow (display, visual_info->screen),
+			    -1, -1, 1, 1, 0,
+			    visual_info->depth,
+			    InputOutput,
+			    visual_info->visual,
+			    CWBorderPixel | CWColormap, &window_attributes);
+    XFreeColormap (display, colormap);
+
+    XFlush (display);
+    return window;
+}
+
+static cairo_bool_t
+multithread_makecurrent_available (Display *display)
+{
+    const char *extensions = glXQueryExtensionsString (display,
+						       DefaultScreen (display));
+    return !! strstr(extensions, "GLX_MESA_multithread_makecurrent");
+}
+
+static void
+draw_to_surface (cairo_surface_t *surface)
+{
+    cairo_t *cr = cairo_create (surface);
+    cairo_paint (cr);
+    cairo_destroy (cr);
+}
+
+static cairo_test_status_t
+preamble (cairo_test_context_t *test_ctx)
+{
+    int rgba_attribs[] = {
+	GLX_RGBA,
+	GLX_RED_SIZE, 1,
+	GLX_GREEN_SIZE, 1,
+	GLX_BLUE_SIZE, 1,
+	GLX_ALPHA_SIZE, 1,
+	GLX_DOUBLEBUFFER,
+	None
+    };
+
+    XVisualInfo *visual_info;
+    GLXContext glx_context;
+    cairo_device_t *device;
+    Display *display;
+    Window test_window;
+    cairo_surface_t *window_surface;
+    cairo_bool_t has_multithread_makecurrent;
+
+    display = XOpenDisplay (NULL);
+    if (display == NULL)
+	return CAIRO_TEST_UNTESTED;
+
+    visual_info = glXChooseVisual (display, DefaultScreen (display), rgba_attribs);
+    if (visual_info == NULL) {
+	XCloseDisplay (display);
+	return CAIRO_TEST_UNTESTED;
+    }
+
+    glx_context = glXCreateContext (display, visual_info, NULL, True);
+    if (glx_context == NULL) {
+	XCloseDisplay (display);
+	return CAIRO_TEST_UNTESTED;
+    }
+
+    test_window = create_test_window (display, glx_context, visual_info);
+    XFree (visual_info);
+    if (test_window == None) {
+	XCloseDisplay (display);
+	return CAIRO_TEST_UNTESTED;
+    }
+
+    has_multithread_makecurrent = multithread_makecurrent_available (display);
+
+    glXMakeCurrent (display, None, None);
+
+    /* Creating the device should actually change the GL context, because of
+     * the creation/activation of a dummy window used for texture surfaces. */
+    device = cairo_glx_device_create (display, glx_context);
+
+    /* It's important that when multithread_makecurrent isn't available the
+     * Cairo backend clears the current context, so that the dummy texture
+     * window is not active while the device is unlocked. */
+    if (has_multithread_makecurrent) {
+	assert (None != glXGetCurrentDrawable ());
+	assert (display == glXGetCurrentDisplay ());
+	assert (glx_context == glXGetCurrentContext ());
+    } else {
+	assert (None == glXGetCurrentDrawable ());
+	assert (None == glXGetCurrentDisplay ());
+	assert (None == glXGetCurrentContext ());
+    }
+
+    window_surface = cairo_gl_surface_create_for_window (device, test_window,
+							 1, 1);
+    assert (cairo_surface_status (window_surface) == CAIRO_STATUS_SUCCESS);
+
+    draw_to_surface (window_surface);
+    if (has_multithread_makecurrent) {
+	assert (test_window == glXGetCurrentDrawable ());
+	assert (display == glXGetCurrentDisplay ());
+	assert (glx_context == glXGetCurrentContext ());
+    } else {
+	assert (None == glXGetCurrentDrawable ());
+	assert (None == glXGetCurrentDisplay ());
+	assert (None == glXGetCurrentContext ());
+    }
+
+    /* In this case, drawing to the window surface will not change the current
+     * GL context, so Cairo setting the current surface and context to none. */
+    glXMakeCurrent (display, test_window, glx_context);
+    draw_to_surface (window_surface);
+    assert (test_window == glXGetCurrentDrawable ());
+    assert (display == glXGetCurrentDisplay ());
+    assert (glx_context == glXGetCurrentContext ());
+
+    /* There should be no context change when destroying the device. */
+    cairo_device_destroy (device);
+    assert (test_window == glXGetCurrentDrawable ());
+    assert (display == glXGetCurrentDisplay ());
+    assert (glx_context == glXGetCurrentContext ());
+
+    glXDestroyContext(display, glx_context);
+    XDestroyWindow (display, test_window);
+    XCloseDisplay (display);
+
+    return CAIRO_TEST_SUCCESS;
+}
+
+CAIRO_TEST (gl_device_creation_changes_context,
+	    "Test that using the Cairo GL backend leaves the current GL context in the appropriate state",
+	    "gl", /* keywords */
+	    NULL, /* requirements */
+	    0, 0,
+	    preamble, NULL)