[Spice-devel] [PATCH 9/9] worker: painfully move display_channel_add_drawable

Frediano Ziglio fziglio at redhat.com
Tue Nov 17 10:20:01 PST 2015


I generated a diff without the move, see
http://pastebin.com/4G6BtBff

> 
> On Fri, 2015-11-13 at 10:29 -0600, Jonathon Jongsma wrote:
> > From: Marc-André Lureau <marcandre.lureau at gmail.com>
> > 
> > ---
> >  server/display-channel.c | 526
> >  ++++++++++++++++++++++++++++++++++++++++++++
> >  server/display-channel.h |  15 +-
> >  server/red_worker.c      | 559
> >  ++--------------------------------------------
> > -
> >  3 files changed, 552 insertions(+), 548 deletions(-)
> > 
> > diff --git a/server/display-channel.c b/server/display-channel.c
> > index 1d5d8d3..4f6cfc4 100644
> > --- a/server/display-channel.c
> > +++ b/server/display-channel.c
> > @@ -20,6 +20,21 @@
> >  
> >  #include "display-channel.h"
> >  
> > +static stat_time_t display_channel_stat_now(DisplayChannel *display)
> > +{
> > +#ifdef RED_WORKER_STAT
> > +    RedWorker *worker = COMMON_CHANNEL(display)->worker;
> > +
> > +    return stat_now(red_worker_get_clockid(worker));
> > +
> > +#else
> > +    return 0;
> > +#endif
> > +}
> > +
> > +#define stat_start(display, var)                                        \
> > +    G_GNUC_UNUSED stat_time_t var = display_channel_stat_now((display));
> > +
> >  void display_channel_compress_stats_reset(DisplayChannel *display)
> >  {
> >      spice_return_if_fail(display);
> > @@ -406,3 +421,514 @@ bool
> > display_channel_surface_has_canvas(DisplayChannel
> > *display,
> >  {
> >      return display->surfaces[surface_id].context.canvas != NULL;
> >  }
> > +
> > +static void streams_update_visible_region(DisplayChannel *display,
> > Drawable
> > *drawable)
> > +{
> > +    Ring *ring;
> > +    RingItem *item;
> > +    RingItem *dcc_ring_item, *next;
> > +    DisplayChannelClient *dcc;
> > +
> > +    if (!red_channel_is_connected(RED_CHANNEL(display))) {
> > +        return;
> > +    }
> > +
> > +    if (!is_primary_surface(display, drawable->surface_id)) {
> > +        return;
> > +    }
> > +
> > +    ring = &display->streams;
> > +    item = ring_get_head(ring);
> > +
> > +    while (item) {
> > +        Stream *stream = SPICE_CONTAINEROF(item, Stream, link);
> > +        StreamAgent *agent;
> > +
> > +        item = ring_next(ring, item);
> > +
> > +        if (stream->current == drawable) {
> > +            continue;
> > +        }
> > +
> > +        FOREACH_DCC(display, dcc_ring_item, next, dcc) {
> > +            agent = &dcc->stream_agents[get_stream_id(display, stream)];
> > +
> > +            if (region_intersects(&agent->vis_region, &drawable
> > ->tree_item.base.rgn)) {
> > +                region_exclude(&agent->vis_region, &drawable
> > ->tree_item.base.rgn);
> > +                region_exclude(&agent->clip,
> > &drawable->tree_item.base.rgn);
> > +                dcc_add_stream_agent_clip(dcc, agent);
> > +            }
> > +        }
> > +    }
> > +}
> > +
> > +
> > +static void current_add_drawable(DisplayChannel *display,
> > +                                 Drawable *drawable, RingItem *pos)
> > +{
> > +    RedSurface *surface;
> > +    uint32_t surface_id = drawable->surface_id;
> > +
> > +    surface = &display->surfaces[surface_id];
> > +    ring_add_after(&drawable->tree_item.base.siblings_link, pos);
> > +    ring_add(&display->current_list, &drawable->list_link);
> > +    ring_add(&surface->current_list, &drawable->surface_list_link);
> > +    display->current_size++;
> > +    drawable->refs++;
> > +}
> > +
> > +static int current_add_equal(DisplayChannel *display, DrawItem *item,
> > TreeItem *other)
> > +{
> > +    DrawItem *other_draw_item;
> > +    Drawable *drawable;
> > +    Drawable *other_drawable;
> > +
> > +    if (other->type != TREE_ITEM_TYPE_DRAWABLE) {
> > +        return FALSE;
> > +    }
> > +    other_draw_item = (DrawItem *)other;
> > +
> > +    if (item->shadow || other_draw_item->shadow || item->effect !=
> > other_draw_item->effect) {
> > +        return FALSE;
> > +    }
> > +
> > +    drawable = SPICE_CONTAINEROF(item, Drawable, tree_item);
> > +    other_drawable = SPICE_CONTAINEROF(other_draw_item, Drawable,
> > tree_item);
> > +
> > +    if (item->effect == QXL_EFFECT_OPAQUE) {
> > +        int add_after = !!other_drawable->stream &&
> > +                        is_drawable_independent_from_surfaces(drawable);
> > +        stream_maintenance(display, drawable, other_drawable);
> > +        current_add_drawable(display, drawable, &other->siblings_link);
> > +        other_drawable->refs++;
> > +        current_remove_drawable(display, other_drawable);
> > +        if (add_after) {
> > +            red_pipes_add_drawable_after(display, drawable,
> > other_drawable);
> > +        } else {
> > +            red_pipes_add_drawable(display, drawable);
> > +        }
> > +        red_pipes_remove_drawable(other_drawable);
> > +        display_channel_drawable_unref(display, other_drawable);
> > +        return TRUE;
> > +    }
> > +
> > +    switch (item->effect) {
> > +    case QXL_EFFECT_REVERT_ON_DUP:
> > +        if (is_same_drawable(drawable, other_drawable)) {
> > +
> > +            DisplayChannelClient *dcc;
> > +            DrawablePipeItem *dpi;
> > +            RingItem *worker_ring_item, *dpi_ring_item;
> > +
> > +            other_drawable->refs++;
> > +            current_remove_drawable(display, other_drawable);
> > +
> > +            /* sending the drawable to clients that already received
> > +             * (or will receive) other_drawable */
> > +            worker_ring_item =
> > ring_get_head(&RED_CHANNEL(display)->clients);
> > +            dpi_ring_item = ring_get_head(&other_drawable->pipes);
> > +            /* dpi contains a sublist of dcc's, ordered the same */
> > +            while (worker_ring_item) {
> > +                dcc = SPICE_CONTAINEROF(worker_ring_item,
> > DisplayChannelClient,
> > +                                        common.base.channel_link);
> > +                dpi = SPICE_CONTAINEROF(dpi_ring_item, DrawablePipeItem,
> > base);
> > +                while (worker_ring_item && (!dpi || dcc != dpi->dcc)) {
> > +                    dcc_add_drawable(dcc, drawable);
> > +                    worker_ring_item = ring_next(&RED_CHANNEL(display)
> > ->clients,
> > +                                                 worker_ring_item);
> > +                    dcc = SPICE_CONTAINEROF(worker_ring_item,
> > DisplayChannelClient,
> > +                                            common.base.channel_link);
> > +                }
> > +
> > +                if (dpi_ring_item) {
> > +                    dpi_ring_item = ring_next(&other_drawable->pipes,
> > dpi_ring_item);
> > +                }
> > +                if (worker_ring_item) {
> > +                    worker_ring_item = ring_next(&RED_CHANNEL(display)
> > ->clients,
> > +                                                 worker_ring_item);
> > +                }
> > +            }
> > +            /* not sending other_drawable where possible */
> > +            red_pipes_remove_drawable(other_drawable);
> > +
> > +            display_channel_drawable_unref(display, other_drawable);
> > +            return TRUE;
> > +        }
> > +        break;
> > +    case QXL_EFFECT_OPAQUE_BRUSH:
> > +        if (is_same_geometry(drawable, other_drawable)) {
> > +            current_add_drawable(display, drawable,
> > &other->siblings_link);
> > +            red_pipes_remove_drawable(other_drawable);
> > +            current_remove_drawable(display, other_drawable);
> > +            red_pipes_add_drawable(display, drawable);
> > +            return TRUE;
> > +        }
> > +        break;
> > +    case QXL_EFFECT_NOP_ON_DUP:
> > +        if (is_same_drawable(drawable, other_drawable)) {
> > +            return TRUE;
> > +        }
> > +        break;
> > +    }
> > +    return FALSE;
> > +}
> > +
> > +static void __exclude_region(DisplayChannel *display, Ring *ring, TreeItem
> > *item, QRegion *rgn,
> > +                             Ring **top_ring, Drawable *frame_candidate)
> > +{
> > +    QRegion and_rgn;
> > +    stat_start(display, start_time);
> > +
> > +    region_clone(&and_rgn, rgn);
> > +    region_and(&and_rgn, &item->rgn);
> > +    if (!region_is_empty(&and_rgn)) {
> > +        if (IS_DRAW_ITEM(item)) {
> > +            DrawItem *draw = (DrawItem *)item;
> > +
> > +            if (draw->effect == QXL_EFFECT_OPAQUE) {
> > +                region_exclude(rgn, &and_rgn);
> > +            }
> > +
> > +            if (draw->shadow) {
> > +                Shadow *shadow;
> > +                int32_t x = item->rgn.extents.x1;
> > +                int32_t y = item->rgn.extents.y1;
> > +
> > +                region_exclude(&draw->base.rgn, &and_rgn);
> > +                shadow = draw->shadow;
> > +                region_offset(&and_rgn, shadow->base.rgn.extents.x1 - x,
> > +                              shadow->base.rgn.extents.y1 - y);
> > +                region_exclude(&shadow->base.rgn, &and_rgn);
> > +                region_and(&and_rgn, &shadow->on_hold);
> > +                if (!region_is_empty(&and_rgn)) {
> > +                    region_exclude(&shadow->on_hold, &and_rgn);
> > +                    region_or(rgn, &and_rgn);
> > +                    // in flat representation of current, shadow is always
> > his owner next
> > +                    if (!tree_item_contained_by((TreeItem*)shadow,
> > *top_ring)) {
> > +                        *top_ring =
> > tree_item_container_items((TreeItem*)shadow, ring);
> > +                    }
> > +                }
> > +            } else {
> > +                if (frame_candidate) {
> > +                    Drawable *drawable = SPICE_CONTAINEROF(draw, Drawable,
> > tree_item);
> > +                    stream_maintenance(display, frame_candidate,
> > drawable);
> > +                }
> > +                region_exclude(&draw->base.rgn, &and_rgn);
> > +            }
> > +        } else if (item->type == TREE_ITEM_TYPE_CONTAINER) {
> > +            region_exclude(&item->rgn, &and_rgn);
> > +
> > +            if (region_is_empty(&item->rgn)) {  //assume container removal
> > will follow
> > +                Shadow *shadow;
> > +
> > +                region_exclude(rgn, &and_rgn);
> > +                if ((shadow = tree_item_find_shadow(item))) {
> > +                    region_or(rgn, &shadow->on_hold);
> > +                    if (!tree_item_contained_by((TreeItem*)shadow,
> > *top_ring)) {
> > +                        *top_ring =
> > tree_item_container_items((TreeItem*)shadow, ring);
> > +                    }
> > +                }
> > +            }
> > +        } else {
> > +            Shadow *shadow;
> > +
> > +            spice_assert(item->type == TREE_ITEM_TYPE_SHADOW);
> > +            shadow = (Shadow *)item;
> > +            region_exclude(rgn, &and_rgn);
> > +            region_or(&shadow->on_hold, &and_rgn);
> > +        }
> > +    }
> > +    region_destroy(&and_rgn);
> > +    stat_add(&display->__exclude_stat, start_time);
> > +}
> > +
> > +static void exclude_region(DisplayChannel *display, Ring *ring, RingItem
> > *ring_item,
> > +                           QRegion *rgn, TreeItem **last, Drawable
> > *frame_candidate)
> > +{
> > +    Ring *top_ring;
> > +    stat_start(display, start_time);
> > +
> > +    if (!ring_item) {
> > +        return;
> > +    }
> > +
> > +    top_ring = ring;
> > +
> > +    for (;;) {
> > +        TreeItem *now = SPICE_CONTAINEROF(ring_item, TreeItem,
> > siblings_link);
> > +        Container *container = now->container;
> > +
> > +        spice_assert(!region_is_empty(&now->rgn));
> > +
> > +        if (region_intersects(rgn, &now->rgn)) {
> > +            __exclude_region(display, ring, now, rgn, &top_ring,
> > frame_candidate);
> > +
> > +            if (region_is_empty(&now->rgn)) {
> > +                spice_assert(now->type != TREE_ITEM_TYPE_SHADOW);
> > +                ring_item = now->siblings_link.prev;
> > +                current_remove(display, now);
> > +                if (last && *last == now) {
> > +                    *last = (TreeItem *)ring_next(ring, ring_item);
> > +                }
> > +            } else if (now->type == TREE_ITEM_TYPE_CONTAINER) {
> > +                Container *container = (Container *)now;
> > +                if ((ring_item = ring_get_head(&container->items))) {
> > +                    ring = &container->items;
> > +                    spice_assert(((TreeItem *)ring_item)->container);
> > +                    continue;
> > +                }
> > +                ring_item = &now->siblings_link;
> > +            }
> > +
> > +            if (region_is_empty(rgn)) {
> > +                stat_add(&display->exclude_stat, start_time);
> > +                return;
> > +            }
> > +        }
> > +
> > +        while ((last && *last == (TreeItem *)ring_item) ||
> > +               !(ring_item = ring_next(ring, ring_item))) {
> > +            if (ring == top_ring) {
> > +                stat_add(&display->exclude_stat, start_time);
> > +                return;
> > +            }
> > +            ring_item = &container->base.siblings_link;
> > +            container = container->base.container;
> > +            ring = (container) ? &container->items : top_ring;
> > +        }
> > +    }
> > +}
> > +
> > +static int current_add_with_shadow(DisplayChannel *display, Ring *ring,
> > Drawable *item)
> > +{
> > +    stat_start(display, start_time);
> > +    ++display->add_with_shadow_count;
> > +
> > +    RedDrawable *red_drawable = item->red_drawable;
> > +    SpicePoint delta = {
> > +        .x = red_drawable->u.copy_bits.src_pos.x -
> > red_drawable->bbox.left,
> > +        .y = red_drawable->u.copy_bits.src_pos.y - red_drawable->bbox.top
> > +    };
> > +
> > +    Shadow *shadow = shadow_new(&item->tree_item, &delta);
> > +    if (!shadow) {
> > +        stat_add(&display->add_stat, start_time);
> > +        return FALSE;
> > +    }
> > +    // item and his shadow must initially be placed in the same container.
> > +    // for now putting them on root.
> > +
> > +    // only primary surface streams are supported
> > +    if (is_primary_surface(display, item->surface_id)) {
> > +        detach_streams_behind(display, &shadow->base.rgn, NULL);
> > +    }
> > +
> > +    ring_add(ring, &shadow->base.siblings_link);
> > +    current_add_drawable(display, item, ring);
> > +    if (item->tree_item.effect == QXL_EFFECT_OPAQUE) {
> > +        QRegion exclude_rgn;
> > +        region_clone(&exclude_rgn, &item->tree_item.base.rgn);
> > +        exclude_region(display, ring, &shadow->base.siblings_link,
> > &exclude_rgn, NULL, NULL);
> > +        region_destroy(&exclude_rgn);
> > +        streams_update_visible_region(display, item);
> > +    } else {
> > +        if (is_primary_surface(display, item->surface_id)) {
> > +            detach_streams_behind(display, &item->tree_item.base.rgn,
> > item);
> > +        }
> > +    }
> > +    stat_add(&display->add_stat, start_time);
> > +    return TRUE;
> > +}
> > +
> > +static int current_add(DisplayChannel *display, Ring *ring, Drawable
> > *drawable)
> > +{
> > +    DrawItem *item = &drawable->tree_item;
> > +    RingItem *now;
> > +    QRegion exclude_rgn;
> > +    RingItem *exclude_base = NULL;
> > +    stat_start(display, start_time);
> > +
> > +    spice_return_val_if_fail(!region_is_empty(&item->base.rgn), FALSE);
> > +    region_init(&exclude_rgn);
> > +    now = ring_next(ring, ring);
> > +
> > +    while (now) {
> > +        TreeItem *sibling = SPICE_CONTAINEROF(now, TreeItem,
> > siblings_link);
> > +        int test_res;
> > +
> > +        if (!region_bounds_intersects(&item->base.rgn, &sibling->rgn)) {
> > +            now = ring_next(ring, now);
> > +            continue;
> > +        }
> > +        test_res = region_test(&item->base.rgn, &sibling->rgn,
> > REGION_TEST_ALL);
> > +        if (!(test_res & REGION_TEST_SHARED)) {
> > +            now = ring_next(ring, now);
> > +            continue;
> > +        } else if (sibling->type != TREE_ITEM_TYPE_SHADOW) {
> > +            if (!(test_res & REGION_TEST_RIGHT_EXCLUSIVE) &&
> > +                                                   !(test_res &
> > REGION_TEST_LEFT_EXCLUSIVE) &&
> > +
> > current_add_equal(display,
> > item, sibling)) {
> > +                stat_add(&display->add_stat, start_time);
> > +                return FALSE;
> > +            }
> > +
> > +            if (!(test_res & REGION_TEST_RIGHT_EXCLUSIVE) && item->effect
> > ==
> > QXL_EFFECT_OPAQUE) {
> > +                Shadow *shadow;
> > +                int skip = now == exclude_base;
> > +
> > +                if ((shadow = tree_item_find_shadow(sibling))) {
> > +                    if (exclude_base) {
> > +                        TreeItem *next = sibling;
> > +                        exclude_region(display, ring, exclude_base,
> > &exclude_rgn, &next, NULL);
> > +                        if (next != sibling) {
> > +                            now = next ? &next->siblings_link : NULL;
> > +                            exclude_base = NULL;
> > +                            continue;
> > +                        }
> > +                    }
> > +                    region_or(&exclude_rgn, &shadow->on_hold);
> > +                }
> > +                now = now->prev;
> > +                current_remove(display, sibling);
> > +                now = ring_next(ring, now);
> > +                if (shadow || skip) {
> > +                    exclude_base = now;
> > +                }
> > +                continue;
> > +            }
> > +
> > +            if (!(test_res & REGION_TEST_LEFT_EXCLUSIVE) &&
> > is_opaque_item(sibling)) {
> > +                Container *container;
> > +
> > +                if (exclude_base) {
> > +                    exclude_region(display, ring, exclude_base,
> > &exclude_rgn,
> > NULL, NULL);
> > +                    region_clear(&exclude_rgn);
> > +                    exclude_base = NULL;
> > +                }
> > +                if (sibling->type == TREE_ITEM_TYPE_CONTAINER) {
> > +                    container = (Container *)sibling;
> > +                    ring = &container->items;
> > +                    item->base.container = container;
> > +                    now = ring_next(ring, ring);
> > +                    continue;
> > +                }
> > +                spice_assert(IS_DRAW_ITEM(sibling));
> > +                if (!DRAW_ITEM(sibling)->container_root) {
> > +                    container = container_new(DRAW_ITEM(sibling));
> > +                    if (!container) {
> > +                        spice_warning("create new container failed");
> > +                        region_destroy(&exclude_rgn);
> > +                        return FALSE;
> > +                    }
> > +                    item->base.container = container;
> > +                    ring = &container->items;
> > +                }
> > +            }
> > +        }
> > +        if (!exclude_base) {
> > +            exclude_base = now;
> > +        }
> > +        break;
> > +    }
> > +    if (item->effect == QXL_EFFECT_OPAQUE) {
> > +        region_or(&exclude_rgn, &item->base.rgn);
> > +        exclude_region(display, ring, exclude_base, &exclude_rgn, NULL,
> > drawable);
> > +        stream_trace_update(display, drawable);
> > +        streams_update_visible_region(display, drawable);
> > +        /*
> > +         * Performing the insertion after exclude_region for
> > +         * safety (todo: Not sure if exclude_region can affect the
> > drawable
> > +         * if it is added to the tree before calling exclude_region).
> > +         */
> > +        current_add_drawable(display, drawable, ring);
> > +    } else {
> > +        /*
> > +         * red_detach_streams_behind can affect the current tree since it
> > may
> > +         * trigger calls to update_area. Thus, the drawable should be
> > added
> > to the tree
> > +         * before calling red_detach_streams_behind
> > +         */
> > +        current_add_drawable(display, drawable, ring);
> > +        if (is_primary_surface(display, drawable->surface_id)) {
> > +            detach_streams_behind(display, &drawable->tree_item.base.rgn,
> > drawable);
> > +        }
> > +    }
> > +    region_destroy(&exclude_rgn);
> > +    stat_add(&display->add_stat, start_time);
> > +    return TRUE;
> > +}
> > +
> > +static bool drawable_can_stream(DisplayChannel *display, Drawable
> > *drawable)
> > +{
> > +    RedDrawable *red_drawable = drawable->red_drawable;
> > +    SpiceImage *image;
> > +
> > +    if (display->stream_video == SPICE_STREAM_VIDEO_OFF)
> > +        return FALSE;
> 
> on this test and the following ones, the braces were removed compared to the
> previous version. Not a big deal, but in general our coding style is to
> always
> use braces.
> 

I'll add again braces.

> 
> > +
> > +    if (!is_primary_surface(display, drawable->surface_id))
> > +        return FALSE;
> > +
> > +    if (drawable->tree_item.effect != QXL_EFFECT_OPAQUE ||
> > +        red_drawable->type != QXL_DRAW_COPY ||
> > +        red_drawable->u.copy.rop_descriptor != SPICE_ROPD_OP_PUT)
> > +        return FALSE;
> > +
> > +    image = red_drawable->u.copy.src_bitmap;
> > +    if (image == NULL ||
> > +        image->descriptor.type != SPICE_IMAGE_TYPE_BITMAP)
> > +        return FALSE;
> > +
> > +    if (display->stream_video == SPICE_STREAM_VIDEO_FILTER) {
> > +        SpiceRect* rect;
> > +        int size;
> > +
> > +        rect = &drawable->red_drawable->u.copy.src_area;
> > +        size = (rect->right - rect->left) * (rect->bottom - rect->top);
> > +        if (size < RED_STREAM_MIN_SIZE)
> > +            return FALSE;
> > +    }
> > +
> > +    return TRUE;
> > +}
> > +
> > +void display_channel_print_stats(DisplayChannel *display)
> > +{
> > +#ifdef RED_WORKER_STAT
> > +    stat_time_t total = display->add_stat.total;
> > +    spice_info("add with shadow count %u",
> > +               display->add_with_shadow_count);
> > +    display->add_with_shadow_count = 0;
> > +    spice_info("add[%u] %f exclude[%u] %f __exclude[%u] %f",
> > +               display->add_stat.count,
> > +               stat_cpu_time_to_sec(total),
> > +               display->exclude_stat.count,
> > +               stat_cpu_time_to_sec(display->exclude_stat.total),
> > +               display->__exclude_stat.count,
> > +               stat_cpu_time_to_sec(display->__exclude_stat.total));
> > +    spice_info("add %f%% exclude %f%% exclude2 %f%% __exclude %f%%",
> > +               (double)(total - display->exclude_stat.total) / total *
> > 100,
> > +               (double)(display->exclude_stat.total) / total * 100,
> > +               (double)(display->exclude_stat.total -
> > +                        display->__exclude_stat.total) / display
> > ->exclude_stat.total * 100,
> > +               (double)(display->__exclude_stat.total) / display
> > ->exclude_stat.total * 100);
> > +    stat_reset(&display->add_stat);
> > +    stat_reset(&display->exclude_stat);
> > +    stat_reset(&display->__exclude_stat);
> > +#endif
> > +}
> 
> As far as I can tell, this function is no longer used. See below.
> 

I think for the moment I'll add again the code that calls this functions.

> 
> > +
> > +int display_channel_add_drawable(DisplayChannel *display, Drawable
> > *drawable)
> > +{
> > +    int ret = FALSE, surface_id = drawable->surface_id;
> > +    RedDrawable *red_drawable = drawable->red_drawable;
> > +    Ring *ring = &display->surfaces[surface_id].current;
> > +
> > +    if (has_shadow(red_drawable)) {
> > +        ret = current_add_with_shadow(display, ring, drawable);
> > +    } else {
> > +        drawable->streamable = drawable_can_stream(display, drawable);
> > +        ret = current_add(display, ring, drawable);
> > +    }
> > +
> > +    return ret;
> > +}
> 
> compared with the previous version (red_add_drawable()), this function
> removes
> the call to print_stats() (which was renamed to
> display_channel_print_stats()).
> I think this was the only location where this function was called. I don't
> know
> whether this debug information was useful or not, since I've never used it.
> But
> if we're not going to ever print out the stats, we should probably just get
> rid
> of the funciton.
> 

If we decide to remove this feature we should have a better change than this

> 
> > diff --git a/server/display-channel.h b/server/display-channel.h
> > index ce9ce50..432cc61 100644
> > --- a/server/display-channel.h
> > +++ b/server/display-channel.h
> > @@ -356,12 +356,12 @@ struct DisplayChannel {
> >      RedCompressBuf *free_compress_bufs;
> >  
> >  /* TODO: some day unify this, make it more runtime.. */
> > +    uint32_t add_count;
> 
> As far as I can tell, this is no longer used anywhere due to the removal of
> the
> print_stats() functionality from add_drawable() above
> 
> 
> > +    uint32_t add_with_shadow_count;
> 
> ...and this one is incremented in current_add_with_shadow(), but it is only
> read
> in display_channel_print_stats() (which is no longer called from anywhere).
> So it seems these two values could perhaps be removed. So I don't understand
> why
> they were moved out from inside the #ifdef...
> 

I moved them back adding a #ifdef where needed for the shadow.

> 
> >  #ifdef RED_WORKER_STAT
> >      stat_info_t add_stat;
> >      stat_info_t exclude_stat;
> >      stat_info_t __exclude_stat;
> > -    uint32_t add_count;
> > -    uint32_t add_with_shadow_count;
> >  #endif
> >  #ifdef RED_STATISTICS
> >      uint64_t *cache_hits_counter;
> > @@ -415,6 +415,8 @@ void
> > display_channel_surface_unref
> >            (DisplayCha
> >  bool                       display_channel_surface_has_canvas
> >  (DisplayChannel *display,
> >                                                                       
> >  uint32_t surface_id);
> >  void                       display_channel_show_tree
> >  (DisplayChannel *display);
> > +int                        display_channel_add_drawable
> >  (DisplayChannel *display,
> > +
> >  Drawable *drawable);
> >  
> >  static inline int is_equal_path(SpicePath *path1, SpicePath *path2)
> >  {
> > @@ -531,4 +533,13 @@ static inline void region_add_clip_rects(QRegion *rgn,
> > SpiceClipRects *data)
> >      }
> >  }
> >  
> > +void red_pipes_add_drawable(DisplayChannel *display, Drawable *drawable);
> > +void current_remove_drawable(DisplayChannel *display, Drawable *item);
> > +void red_pipes_add_drawable_after(DisplayChannel *display,
> > +                                  Drawable *drawable, Drawable
> > *pos_after);
> > +void red_pipes_remove_drawable(Drawable *drawable);
> > +void dcc_add_drawable(DisplayChannelClient *dcc, Drawable *drawable);
> > +void current_remove(DisplayChannel *display, TreeItem *item);
> > +void detach_streams_behind(DisplayChannel *display, QRegion *region,
> > Drawable
> > *drawable);
> > +
> >  #endif /* DISPLAY_CHANNEL_H_ */
> > diff --git a/server/red_worker.c b/server/red_worker.c
> > index fcb5128..8aa7e70 100644
> > --- a/server/red_worker.c
> > +++ b/server/red_worker.c
> > @@ -492,7 +492,7 @@ static int cursor_is_connected(RedWorker *worker)
> >          red_channel_is_connected(RED_CHANNEL(worker->cursor_channel));
> >  }
> >  
> > -static void dcc_add_drawable(DisplayChannelClient *dcc, Drawable
> > *drawable)
> > +void dcc_add_drawable(DisplayChannelClient *dcc, Drawable *drawable)
> >  {
> >      DrawablePipeItem *dpi;
> >  
> > @@ -501,7 +501,7 @@ static void dcc_add_drawable(DisplayChannelClient *dcc,
> > Drawable *drawable)
> >      red_channel_client_pipe_add(RED_CHANNEL_CLIENT(dcc), &dpi
> > ->dpi_pipe_item);
> >  }
> >  
> > -static void red_pipes_add_drawable(DisplayChannel *display, Drawable
> > *drawable)
> > +void red_pipes_add_drawable(DisplayChannel *display, Drawable *drawable)
> >  {
> >      DisplayChannelClient *dcc;
> >      RingItem *dcc_ring_item, *next;
> > @@ -524,8 +524,8 @@ static void
> > dcc_add_drawable_to_tail(DisplayChannelClient
> > *dcc, Drawable *drawab
> >      red_channel_client_pipe_add_tail(RED_CHANNEL_CLIENT(dcc), &dpi
> > ->dpi_pipe_item);
> >  }
> >  
> > -static inline void red_pipes_add_drawable_after(DisplayChannel *display,
> > -                                                Drawable *drawable,
> > Drawable
> > *pos_after)
> > +void red_pipes_add_drawable_after(DisplayChannel *display,
> > +                                  Drawable *drawable, Drawable *pos_after)
> >  {
> >      DrawablePipeItem *dpi, *dpi_pos_after;
> >      RingItem *dpi_link, *dpi_next;
> > @@ -571,7 +571,7 @@ static inline PipeItem
> > *red_pipe_get_tail(DisplayChannelClient *dcc)
> >      return (PipeItem*)ring_get_tail(&RED_CHANNEL_CLIENT(dcc)->pipe);
> >  }
> >  
> > -static inline void red_pipes_remove_drawable(Drawable *drawable)
> > +void red_pipes_remove_drawable(Drawable *drawable)
> >  {
> >      DrawablePipeItem *dpi;
> >      RingItem *item, *next;
> > @@ -816,8 +816,9 @@ static void red_flush_source_surfaces(DisplayChannel
> > *display, Drawable *drawabl
> >      }
> >  }
> >  
> > -static inline void current_remove_drawable(DisplayChannel *display,
> > Drawable
> > *item)
> > +void current_remove_drawable(DisplayChannel *display, Drawable *item)
> >  {
> > +    /* todo: move all to unref? */
> >      display_stream_trace_add_drawable(display, item);
> >      draw_item_remove_shadow(&item->tree_item);
> >      ring_remove(&item->tree_item.base.siblings_link);
> > @@ -827,13 +828,7 @@ static inline void
> > current_remove_drawable(DisplayChannel
> > *display, Drawable *it
> >      display->current_size--;
> >  }
> >  
> > -static void remove_drawable(DisplayChannel *display, Drawable *drawable)
> > -{
> > -    red_pipes_remove_drawable(drawable);
> > -    current_remove_drawable(display, drawable);
> > -}
> > -
> > -static inline void current_remove(DisplayChannel *display, TreeItem *item)
> > +void current_remove(DisplayChannel *display, TreeItem *item)
> >  {
> >      TreeItem *now = item;
> >  
> > @@ -843,8 +838,10 @@ static inline void current_remove(DisplayChannel
> > *display, TreeItem *item)
> >          RingItem *ring_item;
> >  
> >          if (now->type == TREE_ITEM_TYPE_DRAWABLE) {
> > +            Drawable *drawable = SPICE_CONTAINEROF(now, Drawable,
> > tree_item);
> >              ring_item = now->siblings_link.prev;
> > -            remove_drawable(display, SPICE_CONTAINEROF(now, Drawable,
> > tree_item));
> > +            red_pipes_remove_drawable(drawable);
> > +            current_remove_drawable(display, drawable);
> >          } else {
> >              Container *container = (Container *)now;
> >  
> > @@ -975,152 +972,6 @@ static void
> > red_clear_surface_drawables_from_pipes(DisplayChannel *display,
> >      }
> >  }
> >  
> > -static void __exclude_region(DisplayChannel *display, Ring *ring, TreeItem
> > *item, QRegion *rgn,
> > -                             Ring **top_ring, Drawable *frame_candidate)
> > -{
> > -    QRegion and_rgn;
> > -#ifdef RED_WORKER_STAT
> > -    RedWorker *worker = COMMON_CHANNEL(display)->worker;
> > -    stat_time_t start_time = stat_now(worker->clockid);
> > -#endif
> > -
> > -    region_clone(&and_rgn, rgn);
> > -    region_and(&and_rgn, &item->rgn);
> > -    if (!region_is_empty(&and_rgn)) {
> > -        if (IS_DRAW_ITEM(item)) {
> > -            DrawItem *draw = (DrawItem *)item;
> > -
> > -            if (draw->effect == QXL_EFFECT_OPAQUE) {
> > -                region_exclude(rgn, &and_rgn);
> > -            }
> > -
> > -            if (draw->shadow) {
> > -                Shadow *shadow;
> > -                int32_t x = item->rgn.extents.x1;
> > -                int32_t y = item->rgn.extents.y1;
> > -
> > -                region_exclude(&draw->base.rgn, &and_rgn);
> > -                shadow = draw->shadow;
> > -                region_offset(&and_rgn, shadow->base.rgn.extents.x1 - x,
> > -                              shadow->base.rgn.extents.y1 - y);
> > -                region_exclude(&shadow->base.rgn, &and_rgn);
> > -                region_and(&and_rgn, &shadow->on_hold);
> > -                if (!region_is_empty(&and_rgn)) {
> > -                    region_exclude(&shadow->on_hold, &and_rgn);
> > -                    region_or(rgn, &and_rgn);
> > -                    // in flat representation of current, shadow is always
> > his owner next
> > -                    if (!tree_item_contained_by((TreeItem*)shadow,
> > *top_ring)) {
> > -                        *top_ring =
> > tree_item_container_items((TreeItem*)shadow, ring);
> > -                    }
> > -                }
> > -            } else {
> > -                if (frame_candidate) {
> > -                    Drawable *drawable = SPICE_CONTAINEROF(draw, Drawable,
> > tree_item);
> > -                    stream_maintenance(display, frame_candidate,
> > drawable);
> > -                }
> > -                region_exclude(&draw->base.rgn, &and_rgn);
> > -            }
> > -        } else if (item->type == TREE_ITEM_TYPE_CONTAINER) {
> > -            region_exclude(&item->rgn, &and_rgn);
> > -
> > -            if (region_is_empty(&item->rgn)) {  //assume container removal
> > will follow
> > -                Shadow *shadow;
> > -
> > -                region_exclude(rgn, &and_rgn);
> > -                if ((shadow = tree_item_find_shadow(item))) {
> > -                    region_or(rgn, &shadow->on_hold);
> > -                    if (!tree_item_contained_by((TreeItem*)shadow,
> > *top_ring)) {
> > -                        *top_ring =
> > tree_item_container_items((TreeItem*)shadow, ring);
> > -                    }
> > -                }
> > -            }
> > -        } else {
> > -            Shadow *shadow;
> > -
> > -            spice_assert(item->type == TREE_ITEM_TYPE_SHADOW);
> > -            shadow = (Shadow *)item;
> > -            region_exclude(rgn, &and_rgn);
> > -            region_or(&shadow->on_hold, &and_rgn);
> > -        }
> > -    }
> > -    region_destroy(&and_rgn);
> > -    stat_add(&display->__exclude_stat, start_time);
> > -}
> > -
> > -static void exclude_region(DisplayChannel *display, Ring *ring, RingItem
> > *ring_item,
> > -                           QRegion *rgn, TreeItem **last, Drawable
> > *frame_candidate)
> > -{
> > -#ifdef RED_WORKER_STAT
> > -    RedWorker *worker = COMMON_CHANNEL(display)->worker;
> > -    stat_time_t start_time = stat_now(worker->clockid);
> > -#endif
> > -    Ring *top_ring;
> > -
> > -    if (!ring_item) {
> > -        return;
> > -    }
> > -
> > -    top_ring = ring;
> > -
> > -    for (;;) {
> > -        TreeItem *now = SPICE_CONTAINEROF(ring_item, TreeItem,
> > siblings_link);
> > -        Container *container = now->container;
> > -
> > -        spice_assert(!region_is_empty(&now->rgn));
> > -
> > -        if (region_intersects(rgn, &now->rgn)) {
> > -            __exclude_region(display, ring, now, rgn, &top_ring,
> > frame_candidate);
> > -
> > -            if (region_is_empty(&now->rgn)) {
> > -                spice_assert(now->type != TREE_ITEM_TYPE_SHADOW);
> > -                ring_item = now->siblings_link.prev;
> > -                current_remove(display, now);
> > -                if (last && *last == now) {
> > -                    *last = (TreeItem *)ring_next(ring, ring_item);
> > -                }
> > -            } else if (now->type == TREE_ITEM_TYPE_CONTAINER) {
> > -                Container *container = (Container *)now;
> > -                if ((ring_item = ring_get_head(&container->items))) {
> > -                    ring = &container->items;
> > -                    spice_assert(((TreeItem *)ring_item)->container);
> > -                    continue;
> > -                }
> > -                ring_item = &now->siblings_link;
> > -            }
> > -
> > -            if (region_is_empty(rgn)) {
> > -                stat_add(&display->exclude_stat, start_time);
> > -                return;
> > -            }
> > -        }
> > -
> > -        while ((last && *last == (TreeItem *)ring_item) ||
> > -               !(ring_item = ring_next(ring, ring_item))) {
> > -            if (ring == top_ring) {
> > -                stat_add(&display->exclude_stat, start_time);
> > -                return;
> > -            }
> > -            ring_item = &container->base.siblings_link;
> > -            container = container->base.container;
> > -            ring = (container) ? &container->items : top_ring;
> > -        }
> > -    }
> > -}
> > -
> > -static void current_add_drawable(DisplayChannel *display,
> > -                                 Drawable *drawable, RingItem *pos)
> > -{
> > -    RedSurface *surface;
> > -    uint32_t surface_id = drawable->surface_id;
> > -
> > -    surface = &display->surfaces[surface_id];
> > -    ring_add_after(&drawable->tree_item.base.siblings_link, pos);
> > -    ring_add(&display->current_list, &drawable->list_link);
> > -    ring_add(&surface->current_list, &drawable->surface_list_link);
> > -    display->current_size++;
> > -    drawable->refs++;
> > -}
> > -
> >  void detach_stream(DisplayChannel *display, Stream *stream,
> >                     int detach_sized)
> >  {
> > @@ -1243,7 +1094,7 @@ static void detach_stream_gracefully(DisplayChannel
> > *display, Stream *stream,
> >   *           involves sending an upgrade image to the client, this
> >   drawable
> > won't be rendered
> >   *           (see dcc_detach_stream_gracefully).
> >   */
> > -static void detach_streams_behind(DisplayChannel *display, QRegion
> > *region,
> > Drawable *drawable)
> > +void detach_streams_behind(DisplayChannel *display, QRegion *region,
> > Drawable
> > *drawable)
> >  {
> >      Ring *ring = &display->streams;
> >      RingItem *item = ring_get_head(ring);
> > @@ -1276,46 +1127,6 @@ static void detach_streams_behind(DisplayChannel
> > *display, QRegion *region, Draw
> >      }
> >  }
> >  
> > -static void streams_update_visible_region(DisplayChannel *display,
> > Drawable
> > *drawable)
> > -{
> > -    Ring *ring;
> > -    RingItem *item;
> > -    RingItem *dcc_ring_item, *next;
> > -    DisplayChannelClient *dcc;
> > -
> > -    if (!red_channel_is_connected(RED_CHANNEL(display))) {
> > -        return;
> > -    }
> > -
> > -    if (!is_primary_surface(display, drawable->surface_id)) {
> > -        return;
> > -    }
> > -
> > -    ring = &display->streams;
> > -    item = ring_get_head(ring);
> > -
> > -    while (item) {
> > -        Stream *stream = SPICE_CONTAINEROF(item, Stream, link);
> > -        StreamAgent *agent;
> > -
> > -        item = ring_next(ring, item);
> > -
> > -        if (stream->current == drawable) {
> > -            continue;
> > -        }
> > -
> > -        FOREACH_DCC(display, dcc_ring_item, next, dcc) {
> > -            agent = &dcc->stream_agents[get_stream_id(display, stream)];
> > -
> > -            if (region_intersects(&agent->vis_region, &drawable
> > ->tree_item.base.rgn)) {
> > -                region_exclude(&agent->vis_region, &drawable
> > ->tree_item.base.rgn);
> > -                region_exclude(&agent->clip,
> > &drawable->tree_item.base.rgn);
> > -                dcc_add_stream_agent_clip(dcc, agent);
> > -            }
> > -        }
> > -    }
> > -}
> > -
> >  static void display_channel_streams_timeout(DisplayChannel *display)
> >  {
> >      Ring *ring = &display->streams;
> > @@ -1552,350 +1363,6 @@ static void
> > dcc_destroy_stream_agents(DisplayChannelClient *dcc)
> >      }
> >  }
> >  
> > -static inline int red_current_add_equal(DisplayChannel *display, DrawItem
> > *item, TreeItem *other)
> > -{
> > -    DrawItem *other_draw_item;
> > -    Drawable *drawable;
> > -    Drawable *other_drawable;
> > -
> > -    if (other->type != TREE_ITEM_TYPE_DRAWABLE) {
> > -        return FALSE;
> > -    }
> > -    other_draw_item = (DrawItem *)other;
> > -
> > -    if (item->shadow || other_draw_item->shadow || item->effect !=
> > other_draw_item->effect) {
> > -        return FALSE;
> > -    }
> > -
> > -    drawable = SPICE_CONTAINEROF(item, Drawable, tree_item);
> > -    other_drawable = SPICE_CONTAINEROF(other_draw_item, Drawable,
> > tree_item);
> > -
> > -    if (item->effect == QXL_EFFECT_OPAQUE) {
> > -        int add_after = !!other_drawable->stream &&
> > -                        is_drawable_independent_from_surfaces(drawable);
> > -        stream_maintenance(display, drawable, other_drawable);
> > -        current_add_drawable(display, drawable, &other->siblings_link);
> > -        other_drawable->refs++;
> > -        current_remove_drawable(display, other_drawable);
> > -        if (add_after) {
> > -            red_pipes_add_drawable_after(display, drawable,
> > other_drawable);
> > -        } else {
> > -            red_pipes_add_drawable(display, drawable);
> > -        }
> > -        red_pipes_remove_drawable(other_drawable);
> > -        display_channel_drawable_unref(display, other_drawable);
> > -        return TRUE;
> > -    }
> > -
> > -    switch (item->effect) {
> > -    case QXL_EFFECT_REVERT_ON_DUP:
> > -        if (is_same_drawable(drawable, other_drawable)) {
> > -
> > -            DisplayChannelClient *dcc;
> > -            DrawablePipeItem *dpi;
> > -            RingItem *worker_ring_item, *dpi_ring_item;
> > -
> > -            other_drawable->refs++;
> > -            current_remove_drawable(display, other_drawable);
> > -
> > -            /* sending the drawable to clients that already received
> > -             * (or will receive) other_drawable */
> > -            worker_ring_item =
> > ring_get_head(&RED_CHANNEL(display)->clients);
> > -            dpi_ring_item = ring_get_head(&other_drawable->pipes);
> > -            /* dpi contains a sublist of dcc's, ordered the same */
> > -            while (worker_ring_item) {
> > -                dcc = SPICE_CONTAINEROF(worker_ring_item,
> > DisplayChannelClient,
> > -                                        common.base.channel_link);
> > -                dpi = SPICE_CONTAINEROF(dpi_ring_item, DrawablePipeItem,
> > base);
> > -                while (worker_ring_item && (!dpi || dcc != dpi->dcc)) {
> > -                    dcc_add_drawable(dcc, drawable);
> > -                    worker_ring_item = ring_next(&RED_CHANNEL(display)
> > ->clients,
> > -                                                 worker_ring_item);
> > -                    dcc = SPICE_CONTAINEROF(worker_ring_item,
> > DisplayChannelClient,
> > -                                            common.base.channel_link);
> > -                }
> > -
> > -                if (dpi_ring_item) {
> > -                    dpi_ring_item = ring_next(&other_drawable->pipes,
> > dpi_ring_item);
> > -                }
> > -                if (worker_ring_item) {
> > -                    worker_ring_item = ring_next(&RED_CHANNEL(display)
> > ->clients,
> > -                                                 worker_ring_item);
> > -                }
> > -            }
> > -            /* not sending other_drawable where possible */
> > -            red_pipes_remove_drawable(other_drawable);
> > -
> > -            display_channel_drawable_unref(display, other_drawable);
> > -            return TRUE;
> > -        }
> > -        break;
> > -    case QXL_EFFECT_OPAQUE_BRUSH:
> > -        if (is_same_geometry(drawable, other_drawable)) {
> > -            current_add_drawable(display, drawable,
> > &other->siblings_link);
> > -            remove_drawable(display, other_drawable);
> > -            red_pipes_add_drawable(display, drawable);
> > -            return TRUE;
> > -        }
> > -        break;
> > -    case QXL_EFFECT_NOP_ON_DUP:
> > -        if (is_same_drawable(drawable, other_drawable)) {
> > -            return TRUE;
> > -        }
> > -        break;
> > -    }
> > -    return FALSE;
> > -}
> > -
> > -static int current_add(DisplayChannel *display, Ring *ring, Drawable
> > *drawable)
> > -{
> > -    DrawItem *item = &drawable->tree_item;
> > -#ifdef RED_WORKER_STAT
> > -    RedWorker *worker = COMMON_CHANNEL(display)->worker;
> > -    stat_time_t start_time = stat_now(worker->clockid);
> > -#endif
> > -    RingItem *now;
> > -    QRegion exclude_rgn;
> > -    RingItem *exclude_base = NULL;
> > -
> > -    spice_return_val_if_fail(!region_is_empty(&item->base.rgn), FALSE);
> > -    region_init(&exclude_rgn);
> > -    now = ring_next(ring, ring);
> > -
> > -    while (now) {
> > -        TreeItem *sibling = SPICE_CONTAINEROF(now, TreeItem,
> > siblings_link);
> > -        int test_res;
> > -
> > -        if (!region_bounds_intersects(&item->base.rgn, &sibling->rgn)) {
> > -            now = ring_next(ring, now);
> > -            continue;
> > -        }
> > -        test_res = region_test(&item->base.rgn, &sibling->rgn,
> > REGION_TEST_ALL);
> > -        if (!(test_res & REGION_TEST_SHARED)) {
> > -            now = ring_next(ring, now);
> > -            continue;
> > -        } else if (sibling->type != TREE_ITEM_TYPE_SHADOW) {
> > -            if (!(test_res & REGION_TEST_RIGHT_EXCLUSIVE) &&
> > -                                                   !(test_res &
> > REGION_TEST_LEFT_EXCLUSIVE) &&
> > -
> >  red_current_add_equal(display, item, sibling)) {
> > -                stat_add(&display->add_stat, start_time);
> > -                return FALSE;
> > -            }
> > -
> > -            if (!(test_res & REGION_TEST_RIGHT_EXCLUSIVE) && item->effect
> > ==
> > QXL_EFFECT_OPAQUE) {
> > -                Shadow *shadow;
> > -                int skip = now == exclude_base;
> > -
> > -                if ((shadow = tree_item_find_shadow(sibling))) {
> > -                    if (exclude_base) {
> > -                        TreeItem *next = sibling;
> > -                        exclude_region(display, ring, exclude_base,
> > &exclude_rgn, &next, NULL);
> > -                        if (next != sibling) {
> > -                            now = next ? &next->siblings_link : NULL;
> > -                            exclude_base = NULL;
> > -                            continue;
> > -                        }
> > -                    }
> > -                    region_or(&exclude_rgn, &shadow->on_hold);
> > -                }
> > -                now = now->prev;
> > -                current_remove(display, sibling);
> > -                now = ring_next(ring, now);
> > -                if (shadow || skip) {
> > -                    exclude_base = now;
> > -                }
> > -                continue;
> > -            }
> > -
> > -            if (!(test_res & REGION_TEST_LEFT_EXCLUSIVE) &&
> > is_opaque_item(sibling)) {
> > -                Container *container;
> > -
> > -                if (exclude_base) {
> > -                    exclude_region(display, ring, exclude_base,
> > &exclude_rgn,
> > NULL, NULL);
> > -                    region_clear(&exclude_rgn);
> > -                    exclude_base = NULL;
> > -                }
> > -                if (sibling->type == TREE_ITEM_TYPE_CONTAINER) {
> > -                    container = (Container *)sibling;
> > -                    ring = &container->items;
> > -                    item->base.container = container;
> > -                    now = ring_next(ring, ring);
> > -                    continue;
> > -                }
> > -                spice_assert(IS_DRAW_ITEM(sibling));
> > -                if (!DRAW_ITEM(sibling)->container_root) {
> > -                    container = container_new(DRAW_ITEM(sibling));
> > -                    if (!container) {
> > -                        spice_warning("create new container failed");
> > -                        region_destroy(&exclude_rgn);
> > -                        return FALSE;
> > -                    }
> > -                    item->base.container = container;
> > -                    ring = &container->items;
> > -                }
> > -            }
> > -        }
> > -        if (!exclude_base) {
> > -            exclude_base = now;
> > -        }
> > -        break;
> > -    }
> > -    if (item->effect == QXL_EFFECT_OPAQUE) {
> > -        region_or(&exclude_rgn, &item->base.rgn);
> > -        exclude_region(display, ring, exclude_base, &exclude_rgn, NULL,
> > drawable);
> > -        stream_trace_update(display, drawable);
> > -        streams_update_visible_region(display, drawable);
> > -        /*
> > -         * Performing the insertion after exclude_region for
> > -         * safety (todo: Not sure if exclude_region can affect the
> > drawable
> > -         * if it is added to the tree before calling exclude_region).
> > -         */
> > -        current_add_drawable(display, drawable, ring);
> > -    } else {
> > -        /*
> > -         * red_detach_streams_behind can affect the current tree since it
> > may
> > -         * trigger calls to update_area. Thus, the drawable should be
> > added
> > to the tree
> > -         * before calling red_detach_streams_behind
> > -         */
> > -        current_add_drawable(display, drawable, ring);
> > -        if (is_primary_surface(display, drawable->surface_id)) {
> > -            detach_streams_behind(display, &drawable->tree_item.base.rgn,
> > drawable);
> > -        }
> > -    }
> > -    region_destroy(&exclude_rgn);
> > -    stat_add(&display->add_stat, start_time);
> > -    return TRUE;
> > -}
> > -
> > -static int current_add_with_shadow(DisplayChannel *display, Ring *ring,
> > Drawable *item)
> > -{
> > -#ifdef RED_WORKER_STAT
> > -    RedWorker *worker = COMMON_CHANNEL(display)->worker;
> > -    stat_time_t start_time = stat_now(worker->clockid);
> > -    ++display->add_with_shadow_count;
> > -#endif
> > -
> > -    RedDrawable *red_drawable = item->red_drawable;
> > -    SpicePoint delta = {
> > -        .x = red_drawable->u.copy_bits.src_pos.x -
> > red_drawable->bbox.left,
> > -        .y = red_drawable->u.copy_bits.src_pos.y - red_drawable->bbox.top
> > -    };
> > -
> > -    Shadow *shadow = shadow_new(&item->tree_item, &delta);
> > -    if (!shadow) {
> > -        stat_add(&display->add_stat, start_time);
> > -        return FALSE;
> > -    }
> > -    // item and his shadow must initially be placed in the same container.
> > -    // for now putting them on root.
> > -
> > -    // only primary surface streams are supported
> > -    if (is_primary_surface(display, item->surface_id)) {
> > -        detach_streams_behind(display, &shadow->base.rgn, NULL);
> > -    }
> > -
> > -    ring_add(ring, &shadow->base.siblings_link);
> > -    current_add_drawable(display, item, ring);
> > -    if (item->tree_item.effect == QXL_EFFECT_OPAQUE) {
> > -        QRegion exclude_rgn;
> > -        region_clone(&exclude_rgn, &item->tree_item.base.rgn);
> > -        exclude_region(display, ring, &shadow->base.siblings_link,
> > &exclude_rgn, NULL, NULL);
> > -        region_destroy(&exclude_rgn);
> > -        streams_update_visible_region(display, item);
> > -    } else {
> > -        if (is_primary_surface(display, item->surface_id)) {
> > -            detach_streams_behind(display, &item->tree_item.base.rgn,
> > item);
> > -        }
> > -    }
> > -    stat_add(&display->add_stat, start_time);
> > -    return TRUE;
> > -}
> > -
> > -static void drawable_update_streamable(DisplayChannel *display, Drawable
> > *drawable)
> > -{
> > -    RedDrawable *red_drawable = drawable->red_drawable;
> > -    SpiceImage *image;
> > -
> > -    if (display->stream_video == SPICE_STREAM_VIDEO_OFF) {
> > -        return;
> > -    }
> > -
> > -    if (!is_primary_surface(display, drawable->surface_id)) {
> > -        return;
> > -    }
> > -
> > -    if (drawable->tree_item.effect != QXL_EFFECT_OPAQUE ||
> > -        red_drawable->type != QXL_DRAW_COPY ||
> > -        red_drawable->u.copy.rop_descriptor != SPICE_ROPD_OP_PUT) {
> > -        return;
> > -    }
> > -
> > -    image = red_drawable->u.copy.src_bitmap;
> > -    if (image == NULL ||
> > -        image->descriptor.type != SPICE_IMAGE_TYPE_BITMAP) {
> > -        return;
> > -    }
> > -
> > -    if (display->stream_video == SPICE_STREAM_VIDEO_FILTER) {
> > -        SpiceRect* rect;
> > -        int size;
> > -
> > -        rect = &drawable->red_drawable->u.copy.src_area;
> > -        size = (rect->right - rect->left) * (rect->bottom - rect->top);
> > -        if (size < RED_STREAM_MIN_SIZE) {
> > -            return;
> > -        }
> > -    }
> > -
> > -    drawable->streamable = TRUE;
> > -}
> > -
> > -void print_stats(DisplayChannel *display)
> > -{
> > -#ifdef RED_WORKER_STAT
> > -    stat_time_t total = display->add_stat.total;
> > -    spice_info("add with shadow count %u",
> > -               display->add_with_shadow_count);
> > -    display->add_with_shadow_count = 0;
> > -    spice_info("add[%u] %f exclude[%u] %f __exclude[%u] %f",
> > -               display->add_stat.count,
> > -               stat_cpu_time_to_sec(total),
> > -               display->exclude_stat.count,
> > -               stat_cpu_time_to_sec(display->exclude_stat.total),
> > -               display->__exclude_stat.count,
> > -               stat_cpu_time_to_sec(display->__exclude_stat.total));
> > -    spice_info("add %f%% exclude %f%% exclude2 %f%% __exclude %f%%",
> > -               (double)(total - display->exclude_stat.total) / total *
> > 100,
> > -               (double)(display->exclude_stat.total) / total * 100,
> > -               (double)(display->exclude_stat.total -
> > -                        display->__exclude_stat.total) / display
> > ->exclude_stat.total * 100,
> > -               (double)(display->__exclude_stat.total) / display
> > ->exclude_stat.total * 100);
> > -    stat_reset(&display->add_stat);
> > -    stat_reset(&display->exclude_stat);
> > -    stat_reset(&display->__exclude_stat);
> > -#endif
> > -}
> > -
> > - static int red_add_drawable(DisplayChannel *display, Drawable *drawable)
> > -{
> > -    int ret = FALSE, surface_id = drawable->surface_id;
> > -    RedDrawable *red_drawable = drawable->red_drawable;
> > -    Ring *ring = &display->surfaces[surface_id].current;
> > -
> > -    if (has_shadow(red_drawable)) {
> > -        ret = current_add_with_shadow(display, ring, drawable);
> > -    } else {
> > -        drawable_update_streamable(display, drawable);
> > -        ret = current_add(display, ring, drawable);
> > -    }
> > -
> > -#ifdef RED_WORKER_STAT
> > -    if ((++display->add_count % 100) == 0)
> > -        print_stats(display);
> > -#endif
> > -    return ret;
> > -}
> > -
> >  static void red_get_area(DisplayChannel *display, int surface_id, const
> > SpiceRect *area,
> >                           uint8_t *dest, int dest_stride, int update)
> >  {
> > @@ -2193,7 +1660,7 @@ static inline void red_process_draw(RedWorker
> > *worker,
> > RedDrawable *red_drawable
> >          goto cleanup;
> >      }
> >  
> > -    if (red_add_drawable(worker->display_channel, drawable)) {
> > +    if (display_channel_add_drawable(worker->display_channel, drawable)) {
> >          red_pipes_add_drawable(worker->display_channel, drawable);
> >      }
> >  cleanup:

Frediano


More information about the Spice-devel mailing list