SDL2 wayland SDL_KEYUP/SDL_KEYDOWN input mapping code possibly confused by changing key maps [major issue on Librem 5/PinePhone]

[ell1e]

When using SDL2 with SDL_VIDEODRIVER=wayland with the common Linux phone desktop Phosh with it's common on-screen keyboard squeekboard, all the SDL_KEYUP/SDL_KEYDOWN events will have very wrong values for what keys the event supposedly maps to.

It was suggested on the Squeekboard issue tracker that this might actually be either a bug, or shortcoming on the SDL2 side, since squeekboard basically switches key maps out a lot which may not be something SDL2's code is tested to handle: https://source.puri.sm/Librem5/squeekboard/-/issues/218

Would it be possible to maybe look into this? Is this a scenario that is at least supposed to work, or is SDL2 currently not equipped at all to deal with live switching keymaps under Wayland on a conceptual level?

Steps to reproduce:

1. Compile this as ./testapp:

#include <SDL2/SDL.h>
#include <signal.h>

volatile _Atomic int sigterm_received = 0;
volatile _Atomic int sigint_received = 0;

void handle_sigterm(int signum) {
    sigterm_received = 1;
}

void handle_sigint(int signum) {
    sigint_received = 1;
}

int main(int argc, const char **argv) {
    signal(SIGTERM, handle_sigterm);
    signal(SIGINT, handle_sigint);

    printf("testapp: verbose: running SDL_Init()\n");
    if (SDL_Init(SDL_INIT_VIDEO|SDL_INIT_EVENTS|SDL_INIT_TIMER) < 0) {
        fprintf(stderr, "testapp: error: SDL_Init() failed\n");
        return 1;
    }
    printf("testapp: verbose: opening window\n");
    SDL_Window *window = SDL_CreateWindow(
        "testapp", SDL_WINDOWPOS_CENTERED, SDL_WINDOWPOS_CENTERED,
        512, 512, SDL_WINDOW_RESIZABLE | SDL_WINDOW_ALLOW_HIGHDPI |
        SDL_WINDOW_OPENGL
    );
    if (!window) {
        fprintf(stderr, "testapp: error: SDL_CreateWindow() failed\n");
        return 1;
    }
    printf("testapp: verbose: entering event loop\n");

    int windowclosed = 0;
    while (!sigterm_received && !sigint_received && !windowclosed) {
        SDL_Event e;
        while (SDL_PollEvent(&e) > 0) {
            switch (e.type) {
                case SDL_QUIT: {
                    windowclosed = 1;
                    break;
                }
                case SDL_WINDOWEVENT: {
                    if (e.window.event ==
                            SDL_WINDOWEVENT_CLOSE) {
                        windowclosed = 1;
                        break;
                    }
                    break;
                }
                case SDL_KEYDOWN: {
                    printf(
                        "testapp: verbose: "
                        "SDL_KEYDOWN: keysym.sym=%d\n",
                        (int)e.key.keysym.sym
                    );
                    break;
                }
            }
        }
        SDL_Surface *srf = SDL_GetWindowSurface(window);
        SDL_FillRect(
            srf, NULL, SDL_MapRGB(srf->format, 255, 200, 220)
        );
        SDL_UpdateWindowSurface(window);
        SDL_Delay(100);
    }
    SDL_DestroyWindow(window);
    SDL_Quit();
    return 0;
}

2. Launch a desktop using Phosh - I tested this on a PinePhone with the mobian distribution, although some desktop distributions like Fedora I think also ship Phosh for x64 machines where this should at least in theory also reproduce

3. When the desktop is running, launch the application like this: SDL_VIDEODRIVER=wayland ./testapp

4. Press C on squeekboard (Phosh's default visual keyboard)

Expected result:

$ SDL_VIDEODRIVER=wayland ./testapp
testapp: verbose: running SDL_Init()
testapp: verbose: opening window
testapp: verbose: entering event loop
testapp: verbose: SDL_KEYDOWN: keysym.sym=99

Actual result:

$ SDL_VIDEODRIVER=wayland ./testapp
testapp: verbose: running SDL_Init()
testapp: verbose: opening window
testapp: verbose: entering event loop
testapp: verbose: SDL_KEYDOWN: keysym.sym=1073742106

[ell1e]

For what it's worth this bug/limitation/whatever makes SDL2 apps completely unusable on Linux phones since if using X11/XWayland/SDL_VIDEODRIVER=x11 there is another bug that breaks which=SDL_TOUCH_MOUSEID/some SDL_FINGERUP events (which is probably not worth fixing since all Linux phone desktops are Wayland-only anyway, so there is no point in investing into a blurry inferior X11/XWayland mode with libdecoration support on the way). But this means this issue here is a major blocker for Linux phone adoption, way more than e.g. virtual keyboard toggle support on Wayland in my opinion. (Since e.g. the Phosh mobile desktop has a manual toggle for that as a workaround.)

[Cacodemon345]

The SDL_KEYMAPCHANGED event isn't sent at all on Wayland and there seems to be no mechanism to detect keymap changes there which would explain this bug.

[Cacodemon345]

Actually, I was wrong; the Wayland backend does have the mechanism to detect keymap changes but it doesn't seem to be made to handle inputs from multiple keyboards.

[ell1e]

I could be very wrong on this, but from my very limited understanding of the squeekboard discussions I don't think multiple keyboard support should be required. It seems to basically compose a fully artificial keymap on-the-fly with all the needed keys. But it seems like something goes wrong when the keymap change happens under wayland. (With SDL_VIDEODRIVER=x11 everything related to key input works.) SDL_TEXTINPUT also works, so it must be something specific to the key sym translation under Wayland that gets confused.

[Cacodemon345]

Taking a look at XWayland's source code might be of help then.

[flibitijibibo]

This may be a duplicate of #3471 - though I noticed that the latest revision does have the keymap handler...

https://github.com/libsdl-org/SDL/blob/main/src/video/wayland/SDL_waylandevents.c#L645

... we don't ever queue the event to update the keymap. This is probably what the issue is. X11 for reference:

https://github.com/libsdl-org/SDL/blob/main/src/video/x11/SDL_x11events.c#L783

[ell1e]

Would be cool if this could be looked into. Wayland is really way beyond my skillset, but this would help a ton with SDL2's current borderline unusable state on Linux phones

[flibitijibibo]

Tried to get some more details on how the callbacks work... I think there might be a pretty big feature gap somewhere, and it's not just Wayland (as #4187 points out).

In short: There is a keymap update, but it only gets fired when you change your global keyboard settings in your desktop's settings menu. It does NOT fire when you change layout via a shortcut (i.e. the button in your taskbar). As far as I can tell, the behavior is identical between X and Wayland, so unfortunately this is NOTABUG in the sense that Wayland and X are the same, but it's still a blatant issue and I'm not actually sure that it's something we can fix on our end without an updated protocol (X is probably screwed at this point).

The good news is that text events do update layout properly, and that's probably why most applications other than games appear to work just fine - it just happens to be that games care about the keysym, which we don't get any updates for.

I did find one place where X and Wayland were different, and that was the lack of an UpdateKeymap routine. I have this set up here...

flibitijibibo@ec6ad51

... but as you might expect, this doesn't work (if anything it just breaks the stock layout). I swear xkbcommon has a real way to map scancodes to keycodes in a way that lets us just call the equivalent of "GetKeyFromScancode" but every path I take just returns exactly what I pass no matter what my layout is. This is where someone who actually understands xkbcommon will have to take over; I at least set up the SDL side but this loop isn't cooperating with me!

[flibitijibibo]

Today was an errand day for me so I wasn't about to get any real work done... which means running head-first into the xkbcommon wall at full speed was on my list!

I was finally able to dig up the APIs that should let us get the layout properly. Here's the latest version of my attempt at fixing this: flibitijibibo@36f73f6

Two items of concern:

• I discovered that xkb_layout_index_t is the system that lets you get the layout based on the current settings, and I can now get proper keysyms out of the layout! Problem is, I can't seem to get it to give me the correct index. I believe it has to do with the XKB_STATE_LAYOUT enum: flibitijibibo@36f73f6#diff-8a9cade1c69cd3c5c95378f2b386222c68aa893ea73ae792bb865061b8b9ebf2R688
• The xkb_keysym_t needs to be translated to SDL scancode/keycode values. For now I'm just printing the sym name: flibitijibibo@36f73f6#diff-8a9cade1c69cd3c5c95378f2b386222c68aa893ea73ae792bb865061b8b9ebf2R636

Any further insight on this new stuff is appreciated!

[flibitijibibo]

Oh my gracious I figured it out:

flibitijibibo@410b6cb

What I was looking for was groups - probably obvious if, unlike me, you are familiar with X keyboard support, but xkbcommon decided to rename it to layout_index_t and I couldn't quite parse what happened until I saw it in some X documentation and eventually tripped over it in waylandevents.c by accident.

With this commit, live layout changes now actually work! The last item is making the big ugly dictionary to translate xkb_keysym_t to SDL_Keycode, which can get assigned to the keymap array here:

flibitijibibo@410b6cb#diff-8a9cade1c69cd3c5c95378f2b386222c68aa893ea73ae792bb865061b8b9ebf2R786-R788

[flibitijibibo]

#4303 is live and ready to test.

[ghost]

Hi, @flibitijibibo I've tested this patch, here is my feedback:

• all alphanumeric keys work on the virtual keyboard
• some symbols work on the virtual keyboard, many do nothing
• all numbers work with a physical keyboard
• letters are still messed up on physical keyboards
  here is a video demonstration

[ghost]

I'm not very familiar with this codebase, but if you have a systematic way for me to test out the keycodes or dump debug information for you, then I'd be willing to try that and get back to you as I have a device availiable.
But in any case, I think this issue should be re-opened because some use-cases, for example anbox, require keys like ESC to work.

[flibitijibibo]

These are the parts you'll care about:

https://github.com/libsdl-org/SDL/blob/main/src/video/wayland/SDL_waylandevents.c#L646

https://github.com/libsdl-org/SDL/blob/main/src/video/wayland/SDL_waylandevents.c#L799

https://github.com/libsdl-org/SDL/blob/main/src/video/wayland/SDL_waylandevents.c#L843