[Spice-commits] 3 commits - common/cairo_canvas.c common/canvas_base.c server/red_worker.c

Alexander Larsson alexl at kemper.freedesktop.org
Wed Apr 28 07:03:14 PDT 2010


 common/cairo_canvas.c |   85 ++++++++++++++++++++++++++++++++++++++++++++++----
 common/canvas_base.c  |   14 +++++++-
 server/red_worker.c   |   48 ++++++++++++++++++++++------
 3 files changed, 129 insertions(+), 18 deletions(-)

New commits:
commit 42e83e384380d6459918f3b6ba2ee33251b7245d
Author: Alexander Larsson <alexl at redhat.com>
Date:   Wed Apr 28 15:43:41 2010 +0200

    Add support for SPICE_IMAGE_FLAGS_HIGH_BITS_SET

diff --git a/common/canvas_base.c b/common/canvas_base.c
index add43eb..3600a2f 100644
--- a/common/canvas_base.c
+++ b/common/canvas_base.c
@@ -1025,6 +1025,18 @@ static pixman_image_t *canvas_get_image_internal(CanvasBase *canvas, SPICE_ADDRE
         CANVAS_ERROR("invalid image type");
     }
 
+    surface_format = spice_pixman_image_get_format(surface);
+
+    if (descriptor->flags & SPICE_IMAGE_FLAGS_HIGH_BITS_SET &&
+        descriptor->type != SPICE_IMAGE_TYPE_FROM_CACHE &&
+        surface_format == PIXMAN_x8r8g8b8) {
+        spice_pixman_fill_rect_rop(surface,
+                                   0, 0,
+                                   pixman_image_get_width(surface),
+                                   pixman_image_get_height(surface),
+                                   0xff000000U, SPICE_ROP_OR);
+    }
+
     if (descriptor->flags & SPICE_IMAGE_FLAGS_CACHE_ME &&
         descriptor->type != SPICE_IMAGE_TYPE_FROM_CACHE) {
         canvas->bits_cache->ops->put(canvas->bits_cache, descriptor->id, surface);
@@ -1042,8 +1054,6 @@ static pixman_image_t *canvas_get_image_internal(CanvasBase *canvas, SPICE_ADDRE
         return NULL;
     }
 
-    surface_format = spice_pixman_image_get_format(surface);
-
     if (!saved_want_original) {
         /* Conversion to canvas format was requested, but maybe it didn't
            happen above (due to save/load to cache for instance, or
diff --git a/server/red_worker.c b/server/red_worker.c
index 042f59e..68cf230 100644
--- a/server/red_worker.c
+++ b/server/red_worker.c
@@ -400,6 +400,7 @@ typedef struct ImageItem {
     int top_down;
     int surface_id;
     int image_format;
+    uint32_t image_flags;
     uint8_t data[0];
 } ImageItem;
 
@@ -3582,22 +3583,31 @@ static int surface_format_to_image_type(uint32_t surface_format)
 }
 
 static int rgb32_data_has_alpha(int width, int height, size_t stride,
-                                uint8_t *data)
+                                uint8_t *data, int *all_set_out)
 {
-    uint32_t *line, *end;
+    uint32_t *line, *end, alpha;
+    int has_alpha;
 
+    has_alpha = FALSE;
     while (height-- > 0) {
         line = (uint32_t *)data;
         end = line + width;
         data += stride;
         while (line != end) {
-            if ((*line & 0xff000000) != 0) {
-                return 1;
+            alpha = *line & 0xff000000U;
+            if (alpha != 0) {
+                has_alpha = TRUE;
+                if (alpha != 0xff000000U) {
+                    *all_set_out = FALSE;
+                    return TRUE;
+                }
             }
             line++;
         }
     }
-    return 0;
+
+    *all_set_out = has_alpha;
+    return has_alpha;
 }
 
 static inline int red_handle_self_bitmap(RedWorker *worker, Drawable *drawable)
@@ -3609,6 +3619,7 @@ static inline int red_handle_self_bitmap(RedWorker *worker, Drawable *drawable)
     int dest_stride;
     RedSurface *surface;
     int bpp;
+    int all_set;
 
     if (!drawable->qxl_drawable->self_bitmap) {
         return TRUE;
@@ -3643,8 +3654,12 @@ static inline int red_handle_self_bitmap(RedWorker *worker, Drawable *drawable)
                  &drawable->qxl_drawable->self_bitmap_area, dest, dest_stride, TRUE);
 
     if (image->bitmap.format == SPICE_BITMAP_FMT_32BIT &&
-        rgb32_data_has_alpha(width, height, dest_stride, dest)) {
-        image->bitmap.format = SPICE_BITMAP_FMT_RGBA;
+        rgb32_data_has_alpha(width, height, dest_stride, dest, &all_set)) {
+        if (all_set) {
+            image->descriptor.flags |= SPICE_IMAGE_FLAGS_HIGH_BITS_SET;
+        } else {
+            image->bitmap.format = SPICE_BITMAP_FMT_RGBA;
+        }
     }
 
     drawable->self_bitmap = (uint8_t *)image;
@@ -4192,6 +4207,10 @@ static void localize_bitmap(RedWorker *worker, QXLPHYSICAL *in_bitmap, uint32_t
         return;
     }
 
+    if (image->descriptor.flags & QXL_IMAGE_HIGH_BITS_SET) {
+        local_image->descriptor.flags |= SPICE_IMAGE_FLAGS_HIGH_BITS_SET;
+    }
+
     switch (local_image->descriptor.type) {
     case SPICE_IMAGE_TYPE_QUIC: {
         QXLDataChunk **chanks_head;
@@ -4937,6 +4956,7 @@ static void red_add_surface_image(RedWorker *worker, int surface_id)
     SpiceRect area;
     SpiceCanvas *canvas = worker->surfaces[surface_id].context.canvas;
     RedSurface *surface;
+    int all_set;
 
     surface = &worker->surfaces[surface_id];
 
@@ -4955,6 +4975,7 @@ static void red_add_surface_image(RedWorker *worker, int surface_id)
     item->surface_id = surface_id;
     item->image_format =
         surface_format_to_image_type(surface->context.format);
+    item->image_flags = 0;
     item->pos.x = item->pos.y = 0;
     item->width = surface->context.width;
     item->height = surface->context.height;
@@ -4967,8 +4988,12 @@ static void red_add_surface_image(RedWorker *worker, int surface_id)
     canvas->ops->read_bits(canvas, item->data, stride, &area);
 
     if (item->image_format == SPICE_BITMAP_FMT_32BIT &&
-        rgb32_data_has_alpha(item->width, item->height, item->stride, item->data)) {
-        item->image_format = SPICE_BITMAP_FMT_RGBA;
+        rgb32_data_has_alpha(item->width, item->height, item->stride, item->data, &all_set)) {
+        if (all_set) {
+            item->image_flags |= SPICE_IMAGE_FLAGS_HIGH_BITS_SET;
+        } else {
+            item->image_format = SPICE_BITMAP_FMT_RGBA;
+        }
     }
 
     red_pipe_add_image_item(worker, item);
@@ -6287,6 +6312,9 @@ static void fill_bits(DisplayChannel *display_channel, QXLPHYSICAL *in_bitmap, D
     image->descriptor.id = qxl_image->descriptor.id;
     image->descriptor.type = qxl_image->descriptor.type;
     image->descriptor.flags = 0;
+    if (qxl_image->descriptor.flags & QXL_IMAGE_HIGH_BITS_SET) {
+        image->descriptor.flags |= SPICE_IMAGE_FLAGS_HIGH_BITS_SET;
+    }
     image->descriptor.width = qxl_image->descriptor.width;
     image->descriptor.height = qxl_image->descriptor.height;
 
@@ -7279,7 +7307,7 @@ static void red_send_image(DisplayChannel *display_channel, ImageItem *item)
     red_image->descriptor.height = item->height;
 
     bitmap.format = item->image_format;
-    bitmap.flags = QXL_BITMAP_DIRECT;
+    bitmap.flags = QXL_BITMAP_DIRECT | item->image_flags;
     bitmap.flags |= item->top_down ? QXL_BITMAP_TOP_DOWN : 0;
     bitmap.x = item->width;
     bitmap.y = item->height;
commit bc6029d0b0968e7baa10d933a6b7533e3ef1c954
Author: Alexander Larsson <alexl at redhat.com>
Date:   Wed Apr 28 12:15:24 2010 +0200

    Clear alpha in xRGB destination to avoid pixman setting it to 0xff
    
    Pixman sometimes sets the ignored high byte to 0xff during alpha
    blending. This is correct according to pixman specs, as the high
    byte is ignored. However its not what windows expects, and it causes
    unnecessary regions with non-zero high byte, causing us to
    send rgba data instead of rgb which compresses worse.
    
    So, we detect this and clear the high byte.

diff --git a/common/cairo_canvas.c b/common/cairo_canvas.c
index 6498142..48f14a9 100644
--- a/common/cairo_canvas.c
+++ b/common/cairo_canvas.c
@@ -292,6 +292,57 @@ static void fill_tiled_rects_rop_from_surface(SpiceCanvas *spice_canvas,
                            offset_y, rop);
 }
 
+/* Some pixman implementations of OP_OVER on xRGB32 sets
+   the high bit to 0xff (which is the right value if the
+   destination was ARGB32, and it should be ignored for
+   xRGB32. However, this fills our alpha bits with
+   data that is not wanted or expected by windows, and its
+   causing us to send rgba images rather than rgb images to
+   the client. So, we manually clear these bytes. */
+static void clear_dest_alpha(pixman_image_t *dest,
+                             int x, int y,
+                             int width, int height)
+{
+    uint32_t *data;
+    int stride;
+    int w, h;
+
+    w = pixman_image_get_width(dest);
+    h = pixman_image_get_height(dest);
+
+    if (x + width <= 0 || x >= w ||
+        y + height <= 0 || y >= h ||
+        width == 0 || height == 0) {
+        return;
+    }
+
+    if (x < 0) {
+        width += x;
+        x = 0;
+    }
+    if (x + width > w) {
+        width = w - x;
+    }
+
+    if (y < 0) {
+        height += y;
+        y = 0;
+    }
+    if (y + height > h) {
+        height = h - y;
+    }
+
+    stride = pixman_image_get_stride(dest);
+    data = (uint32_t *) (
+        (uint8_t *)pixman_image_get_data(dest) + y * stride + 4 * x);
+
+    if ((*data & 0xff000000U) == 0xff000000U) {
+        spice_pixman_fill_rect_rop(dest,
+                                   x, y, width, height,
+                                   0x00ffffff, SPICE_ROP_AND);
+    }
+}
+
 static void __blit_image(SpiceCanvas *spice_canvas,
                          pixman_region32_t *region,
                          pixman_image_t *src_image,
@@ -611,6 +662,11 @@ static void __blend_image(SpiceCanvas *spice_canvas,
                              width,
                              height);
 
+    if (canvas->base.format == SPICE_SURFACE_FMT_32_xRGB &&
+        !dest_has_alpha) {
+        clear_dest_alpha(dest, dest_x, dest_y, width, height);
+    }
+
     if (mask) {
         pixman_image_unref(mask);
     }
@@ -704,6 +760,11 @@ static void __blend_scale_image(SpiceCanvas *spice_canvas,
                              dest_x, dest_y, /* dst */
                              dest_width, dest_height);
 
+    if (canvas->base.format == SPICE_SURFACE_FMT_32_xRGB &&
+        !dest_has_alpha) {
+        clear_dest_alpha(dest, dest_x, dest_y, dest_width, dest_height);
+    }
+
     pixman_transform_init_identity(&transform);
     pixman_image_set_transform(src, &transform);
 
@@ -1035,6 +1096,11 @@ static void canvas_draw_text(SpiceCanvas *spice_canvas, SpiceRect *bbox,
                                  pos.x, pos.y,
                                  pixman_image_get_width(str_mask),
                                  pixman_image_get_height(str_mask));
+        if (canvas->base.format == SPICE_SURFACE_FMT_32_xRGB) {
+            clear_dest_alpha(canvas->image, pos.x, pos.y,
+                             pixman_image_get_width(str_mask),
+                             pixman_image_get_height(str_mask));
+        }
         pixman_image_unref(brush);
 
         pixman_image_set_clip_region32(canvas->image, NULL);
commit ac5535048c969e6fc574120325249dc823ddd3af
Author: Alexander Larsson <alexl at redhat.com>
Date:   Wed Apr 28 12:00:25 2010 +0200

    Fix line lengths and tabs

diff --git a/common/cairo_canvas.c b/common/cairo_canvas.c
index 358b036..6498142 100644
--- a/common/cairo_canvas.c
+++ b/common/cairo_canvas.c
@@ -113,7 +113,9 @@ static void copy_region(SpiceCanvas *spice_canvas,
 
             while (i >= 0) {
                 /* Copy all rects with same y in forward order */
-                for (end_line = i - 1; end_line >= 0 && dest_rects[end_line].y1 == dest_rects[i].y1; end_line--) {
+                for (end_line = i - 1;
+                     end_line >= 0 && dest_rects[end_line].y1 == dest_rects[i].y1;
+                     end_line--) {
                 }
                 for (j = end_line + 1; j <= i; j++) {
                     spice_pixman_copy_rect(canvas->image,
@@ -132,7 +134,9 @@ static void copy_region(SpiceCanvas *spice_canvas,
 
             while (i < n_rects) {
                 /* Copy all rects with same y in reverse order */
-                for (end_line = i; end_line < n_rects && dest_rects[end_line].y1 == dest_rects[i].y1; end_line++) {
+                for (end_line = i;
+                     end_line < n_rects && dest_rects[end_line].y1 == dest_rects[i].y1;
+                     end_line++) {
                 }
                 for (j = end_line - 1; j >= i; j--) {
                     spice_pixman_copy_rect(canvas->image,
@@ -956,7 +960,8 @@ static void canvas_put_image(SpiceCanvas *spice_canvas,
 }
 
 
-static void canvas_draw_text(SpiceCanvas *spice_canvas, SpiceRect *bbox, SpiceClip *clip, SpiceText *text)
+static void canvas_draw_text(SpiceCanvas *spice_canvas, SpiceRect *bbox,
+                             SpiceClip *clip, SpiceText *text)
 {
     CairoCanvas *canvas = (CairoCanvas *)spice_canvas;
     pixman_region32_t dest_region;
@@ -1038,7 +1043,8 @@ static void canvas_draw_text(SpiceCanvas *spice_canvas, SpiceRect *bbox, SpiceCl
     pixman_region32_fini(&dest_region);
 }
 
-static void canvas_read_bits(SpiceCanvas *spice_canvas, uint8_t *dest, int dest_stride, const SpiceRect *area)
+static void canvas_read_bits(SpiceCanvas *spice_canvas, uint8_t *dest,
+                             int dest_stride, const SpiceRect *area)
 {
     CairoCanvas *canvas = (CairoCanvas *)spice_canvas;
     pixman_image_t* surface;
@@ -1072,7 +1078,8 @@ static void canvas_clear(SpiceCanvas *spice_canvas)
                            0);
 }
 
-static void canvas_set_access_params(SpiceCanvas *spice_canvas, unsigned long base, unsigned long max)
+static void canvas_set_access_params(SpiceCanvas *spice_canvas,
+                                     unsigned long base, unsigned long max)
 {
 #ifdef CAIRO_CANVAS_ACCESS_TEST
     CairoCanvas *canvas = (CairoCanvas *)spice_canvas;
@@ -1188,7 +1195,7 @@ SpiceCanvas *canvas_create_for_data(int width, int height, uint32_t format,
 #elif defined(CAIRO_CANVAS_IMAGE_CACHE)
                            , SpiceImageCache *bits_cache
 #endif
-			   , SpiceImageSurfaces *surfaces
+                           , SpiceImageSurfaces *surfaces
                            , SpiceGlzDecoder *glz_decoder
 #ifndef CAIRO_CANVAS_NO_CHUNKS
                            , SpiceVirtMapping *virt_mapping


More information about the Spice-commits mailing list