[Qemu-devel] [PATCH 00/10] ui: gtk and sdl2 fixes

[Qemu-devel] [PATCH 00/10] ui: gtk and sdl2 fixes

[Cole Robinson]

gtk and sdl2 are nearly unusable on Fedora 24 at the moment.

The first two patches fix the worst issues. The remaining patches
are just extra bits that came about while I was digging into this.

Thanks,
Cole

Cole Robinson (10):
  ui: gtk: fix crash when terminal inner-border is NULL
  ui: sdl2: Release grab before opening console window
  configure: build SDL if only SDL2 available
  configure: error on unknown --with-sdlabi value
  configure: add echo_version helper
  configure: report GTK version
  configure: report SDL version
  configure: support vte-2.91
  ui: gtk: Fix a runtime warning on vte >= 0.37
  ui: gtk: Fix some deprecation warnings

 configure | 62 +++++++++++++++++++++++++++++++++++++++-----------------------
 ui/gtk.c  | 41 ++++++++++++++++++++++++++++++-----------
 ui/sdl2.c |  4 ++++
 3 files changed, 73 insertions(+), 34 deletions(-)

-- 
2.7.4

[Qemu-devel] [PATCH 01/10] ui: gtk: fix crash when terminal inner-border is NULL

[Cole Robinson]

VTE terminal inner-border can be NULL. The vte-0.36 (API 2.90)
code checks for the condition too so I assume it's not just a bug

Fixes a crash on Fedora 24 with gtk 3.20

Signed-off-by: Cole Robinson <crobinso@redhat.com>
---
 ui/gtk.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/ui/gtk.c b/ui/gtk.c
index f372a6d..9876d89 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -340,10 +340,12 @@ static void gd_update_geometry_hints(VirtualConsole *vc)
         geo.min_height = geo.height_inc * VC_TERM_Y_MIN;
         mask |= GDK_HINT_MIN_SIZE;
         gtk_widget_style_get(vc->vte.terminal, "inner-border", &ib, NULL);
-        geo.base_width  += ib->left + ib->right;
-        geo.base_height += ib->top + ib->bottom;
-        geo.min_width   += ib->left + ib->right;
-        geo.min_height  += ib->top + ib->bottom;
+        if (ib) {
+            geo.base_width  += ib->left + ib->right;
+            geo.base_height += ib->top + ib->bottom;
+            geo.min_width   += ib->left + ib->right;
+            geo.min_height  += ib->top + ib->bottom;
+        }
         geo_widget = vc->vte.terminal;
 #endif
     }
-- 
2.7.4

[Qemu-devel] [PATCH 02/10] ui: sdl2: Release grab before opening console window

[Cole Robinson]

sdl 2.0.4 currently has a bug which causes our UI shortcuts to fire
rapidly in succession:

  https://bugzilla.libsdl.org/show_bug.cgi?id=3287

It's a toss up whether ctrl+alt+f or ctrl+alt+2 will fire an
odd or even number of times, thus determining whether the action
succeeds or fails.

Opening monitor/serial windows is doubly broken, since it will often
lock the UI trying to grab the pointer:

  0x00007fffef3720a5 in SDL_Delay_REAL () at /lib64/libSDL2-2.0.so.0
  0x00007fffef3688ba in X11_SetWindowGrab () at /lib64/libSDL2-2.0.so.0
  0x00007fffef2f2da7 in SDL_SendWindowEvent () at /lib64/libSDL2-2.0.so.0
  0x00007fffef2f080b in SDL_SetKeyboardFocus () at /lib64/libSDL2-2.0.so.0
  0x00007fffef35d784 in X11_DispatchFocusIn.isra.8 () at /lib64/libSDL2-2.0.so.0
  0x00007fffef35dbce in X11_DispatchEvent () at /lib64/libSDL2-2.0.so.0
  0x00007fffef35ee4a in X11_PumpEvents () at /lib64/libSDL2-2.0.so.0
  0x00007fffef2eea6a in SDL_PumpEvents_REAL () at /lib64/libSDL2-2.0.so.0
  0x00007fffef2eeab5 in SDL_WaitEventTimeout_REAL () at /lib64/libSDL2-2.0.so.0
  0x000055555597eed0 in sdl2_poll_events (scon=0x55555876f928) at ui/sdl2.c:593

We can work around that hang by ungrabbing the pointer before launching
a new window. This roughly matches what our sdl1 code does

Signed-off-by: Cole Robinson <crobinso@redhat.com>
---
 ui/sdl2.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/ui/sdl2.c b/ui/sdl2.c
index d042442..909038f 100644
--- a/ui/sdl2.c
+++ b/ui/sdl2.c
@@ -357,6 +357,10 @@ static void handle_keydown(SDL_Event *ev)
         case SDL_SCANCODE_7:
         case SDL_SCANCODE_8:
         case SDL_SCANCODE_9:
+            if (gui_grab) {
+                sdl_grab_end(scon);
+            }
+
             win = ev->key.keysym.scancode - SDL_SCANCODE_1;
             if (win < sdl2_num_outputs) {
                 sdl2_console[win].hidden = !sdl2_console[win].hidden;
-- 
2.7.4

[Qemu-devel] [PATCH 03/10] configure: build SDL if only SDL2 available

[Cole Robinson]

Right now if SDL2 is installed but not SDL1, default configure will
entirely disable SDL. Check upfront for SDL2 using pkg-config, but
still prefer SDL1 if both versions are installed.

Signed-off-by: Cole Robinson <crobinso@redhat.com>
---
 configure | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/configure b/configure
index c37fc5f..0b53fac 100755
--- a/configure
+++ b/configure
@@ -207,7 +207,7 @@ fdt=""
 netmap="no"
 pixman=""
 sdl=""
-sdlabi="1.2"
+sdlabi=""
 virtfs=""
 vnc="yes"
 sparse="no"
@@ -2420,6 +2420,16 @@ fi
 # Look for sdl configuration program (pkg-config or sdl-config).  Try
 # sdl-config even without cross prefix, and favour pkg-config over sdl-config.
 
+if test "$sdlabi" = ""; then
+    if $pkg_config --exists "sdl"; then
+        sdlabi=1.2
+    elif $pkg_config --exists "sdl2"; then
+        sdlabi=2.0
+    else
+        sdlabi=1.2
+    fi
+fi
+
 if test $sdlabi = "2.0"; then
     sdl_config=$sdl2_config
     sdlname=sdl2
-- 
2.7.4

[Qemu-devel] [PATCH 04/10] configure: error on unknown --with-sdlabi value

[Cole Robinson]

I accidentally tried --with-sdlabi="1.0", and it failed much later in
a weird way. Instead, throw an error if the value isn't in our
whitelist.

Signed-off-by: Cole Robinson <crobinso@redhat.com>
---
 configure | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/configure b/configure
index 0b53fac..8e25a24 100755
--- a/configure
+++ b/configure
@@ -2434,9 +2434,11 @@ if test $sdlabi = "2.0"; then
     sdl_config=$sdl2_config
     sdlname=sdl2
     sdlconfigname=sdl2_config
-else
+elif test $sdlabi = "1.2"; then
     sdlname=sdl
     sdlconfigname=sdl_config
+else
+    error_exit "Unknown sdlabi $sdlabi, must be 1.2 or 2.0"
 fi
 
 if test "`basename $sdl_config`" != $sdlconfigname && ! has ${sdl_config}; then
-- 
2.7.4

[Qemu-devel] [PATCH 05/10] configure: add echo_version helper

[Cole Robinson]

Simplifies printing library versions, dependent on if the library
was even found

Signed-off-by: Cole Robinson <crobinso@redhat.com>
---
 configure | 18 ++++++++----------
 1 file changed, 8 insertions(+), 10 deletions(-)

diff --git a/configure b/configure
index 8e25a24..76600f4 100755
--- a/configure
+++ b/configure
@@ -4730,6 +4730,12 @@ EOF
   fi
 fi
 
+echo_version() {
+    if test "$1" = "yes" ; then
+        echo "($2)"
+    fi
+}
+
 # prepend pixman and ftd flags after all config tests are done
 QEMU_CFLAGS="$pixman_cflags $fdt_cflags $QEMU_CFLAGS"
 libs_softmmu="$pixman_libs $libs_softmmu"
@@ -4787,11 +4793,7 @@ echo "GNUTLS hash       $gnutls_hash"
 echo "GNUTLS rnd        $gnutls_rnd"
 echo "libgcrypt         $gcrypt"
 echo "libgcrypt kdf     $gcrypt_kdf"
-if test "$nettle" = "yes"; then
-    echo "nettle            $nettle ($nettle_version)"
-else
-    echo "nettle            $nettle"
-fi
+echo "nettle            $nettle `echo_version $nettle $nettle_version`"
 echo "nettle kdf        $nettle_kdf"
 echo "libtasn1          $tasn1"
 echo "VTE support       $vte"
@@ -4843,11 +4845,7 @@ echo "Trace backends    $trace_backends"
 if have_backend "simple"; then
 echo "Trace output file $trace_file-<pid>"
 fi
-if test "$spice" = "yes"; then
-echo "spice support     $spice ($spice_protocol_version/$spice_server_version)"
-else
-echo "spice support     $spice"
-fi
+echo "spice support     $spice `echo_version $spice $spice_protocol_version/$spice_server_version`"
 echo "rbd support       $rbd"
 echo "xfsctl support    $xfs"
 echo "smartcard support $smartcard"
-- 
2.7.4

[Qemu-devel] [PATCH 06/10] configure: report GTK version

[Cole Robinson]

Signed-off-by: Cole Robinson <crobinso@redhat.com>
---
 configure | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/configure b/configure
index 76600f4..55bd354 100755
--- a/configure
+++ b/configure
@@ -2157,6 +2157,7 @@ if test "$gtk" != "no"; then
     if $pkg_config --exists "$gtkpackage >= $gtkversion"; then
         gtk_cflags=`$pkg_config --cflags $gtkpackage`
         gtk_libs=`$pkg_config --libs $gtkpackage`
+        gtk_version=`$pkg_config --modversion $gtkpackage`
         if $pkg_config --exists "$gtkx11package >= $gtkversion"; then
             gtk_cflags="$gtk_cflags $x11_cflags"
             gtk_libs="$gtk_libs $x11_libs"
@@ -4786,7 +4787,7 @@ if test "$darwin" = "yes" ; then
 fi
 echo "pixman            $pixman"
 echo "SDL support       $sdl"
-echo "GTK support       $gtk"
+echo "GTK support       $gtk `echo_version $gtk $gtk_version`"
 echo "GTK GL support    $gtk_gl"
 echo "GNUTLS support    $gnutls"
 echo "GNUTLS hash       $gnutls_hash"
-- 
2.7.4

[Qemu-devel] [PATCH 07/10] configure: report SDL version

[Cole Robinson]

Signed-off-by: Cole Robinson <crobinso@redhat.com>
---
 configure | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/configure b/configure
index 55bd354..22cf55c 100755
--- a/configure
+++ b/configure
@@ -2448,10 +2448,10 @@ fi
 
 if $pkg_config $sdlname --exists; then
   sdlconfig="$pkg_config $sdlname"
-  _sdlversion=`$sdlconfig --modversion 2>/dev/null | sed 's/[^0-9]//g'`
+  sdlversion=`$sdlconfig --modversion 2>/dev/null`
 elif has ${sdl_config}; then
   sdlconfig="$sdl_config"
-  _sdlversion=`$sdlconfig --version | sed 's/[^0-9]//g'`
+  sdlversion=`$sdlconfig --version`
 else
   if test "$sdl" = "yes" ; then
     feature_not_found "sdl" "Install SDL devel"
@@ -2476,7 +2476,7 @@ EOF
     sdl_libs=`$sdlconfig --libs 2> /dev/null`
   fi
   if compile_prog "$sdl_cflags" "$sdl_libs" ; then
-    if test "$_sdlversion" -lt 121 ; then
+    if test `echo $sdlversion | sed 's/[^0-9]//g'` -lt 121 ; then
       sdl_too_old=yes
     else
       sdl=yes
@@ -4786,7 +4786,7 @@ if test "$darwin" = "yes" ; then
     echo "Cocoa support     $cocoa"
 fi
 echo "pixman            $pixman"
-echo "SDL support       $sdl"
+echo "SDL support       $sdl `echo_version $sdl $sdlversion`"
 echo "GTK support       $gtk `echo_version $gtk $gtk_version`"
 echo "GTK GL support    $gtk_gl"
 echo "GNUTLS support    $gnutls"
-- 
2.7.4

[Qemu-devel] [PATCH 08/10] configure: support vte-2.91

[Cole Robinson]

vte >= 0.37 expores API version 2.91, which is where all the active
development is. qemu builds and runs fine with that version, so use it
if it's available.

Signed-off-by: Cole Robinson <crobinso@redhat.com>
---
 configure | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/configure b/configure
index 22cf55c..bbf9005 100755
--- a/configure
+++ b/configure
@@ -2393,20 +2393,25 @@ fi
 
 if test "$vte" != "no"; then
     if test "$gtkabi" = "3.0"; then
-      vtepackage="vte-2.90"
-      vteversion="0.32.0"
+      vteminversion="0.32.0"
+      if $pkg_config --exists "vte-2.91"; then
+        vtepackage="vte-2.91"
+      else
+        vtepackage="vte-2.90"
+      fi
     else
       vtepackage="vte"
-      vteversion="0.24.0"
+      vteminversion="0.24.0"
     fi
-    if $pkg_config --exists "$vtepackage >= $vteversion"; then
+    if $pkg_config --exists "$vtepackage >= $vteminversion"; then
         vte_cflags=`$pkg_config --cflags $vtepackage`
         vte_libs=`$pkg_config --libs $vtepackage`
+        vteversion=`$pkg_config --modversion $vtepackage`
         libs_softmmu="$vte_libs $libs_softmmu"
         vte="yes"
     elif test "$vte" = "yes"; then
         if test "$gtkabi" = "3.0"; then
-            feature_not_found "vte" "Install libvte-2.90 devel"
+            feature_not_found "vte" "Install libvte-2.90/2.91 devel"
         else
             feature_not_found "vte" "Install libvte devel"
         fi
@@ -4789,6 +4794,7 @@ echo "pixman            $pixman"
 echo "SDL support       $sdl `echo_version $sdl $sdlversion`"
 echo "GTK support       $gtk `echo_version $gtk $gtk_version`"
 echo "GTK GL support    $gtk_gl"
+echo "VTE support       $vte `echo_version $vte $vteversion`"
 echo "GNUTLS support    $gnutls"
 echo "GNUTLS hash       $gnutls_hash"
 echo "GNUTLS rnd        $gnutls_rnd"
@@ -4797,7 +4803,6 @@ echo "libgcrypt kdf     $gcrypt_kdf"
 echo "nettle            $nettle `echo_version $nettle $nettle_version`"
 echo "nettle kdf        $nettle_kdf"
 echo "libtasn1          $tasn1"
-echo "VTE support       $vte"
 echo "curses support    $curses"
 echo "virgl support     $virglrenderer"
 echo "curl support      $curl"
-- 
2.7.4

[Qemu-devel] [PATCH 09/10] ui: gtk: Fix a runtime warning on vte >= 0.37

[Cole Robinson]

inner-border was dropped in vte API 2.91, in favor of the standard
padding style

Signed-off-by: Cole Robinson <crobinso@redhat.com>
---
 ui/gtk.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/ui/gtk.c b/ui/gtk.c
index 9876d89..d156c8a 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -329,6 +329,7 @@ static void gd_update_geometry_hints(VirtualConsole *vc)
     } else if (vc->type == GD_VC_VTE) {
         VteTerminal *term = VTE_TERMINAL(vc->vte.terminal);
         GtkBorder *ib;
+        GtkBorder padding;
 
         geo.width_inc  = vte_terminal_get_char_width(term);
         geo.height_inc = vte_terminal_get_char_height(term);
@@ -339,7 +340,17 @@ static void gd_update_geometry_hints(VirtualConsole *vc)
         geo.min_width  = geo.width_inc * VC_TERM_X_MIN;
         geo.min_height = geo.height_inc * VC_TERM_Y_MIN;
         mask |= GDK_HINT_MIN_SIZE;
+
+#if VTE_CHECK_VERSION(0, 37, 0)
+        gtk_style_context_get_padding(
+                gtk_widget_get_style_context(vc->vte.terminal),
+                gtk_widget_get_state_flags(vc->vte.terminal),
+                &padding);
+        ib = &padding;
+#else
         gtk_widget_style_get(vc->vte.terminal, "inner-border", &ib, NULL);
+#endif
+
         if (ib) {
             geo.base_width  += ib->left + ib->right;
             geo.base_height += ib->top + ib->bottom;
-- 
2.7.4

[Qemu-devel] [PATCH 10/10] ui: gtk: Fix some deprecation warnings

[Cole Robinson]

All device manager APIs are deprecated now. Much of our usage is
just to get the current pointer, so centralize that logic and use
the new seat APIs

Signed-off-by: Cole Robinson <crobinso@redhat.com>
---
The remaining warnings look like they'll take a bit more effort

 ui/gtk.c | 20 +++++++++++++-------
 1 file changed, 13 insertions(+), 7 deletions(-)

diff --git a/ui/gtk.c b/ui/gtk.c
index d156c8a..d3d7f62 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -476,12 +476,21 @@ static void gd_refresh(DisplayChangeListener *dcl)
 }
 
 #if GTK_CHECK_VERSION(3, 0, 0)
+static GdkDevice *gd_get_pointer(GdkDisplay *dpy)
+{
+#if GTK_CHECK_VERSION(3, 20, 0)
+    return gdk_seat_get_pointer(gdk_display_get_default_seat(dpy));
+#else
+    return gdk_device_manager_get_client_pointer(
+        gdk_display_get_device_manager(dpy));
+#endif
+}
+
 static void gd_mouse_set(DisplayChangeListener *dcl,
                          int x, int y, int visible)
 {
     VirtualConsole *vc = container_of(dcl, VirtualConsole, gfx.dcl);
     GdkDisplay *dpy;
-    GdkDeviceManager *mgr;
     gint x_root, y_root;
 
     if (qemu_input_is_absolute()) {
@@ -489,10 +498,9 @@ static void gd_mouse_set(DisplayChangeListener *dcl,
     }
 
     dpy = gtk_widget_get_display(vc->gfx.drawing_area);
-    mgr = gdk_display_get_device_manager(dpy);
     gdk_window_get_root_coords(gtk_widget_get_window(vc->gfx.drawing_area),
                                x, y, &x_root, &y_root);
-    gdk_device_warp(gdk_device_manager_get_client_pointer(mgr),
+    gdk_device_warp(gd_get_pointer(dpy),
                     gtk_widget_get_screen(vc->gfx.drawing_area),
                     x_root, y_root);
     vc->s->last_x = x;
@@ -1402,7 +1410,6 @@ static void gd_grab_pointer(VirtualConsole *vc, const char *reason)
     }
 
 #if GTK_CHECK_VERSION(3, 0, 0)
-    GdkDeviceManager *mgr = gdk_display_get_device_manager(display);
     gd_grab_devices(vc, true, GDK_SOURCE_MOUSE,
                     GDK_POINTER_MOTION_MASK |
                     GDK_BUTTON_PRESS_MASK |
@@ -1410,7 +1417,7 @@ static void gd_grab_pointer(VirtualConsole *vc, const char *reason)
                     GDK_BUTTON_MOTION_MASK |
                     GDK_SCROLL_MASK,
                     vc->s->null_cursor);
-    gdk_device_get_position(gdk_device_manager_get_client_pointer(mgr),
+    gdk_device_get_position(gd_get_pointer(display),
                             NULL, &vc->s->grab_x_root, &vc->s->grab_y_root);
 #else
     gdk_pointer_grab(gtk_widget_get_window(vc->gfx.drawing_area),
@@ -1442,9 +1449,8 @@ static void gd_ungrab_pointer(GtkDisplayState *s)
 
     GdkDisplay *display = gtk_widget_get_display(vc->gfx.drawing_area);
 #if GTK_CHECK_VERSION(3, 0, 0)
-    GdkDeviceManager *mgr = gdk_display_get_device_manager(display);
     gd_grab_devices(vc, false, GDK_SOURCE_MOUSE, 0, NULL);
-    gdk_device_warp(gdk_device_manager_get_client_pointer(mgr),
+    gdk_device_warp(gd_get_pointer(display),
                     gtk_widget_get_screen(vc->gfx.drawing_area),
                     vc->s->grab_x_root, vc->s->grab_y_root);
 #else
-- 
2.7.4