[Spice-devel] [PATCH 03/18] worker: move dcc_add_drawable*

Fabiano Fidêncio fidencio at redhat.com
Mon Nov 23 09:43:45 PST 2015


On Mon, Nov 23, 2015 at 6:01 PM, Frediano Ziglio <fziglio at redhat.com> wrote:
> From: Marc-André Lureau <marcandre.lureau at gmail.com>
>
> ---
>  server/dcc.c             |  87 ++++++++++++++++++++++++++++
>  server/dcc.h             |  18 ++++++
>  server/display-channel.c |  65 +++++++++++++++++----
>  server/display-channel.h |  19 +------
>  server/red_worker.c      | 144 +----------------------------------------------
>  5 files changed, 164 insertions(+), 169 deletions(-)
>
> diff --git a/server/dcc.c b/server/dcc.c
> index ec0295a..1c82139 100644
> --- a/server/dcc.c
> +++ b/server/dcc.c
> @@ -155,6 +155,93 @@ void dcc_push_surface_image(DisplayChannelClient *dcc, int surface_id)
>      red_channel_client_push(RED_CHANNEL_CLIENT(dcc));
>  }
>
> +static void add_drawable_surface_images(DisplayChannelClient *dcc, Drawable *drawable)
> +{
> +    DisplayChannel *display = DCC_TO_DC(dcc);
> +    int x;
> +
> +    for (x = 0; x < 3; ++x) {
> +        int surface_id;
> +
> +        surface_id = drawable->surface_deps[x];
> +        if (surface_id != -1) {
> +            if (dcc->surface_client_created[surface_id] == TRUE) {
> +                continue;
> +            }
> +            dcc_create_surface(dcc, surface_id);
> +            display_channel_current_flush(display, surface_id);
> +            dcc_push_surface_image(dcc, surface_id);
> +        }
> +    }
> +
> +    if (dcc->surface_client_created[drawable->surface_id] == TRUE) {
> +        return;
> +    }
> +
> +    dcc_create_surface(dcc, drawable->surface_id);
> +    display_channel_current_flush(display, drawable->surface_id);
> +    dcc_push_surface_image(dcc, drawable->surface_id);
> +}
> +
> +DrawablePipeItem *drawable_pipe_item_ref(DrawablePipeItem *dpi)
> +{
> +    dpi->refs++;
> +    return dpi;
> +}
> +
> +void drawable_pipe_item_unref(DrawablePipeItem *dpi)
> +{
> +    DisplayChannel *display = DCC_TO_DC(dpi->dcc);
> +
> +    if (--dpi->refs != 0)
> +        return;
> +
> +    spice_warn_if_fail(!ring_item_is_linked(&dpi->dpi_pipe_item.link));
> +    spice_warn_if_fail(!ring_item_is_linked(&dpi->base));
> +    display_channel_drawable_unref(display, dpi->drawable);
> +    free(dpi);
> +}
> +
> +static DrawablePipeItem *drawable_pipe_item_new(DisplayChannelClient *dcc, Drawable *drawable)
> +{
> +    DrawablePipeItem *dpi;
> +
> +    dpi = spice_malloc0(sizeof(*dpi));
> +    dpi->drawable = drawable;
> +    dpi->dcc = dcc;
> +    ring_item_init(&dpi->base);
> +    ring_add(&drawable->pipes, &dpi->base);
> +    red_channel_pipe_item_init(RED_CHANNEL_CLIENT(dcc)->channel,
> +                               &dpi->dpi_pipe_item, PIPE_ITEM_TYPE_DRAW);
> +    dpi->refs++;
> +    drawable->refs++;
> +    return dpi;
> +}
> +
> +void dcc_prepend_drawable(DisplayChannelClient *dcc, Drawable *drawable)
> +{
> +    DrawablePipeItem *dpi = drawable_pipe_item_new(dcc, drawable);
> +
> +    add_drawable_surface_images(dcc, drawable);
> +    red_channel_client_pipe_add(RED_CHANNEL_CLIENT(dcc), &dpi->dpi_pipe_item);
> +}
> +
> +void dcc_append_drawable(DisplayChannelClient *dcc, Drawable *drawable)
> +{
> +    DrawablePipeItem *dpi = drawable_pipe_item_new(dcc, drawable);
> +
> +    add_drawable_surface_images(dcc, drawable);
> +    red_channel_client_pipe_add_tail(RED_CHANNEL_CLIENT(dcc), &dpi->dpi_pipe_item);
> +}
> +
> +void dcc_add_drawable_after(DisplayChannelClient *dcc, Drawable *drawable, PipeItem *pos)
> +{
> +    DrawablePipeItem *dpi = drawable_pipe_item_new(dcc, drawable);
> +
> +    add_drawable_surface_images(dcc, drawable);
> +    red_channel_client_pipe_add_after(RED_CHANNEL_CLIENT(dcc), &dpi->dpi_pipe_item, pos);
> +}
> +
>  static void dcc_init_stream_agents(DisplayChannelClient *dcc)
>  {
>      int i;
> diff --git a/server/dcc.h b/server/dcc.h
> index a6e1cc7..12bb38e 100644
> --- a/server/dcc.h
> +++ b/server/dcc.h
> @@ -135,6 +135,17 @@ typedef struct ImageItem {
>      uint8_t data[0];
>  } ImageItem;
>
> +typedef struct DrawablePipeItem {
> +    RingItem base;  /* link for a list of pipe items held by Drawable */
> +    PipeItem dpi_pipe_item; /* link for the client's pipe itself */
> +    Drawable *drawable;
> +    DisplayChannelClient *dcc;
> +    uint8_t refs;
> +} DrawablePipeItem;
> +
> +void                       drawable_pipe_item_unref                  (DrawablePipeItem *dpi);
> +DrawablePipeItem*          drawable_pipe_item_ref                    (DrawablePipeItem *dpi);
> +
>  DisplayChannelClient*      dcc_new                                   (DisplayChannel *display,
>                                                                        RedClient *client,
>                                                                        RedsStream *stream,
> @@ -172,6 +183,13 @@ void                       dcc_palette_cache_palette                 (DisplayCha
>                                                                        uint8_t *flags);
>  int                        dcc_pixmap_cache_unlocked_add             (DisplayChannelClient *dcc,
>                                                                        uint64_t id, uint32_t size, int lossy);
> +void                       dcc_prepend_drawable                      (DisplayChannelClient *dcc,
> +                                                                      Drawable *drawable);
> +void                       dcc_append_drawable                       (DisplayChannelClient *dcc,
> +                                                                      Drawable *drawable);
> +void                       dcc_add_drawable_after                    (DisplayChannelClient *dcc,
> +                                                                      Drawable *drawable,
> +                                                                      PipeItem *pos);
>
>  typedef struct compress_send_data_t {
>      void*    comp_buf;
> diff --git a/server/display-channel.c b/server/display-channel.c
> index 0be3e42..ba7f443 100644
> --- a/server/display-channel.c
> +++ b/server/display-channel.c
> @@ -325,6 +325,50 @@ static void streams_update_visible_region(DisplayChannel *display, Drawable *dra
>      }
>  }
>
> +static void pipes_add_drawable(DisplayChannel *display, Drawable *drawable)
> +{
> +    DisplayChannelClient *dcc;
> +    RingItem *dcc_ring_item, *next;
> +
> +    spice_warn_if(!ring_is_empty(&drawable->pipes));
> +    FOREACH_DCC(display, dcc_ring_item, next, dcc) {
> +        dcc_prepend_drawable(dcc, drawable);
> +    }
> +}
> +
> +static void pipes_add_drawable_after(DisplayChannel *display,
> +                                     Drawable *drawable, Drawable *pos_after)
> +{
> +    DrawablePipeItem *dpi_pos_after;
> +    RingItem *dpi_link, *dpi_next;
> +    DisplayChannelClient *dcc;
> +    int num_other_linked = 0;
> +
> +    DRAWABLE_FOREACH_DPI_SAFE(pos_after, dpi_link, dpi_next, dpi_pos_after) {
> +        num_other_linked++;
> +        dcc_add_drawable_after(dpi_pos_after->dcc, drawable, &dpi_pos_after->dpi_pipe_item);
> +    }
> +    if (num_other_linked == 0) {
> +        pipes_add_drawable(display, drawable);
> +        return;
> +    }
> +    if (num_other_linked != display->common.base.clients_num) {
> +        RingItem *item, *next;
> +        spice_debug("TODO: not O(n^2)");
> +        FOREACH_DCC(display, item, next, dcc) {
> +            int sent = 0;
> +            DRAWABLE_FOREACH_DPI_SAFE(pos_after, dpi_link, dpi_next, dpi_pos_after) {
> +                if (dpi_pos_after->dcc == dcc) {
> +                    sent = 1;
> +                    break;
> +                }
> +            }
> +            if (!sent) {
> +                dcc_prepend_drawable(dcc, drawable);
> +            }
> +        }
> +    }
> +}
>
>  static void current_add_drawable(DisplayChannel *display,
>                                   Drawable *drawable, RingItem *pos)
> @@ -366,9 +410,9 @@ static int current_add_equal(DisplayChannel *display, DrawItem *item, TreeItem *
>          other_drawable->refs++;
>          current_remove_drawable(display, other_drawable);
>          if (add_after) {
> -            red_pipes_add_drawable_after(display, drawable, other_drawable);
> +            pipes_add_drawable_after(display, drawable, other_drawable);
>          } else {
> -            red_pipes_add_drawable(display, drawable);
> +            pipes_add_drawable(display, drawable);
>          }
>          red_pipes_remove_drawable(other_drawable);
>          display_channel_drawable_unref(display, other_drawable);
> @@ -396,7 +440,7 @@ static int current_add_equal(DisplayChannel *display, DrawItem *item, TreeItem *
>                                          common.base.channel_link);
>                  dpi = SPICE_CONTAINEROF(dpi_ring_item, DrawablePipeItem, base);
>                  while (worker_ring_item && (!dpi || dcc != dpi->dcc)) {
> -                    dcc_add_drawable(dcc, drawable);
> +                    dcc_prepend_drawable(dcc, drawable);
>                      worker_ring_item = ring_next(&RED_CHANNEL(display)->clients,
>                                                   worker_ring_item);
>                      dcc = SPICE_CONTAINEROF(worker_ring_item, DisplayChannelClient,
> @@ -423,7 +467,7 @@ static int current_add_equal(DisplayChannel *display, DrawItem *item, TreeItem *
>              current_add_drawable(display, drawable, &other->siblings_link);
>              red_pipes_remove_drawable(other_drawable);
>              current_remove_drawable(display, other_drawable);
> -            red_pipes_add_drawable(display, drawable);
> +            pipes_add_drawable(display, drawable);
>              return TRUE;
>          }
>          break;
> @@ -788,25 +832,26 @@ void display_channel_print_stats(DisplayChannel *display)
>  #endif
>  }
>
> -int display_channel_add_drawable(DisplayChannel *display, Drawable *drawable)
> +void display_channel_add_drawable(DisplayChannel *display, Drawable *drawable)
>  {
> -    int ret = FALSE, surface_id = drawable->surface_id;
> +    int success = FALSE, surface_id = drawable->surface_id;
>      RedDrawable *red_drawable = drawable->red_drawable;
>      Ring *ring = &display->surfaces[surface_id].current;
>
>      if (has_shadow(red_drawable)) {
> -        ret = current_add_with_shadow(display, ring, drawable);
> +        success = current_add_with_shadow(display, ring, drawable);
>      } else {
>          drawable->streamable = drawable_can_stream(display, drawable);
> -        ret = current_add(display, ring, drawable);
> +        success = current_add(display, ring, drawable);
>      }
>
> +    if (success)
> +        pipes_add_drawable(display, drawable);
> +
>  #ifdef RED_WORKER_STAT
>      if ((++display->add_count % 100) == 0)
>          display_channel_print_stats(display);
>  #endif
> -
> -    return ret;
>  }
>
>  int display_channel_wait_for_migrate_data(DisplayChannel *display)
> diff --git a/server/display-channel.h b/server/display-channel.h
> index 29c1151..e26a1d8 100644
> --- a/server/display-channel.h
> +++ b/server/display-channel.h
> @@ -109,19 +109,6 @@ enum {
>      PIPE_ITEM_TYPE_STREAM_ACTIVATE_REPORT,
>  };
>
> -typedef struct DrawablePipeItem {
> -    RingItem base;  /* link for a list of pipe items held by Drawable */
> -    PipeItem dpi_pipe_item; /* link for the client's pipe itself */
> -    Drawable *drawable;
> -    DisplayChannelClient *dcc;
> -    uint8_t refs;
> -} DrawablePipeItem;
> -
> -DrawablePipeItem*          drawable_pipe_item_new                    (DisplayChannelClient *dcc,
> -                                                                      Drawable *drawable);
> -void                       drawable_pipe_item_unref                  (DrawablePipeItem *dpi);
> -DrawablePipeItem*          drawable_pipe_item_ref                    (DrawablePipeItem *dpi);
> -
>  typedef struct MonitorsConfig {
>      int refs;
>      int count;
> @@ -271,7 +258,7 @@ void                       display_channel_surface_unref             (DisplayCha
>                                                                        uint32_t surface_id);
>  bool                       display_channel_surface_has_canvas        (DisplayChannel *display,
>                                                                        uint32_t surface_id);
> -int                        display_channel_add_drawable              (DisplayChannel *display,
> +void                       display_channel_add_drawable              (DisplayChannel *display,
>                                                                        Drawable *drawable);
>  void                       display_channel_current_flush             (DisplayChannel *display,
>                                                                        int surface_id);
> @@ -395,12 +382,8 @@ static inline void region_add_clip_rects(QRegion *rgn, SpiceClipRects *data)
>      }
>  }
>
> -void red_pipes_add_drawable(DisplayChannel *display, Drawable *drawable);
>  void current_remove_drawable(DisplayChannel *display, Drawable *item);
> -void red_pipes_add_drawable_after(DisplayChannel *display,
> -                                  Drawable *drawable, Drawable *pos_after);
>  void red_pipes_remove_drawable(Drawable *drawable);
> -void dcc_add_drawable(DisplayChannelClient *dcc, Drawable *drawable);
>  void current_remove(DisplayChannel *display, TreeItem *item);
>  void detach_streams_behind(DisplayChannel *display, QRegion *region, Drawable *drawable);
>  void drawable_draw(DisplayChannel *display, Drawable *item);
> diff --git a/server/red_worker.c b/server/red_worker.c
> index e655876..443e00a 100644
> --- a/server/red_worker.c
> +++ b/server/red_worker.c
> @@ -203,44 +203,6 @@ void attach_stream(DisplayChannel *display, Drawable *drawable, Stream *stream)
>      }
>  }
>
> -/* fixme: move to display channel */
> -DrawablePipeItem *drawable_pipe_item_new(DisplayChannelClient *dcc,
> -                                         Drawable *drawable)
> -{
> -    DrawablePipeItem *dpi;
> -
> -    dpi = spice_malloc0(sizeof(*dpi));
> -    dpi->drawable = drawable;
> -    dpi->dcc = dcc;
> -    ring_item_init(&dpi->base);
> -    ring_add(&drawable->pipes, &dpi->base);
> -    red_channel_pipe_item_init(RED_CHANNEL_CLIENT(dcc)->channel,
> -                               &dpi->dpi_pipe_item, PIPE_ITEM_TYPE_DRAW);
> -    dpi->refs++;
> -    drawable->refs++;
> -    return dpi;
> -}
> -
> -DrawablePipeItem *drawable_pipe_item_ref(DrawablePipeItem *dpi)
> -{
> -    dpi->refs++;
> -    return dpi;
> -}
> -
> -void drawable_pipe_item_unref(DrawablePipeItem *dpi)
> -{
> -    DisplayChannel *display = DCC_TO_DC(dpi->dcc);
> -
> -    if (--dpi->refs != 0) {
> -        return;
> -    }
> -
> -    spice_warn_if_fail(!ring_item_is_linked(&dpi->dpi_pipe_item.link));
> -    spice_warn_if_fail(!ring_item_is_linked(&dpi->base));
> -    display_channel_drawable_unref(display, dpi->drawable);
> -    free(dpi);
> -}
> -
>  QXLInstance* red_worker_get_qxl(RedWorker *worker)
>  {
>      spice_return_val_if_fail(worker != NULL, NULL);
> @@ -292,35 +254,6 @@ static inline int validate_surface(DisplayChannel *display, uint32_t surface_id)
>      return 1;
>  }
>
> -static inline void red_handle_drawable_surfaces_client_synced(
> -                        DisplayChannelClient *dcc, Drawable *drawable)
> -{
> -    DisplayChannel *display = DCC_TO_DC(dcc);
> -    int x;
> -
> -    for (x = 0; x < 3; ++x) {
> -        int surface_id;
> -
> -        surface_id = drawable->surface_deps[x];
> -        if (surface_id != -1) {
> -            if (dcc->surface_client_created[surface_id] == TRUE) {
> -                continue;
> -            }
> -            dcc_create_surface(dcc, surface_id);
> -            display_channel_current_flush(display, surface_id);
> -            dcc_push_surface_image(dcc, surface_id);
> -        }
> -    }
> -
> -    if (dcc->surface_client_created[drawable->surface_id] == TRUE) {
> -        return;
> -    }
> -
> -    dcc_create_surface(dcc, drawable->surface_id);
> -    display_channel_current_flush(display, drawable->surface_id);
> -    dcc_push_surface_image(dcc, drawable->surface_id);
> -}
> -
>  static int display_is_connected(RedWorker *worker)
>  {
>      return (worker->display_channel && red_channel_is_connected(
> @@ -333,76 +266,6 @@ static int cursor_is_connected(RedWorker *worker)
>          red_channel_is_connected(RED_CHANNEL(worker->cursor_channel));
>  }
>
> -void dcc_add_drawable(DisplayChannelClient *dcc, Drawable *drawable)
> -{
> -    DrawablePipeItem *dpi;
> -
> -    red_handle_drawable_surfaces_client_synced(dcc, drawable);
> -    dpi = drawable_pipe_item_new(dcc, drawable);
> -    red_channel_client_pipe_add(RED_CHANNEL_CLIENT(dcc), &dpi->dpi_pipe_item);
> -}
> -
> -void red_pipes_add_drawable(DisplayChannel *display, Drawable *drawable)
> -{
> -    DisplayChannelClient *dcc;
> -    RingItem *dcc_ring_item, *next;
> -
> -    spice_warn_if(!ring_is_empty(&drawable->pipes));
> -    FOREACH_DCC(display, dcc_ring_item, next, dcc) {
> -        dcc_add_drawable(dcc, drawable);
> -    }
> -}
> -
> -static void dcc_add_drawable_to_tail(DisplayChannelClient *dcc, Drawable *drawable)
> -{
> -    DrawablePipeItem *dpi;
> -
> -    if (!dcc) {
> -        return;
> -    }
> -    red_handle_drawable_surfaces_client_synced(dcc, drawable);
> -    dpi = drawable_pipe_item_new(dcc, drawable);
> -    red_channel_client_pipe_add_tail(RED_CHANNEL_CLIENT(dcc), &dpi->dpi_pipe_item);
> -}
> -
> -void red_pipes_add_drawable_after(DisplayChannel *display,
> -                                  Drawable *drawable, Drawable *pos_after)
> -{
> -    DrawablePipeItem *dpi, *dpi_pos_after;
> -    RingItem *dpi_link, *dpi_next;
> -    DisplayChannelClient *dcc;
> -    int num_other_linked = 0;
> -
> -    DRAWABLE_FOREACH_DPI_SAFE(pos_after, dpi_link, dpi_next, dpi_pos_after) {
> -        num_other_linked++;
> -        dcc = dpi_pos_after->dcc;
> -        red_handle_drawable_surfaces_client_synced(dcc, drawable);
> -        dpi = drawable_pipe_item_new(dcc, drawable);
> -        red_channel_client_pipe_add_after(RED_CHANNEL_CLIENT(dcc), &dpi->dpi_pipe_item,
> -                                          &dpi_pos_after->dpi_pipe_item);
> -    }
> -    if (num_other_linked == 0) {
> -        red_pipes_add_drawable(display, drawable);
> -        return;
> -    }
> -    if (num_other_linked != display->common.base.clients_num) {
> -        RingItem *item, *next;
> -        spice_debug("TODO: not O(n^2)");
> -        FOREACH_DCC(display, item, next, dcc) {
> -            int sent = 0;
> -            DRAWABLE_FOREACH_DPI_SAFE(pos_after, dpi_link, dpi_next, dpi_pos_after) {
> -                if (dpi_pos_after->dcc == dcc) {
> -                    sent = 1;
> -                    break;
> -                }
> -            }
> -            if (!sent) {
> -                dcc_add_drawable(dcc, drawable);
> -            }
> -        }
> -    }
> -}
> -
>  static PipeItem *dcc_get_tail(DisplayChannelClient *dcc)
>  {
>      return (PipeItem*)ring_get_tail(&RED_CHANNEL_CLIENT(dcc)->pipe);
> @@ -1179,9 +1042,8 @@ static inline void red_process_draw(RedWorker *worker, RedDrawable *red_drawable
>          goto cleanup;
>      }
>
> -    if (display_channel_add_drawable(worker->display_channel, drawable)) {
> -        red_pipes_add_drawable(worker->display_channel, drawable);
> -    }
> +    display_channel_add_drawable(worker->display_channel, drawable);
> +
>  cleanup:
>      display_channel_drawable_unref(display, drawable);
>  }
> @@ -2338,7 +2200,7 @@ static void red_add_lossless_drawable_dependencies(RedChannelClient *rcc,
>
>      if (!sync_rendered) {
>          // pushing the pipe item back to the pipe
> -        dcc_add_drawable_to_tail(dcc, item);
> +        dcc_append_drawable(dcc, item);
>          // the surfaces areas will be sent as DRAW_COPY commands, that
>          // will be executed before the current drawable
>          for (i = 0; i < num_deps; i++) {
> --
> 2.4.3
>
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/spice-devel

Many thanks for attending my personal preference :-)
Acked-by: Fabiano Fidêncio <fidencio at redhat.com>


More information about the Spice-devel mailing list