Re: [Gimp-developer] [PATCH] app: Use SHM transport for data transfer for display
- From: Michael Natterer <mitch gimp org>
- To: Chris Wilson <chris chris-wilson co uk>
- Cc: gimp-developer-list gnome org
- Subject: Re: [Gimp-developer] [PATCH] app: Use SHM transport for data transfer for display
- Date: Wed, 30 Jan 2013 16:55:37 +0100
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 gimp org>
> ---
> 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])
[
Date Prev][
Date Next] [
Thread Prev][Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]
