[Spice-devel] [PATCH spice-gtk 2/5] widget: move canvas related data in its own structure
Marc-André Lureau
mlureau at redhat.com
Wed May 25 10:11:45 UTC 2016
Hi
----- Original Message -----
> Hi,
>
> On Tue, 2016-05-24 at 21:31 +0200, Marc-André Lureau wrote:
> > Group canvas related data to a sub-structure.
> I am ok with groupping part of the patch, but there are some changes which
> should be documented & go in a separate patch.
> >
> > Signed-off-by: Marc-André Lureau <marcandre.lureau at gmail.com>
> > ---
> > src/spice-widget-cairo.c | 40 +++++++++++++++--------------
> > src/spice-widget-priv.h | 17 ++++++-------
> > src/spice-widget.c | 65
> > +++++++++++++++++++++++++--------------------
> > ---
> > 3 files changed, 64 insertions(+), 58 deletions(-)
> >
> > diff --git a/src/spice-widget-cairo.c b/src/spice-widget-cairo.c
> > index 5d93a9f..7f5f79d 100644
> > --- a/src/spice-widget-cairo.c
> > +++ b/src/spice-widget-cairo.c
> > @@ -27,22 +27,24 @@ int spicex_image_create(SpiceDisplay *display)
> > {
> > SpiceDisplayPrivate *d = display->priv;
> >
> > - if (d->ximage != NULL)
> > + if (d->canvas.surface != NULL)
> > return 0;
> >
> > - if (d->format == SPICE_SURFACE_FMT_16_555 ||
> > - d->format == SPICE_SURFACE_FMT_16_565) {
> > - d->convert = TRUE;
> > - d->data = g_malloc0(d->area.width * d->area.height * 4);
> > + if (d->canvas.format == SPICE_SURFACE_FMT_16_555 ||
> > + d->canvas.format == SPICE_SURFACE_FMT_16_565) {
> > + d->canvas.convert = TRUE;
> > + d->canvas.data = g_malloc0(d->area.width * d->area.height * 4);
> >
> > - d->ximage = cairo_image_surface_create_for_data
> > - (d->data, CAIRO_FORMAT_RGB24, d->area.width, d->area.height,
> > d-
> > >area.width * 4);
> > + d->canvas.surface = cairo_image_surface_create_for_data
> > + (d->canvas.data, CAIRO_FORMAT_RGB24,
> > + d->area.width, d->area.height, d->area.width * 4);
> >
> > } else {
> > - d->convert = FALSE;
> > + d->canvas.convert = FALSE;
> >
> > - d->ximage = cairo_image_surface_create_for_data
> > - (d->data, CAIRO_FORMAT_RGB24, d->width, d->height, d->stride);
> > + d->canvas.surface = cairo_image_surface_create_for_data
> > + (d->canvas.data, CAIRO_FORMAT_RGB24,
> > + d->canvas.width, d->canvas.height, d->canvas.stride);
> > }
> >
> > return 0;
> > @@ -53,10 +55,10 @@ void spicex_image_destroy(SpiceDisplay *display)
> > {
> > SpiceDisplayPrivate *d = display->priv;
> >
> > - g_clear_pointer(&d->ximage, cairo_surface_destroy);
> > - if (d->convert)
> > - g_clear_pointer(&d->data, g_free);
> > - d->convert = FALSE;
> > + g_clear_pointer(&d->canvas.surface, cairo_surface_destroy);
> > + if (d->canvas.convert)
> > + g_clear_pointer(&d->canvas.data, g_free);
> > + d->canvas.convert = FALSE;
> > }
> >
> > G_GNUC_INTERNAL
> > @@ -70,6 +72,8 @@ void spicex_draw_event(SpiceDisplay *display, cairo_t
> > *cr)
> > int ww, wh;
> > int w, h;
> >
> > + g_return_if_fail(d->canvas.surface != NULL);
> > +
> not related, it should go in a different patch
Ok, I'll actually remove it, as it is no longer needed, as you pointed out.
>
> if it is always nonnull, then ...
> > spice_display_get_scaling(display, &s, &x, &y, &w, &h);
> >
> > ww = gtk_widget_get_allocated_width(GTK_WIDGET(display));
> > @@ -85,7 +89,7 @@ void spicex_draw_event(SpiceDisplay *display, cairo_t
> > *cr)
> > /* Optionally cut out the inner area where the pixmap
> > will be drawn. This avoids 'flashing' since we're
> > not double-buffering. */
> > - if (d->ximage) {
> > + if (d->canvas.surface) {
> ... this check is not needed
Yep
> > rect.x = x;
> > rect.y = y;
> > rect.width = w;
> > @@ -103,13 +107,13 @@ void spicex_draw_event(SpiceDisplay *display, cairo_t
> > *cr)
> > cairo_fill(cr);
> >
> > /* Draw the display */
> > - if (d->ximage) {
> > + if (d->canvas.surface) {
> > cairo_translate(cr, x, y);
> > cairo_rectangle(cr, 0, 0, w, h);
> > cairo_scale(cr, s, s);
> > - if (!d->convert)
> > + if (!d->canvas.convert)
> > cairo_translate(cr, -d->area.x, -d->area.y);
> > - cairo_set_source_surface(cr, d->ximage, 0, 0);
> > + cairo_set_source_surface(cr, d->canvas.surface, 0, 0);
> > cairo_fill(cr);
> >
> > if (d->mouse_mode == SPICE_MOUSE_MODE_SERVER &&
> > diff --git a/src/spice-widget-priv.h b/src/spice-widget-priv.h
> > index 50d5ea1..266cc87 100644
> > --- a/src/spice-widget-priv.h
> > +++ b/src/spice-widget-priv.h
> > @@ -66,23 +66,22 @@ struct _SpiceDisplayPrivate {
> > /* state */
> > gboolean ready;
> > gboolean monitor_ready;
> > - enum SpiceSurfaceFmt format;
> > - gint width, height, stride;
> > - gpointer data_origin; /* the original display image
> > data
> > */
> > - gpointer data; /* converted if necessary to 32 bits */
> > -
> > + struct {
> > + enum SpiceSurfaceFmt format;
> > + gint width, height, stride;
> > + gpointer data_origin; /* the original display image
> > data */
> > + gpointer data; /* converted if necessary to 32 bits
> > */
> > + bool convert;
> > + cairo_surface_t *surface;
> > + } canvas;
> > GdkRectangle area;
> > /* window border */
> > gint ww, wh, mx, my;
> >
> > - bool convert;
> > gboolean allow_scaling;
> > gboolean only_downscale;
> > gboolean disable_inputs;
> >
> > - /* TODO: make a display object instead? */
> > - cairo_surface_t *ximage;
> > -
> > SpiceSession *session;
> > SpiceGtkSession *gtk_session;
> > SpiceMainChannel *main;
> > diff --git a/src/spice-widget.c b/src/spice-widget.c
> > index cbca5dc..f788dc1 100644
> > --- a/src/spice-widget.c
> > +++ b/src/spice-widget.c
> > @@ -178,7 +178,7 @@ static void scaling_updated(SpiceDisplay *display)
> > GdkWindow *window = gtk_widget_get_window(GTK_WIDGET(display));
> >
> > recalc_geometry(GTK_WIDGET(display));
> > - if (d->ximage && window) { /* if not yet shown */
> > + if (d->canvas.surface && window) { /* if not yet shown */
> > gtk_widget_queue_draw(GTK_WIDGET(display));
> > }
> > update_size_request(display);
> > @@ -320,7 +320,7 @@ void
> > spice_display_widget_update_monitor_area(SpiceDisplay
> > *display)
> > whole:
> > g_clear_pointer(&monitors, g_array_unref);
> > /* by display whole surface */
> > - update_area(display, 0, 0, d->width, d->height);
> > + update_area(display, 0, 0, d->canvas.width, d->canvas.height);
> > set_monitor_ready(display, true);
> > }
> >
> > @@ -1144,34 +1144,34 @@ static void recalc_geometry(GtkWidget *widget)
> > static gboolean do_color_convert(SpiceDisplay *display, GdkRectangle *r)
> > {
> > SpiceDisplayPrivate *d = display->priv;
> > - guint32 *dest = d->data;
> > - guint16 *src = d->data_origin;
> > + guint32 *dest = d->canvas.data;
> > + guint16 *src = d->canvas.data_origin;
> > gint x, y;
> >
> > g_return_val_if_fail(r != NULL, false);
> > - g_return_val_if_fail(d->format == SPICE_SURFACE_FMT_16_555 ||
> > - d->format == SPICE_SURFACE_FMT_16_565, false);
> > + g_return_val_if_fail(d->canvas.format == SPICE_SURFACE_FMT_16_555 ||
> > + d->canvas.format == SPICE_SURFACE_FMT_16_565,
> > false);
> >
> > - src += (d->stride / 2) * r->y + r->x;
> > + src += (d->canvas.stride / 2) * r->y + r->x;
> > dest += d->area.width * (r->y - d->area.y) + (r->x - d->area.x);
> >
> > - if (d->format == SPICE_SURFACE_FMT_16_555) {
> > + if (d->canvas.format == SPICE_SURFACE_FMT_16_555) {
> > for (y = 0; y < r->height; y++) {
> > for (x = 0; x < r->width; x++) {
> > dest[x] = CONVERT_0555_TO_0888(src[x]);
> > }
> >
> > dest += d->area.width;
> > - src += d->stride / 2;
> > + src += d->canvas.stride / 2;
> > }
> > - } else if (d->format == SPICE_SURFACE_FMT_16_565) {
> > + } else if (d->canvas.format == SPICE_SURFACE_FMT_16_565) {
> > for (y = 0; y < r->height; y++) {
> > for (x = 0; x < r->width; x++) {
> > dest[x] = CONVERT_0565_TO_0888(src[x]);
> > }
> >
> > dest += d->area.width;
> > - src += d->stride / 2;
> > + src += d->canvas.stride / 2;
> > }
> > }
> >
> > @@ -1224,10 +1224,9 @@ static gboolean draw_event(GtkWidget *widget,
> > cairo_t
> > *cr, gpointer data)
> > }
> > #endif
> >
> > - if (d->mark == 0 || d->data == NULL ||
> > + if (d->mark == 0 || d->canvas.data == NULL ||
> > d->area.width == 0 || d->area.height == 0)
> > return false;
> > - g_return_val_if_fail(d->ximage != NULL, false);
> not related
> >
> > spicex_draw_event(display, cr);
> > update_mouse_pointer(display);
> > @@ -1957,7 +1956,7 @@ static void update_image(SpiceDisplay *display)
> > SpiceDisplayPrivate *d = display->priv;
> >
> > spicex_image_create(display);
> > - if (d->convert)
> > + if (d->canvas.convert)
> > do_color_convert(display, &d->area);
> > }
> >
> > @@ -2306,11 +2305,15 @@ static void update_area(SpiceDisplay *display,
> > #endif
> > {
> > primary = (GdkRectangle) {
> > - .width = d->width,
> > - .height = d->height
> > + .width = d->canvas.width,
> > + .height = d->canvas.height
> > };
> > }
> >
> > + SPICE_DEBUG("update area, primary: %dx%d, area: +%d+%d %dx%d",
> > + primary.width, primary.height,
> > + d->area.x, d->area.y, d->area.width, d->area.height);
> > +
> > SPICE_DEBUG("primary: %dx%d", primary.width, primary.height);
> the debug message can be improved (not duplicated imo) in a different patch
>
Not really needed, I wonder how it slipped in.
> Thanks,
> Pavel
> > if (!gdk_rectangle_intersect(&primary, &d->area, &d->area)) {
> > SPICE_DEBUG("The monitor area is not intersecting primary
> > surface");
> > @@ -2340,11 +2343,11 @@ static void primary_create(SpiceChannel *channel,
> > gint
> > format,
> > SpiceDisplay *display = data;
> > SpiceDisplayPrivate *d = display->priv;
> >
> > - d->format = format;
> > - d->stride = stride;
> > - d->width = width;
> > - d->height = height;
> > - d->data_origin = d->data = imgdata;
> > + d->canvas.format = format;
> > + d->canvas.stride = stride;
> > + d->canvas.width = width;
> > + d->canvas.height = height;
> > + d->canvas.data_origin = d->canvas.data = imgdata;
> >
> > spice_display_widget_update_monitor_area(display);
> > }
> > @@ -2355,11 +2358,11 @@ static void primary_destroy(SpiceChannel *channel,
> > gpointer data)
> > SpiceDisplayPrivate *d = display->priv;
> >
> > spicex_image_destroy(display);
> > - d->width = 0;
> > - d->height = 0;
> > - d->stride = 0;
> > - d->data = NULL;
> > - d->data_origin = NULL;
> > + d->canvas.width = 0;
> > + d->canvas.height = 0;
> > + d->canvas.stride = 0;
> > + d->canvas.data = NULL;
> > + d->canvas.data_origin = NULL;
> > set_monitor_ready(display, false);
> > }
> >
> > @@ -2403,7 +2406,7 @@ static void invalidate(SpiceChannel *channel,
> > if (!gdk_rectangle_intersect(&rect, &d->area, &rect))
> > return;
> >
> > - if (d->convert)
> > + if (d->canvas.convert)
> > do_color_convert(display, &rect);
> >
> > spice_display_get_scaling(display, &s,
> > @@ -2874,12 +2877,12 @@ GdkPixbuf *spice_display_get_pixbuf(SpiceDisplay
> > *display)
> > int x, y;
> >
> > /* TODO: ensure d->data has been exposed? */
> > - g_return_val_if_fail(d->data != NULL, NULL);
> > + g_return_val_if_fail(d->canvas.data != NULL, NULL);
> > data = g_malloc0(d->area.width * d->area.height * 3);
> > - src = d->data;
> > + src = d->canvas.data;
> > dest = data;
> >
> > - src += d->area.y * d->stride + d->area.x * 4;
> > + src += d->area.y * d->canvas.stride + d->area.x * 4;
> > for (y = 0; y < d->area.height; ++y) {
> > for (x = 0; x < d->area.width; ++x) {
> > dest[0] = src[x * 4 + 2];
> > @@ -2887,7 +2890,7 @@ GdkPixbuf *spice_display_get_pixbuf(SpiceDisplay
> > *display)
> > dest[2] = src[x * 4 + 0];
> > dest += 3;
> > }
> > - src += d->stride;
> > + src += d->canvas.stride;
> > }
> > pixbuf = gdk_pixbuf_new_from_data(data, GDK_COLORSPACE_RGB, false,
> > 8, d->area.width,
> > d->area.height,
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel
>
More information about the Spice-devel
mailing list