[Spice-devel] [PATCH v3] server: add QXLInterface::surface_updated callback

Yonit Halperin yhalperi at redhat.com
Sat Jul 9 23:55:21 PDT 2011


On 07/07/2011 07:33 PM, Alon Levy wrote:
> used to move dirty rectangle notification from update_area to surface_updated.
>
> This is RfC quality. Specifically where to call surface_updated. Currently I
> only call it from stop. This should be good enough for migration, which is
> the only use case I think. This means after stop qemu is guranteed to know
> the dirty regions of all surfaces.
> ---
>   server/red_dispatcher.c          |   20 +++--------
>   server/red_worker.c              |   71 +++++++++++++++++++++++++------------
>   server/spice.h                   |    8 ++--
>   server/tests/test_display_base.c |    2 +-
>   4 files changed, 58 insertions(+), 43 deletions(-)
>
> diff --git a/server/red_dispatcher.c b/server/red_dispatcher.c
> index 3dfd4e5..996e1ed 100644
> --- a/server/red_dispatcher.c
> +++ b/server/red_dispatcher.c
> @@ -210,9 +210,7 @@ static void update_client_mouse_allowed(void)
>   }
>
>   static void qxl_worker_update_area_helper(QXLWorker *qxl_worker, uint32_t surface_id,
> -                                   QXLRect *qxl_area, QXLRect *qxl_dirty_rects,
> -                                   uint32_t num_dirty_rects, uint32_t clear_dirty_region,
> -                                   int async, uint64_t cookie)
> +                                   QXLRect *qxl_area, int async, uint64_t cookie)
>   {
>       RedDispatcher *dispatcher = (RedDispatcher *)qxl_worker;
>       RedWorkerMessage message;
> @@ -228,9 +226,6 @@ static void qxl_worker_update_area_helper(QXLWorker *qxl_worker, uint32_t surfac
>       }
>       send_data(dispatcher->channel,&surface_id, sizeof(uint32_t));
>       send_data(dispatcher->channel,&qxl_area, sizeof(QXLRect *));
> -    send_data(dispatcher->channel,&qxl_dirty_rects, sizeof(QXLRect *));
> -    send_data(dispatcher->channel,&num_dirty_rects, sizeof(uint32_t));
> -    send_data(dispatcher->channel,&clear_dirty_region, sizeof(uint32_t));
>       if (async) {
>           return;
>       }
> @@ -239,22 +234,17 @@ static void qxl_worker_update_area_helper(QXLWorker *qxl_worker, uint32_t surfac
>   }
>
>   static void qxl_worker_update_area(QXLWorker *qxl_worker, uint32_t surface_id,
> -                                   QXLRect *qxl_area, QXLRect *qxl_dirty_rects,
> -                                   uint32_t num_dirty_rects, uint32_t clear_dirty_region)
> +                                   QXLRect *qxl_area)
>   {
> -    qxl_worker_update_area_helper(qxl_worker, surface_id, qxl_area, qxl_dirty_rects, num_dirty_rects,
> -        clear_dirty_region, 0, 0);
> +    qxl_worker_update_area_helper(qxl_worker, surface_id, qxl_area, 0, 0);
>   }
>
>   static void qxl_worker_update_area_async(QXLWorker *qxl_worker, uint32_t surface_id,
> -                                   QXLRect *qxl_area, QXLRect *qxl_dirty_rects,
> -                                   uint32_t num_dirty_rects, uint32_t clear_dirty_region, uint64_t cookie)
> +                                         QXLRect *qxl_area, uint64_t cookie)
>   {
> -    qxl_worker_update_area_helper(qxl_worker, surface_id, qxl_area, qxl_dirty_rects, num_dirty_rects,
> -        clear_dirty_region, 1, cookie);
> +    qxl_worker_update_area_helper(qxl_worker, surface_id, qxl_area, 1, cookie);
>   }
>
> -
>   static void qxl_worker_add_memslot(QXLWorker *qxl_worker, QXLDevMemSlot *mem_slot)
>   {
>       RedDispatcher *dispatcher = (RedDispatcher *)qxl_worker;
> diff --git a/server/red_worker.c b/server/red_worker.c
> index cb84754..f5b5570 100644
> --- a/server/red_worker.c
> +++ b/server/red_worker.c
> @@ -9415,25 +9415,56 @@ static void red_wait_pipe_item_sent(RedChannel *channel, PipeItem *item)
>       red_unref_channel(channel);
>   }
>
> +static int surface_updated_available(RedWorker *worker)
> +{
> +    int minor = worker->qxl->st->qif->base.minor_version;
> +    int major = worker->qxl->st->qif->base.major_version;
> +
> +    return (major>  3 || (major == 3&&  minor>= 1))&&
> +            worker->qxl->st->qif->surface_updated != NULL;
> +}
Hi,
can qxl < 3.1 be compiled with the interface changes to update_area?

cheers,
Yonit.
> +


> +static void notify_surface_updated(RedWorker *worker, uint32_t surface_id)
> +{
> +    QXLRect *qxl_dirty_rects;
> +    SpiceRect* dirty_rects;
> +    uint32_t num_dirty_rects;
> +    QRegion *surface_dirty_region;
> +    RedSurface *surface;
> +    int i;
> +
> +    if (!surface_updated_available(worker)) {
> +        return;
> +    }
> +
> +    surface =&worker->surfaces[surface_id];
> +    surface_dirty_region =&surface->draw_dirty_region;
> +    num_dirty_rects = pixman_region32_n_rects((pixman_region32_t *)&surface_dirty_region);
> +    dirty_rects = spice_new0(SpiceRect, num_dirty_rects);
> +    region_ret_rects(surface_dirty_region, dirty_rects, num_dirty_rects);
> +    region_clear(surface_dirty_region);
> +    qxl_dirty_rects = spice_new0(QXLRect, num_dirty_rects);
> +    for (i = 0; i<  num_dirty_rects; i++) {
> +        qxl_dirty_rects[i].top    = dirty_rects[i].top;
> +        qxl_dirty_rects[i].left   = dirty_rects[i].left;
> +        qxl_dirty_rects[i].bottom = dirty_rects[i].bottom;
> +        qxl_dirty_rects[i].right  = dirty_rects[i].right;
> +    }
> +    free(dirty_rects);
> +    worker->qxl->st->qif->surface_updated(worker->qxl, surface_id,
> +                                          qxl_dirty_rects, num_dirty_rects);
> +    free(qxl_dirty_rects);
> +}
> +
>   static inline void handle_dev_update(RedWorker *worker)
>   {
>       const QXLRect *qxl_rect;
>       SpiceRect *rect = spice_new0(SpiceRect, 1);
> -    QXLRect *qxl_dirty_rects;
> -    SpiceRect *dirty_rects;
> -    RedSurface *surface;
> -    uint32_t num_dirty_rects;
>       uint32_t surface_id;
> -    uint32_t clear_dirty_region;
> -    int i;
>
>       receive_data(worker->channel,&surface_id, sizeof(uint32_t));
>       receive_data(worker->channel,&qxl_rect, sizeof(QXLRect *));
> -    receive_data(worker->channel,&qxl_dirty_rects, sizeof(QXLRect *));
> -    receive_data(worker->channel,&num_dirty_rects, sizeof(uint32_t));
> -    receive_data(worker->channel,&clear_dirty_region, sizeof(uint32_t));
>
> -    dirty_rects = spice_new0(SpiceRect, num_dirty_rects);
>       red_get_rect_ptr(rect, qxl_rect);
>       flush_display_commands(worker);
>
> @@ -9441,19 +9472,6 @@ static inline void handle_dev_update(RedWorker *worker)
>
>       validate_surface(worker, surface_id);
>       red_update_area(worker, rect, surface_id);
> -
> -    surface =&worker->surfaces[surface_id];
> -    region_ret_rects(&surface->draw_dirty_region, dirty_rects, num_dirty_rects);
> -
> -    if (clear_dirty_region) {
> -        region_clear(&surface->draw_dirty_region);
> -    }
> -    for (i = 0; i<  num_dirty_rects; i++) {
> -        qxl_dirty_rects[i].top    = dirty_rects[i].top;
> -        qxl_dirty_rects[i].left   = dirty_rects[i].left;
> -        qxl_dirty_rects[i].bottom = dirty_rects[i].bottom;
> -        qxl_dirty_rects[i].right  = dirty_rects[i].right;
> -    }
>   }
>
>
> @@ -9636,12 +9654,19 @@ static void handle_dev_flush_surfaces(RedWorker *worker)
>
>   static void handle_dev_stop(RedWorker *worker)
>   {
> +    int x;
> +
>       ASSERT(worker->running);
>       worker->running = FALSE;
>       red_display_clear_glz_drawables(worker->display_channel);
>       flush_all_surfaces(worker);
>       red_wait_outgoing_item((RedChannel *)worker->display_channel);
>       red_wait_outgoing_item((RedChannel *)worker->cursor_channel);
> +    for (x = 0; x<  NUM_SURFACES; ++x) {
> +        if (worker->surfaces[x].context.canvas) {
> +            notify_surface_updated(worker, x);
> +        }
> +    }
>   }
>
>   static void handle_dev_start(RedWorker *worker)
> diff --git a/server/spice.h b/server/spice.h
> index 048462f..4fcc1cf 100644
> --- a/server/spice.h
> +++ b/server/spice.h
> @@ -109,8 +109,7 @@ struct QXLWorker {
>       void (*start)(QXLWorker *worker);
>       void (*stop)(QXLWorker *worker);
>       void (*update_area)(QXLWorker *qxl_worker, uint32_t surface_id,
> -                       struct QXLRect *area, struct QXLRect *dirty_rects,
> -                       uint32_t num_dirty_rects, uint32_t clear_dirty_region);
> +                       struct QXLRect *area);
>       void (*add_memslot)(QXLWorker *worker, QXLDevMemSlot *slot);
>       void (*del_memslot)(QXLWorker *worker, uint32_t slot_group_id, uint32_t slot_id);
>       void (*reset_memslots)(QXLWorker *worker);
> @@ -124,8 +123,7 @@ struct QXLWorker {
>       void (*loadvm_commands)(QXLWorker *worker, struct QXLCommandExt *ext, uint32_t count);
>       /* async versions of commands. when complete spice calls async_complete */
>       void (*update_area_async)(QXLWorker *qxl_worker, uint32_t surface_id,
> -                       struct QXLRect *area, struct QXLRect *dirty_rects,
> -                       uint32_t num_dirty_rects, uint32_t clear_dirty_region, uint64_t cookie);
> +                       struct QXLRect *area, uint64_t cookie);
>       void (*add_memslot_async)(QXLWorker *worker, QXLDevMemSlot *slot, uint64_t cookie);
>       void (*destroy_surfaces_async)(QXLWorker *worker, uint64_t cookie);
>       void (*destroy_primary_surface_async)(QXLWorker *worker, uint32_t surface_id, uint64_t cookie);
> @@ -205,6 +203,8 @@ struct QXLInterface {
>       void (*notify_update)(QXLInstance *qin, uint32_t update_id);
>       int (*flush_resources)(QXLInstance *qin);
>       void (*async_complete)(QXLInstance *qin, uint64_t cookie);
> +    void (*surface_updated)(QXLInstance *qin, uint32_t surface_id, struct QXLRect *updated_rects,
> +                       uint32_t num_updated_rects);
>   };
>
>   struct QXLInstance {
> diff --git a/server/tests/test_display_base.c b/server/tests/test_display_base.c
> index 76817d9..57c593a 100644
> --- a/server/tests/test_display_base.c
> +++ b/server/tests/test_display_base.c
> @@ -364,7 +364,7 @@ static void produce_command()
>           case SIMPLE_UPDATE: {
>               QXLRect rect = {.left = 0, .right = WIDTH,
>                               .top = 0, .bottom = HEIGHT};
> -            qxl_worker->update_area(qxl_worker, 0,&rect, NULL, 0, 1);
> +            qxl_worker->update_area(qxl_worker, 0,&rect);
>               break;
>           }
>           case SIMPLE_COPY_BITS:



More information about the Spice-devel mailing list