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

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

[Takashi Iwai]

Hi,

this is the renewed patch series, just rebased the previous ones on
top of the current git tree.  Below is the original description:

***

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 v2 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 f056e4034b3e..54b835428a28 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -765,6 +765,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)
@@ -1267,8 +1274,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.9.1

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

[Cole Robinson]

On 04/02/2014 08:32 AM, Takashi Iwai wrote:
> 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 f056e4034b3e..54b835428a28 100644
> --- a/ui/gtk.c
> +++ b/ui/gtk.c
> @@ -765,6 +765,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)
> @@ -1267,8 +1274,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",
> 

Tested-by: Cole Robinson <crobinso@redhat.com>
Reviewed-by: Cole Robinson <crobinso@redhat.com>

Thanks,
Cole

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

[Gerd Hoffmann]

On Mi, 2014-04-02 at 14:32 +0200, Takashi Iwai wrote:
> +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;
> +}

Fails checkpatch:

WARNING: braces {} are necessary for all arms of this statement
#12: FILE: ui/gtk.c:771:
+    if (event->type == GDK_MOTION_NOTIFY)
[...]

total: 0 errors, 1 warnings, 23 lines checked

cheers,
  Gerd

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

[Takashi Iwai]

At Wed, 02 Apr 2014 17:17:54 +0200,
Gerd Hoffmann wrote:
> 
> On Mi, 2014-04-02 at 14:32 +0200, Takashi Iwai wrote:
> > +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;
> > +}
> 
> Fails checkpatch:
> 
> WARNING: braces {} are necessary for all arms of this statement
> #12: FILE: ui/gtk.c:771:
> +    if (event->type == GDK_MOTION_NOTIFY)
> [...]
> 
> total: 0 errors, 1 warnings, 23 lines checked

OK, you guys have a different checkpatch bible than the kernel tree :)
I'll tidy up later, if the patch is really acceptable.


thanks,

Takashi

[Qemu-devel] [PATCH v2 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 54b835428a28..c407fc8dd1a3 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -156,6 +156,7 @@ typedef struct GtkDisplayState
     DisplayChangeListener dcl;
     DisplaySurface *ds;
     int button_mask;
+    gboolean last_set;
     int last_x;
     int last_y;
 
@@ -616,25 +617,25 @@ 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 (qemu_input_is_absolute()) {
+        if (x < 0 || y < 0 ||
+            x >= surface_width(s->ds) ||
+            y >= surface_height(s->ds)) {
+            return TRUE;
+        }
         qemu_input_queue_abs(s->dcl.con, INPUT_AXIS_X, x,
                              surface_width(s->ds));
         qemu_input_queue_abs(s->dcl.con, INPUT_AXIS_Y, y,
                              surface_height(s->ds));
         qemu_input_event_sync();
-    } else if (s->last_x != -1 && s->last_y != -1 && gd_is_grab_active(s)) {
+    } else if (s->last_set && gd_is_grab_active(s)) {
         qemu_input_queue_rel(s->dcl.con, INPUT_AXIS_X, x - s->last_x);
         qemu_input_queue_rel(s->dcl.con, INPUT_AXIS_Y, y - s->last_y);
         qemu_input_event_sync();
     }
     s->last_x = x;
     s->last_y = y;
+    s->last_set = TRUE;
 
     if (!qemu_input_is_absolute() && gd_is_grab_active(s)) {
         GdkScreen *screen = gtk_widget_get_screen(s->drawing_area);
@@ -669,8 +670,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.9.1

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

[Cole Robinson]

On 04/02/2014 08:32 AM, Takashi Iwai wrote:
> 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>

Tested-by: Cole Robinson <crobinso@redhat.com>
Reviewed-by: Cole Robinson <crobinso@redhat.com>

Thanks,
Cole

[Qemu-devel] [PATCH v2 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 c407fc8dd1a3..9b8df1224fdb 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -159,6 +159,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;
@@ -970,8 +972,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);
@@ -995,6 +997,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 */
@@ -1006,13 +1010,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);
@@ -1026,8 +1032,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.9.1

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

[Cole Robinson]

On 04/02/2014 08:32 AM, Takashi Iwai wrote:
> 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>

I agree, and IMO it's a good idea in general to keep behavior similar between
the gtk and sdl frontends.

Tested-by: Cole Robinson <crobinso@redhat.com>
Reviewed-by: Cole Robinson <crobinso@redhat.com>

Thanks,
Cole

[Qemu-devel] [PATCH v2 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 9b8df1224fdb..ffaf91ea453a 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -141,6 +141,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;
@@ -189,6 +190,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;
@@ -685,6 +691,12 @@ static gboolean gd_button_event(GtkWidget *widget, GdkEventButton *button,
     GtkDisplayState *s = opaque;
     InputButton btn;
 
+    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) {
         btn = INPUT_BUTTON_LEFT;
     } else if (button->button == 2) {
@@ -1416,6 +1428,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.9.1

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

[Cole Robinson]

On 04/02/2014 08:32 AM, Takashi Iwai wrote:
> I simply like it better, you don't? :)
> 

In fact, relative mouse mode is a pain without this feature, you need to
manually initiate a grab with ctrl+alt+g before mouse movement will even work.
Compare to our sdl front end, or virt-viewer, vinagre, virt-manager, where
grab-on-click is the default (there isn't even an option to disable that
behavior).

I don't know what the original intention of the code was, but I think this
behavior should be the default. Anthony, Gerd, thoughts?

- Cole

> 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 9b8df1224fdb..ffaf91ea453a 100644
> --- a/ui/gtk.c
> +++ b/ui/gtk.c
> @@ -141,6 +141,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;
> @@ -189,6 +190,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;
> @@ -685,6 +691,12 @@ static gboolean gd_button_event(GtkWidget *widget, GdkEventButton *button,
>      GtkDisplayState *s = opaque;
>      InputButton btn;
>  
> +    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) {
>          btn = INPUT_BUTTON_LEFT;
>      } else if (button->button == 2) {
> @@ -1416,6 +1428,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");
> 

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

[Takashi Iwai]

At Wed, 02 Apr 2014 09:28:54 -0400,
Cole Robinson wrote:
> 
> On 04/02/2014 08:32 AM, Takashi Iwai wrote:
> > I simply like it better, you don't? :)
> > 
> 
> In fact, relative mouse mode is a pain without this feature, you need to
> manually initiate a grab with ctrl+alt+g before mouse movement will even work.
> Compare to our sdl front end, or virt-viewer, vinagre, virt-manager, where
> grab-on-click is the default (there isn't even an option to disable that
> behavior).
> 
> I don't know what the original intention of the code was, but I think this
> behavior should be the default. Anthony, Gerd, thoughts?

I noticed later that the original gtk-ui behavior is better when
vmmouse driver is available, i.e. the absolute mode is working.  With
grab-on-click, you have to ungrab at each time you want to leave from
VM window.

So I think we should leave the default behavior as is, since vmmouse
driver is likely available in most cases.  But, still having this in
the menu makes our lives easier in case vmmouse isn't available or
doesn't work with QEMU.


Takashi

> 
> - Cole
> 
> > 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 9b8df1224fdb..ffaf91ea453a 100644
> > --- a/ui/gtk.c
> > +++ b/ui/gtk.c
> > @@ -141,6 +141,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;
> > @@ -189,6 +190,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;
> > @@ -685,6 +691,12 @@ static gboolean gd_button_event(GtkWidget *widget, GdkEventButton *button,
> >      GtkDisplayState *s = opaque;
> >      InputButton btn;
> >  
> > +    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) {
> >          btn = INPUT_BUTTON_LEFT;
> >      } else if (button->button == 2) {
> > @@ -1416,6 +1428,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");
> > 
> 

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

[Cole Robinson]

On 04/02/2014 09:42 AM, Takashi Iwai wrote:
> At Wed, 02 Apr 2014 09:28:54 -0400,
> Cole Robinson wrote:
>>
>> On 04/02/2014 08:32 AM, Takashi Iwai wrote:
>>> I simply like it better, you don't? :)
>>>
>>
>> In fact, relative mouse mode is a pain without this feature, you need to
>> manually initiate a grab with ctrl+alt+g before mouse movement will even work.
>> Compare to our sdl front end, or virt-viewer, vinagre, virt-manager, where
>> grab-on-click is the default (there isn't even an option to disable that
>> behavior).
>>
>> I don't know what the original intention of the code was, but I think this
>> behavior should be the default. Anthony, Gerd, thoughts?
> 
> I noticed later that the original gtk-ui behavior is better when
> vmmouse driver is available, i.e. the absolute mode is working.  With
> grab-on-click, you have to ungrab at each time you want to leave from
> VM window.
> 
> So I think we should leave the default behavior as is, since vmmouse
> driver is likely available in most cases.  But, still having this in
> the menu makes our lives easier in case vmmouse isn't available or
> doesn't work with QEMU.
> 

Or we just do what sdl.c does (and virt-viewer, and vinagre, and virt-manager):

absolute mode == grab on hover, ungrab when pointer leaves the window
relative mode == grab on click, require manual ungrab

- Cole

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

[Gerd Hoffmann]

On Mi, 2014-04-02 at 09:28 -0400, Cole Robinson wrote:
> On 04/02/2014 08:32 AM, Takashi Iwai wrote:
> > I simply like it better, you don't? :)
> > 
> 
> In fact, relative mouse mode is a pain without this feature, you need to
> manually initiate a grab with ctrl+alt+g before mouse movement will even work.
> Compare to our sdl front end, or virt-viewer, vinagre, virt-manager, where
> grab-on-click is the default (there isn't even an option to disable that
> behavior).
> 
> I don't know what the original intention of the code was, but I think this
> behavior should be the default. Anthony, Gerd, thoughts?

There are *two* grabs.  Pointer grab and keyboard grab.

grab-on-hover is for the keyboard, i.e. if the qemu gtk window has the
mouse focus it will grab the keyboard (if enabled), with the effect that
special keys like ctrl-alt-del are received by qemu and forwarded to the
guest instead of being intercepted by the hosts window manager.

grab-on-click is for the mouse.  I don't think we need an option for it,
instead it should depend on whenever the pointer works in absolute or
relative mode.  In absolute mode you don't need a pointer grab in the
first place.  Relative mode is pretty much unusable without pointer
grab.  Activating the pointer grab without a mouse click is pretty
annoying though.

hope that clarifies,
  Gerd

[Qemu-devel] [PATCH] gtk: Grab pointer on click when in relative mode

[Cole Robinson]

Without this, we need to initiate a manual grab with ctrl+alt+g just
to get a usable mouse.

Signed-off-by: Cole Robinson <crobinso@redhat.com>
---
Gerd, Takashi, I think this should do what we want.

 ui/gtk.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/ui/gtk.c b/ui/gtk.c
index 9b8df12..ebaade2 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -685,6 +685,15 @@ static gboolean gd_button_event(GtkWidget *widget, GdkEventButton *button,
     GtkDisplayState *s = opaque;
     InputButton btn;
 
+    if (button->button == 1 &&
+        button->type == GDK_BUTTON_PRESS &&
+        !gd_is_grab_active(s) &&
+        !qemu_input_is_absolute()) {
+        gtk_check_menu_item_set_active(GTK_CHECK_MENU_ITEM(s->grab_item),
+                                       TRUE);
+        return TRUE;
+    }
+
     if (button->button == 1) {
         btn = INPUT_BUTTON_LEFT;
     } else if (button->button == 2) {
-- 
1.9.0

Re: [Qemu-devel] [PATCH] gtk: Grab pointer on click when in relative mode

[Gerd Hoffmann]

On Mi, 2014-04-02 at 11:07 -0400, Cole Robinson wrote:
> +    if (button->button == 1 &&
> +        button->type == GDK_BUTTON_PRESS &&
> +        !gd_is_grab_active(s) &&
> +        !qemu_input_is_absolute()) {
> +
> gtk_check_menu_item_set_active(GTK_CHECK_MENU_ITEM(s->grab_item),
> +                                       TRUE);
> +        return TRUE;
> +    }

Looks sane on a quick glance (as replacement for patch #4 of Takashi's
patch series).  Didn't test yet.

cheers,
  Gerd

Re: [Qemu-devel] [PATCH] gtk: Grab pointer on click when in relative mode

[Takashi Iwai]

At Wed, 02 Apr 2014 17:19:51 +0200,
Gerd Hoffmann wrote:
> 
> On Mi, 2014-04-02 at 11:07 -0400, Cole Robinson wrote:
> > +    if (button->button == 1 &&
> > +        button->type == GDK_BUTTON_PRESS &&
> > +        !gd_is_grab_active(s) &&
> > +        !qemu_input_is_absolute()) {
> > +
> > gtk_check_menu_item_set_active(GTK_CHECK_MENU_ITEM(s->grab_item),
> > +                                       TRUE);
> > +        return TRUE;
> > +    }
> 
> Looks sane on a quick glance (as replacement for patch #4 of Takashi's
> patch series).  Didn't test yet.

Unfortunately, this doesn't work perfectly as expected.
The input mode change happens after the first click action, thus
this always results in grabbing if you do left-click at first, even
after X starts up and vmmouse gets active.

I thought of checking it via notifier, but the notification happens
also after the first mouse click event.  Hmm.


Takashi

Re: [Qemu-devel] [PATCH] gtk: Grab pointer on click when in relative mode

[Cole Robinson]

On 04/02/2014 11:25 AM, Takashi Iwai wrote:
> At Wed, 02 Apr 2014 17:19:51 +0200,
> Gerd Hoffmann wrote:
>>
>> On Mi, 2014-04-02 at 11:07 -0400, Cole Robinson wrote:
>>> +    if (button->button == 1 &&
>>> +        button->type == GDK_BUTTON_PRESS &&
>>> +        !gd_is_grab_active(s) &&
>>> +        !qemu_input_is_absolute()) {
>>> +
>>> gtk_check_menu_item_set_active(GTK_CHECK_MENU_ITEM(s->grab_item),
>>> +                                       TRUE);
>>> +        return TRUE;
>>> +    }
>>
>> Looks sane on a quick glance (as replacement for patch #4 of Takashi's
>> patch series).  Didn't test yet.
> 
> Unfortunately, this doesn't work perfectly as expected.
> The input mode change happens after the first click action, thus
> this always results in grabbing if you do left-click at first, even
> after X starts up and vmmouse gets active.
> 
> I thought of checking it via notifier, but the notification happens
> also after the first mouse click event.  Hmm.

We could ungrab the pointer if transitioning from relative to absolute mode,
that's effectively what virt-viewer/remote-viewer does (though it also
completely disallows pointer grab in absolute mode... not sure we want to go
that far).

- Cole

Re: [Qemu-devel] [PATCH] gtk: Grab pointer on click when in relative mode

[Takashi Iwai]

At Wed, 02 Apr 2014 16:18:15 -0400,
Cole Robinson wrote:
> 
> On 04/02/2014 11:25 AM, Takashi Iwai wrote:
> > At Wed, 02 Apr 2014 17:19:51 +0200,
> > Gerd Hoffmann wrote:
> >>
> >> On Mi, 2014-04-02 at 11:07 -0400, Cole Robinson wrote:
> >>> +    if (button->button == 1 &&
> >>> +        button->type == GDK_BUTTON_PRESS &&
> >>> +        !gd_is_grab_active(s) &&
> >>> +        !qemu_input_is_absolute()) {
> >>> +
> >>> gtk_check_menu_item_set_active(GTK_CHECK_MENU_ITEM(s->grab_item),
> >>> +                                       TRUE);
> >>> +        return TRUE;
> >>> +    }
> >>
> >> Looks sane on a quick glance (as replacement for patch #4 of Takashi's
> >> patch series).  Didn't test yet.
> > 
> > Unfortunately, this doesn't work perfectly as expected.
> > The input mode change happens after the first click action, thus
> > this always results in grabbing if you do left-click at first, even
> > after X starts up and vmmouse gets active.
> > 
> > I thought of checking it via notifier, but the notification happens
> > also after the first mouse click event.  Hmm.
> 
> We could ungrab the pointer if transitioning from relative to absolute mode,
> that's effectively what virt-viewer/remote-viewer does (though it also
> completely disallows pointer grab in absolute mode... not sure we want to go
> that far).

Yeah, that's my concern, too.  Allowing the input grab in absolute
mode still makes some sense although much less than relative mode.


Takashi