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