[Spice-devel] [PATCH 06/11] Move some tree item functions to tree.[ch]
Frediano Ziglio
fziglio at redhat.com
Mon Nov 16 05:35:21 PST 2015
>
> On Mon, Nov 16, 2015 at 12:06 PM, Frediano Ziglio <fziglio at redhat.com> 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();
> > ---
> > 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 556963b..4734187 100644
> > --- a/server/red_worker.c
> > +++ b/server/red_worker.c
> > @@ -787,21 +787,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;
> > @@ -843,7 +828,7 @@ static void red_flush_source_surfaces(DisplayChannel
> > *display, Drawable *drawabl
> > static inline void current_remove_drawable(DisplayChannel *display,
> > Drawable *item)
> > {
> > 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);
> > @@ -999,39 +984,6 @@ static void
> > red_clear_surface_drawables_from_pipes(DisplayChannel *display,
> > }
> > }
> >
> > -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 void __exclude_region(DisplayChannel *display, Ring *ring, TreeItem
> > *item, QRegion *rgn,
> > Ring **top_ring, Drawable *frame_candidate)
> > {
> > @@ -1066,8 +1018,8 @@ static void __exclude_region(DisplayChannel *display,
> > Ring *ring, TreeItem *item
> > 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 {
> > @@ -1084,10 +1036,10 @@ static void __exclude_region(DisplayChannel
> > *display, Ring *ring, TreeItem *item
> > 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);
> > }
> > }
> > }
> > @@ -2112,7 +2064,7 @@ static int current_add(DisplayChannel *display, 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(display, 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? */
This comment is quite weird. I actually cannot address it.
Why considering only last items (ring_get_tail) ?
> > +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? */
I think this comment is suggesting to add the function,
would be better
/* TODO add and use a shadow_free */
I think there are 3 choices:
- remove the comment
- change to a proper TODO
- do it (add the function and call it)
> > + 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);
> > --
> > 2.4.3
> >
> > _______________________________________________
> > Spice-devel mailing list
> > Spice-devel at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/spice-devel
>
> Seems okay, ACK!
>
Frediano
More information about the Spice-devel
mailing list