| Message ID | 20220615231942.29981-3-dongwon.kim@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | handling guest multiple displays | expand |
Dongwon Kim <dongwon.kim@intel.com> writes: > New integer array parameter, 'monitor' is for specifying the target > displays where individual QEMU windows are placed upon launching. > > The array contains a series of numbers representing the monitor where > QEMU windows are placed. > > Numbers in the array are mapped to QEMU windows like, > > [1st detached window, 2nd detached window,.... Main window] > > Usage example: -display gtk,monitor.0=0,monitor.1=1..... > > Cc: Daniel P. Berrangé <berrange@redhat.com> > Cc: Markus Armbruster <armbru@redhat.com> > Cc: Philippe Mathieu-Daudé <philmd@redhat.com> > Cc: Paolo Bonzini <pbonzini@redhat.com> > Cc: Gerd Hoffmann <kraxel@redhat.com> > Cc: Vivek Kasireddy <vivek.kasireddy@intel.com> > Signed-off-by: Dongwon Kim <dongwon.kim@intel.com> > --- > qapi/ui.json | 7 ++++++- > qemu-options.hx | 2 +- > ui/gtk.c | 32 +++++++++++++++++++++++++++++++- > 3 files changed, 38 insertions(+), 3 deletions(-) > > diff --git a/qapi/ui.json b/qapi/ui.json > index 413371d5e8..5980f30c7f 100644 > --- a/qapi/ui.json > +++ b/qapi/ui.json > @@ -1195,12 +1195,17 @@ > # assuming the guest will resize the display to match > # the window size then. Otherwise it defaults to "off". > # Since 3.1 > +# @monitor: Array of numbers, each of which represents physical > +# monitor where individual QEMU window is placed in case > +# there are multiple of them End you sentence with a period, and ... > +# since 7.1 ... start the next phrase with a capital letter. The documentation text feels vague. Possibly because I lack familiarity with this part of the user interface. What are the "individual QEMU windows"? How are they numbered? > # > # Since: 2.12 > ## > { 'struct' : 'DisplayGTK', > 'data' : { '*grab-on-hover' : 'bool', > - '*zoom-to-fit' : 'bool' } } > + '*zoom-to-fit' : 'bool', > + '*monitor' : ['uint16'] } } > > ## > # @DisplayEGLHeadless: > diff --git a/qemu-options.hx b/qemu-options.hx > index 377d22fbd8..f79f533e9d 100644 > --- a/qemu-options.hx > +++ b/qemu-options.hx > @@ -1938,7 +1938,7 @@ DEF("display", HAS_ARG, QEMU_OPTION_display, > #endif > #if defined(CONFIG_GTK) > "-display gtk[,full-screen=on|off][,gl=on|off][,grab-on-hover=on|off]\n" > - " [,show-cursor=on|off][,window-close=on|off]\n" > + " [,monitor.<order>=<value>][,show-cursor=on|off][,window-close=on|off]\n" > #endif > #if defined(CONFIG_VNC) > "-display vnc=<display>[,<optargs>]\n" > diff --git a/ui/gtk.c b/ui/gtk.c > index e6878c3209..fc9bf04680 100644 > --- a/ui/gtk.c > +++ b/ui/gtk.c > @@ -2316,6 +2316,10 @@ static void gtk_display_init(DisplayState *ds, DisplayOptions *opts) > GtkDisplayState *s = g_malloc0(sizeof(*s)); > GdkDisplay *window_display; > GtkIconTheme *theme; > + GtkWidget *win; > + GdkRectangle dest; > + uint16List *mon; > + int n_mon; > int i; > char *dir; > > @@ -2393,7 +2397,33 @@ static void gtk_display_init(DisplayState *ds, DisplayOptions *opts) > gtk_menu_item_activate(GTK_MENU_ITEM(s->untabify_item)); > } > } > - if (opts->has_full_screen && > + if (opts->u.gtk.has_monitor) { > + i = 0; > + n_mon = gdk_display_get_n_monitors(window_display); > + for (mon = opts->u.gtk.monitor; mon; mon = mon->next) { > + if (mon->value < n_mon) { > + for (; i < s->nb_vcs; i++) { > + win = s->vc[i].window ? s->vc[i].window : s->window; > + if (opts->has_full_screen && opts->full_screen) { > + gtk_window_fullscreen_on_monitor( > + GTK_WINDOW(win), > + gdk_display_get_default_screen(window_display), > + mon->value); > + } else { > + gdk_monitor_get_geometry( > + gdk_display_get_monitor(window_display, mon->value), > + &dest); > + gtk_window_move(GTK_WINDOW(win), > + dest.x, dest.y); > + } > + i++; > + break; > + } This loop is odd. It's of the form for (; COND; STEP) { ... break; } STEP is unreachable. The whole thing boils down to if (COND) { .... } doesn't it? > + } > + } > + } > + if (!opts->u.gtk.has_monitor && > + opts->has_full_screen && > opts->full_screen) { > gtk_menu_item_activate(GTK_MENU_ITEM(s->full_screen_item)); > } This is if (COND1) { ... } if (!COND1 && COND2) { ... } I'd prefer if (COND1) { ... } else if (COND2) { ... }
Hi Markus, On Mon, Jun 20, 2022 at 09:07:04AM +0200, Markus Armbruster wrote: > Dongwon Kim <dongwon.kim@intel.com> writes: > > > New integer array parameter, 'monitor' is for specifying the target > > displays where individual QEMU windows are placed upon launching. > > > > The array contains a series of numbers representing the monitor where > > QEMU windows are placed. > > > > Numbers in the array are mapped to QEMU windows like, > > > > [1st detached window, 2nd detached window,.... Main window] > > > > Usage example: -display gtk,monitor.0=0,monitor.1=1..... > > > > Cc: Daniel P. Berrangé <berrange@redhat.com> > > Cc: Markus Armbruster <armbru@redhat.com> > > Cc: Philippe Mathieu-Daudé <philmd@redhat.com> > > Cc: Paolo Bonzini <pbonzini@redhat.com> > > Cc: Gerd Hoffmann <kraxel@redhat.com> > > Cc: Vivek Kasireddy <vivek.kasireddy@intel.com> > > Signed-off-by: Dongwon Kim <dongwon.kim@intel.com> > > --- > > qapi/ui.json | 7 ++++++- > > qemu-options.hx | 2 +- > > ui/gtk.c | 32 +++++++++++++++++++++++++++++++- > > 3 files changed, 38 insertions(+), 3 deletions(-) > > > > diff --git a/qapi/ui.json b/qapi/ui.json > > index 413371d5e8..5980f30c7f 100644 > > --- a/qapi/ui.json > > +++ b/qapi/ui.json > > @@ -1195,12 +1195,17 @@ > > # assuming the guest will resize the display to match > > # the window size then. Otherwise it defaults to "off". > > # Since 3.1 > > +# @monitor: Array of numbers, each of which represents physical > > +# monitor where individual QEMU window is placed in case > > +# there are multiple of them > > End you sentence with a period, and ... > > > +# since 7.1 > > ... start the next phrase with a capital letter. > > The documentation text feels vague. Possibly because I lack familiarity > with this part of the user interface. What are the "individual QEMU > windows"? How are they numbered? > Will rework on the phrase. So there is only one QEMU window normally when you start the guest os. And this window usually contains multiple virtual consoles by default. You can detach any of them by clicking detach menu item from UI. In this case, a new window will be created for the detached VC. By doing this, we could have primary window and newly created n number of windows. Individual windows means these windows. In this patch, VC number is being used as windows number. The primary one - guest display is '0'. The guest's secondary display will be '1' and so on. Thanks for the feedback. > > # > > # Since: 2.12 > > ## > > { 'struct' : 'DisplayGTK', > > 'data' : { '*grab-on-hover' : 'bool', > > - '*zoom-to-fit' : 'bool' } } > > + '*zoom-to-fit' : 'bool', > > + '*monitor' : ['uint16'] } } > > > > ## > > # @DisplayEGLHeadless: > > diff --git a/qemu-options.hx b/qemu-options.hx > > index 377d22fbd8..f79f533e9d 100644 > > --- a/qemu-options.hx > > +++ b/qemu-options.hx > > @@ -1938,7 +1938,7 @@ DEF("display", HAS_ARG, QEMU_OPTION_display, > > #endif > > #if defined(CONFIG_GTK) > > "-display gtk[,full-screen=on|off][,gl=on|off][,grab-on-hover=on|off]\n" > > - " [,show-cursor=on|off][,window-close=on|off]\n" > > + " [,monitor.<order>=<value>][,show-cursor=on|off][,window-close=on|off]\n" > > #endif > > #if defined(CONFIG_VNC) > > "-display vnc=<display>[,<optargs>]\n" > > diff --git a/ui/gtk.c b/ui/gtk.c > > index e6878c3209..fc9bf04680 100644 > > --- a/ui/gtk.c > > +++ b/ui/gtk.c > > @@ -2316,6 +2316,10 @@ static void gtk_display_init(DisplayState *ds, DisplayOptions *opts) > > GtkDisplayState *s = g_malloc0(sizeof(*s)); > > GdkDisplay *window_display; > > GtkIconTheme *theme; > > + GtkWidget *win; > > + GdkRectangle dest; > > + uint16List *mon; > > + int n_mon; > > int i; > > char *dir; > > > > @@ -2393,7 +2397,33 @@ static void gtk_display_init(DisplayState *ds, DisplayOptions *opts) > > gtk_menu_item_activate(GTK_MENU_ITEM(s->untabify_item)); > > } > > } > > - if (opts->has_full_screen && > > + if (opts->u.gtk.has_monitor) { > > + i = 0; > > + n_mon = gdk_display_get_n_monitors(window_display); > > + for (mon = opts->u.gtk.monitor; mon; mon = mon->next) { > > + if (mon->value < n_mon) { > > + for (; i < s->nb_vcs; i++) { > > + win = s->vc[i].window ? s->vc[i].window : s->window; > > + if (opts->has_full_screen && opts->full_screen) { > > + gtk_window_fullscreen_on_monitor( > > + GTK_WINDOW(win), > > + gdk_display_get_default_screen(window_display), > > + mon->value); > > + } else { > > + gdk_monitor_get_geometry( > > + gdk_display_get_monitor(window_display, mon->value), > > + &dest); > > + gtk_window_move(GTK_WINDOW(win), > > + dest.x, dest.y); > > + } > > + i++; > > + break; > > + } > > This loop is odd. It's of the form > > for (; COND; STEP) { > ... > break; > } > > STEP is unreachable. The whole thing boils down to > > if (COND) { > .... > } > > doesn't it? You are definitely right. if (COND) should be the one here. I will fix this in the next version. > > > + } > > + } > > + } > > + if (!opts->u.gtk.has_monitor && > > + opts->has_full_screen && > > opts->full_screen) { > > gtk_menu_item_activate(GTK_MENU_ITEM(s->full_screen_item)); > > } > > > This is > > if (COND1) { > ... > } > if (!COND1 && COND2) { > ... > } > > I'd prefer > > if (COND1) { > ... > } else if (COND2) { > ... > } > I will take a look at this as well.
diff --git a/qapi/ui.json b/qapi/ui.json index 413371d5e8..5980f30c7f 100644 --- a/qapi/ui.json +++ b/qapi/ui.json @@ -1195,12 +1195,17 @@ # assuming the guest will resize the display to match # the window size then. Otherwise it defaults to "off". # Since 3.1 +# @monitor: Array of numbers, each of which represents physical +# monitor where individual QEMU window is placed in case +# there are multiple of them +# since 7.1 # # Since: 2.12 ## { 'struct' : 'DisplayGTK', 'data' : { '*grab-on-hover' : 'bool', - '*zoom-to-fit' : 'bool' } } + '*zoom-to-fit' : 'bool', + '*monitor' : ['uint16'] } } ## # @DisplayEGLHeadless: diff --git a/qemu-options.hx b/qemu-options.hx index 377d22fbd8..f79f533e9d 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -1938,7 +1938,7 @@ DEF("display", HAS_ARG, QEMU_OPTION_display, #endif #if defined(CONFIG_GTK) "-display gtk[,full-screen=on|off][,gl=on|off][,grab-on-hover=on|off]\n" - " [,show-cursor=on|off][,window-close=on|off]\n" + " [,monitor.<order>=<value>][,show-cursor=on|off][,window-close=on|off]\n" #endif #if defined(CONFIG_VNC) "-display vnc=<display>[,<optargs>]\n" diff --git a/ui/gtk.c b/ui/gtk.c index e6878c3209..fc9bf04680 100644 --- a/ui/gtk.c +++ b/ui/gtk.c @@ -2316,6 +2316,10 @@ static void gtk_display_init(DisplayState *ds, DisplayOptions *opts) GtkDisplayState *s = g_malloc0(sizeof(*s)); GdkDisplay *window_display; GtkIconTheme *theme; + GtkWidget *win; + GdkRectangle dest; + uint16List *mon; + int n_mon; int i; char *dir; @@ -2393,7 +2397,33 @@ static void gtk_display_init(DisplayState *ds, DisplayOptions *opts) gtk_menu_item_activate(GTK_MENU_ITEM(s->untabify_item)); } } - if (opts->has_full_screen && + if (opts->u.gtk.has_monitor) { + i = 0; + n_mon = gdk_display_get_n_monitors(window_display); + for (mon = opts->u.gtk.monitor; mon; mon = mon->next) { + if (mon->value < n_mon) { + for (; i < s->nb_vcs; i++) { + win = s->vc[i].window ? s->vc[i].window : s->window; + if (opts->has_full_screen && opts->full_screen) { + gtk_window_fullscreen_on_monitor( + GTK_WINDOW(win), + gdk_display_get_default_screen(window_display), + mon->value); + } else { + gdk_monitor_get_geometry( + gdk_display_get_monitor(window_display, mon->value), + &dest); + gtk_window_move(GTK_WINDOW(win), + dest.x, dest.y); + } + i++; + break; + } + } + } + } + if (!opts->u.gtk.has_monitor && + opts->has_full_screen && opts->full_screen) { gtk_menu_item_activate(GTK_MENU_ITEM(s->full_screen_item)); }
New integer array parameter, 'monitor' is for specifying the target displays where individual QEMU windows are placed upon launching. The array contains a series of numbers representing the monitor where QEMU windows are placed. Numbers in the array are mapped to QEMU windows like, [1st detached window, 2nd detached window,.... Main window] Usage example: -display gtk,monitor.0=0,monitor.1=1..... Cc: Daniel P. Berrangé <berrange@redhat.com> Cc: Markus Armbruster <armbru@redhat.com> Cc: Philippe Mathieu-Daudé <philmd@redhat.com> Cc: Paolo Bonzini <pbonzini@redhat.com> Cc: Gerd Hoffmann <kraxel@redhat.com> Cc: Vivek Kasireddy <vivek.kasireddy@intel.com> Signed-off-by: Dongwon Kim <dongwon.kim@intel.com> --- qapi/ui.json | 7 ++++++- qemu-options.hx | 2 +- ui/gtk.c | 32 +++++++++++++++++++++++++++++++- 3 files changed, 38 insertions(+), 3 deletions(-)