Dev docs for Keyboard Manager

[arjunbalgovind]

Summary of the Pull Request

What is this about?
This PR adds dev docs for KBM.
The docs can be viewed in the rendered state at https://github.com/arjunbalgovind/PowerToys/tree/user/arbalgov/kbmDevDocs/doc/devdocs/modules/keyboardmanager
Navigate using the links from the Readme.

PR Checklist

• Applies to Keyboard Manager Dev Docs #7158
• CLA signed. If not, go over here and sign the CLA

Info on Pull Request

What does this include?

Validation Steps Performed

How does someone test & validate?
Validate that the links to different sections work as expected.

[mykhailopylyp]

@arjunbalgovind
The following question is related to KBM in overall.
Are we aiming to minimize the number of sent keys(with SendInput) from KBM?
It seems possible to simplify shortcut remap logic a bit sacrificing with sending extra key events to the system.
For example, we can keep the state of physically pressed keys and keys sent by KBM(so we can send the corresponding key up events later). The logic should look something like:

• On key event update the state
• If shortcut remap is active then propagate active remapping
• If we have applied shortcut remap but origin keys have changed then we have to send key up events for the target shortcut
• Check for possible shortcut remap and apply it.
  I hope the idea is clear. What do you think?

[arjunbalgovind]

@arjunbalgovind
The following question is related to KBM in overall.
Are we aiming to minimize the number of sent keys(with SendInput) from KBM?
It seems possible to simplify shortcut remap logic a bit sacrificing with sending extra key events to the system.

It is ideal to minimize the number of sent keys for performance, because ensuring that key remaps are responsive is very important. Since most keyboard have input latency in the order of several milliseconds we have to make sure that any latency added by us is small enough that users will not feel it compared to typing without remaps. Even if performance is not a concern, I have seen cases where minimizing the key events fixes certain OS scenarios, such as remaps to Win key not opening start menu (#7171 recently fixed remapping shortcuts to Win key remaps. Even before the PR the logic made sense that when you release the shortcut it should just reset the state but for some reason it wouldn't open start menu properly (it would only appear some times).

For example, we can keep the state of physically pressed keys and keys sent by KBM(so we can send the corresponding key up events later). The logic should look something like:

I'm not sure I'm understanding this correctly, but if by keeping the state of physically pressed keys you mean having a buffer of size 256 and updating each key (like we do in tests), there can be some issues with that if the user is not running as admin. This is somewhat what happened in ColorPicker and PowerToys Run since the old logic for those two keyboard hooks was storing key state. It can lead to false positives because an app running as admin may be in the foreground in between, so any key events there would not be updated in our local buffers. Normally this is not an issue (and it is sufficient to tell users that for KBM to work with elevated windows, KBM needs to be run as admin), but it can lead to cases where user holds Alt, presses an admin window and releases Alt while that is in focus. Now until a user presses Alt and releases Alt again on a non-admin window, KBM will think that Alt is pressed as per its local buffer.
If KBM is forced to always run as admin, or as a driver (an option that @enricogior and @yuyoyuppe were suggesting), then it should generally be safe to store key states like this without side effects. There are still some cases where it may not work though if another app uses low level hooks. Here is an example:

• KBM is running as driver and remaps Alt+A->Ctrl+C (benefit of running as driver is that it receives key event before everything else)
• AutoHotKey script running normally remapping Alt->Win
• When user presses Alt (keydown), KBM would just store the key state but not suppress the event. When AHK gets it, it will remap Alt to Win, so the OS sees Win(key down).
• User presses A. Since we store keys states we detect Alt+A and we should send Ctrl+C for the remap. At this point however, the OS sees Win being pressed (which is what we would get from GetAsyncKeyState). I'm not really sure how we would handle such scenarios.

• On key event update the state
• If shortcut remap is active then propagate active remapping
• If we have applied shortcut remap but origin keys have changed then we have to send key up events for the target shortcut
• Check for possible shortcut remap and apply it.
  I hope the idea is clear. What do you think?

This logic seems straightforward and makes sense (assuming updating the state has no side effects). But in code, apart from the first point, almost everything matches what is currently there in the HandleShortcutRemapEvent. While it is possible to reduce some lines of code by removing the logic which is used for minimizing number of key events, for the send key up events for the target shortcut there will still have to be separate cases of logic, since in different situations, different key events have to be sent. I think there is definitely some room for cleaning up in the code (such as the outer loop which iterates over all remappings might not be required). I had created an issue for that here #6842. Instead of iterating, like you said we can use the key state and directly check what is that shortcut, and if there is a mapping for it.

The only part which could cause issues in non-elevated state is the storing key state part.

[mykhailopylyp]

LGTM!
I think it could have been less detailed, keyboardeventhandlers.md file almost duplicates code and can become irrelevant in the future.