qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

RE: [PATCH v4] ui/gtk: New -display gtk option 'full-screen-on-monitor'.


From: Romli, Khairul Anuar
Subject: RE: [PATCH v4] ui/gtk: New -display gtk option 'full-screen-on-monitor'.
Date: Fri, 25 Jun 2021 04:28:26 +0000

Hi Swee Aun,

I have some comment on the patch.

> -----Original Message-----
> From: Khor, Swee Aun <swee.aun.khor@intel.com>
> Sent: Thursday, June 24, 2021 4:43 PM
> To: qemu-devel@nongnu.org
> Cc: Khor, Swee Aun <swee.aun.khor@intel.com>; kraxel@redhat.com;
> armbru@redhat.com; eblake@redhat.com; Romli, Khairul Anuar
> <khairul.anuar.romli@intel.com>; Kasireddy, Vivek
> <vivek.kasireddy@intel.com>; Mazlan, Hazwan Arif
> <hazwan.arif.mazlan@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.khor@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




reply via email to

[Prev in Thread] Current Thread [Next in Thread]