Re: [PATCH] Implement custom grab key combination definition
- From: Michal Novotny <minovotn redhat com>
- To: "Daniel P. Berrange" <dan berrange com>
- Cc: gtk-vnc-list gnome org
- Subject: Re: [PATCH] Implement custom grab key combination definition
- Date: Mon, 07 Jun 2010 11:03:27 +0200
On 06/04/2010 05:43 PM, Daniel P. Berrange wrote:
On Fri, Jun 04, 2010 at 11:17:31AM +0200, Michal Novotny wrote:
Hi all,
I'm working in virtualization and I'm using virt-manager and virt-viewer
tools to see the domain's VNC window where the grab key is defined to be
"Ctrl + Alt". Since this configuration is not that good in some cases
(since "Ctrl+Alt" could be defined for anything else in the window
manager) this patch allows you to set the grab key combination in the
running GTK-VNC application. I did work on this one in my spare time
since I think this is a useful feature to be implemented. At least I've
learned how to use GTK in C language if you won't apply the patch.
Thanks for working on this - its been a long standing TODO item
No problem. I just find this useful so I did the patch.
Technically 2 new functions have been added to do the job,
vnc_display_set_grab_keys() and vnc_display_get_grab_keys() with the old
definition to "Ctrl+Alt" preserved by default.
Please note that the gvncviewer.c and gvncviewer.py codes have been
changed to use both of the functions and the definition can be modifiers
only, or now even modifiers in combination with some key, i.e. something
like: "Ctrl+Alt+[F]" or "Ctrl+Alt+[F|f]". The brackets are important
since they specify the character or set of 2 characters to be used with
the modifiers combination to trigger the grab and they have to be at the
end of the expression. When 2 characters, delimited by logical "or" (|)
are there, it means that the expression is being matched by either of
them therefore you can specify both uppercase and lowercase letters here
to the expression or make possibility of 2 different keys with the
expression like "Ctrl+Alt+[a|b]" to match both 'a' and 'b' keys with the
Control and Alt modifiers pressed.
Do we really need to have the ability to match multiple different
key sequences ? I don't see why you'd need both Ctrl+Alt+a and
Ctrl+Alt+b to be release sequences. Likewise I don't see why we'd
need to cope with upper and lowercase matching of letters - why
would the user go to the bother of holding Ctrl+Alt+Shift+f (to get
the uppercase F) when just Ctrl+Alt+f is already a release sequence.
Well, I can see your point. Of course this should be OK as well but I
just didn't want to bother user with the additional Shift key. Moreover
some other keycodes should be added, like Tab and function keys. I'll do
this for the next version of this patch. This was just an alpha version
to fulfil my purpose and honestly I was not certain the patch won't get
dropped so I'm glad I can work on this one and it's useful not only by
me but by you as well ;)
commit 4f39be84f6adf55896ea3bb5cbdc81dee075bb8e
Author: Michal Novotny<minovotn redhat com>
Date: Fri Jun 4 01:43:50 2010 +0200
Implement custom grab key combination definition
This is the patch to implement custom grab key combination definition
support. The new functions {get|set}_grab_keys has been introduced in
this patch. Definition can be done either for modifier keys (e.g.
"Ctrl + Alt") or some key could be added to the modifiers, e.g.
"Ctrl + Alt + [F]". Please note the brackets in the definition which
is necessary to specify a character or an alternative character using
"Ctrl + Alt + [F|f]" which allows you to define both uppercase and
lowercase letters or two printable letters to grab the VNC window.
Please note that only the printable letters can be put into those
brackets and no function keys [F1 - F12] or any others yet.
Signed-off-by: Michal Novotny<minovotn redhat com>
diff --git a/examples/gvncviewer.c b/examples/gvncviewer.c
index c3fbd74..058ea77 100644
--- a/examples/gvncviewer.c
+++ b/examples/gvncviewer.c
@@ -48,12 +48,13 @@ static void set_title(VncDisplay *vncdisplay, GtkWidget *window,
{
const char *name;
char title[1024];
- const char *subtitle;
+ char subtitle[512];
if (grabbed)
- subtitle = "(Press Ctrl+Alt to release pointer) ";
+ snprintf(subtitle, 512, "(Press %s to release pointer) ",
+ vnc_display_get_grab_keys(VNC_DISPLAY(vncdisplay)));
else
- subtitle = "";
+ memset(subtitle, 0, 512);
Use of snprintf() is generally a bad idea because its fragile with
fixed length buffers. GLib provides a handy alternative that allocs
enough memory:
g_strdup_printf("(Press %s to release pointer) ",
vnc_display_get_grab_keys(VNC_DISPLAY(vncdisplay)));
Oh, ok. I didn't know that. Thanks for your input.
name = vnc_display_get_name(VNC_DISPLAY(vncdisplay));
snprintf(title, sizeof(title), "%s%s - GVncViewer",
@@ -467,6 +468,8 @@ int main(int argc, char **argv)
vnc_display_set_keyboard_grab(VNC_DISPLAY(vnc), TRUE);
vnc_display_set_pointer_grab(VNC_DISPLAY(vnc), TRUE);
+ vnc_display_set_grab_keys(VNC_DISPLAY(vnc), "Ctrl+Alt+[F|f]");
+
There's a indentation bug there.
Ok, it will be fixed in the next version of the patch.
if (!gtk_widget_is_composited(window)) {
vnc_display_set_scaling(VNC_DISPLAY(vnc), TRUE);
gtk_check_menu_item_set_active(GTK_CHECK_MENU_ITEM(scaling), TRUE);
diff --git a/examples/gvncviewer.py b/examples/gvncviewer.py
old mode 100644
new mode 100755
index 9a74268..0572309
--- a/examples/gvncviewer.py
+++ b/examples/gvncviewer.py
@@ -29,7 +29,7 @@ if len(sys.argv) != 2 and len(sys.argv) != 3:
def set_title(vnc, window, grabbed):
name = vnc.get_name()
if grabbed:
- subtitle = "(Press Ctrl+Alt to release pointer) "
+ subtitle = "(Press %s to release pointer) " % vnc.get_grab_keys()
else:
subtitle = ""
@@ -173,6 +173,8 @@ layout.add(vnc)
vnc.realize()
vnc.set_pointer_grab(True)
vnc.set_keyboard_grab(True)
+# Example to define "Ctrl+Alt+F" or "Ctrl+Alt+f" as a grab key combo
+vnc.set_grab_keys("Ctrl+Alt+[F|f]")
#v.set_pointer_local(True)
if len(sys.argv) == 3:
diff --git a/src/libgtk-vnc_sym.version b/src/libgtk-vnc_sym.version
index b6f5347..41bcf21 100644
--- a/src/libgtk-vnc_sym.version
+++ b/src/libgtk-vnc_sym.version
@@ -68,6 +68,10 @@
vnc_image_framebuffer_new;
vnc_image_framebuffer_get_image;
+# grab key settings support
+ vnc_display_set_grab_keys;
+ vnc_display_get_grab_keys;
+
local:
*;
};
diff --git a/src/vncdisplay.c b/src/vncdisplay.c
index a031125..960b43c 100644
--- a/src/vncdisplay.c
+++ b/src/vncdisplay.c
@@ -44,6 +44,8 @@
#include<winsock2.h>
#endif
+#define VNC_DEFAULT_GRAB_KEYS "Ctrl+Alt"
+
static void winsock_startup (void);
static void winsock_cleanup (void);
@@ -85,6 +87,13 @@ struct _VncDisplayPrivate
GSList *preferable_auths;
const guint8 const *keycode_map;
+
+ /* Variables for the grab key */
+ gchar *grab_key_combo;
+ guint grab_key;
+ guint grab_key_alt;
+ guint grab_key_modifiers;
+ guint _grab_key_mod;
};
Indentation again here
Ok, will be fixed in v2.
G_DEFINE_TYPE(VncDisplay, vnc_display, GTK_TYPE_DRAWING_AREA)
@@ -95,6 +104,7 @@ enum
PROP_0,
PROP_POINTER_LOCAL,
PROP_POINTER_GRAB,
+ PROP_GRAB_KEYS,
PROP_KEYBOARD_GRAB,
PROP_READ_ONLY,
PROP_WIDTH,
@@ -162,6 +172,9 @@ vnc_display_get_property (GObject *object,
switch (prop_id)
{
+ case PROP_GRAB_KEYS:
+ g_value_set_string(value, vnc->priv->grab_key_combo);
+ break;
case PROP_POINTER_LOCAL:
g_value_set_boolean (value, vnc->priv->local_pointer);
break;
@@ -220,6 +233,9 @@ vnc_display_set_property (GObject *object,
case PROP_POINTER_GRAB:
vnc_display_set_pointer_grab (vnc, g_value_get_boolean (value));
break;
+ case PROP_GRAB_KEYS:
+ vnc_display_set_grab_keys (vnc, g_value_get_string (value));
+ break;
case PROP_KEYBOARD_GRAB:
vnc_display_set_keyboard_grab (vnc, g_value_get_boolean (value));
break;
@@ -673,6 +689,93 @@ static gboolean motion_event(GtkWidget *widget, GdkEventMotion *motion)
return TRUE;
}
+#define KEY_MOD_CTRL 0x01
+#define KEY_MOD_ALT 0x02
+#define KEY_MOD_SHIFT 0x04
+#define KEY_MOD_SUPER 0x08
+#define KEY_MOD_HYPER 0x16
+
+static void update_grab_keys(VncDisplay *obj)
+{
+ VncDisplayPrivate *priv = obj->priv;
+ guint mod, len;
+
+ if ((len = strlen(priv->grab_key_combo))< 3)
+ return;
+
+ mod = 0;
+ if (strstr(priv->grab_key_combo, "Ctrl") != NULL)
+ mod |= KEY_MOD_CTRL;
+ if (strstr(priv->grab_key_combo, "Alt") != NULL)
+ mod |= KEY_MOD_ALT;
+ if (strstr(priv->grab_key_combo, "Shift") != NULL)
+ mod |= KEY_MOD_SHIFT;
+ if (strstr(priv->grab_key_combo, "Super") != NULL)
+ mod |= KEY_MOD_SUPER;
+ if (strstr(priv->grab_key_combo, "Hyper") != NULL)
+ mod |= KEY_MOD_HYPER;
+
+ priv->grab_key_modifiers = mod;
+
+ if ((priv->grab_key_combo[ len - 3 ] == '[')
+&& (priv->grab_key_combo[ len - 1 ] == ']'))
+ priv->grab_key = priv->grab_key_combo[ len - 2 ];
+
+ if ((priv->grab_key_combo[ len - 5 ] == '[')
+&& (priv->grab_key_combo[ len - 3 ] == '|')
+&& (priv->grab_key_combo[ len - 1 ] == ']')) {
+ priv->grab_key = priv->grab_key_combo[ len - 4 ];
+ priv->grab_key_alt = priv->grab_key_combo[ len - 2 ];
+ }
+}
+
+static gboolean check_for_grab_key(GtkWidget *widget, int type, int keyval)
+{
+ VncDisplayPrivate *priv = VNC_DISPLAY(widget)->priv;
+ int val;
+
+ switch (keyval) {
+ case GDK_Control_L:
+ case GDK_Control_R:
+ val = KEY_MOD_CTRL;
+ break;
+ case GDK_Alt_L:
+ case GDK_Alt_R:
+ val = KEY_MOD_ALT;
+ break;
+ case GDK_Shift_L:
+ case GDK_Shift_R:
+ val = KEY_MOD_SHIFT;
+ break;
+ case GDK_Super_L:
+ case GDK_Super_R:
+ val = KEY_MOD_SUPER;
+ break;
+ case GDK_Hyper_L:
+ case GDK_Hyper_R:
+ val = KEY_MOD_HYPER;
+ break;
+ default:
+ val = 0;
+ break;
+ }
+
+ if (val) {
+ if (priv->_grab_key_mod& val)
+ priv->_grab_key_mod -= val;
+ else
+ priv->_grab_key_mod |= val;
+ }
+
+ if ((priv->_grab_key_mod == priv->grab_key_modifiers)
+&& (((keyval == priv->grab_key) || (keyval == priv->grab_key_alt))
+ || ((priv->grab_key == 0)&& (priv->grab_key_alt == 0)))
+&& (type == GDK_KEY_PRESS))
+ return TRUE;
+
+ return FALSE;
+}
+
static gboolean key_event(GtkWidget *widget, GdkEventKey *key)
{
VncDisplayPrivate *priv = VNC_DISPLAY(widget)->priv;
@@ -752,9 +855,7 @@ static gboolean key_event(GtkWidget *widget, GdkEventKey *key)
}
}
- if (key->type == GDK_KEY_PRESS&&
- ((keyval == GDK_Control_L&& (key->state& GDK_MOD1_MASK)) ||
- (keyval == GDK_Alt_L&& (key->state& GDK_CONTROL_MASK)))) {
+ if (check_for_grab_key(widget, key->type, key->keyval)) {
if (priv->in_pointer_grab)
do_pointer_ungrab(VNC_DISPLAY(widget), FALSE);
else if (!priv->grab_keyboard || !priv->absolute)
@@ -1817,6 +1918,8 @@ static void vnc_display_init(VncDisplay *display)
priv->shared_flag = FALSE;
priv->force_size = TRUE;
+ vnc_display_set_grab_keys(display, VNC_DEFAULT_GRAB_KEYS);
+
/*
* Both these two provide TLS based auth, and can layer
* all the other auth types on top. So these two must
@@ -1898,6 +2001,19 @@ void vnc_display_set_pointer_grab(VncDisplay *obj, gboolean enable)
do_pointer_ungrab(obj, FALSE);
}
+void vnc_display_set_grab_keys(VncDisplay *obj, const gchar *key_combo)
+{
+ VncDisplayPrivate *priv = obj->priv;
+
+ priv->grab_key_combo = (gchar *)key_combo;
+ update_grab_keys(obj);
+}
+
+const gchar *vnc_display_get_grab_keys(VncDisplay *obj)
+{
+ return obj->priv->grab_key_combo;
+}
+
void vnc_display_set_keyboard_grab(VncDisplay *obj, gboolean enable)
{
VncDisplayPrivate *priv = obj->priv;
diff --git a/src/vncdisplay.h b/src/vncdisplay.h
index 1b5b297..1f00f5b 100644
--- a/src/vncdisplay.h
+++ b/src/vncdisplay.h
@@ -94,6 +94,8 @@ void vnc_display_send_keys_ex(VncDisplay *obj, const guint *keyvals,
int nkeyvals, VncDisplayKeyEvent kind);
void vnc_display_send_pointer(VncDisplay *obj, gint x, gint y, int button_mask);
+void vnc_display_set_grab_keys(VncDisplay *obj, const gchar *key_combo);
+const gchar* vnc_display_get_grab_keys(VncDisplay *obj);
I'm not a huge fan of this as an API signature because we're inventing
a compound string format that people now have to parse/build.
I'm somewhat inclined to think we should be taking a list of modifiers +
list of keysyms, but that's not all that pleasant either.
eg, to set Ctrl+Alt+f as the grab sequence, an app could call
int[] grabmods = { GDK_CONTROL_MASK , GDK_MOD1_MASK, NULL }
int[] grabkeys = { GDK_f, NULL };
vnc_display_set_grab_keys(vnc, grabmods, grabkeys);
Yeah, this is right Daniel but the definition is not that easy for the
end user... Or do you think we would parse the key combination
(Ctrl+Alt+f) to make the grabmods and grabkeys from it like stated above?
Thanks for your input,
Michal
Or perhaps a simple data type
typedef struct {
int modmask;
int nkeysyms;
int *keysyms;
} VncGrabSequence;
VncGrabSequence seq = {
.modmask = GDK_CONTROL_MASK | GDK_MOD1_MASK,
.nkeysyms = 1, .keysyms = { GDK_f }
};
vnc_display_set_grab_keys(vnc, seq);
Regards,
Daniel
--
Michal Novotny<minovotn redhat com>, RHCE
Virtualization Team (xen userspace), Red Hat
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]
