From: "Daniel P. Berrangé" <berrange@redhat.com>
To: Dongwon Kim <dongwon.kim@intel.com>
Cc: qemu-devel@nongnu.org, philmd@redhat.com, kraxel@redhat.com,
pbonzini@redhat.com, Vivek Kasireddy <vivek.kasireddy@intel.com>
Subject: Re: [PATCH 3/3] ui/gtk: specify detached window's size and location
Date: Fri, 6 May 2022 17:34:21 +0100 [thread overview]
Message-ID: <YnVODWc9uq8sx+Cu@redhat.com> (raw)
In-Reply-To: <20220503233348.GA382@dongwonk-MOBL.amr.corp.intel.com>
On Tue, May 03, 2022 at 04:33:48PM -0700, Dongwon Kim wrote:
> I saw windows, especially, third and fourth ones are 1/4 size of
> the first when detached regardless of resolutions.
>
> And the position is also pretty random and detached windows are usually
> placed somewhere on the previous window.
>
> This patch is to make the sizes same as the original window's and make
> sure all detached windows are not overlapped each other.
In terms of size, I think you need to just honour the surface
size like this:
@@ -1354,6 +1354,9 @@ static void gd_menu_untabify(GtkMenuItem *item, void *opaque)
g_signal_connect(vc->window, "delete-event",
G_CALLBACK(gd_tab_window_close), vc);
+ gtk_window_set_default_size(GTK_WINDOW(vc->window),
+ surface_width(vc->gfx.ds),
+ surface_height(vc->gfx.ds));
gtk_widget_show_all(vc->window);
if (qemu_console_is_graphic(vc->gfx.dcl.con)) {
for position, I don't think we should be overriding the window
manager placement, as the logic applied could result in us
placing windows off screen.
>
> On Tue, May 03, 2022 at 10:17:46AM +0100, Daniel P. Berrangé wrote:
> > On Thu, Apr 28, 2022 at 04:13:04PM -0700, Dongwon Kim wrote:
> > > Specify location and size of detached window based on top level
> > > window's location and size info when detachment happens.
> >
> > Can you explain what problem is being solved by this change ?
> > What's wrong with default size/placement logic ?
> >
> > In terms of size at least, I would hope we are resizing
> > windows any time the guest changes the resolution of the
> > virtual video adapter. If there are 2 outputs, they can
> > be at different resolution, so copying the size of the
> > existing window feels wrong - we need to copy the guest
> > resolution currently set.
> >
> > Why do we need to mess around with position at all ?
> >
> > > 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>
> > > ---
> > > ui/gtk.c | 13 +++++++++++++
> > > 1 file changed, 13 insertions(+)
> > >
> > > diff --git a/ui/gtk.c b/ui/gtk.c
> > > index f1ca6a7275..7dadf3b588 100644
> > > --- a/ui/gtk.c
> > > +++ b/ui/gtk.c
> > > @@ -1338,6 +1338,8 @@ static void gd_menu_untabify(GtkMenuItem *item, void *opaque)
> > > FALSE);
> > > }
> > > if (!vc->window) {
> > > + gint x, y, w, h;
> > > + int i;
> > > gtk_widget_set_sensitive(vc->menu_item, false);
> > > vc->window = gtk_window_new(GTK_WINDOW_TOPLEVEL);
> > > #if defined(CONFIG_OPENGL)
> > > @@ -1351,7 +1353,18 @@ static void gd_menu_untabify(GtkMenuItem *item, void *opaque)
> > > }
> > > #endif
> > > gd_widget_reparent(s->notebook, vc->window, vc->tab_item);
> > > + gtk_window_get_position(GTK_WINDOW(s->window), &x, &y);
> > > + gtk_window_get_size(GTK_WINDOW(s->window), &w, &h);
> > > +
> > > + for (i = 0; i < s->nb_vcs; i++) {
> > > + if (vc == &s->vc[i]) {
> > > + break;
> > > + }
> > > + }
> > >
> > > + gtk_window_move(GTK_WINDOW(vc->window),
> > > + x + w * (i % (s->nb_vcs/2) + 1), y + h * (i / (s->nb_vcs/2)));
> > > + gtk_window_resize(GTK_WINDOW(vc->window), w, h);
> > > g_signal_connect(vc->window, "delete-event",
> > > G_CALLBACK(gd_tab_window_close), vc);
> > > gtk_widget_show_all(vc->window);
> > > --
> > > 2.30.2
> > >
> > >
> >
> > 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 :|
> >
>
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 :|
next prev parent reply other threads:[~2022-05-06 16:36 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-04-28 23:13 [PATCH 0/3] ui/gtk: new options, monitor and detach-all Dongwon Kim
2022-04-28 23:13 ` [PATCH 1/3] ui/gtk: new param monitor to specify target monitor for launching QEMU Dongwon Kim
2022-05-03 9:15 ` Daniel P. Berrangé
2022-05-03 23:14 ` Dongwon Kim
2022-05-09 21:31 ` Dongwon Kim
2022-05-10 10:58 ` Gerd Hoffmann
2022-05-17 7:46 ` Markus Armbruster
2022-04-28 23:13 ` [PATCH 2/3] ui/gtk: detach_all option for making all VCs detached upon starting Dongwon Kim
2022-05-03 9:12 ` Daniel P. Berrangé
2022-05-03 23:21 ` Dongwon Kim
2022-05-04 8:28 ` Daniel P. Berrangé
2022-04-28 23:13 ` [PATCH 3/3] ui/gtk: specify detached window's size and location Dongwon Kim
2022-05-03 9:17 ` Daniel P. Berrangé
2022-05-03 23:33 ` Dongwon Kim
2022-05-06 16:34 ` Daniel P. Berrangé [this message]
2022-05-06 17:05 ` Dongwon Kim
2022-05-31 20:33 ` [PATCH 0/3] ui/gtk: new options, monitor and detach-all Dongwon Kim
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=YnVODWc9uq8sx+Cu@redhat.com \
--to=berrange@redhat.com \
--cc=dongwon.kim@intel.com \
--cc=kraxel@redhat.com \
--cc=pbonzini@redhat.com \
--cc=philmd@redhat.com \
--cc=qemu-devel@nongnu.org \
--cc=vivek.kasireddy@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).