[Spice-devel] [PATCH 1/9] Move some tree item functions to tree.[ch]

[Jonathon Jongsma]

Hmm, it doesn't seem that this patch changed since the last version you sent.
Except some of the context is slightly different, perhaps because of re
-ordering? (e.g. exclude_region has a RedWorker* as the first parameter in this
patch. In the previous patch, it had DisplayChannel* as the first parameter).

Were there supposed to be other changes? Perhaps removing the shadow_free
comment like we discussed?


On Tue, 2015-11-17 at 16:37 +0000, Frediano Ziglio wrote:
> From: Jonathon Jongsma <jjongsma at redhat.com>
> 
> Also rename some functions slightly:
>      __find_shadow -> tree_item_find_shadow()
>      __contained_by -> tree_item_contained_by()
>      ring_of -> tree_item_container_items();
> 
> Acked-by: Fabiano Fidêncio <fidencio at redhat.com>
> ---
>  server/red_worker.c | 62 ++++++----------------------------------------------
> -
>  server/tree.c       | 50 ++++++++++++++++++++++++++++++++++++++++++
>  server/tree.h       |  5 +++++
>  3 files changed, 62 insertions(+), 55 deletions(-)
> 
> diff --git a/server/red_worker.c b/server/red_worker.c
> index 3a718fd..9dca861 100644
> --- a/server/red_worker.c
> +++ b/server/red_worker.c
> @@ -943,21 +943,6 @@ void display_channel_drawable_unref(DisplayChannel
> *display, Drawable *drawable)
>      display->drawable_count--;
>  }
>  
> -static inline void remove_shadow(DrawItem *item)
> -{
> -    Shadow *shadow;
> -
> -    if (!item->shadow) {
> -        return;
> -    }
> -    shadow = item->shadow;
> -    item->shadow = NULL;
> -    ring_remove(&shadow->base.siblings_link);
> -    region_destroy(&shadow->base.rgn);
> -    region_destroy(&shadow->on_hold);
> -    free(shadow);
> -}
> -
>  static void display_stream_trace_add_drawable(DisplayChannel *display,
> Drawable *item)
>  {
>      ItemTrace *trace;
> @@ -1001,7 +986,7 @@ static inline void current_remove_drawable(RedWorker
> *worker, Drawable *item)
>      DisplayChannel *display = worker->display_channel;
>  
>      display_stream_trace_add_drawable(display, item);
> -    remove_shadow(&item->tree_item);
> +    draw_item_remove_shadow(&item->tree_item);
>      ring_remove(&item->tree_item.base.siblings_link);
>      ring_remove(&item->list_link);
>      ring_remove(&item->surface_list_link);
> @@ -1155,39 +1140,6 @@ static void
> red_clear_surface_drawables_from_pipes(RedWorker *worker,
>      }
>  }
>  
> -static inline Shadow *__find_shadow(TreeItem *item)
> -{
> -    while (item->type == TREE_ITEM_TYPE_CONTAINER) {
> -        if (!(item = (TreeItem *)ring_get_tail(&((Container *)item)->items)))
> {
> -            return NULL;
> -        }
> -    }
> -
> -    if (item->type != TREE_ITEM_TYPE_DRAWABLE) {
> -        return NULL;
> -    }
> -
> -    return ((DrawItem *)item)->shadow;
> -}
> -
> -static inline Ring *ring_of(Ring *ring, TreeItem *item)
> -{
> -    return (item->container) ? &item->container->items : ring;
> -}
> -
> -static inline int __contained_by(TreeItem *item, Ring *ring)
> -{
> -    spice_assert(item && ring);
> -    do {
> -        Ring *now = ring_of(ring, item);
> -        if (now == ring) {
> -            return TRUE;
> -        }
> -    } while ((item = (TreeItem *)item->container));
> -
> -    return FALSE;
> -}
> -
>  static inline void __exclude_region(RedWorker *worker, Ring *ring, TreeItem
> *item, QRegion *rgn,
>                                      Ring **top_ring, Drawable
> *frame_candidate)
>  {
> @@ -1221,8 +1173,8 @@ static inline void __exclude_region(RedWorker *worker,
> Ring *ring, TreeItem *ite
>                      region_exclude(&shadow->on_hold, &and_rgn);
>                      region_or(rgn, &and_rgn);
>                      // in flat representation of current, shadow is always
> his owner next
> -                    if (!__contained_by((TreeItem*)shadow, *top_ring)) {
> -                        *top_ring = ring_of(ring, (TreeItem*)shadow);
> +                    if (!tree_item_contained_by((TreeItem*)shadow,
> *top_ring)) {
> +                        *top_ring =
> tree_item_container_items((TreeItem*)shadow, ring);
>                      }
>                  }
>              } else {
> @@ -1239,10 +1191,10 @@ static inline void __exclude_region(RedWorker *worker,
> Ring *ring, TreeItem *ite
>                  Shadow *shadow;
>  
>                  region_exclude(rgn, &and_rgn);
> -                if ((shadow = __find_shadow(item))) {
> +                if ((shadow = tree_item_find_shadow(item))) {
>                      region_or(rgn, &shadow->on_hold);
> -                    if (!__contained_by((TreeItem*)shadow, *top_ring)) {
> -                        *top_ring = ring_of(ring, (TreeItem*)shadow);
> +                    if (!tree_item_contained_by((TreeItem*)shadow,
> *top_ring)) {
> +                        *top_ring =
> tree_item_container_items((TreeItem*)shadow, ring);
>                      }
>                  }
>              }
> @@ -2254,7 +2206,7 @@ static inline int current_add(RedWorker *worker, Ring
> *ring, Drawable *drawable)
>                  Shadow *shadow;
>                  int skip = now == exclude_base;
>  
> -                if ((shadow = __find_shadow(sibling))) {
> +                if ((shadow = tree_item_find_shadow(sibling))) {
>                      if (exclude_base) {
>                          TreeItem *next = sibling;
>                          exclude_region(worker, ring, exclude_base,
> &exclude_rgn, &next, NULL);
> diff --git a/server/tree.c b/server/tree.c
> index ad31f09..1daa90c 100644
> --- a/server/tree.c
> +++ b/server/tree.c
> @@ -250,3 +250,53 @@ void container_cleanup(Container *container)
>          container = next;
>      }
>  }
> +
> +/* FIXME: document weird function: go down containers, and return drawable
> ->shadow? */
> +Shadow* tree_item_find_shadow(TreeItem *item)
> +{
> +    while (item->type == TREE_ITEM_TYPE_CONTAINER) {
> +        if (!(item = (TreeItem *)ring_get_tail(&((Container *)item)->items)))
> {
> +            return NULL;
> +        }
> +    }
> +
> +    if (item->type != TREE_ITEM_TYPE_DRAWABLE) {
> +        return NULL;
> +    }
> +
> +    return ((DrawItem *)item)->shadow;
> +}
> +
> +Ring *tree_item_container_items(TreeItem *item, Ring *ring)
> +{
> +    return (item->container) ? &item->container->items : ring;
> +}
> +
> +int tree_item_contained_by(TreeItem *item, Ring *ring)
> +{
> +    spice_assert(item && ring);
> +    do {
> +        Ring *now = tree_item_container_items(item, ring);
> +        if (now == ring) {
> +            return TRUE;
> +        }
> +    } while ((item = (TreeItem *)item->container));
> +
> +    return FALSE;
> +}
> +
> +void draw_item_remove_shadow(DrawItem *item)
> +{
> +    Shadow *shadow;
> +
> +    if (!item->shadow) {
> +        return;
> +    }
> +    shadow = item->shadow;
> +    item->shadow = NULL;
> +    /* shadow_free? */
> +    ring_remove(&shadow->base.siblings_link);
> +    region_destroy(&shadow->base.rgn);
> +    region_destroy(&shadow->on_hold);
> +    free(shadow);
> +}
> diff --git a/server/tree.h b/server/tree.h
> index 01d4ff9..8b9c6ba 100644
> --- a/server/tree.h
> +++ b/server/tree.h
> @@ -80,6 +80,11 @@ static inline int is_opaque_item(TreeItem *item)
>  }
>  
>  void       tree_item_dump                           (TreeItem *item);
> +Shadow*    tree_item_find_shadow                    (TreeItem *item);
> +int        tree_item_contained_by                   (TreeItem *item, Ring
> *ring);
> +Ring*      tree_item_container_items                (TreeItem *item, Ring
> *ring);
> +
> +void       draw_item_remove_shadow                  (DrawItem *item);
>  Shadow*    shadow_new                               (DrawItem *item, const
> SpicePoint *delta);
>  Container* container_new                            (DrawItem *item);
>  void       container_free                           (Container *container);