Thanks a lot Chris! I will take care of applying the patch. Regards, Mitch On Wed, 2013-01-30 at 09:50 +0000, Chris Wilson wrote: > Recent Cairo uses SHM transports when available, and exposes the ability > for its users to manage images shared between it and the display. > This allows us to eliminate copies, and if the architecture supports it > even to upload directly into GPU addressable memory without any copies > (all in normal system memory so we suffer no performance penalty when > applying the filters). The caveat is that we need to be aware of the > synchronize requirements, the cairo_surface_flush and > cairo_surface_mark_dirty, around access to the transport image. To > reduce the frequency of these barriers, we can subdivide the transport > image into small chunks as to satisfy individual updates and delay the > synchronisation barrier until we are forced to reuse earlier pixels. > > Note this bumps the required Cairo version to 1.12, and please be aware > that the XSHM transport requires bug fixes from cairo.git (will be > 1.12.12) > > v2: After further reflections with Mitch, we realized we can share the > transport surface between all canvases by attaching it to the common > screen. > > v3: Fix a couple of typos in insert_node() introduced when switching > variables names. > > v4: Encapsulating within an image surface rather than a subsurface was > hiding the backing SHM segment from cairo, causing it to allocate > further SHM resources to stream the upload. We should be able to use a > sub-surface here, but it is more convenient to wrap the pixels in an > image surface for rendering the filters (and conveniently masking the > callee flushes from invalidating our parent transport surface). > > Cc: Michael Natterer <mitch@xxxxxxxx> > --- > app/display/Makefile.am | 2 + > app/display/gimpdisplay-transport.c | 228 ++++++++++++++++++++++++++++++ > app/display/gimpdisplay-transport.h | 44 ++++++ > app/display/gimpdisplayshell-callbacks.c | 2 + > app/display/gimpdisplayshell-render.c | 47 +++--- > app/display/gimpdisplayshell-render.h | 14 -- > app/display/gimpdisplayshell.c | 12 -- > app/display/gimpdisplayshell.h | 3 +- > configure.ac | 2 +- > 9 files changed, 302 insertions(+), 52 deletions(-) > create mode 100644 app/display/gimpdisplay-transport.c > create mode 100644 app/display/gimpdisplay-transport.h > > diff --git a/app/display/Makefile.am b/app/display/Makefile.am > index d0b5413..3756b5b 100644 > --- a/app/display/Makefile.am > +++ b/app/display/Makefile.am > @@ -75,6 +75,8 @@ libappdisplay_a_sources = \ > gimpdisplay-foreach.h \ > gimpdisplay-handlers.c \ > gimpdisplay-handlers.h \ > + gimpdisplay-transport.c \ > + gimpdisplay-transport.h \ > gimpdisplayshell.c \ > gimpdisplayshell.h \ > gimpdisplayshell-appearance.c \ > diff --git a/app/display/gimpdisplay-transport.c b/app/display/gimpdisplay-transport.c > new file mode 100644 > index 0000000..dde86ee > --- /dev/null > +++ b/app/display/gimpdisplay-transport.c > @@ -0,0 +1,228 @@ > +/* GIMP - The GNU Image Manipulation Program > + * Copyright (C) 1995 Spencer Kimball and Peter Mattis > + * > + * This program is free software: you can redistribute it and/or modify > + * it under the terms of the GNU General Public License as published by > + * the Free Software Foundation; either version 3 of the License, or > + * (at your option) any later version. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + * > + * You should have received a copy of the GNU General Public License > + * along with this program. If not, see <http://www.gnu.org/licenses/>. > + */ > + > +#include "config.h" > + > +#include <gegl.h> > +#include <gtk/gtk.h> > + > +#include "gimpdisplay-transport.h" > + > +#define NUM_PAGES 2 > + > +struct GimpDisplayXfer { > + struct rtree { > + struct rtree_node { > + struct rtree_node *children[2]; > + struct rtree_node *next; > + int x, y, w, h; > + } root, *available; > + } rtree; /* track subregions of render_surface for efficient uploads */ > + cairo_surface_t *render_surface[NUM_PAGES]; > + int page; > +}; > + > +static struct rtree_node * > +rtree_node_create (struct rtree *rtree, struct rtree_node **prev, > + int x, int y, int w, int h) > +{ > + struct rtree_node *node; > + > + g_assert(x >= 0 && x+w <= rtree->root.w); > + g_assert(y >= 0 && y+h <= rtree->root.h); > + > + node = g_slice_alloc (sizeof (*node)); > + if (node == NULL) > + return NULL; > + > + node->children[0] = NULL; > + node->children[1] = NULL; > + node->x = x; > + node->y = y; > + node->w = w; > + node->h = h; > + > + node->next = *prev; > + *prev = node; > + > + return node; > +} > + > +static void > +rtree_node_destroy (struct rtree *rtree, struct rtree_node *node) > +{ > + int i; > + > + for (i = 0; i < 2; i++) > + { > + if (node->children[i]) > + rtree_node_destroy (rtree, node->children[i]); > + } > + > + g_slice_free (struct rtree_node, node); > +} > + > +static struct rtree_node * > +rtree_node_insert (struct rtree *rtree, struct rtree_node **prev, > + struct rtree_node *node, int w, int h) > +{ > + *prev = node->next; > + > + if (((node->w - w) | (node->h - h)) > 1) > + { > + int ww = node->w - w; > + int hh = node->h - h; > + > + if (ww >= hh) > + { > + node->children[0] = rtree_node_create (rtree, prev, > + node->x + w, node->y, > + ww, node->h); > + node->children[1] = rtree_node_create (rtree, prev, > + node->x, node->y + h, > + w, hh); > + } > + else > + { > + node->children[0] = rtree_node_create (rtree, prev, > + node->x, node->y + h, > + node->w, hh); > + node->children[1] = rtree_node_create (rtree, prev, > + node->x + w, node->y, > + ww, h); > + } > + } > + > + return node; > +} > + > +static struct rtree_node * > +rtree_insert (struct rtree *rtree, int w, int h) > +{ > + struct rtree_node *node, **prev; > + > + for (prev = &rtree->available; (node = *prev); prev = &node->next) > + if (node->w >= w && w < 2 * node->w && node->h >= h && h < 2 * node->h) > + return rtree_node_insert (rtree, prev, node, w, h); > + > + for (prev = &rtree->available; (node = *prev); prev = &node->next) > + if (node->w >= w && node->h >= h) > + return rtree_node_insert (rtree, prev, node, w, h); > + > + return NULL; > +} > + > +static void > +rtree_init (struct rtree *rtree, int w, int h) > +{ > + rtree->root.x = 0; > + rtree->root.y = 0; > + rtree->root.w = w; > + rtree->root.h = h; > + rtree->root.children[0] = NULL; > + rtree->root.children[1] = NULL; > + rtree->root.next = NULL; > + rtree->available = &rtree->root; > +} > + > +static void > +rtree_reset (struct rtree *rtree) > +{ > + int i; > + > + for (i = 0; i < 2; i++) > + { > + if (rtree->root.children[i] == NULL) > + continue; > + > + rtree_node_destroy (rtree, rtree->root.children[i]); > + rtree->root.children[i] = NULL; > + } > + > + rtree->root.next = NULL; > + rtree->available = &rtree->root; > +} > + > +static void xfer_destroy (void *data) > +{ > + GimpDisplayXfer *xfer = data; > + > + cairo_surface_destroy (xfer->render_surface); > + rtree_reset (&xfer->rtree); > + g_free (xfer); > +} > + > +GimpDisplayXfer *gimp_display_xfer_realize (GtkWidget *widget) > +{ > + GdkScreen *screen; > + GimpDisplayXfer *xfer; > + > + screen = gtk_widget_get_screen (widget); > + xfer = g_object_get_data (G_OBJECT (screen), "gimpdisplay-transport"); > + if (xfer == NULL) > + { > + gint w = GIMP_DISPLAY_RENDER_BUF_WIDTH * GIMP_DISPLAY_RENDER_MAX_SCALE; > + gint h = GIMP_DISPLAY_RENDER_BUF_HEIGHT * GIMP_DISPLAY_RENDER_MAX_SCALE; > + cairo_t *cr; > + int n; > + > + xfer = g_new (GimpDisplayXfer, 1); > + rtree_init (&xfer->rtree, w, h); > + > + cr = gdk_cairo_create (gtk_widget_get_window (widget)); > + for (n = 0; n < NUM_PAGES; n++) { > + xfer->render_surface[n] = > + cairo_surface_create_similar_image (cairo_get_target (cr), > + CAIRO_FORMAT_ARGB32, w, h); > + cairo_surface_mark_dirty (xfer->render_surface[n]); > + } > + cairo_destroy (cr); > + xfer->page = 0; > + > + g_object_set_data_full (G_OBJECT (screen), > + "gimpdisplay-transport", > + xfer, xfer_destroy); > + } > + > + return xfer; > +} > + > +cairo_surface_t * > +gimp_display_xfer_get_surface (GimpDisplayXfer *xfer, > + gint w, gint h, > + gint *src_x, gint *src_y) > +{ > + struct rtree_node *node; > + > + g_assert (w <= GIMP_DISPLAY_RENDER_BUF_WIDTH * GIMP_DISPLAY_RENDER_MAX_SCALE && > + h <= GIMP_DISPLAY_RENDER_BUF_HEIGHT * GIMP_DISPLAY_RENDER_MAX_SCALE); > + > + node = rtree_insert (&xfer->rtree, w, h); > + if (node == NULL) > + { > + xfer->page = (xfer->page + 1) % NUM_PAGES; > + cairo_surface_flush (xfer->render_surface[xfer->page]); > + rtree_reset (&xfer->rtree); > + cairo_surface_mark_dirty (xfer->render_surface[xfer->page]); /* XXX */ > + node = rtree_insert (&xfer->rtree, w, h); > + g_assert (node != NULL); > + } > + > + *src_x = node->x; > + *src_y = node->y; > + return xfer->render_surface[xfer->page]; > +} > diff --git a/app/display/gimpdisplay-transport.h b/app/display/gimpdisplay-transport.h > new file mode 100644 > index 0000000..6c7b21f > --- /dev/null > +++ b/app/display/gimpdisplay-transport.h > @@ -0,0 +1,44 @@ > +/* GIMP - The GNU Image Manipulation Program > + * Copyright (C) 1995 Spencer Kimball and Peter Mattis > + * > + * This program is free software: you can redistribute it and/or modify > + * it under the terms of the GNU General Public License as published by > + * the Free Software Foundation; either version 3 of the License, or > + * (at your option) any later version. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + * > + * You should have received a copy of the GNU General Public License > + * along with this program. If not, see <http://www.gnu.org/licenses/>. > + */ > + > +#ifndef __GIMP_DISPLAY_TRANSPORT_H__ > +#define __GIMP_DISPLAY_TRANSPORT_H__ > + > +#include <cairo.h> > + > +/* #define GIMP_DISPLAY_RENDER_ENABLE_SCALING 1 */ > + > +#define GIMP_DISPLAY_RENDER_BUF_WIDTH 256 > +#define GIMP_DISPLAY_RENDER_BUF_HEIGHT 256 > + > +#ifdef GIMP_DISPLAY_RENDER_ENABLE_SCALING > +#define GIMP_DISPLAY_RENDER_MAX_SCALE 2.0 > +#else > +#define GIMP_DISPLAY_RENDER_MAX_SCALE 1.0 > +#endif > + > +typedef struct GimpDisplayXfer GimpDisplayXfer; > + > +GimpDisplayXfer * > +gimp_display_xfer_realize (GtkWidget *widget); > + > +cairo_surface_t * > +gimp_display_xfer_get_surface (GimpDisplayXfer *xfer, > + gint w, gint h, > + gint *src_x, gint *src_y); > + > +#endif /* __GIMP_DISPLAY_TRANSPORT_H__ */ > diff --git a/app/display/gimpdisplayshell-callbacks.c b/app/display/gimpdisplayshell-callbacks.c > index e8cf44d..8a55932 100644 > --- a/app/display/gimpdisplayshell-callbacks.c > +++ b/app/display/gimpdisplayshell-callbacks.c > @@ -109,6 +109,8 @@ gimp_display_shell_canvas_realize (GtkWidget *canvas, > > /* allow shrinking */ > gtk_widget_set_size_request (GTK_WIDGET (shell), 0, 0); > + > + shell->xfer = gimp_display_xfer_realize (GTK_WIDGET(shell)); > } > > void > diff --git a/app/display/gimpdisplayshell-render.c b/app/display/gimpdisplayshell-render.c > index 67d1c50..ed4024b 100644 > --- a/app/display/gimpdisplayshell-render.c > +++ b/app/display/gimpdisplayshell-render.c > @@ -42,7 +42,6 @@ > #include "gimpdisplayshell-render.h" > #include "gimpdisplayshell-scroll.h" > > - > void > gimp_display_shell_render (GimpDisplayShell *shell, > cairo_t *cr, > @@ -59,6 +58,10 @@ gimp_display_shell_render (GimpDisplayShell *shell, > gint viewport_offset_y; > gint viewport_width; > gint viewport_height; > + cairo_surface_t *xfer; > + gint src_x, src_y; > + gint stride; > + unsigned char *data; > > g_return_if_fail (GIMP_IS_DISPLAY_SHELL (shell)); > g_return_if_fail (cr != NULL); > @@ -80,6 +83,14 @@ gimp_display_shell_render (GimpDisplayShell *shell, > &viewport_offset_y, > &viewport_width, > &viewport_height); > + xfer = gimp_display_xfer_get_surface (shell->xfer, > + w * window_scale, > + h * window_scale, > + &src_x, &src_y); > + > + stride =cairo_image_surface_get_stride (xfer); > + data = cairo_image_surface_get_data (xfer); > + data += src_y * stride + src_x * 4; > > gegl_buffer_get (buffer, > GEGL_RECTANGLE ((x + viewport_offset_x) * window_scale, > @@ -88,33 +99,21 @@ gimp_display_shell_render (GimpDisplayShell *shell, > h * window_scale), > shell->scale_x * window_scale, > babl_format ("cairo-ARGB32"), > - cairo_image_surface_get_data (shell->render_surface), > - cairo_image_surface_get_stride (shell->render_surface), > + data, stride, > GEGL_ABYSS_NONE); > > /* apply filters to the rendered projection */ > if (shell->filter_stack) > { > - cairo_surface_t *sub = shell->render_surface; > - > - if (w != GIMP_DISPLAY_RENDER_BUF_WIDTH || > - h != GIMP_DISPLAY_RENDER_BUF_HEIGHT) > - sub = cairo_image_surface_create_for_data (cairo_image_surface_get_data (sub), > - CAIRO_FORMAT_ARGB32, > - w * window_scale, > - h * window_scale, > - GIMP_DISPLAY_RENDER_BUF_WIDTH * 4); > - > - gimp_color_display_stack_convert_surface (shell->filter_stack, sub); > - > - if (sub != shell->render_surface) > - cairo_surface_destroy (sub); > + cairo_surface_t *image = > + cairo_image_surface_create_for_data (data, CAIRO_FORMAT_ARGB32, > + w * window_scale, > + h * window_scale, > + stride); > + gimp_color_display_stack_convert_surface (shell->filter_stack, image); > + cairo_surface_destroy (image); > } > > - cairo_surface_mark_dirty_rectangle (shell->render_surface, > - 0, 0, > - w * window_scale, h * window_scale); > - > #if 0 > if (shell->mask) > { > @@ -151,9 +150,9 @@ gimp_display_shell_render (GimpDisplayShell *shell, > > cairo_scale (cr, 1.0 / window_scale, 1.0 / window_scale); > > - cairo_set_source_surface (cr, shell->render_surface, > - x * window_scale, > - y * window_scale); > + cairo_set_source_surface (cr, xfer, > + (x - src_x) * window_scale, > + (y - src_y) * window_scale); > > cairo_paint (cr); > > diff --git a/app/display/gimpdisplayshell-render.h b/app/display/gimpdisplayshell-render.h > index 652cddd..a84bc21 100644 > --- a/app/display/gimpdisplayshell-render.h > +++ b/app/display/gimpdisplayshell-render.h > @@ -18,19 +18,6 @@ > #ifndef __GIMP_DISPLAY_SHELL_RENDER_H__ > #define __GIMP_DISPLAY_SHELL_RENDER_H__ > > - > -/* #define GIMP_DISPLAY_RENDER_ENABLE_SCALING 1 */ > - > -#define GIMP_DISPLAY_RENDER_BUF_WIDTH 256 > -#define GIMP_DISPLAY_RENDER_BUF_HEIGHT 256 > - > -#ifdef GIMP_DISPLAY_RENDER_ENABLE_SCALING > -#define GIMP_DISPLAY_RENDER_MAX_SCALE 2.0 > -#else > -#define GIMP_DISPLAY_RENDER_MAX_SCALE 1.0 > -#endif > - > - > void gimp_display_shell_render (GimpDisplayShell *shell, > cairo_t *cr, > gint x, > @@ -38,5 +25,4 @@ void gimp_display_shell_render (GimpDisplayShell *shell, > gint w, > gint h); > > - > #endif /* __GIMP_DISPLAY_SHELL_RENDER_H__ */ > diff --git a/app/display/gimpdisplayshell.c b/app/display/gimpdisplayshell.c > index 3e99799..5d17c7c 100644 > --- a/app/display/gimpdisplayshell.c > +++ b/app/display/gimpdisplayshell.c > @@ -299,12 +299,6 @@ gimp_display_shell_init (GimpDisplayShell *shell) > shell->x_src_dec = 1; > shell->y_src_dec = 1; > > - shell->render_surface = cairo_image_surface_create (CAIRO_FORMAT_ARGB32, > - GIMP_DISPLAY_RENDER_BUF_WIDTH * > - GIMP_DISPLAY_RENDER_MAX_SCALE, > - GIMP_DISPLAY_RENDER_BUF_HEIGHT * > - GIMP_DISPLAY_RENDER_MAX_SCALE); > - > gimp_display_shell_items_init (shell); > > shell->icon_size = 32; > @@ -783,12 +777,6 @@ gimp_display_shell_dispose (GObject *object) > shell->filter_idle_id = 0; > } > > - if (shell->render_surface) > - { > - cairo_surface_destroy (shell->render_surface); > - shell->render_surface = NULL; > - } > - > if (shell->mask_surface) > { > cairo_surface_destroy (shell->mask_surface); > diff --git a/app/display/gimpdisplayshell.h b/app/display/gimpdisplayshell.h > index af825f8..f3d68a5 100644 > --- a/app/display/gimpdisplayshell.h > +++ b/app/display/gimpdisplayshell.h > @@ -18,6 +18,7 @@ > #ifndef __GIMP_DISPLAY_SHELL_H__ > #define __GIMP_DISPLAY_SHELL_H__ > > +#include "gimpdisplay-transport.h" > > /* Apply to a float the same rounding mode used in the renderer */ > #define PROJ_ROUND(coord) ((gint) RINT (coord)) > @@ -114,7 +115,7 @@ struct _GimpDisplayShell > > GtkWidget *statusbar; /* statusbar */ > > - cairo_surface_t *render_surface; /* buffer for rendering the image */ > + GimpDisplayXfer *xfer; > cairo_surface_t *mask_surface; /* buffer for rendering the mask */ > cairo_pattern_t *checkerboard; /* checkerboard pattern */ > > diff --git a/configure.ac b/configure.ac > index 4792277..d8176f7 100644 > --- a/configure.ac > +++ b/configure.ac > @@ -46,7 +46,7 @@ m4_define([glib_required_version], [2.32.0]) > m4_define([atk_required_version], [2.2.0]) > m4_define([gtk_required_version], [2.24.10]) > m4_define([gdk_pixbuf_required_version], [2.24.1]) > -m4_define([cairo_required_version], [1.10.2]) > +m4_define([cairo_required_version], [1.12.0]) > m4_define([cairo_pdf_required_version], [1.10.2]) > m4_define([pangocairo_required_version], [1.29.4]) > m4_define([fontconfig_required_version], [2.2.0]) _______________________________________________ gimp-developer-list mailing list gimp-developer-list@xxxxxxxxx https://mail.gnome.org/mailman/listinfo/gimp-developer-list
