Hi Swee Aun, I have some comment on the patch.
> -----Original Message----- > From: Khor, Swee Aun <swee.aun.k...@intel.com> > Sent: Thursday, June 24, 2021 4:43 PM > To: qemu-devel@nongnu.org > Cc: Khor, Swee Aun <swee.aun.k...@intel.com>; kra...@redhat.com; > arm...@redhat.com; ebl...@redhat.com; Romli, Khairul Anuar > <khairul.anuar.ro...@intel.com>; Kasireddy, Vivek > <vivek.kasire...@intel.com>; Mazlan, Hazwan Arif > <hazwan.arif.maz...@intel.com>; Khor > Subject: [PATCH v4] ui/gtk: New -display gtk option 'full-screen-on-monitor'. > > This lets user select monitor number to display QEMU in full screen with - > display gtk,full-screen-on-monitor=<value>. > > v2: > - Added documentation for new member. > - Renamed member name from monitor-num to monitor. > > v3: > - Cleaned up commit message subject and signed-off format. > - Renamed member name from monitor to full-screen-on-monitor to make > clear this option automatically enables full screen. > - Added more detail documentation to specify full-screen-on-monitor option > index started from 1. > - Added code to check windows has been launched successfully at specified > monitor. > > v4: > - Used PRId64 format specifier for int64_t variable in warn_report(). > > Signed-off-by: Khor, Swee Aun <swee.aun.k...@intel.com> > --- > qapi/ui.json | 10 +++++++--- > qemu-options.hx | 2 +- > ui/gtk.c | 35 +++++++++++++++++++++++++++++++++++ > 3 files changed, 43 insertions(+), 4 deletions(-) > > diff --git a/qapi/ui.json b/qapi/ui.json index 1052ca9c38..d775c29534 100644 > --- a/qapi/ui.json > +++ b/qapi/ui.json > @@ -1035,13 +1035,17 @@ > # assuming the guest will resize the display to match > # the window size then. Otherwise it defaults to "off". > # Since 3.1 > -# > +# @full-screen-on-monitor: Monitor number to display QEMU in full screen. > +# Monitor number started from index 1. If total > number > +# of monitors is 3, possible values for this option > are > +# 1, 2 or 3. > # Since: 2.12 > # > ## > { 'struct' : 'DisplayGTK', > - 'data' : { '*grab-on-hover' : 'bool', > - '*zoom-to-fit' : 'bool' } } > + 'data' : { '*grab-on-hover' : 'bool', > + '*zoom-to-fit' : 'bool', > + '*full-screen-on-monitor' : 'int' } } > > ## > # @DisplayEGLHeadless: > diff --git a/qemu-options.hx b/qemu-options.hx index > 14258784b3..29836db663 100644 > --- a/qemu-options.hx > +++ b/qemu-options.hx > @@ -1787,7 +1787,7 @@ DEF("display", HAS_ARG, QEMU_OPTION_display, > " [,window_close=on|off][,gl=on|core|es|off]\n" > #endif > #if defined(CONFIG_GTK) > - "-display gtk[,grab_on_hover=on|off][,gl=on|off]|\n" > + "-display gtk[,grab-on-hover=on|off][,gl=on|off][,full-screen-on- > monitor=<value>]\n" > #endif > #if defined(CONFIG_VNC) > "-display vnc=<display>[,<optargs>]\n" > diff --git a/ui/gtk.c b/ui/gtk.c > index 98046f577b..255f25cabd 100644 > --- a/ui/gtk.c > +++ b/ui/gtk.c > @@ -2189,6 +2189,10 @@ static void gtk_display_init(DisplayState *ds, > DisplayOptions *opts) > GdkDisplay *window_display; > GtkIconTheme *theme; > char *dir; > + int monitor_n; > + GdkScreen *gdk_screen; > + GdkMonitor *gdk_monitor; > + bool monitor_status = false; > > if (!gtkinit) { > fprintf(stderr, "gtk initialization failed\n"); @@ -2268,6 +2272,37 > @@ > static void gtk_display_init(DisplayState *ds, DisplayOptions *opts) > gtk_menu_item_activate(GTK_MENU_ITEM(s->grab_on_hover_item)); > } > gd_clipboard_init(s); > + > + if (opts->u.gtk.has_full_screen_on_monitor) { > + monitor_n = gdk_display_get_n_monitors(window_display); > + > + if (opts->u.gtk.full_screen_on_monitor <= monitor_n && > + opts->u.gtk.full_screen_on_monitor > 0) { > + gdk_screen = gdk_display_get_default_screen(window_display); > + gtk_window_fullscreen_on_monitor(GTK_WINDOW(s->window), > gdk_screen, > + > opts->u.gtk.full_screen_on_monitor > + - 1); > + > + gdk_monitor = gdk_display_get_monitor(window_display, > + > opts->u.gtk.full_screen_on_monitor > + - 1); > + if (gdk_monitor != NULL) { > + monitor_status = true; [Romli, Khairul Anuar] Do you think we should use gdk_display_get_monitor inside the if check against the NULL value rather than using a variable? Indeed that with cause some code readability difficulty but I don't see gdk_monitor is being used beyond this check. > + } > + } > + > + if (monitor_status == false) { > + /* > + * Calling GDK API to read the number of monitors again in case > + * monitor added or removed since last read. > + */ > + monitor_n = gdk_display_get_n_monitors(window_display); > + warn_report("Failed to enable full screen on monitor %" PRId64 > ". " > + "Current total number of monitors is %d, " > + "please verify full-screen-on-monitor option value.", > + opts->u.gtk.full_screen_on_monitor, monitor_n); > + } > + } > } > > static void early_gtk_display_init(DisplayOptions *opts) > -- > 2.24.3