[Spice-devel] [PATCH 04/18] worker: move dcc_add_drawable*
Fabiano Fidêncio
fabiano at fidencio.org
Fri Nov 20 06:28:02 PST 2015
On Fri, Nov 20, 2015 at 12:17 PM, Frediano Ziglio <fziglio at redhat.com> wrote:
> From: Marc-André Lureau <marcandre.lureau at gmail.com>
>
> ---
> server/dcc.c | 82 +++++++++++++++++++++++++++
> server/dcc.h | 17 ++++++
> server/display-channel.c | 65 +++++++++++++++++----
> server/display-channel.h | 19 +------
> server/red_worker.c | 144 +----------------------------------------------
> 5 files changed, 158 insertions(+), 169 deletions(-)
>
> diff --git a/server/dcc.c b/server/dcc.c
> index 8cc2c0a..a17fa7d 100644
> --- a/server/dcc.c
> +++ b/server/dcc.c
> @@ -93,6 +93,88 @@ 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_return_if_fail(!ring_item_is_linked(&dpi->dpi_pipe_item.link));
> + spice_return_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_add_drawable(DisplayChannelClient *dcc, Drawable *drawable, bool to_tail)
> +{
> + DrawablePipeItem *dpi = drawable_pipe_item_new(dcc, drawable);
> +
> + add_drawable_surface_images(dcc, drawable);
> + if (to_tail)
> + red_channel_client_pipe_add_tail(RED_CHANNEL_CLIENT(dcc), &dpi->dpi_pipe_item);
> + else
> + red_channel_client_pipe_add(RED_CHANNEL_CLIENT(dcc), &dpi->dpi_pipe_item);
> +}
Not related to this patch but would be _way_ more clear if we have
dcc_append_drawable() and dcc_prepend_drawable().
> +
> +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 c62c3c9..f7c01a1 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,
> @@ -169,6 +180,12 @@ 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_add_drawable (DisplayChannelClient *dcc,
> + Drawable *drawable,
> + bool to_tail);
> +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 d455a0d..ecf6f7d 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_add_drawable(dcc, drawable, FALSE);
> + }
> +}
> +
> +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_add_drawable(dcc, drawable, FALSE);
> + }
> + }
> + }
> +}
>
> 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_add_drawable(dcc, drawable, FALSE);
> 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;
> @@ -787,25 +831,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 bad61d1..233c391 100644
> --- a/server/display-channel.h
> +++ b/server/display-channel.h
> @@ -113,19 +113,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;
> @@ -270,7 +257,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);
> @@ -394,12 +381,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);
>
> diff --git a/server/red_worker.c b/server/red_worker.c
> index f6dfe28..b8dfbb9 100644
> --- a/server/red_worker.c
> +++ b/server/red_worker.c
> @@ -191,44 +191,6 @@ static void red_create_surface(DisplayChannel *display, uint32_t surface_id, uin
> uint32_t height, int32_t stride, uint32_t format,
> void *line_0, int data_is_valid, int send_client);
>
> -/* 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_return_if_fail(!ring_item_is_linked(&dpi->dpi_pipe_item.link));
> - spice_return_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);
> @@ -252,35 +214,6 @@ static inline int validate_surface(DisplayChannel *display, uint32_t surface_id)
> }
>
>
> -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(
> @@ -293,76 +226,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 inline PipeItem *red_pipe_get_tail(DisplayChannelClient *dcc)
> {
> if (!dcc) {
> @@ -1226,9 +1089,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);
> }
> @@ -2509,7 +2371,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_add_drawable(dcc, item, TRUE);
> // 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
Patch seems good, ACK!
--
Fabiano Fidêncio
More information about the Spice-devel
mailing list