Attachment 674984 Details for Bug 891326 – fixing patch

[patch] fixing patch

0001-red_worker.c-insert-a-drawable-to-its-position-in-th.patch (text/plain), 7.96 KB, created by Yonit Halperin on 2013-01-08 18:12:52 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Yonit Halperin

Created: 2013-01-08 18:12:52 UTC

Size: 7.96 KB

patch

obsolete

>From b22e82ad578c835996e184cefe5a26a70378714a Mon Sep 17 00:00:00 2001
>From: Yonit Halperin <yhalperi@redhat.com>
>Date: Tue, 8 Jan 2013 10:47:53 -0500
>Subject: [spice-server PATCH 1/2] red_worker.c: insert a drawable to its
> position in the current tree before calling red_detach_streams_behind
>
>resolves: rhbz#891326
>
>Starting from commit 81fe00b08ad4f, red_detach_streams_behind can
>trigger modifications in the current tree (by update_area calls). Thus,
>after calling red_detach_streams_behind it is not safe to access tree
>entries that were calculated before the call.
>This patch inserts the drawable to the tree before the call to
>red_detach_streams_behind. This change also requires making sure
>that rendering operations that can be triggered by
>red_detach_streams_behind will not include this drawable (which is now part of the tree).
>---
> server/red_worker.c | 55 ++++++++++++++++++++++++++++++++++++++++-------------
> 1 file changed, 42 insertions(+), 13 deletions(-)
>
>diff --git a/server/red_worker.c b/server/red_worker.c
>index 1a9c375..5e00cb6 100644
>--- a/server/red_worker.c
>+++ b/server/red_worker.c
>@@ -1022,6 +1022,8 @@ static void red_current_flush(RedWorker *worker, int surface_id);
> #else
> static void red_draw_drawable(RedWorker *worker, Drawable *item);
> static void red_update_area(RedWorker *worker, const SpiceRect *area, int surface_id);
>+static void red_update_area_till(RedWorker *worker, const SpiceRect *area, int surface_id,
>+                                 Drawable *last);
> #endif
> static void red_release_cursor(RedWorker *worker, CursorItem *cursor);
> static inline void release_drawable(RedWorker *worker, Drawable *item);
>@@ -2615,7 +2617,9 @@ static int red_display_drawable_is_in_pipe(DisplayChannelClient *dcc, Drawable *
>  * after red_display_detach_stream_gracefully is called for all the display channel clients,
>  * red_detach_stream should be called. See comment (1).
>  */
>-static inline void red_display_detach_stream_gracefully(DisplayChannelClient *dcc, Stream *stream)
>+static inline void red_display_detach_stream_gracefully(DisplayChannelClient *dcc,
>+                                                        Stream *stream,
>+                                                        Drawable *update_area_limit)
> {
>     int stream_id = get_stream_id(dcc->common.worker, stream);
>     StreamAgent *agent = &dcc->stream_agents[stream_id];
>@@ -2669,27 +2673,41 @@ static inline void red_display_detach_stream_gracefully(DisplayChannelClient *dc
>         spice_debug("stream %d: upgrade by screenshot. has current %d. box ==>",
>                     stream_id, stream->current != NULL);
>         rect_debug(&upgrade_area);
>-        red_update_area(dcc->common.worker, &upgrade_area, 0);
>+        if (update_area_limit) {
>+            red_update_area_till(dcc->common.worker, &upgrade_area, 0, update_area_limit);
>+        } else {
>+            red_update_area(dcc->common.worker, &upgrade_area, 0);
>+        }
>         red_add_surface_area_image(dcc, 0, &upgrade_area, NULL, FALSE);
>     }
> 
> }
> 
>-static inline void red_detach_stream_gracefully(RedWorker *worker, Stream *stream)
>+static inline void red_detach_stream_gracefully(RedWorker *worker, Stream *stream,
>+                                                Drawable *update_area_limit)
> {
>     RingItem *item;
>     DisplayChannelClient *dcc;
> 
>     WORKER_FOREACH_DCC(worker, item, dcc) {
>-        red_display_detach_stream_gracefully(dcc, stream);
>+        red_display_detach_stream_gracefully(dcc, stream, update_area_limit);
>     }
>     if (stream->current) {
>         red_detach_stream(worker, stream, TRUE);
>     }
> }
> 
>-// region should be a primary surface region
>-static void red_detach_streams_behind(RedWorker *worker, QRegion *region)
>+/*
>+ * region  : a primary surface region. Streams that intersects with the given
>+ *           region will be detached.
>+ * drawable: If detaching the stream is triggered by the addition of a new drawable
>+ *           that is dependent on the given region, and the drawable is already a part
>+ *           of the "current tree", the drawable parameter should be set with
>+ *           this drawable, otherwise, it should be NULL. Then, if detaching the stream
>+ *           involves sending an upgrade image to the client, this drawable won't be rendered
>+ *           (see red_display_detach_stream_gracefully).
>+ */
>+static void red_detach_streams_behind(RedWorker *worker, QRegion *region, Drawable *drawable)
> {
>     Ring *ring = &worker->streams;
>     RingItem *item = ring_get_head(ring);
>@@ -2706,7 +2724,7 @@ static void red_detach_streams_behind(RedWorker *worker, QRegion *region)
>             StreamAgent *agent = &dcc->stream_agents[get_stream_id(worker, stream)];
> 
>             if (region_intersects(&agent->vis_region, region)) {
>-                red_display_detach_stream_gracefully(dcc, stream);
>+                red_display_detach_stream_gracefully(dcc, stream, drawable);
>                 detach_stream = 1;
>                 spice_debug("stream %d", get_stream_id(worker, stream));
>             }
>@@ -2798,7 +2816,7 @@ static inline void red_handle_streams_timout(RedWorker *worker)
>         Stream *stream = SPICE_CONTAINEROF(item, Stream, link);
>         item = ring_next(ring, item);
>         if (now >= (stream->last_time + RED_STREAM_TIMOUT)) {
>-            red_detach_stream_gracefully(worker, stream);
>+            red_detach_stream_gracefully(worker, stream, NULL);
>             red_stop_stream(worker, stream);
>         }
>     }
>@@ -3507,13 +3525,24 @@ static inline int red_current_add(RedWorker *worker, Ring *ring, Drawable *drawa
>         exclude_region(worker, ring, exclude_base, &exclude_rgn, NULL, drawable);
>         red_use_stream_trace(worker, drawable);
>         red_streams_update_visible_region(worker, drawable);
>+        /*
>+         * Performing the insertion after exclude_region for
>+         * safety (todo: Not sure if exclude_region can affect the drawable
>+         * if it is added to the tree before calling exclude_region).
>+         */
>+        __current_add_drawable(worker, drawable, ring);
>     } else {
>+        /*
>+         * red_detach_streams_behind can affect the current tree since it may
>+         * trigger calls to update_area. Thus, the drawable should be added to the tree
>+         * before calling red_detach_streams_behind
>+         */
>+        __current_add_drawable(worker, drawable, ring);
>         if (drawable->surface_id == 0) {
>-            red_detach_streams_behind(worker, &drawable->tree_item.base.rgn);
>+            red_detach_streams_behind(worker, &drawable->tree_item.base.rgn, drawable);
>         }
>     }
>     region_destroy(&exclude_rgn);
>-    __current_add_drawable(worker, drawable, ring);
>     stat_add(&worker->add_stat, start_time);
>     return TRUE;
> }
>@@ -3567,7 +3596,7 @@ static inline int red_current_add_with_shadow(RedWorker *worker, Ring *ring, Dra
> 
>     // only primary surface streams are supported
>     if (is_primary_surface(worker, item->surface_id)) {
>-        red_detach_streams_behind(worker, &shadow->base.rgn);
>+        red_detach_streams_behind(worker, &shadow->base.rgn, NULL);
>     }
>     ring_add(ring, &shadow->base.siblings_link);
>     __current_add_drawable(worker, item, ring);
>@@ -3579,7 +3608,7 @@ static inline int red_current_add_with_shadow(RedWorker *worker, Ring *ring, Dra
>         red_streams_update_visible_region(worker, item);
>     } else {
>         if (item->surface_id == 0) {
>-            red_detach_streams_behind(worker, &item->tree_item.base.rgn);
>+            red_detach_streams_behind(worker, &item->tree_item.base.rgn, item);
>         }
>     }
>     stat_add(&worker->add_stat, start_time);
>@@ -3911,7 +3940,7 @@ static inline int red_handle_surfaces_dependencies(RedWorker *worker, Drawable *
>                 QRegion depend_region;
>                 region_init(&depend_region);
>                 region_add(&depend_region, &drawable->red_drawable->surfaces_rects[x]);
>-                red_detach_streams_behind(worker, &depend_region);
>+                red_detach_streams_behind(worker, &depend_region, NULL);
>             }
>         }
>     }
>-- 
>1.8.0.1
>

Actions: View | Diff

Attachments on bug 891326: 674984 | 678812