[Qemu-devel] [PATCH 0/4] Fix relative pointer tracking on Gtk UI

[Qemu-devel] [PATCH 0/4] Fix relative pointer tracking on Gtk UI

[Takashi Iwai]

Hi,

this is a series of patches to fix / improve the behavior of Gtk UI
in the relative pointer tracking mode.  Most people didn't notice
the bug likely because it doesn't appear as long as the aboslute
mode is used, e.g. when vmmouse input driver is enabled.  But I hit
this annoying behavior because of the recent regression of vmmouse
driver (that doesn't work as default).  Or, it might be that this
didn't happen with the old Gtk2.  Dunno.

The last two patches are no actual fixes but rather behavior
changes for my tastes.  Both are to align the behavior compatible
with SDL UI, and I like it better just because I got used to it.
So, I don't mind if they are skipped.


thanks,

Takashi

[Qemu-devel] [PATCH 1/4] gtk: Use gtk generic event signal instead of motion-notify-event

[Takashi Iwai]

The GDK motion-notify-event isn't generated when the pointer goes out
of the target window even if the pointer is grabbed, which essentially
means to lose the pointer tracking in gtk-ui.

Meanwhile the generic "event" signal is sent when the pointer is
grabbed, so we can use this and pick the motion notify events manually
there instead.

Reference: https://bugzilla.novell.com/show_bug.cgi?id=849587
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 ui/gtk.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/ui/gtk.c b/ui/gtk.c
index a633d8934633..4d5ea8e6bb38 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -760,6 +760,13 @@ static gboolean gd_key_event(GtkWidget *widget, GdkEventKey *key, void *opaque)
     return TRUE;
 }
 
+static gboolean gd_event(GtkWidget *widget, GdkEvent *event, void *opaque)
+{
+    if (event->type == GDK_MOTION_NOTIFY)
+        return gd_motion_event(widget, &event->motion, opaque);
+    return FALSE;
+}
+
 /** Window Menu Actions **/
 
 static void gd_menu_pause(GtkMenuItem *item, void *opaque)
@@ -1254,8 +1261,8 @@ static void gd_connect_signals(GtkDisplayState *s)
     g_signal_connect(s->drawing_area, "expose-event",
                      G_CALLBACK(gd_expose_event), s);
 #endif
-    g_signal_connect(s->drawing_area, "motion-notify-event",
-                     G_CALLBACK(gd_motion_event), s);
+    g_signal_connect(s->drawing_area, "event",
+                     G_CALLBACK(gd_event), s);
     g_signal_connect(s->drawing_area, "button-press-event",
                      G_CALLBACK(gd_button_event), s);
     g_signal_connect(s->drawing_area, "button-release-event",
-- 
1.8.5.2

[Qemu-devel] [PATCH 2/4] gtk: Fix the relative pointer tracking mode

[Takashi Iwai]

The relative pointer tracking mode was still buggy even after the
previous fix of the motion-notify-event since the events are filtered
out when the pointer moves outside the drawing window due to the
boundary check for the absolute mode.

This patch fixes the issue by moving the unnecessary boundary check
into the if block of absolute mode, and keep the coordinate in the
relative mode even if it's outside the drawing area.  But this makes
the coordinate (last_x, last_y) possibly pointing to (-1,-1),
introduce a new flag to indicate the last coordinate has been
updated.

Reference: https://bugzilla.novell.com/show_bug.cgi?id=849587
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 ui/gtk.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/ui/gtk.c b/ui/gtk.c
index 4d5ea8e6bb38..b72272ebac3f 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -148,6 +148,7 @@ typedef struct GtkDisplayState
     DisplayChangeListener dcl;
     DisplaySurface *ds;
     int button_mask;
+    gboolean last_set;
     int last_x;
     int last_y;
 
@@ -604,16 +605,15 @@ static gboolean gd_motion_event(GtkWidget *widget, GdkEventMotion *motion,
     x = (motion->x - mx) / s->scale_x;
     y = (motion->y - my) / s->scale_y;
 
-    if (x < 0 || y < 0 ||
-        x >= surface_width(s->ds) ||
-        y >= surface_height(s->ds)) {
-        return TRUE;
-    }
-
     if (kbd_mouse_is_absolute()) {
+        if (x < 0 || y < 0 ||
+            x >= surface_width(s->ds) ||
+            y >= surface_height(s->ds)) {
+            return TRUE;
+        }
         dx = x * 0x7FFF / (surface_width(s->ds) - 1);
         dy = y * 0x7FFF / (surface_height(s->ds) - 1);
-    } else if (s->last_x == -1 || s->last_y == -1) {
+    } else if (!s->last_set) {
         dx = 0;
         dy = 0;
     } else {
@@ -623,6 +623,7 @@ static gboolean gd_motion_event(GtkWidget *widget, GdkEventMotion *motion,
 
     s->last_x = x;
     s->last_y = y;
+    s->last_set = TRUE;
 
     if (kbd_mouse_is_absolute() || gd_is_grab_active(s)) {
         kbd_mouse_event(dx, dy, 0, s->button_mask);
@@ -661,8 +662,7 @@ static gboolean gd_motion_event(GtkWidget *widget, GdkEventMotion *motion,
             GdkDisplay *display = gtk_widget_get_display(widget);
             gdk_display_warp_pointer(display, screen, x, y);
 #endif
-            s->last_x = -1;
-            s->last_y = -1;
+            s->last_set = FALSE;
             return FALSE;
         }
     }
-- 
1.8.5.2

[Qemu-devel] [PATCH 3/4] gtk: Remember the last grabbed pointer position

[Takashi Iwai]

It's pretty annoying that the pointer reappears at a random place once
after grabbing and ungrabbing the input.  Better to restore to the
original position where the pointer was grabbed.

Reference: https://bugzilla.novell.com/show_bug.cgi?id=849587
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 ui/gtk.c | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/ui/gtk.c b/ui/gtk.c
index b72272ebac3f..e04f0781ae6c 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -151,6 +151,8 @@ typedef struct GtkDisplayState
     gboolean last_set;
     int last_x;
     int last_y;
+    int grab_x_root;
+    int grab_y_root;
 
     double scale_x;
     double scale_y;
@@ -965,8 +967,8 @@ static void gd_ungrab_keyboard(GtkDisplayState *s)
 
 static void gd_grab_pointer(GtkDisplayState *s)
 {
-#if GTK_CHECK_VERSION(3, 0, 0)
     GdkDisplay *display = gtk_widget_get_display(s->drawing_area);
+#if GTK_CHECK_VERSION(3, 0, 0)
     GdkDeviceManager *mgr = gdk_display_get_device_manager(display);
     GList *devices = gdk_device_manager_list_devices(mgr,
                                                      GDK_DEVICE_TYPE_MASTER);
@@ -990,6 +992,8 @@ static void gd_grab_pointer(GtkDisplayState *s)
         tmp = tmp->next;
     }
     g_list_free(devices);
+    gdk_device_get_position(gdk_device_manager_get_client_pointer(mgr),
+                            NULL, &s->grab_x_root, &s->grab_y_root);
 #else
     gdk_pointer_grab(gtk_widget_get_window(s->drawing_area),
                      FALSE, /* All events to come to our window directly */
@@ -1001,13 +1005,15 @@ static void gd_grab_pointer(GtkDisplayState *s)
                      NULL, /* Allow cursor to move over entire desktop */
                      s->null_cursor,
                      GDK_CURRENT_TIME);
+    gdk_display_get_pointer(display, NULL,
+                            &s->grab_x_root, &s->grab_y_root, NULL);
 #endif
 }
 
 static void gd_ungrab_pointer(GtkDisplayState *s)
 {
-#if GTK_CHECK_VERSION(3, 0, 0)
     GdkDisplay *display = gtk_widget_get_display(s->drawing_area);
+#if GTK_CHECK_VERSION(3, 0, 0)
     GdkDeviceManager *mgr = gdk_display_get_device_manager(display);
     GList *devices = gdk_device_manager_list_devices(mgr,
                                                      GDK_DEVICE_TYPE_MASTER);
@@ -1021,8 +1027,14 @@ static void gd_ungrab_pointer(GtkDisplayState *s)
         tmp = tmp->next;
     }
     g_list_free(devices);
+    gdk_device_warp(gdk_device_manager_get_client_pointer(mgr),
+                    gtk_widget_get_screen(s->drawing_area),
+                    s->grab_x_root, s->grab_y_root);
 #else
     gdk_pointer_ungrab(GDK_CURRENT_TIME);
+    gdk_display_warp_pointer(display, 
+                             gtk_widget_get_screen(s->drawing_area),
+                             s->grab_x_root, s->grab_y_root);
 #endif
 }
 
-- 
1.8.5.2

[Qemu-devel] [PATCH 4/4] gtk: Add "Grab On Click" option

[Takashi Iwai]

I simply like it better, you don't? :)

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 ui/gtk.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/ui/gtk.c b/ui/gtk.c
index e04f0781ae6c..4942c8330505 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -133,6 +133,7 @@ typedef struct GtkDisplayState
     GtkWidget *zoom_fit_item;
     GtkWidget *grab_item;
     GtkWidget *grab_on_hover_item;
+    GtkWidget *grab_on_click_item;
     GtkWidget *vga_item;
 
     int nb_vcs;
@@ -181,6 +182,11 @@ static bool gd_grab_on_hover(GtkDisplayState *s)
     return gtk_check_menu_item_get_active(GTK_CHECK_MENU_ITEM(s->grab_on_hover_item));
 }
 
+static bool gd_grab_on_click(GtkDisplayState *s)
+{
+    return gtk_check_menu_item_get_active(GTK_CHECK_MENU_ITEM(s->grab_on_click_item));
+}
+
 static bool gd_on_vga(GtkDisplayState *s)
 {
     return gtk_notebook_get_current_page(GTK_NOTEBOOK(s->notebook)) == 0;
@@ -678,6 +684,12 @@ static gboolean gd_button_event(GtkWidget *widget, GdkEventButton *button,
     int dx, dy;
     int n;
 
+    if (button->button == 1 && button->type == GDK_BUTTON_PRESS &&
+	!gd_is_grab_active(s) && gd_grab_on_click(s) && button->button == 1) {
+        gtk_check_menu_item_set_active(GTK_CHECK_MENU_ITEM(s->grab_item), TRUE);
+	return TRUE;
+    }
+
     if (button->button == 1) {
         n = 0x01;
     } else if (button->button == 2) {
@@ -1401,6 +1413,9 @@ static GtkWidget *gd_create_menu_view(GtkDisplayState *s, GtkAccelGroup *accel_g
     s->grab_on_hover_item = gtk_check_menu_item_new_with_mnemonic(_("Grab On _Hover"));
     gtk_menu_shell_append(GTK_MENU_SHELL(view_menu), s->grab_on_hover_item);
 
+    s->grab_on_click_item = gtk_check_menu_item_new_with_mnemonic(_("Grab On _Click"));
+    gtk_menu_shell_append(GTK_MENU_SHELL(view_menu), s->grab_on_click_item);
+
     s->grab_item = gtk_check_menu_item_new_with_mnemonic(_("_Grab Input"));
     gtk_menu_item_set_accel_path(GTK_MENU_ITEM(s->grab_item),
                                  "<QEMU>/View/Grab Input");
-- 
1.8.5.2

Re: [Qemu-devel] [PATCH 0/4] Fix relative pointer tracking on Gtk UI

[Cole Robinson]

On 02/13/2014 06:15 AM, Takashi Iwai wrote:
> Hi,
> 
> this is a series of patches to fix / improve the behavior of Gtk UI
> in the relative pointer tracking mode.  Most people didn't notice
> the bug likely because it doesn't appear as long as the aboslute
> mode is used, e.g. when vmmouse input driver is enabled.  But I hit
> this annoying behavior because of the recent regression of vmmouse
> driver (that doesn't work as default).  Or, it might be that this
> didn't happen with the old Gtk2.  Dunno.
> 
> The last two patches are no actual fixes but rather behavior
> changes for my tastes.  Both are to align the behavior compatible
> with SDL UI, and I like it better just because I got used to it.
> So, I don't mind if they are skipped.
> 

I just hit this issue too. Unfortunately Anthony hasn't been around qemu-devel
much recently so these slipped through the cracks.

Can you rebase on qemu.git (you'll have to
s/kbd_mouse_is_absolute/qemu_mouse_is_absolute/ at least) and repost? Please
cc me and kraxel@redhat.com, I'll test and review

Thanks,
Cole

Re: [Qemu-devel] [PATCH 0/4] Fix relative pointer tracking on Gtk UI

[Takashi Iwai]

At Tue, 01 Apr 2014 16:34:15 -0400,
Cole Robinson wrote:
> 
> On 02/13/2014 06:15 AM, Takashi Iwai wrote:
> > Hi,
> > 
> > this is a series of patches to fix / improve the behavior of Gtk UI
> > in the relative pointer tracking mode.  Most people didn't notice
> > the bug likely because it doesn't appear as long as the aboslute
> > mode is used, e.g. when vmmouse input driver is enabled.  But I hit
> > this annoying behavior because of the recent regression of vmmouse
> > driver (that doesn't work as default).  Or, it might be that this
> > didn't happen with the old Gtk2.  Dunno.
> > 
> > The last two patches are no actual fixes but rather behavior
> > changes for my tastes.  Both are to align the behavior compatible
> > with SDL UI, and I like it better just because I got used to it.
> > So, I don't mind if they are skipped.
> > 
> 
> I just hit this issue too. Unfortunately Anthony hasn't been around qemu-devel
> much recently so these slipped through the cracks.
> 
> Can you rebase on qemu.git (you'll have to
> s/kbd_mouse_is_absolute/qemu_mouse_is_absolute/ at least) and repost? Please
> cc me and kraxel@redhat.com, I'll test and review

OK, I'm going to submit the renewed patch series.


thanks,

Takashi