[Spice-devel] [PATCH 13/18] worker: move destroy_surface() familly

Frediano Ziglio fziglio at redhat.com
Tue Nov 24 03:13:17 PST 2015


From: Marc-André Lureau <marcandre.lureau at gmail.com>

---
 server/dcc.c             |  80 ++++++++++++++++++++++
 server/dcc.h             |   2 +
 server/display-channel.c |  69 ++++++++++++++++++-
 server/display-channel.h |  20 ++++++
 server/red_worker.c      | 173 ++---------------------------------------------
 5 files changed, 175 insertions(+), 169 deletions(-)

diff --git a/server/dcc.c b/server/dcc.c
index e3b0c55..6c089da 100644
--- a/server/dcc.c
+++ b/server/dcc.c
@@ -22,6 +22,8 @@
 #include "dcc.h"
 #include "display-channel.h"
 
+#define DISPLAY_CLIENT_SHORT_TIMEOUT 15000000000ULL //nano
+
 static SurfaceCreateItem *surface_create_item_new(RedChannel* channel,
                                                   uint32_t surface_id, uint32_t width,
                                                   uint32_t height, uint32_t format, uint32_t flags)
@@ -41,6 +43,84 @@ static SurfaceCreateItem *surface_create_item_new(RedChannel* channel,
     return create;
 }
 
+/*
+ * 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.
+ */
+int dcc_clear_surface_drawables_from_pipe(DisplayChannelClient *dcc, int surface_id,
+                                          int wait_if_used)
+{
+    Ring *ring;
+    PipeItem *item;
+    int x;
+    RedChannelClient *rcc;
+
+    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 */
+
+    rcc = RED_CHANNEL_CLIENT(dcc);
+    ring = &rcc->pipe;
+    item = (PipeItem *) ring;
+    while ((item = (PipeItem *)ring_next(ring, (RingItem *)item))) {
+        Drawable *drawable;
+        DrawablePipeItem *dpi = NULL;
+        int depend_found = FALSE;
+
+        if (item->type == PIPE_ITEM_TYPE_DRAW) {
+            dpi = SPICE_CONTAINEROF(item, DrawablePipeItem, dpi_pipe_item);
+            drawable = dpi->drawable;
+        } else if (item->type == PIPE_ITEM_TYPE_UPGRADE) {
+            drawable = ((UpgradeItem *)item)->drawable;
+        } else {
+            continue;
+        }
+
+        if (drawable->surface_id == surface_id) {
+            PipeItem *tmp_item = item;
+            item = (PipeItem *)ring_prev(ring, (RingItem *)item);
+            red_channel_client_pipe_remove_and_release(rcc, tmp_item);
+            if (!item) {
+                item = (PipeItem *)ring;
+            }
+            continue;
+        }
+
+        for (x = 0; x < 3; ++x) {
+            if (drawable->surface_deps[x] == surface_id) {
+                depend_found = TRUE;
+                break;
+            }
+        }
+
+        if (depend_found) {
+            spice_debug("surface %d dependent item found %p, %p", surface_id, drawable, item);
+            if (wait_if_used) {
+                break;
+            } else {
+                return TRUE;
+            }
+        }
+    }
+
+    if (!wait_if_used) {
+        return TRUE;
+    }
+
+    if (item) {
+        return red_channel_client_wait_pipe_item_sent(RED_CHANNEL_CLIENT(dcc), item,
+                                                      DISPLAY_CLIENT_TIMEOUT);
+    } else {
+        /*
+         * in case that the pipe didn't contain any item that is dependent on the surface, but
+         * there is one during sending. Use a shorter timeout, since it is just one item
+         */
+        return red_channel_client_wait_outgoing_item(RED_CHANNEL_CLIENT(dcc),
+                                                     DISPLAY_CLIENT_SHORT_TIMEOUT);
+    }
+    return TRUE;
+}
+
 void dcc_create_surface(DisplayChannelClient *dcc, int surface_id)
 {
     DisplayChannel *display;
diff --git a/server/dcc.h b/server/dcc.h
index dc6f1e7..c5767c9 100644
--- a/server/dcc.h
+++ b/server/dcc.h
@@ -201,6 +201,8 @@ void                       dcc_release_item                          (DisplayCha
                                                                       int item_pushed);
 void                       dcc_send_item                             (DisplayChannelClient *dcc,
                                                                       PipeItem *item);
+int                        dcc_clear_surface_drawables_from_pipe     (DisplayChannelClient *dcc,
+                                                                      int surface_id, int force);
 
 typedef struct compress_send_data_t {
     void*    comp_buf;
diff --git a/server/display-channel.c b/server/display-channel.c
index 66b661e..ec076ce 100644
--- a/server/display-channel.c
+++ b/server/display-channel.c
@@ -1374,7 +1374,7 @@ void display_channel_draw_until(DisplayChannel *display, const SpiceRect *area,
      * newer item then 'last' in one of the surfaces that
      * display_channel_draw is called for, Otherwise, 'now' would have
      * already been rendered.  See the call for
-     * red_handle_depends_on_target_surface in red_process_draw
+     * draw_depend_on_me in red_process_draw
      */
     draw_until(display, surface, last);
     surface_update_dest(surface, area);
@@ -1403,6 +1403,73 @@ void display_channel_draw(DisplayChannel *display, const SpiceRect *area, int su
     surface_update_dest(surface, area);
 }
 
+static void clear_surface_drawables_from_pipes(DisplayChannel *display, int surface_id,
+                                               int wait_if_used)
+{
+    RingItem *item, *next;
+    DisplayChannelClient *dcc;
+
+    FOREACH_DCC(display, item, next, dcc) {
+        if (!dcc_clear_surface_drawables_from_pipe(dcc, surface_id, wait_if_used)) {
+            red_channel_client_disconnect(RED_CHANNEL_CLIENT(dcc));
+        }
+    }
+}
+
+/* TODO: cleanup/refactor destroy functions */
+void display_channel_destroy_surface(DisplayChannel *display, uint32_t 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, surface_id);
+    clear_surface_drawables_from_pipes(display, surface_id, FALSE);
+    display_channel_surface_unref(display, surface_id);
+}
+
+void display_channel_destroy_surface_wait(DisplayChannel *display, uint32_t surface_id)
+{
+    if (!validate_surface(display, surface_id))
+        return;
+    if (!display->surfaces[surface_id].context.canvas)
+        return;
+
+    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, surface_id);
+    clear_surface_drawables_from_pipes(display, surface_id, TRUE);
+}
+
+/* called upon device reset */
+/* TODO: split me*/
+void display_channel_destroy_surfaces(DisplayChannel *display)
+{
+    int i;
+
+    spice_debug(NULL);
+    //to handle better
+    for (i = 0; i < NUM_SURFACES; ++i) {
+        if (display->surfaces[i].context.canvas) {
+            display_channel_destroy_surface_wait(display, i);
+            if (display->surfaces[i].context.canvas) {
+                display_channel_surface_unref(display, i);
+            }
+            spice_assert(!display->surfaces[i].context.canvas);
+        }
+    }
+    spice_warn_if_fail(ring_is_empty(&display->streams));
+
+    if (red_channel_is_connected(RED_CHANNEL(display))) {
+        red_channel_pipes_add_type(RED_CHANNEL(display), PIPE_ITEM_TYPE_INVAL_PALETTE_CACHE);
+        red_pipes_add_verb(RED_CHANNEL(display), SPICE_MSG_DISPLAY_STREAM_DESTROY_ALL);
+    }
+
+    display_channel_free_glz_drawables(display);
+}
+
 static void on_disconnect(RedChannelClient *rcc)
 {
     DisplayChannel *display;
diff --git a/server/display-channel.h b/server/display-channel.h
index 42b3850..34caafe 100644
--- a/server/display-channel.h
+++ b/server/display-channel.h
@@ -284,6 +284,11 @@ int                        display_channel_wait_for_migrate_data     (DisplayCha
 void                       display_channel_flush_all_surfaces        (DisplayChannel *display);
 void                       display_channel_free_glz_drawables_to_free(DisplayChannel *display);
 void                       display_channel_free_glz_drawables        (DisplayChannel *display);
+void                       display_channel_destroy_surface_wait      (DisplayChannel *display,
+                                                                      uint32_t surface_id);
+void                       display_channel_destroy_surfaces          (DisplayChannel *display);
+void                       display_channel_destroy_surface           (DisplayChannel *display,
+                                                                      uint32_t surface_id);
 
 static inline int validate_surface(DisplayChannel *display, uint32_t surface_id)
 {
@@ -415,6 +420,21 @@ static inline void region_add_clip_rects(QRegion *rgn, SpiceClipRects *data)
     }
 }
 
+static inline void draw_depend_on_me(DisplayChannel *display, uint32_t surface_id)
+{
+    RedSurface *surface;
+    RingItem *ring_item;
+
+    surface = &display->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);
+        drawable = depended_item->drawable;
+        display_channel_draw(display, &drawable->red_drawable->bbox, drawable->surface_id);
+    }
+}
+
 void current_remove_drawable(DisplayChannel *display, Drawable *item);
 void red_pipes_remove_drawable(Drawable *drawable);
 void current_remove(DisplayChannel *display, TreeItem *item);
diff --git a/server/red_worker.c b/server/red_worker.c
index cef546e..7af9c9b 100644
--- a/server/red_worker.c
+++ b/server/red_worker.c
@@ -72,8 +72,6 @@
 #define CMD_RING_POLL_TIMEOUT 10 //milli
 #define CMD_RING_POLL_RETRIES 200
 
-#define DISPLAY_CLIENT_SHORT_TIMEOUT 15000000000ULL //nano
-
 #define MAX_EVENT_SOURCES 20
 #define INF_EVENT_WAIT ~0
 
@@ -334,101 +332,6 @@ void current_remove_all(DisplayChannel *display, int surface_id)
     }
 }
 
-/*
- * 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.
- */
-static int red_clear_surface_drawables_from_pipe(DisplayChannelClient *dcc, int surface_id,
-                                                 int wait_if_used)
-{
-    Ring *ring;
-    PipeItem *item;
-    int x;
-    RedChannelClient *rcc;
-
-    if (!dcc) {
-        return TRUE;
-    }
-
-    /* removing the newest drawables that their destination is surface_id and
-       no other drawable depends on them */
-
-    rcc = RED_CHANNEL_CLIENT(dcc);
-    ring = &rcc->pipe;
-    item = (PipeItem *) ring;
-    while ((item = (PipeItem *)ring_next(ring, (RingItem *)item))) {
-        Drawable *drawable;
-        DrawablePipeItem *dpi = NULL;
-        int depend_found = FALSE;
-
-        if (item->type == PIPE_ITEM_TYPE_DRAW) {
-            dpi = SPICE_CONTAINEROF(item, DrawablePipeItem, dpi_pipe_item);
-            drawable = dpi->drawable;
-        } else if (item->type == PIPE_ITEM_TYPE_UPGRADE) {
-            drawable = ((UpgradeItem *)item)->drawable;
-        } else {
-            continue;
-        }
-
-        if (drawable->surface_id == surface_id) {
-            PipeItem *tmp_item = item;
-            item = (PipeItem *)ring_prev(ring, (RingItem *)item);
-            red_channel_client_pipe_remove_and_release(rcc, tmp_item);
-            if (!item) {
-                item = (PipeItem *)ring;
-            }
-            continue;
-        }
-
-        for (x = 0; x < 3; ++x) {
-            if (drawable->surface_deps[x] == surface_id) {
-                depend_found = TRUE;
-                break;
-            }
-        }
-
-        if (depend_found) {
-            spice_debug("surface %d dependent item found %p, %p", surface_id, drawable, item);
-            if (wait_if_used) {
-                break;
-            } else {
-                return TRUE;
-            }
-        }
-    }
-
-    if (!wait_if_used) {
-        return TRUE;
-    }
-
-    if (item) {
-        return red_channel_client_wait_pipe_item_sent(RED_CHANNEL_CLIENT(dcc), item,
-                                                      DISPLAY_CLIENT_TIMEOUT);
-    } else {
-        /*
-         * in case that the pipe didn't contain any item that is dependent on the surface, but
-         * there is one during sending. Use a shorter timeout, since it is just one item
-         */
-        return red_channel_client_wait_outgoing_item(RED_CHANNEL_CLIENT(dcc),
-                                                     DISPLAY_CLIENT_SHORT_TIMEOUT);
-    }
-    return TRUE;
-}
-
-static void red_clear_surface_drawables_from_pipes(DisplayChannel *display,
-                                                   int surface_id,
-                                                   int wait_if_used)
-{
-    RingItem *item, *next;
-    DisplayChannelClient *dcc;
-
-    FOREACH_DCC(display, item, next, dcc) {
-        if (!red_clear_surface_drawables_from_pipe(dcc, surface_id, wait_if_used)) {
-            red_channel_client_disconnect(RED_CHANNEL_CLIENT(dcc));
-        }
-    }
-}
-
 static int red_display_drawable_is_in_pipe(DisplayChannelClient *dcc, Drawable *drawable)
 {
     DrawablePipeItem *dpi;
@@ -697,23 +600,6 @@ static Drawable *get_drawable(RedWorker *worker, uint8_t effect, RedDrawable *re
     return drawable;
 }
 
-static inline int red_handle_depends_on_target_surface(DisplayChannel *display, uint32_t surface_id)
-{
-    RedSurface *surface;
-    RingItem *ring_item;
-
-    surface = &display->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);
-        drawable = depended_item->drawable;
-        display_channel_draw(display, &drawable->red_drawable->bbox, drawable->surface_id);
-    }
-
-    return TRUE;
-}
-
 static inline void add_to_surface_dependency(DisplayChannel *display, int depend_on_surface_id,
                                              DependItem *depend_item, Drawable *drawable)
 {
@@ -812,9 +698,7 @@ static inline void red_process_draw(RedWorker *worker, RedDrawable *red_drawable
         goto cleanup;
     }
 
-    if (!red_handle_depends_on_target_surface(worker->display_channel, surface_id)) {
-        goto cleanup;
-    }
+    draw_depend_on_me(display, surface_id);
 
     if (!red_handle_surfaces_dependencies(worker->display_channel, drawable)) {
         goto cleanup;
@@ -870,16 +754,10 @@ static inline void red_process_surface(RedWorker *worker, RedSurfaceCmd *surface
             break;
         }
         set_surface_release_info(&red_surface->destroy, surface->release_info, group_id);
-        red_handle_depends_on_target_surface(display, surface_id);
-        /* note that red_handle_depends_on_target_surface 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);
-        red_clear_surface_drawables_from_pipes(display, surface_id, FALSE);
-        display_channel_surface_unref(display, surface_id);
+        display_channel_destroy_surface(display, surface_id);
         break;
     default:
-            spice_error("unknown surface command");
+        spice_warn_if_reached();
     };
 exit:
     red_put_surface_cmd(surface);
@@ -4118,21 +3996,6 @@ static void handle_dev_del_memslot(void *opaque, void *payload)
     red_memslot_info_del_slot(&worker->mem_slots, slot_group_id, slot_id);
 }
 
-void display_channel_destroy_surface_wait(DisplayChannel *display, int surface_id)
-{
-    if (!validate_surface(display, surface_id))
-        return;
-    if (!display->surfaces[surface_id].context.canvas)
-        return;
-
-    red_handle_depends_on_target_surface(display, surface_id);
-    /* note that red_handle_depends_on_target_surface 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);
-    red_clear_surface_drawables_from_pipes(display, surface_id, TRUE);
-}
-
 static void handle_dev_destroy_surface_wait(void *opaque, void *payload)
 {
     RedWorkerMessageDestroySurfaceWait *msg = payload;
@@ -4144,34 +4007,6 @@ static void handle_dev_destroy_surface_wait(void *opaque, void *payload)
     display_channel_destroy_surface_wait(worker->display_channel, msg->surface_id);
 }
 
-/* called upon device reset */
-
-/* TODO: split me*/
-void display_channel_destroy_surfaces(DisplayChannel *display)
-{
-    int i;
-
-    spice_debug(NULL);
-    //to handle better
-    for (i = 0; i < NUM_SURFACES; ++i) {
-        if (display->surfaces[i].context.canvas) {
-            display_channel_destroy_surface_wait(display, i);
-            if (display->surfaces[i].context.canvas) {
-                display_channel_surface_unref(display, i);
-            }
-            spice_assert(!display->surfaces[i].context.canvas);
-        }
-    }
-    spice_warn_if_fail(ring_is_empty(&display->streams));
-
-    if (red_channel_is_connected(RED_CHANNEL(display))) {
-        red_channel_pipes_add_type(RED_CHANNEL(display), PIPE_ITEM_TYPE_INVAL_PALETTE_CACHE);
-        red_pipes_add_verb(RED_CHANNEL(display), SPICE_MSG_DISPLAY_STREAM_DESTROY_ALL);
-    }
-
-    display_channel_free_glz_drawables(display);
-}
-
 static void handle_dev_destroy_surfaces(void *opaque, void *payload)
 {
     RedWorker *worker = opaque;
@@ -4739,6 +4574,8 @@ static void register_callbacks(Dispatcher *dispatcher)
     dispatcher_register_async_done_callback(
                                     dispatcher,
                                     worker_handle_dispatcher_async_done);
+
+    /* TODO: register cursor & display specific msg in respective channel files */
     dispatcher_register_handler(dispatcher,
                                 RED_WORKER_MESSAGE_DISPLAY_CONNECT,
                                 handle_dev_display_connect,
-- 
2.4.3



More information about the Spice-devel mailing list