The new keyboard hook lets keypresses go through #294
Comments
|
Actually the problem seems to be mostly related to mode changes, at least that's how I can most easily reproduce. So it seems the remapping is quite slow. But it probably also happens when doing more complex window manipulation, so I don't think it's restricted to that. I was actually hoping that the keyboard hooks would solve the slow remapping, as I could not use the modes like I wanted to before either, it was simply to slow to re-map, so following keys got ignored. With the support of the QMK firmware for the keyboard, my goal is to use just a single stolen hotkey, which then enables different modes on following keypresses, and automatically sends esc to exit the modes when I release that key. The chords feature request might also be applicable for that behaviour, but using manual modes would be even more flexible, for example I could keep holding down my trigger key, while pressing multiple navigation keys. |
My first idea would be to see why the check even takes more than 100ms. If you wrap everything inside this while loop with our time!("kb event loop", { ... })you can see how long each iteration in the event loop takes. Mine take 5 - 10 µs |
|
I didn't add the timing but I have a log with reasonable timestamps and output to see the problem. It actually goes out of sync at the very start, when entering work mode. It is possible that some of my display changes makes that part slower. The first two key ups (the For example the event received by the hook at 16:39:32.633359, is processed at 16:39:33.045372, between that is So my bet is that either of those blocks for too long. Note that I currently have a timeout set to 200ms, so that's why you see around that time until I print the timeout message. Also note that the channel is at the wrong state at that point, so it's hard to tell if something else takes too long. But yes generally at least the timing between the KeyDown or KeyUp in the hook and the sending to the channel in the |
|
I added some quick timings to the I don't have time to look at it further, but there might also be some semi deadlocking going on. Since when I increase the |
|
I was supposed to do other things, but I could not stop myself, since at least a temporary solution came to my mind. It's clear that it was the But there was still a problem after that, the UpdateKeybindings event can be sent without keypresses, so they can queue up a bit, which made the keybindings not update until you had pressed a certain amount of time. So I just used a loop instead of if, and it seems to be working now. I will run with this for a while now and see if it's really fixed, and if it is, send a pull request at some point. |
Version (which release did you use?)
The latest git version
Describe the bug
The new keyboard hook functionality does not always work correctly, and some of the keypresses gets sent to the application in additon to Nog handling it. In fact for me this is so common that it's almost unusable at the moment.
To Reproduce
The easiest way to produce it is probably to have a workspace with text editors open, and switch the focus between them. But I'm not sure if it's reproducable on all systems.
Expected behavior
All handled keypresses should be swallowed.
More details
I think I know what's going on, since I debugged this a bit. When a key is pressed, a message is sent to another thread through a channel, and once it's handled there, another message is sent back indicate that the key was handled. The main hook thread waits for 100ms to get this message back, and if it doesn't it lets the key press through to other applications. So that's one press getting through even if it's not handled. But what's worse is that the channel is now out of sync (there's more than one message in the queue), so it will start to block KeyUp events instead of KeyDown, which makes things worse.
This 100ms is warranted, as Windows requires the keyboard hooks to not take too long, so that can't be changed. One way to mitigate the issue would be to use a higher thread priority for the processing thread, but that's still quite fragile, it's quite hard to guarantee these timing requirements in all cases.
So instead I recommend, that the actual keymappig is checked inside the hook function, to avoid having to rely on context switches being fast enough. Another improvement for that would be to make sure that no thread locking happens there, for example by making the keymappings buffered so that atomic swaps are used to bring in the new mappings.
This still has a small problem that I don't know how to solve easily, if the mapping is changed by a mode switch, and a new keypress happens immediately after, the re-mapping might not be processed fast enough. But that's probably a very small problem. I can think of a couple of wayst to solve it, but neither of them is easy or desirable.
One way is to have a completely static mapping, that is the mapping of all modes and the mode switches are know in advance. Then the switching would be easily done quickly in the hook itself.
Another way would be to somehow be able to call lua in a way that only allows remapping calls, which probably also would be quick enough.
A third way, very similar to the second one is to queue all real actions, like focusing windows, switching workspaces and so on, but still call lua for every keypress. Lua is quite fast by itself so I think it should be fast enough, and if not, luajit would definitely be able to do it. So that would IMO be the eventual real fix for the issue. This way the lua script could probably be called from the hook function itself on every keypress. And then there's a separate processor that makes all the magic happen.
Note that the issue is probably much worse than it should be right now, at the moment Nog seems to use 100% cpu of one core at all times, which probably delays some actions for more than 100ms. I haven't tracked down what's causing that.
Also note that it would probably be worth blocking key up events as well. They could be done by for example storing handled keys (without modifiers), in a HashSet, and then check that and remove the key on key up.
The text was updated successfully, but these errors were encountered: