Re: gvncviewer improvement - input grab problems

["Daniel P. Berrange" <dan berrange com>]

On Sat, Aug 08, 2009 at 04:02:11PM +0200, Michal Suchanek wrote:
> Debian package installs the gvncviewer example as an application so I
> was trying to use it as a vnc viewer instead of the xvncviewer and
> found that it has some advantages and problems.
> 
> The major advantage over xvncviewer version in Debian is support for
> resizing the remote desktop.
> 
> The drawback which is easily resolved is that with gvncviewer one
> cannot specify local vnc server by display number only.
> 
> There is also another issue that the vnc viewer reports "disconnected
> from server" when it in fact did not manage to connect at all.
> 
> The problem I did not manage to resolve is that the viewer grabs the
> input on some occasions and I did not manage to turn off this feature
> completely.
> 
> The grab comes in two flavours - one is grabbing the keyboard while
> the mouse is over the remote display area which prevents using window
> manager shortcuts.
> 
> The other is grabbing both keyboard and pointer when Ctrl+Alt is pressed.

There are 2 modes for VNC servers, relative and absolute pointer motion.
Traditional desktop VNC servers always operate in absolute mode, and
for them GTK-VNC client will never grab the pointer, even with Ctrl+alt.

Virtual machines like QEMU/KVM can provide a relative mouse motion
extension. They start in absolute mode, and GTK-VNC negotiates for
relative mode. Once switched into relative mode, GTK-VNC *must*
grab the pointer at all times in order for it to work correctly.

So vnc_display_set_pointer_grab(FALSE) should really never be done.
It has no effect on servers only supporting absolute mode, and will
definitely break interaction for servers with relative mode active.

With keyboard grab. If the pointer is already grabbed, then it'll
always grab keyboard too.  If the pointer is not grabbed, then it
will grab the keyboard upon mouse enter. It makes sense to allow
this to be configurable via the menu.

> I noticed that there is keyboard grab and input grab set to TRUE in
> the initialization of the viewer widget so I tried to add a menu item
> which toggles these settings.
> 
> This does not give the expected results, however.
> 
> When grab is off I can use WM shortcuts while the pointer is over the
> remote desktop but whenever I press Ctrl+Alt the input is grabbed.
> 
> The other issue with the input grab is that it is not really that much
> helpful: when you press Ctrl+Alt it is still possible to activate the
> gvncviewer menu items with keyboard shortcuts meaning that these
> shortcuts are not available on the remote desktop.

That's a bug in the viewer program - it needs to properly block  the
menus / accelerators when grabbed. I just fixed this in virt-viewer,
so I'll port the same fix to gvncviewer.

Daniel

> diff --git a/examples/gvncviewer.c b/examples/gvncviewer.c
> index 97bdef8..04824f6 100644
> --- a/examples/gvncviewer.c
> +++ b/examples/gvncviewer.c
> @@ -36,7 +36,7 @@ static const GOptionEntry options [] =
>  {
>    {
>      G_OPTION_REMAINING, '\0', 0, G_OPTION_ARG_STRING_ARRAY, &args,
> -    NULL, "hostname[:display]" },
> +    NULL, "[hostname][:display]" },
>    { NULL, 0, 0, G_OPTION_ARG_NONE, NULL, NULL, 0 }
>  };
>  
> @@ -323,7 +323,7 @@ int main(int argc, char **argv)
>  		return 1;
>  	}
>  	if (!args || (g_strv_length(args) != 1)) {
> -		fprintf(stderr, "Usage: gvncviewer hostname[:display]\n%s\n", help_msg);
> +		fprintf(stderr, "Usage: gvncviewer [hostname][:display]\n%s\n", help_msg);
>  		return 1;
>  	}
>  
> @@ -389,6 +389,8 @@ int main(int argc, char **argv)
>  	} else
>  		snprintf(port, sizeof(port), "%d", 5900);
>  
> +        if(! *hostname) 
> +          snprintf(hostname, sizeof(hostname), "%s", "127.0.0.1");
>  	vnc_display_open_host(VNC_DISPLAY(vnc), hostname, port);
>  	vnc_display_set_keyboard_grab(VNC_DISPLAY(vnc), TRUE);
>  	vnc_display_set_pointer_grab(VNC_DISPLAY(vnc), TRUE);


Functionally this looks ok, but identation whitespace is messed up.


> diff --git a/examples/gvncviewer.c b/examples/gvncviewer.c
> index 04824f6..4cf573f 100644
> --- a/examples/gvncviewer.c
> +++ b/examples/gvncviewer.c
> @@ -84,9 +84,12 @@ static void vnc_ungrab(GtkWidget *vncdisplay, GtkWidget *window)
>  	set_title(VNC_DISPLAY(vncdisplay), window, FALSE);
>  }
>  
> +static int connected = 0;
> +
>  static void vnc_connected(GtkWidget *vncdisplay G_GNUC_UNUSED)
>  {
>  	printf("Connected to server\n");
> +        connected = 1;
>  }
>  
>  static void vnc_initialized(GtkWidget *vncdisplay, GtkWidget *window)
> @@ -110,7 +113,10 @@ static void vnc_desktop_resize(GtkWidget *vncdisplay G_GNUC_UNUSED,
>  
>  static void vnc_disconnected(GtkWidget *vncdisplay G_GNUC_UNUSED)
>  {
> -	printf("Disconnected from server\n");
> +	if(connected)
> +          printf("Disconnected from server\n");
> +        else
> +          printf("Failed to connect to server\n");
>  	gtk_main_quit();
>  }


This looks ok, aside from the same messed up whitespace issue.


> diff --git a/examples/gvncviewer.c b/examples/gvncviewer.c
> index 4cf573f..2d5c19e 100644
> --- a/examples/gvncviewer.c
> +++ b/examples/gvncviewer.c
> @@ -40,6 +40,8 @@ static const GOptionEntry options [] =
>    { NULL, 0, 0, G_OPTION_ARG_NONE, NULL, NULL, 0 }
>  };
>  
> +static int allow_grab = TRUE;
> +
>  
>  static GtkWidget *vnc;
>  
> @@ -168,6 +170,23 @@ static void do_scaling(GtkWidget *menu, GtkWidget *vncdisplay)
>  		vnc_display_set_scaling(VNC_DISPLAY(vncdisplay), FALSE);
>  }
>  
> +static void set_grab(GtkWidget * vncdisplay)
> +{
> +	vnc_display_set_keyboard_grab(VNC_DISPLAY(vncdisplay), allow_grab);
> +	vnc_display_set_pointer_grab(VNC_DISPLAY(vncdisplay), allow_grab);
> +}

This should only toggle keyboard grab, not pointer grab.

> +static void do_allow_grab(GtkWidget *menu, GtkWidget *vncdisplay)
> +{
> +	if (gtk_check_menu_item_get_active(GTK_CHECK_MENU_ITEM(menu)))
> +          allow_grab = TRUE;
> +	else
> +          allow_grab = FALSE;
> +
> +        set_grab(vncdisplay);
> +}
> +
>  static void vnc_credential(GtkWidget *vncdisplay, GValueArray *credList)
>  {
>  	GtkWidget *dialog = NULL;
> @@ -313,6 +332,7 @@ int main(int argc, char **argv)
>  	GtkWidget *cab;
>  	GtkWidget *fullscreen;
>  	GtkWidget *scaling;
> +	GtkWidget *grab;
>  	const char *help_msg = "Run 'gvncviewer --help' to see a full list of available command line options";
>  
>  	/* Setup command line options */
> @@ -375,6 +395,10 @@ int main(int argc, char **argv)
>  
>  	gtk_menu_item_set_submenu(GTK_MENU_ITEM(view), submenu);
>  
> +        grab = gtk_check_menu_item_new_with_mnemonic("Allow Input _Grab");
> +        gtk_menu_bar_append(GTK_MENU_BAR(menubar), grab);
> +	gtk_check_menu_item_set_active(GTK_CHECK_MENU_ITEM(grab), allow_grab);
> +
>  #if WITH_LIBVIEW
>  	ViewAutoDrawer_SetActive(VIEW_AUTODRAWER(layout), FALSE);
>  	ViewOvBox_SetOver(VIEW_OV_BOX(layout), menubar);
> @@ -398,8 +422,7 @@ int main(int argc, char **argv)
>          if(! *hostname) 
>            snprintf(hostname, sizeof(hostname), "%s", "127.0.0.1");
>  	vnc_display_open_host(VNC_DISPLAY(vnc), hostname, port);
> -	vnc_display_set_keyboard_grab(VNC_DISPLAY(vnc), TRUE);
> -	vnc_display_set_pointer_grab(VNC_DISPLAY(vnc), TRUE);
> +        set_grab(vnc);
>  
>  	if (!gtk_widget_is_composited(window)) {
>  		vnc_display_set_scaling(VNC_DISPLAY(vnc), TRUE);
> @@ -442,6 +465,8 @@ int main(int argc, char **argv)
>  			 G_CALLBACK(do_fullscreen), window);
>  	g_signal_connect(scaling, "toggled",
>  			 G_CALLBACK(do_scaling), vnc);
> +	g_signal_connect(grab, "toggled",
> +			 G_CALLBACK(do_allow_grab), vnc);
>  #if WITH_LIBVIEW
>  	g_signal_connect(window, "window-state-event",
>  			 G_CALLBACK(window_state_event), layout);


Regards,
Daniel
-- 
|: http://berrange.com/     -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://freshmeat.net/~danielpb/    -o-   http://gtk-vnc.sourceforge.net :|