[Spice-devel] [PATCH 16/16] worker: rename process_commands process_display

[Jonathon Jongsma]

On Fri, 2015-11-27 at 04:16 -0500, Frediano Ziglio wrote:
> And move a function from a line to another on same file.
> I'll revert the move.
> 
> Does this change (the rename) sounds good ?

I get the impression that the rename was done to make it easy to distinguish
from red_process_cursor.


> 
> I don't know to me would sound good to move all display
> processing to display-channel.c (with the generation field too).

if red_process_display() was moved into display-channel.c (and
red_process_cursor() was moved into cursor-channel.c) then the names would not
be easily confused and so the rename would not be necessary. However, I suspect
that some more refactoring might need to be done to enable this move.


> 
> Frediano
> 
> 
> > 
> > From: Marc-André Lureau <marcandre.lureau at gmail.com>
> > 
> > ---
> >  server/red_worker.c | 53
> >  ++++++++++++++++++++++++++---------------------------
> >  1 file changed, 26 insertions(+), 27 deletions(-)
> > 
> > diff --git a/server/red_worker.c b/server/red_worker.c
> > index 5175839..d9b98d5 100644
> > --- a/server/red_worker.c
> > +++ b/server/red_worker.c
> > @@ -87,7 +87,7 @@ struct RedWorker {
> >      spice_wan_compression_t jpeg_state;
> >      spice_wan_compression_t zlib_glz_state;
> >  
> > -    uint32_t process_commands_generation;
> > +    uint32_t process_display_generation;
> >  #ifdef RED_STATISTICS
> >      StatNodeRef stat;
> >      uint64_t *wakeup_counter;
> > @@ -165,26 +165,6 @@ void red_drawable_unref(RedWorker *worker, RedDrawable
> > *red_drawable,
> >      free(red_drawable);
> >  }
> >  
> > -static void red_process_draw(RedWorker *worker, RedDrawable *red_drawable,
> > -                             uint32_t group_id)
> > -{
> > -    DisplayChannel *display = worker->display_channel;
> > -    Drawable *drawable;
> > -    bool success = FALSE;
> > -
> > -    drawable = display_channel_drawable_try_new(display, group_id,
> > -
> > worker->process_commands_generation);
> > -    if (!drawable) {
> > -        return;
> > -    }
> > -
> > -    success = display_channel_add_drawable(worker->display_channel,
> > drawable, red_drawable);
> > -    spice_warn_if_fail(success);
> > -
> > -    display_channel_drawable_unref(display, drawable);
> > -}
> > -
> > -
> >  static int red_process_cursor(RedWorker *worker, uint32_t max_pipe_size,
> > int
> >  *ring_is_empty)
> >  {
> >      QXLCommandExt ext_cmd;
> > @@ -244,7 +224,26 @@ static RedDrawable *red_drawable_new(RedWorker *worker)
> >      return red;
> >  }
> >  
> > -static int red_process_commands(RedWorker *worker, uint32_t max_pipe_size,
> > int *ring_is_empty)
> > +static void red_process_draw(RedWorker *worker, RedDrawable *red_drawable,
> > +                                    uint32_t group_id)
> > +{
> > +    DisplayChannel *display = worker->display_channel;
> > +    Drawable *drawable;
> > +    bool success = FALSE;
> > +
> > +    drawable = display_channel_drawable_try_new(display, group_id,
> > +
> > worker->process_display_generation);
> > +    if (!drawable) {
> > +        return;
> > +    }
> > +
> > +    success = display_channel_add_drawable(worker->display_channel,
> > drawable, red_drawable);
> > +    spice_warn_if_fail(success);
> > +
> > +    display_channel_drawable_unref(display, drawable);
> > +}
> > +
> > +static int red_process_display(RedWorker *worker, uint32_t max_pipe_size,
> > int *ring_is_empty)
> >  {
> >      QXLCommandExt ext_cmd;
> >      int n = 0;
> > @@ -255,7 +254,7 @@ static int red_process_commands(RedWorker *worker,
> > uint32_t max_pipe_size, int *
> >          return n;
> >      }
> >  
> > -    worker->process_commands_generation++;
> > +    worker->process_display_generation++;
> >      *ring_is_empty = FALSE;
> >      while (!display_is_connected(worker) ||
> >             // TODO: change to average pipe size?
> > @@ -386,12 +385,12 @@ static void flush_display_commands(RedWorker *worker)
> >          uint64_t end_time;
> >          int ring_is_empty;
> >  
> > -        red_process_commands(worker, MAX_PIPE_SIZE, &ring_is_empty);
> > +        red_process_display(worker, MAX_PIPE_SIZE, &ring_is_empty);
> >          if (ring_is_empty) {
> >              break;
> >          }
> >  
> > -        while (red_process_commands(worker, MAX_PIPE_SIZE, &ring_is_empty))
> > {
> > +        while (red_process_display(worker, MAX_PIPE_SIZE, &ring_is_empty))
> > {
> >              red_channel_push(RED_CHANNEL(worker->display_channel));
> >          }
> >  
> > @@ -1004,7 +1003,7 @@ static void handle_dev_oom(void *opaque, void
> > *payload)
> >                  display->glz_drawable_count,
> >                  display->current_size,
> >                  red_channel_sum_pipes_size(display_red_channel));
> > -    while (red_process_commands(worker, MAX_PIPE_SIZE, &ring_is_empty)) {
> > +    while (red_process_display(worker, MAX_PIPE_SIZE, &ring_is_empty)) {
> >          red_channel_push(display_red_channel);
> >      }
> >      if (worker->qxl->st->qif->flush_resources(worker->qxl) == 0) {
> > @@ -1681,7 +1680,7 @@ SPICE_GNUC_NORETURN static void *red_worker_main(void
> > *arg)
> >          if (worker->running) {
> >              int ring_is_empty;
> >              red_process_cursor(worker, MAX_PIPE_SIZE, &ring_is_empty);
> > -            red_process_commands(worker, MAX_PIPE_SIZE, &ring_is_empty);
> > +            red_process_display(worker, MAX_PIPE_SIZE, &ring_is_empty);
> >          }
> >          red_push(worker);
> >      }
> 
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/spice-devel