From d0fe666a6ab1664af36a7b242d763c72f4e9f81b Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Wed, 28 Feb 2007 14:58:57 -0500 Subject: [PATCH] Revert the solid-pattern cache This reverts the following commits: 2715f2098127d04d2f9e304580a26cd0842c0e64 67e3b3c53bdd69e4d3503eed2db66953f1ac038c See this thread for an analysis of the problems it caused: http://lists.freedesktop.org/archives/cairo/2007-February/009825.html In short, a single cache for all backends doesn't work, as one thread using any backend can cause an unused xlib pattern to be evicted from the cache, and trigger an xlib call while the display is being used from another thread. Xlib is not prepared for this. --- src/cairo-color.c | 7 --- src/cairo-debug.c | 2 - src/cairo-directfb-surface.c | 12 +---- src/cairo-glitz-surface.c | 24 +--------- src/cairo-pattern.c | 91 +++--------------------------------- src/cairo-surface.c | 26 ----------- src/cairo-win32-surface.c | 13 +----- src/cairo-xcb-surface.c | 17 +------ src/cairo-xlib-surface.c | 17 +------ src/cairoint.h | 15 ------ 10 files changed, 12 insertions(+), 212 deletions(-) diff --git a/src/cairo-color.c b/src/cairo-color.c index 068816740..a34883926 100644 --- a/src/cairo-color.c +++ b/src/cairo-color.c @@ -159,10 +159,3 @@ _cairo_color_get_rgba_premultiplied (cairo_color_t *color, *blue = color->blue * color->alpha; *alpha = color->alpha; } - -cairo_bool_t -_cairo_color_equal (cairo_color_t *color_a, - cairo_color_t *color_b) -{ - return (memcmp (color_a, color_b, sizeof (cairo_color_t)) == 0); -} diff --git a/src/cairo-debug.c b/src/cairo-debug.c index dd57dd780..7c299325f 100644 --- a/src/cairo-debug.c +++ b/src/cairo-debug.c @@ -68,6 +68,4 @@ cairo_debug_reset_static_data (void) #if CAIRO_HAS_FT_FONT _cairo_ft_font_reset_static_data (); #endif - - _cairo_pattern_reset_static_data (); } diff --git a/src/cairo-directfb-surface.c b/src/cairo-directfb-surface.c index 96ee2125d..686510d61 100644 --- a/src/cairo-directfb-surface.c +++ b/src/cairo-directfb-surface.c @@ -1514,15 +1514,6 @@ _cairo_directfb_surface_show_glyphs ( void *abstract_dst, } #endif /* DFB_SHOW_GLYPHS */ -static cairo_bool_t -_cairo_directfb_surface_is_compatible (void *surface_a, - void *surface_b) -{ - cairo_directfb_surface_t *a = (cairo_directfb_surface_t*) surface_a; - cairo_directfb_surface_t *b = (cairo_directfb_surface_t*) surface_b; - - return (a->dfb == b->dfb); -} static cairo_surface_backend_t cairo_directfb_surface_backend = { CAIRO_SURFACE_TYPE_DIRECTFB, /*type*/ @@ -1573,8 +1564,7 @@ static cairo_surface_backend_t cairo_directfb_surface_backend = { #else NULL, /* show_glyphs */ #endif - NULL, /* snapshot */ - _cairo_directfb_is_compatible + NULL /* snapshot */ }; diff --git a/src/cairo-glitz-surface.c b/src/cairo-glitz-surface.c index 817eb8860..f49273917 100644 --- a/src/cairo-glitz-surface.c +++ b/src/cairo-glitz-surface.c @@ -1799,19 +1799,6 @@ _cairo_glitz_surface_scaled_glyph_fini (cairo_scaled_glyph_t *scaled_glyph, } } -static cairo_bool_t -_cairo_glitz_surface_is_compatible (void *surface_a, - void *surface_b) -{ - cairo_glitz_surface_t *a = (cairo_glitz_surface_t*) surface_a; - cairo_glitz_surface_t *b = (cairo_glitz_surface_t*) surface_b; - - glitz_drawable_t *drawable_a = glitz_surface_get_drawable (a->surface); - glitz_drawable_t *drawable_b = glitz_surface_get_drawable (b->surface); - - return (drawable_a == drawable_b); -} - #define FIXED_TO_FLOAT(f) (((glitz_float_t) (f)) / 65536) static cairo_status_t @@ -2207,16 +2194,7 @@ static const cairo_surface_backend_t cairo_glitz_surface_backend = { _cairo_glitz_surface_flush, NULL, /* mark_dirty_rectangle */ _cairo_glitz_surface_scaled_font_fini, - _cairo_glitz_surface_scaled_glyph_fini, - - NULL, /* paint */ - NULL, /* mask */ - NULL, /* stroke */ - NULL, /* fill */ - NULL, /* show_glyphs */ - - NULL, /* snapshot */ - _cairo_glitz_surface_is_compatible + _cairo_glitz_surface_scaled_glyph_fini }; static const cairo_surface_backend_t * diff --git a/src/cairo-pattern.c b/src/cairo-pattern.c index 9d999765d..1e031a803 100644 --- a/src/cairo-pattern.c +++ b/src/cairo-pattern.c @@ -1099,21 +1099,6 @@ _cairo_pattern_acquire_surface_for_gradient (cairo_gradient_pattern_t *pattern, return status; } -/* We have a small cache here, because we don't want to constantly - * recreate surfaces for simple solid colors */ -#define MAX_SURFACE_CACHE_SIZE 16 - -static struct { - struct { - cairo_color_t color; - cairo_surface_t *surface; - } cache[MAX_SURFACE_CACHE_SIZE]; - - int size; -} surface_cache; - -CAIRO_MUTEX_DECLARE (surface_cache_lock); - static cairo_int_status_t _cairo_pattern_acquire_surface_for_solid (cairo_solid_pattern_t *pattern, cairo_surface_t *dst, @@ -1124,53 +1109,12 @@ _cairo_pattern_acquire_surface_for_solid (cairo_solid_pattern_t *pattern, cairo_surface_t **out, cairo_surface_attributes_t *attribs) { - static int i = 0; - - cairo_surface_t *surface; - cairo_status_t status; - - CAIRO_MUTEX_LOCK (surface_cache_lock); - - /* Check cache first */ - if (i < surface_cache.size) - if (_cairo_color_equal (&surface_cache.cache[i].color, - &pattern->color) && - _cairo_surface_is_compatible (surface_cache.cache[i].surface, dst)) - goto DONE; - - for (i = 0; i < surface_cache.size; i++) - if (_cairo_color_equal (&surface_cache.cache[i].color, - &pattern->color) && - _cairo_surface_is_compatible (surface_cache.cache[i].surface, dst)) - goto DONE; - - /* Not cached, need to create new */ - surface = _cairo_surface_create_similar_solid (dst, - CAIRO_CONTENT_COLOR_ALPHA, - 1, 1, - &pattern->color); - if (surface->status) { - status = CAIRO_STATUS_NO_MEMORY; - - goto UNLOCK; - } - - /* Cache new */ - if (surface_cache.size < MAX_SURFACE_CACHE_SIZE) - surface_cache.size++; - else { - i = rand () % MAX_SURFACE_CACHE_SIZE; - - /* Evict old */ - cairo_surface_destroy (surface_cache.cache[i].surface); - } - - surface_cache.cache[i].color = pattern->color; - surface_cache.cache[i].surface = surface; - -DONE: - - *out = cairo_surface_reference (surface_cache.cache[i].surface); + *out = _cairo_surface_create_similar_solid (dst, + CAIRO_CONTENT_COLOR_ALPHA, + 1, 1, + &pattern->color); + if ((*out)->status) + return CAIRO_STATUS_NO_MEMORY; attribs->x_offset = attribs->y_offset = 0; cairo_matrix_init_identity (&attribs->matrix); @@ -1178,28 +1122,7 @@ DONE: attribs->filter = CAIRO_FILTER_NEAREST; attribs->acquired = FALSE; - status = CAIRO_STATUS_SUCCESS; - -UNLOCK: - - CAIRO_MUTEX_UNLOCK (surface_cache_lock); - - return status; -} - -void -_cairo_pattern_reset_static_data (void) -{ - int i; - - CAIRO_MUTEX_LOCK (surface_cache_lock); - - for (i = 0; i < surface_cache.size; i++) - cairo_surface_destroy (surface_cache.cache[i].surface); - - surface_cache.size = 0; - - CAIRO_MUTEX_UNLOCK (surface_cache_lock); + return CAIRO_STATUS_SUCCESS; } /** diff --git a/src/cairo-surface.c b/src/cairo-surface.c index 7647db707..934a667a7 100644 --- a/src/cairo-surface.c +++ b/src/cairo-surface.c @@ -1045,32 +1045,6 @@ _cairo_surface_snapshot (cairo_surface_t *surface) return _cairo_surface_fallback_snapshot (surface); } -/** - * _cairo_surface_is_compatible - * @surface_a: a #cairo_surface_t - * @surface_b: a #cairo_surface_t - * - * Find out whether the given surfaces share the same backend, - * and if so, whether they can be considered compatible. - * - * The definition of "compatible" depends on the backend. In the - * xlib case, it means the surfaces share the same display. - * - * Return value: TRUE if the surfaces are compatible. - **/ -cairo_bool_t -_cairo_surface_is_compatible (cairo_surface_t *surface_a, - cairo_surface_t *surface_b) -{ - if (surface_a->backend != surface_b->backend) - return FALSE; - - if (surface_a->backend->is_compatible) - return surface_a->backend->is_compatible (surface_a, surface_b); - - return TRUE; -} - cairo_status_t _cairo_surface_composite (cairo_operator_t op, cairo_pattern_t *src, diff --git a/src/cairo-win32-surface.c b/src/cairo-win32-surface.c index 9c4151702..20d75e7bc 100644 --- a/src/cairo-win32-surface.c +++ b/src/cairo-win32-surface.c @@ -1615,16 +1615,6 @@ _cairo_win32_surface_show_glyphs (void *surface, #undef STACK_GLYPH_SIZE -static cairo_bool_t -_cairo_win32_surface_is_compatible (void *surface_a, - void *surface_b) -{ - cairo_win32_surface_t *a = (cairo_win32_surface_t*) surface_a; - cairo_win32_surface_t *b = (cairo_win32_surface_t*) surface_b; - - return (a->dc == b->dc); -} - /** * cairo_win32_surface_create: * @hdc: the DC to create a surface for @@ -1893,8 +1883,7 @@ static const cairo_surface_backend_t cairo_win32_surface_backend = { NULL, /* fill */ _cairo_win32_surface_show_glyphs, - NULL, /* snapshot */ - _cairo_win32_surface_is_compatible + NULL /* snapshot */ }; /* diff --git a/src/cairo-xcb-surface.c b/src/cairo-xcb-surface.c index dbdcee864..2fdf8a1d5 100644 --- a/src/cairo-xcb-surface.c +++ b/src/cairo-xcb-surface.c @@ -1578,10 +1578,6 @@ _cairo_xcb_surface_show_glyphs (void *abstract_dst, int num_glyphs, cairo_scaled_font_t *scaled_font); -static cairo_bool_t -_cairo_xcb_surface_is_compatible (void *surface_a, - void *surface_b); - /* XXX: move this to the bottom of the file, XCB and Xlib */ static const cairo_surface_backend_t cairo_xcb_surface_backend = { @@ -1613,8 +1609,7 @@ static const cairo_surface_backend_t cairo_xcb_surface_backend = { NULL, /* stroke */ NULL, /* fill */ _cairo_xcb_surface_show_glyphs, - NULL, /* snapshot */ - _cairo_xcb_surface_is_compatible + NULL /* snapshot */ }; /** @@ -2456,13 +2451,3 @@ _cairo_xcb_surface_show_glyphs (void *abstract_dst, return status; } - -static cairo_bool_t -_cairo_xcb_surface_is_compatible (void *surface_a, - void *surface_b) -{ - cairo_xcb_surface_t *a = (cairo_xcb_surface_t*) surface_a; - cairo_xcb_surface_t *b = (cairo_xcb_surface_t*) surface_b; - - return _cairo_xcb_surface_same_screen (a, b); -} diff --git a/src/cairo-xlib-surface.c b/src/cairo-xlib-surface.c index baa6f23f0..6a0d3e441 100644 --- a/src/cairo-xlib-surface.c +++ b/src/cairo-xlib-surface.c @@ -74,10 +74,6 @@ _cairo_xlib_surface_show_glyphs (void *abstract_dst, int num_glyphs, cairo_scaled_font_t *scaled_font); -static cairo_bool_t -_cairo_xlib_surface_is_compatible (void *surface_a, - void *surface_b); - /* * Instead of taking two round trips for each blending request, * assume that if a particular drawable fails GetImage that it will @@ -1787,8 +1783,7 @@ static const cairo_surface_backend_t cairo_xlib_surface_backend = { NULL, /* stroke */ NULL, /* fill */ _cairo_xlib_surface_show_glyphs, - NULL, /* snapshot */ - _cairo_xlib_surface_is_compatible + NULL /* snapshot */ }; /** @@ -2918,13 +2913,3 @@ _cairo_xlib_surface_show_glyphs (void *abstract_dst, return status; } - -static cairo_bool_t -_cairo_xlib_surface_is_compatible (void *surface_a, - void *surface_b) -{ - cairo_xlib_surface_t *a = (cairo_xlib_surface_t*) surface_a; - cairo_xlib_surface_t *b = (cairo_xlib_surface_t*) surface_b; - - return _cairo_xlib_surface_same_screen (a, b); -} diff --git a/src/cairoint.h b/src/cairoint.h index 2233c9c5b..890929f01 100755 --- a/src/cairoint.h +++ b/src/cairoint.h @@ -1005,10 +1005,6 @@ struct _cairo_surface_backend { cairo_surface_t * (*snapshot) (void *surface); - - cairo_bool_t - (*is_compatible) (void *surface_a, - void *surface_b); }; typedef struct _cairo_format_masks { @@ -1579,10 +1575,6 @@ _cairo_color_get_rgba_premultiplied (cairo_color_t *color, double *blue, double *alpha); -cairo_private cairo_bool_t -_cairo_color_equal (cairo_color_t *color_a, - cairo_color_t *color_b); - /* cairo-font.c */ cairo_private void @@ -2010,10 +2002,6 @@ _cairo_surface_clone_similar (cairo_surface_t *surface, cairo_private cairo_surface_t * _cairo_surface_snapshot (cairo_surface_t *surface); -cairo_private cairo_bool_t -_cairo_surface_is_compatible (cairo_surface_t *surface_a, - cairo_surface_t *surface_b); - cairo_private unsigned int _cairo_surface_get_current_clip_serial (cairo_surface_t *surface); @@ -2445,9 +2433,6 @@ cairo_private cairo_status_t _cairo_pattern_get_extents (cairo_pattern_t *pattern, cairo_rectangle_int16_t *extents); -cairo_private void -_cairo_pattern_reset_static_data (void); - cairo_private cairo_status_t _cairo_gstate_set_antialias (cairo_gstate_t *gstate, cairo_antialias_t antialias);