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

[Frediano Ziglio]

> 
> On Wed, Nov 25, 2015 at 2:50 PM, Frediano Ziglio <fziglio at redhat.com> wrote:
> >>
> >> 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.
> 
> Moved the comment with the rest of the code in that function.
> Can I push both patches?
> 

Yes, you get my ack to do it!

Frediano

> >
> >> +    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
> > _______________________________________________
> > Spice-devel mailing list
> > Spice-devel at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/spice-devel
> 
> 
> --
> Fabiano Fidêncio
>