[Commits] SDL: Fixed bug 3249 - keysym.mod is incorrect when mod keys are ...

libsdl.org revision control commits-owner at libsdl.org
Sat Aug 12 12:34:13 PDT 2017


details:   https://hg.libsdl.org/SDL/rev/1f6105d09344
changeset: 11240:1f6105d09344
user:      Sam Lantinga <slouken at libsdl.org>
date:      Sat Aug 12 12:34:09 2017 -0700
description:
Fixed bug 3249 - keysym.mod is incorrect when mod keys are pressed for SDL_KEYDOWN events

Adam M.

The keysym.mod field does not reflect the state of the modified keys when processing key down events for the modifier keys themselves. The documentation says that it returns the current key modifiers, but they are not current for key down events involving modifier keys. I interpret "current" to mean "equal to SDL_GetModState() at the instant the event is processed/enqueued".

For example, if you depress the Shift key you get a key down event with .mod == 0. However, .mod should not be 0 because a shift key is down. If you then release the Shift key, you get a key up event with .mod == 0. Neither event reports the modifier key.

If you press Shift and then A, .mod is incorrect (== 0) when Shift is pressed, but is correct later when A is pressed (== KMOD_LSHIFT).

You might say this behavior is deliberate, i.e. keysym.mod is the value /before/ the event, not the current value as documented, but that explanation is incorrect because only key down events behave that way. Key up events correctly give the current value, not the value before the event.

Not only is it inconsistent with itself, I think it makes keyboard processing harder.

The problem is near line 740 in SDL_keyboard.c:

if (SDL_KEYDOWN == type) {
    modstate = keyboard->modstate; // SHOULD THIS BE MOVED DOWN?
    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;
}

In the key down path, modstate (and thus keysym.mod) ends up being the modifier state /before/ the event, but in the key up path modstate ends up being the modifier state /after/ the event. Personally I think the "modstate = keyboard->modstate" line should just be moved after the entire if/else statement, so that keysym.mod always reflects the current state.

diffstat:

 src/events/SDL_keyboard.c |  5 +----
 1 files changed, 1 insertions(+), 4 deletions(-)

diffs (36 lines):

diff -r 856e5e3c1e86 -r 1f6105d09344 src/events/SDL_keyboard.c
--- a/src/events/SDL_keyboard.c	Sat Aug 12 12:24:59 2017 -0700
+++ b/src/events/SDL_keyboard.c	Sat Aug 12 12:34:09 2017 -0700
@@ -672,7 +672,6 @@
     int posted;
     SDL_Keymod modifier;
     SDL_Keycode keycode;
-    Uint16 modstate;
     Uint32 type;
     Uint8 repeat;
 
@@ -745,7 +744,6 @@
         break;
     }
     if (SDL_KEYDOWN == type) {
-        modstate = keyboard->modstate;
         switch (keycode) {
         case SDLK_NUMLOCKCLEAR:
             keyboard->modstate ^= KMOD_NUM;
@@ -759,7 +757,6 @@
         }
     } else {
         keyboard->modstate &= ~modifier;
-        modstate = keyboard->modstate;
     }
 
     /* Post the event, if desired */
@@ -771,7 +768,7 @@
         event.key.repeat = repeat;
         event.key.keysym.scancode = scancode;
         event.key.keysym.sym = keycode;
-        event.key.keysym.mod = modstate;
+        event.key.keysym.mod = keyboard->modstate;
         event.key.windowID = keyboard->focus ? keyboard->focus->id : 0;
         posted = (SDL_PushEvent(&event) > 0);
     }


More information about the commits mailing list