Re: [PATCH spice-gtk 2/5] widget: move canvas related data in its own structure

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxx>
> ---
>  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

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
>          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

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@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]