From 07122f37d11eabe62bc9c81ccbf71bbe8b7a1005 Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Thu, 29 Apr 2010 15:19:18 +0100 Subject: [PATCH] surface: Convert snapshots from an array to a double-linked list. Saves the memory allocation for the array, and the overhead of maintaining the area for both insertions and more importantly deletes. --- src/cairo-recording-surface.c | 6 +- src/cairo-surface-private.h | 5 +- src/cairo-surface-snapshot.c | 10 +-- src/cairo-surface.c | 130 +++++++++++------------------ src/cairo-vg-surface.c | 8 +- src/cairo-xcb-surface-core.c | 6 +- src/cairo-xcb-surface-render.c | 10 +-- src/cairo-xcb-surface.c | 12 +-- src/cairoint.h | 2 +- src/drm/cairo-drm-i915-shader.c | 11 ++- src/drm/cairo-drm-i965-shader.c | 11 ++- src/drm/cairo-drm-intel-surface.c | 8 +- src/drm/cairo-drm-radeon-surface.c | 8 +- 13 files changed, 77 insertions(+), 150 deletions(-) diff --git a/src/cairo-recording-surface.c b/src/cairo-recording-surface.c index 7a931ee76..099fdd5dc 100644 --- a/src/cairo-recording-surface.c +++ b/src/cairo-recording-surface.c @@ -269,11 +269,7 @@ _cairo_recording_surface_acquire_source_image (void *abstract_surface, return status; } - status = _cairo_surface_attach_snapshot (&surface->base, image, NULL); - if (unlikely (status)) { - cairo_surface_destroy (image); - return status; - } + _cairo_surface_attach_snapshot (&surface->base, image, NULL); *image_out = (cairo_image_surface_t *) image; *image_extra = NULL; diff --git a/src/cairo-surface-private.h b/src/cairo-surface-private.h index e7a3da435..560929004 100644 --- a/src/cairo-surface-private.h +++ b/src/cairo-surface-private.h @@ -41,6 +41,7 @@ #include "cairo.h" #include "cairo-types-private.h" +#include "cairo-list-private.h" #include "cairo-reference-count-private.h" #include "cairo-clip-private.h" @@ -85,8 +86,8 @@ struct _cairo_surface { /* A "snapshot" surface is immutable. See _cairo_surface_snapshot. */ cairo_surface_t *snapshot_of; cairo_surface_func_t snapshot_detach; - /* current snapshots of this surface */ - cairo_array_t snapshots; + /* current snapshots of this surface, or place upon snapshot list */ + cairo_list_t snapshots; /* * Surface font options, falling back to backend's default options, diff --git a/src/cairo-surface-snapshot.c b/src/cairo-surface-snapshot.c index 4ad16aa3f..02c79c1ce 100644 --- a/src/cairo-surface-snapshot.c +++ b/src/cairo-surface-snapshot.c @@ -228,13 +228,9 @@ _cairo_surface_snapshot (cairo_surface_t *surface) snapshot->base.device_transform = surface->device_transform; snapshot->base.device_transform_inverse = surface->device_transform_inverse; - status = _cairo_surface_attach_snapshot (surface, - &snapshot->base, - _cairo_surface_snapshot_copy_on_write); - if (unlikely (status)) { - cairo_surface_destroy (&snapshot->base); - return _cairo_surface_create_in_error (status); - } + _cairo_surface_attach_snapshot (surface, + &snapshot->base, + _cairo_surface_snapshot_copy_on_write); return &snapshot->base; } diff --git a/src/cairo-surface.c b/src/cairo-surface.c index ddb1ffd8b..804f38ea8 100644 --- a/src/cairo-surface.c +++ b/src/cairo-surface.c @@ -68,11 +68,7 @@ const cairo_surface_t name = { \ 0.0, /* y_fallback_resolution */ \ NULL, /* snapshot_of */ \ NULL, /* snapshot_detach */ \ - { 0, /* size */ \ - 0, /* num_elements */ \ - 0, /* element_size */ \ - NULL, /* elements */ \ - }, /* snapshots */ \ + { NULL, NULL }, /* snapshots */ \ { CAIRO_ANTIALIAS_DEFAULT, /* antialias */ \ CAIRO_SUBPIXEL_ORDER_DEFAULT, /* subpixel_order */ \ CAIRO_HINT_STYLE_DEFAULT, /* hint_style */ \ @@ -240,30 +236,7 @@ cairo_surface_get_device (cairo_surface_t *surface) static cairo_bool_t _cairo_surface_has_snapshots (cairo_surface_t *surface) { - return surface->snapshots.num_elements != 0; -} - -static void -_cairo_surface_detach_snapshots (cairo_surface_t *surface) -{ - cairo_surface_t **snapshots; - unsigned int i; - - if (! _cairo_surface_has_snapshots (surface)) - return; - - snapshots = _cairo_array_index (&surface->snapshots, 0); - for (i = 0; i < surface->snapshots.num_elements; i++) { - snapshots[i]->snapshot_of = NULL; - - if (snapshots[i]->snapshot_detach != NULL) - snapshots[i]->snapshot_detach (snapshots[i]); - - cairo_surface_destroy (snapshots[i]); - } - surface->snapshots.num_elements = 0; - - assert (! _cairo_surface_has_snapshots (surface)); + return ! cairo_list_is_empty (&surface->snapshots); } static cairo_bool_t @@ -282,14 +255,43 @@ _cairo_surface_detach_mime_data (cairo_surface_t *surface) _cairo_user_data_array_init (&surface->mime_data); } -cairo_status_t -_cairo_surface_attach_snapshot (cairo_surface_t *surface, - cairo_surface_t *snapshot, - cairo_surface_func_t detach_func) +static void +_cairo_surface_detach_snapshots (cairo_surface_t *surface) { - cairo_status_t status; + if (surface->snapshot_of != NULL) + return; + while (_cairo_surface_has_snapshots (surface)) { + _cairo_surface_detach_snapshot (cairo_list_first_entry (&surface->snapshots, + cairo_surface_t, + snapshots)); + } +} + +void +_cairo_surface_detach_snapshot (cairo_surface_t *snapshot) +{ + assert (snapshot->snapshot_of != NULL); + + snapshot->snapshot_of = NULL; + cairo_list_del (&snapshot->snapshots); + + if (snapshot->snapshot_detach != NULL) + snapshot->snapshot_detach (snapshot); + + cairo_surface_destroy (snapshot); +} + +void +_cairo_surface_attach_snapshot (cairo_surface_t *surface, + cairo_surface_t *snapshot, + cairo_surface_func_t detach_func) +{ assert (surface != snapshot); + assert (surface->snapshot_of == NULL); + assert (snapshot->snapshot_of != surface); + + cairo_surface_reference (snapshot); if (snapshot->snapshot_of != NULL) _cairo_surface_detach_snapshot (snapshot); @@ -297,61 +299,28 @@ _cairo_surface_attach_snapshot (cairo_surface_t *surface, snapshot->snapshot_of = surface; snapshot->snapshot_detach = detach_func; - status = _cairo_array_append (&surface->snapshots, &snapshot); - if (unlikely (status)) - return status; + cairo_list_add (&snapshot->snapshots, &surface->snapshots); - cairo_surface_reference (snapshot); - return CAIRO_STATUS_SUCCESS; + assert (_cairo_surface_has_snapshot (surface, snapshot->backend) == snapshot); } cairo_surface_t * _cairo_surface_has_snapshot (cairo_surface_t *surface, const cairo_surface_backend_t *backend) { - cairo_surface_t **snapshots; - unsigned int i; + cairo_surface_t *snapshot; - /* XXX is_similar? */ - snapshots = _cairo_array_index (&surface->snapshots, 0); - for (i = 0; i < surface->snapshots.num_elements; i++) { - if (snapshots[i]->backend == backend) - return snapshots[i]; + cairo_list_foreach_entry (snapshot, cairo_surface_t, + &surface->snapshots, snapshots) + { + /* XXX is_similar? */ + if (snapshot->backend == backend) + return snapshot; } return NULL; } -void -_cairo_surface_detach_snapshot (cairo_surface_t *snapshot) -{ - cairo_surface_t *surface; - cairo_surface_t **snapshots; - unsigned int i; - - assert (snapshot->snapshot_of != NULL); - surface = snapshot->snapshot_of; - - snapshots = _cairo_array_index (&surface->snapshots, 0); - for (i = 0; i < surface->snapshots.num_elements; i++) { - if (snapshots[i] == snapshot) - break; - } - assert (i < surface->snapshots.num_elements); - - surface->snapshots.num_elements--; - memmove (&snapshots[i], - &snapshots[i+1], - sizeof (cairo_surface_t *)*(surface->snapshots.num_elements - i)); - - snapshot->snapshot_of = NULL; - - if (snapshot->snapshot_detach != NULL) - snapshot->snapshot_detach (snapshot); - - cairo_surface_destroy (snapshot); -} - static cairo_bool_t _cairo_surface_is_writable (cairo_surface_t *surface) { @@ -403,7 +372,7 @@ _cairo_surface_init (cairo_surface_t *surface, surface->x_fallback_resolution = CAIRO_SURFACE_FALLBACK_RESOLUTION_DEFAULT; surface->y_fallback_resolution = CAIRO_SURFACE_FALLBACK_RESOLUTION_DEFAULT; - _cairo_array_init (&surface->snapshots, sizeof (cairo_surface_t *)); + cairo_list_init (&surface->snapshots); surface->snapshot_of = NULL; surface->has_font_options = FALSE; @@ -630,7 +599,6 @@ cairo_surface_destroy (cairo_surface_t *surface) _cairo_user_data_array_fini (&surface->user_data); _cairo_user_data_array_fini (&surface->mime_data); - _cairo_array_fini (&surface->snapshots); free (surface); } @@ -1532,11 +1500,7 @@ _cairo_recording_surface_clone_similar (cairo_surface_t *surface, return status; } - status = _cairo_surface_attach_snapshot (src, similar, NULL); - if (unlikely (status)) { - cairo_surface_destroy (similar); - return status; - } + _cairo_surface_attach_snapshot (src, similar, NULL); src_x = src_y = 0; } diff --git a/src/cairo-vg-surface.c b/src/cairo-vg-surface.c index 169379a9d..1aa4fe242 100644 --- a/src/cairo-vg-surface.c +++ b/src/cairo-vg-surface.c @@ -982,12 +982,8 @@ _vg_setup_surface_source (cairo_vg_context_t *context, return status; } - status = _cairo_surface_attach_snapshot (spat->surface, &clone->base, - _vg_surface_remove_from_cache); - if (unlikely (status)) { - cairo_surface_destroy (&clone->base); - return status; - } + _cairo_surface_attach_snapshot (spat->surface, &clone->base, + _vg_surface_remove_from_cache); DONE: cairo_surface_destroy (&context->source->base); diff --git a/src/cairo-xcb-surface-core.c b/src/cairo-xcb-surface-core.c index d609996ef..2a903a077 100644 --- a/src/cairo-xcb-surface-core.c +++ b/src/cairo-xcb-surface-core.c @@ -405,11 +405,7 @@ _cairo_xcb_surface_pixmap (cairo_xcb_surface_t *target, if (unlikely (pixmap->base.status)) return pixmap; - status = _cairo_surface_attach_snapshot (source, &pixmap->base, NULL); - if (unlikely (status)) { - cairo_surface_destroy (&pixmap->base); - return (cairo_xcb_pixmap_t *) _cairo_surface_create_in_error (status); - } + _cairo_surface_attach_snapshot (source, &pixmap->base, NULL); if (pattern->base.extend != CAIRO_EXTEND_NONE) { if (extents->x < 0 || extents->y < 0 || diff --git a/src/cairo-xcb-surface-render.c b/src/cairo-xcb-surface-render.c index 55a3ec8c7..5ea02e86d 100644 --- a/src/cairo-xcb-surface-render.c +++ b/src/cairo-xcb-surface-render.c @@ -1367,13 +1367,9 @@ _cairo_xcb_surface_picture (cairo_xcb_surface_t *target, return (cairo_xcb_picture_t *) _cairo_surface_create_in_error (status); } - status = _cairo_surface_attach_snapshot (source, - &picture->base, - cairo_surface_finish); - if (unlikely (status)) { - cairo_surface_destroy (&picture->base); - return (cairo_xcb_picture_t *) _cairo_surface_create_in_error (status); - } + _cairo_surface_attach_snapshot (source, + &picture->base, + cairo_surface_finish); setup_picture: filter = pattern->base.filter; diff --git a/src/cairo-xcb-surface.c b/src/cairo-xcb-surface.c index 9945adcc2..f21d580fb 100644 --- a/src/cairo-xcb-surface.c +++ b/src/cairo-xcb-surface.c @@ -553,11 +553,7 @@ _cairo_xcb_surface_acquire_source_image (void *abstract_surface, if (unlikely (status)) return status; - status = _cairo_surface_attach_snapshot (&surface->base, &image->base, NULL); - if (unlikely (status)) { - cairo_surface_destroy (&image->base); - return status; - } + _cairo_surface_attach_snapshot (&surface->base, &image->base, NULL); DONE: *image_out = image; @@ -704,9 +700,9 @@ _cairo_xcb_surface_flush (void *abstract_surface) } if (status == CAIRO_STATUS_SUCCESS) { - status = _cairo_surface_attach_snapshot (&surface->base, - surface->fallback, - cairo_surface_finish); + _cairo_surface_attach_snapshot (&surface->base, + surface->fallback, + cairo_surface_finish); } } diff --git a/src/cairoint.h b/src/cairoint.h index 5deb973bf..96fd71494 100644 --- a/src/cairoint.h +++ b/src/cairoint.h @@ -2033,7 +2033,7 @@ _cairo_surface_clone_similar (cairo_surface_t *surface, cairo_private cairo_surface_t * _cairo_surface_snapshot (cairo_surface_t *surface); -cairo_private cairo_status_t +cairo_private void _cairo_surface_attach_snapshot (cairo_surface_t *surface, cairo_surface_t *snapshot, cairo_surface_func_t detach_func); diff --git a/src/drm/cairo-drm-i915-shader.c b/src/drm/cairo-drm-i915-shader.c index f216c298d..4a7c6b99b 100644 --- a/src/drm/cairo-drm-i915-shader.c +++ b/src/drm/cairo-drm-i915-shader.c @@ -1281,17 +1281,16 @@ i915_surface_clone (i915_device_t *device, if (unlikely (clone->intel.drm.base.status)) return clone->intel.drm.base.status; - status = _cairo_surface_attach_snapshot (&image->base, - &clone->intel.drm.base, - intel_surface_detach_snapshot); - if (likely (status == CAIRO_STATUS_SUCCESS)) - status = intel_snapshot_cache_insert (&device->intel, &clone->intel); - + status = intel_snapshot_cache_insert (&device->intel, &clone->intel); if (unlikely (status)) { cairo_surface_destroy (&clone->intel.drm.base); return status; } + _cairo_surface_attach_snapshot (&image->base, + &clone->intel.drm.base, + intel_surface_detach_snapshot); + *clone_out = clone; return CAIRO_STATUS_SUCCESS; } diff --git a/src/drm/cairo-drm-i965-shader.c b/src/drm/cairo-drm-i965-shader.c index d76726c72..377b478d6 100644 --- a/src/drm/cairo-drm-i965-shader.c +++ b/src/drm/cairo-drm-i965-shader.c @@ -282,17 +282,16 @@ i965_surface_clone (i965_device_t *device, return status; } - status = _cairo_surface_attach_snapshot (&image->base, - &clone->intel.drm.base, - intel_surface_detach_snapshot); - if (likely (status == CAIRO_STATUS_SUCCESS)) - status = intel_snapshot_cache_insert (&device->intel, &clone->intel); - + status = intel_snapshot_cache_insert (&device->intel, &clone->intel); if (unlikely (status)) { cairo_surface_destroy (&clone->intel.drm.base); return status; } + _cairo_surface_attach_snapshot (&image->base, + &clone->intel.drm.base, + intel_surface_detach_snapshot); + *clone_out = clone; return CAIRO_STATUS_SUCCESS; } diff --git a/src/drm/cairo-drm-intel-surface.c b/src/drm/cairo-drm-intel-surface.c index bfe181b10..9b06c2286 100644 --- a/src/drm/cairo-drm-intel-surface.c +++ b/src/drm/cairo-drm-intel-surface.c @@ -90,13 +90,7 @@ intel_surface_acquire_source_image (void *abstract_surface, if (unlikely (status)) return status; - status = _cairo_surface_attach_snapshot (&surface->drm.base, - image, - cairo_surface_destroy); - if (unlikely (status)) { - cairo_surface_destroy (image); - return status; - } + _cairo_surface_attach_snapshot (&surface->drm.base, image, cairo_surface_destroy); DONE: *image_out = (cairo_image_surface_t *) cairo_surface_reference (image); diff --git a/src/drm/cairo-drm-radeon-surface.c b/src/drm/cairo-drm-radeon-surface.c index 2a5fb105f..df95d8c6f 100644 --- a/src/drm/cairo-drm-radeon-surface.c +++ b/src/drm/cairo-drm-radeon-surface.c @@ -106,13 +106,7 @@ radeon_surface_acquire_source_image (void *abstract_surface, if (unlikely (status)) return status; - status = _cairo_surface_attach_snapshot (&surface->base.base, - image, - cairo_surface_destroy); - if (unlikely (status)) { - cairo_surface_destroy (image); - return status; - } + _cairo_surface_attach_snapshot (&surface->base.base, image, cairo_surface_destroy); DONE: *image_out = (cairo_image_surface_t *) cairo_surface_reference (image);