[Spice-devel] [PATCH] server: fix update_area

[Alon Levy]

On Sun, May 08, 2011 at 05:06:46PM +0300, Alon Levy wrote:
> in next revision (v6) this will be folded into the patch that introduced
> the bug. update_area is a request from the guest to render everything affecting
> a specific area so the guest can read the contents directly via the pci bar
> surface. The operations were wrongly performed more then once. This fixes
> handle_dev_update and red_update_area (which when folded will just be the
> renaming of red_render_update_area).

The fix is worst then the disease - with this patch rendering is correct on update_area
but drawables are not released leading to eventual assert (pretty quick if you move a window
around). I'll research a real fix, don't apply.

> ---
>  server/red_worker.c |   31 ++++++++-----------------------
>  1 files changed, 8 insertions(+), 23 deletions(-)
> 
> diff --git a/server/red_worker.c b/server/red_worker.c
> index c9d6211..74a72a9 100644
> --- a/server/red_worker.c
> +++ b/server/red_worker.c
> @@ -970,12 +970,10 @@ typedef struct BitmapData {
>  static void red_draw_qxl_drawable(RedRender *render, Drawable *drawable);
>  static void red_current_flush(RedRender *render, int surface_id);
>  #ifdef DRAW_ALL
> -#define red_update_area(worker, rect, surface_id)
>  #define red_render_update_area(render, rect, surface_id)
>  #define red_draw_drawable(render, item)
>  #else
>  static void red_draw_drawable(RedRender *render, Drawable *item);
> -static void red_update_area(RedWorker *worker, const SpiceRect *area, int surface_id);
>  static void red_render_update_area(RedRender *render, const SpiceRect *area, int surface_id);
>  #endif
>  static void red_release_cursor(RedWorker *worker, CursorItem *cursor);
> @@ -4378,17 +4376,6 @@ static void red_render_update_area(RedRender *render, const SpiceRect *area, int
>  }
>  
>  #endif
> -
> -static void red_update_area(RedWorker *worker, const SpiceRect *area, int surface_id)
> -{
> -    RedRender *render;
> -    RingItem *link;
> -
> -    RENDER_FOREACH(link, render, worker) {
> -        red_render_update_area(render, area, surface_id);
> -    }
> -}
> -
>  #endif
>  
>  static inline void free_cursor_item(RedWorker *worker, CursorItem *item);
> @@ -4633,7 +4620,8 @@ static int red_process_commands(RedWorker *worker, uint32_t max_pipe_size, int *
>              red_get_update_cmd(&worker->mem_slots, ext_cmd.group_id,
>                                 &update, ext_cmd.cmd.data);
>              validate_surface(&worker->render, update.surface_id);
> -            red_update_area(worker, &update.area, update.surface_id);
> +            red_render_update_area(&worker->render, &update.area,
> +                                   update.surface_id);
>              worker->qxl->st->qif->notify_update(worker->qxl, update.update_id);
>              release_info_ext.group_id = ext_cmd.group_id;
>              release_info_ext.info = update.release_info;
> @@ -10271,8 +10259,7 @@ static inline void handle_dev_update(RedWorker *worker)
>      uint32_t num_dirty_rects;
>      uint32_t surface_id;
>      uint32_t clear_dirty_region;
> -    RingItem *link;
> -    RedRender *render;
> +    RedRender *render = &worker->render;
>  
>      receive_data(worker->channel, &surface_id, sizeof(uint32_t));
>      receive_data(worker->channel, &rect, sizeof(SpiceRect *));
> @@ -10285,14 +10272,12 @@ static inline void handle_dev_update(RedWorker *worker)
>      ASSERT(worker->running);
>  
>      validate_surface(&worker->render, surface_id);
> -    red_update_area(worker, rect, surface_id);
> +    red_render_update_area(&worker->render, rect, surface_id);
>  
> -    RENDER_FOREACH(link, render, worker) {
> -        surface = &render->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);
> -        }
> +    surface = &render->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);
>      }
>      message = RED_WORKER_MESSAGE_READY;
>      write_message(worker->channel, &message);
> -- 
> 1.7.5.1
> 
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/spice-devel