[Spice-commits] 2 commits - gtk/spice-widget.c

[Hans de Goede]

 gtk/spice-widget.c |   28 +++++++++++++---------------
 1 file changed, 13 insertions(+), 15 deletions(-)

New commits:
commit e2a7c5505df472d23bf1af73e00894e0613d61f4
Author: Hans de Goede <hdegoede at redhat.com>
Date:   Fri Dec 9 18:41:08 2011 +0100

    spice-widget: rename mouse_update to update_mouse_mode.
    
    We already have update_mouse_pointer and update_mouse_grab, so having
    a mouse_update function without specifying what "part" of the mouse handling
    it exactly updates is confusing, rename it to update_mouse_mode to reflect
    what it does and to match the other names.
    
    Signed-off-by: Hans de Goede <hdegoede at redhat.com>

diff --git a/gtk/spice-widget.c b/gtk/spice-widget.c
index bff529c..63e878e 100644
--- a/gtk/spice-widget.c
+++ b/gtk/spice-widget.c
@@ -1406,7 +1406,7 @@ static void spice_display_class_init(SpiceDisplayClass *klass)
 
 /* ---------------------------------------------------------------- */
 
-static void mouse_update(SpiceChannel *channel, gpointer data)
+static void update_mouse_mode(SpiceChannel *channel, gpointer data)
 {
     SpiceDisplay *display = data;
     SpiceDisplayPrivate *d = SPICE_DISPLAY_GET_PRIVATE(display);
@@ -1634,7 +1634,7 @@ static void disconnect_main(SpiceDisplay *display)
 
     if (d->main == NULL)
         return;
-    g_signal_handlers_disconnect_by_func(d->main, G_CALLBACK(mouse_update),
+    g_signal_handlers_disconnect_by_func(d->main, G_CALLBACK(update_mouse_mode),
                                          display);
     d->main = NULL;
 }
@@ -1681,8 +1681,8 @@ static void channel_new(SpiceSession *s, SpiceChannel *channel, gpointer data)
     if (SPICE_IS_MAIN_CHANNEL(channel)) {
         d->main = SPICE_MAIN_CHANNEL(channel);
         g_signal_connect(channel, "main-mouse-update",
-                         G_CALLBACK(mouse_update), display);
-        mouse_update(channel, display);
+                         G_CALLBACK(update_mouse_mode), display);
+        update_mouse_mode(channel, display);
         return;
     }
 
commit 5af6cfb18541aa0aafe9d7349c7e7046f8fe448d
Author: Hans de Goede <hdegoede at redhat.com>
Date:   Fri Dec 9 17:39:24 2011 +0100

    spice-widget: cleanup mouse handling.
    
    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

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();