Yup, had a very similar patch locally, which I never tested/sent /o\
Should we change the naming of the methods? (red_qxl_xxx)
Acked-by: Christophe Fergeau <cfergeau@xxxxxxxxxx>
On Tue, Aug 29, 2017 at 05:28:08PM -0500, Jonathon Jongsma wrote:
> Since we can retrieve the dispatcher from the channel's qxl instance, we
> can move the client callbacks into the channel implementation rather
> than having it within the qxl implementation.
>
> Signed-off-by: Jonathon Jongsma <jjongsma@xxxxxxxxxx>
> ---
> server/cursor-channel.c | 75 +++++++++++++++++++++++++
> server/display-channel.c | 73 ++++++++++++++++++++++++
> server/red-qxl.c | 141 +----------------------------------------------
> server/red-worker.c | 6 +-
> server/red-worker.h | 4 +-
> 5 files changed, 151 insertions(+), 148 deletions(-)
>
> diff --git a/server/cursor-channel.c b/server/cursor-channel.c
> index 63d6886d9..02a9fe3a1 100644
> --- a/server/cursor-channel.c
> +++ b/server/cursor-channel.c
> @@ -436,12 +436,87 @@ cursor_channel_finalize(GObject *object)
> G_OBJECT_CLASS(cursor_channel_parent_class)->finalize(object);
> }
>
> +static void red_qxl_set_cursor_peer(RedChannel *channel, RedClient *client, RedsStream *stream,
> + int migration,
> + RedChannelCapabilities *caps)
> +{
> + RedWorkerMessageCursorConnect payload = {0,};
> + QXLInstance *qxl = common_graphics_channel_get_qxl(COMMON_GRAPHICS_CHANNEL(channel));
> + Dispatcher *dispatcher = red_qxl_get_dispatcher(qxl);
> + spice_printerr("");
> + payload.client = client;
> + payload.stream = stream;
> + payload.migration = migration;
> + red_channel_capabilities_init(&payload.caps, caps);
> +
> + dispatcher_send_message(dispatcher,
> + RED_WORKER_MESSAGE_CURSOR_CONNECT,
> + &payload);
> +}
> +
> +static void red_qxl_disconnect_cursor_peer(RedChannelClient *rcc)
> +{
> + RedWorkerMessageCursorDisconnect payload;
> + QXLInstance *qxl;
> + Dispatcher *dispatcher;
> + RedChannel *channel = red_channel_client_get_channel(rcc);
> +
> + if (!channel) {
> + return;
> + }
> +
> + qxl = common_graphics_channel_get_qxl(COMMON_GRAPHICS_CHANNEL(channel));
> + dispatcher = red_qxl_get_dispatcher(qxl);
> + spice_printerr("");
> + payload.rcc = rcc;
> +
> + dispatcher_send_message(dispatcher,
> + RED_WORKER_MESSAGE_CURSOR_DISCONNECT,
> + &payload);
> +}
> +
> +static void red_qxl_cursor_migrate(RedChannelClient *rcc)
> +{
> + RedWorkerMessageCursorMigrate payload;
> + QXLInstance *qxl;
> + Dispatcher *dispatcher;
> + RedChannel *channel = red_channel_client_get_channel(rcc);
> + uint32_t type, id;
> +
> + if (!channel) {
> + return;
> + }
> + g_object_get(channel, "channel-type", &type, "id", &id, NULL);
> + spice_printerr("channel type %u id %u", type, id);
> + qxl = common_graphics_channel_get_qxl(COMMON_GRAPHICS_CHANNEL(channel));
> + dispatcher = red_qxl_get_dispatcher(qxl);
> + payload.rcc = rcc;
> + dispatcher_send_message(dispatcher,
> + RED_WORKER_MESSAGE_CURSOR_MIGRATE,
> + &payload);
> +}
> +
> +static void
> +cursor_channel_constructed(GObject *object)
> +{
> + RedChannel *channel = RED_CHANNEL(object);
> + ClientCbs client_cbs = { NULL, };
> +
> + G_OBJECT_CLASS(cursor_channel_parent_class)->constructed(object);
> +
> + client_cbs.connect = red_qxl_set_cursor_peer;
> + client_cbs.disconnect = red_qxl_disconnect_cursor_peer;
> + client_cbs.migrate = red_qxl_cursor_migrate;
> + red_channel_register_client_cbs(channel, &client_cbs, NULL);
> +}
> +
> static void
> cursor_channel_class_init(CursorChannelClass *klass)
> {
> GObjectClass *object_class = G_OBJECT_CLASS(klass);
> RedChannelClass *channel_class = RED_CHANNEL_CLASS(klass);
>
> + object_class->constructed = cursor_channel_constructed;
> object_class->finalize = cursor_channel_finalize;
>
> channel_class->parser = spice_get_client_channel_parser(SPICE_CHANNEL_CURSOR, NULL);
> diff --git a/server/display-channel.c b/server/display-channel.c
> index 792fbd25b..cccf1dea9 100644
> --- a/server/display-channel.c
> +++ b/server/display-channel.c
> @@ -2291,11 +2291,78 @@ display_channel_init(DisplayChannel *self)
> self->priv->image_surfaces.ops = &image_surfaces_ops;
> }
>
> +static void red_qxl_set_display_peer(RedChannel *channel, RedClient *client,
> + RedsStream *stream, int migration,
> + RedChannelCapabilities *caps)
> +{
> + RedWorkerMessageDisplayConnect payload = {0,};
> + QXLInstance *qxl = common_graphics_channel_get_qxl(COMMON_GRAPHICS_CHANNEL(channel));
> + Dispatcher *dispatcher = red_qxl_get_dispatcher(qxl);
> +
> + spice_debug("%s", "");
> + payload.client = client;
> + payload.stream = stream;
> + payload.migration = migration;
> + red_channel_capabilities_init(&payload.caps, caps);
> +
> + dispatcher_send_message(dispatcher,
> + RED_WORKER_MESSAGE_DISPLAY_CONNECT,
> + &payload);
> +}
> +
> +static void red_qxl_disconnect_display_peer(RedChannelClient *rcc)
> +{
> + RedWorkerMessageDisplayDisconnect payload;
> + QXLInstance *qxl;
> + Dispatcher *dispatcher;
> + RedChannel *channel = red_channel_client_get_channel(rcc);
> +
> + if (!channel) {
> + return;
> + }
> +
> + qxl = common_graphics_channel_get_qxl(COMMON_GRAPHICS_CHANNEL(channel));
> + dispatcher = red_qxl_get_dispatcher(qxl);
> +
> + spice_printerr("");
> + payload.rcc = rcc;
> +
> + // TODO: we turned it to be sync, due to client_destroy . Should we support async? - for this we will need ref count
> + // for channels
> + dispatcher_send_message(dispatcher,
> + RED_WORKER_MESSAGE_DISPLAY_DISCONNECT,
> + &payload);
> +}
> +
> +static void red_qxl_display_migrate(RedChannelClient *rcc)
> +{
> + RedWorkerMessageDisplayMigrate payload;
> + QXLInstance *qxl;
> + Dispatcher *dispatcher;
> + RedChannel *channel = red_channel_client_get_channel(rcc);
> + uint32_t type, id;
> +
> + if (!channel) {
> + return;
> + }
> + g_object_get(channel, "channel-type", &type, "id", &id,
> + NULL);
> +
> + qxl = common_graphics_channel_get_qxl(COMMON_GRAPHICS_CHANNEL(channel));
> + dispatcher = red_qxl_get_dispatcher(qxl);
> + spice_printerr("channel type %u id %u", type, id);
> + payload.rcc = rcc;
> + dispatcher_send_message(dispatcher,
> + RED_WORKER_MESSAGE_DISPLAY_MIGRATE,
> + &payload);
> +}
> +
> static void
> display_channel_constructed(GObject *object)
> {
> DisplayChannel *self = DISPLAY_CHANNEL(object);
> RedChannel *channel = RED_CHANNEL(self);
> + ClientCbs client_cbs = { NULL, };
>
> G_OBJECT_CLASS(display_channel_parent_class)->constructed(object);
>
> @@ -2322,6 +2389,12 @@ display_channel_constructed(GObject *object)
> red_channel_set_cap(channel, SPICE_DISPLAY_CAP_PREF_COMPRESSION);
> red_channel_set_cap(channel, SPICE_DISPLAY_CAP_PREF_VIDEO_CODEC_TYPE);
> red_channel_set_cap(channel, SPICE_DISPLAY_CAP_STREAM_REPORT);
> +
> + client_cbs.connect = red_qxl_set_display_peer;
> + client_cbs.disconnect = red_qxl_disconnect_display_peer;
> + client_cbs.migrate = red_qxl_display_migrate;
> +
> + red_channel_register_client_cbs(channel, &client_cbs, NULL);
> }
>
> void display_channel_process_surface_cmd(DisplayChannel *display,
> diff --git a/server/red-qxl.c b/server/red-qxl.c
> index 4a8f523b7..03a538e1d 100644
> --- a/server/red-qxl.c
> +++ b/server/red-qxl.c
> @@ -37,7 +37,6 @@
> #include "dispatcher.h"
> #include "red-parse-qxl.h"
> #include "red-channel-client.h"
> -#include "common-graphics-channel.h"
>
> #include "red-qxl.h"
>
> @@ -75,132 +74,6 @@ int red_qxl_check_qxl_version(QXLInstance *qxl, int major, int minor)
> ((qxl_major == major) && (qxl_minor >= minor)));
> }
>
> -static void red_qxl_set_display_peer(RedChannel *channel, RedClient *client,
> - RedsStream *stream, int migration,
> - RedChannelCapabilities *caps)
> -{
> - RedWorkerMessageDisplayConnect payload = {0,};
> - QXLInstance *qxl = common_graphics_channel_get_qxl(COMMON_GRAPHICS_CHANNEL(channel));
> - Dispatcher *dispatcher = red_qxl_get_dispatcher(qxl);
> -
> - spice_debug("%s", "");
> - payload.client = client;
> - payload.stream = stream;
> - payload.migration = migration;
> - red_channel_capabilities_init(&payload.caps, caps);
> -
> - dispatcher_send_message(dispatcher,
> - RED_WORKER_MESSAGE_DISPLAY_CONNECT,
> - &payload);
> -}
> -
> -static void red_qxl_disconnect_display_peer(RedChannelClient *rcc)
> -{
> - RedWorkerMessageDisplayDisconnect payload;
> - QXLInstance *qxl;
> - Dispatcher *dispatcher;
> - RedChannel *channel = red_channel_client_get_channel(rcc);
> -
> - if (!channel) {
> - return;
> - }
> -
> - qxl = common_graphics_channel_get_qxl(COMMON_GRAPHICS_CHANNEL(channel));
> - dispatcher = red_qxl_get_dispatcher(qxl);
> -
> - spice_printerr("");
> - payload.rcc = rcc;
> -
> - // TODO: we turned it to be sync, due to client_destroy . Should we support async? - for this we will need ref count
> - // for channels
> - dispatcher_send_message(dispatcher,
> - RED_WORKER_MESSAGE_DISPLAY_DISCONNECT,
> - &payload);
> -}
> -
> -static void red_qxl_display_migrate(RedChannelClient *rcc)
> -{
> - RedWorkerMessageDisplayMigrate payload;
> - QXLInstance *qxl;
> - Dispatcher *dispatcher;
> - RedChannel *channel = red_channel_client_get_channel(rcc);
> - uint32_t type, id;
> -
> - if (!channel) {
> - return;
> - }
> - g_object_get(channel, "channel-type", &type, "id", &id,
> - NULL);
> -
> - qxl = common_graphics_channel_get_qxl(COMMON_GRAPHICS_CHANNEL(channel));
> - dispatcher = red_qxl_get_dispatcher(qxl);
> - spice_printerr("channel type %u id %u", type, id);
> - payload.rcc = rcc;
> - dispatcher_send_message(dispatcher,
> - RED_WORKER_MESSAGE_DISPLAY_MIGRATE,
> - &payload);
> -}
> -
> -static void red_qxl_set_cursor_peer(RedChannel *channel, RedClient *client, RedsStream *stream,
> - int migration,
> - RedChannelCapabilities *caps)
> -{
> - RedWorkerMessageCursorConnect payload = {0,};
> - QXLInstance *qxl = common_graphics_channel_get_qxl(COMMON_GRAPHICS_CHANNEL(channel));
> - Dispatcher *dispatcher = red_qxl_get_dispatcher(qxl);
> - spice_printerr("");
> - payload.client = client;
> - payload.stream = stream;
> - payload.migration = migration;
> - red_channel_capabilities_init(&payload.caps, caps);
> -
> - dispatcher_send_message(dispatcher,
> - RED_WORKER_MESSAGE_CURSOR_CONNECT,
> - &payload);
> -}
> -
> -static void red_qxl_disconnect_cursor_peer(RedChannelClient *rcc)
> -{
> - RedWorkerMessageCursorDisconnect payload;
> - QXLInstance *qxl;
> - Dispatcher *dispatcher;
> - RedChannel *channel = red_channel_client_get_channel(rcc);
> -
> - if (!channel) {
> - return;
> - }
> -
> - qxl = common_graphics_channel_get_qxl(COMMON_GRAPHICS_CHANNEL(channel));
> - dispatcher = red_qxl_get_dispatcher(qxl);
> - spice_printerr("");
> - payload.rcc = rcc;
> -
> - dispatcher_send_message(dispatcher,
> - RED_WORKER_MESSAGE_CURSOR_DISCONNECT,
> - &payload);
> -}
> -
> -static void red_qxl_cursor_migrate(RedChannelClient *rcc)
> -{
> - RedWorkerMessageCursorMigrate payload;
> - QXLInstance *qxl;
> - Dispatcher *dispatcher;
> - RedChannel *channel = red_channel_client_get_channel(rcc);
> - uint32_t type, id;
> -
> - if (!channel) {
> - return;
> - }
> - g_object_get(channel, "channel-type", &type, "id", &id, NULL);
> - spice_printerr("channel type %u id %u", type, id);
> - qxl = common_graphics_channel_get_qxl(COMMON_GRAPHICS_CHANNEL(channel));
> - dispatcher = red_qxl_get_dispatcher(qxl);
> - payload.rcc = rcc;
> - dispatcher_send_message(dispatcher,
> - RED_WORKER_MESSAGE_CURSOR_MIGRATE,
> - &payload);
> -}
> -
> static void red_qxl_update_area(QXLState *qxl_state, uint32_t surface_id,
> QXLRect *qxl_area, QXLRect *qxl_dirty_rects,
> uint32_t num_dirty_rects, uint32_t clear_dirty_region)
> @@ -965,8 +838,6 @@ void red_qxl_gl_draw_async_complete(QXLInstance *qxl)
> void red_qxl_init(RedsState *reds, QXLInstance *qxl)
> {
> QXLState *qxl_state;
> - ClientCbs client_cursor_cbs = { NULL, };
> - ClientCbs client_display_cbs = { NULL, };
>
> spice_return_if_fail(qxl != NULL);
>
> @@ -998,17 +869,7 @@ void red_qxl_init(RedsState *reds, QXLInstance *qxl)
> qxl_state->max_monitors = UINT_MAX;
> qxl->st = qxl_state;
>
> - // TODO: move to their respective channel files
> - client_cursor_cbs.connect = red_qxl_set_cursor_peer;
> - client_cursor_cbs.disconnect = red_qxl_disconnect_cursor_peer;
> - client_cursor_cbs.migrate = red_qxl_cursor_migrate;
> -
> - client_display_cbs.connect = red_qxl_set_display_peer;
> - client_display_cbs.disconnect = red_qxl_disconnect_display_peer;
> - client_display_cbs.migrate = red_qxl_display_migrate;
> -
> - qxl_state->worker = red_worker_new(qxl, &client_cursor_cbs,
> - &client_display_cbs);
> + qxl_state->worker = red_worker_new(qxl);
>
> red_worker_run(qxl_state->worker);
> }
> diff --git a/server/red-worker.c b/server/red-worker.c
> index c43bb1244..f1225d2e2 100644
> --- a/server/red-worker.c
> +++ b/server/red-worker.c
> @@ -1286,9 +1286,7 @@ static GSourceFuncs worker_source_funcs = {
> .dispatch = worker_source_dispatch,
> };
>
> -RedWorker* red_worker_new(QXLInstance *qxl,
> - const ClientCbs *client_cursor_cbs,
> - const ClientCbs *client_display_cbs)
> +RedWorker* red_worker_new(QXLInstance *qxl)
> {
> QXLDevInitInfo init_info;
> RedWorker *worker;
> @@ -1344,7 +1342,6 @@ RedWorker* red_worker_new(QXLInstance *qxl,
> worker->cursor_channel = cursor_channel_new(reds, qxl, &worker->core);
> channel = RED_CHANNEL(worker->cursor_channel);
> red_channel_init_stat_node(channel, &worker->stat, "cursor_channel");
> - red_channel_register_client_cbs(channel, client_cursor_cbs, NULL);
> reds_register_channel(reds, channel);
>
> // TODO: handle seemless migration. Temp, setting migrate to FALSE
> @@ -1354,7 +1351,6 @@ RedWorker* red_worker_new(QXLInstance *qxl,
> init_info.n_surfaces);
> channel = RED_CHANNEL(worker->display_channel);
> red_channel_init_stat_node(channel, &worker->stat, "display_channel");
> - red_channel_register_client_cbs(channel, client_display_cbs, NULL);
> reds_register_channel(reds, channel);
>
> return worker;
> diff --git a/server/red-worker.h b/server/red-worker.h
> index 8ec28f144..b9677a559 100644
> --- a/server/red-worker.h
> +++ b/server/red-worker.h
> @@ -25,9 +25,7 @@
>
> typedef struct RedWorker RedWorker;
>
> -RedWorker* red_worker_new(QXLInstance *qxl,
> - const ClientCbs *client_cursor_cbs,
> - const ClientCbs *client_display_cbs);
> +RedWorker* red_worker_new(QXLInstance *qxl);
> bool red_worker_run(RedWorker *worker);
> void red_worker_free(RedWorker *worker);
>
> --
> 2.13.3
>
> _______________________________________________
> Spice-devel mailing list
> Spice-devel@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/spice-devel
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel