[Spice-devel] [PATCH 16/18] worker: move display_channel_draw
Frediano Ziglio
fziglio at redhat.com
Tue Nov 24 02:05:16 PST 2015
>
> On Mon, Nov 23, 2015 at 6:02 PM, Frediano Ziglio <fziglio at redhat.com> wrote:
> > From: Marc-André Lureau <marcandre.lureau at gmail.com>
> >
> > ---
> > 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 at lists.freedesktop.org
> > 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 at redhat.com>
>
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
More information about the Spice-devel
mailing list