Re: [Qemu-devel] [PATCH v3 04/12] gtk: add and use DisplayOptions + DisplayGTK

2018-02-02 Thread Eric Blake
On 02/02/2018 05:10 AM, Gerd Hoffmann wrote:
> Add QAPI DisplayType enum, DisplayOptions union and DisplayGTK struct.
> Switch gtk configuration to use the qapi type.
> 
> Some bookkeeping (fullscreen for example) is done twice now, this is
> temporary until more/all UIs are switched over to qapi configuration.
> 
> Signed-off-by: Gerd Hoffmann 
> ---
>  include/ui/console.h |  9 
>  ui/gtk.c | 32 -
>  vl.c | 23 -
>  qapi/ui.json | 58 
> 
>  4 files changed, 98 insertions(+), 24 deletions(-)
> 

> +++ b/vl.c

>  if (strstart(opts, ",grab_on_hover=", &nextopt)) {
>  opts = nextopt;
> +dpy.u.gtk.has_grab_on_hover = true;
>  if (strstart(opts, "on", &nextopt)) {

Pre-existing: this doesn't flag stuff like "grab_on_hover=only" as
bogus.  But doing further cleanups can be later patches; for example,
Markus' work to unify command-line parsing through QAPI visits.

> +++ b/qapi/ui.json
> @@ -982,3 +982,61 @@
>'data': { '*device': 'str',
>  '*head'  : 'int',
>  'events' : [ 'InputEvent' ] } }
> +
> +
> +##

Is two blank lines typical here, or just one?

> +# @DisplayNoOpts:
> +#
> +# Empty struct for displays without config options.
> +#
> +# Since: 2.12
> +#
> +##
> +{ 'struct'  : 'DisplayNoOpts',
> +  'data': { } }
> +
> +##
> +# @DisplayGTK:
> +#
> +# GTK display options.
> +#
> +# @grab-on-hover: Grab keyboard input on mouse hover.
> +#
> +# Since: 2.12
> +#
> +##
> +{ 'struct'  : 'DisplayGTK',
> +  'data': { '*grab-on-hover' : 'bool' } }
> +
> +##
> +# @DisplayType:
> +#
> +# Display (user interface) type.
> +#
> +# Since: 2.12
> +#
> +##
> +{ 'enum': 'DisplayType',
> +  'data': [ 'none', 'gtk' ] }
> +
> +##
> +# @DisplayOptions:
> +#
> +# Display (user interface) options.
> +#
> +# @type:  Which DisplayType qemu should use.
> +# @full-screen:   Start user interface in fullscreen mode (default: off).
> +# @window-close:  Allow to quit qemu with window close button (default: on).

s/Allow to quit/Allow quitting/

> +# @gl:Enable OpenGL support (default: off).
> +#
> +# Since: 2.12
> +#
> +##
> +{ 'union'   : 'DisplayOptions',
> +  'base': { 'type'   : 'DisplayType',
> +'*full-screen'   : 'bool',
> +'*window-close'  : 'bool',
> +'*gl': 'bool' },
> +  'discriminator' : 'type',
> +  'data': { 'none'   : 'DisplayNoOpts',
> +'gtk': 'DisplayGTK' } }
> 

Struct looks okay, and grammar tweak is minor, so you can add
Reviewed-by: Eric Blake 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org



signature.asc
Description: OpenPGP digital signature


[Qemu-devel] [PATCH v3 04/12] gtk: add and use DisplayOptions + DisplayGTK

2018-02-02 Thread Gerd Hoffmann
Add QAPI DisplayType enum, DisplayOptions union and DisplayGTK struct.
Switch gtk configuration to use the qapi type.

Some bookkeeping (fullscreen for example) is done twice now, this is
temporary until more/all UIs are switched over to qapi configuration.

Signed-off-by: Gerd Hoffmann 
---
 include/ui/console.h |  9 
 ui/gtk.c | 32 -
 vl.c | 23 -
 qapi/ui.json | 58 
 4 files changed, 98 insertions(+), 24 deletions(-)

diff --git a/include/ui/console.h b/include/ui/console.h
index 7b35778444..58d1a3d27c 100644
--- a/include/ui/console.h
+++ b/include/ui/console.h
@@ -511,18 +511,17 @@ int index_from_key(const char *key, size_t key_length);
 
 /* gtk.c */
 #ifdef CONFIG_GTK
-void early_gtk_display_init(int opengl);
-void gtk_display_init(DisplayState *ds, bool full_screen, bool grab_on_hover);
+void early_gtk_display_init(DisplayOptions *opts);
+void gtk_display_init(DisplayState *ds, DisplayOptions *opts);
 #else
-static inline void gtk_display_init(DisplayState *ds, bool full_screen,
-bool grab_on_hover)
+static inline void gtk_display_init(DisplayState *ds, DisplayOptions *opts)
 {
 /* This must never be called if CONFIG_GTK is disabled */
 error_report("GTK support is disabled");
 abort();
 }
 
-static inline void early_gtk_display_init(int opengl)
+static inline void early_gtk_display_init(DisplayOptions *opts)
 {
 /* This must never be called if CONFIG_GTK is disabled */
 error_report("GTK support is disabled");
diff --git a/ui/gtk.c b/ui/gtk.c
index f0ad63e431..c12d5e020c 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -229,6 +229,8 @@ struct GtkDisplayState {
 
 bool modifier_pressed[ARRAY_SIZE(modifier_keycode)];
 bool ignore_keys;
+
+DisplayOptions *opts;
 };
 
 typedef struct VCChardev {
@@ -777,9 +779,14 @@ static gboolean gd_window_close(GtkWidget *widget, 
GdkEvent *event,
 void *opaque)
 {
 GtkDisplayState *s = opaque;
+bool allow_close = true;
 int i;
 
-if (!no_quit) {
+if (s->opts->has_window_close && !s->opts->window_close) {
+allow_close = false;
+}
+
+if (allow_close) {
 for (i = 0; i < s->nb_vcs; i++) {
 if (s->vc[i].type != GD_VC_GFX) {
 continue;
@@ -2289,7 +2296,7 @@ static void gd_create_menus(GtkDisplayState *s)
 
 static gboolean gtkinit;
 
-void gtk_display_init(DisplayState *ds, bool full_screen, bool grab_on_hover)
+void gtk_display_init(DisplayState *ds, DisplayOptions *opts)
 {
 VirtualConsole *vc;
 
@@ -2301,6 +2308,8 @@ void gtk_display_init(DisplayState *ds, bool full_screen, 
bool grab_on_hover)
 fprintf(stderr, "gtk initialization failed\n");
 exit(1);
 }
+assert(opts->type == DISPLAY_TYPE_GTK);
+s->opts = opts;
 
 #if !GTK_CHECK_VERSION(3, 0, 0)
 g_printerr("Running QEMU with GTK 2.x is deprecated, and will be removed\n"
@@ -2387,15 +2396,17 @@ void gtk_display_init(DisplayState *ds, bool 
full_screen, bool grab_on_hover)
  vc && vc->type == GD_VC_VTE);
 #endif
 
-if (full_screen) {
+if (opts->has_full_screen &&
+opts->full_screen) {
 gtk_menu_item_activate(GTK_MENU_ITEM(s->full_screen_item));
 }
-if (grab_on_hover) {
+if (opts->u.gtk.has_grab_on_hover &&
+opts->u.gtk.grab_on_hover) {
 gtk_menu_item_activate(GTK_MENU_ITEM(s->grab_on_hover_item));
 }
 }
 
-void early_gtk_display_init(int opengl)
+void early_gtk_display_init(DisplayOptions *opts)
 {
 /* The QEMU code relies on the assumption that it's always run in
  * the C locale. Therefore it is not prepared to deal with
@@ -2421,11 +2432,8 @@ void early_gtk_display_init(int opengl)
 return;
 }
 
-switch (opengl) {
-case -1: /* default */
-case 0:  /* off */
-break;
-case 1: /* on */
+assert(opts->type == DISPLAY_TYPE_GTK);
+if (opts->has_gl && opts->gl) {
 #if defined(CONFIG_OPENGL)
 #if defined(CONFIG_GTK_GL)
 gtk_gl_area_init();
@@ -2433,10 +2441,6 @@ void early_gtk_display_init(int opengl)
 gtk_egl_init();
 #endif
 #endif
-break;
-default:
-g_assert_not_reached();
-break;
 }
 
 keycode_map = gd_get_keymap(&keycode_maplen);
diff --git a/vl.c b/vl.c
index a2478412c7..4a555de0cf 100644
--- a/vl.c
+++ b/vl.c
@@ -150,9 +150,9 @@ static int rtc_date_offset = -1; /* -1 means no change */
 QEMUClockType rtc_clock;
 int vga_interface_type = VGA_NONE;
 static int full_screen = 0;
+static DisplayOptions dpy;
 int no_frame;
 int no_quit = 0;
-static bool grab_on_hover;
 Chardev *serial_hds[MAX_SERIAL_PORTS];
 Chardev *parallel_hds[MAX_PARALLEL_PORTS];
 Chardev *virtcon_hds[MAX_VIRTIO_CONSOLES];
@@ -2191,24 +2191,29 @@ static LegacyDisplayType select_display(const char *p)
 } else if (strstart(p, "gt