qxl-abi: handle clip rect and path references.

red_parse_qxl.c starts to follow QXLPHYSICAL references and build up
data structures.  Can zap a bunch of get_virt calls in red_worker.c,
followed by cleanups.

(de-) marshaller needs updates to deal with that.  Also I suspect with
the get_virt() calls being gone we can offload more work to generated
marshaller code.

client doesn't build.

2 files changed, 206 insertions, 282 deletions

diff --git a/server/red_parse_qxl.c b/server/red_parse_qxl.c
index 37aafad..de5501b 100644
--- a/server/red_parse_qxl.c
+++ b/server/red_parse_qxl.c
@@ -16,33 +16,80 @@
    along with this program.  If not, see <http://www.gnu.org/licenses/>.
 */
 
+#include <stdbool.h>
 #include "red_common.h"
 #include "red_memslots.h"
 #include "red_parse_qxl.h"
 
-static size_t red_get_data_chunks(RedMemSlotInfo *slots, int group_id,
-                                RedDataChunk *red, SPICE_ADDRESS addr)
+#if 0
+static void hexdump_qxl(RedMemSlotInfo *slots, int group_id,
+                        SPICE_ADDRESS addr, uint8_t bytes)
+{
+    uint8_t *hex;
+    int i;
+
+    hex = (uint8_t*)get_virt(slots, addr, bytes, group_id);
+    for (i = 0; i < bytes; i++) {
+        if (0 == i % 16) {
+            fprintf(stderr, "%lx: ", addr+i);
+        }
+        if (0 == i % 4) {
+            fprintf(stderr, " ");
+        }
+        fprintf(stderr, " %02x", hex[i]);
+        if (15 == i % 16) {
+            fprintf(stderr, "\n");
+        }
+    }
+}
+#endif
+
+static uint8_t *red_linearize_chunk(RedDataChunk *head, size_t size, bool *free_chunk)
+{
+    uint8_t *data, *ptr;
+    RedDataChunk *chunk;
+    uint32_t copy;
+
+    if (head->next_chunk == NULL) {
+        ASSERT(size <= head->data_size);
+        *free_chunk = false;
+        return head->data;
+    }
+
+    ptr = data = spice_malloc(size);
+    *free_chunk = true;
+    for (chunk = head; chunk != NULL && size > 0; chunk = chunk->next_chunk) {
+        copy = MIN(chunk->data_size, size);
+        memcpy(ptr, chunk->data, copy);
+        ptr += copy;
+        size -= copy;
+    }
+    ASSERT(size == 0);
+    return data;
+}
+
+static size_t red_get_data_chunks_ptr(RedMemSlotInfo *slots, int group_id,
+                                      int memslot_id,
+                                      RedDataChunk *red, QXLDataChunk *qxl)
 {
-    QXLDataChunk *qxl;
     RedDataChunk *red_prev;
     size_t data_size = 0;
 
-    qxl = (QXLDataChunk*)get_virt(slots, addr, sizeof(*qxl), group_id);
     red->data_size = qxl->data_size;
     data_size += red->data_size;
-    validate_virt(slots, (intptr_t)qxl->data, get_memslot_id(slots, addr),
-                  red->data_size, group_id);
+    validate_virt(slots, (intptr_t)qxl->data, memslot_id, red->data_size, group_id);
     red->data = qxl->data;
     red->prev_chunk = NULL;
 
     while (qxl->next_chunk) {
         red_prev = red;
         red = spice_new(RedDataChunk, 1);
+        memslot_id = get_memslot_id(slots, qxl->next_chunk);
         qxl = (QXLDataChunk*)get_virt(slots, qxl->next_chunk, sizeof(*qxl), group_id);
+
         red->data_size = qxl->data_size;
         data_size += red->data_size;
-        validate_virt(slots, (intptr_t)qxl->data, get_memslot_id(slots, addr),
-                      red->data_size, group_id);
+        validate_virt(slots, (intptr_t)qxl->data, memslot_id, red->data_size, group_id);
         red->data = qxl->data;
         red->prev_chunk = red_prev;
         red_prev->next_chunk = red;
@@ -52,6 +99,18 @@ static size_t red_get_data_chunks(RedMemSlotInfo *slots, int group_id,
     return data_size;
 }
 
+#if 0
+static size_t red_get_data_chunks(RedMemSlotInfo *slots, int group_id,
+                                  RedDataChunk *red, SPICE_ADDRESS addr)
+{
+    QXLDataChunk *qxl;
+    int memslot_id = get_memslot_id(slots, addr);
+
+    qxl = (QXLDataChunk*)get_virt(slots, addr, sizeof(*qxl), group_id);
+    return red_get_data_chunks_ptr(slots, group_id, memslot_id, red, qxl);
+}
+#endif
+
 static void red_put_data_chunks(RedDataChunk *red)
 {
     RedDataChunk *tmp;
@@ -84,6 +143,86 @@ void red_get_rect_ptr(SpiceRect *red, QXLRect *qxl)
     red->right  = qxl->right;
 }
 
+static SpicePath *red_get_path(RedMemSlotInfo *slots, int group_id,
+                               SPICE_ADDRESS addr)
+{
+    RedDataChunk chunks;
+    QXLPathSeg *start, *end;
+    SpicePathSeg *seg;
+    uint8_t *data;
+    bool free_data;
+    QXLPath *qxl;
+    SpicePath *red;
+    size_t size;
+    int i;
+
+    qxl = (QXLPath *)get_virt(slots, addr, sizeof(*qxl), group_id);
+    size = red_get_data_chunks_ptr(slots, group_id,
+                                   get_memslot_id(slots, addr),
+                                   &chunks, &qxl->chunk);
+    data = red_linearize_chunk(&chunks, size, &free_data);
+    red_put_data_chunks(&chunks);
+
+    ASSERT(qxl->data_size == size);
+    ASSERT(sizeof(QXLPathSeg) == sizeof(SpicePathSeg)); /* FIXME */
+    red = spice_malloc(sizeof(*red) + size);
+    red->size = qxl->data_size;
+
+    start = (QXLPathSeg*)data;
+    end = (QXLPathSeg*)(data + size);
+    seg = red->segments;
+    while (start < end) {
+        seg->flags = start->flags;
+        seg->count = start->count;
+        for (i = 0; i < seg->count; i++) {
+            seg->points[i].x = start->points[i].x;
+            seg->points[i].y = start->points[i].y;
+        }
+        start = (QXLPathSeg*)(&start->points[i]);
+        seg = (SpicePathSeg*)(&seg->points[i]);
+    }
+
+    if (free_data) {
+        free(data);
+    }
+    return red;
+}
+
+static SpiceClipRects *red_get_clip_rects(RedMemSlotInfo *slots, int group_id,
+                                          SPICE_ADDRESS addr)
+{
+    RedDataChunk chunks;
+    QXLClipRects *qxl;
+    SpiceClipRects *red;
+    QXLRect *start, *end;
+    uint8_t *data;
+    bool free_data;
+    size_t size;
+    int i;
+
+    qxl = (QXLClipRects *)get_virt(slots, addr, sizeof(*qxl), group_id);
+    size = red_get_data_chunks_ptr(slots, group_id,
+                                   get_memslot_id(slots, addr),
+                                   &chunks, &qxl->chunk);
+    data = red_linearize_chunk(&chunks, size, &free_data);
+    red_put_data_chunks(&chunks);
+
+    ASSERT(qxl->num_rects * sizeof(QXLRect) == size);
+    red = spice_malloc(sizeof(*red) + qxl->num_rects * sizeof(SpiceRect));
+    red->num_rects = qxl->num_rects;
+
+    start = (QXLRect*)data;
+    end = (QXLRect*)(data + size);
+    for (i = 0; i < red->num_rects; i++) {
+        red_get_rect_ptr(red->rects + i, start++);
+    }
+
+    if (free_data) {
+        free(data);
+    }
+    return red;
+}
+
 static void red_get_brush_ptr(RedMemSlotInfo *slots, int group_id,
                               SpiceBrush *red, QXLBrush *qxl)
 {
@@ -186,7 +325,7 @@ static void red_get_rop3_ptr(RedMemSlotInfo *slots, int group_id,
 static void red_get_stroke_ptr(RedMemSlotInfo *slots, int group_id,
                                SpiceStroke *red, QXLStroke *qxl)
 {
-   red->path             = qxl->path;
+   red->path = red_get_path(slots, group_id, qxl->path);
    red->attr.flags       = qxl->attr.flags;
    red->attr.join_style  = qxl->attr.join_style;
    red->attr.end_style   = qxl->attr.end_style;
@@ -199,6 +338,11 @@ static void red_get_stroke_ptr(RedMemSlotInfo *slots, int group_id,
    red->back_mode        = qxl->back_mode;
 }
 
+static void red_put_stroke_ptr(SpiceStroke *red)
+{
+    free(red->path);
+}
+
 static void red_get_text_ptr(RedMemSlotInfo *slots, int group_id,
                                SpiceText *red, QXLText *qxl)
 {
@@ -232,7 +376,20 @@ static void red_get_clip_ptr(RedMemSlotInfo *slots, int group_id,
                              SpiceClip *red, QXLClip *qxl)
 {
     red->type = qxl->type;
-    red->data = qxl->data;
+    switch (red->type) {
+    case SPICE_CLIP_TYPE_RECTS:
+        red->rects = red_get_clip_rects(slots, group_id, qxl->data);
+        break;
+    }
+}
+
+static void red_put_clip(SpiceClip *red)
+{
+    switch (red->type) {
+    case SPICE_CLIP_TYPE_RECTS:
+        free(red->rects);
+        break;
+    }
 }
 
 void red_get_drawable(RedMemSlotInfo *slots, int group_id,
@@ -374,7 +531,12 @@ void red_get_compat_drawable(RedMemSlotInfo *slots, int group_id,
 
 void red_put_drawable(RedDrawable *red)
 {
-    /* nothing yet */
+    red_put_clip(&red->clip);
+    switch (red->type) {
+    case QXL_DRAW_STROKE:
+        red_put_stroke_ptr(&red->u.stroke);
+        break;
+    }
 }
 
 void red_get_update_cmd(RedMemSlotInfo *slots, int group_id,
diff --git a/server/red_worker.c b/server/red_worker.c
index 9ff852c..272b68d 100644
--- a/server/red_worker.c
+++ b/server/red_worker.c
@@ -2289,66 +2289,13 @@ static inline void __current_add_drawable(RedWorker *worker, Drawable *drawable,
 
 #ifdef USE_EXCLUDE_RGN
 
-static int is_equal_path(RedWorker *worker, SPICE_ADDRESS p1, SPICE_ADDRESS p2, uint32_t group_id1,
-                         uint32_t group_id2)
-{
-    QXLPath *path1;
-    QXLPath *path2;
-    QXLDataChunk *chunk1;
-    QXLDataChunk *chunk2;
-    uint8_t *data1;
-    uint8_t *data2;
-    int size;
-    int size1;
-    int size2;
-
-    ASSERT(p1 && p2);
-
-    path1 = (QXLPath *)get_virt(&worker->mem_slots, p1, sizeof(QXLPath), group_id1);
-    path2 = (QXLPath *)get_virt(&worker->mem_slots, p2, sizeof(QXLPath), group_id2);
-
-    if ((size = path1->data_size) != path2->data_size) {
+static int is_equal_path(RedWorker *worker, SpicePath *path1, SpicePath *path2)
+{
+    if (path1->size != path2->size)
         return FALSE;
-    }
-    ASSERT(size);
-
-    chunk1 = &path1->chunk;
-    size1 = chunk1->data_size;
-    data1 = chunk1->data;
-
-    chunk2 = &path2->chunk;
-    size2 = chunk2->data_size;
-    data2 = chunk2->data;
-
-    for (;;) {
-        int now = MIN(size1, size2);
-        ASSERT(now);
-        if (memcmp(data1, data2, now)) {
-            return FALSE;
-        }
-        if (!(size -= now)) {
-            return TRUE;
-        }
-        if ((size1 -= now) == 0) {
-            ASSERT(chunk1->next_chunk)
-            chunk1 = (QXLDataChunk *)get_virt(&worker->mem_slots, chunk1->next_chunk, sizeof(QXLDataChunk),
-                                              group_id1);
-            size1 = chunk1->data_size;
-            data1 = chunk1->data;
-        } else {
-            data1 += now;
-        }
-
-        if ((size2 -= now) == 0) {
-            ASSERT(chunk2->next_chunk)
-            chunk2 = (QXLDataChunk *)get_virt(&worker->mem_slots, chunk2->next_chunk, sizeof(QXLDataChunk),
-                                              group_id2);
-            size2 = chunk2->data_size;
-            data2 = chunk2->data;
-        } else {
-            data2 += now;
-        }
-    }
+    if (memcmp(path1->segments, path2->segments, path1->size) != 0)
+        return FALSE;
+    return TRUE;
 }
 
 // partial imp
@@ -2378,8 +2325,7 @@ static int is_same_geometry(RedWorker *worker, Drawable *d1, Drawable *d2)
         return is_equal_line_attr(&d1->red_drawable->u.stroke.attr,
                                   &d2->red_drawable->u.stroke.attr) &&
                is_equal_path(worker, d1->red_drawable->u.stroke.path,
-                             d2->red_drawable->u.stroke.path, d1->group_id,
-                             d2->group_id);
+                             d2->red_drawable->u.stroke.path);
     case QXL_DRAW_FILL:
         return rect_is_equal(&d1->red_drawable->bbox, &d2->red_drawable->bbox);
     default:
@@ -3445,26 +3391,12 @@ static inline int red_current_add(RedWorker *worker, Ring *ring, Drawable *drawa
 
 #endif
 
-static void add_clip_rects(RedWorker *worker, QRegion *rgn, QXLPHYSICAL data, uint32_t group_id)
+static void add_clip_rects(QRegion *rgn, SpiceClipRects *data)
 {
-    while (data) {
-        SpiceRect *now;
-        SpiceRect *end;
-        uint32_t data_size;
-
-        now = (SpiceRect *)validate_chunk(&worker->mem_slots, data, group_id, &data_size, &data);
-        end = now + data_size / sizeof(SpiceRect);
-
-        for (; now < end; now++) {
-            SpiceRect* r = (SpiceRect *)now;
+    int i;
 
-            ASSERT(now->top == r->top && now->left == r->left &&
-                   now->bottom == r->bottom && now->right == r->right);
-#ifdef PIPE_DEBUG
-            printf("TEST: DRAWABLE: RECT: %u %u %u %u\n", r->top, r->left, r->bottom, r->right);
-#endif
-            region_add(rgn, r);
-        }
+    for (i = 0; i < data->num_rects; i++) {
+        region_add(rgn, data->rects + i);
     }
 }
 
@@ -3888,9 +3820,8 @@ static inline void red_process_drawable(RedWorker *worker, RedDrawable *drawable
     if (drawable->clip.type == SPICE_CLIP_TYPE_RECTS) {
         QRegion rgn;
 
-        region_init(&
-		    rgn);
-        add_clip_rects(worker, &rgn, drawable->clip.data + SPICE_OFFSETOF(QXLClipRects, chunk), group_id);
+        region_init(&rgn);
+        add_clip_rects(&rgn, drawable->clip.rects);
         region_and(&item->tree_item.base.rgn, &rgn);
         region_destroy(&rgn);
     }
@@ -3983,38 +3914,6 @@ static inline void red_process_surface(RedWorker *worker, RedSurfaceCmd *surface
     free(surface);
 }
 
-static void localize_path(RedWorker *worker, QXLPHYSICAL *in_path, uint32_t group_id)
-{
-    QXLPath *path;
-    uint8_t *data;
-    uint32_t data_size;
-    QXLDataChunk *chunk;
-    int memslot_id = get_memslot_id(&worker->mem_slots, *in_path);
-
-    ASSERT(in_path && *in_path);
-    path = (QXLPath *)get_virt(&worker->mem_slots, *in_path, sizeof(QXLPath), group_id);
-    data = spice_malloc_n_m(1, path->data_size, sizeof(uint32_t));
-    *in_path = (QXLPHYSICAL)data;
-    *(uint32_t *)data = path->data_size;
-    data += sizeof(uint32_t);
-    chunk = &path->chunk;
-    do {
-        data_size = chunk->data_size;
-        validate_virt(&worker->mem_slots, (unsigned long)chunk->data, memslot_id, data_size, group_id);
-        memcpy(data, chunk->data, data_size);
-        data += data_size;
-        chunk = chunk->next_chunk ? (QXLDataChunk *)get_virt(&worker->mem_slots, chunk->next_chunk,
-                                                             sizeof(QXLDataChunk), group_id) : NULL;
-    } while (chunk);
-}
-
-static void unlocalize_path(QXLPHYSICAL *path)
-{
-    ASSERT(path && *path);
-    free((void *)*path);
-    *path = 0;
-}
-
 static void localize_str(RedWorker *worker, QXLPHYSICAL *in_str, uint32_t group_id)
 {
     QXLString *qxl_str = (QXLString *)get_virt(&worker->mem_slots, *in_str, sizeof(QXLString), group_id);
@@ -4054,54 +3953,6 @@ static void unlocalize_str(QXLPHYSICAL *str)
     *str = 0;
 }
 
-static void localize_clip(RedWorker *worker, SpiceClip *clip, uint32_t group_id)
-{
-    switch (clip->type) {
-    case SPICE_CLIP_TYPE_NONE:
-        return;
-    case SPICE_CLIP_TYPE_RECTS: {
-        QXLClipRects *clip_rects;
-        QXLDataChunk *chunk;
-        int memslot_id = get_memslot_id(&worker->mem_slots, clip->data);
-        uint8_t *data;
-        uint32_t data_size;
-        clip_rects = (QXLClipRects *)get_virt(&worker->mem_slots, clip->data, sizeof(QXLClipRects), group_id);
-        chunk = &clip_rects->chunk;
-        ASSERT(clip->data);
-        data = spice_malloc_n_m(clip_rects->num_rects, sizeof(SpiceRect), sizeof(uint32_t));
-        clip->data = (QXLPHYSICAL)data;
-        *(uint32_t *)(data) = clip_rects->num_rects;
-        data += sizeof(uint32_t);
-        do {
-            data_size = chunk->data_size;
-            validate_virt(&worker->mem_slots, (unsigned long)chunk->data, memslot_id, data_size, group_id);
-            memcpy(data, chunk->data, data_size);
-            data += data_size;
-            chunk = chunk->next_chunk ? (QXLDataChunk *)get_virt(&worker->mem_slots, chunk->next_chunk,
-                                                                 sizeof(QXLDataChunk), group_id) :
-                                         NULL;
-        } while (chunk);
-        break;
-    }
-    default:
-        red_printf("invalid clip type");
-    }
-}
-
-static void unlocalize_clip(SpiceClip *clip)
-{
-    switch (clip->type) {
-    case SPICE_CLIP_TYPE_NONE:
-        return;
-    case SPICE_CLIP_TYPE_RECTS:
-        free((void *)clip->data);
-        clip->data = 0;
-        break;
-    default:
-        red_printf("invalid clip type");
-    }
-}
-
 static LocalImage *alloc_local_image(RedWorker *worker)
 {
     ASSERT(worker->local_images_pos < MAX_BITMAPS);
@@ -4454,7 +4305,6 @@ static void red_draw_qxl_drawable(RedWorker *worker, Drawable *drawable)
 
     region_add(&surface->draw_dirty_region, &drawable->red_drawable->bbox);
 
-    localize_clip(worker, &clip, drawable->group_id);
     switch (drawable->red_drawable->type) {
     case QXL_DRAW_FILL: {
         SpiceFill fill = drawable->red_drawable->u.fill;
@@ -4555,12 +4405,10 @@ static void red_draw_qxl_drawable(RedWorker *worker, Drawable *drawable)
     case QXL_DRAW_STROKE: {
         SpiceStroke stroke = drawable->red_drawable->u.stroke;
         localize_brush(worker, &stroke.brush, drawable->group_id);
-        localize_path(worker, &stroke.path, drawable->group_id);
         localize_attr(worker, &stroke.attr, drawable->group_id);
         canvas->ops->draw_stroke(canvas,
                                  &drawable->red_drawable->bbox, &clip, &stroke);
         unlocalize_attr(&stroke.attr);
-        unlocalize_path(&stroke.path);
         unlocalize_brush(&stroke.brush);
         break;
     }
@@ -4579,7 +4427,6 @@ static void red_draw_qxl_drawable(RedWorker *worker, Drawable *drawable)
     default:
         red_printf("invalid type");
     }
-    unlocalize_clip(&clip);
 }
 
 #ifndef DRAW_ALL
@@ -5212,55 +5059,6 @@ static void red_add_surface_image(RedWorker *worker, int surface_id)
     display_channel_push(worker);
 }
 
-static uint8_t *chunk_linearize(RedWorker *worker, QXLDataChunk *first_chunk,
-                                int memslot_id, uint32_t group_id, size_t expected_size,
-                                int *free_chunk)
-{
-    uint8_t *data, *ptr;
-    QXLDataChunk *chunk;
-    int data_size;
-    QXLPHYSICAL next_chunk;
-
-    data_size = first_chunk->data_size;
-    next_chunk = first_chunk->next_chunk;
-
-    if (next_chunk == 0) {
-        ASSERT(expected_size <= data_size);
-        validate_virt(&worker->mem_slots, (unsigned long)first_chunk->data, memslot_id, data_size, group_id);
-        *free_chunk = FALSE;
-        return first_chunk->data;
-    }
-
-    data = spice_malloc(expected_size);
-    *free_chunk = TRUE;
-
-    ptr = data;
-    chunk = first_chunk;
-    while (chunk != NULL && expected_size > 0) {
-        data_size = chunk->data_size;
-        next_chunk = chunk->next_chunk;
-
-        if (data_size > expected_size) {
-            data_size = expected_size;
-        }
-        expected_size -= data_size;
-
-        if (data_size) {
-            validate_virt(&worker->mem_slots, (unsigned long)chunk->data, memslot_id, data_size, group_id);
-            memcpy(ptr, chunk->data, data_size);
-            ptr += data_size;
-        }
-
-        chunk = next_chunk ?
-            (QXLDataChunk *)get_virt(&worker->mem_slots, next_chunk, sizeof(QXLDataChunk),
-                                     group_id) :
-            NULL;
-    }
-    ASSERT(expected_size == 0)
-
-    return data;
-}
-
 typedef struct {
     uint32_t type;
     void *data;
@@ -5323,34 +5121,15 @@ static void add_buf_from_info(RedChannel *channel, SpiceMarshaller *m, AddBufInf
 }
 
 
-static void fill_path(DisplayChannel *display_channel, SpiceMarshaller *m,
-                      QXLPHYSICAL in_path, uint32_t group_id)
+static void fill_path(SpiceMarshaller *m, SpicePath *path)
 {
-    RedWorker *worker;
-    RedChannel *channel = &display_channel->base;
-    int memslot_id;
-    uint8_t *path_data, *p;
-    int free_path_data;
-    worker = channel->worker;
-    size_t data_size;
-    ASSERT(in_path);
-    memslot_id  = get_memslot_id(&worker->mem_slots, in_path);
-
-    QXLPath *path = (QXLPath *)get_virt(&worker->mem_slots, in_path, sizeof(QXLPath), group_id);
-
-    data_size = path->data_size;
-    spice_marshaller_add_uint32(m, data_size);
-    path_data = chunk_linearize(worker, &path->chunk,
-                                memslot_id, group_id, data_size,
-                                &free_path_data);
+    SpicePathSeg *start, *end;
 
-    p = path_data;
-    while (p < path_data + data_size) {
-        p = spice_marshall_PathSegment(m, (SpicePathSeg *)p);
-    }
-
-    if (free_path_data) {
-        free(path_data);
+    spice_marshaller_add_uint32(m, path->size);
+    start = path->segments;
+    end = (SpicePathSeg*)((uint8_t*)(path->segments) + path->size);
+    while (start < end) {
+        start = spice_marshall_PathSegment(m, start);
     }
 }
 
@@ -5375,28 +5154,13 @@ static void fill_str(DisplayChannel *display_channel, SpiceMarshaller *m,
                          str->data_size, memslot_id, group_id);
 }
 
-static inline void fill_rects_clip(RedChannel *channel, SpiceMarshaller *m, QXLPHYSICAL in_clip, uint32_t group_id)
+static inline void fill_rects_clip(SpiceMarshaller *m, SpiceClipRects *data)
 {
-    RedWorker *worker = channel->worker;
-    QXLClipRects *clip;
-    uint8_t *rects, *r;
-    int free_rects, i;
-    int memslot_id = get_memslot_id(&worker->mem_slots, in_clip);
-
-    ASSERT(in_clip);
-    clip = (QXLClipRects *)get_virt(&worker->mem_slots, in_clip, sizeof(QXLClipRects), group_id);
-    spice_marshaller_add_uint32(m, clip->num_rects);
-    rects = chunk_linearize(worker, &clip->chunk,
-                            memslot_id, group_id, clip->num_rects * sizeof(SpiceRect),
-                            &free_rects);
-
-    r = rects;
-    for (i = 0; i < clip->num_rects; i++) {
-        r = spice_marshall_Rect(m, (SpiceRect *)r);
-    }
+    int i;
 
-    if (free_rects) {
-        free(rects);
+    spice_marshaller_add_uint32(m, data->num_rects);
+    for (i = 0; i < data->num_rects; i++) {
+        spice_marshall_Rect(m, data->rects + i);
     }
 }
 
@@ -5413,7 +5177,7 @@ static void fill_base(DisplayChannel *display_channel, Drawable *drawable)
     spice_marshall_DisplayBase(channel->send_data.marshaller, &base,
                               &cliprects_data_out);
     if (cliprects_data_out) {
-        fill_rects_clip(channel, cliprects_data_out, base.clip.data, drawable->group_id);
+        fill_rects_clip(cliprects_data_out, base.clip.rects);
     }
 }
 
@@ -7369,9 +7133,7 @@ static void surface_lossy_region_update(RedWorker *worker, DisplayChannel *displ
         region_init(&clip_rgn);
         region_init(&draw_region);
         region_add(&draw_region, &drawable->bbox);
-        add_clip_rects(worker, &clip_rgn,
-                       drawable->clip.data + SPICE_OFFSETOF(QXLClipRects, chunk),
-                       item->group_id);
+        add_clip_rects(&clip_rgn, drawable->clip.rects);
         region_and(&draw_region, &clip_rgn);
         if (lossy) {
             region_or(surface_lossy_region, &draw_region);
@@ -8242,7 +8004,7 @@ static void red_send_qxl_draw_stroke(RedWorker *worker,
                           &style_out,
                           &brush_pat_out);
 
-    fill_path(display_channel, path_out, stroke.path, item->group_id);
+    fill_path(path_out, stroke.path);
     fill_attr(display_channel, style_out, &stroke.attr, item->group_id);
     if (brush_pat_out) {
         fill_bits(display_channel, brush_pat_out, stroke.brush.u.pattern.pat, item, FALSE);
@@ -9130,7 +8892,6 @@ static void red_send_image(DisplayChannel *display_channel, ImageItem *item)
     copy.base.box.right = item->pos.x + bitmap.x;
     copy.base.box.bottom = item->pos.y + bitmap.y;
     copy.base.clip.type = SPICE_CLIP_TYPE_NONE;
-    copy.base.clip.data = 0;
     copy.data.rop_descriptor = SPICE_ROPD_OP_PUT;
     copy.data.src_area.left = 0;
     copy.data.src_area.top = 0;
@@ -9241,7 +9002,7 @@ static void red_display_send_upgrade(DisplayChannel *display_channel, UpgradeIte
     copy.base.surface_id = 0;
     copy.base.box = red_drawable->bbox;
     copy.base.clip.type = SPICE_CLIP_TYPE_RECTS;
-    copy.base.clip.data = 0;
+    copy.base.clip.rects = NULL;
     copy.data = red_drawable->u.copy;
 
     SpiceMarshaller *m = channel->send_data.marshaller;
@@ -9286,7 +9047,7 @@ static void red_display_send_stream_start(DisplayChannel *display_channel, Strea
         stream_create.clip = red_drawable->clip;
     } else {
         stream_create.clip.type = SPICE_CLIP_TYPE_RECTS;
-        stream_create.clip.data = 0;
+        stream_create.clip.rects = NULL;
     }
 
     spice_marshall_msg_display_stream_create(channel->send_data.marshaller, &stream_create,
@@ -9296,8 +9057,7 @@ static void red_display_send_stream_start(DisplayChannel *display_channel, Strea
     if (stream->current) {
         RedDrawable *red_drawable = stream->current->red_drawable;
         if (red_drawable->clip.type == SPICE_CLIP_TYPE_RECTS) {
-            fill_rects_clip(channel, cliprects_data_out, stream_create.clip.data,
-                            stream->current->group_id);
+            fill_rects_clip(cliprects_data_out, stream_create.clip.rects);
         } else {
             ASSERT(red_drawable->clip.type == SPICE_CLIP_TYPE_NONE);
         }
@@ -9325,7 +9085,9 @@ static void red_display_send_stream_clip(DisplayChannel *display_channel,
 
     stream_clip.id = agent - display_channel->stream_agents;
     stream_clip.clip.type = item->clip_type;
+#if 0 /* FIXME */
     stream_clip.clip.data = 0;
+#endif
 
     spice_marshall_msg_display_stream_clip(channel->send_data.marshaller, &stream_clip,
                                            &cliprects_data_out);