On Fri, 2015-10-30 at 03:39 -0400, Frediano Ziglio wrote:
> >
> > From: Marc-André Lureau <marcandre.lureau@xxxxxxxxx>
> >
> > Signed-off-by: Jonathon Jongsma <jjongsma@xxxxxxxxxx>
> > ---
> > server/cursor-channel.c | 30 +++++++++++++++++-------------
> > server/red_worker.c | 7 ++++---
> > 2 files changed, 21 insertions(+), 16 deletions(-)
> >
> > diff --git a/server/cursor-channel.c b/server/cursor-channel.c
> > index ecb49b2..9935c00 100644
> > --- a/server/cursor-channel.c
> > +++ b/server/cursor-channel.c
> > @@ -232,17 +232,17 @@ static void
> > red_marshall_cursor_init(RedChannelClient
> > *rcc, SpiceMarshaller *bas
> > }
> >
> > static void cursor_marshall(RedChannelClient *rcc,
> > - SpiceMarshaller *m, CursorPipeItem
> > *cursor_pipe_item)
> > + SpiceMarshaller *m, CursorPipeItem
> > *cursor_pipe_item)
> > {
> > CursorChannel *cursor_channel = SPICE_CONTAINEROF(rcc
> > ->channel,
> > CursorChannel, common.base);
> > CursorChannelClient *ccc = RCC_TO_CCC(rcc);
> > - CursorItem *cursor = cursor_pipe_item->cursor_item;
> > + CursorItem *item = cursor_pipe_item->cursor_item;
> > PipeItem *pipe_item = &cursor_pipe_item->base;
> > RedCursorCmd *cmd;
> >
> > spice_return_if_fail(cursor_channel);
> >
> > - cmd = cursor->red_cursor;
> > + cmd = item->red_cursor;
> > switch (cmd->type) {
> > case QXL_CURSOR_MOVE:
> > {
> > @@ -261,7 +261,7 @@ static void cursor_marshall(RedChannelClient
> > *rcc,
> > cursor_set.position = cmd->u.set.position;
> > cursor_set.visible = cursor_channel->cursor_visible;
> >
> > - cursor_fill(ccc, &cursor_set.cursor, cursor, &info);
> > + cursor_fill(ccc, &cursor_set.cursor, item, &info);
> > spice_marshall_msg_cursor_set(m, &cursor_set);
> > add_buf_from_info(m, &info);
> > break;
> > @@ -285,7 +285,7 @@ static void cursor_marshall(RedChannelClient
> > *rcc,
> > }
> >
> > static inline void red_marshall_inval(RedChannelClient *rcc,
> > - SpiceMarshaller *base_marshaller, CacheItem *cach_item)
> > + SpiceMarshaller
> > *base_marshaller,
> > CacheItem *cach_item)
> > {
> > SpiceMsgDisplayInvalOne inval_one;
> >
> > @@ -446,10 +446,11 @@ void cursor_channel_process_cmd(CursorChannel
> > *cursor,
> > RedCursorCmd *cursor_cmd,
> > return;
> > }
> >
> > - if (red_channel_is_connected(&cursor->common.base) &&
> > (cursor->mouse_mode == SPICE_MOUSE_MODE_SERVER ||
> > - cursor_cmd->type !=
> > QXL_CURSOR_MOVE ||
> > cursor_show)) {
> > - red_channel_pipes_new_add(&cursor->common.base,
> > new_cursor_pipe_item,
> > - (void*)cursor_item);
> > + if (cursor->mouse_mode == SPICE_MOUSE_MODE_SERVER
> > + || cursor_cmd->type != QXL_CURSOR_MOVE
> > + || cursor_show) {
> > + red_channel_pipes_new_add(&cursor->common.base,
> > + new_cursor_pipe_item,
> > cursor_item);
> > }
>
> Why silently remove the red_channel_is_connected check?
Yes, I had the same question and was going to comment on it as well.
Marc-Andre, if you're reading this, do you remember?
>
> >
> > cursor_item_unref(cursor_item);
> > @@ -457,21 +458,24 @@ void cursor_channel_process_cmd(CursorChannel
> > *cursor,
> > RedCursorCmd *cursor_cmd,
> >
> > void cursor_channel_reset(CursorChannel *cursor)
> > {
> > + RedChannel *channel = (RedChannel *)cursor;
> > +
>
> I would prefer channel = &cursor->common.base.
>
> > + spice_return_if_fail(cursor);
> > +
> > cursor_set_item(cursor, NULL);
> > cursor->cursor_visible = TRUE;
> > cursor->cursor_position.x = cursor->cursor_position.y = 0;
> > cursor->cursor_trail_length = cursor->cursor_trail_frequency =
> > 0;
> >
> > - if (red_channel_is_connected(&cursor->common.base)) {
> > - red_channel_pipes_add_type(&cursor->common.base,
> > -
> > PIPE_ITEM_TYPE_INVAL_CURSOR_CACHE);
> > + if (red_channel_is_connected(channel)) {
> > + red_channel_pipes_add_type(&cursor->common.base,
>
> change both to use channel or none here.
>
> > PIPE_ITEM_TYPE_INVAL_CURSOR_CACHE);
> > if (!cursor->common.during_target_migrate) {
> > red_pipes_add_verb(&cursor->common.base,
> > SPICE_MSG_CURSOR_RESET);
> > }
> > if (!red_channel_wait_all_sent(&cursor->common.base,
>
> another occurrence.
>
> > DISPLAY_CLIENT_TIMEOUT)) {
> > red_channel_apply_clients(&cursor->common.base,
> > -
> > red_channel_client_disconnect_if_pending_send);
> > +
> > red_channel_client_disconnect_if_pending_send);
> > }
> > }
> > }
> > diff --git a/server/red_worker.c b/server/red_worker.c
> > index b93676e..8bb048b 100644
> > --- a/server/red_worker.c
> > +++ b/server/red_worker.c
> > @@ -10272,10 +10272,9 @@ static void
> > dev_create_primary_surface(RedWorker
> > *worker, uint32_t surface_id,
> > red_channel_push(&worker->display_channel->common.base);
> > }
> >
> > - if (cursor_is_connected(worker) &&
> > !worker->cursor_channel->common.during_target_migrate) {
> > + if (!worker->cursor_channel->common.during_target_migrate)
>
> check removed without explanation. Also I think mostly if use blocks
> so
> I don't agree with this style change.
>
> > red_channel_pipes_add_type(&worker->cursor_channel
> > ->common.base,
> > PIPE_ITEM_TYPE_CURSOR_INIT);
> > - }
> > }
> >
> > void handle_dev_create_primary_surface(void *opaque, void
> > *payload)
> > @@ -10474,7 +10473,9 @@ void handle_dev_oom(void *opaque, void
> > *payload)
> >
> > void handle_dev_reset_cursor(void *opaque, void *payload)
> > {
> > - cursor_channel_reset(((RedWorker*)opaque)->cursor_channel);
> > + RedWorker *worker = opaque;
> > +
> > + cursor_channel_reset(worker->cursor_channel);
> > }
> >
> > void handle_dev_reset_image_cache(void *opaque, void *payload)
> > --
> > 2.4.3
>
> Frediano
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/spice-devel