Fixed bug 2096 - Mapping from scancode to keycode doesn't work for remapped modifier keys

Jacob Lee

If a user has a non-standard keyboard mapping -- say, their caps lock key has been mapped to Ctrl -- then SDL_GetModState() is no longer accurate: it only considers the unmapped keys. This is a regression from SDL 1.2.

I think there are two parts to this bug: first, GetModState should use keycodes, rather than scancodes, which is easy enough.

Unfortunately, on my system, SDL considers Caps Lock, even when mapped as Control, to be both SDL_SCANCODE_CAPSLOCK and SDLK_CAPSLOCK. The output from checkkeys for it is:

INFO: Key pressed :  scancode 57 = CapsLock, keycode 0x40000039 = CapsLock  modifiers: CAPS

Whereas the output for xev is:

KeyPress event, serial 41, synthetic NO, window 0x4a00001,
    root 0x9a, subw 0x0, time 40218333, (144,177), root:(1458,222),
    state 0x10, keycode 66 (keysym 0xffe3, Control_L), same_screen YES,
    XKeysymToKeycode returns keycode: 37
    XLookupString gives 0 bytes:
    XmbLookupString gives 0 bytes:
    XFilterEvent returns: False

I think the problem is that X11_UpdateKeymap in SDL_x11keyboard.c only builds a mapping for keycodes associated with a Unicode character (anything where X11_KeyCodeToUcs returns a value). In the case of caps lock, SDL scancode 57 becomes x11 keycode 66, which becomes x11 keysym 65507(Control_L), which does not have a unicode value.

To fix this, I suspect that SDL needs a mapping of the rest of the x11 keysyms to their corresponding SDL key codes.

src/events/SDL_keyboard.c

@@ -662,6 +662,8 @@ SDL_SendKeyboardKey(Uint8 state, SDL_Scancode scancode)
 {
     SDL_Keyboard *keyboard = &SDL_keyboard;
     int posted;
+    SDL_Keymod modifier;
+    SDL_Keycode keycode;
     Uint16 modstate;
     Uint32 type;
     Uint8 repeat;
@@ -673,82 +675,6 @@ SDL_SendKeyboardKey(Uint8 state, SDL_Scancode scancode)
     printf("The '%s' key has been %s\n", SDL_GetScancodeName(scancode),
            state == SDL_PRESSED ? "pressed" : "released");
 #endif
-    if (state == SDL_PRESSED) {
-        modstate = keyboard->modstate;
-        switch (scancode) {
-        case SDL_SCANCODE_NUMLOCKCLEAR:
-            keyboard->modstate ^= KMOD_NUM;
-            break;
-        case SDL_SCANCODE_CAPSLOCK:
-            keyboard->modstate ^= KMOD_CAPS;
-            break;
-        case SDL_SCANCODE_LCTRL:
-            keyboard->modstate |= KMOD_LCTRL;
-            break;
-        case SDL_SCANCODE_RCTRL:
-            keyboard->modstate |= KMOD_RCTRL;
-            break;
-        case SDL_SCANCODE_LSHIFT:
-            keyboard->modstate |= KMOD_LSHIFT;
-            break;
-        case SDL_SCANCODE_RSHIFT:
-            keyboard->modstate |= KMOD_RSHIFT;
-            break;
-        case SDL_SCANCODE_LALT:
-            keyboard->modstate |= KMOD_LALT;
-            break;
-        case SDL_SCANCODE_RALT:
-            keyboard->modstate |= KMOD_RALT;
-            break;
-        case SDL_SCANCODE_LGUI:
-            keyboard->modstate |= KMOD_LGUI;
-            break;
-        case SDL_SCANCODE_RGUI:
-            keyboard->modstate |= KMOD_RGUI;
-            break;
-        case SDL_SCANCODE_MODE:
-            keyboard->modstate |= KMOD_MODE;
-            break;
-        default:
-            break;
-        }
-    } else {
-        switch (scancode) {
-        case SDL_SCANCODE_NUMLOCKCLEAR:
-        case SDL_SCANCODE_CAPSLOCK:
-            break;
-        case SDL_SCANCODE_LCTRL:
-            keyboard->modstate &= ~KMOD_LCTRL;
-            break;
-        case SDL_SCANCODE_RCTRL:
-            keyboard->modstate &= ~KMOD_RCTRL;
-            break;
-        case SDL_SCANCODE_LSHIFT:
-            keyboard->modstate &= ~KMOD_LSHIFT;
-            break;
-        case SDL_SCANCODE_RSHIFT:
-            keyboard->modstate &= ~KMOD_RSHIFT;
-            break;
-        case SDL_SCANCODE_LALT:
-            keyboard->modstate &= ~KMOD_LALT;
-            break;
-        case SDL_SCANCODE_RALT:
-            keyboard->modstate &= ~KMOD_RALT;
-            break;
-        case SDL_SCANCODE_LGUI:
-            keyboard->modstate &= ~KMOD_LGUI;
-            break;
-        case SDL_SCANCODE_RGUI:
-            keyboard->modstate &= ~KMOD_RGUI;
-            break;
-        case SDL_SCANCODE_MODE:
-            keyboard->modstate &= ~KMOD_MODE;
-            break;
-        default:
-            break;
-        }
-        modstate = keyboard->modstate;
-    }
 
     /* Figure out what type of event this is */
     switch (state) {
@@ -775,6 +701,59 @@ SDL_SendKeyboardKey(Uint8 state, SDL_Scancode scancode)
     /* Update internal keyboard state */
     keyboard->keystate[scancode] = state;
 
+    keycode = keyboard->keymap[scancode];
+
+    /* Update modifiers state if applicable */
+    switch (keycode) {
+    case SDLK_LCTRL:
+        modifier = KMOD_LCTRL;
+        break;
+    case SDLK_RCTRL:
+        modifier = KMOD_RCTRL;
+        break;
+    case SDLK_LSHIFT:
+        modifier = KMOD_LSHIFT;
+        break;
+    case SDLK_RSHIFT:
+        modifier = KMOD_RSHIFT;
+        break;
+    case SDLK_LALT:
+        modifier = KMOD_LALT;
+        break;
+    case SDLK_RALT:
+        modifier = KMOD_RALT;
+        break;
+    case SDLK_LGUI:
+        modifier = KMOD_LGUI;
+        break;
+    case SDLK_RGUI:
+        modifier = KMOD_RGUI;
+        break;
+    case SDLK_MODE:
+        modifier = KMOD_MODE;
+        break;
+    default:
+        modifier = KMOD_NONE;
+        break;
+    }
+    if (SDL_KEYDOWN == type) {
+        modstate = keyboard->modstate;
+        switch (keycode) {
+        case SDLK_NUMLOCKCLEAR:
+            keyboard->modstate ^= KMOD_NUM;
+            break;
+        case SDLK_CAPSLOCK:
+            keyboard->modstate ^= KMOD_CAPS;
+            break;
+        default:
+            keyboard->modstate |= modifier;
+            break;
+        }
+    } else {
+        keyboard->modstate &= ~modifier;
+        modstate = keyboard->modstate;
+    }
+
     /* Post the event, if desired */
     posted = 0;
     if (SDL_GetEventState(type) == SDL_ENABLE) {
@@ -783,7 +762,7 @@ SDL_SendKeyboardKey(Uint8 state, SDL_Scancode scancode)
         event.key.state = state;
         event.key.repeat = repeat;
         event.key.keysym.scancode = scancode;
-        event.key.keysym.sym = keyboard->keymap[scancode];
+        event.key.keysym.sym = keycode;
         event.key.keysym.mod = modstate;
         event.key.windowID = keyboard->focus ? keyboard->focus->id : 0;
         posted = (SDL_PushEvent(&event) > 0);

src/video/x11/SDL_x11keyboard.c

@@ -252,8 +252,7 @@ X11_InitKeyboard(_THIS)
 #endif
         SDL_memcpy(&data->key_layout[min_keycode], scancode_set[best_index].table,
                    sizeof(SDL_Scancode) * scancode_set[best_index].table_size);
-    }
+    } else {
-    else {
         SDL_Keycode keymap[SDL_NUM_SCANCODES];
 
         printf
@@ -316,6 +315,8 @@ X11_UpdateKeymap(_THIS)
         key = X11_KeyCodeToUcs4(data->display, (KeyCode)i);
         if (key) {
             keymap[scancode] = key;
+        } else {
+            keymap[scancode] = SDL_SCANCODE_TO_KEYCODE(X11_KeyCodeToSDLScancode(data->display, (KeyCode)i));
         }
     }
     SDL_SetKeymap(0, keymap, SDL_NUM_SCANCODES);