[Spice-commits] 16 commits - server/dcc.cpp server/dcc.h server/dcc-send.cpp server/display-channel.cpp server/display-channel.h server/display-channel-private.h server/red-worker.cpp server/video-stream.cpp

GitLab Mirror gitlab-mirror at kemper.freedesktop.org
Sat Aug 7 06:48:12 UTC 2021


 server/dcc-send.cpp              |  219 ++++++++++++++-------------
 server/dcc.cpp                   |   72 ++++-----
 server/dcc.h                     |   14 -
 server/display-channel-private.h |   32 +---
 server/display-channel.cpp       |  306 ++++++++++++++++++++-------------------
 server/display-channel.h         |   21 +-
 server/red-worker.cpp            |    7 
 server/video-stream.cpp          |    5 
 8 files changed, 342 insertions(+), 334 deletions(-)

New commits:
commit 258fda6290e61b79ce99a8aabb70ecb22c322168
Author: Frediano Ziglio <freddy77 at gmail.com>
Date:   Wed May 26 20:03:39 2021 +0100

    Remove now useless check
    
    display_channel_surface_id_unref resets the surface so
    display_channel_surface_has_canvas will return false.
    
    Signed-off-by: Frediano Ziglio <freddy77 at gmail.com>
    Acked-by: Victor Toso <victortoso at redhat.com>

diff --git a/server/display-channel.cpp b/server/display-channel.cpp
index d1920584..1e6228af 100644
--- a/server/display-channel.cpp
+++ b/server/display-channel.cpp
@@ -255,13 +255,6 @@ static void display_channel_surface_unref(DisplayChannel *display, RedSurface *s
     delete surface;
 }
 
-/* TODO: perhaps rename to "ready" or "realized" ? */
-gboolean display_channel_surface_has_canvas(DisplayChannel *display,
-                                            uint32_t surface_id)
-{
-    return display->priv->surfaces[surface_id] != nullptr;
-}
-
 void display_channel_surface_id_unref(DisplayChannel *display, uint32_t surface_id)
 {
     display_channel_surface_unref(display, display->priv->surfaces[surface_id]);
diff --git a/server/display-channel.h b/server/display-channel.h
index 13f3b8d1..3319cf22 100644
--- a/server/display-channel.h
+++ b/server/display-channel.h
@@ -140,7 +140,6 @@ void display_channel_set_monitors_config_to_primary(DisplayChannel *display);
 void display_channel_push_monitors_config(DisplayChannel *display);
 
 RedSurface *display_channel_validate_surface(DisplayChannel *display, uint32_t surface_id);
-gboolean display_channel_surface_has_canvas(DisplayChannel *display, uint32_t surface_id);
 void display_channel_reset_image_cache(DisplayChannel *self);
 
 void display_channel_debug_oom(DisplayChannel *display, const char *msg);
diff --git a/server/red-worker.cpp b/server/red-worker.cpp
index b5abb82d..2696c305 100644
--- a/server/red-worker.cpp
+++ b/server/red-worker.cpp
@@ -450,11 +450,6 @@ static void destroy_primary_surface(RedWorker *worker, uint32_t surface_id)
     display_channel_destroy_surface_wait(display, 0);
     display_channel_surface_id_unref(display, 0);
 
-    /* FIXME: accessing private data only for warning purposes...
-    spice_warn_if_fail(ring_is_empty(&display->streams));
-    */
-    spice_warn_if_fail(!display_channel_surface_has_canvas(display, surface_id));
-
     worker->cursor_channel->reset();
 }
 
commit d8bca15f2b0ab0a25b739a26f6aef278e0d0064a
Author: Frediano Ziglio <freddy77 at gmail.com>
Date:   Sun May 23 22:13:56 2021 +0100

    Allows surfaces to be updated without having to wait
    
    Store pointers to surface object in "surfaces" and allows to
    have different surfaces with same ID in memory.
    The surface was keep "busy" if there was pending drawing around.
    
    Consider the following case:
     1- receive drawing command
     2- queue command on DCCs
     3- destroy surface
     4- send draw
    Previously at point 4) you would have to use a surface from
    "surfaces" which was destroyed, that is we would have to maintain
    the pointer (and canvas) to the surface until reference counter
    was 0.
    
    However consider this case:
     1- receive drawing command
     2- queue command on DCCs
     3- destroy surface
     4- create surface
     5- send draw
    What would happen in point 4) ?
    We could not change the surface as it will be used by point 5).
    To avoid this the code attempts to wait the commands to release the
    surface. However this can be an issue, you can't force the clients
    to receive pending data if network is slow.
    
    So this patch change this allowing to create surfaces while the old
    version will still be used.
    
    This is also more clean from the reference pointer prospective,
    as the reference is increased for a specific surface.
    
    Note that now instead of checking for canvas to not be NULL a
    simple check for surface pointer is enough.
    
    Signed-off-by: Frediano Ziglio <freddy77 at gmail.com>
    Acked-by: Victor Toso <victortoso at redhat.com>

diff --git a/server/dcc-send.cpp b/server/dcc-send.cpp
index 996e491f..d8a737e2 100644
--- a/server/dcc-send.cpp
+++ b/server/dcc-send.cpp
@@ -117,11 +117,23 @@ static bool is_surface_area_lossy(DisplayChannelClient *dcc, RedSurface *surface
     return TRUE;
 }
 
+static RedSurface*
+get_dependent_surface(const Drawable *drawable, uint32_t surface_id)
+{
+    for (auto surface : drawable->surface_deps) {
+        if (surface && surface->id == surface_id) {
+            return surface;
+        }
+    }
+    return nullptr;
+}
+
 /* returns if the bitmap was already sent lossy to the client. If the bitmap hasn't been sent yet
    to the client, returns false. "area" is for surfaces. If area = NULL,
    all the surface is considered. out_lossy_data will hold info about the bitmap, and its lossy
    area in case it is lossy and part of a surface. */
-static bool is_bitmap_lossy(DisplayChannelClient *dcc, SpiceImage *image, SpiceRect *area,
+static bool is_bitmap_lossy(DisplayChannelClient *dcc, const Drawable *drawable,
+                            const SpiceImage *image, SpiceRect *area,
                             BitmapData *out_data)
 {
     out_data->type = BITMAP_DATA_TYPE_BITMAP;
@@ -144,7 +156,8 @@ static bool is_bitmap_lossy(DisplayChannelClient *dcc, SpiceImage *image, SpiceR
         return FALSE;
     }
 
-    auto surface = display_channel_validate_surface(DCC_TO_DC(dcc), image->u.surface.surface_id);
+    // the surface should be in the dependent list
+    auto surface = get_dependent_surface(drawable, image->u.surface.surface_id);
     if (!surface) {
         return false;
     }
@@ -156,11 +169,12 @@ static bool is_bitmap_lossy(DisplayChannelClient *dcc, SpiceImage *image, SpiceR
                                  area, &out_data->lossy_rect);
 }
 
-static bool is_brush_lossy(DisplayChannelClient *dcc, SpiceBrush *brush,
+static bool is_brush_lossy(DisplayChannelClient *dcc,
+                           const Drawable *drawable, SpiceBrush *brush,
                            BitmapData *out_data)
 {
     if (brush->type == SPICE_BRUSH_TYPE_PATTERN) {
-        return is_bitmap_lossy(dcc, brush->u.pattern.pat, nullptr,
+        return is_bitmap_lossy(dcc, drawable, brush->u.pattern.pat, nullptr,
                                out_data);
     }
     out_data->type = BITMAP_DATA_TYPE_INVALID;
@@ -394,7 +408,7 @@ static FillBitsType fill_bits(DisplayChannelClient *dcc, SpiceMarshaller *m,
         RedSurface *surface;
 
         auto surface_id = simage->u.surface.surface_id;
-        surface = display_channel_validate_surface(display, surface_id);
+        surface = get_dependent_surface(drawable, surface_id);
         if (!surface) {
             spice_warning("Invalid surface in SPICE_IMAGE_TYPE_SURFACE");
             pthread_mutex_unlock(&dcc->priv->pixmap_cache->lock);
@@ -811,7 +825,7 @@ static void red_lossy_marshall_qxl_draw_fill(DisplayChannelClient *dcc,
                            (rop & SPICE_ROPD_OP_AND) ||
                            (rop & SPICE_ROPD_OP_XOR));
 
-    brush_is_lossy = is_brush_lossy(dcc, &drawable->u.fill.brush,
+    brush_is_lossy = is_brush_lossy(dcc, item, &drawable->u.fill.brush,
                                     &brush_bitmap_data);
     if (!dest_allowed_lossy) {
         dest_is_lossy = is_surface_area_lossy(dcc, item->surface, &drawable->bbox,
@@ -899,11 +913,11 @@ static void red_lossy_marshall_qxl_draw_opaque(DisplayChannelClient *dcc,
                           (rop & SPICE_ROPD_OP_AND) ||
                           (rop & SPICE_ROPD_OP_XOR));
 
-    brush_is_lossy = is_brush_lossy(dcc, &drawable->u.opaque.brush,
+    brush_is_lossy = is_brush_lossy(dcc, item, &drawable->u.opaque.brush,
                                     &brush_bitmap_data);
 
     if (!src_allowed_lossy) {
-        src_is_lossy = is_bitmap_lossy(dcc, drawable->u.opaque.src_bitmap,
+        src_is_lossy = is_bitmap_lossy(dcc, item, drawable->u.opaque.src_bitmap,
                                        &drawable->u.opaque.src_area,
                                        &src_bitmap_data);
     }
@@ -980,7 +994,7 @@ static void red_lossy_marshall_qxl_draw_copy(DisplayChannelClient *dcc,
     BitmapData src_bitmap_data;
     FillBitsType src_send_type;
 
-    src_is_lossy = is_bitmap_lossy(dcc, drawable->u.copy.src_bitmap,
+    src_is_lossy = is_bitmap_lossy(dcc, item, drawable->u.copy.src_bitmap,
                                    &drawable->u.copy.src_area, &src_bitmap_data);
 
     src_send_type = red_marshall_qxl_draw_copy(dcc, base_marshaller, dpi, TRUE);
@@ -1020,7 +1034,7 @@ static void red_lossy_marshall_qxl_draw_transparent(DisplayChannelClient *dcc,
     int src_is_lossy;
     BitmapData src_bitmap_data;
 
-    src_is_lossy = is_bitmap_lossy(dcc, drawable->u.transparent.src_bitmap,
+    src_is_lossy = is_bitmap_lossy(dcc, item, drawable->u.transparent.src_bitmap,
                                    &drawable->u.transparent.src_area, &src_bitmap_data);
 
     if (!src_is_lossy || (src_bitmap_data.type != BITMAP_DATA_TYPE_SURFACE)) {
@@ -1071,7 +1085,7 @@ static void red_lossy_marshall_qxl_draw_alpha_blend(DisplayChannelClient *dcc,
     BitmapData src_bitmap_data;
     FillBitsType src_send_type;
 
-    src_is_lossy = is_bitmap_lossy(dcc, drawable->u.alpha_blend.src_bitmap,
+    src_is_lossy = is_bitmap_lossy(dcc, item, drawable->u.alpha_blend.src_bitmap,
                                    &drawable->u.alpha_blend.src_area, &src_bitmap_data);
 
     src_send_type = red_marshall_qxl_draw_alpha_blend(dcc, base_marshaller, dpi, TRUE);
@@ -1164,7 +1178,7 @@ static void red_lossy_marshall_qxl_draw_blend(DisplayChannelClient *dcc,
     int dest_is_lossy;
     SpiceRect dest_lossy_area;
 
-    src_is_lossy = is_bitmap_lossy(dcc, drawable->u.blend.src_bitmap,
+    src_is_lossy = is_bitmap_lossy(dcc, item, drawable->u.blend.src_bitmap,
                                    &drawable->u.blend.src_area, &src_bitmap_data);
     dest_is_lossy = is_surface_area_lossy(dcc, item->surface,
                                           &drawable->bbox, &dest_lossy_area);
@@ -1327,9 +1341,9 @@ static void red_lossy_marshall_qxl_draw_rop3(DisplayChannelClient *dcc,
     int dest_is_lossy;
     SpiceRect dest_lossy_area;
 
-    src_is_lossy = is_bitmap_lossy(dcc, drawable->u.rop3.src_bitmap,
+    src_is_lossy = is_bitmap_lossy(dcc, item, drawable->u.rop3.src_bitmap,
                                    &drawable->u.rop3.src_area, &src_bitmap_data);
-    brush_is_lossy = is_brush_lossy(dcc, &drawable->u.rop3.brush,
+    brush_is_lossy = is_brush_lossy(dcc, item, &drawable->u.rop3.brush,
                                     &brush_bitmap_data);
     dest_is_lossy = is_surface_area_lossy(dcc, item->surface,
                                           &drawable->bbox, &dest_lossy_area);
@@ -1405,10 +1419,10 @@ static void red_lossy_marshall_qxl_draw_composite(DisplayChannelClient *dcc,
     int dest_is_lossy;
     SpiceRect dest_lossy_area;
 
-    src_is_lossy = is_bitmap_lossy(dcc, drawable->u.composite.src_bitmap,
+    src_is_lossy = is_bitmap_lossy(dcc, item, drawable->u.composite.src_bitmap,
                                    nullptr, &src_bitmap_data);
     mask_is_lossy = drawable->u.composite.mask_bitmap &&
-        is_bitmap_lossy(dcc, drawable->u.composite.mask_bitmap, nullptr, &mask_bitmap_data);
+        is_bitmap_lossy(dcc, item, drawable->u.composite.mask_bitmap, nullptr, &mask_bitmap_data);
 
     dest_is_lossy = is_surface_area_lossy(dcc, item->surface,
                                           &drawable->bbox, &dest_lossy_area);
@@ -1483,7 +1497,7 @@ static void red_lossy_marshall_qxl_draw_stroke(DisplayChannelClient *dcc,
     SpiceRect dest_lossy_area;
     int rop;
 
-    brush_is_lossy = is_brush_lossy(dcc, &drawable->u.stroke.brush,
+    brush_is_lossy = is_brush_lossy(dcc, item, &drawable->u.stroke.brush,
                                     &brush_bitmap_data);
 
     // back_mode is not used at the client. Ignoring.
@@ -1565,9 +1579,9 @@ static void red_lossy_marshall_qxl_draw_text(DisplayChannelClient *dcc,
     SpiceRect dest_lossy_area;
     int rop = 0;
 
-    fg_is_lossy = is_brush_lossy(dcc, &drawable->u.text.fore_brush,
+    fg_is_lossy = is_brush_lossy(dcc, item, &drawable->u.text.fore_brush,
                                  &fg_bitmap_data);
-    bg_is_lossy = is_brush_lossy(dcc, &drawable->u.text.back_brush,
+    bg_is_lossy = is_brush_lossy(dcc, item, &drawable->u.text.back_brush,
                                  &bg_bitmap_data);
 
     // assuming that if the brush type is solid, the destination can
diff --git a/server/dcc.cpp b/server/dcc.cpp
index 54f55484..2a5159a8 100644
--- a/server/dcc.cpp
+++ b/server/dcc.cpp
@@ -243,7 +243,7 @@ void dcc_push_surface_image(DisplayChannelClient *dcc, RedSurface *surface)
         return;
     }
 
-    if (!surface->context.canvas) {
+    if (!surface) {
         return;
     }
     area.top = area.left = 0;
@@ -407,11 +407,12 @@ void dcc_start(DisplayChannelClient *dcc)
 
     red::shared_ptr<DisplayChannelClient> self(dcc);
     dcc->ack_zero_messages_window();
-    if (display->priv->surfaces[0].context.canvas) {
-        display_channel_current_flush(display, &display->priv->surfaces[0]);
+    auto surface0 = display->priv->surfaces[0];
+    if (surface0) {
+        display_channel_current_flush(display, surface0);
         dcc->pipe_add_type(RED_PIPE_ITEM_TYPE_INVAL_PALETTE_CACHE);
-        dcc_create_surface(dcc, &display->priv->surfaces[0]);
-        dcc_push_surface_image(dcc, &display->priv->surfaces[0]);
+        dcc_create_surface(dcc, surface0);
+        dcc_push_surface_image(dcc, surface0);
         dcc_push_monitors_config(dcc);
         dcc->pipe_add_empty_msg(SPICE_MSG_DISPLAY_MARK);
         dcc_create_all_streams(dcc);
diff --git a/server/display-channel-private.h b/server/display-channel-private.h
index 48762736..650b4782 100644
--- a/server/display-channel-private.h
+++ b/server/display-channel-private.h
@@ -36,6 +36,8 @@ typedef struct DrawContext {
 } DrawContext;
 
 struct RedSurface {
+    SPICE_CXX_GLIB_ALLOCATOR
+
     uint32_t refs;
     uint16_t id;
     /* A Ring representing a hierarchical tree structure. This tree includes
@@ -114,7 +116,7 @@ struct DisplayChannelPrivate
     uint32_t next_item_trace;
     uint64_t streams_size_total;
 
-    RedSurface surfaces[NUM_SURFACES];
+    RedSurface *surfaces[NUM_SURFACES];
     uint32_t n_surfaces;
     SpiceImageSurfaces image_surfaces;
 
diff --git a/server/display-channel.cpp b/server/display-channel.cpp
index 1db2eaed..d1920584 100644
--- a/server/display-channel.cpp
+++ b/server/display-channel.cpp
@@ -45,7 +45,7 @@ DisplayChannel::~DisplayChannel()
         spice_assert(ring_is_empty(&priv->streams));
 
         for (const auto &surface : priv->surfaces) {
-            spice_assert(surface.context.canvas == nullptr);
+            spice_assert(!surface);
         }
     }
 
@@ -242,28 +242,30 @@ static void display_channel_surface_unref(DisplayChannel *display, RedSurface *s
     spice_assert(surface->context.canvas);
 
     surface->context.canvas->ops->destroy(surface->context.canvas);
+    surface->context.canvas = nullptr;
     surface->create_cmd.reset();
     surface->destroy_cmd.reset();
 
     region_destroy(&surface->draw_dirty_region);
-    surface->context.canvas = nullptr;
     FOREACH_DCC(display, dcc) {
         dcc_destroy_surface(dcc, surface->id);
     }
 
     spice_warn_if_fail(ring_is_empty(&surface->depend_on_me));
+    delete surface;
 }
 
 /* TODO: perhaps rename to "ready" or "realized" ? */
 gboolean display_channel_surface_has_canvas(DisplayChannel *display,
                                             uint32_t surface_id)
 {
-    return display->priv->surfaces[surface_id].context.canvas != nullptr;
+    return display->priv->surfaces[surface_id] != nullptr;
 }
 
 void display_channel_surface_id_unref(DisplayChannel *display, uint32_t surface_id)
 {
-    display_channel_surface_unref(display, &display->priv->surfaces[surface_id]);
+    display_channel_surface_unref(display, display->priv->surfaces[surface_id]);
+    display->priv->surfaces[surface_id] = nullptr;
 }
 
 static void streams_update_visible_region(DisplayChannel *display, Drawable *drawable)
@@ -1120,7 +1122,7 @@ static void drawable_ref_surface_deps(DisplayChannel *display, Drawable *drawabl
             continue;
         }
 
-        RedSurface *surface = &display->priv->surfaces[surface_id];
+        RedSurface *surface = display->priv->surfaces[surface_id];
         surface->refs++;
         drawable->surface_deps[x] = surface;
     }
@@ -1299,7 +1301,7 @@ static Drawable *display_channel_get_drawable(DisplayChannel *display, uint8_t e
 
     drawable->tree_item.effect = effect;
 
-    drawable->surface = &display->priv->surfaces[red_drawable->surface_id];
+    drawable->surface = display->priv->surfaces[red_drawable->surface_id];
     drawable->surface->refs++;
 
     drawable->red_drawable = red_drawable;
@@ -1424,11 +1426,9 @@ bool display_channel_wait_for_migrate_data(DisplayChannel *display)
 
 void display_channel_flush_all_surfaces(DisplayChannel *display)
 {
-    int x;
-
-    for (x = 0; x < NUM_SURFACES; ++x) {
-        if (display->priv->surfaces[x].context.canvas) {
-            display_channel_current_flush(display, &display->priv->surfaces[x]);
+    for (const auto& surface : display->priv->surfaces) {
+        if (surface) {
+            display_channel_current_flush(display, surface);
         }
     }
 }
@@ -1914,7 +1914,7 @@ void display_channel_draw(DisplayChannel *display, const SpiceRect *area, int su
     spice_return_if_fail(area->left >= 0 && area->top >= 0 &&
                          area->left < area->right && area->top < area->bottom);
 
-    surface = &display->priv->surfaces[surface_id];
+    surface = display->priv->surfaces[surface_id];
 
     display_channel_surface_draw(display, surface, area);
 }
@@ -2016,17 +2016,15 @@ void display_channel_destroy_surface_wait(DisplayChannel *display, uint32_t surf
 /* TODO: split me*/
 void display_channel_destroy_surfaces(DisplayChannel *display)
 {
-    int i;
-
     spice_debug("trace");
     //to handle better
-    for (i = 0; i < NUM_SURFACES; ++i) {
-        if (display->priv->surfaces[i].context.canvas) {
-            display_channel_destroy_surface_wait(display, i);
-            if (display->priv->surfaces[i].context.canvas) {
-                display_channel_surface_unref(display, &display->priv->surfaces[i]);
+    for (auto& surface : display->priv->surfaces) {
+        if (surface) {
+            display_channel_destroy_surface_wait(display, surface->id);
+            if (surface) {
+                display_channel_surface_unref(display, surface);
+                surface = nullptr;
             }
-            spice_assert(!display->priv->surfaces[i].context.canvas);
         }
     }
     spice_warn_if_fail(ring_is_empty(&display->priv->streams));
@@ -2072,13 +2070,14 @@ create_canvas_for_surface(DisplayChannel *display, RedSurface *surface, uint32_t
     return nullptr;
 }
 
-void display_channel_create_surface(DisplayChannel *display, uint32_t surface_id, uint32_t width,
-                                    uint32_t height, int32_t stride, uint32_t format,
-                                    void *line_0, int data_is_valid, int send_client)
+RedSurface*
+display_channel_create_surface(DisplayChannel *display, uint32_t surface_id, uint32_t width,
+                               uint32_t height, int32_t stride, uint32_t format,
+                               void *line_0, int data_is_valid, int send_client)
 {
-    RedSurface *surface = &display->priv->surfaces[surface_id];
+    RedSurface *surface = new RedSurface;
 
-    spice_warn_if_fail(!surface->context.canvas);
+    spice_warn_if_fail(!display->priv->surfaces[surface_id]);
 
     surface->context.canvas_draws_on_surface = FALSE;
     surface->context.width = width;
@@ -2095,10 +2094,6 @@ void display_channel_create_surface(DisplayChannel *display, uint32_t surface_id
     }
     g_warn_if_fail(!surface->create_cmd);
     g_warn_if_fail(!surface->destroy_cmd);
-    ring_init(&surface->current);
-    ring_init(&surface->current_list);
-    ring_init(&surface->depend_on_me);
-    region_init(&surface->draw_dirty_region);
     surface->refs = 1;
     surface->id = surface_id;
 
@@ -2118,10 +2113,26 @@ void display_channel_create_surface(DisplayChannel *display, uint32_t surface_id
         surface->context.canvas = create_canvas_for_surface(display, surface, display->priv->renderer);
     }
 
-    spice_return_if_fail(surface->context.canvas);
+    if (!surface->context.canvas) {
+        delete surface;
+        return nullptr;
+    }
+
+    // finish initialization
+    ring_init(&surface->current);
+    ring_init(&surface->current_list);
+    ring_init(&surface->depend_on_me);
+    region_init(&surface->draw_dirty_region);
+
+    if (display->priv->surfaces[surface_id]) {
+        display_channel_surface_unref(display, display->priv->surfaces[surface_id]);
+    }
+    display->priv->surfaces[surface_id] = surface;
+
     if (send_client) {
         send_create_surface(display, surface, data_is_valid);
     }
+    return surface;
 }
 
 void DisplayChannelClient::handle_migrate_flush_mark()
@@ -2239,7 +2250,7 @@ void display_channel_process_surface_cmd(DisplayChannel *display,
         return;
     }
 
-    surface = &display->priv->surfaces[surface_id];
+    surface = display->priv->surfaces[surface_id];
 
     switch (surface_cmd->type) {
     case QXL_SURFACE_CMD_CREATE: {
@@ -2248,7 +2259,7 @@ void display_channel_process_surface_cmd(DisplayChannel *display,
         int32_t stride = create->stride;
         int reloaded_surface = loadvm || (surface_cmd->flags & QXL_SURF_FLAG_KEEP_DATA);
 
-        if (surface->refs) {
+        if (surface) {
             spice_warning("avoiding creating a surface twice");
             break;
         }
@@ -2258,21 +2269,24 @@ void display_channel_process_surface_cmd(DisplayChannel *display,
              * when it is read, specifically red_get_surface_cmd */
             data -= (int32_t)(stride * (height - 1));
         }
-        display_channel_create_surface(display, surface_id, create->width,
-                                       height, stride, create->format, data,
-                                       reloaded_surface,
-                                       // reloaded surfaces will be sent on demand
-                                       !reloaded_surface);
-        surface->create_cmd = surface_cmd;
+        surface = display_channel_create_surface(display, surface_id, create->width,
+                                                 height, stride, create->format, data,
+                                                 reloaded_surface,
+                                                 // reloaded surfaces will be sent on demand
+                                                 !reloaded_surface);
+        if (surface) {
+            surface->create_cmd = surface_cmd;
+        }
         break;
     }
     case QXL_SURFACE_CMD_DESTROY:
-        if (!surface->refs) {
+        if (!surface) {
             spice_warning("avoiding destroying a surface twice");
             break;
         }
         surface->destroy_cmd = surface_cmd;
         display_channel_destroy_surface(display, surface);
+        display->priv->surfaces[surface_id] = nullptr;
         break;
     default:
         spice_warn_if_reached();
@@ -2341,11 +2355,10 @@ RedSurface *display_channel_validate_surface(DisplayChannel *display, uint32_t s
         spice_warning("invalid surface_id %u", surface_id);
         return nullptr;
     }
-    RedSurface *surface = &display->priv->surfaces[surface_id];
-    if (!surface->context.canvas) {
-        spice_warning("canvas address is %p for %d (and is NULL)",
-                      &(surface->context.canvas), surface_id);
-        spice_warning("failed on %d", surface_id);
+
+    RedSurface *surface = display->priv->surfaces[surface_id];
+    if (!surface) {
+        spice_warning("surface %d is NULL", surface_id);
         return nullptr;
     }
     return surface;
diff --git a/server/display-channel.h b/server/display-channel.h
index 30c628f7..13f3b8d1 100644
--- a/server/display-channel.h
+++ b/server/display-channel.h
@@ -95,10 +95,10 @@ display_channel_new(RedsState *reds, QXLInstance *qxl,
                     GArray *video_codecs,
                     uint32_t n_surfaces);
 void display_channel_surface_id_unref(DisplayChannel *display, uint32_t surface_id);
-void                       display_channel_create_surface            (DisplayChannel *display, uint32_t surface_id,
-                                                                      uint32_t width, uint32_t height,
-                                                                      int32_t stride, uint32_t format, void *line_0,
-                                                                      int data_is_valid, int send_client);
+RedSurface *display_channel_create_surface(DisplayChannel *display, uint32_t surface_id,
+                                           uint32_t width, uint32_t height,
+                                           int32_t stride, uint32_t format, void *line_0,
+                                           int data_is_valid, int send_client);
 void                       display_channel_draw                      (DisplayChannel *display,
                                                                       const SpiceRect *area,
                                                                       int surface_id);
diff --git a/server/video-stream.cpp b/server/video-stream.cpp
index 11542bb9..667d2713 100644
--- a/server/video-stream.cpp
+++ b/server/video-stream.cpp
@@ -852,11 +852,11 @@ static void dcc_detach_stream_gracefully(DisplayChannelClient *dcc,
                     stream_id, stream->current != nullptr);
         rect_debug(&upgrade_area);
         if (update_area_limit) {
-            display_channel_draw_until(display, &upgrade_area, &display->priv->surfaces[0], update_area_limit);
+            display_channel_draw_until(display, &upgrade_area, display->priv->surfaces[0], update_area_limit);
         } else {
             display_channel_draw(display, &upgrade_area, 0);
         }
-        dcc_add_surface_area_image(dcc, &display->priv->surfaces[0], &upgrade_area,
+        dcc_add_surface_area_image(dcc, display->priv->surfaces[0], &upgrade_area,
                                    dcc->get_pipe().end(), false);
     }
 clear_vis_region:
commit f5c2043143f93ef8919f30e09f6331e0b12e5431
Author: Frediano Ziglio <fziglio at redhat.com>
Date:   Tue Sep 27 22:36:37 2016 +0100

    Remove last direct surface IDs usages
    
    Mostly left on dcc-send.cpp.
    Other minor too.
    
    The change in BitmapData seems odd but the id for cached image
    was not used so the only information left was the surface.
    
    Signed-off-by: Frediano Ziglio <freddy77 at gmail.com>
    Acked-by: Victor Toso <victortoso at redhat.com>

diff --git a/server/dcc-send.cpp b/server/dcc-send.cpp
index 5d3aad0e..996e491f 100644
--- a/server/dcc-send.cpp
+++ b/server/dcc-send.cpp
@@ -43,7 +43,7 @@ enum BitmapDataType {
 
 struct BitmapData {
     BitmapDataType type;
-    uint64_t id; // surface id or cache item id
+    RedSurface *surface;
     SpiceRect lossy_rect;
 };
 
@@ -84,18 +84,13 @@ static int dcc_pixmap_cache_hit(DisplayChannelClient *dcc, uint64_t id, int *los
 }
 
 /* set area=NULL for testing the whole surface */
-static bool is_surface_area_lossy(DisplayChannelClient *dcc, uint32_t surface_id,
+static bool is_surface_area_lossy(DisplayChannelClient *dcc, RedSurface *surface,
                                   const SpiceRect *area, SpiceRect *out_lossy_area)
 {
-    RedSurface *surface;
     QRegion *surface_lossy_region;
     QRegion lossy_region;
-    DisplayChannel *display = DCC_TO_DC(dcc);
 
-    surface = display_channel_validate_surface(display, surface_id);
-    spice_return_val_if_fail(surface, false);
-
-    surface_lossy_region = &dcc->priv->surface_client_lossy_region[surface_id];
+    surface_lossy_region = &dcc->priv->surface_client_lossy_region[surface->id];
 
     if (!area) {
         if (region_is_empty(surface_lossy_region)) {
@@ -129,33 +124,35 @@ static bool is_surface_area_lossy(DisplayChannelClient *dcc, uint32_t surface_id
 static bool is_bitmap_lossy(DisplayChannelClient *dcc, SpiceImage *image, SpiceRect *area,
                             BitmapData *out_data)
 {
+    out_data->type = BITMAP_DATA_TYPE_BITMAP;
     if (image == nullptr) {
         // self bitmap
-        out_data->type = BITMAP_DATA_TYPE_BITMAP;
         return FALSE;
     }
 
     if ((image->descriptor.flags & SPICE_IMAGE_FLAGS_CACHE_ME)) {
         int is_hit_lossy;
 
-        out_data->id = image->descriptor.id;
         if (dcc_pixmap_cache_hit(dcc, image->descriptor.id, &is_hit_lossy)) {
             out_data->type = BITMAP_DATA_TYPE_CACHE;
             return is_hit_lossy;
         }
         out_data->type = BITMAP_DATA_TYPE_BITMAP_TO_CACHE;
-    } else {
-         out_data->type = BITMAP_DATA_TYPE_BITMAP;
     }
 
     if (image->descriptor.type != SPICE_IMAGE_TYPE_SURFACE) {
         return FALSE;
     }
 
+    auto surface = display_channel_validate_surface(DCC_TO_DC(dcc), image->u.surface.surface_id);
+    if (!surface) {
+        return false;
+    }
+
     out_data->type = BITMAP_DATA_TYPE_SURFACE;
-    out_data->id = image->u.surface.surface_id;
+    out_data->surface = surface;
 
-    return is_surface_area_lossy(dcc, out_data->id,
+    return is_surface_area_lossy(dcc, out_data->surface,
                                  area, &out_data->lossy_rect);
 }
 
@@ -394,10 +391,9 @@ static FillBitsType fill_bits(DisplayChannelClient *dcc, SpiceMarshaller *m,
 
     switch (simage->descriptor.type) {
     case SPICE_IMAGE_TYPE_SURFACE: {
-        int surface_id;
         RedSurface *surface;
 
-        surface_id = simage->u.surface.surface_id;
+        auto surface_id = simage->u.surface.surface_id;
         surface = display_channel_validate_surface(display, surface_id);
         if (!surface) {
             spice_warning("Invalid surface in SPICE_IMAGE_TYPE_SURFACE");
@@ -588,13 +584,13 @@ static void surface_lossy_region_update(DisplayChannelClient *dcc,
     }
 }
 
-static bool drawable_intersects_with_areas(Drawable *drawable, int surface_ids[],
+static bool drawable_intersects_with_areas(Drawable *drawable, RedSurface *surfaces[],
                                            SpiceRect *surface_areas[],
                                            int num_surfaces)
 {
     int i;
     for (i = 0; i < num_surfaces; i++) {
-        if (surface_ids[i] == drawable->red_drawable->surface_id) {
+        if (surfaces[i] == drawable->surface) {
             if (rect_intersects(surface_areas[i], &drawable->red_drawable->bbox)) {
                 return TRUE;
             }
@@ -604,7 +600,7 @@ static bool drawable_intersects_with_areas(Drawable *drawable, int surface_ids[]
 }
 
 static bool pipe_rendered_drawables_intersect_with_areas(DisplayChannelClient *dcc,
-                                                         int surface_ids[],
+                                                         RedSurface *surfaces[],
                                                          SpiceRect *surface_areas[],
                                                          int num_surfaces)
 {
@@ -620,7 +616,7 @@ static bool pipe_rendered_drawables_intersect_with_areas(DisplayChannelClient *d
         if (ring_item_is_linked(&drawable->list_link))
             continue; // item hasn't been rendered
 
-        if (drawable_intersects_with_areas(drawable, surface_ids, surface_areas, num_surfaces)) {
+        if (drawable_intersects_with_areas(drawable, surfaces, surface_areas, num_surfaces)) {
             return TRUE;
         }
     }
@@ -628,15 +624,14 @@ static bool pipe_rendered_drawables_intersect_with_areas(DisplayChannelClient *d
     return FALSE;
 }
 
-static bool drawable_depends_on_areas(Drawable *drawable, int surface_ids[],
+static bool drawable_depends_on_areas(Drawable *drawable, RedSurface *surfaces[],
                                       SpiceRect surface_areas[], int num_surfaces)
 {
     int i;
-    RedDrawable *red_drawable;
     int drawable_has_shadow;
     SpiceRect shadow_rect = {0, 0, 0, 0};
 
-    red_drawable = drawable->red_drawable.get();
+    RedDrawable *const red_drawable = drawable->red_drawable.get();
     drawable_has_shadow = has_shadow(red_drawable);
 
     if (drawable_has_shadow) {
@@ -651,21 +646,16 @@ static bool drawable_depends_on_areas(Drawable *drawable, int surface_ids[],
 
     for (i = 0; i < num_surfaces; i++) {
         int x;
-        int dep_surface_id;
 
          for (x = 0; x < 3; ++x) {
-            if (!drawable->surface_deps[x]) {
-                continue;
-            }
-            dep_surface_id = drawable->surface_deps[x]->id;
-            if (dep_surface_id == surface_ids[i]) {
+            if (drawable->surface_deps[x] == surfaces[i]) {
                 if (rect_intersects(&surface_areas[i], &red_drawable->surfaces_rects[x])) {
                     return TRUE;
                 }
             }
         }
 
-        if (surface_ids[i] == red_drawable->surface_id) {
+        if (surfaces[i] == drawable->surface) {
             if (drawable_has_shadow) {
                 if (rect_intersects(&surface_areas[i], &shadow_rect)) {
                     return TRUE;
@@ -687,14 +677,14 @@ static bool drawable_depends_on_areas(Drawable *drawable, int surface_ids[],
 }
 
 static void red_pipe_replace_rendered_drawables_with_images(DisplayChannelClient *dcc,
-                                                            int first_surface_id,
+                                                            RedSurface *first_surface,
                                                             SpiceRect *first_area)
 {
-    int resent_surface_ids[MAX_PIPE_SIZE];
+    RedSurface *resent_surfaces[MAX_PIPE_SIZE];
     SpiceRect resent_areas[MAX_PIPE_SIZE]; // not pointers since drawables may be released
     int num_resent;
 
-    resent_surface_ids[0] = first_surface_id;
+    resent_surfaces[0] = first_surface;
     resent_areas[0] = *first_area;
     num_resent = 1;
 
@@ -717,15 +707,15 @@ static void red_pipe_replace_rendered_drawables_with_images(DisplayChannelClient
         // (i.e., the bitmaps can be more futuristic w.r.t X). Thus, X shouldn't
         // be rendered at the client, and we replace it with an image as well.
         if (!drawable_depends_on_areas(drawable,
-                                       resent_surface_ids,
+                                       resent_surfaces,
                                        resent_areas,
                                        num_resent)) {
             continue;
         }
 
-        dcc_add_surface_area_image(dcc, drawable->red_drawable->surface_id,
+        dcc_add_surface_area_image(dcc, drawable->surface,
                                    &drawable->red_drawable->bbox, l, TRUE);
-        resent_surface_ids[num_resent] = drawable->red_drawable->surface_id;
+        resent_surfaces[num_resent] = drawable->surface;
         resent_areas[num_resent] = drawable->red_drawable->bbox;
         num_resent++;
 
@@ -735,7 +725,7 @@ static void red_pipe_replace_rendered_drawables_with_images(DisplayChannelClient
 
 static void red_add_lossless_drawable_dependencies(DisplayChannelClient *dcc,
                                                    Drawable *item,
-                                                   int deps_surfaces_ids[],
+                                                   RedSurface *deps_surfaces[],
                                                    SpiceRect *deps_areas[],
                                                    int num_deps)
 {
@@ -751,10 +741,10 @@ static void red_add_lossless_drawable_dependencies(DisplayChannelClient *dcc,
 
         // checking if the drawable itself or one of the other commands
         // that were rendered, affected the areas that need to be resent
-        if (!drawable_intersects_with_areas(item, deps_surfaces_ids,
+        if (!drawable_intersects_with_areas(item, deps_surfaces,
                                             deps_areas, num_deps)) {
             if (pipe_rendered_drawables_intersect_with_areas(dcc,
-                                                             deps_surfaces_ids,
+                                                             deps_surfaces,
                                                              deps_areas,
                                                              num_deps)) {
                 sync_rendered = TRUE;
@@ -765,7 +755,7 @@ static void red_add_lossless_drawable_dependencies(DisplayChannelClient *dcc,
     } else {
         sync_rendered = FALSE;
         for (i = 0; i < num_deps; i++) {
-            display_channel_draw_until(display, deps_areas[i], deps_surfaces_ids[i], item);
+            display_channel_draw_until(display, deps_areas[i], deps_surfaces[i], item);
         }
     }
 
@@ -775,28 +765,28 @@ static void red_add_lossless_drawable_dependencies(DisplayChannelClient *dcc,
         // the surfaces areas will be sent as DRAW_COPY commands, that
         // will be executed before the current drawable
         for (i = 0; i < num_deps; i++) {
-            dcc_add_surface_area_image(dcc, deps_surfaces_ids[i], deps_areas[i],
+            dcc_add_surface_area_image(dcc, deps_surfaces[i], deps_areas[i],
                                        get_pipe_tail(dcc->get_pipe()), FALSE);
 
         }
     } else {
-        int drawable_surface_id[1];
+        RedSurface *drawable_surface[1];
         SpiceRect *drawable_bbox[1];
 
-        drawable_surface_id[0] = drawable->surface_id;
+        drawable_surface[0] = item->surface;
         drawable_bbox[0] = &drawable->bbox;
 
         // check if the other rendered images in the pipe have updated the drawable bbox
         if (pipe_rendered_drawables_intersect_with_areas(dcc,
-                                                         drawable_surface_id,
+                                                         drawable_surface,
                                                          drawable_bbox,
                                                          1)) {
             red_pipe_replace_rendered_drawables_with_images(dcc,
-                                                            drawable->surface_id,
+                                                            item->surface,
                                                             &drawable->bbox);
         }
 
-        dcc_add_surface_area_image(dcc, drawable->surface_id, &drawable->bbox,
+        dcc_add_surface_area_image(dcc, item->surface, &drawable->bbox,
                                    get_pipe_tail(dcc->get_pipe()), TRUE);
     }
 }
@@ -824,7 +814,7 @@ static void red_lossy_marshall_qxl_draw_fill(DisplayChannelClient *dcc,
     brush_is_lossy = is_brush_lossy(dcc, &drawable->u.fill.brush,
                                     &brush_bitmap_data);
     if (!dest_allowed_lossy) {
-        dest_is_lossy = is_surface_area_lossy(dcc, item->surface->id, &drawable->bbox,
+        dest_is_lossy = is_surface_area_lossy(dcc, item->surface, &drawable->bbox,
                                               &dest_lossy_area);
     }
 
@@ -836,24 +826,24 @@ static void red_lossy_marshall_qxl_draw_fill(DisplayChannelClient *dcc,
         // either the brush operation is opaque, or the dest is not lossy
         surface_lossy_region_update(dcc, item, has_mask, FALSE);
     } else {
-        int resend_surface_ids[2];
+        RedSurface *resend_surfaces[2];
         SpiceRect *resend_areas[2];
         int num_resend = 0;
 
         if (dest_is_lossy) {
-            resend_surface_ids[num_resend] = item->surface->id;
+            resend_surfaces[num_resend] = item->surface;
             resend_areas[num_resend] = &dest_lossy_area;
             num_resend++;
         }
 
         if (brush_is_lossy && (brush_bitmap_data.type == BITMAP_DATA_TYPE_SURFACE)) {
-            resend_surface_ids[num_resend] = brush_bitmap_data.id;
+            resend_surfaces[num_resend] = brush_bitmap_data.surface;
             resend_areas[num_resend] = &brush_bitmap_data.lossy_rect;
             num_resend++;
         }
 
         red_add_lossless_drawable_dependencies(dcc, item,
-                                               resend_surface_ids, resend_areas, num_resend);
+                                               resend_surfaces, resend_areas, num_resend);
     }
 }
 
@@ -932,24 +922,24 @@ static void red_lossy_marshall_qxl_draw_opaque(DisplayChannelClient *dcc,
 
         surface_lossy_region_update(dcc, item, has_mask, src_is_lossy);
     } else {
-        int resend_surface_ids[2];
+        RedSurface *resend_surfaces[2];
         SpiceRect *resend_areas[2];
         int num_resend = 0;
 
         if (src_is_lossy && (src_bitmap_data.type == BITMAP_DATA_TYPE_SURFACE)) {
-            resend_surface_ids[num_resend] = src_bitmap_data.id;
+            resend_surfaces[num_resend] = src_bitmap_data.surface;
             resend_areas[num_resend] = &src_bitmap_data.lossy_rect;
             num_resend++;
         }
 
         if (brush_is_lossy && (brush_bitmap_data.type == BITMAP_DATA_TYPE_SURFACE)) {
-            resend_surface_ids[num_resend] = brush_bitmap_data.id;
+            resend_surfaces[num_resend] = brush_bitmap_data.surface;
             resend_areas[num_resend] = &brush_bitmap_data.lossy_rect;
             num_resend++;
         }
 
         red_add_lossless_drawable_dependencies(dcc, item,
-                                               resend_surface_ids, resend_areas, num_resend);
+                                               resend_surfaces, resend_areas, num_resend);
     }
 }
 
@@ -1035,16 +1025,16 @@ static void red_lossy_marshall_qxl_draw_transparent(DisplayChannelClient *dcc,
 
     if (!src_is_lossy || (src_bitmap_data.type != BITMAP_DATA_TYPE_SURFACE)) {
         red_marshall_qxl_draw_transparent(dcc, base_marshaller, dpi);
-        // don't update surface lossy region since transperent areas might be lossy
+        // don't update surface lossy region since transparent areas might be lossy
     } else {
-        int resend_surface_ids[1];
+        RedSurface *resend_surfaces[1];
         SpiceRect *resend_areas[1];
 
-        resend_surface_ids[0] = src_bitmap_data.id;
+        resend_surfaces[0] = src_bitmap_data.surface;
         resend_areas[0] = &src_bitmap_data.lossy_rect;
 
         red_add_lossless_drawable_dependencies(dcc, item,
-                                               resend_surface_ids, resend_areas, 1);
+                                               resend_surfaces, resend_areas, 1);
     }
 }
 
@@ -1134,7 +1124,7 @@ static void red_lossy_marshall_qxl_copy_bits(DisplayChannelClient *dcc,
     src_rect.right = drawable->bbox.right + horz_offset;
     src_rect.bottom = drawable->bbox.bottom + vert_offset;
 
-    src_is_lossy = is_surface_area_lossy(dcc, item->surface->id,
+    src_is_lossy = is_surface_area_lossy(dcc, item->surface,
                                          &src_rect, &src_lossy_area);
 
     surface_lossy_region_update(dcc, item, FALSE, src_is_lossy);
@@ -1176,31 +1166,31 @@ static void red_lossy_marshall_qxl_draw_blend(DisplayChannelClient *dcc,
 
     src_is_lossy = is_bitmap_lossy(dcc, drawable->u.blend.src_bitmap,
                                    &drawable->u.blend.src_area, &src_bitmap_data);
-    dest_is_lossy = is_surface_area_lossy(dcc, drawable->surface_id,
+    dest_is_lossy = is_surface_area_lossy(dcc, item->surface,
                                           &drawable->bbox, &dest_lossy_area);
 
     if (!dest_is_lossy &&
         (!src_is_lossy || (src_bitmap_data.type != BITMAP_DATA_TYPE_SURFACE))) {
         red_marshall_qxl_draw_blend(dcc, base_marshaller, dpi);
     } else {
-        int resend_surface_ids[2];
+        RedSurface *resend_surfaces[2];
         SpiceRect *resend_areas[2];
         int num_resend = 0;
 
         if (src_is_lossy && (src_bitmap_data.type == BITMAP_DATA_TYPE_SURFACE)) {
-            resend_surface_ids[num_resend] = src_bitmap_data.id;
+            resend_surfaces[num_resend] = src_bitmap_data.surface;
             resend_areas[num_resend] = &src_bitmap_data.lossy_rect;
             num_resend++;
         }
 
         if (dest_is_lossy) {
-            resend_surface_ids[num_resend] = item->surface->id;
+            resend_surfaces[num_resend] = item->surface;
             resend_areas[num_resend] = &dest_lossy_area;
             num_resend++;
         }
 
         red_add_lossless_drawable_dependencies(dcc, item,
-                                               resend_surface_ids, resend_areas, num_resend);
+                                               resend_surfaces, resend_areas, num_resend);
     }
 }
 
@@ -1341,7 +1331,7 @@ static void red_lossy_marshall_qxl_draw_rop3(DisplayChannelClient *dcc,
                                    &drawable->u.rop3.src_area, &src_bitmap_data);
     brush_is_lossy = is_brush_lossy(dcc, &drawable->u.rop3.brush,
                                     &brush_bitmap_data);
-    dest_is_lossy = is_surface_area_lossy(dcc, drawable->surface_id,
+    dest_is_lossy = is_surface_area_lossy(dcc, item->surface,
                                           &drawable->bbox, &dest_lossy_area);
 
     if ((!src_is_lossy || (src_bitmap_data.type != BITMAP_DATA_TYPE_SURFACE)) &&
@@ -1351,30 +1341,30 @@ static void red_lossy_marshall_qxl_draw_rop3(DisplayChannelClient *dcc,
         red_marshall_qxl_draw_rop3(dcc, base_marshaller, dpi);
         surface_lossy_region_update(dcc, item, has_mask, FALSE);
     } else {
-        int resend_surface_ids[3];
+        RedSurface *resend_surfaces[3];
         SpiceRect *resend_areas[3];
         int num_resend = 0;
 
         if (src_is_lossy && (src_bitmap_data.type == BITMAP_DATA_TYPE_SURFACE)) {
-            resend_surface_ids[num_resend] = src_bitmap_data.id;
+            resend_surfaces[num_resend] = src_bitmap_data.surface;
             resend_areas[num_resend] = &src_bitmap_data.lossy_rect;
             num_resend++;
         }
 
         if (brush_is_lossy && (brush_bitmap_data.type == BITMAP_DATA_TYPE_SURFACE)) {
-            resend_surface_ids[num_resend] = brush_bitmap_data.id;
+            resend_surfaces[num_resend] = brush_bitmap_data.surface;
             resend_areas[num_resend] = &brush_bitmap_data.lossy_rect;
             num_resend++;
         }
 
         if (dest_is_lossy) {
-            resend_surface_ids[num_resend] = item->surface->id;
+            resend_surfaces[num_resend] = item->surface;
             resend_areas[num_resend] = &dest_lossy_area;
             num_resend++;
         }
 
         red_add_lossless_drawable_dependencies(dcc, item,
-                                               resend_surface_ids, resend_areas, num_resend);
+                                               resend_surfaces, resend_areas, num_resend);
     }
 }
 
@@ -1420,7 +1410,7 @@ static void red_lossy_marshall_qxl_draw_composite(DisplayChannelClient *dcc,
     mask_is_lossy = drawable->u.composite.mask_bitmap &&
         is_bitmap_lossy(dcc, drawable->u.composite.mask_bitmap, nullptr, &mask_bitmap_data);
 
-    dest_is_lossy = is_surface_area_lossy(dcc, drawable->surface_id,
+    dest_is_lossy = is_surface_area_lossy(dcc, item->surface,
                                           &drawable->bbox, &dest_lossy_area);
 
     if ((!src_is_lossy || (src_bitmap_data.type != BITMAP_DATA_TYPE_SURFACE))   &&
@@ -1430,30 +1420,30 @@ static void red_lossy_marshall_qxl_draw_composite(DisplayChannelClient *dcc,
         surface_lossy_region_update(dcc, item, FALSE, FALSE);
     }
     else {
-        int resend_surface_ids[3];
+        RedSurface *resend_surfaces[3];
         SpiceRect *resend_areas[3];
         int num_resend = 0;
 
         if (src_is_lossy && (src_bitmap_data.type == BITMAP_DATA_TYPE_SURFACE)) {
-            resend_surface_ids[num_resend] = src_bitmap_data.id;
+            resend_surfaces[num_resend] = src_bitmap_data.surface;
             resend_areas[num_resend] = &src_bitmap_data.lossy_rect;
             num_resend++;
         }
 
         if (mask_is_lossy && (mask_bitmap_data.type == BITMAP_DATA_TYPE_SURFACE)) {
-            resend_surface_ids[num_resend] = mask_bitmap_data.id;
+            resend_surfaces[num_resend] = mask_bitmap_data.surface;
             resend_areas[num_resend] = &mask_bitmap_data.lossy_rect;
             num_resend++;
         }
 
         if (dest_is_lossy) {
-            resend_surface_ids[num_resend] = item->surface->id;
+            resend_surfaces[num_resend] = item->surface;
             resend_areas[num_resend] = &dest_lossy_area;
             num_resend++;
         }
 
         red_add_lossless_drawable_dependencies(dcc, item,
-                                               resend_surface_ids, resend_areas, num_resend);
+                                               resend_surfaces, resend_areas, num_resend);
     }
 }
 
@@ -1504,7 +1494,7 @@ static void red_lossy_marshall_qxl_draw_stroke(DisplayChannelClient *dcc,
     if (drawable->u.stroke.brush.type != SPICE_BRUSH_TYPE_SOLID &&
         ((rop & SPICE_ROPD_OP_OR) || (rop & SPICE_ROPD_OP_AND) ||
         (rop & SPICE_ROPD_OP_XOR))) {
-        dest_is_lossy = is_surface_area_lossy(dcc, drawable->surface_id,
+        dest_is_lossy = is_surface_area_lossy(dcc, item->surface,
                                               &drawable->bbox, &dest_lossy_area);
     }
 
@@ -1513,25 +1503,25 @@ static void red_lossy_marshall_qxl_draw_stroke(DisplayChannelClient *dcc,
     {
         red_marshall_qxl_draw_stroke(dcc, base_marshaller, dpi);
     } else {
-        int resend_surface_ids[2];
+        RedSurface *resend_surfaces[2];
         SpiceRect *resend_areas[2];
         int num_resend = 0;
 
         if (brush_is_lossy && (brush_bitmap_data.type == BITMAP_DATA_TYPE_SURFACE)) {
-            resend_surface_ids[num_resend] = brush_bitmap_data.id;
+            resend_surfaces[num_resend] = brush_bitmap_data.surface;
             resend_areas[num_resend] = &brush_bitmap_data.lossy_rect;
             num_resend++;
         }
 
         // TODO: use the path in order to resend smaller areas
         if (dest_is_lossy) {
-            resend_surface_ids[num_resend] = drawable->surface_id;
+            resend_surfaces[num_resend] = item->surface;
             resend_areas[num_resend] = &dest_lossy_area;
             num_resend++;
         }
 
         red_add_lossless_drawable_dependencies(dcc, item,
-                                               resend_surface_ids, resend_areas, num_resend);
+                                               resend_surfaces, resend_areas, num_resend);
     }
 }
 
@@ -1592,7 +1582,7 @@ static void red_lossy_marshall_qxl_draw_text(DisplayChannelClient *dcc,
 
     if ((rop & SPICE_ROPD_OP_OR) || (rop & SPICE_ROPD_OP_AND) ||
         (rop & SPICE_ROPD_OP_XOR)) {
-        dest_is_lossy = is_surface_area_lossy(dcc, drawable->surface_id,
+        dest_is_lossy = is_surface_area_lossy(dcc, item->surface,
                                               &drawable->bbox, &dest_lossy_area);
     }
 
@@ -1601,29 +1591,29 @@ static void red_lossy_marshall_qxl_draw_text(DisplayChannelClient *dcc,
         (!bg_is_lossy || (bg_bitmap_data.type != BITMAP_DATA_TYPE_SURFACE))) {
         red_marshall_qxl_draw_text(dcc, base_marshaller, dpi);
     } else {
-        int resend_surface_ids[3];
+        RedSurface *resend_surface[3];
         SpiceRect *resend_areas[3];
         int num_resend = 0;
 
         if (fg_is_lossy && (fg_bitmap_data.type == BITMAP_DATA_TYPE_SURFACE)) {
-            resend_surface_ids[num_resend] = fg_bitmap_data.id;
+            resend_surface[num_resend] = fg_bitmap_data.surface;
             resend_areas[num_resend] = &fg_bitmap_data.lossy_rect;
             num_resend++;
         }
 
         if (bg_is_lossy && (bg_bitmap_data.type == BITMAP_DATA_TYPE_SURFACE)) {
-            resend_surface_ids[num_resend] = bg_bitmap_data.id;
+            resend_surface[num_resend] = bg_bitmap_data.surface;
             resend_areas[num_resend] = &bg_bitmap_data.lossy_rect;
             num_resend++;
         }
 
         if (dest_is_lossy) {
-            resend_surface_ids[num_resend] = drawable->surface_id;
+            resend_surface[num_resend] = item->surface;
             resend_areas[num_resend] = &dest_lossy_area;
             num_resend++;
         }
         red_add_lossless_drawable_dependencies(dcc, item,
-                                               resend_surface_ids, resend_areas, num_resend);
+                                               resend_surface, resend_areas, num_resend);
     }
 }
 
diff --git a/server/dcc.cpp b/server/dcc.cpp
index f61f96f8..54f55484 100644
--- a/server/dcc.cpp
+++ b/server/dcc.cpp
@@ -181,12 +181,11 @@ void dcc_create_surface(DisplayChannelClient *dcc, RedSurface *surface)
 
 // adding the pipe item after pos. If pos == NULL, adding to head.
 void
-dcc_add_surface_area_image(DisplayChannelClient *dcc, int surface_id,
+dcc_add_surface_area_image(DisplayChannelClient *dcc, RedSurface *surface,
                            SpiceRect *area, RedChannelClient::Pipe::iterator pipe_item_pos,
                            int can_lossy)
 {
     DisplayChannel *display = DCC_TO_DC(dcc);
-    RedSurface *surface = &display->priv->surfaces[surface_id];
     SpiceCanvas *canvas = surface->context.canvas;
     int stride;
     int width;
@@ -203,7 +202,7 @@ dcc_add_surface_area_image(DisplayChannelClient *dcc, int surface_id,
 
     red::shared_ptr<RedImageItem> item(new (height * stride) RedImageItem());
 
-    item->surface_id = surface_id;
+    item->surface_id = surface->id;
     item->image_format =
         spice_bitmap_from_surface_type(surface->context.format);
     item->image_flags = 0;
@@ -253,7 +252,7 @@ void dcc_push_surface_image(DisplayChannelClient *dcc, RedSurface *surface)
 
     /* not allowing lossy compression because probably, especially if it is a primary surface,
        it combines both "picture-like" areas with areas that are more "artificial"*/
-    dcc_add_surface_area_image(dcc, surface->id, &area, dcc->get_pipe().end(), false);
+    dcc_add_surface_area_image(dcc, surface, &area, dcc->get_pipe().end(), false);
 }
 
 static void add_drawable_surface_images(DisplayChannelClient *dcc, Drawable *drawable)
diff --git a/server/dcc.h b/server/dcc.h
index 2693d55f..89f26233 100644
--- a/server/dcc.h
+++ b/server/dcc.h
@@ -140,7 +140,7 @@ void dcc_create_surface(DisplayChannelClient *dcc, struct RedSurface *surface);
 void dcc_push_surface_image(DisplayChannelClient *dcc, struct RedSurface *surface);
 bool dcc_clear_surface_drawables_from_pipe(DisplayChannelClient *dcc,
                                            RedSurface *surface, bool wait_if_used);
-void dcc_add_surface_area_image(DisplayChannelClient *dcc, int surface_id,
+void dcc_add_surface_area_image(DisplayChannelClient *dcc, RedSurface *surface,
                                 SpiceRect *area, RedChannelClient::Pipe::iterator pipe_item_pos,
                                 int can_lossy);
 RedPipeItemPtr dcc_gl_scanout_item_new(RedChannelClient *rcc, void *data, int num);
diff --git a/server/display-channel-private.h b/server/display-channel-private.h
index 067aad29..48762736 100644
--- a/server/display-channel-private.h
+++ b/server/display-channel-private.h
@@ -172,7 +172,7 @@ void monitors_config_unref(MonitorsConfig *config);
 
 void display_channel_draw_until(DisplayChannel *display,
                                 const SpiceRect *area,
-                                int surface_id,
+                                RedSurface *surface,
                                 Drawable *last);
 GArray* display_channel_get_video_codecs(DisplayChannel *display);
 int display_channel_get_stream_video(DisplayChannel *display);
diff --git a/server/display-channel.cpp b/server/display-channel.cpp
index 6ed24703..1db2eaed 100644
--- a/server/display-channel.cpp
+++ b/server/display-channel.cpp
@@ -1864,10 +1864,9 @@ static Drawable* current_find_intersects_rect(Ring *current, RingItem *from,
  * than 'last' (exclusive).
  * FIXME: merge with display_channel_draw()?
  */
-void display_channel_draw_until(DisplayChannel *display, const SpiceRect *area, int surface_id,
+void display_channel_draw_until(DisplayChannel *display, const SpiceRect *area, RedSurface *surface,
                                 Drawable *last)
 {
-    RedSurface *surface;
     Drawable *surface_last = nullptr;
     Ring *ring;
     RingItem *ring_item;
@@ -1876,8 +1875,6 @@ void display_channel_draw_until(DisplayChannel *display, const SpiceRect *area,
     spice_return_if_fail(last);
     spice_return_if_fail(ring_item_is_linked(&last->list_link));
 
-    surface = &display->priv->surfaces[surface_id];
-
     if (surface != last->surface) {
         // find the nearest older drawable from the appropriate surface
         ring = &display->priv->current_list;
diff --git a/server/video-stream.cpp b/server/video-stream.cpp
index 9281d5f4..11542bb9 100644
--- a/server/video-stream.cpp
+++ b/server/video-stream.cpp
@@ -852,11 +852,12 @@ static void dcc_detach_stream_gracefully(DisplayChannelClient *dcc,
                     stream_id, stream->current != nullptr);
         rect_debug(&upgrade_area);
         if (update_area_limit) {
-            display_channel_draw_until(display, &upgrade_area, 0, update_area_limit);
+            display_channel_draw_until(display, &upgrade_area, &display->priv->surfaces[0], update_area_limit);
         } else {
             display_channel_draw(display, &upgrade_area, 0);
         }
-        dcc_add_surface_area_image(dcc, 0, &upgrade_area, dcc->get_pipe().end(), FALSE);
+        dcc_add_surface_area_image(dcc, &display->priv->surfaces[0], &upgrade_area,
+                                   dcc->get_pipe().end(), false);
     }
 clear_vis_region:
     region_clear(&agent->vis_region);
commit 4434c6fb967f2d47e0a2a9450d77e8be447c2e97
Author: Frediano Ziglio <fziglio at redhat.com>
Date:   Fri Sep 16 22:09:03 2016 +0100

    Change validate_surface to return surface pointer
    
    Instead of computing the value inside the function to then
    compute also in the caller return it.
    
    Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
    Acked-by: Victor Toso <victortoso at redhat.com>

diff --git a/server/dcc-send.cpp b/server/dcc-send.cpp
index 8c1aa6e0..5d3aad0e 100644
--- a/server/dcc-send.cpp
+++ b/server/dcc-send.cpp
@@ -92,9 +92,9 @@ static bool is_surface_area_lossy(DisplayChannelClient *dcc, uint32_t surface_id
     QRegion lossy_region;
     DisplayChannel *display = DCC_TO_DC(dcc);
 
-    spice_return_val_if_fail(display_channel_validate_surface(display, surface_id), FALSE);
+    surface = display_channel_validate_surface(display, surface_id);
+    spice_return_val_if_fail(surface, false);
 
-    surface = &display->priv->surfaces[surface_id];
     surface_lossy_region = &dcc->priv->surface_client_lossy_region[surface_id];
 
     if (!area) {
@@ -398,13 +398,13 @@ static FillBitsType fill_bits(DisplayChannelClient *dcc, SpiceMarshaller *m,
         RedSurface *surface;
 
         surface_id = simage->u.surface.surface_id;
-        if (!display_channel_validate_surface(display, surface_id)) {
+        surface = display_channel_validate_surface(display, surface_id);
+        if (!surface) {
             spice_warning("Invalid surface in SPICE_IMAGE_TYPE_SURFACE");
             pthread_mutex_unlock(&dcc->priv->pixmap_cache->lock);
             return FILL_BITS_TYPE_SURFACE;
         }
 
-        surface = &display->priv->surfaces[surface_id];
         image.descriptor.type = SPICE_IMAGE_TYPE_SURFACE;
         image.descriptor.flags = 0;
         image.descriptor.width = surface->context.width;
diff --git a/server/display-channel.cpp b/server/display-channel.cpp
index aa4bccb2..6ed24703 100644
--- a/server/display-channel.cpp
+++ b/server/display-channel.cpp
@@ -1241,15 +1241,15 @@ static void draw_depend_on_me(DisplayChannel *display, RedSurface *surface)
 static bool validate_drawable_bbox(DisplayChannel *display, RedDrawable *drawable)
 {
         DrawContext *context;
-        uint32_t surface_id = drawable->surface_id;
 
         /* surface_id must be validated before calling into
          * validate_drawable_bbox
          */
-        if (!display_channel_validate_surface(display, drawable->surface_id)) {
+        RedSurface *surface = display_channel_validate_surface(display, drawable->surface_id);
+        if (!surface) {
             return FALSE;
         }
-        context = &display->priv->surfaces[surface_id].context;
+        context = &surface->context;
 
         if (drawable->bbox.top < 0)
                 return FALSE;
@@ -1955,16 +1955,15 @@ void display_channel_update(DisplayChannel *display,
                             QXLRect **qxl_dirty_rects, uint32_t *num_dirty_rects)
 {
     SpiceRect rect;
-    RedSurface *surface;
 
     // Check that the request is valid, the surface_id comes directly from the guest
-    if (!display_channel_validate_surface(display, surface_id)) {
+    RedSurface *surface = display_channel_validate_surface(display, surface_id);
+    if (!surface) {
         // just return, display_channel_validate_surface already logged a warning
         return;
     }
 
     red_get_rect_ptr(&rect, area);
-    surface = &display->priv->surfaces[surface_id];
     display_channel_surface_draw(display, surface, &rect);
 
     if (*qxl_dirty_rects == nullptr) {
@@ -2003,11 +2002,10 @@ static void display_channel_destroy_surface(DisplayChannel *display, RedSurface
 
 void display_channel_destroy_surface_wait(DisplayChannel *display, uint32_t surface_id)
 {
-    if (!display_channel_validate_surface(display, surface_id))
-        return;
-    RedSurface *surface = &display->priv->surfaces[surface_id];
-    if (!surface->context.canvas)
+    RedSurface *surface = display_channel_validate_surface(display, surface_id);
+    if (!surface) {
         return;
+    }
 
     draw_depend_on_me(display, surface);
     /* note that draw_depend_on_me must be called before current_remove_all.
@@ -2152,9 +2150,9 @@ static SpiceCanvas *image_surfaces_get(SpiceImageSurfaces *surfaces, uint32_t su
     DisplayChannelPrivate *p = SPICE_CONTAINEROF(surfaces, DisplayChannelPrivate, image_surfaces);
     DisplayChannel *display = p->pub;
 
-    spice_return_val_if_fail(display_channel_validate_surface(display, surface_id), NULL);
+    auto surface = display_channel_validate_surface(display, surface_id);
 
-    return p->surfaces[surface_id].context.canvas;
+    return surface ? surface->context.canvas : nullptr;
 }
 
 red::shared_ptr<DisplayChannel>
@@ -2340,19 +2338,20 @@ VideoStream *display_channel_get_nth_video_stream(DisplayChannel *display, gint
     return &display->priv->streams_buf[i];
 }
 
-gboolean display_channel_validate_surface(DisplayChannel *display, uint32_t surface_id)
+RedSurface *display_channel_validate_surface(DisplayChannel *display, uint32_t surface_id)
 {
     if SPICE_UNLIKELY(surface_id >= display->priv->n_surfaces) {
         spice_warning("invalid surface_id %u", surface_id);
-        return FALSE;
+        return nullptr;
     }
-    if (!display->priv->surfaces[surface_id].context.canvas) {
+    RedSurface *surface = &display->priv->surfaces[surface_id];
+    if (!surface->context.canvas) {
         spice_warning("canvas address is %p for %d (and is NULL)",
-                   &(display->priv->surfaces[surface_id].context.canvas), surface_id);
+                      &(surface->context.canvas), surface_id);
         spice_warning("failed on %d", surface_id);
-        return FALSE;
+        return nullptr;
     }
-    return TRUE;
+    return surface;
 }
 
 void display_channel_push_monitors_config(DisplayChannel *display)
@@ -2380,17 +2379,19 @@ void display_channel_update_monitors_config(DisplayChannel *display,
 
 void display_channel_set_monitors_config_to_primary(DisplayChannel *display)
 {
-    DrawContext *context = &display->priv->surfaces[0].context;
-    QXLHead head = { 0, };
-    uint16_t old_max = 1;
+    RedSurface *surface = display_channel_validate_surface(display, 0);
+
+    spice_return_if_fail(surface);
 
-    spice_return_if_fail(display->priv->surfaces[0].context.canvas);
+    DrawContext *context = &surface->context;
+    uint16_t old_max = 1;
 
     if (display->priv->monitors_config) {
         old_max = display->priv->monitors_config->max_allowed;
         monitors_config_unref(display->priv->monitors_config);
     }
 
+    QXLHead head = { 0, };
     head.width = context->width;
     head.height = context->height;
     display->priv->monitors_config = monitors_config_new(&head, 1, old_max);
diff --git a/server/display-channel.h b/server/display-channel.h
index 591e15a3..30c628f7 100644
--- a/server/display-channel.h
+++ b/server/display-channel.h
@@ -139,7 +139,7 @@ void display_channel_update_monitors_config(DisplayChannel *display, const QXLMo
 void display_channel_set_monitors_config_to_primary(DisplayChannel *display);
 void display_channel_push_monitors_config(DisplayChannel *display);
 
-gboolean display_channel_validate_surface(DisplayChannel *display, uint32_t surface_id);
+RedSurface *display_channel_validate_surface(DisplayChannel *display, uint32_t surface_id);
 gboolean display_channel_surface_has_canvas(DisplayChannel *display, uint32_t surface_id);
 void display_channel_reset_image_cache(DisplayChannel *self);
 
commit 1d123192e71444f3d4cc83d028a67bc06a187615
Author: Frediano Ziglio <fziglio at redhat.com>
Date:   Tue Sep 27 22:06:26 2016 +0100

    Use directly surface instead of id
    
    Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
    Acked-by: Victor Toso <victortoso at redhat.com>

diff --git a/server/dcc.cpp b/server/dcc.cpp
index bc4b77fd..f61f96f8 100644
--- a/server/dcc.cpp
+++ b/server/dcc.cpp
@@ -261,9 +261,8 @@ static void add_drawable_surface_images(DisplayChannelClient *dcc, Drawable *dra
     DisplayChannel *display = DCC_TO_DC(dcc);
 
     for (const auto surface : drawable->surface_deps) {
-        if (surface != nullptr) {
-            const auto surface_id = surface->id;
-            if (dcc->priv->surface_client_created[surface_id]) {
+        if (surface) {
+            if (dcc->priv->surface_client_created[surface->id]) {
                 continue;
             }
             dcc_create_surface(dcc, surface);
@@ -272,13 +271,14 @@ static void add_drawable_surface_images(DisplayChannelClient *dcc, Drawable *dra
         }
     }
 
-    if (dcc->priv->surface_client_created[drawable->surface->id]) {
+    const auto surface = drawable->surface;
+    if (dcc->priv->surface_client_created[surface->id]) {
         return;
     }
 
-    dcc_create_surface(dcc, drawable->surface);
-    display_channel_current_flush(display, drawable->surface);
-    dcc_push_surface_image(dcc, drawable->surface);
+    dcc_create_surface(dcc, surface);
+    display_channel_current_flush(display, surface);
+    dcc_push_surface_image(dcc, surface);
 }
 
 RedDrawablePipeItem::RedDrawablePipeItem(DisplayChannelClient *init_dcc, Drawable *init_drawable):
commit 74c660ddada35c1b3c05caf2506aa4529b2979b6
Author: Frediano Ziglio <fziglio at redhat.com>
Date:   Tue Sep 27 22:10:58 2016 +0100

    Pass surface directly to dcc_push_surface_image
    
    Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
    Acked-by: Victor Toso <victortoso at redhat.com>

diff --git a/server/dcc.cpp b/server/dcc.cpp
index a0161637..bc4b77fd 100644
--- a/server/dcc.cpp
+++ b/server/dcc.cpp
@@ -236,18 +236,14 @@ dcc_add_surface_area_image(DisplayChannelClient *dcc, int surface_id,
     }
 }
 
-void dcc_push_surface_image(DisplayChannelClient *dcc, int surface_id)
+void dcc_push_surface_image(DisplayChannelClient *dcc, RedSurface *surface)
 {
-    DisplayChannel *display;
     SpiceRect area;
-    RedSurface *surface;
 
     if (!dcc) {
         return;
     }
 
-    display = DCC_TO_DC(dcc);
-    surface = &display->priv->surfaces[surface_id];
     if (!surface->context.canvas) {
         return;
     }
@@ -257,7 +253,7 @@ void dcc_push_surface_image(DisplayChannelClient *dcc, int surface_id)
 
     /* not allowing lossy compression because probably, especially if it is a primary surface,
        it combines both "picture-like" areas with areas that are more "artificial"*/
-    dcc_add_surface_area_image(dcc, surface_id, &area, dcc->get_pipe().end(), FALSE);
+    dcc_add_surface_area_image(dcc, surface->id, &area, dcc->get_pipe().end(), false);
 }
 
 static void add_drawable_surface_images(DisplayChannelClient *dcc, Drawable *drawable)
@@ -272,7 +268,7 @@ static void add_drawable_surface_images(DisplayChannelClient *dcc, Drawable *dra
             }
             dcc_create_surface(dcc, surface);
             display_channel_current_flush(display, surface);
-            dcc_push_surface_image(dcc, surface_id);
+            dcc_push_surface_image(dcc, surface);
         }
     }
 
@@ -282,7 +278,7 @@ static void add_drawable_surface_images(DisplayChannelClient *dcc, Drawable *dra
 
     dcc_create_surface(dcc, drawable->surface);
     display_channel_current_flush(display, drawable->surface);
-    dcc_push_surface_image(dcc, drawable->surface->id);
+    dcc_push_surface_image(dcc, drawable->surface);
 }
 
 RedDrawablePipeItem::RedDrawablePipeItem(DisplayChannelClient *init_dcc, Drawable *init_drawable):
@@ -416,7 +412,7 @@ void dcc_start(DisplayChannelClient *dcc)
         display_channel_current_flush(display, &display->priv->surfaces[0]);
         dcc->pipe_add_type(RED_PIPE_ITEM_TYPE_INVAL_PALETTE_CACHE);
         dcc_create_surface(dcc, &display->priv->surfaces[0]);
-        dcc_push_surface_image(dcc, 0);
+        dcc_push_surface_image(dcc, &display->priv->surfaces[0]);
         dcc_push_monitors_config(dcc);
         dcc->pipe_add_empty_msg(SPICE_MSG_DISPLAY_MARK);
         dcc_create_all_streams(dcc);
diff --git a/server/dcc.h b/server/dcc.h
index 4b8abf64..2693d55f 100644
--- a/server/dcc.h
+++ b/server/dcc.h
@@ -115,8 +115,6 @@ void                       dcc_video_stream_agent_clip               (DisplayCha
                                                                       VideoStreamAgent *agent);
 void                       dcc_create_stream                         (DisplayChannelClient *dcc,
                                                                       VideoStream *stream);
-void                       dcc_push_surface_image                    (DisplayChannelClient *dcc,
-                                                                      int surface_id);
 void                       dcc_palette_cache_reset                   (DisplayChannelClient *dcc);
 void                       dcc_palette_cache_palette                 (DisplayChannelClient *dcc,
                                                                       SpicePalette *palette,
@@ -139,6 +137,7 @@ int                        dcc_compress_image                        (DisplayCha
                                                                       compress_send_data_t* o_comp_data);
 
 void dcc_create_surface(DisplayChannelClient *dcc, struct RedSurface *surface);
+void dcc_push_surface_image(DisplayChannelClient *dcc, struct RedSurface *surface);
 bool dcc_clear_surface_drawables_from_pipe(DisplayChannelClient *dcc,
                                            RedSurface *surface, bool wait_if_used);
 void dcc_add_surface_area_image(DisplayChannelClient *dcc, int surface_id,
diff --git a/server/display-channel.cpp b/server/display-channel.cpp
index c7d4d959..aa4bccb2 100644
--- a/server/display-channel.cpp
+++ b/server/display-channel.cpp
@@ -2050,8 +2050,9 @@ static void send_create_surface(DisplayChannel *display, RedSurface *surface, in
 
     FOREACH_DCC(display, dcc) {
         dcc_create_surface(dcc, surface);
-        if (image_ready)
-            dcc_push_surface_image(dcc, surface->id);
+        if (image_ready) {
+            dcc_push_surface_image(dcc, surface);
+        }
     }
 }
 
commit 3b790b8ad6a039a0a0804a96cd81e0c681e6e5b6
Author: Frediano Ziglio <fziglio at redhat.com>
Date:   Tue Sep 27 22:13:30 2016 +0100

    Pass surface directly to dcc_create_surface
    
    Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
    Acked-by: Victor Toso <victortoso at redhat.com>

diff --git a/server/dcc.cpp b/server/dcc.cpp
index 6963e5ac..a0161637 100644
--- a/server/dcc.cpp
+++ b/server/dcc.cpp
@@ -154,10 +154,10 @@ bool dcc_clear_surface_drawables_from_pipe(DisplayChannelClient *dcc, RedSurface
     return dcc->wait_outgoing_item(DISPLAY_CLIENT_SHORT_TIMEOUT);
 }
 
-void dcc_create_surface(DisplayChannelClient *dcc, int surface_id)
+void dcc_create_surface(DisplayChannelClient *dcc, RedSurface *surface)
 {
     DisplayChannel *display;
-    RedSurface *surface;
+    uint32_t surface_id = surface->id;
     uint32_t flags;
 
     if (!dcc) {
@@ -165,14 +165,13 @@ void dcc_create_surface(DisplayChannelClient *dcc, int surface_id)
     }
 
     display = DCC_TO_DC(dcc);
-    flags = is_primary_surface_id(display, surface_id) ? SPICE_SURFACE_FLAGS_PRIMARY : 0;
+    flags = is_primary_surface(display, surface) ? SPICE_SURFACE_FLAGS_PRIMARY : 0;
 
     /* don't send redundant create surface commands to client */
     if (display->get_during_target_migrate() ||
         dcc->priv->surface_client_created[surface_id]) {
         return;
     }
-    surface = &display->priv->surfaces[surface_id];
     auto create = red::make_shared<RedSurfaceCreateItem>(surface_id, surface->context.width,
                                                          surface->context.height,
                                                          surface->context.format, flags);
@@ -271,7 +270,7 @@ static void add_drawable_surface_images(DisplayChannelClient *dcc, Drawable *dra
             if (dcc->priv->surface_client_created[surface_id]) {
                 continue;
             }
-            dcc_create_surface(dcc, surface_id);
+            dcc_create_surface(dcc, surface);
             display_channel_current_flush(display, surface);
             dcc_push_surface_image(dcc, surface_id);
         }
@@ -281,7 +280,7 @@ static void add_drawable_surface_images(DisplayChannelClient *dcc, Drawable *dra
         return;
     }
 
-    dcc_create_surface(dcc, drawable->surface->id);
+    dcc_create_surface(dcc, drawable->surface);
     display_channel_current_flush(display, drawable->surface);
     dcc_push_surface_image(dcc, drawable->surface->id);
 }
@@ -416,7 +415,7 @@ void dcc_start(DisplayChannelClient *dcc)
     if (display->priv->surfaces[0].context.canvas) {
         display_channel_current_flush(display, &display->priv->surfaces[0]);
         dcc->pipe_add_type(RED_PIPE_ITEM_TYPE_INVAL_PALETTE_CACHE);
-        dcc_create_surface(dcc, 0);
+        dcc_create_surface(dcc, &display->priv->surfaces[0]);
         dcc_push_surface_image(dcc, 0);
         dcc_push_monitors_config(dcc);
         dcc->pipe_add_empty_msg(SPICE_MSG_DISPLAY_MARK);
diff --git a/server/dcc.h b/server/dcc.h
index baacc56d..4b8abf64 100644
--- a/server/dcc.h
+++ b/server/dcc.h
@@ -115,8 +115,6 @@ void                       dcc_video_stream_agent_clip               (DisplayCha
                                                                       VideoStreamAgent *agent);
 void                       dcc_create_stream                         (DisplayChannelClient *dcc,
                                                                       VideoStream *stream);
-void                       dcc_create_surface                        (DisplayChannelClient *dcc,
-                                                                      int surface_id);
 void                       dcc_push_surface_image                    (DisplayChannelClient *dcc,
                                                                       int surface_id);
 void                       dcc_palette_cache_reset                   (DisplayChannelClient *dcc);
@@ -140,6 +138,7 @@ int                        dcc_compress_image                        (DisplayCha
                                                                       int can_lossy,
                                                                       compress_send_data_t* o_comp_data);
 
+void dcc_create_surface(DisplayChannelClient *dcc, struct RedSurface *surface);
 bool dcc_clear_surface_drawables_from_pipe(DisplayChannelClient *dcc,
                                            RedSurface *surface, bool wait_if_used);
 void dcc_add_surface_area_image(DisplayChannelClient *dcc, int surface_id,
diff --git a/server/display-channel-private.h b/server/display-channel-private.h
index 975f7bd4..067aad29 100644
--- a/server/display-channel-private.h
+++ b/server/display-channel-private.h
@@ -336,11 +336,6 @@ static inline int has_shadow(RedDrawable *drawable)
     return drawable->type == QXL_COPY_BITS;
 }
 
-static inline bool is_primary_surface_id(DisplayChannel *display, uint32_t surface_id)
-{
-    return surface_id == 0;
-}
-
 static inline bool is_primary_surface(DisplayChannel *display, const RedSurface *surface)
 {
     return surface->id == 0;
diff --git a/server/display-channel.cpp b/server/display-channel.cpp
index 2c968a8c..c7d4d959 100644
--- a/server/display-channel.cpp
+++ b/server/display-channel.cpp
@@ -2044,14 +2044,14 @@ void display_channel_destroy_surfaces(DisplayChannel *display)
     display_channel_free_glz_drawables(display);
 }
 
-static void send_create_surface(DisplayChannel *display, int surface_id, int image_ready)
+static void send_create_surface(DisplayChannel *display, RedSurface *surface, int image_ready)
 {
     DisplayChannelClient *dcc;
 
     FOREACH_DCC(display, dcc) {
-        dcc_create_surface(dcc, surface_id);
+        dcc_create_surface(dcc, surface);
         if (image_ready)
-            dcc_push_surface_image(dcc, surface_id);
+            dcc_push_surface_image(dcc, surface->id);
     }
 }
 
@@ -2123,8 +2123,9 @@ void display_channel_create_surface(DisplayChannel *display, uint32_t surface_id
     }
 
     spice_return_if_fail(surface->context.canvas);
-    if (send_client)
-        send_create_surface(display, surface_id, data_is_valid);
+    if (send_client) {
+        send_create_surface(display, surface, data_is_valid);
+    }
 }
 
 void DisplayChannelClient::handle_migrate_flush_mark()
commit 78bc3a637a795b3701f39062a661aa90727fb2a8
Author: Frediano Ziglio <fziglio at redhat.com>
Date:   Tue Sep 27 22:15:47 2016 +0100

    New function to pass surface directly to display_channel_draw
    
    Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
    Acked-by: Victor Toso <victortoso at redhat.com>

diff --git a/server/display-channel.cpp b/server/display-channel.cpp
index 83d5d272..2c968a8c 100644
--- a/server/display-channel.cpp
+++ b/server/display-channel.cpp
@@ -56,6 +56,8 @@ DisplayChannel::~DisplayChannel()
 static void drawable_draw(DisplayChannel *display, Drawable *drawable);
 static Drawable *display_channel_drawable_try_new(DisplayChannel *display,
                                                   uint32_t process_commands_generation);
+static void display_channel_surface_draw(DisplayChannel *display, RedSurface *surface,
+                                         const SpiceRect *area);
 
 uint32_t display_channel_generate_uid(DisplayChannel *display)
 {
@@ -1168,7 +1170,8 @@ static void handle_self_bitmap(DisplayChannel *display, Drawable *drawable)
     image->u.bitmap.data = spice_chunks_new_linear(dest, height * dest_stride);
     image->u.bitmap.data->flags |= SPICE_CHUNKS_FLAGS_FREE;
 
-    display_channel_draw(display, &red_drawable->self_bitmap_area, drawable->surface->id);
+    display_channel_surface_draw(display, drawable->surface,
+                                 &red_drawable->self_bitmap_area);
     surface_read_bits(display, drawable->surface,
         &red_drawable->self_bitmap_area, dest, dest_stride);
 
@@ -1230,7 +1233,8 @@ static void draw_depend_on_me(DisplayChannel *display, RedSurface *surface)
         Drawable *drawable;
         DependItem *depended_item = SPICE_CONTAINEROF(ring_item, DependItem, ring_item);
         drawable = depended_item->drawable;
-        display_channel_draw(display, &drawable->red_drawable->bbox, drawable->surface->id);
+        display_channel_surface_draw(display, drawable->surface,
+                                     &drawable->red_drawable->bbox);
     }
 }
 
@@ -1637,7 +1641,8 @@ static void drawable_deps_draw(DisplayChannel *display, Drawable *drawable)
         RedSurface *surface = drawable->surface_deps[x];
         if (surface && drawable->depend_items[x].drawable) {
             depended_item_remove(&drawable->depend_items[x]);
-            display_channel_draw(display, &drawable->red_drawable->surfaces_rects[x], surface->id);
+            display_channel_surface_draw(display, surface,
+                                         &drawable->red_drawable->surfaces_rects[x]);
         }
     }
 }
@@ -1906,7 +1911,6 @@ void display_channel_draw_until(DisplayChannel *display, const SpiceRect *area,
 void display_channel_draw(DisplayChannel *display, const SpiceRect *area, int surface_id)
 {
     RedSurface *surface;
-    Drawable *last;
 
     spice_return_if_fail(surface_id >= 0 && surface_id < NUM_SURFACES);
     spice_return_if_fail(area);
@@ -1915,6 +1919,14 @@ void display_channel_draw(DisplayChannel *display, const SpiceRect *area, int su
 
     surface = &display->priv->surfaces[surface_id];
 
+    display_channel_surface_draw(display, surface, area);
+}
+
+static void display_channel_surface_draw(DisplayChannel *display, RedSurface *surface,
+                                         const SpiceRect *area)
+{
+    Drawable *last;
+
     last = current_find_intersects_rect(&surface->current_list, nullptr, area);
     if (last)
         draw_until(display, surface, last);
@@ -1952,9 +1964,9 @@ void display_channel_update(DisplayChannel *display,
     }
 
     red_get_rect_ptr(&rect, area);
-    display_channel_draw(display, &rect, surface_id);
-
     surface = &display->priv->surfaces[surface_id];
+    display_channel_surface_draw(display, surface, &rect);
+
     if (*qxl_dirty_rects == nullptr) {
         *num_dirty_rects = pixman_region32_n_rects(&surface->draw_dirty_region);
         *qxl_dirty_rects = g_new0(QXLRect, *num_dirty_rects);
@@ -1993,10 +2005,10 @@ void display_channel_destroy_surface_wait(DisplayChannel *display, uint32_t surf
 {
     if (!display_channel_validate_surface(display, surface_id))
         return;
-    if (!display->priv->surfaces[surface_id].context.canvas)
+    RedSurface *surface = &display->priv->surfaces[surface_id];
+    if (!surface->context.canvas)
         return;
 
-    RedSurface *surface = &display->priv->surfaces[surface_id];
     draw_depend_on_me(display, surface);
     /* note that draw_depend_on_me must be called before current_remove_all.
        otherwise "current" will hold items that other drawables may depend on, and then
commit 5a5d1e21b536549cc23ed05d43a73c97bcf9d4ef
Author: Frediano Ziglio <fziglio at redhat.com>
Date:   Tue Sep 27 22:18:06 2016 +0100

    Pass surface directly calling display_channel_current_flush
    
    Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
    Acked-by: Victor Toso <victortoso at redhat.com>

diff --git a/server/dcc.cpp b/server/dcc.cpp
index 7c9f0615..6963e5ac 100644
--- a/server/dcc.cpp
+++ b/server/dcc.cpp
@@ -272,7 +272,7 @@ static void add_drawable_surface_images(DisplayChannelClient *dcc, Drawable *dra
                 continue;
             }
             dcc_create_surface(dcc, surface_id);
-            display_channel_current_flush(display, surface_id);
+            display_channel_current_flush(display, surface);
             dcc_push_surface_image(dcc, surface_id);
         }
     }
@@ -282,7 +282,7 @@ static void add_drawable_surface_images(DisplayChannelClient *dcc, Drawable *dra
     }
 
     dcc_create_surface(dcc, drawable->surface->id);
-    display_channel_current_flush(display, drawable->surface->id);
+    display_channel_current_flush(display, drawable->surface);
     dcc_push_surface_image(dcc, drawable->surface->id);
 }
 
@@ -414,7 +414,7 @@ void dcc_start(DisplayChannelClient *dcc)
     red::shared_ptr<DisplayChannelClient> self(dcc);
     dcc->ack_zero_messages_window();
     if (display->priv->surfaces[0].context.canvas) {
-        display_channel_current_flush(display, 0);
+        display_channel_current_flush(display, &display->priv->surfaces[0]);
         dcc->pipe_add_type(RED_PIPE_ITEM_TYPE_INVAL_PALETTE_CACHE);
         dcc_create_surface(dcc, 0);
         dcc_push_surface_image(dcc, 0);
diff --git a/server/display-channel-private.h b/server/display-channel-private.h
index d1829955..975f7bd4 100644
--- a/server/display-channel-private.h
+++ b/server/display-channel-private.h
@@ -177,7 +177,7 @@ void display_channel_draw_until(DisplayChannel *display,
 GArray* display_channel_get_video_codecs(DisplayChannel *display);
 int display_channel_get_stream_video(DisplayChannel *display);
 void display_channel_current_flush(DisplayChannel *display,
-                                   int surface_id);
+                                   RedSurface *surface);
 uint32_t display_channel_generate_uid(DisplayChannel *display);
 
 int display_channel_get_video_stream_id(DisplayChannel *display, VideoStream *stream);
diff --git a/server/display-channel.cpp b/server/display-channel.cpp
index 6b880bc4..83d5d272 100644
--- a/server/display-channel.cpp
+++ b/server/display-channel.cpp
@@ -1424,7 +1424,7 @@ void display_channel_flush_all_surfaces(DisplayChannel *display)
 
     for (x = 0; x < NUM_SURFACES; ++x) {
         if (display->priv->surfaces[x].context.canvas) {
-            display_channel_current_flush(display, x);
+            display_channel_current_flush(display, &display->priv->surfaces[x]);
         }
     }
 }
@@ -1473,12 +1473,12 @@ static bool free_one_drawable(DisplayChannel *display, int force_glz_free)
     return TRUE;
 }
 
-void display_channel_current_flush(DisplayChannel *display, int surface_id)
+void display_channel_current_flush(DisplayChannel *display, RedSurface *surface)
 {
-    while (!ring_is_empty(&display->priv->surfaces[surface_id].current_list)) {
+    while (!ring_is_empty(&surface->current_list)) {
         free_one_drawable(display, FALSE);
     }
-    current_remove_all(display, &display->priv->surfaces[surface_id]);
+    current_remove_all(display, surface);
 }
 
 void display_channel_free_some(DisplayChannel *display)
commit 58a612a039b85938dbafdb5d56d0efd7443bb88d
Author: Frediano Ziglio <fziglio at redhat.com>
Date:   Tue Sep 27 22:20:03 2016 +0100

    Pass surface directly calling dcc_clear_surface_drawables_from_pipe
    
    Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
    Acked-by: Victor Toso <victortoso at redhat.com>

diff --git a/server/dcc.cpp b/server/dcc.cpp
index 0178b212..7c9f0615 100644
--- a/server/dcc.cpp
+++ b/server/dcc.cpp
@@ -99,22 +99,16 @@ bool dcc_drawable_is_in_pipe(DisplayChannelClient *dcc, Drawable *drawable)
 }
 
 /*
- * Return: TRUE if wait_if_used == FALSE, or otherwise, if all of the pipe items that
+ * Return: true if wait_if_used == false, or otherwise, if all of the pipe items that
  * are related to the surface have been cleared (or sent) from the pipe.
  */
-bool dcc_clear_surface_drawables_from_pipe(DisplayChannelClient *dcc, int surface_id,
-                                           int wait_if_used)
+bool dcc_clear_surface_drawables_from_pipe(DisplayChannelClient *dcc, RedSurface *surface,
+                                           bool wait_if_used)
 {
-    DisplayChannel *display;
-    RedSurface *surface;
-
     spice_return_val_if_fail(dcc != nullptr, TRUE);
-    /* removing the newest drawables that their destination is surface_id and
+    /* removing the newest drawables that their destination is surface and
        no other drawable depends on them */
 
-    display = DCC_TO_DC(dcc);
-    surface = &display->priv->surfaces[surface_id];
-
     auto &pipe = dcc->get_pipe();
     for (auto l = pipe.begin(); l != pipe.end(); ) {
         Drawable *drawable;
@@ -141,16 +135,16 @@ bool dcc_clear_surface_drawables_from_pipe(DisplayChannelClient *dcc, int surfac
             std::find(std::begin(drawable->surface_deps), std::end(drawable->surface_deps),
                       surface) != std::end(drawable->surface_deps);
         if (depend_found) {
-            spice_debug("surface %d dependent item found %p, %p", surface_id, drawable, item);
+            spice_debug("surface %d dependent item found %p, %p", surface->id, drawable, item);
             if (!wait_if_used) {
-                return TRUE;
+                return true;
             }
             return dcc->wait_pipe_item_sent(item_pos, COMMON_CLIENT_TIMEOUT);
         }
     }
 
     if (!wait_if_used) {
-        return TRUE;
+        return true;
     }
 
     /*
diff --git a/server/dcc.h b/server/dcc.h
index 19005579..baacc56d 100644
--- a/server/dcc.h
+++ b/server/dcc.h
@@ -81,6 +81,7 @@ public:
 struct DisplayChannel;
 struct VideoStream;
 struct VideoStreamAgent;
+struct RedSurface;
 
 typedef struct WaitForChannels {
     SpiceMsgWaitForChannels header;
@@ -131,9 +132,6 @@ void                       dcc_append_drawable                       (DisplayCha
 void                       dcc_add_drawable_after                    (DisplayChannelClient *dcc,
                                                                       Drawable *drawable,
                                                                       RedPipeItem *pos);
-bool                       dcc_clear_surface_drawables_from_pipe     (DisplayChannelClient *dcc,
-                                                                      int surface_id,
-                                                                      int wait_if_used);
 bool                       dcc_drawable_is_in_pipe                   (DisplayChannelClient *dcc,
                                                                       Drawable *drawable);
 
@@ -142,6 +140,8 @@ int                        dcc_compress_image                        (DisplayCha
                                                                       int can_lossy,
                                                                       compress_send_data_t* o_comp_data);
 
+bool dcc_clear_surface_drawables_from_pipe(DisplayChannelClient *dcc,
+                                           RedSurface *surface, bool wait_if_used);
 void dcc_add_surface_area_image(DisplayChannelClient *dcc, int surface_id,
                                 SpiceRect *area, RedChannelClient::Pipe::iterator pipe_item_pos,
                                 int can_lossy);
diff --git a/server/display-channel.cpp b/server/display-channel.cpp
index 5299fd71..6b880bc4 100644
--- a/server/display-channel.cpp
+++ b/server/display-channel.cpp
@@ -1965,13 +1965,13 @@ void display_channel_update(DisplayChannel *display,
         region_clear(&surface->draw_dirty_region);
 }
 
-static void clear_surface_drawables_from_pipes(DisplayChannel *display, int surface_id,
+static void clear_surface_drawables_from_pipes(DisplayChannel *display, RedSurface *surface,
                                                int wait_if_used)
 {
     DisplayChannelClient *dcc;
 
     FOREACH_DCC(display, dcc) {
-        if (!dcc_clear_surface_drawables_from_pipe(dcc, surface_id, wait_if_used)) {
+        if (!dcc_clear_surface_drawables_from_pipe(dcc, surface, wait_if_used)) {
             dcc->disconnect();
         }
     }
@@ -1985,7 +1985,7 @@ static void display_channel_destroy_surface(DisplayChannel *display, RedSurface
        otherwise "current" will hold items that other drawables may depend on, and then
        current_remove_all will remove them from the pipe. */
     current_remove_all(display, surface);
-    clear_surface_drawables_from_pipes(display, surface->id, false);
+    clear_surface_drawables_from_pipes(display, surface, false);
     display_channel_surface_unref(display, surface);
 }
 
@@ -2002,7 +2002,7 @@ void display_channel_destroy_surface_wait(DisplayChannel *display, uint32_t surf
        otherwise "current" will hold items that other drawables may depend on, and then
        current_remove_all will remove them from the pipe. */
     current_remove_all(display, surface);
-    clear_surface_drawables_from_pipes(display, surface_id, TRUE);
+    clear_surface_drawables_from_pipes(display, surface, true);
 }
 
 /* called upon device reset */
commit 2b219cace592f0f8473d50c01cc083dd6f449b4a
Author: Frediano Ziglio <fziglio at redhat.com>
Date:   Tue Sep 27 22:22:20 2016 +0100

    Pass surface directly calling draw_depend_on_me and display_channel_destroy_surface
    
    Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
    Acked-by: Victor Toso <victortoso at redhat.com>

diff --git a/server/display-channel.cpp b/server/display-channel.cpp
index ceff2fb0..5299fd71 100644
--- a/server/display-channel.cpp
+++ b/server/display-channel.cpp
@@ -1222,13 +1222,10 @@ static bool handle_surface_deps(DisplayChannel *display, Drawable *drawable)
     return TRUE;
 }
 
-static void draw_depend_on_me(DisplayChannel *display, uint32_t surface_id)
+static void draw_depend_on_me(DisplayChannel *display, RedSurface *surface)
 {
-    RedSurface *surface;
     RingItem *ring_item;
 
-    surface = &display->priv->surfaces[surface_id];
-
     while ((ring_item = ring_get_tail(&surface->depend_on_me))) {
         Drawable *drawable;
         DependItem *depended_item = SPICE_CONTAINEROF(ring_item, DependItem, ring_item);
@@ -1320,7 +1317,6 @@ static Drawable *display_channel_get_drawable(DisplayChannel *display, uint8_t e
  */
 static void display_channel_add_drawable(DisplayChannel *display, Drawable *drawable)
 {
-    int surface_id = drawable->surface->id;
     RedDrawable *red_drawable = drawable->red_drawable.get();
 
     red_drawable->mm_time = reds_get_mm_time();
@@ -1344,7 +1340,7 @@ static void display_channel_add_drawable(DisplayChannel *display, Drawable *draw
         handle_self_bitmap(display, drawable);
     }
 
-    draw_depend_on_me(display, surface_id);
+    draw_depend_on_me(display, drawable->surface);
 
     if (!handle_surface_deps(display, drawable)) {
         return;
@@ -1982,15 +1978,14 @@ static void clear_surface_drawables_from_pipes(DisplayChannel *display, int surf
 }
 
 /* TODO: cleanup/refactor destroy functions */
-static void display_channel_destroy_surface(DisplayChannel *display, uint32_t surface_id)
+static void display_channel_destroy_surface(DisplayChannel *display, RedSurface *surface)
 {
-    RedSurface *surface = &display->priv->surfaces[surface_id];
-    draw_depend_on_me(display, surface_id);
+    draw_depend_on_me(display, surface);
     /* note that draw_depend_on_me must be called before current_remove_all.
        otherwise "current" will hold items that other drawables may depend on, and then
        current_remove_all will remove them from the pipe. */
     current_remove_all(display, surface);
-    clear_surface_drawables_from_pipes(display, surface_id, FALSE);
+    clear_surface_drawables_from_pipes(display, surface->id, false);
     display_channel_surface_unref(display, surface);
 }
 
@@ -2001,11 +1996,12 @@ void display_channel_destroy_surface_wait(DisplayChannel *display, uint32_t surf
     if (!display->priv->surfaces[surface_id].context.canvas)
         return;
 
-    draw_depend_on_me(display, surface_id);
+    RedSurface *surface = &display->priv->surfaces[surface_id];
+    draw_depend_on_me(display, surface);
     /* note that draw_depend_on_me must be called before current_remove_all.
        otherwise "current" will hold items that other drawables may depend on, and then
        current_remove_all will remove them from the pipe. */
-    current_remove_all(display, &display->priv->surfaces[surface_id]);
+    current_remove_all(display, surface);
     clear_surface_drawables_from_pipes(display, surface_id, TRUE);
 }
 
@@ -2267,7 +2263,7 @@ void display_channel_process_surface_cmd(DisplayChannel *display,
             break;
         }
         surface->destroy_cmd = surface_cmd;
-        display_channel_destroy_surface(display, surface_id);
+        display_channel_destroy_surface(display, surface);
         break;
     default:
         spice_warn_if_reached();
commit 493077d0808909d9bb5d9f5fee2ed3dff51c3462
Author: Frediano Ziglio <fziglio at redhat.com>
Date:   Tue Sep 27 22:24:29 2016 +0100

    Pass surface directly in is_primary_surface
    
    Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
    Acked-by: Victor Toso <victortoso at redhat.com>

diff --git a/server/dcc.cpp b/server/dcc.cpp
index b92eb3fb..0178b212 100644
--- a/server/dcc.cpp
+++ b/server/dcc.cpp
@@ -171,7 +171,7 @@ void dcc_create_surface(DisplayChannelClient *dcc, int surface_id)
     }
 
     display = DCC_TO_DC(dcc);
-    flags = is_primary_surface(display, surface_id) ? SPICE_SURFACE_FLAGS_PRIMARY : 0;
+    flags = is_primary_surface_id(display, surface_id) ? SPICE_SURFACE_FLAGS_PRIMARY : 0;
 
     /* don't send redundant create surface commands to client */
     if (display->get_during_target_migrate() ||
@@ -226,7 +226,7 @@ dcc_add_surface_area_image(DisplayChannelClient *dcc, int surface_id,
 
     /* For 32bit non-primary surfaces we need to keep any non-zero
        high bytes as the surface may be used as source to an alpha_blend */
-    if (!is_primary_surface(display, surface_id) &&
+    if (!is_primary_surface(display, surface) &&
         item->image_format == SPICE_BITMAP_FMT_32BIT &&
         rgb32_data_has_alpha(item->width, item->height, item->stride, item->data, &all_set)) {
         if (all_set) {
diff --git a/server/display-channel-private.h b/server/display-channel-private.h
index 68904cc2..d1829955 100644
--- a/server/display-channel-private.h
+++ b/server/display-channel-private.h
@@ -336,12 +336,14 @@ static inline int has_shadow(RedDrawable *drawable)
     return drawable->type == QXL_COPY_BITS;
 }
 
-static inline int is_primary_surface(DisplayChannel *display, uint32_t surface_id)
+static inline bool is_primary_surface_id(DisplayChannel *display, uint32_t surface_id)
 {
-    if (surface_id == 0) {
-        return TRUE;
-    }
-    return FALSE;
+    return surface_id == 0;
+}
+
+static inline bool is_primary_surface(DisplayChannel *display, const RedSurface *surface)
+{
+    return surface->id == 0;
 }
 
 static inline void region_add_clip_rects(QRegion *rgn, SpiceClipRects *data)
diff --git a/server/display-channel.cpp b/server/display-channel.cpp
index 0cb64006..ceff2fb0 100644
--- a/server/display-channel.cpp
+++ b/server/display-channel.cpp
@@ -234,7 +234,7 @@ static void display_channel_surface_unref(DisplayChannel *display, RedSurface *s
     }
 
     // only primary surface streams are supported
-    if (is_primary_surface(display, surface->id)) {
+    if (is_primary_surface(display, surface)) {
         stop_streams(display);
     }
     spice_assert(surface->context.canvas);
@@ -274,7 +274,7 @@ static void streams_update_visible_region(DisplayChannel *display, Drawable *dra
         return;
     }
 
-    if (!is_primary_surface(display, drawable->surface->id)) {
+    if (!is_primary_surface(display, drawable->surface)) {
         return;
     }
 
@@ -817,7 +817,7 @@ static bool current_add_with_shadow(DisplayChannel *display, Ring *ring, Drawabl
     // for now putting them on root.
 
     // only primary surface streams are supported
-    if (is_primary_surface(display, item->surface->id)) {
+    if (is_primary_surface(display, item->surface)) {
         video_stream_detach_behind(display, &shadow->base.rgn, nullptr);
     }
 
@@ -838,7 +838,7 @@ static bool current_add_with_shadow(DisplayChannel *display, Ring *ring, Drawabl
         region_destroy(&exclude_rgn);
         streams_update_visible_region(display, item);
     } else {
-        if (is_primary_surface(display, item->surface->id)) {
+        if (is_primary_surface(display, item->surface)) {
             video_stream_detach_behind(display, &item->tree_item.base.rgn, item);
         }
     }
@@ -1035,7 +1035,7 @@ static bool current_add(DisplayChannel *display, Ring *ring, Drawable *drawable)
          * video_stream_detach_behind
          */
         current_add_drawable(display, drawable, ring);
-        if (is_primary_surface(display, drawable->surface->id)) {
+        if (is_primary_surface(display, drawable->surface)) {
             video_stream_detach_behind(display, &drawable->tree_item.base.rgn, drawable);
         }
     }
@@ -1053,7 +1053,7 @@ static bool drawable_can_stream(DisplayChannel *display, Drawable *drawable)
         return FALSE;
     }
 
-    if (!is_primary_surface(display, drawable->surface->id)) {
+    if (!is_primary_surface(display, drawable->surface)) {
         return FALSE;
     }
 
@@ -1174,7 +1174,7 @@ static void handle_self_bitmap(DisplayChannel *display, Drawable *drawable)
 
     /* For 32bit non-primary surfaces we need to keep any non-zero
        high bytes as the surface may be used as source to an alpha_blend */
-    if (!is_primary_surface(display, drawable->surface->id) &&
+    if (!is_primary_surface(display, drawable->surface) &&
         image->u.bitmap.format == SPICE_BITMAP_FMT_32BIT &&
         rgb32_data_has_alpha(width, height, dest_stride, dest, &all_set)) {
         if (all_set) {
commit f418bbb522292e7d411ca62b681a324d3868be54
Author: Frediano Ziglio <fziglio at redhat.com>
Date:   Tue Sep 27 22:27:16 2016 +0100

    Pass surface directly to display_channel_surface_unref
    
    Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
    Acked-by: Victor Toso <victortoso at redhat.com>

diff --git a/server/display-channel.cpp b/server/display-channel.cpp
index e5471313..0cb64006 100644
--- a/server/display-channel.cpp
+++ b/server/display-channel.cpp
@@ -225,9 +225,8 @@ static void stop_streams(DisplayChannel *display)
     memset(display->priv->items_trace, 0, sizeof(display->priv->items_trace));
 }
 
-void display_channel_surface_unref(DisplayChannel *display, uint32_t surface_id)
+static void display_channel_surface_unref(DisplayChannel *display, RedSurface *surface)
 {
-    RedSurface *surface = &display->priv->surfaces[surface_id];
     DisplayChannelClient *dcc;
 
     if (--surface->refs != 0) {
@@ -235,7 +234,7 @@ void display_channel_surface_unref(DisplayChannel *display, uint32_t surface_id)
     }
 
     // only primary surface streams are supported
-    if (is_primary_surface(display, surface_id)) {
+    if (is_primary_surface(display, surface->id)) {
         stop_streams(display);
     }
     spice_assert(surface->context.canvas);
@@ -247,7 +246,7 @@ void display_channel_surface_unref(DisplayChannel *display, uint32_t surface_id)
     region_destroy(&surface->draw_dirty_region);
     surface->context.canvas = nullptr;
     FOREACH_DCC(display, dcc) {
-        dcc_destroy_surface(dcc, surface_id);
+        dcc_destroy_surface(dcc, surface->id);
     }
 
     spice_warn_if_fail(ring_is_empty(&surface->depend_on_me));
@@ -260,6 +259,11 @@ gboolean display_channel_surface_has_canvas(DisplayChannel *display,
     return display->priv->surfaces[surface_id].context.canvas != nullptr;
 }
 
+void display_channel_surface_id_unref(DisplayChannel *display, uint32_t surface_id)
+{
+    display_channel_surface_unref(display, &display->priv->surfaces[surface_id]);
+}
+
 static void streams_update_visible_region(DisplayChannel *display, Drawable *drawable)
 {
     Ring *ring;
@@ -1601,7 +1605,7 @@ static void drawable_unref_surface_deps(DisplayChannel *display, Drawable *drawa
         if (surface == nullptr) {
             continue;
         }
-        display_channel_surface_unref(display, surface->id);
+        display_channel_surface_unref(display, surface);
     }
 }
 
@@ -1622,7 +1626,7 @@ void drawable_unref(Drawable *drawable)
 
     drawable_remove_dependencies(drawable);
     drawable_unref_surface_deps(display, drawable);
-    display_channel_surface_unref(display, drawable->surface->id);
+    display_channel_surface_unref(display, drawable->surface);
 
     glz_retention_detach_drawables(&drawable->glz_retention);
 
@@ -1980,13 +1984,14 @@ static void clear_surface_drawables_from_pipes(DisplayChannel *display, int surf
 /* TODO: cleanup/refactor destroy functions */
 static void display_channel_destroy_surface(DisplayChannel *display, uint32_t surface_id)
 {
+    RedSurface *surface = &display->priv->surfaces[surface_id];
     draw_depend_on_me(display, surface_id);
     /* note that draw_depend_on_me must be called before current_remove_all.
        otherwise "current" will hold items that other drawables may depend on, and then
        current_remove_all will remove them from the pipe. */
-    current_remove_all(display, &display->priv->surfaces[surface_id]);
+    current_remove_all(display, surface);
     clear_surface_drawables_from_pipes(display, surface_id, FALSE);
-    display_channel_surface_unref(display, surface_id);
+    display_channel_surface_unref(display, surface);
 }
 
 void display_channel_destroy_surface_wait(DisplayChannel *display, uint32_t surface_id)
@@ -2016,7 +2021,7 @@ void display_channel_destroy_surfaces(DisplayChannel *display)
         if (display->priv->surfaces[i].context.canvas) {
             display_channel_destroy_surface_wait(display, i);
             if (display->priv->surfaces[i].context.canvas) {
-                display_channel_surface_unref(display, i);
+                display_channel_surface_unref(display, &display->priv->surfaces[i]);
             }
             spice_assert(!display->priv->surfaces[i].context.canvas);
         }
diff --git a/server/display-channel.h b/server/display-channel.h
index 5366c052..591e15a3 100644
--- a/server/display-channel.h
+++ b/server/display-channel.h
@@ -94,6 +94,7 @@ display_channel_new(RedsState *reds, QXLInstance *qxl,
                     int migrate, int stream_video,
                     GArray *video_codecs,
                     uint32_t n_surfaces);
+void display_channel_surface_id_unref(DisplayChannel *display, uint32_t surface_id);
 void                       display_channel_create_surface            (DisplayChannel *display, uint32_t surface_id,
                                                                       uint32_t width, uint32_t height,
                                                                       int32_t stride, uint32_t format, void *line_0,
@@ -115,8 +116,6 @@ void                       display_channel_set_video_codecs          (DisplayCha
 int                        display_channel_get_streams_timeout       (DisplayChannel *display);
 void                       display_channel_compress_stats_print      (DisplayChannel *display);
 void                       display_channel_compress_stats_reset      (DisplayChannel *display);
-void                       display_channel_surface_unref             (DisplayChannel *display,
-                                                                      uint32_t surface_id);
 bool                       display_channel_wait_for_migrate_data     (DisplayChannel *display);
 void                       display_channel_flush_all_surfaces        (DisplayChannel *display);
 void                       display_channel_free_glz_drawables_to_free(DisplayChannel *display);
diff --git a/server/red-worker.cpp b/server/red-worker.cpp
index d52f375a..b5abb82d 100644
--- a/server/red-worker.cpp
+++ b/server/red-worker.cpp
@@ -448,7 +448,7 @@ static void destroy_primary_surface(RedWorker *worker, uint32_t surface_id)
 
     flush_all_qxl_commands(worker);
     display_channel_destroy_surface_wait(display, 0);
-    display_channel_surface_unref(display, 0);
+    display_channel_surface_id_unref(display, 0);
 
     /* FIXME: accessing private data only for warning purposes...
     spice_warn_if_fail(ring_is_empty(&display->streams));
commit 977e72eb480a7cc46a19ad230181ecc0ef878d95
Author: Frediano Ziglio <fziglio at redhat.com>
Date:   Thu Sep 15 10:41:22 2016 +0100

    Pass surface directly to current_remove_all
    
    Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
    Acked-by: Victor Toso <victortoso at redhat.com>

diff --git a/server/display-channel.cpp b/server/display-channel.cpp
index c634785b..e5471313 100644
--- a/server/display-channel.cpp
+++ b/server/display-channel.cpp
@@ -436,9 +436,9 @@ static void current_remove(DisplayChannel *display, TreeItem *item)
     }
 }
 
-static void current_remove_all(DisplayChannel *display, int surface_id)
+static void current_remove_all(DisplayChannel *display, RedSurface *surface)
 {
-    Ring *ring = &display->priv->surfaces[surface_id].current;
+    Ring *ring = &surface->current;
     RingItem *ring_item;
 
     while ((ring_item = ring_get_head(ring))) {
@@ -1478,7 +1478,7 @@ void display_channel_current_flush(DisplayChannel *display, int surface_id)
     while (!ring_is_empty(&display->priv->surfaces[surface_id].current_list)) {
         free_one_drawable(display, FALSE);
     }
-    current_remove_all(display, surface_id);
+    current_remove_all(display, &display->priv->surfaces[surface_id]);
 }
 
 void display_channel_free_some(DisplayChannel *display)
@@ -1984,7 +1984,7 @@ static void display_channel_destroy_surface(DisplayChannel *display, uint32_t su
     /* note that draw_depend_on_me must be called before current_remove_all.
        otherwise "current" will hold items that other drawables may depend on, and then
        current_remove_all will remove them from the pipe. */
-    current_remove_all(display, surface_id);
+    current_remove_all(display, &display->priv->surfaces[surface_id]);
     clear_surface_drawables_from_pipes(display, surface_id, FALSE);
     display_channel_surface_unref(display, surface_id);
 }
@@ -2000,7 +2000,7 @@ void display_channel_destroy_surface_wait(DisplayChannel *display, uint32_t surf
     /* note that draw_depend_on_me must be called before current_remove_all.
        otherwise "current" will hold items that other drawables may depend on, and then
        current_remove_all will remove them from the pipe. */
-    current_remove_all(display, surface_id);
+    current_remove_all(display, &display->priv->surfaces[surface_id]);
     clear_surface_drawables_from_pipes(display, surface_id, TRUE);
 }
 
commit 19eb9995b2567671f2a539d5eacf8550d297e311
Author: Frediano Ziglio <fziglio at redhat.com>
Date:   Tue Sep 27 22:29:16 2016 +0100

    Pass surface directly for surface_read_bits
    
    Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
    Acked-by: Victor Toso <victortoso at redhat.com>

diff --git a/server/display-channel.cpp b/server/display-channel.cpp
index 2ea6801f..c634785b 100644
--- a/server/display-channel.cpp
+++ b/server/display-channel.cpp
@@ -1120,11 +1120,10 @@ static void drawable_ref_surface_deps(DisplayChannel *display, Drawable *drawabl
     }
 }
 
-static void surface_read_bits(DisplayChannel *display, int surface_id,
+static void surface_read_bits(DisplayChannel *display, RedSurface *surface,
                               const SpiceRect *area, uint8_t *dest, int dest_stride)
 {
     SpiceCanvas *canvas;
-    RedSurface *surface = &display->priv->surfaces[surface_id];
 
     canvas = surface->context.canvas;
     canvas->ops->read_bits(canvas, dest, dest_stride, area);
@@ -1166,7 +1165,7 @@ static void handle_self_bitmap(DisplayChannel *display, Drawable *drawable)
     image->u.bitmap.data->flags |= SPICE_CHUNKS_FLAGS_FREE;
 
     display_channel_draw(display, &red_drawable->self_bitmap_area, drawable->surface->id);
-    surface_read_bits(display, drawable->surface->id,
+    surface_read_bits(display, drawable->surface,
         &red_drawable->self_bitmap_area, dest, dest_stride);
 
     /* For 32bit non-primary surfaces we need to keep any non-zero
commit 6933c6dd140d270525dddbab202ff1e6858c93a9
Author: Frediano Ziglio <fziglio at redhat.com>
Date:   Thu Sep 15 10:27:36 2016 +0100

    Use direct pointers for surface and surface dependencies from Drawable
    
    As we use reference counting is more direct to use direct pointers.
    Also this will allow to have a surface in a "released state"
    reducing the complexity of code destroying a surface.
    
    Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
    Acked-by: Victor Toso <victortoso at redhat.com>

diff --git a/server/dcc-send.cpp b/server/dcc-send.cpp
index b43b7a1f..8c1aa6e0 100644
--- a/server/dcc-send.cpp
+++ b/server/dcc-send.cpp
@@ -307,7 +307,7 @@ static void fill_base(SpiceMarshaller *base_marshaller, Drawable *drawable)
 {
     SpiceMsgDisplayBase base;
 
-    base.surface_id = drawable->surface_id;
+    base.surface_id = drawable->surface->id;
     base.box = drawable->red_drawable->bbox;
     base.clip = drawable->red_drawable->clip;
 
@@ -560,7 +560,7 @@ static void surface_lossy_region_update(DisplayChannelClient *dcc,
         return;
     }
 
-    surface_lossy_region = &dcc->priv->surface_client_lossy_region[item->surface_id];
+    surface_lossy_region = &dcc->priv->surface_client_lossy_region[item->surface->id];
     drawable = item->red_drawable.get();
 
     if (drawable->clip.type == SPICE_CLIP_TYPE_RECTS ) {
@@ -654,7 +654,10 @@ static bool drawable_depends_on_areas(Drawable *drawable, int surface_ids[],
         int dep_surface_id;
 
          for (x = 0; x < 3; ++x) {
-            dep_surface_id = drawable->surface_deps[x];
+            if (!drawable->surface_deps[x]) {
+                continue;
+            }
+            dep_surface_id = drawable->surface_deps[x]->id;
             if (dep_surface_id == surface_ids[i]) {
                 if (rect_intersects(&surface_areas[i], &red_drawable->surfaces_rects[x])) {
                     return TRUE;
@@ -821,7 +824,7 @@ static void red_lossy_marshall_qxl_draw_fill(DisplayChannelClient *dcc,
     brush_is_lossy = is_brush_lossy(dcc, &drawable->u.fill.brush,
                                     &brush_bitmap_data);
     if (!dest_allowed_lossy) {
-        dest_is_lossy = is_surface_area_lossy(dcc, item->surface_id, &drawable->bbox,
+        dest_is_lossy = is_surface_area_lossy(dcc, item->surface->id, &drawable->bbox,
                                               &dest_lossy_area);
     }
 
@@ -838,7 +841,7 @@ static void red_lossy_marshall_qxl_draw_fill(DisplayChannelClient *dcc,
         int num_resend = 0;
 
         if (dest_is_lossy) {
-            resend_surface_ids[num_resend] = item->surface_id;
+            resend_surface_ids[num_resend] = item->surface->id;
             resend_areas[num_resend] = &dest_lossy_area;
             num_resend++;
         }
@@ -1131,7 +1134,7 @@ static void red_lossy_marshall_qxl_copy_bits(DisplayChannelClient *dcc,
     src_rect.right = drawable->bbox.right + horz_offset;
     src_rect.bottom = drawable->bbox.bottom + vert_offset;
 
-    src_is_lossy = is_surface_area_lossy(dcc, item->surface_id,
+    src_is_lossy = is_surface_area_lossy(dcc, item->surface->id,
                                          &src_rect, &src_lossy_area);
 
     surface_lossy_region_update(dcc, item, FALSE, src_is_lossy);
@@ -1191,7 +1194,7 @@ static void red_lossy_marshall_qxl_draw_blend(DisplayChannelClient *dcc,
         }
 
         if (dest_is_lossy) {
-            resend_surface_ids[num_resend] = item->surface_id;
+            resend_surface_ids[num_resend] = item->surface->id;
             resend_areas[num_resend] = &dest_lossy_area;
             num_resend++;
         }
@@ -1365,7 +1368,7 @@ static void red_lossy_marshall_qxl_draw_rop3(DisplayChannelClient *dcc,
         }
 
         if (dest_is_lossy) {
-            resend_surface_ids[num_resend] = item->surface_id;
+            resend_surface_ids[num_resend] = item->surface->id;
             resend_areas[num_resend] = &dest_lossy_area;
             num_resend++;
         }
@@ -1444,7 +1447,7 @@ static void red_lossy_marshall_qxl_draw_composite(DisplayChannelClient *dcc,
         }
 
         if (dest_is_lossy) {
-            resend_surface_ids[num_resend] = item->surface_id;
+            resend_surface_ids[num_resend] = item->surface->id;
             resend_areas[num_resend] = &dest_lossy_area;
             num_resend++;
         }
diff --git a/server/dcc.cpp b/server/dcc.cpp
index 74e4a821..b92eb3fb 100644
--- a/server/dcc.cpp
+++ b/server/dcc.cpp
@@ -105,10 +105,16 @@ bool dcc_drawable_is_in_pipe(DisplayChannelClient *dcc, Drawable *drawable)
 bool dcc_clear_surface_drawables_from_pipe(DisplayChannelClient *dcc, int surface_id,
                                            int wait_if_used)
 {
+    DisplayChannel *display;
+    RedSurface *surface;
+
     spice_return_val_if_fail(dcc != nullptr, TRUE);
     /* removing the newest drawables that their destination is surface_id and
        no other drawable depends on them */
 
+    display = DCC_TO_DC(dcc);
+    surface = &display->priv->surfaces[surface_id];
+
     auto &pipe = dcc->get_pipe();
     for (auto l = pipe.begin(); l != pipe.end(); ) {
         Drawable *drawable;
@@ -126,14 +132,14 @@ bool dcc_clear_surface_drawables_from_pipe(DisplayChannelClient *dcc, int surfac
             continue;
         }
 
-        if (drawable->surface_id == surface_id) {
+        if (drawable->surface == surface) {
             l = pipe.erase(item_pos);
             continue;
         }
 
         auto depend_found =
             std::find(std::begin(drawable->surface_deps), std::end(drawable->surface_deps),
-                      surface_id) != std::end(drawable->surface_deps);
+                      surface) != std::end(drawable->surface_deps);
         if (depend_found) {
             spice_debug("surface %d dependent item found %p, %p", surface_id, drawable, item);
             if (!wait_if_used) {
@@ -265,8 +271,9 @@ static void add_drawable_surface_images(DisplayChannelClient *dcc, Drawable *dra
 {
     DisplayChannel *display = DCC_TO_DC(dcc);
 
-    for (const auto surface_id : drawable->surface_deps) {
-        if (surface_id != -1) {
+    for (const auto surface : drawable->surface_deps) {
+        if (surface != nullptr) {
+            const auto surface_id = surface->id;
             if (dcc->priv->surface_client_created[surface_id]) {
                 continue;
             }
@@ -276,13 +283,13 @@ static void add_drawable_surface_images(DisplayChannelClient *dcc, Drawable *dra
         }
     }
 
-    if (dcc->priv->surface_client_created[drawable->surface_id]) {
+    if (dcc->priv->surface_client_created[drawable->surface->id]) {
         return;
     }
 
-    dcc_create_surface(dcc, drawable->surface_id);
-    display_channel_current_flush(display, drawable->surface_id);
-    dcc_push_surface_image(dcc, drawable->surface_id);
+    dcc_create_surface(dcc, drawable->surface->id);
+    display_channel_current_flush(display, drawable->surface->id);
+    dcc_push_surface_image(dcc, drawable->surface->id);
 }
 
 RedDrawablePipeItem::RedDrawablePipeItem(DisplayChannelClient *init_dcc, Drawable *init_drawable):
diff --git a/server/display-channel-private.h b/server/display-channel-private.h
index 49026663..68904cc2 100644
--- a/server/display-channel-private.h
+++ b/server/display-channel-private.h
@@ -35,8 +35,9 @@ typedef struct DrawContext {
     void *line_0;
 } DrawContext;
 
-typedef struct RedSurface {
+struct RedSurface {
     uint32_t refs;
+    uint16_t id;
     /* A Ring representing a hierarchical tree structure. This tree includes
      * DrawItems, Containers, and Shadows. It is used to efficiently determine
      * which drawables overlap, and to exclude regions of drawables that are
@@ -58,7 +59,7 @@ typedef struct RedSurface {
     /* QEMU expects the guest data for the command to be valid as long as the
      * surface is valid */
     red::shared_ptr<const RedSurfaceCmd> destroy_cmd;
-} RedSurface;
+};
 
 typedef struct MonitorsConfig {
     int refs;
@@ -320,16 +321,14 @@ static inline int is_same_drawable(Drawable *d1, Drawable *d2)
     }
 }
 
-static inline int is_drawable_independent_from_surfaces(Drawable *drawable)
+static inline bool is_drawable_independent_from_surfaces(const Drawable *drawable)
 {
-    int x;
-
-    for (x = 0; x < 3; ++x) {
-        if (drawable->surface_deps[x] != -1) {
-            return FALSE;
+    for (const auto& surface : drawable->surface_deps) {
+        if (surface) {
+            return false;
         }
     }
-    return TRUE;
+    return true;
 }
 
 static inline int has_shadow(RedDrawable *drawable)
diff --git a/server/display-channel.cpp b/server/display-channel.cpp
index 5093ae03..2ea6801f 100644
--- a/server/display-channel.cpp
+++ b/server/display-channel.cpp
@@ -270,7 +270,7 @@ static void streams_update_visible_region(DisplayChannel *display, Drawable *dra
         return;
     }
 
-    if (!is_primary_surface(display, drawable->surface_id)) {
+    if (!is_primary_surface(display, drawable->surface->id)) {
         return;
     }
 
@@ -351,9 +351,8 @@ static void current_add_drawable(DisplayChannel *display,
                                  Drawable *drawable, RingItem *pos)
 {
     RedSurface *surface;
-    uint32_t surface_id = drawable->surface_id;
 
-    surface = &display->priv->surfaces[surface_id];
+    surface = drawable->surface;
     ring_add_after(&drawable->tree_item.base.siblings_link, pos);
     ring_add(&display->priv->current_list, &drawable->list_link);
     ring_add(&surface->current_list, &drawable->surface_list_link);
@@ -814,7 +813,7 @@ static bool current_add_with_shadow(DisplayChannel *display, Ring *ring, Drawabl
     // for now putting them on root.
 
     // only primary surface streams are supported
-    if (is_primary_surface(display, item->surface_id)) {
+    if (is_primary_surface(display, item->surface->id)) {
         video_stream_detach_behind(display, &shadow->base.rgn, nullptr);
     }
 
@@ -835,7 +834,7 @@ static bool current_add_with_shadow(DisplayChannel *display, Ring *ring, Drawabl
         region_destroy(&exclude_rgn);
         streams_update_visible_region(display, item);
     } else {
-        if (is_primary_surface(display, item->surface_id)) {
+        if (is_primary_surface(display, item->surface->id)) {
             video_stream_detach_behind(display, &item->tree_item.base.rgn, item);
         }
     }
@@ -1032,7 +1031,7 @@ static bool current_add(DisplayChannel *display, Ring *ring, Drawable *drawable)
          * video_stream_detach_behind
          */
         current_add_drawable(display, drawable, ring);
-        if (is_primary_surface(display, drawable->surface_id)) {
+        if (is_primary_surface(display, drawable->surface->id)) {
             video_stream_detach_behind(display, &drawable->tree_item.base.rgn, drawable);
         }
     }
@@ -1050,7 +1049,7 @@ static bool drawable_can_stream(DisplayChannel *display, Drawable *drawable)
         return FALSE;
     }
 
-    if (!is_primary_surface(display, drawable->surface_id)) {
+    if (!is_primary_surface(display, drawable->surface->id)) {
         return FALSE;
     }
 
@@ -1108,14 +1107,16 @@ static void display_channel_print_stats(DisplayChannel *display)
 
 static void drawable_ref_surface_deps(DisplayChannel *display, Drawable *drawable)
 {
-    RedSurface *surface;
-
-    for (const auto surface_id : drawable->surface_deps) {
+    for (int x = 0; x < 3; ++x) {
+        const int surface_id = drawable->red_drawable->surface_deps[x];
         if (surface_id == -1) {
+            drawable->surface_deps[x] = nullptr;
             continue;
         }
-        surface = &display->priv->surfaces[surface_id];
+
+        RedSurface *surface = &display->priv->surfaces[surface_id];
         surface->refs++;
+        drawable->surface_deps[x] = surface;
     }
 }
 
@@ -1141,7 +1142,7 @@ static void handle_self_bitmap(DisplayChannel *display, Drawable *drawable)
     int bpp;
     int all_set;
 
-    surface = &display->priv->surfaces[drawable->surface_id];
+    surface = drawable->surface;
 
     bpp = SPICE_SURFACE_FMT_DEPTH(surface->context.format) / 8;
     width = red_drawable->self_bitmap_area.right - red_drawable->self_bitmap_area.left;
@@ -1164,13 +1165,13 @@ static void handle_self_bitmap(DisplayChannel *display, Drawable *drawable)
     image->u.bitmap.data = spice_chunks_new_linear(dest, height * dest_stride);
     image->u.bitmap.data->flags |= SPICE_CHUNKS_FLAGS_FREE;
 
-    display_channel_draw(display, &red_drawable->self_bitmap_area, drawable->surface_id);
-    surface_read_bits(display, drawable->surface_id,
+    display_channel_draw(display, &red_drawable->self_bitmap_area, drawable->surface->id);
+    surface_read_bits(display, drawable->surface->id,
         &red_drawable->self_bitmap_area, dest, dest_stride);
 
     /* For 32bit non-primary surfaces we need to keep any non-zero
        high bytes as the surface may be used as source to an alpha_blend */
-    if (!is_primary_surface(display, drawable->surface_id) &&
+    if (!is_primary_surface(display, drawable->surface->id) &&
         image->u.bitmap.format == SPICE_BITMAP_FMT_32BIT &&
         rgb32_data_has_alpha(width, height, dest_stride, dest, &all_set)) {
         if (all_set) {
@@ -1183,18 +1184,14 @@ static void handle_self_bitmap(DisplayChannel *display, Drawable *drawable)
     red_drawable->self_bitmap_image = image;
 }
 
-static void surface_add_reverse_dependency(DisplayChannel *display, int surface_id,
-                                             DependItem *depend_item, Drawable *drawable)
+static void surface_add_reverse_dependency(DisplayChannel *display, RedSurface *surface,
+                                           DependItem *depend_item, Drawable *drawable)
 {
-    RedSurface *surface;
-
-    if (surface_id == -1) {
+    if (!surface) {
         depend_item->drawable = nullptr;
         return;
     }
 
-    surface = &display->priv->surfaces[surface_id];
-
     depend_item->drawable = drawable;
     ring_add(&surface->depend_on_me, &depend_item->ring_item);
 }
@@ -1206,11 +1203,11 @@ static bool handle_surface_deps(DisplayChannel *display, Drawable *drawable)
     for (x = 0; x < 3; ++x) {
         // surface self dependency is handled by shadows in "current", or by
         // handle_self_bitmap
-        if (drawable->surface_deps[x] != drawable->surface_id) {
+        if (drawable->surface_deps[x] != drawable->surface) {
             surface_add_reverse_dependency(display, drawable->surface_deps[x],
                                       &drawable->depend_items[x], drawable);
 
-            if (drawable->surface_deps[x] == 0) {
+            if (drawable->surface_deps[x] && drawable->surface_deps[x]->id == 0) {
                 QRegion depend_region;
                 region_init(&depend_region);
                 region_add(&depend_region, &drawable->red_drawable->surfaces_rects[x]);
@@ -1233,7 +1230,7 @@ static void draw_depend_on_me(DisplayChannel *display, uint32_t surface_id)
         Drawable *drawable;
         DependItem *depended_item = SPICE_CONTAINEROF(ring_item, DependItem, ring_item);
         drawable = depended_item->drawable;
-        display_channel_draw(display, &drawable->red_drawable->bbox, drawable->surface_id);
+        display_channel_draw(display, &drawable->red_drawable->bbox, drawable->surface->id);
     }
 }
 
@@ -1298,10 +1295,8 @@ static Drawable *display_channel_get_drawable(DisplayChannel *display, uint8_t e
 
     drawable->tree_item.effect = effect;
 
-    drawable->surface_id = red_drawable->surface_id;
-    display->priv->surfaces[drawable->surface_id].refs++;
-
-    memcpy(drawable->surface_deps, red_drawable->surface_deps, sizeof(drawable->surface_deps));
+    drawable->surface = &display->priv->surfaces[red_drawable->surface_id];
+    drawable->surface->refs++;
 
     drawable->red_drawable = red_drawable;
 
@@ -1322,7 +1317,7 @@ static Drawable *display_channel_get_drawable(DisplayChannel *display, uint8_t e
  */
 static void display_channel_add_drawable(DisplayChannel *display, Drawable *drawable)
 {
-    int surface_id = drawable->surface_id;
+    int surface_id = drawable->surface->id;
     RedDrawable *red_drawable = drawable->red_drawable.get();
 
     red_drawable->mm_time = reds_get_mm_time();
@@ -1352,7 +1347,7 @@ static void display_channel_add_drawable(DisplayChannel *display, Drawable *draw
         return;
     }
 
-    Ring *ring = &display->priv->surfaces[surface_id].current;
+    Ring *ring = &drawable->surface->current;
     int add_to_pipe;
     if (has_shadow(red_drawable)) {
         add_to_pipe = current_add_with_shadow(display, ring, drawable);
@@ -1592,11 +1587,10 @@ static void depended_item_remove(DependItem *item)
 static void drawable_remove_dependencies(Drawable *drawable)
 {
     int x;
-    int surface_id;
 
     for (x = 0; x < 3; ++x) {
-        surface_id = drawable->surface_deps[x];
-        if (surface_id != -1 && drawable->depend_items[x].drawable) {
+        RedSurface* surface = drawable->surface_deps[x];
+        if (surface && drawable->depend_items[x].drawable) {
             depended_item_remove(&drawable->depend_items[x]);
         }
     }
@@ -1604,11 +1598,11 @@ static void drawable_remove_dependencies(Drawable *drawable)
 
 static void drawable_unref_surface_deps(DisplayChannel *display, Drawable *drawable)
 {
-    for (const auto surface_id : drawable->surface_deps) {
-        if (surface_id == -1) {
+    for (const auto surface : drawable->surface_deps) {
+        if (surface == nullptr) {
             continue;
         }
-        display_channel_surface_unref(display, surface_id);
+        display_channel_surface_unref(display, surface->id);
     }
 }
 
@@ -1629,7 +1623,7 @@ void drawable_unref(Drawable *drawable)
 
     drawable_remove_dependencies(drawable);
     drawable_unref_surface_deps(display, drawable);
-    display_channel_surface_unref(display, drawable->surface_id);
+    display_channel_surface_unref(display, drawable->surface->id);
 
     glz_retention_detach_drawables(&drawable->glz_retention);
 
@@ -1639,13 +1633,12 @@ void drawable_unref(Drawable *drawable)
 static void drawable_deps_draw(DisplayChannel *display, Drawable *drawable)
 {
     int x;
-    int surface_id;
 
     for (x = 0; x < 3; ++x) {
-        surface_id = drawable->surface_deps[x];
-        if (surface_id != -1 && drawable->depend_items[x].drawable) {
+        RedSurface *surface = drawable->surface_deps[x];
+        if (surface && drawable->depend_items[x].drawable) {
             depended_item_remove(&drawable->depend_items[x]);
-            display_channel_draw(display, &drawable->red_drawable->surfaces_rects[x], surface_id);
+            display_channel_draw(display, &drawable->red_drawable->surfaces_rects[x], surface->id);
         }
     }
 }
@@ -1658,7 +1651,7 @@ static void drawable_draw(DisplayChannel *display, Drawable *drawable)
 
     drawable_deps_draw(display, drawable);
 
-    surface = &display->priv->surfaces[drawable->surface_id];
+    surface = drawable->surface;
     canvas = surface->context.canvas;
     spice_return_if_fail(canvas);
 
@@ -1868,7 +1861,7 @@ static Drawable* current_find_intersects_rect(Ring *current, RingItem *from,
  * FIXME: merge with display_channel_draw()?
  */
 void display_channel_draw_until(DisplayChannel *display, const SpiceRect *area, int surface_id,
-                               Drawable *last)
+                                Drawable *last)
 {
     RedSurface *surface;
     Drawable *surface_last = nullptr;
@@ -1881,13 +1874,13 @@ void display_channel_draw_until(DisplayChannel *display, const SpiceRect *area,
 
     surface = &display->priv->surfaces[surface_id];
 
-    if (surface_id != last->surface_id) {
+    if (surface != last->surface) {
         // find the nearest older drawable from the appropriate surface
         ring = &display->priv->current_list;
         ring_item = &last->list_link;
         while ((ring_item = ring_next(ring, ring_item))) {
             now = SPICE_CONTAINEROF(ring_item, Drawable, list_link);
-            if (now->surface_id == surface_id) {
+            if (now->surface == surface) {
                 surface_last = now;
                 break;
             }
@@ -2099,6 +2092,7 @@ void display_channel_create_surface(DisplayChannel *display, uint32_t surface_id
     ring_init(&surface->depend_on_me);
     region_init(&surface->draw_dirty_region);
     surface->refs = 1;
+    surface->id = surface_id;
 
     if (display->priv->renderer == RED_RENDERER_INVALID) {
         int i;
diff --git a/server/display-channel.h b/server/display-channel.h
index 86784131..5366c052 100644
--- a/server/display-channel.h
+++ b/server/display-channel.h
@@ -37,6 +37,7 @@
 #include "push-visibility.h"
 
 struct DisplayChannelPrivate;
+struct RedSurface;
 
 struct DisplayChannel final: public CommonGraphicsChannel
 {
@@ -78,8 +79,10 @@ struct Drawable {
     BitmapGradualType copy_bitmap_graduality;
     DependItem depend_items[3];
 
-    int surface_id;
-    int surface_deps[3];
+    /* destination surface. This pointer is not NULL. A reference is hold */
+    RedSurface *surface;
+    /* dependency surfaces. They can be NULL. A reference is hold. */
+    RedSurface *surface_deps[3];
 
     uint32_t process_commands_generation;
     DisplayChannel *display;


More information about the Spice-commits mailing list