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

[Fabiano Fidêncio]

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?

>
>> +    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