[Spice-devel] [PATCH spice-gtk 1/2] spice-widget: cleanup mouse handling.

[Hans de Goede]

Some background on this patch, we currently behave
as follows in server mouse mode:

1) When spicy first connects, a frozen guest cursor is shown
 (no mouse events are send to the guest) and the client cursor
 is set to the standard right pointer,

2) When one clicks on / inside the spice-widget, the mouse is
 grabbed, the client cursor is hidden and the guest pointer
 becomes alive (gets events).

3) On ungrab we move back to state 1)

Which is fine, but the code implementing it is somewhat convoluted.
We have update_mouse_pointer(), which does more then just update the mouse
pointer, it also calls try_mouse_grab(), and we've update_mouse_grab(),
which calls update_mouse_pointer() rather then try_mouse_grab().

* I. lets look at what we currently have in update_mouse_pointer(): *

    case SPICE_MOUSE_MODE_SERVER:
        if (!d->mouse_grab_active) {
            if (gdk_window_get_cursor(window) != NULL)
                gdk_window_set_cursor(window, NULL);
        } else {
            try_mouse_grab(display);
        }
        break;

Now lets invert the test to make it more readable:

    case SPICE_MOUSE_MODE_SERVER:
        if (d->mouse_grab_active) {
            try_mouse_grab(display);
        } else {
            if (gdk_window_get_cursor(window) != NULL)
                gdk_window_set_cursor(window, NULL);
        }
        break;

Hmm, so if we're grabbing the mouse, we try to grab the mouse
-> does not make sense, esp not since try_mouse_grab() has:

    if (d->mouse_grab_active)
        return;

So we can drop the else block.

But why the if? Well that would be because when we're grabbing
the windows should have a blank cursor. But where does that get set?

The blank cursor gets set in do_pointer_grab(), by specifying a blank
cursor as the cursor to show as long as the grab is active.

Since that cursor will be active as long as we're grabbing, so we can
drop the if (!d->mouse_grab_active) check as well.

And with the mouse_grab_active check gone, we no longer need to call
update_mouse_pointer() from try_mouse_ungrab().

* II. lets look at update_mouse_grab() *

So currently when the mouse should be grabbed according to the
mouse-grab and disable-inputs properties, update_mouse_grab() calls
update_mouse_pointer(), which as discussed above does nothing with the grab,
as the code path calling try_mouse_grab() is dead code. The right thing to do
would of course be to have update_mouse_grab() call try_mouse_grab() in this
case.

But, that would mean that as soon as the property is changed the cursor will
get torn away from whereever it is and get grabbed, which may not always
be desirable. To fix this this patch also moves the mouse_have_pointer
and keyboard_have_focus checks from mouse_update() to try_mouse_grab()

Signed-off-by: Hans de Goede <hdegoede at redhat.com>

foo
---
 gtk/spice-widget.c |   20 +++++++++-----------
 1 files changed, 9 insertions(+), 11 deletions(-)

diff --git a/gtk/spice-widget.c b/gtk/spice-widget.c
index e270c06..bff529c 100644
--- a/gtk/spice-widget.c
+++ b/gtk/spice-widget.c
@@ -575,12 +575,8 @@ static void update_mouse_pointer(SpiceDisplay *display)
             gdk_window_set_cursor(window, d->mouse_cursor);
         break;
     case SPICE_MOUSE_MODE_SERVER:
-        if (!d->mouse_grab_active) {
-            if (gdk_window_get_cursor(window) != NULL)
-                gdk_window_set_cursor(window, NULL);
-        } else {
-            try_mouse_grab(display);
-        }
+        if (gdk_window_get_cursor(window) != NULL)
+            gdk_window_set_cursor(window, NULL);
         break;
     default:
         g_warn_if_reached();
@@ -597,6 +593,11 @@ static void try_mouse_grab(SpiceDisplay *display)
     if (d->disable_inputs)
         return;
 
+    if (!d->mouse_have_pointer)
+        return;
+    if (!d->keyboard_have_focus)
+        return;
+
     if (!d->mouse_grab_enable)
         return;
     if (d->mouse_mode != SPICE_MOUSE_MODE_SERVER)
@@ -654,7 +655,6 @@ static void try_mouse_ungrab(SpiceDisplay *display)
 
     d->mouse_grab_active = false;
 
-    update_mouse_pointer(display);
     g_signal_emit(display, signals[SPICE_DISPLAY_MOUSE_GRAB], 0, false);
 }
 
@@ -663,7 +663,7 @@ static void update_mouse_grab(SpiceDisplay *display)
     SpiceDisplayPrivate *d = SPICE_DISPLAY_GET_PRIVATE(display);
 
     if (d->mouse_grab_enable && !d->disable_inputs)
-        update_mouse_pointer(display);
+        try_mouse_grab(display);
     else
         try_mouse_ungrab(display);
 }
@@ -1419,9 +1419,7 @@ static void mouse_update(SpiceChannel *channel, gpointer data)
         try_mouse_ungrab(display);
         break;
     case SPICE_MOUSE_MODE_SERVER:
-        if (d->mouse_have_pointer &&
-            d->keyboard_have_focus)
-            try_mouse_grab(display);
+        try_mouse_grab(display);
         break;
     default:
         g_warn_if_reached();
-- 
1.7.7.4