[PATCH 0/3] ui/gtk: introducing vc->visible

[PATCH 0/3] ui/gtk: introducing vc->visible

[dongwon.kim]

From: Dongwon Kim <dongwon.kim@intel.com>

Drawing guest display frames can't be completed while the VC is not in
visible state, which could result in timeout in both the host and the
guest especially when using blob scanout. Therefore it is needed to
update and track the visiblity status of the VC and unblock the pipeline
in case when VC becomes invisible (e.g. windows minimization, switching
among tabs) while processing a guest frame.

First patch (0001-ui-gtk-skip...) is introducing a flag 'visible' to
VirtualConsole struct then set it only if the VC and its window is
visible.
 
Second patch (0002-ui-gtk-set-...) sets the ui size to 0 when VC is
invisible when the tab is closed or deactivated. This notifies the guest
that the associated guest display is not active anymore.

Third patch (0003-ui-gtk-reset-visible...) adds a callback for GTK
window-state-event. The flag, 'visible' is updated based on the
minization status of the window.

Dongwon Kim (3):
  ui/gtk: skip drawing guest scanout when associated VC is invisible
  ui/gtk: set the ui size to 0 when invisible
  ui/gtk: reset visible flag when window is minimized

 include/ui/gtk.h |  1 +
 ui/gtk-egl.c     |  8 +++++++
 ui/gtk-gl-area.c |  8 +++++++
 ui/gtk.c         | 62 ++++++++++++++++++++++++++++++++++++++++++++++--
 4 files changed, 77 insertions(+), 2 deletions(-)

-- 
2.34.1

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

[dongwon.kim]

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.

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

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

[dongwon.kim]

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.

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

[PATCH 3/3] ui/gtk: reset visible flag when window is minimized

[dongwon.kim]

From: Dongwon Kim <dongwon.kim@intel.com>

Adding a callback for window-state-event that resets the flag, 'visible'
when associated window is minimized or restored. When minimizing, it cancels
any of queued draw events associated with the VC.

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 | 39 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 39 insertions(+)

diff --git a/ui/gtk.c b/ui/gtk.c
index 651ed3492f..5bbcb7de62 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -1381,6 +1381,37 @@ static gboolean gd_tab_window_close(GtkWidget *widget, GdkEvent *event,
     return TRUE;
 }
 
+static gboolean gd_window_state_event(GtkWidget *widget, GdkEvent *event,
+                                      void *opaque)
+{
+    VirtualConsole *vc = opaque;
+
+    if (!vc) {
+        return TRUE;
+    }
+
+    if (event->window_state.new_window_state & GDK_WINDOW_STATE_ICONIFIED) {
+        vc->gfx.visible = false;
+        gd_set_ui_size(vc, 0, 0);
+        if (vc->gfx.guest_fb.dmabuf &&
+            vc->gfx.guest_fb.dmabuf->draw_submitted) {
+            vc->gfx.guest_fb.dmabuf->draw_submitted = false;
+            graphic_hw_gl_block(vc->gfx.dcl.con, false);
+        }
+    /* Showing the ui only if window exists except for the current vc as GTK
+     * window for 's' is being used to display the GUI */
+    } else if (vc->window || vc == gd_vc_find_current(vc->s)) {
+        GdkWindow *window;
+        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;
+    }
+
+    return TRUE;
+}
+
 static gboolean gd_win_grab(void *opaque)
 {
     VirtualConsole *vc = opaque;
@@ -1422,6 +1453,9 @@ static void gd_menu_untabify(GtkMenuItem *item, void *opaque)
 
         g_signal_connect(vc->window, "delete-event",
                          G_CALLBACK(gd_tab_window_close), vc);
+        g_signal_connect(vc->window, "window-state-event",
+                         G_CALLBACK(gd_window_state_event), vc);
+
         gtk_widget_show_all(vc->window);
 
         if (qemu_console_is_graphic(vc->gfx.dcl.con)) {
@@ -2470,6 +2504,11 @@ static void gtk_display_init(DisplayState *ds, DisplayOptions *opts)
     }
 
     vc = gd_vc_find_current(s);
+
+    g_signal_connect(s->window, "window-state-event",
+                     G_CALLBACK(gd_window_state_event),
+                     vc);
+
     gtk_widget_set_sensitive(s->view_menu, vc != NULL);
 #ifdef CONFIG_VTE
     gtk_widget_set_sensitive(s->copy_item,
-- 
2.34.1

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

[Marc-André Lureau]

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

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

[Marc-André Lureau]

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.

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?

>
> 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

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

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

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

[Marc-André Lureau]

Hi

On Wed, Jan 31, 2024 at 10:56 PM Kim, Dongwon <dongwon.kim@intel.com> wrote:
>
> 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

Right, because minimize != visible. You can still get window preview
with alt-tab and other compositor drawings.

Iow, it should keep rendering even when minimized.

> 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



-- 
Marc-André Lureau

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

[Kim, Dongwon]

Hi Marc-André,

Thanks for your feedback. Yes, you are right, rendering doesn't stop on Ubuntu system
as it has preview even after the window is minimized. But this is not always the case.
Some simple windows managers don't have this preview (thumbnail)
feature and this visible flag is not toggled. And the rendering stops right away there when
the window is minimized.

Detaching then closing the window which makes it go back to tabs does not
make the flag reset, too. To us, having this extra flag for VC is one simple and intuitive
way to handle such situations (including upcoming guest display hot plug in patches)

Thanks!

> Subject: Re: [PATCH 1/3] ui/gtk: skip drawing guest scanout when associated
> VC is invisible
> 
> Hi
> 
> On Wed, Jan 31, 2024 at 10:56 PM Kim, Dongwon <dongwon.kim@intel.com>
> wrote:
> >
> > 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
> 
> Right, because minimize != visible. You can still get window preview with alt-
> tab and other compositor drawings.
> 
> Iow, it should keep rendering even when minimized.
> 
> > 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
> 
> 
> 
> --
> Marc-André Lureau

RE: [PATCH 0/3] ui/gtk: introducing vc->visible

[Kim, Dongwon]

Hi Marc-André Lureau,

Just a reminder.. I need your help reviewing this patch series. Please take a look at my messages at
https://lists.gnu.org/archive/html/qemu-devel/2024-01/msg06636.html and
https://lists.gnu.org/archive/html/qemu-devel/2024-01/msg06637.html

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
> dongwon.kim@intel.com
> Sent: Tuesday, January 30, 2024 3:49 PM
> To: qemu-devel@nongnu.org
> Subject: [PATCH 0/3] ui/gtk: introducing vc->visible
> 
> From: Dongwon Kim <dongwon.kim@intel.com>
> 
> Drawing guest display frames can't be completed while the VC is not in visible
> state, which could result in timeout in both the host and the guest especially
> when using blob scanout. Therefore it is needed to update and track the visiblity
> status of the VC and unblock the pipeline in case when VC becomes invisible (e.g.
> windows minimization, switching among tabs) while processing a guest frame.
> 
> First patch (0001-ui-gtk-skip...) is introducing a flag 'visible' to VirtualConsole
> struct then set it only if the VC and its window is visible.
> 
> Second patch (0002-ui-gtk-set-...) sets the ui size to 0 when VC is invisible when
> the tab is closed or deactivated. This notifies the guest that the associated guest
> display is not active anymore.
> 
> Third patch (0003-ui-gtk-reset-visible...) adds a callback for GTK window-state-
> event. The flag, 'visible' is updated based on the minization status of the window.
> 
> Dongwon Kim (3):
>   ui/gtk: skip drawing guest scanout when associated VC is invisible
>   ui/gtk: set the ui size to 0 when invisible
>   ui/gtk: reset visible flag when window is minimized
> 
>  include/ui/gtk.h |  1 +
>  ui/gtk-egl.c     |  8 +++++++
>  ui/gtk-gl-area.c |  8 +++++++
>  ui/gtk.c         | 62 ++++++++++++++++++++++++++++++++++++++++++++++--
>  4 files changed, 77 insertions(+), 2 deletions(-)
> 
> --
> 2.34.1
> 

Re: [PATCH 0/3] ui/gtk: introducing vc->visible

[Marc-André Lureau]

Hi Kim

I am uncomfortable with the series in general.

Not only we don't have the means to draw dmabuf/scanout "when
required", so resuming drawing won't work until the guest draws (this
is already a problem but you are only making it worse). And I also
think reconfiguring the guest by merely minimizing or switching
window/tabs isn't what most users would expect.

(fwiw, my personal opinion is that QEMU shouldn't provide UIs and
different clients should be able to implement different behaviours,
out of process.. that makes me relatively less motivated to break
things and be responsible)

Daniel, could you have a look too?

thanks

On Fri, Mar 1, 2024 at 4:05 AM Kim, Dongwon <dongwon.kim@intel.com> wrote:
>
> Hi Marc-André Lureau,
>
> Just a reminder.. I need your help reviewing this patch series. Please take a look at my messages at
> https://lists.gnu.org/archive/html/qemu-devel/2024-01/msg06636.html and
> https://lists.gnu.org/archive/html/qemu-devel/2024-01/msg06637.html
>
> 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
> > dongwon.kim@intel.com
> > Sent: Tuesday, January 30, 2024 3:49 PM
> > To: qemu-devel@nongnu.org
> > Subject: [PATCH 0/3] ui/gtk: introducing vc->visible
> >
> > From: Dongwon Kim <dongwon.kim@intel.com>
> >
> > Drawing guest display frames can't be completed while the VC is not in visible
> > state, which could result in timeout in both the host and the guest especially
> > when using blob scanout. Therefore it is needed to update and track the visiblity
> > status of the VC and unblock the pipeline in case when VC becomes invisible (e.g.
> > windows minimization, switching among tabs) while processing a guest frame.
> >
> > First patch (0001-ui-gtk-skip...) is introducing a flag 'visible' to VirtualConsole
> > struct then set it only if the VC and its window is visible.
> >
> > Second patch (0002-ui-gtk-set-...) sets the ui size to 0 when VC is invisible when
> > the tab is closed or deactivated. This notifies the guest that the associated guest
> > display is not active anymore.
> >
> > Third patch (0003-ui-gtk-reset-visible...) adds a callback for GTK window-state-
> > event. The flag, 'visible' is updated based on the minization status of the window.
> >
> > Dongwon Kim (3):
> >   ui/gtk: skip drawing guest scanout when associated VC is invisible
> >   ui/gtk: set the ui size to 0 when invisible
> >   ui/gtk: reset visible flag when window is minimized
> >
> >  include/ui/gtk.h |  1 +
> >  ui/gtk-egl.c     |  8 +++++++
> >  ui/gtk-gl-area.c |  8 +++++++
> >  ui/gtk.c         | 62 ++++++++++++++++++++++++++++++++++++++++++++++--
> >  4 files changed, 77 insertions(+), 2 deletions(-)
> >
> > --
> > 2.34.1
> >
>

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

[Daniel P. Berrangé]

On Tue, Jan 30, 2024 at 03:48:38PM -0800, 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.
> 
> 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) {

If we skip processing these requests, what happens when the
QEMU windows is then re-displayed. Won't it now be missing
updates to various regions of the guest display ?

>
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 :|

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

[Daniel P. Berrangé]

On Thu, Feb 01, 2024 at 06:48:58PM +0000, Kim, Dongwon wrote:
> Hi Marc-André,
> 
> Thanks for your feedback. Yes, you are right, rendering doesn't stop on Ubuntu system
> as it has preview even after the window is minimized. But this is not always the case.
> Some simple windows managers don't have this preview (thumbnail)
> feature and this visible flag is not toggled. And the rendering stops right away there
> when the window is minimized.

This makes me pretty uncomfortable. This is proposing changing QEMU
behaviour to workaround a problem whose behaviour varies based on
what 3rd party software QEMU is running on

What you say "window managers" are you referring to a traditional
X11 based host display only, or does the problem also exist on
modern Wayland host display ?

If the problem is confined to X11, that would steer towards saying
we shouldn't try to workaround it given that X11 is very much
obsolete at this point.

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 :|

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

[Daniel P. Berrangé]

On Wed, Jan 31, 2024 at 11:12:57AM +0400, Marc-André Lureau wrote:
> 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.
> 
> Switching tabs or minimizing the display window shouldn't cause a
> guest display reconfiguration.

I agree, changing the size of displays as a side-effect of
something that isn't a guest owner initiated resize operation
is asking for trouble.


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 :|

Re: [PATCH 0/3] ui/gtk: introducing vc->visible

[Daniel P. Berrangé]

On Tue, Mar 05, 2024 at 04:18:18PM +0400, Marc-André Lureau wrote:
> Hi Kim
> 
> I am uncomfortable with the series in general.
> 
> Not only we don't have the means to draw dmabuf/scanout "when
> required", so resuming drawing won't work until the guest draws (this
> is already a problem but you are only making it worse). And I also
> think reconfiguring the guest by merely minimizing or switching
> window/tabs isn't what most users would expect.
> 
> Daniel, could you have a look too?

I'm similarly pretty uncomfortable with the proposed behaviour
as it feels like it would be introducing a whole new set of
unfortunate side-effects/problems.


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 :|

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

[Kim, Dongwon]

Hi Daniel,

> -----Original Message-----
> From: Daniel P. Berrangé <berrange@redhat.com>
> Sent: Thursday, March 7, 2024 1:41 AM
> To: Kim, Dongwon <dongwon.kim@intel.com>
> Cc: qemu-devel@nongnu.org
> Subject: Re: [PATCH 1/3] ui/gtk: skip drawing guest scanout when associated
> VC is invisible
> 
> On Tue, Jan 30, 2024 at 03:48:38PM -0800, 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.
> >
> > 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) {
> 
> If we skip processing these requests, what happens when the QEMU windows is
> then re-displayed. Won't it now be missing updates to various regions of the
> guest display ?
 
[Kim, Dongwon]  Scanout blob is continuously submitted by the guest even when the vc->visible is false. So it will just display the current guest frame right away when the window is redisplayed again.

> 
> >
> 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 :|

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

[Kim, Dongwon]

Hi Daniel,

> -----Original Message-----
> From: Daniel P. Berrangé <berrange@redhat.com>
> Sent: Thursday, March 7, 2024 1:46 AM
> To: Kim, Dongwon <dongwon.kim@intel.com>
> Cc: Marc-André Lureau <marcandre.lureau@gmail.com>; qemu-
> devel@nongnu.org
> Subject: Re: [PATCH 1/3] ui/gtk: skip drawing guest scanout when associated
> VC is invisible
> 
> On Thu, Feb 01, 2024 at 06:48:58PM +0000, Kim, Dongwon wrote:
> > Hi Marc-André,
> >
> > Thanks for your feedback. Yes, you are right, rendering doesn't stop
> > on Ubuntu system as it has preview even after the window is minimized. But
> this is not always the case.
> > Some simple windows managers don't have this preview (thumbnail)
> > feature and this visible flag is not toggled. And the rendering stops
> > right away there when the window is minimized.
> 
> This makes me pretty uncomfortable. This is proposing changing QEMU
> behaviour to workaround a problem whose behaviour varies based on what 3rd
> party software QEMU is running on
> 
> What you say "window managers" are you referring to a traditional
> X11 based host display only, or does the problem also exist on modern
> Wayland host display ?

[Kim, Dongwon]  I didn't mean anything about the compositor/display server itself. And I am not sure about the general behavior of Wayland compositors but the point here is the rendering while the app is being iconized (minimized) is not always the case. For example, ICE-WM on Yocto Linux doesn't have any preview for the iconized or minimized applications, which means all drawing activities on the minimized app are paused. This is the problem in case blob scanout is used with virtio-gpu on the guest because the guest won't receive the response for the frame submission until the frame is fully rendered on the host. This is causing timeout and further issue on the display pipeline in virtio-gpu driver in the guest.

> 
> If the problem is confined to X11, that would steer towards saying we shouldn't
> try to workaround it given that X11 is very much obsolete at this point.

[Kim, Dongwon]  I think I should try Wayland too but I guess it depends on the compositor and the shell design. Of course I need to test but what if it works ok on the Ubuntu running with wayland backend but not with Weston?

> 
> 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 :|

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

[Daniel P. Berrangé]

On Thu, Mar 07, 2024 at 05:53:24PM +0000, Kim, Dongwon wrote:
> Hi Daniel,
> 
> > -----Original Message-----
> > From: Daniel P. Berrangé <berrange@redhat.com>
> > Sent: Thursday, March 7, 2024 1:46 AM
> > To: Kim, Dongwon <dongwon.kim@intel.com>
> > Cc: Marc-André Lureau <marcandre.lureau@gmail.com>; qemu-
> > devel@nongnu.org
> > Subject: Re: [PATCH 1/3] ui/gtk: skip drawing guest scanout when associated
> > VC is invisible
> > 
> > On Thu, Feb 01, 2024 at 06:48:58PM +0000, Kim, Dongwon wrote:
> > > Hi Marc-André,
> > >
> > > Thanks for your feedback. Yes, you are right, rendering doesn't stop
> > > on Ubuntu system as it has preview even after the window is minimized. But
> > this is not always the case.
> > > Some simple windows managers don't have this preview (thumbnail)
> > > feature and this visible flag is not toggled. And the rendering stops
> > > right away there when the window is minimized.
> > 
> > This makes me pretty uncomfortable. This is proposing changing QEMU
> > behaviour to workaround a problem whose behaviour varies based on what 3rd
> > party software QEMU is running on
> > 
> > What you say "window managers" are you referring to a traditional
> > X11 based host display only, or does the problem also exist on modern
> > Wayland host display ?
> 
> [Kim, Dongwon]  I didn't mean anything about the compositor/display
> server itself. And I am not sure about the general behavior of Wayland
> compositors but the point here is the rendering while the app is being
> iconized (minimized) is not always the case. For example, ICE-WM on
> Yocto Linux doesn't have any preview for the iconized or minimized
> applications, which means all drawing activities on the minimized
> app are paused. This is the problem in case blob scanout is used
> with virtio-gpu on the guest because the guest won't receive the
> response for the frame submission until the frame is fully rendered
> on the host. This is causing timeout and further issue on the display
> pipeline in virtio-gpu driver in the guest.

I guess I'm wondering why we should consider this a bug in QEMU
rather than a bug in either the toolkit or host rendering stack ?

Lets say there was no guest OS here, just a regular host app
issuing drawing requests that were equivalent to the workload
the guest is issuing.  If that applications' execution got
blocked because its drawing requests are not getting processed
when iconified, I'd be inclined to call it a bug.

Or are we perhaps handling drawing in the wrong way in QEMU ?

If the problem is with drawing to a iconified windows, is
there an intermediate target buffer we should be drawing
to, instead of directly to the window. There must be some
supported way to have drawing requests fully processed in
the background indepent of having a visible window, surely ?

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 :|

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

[Kim, Dongwon]

Hi Daniel,

> -----Original Message-----
> From: Daniel P. Berrangé <berrange@redhat.com>
> Sent: Thursday, March 7, 2024 10:01 AM
> To: Kim, Dongwon <dongwon.kim@intel.com>
> Cc: Marc-André Lureau <marcandre.lureau@gmail.com>; qemu-
> devel@nongnu.org
> Subject: Re: [PATCH 1/3] ui/gtk: skip drawing guest scanout when associated
> VC is invisible
> 
> On Thu, Mar 07, 2024 at 05:53:24PM +0000, Kim, Dongwon wrote:
> > Hi Daniel,
> >
> > > -----Original Message-----
> > > From: Daniel P. Berrangé <berrange@redhat.com>
> > > Sent: Thursday, March 7, 2024 1:46 AM
> > > To: Kim, Dongwon <dongwon.kim@intel.com>
> > > Cc: Marc-André Lureau <marcandre.lureau@gmail.com>; qemu-
> > > devel@nongnu.org
> > > Subject: Re: [PATCH 1/3] ui/gtk: skip drawing guest scanout when
> > > associated VC is invisible
> > >
> > > On Thu, Feb 01, 2024 at 06:48:58PM +0000, Kim, Dongwon wrote:
> > > > Hi Marc-André,
> > > >
> > > > Thanks for your feedback. Yes, you are right, rendering doesn't
> > > > stop on Ubuntu system as it has preview even after the window is
> > > > minimized. But
> > > this is not always the case.
> > > > Some simple windows managers don't have this preview (thumbnail)
> > > > feature and this visible flag is not toggled. And the rendering
> > > > stops right away there when the window is minimized.
> > >
> > > This makes me pretty uncomfortable. This is proposing changing QEMU
> > > behaviour to workaround a problem whose behaviour varies based on
> > > what 3rd party software QEMU is running on
> > >
> > > What you say "window managers" are you referring to a traditional
> > > X11 based host display only, or does the problem also exist on
> > > modern Wayland host display ?
> >
> > [Kim, Dongwon]  I didn't mean anything about the compositor/display
> > server itself. And I am not sure about the general behavior of Wayland
> > compositors but the point here is the rendering while the app is being
> > iconized (minimized) is not always the case. For example, ICE-WM on
> > Yocto Linux doesn't have any preview for the iconized or minimized
> > applications, which means all drawing activities on the minimized app
> > are paused. This is the problem in case blob scanout is used with
> > virtio-gpu on the guest because the guest won't receive the response
> > for the frame submission until the frame is fully rendered on the
> > host. This is causing timeout and further issue on the display
> > pipeline in virtio-gpu driver in the guest.
> 
> I guess I'm wondering why we should consider this a bug in QEMU rather than
> a bug in either the toolkit or host rendering stack ?
> 
> Lets say there was no guest OS here, just a regular host app issuing drawing
> requests that were equivalent to the workload the guest is issuing.  If that
> applications' execution got blocked because its drawing requests are not
> getting processed when iconified, I'd be inclined to call it a bug.
> 
> Or are we perhaps handling drawing in the wrong way in QEMU ?
[Kim, Dongwon] Hmm.. Yeah that is a good point.. If non-rendering workload is blocked in the same way, that would be a problem. 
> 
> If the problem is with drawing to a iconified windows, is there an intermediate
> target buffer we should be drawing to, instead of directly to the window. There
> must be some supported way to have drawing requests fully processed in the
> background indepent of having a visible window, surely ?
[Kim, Dongwon]  I will check what other options that don't look like WAs are available.

> 
> 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 :|

RE: [PATCH 0/3] ui/gtk: introducing vc->visible

[Kim, Dongwon]

Hi Marc-André,

> -----Original Message-----
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
> Sent: Tuesday, March 5, 2024 4:18 AM
> To: Kim, Dongwon <dongwon.kim@intel.com>; P. Berrange, Daniel
> <berrange@redhat.com>
> Cc: qemu-devel@nongnu.org
> Subject: Re: [PATCH 0/3] ui/gtk: introducing vc->visible
> 
> Hi Kim
> 
> I am uncomfortable with the series in general.
> 
> Not only we don't have the means to draw dmabuf/scanout "when required", so
> resuming drawing won't work until the guest draws (this is already a problem but
[Kim, Dongwon] Yes, this is right. The display won't be updated until there is a new frame submitted.
> you are only making it worse). And I also think reconfiguring the guest by merely
> minimizing or switching window/tabs isn't what most users would expect.
[Kim, Dongwon] I understand your concern. Then what do you suggest I need to do or look into to avoid the situation where the host rendering of the guest frame is scheduled but pending due to tab switching or minimization of window as the guest (virtio-gpu drv) is waiting for the response to move on to the next frame? Do you think the frame update should just continue on the host side regardless of visibility of the window? (If this is the standard expectation, then one of our Linux configuration - Yocto + ICE WM has some bug in it.)

> 
> (fwiw, my personal opinion is that QEMU shouldn't provide UIs and different
> clients should be able to implement different behaviours, out of process.. that
> makes me relatively less motivated to break things and be responsible)
> 
> Daniel, could you have a look too?
> 
> thanks
> 
> On Fri, Mar 1, 2024 at 4:05 AM Kim, Dongwon <dongwon.kim@intel.com>
> wrote:
> >
> > Hi Marc-André Lureau,
> >
> > Just a reminder.. I need your help reviewing this patch series. Please
> > take a look at my messages at
> > https://lists.gnu.org/archive/html/qemu-devel/2024-01/msg06636.html
> > and
> > https://lists.gnu.org/archive/html/qemu-devel/2024-01/msg06637.html
> >
> > 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
> > > dongwon.kim@intel.com
> > > Sent: Tuesday, January 30, 2024 3:49 PM
> > > To: qemu-devel@nongnu.org
> > > Subject: [PATCH 0/3] ui/gtk: introducing vc->visible
> > >
> > > From: Dongwon Kim <dongwon.kim@intel.com>
> > >
> > > Drawing guest display frames can't be completed while the VC is not
> > > in visible state, which could result in timeout in both the host and
> > > the guest especially when using blob scanout. Therefore it is needed
> > > to update and track the visiblity status of the VC and unblock the pipeline in
> case when VC becomes invisible (e.g.
> > > windows minimization, switching among tabs) while processing a guest
> frame.
> > >
> > > First patch (0001-ui-gtk-skip...) is introducing a flag 'visible' to
> > > VirtualConsole struct then set it only if the VC and its window is visible.
> > >
> > > Second patch (0002-ui-gtk-set-...) sets the ui size to 0 when VC is
> > > invisible when the tab is closed or deactivated. This notifies the
> > > guest that the associated guest display is not active anymore.
> > >
> > > Third patch (0003-ui-gtk-reset-visible...) adds a callback for GTK
> > > window-state- event. The flag, 'visible' is updated based on the minization
> status of the window.
> > >
> > > Dongwon Kim (3):
> > >   ui/gtk: skip drawing guest scanout when associated VC is invisible
> > >   ui/gtk: set the ui size to 0 when invisible
> > >   ui/gtk: reset visible flag when window is minimized
> > >
> > >  include/ui/gtk.h |  1 +
> > >  ui/gtk-egl.c     |  8 +++++++
> > >  ui/gtk-gl-area.c |  8 +++++++
> > >  ui/gtk.c         | 62 ++++++++++++++++++++++++++++++++++++++++++++++--
> > >  4 files changed, 77 insertions(+), 2 deletions(-)
> > >
> > > --
> > > 2.34.1
> > >
> >

Re: [PATCH 0/3] ui/gtk: introducing vc->visible

[Marc-André Lureau]

Hi

On Fri, Mar 8, 2024 at 4:59 AM Kim, Dongwon <dongwon.kim@intel.com> wrote:
>
> Hi Marc-André,
>
> > -----Original Message-----
> > From: Marc-André Lureau <marcandre.lureau@redhat.com>
> > Sent: Tuesday, March 5, 2024 4:18 AM
> > To: Kim, Dongwon <dongwon.kim@intel.com>; P. Berrange, Daniel
> > <berrange@redhat.com>
> > Cc: qemu-devel@nongnu.org
> > Subject: Re: [PATCH 0/3] ui/gtk: introducing vc->visible
> >
> > Hi Kim
> >
> > I am uncomfortable with the series in general.
> >
> > Not only we don't have the means to draw dmabuf/scanout "when required", so
> > resuming drawing won't work until the guest draws (this is already a problem but
> [Kim, Dongwon] Yes, this is right. The display won't be updated until there is a new frame submitted.
> > you are only making it worse). And I also think reconfiguring the guest by merely
> > minimizing or switching window/tabs isn't what most users would expect.
> [Kim, Dongwon] I understand your concern. Then what do you suggest I need to do or look into to avoid the situation where the host rendering of the guest frame is scheduled but pending due to tab switching or minimization of window as the guest (virtio-gpu drv) is waiting for the response to move on to the next frame? Do you think the frame update should just continue on the host side regardless of visibility of the window? (If this is the standard expectation, then one of our Linux configuration - Yocto + ICE WM has some bug in it.)
>


Given that we can't pause/resume the drawing, I think it's best to
always draw regardless of the visibility. If GTK doesn't draw when
minimized or hidden, we should find a way to "force" that.


> >
> > (fwiw, my personal opinion is that QEMU shouldn't provide UIs and different
> > clients should be able to implement different behaviours, out of process.. that
> > makes me relatively less motivated to break things and be responsible)
> >
> > Daniel, could you have a look too?
> >
> > thanks
> >
> > On Fri, Mar 1, 2024 at 4:05 AM Kim, Dongwon <dongwon.kim@intel.com>
> > wrote:
> > >
> > > Hi Marc-André Lureau,
> > >
> > > Just a reminder.. I need your help reviewing this patch series. Please
> > > take a look at my messages at
> > > https://lists.gnu.org/archive/html/qemu-devel/2024-01/msg06636.html
> > > and
> > > https://lists.gnu.org/archive/html/qemu-devel/2024-01/msg06637.html
> > >
> > > 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
> > > > dongwon.kim@intel.com
> > > > Sent: Tuesday, January 30, 2024 3:49 PM
> > > > To: qemu-devel@nongnu.org
> > > > Subject: [PATCH 0/3] ui/gtk: introducing vc->visible
> > > >
> > > > From: Dongwon Kim <dongwon.kim@intel.com>
> > > >
> > > > Drawing guest display frames can't be completed while the VC is not
> > > > in visible state, which could result in timeout in both the host and
> > > > the guest especially when using blob scanout. Therefore it is needed
> > > > to update and track the visiblity status of the VC and unblock the pipeline in
> > case when VC becomes invisible (e.g.
> > > > windows minimization, switching among tabs) while processing a guest
> > frame.
> > > >
> > > > First patch (0001-ui-gtk-skip...) is introducing a flag 'visible' to
> > > > VirtualConsole struct then set it only if the VC and its window is visible.
> > > >
> > > > Second patch (0002-ui-gtk-set-...) sets the ui size to 0 when VC is
> > > > invisible when the tab is closed or deactivated. This notifies the
> > > > guest that the associated guest display is not active anymore.
> > > >
> > > > Third patch (0003-ui-gtk-reset-visible...) adds a callback for GTK
> > > > window-state- event. The flag, 'visible' is updated based on the minization
> > status of the window.
> > > >
> > > > Dongwon Kim (3):
> > > >   ui/gtk: skip drawing guest scanout when associated VC is invisible
> > > >   ui/gtk: set the ui size to 0 when invisible
> > > >   ui/gtk: reset visible flag when window is minimized
> > > >
> > > >  include/ui/gtk.h |  1 +
> > > >  ui/gtk-egl.c     |  8 +++++++
> > > >  ui/gtk-gl-area.c |  8 +++++++
> > > >  ui/gtk.c         | 62 ++++++++++++++++++++++++++++++++++++++++++++++--
> > > >  4 files changed, 77 insertions(+), 2 deletions(-)
> > > >
> > > > --
> > > > 2.34.1
> > > >
> > >
>


-- 
Marc-André Lureau