[virt-tools-list] [PATCH virt-viewer 3/3] Add new functions to enable/disable a display

Pavel Grunt pgrunt at redhat.com
Tue Aug 4 08:25:55 UTC 2015


Hi Jonathon,

On Mon, 2015-08-03 at 15:59 -0500, Jonathon Jongsma wrote:
> Previously, there was a single function for controlling the enabled
> state of a display: virt_viewer_display_set_enabled(). Unfortunately,
> this function is used for two slightly different things:
> 
>  A. It informs the local display widget that the display has become
>     disabled or enabled on the server. In other words, it tries to
>     synchronize the 'enabled' state of the local widget with the actual
>     state of the remote display.
> 
> OR
> 
>  B. It tries to actively enable a currently-disabled display (or vice
>     versa) due to some action by the user in the client application.
>     This causes the client to send a new configuration down to the
>     server. In other words, it tries to change the state of the remote
>     display.
> 
> There is some conflict between these two scenarios. If the change is due
> to a notification from the server, there is no need to send a new
> configuration back down to the server, so this results in unnecessary
> monitor configuration messages and can in fact cause issues that are a
> little bit hard to track down. Because of this, I decided that it was
> really necessary to have two separate functions for these two different
> scenarios. so the existing _set_enabled() function will be used for
> scenario A mentioned above. I added two new
> functiions (_enable() and _disable()) that are used to send new
typo   ^
> configurations down to the server.
> ---
>  configure.ac                    |  2 +-
>  src/virt-viewer-display-spice.c | 29 +++++++++++++++++++++++++----
>  src/virt-viewer-display.c       | 31 +++++++++++++++++++++++++++++++
>  src/virt-viewer-display.h       |  4 ++++
>  src/virt-viewer-window.c        |  6 +++---
>  5 files changed, 64 insertions(+), 8 deletions(-)
> 
> diff --git a/configure.ac b/configure.ac
> index d37863b..a7b7140 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -19,7 +19,7 @@ GTK2_REQUIRED="2.18.0"
>  GTK3_REQUIRED="3.0"
>  GTK_VNC1_REQUIRED="0.3.8"
>  GTK_VNC2_REQUIRED="0.4.0"
> -SPICE_GTK_REQUIRED="0.28"
> +SPICE_GTK_REQUIRED="0.29.17"
It should be "0.30"

So this commit depends on your spice-gtk series ?
http://lists.freedesktop.org/archives/spice-devel/2015-July/020873.html
or just on the patch 2/4 of that series
http://lists.freedesktop.org/archives/spice-devel/2015-July/020875.html

imho it would be better to split this patch otherwise the change
from spice_main_set_display_enabled() to spice_main_update_display_enabled()
will be "hidden".

Pavel
>  SPICE_PROTOCOL_REQUIRED="0.12.7"
>  GOVIRT_REQUIRED="0.3.2"
>  
> diff --git a/src/virt-viewer-display-spice.c b/src/virt-viewer-display-spice.c
> index f5e66a2..c3dbd75 100644
> --- a/src/virt-viewer-display-spice.c
> +++ b/src/virt-viewer-display-spice.c
> @@ -58,6 +58,8 @@ static GdkPixbuf 
> *virt_viewer_display_spice_get_pixbuf(VirtViewerDisplay *displa
>  static void virt_viewer_display_spice_release_cursor(VirtViewerDisplay 
> *display);
>  static void virt_viewer_display_spice_close(VirtViewerDisplay *display 
> G_GNUC_UNUSED);
>  static gboolean virt_viewer_display_spice_selectable(VirtViewerDisplay 
> *display);
> +static void virt_viewer_display_spice_enable(VirtViewerDisplay *display);
> +static void virt_viewer_display_spice_disable(VirtViewerDisplay *display);
>  
>  static void
>  virt_viewer_display_spice_class_init(VirtViewerDisplaySpiceClass *klass)
> @@ -69,6 +71,8 @@ 
> virt_viewer_display_spice_class_init(VirtViewerDisplaySpiceClass *klass)
>      dclass->release_cursor = virt_viewer_display_spice_release_cursor;
>      dclass->close = virt_viewer_display_spice_close;
>      dclass->selectable = virt_viewer_display_spice_selectable;
> +    dclass->enable = virt_viewer_display_spice_enable;
> +    dclass->disable = virt_viewer_display_spice_disable;
>  
>      g_type_class_add_private(klass, sizeof(VirtViewerDisplaySpicePrivate));
>  }
> @@ -89,11 +93,9 @@ 
> virt_viewer_display_spice_monitor_geometry_changed(VirtViewerDisplaySpice 
> *self)
>      g_signal_emit_by_name(self, "monitor-geometry-changed", NULL);
>  }
>  
> -static void
> -show_hint_changed(VirtViewerDisplay *self)
> +static void update_enabled(VirtViewerDisplay *self, gboolean enabled, 
> gboolean send)
>  {
>      SpiceMainChannel *main_channel = get_main(self);
> -    guint enabled = virt_viewer_display_get_enabled(self);
>      guint nth;
>  
>      /* this may happen when finalizing */
> @@ -101,7 +103,26 @@ show_hint_changed(VirtViewerDisplay *self)
>          return;
>  
>      g_object_get(self, "nth-display", &nth, NULL);
> -    spice_main_set_display_enabled(main_channel, nth, enabled);
> +    spice_main_update_display_enabled(main_channel, nth, enabled, send);
> +}
> +
> +static void
> +show_hint_changed(VirtViewerDisplay *self)
> +{
> +    /* just keep spice-gtk state up-to-date, but don't send change anything 
> */
> +    update_enabled(self, virt_viewer_display_get_enabled(self), FALSE);
> +}
> +
> +static void virt_viewer_display_spice_enable(VirtViewerDisplay *self)
> +{
> +    virt_viewer_display_set_enabled(self, TRUE);
> +    update_enabled(self, TRUE, TRUE);
> +}
> +
> +static void virt_viewer_display_spice_disable(VirtViewerDisplay *self)
> +{
> +    virt_viewer_display_set_enabled(self, FALSE);
> +    update_enabled(self, FALSE, TRUE);
>  }
>  
>  static void
> diff --git a/src/virt-viewer-display.c b/src/virt-viewer-display.c
> index 3efe24c..5a5a6c2 100644
> --- a/src/virt-viewer-display.c
> +++ b/src/virt-viewer-display.c
> @@ -682,6 +682,37 @@ void virt_viewer_display_set_show_hint(VirtViewerDisplay 
> *self, guint mask, gboo
>      g_object_notify(G_OBJECT(self), "show-hint");
>  }
>  
> +/* This function attempts to enable the display if supported by the backend 
> */
> +void virt_viewer_display_enable(VirtViewerDisplay *self)
> +{
> +    VirtViewerDisplayClass *klass;
> +
> +    g_return_if_fail(VIRT_VIEWER_IS_DISPLAY(self));
> +
> +    klass = VIRT_VIEWER_DISPLAY_GET_CLASS(self);
> +    if (!klass->enable)
> +        return;
> +
> +    klass->enable(self);
> +}
> +
> +/* This function attempts to disable the display if supported by the backend 
> */
> +void virt_viewer_display_disable(VirtViewerDisplay *self)
> +{
> +    VirtViewerDisplayClass *klass;
> +
> +    g_return_if_fail(VIRT_VIEWER_IS_DISPLAY(self));
> +
> +    klass = VIRT_VIEWER_DISPLAY_GET_CLASS(self);
> +    if (!klass->disable)
> +        return;
> +
> +    klass->disable(self);
> +}
> +
> +/* this function simply informs the display that it is enabled. see
> + * virt_viewer_display_enable()/disable() if you want to attempt to change 
> the
> + * state of the display */
>  void virt_viewer_display_set_enabled(VirtViewerDisplay *self, gboolean 
> enabled)
>  {
>      g_return_if_fail(VIRT_VIEWER_IS_DISPLAY(self));
> diff --git a/src/virt-viewer-display.h b/src/virt-viewer-display.h
> index 9829617..a899bb4 100644
> --- a/src/virt-viewer-display.h
> +++ b/src/virt-viewer-display.h
> @@ -90,6 +90,8 @@ struct _VirtViewerDisplayClass {
>      void (*display_keyboard_ungrab)(VirtViewerDisplay *display);
>  
>      void (*display_desktop_resize)(VirtViewerDisplay *display);
> +    void (*enable)(VirtViewerDisplay *display);
> +    void (*disable)(VirtViewerDisplay *display);
>  };
>  
>  GType virt_viewer_display_get_type(void);
> @@ -125,6 +127,8 @@ void virt_viewer_display_release_cursor(VirtViewerDisplay 
> *display);
>  
>  void virt_viewer_display_close(VirtViewerDisplay *display);
>  void virt_viewer_display_set_enabled(VirtViewerDisplay *display, gboolean 
> enabled);
> +void virt_viewer_display_enable(VirtViewerDisplay *display);
> +void virt_viewer_display_disable(VirtViewerDisplay *display);
>  gboolean virt_viewer_display_get_enabled(VirtViewerDisplay *display);
>  gboolean virt_viewer_display_get_selectable(VirtViewerDisplay *display);
>  void virt_viewer_display_queue_resize(VirtViewerDisplay *display);
> diff --git a/src/virt-viewer-window.c b/src/virt-viewer-window.c
> index e067162..a1b9940 100644
> --- a/src/virt-viewer-window.c
> +++ b/src/virt-viewer-window.c
> @@ -1353,8 +1353,8 @@ virt_viewer_window_enable_kiosk(VirtViewerWindow *self)
>  void
>  virt_viewer_window_show(VirtViewerWindow *self)
>  {
> -    if (self->priv->display)
> -        virt_viewer_display_set_enabled(self->priv->display, TRUE);
> +    if (self->priv->display && !virt_viewer_display_get_enabled(self->priv
> ->display))
> +        virt_viewer_display_enable(self->priv->display);
>  
>      if (self->priv->desktop_resize_pending) {
>          virt_viewer_window_queue_resize(self);
> @@ -1382,7 +1382,7 @@ virt_viewer_window_hide(VirtViewerWindow *self)
>  
>      if (self->priv->display) {
>          VirtViewerDisplay *display = self->priv->display;
> -        virt_viewer_display_set_enabled(display, FALSE);
> +        virt_viewer_display_disable(display);
>      }
>  }
>  




More information about the virt-tools-list mailing list