We are currently migrating Bugzilla to GitHub issues.
Any changes made to the bug tracker now will be lost, so please do not post new bugs or make changes to them.
When we're done, all bug URLs will redirect to their equivalent location on the new bug tracker.

Bug 3249 - keysym.mod is incorrect when mod keys are pressed for SDL_KEYDOWN events
Summary: keysym.mod is incorrect when mod keys are pressed for SDL_KEYDOWN events
Status: RESOLVED FIXED
Alias: None
Product: SDL
Classification: Unclassified
Component: events (show other bugs)
Version: 2.0.4
Hardware: x86_64 Windows 7
: P2 normal
Assignee: Sam Lantinga
QA Contact: Sam Lantinga
URL:
Keywords:
: 3074 (view as bug list)
Depends on:
Blocks:
 
Reported: 2016-01-28 02:56 UTC by Adam M.
Modified: 2017-08-12 19:34 UTC (History)
2 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Adam M. 2016-01-28 02:56:04 UTC
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.
Comment 1 Adam M. 2016-01-28 03:05:54 UTC
*** Bug 3074 has been marked as a duplicate of this bug. ***
Comment 2 Sam Lantinga 2017-08-12 19:34:48 UTC
Fixed, thanks!
https://hg.libsdl.org/SDL/rev/1f6105d09344