gtk: A few bug fixes

PATCH Superseded Cole Robinson 2 participants 11 messages Mar 13, 2014 – Mar 14, 2014

Share & tools

Cole Robinson

author

2014-03-13 19:30 UTC #0 lore raw

v2 cover gtk: A few bug fixes

First patch documents a configure option. Next 3 patches fix deprecation warnings on gtk 3.10. The remaining patches are bug fixes.

v2:
Add configure patch
Fix building against GTK2

Cole Robinson (7):
configure: Document --with-gtkabi
gtk: Don't use deprecated gtk_image_menu_item_new_with_mnemonic
gtk: Don't use deprecated vte_terminal_get_adjustment
gtk: Remove use of deprecated stock items
gtk: Use ctrl+alt+q for quit accelerator
gtk: Fix mouse warping with gtk3
gtk: Don't warp absolute pointer

 configure |  1 +
 ui/gtk.c  | 34 ++++++++++++++++++++++------------
 2 files changed, 23 insertions(+), 12 deletions(-)

Signature · 1 line

1.8.5.3

Cole Robinson

author

2014-03-13 19:31 UTC #1 lore raw

PATCH v2 1/7 configure: Document --with-gtkabi

Signed-off-by Cole Robinson <[email protected]>

---
 configure | 1 +
 1 file changed, 1 insertion(+)

configure +1 -0

index 8c2838e..4572bc5 100755
@@ -1219,6 +1219,7 @@ Advanced options (experts only):
   --with-sdlabi            select preferred SDL ABI 1.2 or 2.0
   --disable-gtk            disable gtk UI
   --enable-gtk             enable gtk UI
+  --with-gtkabi            select preferred GTK ABI 2.0 or 3.0
   --disable-virtfs         disable VirtFS
   --enable-virtfs          enable VirtFS
   --disable-vnc            disable VNC

Signature · 1 line

1.8.5.3

Cole Robinson

author

2014-03-13 19:31 UTC #2 lore raw

PATCH v2 2/7 gtk: Don't use deprecated gtk_image_menu_item_new_with_mnemonic

In reply to Cole Robinson

In these cases we weren't using an image in the menu item anyways, so just do as the suggestion says. Should be fine for all qemu supported gtk versions.

ui/gtk.c: In function ‘gd_create_menu_machine’:
ui/gtk.c:1284:5: error: ‘gtk_image_menu_item_new_with_mnemonic’ is deprecated (declared at /usr/include/gtk-3.0/gtk/deprecated/gtkimagemenuitem.h:76): Use 'gtk_menu_item_new_with_mnemonic' instead [-Werror=deprecated-declarations]

     s->reset_item = gtk_image_menu_item_new_with_mnemonic(_("_Reset"));
     ^
ui/gtk.c:1287:5: error: ‘gtk_image_menu_item_new_with_mnemonic’ is deprecated (declared at /usr/include/gtk-3.0/gtk/deprecated/gtkimagemenuitem.h:76): Use 'gtk_menu_item_new_with_mnemonic' instead [-Werror=deprecated-declarations]
     s->powerdown_item = gtk_image_menu_item_new_with_mnemonic(_("Power _Down"));

Signed-off-by Cole Robinson <[email protected]>

---
 ui/gtk.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

ui/gtk.c +2 -2

index c3ac448..ce24b95 100644
@@ -1304,10 +1304,10 @@ static GtkWidget *gd_create_menu_machine(GtkDisplayState *s, GtkAccelGroup *acce
     separator = gtk_separator_menu_item_new();
     gtk_menu_shell_append(GTK_MENU_SHELL(machine_menu), separator);
 
-    s->reset_item = gtk_image_menu_item_new_with_mnemonic(_("_Reset"));
+    s->reset_item = gtk_menu_item_new_with_mnemonic(_("_Reset"));
     gtk_menu_shell_append(GTK_MENU_SHELL(machine_menu), s->reset_item);
 
-    s->powerdown_item = gtk_image_menu_item_new_with_mnemonic(_("Power _Down"));
+    s->powerdown_item = gtk_menu_item_new_with_mnemonic(_("Power _Down"));
     gtk_menu_shell_append(GTK_MENU_SHELL(machine_menu), s->powerdown_item);
 
     separator = gtk_separator_menu_item_new();

Signature · 1 line

1.8.5.3

Cole Robinson

author

2014-03-13 19:31 UTC #3 lore raw

PATCH v2 3/7 gtk: Don't use deprecated vte_terminal_get_adjustment

In reply to Cole Robinson

Guard this with a VTE and GTK version check so we don't break GTK2 builds.

ui/gtk.c: In function ‘gd_vc_init’:
ui/gtk.c:1176:5: error: ‘vte_terminal_get_adjustment’ is deprecated (declared at /usr/include/vte-2.90/vte/vtedeprecated.h:101) [-Werror=deprecated-declarations]

Signed-off-by Cole Robinson <[email protected]>

---

v2: Fix GTK2 build

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

ui/gtk.c +4 -0

index ce24b95..82a7fd2 100644
@@ -1194,7 +1194,11 @@ static GSList *gd_vc_init(GtkDisplayState *s, VirtualConsole *vc, int index, GSL
 
     vte_terminal_set_scrollback_lines(VTE_TERMINAL(vc->terminal), -1);
 
+#if VTE_CHECK_VERSION(0, 28, 0) && GTK_CHECK_VERSION(3, 0, 0)
+    vadjustment = gtk_scrollable_get_vadjustment(GTK_SCROLLABLE(vc->terminal));
+#else
     vadjustment = vte_terminal_get_adjustment(VTE_TERMINAL(vc->terminal));
+#endif
 
     scrolled_window = gtk_scrolled_window_new(NULL, vadjustment);
     gtk_container_add(GTK_CONTAINER(scrolled_window), vc->terminal);

Signature · 1 line

1.8.5.3

Cole Robinson

author

2014-03-13 19:31 UTC #4 lore raw

PATCH v2 4/7 gtk: Remove use of deprecated stock items

In reply to Cole Robinson

Stock items are deprecated. As are ImageMenuItems. Convert everything to text only MenuItems, with the same text content as mentioned in the conversion guide:

https://docs.google.com/spreadsheet/pub?key=0AsPAM3pPwxagdGF4THNMMUpjUW5xMXZfdUNzMXhEa2c&output=html

gtk2 users lose their menu icons as well, but I don't think that's enough of a problem to warrant keeping around back compat code.

Example error:

ui/gtk.c:1328:5: error: ‘GtkStock’ is deprecated [-Werror=deprecated-declarations]
ui/gtk.c:1335:5: error: ‘gtk_image_menu_item_new_from_stock’ is deprecated (declared at /usr/include/gtk-3.0/gtk/deprecated/gtkimagemenuitem.h:78): Use 'gtk_menu_item_new' instead [-Werror=deprecated-declarations]

     s->zoom_out_item = gtk_image_menu_item_new_from_stock(GTK_STOCK_ZOOM_OUT, NULL);

Signed-off-by Cole Robinson <[email protected]>

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

ui/gtk.c +7 -9

index 82a7fd2..205bb39 100644
@@ -1297,7 +1297,6 @@ static GtkWidget *gd_create_menu_machine(GtkDisplayState *s, GtkAccelGroup *acce
 {
     GtkWidget *machine_menu;
     GtkWidget *separator;
-    GtkStockItem item;
 
     machine_menu = gtk_menu_new();
     gtk_menu_set_accel_group(GTK_MENU(machine_menu), accel_group);

@@ -1317,11 +1316,11 @@ static GtkWidget *gd_create_menu_machine(GtkDisplayState *s, GtkAccelGroup *acce
     separator = gtk_separator_menu_item_new();
     gtk_menu_shell_append(GTK_MENU_SHELL(machine_menu), separator);
 
-    s->quit_item = gtk_image_menu_item_new_from_stock(GTK_STOCK_QUIT, NULL);
-    gtk_stock_lookup(GTK_STOCK_QUIT, &item);
+    s->quit_item = gtk_menu_item_new_with_mnemonic(_("_Quit"));
     gtk_menu_item_set_accel_path(GTK_MENU_ITEM(s->quit_item),
                                  "<QEMU>/Machine/Quit");
-    gtk_accel_map_add_entry("<QEMU>/Machine/Quit", item.keyval, item.modifier);
+    gtk_accel_map_add_entry("<QEMU>/Machine/Quit",
+                            GDK_KEY_q, GDK_CONTROL_MASK);
     gtk_menu_shell_append(GTK_MENU_SHELL(machine_menu), s->quit_item);
 
     return machine_menu;

@@ -1337,8 +1336,7 @@ static GtkWidget *gd_create_menu_view(GtkDisplayState *s, GtkAccelGroup *accel_g
     view_menu = gtk_menu_new();
     gtk_menu_set_accel_group(GTK_MENU(view_menu), accel_group);
 
-    s->full_screen_item =
-        gtk_image_menu_item_new_from_stock(GTK_STOCK_FULLSCREEN, NULL);
+    s->full_screen_item = gtk_menu_item_new_with_mnemonic(_("_Fullscreen"));
     gtk_menu_item_set_accel_path(GTK_MENU_ITEM(s->full_screen_item),
                                  "<QEMU>/View/Full Screen");
     gtk_accel_map_add_entry("<QEMU>/View/Full Screen", GDK_KEY_f,

@@ -1348,21 +1346,21 @@ static GtkWidget *gd_create_menu_view(GtkDisplayState *s, GtkAccelGroup *accel_g
     separator = gtk_separator_menu_item_new();
     gtk_menu_shell_append(GTK_MENU_SHELL(view_menu), separator);
 
-    s->zoom_in_item = gtk_image_menu_item_new_from_stock(GTK_STOCK_ZOOM_IN, NULL);
+    s->zoom_in_item = gtk_menu_item_new_with_mnemonic(_("Zoom _In"));
     gtk_menu_item_set_accel_path(GTK_MENU_ITEM(s->zoom_in_item),
                                  "<QEMU>/View/Zoom In");
     gtk_accel_map_add_entry("<QEMU>/View/Zoom In", GDK_KEY_plus,
                             HOTKEY_MODIFIERS);
     gtk_menu_shell_append(GTK_MENU_SHELL(view_menu), s->zoom_in_item);
 
-    s->zoom_out_item = gtk_image_menu_item_new_from_stock(GTK_STOCK_ZOOM_OUT, NULL);
+    s->zoom_out_item = gtk_menu_item_new_with_mnemonic(_("Zoom _Out"));
     gtk_menu_item_set_accel_path(GTK_MENU_ITEM(s->zoom_out_item),
                                  "<QEMU>/View/Zoom Out");
     gtk_accel_map_add_entry("<QEMU>/View/Zoom Out", GDK_KEY_minus,
                             HOTKEY_MODIFIERS);
     gtk_menu_shell_append(GTK_MENU_SHELL(view_menu), s->zoom_out_item);
 
-    s->zoom_fixed_item = gtk_image_menu_item_new_from_stock(GTK_STOCK_ZOOM_100, NULL);
+    s->zoom_fixed_item = gtk_menu_item_new_with_mnemonic(_("Best _Fit"));
     gtk_menu_item_set_accel_path(GTK_MENU_ITEM(s->zoom_fixed_item),
                                  "<QEMU>/View/Zoom Fixed");
     gtk_accel_map_add_entry("<QEMU>/View/Zoom Fixed", GDK_KEY_0,

Signature · 1 line

1.8.5.3

Cole Robinson

author

2014-03-13 19:31 UTC #5 lore raw

PATCH v2 5/7 gtk: Use ctrl+alt+q for quit accelerator

In reply to Cole Robinson

Using the standard ctrl+q makes it too easy to kill the whole VM. Using ctrl+alt+FOO is consistent with our other accelerators.

https://bugzilla.redhat.com/show_bug.cgi?id=1062393

Signed-off-by Cole Robinson <[email protected]>

---
 ui/gtk.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

ui/gtk.c +1 -1

index 205bb39..f42952e 100644
@@ -1320,7 +1320,7 @@ static GtkWidget *gd_create_menu_machine(GtkDisplayState *s, GtkAccelGroup *acce
     gtk_menu_item_set_accel_path(GTK_MENU_ITEM(s->quit_item),
                                  "<QEMU>/Machine/Quit");
     gtk_accel_map_add_entry("<QEMU>/Machine/Quit",
-                            GDK_KEY_q, GDK_CONTROL_MASK);
+                            GDK_KEY_q, HOTKEY_MODIFIERS);
     gtk_menu_shell_append(GTK_MENU_SHELL(machine_menu), s->quit_item);
 
     return machine_menu;

Signature · 1 line

1.8.5.3

Cole Robinson

author

2014-03-13 19:31 UTC #6 lore raw

PATCH v2 6/7 gtk: Fix mouse warping with gtk3

In reply to Cole Robinson

We were using the wrong coordinates, this fixes things to match the original gtk2 implementation.

You can see this error in action by using -vga qxl, however even after this patch the mouse warps in small increments up and to the left, -7x and -3y pixels at a time, until the pointer is warped off the widget. I think it's a qxl bug, but the next patch covers it up.

Signed-off-by Cole Robinson <[email protected]>

---
 ui/gtk.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

ui/gtk.c +1 -1

index f42952e..8482791 100644
@@ -346,7 +346,7 @@ static void gd_mouse_set(DisplayChangeListener *dcl,
                                x, y, &x_root, &y_root);
     gdk_device_warp(gdk_device_manager_get_client_pointer(mgr),
                     gtk_widget_get_screen(s->drawing_area),
-                    x, y);
+                    x_root, y_root);
 }
 #else
 static void gd_mouse_set(DisplayChangeListener *dcl,

Signature · 1 line

1.8.5.3

Cole Robinson

author

2014-03-13 19:31 UTC #7 lore raw

PATCH v2 7/7 gtk: Don't warp absolute pointer

In reply to Cole Robinson

This matches the behavior of SDL, and makes the mouse usable when using -display gtk -vga qxl

https://bugzilla.redhat.com/show_bug.cgi?id=1051724

Signed-off-by Cole Robinson <[email protected]>

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

ui/gtk.c +8 -0

index 8482791..32e8f01 100644
@@ -340,6 +340,10 @@ static void gd_mouse_set(DisplayChangeListener *dcl,
     GdkDeviceManager *mgr;
     gint x_root, y_root;
 
+    if (qemu_input_is_absolute()) {
+        return;
+    }
+
     dpy = gtk_widget_get_display(s->drawing_area);
     mgr = gdk_display_get_device_manager(dpy);
     gdk_window_get_root_coords(gtk_widget_get_window(s->drawing_area),

@@ -355,6 +359,10 @@ static void gd_mouse_set(DisplayChangeListener *dcl,
     GtkDisplayState *s = container_of(dcl, GtkDisplayState, dcl);
     gint x_root, y_root;
 
+    if (qemu_input_is_absolute()) {
+        return;
+    }
+
     gdk_window_get_root_coords(gtk_widget_get_window(s->drawing_area),
                                x, y, &x_root, &y_root);
     gdk_display_warp_pointer(gtk_widget_get_display(s->drawing_area),

Signature · 1 line

1.8.5.3

Gerd Hoffmann

2014-03-14 09:49 UTC #8 lore raw

In reply to Cole Robinson

Quoted 7 lines · Cole Robinson

First patch documents a configure option. Next 3 patches fix deprecation warnings on gtk 3.10. The remaining patches are bug fixes.
v2:
Add configure patch
Fix building against GTK2

I'll go cherry-pick the two warp bugfixes (patches 6+7) for qemu 2.0.

I'll leave the other cleanups for 2.1. The gtk2/3 compatibility isn't that easy to get right it seems. Our current mimimal required gtk version is 2.18, and on rhel6 (2.20) the build still fails:

CC ui/gtk.o
In file included from /usr/include/gtk-2.0/gtk/gtk.h:233,

                 from ui/gtk.c:48:

/usr/include/gtk-2.0/gtk/gtkitemfactory.h:47: warning: function declaration isn’t a prototype
ui/gtk.c: In function ‘gd_create_menu_machine’:
ui/gtk.c:1308: error: ‘GDK_KEY_q’ undeclared (first use in this function)
ui/gtk.c:1308: error: (Each undeclared identifier is reported only once ui/gtk.c:1308: error: for each function it appears in.)
make: *** [ui/gtk.o] Error 1

cheers,
Gerd

Cole Robinson

author

2014-03-14 12:04 UTC #9 lore raw

Quoted 18 lines · Gerd Hoffmann

Quoted 7 lines · Cole Robinson
First patch documents a configure option. Next 3 patches fix deprecation warnings on gtk 3.10. The remaining patches are bug fixes.
v2:
Add configure patch
Fix building against GTK2
I'll go cherry-pick the two warp bugfixes (patches 6+7) for qemu 2.0.
I'll leave the other cleanups for 2.1. The gtk2/3 compatibility isn't that easy to get right it seems. Our current mimimal required gtk version is 2.18, and on rhel6 (2.20) the build still fails:
CC ui/gtk.o
In file included from /usr/include/gtk-2.0/gtk/gtk.h:233,
                 from ui/gtk.c:48:

Quoted 9 lines

/usr/include/gtk-2.0/gtk/gtkitemfactory.h:47: warning: function declaration isn’t a prototype
ui/gtk.c: In function ‘gd_create_menu_machine’:
ui/gtk.c:1308: error: ‘GDK_KEY_q’ undeclared (first use in this function)
ui/gtk.c:1308: error: (Each undeclared identifier is reported only once ui/gtk.c:1308: error: for each function it appears in.)
make: *** [ui/gtk.o] Error 1

Hmm, thanks for testing. I didn't notice the GDK_KEY_foo back compat defines at the top of gtk.c, we need one for GDK_KEY_q as well.

Would you prefer I wait until after 2.0 is out to resubmit the series?

- Cole

Gerd Hoffmann

2014-03-14 12:09 UTC #10 lore raw

Hi,

Quoted 4 lines

Hmm, thanks for testing. I didn't notice the GDK_KEY_foo back compat defines at the top of gtk.c, we need one for GDK_KEY_q as well.
Would you prefer I wait until after 2.0 is out to resubmit the series?

I don't mind much, do whatever works best for you. Most likely there are no conflicts (at least cherry-picking worked without manual invention), and I can let 'em sit in a branch for a while ...

cheers,
Gerd