RE: [PATCH 1/3] ui/gtk: skip drawing guest scanout when associated VC is invisible

[Kim, Dongwon]

Hi Marc-André,

> https://docs.gtk.org/gtk3/method.Widget.is_visible.html

This is what we had tried first but it didn't seem to work for the case of 
window minimization.
I see the visible flag for the GTK widget didn't seem to be toggled for some 
reason. And when
closing window, vc->window widget is destroyed so it is not possible to check 
the flag using
this GTK function. Having extra flag bound to VC was most intuitive for the 
logic I wanted to
implement.

Thanks!!
DW

> Subject: Re: [PATCH 1/3] ui/gtk: skip drawing guest scanout when associated
> VC is invisible
> 
> Hi Dongwon
> 
> On Wed, Jan 31, 2024 at 3:50 AM <dongwon.kim@intel.com> wrote:
> >
> > From: Dongwon Kim <dongwon.kim@intel.com>
> >
> > A new flag "visible" is added to show visibility status of the gfx console.
> > The flag is set to 'true' when the VC is visible but set to 'false'
> > when it is hidden or closed. When the VC is invisible, drawing guest
> > frames should be skipped as it will never be completed and it would
> > potentially lock up the guest display especially when blob scanout is used.
> 
> Can't it skip drawing when the widget is not visible instead?
> https://docs.gtk.org/gtk3/method.Widget.is_visible.html
> 
> >
> > 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>
> > ---
> >  include/ui/gtk.h |  1 +
> >  ui/gtk-egl.c     |  8 ++++++++
> >  ui/gtk-gl-area.c |  8 ++++++++
> >  ui/gtk.c         | 10 +++++++++-
> >  4 files changed, 26 insertions(+), 1 deletion(-)
> >
> > diff --git a/include/ui/gtk.h b/include/ui/gtk.h index
> > aa3d637029..2de38e5724 100644
> > --- a/include/ui/gtk.h
> > +++ b/include/ui/gtk.h
> > @@ -57,6 +57,7 @@ typedef struct VirtualGfxConsole {
> >      bool y0_top;
> >      bool scanout_mode;
> >      bool has_dmabuf;
> > +    bool visible;
> >  #endif
> >  } VirtualGfxConsole;
> >
> > diff --git a/ui/gtk-egl.c b/ui/gtk-egl.c index 3af5ac5bcf..993c283191
> > 100644
> > --- a/ui/gtk-egl.c
> > +++ b/ui/gtk-egl.c
> > @@ -265,6 +265,10 @@ void
> gd_egl_scanout_dmabuf(DisplayChangeListener
> > *dcl,  #ifdef CONFIG_GBM
> >      VirtualConsole *vc = container_of(dcl, VirtualConsole, gfx.dcl);
> >
> > +    if (!vc->gfx.visible) {
> > +        return;
> > +    }
> > +
> >      eglMakeCurrent(qemu_egl_display, vc->gfx.esurface,
> >                     vc->gfx.esurface, vc->gfx.ectx);
> >
> > @@ -363,6 +367,10 @@ void gd_egl_flush(DisplayChangeListener *dcl,
> >      VirtualConsole *vc = container_of(dcl, VirtualConsole, gfx.dcl);
> >      GtkWidget *area = vc->gfx.drawing_area;
> >
> > +    if (!vc->gfx.visible) {
> > +        return;
> > +    }
> > +
> >      if (vc->gfx.guest_fb.dmabuf && !vc->gfx.guest_fb.dmabuf-
> >draw_submitted) {
> >          graphic_hw_gl_block(vc->gfx.dcl.con, true);
> >          vc->gfx.guest_fb.dmabuf->draw_submitted = true; diff --git
> > a/ui/gtk-gl-area.c b/ui/gtk-gl-area.c index 52dcac161e..04e07bd7ee
> > 100644
> > --- a/ui/gtk-gl-area.c
> > +++ b/ui/gtk-gl-area.c
> > @@ -285,6 +285,10 @@ void
> > gd_gl_area_scanout_flush(DisplayChangeListener *dcl,  {
> >      VirtualConsole *vc = container_of(dcl, VirtualConsole, gfx.dcl);
> >
> > +    if (!vc->gfx.visible) {
> > +        return;
> > +    }
> > +
> >      if (vc->gfx.guest_fb.dmabuf && !vc->gfx.guest_fb.dmabuf-
> >draw_submitted) {
> >          graphic_hw_gl_block(vc->gfx.dcl.con, true);
> >          vc->gfx.guest_fb.dmabuf->draw_submitted = true; @@ -299,6
> > +303,10 @@ void gd_gl_area_scanout_dmabuf(DisplayChangeListener *dcl,
> > #ifdef CONFIG_GBM
> >      VirtualConsole *vc = container_of(dcl, VirtualConsole, gfx.dcl);
> >
> > +    if (!vc->gfx.visible) {
> > +        return;
> > +    }
> > +
> >      gtk_gl_area_make_current(GTK_GL_AREA(vc->gfx.drawing_area));
> >      egl_dmabuf_import_texture(dmabuf);
> >      if (!dmabuf->texture) {
> > diff --git a/ui/gtk.c b/ui/gtk.c
> > index 810d7fc796..02eb667d8a 100644
> > --- a/ui/gtk.c
> > +++ b/ui/gtk.c
> > @@ -1312,15 +1312,20 @@ static void gd_menu_quit(GtkMenuItem *item,
> > void *opaque)  static void gd_menu_switch_vc(GtkMenuItem *item, void
> > *opaque)  {
> >      GtkDisplayState *s = opaque;
> > -    VirtualConsole *vc = gd_vc_find_by_menu(s);
> > +    VirtualConsole *vc;
> >      GtkNotebook *nb = GTK_NOTEBOOK(s->notebook);
> >      gint page;
> >
> > +    vc = gd_vc_find_current(s);
> > +    vc->gfx.visible = false;
> > +
> > +    vc = gd_vc_find_by_menu(s);
> >      gtk_release_modifiers(s);
> >      if (vc) {
> >          page = gtk_notebook_page_num(nb, vc->tab_item);
> >          gtk_notebook_set_current_page(nb, page);
> >          gtk_widget_grab_focus(vc->focus);
> > +        vc->gfx.visible = true;
> >      }
> >  }
> >
> > @@ -1350,6 +1355,7 @@ static gboolean gd_tab_window_close(GtkWidget
> *widget, GdkEvent *event,
> >      VirtualConsole *vc = opaque;
> >      GtkDisplayState *s = vc->s;
> >
> > +    vc->gfx.visible = false;
> >      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),
> > @@ -1423,6 +1429,7 @@ static void gd_menu_untabify(GtkMenuItem *item,
> void *opaque)
> >          gd_update_geometry_hints(vc);
> >          gd_update_caption(s);
> >      }
> > +    vc->gfx.visible = true;
> >  }
> >
> >  static void gd_menu_show_menubar(GtkMenuItem *item, void *opaque)
> @@
> > -2471,6 +2478,7 @@ static void gtk_display_init(DisplayState *ds,
> > DisplayOptions *opts)  #ifdef CONFIG_GTK_CLIPBOARD
> >      gd_clipboard_init(s);
> >  #endif /* CONFIG_GTK_CLIPBOARD */
> > +    vc->gfx.visible = true;
> >  }
> >
> >  static void early_gtk_display_init(DisplayOptions *opts)
> > --
> > 2.34.1
> >
> >
> 
> 
> --
> Marc-André Lureau