[virt-tools-list] [PATCH v2 12/16] app: create a window for VTE displays
Victor Toso
victortoso at redhat.com
Fri Dec 21 11:31:09 UTC 2018
Hi,
On Wed, Sep 26, 2018 at 07:26:35PM +0400, marcandre.lureau at redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau at redhat.com>
>
> virt_viewer_app_display_added() now handles VTE displays. They should
> be skipped for monitor configuration, and they don't emit "show-hint".
>
> (a VTE display has a monitor nth == -1, which is now a valid value)
>
> The associated window will be hidden when virt-viewer is started.
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau at redhat.com>
> ---
> src/virt-viewer-app.c | 69 ++++++++++++++++++++++++++++++++-----------
> 1 file changed, 52 insertions(+), 17 deletions(-)
>
> diff --git a/src/virt-viewer-app.c b/src/virt-viewer-app.c
> index 568af47..376fe98 100644
> --- a/src/virt-viewer-app.c
> +++ b/src/virt-viewer-app.c
> @@ -112,7 +112,7 @@ struct _VirtViewerAppPrivate {
> VirtViewerWindow *main_window;
> GtkWidget *main_notebook;
> GList *windows;
> - GHashTable *displays;
> + GHashTable *displays; /* !vte */
> GHashTable *initial_display_map;
> gchar *clipboard;
> GtkWidget *preferences;
> @@ -822,9 +822,11 @@ virt_viewer_app_window_new(VirtViewerApp *self, gint nth)
> VirtViewerWindow* window;
> GtkWindow *w;
>
> - window = virt_viewer_app_get_nth_window(self, nth);
> - if (window)
> - return window;
> + if (nth >= 0) {
> + window = virt_viewer_app_get_nth_window(self, nth);
> + if (window)
> + return window;
> + }
I think it is better to patch virt_viewer_app_get_nth_window() to
return NULL if nth < 0 ...
> window = g_object_new(VIRT_VIEWER_TYPE_WINDOW, "app", self, NULL);
> virt_viewer_window_set_kiosk(window, self->priv->kiosk);
> @@ -848,11 +850,27 @@ virt_viewer_app_window_new(VirtViewerApp *self, gint nth)
> return window;
> }
>
> +static void
> +window_weak_notify(gpointer data, GObject *where_was G_GNUC_UNUSED)
> +{
> + VirtViewerDisplay *display = VIRT_VIEWER_DISPLAY(data);
> +
> + g_object_set_data(G_OBJECT(display), "virt-viewer-window", NULL);
> +}
> +
> static VirtViewerWindow *
> ensure_window_for_display(VirtViewerApp *self, VirtViewerDisplay *display)
> {
> - gint nth = virt_viewer_display_get_nth(display);
> - VirtViewerWindow *win = virt_viewer_app_get_nth_window(self, nth);
> + VirtViewerWindow *win = NULL;
> + gint nth = -1;
... and nth should be -1 on VTE and win NULL here, so you just
need to add the if below.
> + if (VIRT_VIEWER_IS_DISPLAY_VTE(display)) {
> + win = g_object_get_data(G_OBJECT(display), "virt-viewer-window");
> + } else {
> + nth = virt_viewer_display_get_nth(display);
> + win = virt_viewer_app_get_nth_window(self, nth);
> + }
> +
> if (win == NULL) {
> GList *l = self->priv->windows;
>
> @@ -874,12 +892,26 @@ ensure_window_for_display(VirtViewerApp *self, VirtViewerDisplay *display)
> }
>
> virt_viewer_window_set_display(win, display);
> + if (VIRT_VIEWER_IS_DISPLAY_VTE(display)) {
> + g_object_set_data(G_OBJECT(display), "virt-viewer-window", win);
> + g_object_weak_ref(G_OBJECT(win), window_weak_notify, display);
> + }
> }
> virt_viewer_app_set_window_subtitle(self, win, nth);
>
> return win;
> }
>
> +static VirtViewerWindow *
> +display_show_get_window(VirtViewerApp *self, VirtViewerDisplay *display)
> +{
From the name, it seems like a simple getter but it does a bit
more than that. Perhaps a reference to notebook would improve
eg: display_show_notebook_get_window() but I'm not famous for my
name picking ability.
> + VirtViewerWindow *win = ensure_window_for_display(self, display);
> + VirtViewerNotebook *nb = virt_viewer_window_get_notebook(win);
> +
> + virt_viewer_notebook_show_display(nb);
> + return win;
> +}
> +
> static void
> display_show_hint(VirtViewerDisplay *display,
> GParamSpec *pspec G_GNUC_UNUSED,
> @@ -907,9 +939,7 @@ display_show_hint(VirtViewerDisplay *display,
> virt_viewer_window_hide(win);
> } else {
> if (hint & VIRT_VIEWER_DISPLAY_SHOW_HINT_READY) {
> - win = ensure_window_for_display(self, display);
> - nb = virt_viewer_window_get_notebook(win);
> - virt_viewer_notebook_show_display(nb);
> + win = display_show_get_window(self, display);
> virt_viewer_window_show(win);
> } else {
> if (!self->priv->kiosk && win) {
> @@ -926,19 +956,24 @@ virt_viewer_app_display_added(VirtViewerSession *session G_GNUC_UNUSED,
> VirtViewerDisplay *display,
> VirtViewerApp *self)
> {
> - gint nth;
> + if (VIRT_VIEWER_IS_DISPLAY_VTE(display)) {
> + VirtViewerWindow *win = display_show_get_window(self, display);
> + virt_viewer_window_hide(win);
> + virt_viewer_app_update_menu_displays(self);
> + } else {
> + gint nth;
>
> - g_object_get(display, "nth-display", &nth, NULL);
> + g_object_get(display, "nth-display", &nth, NULL);
>
> - g_debug("Insert display %d %p", nth, display);
I'd move the if above below this debug, so we have a 'insert
display -1 %p' log and add a return in the end of the if. Less
changes and more log but feel free to keep as is.
> - g_hash_table_insert(self->priv->displays, GINT_TO_POINTER(nth), g_object_ref(display));
> + g_debug("Insert display %d %p", nth, display);
> + g_hash_table_insert(self->priv->displays, GINT_TO_POINTER(nth), g_object_ref(display));
>
> - g_signal_connect(display, "notify::show-hint",
> - G_CALLBACK(display_show_hint), NULL);
> - g_object_notify(G_OBJECT(display), "show-hint"); /* call display_show_hint */
> + g_signal_connect(display, "notify::show-hint",
> + G_CALLBACK(display_show_hint), NULL);
> + g_object_notify(G_OBJECT(display), "show-hint"); /* call display_show_hint */
> + }
> }
>
> -
> static void virt_viewer_app_remove_nth_window(VirtViewerApp *self,
> gint nth)
> {
Just nitpicks,
Acked-by: Victor Toso <victortoso at redhat.com>
> --
> 2.19.0.271.gfe8321ec05
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/virt-tools-list/attachments/20181221/c3905e95/attachment.sig>
More information about the virt-tools-list
mailing list