diff mbox series

ui/gtk: Explicitly set the default size of new window when untabifying

Message ID 20240501034133.167321-1-dongwon.kim@intel.com (mailing list archive)
State New, archived
Headers show
Series ui/gtk: Explicitly set the default size of new window when untabifying | expand

Commit Message

Kim, Dongwon May 1, 2024, 3:41 a.m. UTC 
From: Dongwon Kim <dongwon.kim@intel.com>

When untabifying, the default size of the new window was inadvertently
set to the size smaller than quarter of the primary window size due
to lack of explicit configuration. This commit addresses the issue by
ensuring that the size of untabified windows is set to match the surface
size.

Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Marc-André Lureau <marcandre.lureau@redhat.com>
Cc: Vivek Kasireddy <vivek.kasireddy@intel.com>
Signed-off-by: Dongwon Kim <dongwon.kim@intel.com>
---
 ui/gtk.c | 3 +++
 1 file changed, 3 insertions(+)

Comments

Marc-André Lureau May 7, 2024, 3:10 p.m. UTC | #1 
Hi

On Wed, May 1, 2024 at 7:47 AM <dongwon.kim@intel.com> wrote:
>
> From: Dongwon Kim <dongwon.kim@intel.com>
>
> When untabifying, the default size of the new window was inadvertently
> set to the size smaller than quarter of the primary window size due
> to lack of explicit configuration. This commit addresses the issue by
> ensuring that the size of untabified windows is set to match the surface
> size.

From a quick test, I don't see a difference of behaviour after the
patch. Could you help me reproduce the issue?

I also don't think it is correct for two reasons:
- the inner display widget should cause a window size reconfiguration
- the window size != display size

thanks

> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Cc: Marc-André Lureau <marcandre.lureau@redhat.com>
> Cc: Vivek Kasireddy <vivek.kasireddy@intel.com>
> Signed-off-by: Dongwon Kim <dongwon.kim@intel.com>
> ---
>  ui/gtk.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/ui/gtk.c b/ui/gtk.c
> index 810d7fc796..269b8207d7 100644
> --- a/ui/gtk.c
> +++ b/ui/gtk.c
> @@ -1395,6 +1395,9 @@ static void gd_menu_untabify(GtkMenuItem *item, void *opaque)
>      if (!vc->window) {
>          gtk_widget_set_sensitive(vc->menu_item, false);
>          vc->window = gtk_window_new(GTK_WINDOW_TOPLEVEL);
> +        gtk_window_set_default_size(GTK_WINDOW(vc->window),
> +                                    surface_width(vc->gfx.ds),
> +                                    surface_height(vc->gfx.ds));
>  #if defined(CONFIG_OPENGL)
>          if (vc->gfx.esurface) {
>              eglDestroySurface(qemu_egl_display, vc->gfx.esurface);
> --
> 2.34.1
>
>
Kim, Dongwon May 7, 2024, 4:05 p.m. UTC | #2 
Hi Marc-André,

> -----Original Message-----
> From: Marc-André Lureau <marcandre.lureau@gmail.com>
> Sent: Tuesday, May 7, 2024 8:10 AM
> To: Kim, Dongwon <dongwon.kim@intel.com>
> Cc: qemu-devel@nongnu.org; kraxel@redhat.com
> Subject: Re: [PATCH] ui/gtk: Explicitly set the default size of new window when
> untabifying
> 
> Hi
> 
> On Wed, May 1, 2024 at 7:47 AM <dongwon.kim@intel.com> wrote:
> >
> > From: Dongwon Kim <dongwon.kim@intel.com>
> >
> > When untabifying, the default size of the new window was inadvertently
> > set to the size smaller than quarter of the primary window size due to
> > lack of explicit configuration. This commit addresses the issue by
> > ensuring that the size of untabified windows is set to match the
> > surface size.
> 
> From a quick test, I don't see a difference of behaviour after the patch. Could
> you help me reproduce the issue?
> 
> I also don't think it is correct for two reasons:
> - the inner display widget should cause a window size reconfiguration
> - the window size != display size

[Kim, Dongwon] Ok, I see this is happening only when virtio-vga device is used like
qemu-system-x86_64 -m 2048 -enable-kvm -cpu host -smp cores=2 -drive file=./OVMF.fd,format=raw,if=pflash -device virtio-vga -display gtk,gl=on
Maybe some setting of dimensions is missing in there? I will take a look.

> 
> thanks
> 
> > Cc: Gerd Hoffmann <kraxel@redhat.com>
> > Cc: Marc-André Lureau <marcandre.lureau@redhat.com>
> > Cc: Vivek Kasireddy <vivek.kasireddy@intel.com>
> > Signed-off-by: Dongwon Kim <dongwon.kim@intel.com>
> > ---
> >  ui/gtk.c | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/ui/gtk.c b/ui/gtk.c
> > index 810d7fc796..269b8207d7 100644
> > --- a/ui/gtk.c
> > +++ b/ui/gtk.c
> > @@ -1395,6 +1395,9 @@ static void gd_menu_untabify(GtkMenuItem *item,
> void *opaque)
> >      if (!vc->window) {
> >          gtk_widget_set_sensitive(vc->menu_item, false);
> >          vc->window = gtk_window_new(GTK_WINDOW_TOPLEVEL);
> > +        gtk_window_set_default_size(GTK_WINDOW(vc->window),
> > +                                    surface_width(vc->gfx.ds),
> > +                                    surface_height(vc->gfx.ds));
> >  #if defined(CONFIG_OPENGL)
> >          if (vc->gfx.esurface) {
> >              eglDestroySurface(qemu_egl_display, vc->gfx.esurface);
> > --
> > 2.34.1
> >
> >
> 
> 
> --
> Marc-André Lureau
Kim, Dongwon May 8, 2024, 12:02 a.m. UTC | #3 
Hi Marc-André,

I found that the problem is actually due to scaling factor of 0.25 (VC_SCALE_MIN).

static void gd_update_geometry_hints(VirtualConsole *vc)
{
    GtkDisplayState *s = vc->s;
    GdkWindowHints mask = 0;
    GdkGeometry geo = {};
    GtkWidget *geo_widget = NULL;
    GtkWindow *geo_window;

    if (vc->type == GD_VC_GFX) {
        if (!vc->gfx.ds) {
            return;
        }
        if (s->free_scale) {
            geo.min_width  = surface_width(vc->gfx.ds) * VC_SCALE_MIN;
            geo.min_height = surface_height(vc->gfx.ds) * VC_SCALE_MIN;
            mask |= GDK_HINT_MIN_SIZE;
        } else {
            geo.min_width  = surface_width(vc->gfx.ds) * vc->gfx.scale_x;
            geo.min_height = surface_height(vc->gfx.ds) * vc->gfx.scale_y;
            mask |= GDK_HINT_MIN_SIZE;
        }
        geo_widget = vc->gfx.drawing_area;
        gtk_widget_set_size_request(geo_widget, geo.min_width, geo.min_height);

s->free_scale is set when 'zoom_to_fit' is set. This 'zoom_to_fit' is normally set via param but it is unconditionally set when
ui_info is supported like when some display device is enabled like virtio-vga.

So here free_scale is true so min_width and min_height are set to 1/4 of original width/height of the surface (640x480). That is totally fine.
But the real problem is at 

gtk_widget_set_size_request(geo_widget, geo.min_width, geo.min_height);

I do not understand why we are making set size request with these "min" values.

This commit from Gerd originally introduced the 0.25 scaling factor but not sure what is intention there.
(I hope Gerd can comment on this.)

commit 82fc18099aa8ee2533add523cc0069f26a83e7b6
Author: Gerd Hoffmann <kraxel@redhat.com>
Date:   Fri May 16 12:26:12 2014 +0200

    gtk: window sizing overhaul

    Major overhaul for window size handling.  This basically switches qemu
    over to use geometry hints for the window manager instead of trying to
    get the job done with widget resize requests.  This allows to specify
    better what we need and also avoids window resizes.

    FIXME: on gtk2 someone overwrites the geometry hints :(

    Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>

I am wondering if we could just set geo.base_width and height to the normal size then use these instead when making size request there.

Can you share your thought?

Thanks,
DW

> -----Original Message-----
> From: qemu-devel-bounces+dongwon.kim=intel.com@nongnu.org <qemu-
> devel-bounces+dongwon.kim=intel.com@nongnu.org> On Behalf Of Kim,
> Dongwon
> Sent: Tuesday, May 7, 2024 9:06 AM
> To: Marc-André Lureau <marcandre.lureau@gmail.com>
> Cc: qemu-devel@nongnu.org; kraxel@redhat.com
> Subject: RE: [PATCH] ui/gtk: Explicitly set the default size of new window when
> untabifying
> 
> Hi Marc-André,
> 
> > -----Original Message-----
> > From: Marc-André Lureau <marcandre.lureau@gmail.com>
> > Sent: Tuesday, May 7, 2024 8:10 AM
> > To: Kim, Dongwon <dongwon.kim@intel.com>
> > Cc: qemu-devel@nongnu.org; kraxel@redhat.com
> > Subject: Re: [PATCH] ui/gtk: Explicitly set the default size of new
> > window when untabifying
> >
> > Hi
> >
> > On Wed, May 1, 2024 at 7:47 AM <dongwon.kim@intel.com> wrote:
> > >
> > > From: Dongwon Kim <dongwon.kim@intel.com>
> > >
> > > When untabifying, the default size of the new window was
> > > inadvertently set to the size smaller than quarter of the primary
> > > window size due to lack of explicit configuration. This commit
> > > addresses the issue by ensuring that the size of untabified windows
> > > is set to match the surface size.
> >
> > From a quick test, I don't see a difference of behaviour after the
> > patch. Could you help me reproduce the issue?
> >
> > I also don't think it is correct for two reasons:
> > - the inner display widget should cause a window size reconfiguration
> > - the window size != display size
> 
> [Kim, Dongwon] Ok, I see this is happening only when virtio-vga device is used
> like
> qemu-system-x86_64 -m 2048 -enable-kvm -cpu host -smp cores=2 -drive
> file=./OVMF.fd,format=raw,if=pflash -device virtio-vga -display gtk,gl=on Maybe
> some setting of dimensions is missing in there? I will take a look.
> 
> >
> > thanks
> >
> > > Cc: Gerd Hoffmann <kraxel@redhat.com>
> > > Cc: Marc-André Lureau <marcandre.lureau@redhat.com>
> > > Cc: Vivek Kasireddy <vivek.kasireddy@intel.com>
> > > Signed-off-by: Dongwon Kim <dongwon.kim@intel.com>
> > > ---
> > >  ui/gtk.c | 3 +++
> > >  1 file changed, 3 insertions(+)
> > >
> > > diff --git a/ui/gtk.c b/ui/gtk.c
> > > index 810d7fc796..269b8207d7 100644
> > > --- a/ui/gtk.c
> > > +++ b/ui/gtk.c
> > > @@ -1395,6 +1395,9 @@ static void gd_menu_untabify(GtkMenuItem
> > > *item,
> > void *opaque)
> > >      if (!vc->window) {
> > >          gtk_widget_set_sensitive(vc->menu_item, false);
> > >          vc->window = gtk_window_new(GTK_WINDOW_TOPLEVEL);
> > > +        gtk_window_set_default_size(GTK_WINDOW(vc->window),
> > > +                                    surface_width(vc->gfx.ds),
> > > +                                    surface_height(vc->gfx.ds));
> > >  #if defined(CONFIG_OPENGL)
> > >          if (vc->gfx.esurface) {
> > >              eglDestroySurface(qemu_egl_display, vc->gfx.esurface);
> > > --
> > > 2.34.1
> > >
> > >
> >
> >
> > --
> > Marc-André Lureau
diff mbox series

Patch

diff --git a/ui/gtk.c b/ui/gtk.c
index 810d7fc796..269b8207d7 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -1395,6 +1395,9 @@  static void gd_menu_untabify(GtkMenuItem *item, void *opaque)
     if (!vc->window) {
         gtk_widget_set_sensitive(vc->menu_item, false);
         vc->window = gtk_window_new(GTK_WINDOW_TOPLEVEL);
+        gtk_window_set_default_size(GTK_WINDOW(vc->window),
+                                    surface_width(vc->gfx.ds),
+                                    surface_height(vc->gfx.ds));
 #if defined(CONFIG_OPENGL)
         if (vc->gfx.esurface) {
             eglDestroySurface(qemu_egl_display, vc->gfx.esurface);