[Spice-devel] [PATCH] display: factor out draw_until

[Frediano Ziglio]

> 
> From: Marc-André Lureau <marcandre.lureau at gmail.com>
> 
> Acked-by: Fabiano Fidêncio <fidencio at redhat.com>
> ---
>  server/display-channel.c | 62
>  +++++++++++++++++++++++-------------------------
>  1 file changed, 30 insertions(+), 32 deletions(-)
> 
> diff --git a/server/display-channel.c b/server/display-channel.c
> index d54f725..1837c9b 100644
> --- a/server/display-channel.c
> +++ b/server/display-channel.c
> @@ -1282,6 +1282,24 @@ static void surface_update_dest(RedSurface *surface,
> const SpiceRect *area)
>      canvas->ops->read_bits(canvas, dest, -stride, area);
>  }
>  
> +static void draw_until(DisplayChannel *display, RedSurface *surface,
> Drawable *last)
> +{
> +    RingItem *ring_item;
> +    Container *container;
> +    Drawable *now;
> +
> +    do {
> +        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);
> +}
> +
>  /*
>   * Renders drawables for updating the requested area, but only drawables
>   that are older
>   * than 'last' (exclusive).
> @@ -1345,23 +1363,17 @@ void display_channel_draw_till(DisplayChannel
> *display, const SpiceRect *area, i
>          return;
>      }
>  
> -    do {
> -        Container *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
> +     */

I don't understand now the sense of this comment here.
Should not be inside the new function?
It refer to drawable_draw and now but in this function there are neither
of them.

> +    draw_until(display, surface, surface_last);
>  
> -        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);
>  }
>  
> @@ -1398,22 +1410,8 @@ void display_channel_draw(DisplayChannel *display,
> const SpiceRect *area, int su
>      }
>      region_destroy(&rgn);
>  
> -    if (!last) {
> -        surface_update_dest(surface, area);
> -        return;
> -    }
> -
> -    do {
> -        Container *container;
> +    if (last)
> +        draw_until(display, surface, last);
>  
> -        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);
>  }

Frediano