[RFC PATCH 0/9] ui: guest displays multiple connectors suppport and hotplug in

[RFC PATCH 0/9] ui: guest displays multiple connectors suppport and hotplug in

[Dongwon Kim]

(This series replace two patch series,
[PATCH v2 0/6] ui/gtk: Add a new parameter to assign connectors/monitors (v2)
https://lists.gnu.org/archive/html/qemu-devel/2022-11/msg03098.html and
[RFC PATCH 0/3] ui/gtk: no render event when vc is invisible
https://lists.gnu.org/archive/html/qemu-devel/2022-10/msg04926.html) 

There is a need (expressed by several customers/users) to assign
ownership of one or more physical monitors/connectors to individual
Guests such that there is a clear notion of which Guest's contents
are being displayed on any given monitor. Given that there is always
a Display Server/Compositor running on the Host, monitor ownership
can never truly be transferred to Guests. However, the closest we
can come to realizing this concept is to request the Host compositor
to fullscreen the Guest's windows on individual monitors. This way,
it would become possible to have 4 different Guests' windows be
displayed on 4 different monitors or a single Guest's windows (or
virtual consoles/outputs) be displayed on 4 monitors or any such
combination.

This patch series attempts to accomplish this by introducing a new
parameter named "connector" to assign the monitors to the GFX VCs
associated with a Guest. If the assigned monitor is not connected,
then the Guest's window would not be displayed anywhere similar to
how a Host compositor would behave when the connectors are not
connected. Once the monitor is hotplugged, the Guest's window(s)
would be positioned on the assigned monitor.

The first 4 patches (0000~0004) are for some prep work that adds a flag
called 'visible' for VC that indicates the visibility of the associated
GTK window and making drawing operation skipped for invisible VCs.

0005 and 0006 are actual implementation of monitors/connectors mapping
to the guests. 0007 through 0009 are additional code changes for preventing
deadlock situation due to asynchronous display hot plug in event when guest
scanout is shared as blobs (zero copy display sharing)

Example Usage: -device virtio-gpu-pci,max_outputs=2,blob=true......
               -display gtk,gl=on,connector.0=eDP-1,connector.1=DP-1.....

Dongwon Kim (6):
  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
  ui/gtk: unblock gl if draw submitted already or fence is not yet
    signaled
  ui/gtk: skip drawing if any of ctx/surface/image don't exist
  ui/gtk: skip refresh/rendering if VC is invisible

Vivek Kasireddy (3):
  ui/gtk: Disable the scanout when a detached tab is closed
  ui/gtk: Factor out tab window creation into a separate function
  ui/gtk: Add a new parameter to assign connectors/monitors to GFX VCs

 include/ui/gtk.h |   2 +
 qapi/ui.json     |  11 +-
 qemu-options.hx  |   5 +-
 ui/gtk-egl.c     |  20 +++
 ui/gtk-gl-area.c |  14 ++
 ui/gtk.c         | 362 +++++++++++++++++++++++++++++++++++++++++++----
 6 files changed, 384 insertions(+), 30 deletions(-)

-- 
2.34.1

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

[Dongwon Kim]

A new flag "visible" that specifies visibility status of the gfx console.
The polarity of the flag determines whether the drawing surface should
continuously updated upon scanout flush. The flag is set to 'true' when
the window bound to the VC is in visible state  but set to 'false' when
the window is inactivated or closed. When invisible, QEMU will skip any of
draw events.

Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Daniel P. Berrangé <berrange@redhat.com>
Cc: Markus Armbruster <armbru@redhat.com>
Cc: Philippe Mathieu-Daudé <philmd@linaro.org>
Cc: Marc-André Lureau <marcandre.lureau@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 ae0f53740d..e7c4726aad 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 19130041bc..443873e266 100644
--- a/ui/gtk-egl.c
+++ b/ui/gtk-egl.c
@@ -247,6 +247,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);
 
@@ -341,6 +345,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 c384a1516b..68b16a5ff1 100644
--- a/ui/gtk-gl-area.c
+++ b/ui/gtk-gl-area.c
@@ -278,6 +278,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;
@@ -291,6 +295,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 e50f950f2b..84c50d835e 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -1350,15 +1350,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;
     }
 }
 
@@ -1388,6 +1393,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),
@@ -1461,6 +1467,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)
@@ -2499,6 +2506,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

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

[Dongwon Kim]

Getting guest displays disconnected by setting ui size to 0 when
the VC is set as invisible. When the VC is set as visible again,
the ui size is restored back to its previous size to reconnect
guest displays.

Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Daniel P. Berrangé <berrange@redhat.com>
Cc: Markus Armbruster <armbru@redhat.com>
Cc: Philippe Mathieu-Daudé <philmd@linaro.org>
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 | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/ui/gtk.c b/ui/gtk.c
index 84c50d835e..ff4a5c58ea 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -1352,10 +1352,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);
@@ -1363,6 +1365,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;
     }
 }
@@ -1394,6 +1399,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),
@@ -1429,6 +1435,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 &&
@@ -1467,6 +1474,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;
 }
 
@@ -1791,7 +1802,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

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

[Dongwon Kim]

Add a callback for window-state-event that resets vc->gfx.visible when
associated window is minimized or restored.

In case of virtio-gpu blob scanout, if the window is minimized before
the rendering event for the last guest scanout frame is finished, it cancels
the draw submission and unblocks the pipeline to prevent a permanent lockup.

Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Daniel P. Berrangé <berrange@redhat.com>
Cc: Markus Armbruster <armbru@redhat.com>
Cc: Philippe Mathieu-Daudé <philmd@linaro.org>
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 | 37 +++++++++++++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)

diff --git a/ui/gtk.c b/ui/gtk.c
index ff4a5c58ea..f9096aea14 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -1419,6 +1419,35 @@ 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);
+        }
+    } else {
+        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;
@@ -1460,6 +1489,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)) {
@@ -2498,6 +2530,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

[RFC PATCH 4/9] ui/gtk: Disable the scanout when a detached tab is closed

[Dongwon Kim]

From: Vivek Kasireddy <vivek.kasireddy@intel.com>

When a detached tab window is closed, the underlying (EGL) context
is destroyed; therefore, disable the scanout which also destroys the
underlying framebuffer (id) and other objects. Also add calls to
make the context current in disable scanout and other missing places.

Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Daniel P. Berrangé <berrange@redhat.com>
Cc: Markus Armbruster <armbru@redhat.com>
Cc: Philippe Mathieu-Daudé <philmd@linaro.org>
Cc: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
Signed-off-by: Dongwon Kim <dongwon.kim@intel.com>
---
 ui/gtk-egl.c     | 3 +++
 ui/gtk-gl-area.c | 2 ++
 ui/gtk.c         | 1 +
 3 files changed, 6 insertions(+)

diff --git a/ui/gtk-egl.c b/ui/gtk-egl.c
index 443873e266..aa22ebbd98 100644
--- a/ui/gtk-egl.c
+++ b/ui/gtk-egl.c
@@ -214,6 +214,9 @@ void gd_egl_scanout_disable(DisplayChangeListener *dcl)
 {
     VirtualConsole *vc = container_of(dcl, VirtualConsole, gfx.dcl);
 
+    eglMakeCurrent(qemu_egl_display, vc->gfx.esurface,
+                   vc->gfx.esurface, vc->gfx.ectx);
+
     vc->gfx.w = 0;
     vc->gfx.h = 0;
     gtk_egl_set_scanout_mode(vc, false);
diff --git a/ui/gtk-gl-area.c b/ui/gtk-gl-area.c
index 68b16a5ff1..8228cc9f3f 100644
--- a/ui/gtk-gl-area.c
+++ b/ui/gtk-gl-area.c
@@ -270,6 +270,7 @@ void gd_gl_area_scanout_disable(DisplayChangeListener *dcl)
 {
     VirtualConsole *vc = container_of(dcl, VirtualConsole, gfx.dcl);
 
+    gtk_gl_area_make_current(GTK_GL_AREA(vc->gfx.drawing_area));
     gtk_gl_area_set_scanout_mode(vc, false);
 }
 
@@ -282,6 +283,7 @@ void gd_gl_area_scanout_flush(DisplayChangeListener *dcl,
         return;
     }
 
+    gtk_gl_area_make_current(GTK_GL_AREA(vc->gfx.drawing_area));
     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.c b/ui/gtk.c
index f9096aea14..90ecb8b82f 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -1400,6 +1400,7 @@ static gboolean gd_tab_window_close(GtkWidget *widget, GdkEvent *event,
 
     vc->gfx.visible = false;
     gd_set_ui_size(vc, 0, 0);
+    dpy_gl_scanout_disable(vc->gfx.dcl.con);
     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),
-- 
2.34.1

[RFC PATCH 5/9] ui/gtk: Factor out tab window creation into a separate function

[Dongwon Kim]

From: Vivek Kasireddy <vivek.kasireddy@intel.com>

Pull the code that creates a new window associated with a notebook
tab into a separate function. This new function can be useful not
just when user wants to detach a tab but also in the future when
a new window creation is needed in other scenarios.

Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Daniel P. Berrangé <berrange@redhat.com>
Cc: Markus Armbruster <armbru@redhat.com>
Cc: Philippe Mathieu-Daudé <philmd@linaro.org>
Cc: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
Signed-off-by: Dongwon Kim <dongwon.kim@intel.com>
---
 ui/gtk.c | 71 +++++++++++++++++++++++++++++++-------------------------
 1 file changed, 39 insertions(+), 32 deletions(-)

diff --git a/ui/gtk.c b/ui/gtk.c
index 90ecb8b82f..d8323a3a9d 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -1462,6 +1462,44 @@ static gboolean gd_win_grab(void *opaque)
     return TRUE;
 }
 
+static void gd_tab_window_create(VirtualConsole *vc)
+{
+    GtkDisplayState *s = vc->s;
+
+    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);
+	vc->gfx.esurface = NULL;
+    }
+    if (vc->gfx.ectx) {
+	eglDestroyContext(qemu_egl_display, vc->gfx.ectx);
+	vc->gfx.ectx = NULL;
+    }
+#endif
+    gd_widget_reparent(s->notebook, vc->window, vc->tab_item);
+
+    g_signal_connect(vc->window, "delete-event",
+		     G_CALLBACK(gd_tab_window_close), vc);
+    gtk_widget_show_all(vc->window);
+
+    if (qemu_console_is_graphic(vc->gfx.dcl.con)) {
+	GtkAccelGroup *ag = gtk_accel_group_new();
+	gtk_window_add_accel_group(GTK_WINDOW(vc->window), ag);
+
+	GClosure *cb = g_cclosure_new_swap(G_CALLBACK(gd_win_grab),
+					   vc, NULL);
+	gtk_accel_group_connect(ag, GDK_KEY_g, HOTKEY_MODIFIERS, 0, cb);
+    }
+
+    gd_update_geometry_hints(vc);
+    gd_update_caption(s);
+}
+
 static void gd_menu_untabify(GtkMenuItem *item, void *opaque)
 {
     GtkDisplayState *s = opaque;
@@ -1474,38 +1512,7 @@ static void gd_menu_untabify(GtkMenuItem *item, void *opaque)
                                        FALSE);
     }
     if (!vc->window) {
-        gtk_widget_set_sensitive(vc->menu_item, false);
-        vc->window = gtk_window_new(GTK_WINDOW_TOPLEVEL);
-#if defined(CONFIG_OPENGL)
-        if (vc->gfx.esurface) {
-            eglDestroySurface(qemu_egl_display, vc->gfx.esurface);
-            vc->gfx.esurface = NULL;
-        }
-        if (vc->gfx.esurface) {
-            eglDestroyContext(qemu_egl_display, vc->gfx.ectx);
-            vc->gfx.ectx = NULL;
-        }
-#endif
-        gd_widget_reparent(s->notebook, vc->window, vc->tab_item);
-
-        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)) {
-            GtkAccelGroup *ag = gtk_accel_group_new();
-            gtk_window_add_accel_group(GTK_WINDOW(vc->window), ag);
-
-            GClosure *cb = g_cclosure_new_swap(G_CALLBACK(gd_win_grab),
-                                               vc, NULL);
-            gtk_accel_group_connect(ag, GDK_KEY_g, HOTKEY_MODIFIERS, 0, cb);
-        }
-
-        gd_update_geometry_hints(vc);
-        gd_update_caption(s);
+        gd_tab_window_create(vc);
     }
 
     window = gtk_widget_get_window(vc->gfx.drawing_area);
-- 
2.34.1

[RFC PATCH 6/9] ui/gtk: Add a new parameter to assign connectors/monitors to GFX VCs

[Dongwon Kim]

From: Vivek Kasireddy <vivek.kasireddy@intel.com>

The new parameter named "connector" can be used to assign physical
monitors/connectors to individual GFX VCs such that when the monitor
is connected or hotplugged, the associated GTK window would be
moved to it. If the monitor is disconnected or unplugged, the
associated GTK window would be hidden and a relevant disconnect
event would be sent to the Guest.

Usage: -device virtio-gpu-pci,max_outputs=2,blob=true,...
       -display gtk,gl=on,connectors.0=eDP-1,connectors.1=DP-1.....

Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Daniel P. Berrangé <berrange@redhat.com>
Cc: Markus Armbruster <armbru@redhat.com>
Cc: Philippe Mathieu-Daudé <philmd@linaro.org>
Cc: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
Signed-off-by: Dongwon Kim <dongwon.kim@intel.com>
---
 include/ui/gtk.h |   1 +
 qapi/ui.json     |  11 +-
 qemu-options.hx  |   5 +-
 ui/gtk.c         | 271 +++++++++++++++++++++++++++++++++++++++++++----
 4 files changed, 263 insertions(+), 25 deletions(-)

diff --git a/include/ui/gtk.h b/include/ui/gtk.h
index e7c4726aad..189817ab88 100644
--- a/include/ui/gtk.h
+++ b/include/ui/gtk.h
@@ -84,6 +84,7 @@ typedef struct VirtualConsole {
     GtkWidget *menu_item;
     GtkWidget *tab_item;
     GtkWidget *focus;
+    GdkMonitor *monitor;
     VirtualConsoleType type;
     union {
         VirtualGfxConsole gfx;
diff --git a/qapi/ui.json b/qapi/ui.json
index 2755395483..0f5ab35bae 100644
--- a/qapi/ui.json
+++ b/qapi/ui.json
@@ -1315,13 +1315,22 @@
 # @show-menubar: Display the main window menubar.  Defaults to "on".
 #     (Since 8.0)
 #
+# @connectors:  List of physical monitor/connector names where the GTK
+#               windows containing the respective graphics virtual consoles
+#               (VCs) are to be placed. If a mapping exists for a VC, it
+#               will be moved to that specific monitor or else it would
+#               not be displayed anywhere and would appear disconnected
+#               to the guest.
+#               (Since 8.1)
+#
 # Since: 2.12
 ##
 { 'struct'  : 'DisplayGTK',
   'data'    : { '*grab-on-hover' : 'bool',
                 '*zoom-to-fit'   : 'bool',
                 '*show-tabs'     : 'bool',
-                '*show-menubar'  : 'bool'  } }
+                '*show-menubar'  : 'bool',
+                '*connectors'    : ['str'] } }
 
 ##
 # @DisplayEGLHeadless:
diff --git a/qemu-options.hx b/qemu-options.hx
index b57489d7ca..2eb0d6a129 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -2044,7 +2044,7 @@ DEF("display", HAS_ARG, QEMU_OPTION_display,
 #if defined(CONFIG_GTK)
     "-display gtk[,full-screen=on|off][,gl=on|off][,grab-on-hover=on|off]\n"
     "            [,show-tabs=on|off][,show-cursor=on|off][,window-close=on|off]\n"
-    "            [,show-menubar=on|off]\n"
+    "            [,show-menubar=on|off][,connectors.<index>=<connector name>]\n"
 #endif
 #if defined(CONFIG_VNC)
     "-display vnc=<display>[,<optargs>]\n"
@@ -2139,6 +2139,9 @@ SRST
 
         ``show-menubar=on|off`` : Display the main window menubar, defaults to "on"
 
+        ``connectors=<conn name>`` : VC to connector mappings to display the VC
+                                     window on a specific monitor
+
     ``curses[,charset=<encoding>]``
         Display video output via curses. For graphics device models
         which support a text mode, QEMU can display this output using a
diff --git a/ui/gtk.c b/ui/gtk.c
index d8323a3a9d..f4c71454a3 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -38,6 +38,7 @@
 #include "qemu/cutils.h"
 #include "qemu/error-report.h"
 #include "qemu/main-loop.h"
+#include "qemu/option.h"
 
 #include "ui/console.h"
 #include "ui/gtk.h"
@@ -741,6 +742,39 @@ static void gd_set_ui_size(VirtualConsole *vc, gint width, gint height)
     dpy_set_ui_info(vc->gfx.dcl.con, &info, true);
 }
 
+static void gd_ui_hide(VirtualConsole *vc)
+{
+    QemuUIInfo info;
+
+    vc->gfx.visible = false;
+    info = *dpy_get_ui_info(vc->gfx.dcl.con);
+    info.width = 0;
+    info.height = 0;
+    dpy_set_ui_info(vc->gfx.dcl.con, &info, false);
+}
+
+static void gd_ui_show(VirtualConsole *vc)
+{
+    QemuUIInfo info;
+    GtkDisplayState *s = vc->s;
+    GdkWindow *window = gtk_widget_get_window(vc->gfx.drawing_area);
+
+    info = *dpy_get_ui_info(vc->gfx.dcl.con);
+    info.width = gdk_window_get_width(window);
+    info.height = gdk_window_get_height(window);
+    dpy_set_ui_info(vc->gfx.dcl.con, &info, false);
+
+    if (gd_is_grab_active(s)) {
+        gd_grab_keyboard(vc, "user-request-main-window");
+        gd_grab_pointer(vc, "user-request-main-window");
+    } else {
+        gd_ungrab_keyboard(s);
+        gd_ungrab_pointer(s);
+    }
+
+    vc->gfx.visible = true;
+}
+
 #if defined(CONFIG_OPENGL)
 
 static gboolean gd_render_event(GtkGLArea *area, GdkGLContext *context,
@@ -1352,12 +1386,10 @@ 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);
+    gd_ui_hide(vc);
 
     vc = gd_vc_find_by_menu(s);
     gtk_release_modifiers(s);
@@ -1365,10 +1397,7 @@ 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;
+        gd_ui_show(vc);
     }
 }
 
@@ -1398,8 +1427,8 @@ static gboolean gd_tab_window_close(GtkWidget *widget, GdkEvent *event,
     VirtualConsole *vc = opaque;
     GtkDisplayState *s = vc->s;
 
-    vc->gfx.visible = false;
-    gd_set_ui_size(vc, 0, 0);
+    gd_ui_hide(vc);
+
     dpy_gl_scanout_disable(vc->gfx.dcl.con);
     gtk_widget_set_sensitive(vc->menu_item, true);
     gd_widget_reparent(vc->window, s->notebook, vc->tab_item);
@@ -1430,20 +1459,14 @@ static gboolean gd_window_state_event(GtkWidget *widget, GdkEvent *event,
     }
 
     if (event->window_state.new_window_state & GDK_WINDOW_STATE_ICONIFIED) {
-        vc->gfx.visible = false;
-        gd_set_ui_size(vc, 0, 0);
+        gd_ui_hide(vc);
         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);
         }
     } else {
-        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;
+        gd_ui_show(vc);
     }
 
     return TRUE;
@@ -1503,7 +1526,6 @@ static void gd_tab_window_create(VirtualConsole *vc)
 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 &&
@@ -1515,10 +1537,205 @@ static void gd_menu_untabify(GtkMenuItem *item, void *opaque)
         gd_tab_window_create(vc);
     }
 
-    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;
+    gd_ui_show(vc);
+}
+
+static void gd_window_show_on_monitor(GdkDisplay *dpy, VirtualConsole *vc,
+                                      gint monitor_num)
+{
+    GtkDisplayState *s = vc->s;
+    GdkMonitor *monitor = gdk_display_get_monitor(dpy, monitor_num);
+    GdkRectangle geometry;
+
+    if (!vc->window) {
+        gd_tab_window_create(vc);
+    }
+
+    gdk_window_show(gtk_widget_get_window(vc->window));
+    gd_update_windowsize(vc);
+    gdk_monitor_get_geometry(monitor, &geometry);
+    /*
+     * Note: some compositors (mainly Wayland ones) may not honor a
+     * request to move to a particular location. The user is expected
+     * to drag the window to the preferred location in this case.
+     */
+    gtk_window_move(GTK_WINDOW(vc->window), geometry.x, geometry.y);
+
+    if (s->opts->has_full_screen && s->opts->full_screen) {
+        gtk_widget_set_size_request(vc->gfx.drawing_area, -1, -1);
+        gtk_window_fullscreen(GTK_WINDOW(vc->window));
+    }
+
+    vc->monitor = monitor;
+    gd_ui_show(vc);
+
+    if (vc->window) {
+        g_signal_connect(vc->window, "window-state-event",
+                         G_CALLBACK(gd_window_state_event), vc);
+    }
+
+    gd_update_cursor(vc);
+}
+
+static int gd_monitor_lookup(GdkDisplay *dpy, char *label)
+{
+    GdkMonitor *monitor;
+    int total_monitors = gdk_display_get_n_monitors(dpy);
+    int i;
+
+    for (i = 0; i < total_monitors; i++) {
+        monitor = gdk_display_get_monitor(dpy, i);
+        if (monitor && !g_strcmp0(gdk_monitor_get_model(monitor), label)) {
+            return i;
+        }
+    }
+    return -1;
+}
+
+static gboolean gd_vc_is_misplaced(GdkDisplay *dpy, GdkMonitor *monitor,
+                                   VirtualConsole *vc)
+{
+    GdkWindow *window = gtk_widget_get_window(vc->gfx.drawing_area);
+    GdkMonitor *mon = gdk_display_get_monitor_at_window(dpy, window);
+    const char *monitor_name = gdk_monitor_get_model(monitor);
+
+    if (!vc->monitor) {
+        if (!g_strcmp0(monitor_name, vc->label)) {
+            return TRUE;
+        }
+    } else {
+        if (mon && mon != vc->monitor) {
+            return TRUE;
+        }
+    }
+    return FALSE;
+}
+
+static void gd_vc_add_remove_monitor(GdkDisplay *dpy, GtkDisplayState *s)
+{
+    VirtualConsole *vc;
+    GdkMonitor *monitor;
+    gint monitor_num;
+    int i;
+
+    /*
+     * We need to call gd_vc_is_misplaced() after a monitor is added to
+     * ensure that the Host compositor has not moved our windows around.
+     */
+    for (i = 0; i < s->nb_vcs; i++) {
+        vc = &s->vc[i];
+        if (vc->label) {
+            monitor_num = gd_monitor_lookup(dpy, vc->label);
+            if (monitor_num >= 0) {
+                monitor = gdk_display_get_monitor(dpy, monitor_num);
+                if (gd_vc_is_misplaced(dpy, monitor, vc)) {
+                    gd_window_show_on_monitor(dpy, vc, monitor_num);
+                }
+            } else if (vc->monitor) {
+                vc->monitor = NULL;
+                if (vc->window) {
+                    g_signal_handlers_disconnect_by_func(vc->window,
+                                                 G_CALLBACK(gd_window_state_event),
+                                                 vc);
+                }
+
+                gd_ui_hide(vc);
+                /* if window exist, hide it */
+                if (vc->window) {
+                    gdk_window_hide(gtk_widget_get_window(vc->window));
+                }
+            }
+        }
+    }
+}
+
+static void gd_monitors_reset_timer(void *opaque)
+{
+    GtkDisplayState *s = opaque;
+    GdkDisplay *dpy = gdk_display_get_default();
+
+    gd_vc_add_remove_monitor(dpy, s);
+}
+
+static void gd_monitors_changed(GdkScreen *scr, void *opaque)
+{
+    GtkDisplayState *s = opaque;
+    QEMUTimer *mon_reset_timer;
+
+    mon_reset_timer = timer_new_ms(QEMU_CLOCK_REALTIME,
+                                   gd_monitors_reset_timer, s);
+    timer_mod(mon_reset_timer,
+              qemu_clock_get_ms(QEMU_CLOCK_REALTIME) + 2000);
+}
+
+static VirtualConsole *gd_next_gfx_vc(GtkDisplayState *s)
+{
+    VirtualConsole *vc;
+    int i;
+
+    for (i = 0; i < s->nb_vcs; i++) {
+        vc = &s->vc[i];
+        if (vc->type == GD_VC_GFX &&
+            qemu_console_is_graphic(vc->gfx.dcl.con) &&
+            !vc->label) {
+            return vc;
+        }
+    }
+    return NULL;
+}
+
+static void gd_vc_free_labels(GtkDisplayState *s)
+{
+    VirtualConsole *vc;
+    int i;
+
+    for (i = 0; i < s->nb_vcs; i++) {
+        vc = &s->vc[i];
+        if (vc->type == GD_VC_GFX &&
+            qemu_console_is_graphic(vc->gfx.dcl.con)) {
+            g_free(vc->label);
+            vc->label = NULL;
+        }
+    }
+}
+
+static void gd_connectors_init(GdkDisplay *dpy, GtkDisplayState *s)
+{
+    VirtualConsole *vc;
+    strList *conn;
+    gint monitor_num;
+    gboolean first_vc = TRUE;
+
+    gtk_notebook_set_show_tabs(GTK_NOTEBOOK(s->notebook), FALSE);
+    gtk_check_menu_item_set_active(GTK_CHECK_MENU_ITEM(s->grab_item),
+                                   FALSE);
+    gd_vc_free_labels(s);
+    for (conn = s->opts->u.gtk.connectors; conn; conn = conn->next) {
+        vc = gd_next_gfx_vc(s);
+        if (!vc) {
+            break;
+        }
+        if (first_vc) {
+            vc->window = s->window;
+            first_vc = FALSE;
+        }
+
+        vc->label = g_strdup(conn->value);
+        monitor_num = gd_monitor_lookup(dpy, vc->label);
+        if (monitor_num >= 0) {
+            gd_window_show_on_monitor(dpy, vc, monitor_num);
+        } else {
+            if (vc->window) {
+                 g_signal_handlers_disconnect_by_func(vc->window,
+                                             G_CALLBACK(gd_window_state_event),
+                                             vc);
+            }
+            gd_ui_hide(vc);
+            if (vc->window) {
+                gdk_window_hide(gtk_widget_get_window(vc->window));
+            }
+        }
+    }
 }
 
 static void gd_menu_show_menubar(GtkMenuItem *item, void *opaque)
@@ -1843,7 +2060,7 @@ static gboolean gd_configure(GtkWidget *widget,
     VirtualConsole *vc = opaque;
 
     if (vc->gfx.visible) {
-        gd_set_ui_size(vc, cfg->width, cfg->height);
+        gd_ui_show(vc);
     }
     return FALSE;
 }
@@ -2180,6 +2397,10 @@ static void gd_connect_signals(GtkDisplayState *s)
                      G_CALLBACK(gd_menu_grab_input), s);
     g_signal_connect(s->notebook, "switch-page",
                      G_CALLBACK(gd_change_page), s);
+    if (s->opts->u.gtk.has_connectors) {
+        g_signal_connect(gdk_screen_get_default(), "monitors-changed",
+                         G_CALLBACK(gd_monitors_changed), s);
+    }
 }
 
 static GtkWidget *gd_create_menu_machine(GtkDisplayState *s)
@@ -2561,6 +2782,10 @@ static void gtk_display_init(DisplayState *ds, DisplayOptions *opts)
         opts->u.gtk.show_tabs) {
         gtk_menu_item_activate(GTK_MENU_ITEM(s->show_tabs_item));
     }
+
+    if (s->opts->u.gtk.has_connectors) {
+        gd_connectors_init(window_display, s);
+    }
 #ifdef CONFIG_GTK_CLIPBOARD
     gd_clipboard_init(s);
 #endif /* CONFIG_GTK_CLIPBOARD */
-- 
2.34.1

Re: [RFC PATCH 6/9] ui/gtk: Add a new parameter to assign connectors/monitors to GFX VCs

[Markus Armbruster]

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

> From: Vivek Kasireddy <vivek.kasireddy@intel.com>
>
> The new parameter named "connector" can be used to assign physical
> monitors/connectors to individual GFX VCs such that when the monitor
> is connected or hotplugged, the associated GTK window would be
> moved to it. If the monitor is disconnected or unplugged, the
> associated GTK window would be hidden and a relevant disconnect
> event would be sent to the Guest.
>
> Usage: -device virtio-gpu-pci,max_outputs=2,blob=true,...
>        -display gtk,gl=on,connectors.0=eDP-1,connectors.1=DP-1.....
>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Cc: Daniel P. Berrangé <berrange@redhat.com>
> Cc: Markus Armbruster <armbru@redhat.com>
> Cc: Philippe Mathieu-Daudé <philmd@linaro.org>
> Cc: Marc-André Lureau <marcandre.lureau@redhat.com>
> Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
> Signed-off-by: Dongwon Kim <dongwon.kim@intel.com>

[...]

> --- a/qapi/ui.json
> +++ b/qapi/ui.json
> @@ -1315,13 +1315,22 @@
>  # @show-menubar: Display the main window menubar.  Defaults to "on".
>  #     (Since 8.0)
>  #
> +# @connectors:  List of physical monitor/connector names where the GTK
> +#               windows containing the respective graphics virtual consoles
> +#               (VCs) are to be placed. If a mapping exists for a VC, it
> +#               will be moved to that specific monitor or else it would
> +#               not be displayed anywhere and would appear disconnected
> +#               to the guest.
> +#               (Since 8.1)

Please format like

   # @connectors: List of physical monitor/connector names where the GTK
   #     windows containing the respective graphics virtual consoles
   #     (VCs) are to be placed.  If a mapping exists for a VC, it will
   #     be moved to that specific monitor or else it would not be
   #     displayed anywhere and would appear disconnected to the guest.
   #     (Since 8.1)

to blend in with recent commit a937b6aa739 (qapi: Reformat doc comments
to conform to current conventions).

The meaning of @connectors is less than clear.  The phrase "If a mapping
exists for a VC" suggests it is a mapping of sorts.  "List of physical
monitor/connector names" indicates it maps to physical monitor /
connector name.  What does it map from?  VC number?  How are VCs
numbered?  Is it the same number we use in QOM /backend/console[NUM]?

Using a list for the mapping means the mapping must be dense, e.g. I
can't map #0 and #2 but not #1.  Is this what we want?

The sentence "If a mapping exists" confusing has a dangling else
ambiguity of sorts.  I can interpret it as

    If a mapping exists for a VC:
        the window will be moved to that specific monitor
        or else it would not be displayed anywhere and would appear ...

or as

    If a mapping exists for a VC:
        the window will be moved to that specific monitor
    or else it would not be displayed anywhere and would appear ...

I think we have three cases:

0. No mapping provided

1. Mapping provided, and the named monitor / connector exists

2. Mapping provided, and the named monitor / connector does not exist

We can go from case 1 to 2 (disconnect) and vice versa (connect) at any
time.

Please spell out behavior for each case, and for the transitions between
case 1 and 2.

> +#
>  # Since: 2.12
>  ##
>  { 'struct'  : 'DisplayGTK',
>    'data'    : { '*grab-on-hover' : 'bool',
>                  '*zoom-to-fit'   : 'bool',
>                  '*show-tabs'     : 'bool',
> -                '*show-menubar'  : 'bool'  } }
> +                '*show-menubar'  : 'bool',
> +                '*connectors'    : ['str'] } }
>  
>  ##
>  # @DisplayEGLHeadless:

[...]

Re: [RFC PATCH 6/9] ui/gtk: Add a new parameter to assign connectors/monitors to GFX VCs

[Kim, Dongwon]

Hi Markus,

So I've worked on the description of this param. Can you check if this new version looks ok?

# @connectors:  List of physical monitor/connector names where the GTK
#           windows containing the respective graphics virtual consoles (VCs)
#           are to be placed. Index of the connector name in the array directly
#           indicates the id of the VC.
#           For example, with "-device gtk,connectors.0=DP-1, connectors.1=DP-2",
#           a physical display connected to DP-1 port will be the target monitor
#           for VC0 and the one on DP-2 will be the target for VC1. If there is
#           no connector associated with a VC, then that VC won't be placed anywhere
#           before the QEMU is relaunched with a proper connector name set for it.
#           If a connector name exists for a VC but the display cable is not plugged
#           in when guest is launched, the VC will be just hidden but will show up
#           as soon as the cable is plugged in. If a display is connected in the beginning
#           but later disconnected, VC will immediately be hidden and guest will detect
#           it as a disconnected display. This option does not force 1 to 1 mapping
#           between the connector and the VC, which means multiple VCs can be placed
#           on the same display but vice versa is not possible (a single VC duplicated
#           on a multiple displays)
#           (Since 8.1)

Thanks,
DW

On 6/20/2023 10:51 PM, Markus Armbruster wrote:

> Dongwon Kim <dongwon.kim@intel.com> writes:
>
>> From: Vivek Kasireddy <vivek.kasireddy@intel.com>
>>
>> The new parameter named "connector" can be used to assign physical
>> monitors/connectors to individual GFX VCs such that when the monitor
>> is connected or hotplugged, the associated GTK window would be
>> moved to it. If the monitor is disconnected or unplugged, the
>> associated GTK window would be hidden and a relevant disconnect
>> event would be sent to the Guest.
>>
>> Usage: -device virtio-gpu-pci,max_outputs=2,blob=true,...
>>         -display gtk,gl=on,connectors.0=eDP-1,connectors.1=DP-1.....
>>
>> Cc: Gerd Hoffmann <kraxel@redhat.com>
>> Cc: Daniel P. Berrangé <berrange@redhat.com>
>> Cc: Markus Armbruster <armbru@redhat.com>
>> Cc: Philippe Mathieu-Daudé <philmd@linaro.org>
>> Cc: Marc-André Lureau <marcandre.lureau@redhat.com>
>> Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
>> Signed-off-by: Dongwon Kim <dongwon.kim@intel.com>
> [...]
>
>> --- a/qapi/ui.json
>> +++ b/qapi/ui.json
>> @@ -1315,13 +1315,22 @@
>>   # @show-menubar: Display the main window menubar.  Defaults to "on".
>>   #     (Since 8.0)
>>   #
>> +# @connectors:  List of physical monitor/connector names where the GTK
>> +#               windows containing the respective graphics virtual consoles
>> +#               (VCs) are to be placed. If a mapping exists for a VC, it
>> +#               will be moved to that specific monitor or else it would
>> +#               not be displayed anywhere and would appear disconnected
>> +#               to the guest.
>> +#               (Since 8.1)
> Please format like
>
>     # @connectors: List of physical monitor/connector names where the GTK
>     #     windows containing the respective graphics virtual consoles
>     #     (VCs) are to be placed.  If a mapping exists for a VC, it will
>     #     be moved to that specific monitor or else it would not be
>     #     displayed anywhere and would appear disconnected to the guest.
>     #     (Since 8.1)
>
> to blend in with recent commit a937b6aa739 (qapi: Reformat doc comments
> to conform to current conventions).
>
> The meaning of @connectors is less than clear.  The phrase "If a mapping
> exists for a VC" suggests it is a mapping of sorts.  "List of physical
> monitor/connector names" indicates it maps to physical monitor /
> connector name.  What does it map from?  VC number?  How are VCs
> numbered?  Is it the same number we use in QOM /backend/console[NUM]?
>
> Using a list for the mapping means the mapping must be dense, e.g. I
> can't map #0 and #2 but not #1.  Is this what we want?
>
> The sentence "If a mapping exists" confusing has a dangling else
> ambiguity of sorts.  I can interpret it as
>
>      If a mapping exists for a VC:
>          the window will be moved to that specific monitor
>          or else it would not be displayed anywhere and would appear ...
>
> or as
>
>      If a mapping exists for a VC:
>          the window will be moved to that specific monitor
>      or else it would not be displayed anywhere and would appear ...
>
> I think we have three cases:
>
> 0. No mapping provided
>
> 1. Mapping provided, and the named monitor / connector exists
>
> 2. Mapping provided, and the named monitor / connector does not exist
>
> We can go from case 1 to 2 (disconnect) and vice versa (connect) at any
> time.
>
> Please spell out behavior for each case, and for the transitions between
> case 1 and 2.
>
>> +#
>>   # Since: 2.12
>>   ##
>>   { 'struct'  : 'DisplayGTK',
>>     'data'    : { '*grab-on-hover' : 'bool',
>>                   '*zoom-to-fit'   : 'bool',
>>                   '*show-tabs'     : 'bool',
>> -                '*show-menubar'  : 'bool'  } }
>> +                '*show-menubar'  : 'bool',
>> +                '*connectors'    : ['str'] } }
>>   
>>   ##
>>   # @DisplayEGLHeadless:
> [...]
>

Re: [RFC PATCH 6/9] ui/gtk: Add a new parameter to assign connectors/monitors to GFX VCs

[Markus Armbruster]

"Kim, Dongwon" <dongwon.kim@intel.com> writes:

> Hi Markus,
>
> So I've worked on the description of this param. Can you check if this new version looks ok?
>
> # @connectors:  List of physical monitor/connector names where the GTK
> #           windows containing the respective graphics virtual consoles (VCs)
> #           are to be placed. Index of the connector name in the array directly
> #           indicates the id of the VC.
> #           For example, with "-device gtk,connectors.0=DP-1, connectors.1=DP-2",
> #           a physical display connected to DP-1 port will be the target monitor
> #           for VC0 and the one on DP-2 will be the target for VC1. If there is
> #           no connector associated with a VC, then that VC won't be placed anywhere
> #           before the QEMU is relaunched with a proper connector name set for it.
> #           If a connector name exists for a VC but the display cable is not plugged
> #           in when guest is launched, the VC will be just hidden but will show up
> #           as soon as the cable is plugged in. If a display is connected in the beginning
> #           but later disconnected, VC will immediately be hidden and guest will detect
> #           it as a disconnected display. This option does not force 1 to 1 mapping
> #           between the connector and the VC, which means multiple VCs can be placed
> #           on the same display but vice versa is not possible (a single VC duplicated
> #           on a multiple displays)
> #           (Since 8.1)

Better!

Suggest to replace "that VC won't be placed anywhere" by "that VC won't
be displayed".

Ignorant questions:

1. How would I plug / unplug display cables?

2. If I connect multiple VCs to the same display, what will I see?  Are
they multiplexed somehow?

Old question not yet answered: Using a list for the mapping means the
mapping must be dense, e.g. I can't map #0 and #2 but not #1.  Is this
what we want?

[...]

Re: [RFC PATCH 6/9] ui/gtk: Add a new parameter to assign connectors/monitors to GFX VCs

[Kim, Dongwon]

On 7/7/2023 7:07 AM, Markus Armbruster wrote:
> "Kim, Dongwon" <dongwon.kim@intel.com> writes:
>
>> Hi Markus,
>>
>> So I've worked on the description of this param. Can you check if this new version looks ok?
>>
>> # @connectors:  List of physical monitor/connector names where the GTK
>> #           windows containing the respective graphics virtual consoles (VCs)
>> #           are to be placed. Index of the connector name in the array directly
>> #           indicates the id of the VC.
>> #           For example, with "-device gtk,connectors.0=DP-1, connectors.1=DP-2",
>> #           a physical display connected to DP-1 port will be the target monitor
>> #           for VC0 and the one on DP-2 will be the target for VC1. If there is
>> #           no connector associated with a VC, then that VC won't be placed anywhere
>> #           before the QEMU is relaunched with a proper connector name set for it.
>> #           If a connector name exists for a VC but the display cable is not plugged
>> #           in when guest is launched, the VC will be just hidden but will show up
>> #           as soon as the cable is plugged in. If a display is connected in the beginning
>> #           but later disconnected, VC will immediately be hidden and guest will detect
>> #           it as a disconnected display. This option does not force 1 to 1 mapping
>> #           between the connector and the VC, which means multiple VCs can be placed
>> #           on the same display but vice versa is not possible (a single VC duplicated
>> #           on a multiple displays)
>> #           (Since 8.1)
> Better!
>
> Suggest to replace "that VC won't be placed anywhere" by "that VC won't
> be displayed".

yeah, I will update it in v2 and send the new patch shortly.

>
> Ignorant questions:
>
> 1. How would I plug / unplug display cables?

I am not sure if I understood your question correctly but 1 or more 
guest displays (GTK windows) are bound to a certain physical displays 
like HDMI or DP monitors. So plug/unplug means we disconnect those 
physical HDMI or DP cables manually. Or this manual hot plug in can be 
emulated by you write something to sysfs depending on what display 
driver you use.

>
> 2. If I connect multiple VCs to the same display, what will I see?  Are
> they multiplexed somehow?

Yeah multiple VCs will be shown on that display. But those could be 
overlapped since those are all placed at (0, 0) of display in many 
cases.. but this all depends on how the windows manager determines the 
starting locations.

>
> Old question not yet answered: Using a list for the mapping means the
> mapping must be dense, e.g. I can't map #0 and #2 but not #1.  Is this
> what we want?

No, it doesn't have to be dense. In your example, you can just leave the 
place for VC1 blank. For example, you could do 
connectors.0=DP-1,connectors.2=HDMI-1. But in this case, VC1 won't be 
activated and stay as disconnected from guest's perspective. I think 
this info is also needed in v2.

>
> [...]
>

Re: [RFC PATCH 6/9] ui/gtk: Add a new parameter to assign connectors/monitors to GFX VCs

[Markus Armbruster]

"Kim, Dongwon" <dongwon.kim@intel.com> writes:

> On 7/7/2023 7:07 AM, Markus Armbruster wrote:
>> "Kim, Dongwon" <dongwon.kim@intel.com> writes:
>>
>>> Hi Markus,
>>>
>>> So I've worked on the description of this param. Can you check if this new version looks ok?
>>>
>>> # @connectors:  List of physical monitor/connector names where the GTK
>>> #           windows containing the respective graphics virtual consoles (VCs)
>>> #           are to be placed. Index of the connector name in the array directly
>>> #           indicates the id of the VC.
>>> #           For example, with "-device gtk,connectors.0=DP-1, connectors.1=DP-2",
>>> #           a physical display connected to DP-1 port will be the target monitor
>>> #           for VC0 and the one on DP-2 will be the target for VC1. If there is
>>> #           no connector associated with a VC, then that VC won't be placed anywhere
>>> #           before the QEMU is relaunched with a proper connector name set for it.
>>> #           If a connector name exists for a VC but the display cable is not plugged
>>> #           in when guest is launched, the VC will be just hidden but will show up
>>> #           as soon as the cable is plugged in. If a display is connected in the beginning
>>> #           but later disconnected, VC will immediately be hidden and guest will detect
>>> #           it as a disconnected display. This option does not force 1 to 1 mapping
>>> #           between the connector and the VC, which means multiple VCs can be placed
>>> #           on the same display but vice versa is not possible (a single VC duplicated
>>> #           on a multiple displays)
>>> #           (Since 8.1)
>> Better!
>>
>> Suggest to replace "that VC won't be placed anywhere" by "that VC won't
>> be displayed".
>
> yeah, I will update it in v2 and send the new patch shortly.
>
>>
>> Ignorant questions:
>>
>> 1. How would I plug / unplug display cables?
>
> I am not sure if I understood your question correctly but 1 or more guest displays (GTK windows) are bound to a certain physical displays like HDMI or DP monitors. So plug/unplug means we disconnect those physical HDMI or DP cables manually. Or this manual hot plug in can be emulated by you write something to sysfs depending on what display driver you use.

Let's see whether I understand.

A VC is placed on a *physical* monitor, i.e. a window appears on that
monitor.  That monitor's plug / unplug state is passed through to the
guest, i.e. if I physically unplug / plug the monitor, the guest sees an
unplug / plug of its virtual monitor.  Correct?

Permit me another ignorant question...  Say I have a single monitor.  I
configured my X windows manager to show four virtual desktops.  Can I
use your feature to direct on which virtual desktop each VC is placed?

>> 2. If I connect multiple VCs to the same display, what will I see?  Are
>> they multiplexed somehow?
>
> Yeah multiple VCs will be shown on that display. But those could be overlapped since those are all placed at (0, 0) of display in many cases.. but this all depends on how the windows manager determines the starting locations.

Got it, thanks!

>> Old question not yet answered: Using a list for the mapping means the
>> mapping must be dense, e.g. I can't map #0 and #2 but not #1.  Is this
>> what we want?
>
> No, it doesn't have to be dense. In your example, you can just leave the place for VC1 blank. For example, you could do connectors.0=DP-1,connectors.2=HDMI-1. But in this case, VC1 won't be activated and stay as disconnected from guest's perspective. I think this info is also needed in v2.

Have you tried this?  I believe it'll fail with something like
"Parameter 'connectors.1' missing".

>> [...]

Re: [RFC PATCH 6/9] ui/gtk: Add a new parameter to assign connectors/monitors to GFX VCs

[Kim, Dongwon]

On 7/9/2023 11:05 PM, Markus Armbruster wrote:
> "Kim, Dongwon" <dongwon.kim@intel.com> writes:
>
>> On 7/7/2023 7:07 AM, Markus Armbruster wrote:
>>> "Kim, Dongwon" <dongwon.kim@intel.com> writes:
>>>
>>>> Hi Markus,
>>>>
>>>> So I've worked on the description of this param. Can you check if this new version looks ok?
>>>>
>>>> # @connectors:  List of physical monitor/connector names where the GTK
>>>> #           windows containing the respective graphics virtual consoles (VCs)
>>>> #           are to be placed. Index of the connector name in the array directly
>>>> #           indicates the id of the VC.
>>>> #           For example, with "-device gtk,connectors.0=DP-1, connectors.1=DP-2",
>>>> #           a physical display connected to DP-1 port will be the target monitor
>>>> #           for VC0 and the one on DP-2 will be the target for VC1. If there is
>>>> #           no connector associated with a VC, then that VC won't be placed anywhere
>>>> #           before the QEMU is relaunched with a proper connector name set for it.
>>>> #           If a connector name exists for a VC but the display cable is not plugged
>>>> #           in when guest is launched, the VC will be just hidden but will show up
>>>> #           as soon as the cable is plugged in. If a display is connected in the beginning
>>>> #           but later disconnected, VC will immediately be hidden and guest will detect
>>>> #           it as a disconnected display. This option does not force 1 to 1 mapping
>>>> #           between the connector and the VC, which means multiple VCs can be placed
>>>> #           on the same display but vice versa is not possible (a single VC duplicated
>>>> #           on a multiple displays)
>>>> #           (Since 8.1)
>>> Better!
>>>
>>> Suggest to replace "that VC won't be placed anywhere" by "that VC won't
>>> be displayed".
>> yeah, I will update it in v2 and send the new patch shortly.
>>
>>> Ignorant questions:
>>>
>>> 1. How would I plug / unplug display cables?
>> I am not sure if I understood your question correctly but 1 or more guest displays (GTK windows) are bound to a certain physical displays like HDMI or DP monitors. So plug/unplug means we disconnect those physical HDMI or DP cables manually. Or this manual hot plug in can be emulated by you write something to sysfs depending on what display driver you use.
> Let's see whether I understand.
>
> A VC is placed on a *physical* monitor, i.e. a window appears on that
> monitor.  That monitor's plug / unplug state is passed through to the
> guest, i.e. if I physically unplug / plug the monitor, the guest sees an
> unplug / plug of its virtual monitor.  Correct?

This is correct. When a display is disconnected, "monitor-changed" GTK 
event will be triggered then it will call gd_ui_size(0,0) which makes 
the guest display connection status to "disconnected".

>
> Permit me another ignorant question...  Say I have a single monitor.  I
> configured my X windows manager to show four virtual desktops.  Can I
> use your feature to direct on which virtual desktop each VC is placed?

Would those virtual desktops will be their own connector names like HDMI 
or DP? We use the connector name for the actual physical display you see 
when running xrandr. I don't know how virtual desktops are created and 
managed but if they don't have their own connector names that GTK API 
can read, it won't work within our current implementation.

>
>>> 2. If I connect multiple VCs to the same display, what will I see?  Are
>>> they multiplexed somehow?
>> Yeah multiple VCs will be shown on that display. But those could be overlapped since those are all placed at (0, 0) of display in many cases.. but this all depends on how the windows manager determines the starting locations.
> Got it, thanks!
>
>>> Old question not yet answered: Using a list for the mapping means the
>>> mapping must be dense, e.g. I can't map #0 and #2 but not #1.  Is this
>>> what we want?
>> No, it doesn't have to be dense. In your example, you can just leave the place for VC1 blank. For example, you could do connectors.0=DP-1,connectors.2=HDMI-1. But in this case, VC1 won't be activated and stay as disconnected from guest's perspective. I think this info is also needed in v2.
> Have you tried this?  I believe it'll fail with something like
> "Parameter 'connectors.1' missing".

Just tested it. Yeah you are correct. I think I had a bad assumption. 
Let me take a look to see if I can make it work as I assumed.

>
>>> [...]

Re: [RFC PATCH 6/9] ui/gtk: Add a new parameter to assign connectors/monitors to GFX VCs

[Markus Armbruster]

"Kim, Dongwon" <dongwon.kim@intel.com> writes:

> On 7/9/2023 11:05 PM, Markus Armbruster wrote:
>> "Kim, Dongwon" <dongwon.kim@intel.com> writes:
>>
>>> On 7/7/2023 7:07 AM, Markus Armbruster wrote:

[...]

>>>> Ignorant questions:
>>>>
>>>> 1. How would I plug / unplug display cables?
>>>
>>> I am not sure if I understood your question correctly but 1 or more guest displays (GTK windows) are bound to a certain physical displays like HDMI or DP monitors. So plug/unplug means we disconnect those physical HDMI or DP cables manually. Or this manual hot plug in can be emulated by you write something to sysfs depending on what display driver you use.
>>
>> Let's see whether I understand.
>>
>> A VC is placed on a *physical* monitor, i.e. a window appears on that
>> monitor.  That monitor's plug / unplug state is passed through to the
>> guest, i.e. if I physically unplug / plug the monitor, the guest sees an
>> unplug / plug of its virtual monitor.  Correct?
>
> This is correct. When a display is disconnected, "monitor-changed" GTK event will be triggered then it will call gd_ui_size(0,0) which makes the guest display connection status to "disconnected".

Thanks!

>> Permit me another ignorant question...  Say I have a single monitor.  I
>> configured my X windows manager to show four virtual desktops.  Can I
>> use your feature to direct on which virtual desktop each VC is placed?
>
> Would those virtual desktops will be their own connector names like HDMI or DP? We use the connector name for the actual physical display you see when running xrandr.

Output of xrandr is identical on different virtual desktops for me.

> I don't know how virtual desktops are created and managed but if they don't have their own connector names that GTK API can read, it won't work within our current implementation.

After searching around a bit...  Virtual desktops, a.k.a. workspaces,
are a window manager thing.  Completely different from X displays and
monitors.  Programs can mess with placement (wmctrl does).  No idea
whether GDK provides an interface for it.  No need to discuss this
further at this time.

[...]

>>>> Old question not yet answered: Using a list for the mapping means the
>>>> mapping must be dense, e.g. I can't map #0 and #2 but not #1.  Is this
>>>> what we want?
>>>
>>> No, it doesn't have to be dense. In your example, you can just leave the place for VC1 blank. For example, you could do connectors.0=DP-1,connectors.2=HDMI-1. But in this case, VC1 won't be activated and stay as disconnected from guest's perspective. I think this info is also needed in v2.
>>
>> Have you tried this?  I believe it'll fail with something like
>> "Parameter 'connectors.1' missing".
>
> Just tested it. Yeah you are correct. I think I had a bad assumption. Let me take a look to see if I can make it work as I assumed.

If sparse mappings make sense, we should provide for them, I think.

An array like '*connectors': ['str'] maps from integers 0, 1, ...  It
can't do sparse (you can't omit integers in the middle).

Instead of omitting them, we could map them to null: '*connectors':
['StrOrNull'].  JSON input looks like [null, "HDMI-A-0"].  Since dotted
key syntax does not support null at this time, you'd have to use JSON.

Only an object can do sparse.  However, the QAPI schema language can't
express "object where the keys are integers and the values are strings".
We'd have to use 'any', and check everything manually.

Hmm.  Thoughts?

>>>> [...]

Re: [RFC PATCH 6/9] ui/gtk: Add a new parameter to assign connectors/monitors to GFX VCs

[Kim, Dongwon]

On 7/10/2023 11:36 PM, Markus Armbruster wrote:
> "Kim, Dongwon" <dongwon.kim@intel.com> writes:
>
>> On 7/9/2023 11:05 PM, Markus Armbruster wrote:
>>> "Kim, Dongwon" <dongwon.kim@intel.com> writes:
>>>
>>>> On 7/7/2023 7:07 AM, Markus Armbruster wrote:
> [...]
>
>>>>> Ignorant questions:
>>>>>
>>>>> 1. How would I plug / unplug display cables?
>>>> I am not sure if I understood your question correctly but 1 or more guest displays (GTK windows) are bound to a certain physical displays like HDMI or DP monitors. So plug/unplug means we disconnect those physical HDMI or DP cables manually. Or this manual hot plug in can be emulated by you write something to sysfs depending on what display driver you use.
>>> Let's see whether I understand.
>>>
>>> A VC is placed on a *physical* monitor, i.e. a window appears on that
>>> monitor.  That monitor's plug / unplug state is passed through to the
>>> guest, i.e. if I physically unplug / plug the monitor, the guest sees an
>>> unplug / plug of its virtual monitor.  Correct?
>> This is correct. When a display is disconnected, "monitor-changed" GTK event will be triggered then it will call gd_ui_size(0,0) which makes the guest display connection status to "disconnected".
> Thanks!
>
>>> Permit me another ignorant question...  Say I have a single monitor.  I
>>> configured my X windows manager to show four virtual desktops.  Can I
>>> use your feature to direct on which virtual desktop each VC is placed?
>> Would those virtual desktops will be their own connector names like HDMI or DP? We use the connector name for the actual physical display you see when running xrandr.
> Output of xrandr is identical on different virtual desktops for me.
>
>> I don't know how virtual desktops are created and managed but if they don't have their own connector names that GTK API can read, it won't work within our current implementation.
> After searching around a bit...  Virtual desktops, a.k.a. workspaces,
> are a window manager thing.  Completely different from X displays and
> monitors.  Programs can mess with placement (wmctrl does).  No idea
> whether GDK provides an interface for it.  No need to discuss this
> further at this time.
>
> [...]
>
>>>>> Old question not yet answered: Using a list for the mapping means the
>>>>> mapping must be dense, e.g. I can't map #0 and #2 but not #1.  Is this
>>>>> what we want?
>>>> No, it doesn't have to be dense. In your example, you can just leave the place for VC1 blank. For example, you could do connectors.0=DP-1,connectors.2=HDMI-1. But in this case, VC1 won't be activated and stay as disconnected from guest's perspective. I think this info is also needed in v2.
>>> Have you tried this?  I believe it'll fail with something like
>>> "Parameter 'connectors.1' missing".
>> Just tested it. Yeah you are correct. I think I had a bad assumption. Let me take a look to see if I can make it work as I assumed.
> If sparse mappings make sense, we should provide for them, I think.
>
> An array like '*connectors': ['str'] maps from integers 0, 1, ...  It
> can't do sparse (you can't omit integers in the middle).

Yeah, I understand this now. Despite of my initial intention was 
different, I am wondering if we don't allow the sparse mapping in this 
implementation. Any thought on that? The V2 patch is with that change in 
the description. Another thing I think is to change QAPI design little 
bit to make it fill the element with null (0) if it's not given. Would 
this be a feasible option?

>
> Instead of omitting them, we could map them to null: '*connectors':
> ['StrOrNull'].  JSON input looks like [null, "HDMI-A-0"].  Since dotted
> key syntax does not support null at this time, you'd have to use JSON.
>
> Only an object can do sparse.  However, the QAPI schema language can't
> express "object where the keys are integers and the values are strings".
> We'd have to use 'any', and check everything manually.
>
> Hmm.  Thoughts?
>
>>>>> [...]

Re: [RFC PATCH 6/9] ui/gtk: Add a new parameter to assign connectors/monitors to GFX VCs

[Markus Armbruster]

"Kim, Dongwon" <dongwon.kim@intel.com> writes:

> On 7/10/2023 11:36 PM, Markus Armbruster wrote:
>> "Kim, Dongwon" <dongwon.kim@intel.com> writes:
>>
>>> On 7/9/2023 11:05 PM, Markus Armbruster wrote:
>>>> "Kim, Dongwon" <dongwon.kim@intel.com> writes:
>>>>
>>>>> On 7/7/2023 7:07 AM, Markus Armbruster wrote:

[...]

>>>>>> Old question not yet answered: Using a list for the mapping means the
>>>>>> mapping must be dense, e.g. I can't map #0 and #2 but not #1.  Is this
>>>>>> what we want?
>>>>>
>>>>> No, it doesn't have to be dense. In your example, you can just leave the place for VC1 blank. For example, you could do connectors.0=DP-1,connectors.2=HDMI-1. But in this case, VC1 won't be activated and stay as disconnected from guest's perspective. I think this info is also needed in v2.
>>>>
>>>> Have you tried this?  I believe it'll fail with something like
>>>> "Parameter 'connectors.1' missing".
>>>
>>> Just tested it. Yeah you are correct. I think I had a bad assumption. Let me take a look to see if I can make it work as I assumed.
>>
>> If sparse mappings make sense, we should provide for them, I think.
>>
>> An array like '*connectors': ['str'] maps from integers 0, 1, ...  It
>> can't do sparse (you can't omit integers in the middle).
>
> Yeah, I understand this now. Despite of my initial intention was different, I am wondering if we don't allow the sparse mapping in this implementation. Any thought on that?

Repeating myself: if sparse mappings make sense, we should provide for
them, I think.

So, do they make sense?  Or asked differently, could a user conceivably
want to *not* place a VC?

> The V2 patch is with that change in the description. Another thing I think is to change QAPI design little bit to make it fill the element with null (0) if it's not given. Would this be a feasible option?

A 'str' cannot be NULL.  In fact, no QAPI type can be null, except for
'null' (which is always NULL), alternates with a 'null' branch, and
pointer-valued optionals (which are null when absent).

>> Instead of omitting them, we could map them to null: '*connectors':
>> ['StrOrNull'].  JSON input looks like [null, "HDMI-A-0"].  Since dotted
>> key syntax does not support null at this time, you'd have to use JSON.
>>
>> Only an object can do sparse.  However, the QAPI schema language can't
>> express "object where the keys are integers and the values are strings".
>> We'd have to use 'any', and check everything manually.
>>
>> Hmm.  Thoughts?
>>
>>>>>> [...]

Re: [RFC PATCH 6/9] ui/gtk: Add a new parameter to assign connectors/monitors to GFX VCs

[Kim, Dongwon]

On 7/11/2023 10:52 PM, Markus Armbruster wrote:
> "Kim, Dongwon" <dongwon.kim@intel.com> writes:
>
>> On 7/10/2023 11:36 PM, Markus Armbruster wrote:
>>> "Kim, Dongwon" <dongwon.kim@intel.com> writes:
>>>
>>>> On 7/9/2023 11:05 PM, Markus Armbruster wrote:
>>>>> "Kim, Dongwon" <dongwon.kim@intel.com> writes:
>>>>>
>>>>>> On 7/7/2023 7:07 AM, Markus Armbruster wrote:
> [...]
>
>>>>>>> Old question not yet answered: Using a list for the mapping means the
>>>>>>> mapping must be dense, e.g. I can't map #0 and #2 but not #1.  Is this
>>>>>>> what we want?
>>>>>> No, it doesn't have to be dense. In your example, you can just leave the place for VC1 blank. For example, you could do connectors.0=DP-1,connectors.2=HDMI-1. But in this case, VC1 won't be activated and stay as disconnected from guest's perspective. I think this info is also needed in v2.
>>>>> Have you tried this?  I believe it'll fail with something like
>>>>> "Parameter 'connectors.1' missing".
>>>> Just tested it. Yeah you are correct. I think I had a bad assumption. Let me take a look to see if I can make it work as I assumed.
>>> If sparse mappings make sense, we should provide for them, I think.
>>>
>>> An array like '*connectors': ['str'] maps from integers 0, 1, ...  It
>>> can't do sparse (you can't omit integers in the middle).
>> Yeah, I understand this now. Despite of my initial intention was different, I am wondering if we don't allow the sparse mapping in this implementation. Any thought on that?
> Repeating myself: if sparse mappings make sense, we should provide for
> them, I think.
> So, do they make sense?  Or asked differently, could a user conceivably
> want to *not* place a VC?

It should be very rare. I can't think of any valid use case other than 
test cases for validating this feature. If VC is not mapped to anything 
from the beginning, there is no way to get it displayed. So there is no 
value to do so. So I assume provisioning a full list as a requirement 
would make sense here.

>> The V2 patch is with that change in the description. Another thing I think is to change QAPI design little bit to make it fill the element with null (0) if it's not given. Would this be a feasible option?
> A 'str' cannot be NULL.  In fact, no QAPI type can be null, except for
> 'null' (which is always NULL), alternates with a 'null' branch, and
> pointer-valued optionals (which are null when absent).
>
>>> Instead of omitting them, we could map them to null: '*connectors':
>>> ['StrOrNull'].  JSON input looks like [null, "HDMI-A-0"].  Since dotted
>>> key syntax does not support null at this time, you'd have to use JSON.
>>>
>>> Only an object can do sparse.  However, the QAPI schema language can't
>>> express "object where the keys are integers and the values are strings".
>>> We'd have to use 'any', and check everything manually.
>>>
>>> Hmm.  Thoughts?
>>>
>>>>>>> [...]

[RFC PATCH v2 6/9] ui/gtk: Add a new parameter to assign connectors/monitors to GFX VCs

[Dongwon Kim]

From: Vivek Kasireddy <vivek.kasireddy@intel.com>

The new parameter named "connector" can be used to assign physical
monitors/connectors to individual GFX VCs such that when the monitor
is connected or hotplugged, the associated GTK window would be
moved to it. If the monitor is disconnected or unplugged, the
associated GTK window would be hidden and a relevant disconnect
event would be sent to the Guest.

Usage: -device virtio-gpu-pci,max_outputs=2,blob=true,...
       -display gtk,gl=on,connectors.0=eDP-1,connectors.1=DP-1.....

v2: updated the description for 'connectors' param with more details

Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Daniel P. Berrangé <berrange@redhat.com>
Cc: Markus Armbruster <armbru@redhat.com>
Cc: Philippe Mathieu-Daudé <philmd@linaro.org>
Cc: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
Signed-off-by: Dongwon Kim <dongwon.kim@intel.com>
---
 include/ui/gtk.h |   1 +
 qapi/ui.json     |  26 ++++-
 qemu-options.hx  |   5 +-
 ui/gtk.c         | 271 +++++++++++++++++++++++++++++++++++++++++++----
 4 files changed, 278 insertions(+), 25 deletions(-)

diff --git a/include/ui/gtk.h b/include/ui/gtk.h
index 2de38e5724..846d27f13d 100644
--- a/include/ui/gtk.h
+++ b/include/ui/gtk.h
@@ -84,6 +84,7 @@ typedef struct VirtualConsole {
     GtkWidget *menu_item;
     GtkWidget *tab_item;
     GtkWidget *focus;
+    GdkMonitor *monitor;
     VirtualConsoleType type;
     union {
         VirtualGfxConsole gfx;
diff --git a/qapi/ui.json b/qapi/ui.json
index bb06fb6039..6a91c909a2 100644
--- a/qapi/ui.json
+++ b/qapi/ui.json
@@ -1315,13 +1315,37 @@
 # @show-menubar: Display the main window menubar.  Defaults to "on".
 #     (Since 8.0)
 #
+# @connectors: List of physical monitor/connector names where the
+#     GTK windows containing the respective graphics virtual consoles
+#     (VCs) are to be placed. Index of the connector name in the list
+#     directly indicates the id of the VC. For example, with "-device
+#     gtk,connectors.0=DP-1, connectors.1=HDMI-2", a physical display
+#     connected to DP-1 will be the target monitor for VC0 and the one
+#     on HDMI-2 will be the target for VC1. If there is no valid
+#     connector name for a VC, it won't be displayed anywhere in any
+#     situations and the associatedvirtual display will be shown
+#     as disconnected in the guest. If a valid connector name exists for
+#     a VC but its display cable is not plugged in when guest is launched,
+#     the VC won't be displayed at first but will show up on the display
+#     as soon as the cable is plugged in. If the cable is disconnected
+#     again, the VC will immediately be hidden and guest will see its
+#     virtual display disconnected. When this list param is used, there
+#     shouldn't be any missing elements before the last one in the list.
+#     For example, if connectors.3 is specified, then connector names for
+#     connectors.0 through connectors.2 should also be given. Multiple
+#     VCs can have a same connector name. In this case, all of those
+#     VCs will be displayed on the same physical monitor. But multiple
+#     different connector names can't be assigned to one VC.
+#     (Since 8.1)
+#
 # Since: 2.12
 ##
 { 'struct'  : 'DisplayGTK',
   'data'    : { '*grab-on-hover' : 'bool',
                 '*zoom-to-fit'   : 'bool',
                 '*show-tabs'     : 'bool',
-                '*show-menubar'  : 'bool'  } }
+                '*show-menubar'  : 'bool',
+                '*connectors'    : ['str'] } }
 
 ##
 # @DisplayEGLHeadless:
diff --git a/qemu-options.hx b/qemu-options.hx
index f8f384e551..52a2c33836 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -2048,7 +2048,7 @@ DEF("display", HAS_ARG, QEMU_OPTION_display,
 #if defined(CONFIG_GTK)
     "-display gtk[,full-screen=on|off][,gl=on|off][,grab-on-hover=on|off]\n"
     "            [,show-tabs=on|off][,show-cursor=on|off][,window-close=on|off]\n"
-    "            [,show-menubar=on|off]\n"
+    "            [,show-menubar=on|off][,connectors.<index>=<connector name>]\n"
 #endif
 #if defined(CONFIG_VNC)
     "-display vnc=<display>[,<optargs>]\n"
@@ -2146,6 +2146,9 @@ SRST
         ``zoom-to-fit=on|off`` : Expand video output to the window size,
                                  defaults to "off"
 
+        ``connectors=<conn name>`` : VC to connector mappings to display the VC
+                                     window on a specific monitor
+
     ``curses[,charset=<encoding>]``
         Display video output via curses. For graphics device models
         which support a text mode, QEMU can display this output using a
diff --git a/ui/gtk.c b/ui/gtk.c
index f7f8a8e8bd..d30b7758c0 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -38,6 +38,7 @@
 #include "qemu/cutils.h"
 #include "qemu/error-report.h"
 #include "qemu/main-loop.h"
+#include "qemu/option.h"
 
 #include "ui/console.h"
 #include "ui/gtk.h"
@@ -741,6 +742,39 @@ static void gd_set_ui_size(VirtualConsole *vc, gint width, gint height)
     dpy_set_ui_info(vc->gfx.dcl.con, &info, true);
 }
 
+static void gd_ui_hide(VirtualConsole *vc)
+{
+    QemuUIInfo info;
+
+    vc->gfx.visible = false;
+    info = *dpy_get_ui_info(vc->gfx.dcl.con);
+    info.width = 0;
+    info.height = 0;
+    dpy_set_ui_info(vc->gfx.dcl.con, &info, false);
+}
+
+static void gd_ui_show(VirtualConsole *vc)
+{
+    QemuUIInfo info;
+    GtkDisplayState *s = vc->s;
+    GdkWindow *window = gtk_widget_get_window(vc->gfx.drawing_area);
+
+    info = *dpy_get_ui_info(vc->gfx.dcl.con);
+    info.width = gdk_window_get_width(window);
+    info.height = gdk_window_get_height(window);
+    dpy_set_ui_info(vc->gfx.dcl.con, &info, false);
+
+    if (gd_is_grab_active(s)) {
+        gd_grab_keyboard(vc, "user-request-main-window");
+        gd_grab_pointer(vc, "user-request-main-window");
+    } else {
+        gd_ungrab_keyboard(s);
+        gd_ungrab_pointer(s);
+    }
+
+    vc->gfx.visible = true;
+}
+
 #if defined(CONFIG_OPENGL)
 
 static gboolean gd_render_event(GtkGLArea *area, GdkGLContext *context,
@@ -1306,12 +1340,10 @@ 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);
+    gd_ui_hide(vc);
 
     vc = gd_vc_find_by_menu(s);
     gtk_release_modifiers(s);
@@ -1319,10 +1351,7 @@ 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;
+        gd_ui_show(vc);
     }
 }
 
@@ -1352,8 +1381,8 @@ static gboolean gd_tab_window_close(GtkWidget *widget, GdkEvent *event,
     VirtualConsole *vc = opaque;
     GtkDisplayState *s = vc->s;
 
-    vc->gfx.visible = false;
-    gd_set_ui_size(vc, 0, 0);
+    gd_ui_hide(vc);
+
     dpy_gl_scanout_disable(vc->gfx.dcl.con);
     gtk_widget_set_sensitive(vc->menu_item, true);
     gd_widget_reparent(vc->window, s->notebook, vc->tab_item);
@@ -1384,20 +1413,14 @@ static gboolean gd_window_state_event(GtkWidget *widget, GdkEvent *event,
     }
 
     if (event->window_state.new_window_state & GDK_WINDOW_STATE_ICONIFIED) {
-        vc->gfx.visible = false;
-        gd_set_ui_size(vc, 0, 0);
+        gd_ui_hide(vc);
         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);
         }
     } else {
-        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;
+        gd_ui_show(vc);
     }
 
     return TRUE;
@@ -1457,7 +1480,6 @@ static void gd_tab_window_create(VirtualConsole *vc)
 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 &&
@@ -1469,10 +1491,205 @@ static void gd_menu_untabify(GtkMenuItem *item, void *opaque)
         gd_tab_window_create(vc);
     }
 
-    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;
+    gd_ui_show(vc);
+}
+
+static void gd_window_show_on_monitor(GdkDisplay *dpy, VirtualConsole *vc,
+                                      gint monitor_num)
+{
+    GtkDisplayState *s = vc->s;
+    GdkMonitor *monitor = gdk_display_get_monitor(dpy, monitor_num);
+    GdkRectangle geometry;
+
+    if (!vc->window) {
+        gd_tab_window_create(vc);
+    }
+
+    gdk_window_show(gtk_widget_get_window(vc->window));
+    gd_update_windowsize(vc);
+    gdk_monitor_get_geometry(monitor, &geometry);
+    /*
+     * Note: some compositors (mainly Wayland ones) may not honor a
+     * request to move to a particular location. The user is expected
+     * to drag the window to the preferred location in this case.
+     */
+    gtk_window_move(GTK_WINDOW(vc->window), geometry.x, geometry.y);
+
+    if (s->opts->has_full_screen && s->opts->full_screen) {
+        gtk_widget_set_size_request(vc->gfx.drawing_area, -1, -1);
+        gtk_window_fullscreen(GTK_WINDOW(vc->window));
+    }
+
+    vc->monitor = monitor;
+    gd_ui_show(vc);
+
+    if (vc->window) {
+        g_signal_connect(vc->window, "window-state-event",
+                         G_CALLBACK(gd_window_state_event), vc);
+    }
+
+    gd_update_cursor(vc);
+}
+
+static int gd_monitor_lookup(GdkDisplay *dpy, char *label)
+{
+    GdkMonitor *monitor;
+    int total_monitors = gdk_display_get_n_monitors(dpy);
+    int i;
+
+    for (i = 0; i < total_monitors; i++) {
+        monitor = gdk_display_get_monitor(dpy, i);
+        if (monitor && !g_strcmp0(gdk_monitor_get_model(monitor), label)) {
+            return i;
+        }
+    }
+    return -1;
+}
+
+static gboolean gd_vc_is_misplaced(GdkDisplay *dpy, GdkMonitor *monitor,
+                                   VirtualConsole *vc)
+{
+    GdkWindow *window = gtk_widget_get_window(vc->gfx.drawing_area);
+    GdkMonitor *mon = gdk_display_get_monitor_at_window(dpy, window);
+    const char *monitor_name = gdk_monitor_get_model(monitor);
+
+    if (!vc->monitor) {
+        if (!g_strcmp0(monitor_name, vc->label)) {
+            return TRUE;
+        }
+    } else {
+        if (mon && mon != vc->monitor) {
+            return TRUE;
+        }
+    }
+    return FALSE;
+}
+
+static void gd_vc_add_remove_monitor(GdkDisplay *dpy, GtkDisplayState *s)
+{
+    VirtualConsole *vc;
+    GdkMonitor *monitor;
+    gint monitor_num;
+    int i;
+
+    /*
+     * We need to call gd_vc_is_misplaced() after a monitor is added to
+     * ensure that the Host compositor has not moved our windows around.
+     */
+    for (i = 0; i < s->nb_vcs; i++) {
+        vc = &s->vc[i];
+        if (vc->label) {
+            monitor_num = gd_monitor_lookup(dpy, vc->label);
+            if (monitor_num >= 0) {
+                monitor = gdk_display_get_monitor(dpy, monitor_num);
+                if (gd_vc_is_misplaced(dpy, monitor, vc)) {
+                    gd_window_show_on_monitor(dpy, vc, monitor_num);
+                }
+            } else if (vc->monitor) {
+                vc->monitor = NULL;
+                if (vc->window) {
+                    g_signal_handlers_disconnect_by_func(vc->window,
+                                                 G_CALLBACK(gd_window_state_event),
+                                                 vc);
+                }
+
+                gd_ui_hide(vc);
+                /* if window exist, hide it */
+                if (vc->window) {
+                    gdk_window_hide(gtk_widget_get_window(vc->window));
+                }
+            }
+        }
+    }
+}
+
+static void gd_monitors_reset_timer(void *opaque)
+{
+    GtkDisplayState *s = opaque;
+    GdkDisplay *dpy = gdk_display_get_default();
+
+    gd_vc_add_remove_monitor(dpy, s);
+}
+
+static void gd_monitors_changed(GdkScreen *scr, void *opaque)
+{
+    GtkDisplayState *s = opaque;
+    QEMUTimer *mon_reset_timer;
+
+    mon_reset_timer = timer_new_ms(QEMU_CLOCK_REALTIME,
+                                   gd_monitors_reset_timer, s);
+    timer_mod(mon_reset_timer,
+              qemu_clock_get_ms(QEMU_CLOCK_REALTIME) + 2000);
+}
+
+static VirtualConsole *gd_next_gfx_vc(GtkDisplayState *s)
+{
+    VirtualConsole *vc;
+    int i;
+
+    for (i = 0; i < s->nb_vcs; i++) {
+        vc = &s->vc[i];
+        if (vc->type == GD_VC_GFX &&
+            qemu_console_is_graphic(vc->gfx.dcl.con) &&
+            !vc->label) {
+            return vc;
+        }
+    }
+    return NULL;
+}
+
+static void gd_vc_free_labels(GtkDisplayState *s)
+{
+    VirtualConsole *vc;
+    int i;
+
+    for (i = 0; i < s->nb_vcs; i++) {
+        vc = &s->vc[i];
+        if (vc->type == GD_VC_GFX &&
+            qemu_console_is_graphic(vc->gfx.dcl.con)) {
+            g_free(vc->label);
+            vc->label = NULL;
+        }
+    }
+}
+
+static void gd_connectors_init(GdkDisplay *dpy, GtkDisplayState *s)
+{
+    VirtualConsole *vc;
+    strList *conn;
+    gint monitor_num;
+    gboolean first_vc = TRUE;
+
+    gtk_notebook_set_show_tabs(GTK_NOTEBOOK(s->notebook), FALSE);
+    gtk_check_menu_item_set_active(GTK_CHECK_MENU_ITEM(s->grab_item),
+                                   FALSE);
+    gd_vc_free_labels(s);
+    for (conn = s->opts->u.gtk.connectors; conn; conn = conn->next) {
+        vc = gd_next_gfx_vc(s);
+        if (!vc) {
+            break;
+        }
+        if (first_vc) {
+            vc->window = s->window;
+            first_vc = FALSE;
+        }
+
+        vc->label = g_strdup(conn->value);
+        monitor_num = gd_monitor_lookup(dpy, vc->label);
+        if (monitor_num >= 0) {
+            gd_window_show_on_monitor(dpy, vc, monitor_num);
+        } else {
+            if (vc->window) {
+                 g_signal_handlers_disconnect_by_func(vc->window,
+                                             G_CALLBACK(gd_window_state_event),
+                                             vc);
+            }
+            gd_ui_hide(vc);
+            if (vc->window) {
+                gdk_window_hide(gtk_widget_get_window(vc->window));
+            }
+        }
+    }
 }
 
 static void gd_menu_show_menubar(GtkMenuItem *item, void *opaque)
@@ -1797,7 +2014,7 @@ static gboolean gd_configure(GtkWidget *widget,
     VirtualConsole *vc = opaque;
 
     if (vc->gfx.visible) {
-        gd_set_ui_size(vc, cfg->width, cfg->height);
+        gd_ui_show(vc);
     }
     return FALSE;
 }
@@ -2134,6 +2351,10 @@ static void gd_connect_signals(GtkDisplayState *s)
                      G_CALLBACK(gd_menu_grab_input), s);
     g_signal_connect(s->notebook, "switch-page",
                      G_CALLBACK(gd_change_page), s);
+    if (s->opts->u.gtk.has_connectors) {
+        g_signal_connect(gdk_screen_get_default(), "monitors-changed",
+                         G_CALLBACK(gd_monitors_changed), s);
+    }
 }
 
 static GtkWidget *gd_create_menu_machine(GtkDisplayState *s)
@@ -2515,6 +2736,10 @@ static void gtk_display_init(DisplayState *ds, DisplayOptions *opts)
         opts->u.gtk.show_tabs) {
         gtk_menu_item_activate(GTK_MENU_ITEM(s->show_tabs_item));
     }
+
+    if (s->opts->u.gtk.has_connectors) {
+        gd_connectors_init(window_display, s);
+    }
 #ifdef CONFIG_GTK_CLIPBOARD
     gd_clipboard_init(s);
 #endif /* CONFIG_GTK_CLIPBOARD */
-- 
2.20.1

[RFC PATCH 7/9] ui/gtk: unblock gl if draw submitted already or fence is not yet signaled

[Dongwon Kim]

Remove monitor while a guest frame is still being processed could block
the guest (virtio-gpu) scanout pipe line. It is needed to manually flush
the pipeline to prevent the permanent lockup.

Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Daniel P. Berrangé <berrange@redhat.com>
Cc: Markus Armbruster <armbru@redhat.com>
Cc: Philippe Mathieu-Daudé <philmd@linaro.org>
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 | 29 ++++++++++++++++++++---------
 1 file changed, 20 insertions(+), 9 deletions(-)

diff --git a/ui/gtk.c b/ui/gtk.c
index f4c71454a3..e4ef1f7173 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -598,10 +598,21 @@ void gd_hw_gl_flushed(void *vcon)
     VirtualConsole *vc = vcon;
     QemuDmaBuf *dmabuf = vc->gfx.guest_fb.dmabuf;
 
-    qemu_set_fd_handler(dmabuf->fence_fd, NULL, NULL, NULL);
-    close(dmabuf->fence_fd);
-    dmabuf->fence_fd = -1;
-    graphic_hw_gl_block(vc->gfx.dcl.con, false);
+    if (!dmabuf) {
+        return;
+    }
+
+    if (dmabuf->fence_fd > 0) {
+        qemu_set_fd_handler(dmabuf->fence_fd, NULL, NULL, NULL);
+        close(dmabuf->fence_fd);
+        dmabuf->fence_fd = -1;
+        graphic_hw_gl_block(vc->gfx.dcl.con, false);
+    } else if (dmabuf->draw_submitted) {
+        /* if called after a frame is submitted but render event
+         * is not scheduled yet, cancel submitted draw. */
+        dmabuf->draw_submitted = false;
+        graphic_hw_gl_block(vc->gfx.dcl.con, false);
+    }
 }
 
 /** DisplayState Callbacks (opengl version) **/
@@ -742,6 +753,9 @@ static void gd_set_ui_size(VirtualConsole *vc, gint width, gint height)
     dpy_set_ui_info(vc->gfx.dcl.con, &info, true);
 }
 
+static gboolean gd_window_state_event(GtkWidget *widget, GdkEvent *event,
+                                      void *opaque);
+
 static void gd_ui_hide(VirtualConsole *vc)
 {
     QemuUIInfo info;
@@ -751,6 +765,8 @@ static void gd_ui_hide(VirtualConsole *vc)
     info.width = 0;
     info.height = 0;
     dpy_set_ui_info(vc->gfx.dcl.con, &info, false);
+    /* forcefully cancel rendering sequence */
+    gd_hw_gl_flushed(vc);
 }
 
 static void gd_ui_show(VirtualConsole *vc)
@@ -1460,11 +1476,6 @@ static gboolean gd_window_state_event(GtkWidget *widget, GdkEvent *event,
 
     if (event->window_state.new_window_state & GDK_WINDOW_STATE_ICONIFIED) {
         gd_ui_hide(vc);
-        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);
-        }
     } else {
         gd_ui_show(vc);
     }
-- 
2.34.1

[RFC PATCH 8/9] ui/gtk: skip drawing if any of ctx/surface/image don't exist

[Dongwon Kim]

Rendering of scanout could be skipped if ctx/surface/image don't
exist due to an asynchronous event such as monitors being disconnected.

Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Daniel P. Berrangé <berrange@redhat.com>
Cc: Markus Armbruster <armbru@redhat.com>
Cc: Philippe Mathieu-Daudé <philmd@linaro.org>
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-egl.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/ui/gtk-egl.c b/ui/gtk-egl.c
index aa22ebbd98..8eae2b4b1f 100644
--- a/ui/gtk-egl.c
+++ b/ui/gtk-egl.c
@@ -106,6 +106,11 @@ void gd_egl_draw(VirtualConsole *vc)
         if (!vc->gfx.ds) {
             return;
         }
+
+        if (!vc->gfx.esurface || !vc->gfx.ectx || !vc->gfx.ds->image) {
+            return;
+        }
+
         eglMakeCurrent(qemu_egl_display, vc->gfx.esurface,
                        vc->gfx.esurface, vc->gfx.ectx);
 
-- 
2.34.1

[RFC PATCH 9/9] ui/gtk: skip refresh/rendering if VC is invisible

[Dongwon Kim]

Skip any drawing activities if VC is invisible because it can't be finished.

Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Daniel P. Berrangé <berrange@redhat.com>
Cc: Markus Armbruster <armbru@redhat.com>
Cc: Philippe Mathieu-Daudé <philmd@linaro.org>
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-egl.c     | 4 ++++
 ui/gtk-gl-area.c | 4 ++++
 ui/gtk.c         | 4 ++++
 3 files changed, 12 insertions(+)

diff --git a/ui/gtk-egl.c b/ui/gtk-egl.c
index 8eae2b4b1f..63bfad1f06 100644
--- a/ui/gtk-egl.c
+++ b/ui/gtk-egl.c
@@ -148,6 +148,10 @@ void gd_egl_refresh(DisplayChangeListener *dcl)
     gd_update_monitor_refresh_rate(
             vc, vc->window ? vc->window : vc->gfx.drawing_area);
 
+    if (!vc->gfx.visible) {
+        return;
+    }
+
     if (!vc->gfx.esurface) {
         gd_egl_init(vc);
         if (!vc->gfx.esurface) {
diff --git a/ui/gtk-gl-area.c b/ui/gtk-gl-area.c
index 8228cc9f3f..8d01addb3b 100644
--- a/ui/gtk-gl-area.c
+++ b/ui/gtk-gl-area.c
@@ -123,6 +123,10 @@ void gd_gl_area_refresh(DisplayChangeListener *dcl)
 
     gd_update_monitor_refresh_rate(vc, vc->window ? vc->window : vc->gfx.drawing_area);
 
+    if (!vc->gfx.visible) {
+        return;
+    }
+
     if (!vc->gfx.gls) {
         if (!gtk_widget_get_realized(vc->gfx.drawing_area)) {
             return;
diff --git a/ui/gtk.c b/ui/gtk.c
index e4ef1f7173..0bc35b64e0 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -847,6 +847,10 @@ static gboolean gd_draw_event(GtkWidget *widget, cairo_t *cr, void *opaque)
 
 #if defined(CONFIG_OPENGL)
     if (vc->gfx.gls) {
+        if (!vc->gfx.visible) {
+            return TRUE;
+        }
+
         if (gtk_use_gl_area) {
             /* invoke render callback please */
             return FALSE;
-- 
2.34.1