On Thu, Nov 5, 2015 at 10:14 AM, Frediano Ziglio <fziglio@xxxxxxxxxx> wrote:
> From: Marc-André Lureau <marcandre.lureau@xxxxxxxxx>
>
> ---
> 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
>
> _______________________________________________
> Spice-devel mailing list
> Spice-devel@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/spice-devel
Apart from the comment, looks good. ACK with the suggested change.
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/spice-devel