Allegro.cc - Online Community

Allegro.cc Forums » Allegro Development » 5.1 SVN, Windows: keyboard state, wrong modifier flags, patch

Credits go to Bruce Perry, Elias, SiegeLord, Thomas Fjellstrom, and torhu for helping out!
This thread is locked; no one can reply to it. rss feed Print
 1   2 
5.1 SVN, Windows: keyboard state, wrong modifier flags, patch
torhu
Member #2,727
September 2002
avatar

Thanks, SiegeLord. Let me note that this is just the tip of the iceberg, there are about 15 keys that are problematic. The End key and the numpad End key is another example of keys that have this relationship.

Elias
Member #358
May 2000

Ok, so to summarize my understanding:

  • One minor problem in current SVN is that the .modifiers field of ALLEGRO_KEY_DOWN messages has wrong information (SHIFT appears pressed when it is not) when returning after a switch. The patch in this thread fixes that. Except for modifiers which are extended keys, I don't know whether any of them are - does anyone know?

  • al_get_keyboard_state() has stale information when returning after a switch. The patch fixes this, except for extended keys, as explained by entheh and as seen in SiegeLord's example. For those keys the behavior is the same as before the patch.

So, I'd say the patch should be modified to not touch al_get_keyboard_state at all - only fix the event modifiers issue for now.

A separate patch may do something about al_get_keyboard_state afterwards, but according to torhu that may not be possible for extended keys.

(The event generation and switching behavior are separate issues but already work as intended anyway.)

--
"Either help out or stop whining" - Evert

Dennis
Member #1,090
July 2003
avatar

I'll also try to summarize all the issues at hand again (even though this patch was never meant to fix ALL of them).

#SelectExpand
1without any patch 2----------------- 3Window states 4------------ 5input focus no input focus 6---------------------------------------------------------------------------------------------------------------------------- 7Windows Events ok Windows Events are not generated, not evaluated 8al_get_keyboard_state ok al_get_keyboard_state not ok (always holds last known state before losing focus) 9(internal state is maintained by events) (internal is maintained by Windows events, but they are none, so it never gets updated) 10 11state switches 12-------------- 13focus to no focus -> internal state is kept with last known information as it was continuously updated by evaluating events 14no focus to focus -> internal state can hold wrong information about all sorts of keys and modifiers (SHIFT, ALT, CTRL, etc...) 15 this affects events as well, because the use the wrongly stored modifiers in their event data 16 17 18with this patch 19--------------- 20everything as aboves except one state switch difference: 21no focus to focus -> an attempt is made to fix the internal state as good as technically possible for all keys which have a known 22 VK_ to ALLEGRO_KEY_ entry in the current mapping and to update the internal modifier state 23 this fixes the wrong modifier data in allegro event data fields

The misunderstanding here was that it was assumed that the patch in this thread was meant to fix more than the wrong modifiers for now.

It was repeatedly wrongly assumed that the patch was also meant to fix ALL the other issues from the old thread.

The other misunderstanding is that we have different opinions about how the application should behave and what key state changes and key events it should be aware of, depending on Window state.

The one camp want's to simply clear the whole internal state(this should include all key state also for modifiers) on switching out.

The other camp argues that the internal state and the event based approach should display consistent behavior and that since no key-up event is generated on switching away, the state should not get cleared either because that would be the equivalent to a key-up event.

Before further (correct by definition of what the system should do) patching to fix other issues can be made, we need to reach consensus on a few things:

1. Should usage of event based keyboard handling and state based keyboard handling be consistent?
1.a) yes
1.b) no

If 1.a) (keep consistency) then:
2.) How should consistency be achieved?
2.a) keep everything as it is: state will keep last known state before switching away (consistent with no extra events as they are currently not generated, so a program re-acting to just key-down will just as a state based program still think the key is being held)
2.b) clear state and generate (Allegro)key_up events for all keys being held while switching away
2.c) keep state and update state after switching in as good as possible (technically not possible for some keys, maybe possible but needs further investigation of the data returned by the Windows API calls) and generate key-down events for keys being held while switching in

If 1.b) (give up consistency) then:
3.) Should the state simply be cleared on...?
3.a) switching away
3.b) switching back in
3.c) switching away and switching back in

This patch could be best described as a part-solution on the way to achieve 1.a) with option 2.c).

Without using this patch and making a different patch, the technically easiest to achieve behavior would be 1.b) with 3.c).

Another technically possible behavior would be 1.a) in combination with 2.b) to get consistency for switching-away behavior.
That would not fix inconsistencies on switching in though except if in addition to 2.b), 2.c) would also be implemented.

So, in short, the most we could do would be 1.a) with 2.b) AND 2.c) in an attempt to keep consistency.
The easiest we could do is 1.b) with 3.c) while throwing consistency overboard.

Elias
Member #358
May 2000

I'd say consistency between events and state is not important. Events should always correspond to WM_ messages 1:1, for keyboard as well as switch events. We do not and should not create any fake messages.

We do want correct .modifers fields for events though.

The state API has a different purpose, it's supposed to give a snapshot of all keyboard keys at the time you call the function. So there is no concept of consistency between the two APIs, they are not meant to be (but if the implementation makes them, it's not a problem).

--
"Either help out or stop whining" - Evert

Thomas Fjellstrom
Member #476
June 2000
avatar

Elias said:

We do not and should not create any fake messages.

Why not? We paper over issues in the underlying platforms all the time. This would just be another case of us working around an issue in the underlying platform.

--
Thomas Fjellstrom - [website] - [email] - [Allegro Wiki] - [Allegro TODO]
"If you can't think of a better solution, don't try to make a better solution." -- weapon_S
"The less evidence we have for what we believe is certain, the more violently we defend beliefs against those who don't agree" -- https://twitter.com/neiltyson/status/592870205409353730

Elias
Member #358
May 2000

Hm, maybe. We should first find agreement on applying a fix for the .modifers field though since that's all I'm interested in :P

--
"Either help out or stop whining" - Evert

SiegeLord
Member #7,827
October 2006
avatar

Elias said:

I don't know whether any of them are - does anyone know?

There are these two (inside extkey_to_keycode).

case VK_CONTROL:  return ALLEGRO_KEY_RCTRL;
case VK_MENU:     return ALLEGRO_KEY_ALTGR;

Still, there are VK_RCONTROL and VK_RMENU too... I don't know if Windows gives you the correct data for those keycodes, or not.

Quote:

For those keys the behavior is the same as before the patch.

No, it's not. Before the patch you couldn't get numpad enter to cause enter to be pressed. This new behavior is not an omission of a fix, but an introduced bug. The only way to get this quoted statement to be true would be to ignore the keys that have extended key variants, which this patch does not do.

Elias said:

The state API has a different purpose, it's supposed to give a snapshot of all keyboard keys at the time you call the function.

And on Windows the only way to guarantee this is to make it consistent with the WM_ messages, and thus consistent with Allegro events (not fake events though).

Elias said:

So, I'd say the patch should be modified to not touch al_get_keyboard_state at all - only fix the event modifiers issue for now.

I think this is the best solution (/if/ it's even possible, given the beginning of my post) given the current understanding of what's possible under Windows. This decouples modifiers from al_get_keyboard_state (as they are already on Linux, for example) and keeps events consistent with the keyboard state.

"For in much wisdom is much grief: and he that increases knowledge increases sorrow."-Ecclesiastes 1:18
[SiegeLord's Abode][Codes]:[DAllegro5]:[RustAllegro]

Elias
Member #358
May 2000

SiegeLord said:

This new behavior is not an omission of a fix, but an introduced bug.

In current SVN if you keep any key pressed while switching out, it will be reported as stuck. With the new version, only if you keep one of the extended keys presse it will still be reported as stuck. So the new version does not completely fix the problem, but certainly not introduce a new one. (Note that al_get_keyboard_state is not modified except for the re-sync on switch-in.)

--
"Either help out or stop whining" - Evert

SiegeLord
Member #7,827
October 2006
avatar

Elias said:

With the new version, only if you keep one of the extended keys presse it will still be reported as stuck.

No. You press the numpad enter and the regular enter key gets stuck. In my example you never press the regular enter key, and yet it's reported as pressed.

"For in much wisdom is much grief: and he that increases knowledge increases sorrow."-Ecclesiastes 1:18
[SiegeLord's Abode][Codes]:[DAllegro5]:[RustAllegro]

Dennis
Member #1,090
July 2003
avatar

SiegeLord, you're right. It is an introduced bug due to the patch being incomplete and not fixing all the other issues yet.

Elias, I have attached a new patch[1] to this post which only attempts to read in the modifier flags and the toggle modifiers after a switch in, without attempting to get the states for all other keys. This fixes the wrong modifier flags in events.

torhu
Member #2,727
September 2002
avatar

Ok, now we have progress :)

Is the _al_win_wnd_call_proc stuff needed? I don't think so, but maybe you know something I don't. Removing it would simplify a lot.

But what is your actual goal here? It started out being only updating the modifier keys in the state returned by al_get_keyboard_state(). You wanted to get rid of stuck keys, I assumed. The solution is simple, just clear the state on switch in, call update_toggle_modifiers(). This fixes the actual problem: stuck keys. Nobody will know the difference, and the code is simple.

But now the char event modifiers field has entered the picture. The only reliable fix for that is to update the modifiers each time a char event is created in _al_win_kbd_handle_key_press(). GetKeyboardState is already being called there, since ToUnicode requires it.

If you want to fix both issues, just do both fixes.

There are some other things I should mention, but that can wait.

EDIT: Allright, I'm working on a patch...

UPDATE: Patch attached, please comment. Needs some more testing, and I can probably combine update_toggle_modifiers and update_modifiers into a single function, etc.

It does basically two things:

  1. On keyboard initialization and at switch in it clears the keyboard state, but updates the toggle keys so that they match the lights on your keyboard. No stuck keys, but no complete keyboard state update either.

  2. ALLEGRO_EVENT_KEY_CHAR's modifiers field is completely updated every time a char event is sent. This should be 100% reliable, but more testing still needed.

A question: the mouse grab key stuff. Is that still supported? Currently my patch breaks that.

UPDATE2: Just realized, one part of the patch is bogus :P

Dennis
Member #1,090
July 2003
avatar

I don't think we should patch anything other than the incorrect modifier flags in the events until there is consensus about the behavior options I described above in the second half of my longest post.

We should have consistency across different systems. How is switching out/switching in handled on other platforms with regard to the keyboard state?

(and by keyboard state I mean everything: key states, modifier states, toggle key states)

torhu
Member #2,727
September 2002
avatar

Yeah, does anyone feel like running ex_keyboard_events on Linux and OS X and tell us if you get key up events on switch in or switch out? Other platforms could be interesting too.

Elias
Member #358
May 2000

You only get keyboard events when you have focus in X11.

--
"Either help out or stop whining" - Evert

Dennis
Member #1,090
July 2003
avatar

Elias said:

You only get keyboard events when you have focus in X11.

Ok and what about the al_get_keyboard_state() function?
Does that still report keys as pressed which were held down while losing focus while you do not have focus?

torhu
Member #2,727
September 2002
avatar

Looks like nobody cares enough to bother testing. Attached is a simpler patch that just clears the state and the modifiers on switch in. And it doesn't break the mouse grab keys or anything else.

After the information that there are no "fake" events on Linux, this is how I see the relationship between state and events: If you use events for keyboard input in your game, you have to assume that anything can happen while your game does not have input focus, and you have to deal with that. But if you use al_get_keyboard_state, it's reasonable to expect an updated state each time you call the function.

With this patch, al_get_keyboard_state will much less frequently report keys that are up as being down. You can still trick it if you really want to. Anyway, I feel that reporting keys that are down as being up is less of a problem that the other way around, which is what we have now.

Discuss :P

Thomas Fjellstrom
Member #476
June 2000
avatar

Dude it's been two days.

That said I really don't care about the state api. IMO it's a crutch. To me, its like someone setting up a bitmap to do double buffering in A5 the way they did in A4.

torhu said:

But if you use al_get_keyboard_state, it's reasonable to expect an updated state each time you call the function.

Why is it reasonable to asume that the state api doesn't give you incorrect info if the events do?

--
Thomas Fjellstrom - [website] - [email] - [Allegro Wiki] - [Allegro TODO]
"If you can't think of a better solution, don't try to make a better solution." -- weapon_S
"The less evidence we have for what we believe is certain, the more violently we defend beliefs against those who don't agree" -- https://twitter.com/neiltyson/status/592870205409353730

torhu
Member #2,727
September 2002
avatar

torhu said:

But if you use al_get_keyboard_state, it's reasonable to expect an updated state each time you call the function.

Why is it reasonable to asume that the state api doesn't give you incorrect info if the events do?

Well, the docs for al_get_keyboard_state do say "at the time the function is called".

I used to use al_get_keyboard_state in one of my projects. But switched to maintaining my own state based on events, to avoid the occasional stuck keys on switch ins. The problem is you don't know if the state Allegro gives you might have been messed up by a switch out while keys were held down or not. And a stuck key is stuck until you happen to press it again, it doesn't go away by itself.

By the way, I had a look at the X and the OS X keyboard code, and I couldn't find any clearing of the state at switch in or switch out there. So now we know.

Dennis
Member #1,090
July 2003
avatar

torhu said:

By the way, I had a look at the X and the OS X keyboard code, and I couldn't find any clearing of the state at switch in or switch out there. So now we know.

Ok, so all we should do is fix the modifier flags for now without touching anything else. ( this patch does that: http://www.allegro.cc/files/attachment/606198 )
([edit] To clarify: If on other platforms the state is never cleared, neither should the Windows implementation do so. [/edit])

Quote:

Well, the docs for al_get_keyboard_state do say "at the time the function is called".

That's technically impossible in Windows if it is called while the window does not have input focus (But we already knew that, which is why we starting having this discussion about clearing it in the first place to prevent stuck keys.)

So... what are we to do:
A reasonable thing to do is, leave everything as is (except for patching the modifier problem) and add a user level function 'al_clear_keyboard_state' which users can call themselves to achieve any key state behavior they consider most suitable for their application.
If they don't call that function on switch-out, al_get_keyboard_state, will continue to report keys which were pressed while switching away (consistent with never receiving a key-up event).

Bruce Perry
Member #270
April 2000

Tomasu, keyboard state is a very useful thing - I don't think it's reasonable to view it as a crutch.

On the other hand, I would be in favour of being able to get the state at the time of the event most recently pulled from the queue, as opposed to the most up-to-date state at the time of the call. That way you could mix the two mechanisms perfectly. Especially if you're using timer events to drive your main loop.

[APPEND]

torhu said:

Attached is a simpler patch that just clears the state and the modifiers on switch in.

Why don't you clear them on switch-out?

If something steals focus while you're scrolling a map, and that map continues scrolling, that's extremely stressful. No one will ever want keys reported held while there's no focus. Holding a key down is an active behaviour. No one wants that if they're not looking at the window right now. Ever.

--
Bruce "entheh" Perry [ Web site | DUMB | Set Up Us The Bomb !!! | Balls ]
Programming should be fun. That's why I hate C and C++.
The brxybrytl has you.

Elias
Member #358
May 2000

On the other hand, I would be in favour of being able to get the state at the time of the event most recently pulled from the queue, as opposed to the most up-to-date state at the time of the call.

That's very easy to do already (it's exactly how my wrapper works). And it never was the idea of the state API. I'd completely agree to having it work like that (or dropping it completely, as I often said on the mailing list in the past) though :)

But the state API as it is now is for when you want imprecise input like in A4 or for the rare cases where the events API does not work - right at startup, and after a switch-in - since you could not have got a key down event for keys pressed while the application was not running. (It seems this very use-case is broken in the Windows implementation though.)

--
"Either help out or stop whining" - Evert

torhu
Member #2,727
September 2002
avatar

Why don't you clear them on switch-out?

Yes, that's probably better.

Dennis said:

Ok, so all we should do is fix the modifier flags for now without touching anything else. ( this patch does that: http://www.allegro.cc/files/attachment/606198 )

Well, I guess it's better than nothing. I still think just clearing them on switch in would fix the problem, though.

I have some comments on the patch in any case:

  1. Is the _al_win_wnd_call_proc stuff needed? I don't think so, but maybe you know something I don't. What does it achieve?

  2. If you don't use _al_win_wnd_call_proc, there's no point in finding the foreground window etc.

  3. You could just call GetKeyState for the keys you care about, instead of GetKeyboardState. No big deal, though.

  4. init_keyboard() doesn't update the modifiers, should it?

  5. The call to update_toggle_modifiers is not needed, as it's called in _al_win_kbd_handle_key_press every time a char event is created.

  6. You forgot VK_APPS (ALLEGRO_KEY_MENU)

Dennis
Member #1,090
July 2003
avatar

You're right. I left most of the stuff in in preparation for a future attempt at fixing the other problems but since that's not possible, none of that is needed anymore and just clearing the modifier state is sufficient.

Have a look at this minimal modifier state patch then [1].

torhu
Member #2,727
September 2002
avatar

The patch looks fine as far as I'm concerned.

al_install_keyboard() is supposed to support being called multiple times, and I assume it's supposed to be threadsafe, but that's for another day :P

Dennis
Member #1,090
July 2003
avatar

torhu said:

The patch looks fine as far as I'm concerned.

I have submitted that patch to the mailing list (in GIT format for the current 5.1 branch from the recently set up new GIT repository).

 1   2 


Go to: