All of lore.kernel.org
 help / color / mirror / Atom feed
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 :|



  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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.