[Spice-devel] [PATCH spice-server 1/3] red_worker: cleanup red_clear_surface_drawables_from_pipes

[Marc-André Lureau]

Hi,

Since I don't understand the "force" argument, it would be easier to
review to keep current behaviour and just call
red_wait_outgoing_item() at the end.

To me the meaning of "force" argument is rather "do not wait if dep".
When a drawable depend on the surface:
- force == false: return immediately when found
- force == true: wait for the first found item to be sent (I guess it
is the last to be sent, but isn't it supposed to be sent before the
one that depends on it?)

If you could clarified the above, and comment it in the code that
would be helpful! :)


On Thu, Sep 12, 2013 at 10:04 PM, Yonit Halperin <yhalperi at redhat.com> wrote:
> (1) merge 'force' and 'wait_for_outgoing_item' to one parameter.
>     'wait_for_outgoing_item' is a derivative of 'force'.
> (2) move the call to red_wait_outgoing_item to red_clear_surface_drawables_from_pipe
> ---
>  server/red_worker.c | 20 +++++++++++---------
>  1 file changed, 11 insertions(+), 9 deletions(-)
>
> diff --git a/server/red_worker.c b/server/red_worker.c
> index 0c611d0..e9a7fa1 100644
> --- a/server/red_worker.c
> +++ b/server/red_worker.c
> @@ -2064,22 +2064,24 @@ static void red_clear_surface_drawables_from_pipe(DisplayChannelClient *dcc, int
>
>      if (item) {
>          red_channel_client_wait_pipe_item_sent(&dcc->common.base, item);
> +    } else if (force) {
> +        /*
> +         * in case that the pipe didn't contain any item that is dependent on the surface, but
> +         * there is one during sending.
> +         */
> +        red_wait_outgoing_item(&dcc->common.base);
>      }
>  }
>
> -static void red_clear_surface_drawables_from_pipes(RedWorker *worker, int surface_id,
> -    int force, int wait_for_outgoing_item)
> +static void red_clear_surface_drawables_from_pipes(RedWorker *worker,
> +                                                   int surface_id,
> +                                                   int force)
>  {
>      RingItem *item, *next;
>      DisplayChannelClient *dcc;
>
>      WORKER_FOREACH_DCC_SAFE(worker, item, next, dcc) {
>          red_clear_surface_drawables_from_pipe(dcc, surface_id, force);
> -        if (wait_for_outgoing_item) {
> -            // in case that the pipe didn't contain any item that is dependent on the surface, but
> -            // there is one during sending.
> -            red_wait_outgoing_item(&dcc->common.base);
> -        }
>      }
>  }
>
> @@ -4248,7 +4250,7 @@ static inline void red_process_surface(RedWorker *worker, RedSurfaceCmd *surface
>             otherwise "current" will hold items that other drawables may depend on, and then
>             red_current_clear will remove them from the pipe. */
>          red_current_clear(worker, surface_id);
> -        red_clear_surface_drawables_from_pipes(worker, surface_id, FALSE, FALSE);
> +        red_clear_surface_drawables_from_pipes(worker, surface_id, FALSE);
>          red_destroy_surface(worker, surface_id);
>          break;
>      default:
> @@ -10921,7 +10923,7 @@ static inline void destroy_surface_wait(RedWorker *worker, int surface_id)
>         otherwise "current" will hold items that other drawables may depend on, and then
>         red_current_clear will remove them from the pipe. */
>      red_current_clear(worker, surface_id);
> -    red_clear_surface_drawables_from_pipes(worker, surface_id, TRUE, TRUE);
> +    red_clear_surface_drawables_from_pipes(worker, surface_id, TRUE);
>  }
>
>  static void dev_destroy_surface_wait(RedWorker *worker, uint32_t surface_id)
> --
> 1.8.1.4
>
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/spice-devel



-- 
Marc-André Lureau