[cairo-commit] 8 commits - boilerplate/cairo-boilerplate-wgl.c src/cairo-gl-composite.c src/cairo-gl-device.c src/cairo-gl-glyphs.c src/cairo-gl-gradient.c src/cairo-gl-private.h src/cairo-gl-surface.c src/cairo-glx-context.c

Benjamin Otte company at kemper.freedesktop.org
Fri Jun 18 07:32:00 PDT 2010


 boilerplate/cairo-boilerplate-wgl.c |    2 -
 src/cairo-gl-composite.c            |    6 +--
 src/cairo-gl-device.c               |    2 -
 src/cairo-gl-glyphs.c               |   14 ++-----
 src/cairo-gl-gradient.c             |    3 +
 src/cairo-gl-private.h              |   38 ++++++++++----------
 src/cairo-gl-surface.c              |   67 +++++++++++++-----------------------
 src/cairo-glx-context.c             |   19 +---------
 8 files changed, 58 insertions(+), 93 deletions(-)

New commits:
commit 3908d80f57bc0abef47721a5b8b9afd5041118e3
Author: Benjamin Otte <otte at redhat.com>
Date:   Fri Jun 18 16:11:20 2010 +0200

    glx: Remove useless optimization trying to avoid glXMakeCurrent()
    
    The optimization is not performance-relevant. And having less code is
    always a good idea.

diff --git a/src/cairo-glx-context.c b/src/cairo-glx-context.c
index d289051..fa9d8be 100644
--- a/src/cairo-glx-context.c
+++ b/src/cairo-glx-context.c
@@ -52,9 +52,6 @@ typedef struct _cairo_glx_context {
     Display *display;
     Window dummy_window;
     GLXContext context;
-
-    GLXContext prev_context;
-    GLXDrawable prev_drawable;
 } cairo_glx_context_t;
 
 typedef struct _cairo_glx_surface {
@@ -69,9 +66,6 @@ _glx_acquire (void *abstract_ctx)
     cairo_glx_context_t *ctx = abstract_ctx;
     GLXDrawable current_drawable;
 
-    ctx->prev_context = glXGetCurrentContext ();
-    ctx->prev_drawable = glXGetCurrentDrawable ();
-
     if (ctx->base.current_target == NULL ||
         _cairo_gl_surface_is_texture (ctx->base.current_target)) {
         current_drawable = ctx->dummy_window;
@@ -80,10 +74,7 @@ _glx_acquire (void *abstract_ctx)
         current_drawable = surface->win;
     }
 
-    if (ctx->prev_context != ctx->context ||
-        (ctx->prev_drawable != current_drawable &&
-         current_drawable != ctx->dummy_window))
-        glXMakeCurrent (ctx->display, current_drawable, ctx->context);
+    glXMakeCurrent (ctx->display, current_drawable, ctx->context);
 }
 
 static void
@@ -101,11 +92,7 @@ _glx_release (void *abstract_ctx)
 {
     cairo_glx_context_t *ctx = abstract_ctx;
 
-    if (ctx->prev_context != glXGetCurrentContext () ||
-        ctx->prev_drawable != glXGetCurrentDrawable ())
-        glXMakeCurrent (ctx->display, 
-                        ctx->prev_drawable,
-                        ctx->prev_context);
+    glXMakeCurrent (ctx->display, None, None);
 }
 
 static void
@@ -199,8 +186,6 @@ cairo_glx_device_create (Display *dpy, GLXContext gl_ctx)
     ctx->display = dpy;
     ctx->dummy_window = dummy;
     ctx->context = gl_ctx;
-    ctx->prev_context = NULL;
-    ctx->prev_drawable = None;
 
     ctx->base.acquire = _glx_acquire;
     ctx->base.release = _glx_release;
commit 64f90322f73c37ac5667292949bb45b0279239d9
Author: Benjamin Otte <otte at redhat.com>
Date:   Fri Jun 18 12:40:48 2010 +0200

    gl: Refactor status handling in _cairo_gl_context_release()
    
    Previously, the code returned a status and required the caller to mangle
    this status with his own status. Now, the function takes the previous
    status ass an argument and does the mangling itself.
    
    Also contains fixes for all the callers to actually check the return
    value - which is now rather trivial as it just requires passing through
    the status variable.

diff --git a/src/cairo-gl-composite.c b/src/cairo-gl-composite.c
index 0614fa3..67059a7 100644
--- a/src/cairo-gl-composite.c
+++ b/src/cairo-gl-composite.c
@@ -57,9 +57,7 @@ _cairo_gl_create_gradient_texture (cairo_gl_surface_t *dst,
 
     status = _cairo_gl_gradient_create (ctx, pattern->n_stops, pattern->stops, gradient);
 
-    _cairo_gl_context_release (ctx);
-
-    return status;
+    return _cairo_gl_context_release (ctx, status);
 }
 
 /**
@@ -994,7 +992,7 @@ _cairo_gl_composite_begin (cairo_gl_composite_t *setup,
 
 FAIL:
     if (unlikely (status))
-        _cairo_gl_context_release (ctx);
+        status = _cairo_gl_context_release (ctx, status);
 
     return status;
 }
diff --git a/src/cairo-gl-device.c b/src/cairo-gl-device.c
index 380d393..c39489c 100644
--- a/src/cairo-gl-device.c
+++ b/src/cairo-gl-device.c
@@ -92,7 +92,7 @@ _gl_flush (void *device)
     glDisable (GL_SCISSOR_TEST);
     glDisable (GL_BLEND);
 
-    return _cairo_gl_context_release (ctx);
+    return _cairo_gl_context_release (ctx, status);
 }
 
 static void
diff --git a/src/cairo-gl-glyphs.c b/src/cairo-gl-glyphs.c
index b390f82..140a9ea 100644
--- a/src/cairo-gl-glyphs.c
+++ b/src/cairo-gl-glyphs.c
@@ -229,7 +229,7 @@ _render_glyphs (cairo_gl_surface_t	*dst,
     cairo_gl_glyph_cache_t *cache = NULL;
     cairo_gl_context_t *ctx;
     cairo_gl_composite_t setup;
-    cairo_status_t status, status_maybe_ignored;
+    cairo_status_t status;
     int i = 0;
 
     *has_component_alpha = FALSE;
@@ -306,14 +306,12 @@ _render_glyphs (cairo_gl_surface_t	*dst,
 
 	    *has_component_alpha |= cache->pattern.base.has_component_alpha;
 
-            status = _cairo_gl_composite_begin (&setup, &ctx);
-            if (unlikely (status))
-                goto FINISH;
-
             /* XXX: _cairo_gl_composite_begin() acquires the context a
              * second time. Need to refactor this loop so this doesn't happen.
              */
-            status = _cairo_gl_context_release (ctx);
+            status = _cairo_gl_composite_begin (&setup, &ctx);
+
+            status = _cairo_gl_context_release (ctx, status);
 	    if (unlikely (status))
 		goto FINISH;
 	}
@@ -351,9 +349,7 @@ _render_glyphs (cairo_gl_surface_t	*dst,
   FINISH:
     _cairo_scaled_font_thaw_cache (scaled_font);
 
-    status_maybe_ignored = _cairo_gl_context_release (ctx);
-    if (status == CAIRO_STATUS_SUCCESS)
-	status = status_maybe_ignored;
+    status = _cairo_gl_context_release (ctx, status);
 
     _cairo_gl_composite_fini (&setup);
 
diff --git a/src/cairo-gl-gradient.c b/src/cairo-gl-gradient.c
index a6cd1f8..8ebcdfe 100644
--- a/src/cairo-gl-gradient.c
+++ b/src/cairo-gl-gradient.c
@@ -258,6 +258,7 @@ void
 _cairo_gl_gradient_destroy (cairo_gl_gradient_t *gradient)
 {
     cairo_gl_context_t *ctx;
+    cairo_status_t ignore;
 
     assert (CAIRO_REFERENCE_COUNT_HAS_REFERENCE (&gradient->ref_count));
 
@@ -266,7 +267,7 @@ _cairo_gl_gradient_destroy (cairo_gl_gradient_t *gradient)
 
     if (_cairo_gl_context_acquire (gradient->device, &ctx) == CAIRO_STATUS_SUCCESS) {
         glDeleteTextures (1, &gradient->tex);
-        _cairo_gl_context_release (ctx);
+        ignore = _cairo_gl_context_release (ctx, CAIRO_STATUS_SUCCESS);
     }
 
     free (gradient);
diff --git a/src/cairo-gl-private.h b/src/cairo-gl-private.h
index 67fc864..136d48f 100644
--- a/src/cairo-gl-private.h
+++ b/src/cairo-gl-private.h
@@ -292,16 +292,18 @@ _cairo_gl_context_acquire (cairo_device_t *device,
 }
 
 static cairo_always_inline cairo_warn cairo_status_t
-_cairo_gl_context_release (cairo_gl_context_t *ctx)
+_cairo_gl_context_release (cairo_gl_context_t *ctx, cairo_status_t status)
 {
-    cairo_status_t status;
     GLenum err;
 
     err = _cairo_gl_get_error ();
-    if (unlikely (err))
-	status = _cairo_error (CAIRO_STATUS_DEVICE_ERROR);
-    else
-        status = CAIRO_STATUS_SUCCESS;
+
+    if (unlikely (err)) {
+        cairo_status_t new_status;
+	new_status = _cairo_error (CAIRO_STATUS_DEVICE_ERROR);
+        if (status == CAIRO_STATUS_SUCCESS)
+            status = new_status;
+    }
 
     cairo_device_release (&(ctx)->base);
 
diff --git a/src/cairo-gl-surface.c b/src/cairo-gl-surface.c
index 88d58e8..6755955 100644
--- a/src/cairo-gl-surface.c
+++ b/src/cairo-gl-surface.c
@@ -304,7 +304,7 @@ _cairo_gl_surface_clear (cairo_gl_surface_t  *surface,
     glClearColor (r, g, b, a);
     glClear (GL_COLOR_BUFFER_BIT);
 
-    return _cairo_gl_context_release (ctx);
+    return _cairo_gl_context_release (ctx, status);
 }
 
 cairo_surface_t *
@@ -338,21 +338,16 @@ cairo_gl_surface_create (cairo_device_t		*abstract_device,
     surface = (cairo_gl_surface_t *)
 	_cairo_gl_surface_create_scratch (ctx, content, width, height);
     if (unlikely (surface->base.status)) {
-	_cairo_gl_context_release (ctx);
+	status = _cairo_gl_context_release (ctx, surface->base.status);
 	return &surface->base;
     }
 
     /* Cairo surfaces start out initialized to transparent (black) */
     status = _cairo_gl_surface_clear (surface, CAIRO_COLOR_TRANSPARENT);
-    if (unlikely (status)) {
-	cairo_surface_destroy (&surface->base);
-	_cairo_gl_context_release (ctx);
-	return _cairo_surface_create_in_error (status);
-    }
 
-    status = _cairo_gl_context_release (ctx);
+    status = _cairo_gl_context_release (ctx, status);
     if (unlikely (status)) {
-	_cairo_gl_context_release (ctx);
+	cairo_surface_destroy (&surface->base);
 	return _cairo_surface_create_in_error (status);
     }
 
@@ -421,15 +416,19 @@ cairo_gl_surface_swapbuffers (cairo_surface_t *abstract_surface)
 
     if (! _cairo_gl_surface_is_texture (surface)) {
 	cairo_gl_context_t *ctx;
+        cairo_status_t status;
 
-        if (_cairo_gl_context_acquire (surface->base.device, &ctx))
+        status = _cairo_gl_context_acquire (surface->base.device, &ctx);
+        if (unlikely (status))
             return;
 
         cairo_surface_flush (abstract_surface);
 
 	ctx->swap_buffers (ctx, surface);
 
-        _cairo_gl_context_release (ctx);
+        status = _cairo_gl_context_release (ctx, status);
+        if (status)
+            status = _cairo_surface_set_error (abstract_surface, status);         
     }
 }
 
@@ -461,7 +460,11 @@ _cairo_gl_surface_create_similar (void		 *abstract_surface,
     surface = _cairo_gl_surface_create_scratch (ctx, content, width, height);
 
 RELEASE:
-    _cairo_gl_context_release (ctx);
+    status = _cairo_gl_context_release (ctx, status);
+    if (unlikely (status)) {
+        cairo_surface_destroy (surface);
+        return _cairo_surface_create_in_error (status);
+    }
 
     return surface;
 }
@@ -582,7 +585,7 @@ _cairo_gl_surface_draw_image (cairo_gl_surface_t *dst,
 FAIL:
     glPixelStorei (GL_UNPACK_ROW_LENGTH, 0);
 
-    _cairo_gl_context_release (ctx);
+    status = _cairo_gl_context_release (ctx, status);
 
     if (clone)
         cairo_surface_destroy (&clone->base);
@@ -651,7 +654,7 @@ _cairo_gl_surface_get_image (cairo_gl_surface_t      *surface,
     if (! _cairo_gl_surface_is_texture (surface) && GLEW_MESA_pack_invert)
 	glPixelStorei (GL_PACK_INVERT_MESA, 0);
 
-    status = _cairo_gl_context_release (ctx);
+    status = _cairo_gl_context_release (ctx, status);
     if (unlikely (status)) {
 	cairo_surface_destroy (&image->base);
 	return status;
@@ -690,7 +693,7 @@ _cairo_gl_surface_finish (void *abstract_surface)
         glDeleteFramebuffersEXT (1, &surface->fb);
     glDeleteTextures (1, &surface->tex);
 
-    return _cairo_gl_context_release (ctx);
+    return _cairo_gl_context_release (ctx, status);
 }
 
 static cairo_status_t
@@ -945,7 +948,7 @@ _cairo_gl_surface_composite (cairo_operator_t		  op,
                                        0);
     }
 
-    status = _cairo_gl_context_release (ctx);
+    status = _cairo_gl_context_release (ctx, status);
 
   CLEANUP:
     _cairo_gl_composite_fini (&setup);
@@ -1042,7 +1045,7 @@ _cairo_gl_surface_fill_rectangles (void			   *abstract_dst,
                                        0);
     }
 
-    status = _cairo_gl_context_release (ctx);
+    status = _cairo_gl_context_release (ctx, status);
 
   CLEANUP:
     _cairo_gl_composite_fini (&setup);
@@ -1146,13 +1149,15 @@ _cairo_gl_finish_unbounded_spans (void *abstract_renderer)
                                        0);
     }
 
-    return CAIRO_STATUS_SUCCESS;
+    return _cairo_gl_context_release (renderer->ctx, CAIRO_STATUS_SUCCESS);
 }
 
 static cairo_status_t
 _cairo_gl_finish_bounded_spans (void *abstract_renderer)
 {
-    return CAIRO_STATUS_SUCCESS;
+    cairo_gl_surface_span_renderer_t *renderer = abstract_renderer;
+
+    return _cairo_gl_context_release (renderer->ctx, CAIRO_STATUS_SUCCESS);
 }
 
 static void
@@ -1163,8 +1168,6 @@ _cairo_gl_surface_span_renderer_destroy (void *abstract_renderer)
     if (!renderer)
 	return;
 
-    _cairo_gl_context_release (renderer->ctx);
-
     _cairo_gl_composite_fini (&renderer->setup);
 
     free (renderer);
@@ -1287,7 +1290,7 @@ _cairo_gl_surface_flush (void *abstract_surface)
         (ctx->current_target == surface))
       _cairo_gl_composite_flush (ctx);
 
-    return _cairo_gl_context_release (ctx);
+    return _cairo_gl_context_release (ctx, status);
 }
 
 static cairo_int_status_t
commit fc3d521c121da237974e486f1b7735009764b441
Author: Benjamin Otte <otte at redhat.com>
Date:   Fri Jun 18 12:23:37 2010 +0200

    gl: Inline the check_error() function

diff --git a/src/cairo-gl-private.h b/src/cairo-gl-private.h
index e8db124..67fc864 100644
--- a/src/cairo-gl-private.h
+++ b/src/cairo-gl-private.h
@@ -234,21 +234,6 @@ _cairo_gl_get_error (void)
     return err;
 }
 
-static cairo_always_inline cairo_status_t
-_cairo_gl_check_error (void)
-{
-    cairo_status_t status;
-    GLenum err;
-
-    err = _cairo_gl_get_error ();
-    if (unlikely (err))
-	status = _cairo_error (CAIRO_STATUS_DEVICE_ERROR);
-    else
-        status = CAIRO_STATUS_SUCCESS;
-
-    return status;
-}
-
 static inline cairo_device_t *
 _cairo_gl_context_create_in_error (cairo_status_t status)
 {
@@ -310,8 +295,14 @@ static cairo_always_inline cairo_warn cairo_status_t
 _cairo_gl_context_release (cairo_gl_context_t *ctx)
 {
     cairo_status_t status;
+    GLenum err;
+
+    err = _cairo_gl_get_error ();
+    if (unlikely (err))
+	status = _cairo_error (CAIRO_STATUS_DEVICE_ERROR);
+    else
+        status = CAIRO_STATUS_SUCCESS;
 
-    status = _cairo_gl_check_error ();
     cairo_device_release (&(ctx)->base);
 
     return status;
commit 9d7fa289132650261e546b39af2371c262f46d3c
Author: Benjamin Otte <otte at redhat.com>
Date:   Fri Jun 18 12:18:05 2010 +0200

    gl: Don't assert if there used to be a GL error
    
    When acquiring the GL context, do not assert that the GL context is not
    in an error state. Do not even call _cairo_error(). Handling GL errors
    in other code is not Cairo's responsibility.
    
    Instead just clear all previous errors so we don't accidentally set
    surfaces into error states to unrelated errors.

diff --git a/src/cairo-gl-private.h b/src/cairo-gl-private.h
index 2cb1425..e8db124 100644
--- a/src/cairo-gl-private.h
+++ b/src/cairo-gl-private.h
@@ -299,7 +299,8 @@ _cairo_gl_context_acquire (cairo_device_t *device,
     if (unlikely (status))
 	return status;
 
-    assert (_cairo_gl_check_error () == CAIRO_STATUS_SUCCESS);
+    /* clear potential previous GL errors */
+    _cairo_gl_get_error ();
 
     *ctx = (cairo_gl_context_t *) device;
     return CAIRO_STATUS_SUCCESS;
commit 0f9a8cd18f14b1f23aaefe14db3b5ad07c84cff7
Author: Benjamin Otte <otte at redhat.com>
Date:   Fri Jun 18 12:17:36 2010 +0200

    gl: Add a custom glGetError() function
    
    This function clears all errors and returns the first one that happened.

diff --git a/src/cairo-gl-private.h b/src/cairo-gl-private.h
index daba72a..2cb1425 100644
--- a/src/cairo-gl-private.h
+++ b/src/cairo-gl-private.h
@@ -223,15 +223,28 @@ typedef struct _cairo_gl_composite {
 
 cairo_private extern const cairo_surface_backend_t _cairo_gl_surface_backend;
 
+static cairo_always_inline GLenum
+_cairo_gl_get_error (void)
+{
+    GLenum err = glGetError();
+
+    if (unlikely (err))
+        while (glGetError ());
+
+    return err;
+}
+
 static cairo_always_inline cairo_status_t
 _cairo_gl_check_error (void)
 {
-    cairo_status_t status = CAIRO_STATUS_SUCCESS;
+    cairo_status_t status;
     GLenum err;
 
-    while (unlikely ((err = glGetError ()))) {
+    err = _cairo_gl_get_error ();
+    if (unlikely (err))
 	status = _cairo_error (CAIRO_STATUS_DEVICE_ERROR);
-    }
+    else
+        status = CAIRO_STATUS_SUCCESS;
 
     return status;
 }
commit 8048d3aa0a11ab1c054887682b8b2a899a87da0e
Author: Benjamin Otte <otte at redhat.com>
Date:   Fri Jun 18 10:50:21 2010 +0200

    gl: Remove custom fprintf fudging on GL errors
    
    Now that we probably call _cairo_error() on every GL error, there is no
    need to use custom methods to catch those errors. The usual breakpoint
    is enough.

diff --git a/src/cairo-gl-private.h b/src/cairo-gl-private.h
index 2ae1e1e..daba72a 100644
--- a/src/cairo-gl-private.h
+++ b/src/cairo-gl-private.h
@@ -223,23 +223,18 @@ typedef struct _cairo_gl_composite {
 
 cairo_private extern const cairo_surface_backend_t _cairo_gl_surface_backend;
 
-cairo_private const char *_cairo_gl_error_to_string (GLenum err);
 static cairo_always_inline cairo_status_t
-_do_cairo_gl_check_error (const char *file, int line)
+_cairo_gl_check_error (void)
 {
     cairo_status_t status = CAIRO_STATUS_SUCCESS;
     GLenum err;
 
     while (unlikely ((err = glGetError ()))) {
-	fprintf (stderr, "%s:%d: GL error 0x%04x: %s\n",
-		 file, line, (int) err,
-		 _cairo_gl_error_to_string (err));
 	status = _cairo_error (CAIRO_STATUS_DEVICE_ERROR);
     }
 
     return status;
 }
-#define _cairo_gl_check_error() _do_cairo_gl_check_error(__FILE__, __LINE__)
 
 static inline cairo_device_t *
 _cairo_gl_context_create_in_error (cairo_status_t status)
diff --git a/src/cairo-gl-surface.c b/src/cairo-gl-surface.c
index e6aa655..88d58e8 100644
--- a/src/cairo-gl-surface.c
+++ b/src/cairo-gl-surface.c
@@ -74,26 +74,6 @@ static cairo_bool_t _cairo_surface_is_gl (cairo_surface_t *surface)
     return surface->backend == &_cairo_gl_surface_backend;
 }
 
-const char *_cairo_gl_error_to_string (GLenum err)
-{
-    switch ((int) err) {
-    case GL_NO_ERROR:
-	ASSERT_NOT_REACHED;
-	return "success";
-
-    case GL_INVALID_ENUM:      return "invalid enum";
-    case GL_INVALID_VALUE:     return "invalid value";
-    case GL_INVALID_OPERATION: return "invalid operation";
-    case GL_STACK_OVERFLOW:    return "stack overflow";
-    case GL_STACK_UNDERFLOW:   return "stack underflow";
-    case GL_OUT_OF_MEMORY:     return "out of memory";
-    case GL_INVALID_FRAMEBUFFER_OPERATION_EXT:     return "invalid framebuffer operation";
-
-    default:
-	return "unknown error";
-    }
-}
-
 cairo_bool_t
 _cairo_gl_get_image_format_and_type (pixman_format_code_t pixman_format,
 				     GLenum *internal_format, GLenum *format,
commit f4da5048bf87df0651ec83ca3f1ad6a5af7eb16d
Author: Benjamin Otte <otte at redhat.com>
Date:   Fri Jun 18 10:44:15 2010 +0200

    gl: cairo_warn about return value from cairo_gl_context_release()

diff --git a/src/cairo-gl-private.h b/src/cairo-gl-private.h
index 132fcb8..2ae1e1e 100644
--- a/src/cairo-gl-private.h
+++ b/src/cairo-gl-private.h
@@ -297,7 +297,7 @@ _cairo_gl_context_acquire (cairo_device_t *device,
     return CAIRO_STATUS_SUCCESS;
 }
 
-static cairo_always_inline cairo_status_t
+static cairo_always_inline cairo_warn cairo_status_t
 _cairo_gl_context_release (cairo_gl_context_t *ctx)
 {
     cairo_status_t status;
commit d9179f480a43c7171806e2c33804aaae70a2cfa6
Author: Benjamin Otte <otte at redhat.com>
Date:   Fri Jun 18 00:43:09 2010 +0200

    boilerplate: Reinstate glFinish() for wgl
    
    It was accidentally disabled.

diff --git a/boilerplate/cairo-boilerplate-wgl.c b/boilerplate/cairo-boilerplate-wgl.c
index 559cc11..830fc13 100644
--- a/boilerplate/cairo-boilerplate-wgl.c
+++ b/boilerplate/cairo-boilerplate-wgl.c
@@ -200,7 +200,7 @@ _cairo_boilerplate_wgl_synchronize (void *closure)
     if (cairo_device_acquire (wgltc->device))
         return;
 
-    //glFinish ();
+    glFinish ();
 
     cairo_device_release (wgltc->device);
 }


More information about the cairo-commit mailing list