libretro / RetroArch

Cross-platform, sophisticated frontend for the libretro API. Licensed GPLv3.
http://www.libretro.com
GNU General Public License v3.0
10.19k stars 1.82k forks source link

[Linux] [Regression] Keyboard keycodes no longer take into account keyboard layout #8303

Open Zlika opened 5 years ago

Zlika commented 5 years ago

Hi!

Since RA 1.7.6, the keycodes from libretro's keyboard callback no longer take into account the current keyboard layout.

Contrary to scancodes, keycodes should take into account the configuration of the keyboard. This problem seems similar to https://github.com/libretro/RetroArch/issues/6971, except that the current issue is now also present on Linux and is a regression compared to previous RA versions.

orbea commented 5 years ago

Can you please find the first bad commit?

Zlika commented 5 years ago

Hi @orbea Here is the culprit:

347519a4d8f3befb719f11eeb0121c736ce64dee is the first bad commit
commit 347519a4d8f3befb719f11eeb0121c736ce64dee
Author: David Skywalker <dantoine@gmail.com>
Date:   Thu Jan 31 12:50:58 2019 +0100

    now X11 driver using keycodes instead keysyms, fixes international layouts problems.

:040000 040000 ad4fc57f0c84abc7ef94c6fcfa56002a663840ea 14d0582d085ba91aa6d2db1663ac6588e5d417e3 M  gfx
:040000 040000 4a0a5bb7c084fdb4242a536f0b7a1d11a60338bc b7d9c9012fa89e27303b39e892be092df06b5709 M  input

Well, I disagree with the commit message... :-) cc @DSkywalk

orbea commented 5 years ago

Thanks, can you spell out exactly how to reproduce this?

Also here is a clickable link to the commit. 347519a4d8f3befb719f11eeb0121c736ce64dee

Zlika commented 5 years ago

Download and run the "Thomson MO/TO - Theodore" core and run it (you can run it without any content loaded), type "B" to run the BASIC and then type "AZERTY" with an AZERTY keyboard, it will print "QWERTY". With RA 1.7.5 it would print "AZERTY". You can also do a similar test with another core emulating a computer like the Amstrad CPC, but these cores generally requires a content and/or a bios. I tested with CrocoDS and the same problem is present.

DSkywalk commented 5 years ago

Hi @Zlika, first my english is terrible, but i try :)

With RA 1.7.5 (and below), typing the 'A' key on an AZERTY keyboard generates an event with keycode=97 (A).

Please do not confuse a keysyms with a keycode. I try to explain.

imagen If you press this key the keycode 24 is allways the same on any language, but the keysym it changes with different layouts: english 0x71, french 0x61. Is like my XBOX pad the button 1 is labelled with a B on my joystick, but is A if I plug my SNES pad.

All computer emulators (I know at least) works using keycodes because two reasons:

The problem need to be solved on Thomson-CORE, not on RA. This core need to develop a layout function (like it originally existed) to select english/french/... layouts (or auto-loads). Please check dosbox emulator, when emulator is initted, it loads your layout and you get the correct key showed on emulation, isn't it?

imagen

This will fixed on cap32 in a few days too (i'm working on it)

imagen vanilla caprice emulator as implementation example

Finally, I do not recommend it but if a developer wants to ignore the keycode (for any reason), still could use the charcode send it by RA. Check libretro.h

void keyboard_cb(bool down, unsigned keycode, uint32_t character, uint16_t mod)
{

   printf( "Down: %s, Code: %d, Char: %u, Mod: %u.\n",
          down ? "yes" : "no", keycode, character, mod);

   // your code here ...

}

// init callback ...
 struct retro_keyboard_callback cb = { keyboard_cb };
 environ_cb(RETRO_ENVIRONMENT_SET_KEYBOARD_CALLBACK, &cb);

Computer emulators are a complex piece of software. :man_astronaut:

orbea commented 5 years ago

@DSkywalk I don't fully understand why it was working for @Zlika before your change? Can you help explain? :)

DSkywalk commented 5 years ago

bfffff, I try!! ;)

Because the old code send it the CHARACTER/SYM from your keyboard and then generates the retrokey-event. And when @Zlika press "A" we don't know what key is pressed just the symcode of his layout. And this layout changes...

imagen

imagen

imagen

imagen

Check positions of the "A" key , "+", я, acutes, "[", etc... :name_badge:

So issue #6971 is not a RA bug, is just a core bug :+1:

When French keyboard press: imagen Generates keycode 24 => french_layout[24] => keysym 0x61 => RA it sends 0x61 (key) + "A" (character)

When is pressed on English keyboard: Generates keycode 24 => english_layout[24] => keysym 0x71 => RA it sends 0x71 (key) + "Q" (character).

After https://github.com/libretro/RetroArch/commit/347519a4d8f3befb719f11eeb0121c736ce64dee

In a French keyboard generates: Generates keycode 24 => RA it sends 24 (keycode) + "A" (character using his keysym 0x61)

And in an English keyboard: Generates keycode 24=> RA it sends 24 (keycode) + "Q" (character using his keysym 0x71)

Like the windows behaivor :+1:

Hope this helps :*

orbea commented 5 years ago

I think I understand better now where before it didn't know what key code it was using and got the right key in some cases and now its knows what key is being pressed, but the core is missing features for different keyboard layouts?

Thanks for taking the time to explain in more depth!

DSkywalk commented 5 years ago

I think I understand better now where before it didn't know what key code it was using and got the right key in some cases and now its knows what key is being pressed, but the core is missing features for different keyboard layouts?

Yep, and totally unplayable on certain territories with bizarre layouts (see my previous comment), because RETRO_x do not have any symbol in his table. It just a keycode list from a base PC/Keyboard.

Thanks for taking the time to explain in more depth!

You are welcome :+1:

Zlika commented 5 years ago

Hi @DSkywalk Thanks for your explanations, but I don't think I agree with everything. As far as I know, RetroArch uses SDL under the hood. If you take a look at https://wiki.libsdl.org/SDL_Keysym , you see that a keysym is a structure with both a scancode (position of the key on the keyboard, not depending on the layout) and a keycode (the actual letter pressed, which depends on the keyboard layout). The libretro API specifies:

/* Callback type passed in RETRO_ENVIRONMENT_SET_KEYBOARD_CALLBACK.
 * Called by the frontend in response to keyboard events.
 * down is set if the key is being pressed, or false if it is being released.
 * keycode is the RETROK value of the char.
 * character is the text character of the pressed key. (UTF-32).
 * key_modifiers is a set of RETROKMOD values or'ed together.
 *
 * The pressed/keycode state can be indepedent of the character.
 * It is also possible that multiple characters are generated from a
 * single keypress.
 * Keycode events should be treated separately from character events.
 * However, when possible, the frontend should try to synchronize these.
 * If only a character is posted, keycode should be RETROK_UNKNOWN.
 *
 * Similarily if only a keycode event is generated with no corresponding
 * character, character should be 0.
 */
typedef void (RETRO_CALLCONV *retro_keyboard_event_t)(bool down, unsigned keycode,
uint32_t character, uint16_t key_modifiers);

and RETROK here means keysym:

enum retro_key
{
RETROK_UNKNOWN = 0,
...

As far as I understand these comments, the change you made is an incompatible API change, replacing the keycode (real letter pressed) by the scancode (key position pressed). This must have broken every emulator using the keyboard.

Zlika commented 5 years ago

Also from https://wiki.libsdl.org/SDL_Keycode:

Values of this type (also known as keycodes or keysyms) are mapped to the current layout of the keyboard and correlate to an SDL_Scancode. The scancode identifies the location of a key press and the corresponding SDL_Keycode gives that key press meaning in the context of the current keyboard layout. 

Then, at least from a SDL point of view, your explanation keycode = physical location of a key seems to be wrong.

DSkywalk commented 5 years ago

Nope, sorry @Zlika you are wrong. Retroarch do not use SDL under the hood, is a just another driver that you could use (with some issues #8042 too) you are using X11 driver :)

And is broken as least, as is broken in windows ;)

DSkywalk commented 5 years ago

Then, at least from a SDL point of view, your explanation keycode = physical location of a key seems to be wrong.

Ok, but we talk about X11 driver, and is called keycode... imagen

I used SDL some years ago and I don't know if they call it scancode or keycode, my fault :+1:

Do you have tested dosbox emulator on RA1.7.5 and on RA1.7.6?

Zlika commented 5 years ago

Thanks for this explanations. I think the comments in the libretro API should be updated to explain what is called a keycode, or even use a more platform agnostic word like scancode. Because for people coming from a SDL background, or simply people googling "keycode" and reading the SDL documentation, keycode = keysym and keycode != scancode.

And most of all, this behavior must be consistent between drivers/platforms, because if it is not the case cores cannot do anything to fix that on their side.

Also, I think it would be interesting if libretro could manage keyboard layouts instead of requiring every core to do it on their side.

orbea commented 5 years ago

Also, I think it would be interesting if libretro could manage keyboard layouts instead of requiring every core to do it on their side.

I am also wondering if that is possible in the frontend, it would reduce lot of duplication of work.

andres-asm commented 5 years ago

Well... I was thinking the same but after chatting to @DSkywalk I think otherwise. Forwarding keysym to the core is a bit pointless, the emulated system may not even support localization.

So forwarding the symbol would be largely pointless.

DSkywalk commented 5 years ago

@orbea I'm sorry to say it's a bad idea. Let's suppose we have implemented the necessary utf-12 opcodes in RA so that for when a Russian presses the [я] key and arrives to the core of CPC, what should the core do with that character that does not exist in CPC? Ignore It?

imagen How many keys could use a russian user? four/five keys?

Because a CPC cannot draw a RUSSIAN character on the screen and that's correct. (Unless the BIOS of the computer had support for that language, those characters can never be seen in the emulation).

A computer converts (from its BIOS) the keycodes to on-screen graphics. It is something that the computer does and therefore it is the responsibility of the core to implement it correctly.

What, maybe, could be added to the API is to have the user language in an environment variable: "en", "fr", "es", ... and thus be able to detect or load BIOS of its languages (by default) :+1:

Zlika commented 5 years ago

@DSkywalk I understand you point. Now my concern is that comments in libretro-common/libretro.h are either wrong or misleading:

/* Keysyms used for ID in input state callback when polling RETRO_KEYBOARD. */
enum retro_key
{
   RETROK_UNKNOWN        = 0,
   RETROK_FIRST          = 0,
   RETROK_BACKSPACE = 8,
(...)

/* Callback type passed in RETRO_ENVIRONMENT_SET_KEYBOARD_CALLBACK.
 * Called by the frontend in response to keyboard events.
 * down is set if the key is being pressed, or false if it is being released.
 * keycode is the RETROK value of the char.
 * character is the text character of the pressed key. (UTF-32).
 * key_modifiers is a set of RETROKMOD values or'ed together.
 *
 * The pressed/keycode state can be indepedent of the character.
 * It is also possible that multiple characters are generated from a
 * single keypress.
 * Keycode events should be treated separately from character events.
 * However, when possible, the frontend should try to synchronize these.
 * If only a character is posted, keycode should be RETROK_UNKNOWN.
 *
 * Similarily if only a keycode event is generated with no corresponding
 * character, character should be 0.
 */
typedef void (RETRO_CALLCONV *retro_keyboard_event_t)(bool down, unsigned keycode,
uint32_t character, uint16_t key_modifiers);

The comments indicate that the keycode parameter is a RETROK value, and the documentation of the retro_key enum says that their are keysyms. It would be great if someone could propose a PR to clarify all this.

orbea commented 5 years ago

What, maybe, could be added to the API is to have the user language in an environment variable: "en", "fr", "es", ... and thus be able to detect or load BIOS of its languages (by default)

Yes, something like that is what I was thinking. The support could be added to the frontend, but it would only used by cores that supported the feature and requested specific keymaps. If the core doesn't use Russian then it would not be available.

DSkywalk commented 5 years ago

Please @Zlika could test, the last cap32 core? imagen

Really thanks for your help :+1:

Zlika commented 5 years ago

This version is not available for download at this time. I will test it later.

Zlika commented 5 years ago

Hi @DSkywalk . I tested the latest version of cap32 and I can confirm that the french keyboard is working correctly.

DSkywalk commented 5 years ago

@fr500 @orbea I think this issue is solved and it could be closed :+1:

Zlika commented 5 years ago

Before closing it, I think someone with good knowledge on this keyboard/layout subject should update the comments in libretro-common/libretro.h that are wrong and misleading as I pointed out in a previous comment.

DigitalF3 commented 5 years ago

Hi, I hope it's alright to comment on here, as I'm running into the same problem, but with Retroarch 1.7.8 itself (udev for input), I couldn't set the volume with + / -, on the Spanish keyboard layout. Remapping the hotkeys, it maps to right bracket and slash, respectively. Works fine though, and I agree the correct behavior is how DOSBox does it (try sopwith2 with a Spanish keyboard layout ;) OS - Fedora 30

keithbowes commented 3 years ago

When I started using RetroArch, I noticed that it used logical keys in X but physical keys in KMS and Wayland, which was a bit annoying. I assumed that changing it to always use physical keys was intentional, for consistency between the various input drivers/video contexts. I'd prefer it to use logical keys whenever possible, though; perhaps there should be a setting to allow users to specify which they prefer (perhaps with a note that it doesn't work with all input drivers).

LibretroAdmin commented 2 years ago

Where do we stand with this now?

MeisterLLD commented 1 year ago

Could it be related to https://github.com/libretro/RetroArch/issues/13838 ?