[Spice-devel] [PATCH spice-gtk 03/11] Correctly hide client pointer (in server mode)

[Hans de Goede]

Hi,

On 12/08/2011 03:12 PM, Marc-André Lureau wrote:
> The hide cursor event happen when the widget is not yet
> realized. Forcing realize may fail if the widget is not yet embedded
> for example. Instead, let's update the cursor whenever there is a
> draw().

Hmm, I don't like calling it every draw. Assuming gdk does not optimize
it away this will cause an awful lot of code to execute (first in gtk,
then in the Xorg server, all the way down to the hardware to upload the
same cursor to the hardware again and again).

Why don't we add a variable to track the current mouse mode, and only
make update_mouse_pointer() do anything
if (d->current_mouse_mode != d->mouse_mode) ?

Combined with setting d->current_mouse_mode to something invalid (say -1)
on init and whenever cursor_reset gets called.

Once such a check to avoid redoing the same call over and over again
is in place I'm fine with calling update_mouse_pointer() every draw.

While talking about / looking at this code, I notice that the
only way mouse_mode can change is trough mouse_update(), and
that mouse_update() does a try_mouse_ungrab() when the new
mode is SPICE_MOUSE_MODE_CLIENT, whereas update_mouse_pointer
does a try_mouse_grab() when moving to server mode. This seems
inconsistent.

Also note that this means that my current_mouse_mode idea is not as
simple as it first seems, since it seems that update_mouse_pointer()
is also called in various places where the intend seems to be to
do a try_mouse_grab, rather then updating the mouse pointer. I think
we should separate the 2, and make mouse_update() call
update_mouse_grab() and then update_mouse_pointer().

We should then ofcourse also change update_mouse_grab() to check
for mouse_mode and directly call try_mouse_grab() if it should
grab rather then going through update_mouse_pointer().

While at it I would also really really like to get rid of the FIXME
in update_mouse_pointer().

Note I'm not suggesting that you do all this work, but looking at this
I do believe that we should cleanup the mouse handling a bit, and that
that should be done before adding more hacks like this patch.

Regards,

Hans






> ---
>   gtk/spice-widget.c |    3 +++
>   1 files changed, 3 insertions(+), 0 deletions(-)
>
> diff --git a/gtk/spice-widget.c b/gtk/spice-widget.c
> index 816a3e2..5606980 100644
> --- a/gtk/spice-widget.c
> +++ b/gtk/spice-widget.c
> @@ -740,6 +740,7 @@ static gboolean draw_event(GtkWidget *widget, cairo_t *cr)
>       }
>
>       spicex_draw_event(display, cr);
> +    update_mouse_pointer(display);
>
>       return true;
>   }
> @@ -758,6 +759,8 @@ static gboolean expose_event(GtkWidget *widget, GdkEventExpose *expose)
>       }
>
>       spicex_expose_event(display, expose);
> +    update_mouse_pointer(display);
> +
>       return true;
>   }
>   #endif