On Thu, Jun 30, 2022 at 05:19:26PM +0200, Markus Armbruster wrote:
> Dongwon Kim <dongwon....@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é <berra...@redhat.com>
> > Cc: Markus Armbruster <arm...@redhat.com>
> > Cc: Philippe Mathieu-Daudé <phi...@redhat.com>
> > Cc: Paolo Bonzini <pbonz...@redhat.com>
> > Cc: Gerd Hoffmann <kra...@redhat.com>
> > Cc: Vivek Kasireddy <vivek.kasire...@intel.com>
> > Signed-off-by: Dongwon Kim <dongwon....@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));
>
