RE: [PATCH 2/3] ui/gtk: set the ui size to 0 when invisible

[Kim, Dongwon]

Hi Marc-André,

> -----Original Message-----
> From: Marc-André Lureau <marcandre.lureau@gmail.com>
> Sent: Tuesday, January 30, 2024 11:13 PM
> To: Kim, Dongwon <dongwon.kim@intel.com>
> Cc: qemu-devel@nongnu.org
> Subject: Re: [PATCH 2/3] ui/gtk: set the ui size to 0 when invisible
> 
> Hi
> 
> On Wed, Jan 31, 2024 at 3:50 AM <dongwon.kim@intel.com> wrote:
> >
> > From: Dongwon Kim <dongwon.kim@intel.com>
> >
> > UI size is set to 0 when the VC is invisible, which will prevent the
> > further scanout update by notifying the guest that the display is not
> > in active state. Then it is restored to the original size whenever the
> > VC becomes visible again.
> 
> This can have unwanted results on multi monitor setups, such as moving
> windows or icons around on different monitors.

[Kim, Dongwon]  You are right. This is just a choice we made.
> 
> Switching tabs or minimizing the display window shouldn't cause a guest
> display reconfiguration.
> 
> What is the benefit of disabling the monitor here? Is it for performance 
> reasons?

[Kim, Dongwon] Not sure if you recognized it but this patch series was a part of
our VM display hot-plug feature we submitted a few months ago. There, we added 
a new
param called connectors to have a way to fix individual VM displays (in multi 
display env)
on different physical displays there and made the VM display disconnected when
associated physical one is disconnected. We just wanted to make tab switching 
and
window minimization do the similar to make all of this logic consistent. 

However, if it makes more sense to have those displays all connected even when
those are not shown except for the case of hot-plug in, we could change the 
logic.
But as you mentioned, there will be some waste of bandwidth and perf since the
guest will keep sending out scan-out frames that would be just dumped.

This might be a minor thing but another concern is about tab-switching. 
Initially, the guest
will detect only one display even if the max-output is set to N (other than 1). 
Multi displays
will be detected once you detach or switch to another tab. Then if you move to 
the original
tab or close the detached window, the guest won't go back to single display 
setup.
All multi-displays will exist in its setup and this doesn’t look consistent to 
me.

> 
> >
> > Cc: Marc-André Lureau <marcandre.lureau@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 | 15 ++++++++++++++-
> >  1 file changed, 14 insertions(+), 1 deletion(-)
> >
> > diff --git a/ui/gtk.c b/ui/gtk.c
> > index 02eb667d8a..651ed3492f 100644
> > --- a/ui/gtk.c
> > +++ b/ui/gtk.c
> > @@ -1314,10 +1314,12 @@ static void gd_menu_switch_vc(GtkMenuItem
> *item, void *opaque)
> >      GtkDisplayState *s = opaque;
> >      VirtualConsole *vc;
> >      GtkNotebook *nb = GTK_NOTEBOOK(s->notebook);
> > +    GdkWindow *window;
> >      gint page;
> >
> >      vc = gd_vc_find_current(s);
> >      vc->gfx.visible = false;
> > +    gd_set_ui_size(vc, 0, 0);
> >
> >      vc = gd_vc_find_by_menu(s);
> >      gtk_release_modifiers(s);
> > @@ -1325,6 +1327,9 @@ static void gd_menu_switch_vc(GtkMenuItem
> *item, void *opaque)
> >          page = gtk_notebook_page_num(nb, vc->tab_item);
> >          gtk_notebook_set_current_page(nb, page);
> >          gtk_widget_grab_focus(vc->focus);
> > +        window = gtk_widget_get_window(vc->gfx.drawing_area);
> > +        gd_set_ui_size(vc, gdk_window_get_width(window),
> > +                       gdk_window_get_height(window));
> >          vc->gfx.visible = true;
> >      }
> >  }
> > @@ -1356,6 +1361,7 @@ static gboolean gd_tab_window_close(GtkWidget
> *widget, GdkEvent *event,
> >      GtkDisplayState *s = vc->s;
> >
> >      vc->gfx.visible = false;
> > +    gd_set_ui_size(vc, 0, 0);
> >      gtk_widget_set_sensitive(vc->menu_item, true);
> >      gd_widget_reparent(vc->window, s->notebook, vc->tab_item);
> >      gtk_notebook_set_tab_label_text(GTK_NOTEBOOK(s->notebook),
> > @@ -1391,6 +1397,7 @@ static gboolean gd_win_grab(void *opaque)
> > static void gd_menu_untabify(GtkMenuItem *item, void *opaque)  {
> >      GtkDisplayState *s = opaque;
> > +    GdkWindow *window;
> >      VirtualConsole *vc = gd_vc_find_current(s);
> >
> >      if (vc->type == GD_VC_GFX &&
> > @@ -1429,6 +1436,10 @@ static void gd_menu_untabify(GtkMenuItem
> *item, void *opaque)
> >          gd_update_geometry_hints(vc);
> >          gd_update_caption(s);
> >      }
> > +
> > +    window = gtk_widget_get_window(vc->gfx.drawing_area);
> > +    gd_set_ui_size(vc, gdk_window_get_width(window),
> > +                   gdk_window_get_height(window));
> >      vc->gfx.visible = true;
> >  }
> >
> > @@ -1753,7 +1764,9 @@ static gboolean gd_configure(GtkWidget *widget,
> > {
> >      VirtualConsole *vc = opaque;
> >
> > -    gd_set_ui_size(vc, cfg->width, cfg->height);
> > +    if (vc->gfx.visible) {
> > +        gd_set_ui_size(vc, cfg->width, cfg->height);
> > +    }
> >      return FALSE;
> >  }
> >
> > --
> > 2.34.1
> >
> >
> 
> 
> --
> Marc-André Lureau