Re: [PATCH 16/18] worker: move display_channel_draw

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> 
> On Mon, Nov 23, 2015 at 6:02 PM, Frediano Ziglio <fziglio@xxxxxxxxxx> wrote:
> > From: Marc-André Lureau <marcandre.lureau@xxxxxxxxx>
> >
> > ---
> >  server/display-channel.c | 155
> >  +++++++++++++++++++++++++++++++++++++++++++++++
> >  server/red_worker.c      | 154
> >  ----------------------------------------------
> >  2 files changed, 155 insertions(+), 154 deletions(-)
> >
> > diff --git a/server/display-channel.c b/server/display-channel.c
> > index a178cc9..c5a69e3 100644
> > --- a/server/display-channel.c
> > +++ b/server/display-channel.c
> > @@ -1261,3 +1261,158 @@ void drawable_draw(DisplayChannel *display,
> > Drawable *drawable)
> >          spice_warning("invalid type");
> >      }
> >  }
> > +
> > +static void surface_update_dest(RedSurface *surface, const SpiceRect
> > *area)
> > +{
> > +    SpiceCanvas *canvas = surface->context.canvas;
> > +    int h = area->bottom - area->top;
> > +    int stride = surface->context.stride;
> > +    uint8_t *line_0 = surface->context.line_0;
> > +
> > +    if (surface->context.canvas_draws_on_surface)
> > +        return;
> > +    if (h == 0)
> > +        return;
> > +
> > +    spice_return_if_fail(stride < 0);
> > +
> > +    uint8_t *dest = line_0 + (area->top * stride) + area->left *
> > sizeof(uint32_t);
> > +    dest += (h - 1) * stride;
> > +    canvas->ops->read_bits(canvas, dest, -stride, area);
> > +}
> > +
> > +/*
> > + * Renders drawables for updating the requested area, but only drawables
> > that are older
> > + * than 'last' (exclusive).
> > + * FIXME: merge with display_channel_draw()?
> > + */
> > +void display_channel_draw_till(DisplayChannel *display, const SpiceRect
> > *area, int surface_id,
> > +                               Drawable *last)
> > +{
> > +    RedSurface *surface;
> > +    Drawable *surface_last = NULL;
> > +    Ring *ring;
> > +    RingItem *ring_item;
> > +    Drawable *now;
> > +    QRegion rgn;
> > +
> > +    spice_return_if_fail(last);
> > +    spice_return_if_fail(ring_item_is_linked(&last->list_link));
> > +
> > +    surface = &display->surfaces[surface_id];
> > +
> > +    if (surface_id != last->surface_id) {
> > +        // find the nearest older drawable from the appropriate surface
> > +        ring = &display->current_list;
> > +        ring_item = &last->list_link;
> > +        while ((ring_item = ring_next(ring, ring_item))) {
> > +            now = SPICE_CONTAINEROF(ring_item, Drawable, list_link);
> > +            if (now->surface_id == surface_id) {
> > +                surface_last = now;
> > +                break;
> > +            }
> > +        }
> > +    } else {
> > +        ring_item = ring_next(&surface->current_list,
> > &last->surface_list_link);
> > +        if (ring_item) {
> > +            surface_last = SPICE_CONTAINEROF(ring_item, Drawable,
> > surface_list_link);
> > +        }
> > +    }
> > +
> > +    if (!surface_last) {
> > +        return;
> > +    }
> > +
> > +    ring = &surface->current_list;
> > +    ring_item = &surface_last->surface_list_link;
> > +
> > +    region_init(&rgn);
> > +    region_add(&rgn, area);
> > +
> > +    // find the first older drawable that intersects with the area
> > +    do {
> > +        now = SPICE_CONTAINEROF(ring_item, Drawable, surface_list_link);
> > +        if (region_intersects(&rgn, &now->tree_item.base.rgn)) {
> > +            surface_last = now;
> > +            break;
> > +        }
> > +    } while ((ring_item = ring_next(ring, ring_item)));
> > +
> > +    region_destroy(&rgn);
> > +
> > +    if (!surface_last) {
> > +        return;
> > +    }
> > +
> > +    do {
> > +        Container *container;
> > +
> > +        ring_item = ring_get_tail(&surface->current_list);
> > +        now = SPICE_CONTAINEROF(ring_item, Drawable, surface_list_link);
> > +        now->refs++;
> > +        container = now->tree_item.base.container;
> > +        current_remove_drawable(display, now);
> > +        container_cleanup(container);
> > +        /* drawable_draw may call display_channel_draw for the surfaces
> > 'now' depends on. Notice,
> > +           that it is valid to call display_channel_draw in this case and
> > not display_channel_draw_till:
> > +           It is impossible that there was newer item then 'last' in one
> > of the surfaces
> > +           that display_channel_draw is called for, Otherwise, 'now' would
> > have already been rendered.
> > +           See the call for red_handle_depends_on_target_surface in
> > red_process_draw */
> > +        drawable_draw(display, now);
> > +        display_channel_drawable_unref(display, now);
> > +    } while (now != surface_last);
> > +    surface_update_dest(surface, area);
> > +}
> > +
> > +void display_channel_draw(DisplayChannel *display, const SpiceRect *area,
> > int surface_id)
> > +{
> > +    RedSurface *surface;
> > +    Ring *ring;
> > +    RingItem *ring_item;
> > +    QRegion rgn;
> > +    Drawable *last;
> > +    Drawable *now;
> > +    spice_debug("surface %d: area ==>", surface_id);
> > +    rect_debug(area);
> > +
> > +    spice_return_if_fail(surface_id >= 0 && surface_id < NUM_SURFACES);
> > +    spice_return_if_fail(area);
> > +    spice_return_if_fail(area->left >= 0 && area->top >= 0 &&
> > +                         area->left < area->right && area->top <
> > area->bottom);
> > +
> > +    surface = &display->surfaces[surface_id];
> > +
> > +    last = NULL;
> > +    ring = &surface->current_list;
> > +    ring_item = ring;
> > +
> > +    region_init(&rgn);
> > +    region_add(&rgn, area);
> > +    while ((ring_item = ring_next(ring, ring_item))) {
> > +        now = SPICE_CONTAINEROF(ring_item, Drawable, surface_list_link);
> > +        if (region_intersects(&rgn, &now->tree_item.base.rgn)) {
> > +            last = now;
> > +            break;
> > +        }
> > +    }
> > +    region_destroy(&rgn);
> > +
> > +    if (!last) {
> > +        surface_update_dest(surface, area);
> > +        return;
> > +    }
> > +
> > +    do {
> > +        Container *container;
> > +
> > +        ring_item = ring_get_tail(&surface->current_list);
> > +        now = SPICE_CONTAINEROF(ring_item, Drawable, surface_list_link);
> > +        now->refs++;
> > +        container = now->tree_item.base.container;
> > +        current_remove_drawable(display, now);
> > +        container_cleanup(container);
> > +        drawable_draw(display, now);
> > +        display_channel_drawable_unref(display, now);
> > +    } while (now != last);
> > +    surface_update_dest(surface, area);
> > +}
> > diff --git a/server/red_worker.c b/server/red_worker.c
> > index a82a871..1dc75a0 100644
> > --- a/server/red_worker.c
> > +++ b/server/red_worker.c
> > @@ -936,160 +936,6 @@ static void image_surface_init(DisplayChannel
> > *display)
> >      display->image_surfaces.ops = &image_surfaces_ops;
> >  }
> >
> > -static void surface_update_dest(RedSurface *surface, const SpiceRect
> > *area)
> > -{
> > -    SpiceCanvas *canvas = surface->context.canvas;
> > -    int h = area->bottom - area->top;
> > -    int stride = surface->context.stride;
> > -    uint8_t *line_0 = surface->context.line_0;
> > -
> > -    if (surface->context.canvas_draws_on_surface)
> > -        return;
> > -    if (h == 0)
> > -        return;
> > -
> > -    spice_return_if_fail(stride < 0);
> > -
> > -    uint8_t *dest = line_0 + (area->top * stride) + area->left *
> > sizeof(uint32_t);
> > -    dest += (h - 1) * stride;
> > -    canvas->ops->read_bits(canvas, dest, -stride, area);
> > -}
> > -
> > -/*
> > -    Renders drawables for updating the requested area, but only drawables
> > that are older
> > -    than 'last' (exclusive).
> > -*/
> > -void display_channel_draw_till(DisplayChannel *display, const SpiceRect
> > *area, int surface_id,
> > -                          Drawable *last)
> > -{
> > -    RedSurface *surface;
> > -    Drawable *surface_last = NULL;
> > -    Ring *ring;
> > -    RingItem *ring_item;
> > -    Drawable *now;
> > -    QRegion rgn;
> > -
> > -    spice_assert(last);
> > -    spice_assert(ring_item_is_linked(&last->list_link));
> > -
> > -    surface = &display->surfaces[surface_id];
> > -
> > -    if (surface_id != last->surface_id) {
> > -        // find the nearest older drawable from the appropriate surface
> > -        ring = &display->current_list;
> > -        ring_item = &last->list_link;
> > -        while ((ring_item = ring_next(ring, ring_item))) {
> > -            now = SPICE_CONTAINEROF(ring_item, Drawable, list_link);
> > -            if (now->surface_id == surface_id) {
> > -                surface_last = now;
> > -                break;
> > -            }
> > -        }
> > -    } else {
> > -        ring_item = ring_next(&surface->current_list,
> > &last->surface_list_link);
> > -        if (ring_item) {
> > -            surface_last = SPICE_CONTAINEROF(ring_item, Drawable,
> > surface_list_link);
> > -        }
> > -    }
> > -
> > -    if (!surface_last) {
> > -        return;
> > -    }
> > -
> > -    ring = &surface->current_list;
> > -    ring_item = &surface_last->surface_list_link;
> > -
> > -    region_init(&rgn);
> > -    region_add(&rgn, area);
> > -
> > -    // find the first older drawable that intersects with the area
> > -    do {
> > -        now = SPICE_CONTAINEROF(ring_item, Drawable, surface_list_link);
> > -        if (region_intersects(&rgn, &now->tree_item.base.rgn)) {
> > -            surface_last = now;
> > -            break;
> > -        }
> > -    } while ((ring_item = ring_next(ring, ring_item)));
> > -
> > -    region_destroy(&rgn);
> > -
> > -    if (!surface_last) {
> > -        return;
> > -    }
> > -
> > -    do {
> > -        Container *container;
> > -
> > -        ring_item = ring_get_tail(&surface->current_list);
> > -        now = SPICE_CONTAINEROF(ring_item, Drawable, surface_list_link);
> > -        now->refs++;
> > -        container = now->tree_item.base.container;
> > -        current_remove_drawable(display, now);
> > -        container_cleanup(container);
> > -        /* drawable_draw may call display_channel_draw for the surfaces
> > 'now' depends on. Notice,
> > -           that it is valid to call display_channel_draw in this case and
> > not display_channel_draw_till:
> > -           It is impossible that there was newer item then 'last' in one
> > of the surfaces
> > -           that display_channel_draw is called for, Otherwise, 'now' would
> > have already been rendered.
> > -           See the call for red_handle_depends_on_target_surface in
> > red_process_draw */
> > -        drawable_draw(display, now);
> > -        display_channel_drawable_unref(display, now);
> > -    } while (now != surface_last);
> > -    surface_update_dest(surface, area);
> > -}
> > -
> > -void display_channel_draw(DisplayChannel *display, const SpiceRect *area,
> > int surface_id)
> > -{
> > -    RedSurface *surface;
> > -    Ring *ring;
> > -    RingItem *ring_item;
> > -    QRegion rgn;
> > -    Drawable *last;
> > -    Drawable *now;
> > -    spice_debug("surface %d: area ==>", surface_id);
> > -    rect_debug(area);
> > -
> > -    spice_return_if_fail(surface_id >= 0 && surface_id < NUM_SURFACES);
> > -    spice_return_if_fail(area);
> > -    spice_return_if_fail(area->left >= 0 && area->top >= 0 &&
> > -                         area->left < area->right && area->top <
> > area->bottom);
> > -
> > -    surface = &display->surfaces[surface_id];
> > -
> > -    last = NULL;
> > -    ring = &surface->current_list;
> > -    ring_item = ring;
> > -
> > -    region_init(&rgn);
> > -    region_add(&rgn, area);
> > -    while ((ring_item = ring_next(ring, ring_item))) {
> > -        now = SPICE_CONTAINEROF(ring_item, Drawable, surface_list_link);
> > -        if (region_intersects(&rgn, &now->tree_item.base.rgn)) {
> > -            last = now;
> > -            break;
> > -        }
> > -    }
> > -    region_destroy(&rgn);
> > -
> > -    if (!last) {
> > -        surface_update_dest(surface, area);
> > -        return;
> > -    }
> > -
> > -    do {
> > -        Container *container;
> > -
> > -        ring_item = ring_get_tail(&surface->current_list);
> > -        now = SPICE_CONTAINEROF(ring_item, Drawable, surface_list_link);
> > -        now->refs++;
> > -        container = now->tree_item.base.container;
> > -        current_remove_drawable(display, now);
> > -        container_cleanup(container);
> > -        drawable_draw(display, now);
> > -        display_channel_drawable_unref(display, now);
> > -    } while (now != last);
> > -    surface_update_dest(surface, area);
> > -}
> > -
> >  static int red_process_cursor(RedWorker *worker, uint32_t max_pipe_size,
> >  int *ring_is_empty)
> >  {
> >      QXLCommandExt ext_cmd;
> > --
> > 2.4.3
> >
> > _______________________________________________
> > Spice-devel mailing list
> > Spice-devel@xxxxxxxxxxxxxxxxxxxxx
> > http://lists.freedesktop.org/mailman/listinfo/spice-devel
> 
> The move itself is okay, but it depends on another patch that causes a
> regression. So, better not to hav for now.
> Reviewed-by: Fabiano Fidêncio <fidencio@xxxxxxxxxx>
> 

In the meantime... reduced diff


--- before.c	2015-11-24 10:02:59.353153718 +0000
+++ after.c	2015-11-24 10:02:17.603643716 +0000
@@ -1276,35 +1276,36 @@
         return;
 
     spice_return_if_fail(stride < 0);
 
     uint8_t *dest = line_0 + (area->top * stride) + area->left * sizeof(uint32_t);
     dest += (h - 1) * stride;
     canvas->ops->read_bits(canvas, dest, -stride, area);
 }
 
 /*
-    Renders drawables for updating the requested area, but only drawables that are older
-    than 'last' (exclusive).
+ * Renders drawables for updating the requested area, but only drawables that are older
+ * than 'last' (exclusive).
+ * FIXME: merge with display_channel_draw()?
 */
 void display_channel_draw_till(DisplayChannel *display, const SpiceRect *area, int surface_id,
                           Drawable *last)
 {
     RedSurface *surface;
     Drawable *surface_last = NULL;
     Ring *ring;
     RingItem *ring_item;
     Drawable *now;
     QRegion rgn;
 
-    spice_assert(last);
-    spice_assert(ring_item_is_linked(&last->list_link));
+    spice_return_if_fail(last);
+    spice_return_if_fail(ring_item_is_linked(&last->list_link));
 
     surface = &display->surfaces[surface_id];
 
     if (surface_id != last->surface_id) {
         // find the nearest older drawable from the appropriate surface
         ring = &display->current_list;
         ring_item = &last->list_link;
         while ((ring_item = ring_next(ring, ring_item))) {
             now = SPICE_CONTAINEROF(ring_item, Drawable, list_link);
             if (now->surface_id == surface_id) {


So beside the usually not really agreed changes for logging and
the FIXME comment it's a move!

Frediano
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/spice-devel




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]