[virt-tools-list] [PATCH] Update geometry when enabling/disabling displays

Jonathon Jongsma jjongsma at redhat.com
Wed Mar 11 17:10:56 UTC 2015


I discussed this with Fabiano on IRC a little bit, and my feeling is
that this is not the ideal place to handle this case. However, when
trying to write a patch that seemed more "correct" to me
(http://paste.fedoraproject.org/196405/1833114/), it introduced a
regression[1] that does not occur with Fabiano's patch. So I guess I'd
tentatively ACK Fabiano's patch since it appears to work without
regressions. The alternative is to spend the time investigating why my
patch fails, but I suspect that will be a lot of work, and the behavior
may be different on gtk2 vs gtk3, etc.  Anyone else have opinions on the
matter?

Jonathon

[1] When virt-viewer started up, the display windows would appear at the
correct size and then immediately shrink to a very small size. This is a
fairly common regression that keeps cropping up from time to time.
Perhaps some day we can re-factor this whole thing to make it less
susceptible to these sorts of regressions, but I fear that may be a lot
of work, and may not even be possible (especially while we have to
support both gtk2 and gtk3)...


On Wed, 2015-03-04 at 23:40 +0100, Fabiano Fidêncio wrote:
> _update_displays_geometry() must be called every time a display is
> enabled/disabled, avoiding gaps (when a display is disabled) or overlaps
> (when a display is enabled) between the monitors.
> 
> This is what happens when we have 3 displays enabled (each one
> represented by: width x height + x position + y position) ...
> 
>   Display #0       Display #1         Display #2
>  +---------+      +----------+       +---------+
>  |         |      |          |       |         |
>  |         |      |          |       |         |
>  |         |      |          |       |         |
>  |         |      |          |       |         |
>  |         |      |          |       |         |
>  |         |      |          |       |         |
>  +---------+      +----------+       +---------+
> (680x804+0+0)   (504x804+680+0)    (408x804+1184+0)
> 
> Whether the Display #1 is disable, a message will be sent down to the
> vdagent, representing the new arrangement of the monitors:
> 
>   Display #0     Display #2
>  +---------+     +---------+
>  |         |     |         |
>  |         |     |         |
>  |         |     |         |
>  |         |     |         |
>  |         |     |         |
>  |         |     |         |
>  +---------+     +---------+
> (680x804+0+0)  (408x804+1184+0)
> 
> However, taking a look on the x position, a gap can be identified as
> Display #0 starts at position (0,0) and has 680 pixels of width. But
> Display #1 only starts at position (1184, 0), leaving 504 pixels as a
> gap. The proper message, however, should represent the following
> arrangement ...
> 
>   Display #0       Display #2
>  +---------+      +---------+
>  |         |      |         |
>  |         |      |         |
>  |         |      |         |
>  |         |      |         |
>  |         |      |         |
>  |         |      |         |
>  +---------+      +---------+
> (680x804+0+0)   (408x804+680+0)
> 
> ... avoiding then gaps and overlaps.
> ---
> The patch has been tested with gtk2 and gtk3 clients and with
> rhel6 (ums driver) and rhel7 (kms driver) as guests.
> ---
>  src/virt-viewer-app.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/src/virt-viewer-app.c b/src/virt-viewer-app.c
> index 4800beb..2315167 100644
> --- a/src/virt-viewer-app.c
> +++ b/src/virt-viewer-app.c
> @@ -2175,6 +2175,8 @@ menu_display_visible_toggled_cb(GtkCheckMenuItem *checkmenuitem,
>  
>      gtk_check_menu_item_set_active(checkmenuitem, /* will be toggled again */ !visible);
>      reentering = FALSE;
> +
> +    virt_viewer_session_update_displays_geometry(virt_viewer_display_get_session(display));
>  }
>  
>  static gint





More information about the virt-tools-list mailing list