| Message ID | 20220630005141.16680-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 > monitors where individual GTK windows are placed upon launching. > > Monitor numbers in the array are associated with virtual consoles > in the order of [VC0, VC1, VC2 ... VCn]. > > Every GTK window containing each VC will be placed in the region > of corresponding monitors. > > Usage: -display gtk,monitor.<id of VC>=<target monitor>,.. > ex)-display gtk,monitor.0=1,monitor.1=0 > > v3: - Revised commit message > - Rewrote desription of the new parameter (Markus Armbruster) > - Replaced unnecessary 'for' loop with 'if' condition > (Markus Armbruster) Again, patch history ... > 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> > --- ... goes here. > qapi/ui.json | 9 ++++++++- > qemu-options.hx | 3 ++- > ui/gtk.c | 31 +++++++++++++++++++++++++++++-- > 3 files changed, 39 insertions(+), 4 deletions(-) > > diff --git a/qapi/ui.json b/qapi/ui.json > index 413371d5e8..7b4c098bb4 100644 > --- a/qapi/ui.json > +++ b/qapi/ui.json > @@ -1195,12 +1195,19 @@ > # 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 GTK window containing a given VC will be > +# placed. Each monitor number in the array will be > +# associated with a virtual-console starting from VC0. Drop the hyphen in "virtual-console". Is the term "virtual console" obvious? Gerd? > +# > +# 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..aabdfb0636 100644 > --- a/qemu-options.hx > +++ b/qemu-options.hx > @@ -1938,7 +1938,8 @@ 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.<id of VC>=<monitor number>][,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..935176e614 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,10 +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 && > - opts->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 && i < s->nb_vcs) { > + 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++; > + } > + } > + } else if (opts->has_full_screen && > + opts->full_screen) { I'd join these two lines, like } else if (opts->has_full_screen && opts->full_screen) { or even exploit the fact that a opts->full_screen is false when absent } else if (opts->full_screen) { > gtk_menu_item_activate(GTK_MENU_ITEM(s->full_screen_item)); > } > + > 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));
Hi, > > +# @monitor: Array of numbers, each of which represents physical > > +# monitor where GTK window containing a given VC will be > > +# placed. Each monitor number in the array will be > > +# associated with a virtual-console starting from VC0. > > Drop the hyphen in "virtual-console". > > Is the term "virtual console" obvious? Gerd? I think so, same term is used elsewhere too for the same concept. take care, Gerd
Thanks, yeah, I will remove '-' and resubmitt the patch. On Fri, Jul 01, 2022 at 11:58:48AM +0200, Gerd Hoffmann wrote: > Hi, > > > > +# @monitor: Array of numbers, each of which represents physical > > > +# monitor where GTK window containing a given VC will be > > > +# placed. Each monitor number in the array will be > > > +# associated with a virtual-console starting from VC0. > > > > Drop the hyphen in "virtual-console". > > > > Is the term "virtual console" obvious? Gerd? > > I think so, same term is used elsewhere too for the same concept. > > take care, > Gerd >
On Thu, Jun 30, 2022 at 05:19:26PM +0200, Markus Armbruster wrote: > Dongwon Kim <dongwon.kim@intel.com> writes: > > > New integer array parameter, 'monitor' is for specifying the target > > monitors where individual GTK windows are placed upon launching. > > > > Monitor numbers in the array are associated with virtual consoles > > in the order of [VC0, VC1, VC2 ... VCn]. > > > > Every GTK window containing each VC will be placed in the region > > of corresponding monitors. > > > > Usage: -display gtk,monitor.<id of VC>=<target monitor>,.. > > ex)-display gtk,monitor.0=1,monitor.1=0 > > > > v3: - Revised commit message > > - Rewrote desription of the new parameter (Markus Armbruster) > > - Replaced unnecessary 'for' loop with 'if' condition > > (Markus Armbruster) > > Again, patch history ... > > 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> > > --- > > ... goes here. No problem moving down the version history but may I ask you if that is current rule? We don't want to include the history anymore in the git history? And FYI, the cover letter has the whole history already. I guess I can simply remove the history from individual patches then? Thanks!! > > > qapi/ui.json | 9 ++++++++- > > qemu-options.hx | 3 ++- > > ui/gtk.c | 31 +++++++++++++++++++++++++++++-- > > 3 files changed, 39 insertions(+), 4 deletions(-) > > > > diff --git a/qapi/ui.json b/qapi/ui.json > > index 413371d5e8..7b4c098bb4 100644 > > --- a/qapi/ui.json > > +++ b/qapi/ui.json > > @@ -1195,12 +1195,19 @@ > > # 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 GTK window containing a given VC will be > > +# placed. Each monitor number in the array will be > > +# associated with a virtual-console starting from VC0. > > Drop the hyphen in "virtual-console". > > Is the term "virtual console" obvious? Gerd? > I will do so. > > +# > > +# 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..aabdfb0636 100644 > > --- a/qemu-options.hx > > +++ b/qemu-options.hx > > @@ -1938,7 +1938,8 @@ 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.<id of VC>=<monitor number>][,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..935176e614 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,10 +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 && > > - opts->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 && i < s->nb_vcs) { > > + 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++; > > + } > > + } > > + } else if (opts->has_full_screen && > > + opts->full_screen) { > > I'd join these two lines, like > > } else if (opts->has_full_screen && opts->full_screen) { > > or even exploit the fact that a opts->full_screen is false when absent > > } else if (opts->full_screen) { This is simpler. I will go with this. > > > gtk_menu_item_activate(GTK_MENU_ITEM(s->full_screen_item)); > > } > > + > > 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)); >
Dongwon Kim <dongwon.kim@intel.com> writes: > On Thu, Jun 30, 2022 at 05:19:26PM +0200, Markus Armbruster wrote: >> Dongwon Kim <dongwon.kim@intel.com> writes: >> >> > New integer array parameter, 'monitor' is for specifying the target >> > monitors where individual GTK windows are placed upon launching. >> > >> > Monitor numbers in the array are associated with virtual consoles >> > in the order of [VC0, VC1, VC2 ... VCn]. >> > >> > Every GTK window containing each VC will be placed in the region >> > of corresponding monitors. >> > >> > Usage: -display gtk,monitor.<id of VC>=<target monitor>,.. >> > ex)-display gtk,monitor.0=1,monitor.1=0 >> > >> > v3: - Revised commit message >> > - Rewrote desription of the new parameter (Markus Armbruster) >> > - Replaced unnecessary 'for' loop with 'if' condition >> > (Markus Armbruster) >> >> Again, patch history ... >> >> > 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> >> > --- >> >> ... goes here. > > No problem moving down the version history but may I ask you if that > is current rule? We don't want to include the history anymore in the > git history? Patch history is really valuable for reviewers, but once the patch is done, it's rarely interesting anymore, so we keep it out of Git. Sometimes, bits of history are still useful to understand why the patch is done the way it is. Such bits should be worked into the commit message. Don't: v2: A replaced by B [J. Reviewer] Do: I initially tried A, but it turned out to be a bad idea because X, so I did B instead. Makes sense? > And FYI, the cover letter has the whole history already. I guess I can > simply remove the history from individual patches then? I like to keep detailed history in the cover letter. Others like to keep it in each patch. Still others like to keep an overview in the cover letter and details in each patch. All fine. > Thanks!! > >> >> > qapi/ui.json | 9 ++++++++- >> > qemu-options.hx | 3 ++- >> > ui/gtk.c | 31 +++++++++++++++++++++++++++++-- >> > 3 files changed, 39 insertions(+), 4 deletions(-) >> > >> > diff --git a/qapi/ui.json b/qapi/ui.json >> > index 413371d5e8..7b4c098bb4 100644 >> > --- a/qapi/ui.json >> > +++ b/qapi/ui.json >> > @@ -1195,12 +1195,19 @@ >> > # 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 GTK window containing a given VC will be >> > +# placed. Each monitor number in the array will be >> > +# associated with a virtual-console starting from VC0. >> >> Drop the hyphen in "virtual-console". >> >> Is the term "virtual console" obvious? Gerd? >> > > I will do so. Replace it by space, of course. [...]
diff --git a/qapi/ui.json b/qapi/ui.json index 413371d5e8..7b4c098bb4 100644 --- a/qapi/ui.json +++ b/qapi/ui.json @@ -1195,12 +1195,19 @@ # 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 GTK window containing a given VC will be +# placed. Each monitor number in the array will be +# associated with a virtual-console starting from VC0. +# +# 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..aabdfb0636 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -1938,7 +1938,8 @@ 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.<id of VC>=<monitor number>][,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..935176e614 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,10 +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 && - opts->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 && i < s->nb_vcs) { + 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++; + } + } + } else if (opts->has_full_screen && + opts->full_screen) { gtk_menu_item_activate(GTK_MENU_ITEM(s->full_screen_item)); } + 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));
New integer array parameter, 'monitor' is for specifying the target monitors where individual GTK windows are placed upon launching. Monitor numbers in the array are associated with virtual consoles in the order of [VC0, VC1, VC2 ... VCn]. Every GTK window containing each VC will be placed in the region of corresponding monitors. Usage: -display gtk,monitor.<id of VC>=<target monitor>,.. ex)-display gtk,monitor.0=1,monitor.1=0 v3: - Revised commit message - Rewrote desription of the new parameter (Markus Armbruster) - Replaced unnecessary 'for' loop with 'if' condition (Markus Armbruster) 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 | 9 ++++++++- qemu-options.hx | 3 ++- ui/gtk.c | 31 +++++++++++++++++++++++++++++-- 3 files changed, 39 insertions(+), 4 deletions(-)