xcb: Remove the surface pattern cache

Whenever a surface is used as a source, cairo-xcb creates a Picture for it and
adds it as a snapshot to the source. Additionally, there is a per-screen cache
that makes sure old Pictures are destroyed when more than 16 MiB of image data
is used.

There are two problems here:
- Normally, the Picture would be destroyed once it is no longer a snapshot for
  the original source surface. However, the cache kept it alive for longer which
  means it was using memory but couldn't actually be used any more. (The
  Pictures are found via _cairo_surface_has_snapshot)
- When a xcb surface is used as a source, we create a new Picture for it, but
  when that surface is cairo_surface_finish'd, we don't FreePicture this
  Picture. When the drawable was a window, this may later on cause a BadPicture
  error when cairo tries to explicitly free a picture that was already
  implicitly freed when its window was destroyed.

The second point caused lots of tests in the test suite to fail on an assert due
to the above-mentioned BadPicture error, but just removing the cache makes both
these problems go away.

The only downside is that the cache enforced a limit of 16 MiB of pixel data
that was used for source surfaces on the server. After this commit the Picture
will be kept alive until the snapshot is detached. If this becomes a problem
memory-wise, a new solution will have to be invented...

Signed-off-by: Uli Schlachter <psychon@znc.in>

1 files changed, 1 insertions, 85 deletions

diff --git a/src/cairo-xcb-screen.c b/src/cairo-xcb-screen.c
index 839e38e6f..b61f20b72 100644
--- a/src/cairo-xcb-screen.c
+++ b/src/cairo-xcb-screen.c
@@ -70,10 +70,6 @@ _cairo_xcb_screen_finish (cairo_xcb_screen_t *screen)
 
     _cairo_cache_fini (&screen->linear_pattern_cache);
     _cairo_cache_fini (&screen->radial_pattern_cache);
-    /* Do this one last, because the above two will cause picture_t
-     * to be finished. This results in a call to ..._remove_surface_picture
-     * which uses the surface pattern cache. */
-    _cairo_cache_fini (&screen->surface_pattern_cache);
     _cairo_freelist_fini (&screen->pattern_cache_entry_freelist);
 
     cairo_device_finish (screen->device);
@@ -83,14 +79,6 @@ _cairo_xcb_screen_finish (cairo_xcb_screen_t *screen)
 }
 
 static cairo_bool_t
-_surface_pattern_cache_entry_equal (const void *A, const void *B)
-{
-    const struct pattern_cache_entry *a = A, *b = B;
-
-    return a->key.hash == b->key.hash;
-}
-
-static cairo_bool_t
 _linear_pattern_cache_entry_equal (const void *A, const void *B)
 {
     const struct pattern_cache_entry *a = A, *b = B;
@@ -115,21 +103,6 @@ _radial_pattern_cache_entry_equal (const void *A, const void *B)
 }
 
 static void
-_surface_cache_entry_destroy (void *closure)
-{
-    struct pattern_cache_entry *entry = closure;
-
-    /* Destroy all the references to the surfaces that were generated
-     * because of the caching. This means that the cache is giving up
-     * the resources held by that surface. They are now considered as
-     * owned by whatever holds a reference to the surface. */
-    if (entry->picture->snapshot_of != NULL)
-	_cairo_surface_detach_snapshot (entry->picture);
-    cairo_surface_destroy (entry->picture);
-    _cairo_freelist_free (&entry->screen->pattern_cache_entry_freelist, entry);
-}
-
-static void
 _pattern_cache_entry_destroy (void *closure)
 {
     struct pattern_cache_entry *entry = closure;
@@ -271,21 +244,13 @@ _cairo_xcb_screen_get (xcb_connection_t *xcb_connection,
     for (i = 0; i < ARRAY_LENGTH (screen->stock_colors); i++)
 	screen->stock_colors[i] = NULL;
 
-    status = _cairo_cache_init (&screen->surface_pattern_cache,
-				_surface_pattern_cache_entry_equal,
-				NULL,
-				_surface_cache_entry_destroy,
-				16*1024*1024);
-    if (unlikely (status))
-	goto error_screen;
-
     status = _cairo_cache_init (&screen->linear_pattern_cache,
 				_linear_pattern_cache_entry_equal,
 				NULL,
 				_pattern_cache_entry_destroy,
 				16);
     if (unlikely (status))
-	goto error_surface;
+	goto error_screen;
 
     status = _cairo_cache_init (&screen->radial_pattern_cache,
 				_radial_pattern_cache_entry_equal,
@@ -304,8 +269,6 @@ unlock:
 
 error_linear:
     _cairo_cache_fini (&screen->linear_pattern_cache);
-error_surface:
-    _cairo_cache_fini (&screen->surface_pattern_cache);
 error_screen:
     CAIRO_MUTEX_UNLOCK (connection->screens_mutex);
     cairo_device_destroy (screen->device);
@@ -368,53 +331,6 @@ _cairo_xcb_screen_put_gc (cairo_xcb_screen_t *screen, int depth, xcb_gcontext_t
 }
 
 cairo_status_t
-_cairo_xcb_screen_store_surface_picture (cairo_xcb_screen_t *screen,
-					 cairo_surface_t *picture,
-					 unsigned int size)
-{
-    struct pattern_cache_entry *entry;
-    cairo_status_t status;
-
-    assert (CAIRO_MUTEX_IS_LOCKED (screen->connection->device.mutex));
-
-    entry = _cairo_freelist_alloc (&screen->pattern_cache_entry_freelist);
-    if (unlikely (entry == NULL))
-	return _cairo_error (CAIRO_STATUS_NO_MEMORY);
-
-    entry->key.hash = picture->unique_id;
-    entry->key.size = size;
-
-    entry->picture = cairo_surface_reference (picture);
-    entry->screen = screen;
-
-    status = _cairo_cache_insert (&screen->surface_pattern_cache,
-				  &entry->key);
-    if (unlikely (status)) {
-	cairo_surface_destroy (picture);
-	_cairo_freelist_free (&screen->pattern_cache_entry_freelist, entry);
-	return status;
-    }
-
-    return CAIRO_STATUS_SUCCESS;
-}
-
-void
-_cairo_xcb_screen_remove_surface_picture (cairo_xcb_screen_t *screen,
-					  cairo_surface_t *picture)
-{
-    struct pattern_cache_entry tmpl;
-    struct pattern_cache_entry *entry;
-
-    assert (CAIRO_MUTEX_IS_LOCKED (screen->connection->device.mutex));
-
-    tmpl.key.hash = picture->unique_id;
-
-    entry = _cairo_cache_lookup (&screen->surface_pattern_cache, &tmpl.key);
-    if (entry != NULL)
-	_cairo_cache_remove (&screen->surface_pattern_cache, &entry->key);
-}
-
-cairo_status_t
 _cairo_xcb_screen_store_linear_picture (cairo_xcb_screen_t *screen,
 					const cairo_linear_pattern_t *linear,
 					cairo_surface_t *picture)