[Spice-devel] [PATCH 05/10] worker: move shadow_new() and container_new()

Frediano Ziglio fziglio at redhat.com
Fri Nov 6 02:11:47 PST 2015


> 
> On Thu, Nov 5, 2015 at 10:14 AM, Frediano Ziglio <fziglio at redhat.com> wrote:
> > From: Marc-André Lureau <marcandre.lureau at gmail.com>
> >
> > ---
> >  server/red_worker.c | 49 +++++++------------------------------------------
> >  server/tree.c       | 39 +++++++++++++++++++++++++++++++++++++++
> >  server/tree.h       |  9 +++++++++
> >  3 files changed, 55 insertions(+), 42 deletions(-)
> >
> > diff --git a/server/red_worker.c b/server/red_worker.c
> > index 8cd709a..dc646bc 100644
> > --- a/server/red_worker.c
> > +++ b/server/red_worker.c
> > @@ -1585,24 +1585,6 @@ static void exclude_region(RedWorker *worker, Ring
> > *ring, RingItem *ring_item, Q
> >      }
> >  }
> >
> > -static inline Container *__new_container(RedWorker *worker, DrawItem
> > *item)
> > -{
> > -    Container *container = spice_new(Container, 1);
> > -    worker->containers_count++;
> > -    container->base.type = TREE_ITEM_TYPE_CONTAINER;
> > -    container->base.container = item->base.container;
> > -    item->base.container = container;
> > -    item->container_root = TRUE;
> > -    region_clone(&container->base.rgn, &item->base.rgn);
> > -    ring_item_init(&container->base.siblings_link);
> > -    ring_add_after(&container->base.siblings_link,
> > &item->base.siblings_link);
> > -    ring_remove(&item->base.siblings_link);
> > -    ring_init(&container->items);
> > -    ring_add(&container->items, &item->base.siblings_link);
> > -
> > -    return container;
> > -}
> > -
> >  static inline int is_opaque_item(TreeItem *item)
> >  {
> >      return item->type == TREE_ITEM_TYPE_CONTAINER ||
> > @@ -2910,8 +2892,8 @@ static inline int red_current_add(RedWorker *worker,
> > Ring *ring, Drawable *drawa
> >                      continue;
> >                  }
> >                  spice_assert(IS_DRAW_ITEM(sibling));
> > -                if (!((DrawItem *)sibling)->container_root) {
> > -                    container = __new_container(worker, (DrawItem
> > *)sibling);
> > +                if (!DRAW_ITEM(sibling)->container_root) {
> > +                    container = container_new(DRAW_ITEM(sibling));
> 
> When container_new() was added, increasing worker->containers_count
> became responsibility of the caller. So, here (acutally, after the
> below check, you have to increment worker->containers_count (in the
> same way you do for worker->shadows_count).
> 
> >                      if (!container) {
> >                          spice_warning("create new container failed");
> >                          region_destroy(&exclude_rgn);
> > @@ -2945,7 +2927,7 @@ static inline int red_current_add(RedWorker *worker,
> > Ring *ring, Drawable *drawa
> >           * before calling red_detach_streams_behind
> >           */
> >          __current_add_drawable(worker, drawable, ring);
> > -        if (drawable->surface_id == 0) {
> > +        if (is_primary_surface(worker, drawable->surface_id)) {
> >              red_detach_streams_behind(worker,
> >              &drawable->tree_item.base.rgn, drawable);
> >          }
> >      }
> > @@ -2963,25 +2945,6 @@ static void add_clip_rects(QRegion *rgn,
> > SpiceClipRects *data)
> >      }
> >  }
> >
> > -static inline Shadow *__new_shadow(RedWorker *worker, Drawable *item,
> > SpicePoint *delta)
> > -{
> > -    if (!delta->x && !delta->y) {
> > -        return NULL;
> > -    }
> > -
> > -    Shadow *shadow = spice_new(Shadow, 1);
> > -    worker->shadows_count++;
> > -    shadow->base.type = TREE_ITEM_TYPE_SHADOW;
> > -    shadow->base.container = NULL;
> > -    shadow->owner = &item->tree_item;
> > -    region_clone(&shadow->base.rgn, &item->tree_item.base.rgn);
> > -    region_offset(&shadow->base.rgn, delta->x, delta->y);
> > -    ring_item_init(&shadow->base.siblings_link);
> > -    region_init(&shadow->on_hold);
> > -    item->tree_item.shadow = shadow;
> > -    return shadow;
> > -}
> > -
> >  static inline int red_current_add_with_shadow(RedWorker *worker, Ring
> >  *ring, Drawable *item)
> >  {
> >  #ifdef RED_WORKER_STAT
> > @@ -2995,11 +2958,12 @@ static inline int
> > red_current_add_with_shadow(RedWorker *worker, Ring *ring, Dra
> >          .y = red_drawable->u.copy_bits.src_pos.y - red_drawable->bbox.top
> >      };
> >
> > -    Shadow *shadow = __new_shadow(worker, item, &delta);
> > +    Shadow *shadow = shadow_new(&item->tree_item, &delta);
> >      if (!shadow) {
> >          stat_add(&worker->add_stat, start_time);
> >          return FALSE;
> >      }
> > +    worker->shadows_count++;
> >      // item and his shadow must initially be placed in the same container.
> >      // for now putting them on root.
> >
> > @@ -3007,6 +2971,7 @@ static inline int
> > red_current_add_with_shadow(RedWorker *worker, Ring *ring, Dra
> >      if (is_primary_surface(worker, item->surface_id)) {
> >          red_detach_streams_behind(worker, &shadow->base.rgn, NULL);
> >      }
> > +
> >      ring_add(ring, &shadow->base.siblings_link);
> >      __current_add_drawable(worker, item, ring);
> >      if (item->tree_item.effect == QXL_EFFECT_OPAQUE) {
> > @@ -3016,7 +2981,7 @@ static inline int
> > red_current_add_with_shadow(RedWorker *worker, Ring *ring, Dra
> >          region_destroy(&exclude_rgn);
> >          red_streams_update_visible_region(worker, item);
> >      } else {
> > -        if (item->surface_id == 0) {
> > +        if (is_primary_surface(worker, item->surface_id)) {
> >              red_detach_streams_behind(worker, &item->tree_item.base.rgn,
> >              item);
> >          }
> >      }
> > diff --git a/server/tree.c b/server/tree.c
> > index ed7d39a..64fa6db 100644
> > --- a/server/tree.c
> > +++ b/server/tree.c
> > @@ -180,3 +180,42 @@ void tree_item_dump(TreeItem *item)
> >      spice_return_if_fail(item != NULL);
> >      tree_foreach(item, dump_item, &di);
> >  }
> > +
> > +Shadow* shadow_new(DrawItem *item, const SpicePoint *delta)
> > +{
> > +    spice_return_val_if_fail(item->shadow == NULL, NULL);
> > +    if (!delta->x && !delta->y) {
> > +        return NULL;
> > +    }
> > +
> > +    Shadow *shadow = spice_new(Shadow, 1);
> > +
> > +    shadow->base.type = TREE_ITEM_TYPE_SHADOW;
> > +    shadow->base.container = NULL;
> > +    shadow->owner = item;
> > +    region_clone(&shadow->base.rgn, &item->base.rgn);
> > +    region_offset(&shadow->base.rgn, delta->x, delta->y);
> > +    ring_item_init(&shadow->base.siblings_link);
> > +    region_init(&shadow->on_hold);
> > +    item->shadow = shadow;
> > +
> > +    return shadow;
> > +}
> > +
> > +Container* container_new(DrawItem *item)
> > +{
> > +    Container *container = spice_new(Container, 1);
> > +
> > +    container->base.type = TREE_ITEM_TYPE_CONTAINER;
> > +    container->base.container = item->base.container;
> > +    item->base.container = container;
> > +    item->container_root = TRUE;
> > +    region_clone(&container->base.rgn, &item->base.rgn);
> > +    ring_item_init(&container->base.siblings_link);
> > +    ring_add_after(&container->base.siblings_link,
> > &item->base.siblings_link);
> > +    ring_remove(&item->base.siblings_link);
> > +    ring_init(&container->items);
> > +    ring_add(&container->items, &item->base.siblings_link);
> > +
> > +    return container;
> > +}
> > diff --git a/server/tree.h b/server/tree.h
> > index 82f9ce3..6e83f7a 100644
> > --- a/server/tree.h
> > +++ b/server/tree.h
> > @@ -52,11 +52,17 @@ struct Shadow {
> >      DrawItem* owner;
> >  };
> >
> > +#define IS_SHADOW(item) ((item)->type == TREE_ITEM_TYPE_SHADOW)
> > +#define SHADOW(item) ((Shadow*)(item))
> > +
> >  struct Container {
> >      TreeItem base;
> >      Ring items;
> >  };
> >
> > +#define IS_CONTAINER(item) ((item)->type == TREE_ITEM_TYPE_CONTAINER)
> > +#define CONTAINER(item) ((Container*)(item))
> > +
> >  struct DrawItem {
> >      TreeItem base;
> >      uint8_t effect;
> > @@ -65,7 +71,10 @@ struct DrawItem {
> >  };
> >
> >  #define IS_DRAW_ITEM(item) ((item)->type == TREE_ITEM_TYPE_DRAWABLE)
> > +#define DRAW_ITEM(item) ((DrawItem*)(item))
> >
> >  void       tree_item_dump                           (TreeItem *item);
> > +Shadow*    shadow_new                               (DrawItem *item, const
> > SpicePoint *delta);
> > +Container* container_new                            (DrawItem *item);
> >
> >  #endif /* TREE_H_ */
> > --
> > 2.4.3
> >
> 
> Apart from the comment, looks good. ACK with the suggested change.
> 

Merged (with the fix for counter)

Frediano


More information about the Spice-devel mailing list