Add support for blocking hotkeys (#11) #12
Conversation
| private IntPtr HookCallback(int nCode, IntPtr wParam, IntPtr lParam) | ||
| { | ||
| if (nCode >= 0) | ||
| { | ||
| var vkCode = Marshal.ReadInt32(lParam); | ||
|
|
||
| // Debug.WriteLine("Starting"); | ||
| // To prevent slowing keyboard input down, we use handle keyboard inputs in a separate thread | ||
| ThreadPool.QueueUserWorkItem(this.HandleSingleKeyboardInput, new KeyboardParams(wParam, vkCode)); |
There was a problem hiding this comment.
Although it has been a long time since I have coded/used this package, I recall that this had an important positive impact on performance.
Unfortunately, the new behavior you are trying to introduce requires (at the very least) to check if the pressed keys are assigned to a blocking hotkey before proceeding. I do believe that looking up the hotkey is fast enough for us to move it to the main thread, but as soon as we find out that a hotkey is non-blocking I want it to be handled by the thread pool like it used to be.
The bottom line is we have two choices:
either move the parameter up to the KeyboardManager's constructor parameters and then maintain the old thread pool behavior when non-blocking,
or make the handling of non-blocking hotkeys run in a new thread to maintain the old behavior
There was a problem hiding this comment.
(reposting comment from correct account)
Thanks for taking the time to look at the PR. I am not entirely sure I understand your comment correctly.
as soon as we find out that a hotkey is non-blocking I want it to be handled by the thread pool like it used to be.
With the changes in this PR every shortcut callback function will be executed in the thread pool (ref). As you said only the check for a matching shortcut will be performed on the main thread.
Regarding your proposed solutions:
either move the parameter up to the KeyboardManager's constructor parameters and then maintain the old thread pool behavior when non-blocking
You mean dedicating a separate instance of KeyboardManager for either blocking or non-blocking behavior?
make the handling of non-blocking hotkeys run in a new thread to maintain the old behavior
By this, do you mean having two separate dictionaries (e.g. _registeredBlockingCallbacks and _registeredNonBlockingCallbacks) and then iterating over one in the main thread and the other in a new thread?
The first option would probably define the behavior a little more clearly for the user while the second approach would give the user a simpler API to work with (although the implementation would probably make me feel a little uneasy).
It's hard for me to get a feeling of the input delay my changes would have. I can't imagine it being noticeable but also I don't know how you can measure it.
Please let me know what you think and whether I understood your comment correctly.
There was a problem hiding this comment.
My sincerest apologies, I was (and still am) reviewing this PR from an airport terminal. It seems you have already implemented this feature in the way that I described in my 1st comment.
Your changes will likely not impact input delay, and as you said yourself, this API is more developer friendly :)
P.S. what a relief to see another person who struggles juggling 2 github users; I will approve this PR via the correct account in a moment
There was a problem hiding this comment.
No worries, considering your location, thank you even more for finding the time!
|
Great :) I will release a new version to NuGet on Friday. If you see that it is taking longer feel free to comment here |
|
Published to nuget. Unfortunately I did not have the time to install the new version on a local project to verify the new feature, but if you could do it @stnkl that'll be lovely :) |
|
Hi, sorry for the late reply. Unfortunately I am unable to test the nuget package due to the problems described in #7. I was developing and testing the changes of this PR by manually adding this library as a separate project to my solution, I might look into it again when I have some time though. :) |
I added an optional parameter
blockingfor theRegisterHotkeymethod to allow the user to block the original input from propagating on a per-hotkey basis. Default behavior is still to not block propagation.This resolves issue #11.