[Spice-commits] 5 commits - server/red_parse_qxl.c server/red_parse_qxl.h server/red_worker.c

Alon Levy alon at kemper.freedesktop.org
Tue May 15 08:02:23 PDT 2012


 server/red_parse_qxl.c |    3 ++
 server/red_parse_qxl.h |    1 
 server/red_worker.c    |   61 ++++++++++++++++++++-----------------------------
 3 files changed, 30 insertions(+), 35 deletions(-)

New commits:
commit 05f4276cc128c500cd88df13e44b4bff7d5d0937
Author: Alon Levy <alevy at redhat.com>
Date:   Tue May 15 13:36:36 2012 +0300

    server: move self_bitmap_image to RedDrawable
    
    Simplify keeping count of self_bitmap_image by putting it in
    RedDrawable. It is allocated on reading from the command pipe and
    deallocated when the last reference to the RedDrawable is dropped,
    instead of keeping track of it in GlzDrawable and Drawable.

diff --git a/server/red_parse_qxl.c b/server/red_parse_qxl.c
index 3abf638..e6c2705 100644
--- a/server/red_parse_qxl.c
+++ b/server/red_parse_qxl.c
@@ -1004,6 +1004,9 @@ int red_get_drawable(RedMemSlotInfo *slots, int group_id,
 void red_put_drawable(RedDrawable *red)
 {
     red_put_clip(&red->clip);
+    if (red->self_bitmap_image) {
+        red_put_image(red->self_bitmap_image);
+    }
     switch (red->type) {
     case QXL_DRAW_ALPHA_BLEND:
         red_put_alpha_blend(&red->u.alpha_blend);
diff --git a/server/red_parse_qxl.h b/server/red_parse_qxl.h
index d01d363..70f8509 100644
--- a/server/red_parse_qxl.h
+++ b/server/red_parse_qxl.h
@@ -31,6 +31,7 @@ typedef struct RedDrawable {
     uint8_t type;
     uint8_t self_bitmap;
     SpiceRect self_bitmap_area;
+    SpiceImage *self_bitmap_image;
     SpiceRect bbox;
     SpiceClip clip;
     uint32_t mm_time;
diff --git a/server/red_worker.c b/server/red_worker.c
index 9c1266c..dd00bff 100644
--- a/server/red_worker.c
+++ b/server/red_worker.c
@@ -570,7 +570,6 @@ struct RedGlzDrawable {
     RedDrawable *red_drawable;
     Drawable    *drawable;
     uint32_t     group_id;
-    SpiceImage  *self_bitmap_image;
     GlzDrawableInstanceItem instances_pool[MAX_GLZ_DRAWABLE_INSTANCES];
     Ring instances;
     uint8_t instances_count;
@@ -802,7 +801,6 @@ struct Drawable {
     int streamable;
     BitmapGradualType copy_bitmap_graduality;
     uint32_t group_id;
-    SpiceImage *self_bitmap_image;
     DependItem depend_items[3];
 
     uint8_t *backed_surface_data;
@@ -1691,16 +1689,13 @@ static RedDrawable *ref_red_drawable(RedDrawable *drawable)
 
 
 static inline void put_red_drawable(RedWorker *worker, RedDrawable *red_drawable,
-                                    uint32_t group_id, SpiceImage *self_bitmap_image)
+                                    uint32_t group_id)
 {
     QXLReleaseInfoExt release_info_ext;
 
     if (--red_drawable->refs) {
         return;
     }
-    if (self_bitmap_image) {
-        red_put_image(self_bitmap_image);
-    }
     worker->red_drawable_count--;
     release_info_ext.group_id = group_id;
     release_info_ext.info = red_drawable->release_info;
@@ -1765,8 +1760,7 @@ static inline void release_drawable(RedWorker *worker, Drawable *drawable)
             SPICE_CONTAINEROF(item, RedGlzDrawable, drawable_link)->drawable = NULL;
             ring_remove(item);
         }
-        put_red_drawable(worker, drawable->red_drawable,
-                          drawable->group_id, drawable->self_bitmap_image);
+        put_red_drawable(worker, drawable->red_drawable, drawable->group_id);
         free_drawable(worker, drawable);
         worker->drawable_count--;
     }
@@ -3725,7 +3719,7 @@ static inline int red_handle_self_bitmap(RedWorker *worker, Drawable *drawable)
         }
     }
 
-    drawable->self_bitmap_image = image;
+    red_drawable->self_bitmap_image = image;
     return TRUE;
 }
 
@@ -4147,8 +4141,8 @@ static void localize_bitmap(RedWorker *worker, SpiceImage **image_ptr, SpiceImag
 
     if (image == NULL) {
         spice_assert(drawable != NULL);
-        spice_assert(drawable->self_bitmap_image != NULL);
-        *image_ptr = drawable->self_bitmap_image;
+        spice_assert(drawable->red_drawable->self_bitmap_image != NULL);
+        *image_ptr = drawable->red_drawable->self_bitmap_image;
         return;
     }
 
@@ -4851,7 +4845,7 @@ static int red_process_commands(RedWorker *worker, uint32_t max_pipe_size, int *
             }
             red_process_drawable(worker, red_drawable, ext_cmd.group_id);
             // release the red_drawable
-            put_red_drawable(worker, red_drawable, ext_cmd.group_id, NULL);
+            put_red_drawable(worker, red_drawable, ext_cmd.group_id);
             break;
         }
         case QXL_CMD_UPDATE: {
@@ -5202,7 +5196,6 @@ static RedGlzDrawable *red_display_get_glz_drawable(DisplayChannelClient *dcc, D
     ret->red_drawable = ref_red_drawable(drawable->red_drawable);
     ret->drawable = drawable;
     ret->group_id = drawable->group_id;
-    ret->self_bitmap_image = drawable->self_bitmap_image;
     ret->instances_count = 0;
     ring_init(&ret->instances);
 
@@ -5269,7 +5262,7 @@ static void red_display_free_glz_drawable_instance(DisplayChannelClient *dcc,
             ring_remove(&glz_drawable->drawable_link);
         }
         put_red_drawable(worker, glz_drawable->red_drawable,
-                          glz_drawable->group_id, glz_drawable->self_bitmap_image);
+                         glz_drawable->group_id);
         worker->glz_drawable_count--;
         if (ring_item_is_linked(&glz_drawable->link)) {
             ring_remove(&glz_drawable->link);
@@ -6428,8 +6421,8 @@ static FillBitsType fill_bits(DisplayChannelClient *dcc, SpiceMarshaller *m,
     SpiceMarshaller *bitmap_palette_out, *lzplt_palette_out;
 
     if (simage == NULL) {
-        spice_assert(drawable->self_bitmap_image);
-        simage = drawable->self_bitmap_image;
+        spice_assert(drawable->red_drawable->self_bitmap_image);
+        simage = drawable->red_drawable->self_bitmap_image;
     }
 
     image.descriptor = simage->descriptor;
commit cc1fd8eae7a126d9320c4432eb6ebc54b1ffc201
Author: Alon Levy <alevy at redhat.com>
Date:   Tue May 15 12:41:54 2012 +0300

    server/red_worker/put_red_drawable: s/drawable/red_drawable/

diff --git a/server/red_worker.c b/server/red_worker.c
index f0192a3..9c1266c 100644
--- a/server/red_worker.c
+++ b/server/red_worker.c
@@ -1690,12 +1690,12 @@ static RedDrawable *ref_red_drawable(RedDrawable *drawable)
 }
 
 
-static inline void put_red_drawable(RedWorker *worker, RedDrawable *drawable, uint32_t group_id,
-                                     SpiceImage *self_bitmap_image)
+static inline void put_red_drawable(RedWorker *worker, RedDrawable *red_drawable,
+                                    uint32_t group_id, SpiceImage *self_bitmap_image)
 {
     QXLReleaseInfoExt release_info_ext;
 
-    if (--drawable->refs) {
+    if (--red_drawable->refs) {
         return;
     }
     if (self_bitmap_image) {
@@ -1703,10 +1703,10 @@ static inline void put_red_drawable(RedWorker *worker, RedDrawable *drawable, ui
     }
     worker->red_drawable_count--;
     release_info_ext.group_id = group_id;
-    release_info_ext.info = drawable->release_info;
+    release_info_ext.info = red_drawable->release_info;
     worker->qxl->st->qif->release_resource(worker->qxl, release_info_ext);
-    red_put_drawable(drawable);
-    free(drawable);
+    red_put_drawable(red_drawable);
+    free(red_drawable);
 }
 
 static void remove_depended_item(DependItem *item)
commit c6bb40947febf651706a0cd804bb2a8096574ea7
Author: Alon Levy <alevy at redhat.com>
Date:   Tue May 15 12:40:07 2012 +0300

    server/red_worker/red_handle_self_bitmap: add red_drawable local, one extra whitespace line removed

diff --git a/server/red_worker.c b/server/red_worker.c
index bd76e04..f0192a3 100644
--- a/server/red_worker.c
+++ b/server/red_worker.c
@@ -3678,20 +3678,20 @@ static inline int red_handle_self_bitmap(RedWorker *worker, Drawable *drawable)
     RedSurface *surface;
     int bpp;
     int all_set;
+    RedDrawable *red_drawable = drawable->red_drawable;
 
-    if (!drawable->red_drawable->self_bitmap) {
+    if (!red_drawable->self_bitmap) {
         return TRUE;
     }
 
-
     surface = &worker->surfaces[drawable->surface_id];
 
     bpp = SPICE_SURFACE_FMT_DEPTH(surface->context.format) / 8;
 
-    width = drawable->red_drawable->self_bitmap_area.right
-            - drawable->red_drawable->self_bitmap_area.left;
-    height = drawable->red_drawable->self_bitmap_area.bottom
-            - drawable->red_drawable->self_bitmap_area.top;
+    width = red_drawable->self_bitmap_area.right
+            - red_drawable->self_bitmap_area.left;
+    height = red_drawable->self_bitmap_area.bottom
+            - red_drawable->self_bitmap_area.top;
     dest_stride = SPICE_ALIGN(width * bpp, 4);
 
     image = spice_new0(SpiceImage, 1);
@@ -3711,7 +3711,7 @@ static inline int red_handle_self_bitmap(RedWorker *worker, Drawable *drawable)
     image->u.bitmap.data->flags |= SPICE_CHUNKS_FLAGS_FREE;
 
     red_get_area(worker, drawable->surface_id,
-                 &drawable->red_drawable->self_bitmap_area, dest, dest_stride, TRUE);
+                 &red_drawable->self_bitmap_area, dest, dest_stride, TRUE);
 
     /* 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 */
commit 96bfcea9659cebb5b121d1472bde045add3ee491
Author: Alon Levy <alevy at redhat.com>
Date:   Tue May 15 12:35:40 2012 +0300

    server/red_worker: rename SpiceImage *self_bitmap to self_bitmap_image

diff --git a/server/red_worker.c b/server/red_worker.c
index e1c86fa..bd76e04 100644
--- a/server/red_worker.c
+++ b/server/red_worker.c
@@ -570,7 +570,7 @@ struct RedGlzDrawable {
     RedDrawable *red_drawable;
     Drawable    *drawable;
     uint32_t     group_id;
-    SpiceImage  *self_bitmap;
+    SpiceImage  *self_bitmap_image;
     GlzDrawableInstanceItem instances_pool[MAX_GLZ_DRAWABLE_INSTANCES];
     Ring instances;
     uint8_t instances_count;
@@ -802,7 +802,7 @@ struct Drawable {
     int streamable;
     BitmapGradualType copy_bitmap_graduality;
     uint32_t group_id;
-    SpiceImage *self_bitmap;
+    SpiceImage *self_bitmap_image;
     DependItem depend_items[3];
 
     uint8_t *backed_surface_data;
@@ -1691,15 +1691,15 @@ static RedDrawable *ref_red_drawable(RedDrawable *drawable)
 
 
 static inline void put_red_drawable(RedWorker *worker, RedDrawable *drawable, uint32_t group_id,
-                                     SpiceImage *self_bitmap)
+                                     SpiceImage *self_bitmap_image)
 {
     QXLReleaseInfoExt release_info_ext;
 
     if (--drawable->refs) {
         return;
     }
-    if (self_bitmap) {
-        red_put_image(self_bitmap);
+    if (self_bitmap_image) {
+        red_put_image(self_bitmap_image);
     }
     worker->red_drawable_count--;
     release_info_ext.group_id = group_id;
@@ -1766,7 +1766,7 @@ static inline void release_drawable(RedWorker *worker, Drawable *drawable)
             ring_remove(item);
         }
         put_red_drawable(worker, drawable->red_drawable,
-                          drawable->group_id, drawable->self_bitmap);
+                          drawable->group_id, drawable->self_bitmap_image);
         free_drawable(worker, drawable);
         worker->drawable_count--;
     }
@@ -3725,7 +3725,7 @@ static inline int red_handle_self_bitmap(RedWorker *worker, Drawable *drawable)
         }
     }
 
-    drawable->self_bitmap = image;
+    drawable->self_bitmap_image = image;
     return TRUE;
 }
 
@@ -4147,8 +4147,8 @@ static void localize_bitmap(RedWorker *worker, SpiceImage **image_ptr, SpiceImag
 
     if (image == NULL) {
         spice_assert(drawable != NULL);
-        spice_assert(drawable->self_bitmap != NULL);
-        *image_ptr = drawable->self_bitmap;
+        spice_assert(drawable->self_bitmap_image != NULL);
+        *image_ptr = drawable->self_bitmap_image;
         return;
     }
 
@@ -5202,7 +5202,7 @@ static RedGlzDrawable *red_display_get_glz_drawable(DisplayChannelClient *dcc, D
     ret->red_drawable = ref_red_drawable(drawable->red_drawable);
     ret->drawable = drawable;
     ret->group_id = drawable->group_id;
-    ret->self_bitmap = drawable->self_bitmap;
+    ret->self_bitmap_image = drawable->self_bitmap_image;
     ret->instances_count = 0;
     ring_init(&ret->instances);
 
@@ -5269,7 +5269,7 @@ static void red_display_free_glz_drawable_instance(DisplayChannelClient *dcc,
             ring_remove(&glz_drawable->drawable_link);
         }
         put_red_drawable(worker, glz_drawable->red_drawable,
-                          glz_drawable->group_id, glz_drawable->self_bitmap);
+                          glz_drawable->group_id, glz_drawable->self_bitmap_image);
         worker->glz_drawable_count--;
         if (ring_item_is_linked(&glz_drawable->link)) {
             ring_remove(&glz_drawable->link);
@@ -6428,8 +6428,8 @@ static FillBitsType fill_bits(DisplayChannelClient *dcc, SpiceMarshaller *m,
     SpiceMarshaller *bitmap_palette_out, *lzplt_palette_out;
 
     if (simage == NULL) {
-        spice_assert(drawable->self_bitmap);
-        simage = drawable->self_bitmap;
+        spice_assert(drawable->self_bitmap_image);
+        simage = drawable->self_bitmap_image;
     }
 
     image.descriptor = simage->descriptor;
commit 8f65b3fb36e96ab0a9acc0f5b9813cdc5de73096
Author: Alon Levy <alevy at redhat.com>
Date:   Tue May 15 18:01:04 2012 +0300

    Revert "server/red_worker: fix possible leak of self_bitmap"
    
    This reverts commit 35dbf3ccc4b852f9dbb29eb8a53c94f26d2e3a6e.
    
    accidentally pushed v1 of patches, reverting in preperation of pushing
    v2.

diff --git a/server/red_worker.c b/server/red_worker.c
index 1adf4c9..e1c86fa 100644
--- a/server/red_worker.c
+++ b/server/red_worker.c
@@ -3867,8 +3867,8 @@ static inline void red_inc_surfaces_drawable_dependencies(RedWorker *worker, Dra
     }
 }
 
-static inline Drawable *red_process_drawable(RedWorker *worker, RedDrawable *drawable,
-                                             uint32_t group_id)
+static inline void red_process_drawable(RedWorker *worker, RedDrawable *drawable,
+                                        uint32_t group_id)
 {
     int surface_id;
     Drawable *item = get_drawable(worker, drawable->effect, drawable, group_id);
@@ -3931,7 +3931,7 @@ static inline Drawable *red_process_drawable(RedWorker *worker, RedDrawable *dra
 #endif
     }
 cleanup:
-    return item;
+    release_drawable(worker, item);
 }
 
 static inline void red_create_surface(RedWorker *worker, uint32_t surface_id,uint32_t width,
@@ -4844,16 +4844,14 @@ static int red_process_commands(RedWorker *worker, uint32_t max_pipe_size, int *
         switch (ext_cmd.cmd.type) {
         case QXL_CMD_DRAW: {
             RedDrawable *red_drawable = red_drawable_new(); // returns with 1 ref
-            Drawable *drawable;
 
             if (red_get_drawable(&worker->mem_slots, ext_cmd.group_id,
                                  red_drawable, ext_cmd.cmd.data, ext_cmd.flags)) {
                 break;
             }
-            drawable = red_process_drawable(worker, red_drawable, ext_cmd.group_id);
-            // release red_drawable first, it will not die because drawable holds a reference on it.
+            red_process_drawable(worker, red_drawable, ext_cmd.group_id);
+            // release the red_drawable
             put_red_drawable(worker, red_drawable, ext_cmd.group_id, NULL);
-            release_drawable(worker, drawable);
             break;
         }
         case QXL_CMD_UPDATE: {


More information about the Spice-commits mailing list