Implement uinput support for Wayland#129
Conversation
|
From a first glance, that looks good. I'll take a closer look soon and get back at you. I'm not sure I like the idea to add a config option for selecting the kbemu mechanism. I haven't looked into it yet, but I gather that uinput will work not just for wayland, but for X11 and even when there is no window system running at all. If that's true, then we might just always use uinput when compiled with it enabled. |
|
Oh, that makes so much more sense, I don't know why I thought uinput was Wayland specific, serves me right for referencing forums over the docs. Going back over my code from that POV, I think I ended up arriving at what is a universal implementation, I had just assumed it was Wayland specific. I will review in depth later today and submit an updated commit with at least comments updated to reflect this. In regards to the kbemu config option, do you think it serves some utility, at least for testing/fallback? There may be cases where the uinput module is loaded, but something isn't playing nice and the user wants to fall back to the existing X11 implementation. Let me know your thoughts, I don't feel super strongly either way. |
|
You're right, I was mostly thinking of the fact that all configuration options now go through the protocol so that clients (like spnavcfg) can tweak them. So adding a new config option means adding a protocol command, and it's got to live forever then, for backwards compatibility. But now that I think about it, it doesn't have to be an option meant for end-users to regularly tweak. There can be a speratation betwean real "options", and sortof "debug options", that need not become first-class citizens immediately. We can have the option there for people to try if they encounter problems, exposed only through manual editing of the config file, and we'll see if it ends up useful enough to eventually promote... With that in mind, I wouldn't go all the way to a "use this, use that, or auto-detect" config option. I think as a debug fallback, it makes more sense to just have an option "force kbemu to use X11". For non-uinput builds it won't have an effect, for uinput builds we'll default to uinput unless this force flag is set, in which case we'll use the previous X11 mechanisms. How does that sound like? I think I'll also add it in a clearly separated section of the example config file, with a heading like "debug options: if you end up having to change any of these, please open an issue and let us know what went wrong without it" or something. |
|
I realize that "kbemu" is a purely internal name for this in the source code, so probably the best name for the option I was talking about is something like "kbmap_use_x11" instead. |
|
I think that makes a lot of sense, I have implemented those changes. |
|
Yeap that looks good. I'll do a merge in a local branch to take a careful look at the whole thing, maybe change a couple of small things here and there, and I'll go ahead and merge it soon. |
|
I merged manually, because I ended up refactoring this a lot. Commits:
I tested with uinput enabled and disabled, and I'm getting key events both ways, so I assume I didn't break anything. If you have the time to test it again and see if everything works, please let me know. I wanted to simplify the kbemu backends mechanism. Mostly every function except init/cleanup is now static, and registered to global function pointers by each backend. X11 init runs first and sets its callbacks; uinput, if available and not disabled runs last and overrides it. Ifdefs in kbemu.c are greatly reduced, and there's no need for keeping track of a backend variable. The only soup in the ointment is that the X11 keysym functions are needed by the config parser which runs earlier, so I had to make those extern and statically initialize the function pointers. The case of uinput, with X11 entirely excluded from the build seems broken, because the config parser won't be able to do anything with the keysym names in the config file, but I think that was broken in your code too, so I just maked it as TODO, to eventually get our own key name parser and stop depending on X11 for that. I'm closing this since I already merged it. Feel free to open further pull requests if you want to add/improve/fix something. Or just add comments here if you want to discuss it. |
This adds keyboard emulation for Wayland outside of X11 applications using uinput (relates to #91).
Ideally this should be detecting the display server and selecting the uinput backend on Wayland, or falling back to the existing method for X11, but that's not working properly. You have had to add the new parameter
kbemu-backend = uinputto spnavcfg to get the Wayland backend to be selected.I'd love feedback on keymap.c, I'm not 100% sure it was the right approach. Right now it's using a hardcoded lookup table to map X11 KeySym values to Linux keycodes. It covers the common qwerty keys but might need to be expanded or replaced with a more dynamic solution if this feels too in-elegant or restrictive for other keycodes.
This isn't perfect, but definitely functional, if this should have been marked a draft pull request or if the display server detection code should have been held back at this stage, please let me know and I will take that path in the future.