Improve unknown keysym handling

Instead of giving up after all free keycodes have been used, Keycodes from previously added keysyms will be reused.

Re: #93
(cherry picked from commit f65433a113ed2cce39161139982112aaa3ddf6e0)

5 files changed, 154 insertions, 19 deletions

diff --git a/unix/x0vncserver/XDesktop.cxx b/unix/x0vncserver/XDesktop.cxx
index 55ea9667..1e529879 100644
--- a/unix/x0vncserver/XDesktop.cxx
+++ b/unix/x0vncserver/XDesktop.cxx
@@ -405,6 +405,42 @@ KeyCode XDesktop::XkbKeysymToKeycode(Display* dpy, KeySym keysym) {
 
   return keycode;
 }
+/*
+ * Keeps the list in LRU order by moving the used key to front of the list.
+ */
+static void onKeyUsed(std::list<AddedKeySym> &list, KeyCode usedKeycode) {
+  if (list.empty() || list.front().keycode == usedKeycode)
+    return;
+
+  std::list<AddedKeySym>::iterator it = list.begin();
+  ++it;
+  for (; it != list.end(); ++it) {
+    AddedKeySym item = *it;
+    if (item.keycode == usedKeycode) {
+      list.erase(it);
+      list.push_front(item);
+      break;
+    }
+  }
+}
+
+/*
+ * Returns keycode of oldest item from list of manually added keysyms.
+ * The item is removed from the list.
+ * Returns 0 if no usable keycode is found.
+ */
+KeyCode XDesktop::getReusableKeycode(XkbDescPtr xkb) {
+  while (!addedKeysyms.empty()) {
+    AddedKeySym last = addedKeysyms.back();
+    addedKeysyms.pop_back();
+
+    // Make sure someone else hasn't modified the key
+    if (XkbKeyNumGroups(xkb, last.keycode) > 0 &&
+      XkbKeySymsPtr(xkb, last.keycode)[0] == last.keysym)
+      return last.keycode;
+  }
+  return 0;
+}
 
 KeyCode XDesktop::addKeysym(Display* dpy, KeySym keysym)
 {
@@ -426,6 +462,9 @@ KeyCode XDesktop::addKeysym(Display* dpy, KeySym keysym)
   }
 
   if (key < xkb->min_key_code)
+    key = getReusableKeycode(xkb);
+
+  if (!key)
     return 0;
 
   memset(&changes, 0, sizeof(changes));
@@ -453,7 +492,7 @@ KeyCode XDesktop::addKeysym(Display* dpy, KeySym keysym)
 
   if (XkbChangeMap(dpy, xkb, &changes)) {
     vlog.info("Added unknown keysym %s to keycode %d", XKeysymToString(keysym), key);
-    addedKeysyms[keysym] = key;
+    addedKeysyms.push_front({ syms[0], (KeyCode)key });
     return key;
   }
 
@@ -472,21 +511,17 @@ void XDesktop::deleteAddedKeysyms(Display* dpy) {
 
   KeyCode lowestKeyCode = xkb->max_key_code;
   KeyCode highestKeyCode = xkb->min_key_code;
-  std::map<KeySym, KeyCode>::iterator it;
-  for (it = addedKeysyms.begin(); it != addedKeysyms.end(); it++) {
-    if (XkbKeyNumGroups(xkb, it->second) != 0) {
-      // Check if we are removing keysym we added ourself
-      if (XkbKeysymToKeycode(dpy, it->first) != it->second)
-        continue;
+  KeyCode keyCode = getReusableKeycode(xkb);
+  while (keyCode != 0) {
+    XkbChangeTypesOfKey(xkb, keyCode, 0, XkbGroup1Mask, NULL, &changes);
 
-      XkbChangeTypesOfKey(xkb, it->second, 0, XkbGroup1Mask, NULL, &changes);
+    if (keyCode < lowestKeyCode)
+      lowestKeyCode = keyCode;
 
-      if (it->second < lowestKeyCode)
-        lowestKeyCode = it->second;
+    if (keyCode > highestKeyCode)
+      highestKeyCode = keyCode;
 
-      if (it->second > highestKeyCode)
-        highestKeyCode = it->second;
-    }
+    keyCode = getReusableKeycode(xkb);
   }
 
   // Did we actually find something to remove?
@@ -497,8 +532,6 @@ void XDesktop::deleteAddedKeysyms(Display* dpy) {
   changes.first_key_sym = lowestKeyCode;
   changes.num_key_syms = highestKeyCode - lowestKeyCode + 1;
   XkbChangeMap(dpy, xkb, &changes);
-
-  addedKeysyms.clear();
 }
 
 KeyCode XDesktop::keysymToKeycode(Display* dpy, KeySym keysym) {
@@ -552,6 +585,9 @@ void XDesktop::keyEvent(uint32_t keysym, uint32_t xtcode, bool down) {
   else
     pressedKeys.erase(keysym);
 
+  if (down)
+    onKeyUsed(addedKeysyms, keycode);
+
   vlog.debug("%d %s", keycode, down ? "down" : "up");
 
   XTestFakeKeyEvent(dpy, keycode, down, CurrentTime);
diff --git a/unix/x0vncserver/XDesktop.h b/unix/x0vncserver/XDesktop.h
index fc230e5b..4777a65a 100644
--- a/unix/x0vncserver/XDesktop.h
+++ b/unix/x0vncserver/XDesktop.h
@@ -35,6 +35,12 @@
 class Geometry;
 class XPixelBuffer;
 
+struct AddedKeySym
+{
+  KeySym keysym;
+  KeyCode keycode;
+};
+
 // number of XKb indicator leds to handle
 #define XDESKTOP_N_LEDS 3
 
@@ -78,7 +84,7 @@ protected:
   bool haveXtest;
   bool haveDamage;
   int maxButtons;
-  std::map<KeySym, KeyCode> addedKeysyms;
+  std::list<AddedKeySym> addedKeysyms;
   std::map<KeySym, KeyCode> pressedKeys;
   bool running;
 #ifdef HAVE_XDAMAGE
@@ -102,6 +108,7 @@ protected:
 protected:
 #ifdef HAVE_XTEST
   KeyCode XkbKeysymToKeycode(Display* dpy, KeySym keysym);
+  KeyCode getReusableKeycode(XkbDescPtr xkb);
   KeyCode addKeysym(Display* dpy, KeySym keysym);
   void deleteAddedKeysyms(Display* dpy);
   KeyCode keysymToKeycode(Display* dpy, KeySym keysym);
diff --git a/unix/xserver/hw/vnc/vncInput.c b/unix/xserver/hw/vnc/vncInput.c
index b3d0926d..1de41430 100644
--- a/unix/xserver/hw/vnc/vncInput.c
+++ b/unix/xserver/hw/vnc/vncInput.c
@@ -612,6 +612,9 @@ static void vncKeysymKeyboardEvent(KeySym keysym, int down)
 	/* Now press the actual key */
 	pressKey(vncKeyboardDev, keycode, TRUE, "keycode");
 
+	if(down)
+		vncOnKeyUsed(keycode);
+
 	/* And store the mapping so that we can do a proper release later */
 	for (i = 0;i < 256;i++) {
 		if (i == keycode)
diff --git a/unix/xserver/hw/vnc/vncInput.h b/unix/xserver/hw/vnc/vncInput.h
index 08cab6cb..0a130ada 100644
--- a/unix/xserver/hw/vnc/vncInput.h
+++ b/unix/xserver/hw/vnc/vncInput.h
@@ -55,6 +55,7 @@ KeyCode vncKeysymToKeycode(KeySym keysym, unsigned state, unsigned *new_state);
 int vncIsAffectedByNumLock(KeyCode keycode);
 
 KeyCode vncAddKeysym(KeySym keysym, unsigned state);
+void vncOnKeyUsed(KeyCode usedKeycode);
 
 #ifdef __cplusplus
 }
diff --git a/unix/xserver/hw/vnc/vncInputXKB.c b/unix/xserver/hw/vnc/vncInputXKB.c
index d5fe286a..77983482 100644
--- a/unix/xserver/hw/vnc/vncInputXKB.c
+++ b/unix/xserver/hw/vnc/vncInputXKB.c
@@ -30,6 +30,7 @@
 #include <X11/Xlib.h>
 #include <X11/Xutil.h>
 
+#include "list.h"
 #include "xkbsrv.h"
 #include "xkbstr.h"
 #include "eventstr.h"
@@ -56,6 +57,25 @@ static const KeyCode fakeKeys[] = {
 #endif
     };
 
+typedef struct
+{
+	KeySym keysym;
+	KeyCode keycode;
+	struct xorg_list entry;
+} AddedKeySym;
+
+/*
+ * If a KeySym recieved from client is not mapped to any KeyCode, it needs to be
+ * mapped to an unused KeyCode to generate required key events.
+ *
+ * This list tracks such assignments. A KeyCode from this list can be reused if
+ * we run out of unused KeyCodes.
+ *
+ * Items in this list are maintained in LRU order, with most recently used key
+ * in front.
+ */
+static struct xorg_list addedKeysyms;
+
 static void vncXkbProcessDeviceEvent(int screenNum,
                                      InternalEvent *event,
                                      DeviceIntPtr dev);
@@ -218,6 +238,8 @@ void vncPrepareInputDevices(void)
 	 */
 	mieqSetHandler(ET_KeyPress, vncXkbProcessDeviceEvent);
 	mieqSetHandler(ET_KeyRelease, vncXkbProcessDeviceEvent);
+
+	xorg_list_init(&addedKeysyms);
 }
 
 unsigned vncGetKeyboardState(void)
@@ -568,6 +590,68 @@ int vncIsAffectedByNumLock(KeyCode keycode)
 	return 1;
 }
 
+static void saveAddedKeysym(KeyCode code, KeySym sym)
+{
+	AddedKeySym* item;
+
+	item = malloc(sizeof(AddedKeySym));
+	if (!item)
+		return;
+
+	item->keycode = code;
+	item->keysym = sym;
+	xorg_list_add(&item->entry, &addedKeysyms);
+}
+
+/*
+ * Keeps the list in LRU order by moving the used key to front of the list.
+ */
+void vncOnKeyUsed(KeyCode usedKeycode)
+{
+	AddedKeySym* it;
+
+	if (xorg_list_is_empty(&addedKeysyms))
+		return;
+
+	it = xorg_list_first_entry(&addedKeysyms, AddedKeySym, entry);
+	if (it->keycode == usedKeycode)
+		return;
+
+	xorg_list_for_each_entry(it, &addedKeysyms, entry) {
+		if (it->keycode == usedKeycode) {
+			xorg_list_del(&it->entry);
+			xorg_list_add(&it->entry, &addedKeysyms);
+			break;
+		}
+	}
+}
+
+/*
+ * Returns keycode of oldest item from list of manually added keysyms.
+ * The item is removed from the list.
+ * Returns 0 if no usable keycode is found.
+ */
+static KeyCode getReusableKeycode(XkbDescPtr xkb)
+{
+	AddedKeySym* last;
+	KeyCode result;
+
+	result = 0;
+	while (result == 0 && !xorg_list_is_empty(&addedKeysyms)) {
+		last = xorg_list_last_entry(&addedKeysyms, AddedKeySym, entry);
+
+		// Make sure someone else hasn't modified the key
+		if (XkbKeyNumGroups(xkb, last->keycode) > 0 &&
+			XkbKeySymsPtr(xkb, last->keycode)[0] == last->keysym &&
+			(xkb->names == NULL || xkb->names->keys[last->keycode].name[0] == 'T'))
+			result = last->keycode;
+
+		xorg_list_del(&last->entry);
+		free(last);
+	}
+	return result;
+}
+
 KeyCode vncAddKeysym(KeySym keysym, unsigned state)
 {
 	DeviceIntPtr master;
@@ -589,6 +673,9 @@ KeyCode vncAddKeysym(KeySym keysym, unsigned state)
 	}
 
 	if (key < xkb->min_key_code)
+		key = getReusableKeycode(xkb);
+
+	if (!key)
 		return 0;
 
 	memset(&changes, 0, sizeof(changes));
@@ -600,9 +687,8 @@ KeyCode vncAddKeysym(KeySym keysym, unsigned state)
 	 * Tools like xkbcomp get confused if there isn't a name
 	 * assigned to the keycode we're trying to use.
 	 */
-	if (xkb->names && xkb->names->keys &&
-	    (xkb->names->keys[key].name[0] == '\0')) {
-		xkb->names->keys[key].name[0] = 'I';
+	if (xkb->names && xkb->names->keys) {
+		xkb->names->keys[key].name[0] = 'T';
 		xkb->names->keys[key].name[1] = '0' + (key / 100) % 10;
 		xkb->names->keys[key].name[2] = '0' + (key /  10) % 10;
 		xkb->names->keys[key].name[3] = '0' + (key /   1) % 10;
@@ -641,6 +727,8 @@ KeyCode vncAddKeysym(KeySym keysym, unsigned state)
 
 	XkbSendNotification(master, &changes, &cause);
 
+	saveAddedKeysym(key, syms[0]);
+
 	return key;
 }