>
> On Fri, Nov 20, 2015 at 12:17 PM, Frediano Ziglio <fziglio@xxxxxxxxxx> wrote:
> > From: Marc-André Lureau <marcandre.lureau@xxxxxxxxx>
> >
> > ---
> > server/cursor-channel.c | 4 ++++
> > server/red_worker.c | 60
> > +++++++++++++++++++++----------------------------
> > 2 files changed, 30 insertions(+), 34 deletions(-)
> >
> > diff --git a/server/cursor-channel.c b/server/cursor-channel.c
> > index aafc807..5eb2647 100644
> > --- a/server/cursor-channel.c
> > +++ b/server/cursor-channel.c
> > @@ -484,6 +484,10 @@ CursorChannelClient*
> > cursor_channel_client_new(CursorChannel *cursor, RedClient
> > ring_init(&ccc->cursor_cache_lru);
> > ccc->cursor_cache_available = CLIENT_CURSOR_CACHE_SIZE;
> >
> > + RedChannelClient *rcc = RED_CHANNEL_CLIENT(ccc);
> > + red_channel_client_ack_zero_messages_window(rcc);
> > + red_channel_client_push_set_ack(rcc);
> > +
> > return ccc;
> > }
> >
Style opinion: a _new function should just initialize the object,
not also initialize a connection sending messages.
I don't like this part of the patch
> > diff --git a/server/red_worker.c b/server/red_worker.c
> > index b8dfbb9..377eb36 100644
> > --- a/server/red_worker.c
> > +++ b/server/red_worker.c
> > @@ -5420,34 +5420,6 @@ static void handle_new_display_channel(RedWorker
> > *worker, RedClient *client, Red
> > dcc_start(dcc);
> > }
> >
> > -static void red_connect_cursor(RedWorker *worker, RedClient *client,
> > RedsStream *stream,
> > - int migrate,
> > - uint32_t *common_caps, int num_common_caps,
> > - uint32_t *caps, int num_caps)
> > -{
> > - CursorChannel *channel;
> > - CursorChannelClient *ccc;
> > -
> > - spice_return_if_fail(worker->cursor_channel != NULL);
> > -
> > - channel = worker->cursor_channel;
> > - spice_info("add cursor channel client");
> > - ccc = cursor_channel_client_new(channel, client, stream,
> > - migrate,
> > - common_caps, num_common_caps,
> > - caps, num_caps);
> > - spice_return_if_fail(ccc != NULL);
> > -
> > - RedChannelClient *rcc = RED_CHANNEL_CLIENT(ccc);
> > - red_channel_client_ack_zero_messages_window(rcc);
> > - red_channel_client_push_set_ack(rcc);
> > -
> > - // TODO: why do we check for context.canvas? defer this to after
> > display cc is connected
> > - // and test it's canvas? this is just a test to see if there is an
> > active renderer?
> > - if (display_channel_surface_has_canvas(worker->display_channel, 0))
> > - cursor_channel_init(channel, ccc);
> > -}
> > -
> > static void surface_dirty_region_to_rects(RedSurface *surface,
> > QXLRect *qxl_dirty_rects,
> > uint32_t num_dirty_rects)
> > @@ -5955,19 +5927,39 @@ static void handle_dev_monitors_config_async(void
> > *opaque, void *payload)
> > red_worker_push_monitors_config(worker);
> > }
> >
> > +static void cursor_connect(RedWorker *worker, RedClient *client,
> > RedsStream *stream,
> > + int migrate,
> > + uint32_t *common_caps, int num_common_caps,
> > + uint32_t *caps, int num_caps)
> > +{
> > + CursorChannel *channel = worker->cursor_channel;
> > + CursorChannelClient *ccc;
> > +
> > + spice_return_if_fail(worker->cursor_channel != NULL);
> > +
> > + spice_info("add cursor channel client");
> > + ccc = cursor_channel_client_new(channel, client, stream,
> > + migrate,
> > + common_caps, num_common_caps,
> > + caps, num_caps);
> > +
> > + // TODO: why do we check for context.canvas? defer this to after
> > display cc is connected
> > + // and test it's canvas? this is just a test to see if there is an
> > active renderer?
> > + if (display_channel_surface_has_canvas(worker->display_channel, 0))
> > + cursor_channel_init(channel, ccc);
>
> It's not exactly related to this patch, but do we already have an
> answer for this TODO? :-)
>
Usually in the code canvas is checked to check surface validity.
Surfaces are allocated in a static array where you can have used and not used.
I don't understand why the check is not done using reference counting.
> > +}
> > +
> > /* TODO: special, perhaps use another dispatcher? */
> > static void handle_dev_cursor_connect(void *opaque, void *payload)
> > {
> > RedWorkerMessageCursorConnect *msg = payload;
> > RedWorker *worker = opaque;
> > - RedsStream *stream = msg->stream;
> > - RedClient *client = msg->client;
> > - int migration = msg->migration;
> >
> > spice_info("cursor connect");
> > - red_connect_cursor(worker, client, stream, migration,
> > - msg->common_caps, msg->num_common_caps,
> > - msg->caps, msg->num_caps);
> > + cursor_connect(worker,
> > + msg->client, msg->stream, msg->migration,
> > + msg->common_caps, msg->num_common_caps,
> > + msg->caps, msg->num_caps);
> > free(msg->caps);
> > free(msg->common_caps);
> > }
> > --
> > 2.4.3
> >
> > _______________________________________________
> > Spice-devel mailing list
> > Spice-devel@xxxxxxxxxxxxxxxxxxxxx
> > http://lists.freedesktop.org/mailman/listinfo/spice-devel
>
>
> ACK!
> --
> Fabiano Fidêncio
>
I'll try to post an update.
Frediano
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/spice-devel