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 <fzig...@redhat.com>
---
 server/dcc-send.c        | 21 ++++++-----
 server/dcc.c             | 21 ++++++-----
 server/display-channel.c | 91 +++++++++++++++++++++++-------------------------
 server/display-channel.h | 15 +++++---
 4 files changed, 78 insertions(+), 70 deletions(-)

diff --git a/server/dcc-send.c b/server/dcc-send.c
index c49adab..1e31584 100644
--- a/server/dcc-send.c
+++ b/server/dcc-send.c
@@ -316,7 +316,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;
 
@@ -556,7 +556,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;
 
     if (drawable->clip.type == SPICE_CLIP_TYPE_RECTS ) {
@@ -653,7 +653,10 @@ static int 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] == NULL) {
+                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;
@@ -828,7 +831,7 @@ static void 
red_lossy_marshall_qxl_draw_fill(RedChannelClient *rcc,
     brush_is_lossy = is_brush_lossy(rcc, &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);
     }
 
@@ -845,7 +848,7 @@ static void 
red_lossy_marshall_qxl_draw_fill(RedChannelClient *rcc,
         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++;
         }
@@ -1148,7 +1151,7 @@ static void 
red_lossy_marshall_qxl_copy_bits(RedChannelClient *rcc,
     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);
@@ -1210,7 +1213,7 @@ static void 
red_lossy_marshall_qxl_draw_blend(RedChannelClient *rcc,
         }
 
         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++;
         }
@@ -1388,7 +1391,7 @@ static void 
red_lossy_marshall_qxl_draw_rop3(RedChannelClient *rcc,
         }
 
         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++;
         }
@@ -1469,7 +1472,7 @@ static void 
red_lossy_marshall_qxl_draw_composite(RedChannelClient *rcc,
         }
 
         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.c b/server/dcc.c
index ce8677d..af32de7 100644
--- a/server/dcc.c
+++ b/server/dcc.c
@@ -71,11 +71,16 @@ int 
dcc_clear_surface_drawables_from_pipe(DisplayChannelClient *dcc, int surface
     GList *l;
     int x;
     RedChannelClient *rcc;
+    DisplayChannel *display;
+    RedSurface *surface;
 
     spice_return_val_if_fail(dcc != NULL, 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];
+
     rcc = RED_CHANNEL_CLIENT(dcc);
     for (l = rcc->priv->pipe.head; l != NULL; ) {
         Drawable *drawable;
@@ -94,13 +99,13 @@ int 
dcc_clear_surface_drawables_from_pipe(DisplayChannelClient *dcc, int surface
             continue;
         }
 
-        if (drawable->surface_id == surface_id) {
+        if (drawable->surface == surface) {
             red_channel_client_pipe_remove_and_release_pos(rcc, item_pos);
             continue;
         }
 
         for (x = 0; x < 3; ++x) {
-            if (drawable->surface_deps[x] == surface_id) {
+            if (drawable->surface_deps[x] == surface) {
                 depend_found = TRUE;
                 break;
             }
@@ -251,8 +256,8 @@ static void 
add_drawable_surface_images(DisplayChannelClient *dcc, Drawable *dra
     for (x = 0; x < 3; ++x) {
         int surface_id;
 
-        surface_id = drawable->surface_deps[x];
-        if (surface_id != -1) {
+        if (drawable->surface_deps[x] != NULL) {
+            surface_id = drawable->surface_deps[x]->id;
             if (dcc->priv->surface_client_created[surface_id] == TRUE) {
                 continue;
             }
@@ -262,13 +267,13 @@ static void 
add_drawable_surface_images(DisplayChannelClient *dcc, Drawable *dra
         }
     }
 
-    if (dcc->priv->surface_client_created[drawable->surface_id] == TRUE) {
+    if (dcc->priv->surface_client_created[drawable->surface->id] == TRUE) {
         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);
 }
 
 static void red_drawable_pipe_item_free(RedPipeItem *item)
diff --git a/server/display-channel.c b/server/display-channel.c
index d7ea7d5..99082e6 100644
--- a/server/display-channel.c
+++ b/server/display-channel.c
@@ -176,6 +176,7 @@ void display_channel_surface_unref(DisplayChannel *display, 
uint32_t surface_id)
     DisplayChannelClient *dcc;
     GListIter iter;
 
+    spice_assert(surface->refs > 0);
     if (--surface->refs != 0) {
         return;
     }
@@ -221,7 +222,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;
     }
 
@@ -305,9 +306,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);
@@ -625,7 +625,7 @@ static int current_add_with_shadow(DisplayChannel *display, 
Ring *ring, Drawable
     // 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)) {
         stream_detach_behind(display, &shadow->base.rgn, NULL);
     }
 
@@ -638,7 +638,7 @@ static int current_add_with_shadow(DisplayChannel *display, 
Ring *ring, Drawable
         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)) {
             stream_detach_behind(display, &item->tree_item.base.rgn, item);
         }
     }
@@ -755,7 +755,7 @@ static int current_add(DisplayChannel *display, Ring *ring, 
Drawable *drawable)
          * 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)) {
             stream_detach_behind(display, &drawable->tree_item.base.rgn, 
drawable);
         }
     }
@@ -773,7 +773,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;
     }
 
@@ -829,19 +829,21 @@ static void display_channel_print_stats(DisplayChannel 
*display)
 }
 #endif
 
-static void drawable_ref_surface_deps(DisplayChannel *display, Drawable 
*drawable)
+static void drawable_ref_surface_deps(DisplayChannel *display, Drawable 
*drawable,
+                                      RedDrawable *red_drawable)
 {
     int x;
-    int surface_id;
-    RedSurface *surface;
 
     for (x = 0; x < 3; ++x) {
-        surface_id = drawable->surface_deps[x];
+        int surface_id = red_drawable->surface_deps[x];
         if (surface_id == -1) {
+            drawable->surface_deps[x] = NULL;
             continue;
         }
-        surface = &display->priv->surfaces[surface_id];
+
+        RedSurface *surface = &display->priv->surfaces[surface_id];
         surface->refs++;
+        drawable->surface_deps[x] = surface;
     }
 }
 
@@ -867,7 +869,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;
@@ -890,13 +892,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) {
@@ -909,18 +911,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 == NULL) {
         depend_item->drawable = NULL;
         return;
     }
 
-    surface = &display->priv->surfaces[surface_id];
-
     depend_item->drawable = drawable;
     ring_add(&surface->depend_on_me, &depend_item->ring_item);
 }
@@ -932,11 +930,11 @@ static int 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] != NULL && 
drawable->surface_deps[x]->id == 0) {
                 QRegion depend_region;
                 region_init(&depend_region);
                 region_add(&depend_region, 
&drawable->red_drawable->surfaces_rects[x]);
@@ -959,7 +957,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);
     }
 }
 
@@ -1027,17 +1025,16 @@ static Drawable 
*display_channel_get_drawable(DisplayChannel *display, uint8_t e
     drawable->tree_item.effect = effect;
     drawable->red_drawable = red_drawable_ref(red_drawable);
 
-    drawable->surface_id = red_drawable->surface_id;
-    display->priv->surfaces[drawable->surface_id].refs++;
+    drawable->surface = &display->priv->surfaces[red_drawable->surface_id];
+    drawable->surface->refs++;
 
-    memcpy(drawable->surface_deps, red_drawable->surface_deps, 
sizeof(drawable->surface_deps));
     /*
         surface->refs is affected by a drawable (that is
         dependent on the surface) as long as the drawable is alive.
         However, surface->depend_on_me is affected by a drawable only
         as long as it is in the current tree (hasn't been rendered yet).
     */
-    drawable_ref_surface_deps(display, drawable);
+    drawable_ref_surface_deps(display, drawable, red_drawable);
 
     return drawable;
 }
@@ -1048,7 +1045,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;
 
     red_drawable->mm_time = reds_get_mm_time();
@@ -1078,7 +1075,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);
@@ -1319,11 +1316,10 @@ static void depended_item_remove(DependItem *item)
 static void drawable_remove_dependencies(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 != NULL && drawable->depend_items[x].drawable) {
             depended_item_remove(&drawable->depend_items[x]);
         }
     }
@@ -1332,14 +1328,13 @@ static void drawable_remove_dependencies(DisplayChannel 
*display, Drawable *draw
 static void drawable_unref_surface_deps(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) {
+        RedSurface *surface = drawable->surface_deps[x];
+        if (surface == NULL) {
             continue;
         }
-        display_channel_surface_unref(display, surface_id);
+        display_channel_surface_unref(display, surface->id);
     }
 }
 
@@ -1360,7 +1355,7 @@ void drawable_unref(Drawable *drawable)
 
     drawable_remove_dependencies(display, 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);
 
@@ -1374,13 +1369,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 != NULL && 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);
         }
     }
 }
@@ -1393,7 +1387,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);
 
@@ -1596,7 +1590,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 = NULL;
@@ -1609,13 +1603,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;
             }
@@ -1828,6 +1822,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 bbae6f1..8918ccb 100644
--- a/server/display-channel.h
+++ b/server/display-channel.h
@@ -44,6 +44,8 @@
 #include "dcc.h"
 #include "image-encoders.h"
 
+typedef struct RedSurface RedSurface;
+
 typedef struct DependItem {
     Drawable *drawable;
     RingItem ring_item;
@@ -69,8 +71,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;
@@ -129,8 +133,9 @@ typedef struct DrawContext {
     void *line_0;
 } DrawContext;
 
-typedef struct RedSurface {
+struct RedSurface {
     uint32_t refs;
+    uint16_t id;
     Ring current;
     Ring current_list;
     DrawContext context;
@@ -140,7 +145,7 @@ typedef struct RedSurface {
 
     //fix me - better handling here
     QXLReleaseInfoExt create, destroy;
-} RedSurface;
+};
 
 #define NUM_DRAWABLES 1000
 typedef struct _Drawable _Drawable;
@@ -382,7 +387,7 @@ static inline int 
is_drawable_independent_from_surfaces(Drawable *drawable)
     int x;
 
     for (x = 0; x < 3; ++x) {
-        if (drawable->surface_deps[x] != -1) {
+        if (drawable->surface_deps[x]) {
             return FALSE;
         }
     }
-- 
2.7.4

_______________________________________________
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Reply via email to