[Qemu-devel] [PATCH 0/6] gtk: tweak grab handling, misc fixes.

[Qemu-devel] [PATCH 0/6] gtk: tweak grab handling, misc fixes.

[Gerd Hoffmann]

  Hi,

While doing some virgl testing I've trapped into a bunch of issues with
input grabs, especially in fullscreen mode.  So here is a series of
mostly grab-related patches:  Cleanups, improve tracing, don't grab when
entering fullscreen.

Also a small zoom fix, and the setlocale change discussed here recently.

please review and test,
  Gerd

Gerd Hoffmann (6):
  gtk: check for existing grabs in gd_grab_{pointer,keyboard}
  gtk: move gd_update_caption calls to
    gd_{grab,ungrab}_{pointer,keyboard}
  gtk: trace input grab reason
  gtk: set free_scale when setting zoom_fit
  gtk: don't grab input when entering fullscreen.
  gtk: setlocale for LC_MESSAGES only

 trace-events |  3 ++-
 ui/gtk.c     | 65 ++++++++++++++++++++++++++++++++++--------------------------
 2 files changed, 39 insertions(+), 29 deletions(-)

-- 
1.8.3.1

[Qemu-devel] [PATCH 1/6] gtk: check for existing grabs in gd_grab_{pointer, keyboard}

[Gerd Hoffmann]

If a grab is already active for our window, do nothing.
If a grab is already active for another window, release it.

Cleanup some checks and ungrab calls in the code which are
not needed any more.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 ui/gtk.c | 25 ++++++++++++++++++++-----
 1 file changed, 20 insertions(+), 5 deletions(-)

diff --git a/ui/gtk.c b/ui/gtk.c
index df2a79e..24a1edb 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -167,6 +167,8 @@ struct GtkDisplayState {
 
 static void gd_grab_pointer(VirtualConsole *vc);
 static void gd_ungrab_pointer(GtkDisplayState *s);
+static void gd_grab_keyboard(VirtualConsole *vc);
+static void gd_ungrab_keyboard(GtkDisplayState *s);
 
 /** Utility Functions **/
 
@@ -849,7 +851,6 @@ static gboolean gd_button_event(GtkWidget *widget, GdkEventButton *button,
     /* implicitly grab the input at the first click in the relative mode */
     if (button->button == 1 && button->type == GDK_BUTTON_PRESS &&
         !qemu_input_is_absolute() && s->ptr_owner != vc) {
-        gd_ungrab_pointer(s);
         if (!vc->window) {
             gtk_check_menu_item_set_active(GTK_CHECK_MENU_ITEM(s->grab_item),
                                            TRUE);
@@ -1259,6 +1260,14 @@ static void gd_grab_devices(VirtualConsole *vc, bool grab,
 
 static void gd_grab_keyboard(VirtualConsole *vc)
 {
+    if (vc->s->kbd_owner) {
+        if (vc->s->kbd_owner == vc) {
+            return;
+        } else {
+            gd_ungrab_keyboard(vc->s);
+        }
+    }
+
 #if GTK_CHECK_VERSION(3, 0, 0)
     gd_grab_devices(vc, true, GDK_SOURCE_KEYBOARD,
                    GDK_KEY_PRESS_MASK | GDK_KEY_RELEASE_MASK,
@@ -1292,6 +1301,15 @@ static void gd_ungrab_keyboard(GtkDisplayState *s)
 static void gd_grab_pointer(VirtualConsole *vc)
 {
     GdkDisplay *display = gtk_widget_get_display(vc->gfx.drawing_area);
+
+    if (vc->s->ptr_owner) {
+        if (vc->s->ptr_owner == vc) {
+            return;
+        } else {
+            gd_ungrab_pointer(vc->s);
+        }
+    }
+
 #if GTK_CHECK_VERSION(3, 0, 0)
     GdkDeviceManager *mgr = gdk_display_get_device_manager(display);
     gd_grab_devices(vc, true, GDK_SOURCE_MOUSE,
@@ -1352,9 +1370,7 @@ static void gd_menu_grab_input(GtkMenuItem *item, void *opaque)
     VirtualConsole *vc = gd_vc_find_current(s);
 
     if (gd_is_grab_active(s)) {
-        if (!gd_grab_on_hover(s)) {
-            gd_grab_keyboard(vc);
-        }
+        gd_grab_keyboard(vc);
         gd_grab_pointer(vc);
     } else {
         gd_ungrab_keyboard(s);
@@ -1415,7 +1431,6 @@ static gboolean gd_enter_event(GtkWidget *widget, GdkEventCrossing *crossing,
     GtkDisplayState *s = vc->s;
 
     if (gd_grab_on_hover(s)) {
-        gd_ungrab_keyboard(s);
         gd_grab_keyboard(vc);
         gd_update_caption(s);
     }
-- 
1.8.3.1

[Qemu-devel] [PATCH 2/6] gtk: move gd_update_caption calls to gd_{grab, ungrab}_{pointer, keyboard}

[Gerd Hoffmann]

Then we don't have to pair the grab/ungrab calls with update_caption
calls any more because things happen automatically ;)

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 ui/gtk.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/ui/gtk.c b/ui/gtk.c
index 24a1edb..5f87475 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -856,7 +856,6 @@ static gboolean gd_button_event(GtkWidget *widget, GdkEventButton *button,
                                            TRUE);
         } else {
             gd_grab_pointer(vc);
-            gd_update_caption(s);
         }
         return TRUE;
     }
@@ -1095,7 +1094,6 @@ static gboolean gd_win_grab(void *opaque)
     } else {
         gd_grab_pointer(vc);
     }
-    gd_update_caption(vc->s);
     return TRUE;
 }
 
@@ -1278,6 +1276,7 @@ static void gd_grab_keyboard(VirtualConsole *vc)
                       GDK_CURRENT_TIME);
 #endif
     vc->s->kbd_owner = vc;
+    gd_update_caption(vc->s);
     trace_gd_grab(vc->label, "kbd", true);
 }
 
@@ -1295,6 +1294,7 @@ static void gd_ungrab_keyboard(GtkDisplayState *s)
 #else
     gdk_keyboard_ungrab(GDK_CURRENT_TIME);
 #endif
+    gd_update_caption(s);
     trace_gd_grab(vc->label, "kbd", false);
 }
 
@@ -1336,6 +1336,7 @@ static void gd_grab_pointer(VirtualConsole *vc)
                             &vc->s->grab_x_root, &vc->s->grab_y_root, NULL);
 #endif
     vc->s->ptr_owner = vc;
+    gd_update_caption(vc->s);
     trace_gd_grab(vc->label, "ptr", true);
 }
 
@@ -1361,6 +1362,7 @@ static void gd_ungrab_pointer(GtkDisplayState *s)
                              gtk_widget_get_screen(vc->gfx.drawing_area),
                              vc->s->grab_x_root, vc->s->grab_y_root);
 #endif
+    gd_update_caption(s);
     trace_gd_grab(vc->label, "ptr", false);
 }
 
@@ -1377,7 +1379,6 @@ static void gd_menu_grab_input(GtkMenuItem *item, void *opaque)
         gd_ungrab_pointer(s);
     }
 
-    gd_update_caption(s);
     gd_update_cursor(vc);
 }
 
@@ -1432,7 +1433,6 @@ static gboolean gd_enter_event(GtkWidget *widget, GdkEventCrossing *crossing,
 
     if (gd_grab_on_hover(s)) {
         gd_grab_keyboard(vc);
-        gd_update_caption(s);
     }
     return TRUE;
 }
@@ -1445,7 +1445,6 @@ static gboolean gd_leave_event(GtkWidget *widget, GdkEventCrossing *crossing,
 
     if (gd_grab_on_hover(s)) {
         gd_ungrab_keyboard(s);
-        gd_update_caption(s);
     }
     return TRUE;
 }
-- 
1.8.3.1

[Qemu-devel] [PATCH 3/6] gtk: trace input grab reason

[Gerd Hoffmann]

Add a reason to grab calls and trace points,
so it is easier to debug grab related ui issues.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 trace-events |  3 ++-
 ui/gtk.c     | 26 +++++++++++++-------------
 2 files changed, 15 insertions(+), 14 deletions(-)

diff --git a/trace-events b/trace-events
index 0a82f0c..d6d1820 100644
--- a/trace-events
+++ b/trace-events
@@ -1143,7 +1143,8 @@ ppm_save(const char *filename, void *display_surface) "%s surface=%p"
 gd_switch(const char *tab, int width, int height) "tab=%s, width=%d, height=%d"
 gd_update(const char *tab, int x, int y, int w, int h) "tab=%s, x=%d, y=%d, w=%d, h=%d"
 gd_key_event(const char *tab, int gdk_keycode, int qemu_keycode, const char *action) "tab=%s, translated GDK keycode %d to QEMU keycode %d (%s)"
-gd_grab(const char *tab, const char *device, bool on) "tab=%s, %s %d"
+gd_grab(const char *tab, const char *device, const char *reason) "tab=%s, dev=%s, reason=%s"
+gd_ungrab(const char *tab, const char *device) "tab=%s, dev=%s"
 
 # ui/vnc.c
 vnc_key_guest_leds(bool caps, bool num, bool scroll) "caps %d, num %d, scroll %d"
diff --git a/ui/gtk.c b/ui/gtk.c
index 5f87475..322d112 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -165,9 +165,9 @@ struct GtkDisplayState {
     bool ignore_keys;
 };
 
-static void gd_grab_pointer(VirtualConsole *vc);
+static void gd_grab_pointer(VirtualConsole *vc, const char *reason);
 static void gd_ungrab_pointer(GtkDisplayState *s);
-static void gd_grab_keyboard(VirtualConsole *vc);
+static void gd_grab_keyboard(VirtualConsole *vc, const char *reason);
 static void gd_ungrab_keyboard(GtkDisplayState *s);
 
 /** Utility Functions **/
@@ -855,7 +855,7 @@ static gboolean gd_button_event(GtkWidget *widget, GdkEventButton *button,
             gtk_check_menu_item_set_active(GTK_CHECK_MENU_ITEM(s->grab_item),
                                            TRUE);
         } else {
-            gd_grab_pointer(vc);
+            gd_grab_pointer(vc, "relative-mode-click");
         }
         return TRUE;
     }
@@ -1092,7 +1092,7 @@ static gboolean gd_win_grab(void *opaque)
     if (vc->s->ptr_owner) {
         gd_ungrab_pointer(vc->s);
     } else {
-        gd_grab_pointer(vc);
+        gd_grab_pointer(vc, "user-request-detached-tab");
     }
     return TRUE;
 }
@@ -1256,7 +1256,7 @@ static void gd_grab_devices(VirtualConsole *vc, bool grab,
 }
 #endif
 
-static void gd_grab_keyboard(VirtualConsole *vc)
+static void gd_grab_keyboard(VirtualConsole *vc, const char *reason)
 {
     if (vc->s->kbd_owner) {
         if (vc->s->kbd_owner == vc) {
@@ -1277,7 +1277,7 @@ static void gd_grab_keyboard(VirtualConsole *vc)
 #endif
     vc->s->kbd_owner = vc;
     gd_update_caption(vc->s);
-    trace_gd_grab(vc->label, "kbd", true);
+    trace_gd_grab(vc->label, "kbd", reason);
 }
 
 static void gd_ungrab_keyboard(GtkDisplayState *s)
@@ -1295,10 +1295,10 @@ static void gd_ungrab_keyboard(GtkDisplayState *s)
     gdk_keyboard_ungrab(GDK_CURRENT_TIME);
 #endif
     gd_update_caption(s);
-    trace_gd_grab(vc->label, "kbd", false);
+    trace_gd_ungrab(vc->label, "kbd");
 }
 
-static void gd_grab_pointer(VirtualConsole *vc)
+static void gd_grab_pointer(VirtualConsole *vc, const char *reason)
 {
     GdkDisplay *display = gtk_widget_get_display(vc->gfx.drawing_area);
 
@@ -1337,7 +1337,7 @@ static void gd_grab_pointer(VirtualConsole *vc)
 #endif
     vc->s->ptr_owner = vc;
     gd_update_caption(vc->s);
-    trace_gd_grab(vc->label, "ptr", true);
+    trace_gd_grab(vc->label, "ptr", reason);
 }
 
 static void gd_ungrab_pointer(GtkDisplayState *s)
@@ -1363,7 +1363,7 @@ static void gd_ungrab_pointer(GtkDisplayState *s)
                              vc->s->grab_x_root, vc->s->grab_y_root);
 #endif
     gd_update_caption(s);
-    trace_gd_grab(vc->label, "ptr", false);
+    trace_gd_ungrab(vc->label, "ptr");
 }
 
 static void gd_menu_grab_input(GtkMenuItem *item, void *opaque)
@@ -1372,8 +1372,8 @@ static void gd_menu_grab_input(GtkMenuItem *item, void *opaque)
     VirtualConsole *vc = gd_vc_find_current(s);
 
     if (gd_is_grab_active(s)) {
-        gd_grab_keyboard(vc);
-        gd_grab_pointer(vc);
+        gd_grab_keyboard(vc, "user-request-main-window");
+        gd_grab_pointer(vc, "user-request-main-window");
     } else {
         gd_ungrab_keyboard(s);
         gd_ungrab_pointer(s);
@@ -1432,7 +1432,7 @@ static gboolean gd_enter_event(GtkWidget *widget, GdkEventCrossing *crossing,
     GtkDisplayState *s = vc->s;
 
     if (gd_grab_on_hover(s)) {
-        gd_grab_keyboard(vc);
+        gd_grab_keyboard(vc, "grab-on-hover");
     }
     return TRUE;
 }
-- 
1.8.3.1

[Qemu-devel] [PATCH 4/6] gtk: set free_scale when setting zoom_fit

[Gerd Hoffmann]

free_scale field tracks zoom-fit menu toggle state,
so we should keep them in sync ...

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 ui/gtk.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/ui/gtk.c b/ui/gtk.c
index 322d112..2629d97 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -1782,6 +1782,7 @@ static GSList *gd_vc_gfx_init(GtkDisplayState *s, VirtualConsole *vc,
 
     if (dpy_ui_info_supported(vc->gfx.dcl.con)) {
         gtk_menu_item_activate(GTK_MENU_ITEM(s->zoom_fit_item));
+        s->free_scale = true;
     }
 
     return group;
-- 
1.8.3.1

[Qemu-devel] [PATCH 5/6] gtk: don't grab input when entering fullscreen.

[Gerd Hoffmann]

Kick off all grabbing logic from fullscreen mode.  In the current state
it seems to create more problems than it solves though.  Try running
qemu/gtk fullscreen on one head of a multihead host for example ...

There probably was a reason the grab-on-fullscreen logic was added in
the first place.  So please test and report any issues so we can try to
find a sane way to handle it.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 ui/gtk.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/ui/gtk.c b/ui/gtk.c
index 2629d97..a17b1d1 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -1140,10 +1140,6 @@ static void gd_menu_full_screen(GtkMenuItem *item, void *opaque)
         gtk_widget_hide(s->menu_bar);
         if (vc->type == GD_VC_GFX) {
             gtk_widget_set_size_request(vc->gfx.drawing_area, -1, -1);
-            if (qemu_console_is_graphic(vc->gfx.dcl.con)) {
-                gtk_check_menu_item_set_active
-                    (GTK_CHECK_MENU_ITEM(s->grab_item), TRUE);
-            }
         }
         gtk_window_fullscreen(GTK_WINDOW(s->window));
         s->full_screen = TRUE;
@@ -1156,8 +1152,6 @@ static void gd_menu_full_screen(GtkMenuItem *item, void *opaque)
             vc->gfx.scale_x = 1.0;
             vc->gfx.scale_y = 1.0;
             gd_update_windowsize(vc);
-            gtk_check_menu_item_set_active(GTK_CHECK_MENU_ITEM(s->grab_item),
-                                           FALSE);
         }
     }
 
-- 
1.8.3.1

[Qemu-devel] [PATCH 6/6] gtk: setlocale for LC_MESSAGES only

[Gerd Hoffmann]

Setting other LC_* affects printf behavior and breaks QMP.
We don't have much beside menu item strings localized, so
restrict the locale setting to LC_MESSAGES.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 ui/gtk.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ui/gtk.c b/ui/gtk.c
index a17b1d1..a0b2340 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -1950,7 +1950,7 @@ void gtk_display_init(DisplayState *ds, bool full_screen, bool grab_on_hover)
 
     s->free_scale = FALSE;
 
-    setlocale(LC_ALL, "");
+    setlocale(LC_MESSAGES, "");
     bindtextdomain("qemu", CONFIG_QEMU_LOCALEDIR);
     textdomain("qemu");
 
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH 6/6] gtk: setlocale for LC_MESSAGES only

[Paolo Bonzini]

On 09/09/2015 11:28, Gerd Hoffmann wrote:
> Setting other LC_* affects printf behavior and breaks QMP.
> We don't have much beside menu item strings localized, so
> restrict the locale setting to LC_MESSAGES.
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>

Cc: qemu-stable@nongnu.org

> ---
>  ui/gtk.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/ui/gtk.c b/ui/gtk.c
> index a17b1d1..a0b2340 100644
> --- a/ui/gtk.c
> +++ b/ui/gtk.c
> @@ -1950,7 +1950,7 @@ void gtk_display_init(DisplayState *ds, bool full_screen, bool grab_on_hover)
>  
>      s->free_scale = FALSE;
>  
> -    setlocale(LC_ALL, "");
> +    setlocale(LC_MESSAGES, "");
>      bindtextdomain("qemu", CONFIG_QEMU_LOCALEDIR);
>      textdomain("qemu");
>  
> 

Re: [Qemu-devel] [PATCH 1/6] gtk: check for existing grabs in gd_grab_{pointer, keyboard}

[Marc-André Lureau]

On Wed, Sep 9, 2015 at 11:28 AM, Gerd Hoffmann <kraxel@redhat.com> wrote:
> If a grab is already active for our window, do nothing.
> If a grab is already active for another window, release it.
>
> Cleanup some checks and ungrab calls in the code which are
> not needed any more.
>
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  ui/gtk.c | 25 ++++++++++++++++++++-----
>  1 file changed, 20 insertions(+), 5 deletions(-)
>

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>

> diff --git a/ui/gtk.c b/ui/gtk.c
> index df2a79e..24a1edb 100644
> --- a/ui/gtk.c
> +++ b/ui/gtk.c
> @@ -167,6 +167,8 @@ struct GtkDisplayState {
>
>  static void gd_grab_pointer(VirtualConsole *vc);
>  static void gd_ungrab_pointer(GtkDisplayState *s);
> +static void gd_grab_keyboard(VirtualConsole *vc);
> +static void gd_ungrab_keyboard(GtkDisplayState *s);
>
>  /** Utility Functions **/
>
> @@ -849,7 +851,6 @@ static gboolean gd_button_event(GtkWidget *widget, GdkEventButton *button,
>      /* implicitly grab the input at the first click in the relative mode */
>      if (button->button == 1 && button->type == GDK_BUTTON_PRESS &&
>          !qemu_input_is_absolute() && s->ptr_owner != vc) {
> -        gd_ungrab_pointer(s);
>          if (!vc->window) {
>              gtk_check_menu_item_set_active(GTK_CHECK_MENU_ITEM(s->grab_item),
>                                             TRUE);
> @@ -1259,6 +1260,14 @@ static void gd_grab_devices(VirtualConsole *vc, bool grab,
>
>  static void gd_grab_keyboard(VirtualConsole *vc)
>  {
> +    if (vc->s->kbd_owner) {
> +        if (vc->s->kbd_owner == vc) {
> +            return;
> +        } else {
> +            gd_ungrab_keyboard(vc->s);
> +        }
> +    }
> +
>  #if GTK_CHECK_VERSION(3, 0, 0)
>      gd_grab_devices(vc, true, GDK_SOURCE_KEYBOARD,
>                     GDK_KEY_PRESS_MASK | GDK_KEY_RELEASE_MASK,
> @@ -1292,6 +1301,15 @@ static void gd_ungrab_keyboard(GtkDisplayState *s)
>  static void gd_grab_pointer(VirtualConsole *vc)
>  {
>      GdkDisplay *display = gtk_widget_get_display(vc->gfx.drawing_area);
> +
> +    if (vc->s->ptr_owner) {
> +        if (vc->s->ptr_owner == vc) {
> +            return;
> +        } else {
> +            gd_ungrab_pointer(vc->s);
> +        }
> +    }
> +
>  #if GTK_CHECK_VERSION(3, 0, 0)
>      GdkDeviceManager *mgr = gdk_display_get_device_manager(display);
>      gd_grab_devices(vc, true, GDK_SOURCE_MOUSE,
> @@ -1352,9 +1370,7 @@ static void gd_menu_grab_input(GtkMenuItem *item, void *opaque)
>      VirtualConsole *vc = gd_vc_find_current(s);
>
>      if (gd_is_grab_active(s)) {
> -        if (!gd_grab_on_hover(s)) {
> -            gd_grab_keyboard(vc);
> -        }
> +        gd_grab_keyboard(vc);
>          gd_grab_pointer(vc);
>      } else {
>          gd_ungrab_keyboard(s);
> @@ -1415,7 +1431,6 @@ static gboolean gd_enter_event(GtkWidget *widget, GdkEventCrossing *crossing,
>      GtkDisplayState *s = vc->s;
>
>      if (gd_grab_on_hover(s)) {
> -        gd_ungrab_keyboard(s);
>          gd_grab_keyboard(vc);
>          gd_update_caption(s);
>      }
> --
> 1.8.3.1
>
>



-- 
Marc-André Lureau

Re: [Qemu-devel] [PATCH 2/6] gtk: move gd_update_caption calls to gd_{grab, ungrab}_{pointer, keyboard}

[Marc-André Lureau]

On Wed, Sep 9, 2015 at 11:28 AM, Gerd Hoffmann <kraxel@redhat.com> wrote:
> Then we don't have to pair the grab/ungrab calls with update_caption
> calls any more because things happen automatically ;)
>
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  ui/gtk.c | 9 ++++-----
>  1 file changed, 4 insertions(+), 5 deletions(-)

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>

>
> diff --git a/ui/gtk.c b/ui/gtk.c
> index 24a1edb..5f87475 100644
> --- a/ui/gtk.c
> +++ b/ui/gtk.c
> @@ -856,7 +856,6 @@ static gboolean gd_button_event(GtkWidget *widget, GdkEventButton *button,
>                                             TRUE);
>          } else {
>              gd_grab_pointer(vc);
> -            gd_update_caption(s);
>          }
>          return TRUE;
>      }
> @@ -1095,7 +1094,6 @@ static gboolean gd_win_grab(void *opaque)
>      } else {
>          gd_grab_pointer(vc);
>      }
> -    gd_update_caption(vc->s);
>      return TRUE;
>  }
>
> @@ -1278,6 +1276,7 @@ static void gd_grab_keyboard(VirtualConsole *vc)
>                        GDK_CURRENT_TIME);
>  #endif
>      vc->s->kbd_owner = vc;
> +    gd_update_caption(vc->s);
>      trace_gd_grab(vc->label, "kbd", true);
>  }
>
> @@ -1295,6 +1294,7 @@ static void gd_ungrab_keyboard(GtkDisplayState *s)
>  #else
>      gdk_keyboard_ungrab(GDK_CURRENT_TIME);
>  #endif
> +    gd_update_caption(s);
>      trace_gd_grab(vc->label, "kbd", false);
>  }
>
> @@ -1336,6 +1336,7 @@ static void gd_grab_pointer(VirtualConsole *vc)
>                              &vc->s->grab_x_root, &vc->s->grab_y_root, NULL);
>  #endif
>      vc->s->ptr_owner = vc;
> +    gd_update_caption(vc->s);
>      trace_gd_grab(vc->label, "ptr", true);
>  }
>
> @@ -1361,6 +1362,7 @@ static void gd_ungrab_pointer(GtkDisplayState *s)
>                               gtk_widget_get_screen(vc->gfx.drawing_area),
>                               vc->s->grab_x_root, vc->s->grab_y_root);
>  #endif
> +    gd_update_caption(s);
>      trace_gd_grab(vc->label, "ptr", false);
>  }
>
> @@ -1377,7 +1379,6 @@ static void gd_menu_grab_input(GtkMenuItem *item, void *opaque)
>          gd_ungrab_pointer(s);
>      }
>
> -    gd_update_caption(s);
>      gd_update_cursor(vc);
>  }
>
> @@ -1432,7 +1433,6 @@ static gboolean gd_enter_event(GtkWidget *widget, GdkEventCrossing *crossing,
>
>      if (gd_grab_on_hover(s)) {
>          gd_grab_keyboard(vc);
> -        gd_update_caption(s);
>      }
>      return TRUE;
>  }
> @@ -1445,7 +1445,6 @@ static gboolean gd_leave_event(GtkWidget *widget, GdkEventCrossing *crossing,
>
>      if (gd_grab_on_hover(s)) {
>          gd_ungrab_keyboard(s);
> -        gd_update_caption(s);
>      }
>      return TRUE;
>  }
> --
> 1.8.3.1
>
>



-- 
Marc-André Lureau

Re: [Qemu-devel] [PATCH 3/6] gtk: trace input grab reason

[Marc-André Lureau]

On Wed, Sep 9, 2015 at 11:28 AM, Gerd Hoffmann <kraxel@redhat.com> wrote:
> Add a reason to grab calls and trace points,
> so it is easier to debug grab related ui issues.
>
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  trace-events |  3 ++-
>  ui/gtk.c     | 26 +++++++++++++-------------
>  2 files changed, 15 insertions(+), 14 deletions(-)
>

While at it, it doesn't make sense to add reasons for ungrab too?
could be done later if needed, so

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>

> diff --git a/trace-events b/trace-events
> index 0a82f0c..d6d1820 100644
> --- a/trace-events
> +++ b/trace-events
> @@ -1143,7 +1143,8 @@ ppm_save(const char *filename, void *display_surface) "%s surface=%p"
>  gd_switch(const char *tab, int width, int height) "tab=%s, width=%d, height=%d"
>  gd_update(const char *tab, int x, int y, int w, int h) "tab=%s, x=%d, y=%d, w=%d, h=%d"
>  gd_key_event(const char *tab, int gdk_keycode, int qemu_keycode, const char *action) "tab=%s, translated GDK keycode %d to QEMU keycode %d (%s)"
> -gd_grab(const char *tab, const char *device, bool on) "tab=%s, %s %d"
> +gd_grab(const char *tab, const char *device, const char *reason) "tab=%s, dev=%s, reason=%s"
> +gd_ungrab(const char *tab, const char *device) "tab=%s, dev=%s"
>
>  # ui/vnc.c
>  vnc_key_guest_leds(bool caps, bool num, bool scroll) "caps %d, num %d, scroll %d"
> diff --git a/ui/gtk.c b/ui/gtk.c
> index 5f87475..322d112 100644
> --- a/ui/gtk.c
> +++ b/ui/gtk.c
> @@ -165,9 +165,9 @@ struct GtkDisplayState {
>      bool ignore_keys;
>  };
>
> -static void gd_grab_pointer(VirtualConsole *vc);
> +static void gd_grab_pointer(VirtualConsole *vc, const char *reason);
>  static void gd_ungrab_pointer(GtkDisplayState *s);
> -static void gd_grab_keyboard(VirtualConsole *vc);
> +static void gd_grab_keyboard(VirtualConsole *vc, const char *reason);
>  static void gd_ungrab_keyboard(GtkDisplayState *s);
>
>  /** Utility Functions **/
> @@ -855,7 +855,7 @@ static gboolean gd_button_event(GtkWidget *widget, GdkEventButton *button,
>              gtk_check_menu_item_set_active(GTK_CHECK_MENU_ITEM(s->grab_item),
>                                             TRUE);
>          } else {
> -            gd_grab_pointer(vc);
> +            gd_grab_pointer(vc, "relative-mode-click");
>          }
>          return TRUE;
>      }
> @@ -1092,7 +1092,7 @@ static gboolean gd_win_grab(void *opaque)
>      if (vc->s->ptr_owner) {
>          gd_ungrab_pointer(vc->s);
>      } else {
> -        gd_grab_pointer(vc);
> +        gd_grab_pointer(vc, "user-request-detached-tab");
>      }
>      return TRUE;
>  }
> @@ -1256,7 +1256,7 @@ static void gd_grab_devices(VirtualConsole *vc, bool grab,
>  }
>  #endif
>
> -static void gd_grab_keyboard(VirtualConsole *vc)
> +static void gd_grab_keyboard(VirtualConsole *vc, const char *reason)
>  {
>      if (vc->s->kbd_owner) {
>          if (vc->s->kbd_owner == vc) {
> @@ -1277,7 +1277,7 @@ static void gd_grab_keyboard(VirtualConsole *vc)
>  #endif
>      vc->s->kbd_owner = vc;
>      gd_update_caption(vc->s);
> -    trace_gd_grab(vc->label, "kbd", true);
> +    trace_gd_grab(vc->label, "kbd", reason);
>  }
>
>  static void gd_ungrab_keyboard(GtkDisplayState *s)
> @@ -1295,10 +1295,10 @@ static void gd_ungrab_keyboard(GtkDisplayState *s)
>      gdk_keyboard_ungrab(GDK_CURRENT_TIME);
>  #endif
>      gd_update_caption(s);
> -    trace_gd_grab(vc->label, "kbd", false);
> +    trace_gd_ungrab(vc->label, "kbd");
>  }
>
> -static void gd_grab_pointer(VirtualConsole *vc)
> +static void gd_grab_pointer(VirtualConsole *vc, const char *reason)
>  {
>      GdkDisplay *display = gtk_widget_get_display(vc->gfx.drawing_area);
>
> @@ -1337,7 +1337,7 @@ static void gd_grab_pointer(VirtualConsole *vc)
>  #endif
>      vc->s->ptr_owner = vc;
>      gd_update_caption(vc->s);
> -    trace_gd_grab(vc->label, "ptr", true);
> +    trace_gd_grab(vc->label, "ptr", reason);
>  }
>
>  static void gd_ungrab_pointer(GtkDisplayState *s)
> @@ -1363,7 +1363,7 @@ static void gd_ungrab_pointer(GtkDisplayState *s)
>                               vc->s->grab_x_root, vc->s->grab_y_root);
>  #endif
>      gd_update_caption(s);
> -    trace_gd_grab(vc->label, "ptr", false);
> +    trace_gd_ungrab(vc->label, "ptr");
>  }
>
>  static void gd_menu_grab_input(GtkMenuItem *item, void *opaque)
> @@ -1372,8 +1372,8 @@ static void gd_menu_grab_input(GtkMenuItem *item, void *opaque)
>      VirtualConsole *vc = gd_vc_find_current(s);
>
>      if (gd_is_grab_active(s)) {
> -        gd_grab_keyboard(vc);
> -        gd_grab_pointer(vc);
> +        gd_grab_keyboard(vc, "user-request-main-window");
> +        gd_grab_pointer(vc, "user-request-main-window");
>      } else {
>          gd_ungrab_keyboard(s);
>          gd_ungrab_pointer(s);
> @@ -1432,7 +1432,7 @@ static gboolean gd_enter_event(GtkWidget *widget, GdkEventCrossing *crossing,
>      GtkDisplayState *s = vc->s;
>
>      if (gd_grab_on_hover(s)) {
> -        gd_grab_keyboard(vc);
> +        gd_grab_keyboard(vc, "grab-on-hover");
>      }
>      return TRUE;
>  }
> --
> 1.8.3.1
>
>



-- 
Marc-André Lureau

Re: [Qemu-devel] [PATCH 4/6] gtk: set free_scale when setting zoom_fit

[Marc-André Lureau]

On Wed, Sep 9, 2015 at 11:28 AM, Gerd Hoffmann <kraxel@redhat.com> wrote:
> free_scale field tracks zoom-fit menu toggle state,
> so we should keep them in sync ...
>
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  ui/gtk.c | 1 +
>  1 file changed, 1 insertion(+)
>

signal handler is connected later, ok

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>

> diff --git a/ui/gtk.c b/ui/gtk.c
> index 322d112..2629d97 100644
> --- a/ui/gtk.c
> +++ b/ui/gtk.c
> @@ -1782,6 +1782,7 @@ static GSList *gd_vc_gfx_init(GtkDisplayState *s, VirtualConsole *vc,
>
>      if (dpy_ui_info_supported(vc->gfx.dcl.con)) {
>          gtk_menu_item_activate(GTK_MENU_ITEM(s->zoom_fit_item));
> +        s->free_scale = true;
>      }
>
>      return group;
> --
> 1.8.3.1
>
>



-- 
Marc-André Lureau

Re: [Qemu-devel] [PATCH 5/6] gtk: don't grab input when entering fullscreen.

[Marc-André Lureau]

hi

On Wed, Sep 9, 2015 at 11:28 AM, Gerd Hoffmann <kraxel@redhat.com> wrote:
> Kick off all grabbing logic from fullscreen mode.  In the current state
> it seems to create more problems than it solves though.  Try running
> qemu/gtk fullscreen on one head of a multihead host for example ...
>
> There probably was a reason the grab-on-fullscreen logic was added in
> the first place.  So please test and report any issues so we can try to
> find a sane way to handle it.
>
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  ui/gtk.c | 6 ------
>  1 file changed, 6 deletions(-)
>

I agree with you regarding multihead use case, so

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>

> diff --git a/ui/gtk.c b/ui/gtk.c
> index 2629d97..a17b1d1 100644
> --- a/ui/gtk.c
> +++ b/ui/gtk.c
> @@ -1140,10 +1140,6 @@ static void gd_menu_full_screen(GtkMenuItem *item, void *opaque)
>          gtk_widget_hide(s->menu_bar);
>          if (vc->type == GD_VC_GFX) {
>              gtk_widget_set_size_request(vc->gfx.drawing_area, -1, -1);
> -            if (qemu_console_is_graphic(vc->gfx.dcl.con)) {
> -                gtk_check_menu_item_set_active
> -                    (GTK_CHECK_MENU_ITEM(s->grab_item), TRUE);
> -            }
>          }
>          gtk_window_fullscreen(GTK_WINDOW(s->window));
>          s->full_screen = TRUE;
> @@ -1156,8 +1152,6 @@ static void gd_menu_full_screen(GtkMenuItem *item, void *opaque)
>              vc->gfx.scale_x = 1.0;
>              vc->gfx.scale_y = 1.0;
>              gd_update_windowsize(vc);
> -            gtk_check_menu_item_set_active(GTK_CHECK_MENU_ITEM(s->grab_item),
> -                                           FALSE);
>          }
>      }
>
> --
> 1.8.3.1
>
>



-- 
Marc-André Lureau

Re: [Qemu-devel] [PATCH 6/6] gtk: setlocale for LC_MESSAGES only

[Marc-André Lureau]

Hi

On Wed, Sep 9, 2015 at 11:48 AM, Paolo Bonzini <pbonzini@redhat.com> wrote:
>
>
> On 09/09/2015 11:28, Gerd Hoffmann wrote:
>> Setting other LC_* affects printf behavior and breaks QMP.
>> We don't have much beside menu item strings localized, so
>> restrict the locale setting to LC_MESSAGES.
>>
>> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
>
> Cc: qemu-stable@nongnu.org
>
>> ---
>>  ui/gtk.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/ui/gtk.c b/ui/gtk.c
>> index a17b1d1..a0b2340 100644
>> --- a/ui/gtk.c
>> +++ b/ui/gtk.c
>> @@ -1950,7 +1950,7 @@ void gtk_display_init(DisplayState *ds, bool full_screen, bool grab_on_hover)
>>
>>      s->free_scale = FALSE;
>>
>> -    setlocale(LC_ALL, "");
>> +    setlocale(LC_MESSAGES, "");
>>      bindtextdomain("qemu", CONFIG_QEMU_LOCALEDIR);
>>      textdomain("qemu");
>>
>>
>

Don't it need gtk_disable_setlocale() too? (it's marked deprecated in
gtk2, but not in gtk3 interesting)

In the previous thread, Markus suggested a fat comment, you could
place it above the call.

-- 
Marc-André Lureau