diff mbox series

[1/3] ui/gtk: new param monitor to specify target monitor for launching QEMU

Message ID 20220428231304.19472-2-dongwon.kim@intel.com (mailing list archive)
State New, archived
Headers show
Series ui/gtk: new options, monitor and detach-all | expand

Commit Message

Kim, Dongwon April 28, 2022, 11:13 p.m. UTC
Introducing a new integer parameter to specify the monitor where the
Qemu window is placed upon launching.

Monitor can be any number between 0 and (total number of monitors - 1).

It can be used together with full-screen=on, which will make the QEMU
window full-screened on the targeted monitor.

v2: fixed typos and updated commit subject and msg
    (Philippe Mathieu-Daudé)

    changed param name to monitor, removed unnecessary condition check
    on the parameter
    (Paolo Bonzini)

v3: updated Since version to 7.1 for monitor parameter

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    | 6 +++++-
 qemu-options.hx | 2 +-
 ui/gtk.c        | 8 ++++++++
 3 files changed, 14 insertions(+), 2 deletions(-)

Comments

Daniel P. Berrangé May 3, 2022, 9:15 a.m. UTC | #1
On Thu, Apr 28, 2022 at 04:13:02PM -0700, Dongwon Kim wrote:
> Introducing a new integer parameter to specify the monitor where the
> Qemu window is placed upon launching.
> 
> Monitor can be any number between 0 and (total number of monitors - 1).
> 
> It can be used together with full-screen=on, which will make the QEMU
> window full-screened on the targeted monitor.
> 
> v2: fixed typos and updated commit subject and msg
>     (Philippe Mathieu-Daudé)
> 
>     changed param name to monitor, removed unnecessary condition check
>     on the parameter
>     (Paolo Bonzini)
> 
> v3: updated Since version to 7.1 for monitor parameter
> 
> 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    | 6 +++++-
>  qemu-options.hx | 2 +-
>  ui/gtk.c        | 8 ++++++++
>  3 files changed, 14 insertions(+), 2 deletions(-)
> 
> diff --git a/qapi/ui.json b/qapi/ui.json
> index 059302a5ef..ddcea7349b 100644
> --- a/qapi/ui.json
> +++ b/qapi/ui.json
> @@ -1204,13 +1204,17 @@
>  #               assuming the guest will resize the display to match
>  #               the window size then.  Otherwise it defaults to "off".
>  #               Since 3.1
> +# @monitor:     Indicate monitor where QEMU window is lauched. monitor
> +#               could be any number from 0 to (total num of monitors - 1).
> +#               since 7.1
>  #
>  # Since: 2.12
>  #
>  ##
>  { 'struct'  : 'DisplayGTK',
>    'data'    : { '*grab-on-hover' : 'bool',
> -                '*zoom-to-fit'   : 'bool'  } }
> +                '*zoom-to-fit'   : 'bool',
> +                '*monitor'       : 'uint32' } }

I feel like this ought to be an array of monitors, so that we can have
explicit positioning when we have multiple graphical outputs and are
creating a separate window for each.


With regards,
Daniel
Kim, Dongwon May 3, 2022, 11:14 p.m. UTC | #2
On Tue, May 03, 2022 at 10:15:13AM +0100, Daniel P. Berrangé wrote:
> On Thu, Apr 28, 2022 at 04:13:02PM -0700, Dongwon Kim wrote:
> > Introducing a new integer parameter to specify the monitor where the
> > Qemu window is placed upon launching.
> > 
> > Monitor can be any number between 0 and (total number of monitors - 1).
> > 
> > It can be used together with full-screen=on, which will make the QEMU
> > window full-screened on the targeted monitor.
> > 
> > v2: fixed typos and updated commit subject and msg
> >     (Philippe Mathieu-Daudé)
> > 
> >     changed param name to monitor, removed unnecessary condition check
> >     on the parameter
> >     (Paolo Bonzini)
> > 
> > v3: updated Since version to 7.1 for monitor parameter
> > 
> > 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    | 6 +++++-
> >  qemu-options.hx | 2 +-
> >  ui/gtk.c        | 8 ++++++++
> >  3 files changed, 14 insertions(+), 2 deletions(-)
> > 
> > diff --git a/qapi/ui.json b/qapi/ui.json
> > index 059302a5ef..ddcea7349b 100644
> > --- a/qapi/ui.json
> > +++ b/qapi/ui.json
> > @@ -1204,13 +1204,17 @@
> >  #               assuming the guest will resize the display to match
> >  #               the window size then.  Otherwise it defaults to "off".
> >  #               Since 3.1
> > +# @monitor:     Indicate monitor where QEMU window is lauched. monitor
> > +#               could be any number from 0 to (total num of monitors - 1).
> > +#               since 7.1
> >  #
> >  # Since: 2.12
> >  #
> >  ##
> >  { 'struct'  : 'DisplayGTK',
> >    'data'    : { '*grab-on-hover' : 'bool',
> > -                '*zoom-to-fit'   : 'bool'  } }
> > +                '*zoom-to-fit'   : 'bool',
> > +                '*monitor'       : 'uint32' } }
> 
> I feel like this ought to be an array of monitors, so that we can have
> explicit positioning when we have multiple graphical outputs and are
> creating a separate window for each.

That would be ideal but at the same time, wouldn't it make the option to
specific/complicated? And I am not sure how to create an option that takes the
data in the form of array. Do you have any reference?

> 
> 
> With regards,
> Daniel
> -- 
> |: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org         -o-            https://fstop138.berrange.com :|
> |: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|
>
Kim, Dongwon May 9, 2022, 9:31 p.m. UTC | #3
Daniel,

I found a way to make the monitor arguments in array type (['uint32']).
And I know how to retrieve monitor values from it but I could not find
how to pass the monitor values when starting qemu. Like,

qemu-system-x86_64 ..... gtk,gl=on.....monitor=????

I tried several different things but it keeps getting error saying
Invalid parameter type, expected 'array'.

Do you know how to pass this arg?

Thanks,
DW

On Tue, May 03, 2022 at 10:15:13AM +0100, Daniel P. Berrangé wrote:
> On Thu, Apr 28, 2022 at 04:13:02PM -0700, Dongwon Kim wrote:
> > Introducing a new integer parameter to specify the monitor where the
> > Qemu window is placed upon launching.
> > 
> > Monitor can be any number between 0 and (total number of monitors - 1).
> > 
> > It can be used together with full-screen=on, which will make the QEMU
> > window full-screened on the targeted monitor.
> > 
> > v2: fixed typos and updated commit subject and msg
> >     (Philippe Mathieu-Daudé)
> > 
> >     changed param name to monitor, removed unnecessary condition check
> >     on the parameter
> >     (Paolo Bonzini)
> > 
> > v3: updated Since version to 7.1 for monitor parameter
> > 
> > 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    | 6 +++++-
> >  qemu-options.hx | 2 +-
> >  ui/gtk.c        | 8 ++++++++
> >  3 files changed, 14 insertions(+), 2 deletions(-)
> > 
> > diff --git a/qapi/ui.json b/qapi/ui.json
> > index 059302a5ef..ddcea7349b 100644
> > --- a/qapi/ui.json
> > +++ b/qapi/ui.json
> > @@ -1204,13 +1204,17 @@
> >  #               assuming the guest will resize the display to match
> >  #               the window size then.  Otherwise it defaults to "off".
> >  #               Since 3.1
> > +# @monitor:     Indicate monitor where QEMU window is lauched. monitor
> > +#               could be any number from 0 to (total num of monitors - 1).
> > +#               since 7.1
> >  #
> >  # Since: 2.12
> >  #
> >  ##
> >  { 'struct'  : 'DisplayGTK',
> >    'data'    : { '*grab-on-hover' : 'bool',
> > -                '*zoom-to-fit'   : 'bool'  } }
> > +                '*zoom-to-fit'   : 'bool',
> > +                '*monitor'       : 'uint32' } }
> 
> I feel like this ought to be an array of monitors, so that we can have
> explicit positioning when we have multiple graphical outputs and are
> creating a separate window for each.
> 
> 
> With regards,
> Daniel
> -- 
> |: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org         -o-            https://fstop138.berrange.com :|
> |: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|
>
Gerd Hoffmann May 10, 2022, 10:58 a.m. UTC | #4
On Mon, May 09, 2022 at 02:31:05PM -0700, Dongwon Kim wrote:
> Daniel,
> 
> I found a way to make the monitor arguments in array type (['uint32']).
> And I know how to retrieve monitor values from it but I could not find
> how to pass the monitor values when starting qemu. Like,
> 
> qemu-system-x86_64 ..... gtk,gl=on.....monitor=????
> 
> I tried several different things but it keeps getting error saying
> Invalid parameter type, expected 'array'.
> 
> Do you know how to pass this arg?

qemu accepts json for -display, that should work:

-display '{ "type": "gtk", "monitor": [ 1, 2 ] }'

Not sure whenever there is some way to specify arrays using
the -display gtk,$options style.

take care,
  Gerd
Markus Armbruster May 17, 2022, 7:46 a.m. UTC | #5
Gerd Hoffmann <kraxel@redhat.com> writes:

> On Mon, May 09, 2022 at 02:31:05PM -0700, Dongwon Kim wrote:
>> Daniel,
>> 
>> I found a way to make the monitor arguments in array type (['uint32']).
>> And I know how to retrieve monitor values from it but I could not find
>> how to pass the monitor values when starting qemu. Like,
>> 
>> qemu-system-x86_64 ..... gtk,gl=on.....monitor=????
>> 
>> I tried several different things but it keeps getting error saying
>> Invalid parameter type, expected 'array'.
>> 
>> Do you know how to pass this arg?
>
> qemu accepts json for -display, that should work:
>
> -display '{ "type": "gtk", "monitor": [ 1, 2 ] }'
>
> Not sure whenever there is some way to specify arrays using
> the -display gtk,$options style.

There is, but it's somewhat ugly:

    -display gtk,monitor.0=1,monitor.1=2

See util/keyval.c.
diff mbox series

Patch

diff --git a/qapi/ui.json b/qapi/ui.json
index 059302a5ef..ddcea7349b 100644
--- a/qapi/ui.json
+++ b/qapi/ui.json
@@ -1204,13 +1204,17 @@ 
 #               assuming the guest will resize the display to match
 #               the window size then.  Otherwise it defaults to "off".
 #               Since 3.1
+# @monitor:     Indicate monitor where QEMU window is lauched. monitor
+#               could be any number from 0 to (total num of monitors - 1).
+#               since 7.1
 #
 # Since: 2.12
 #
 ##
 { 'struct'  : 'DisplayGTK',
   'data'    : { '*grab-on-hover' : 'bool',
-                '*zoom-to-fit'   : 'bool'  } }
+                '*zoom-to-fit'   : 'bool',
+                '*monitor'       : 'uint32' } }
 
 ##
 # @DisplayEGLHeadless:
diff --git a/qemu-options.hx b/qemu-options.hx
index 5f69b94b8e..dbf9b223dd 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -1908,7 +1908,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=<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 c57c36749e..d9971d65ac 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -2375,6 +2375,14 @@  static void gtk_display_init(DisplayState *ds, DisplayOptions *opts)
                              vc && vc->type == GD_VC_VTE);
 #endif
 
+    if (opts->u.gtk.has_monitor &&
+        opts->u.gtk.monitor < gdk_display_get_n_monitors(window_display)) {
+        GdkRectangle mon_dest;
+        gdk_monitor_get_geometry(
+            gdk_display_get_monitor(window_display, opts->u.gtk.monitor),
+            &mon_dest);
+        gtk_window_move(GTK_WINDOW(s->window), mon_dest.x, mon_dest.y);
+    }
     if (opts->has_full_screen &&
         opts->full_screen) {
         gtk_menu_item_activate(GTK_MENU_ITEM(s->full_screen_item));