Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[MouseJump] WIP - long-lived executable with hotkey handler (#26073) #27558

Closed

Conversation

mikeclayton
Copy link
Contributor

@mikeclayton mikeclayton commented Jul 20, 2023

Summary of the Pull Request

Changes the way activation works in Mouse Jump to use a long-lived background process that handles its own hotkey activation, rather than having the main runner launch a short-lived instance of MouseJumpUI.exe every time the activation keys are pressed.

This reduces the time to display the preview popup since the Mouse Jump application itself doesn't have to load up from scratch for every activation.

PR Checklist

Detailed Description of the Pull Request / Additional comments

The main PowerToys runner now simply starts MouseJumpUI.exe when the tool is enabled, and terminates the process when the tool is disabled - this is similar to how FancyZones works, except there's no graceful IPC at present to signal to the MouseJumpUI.exe instance to close itself before it gets terminated.

MouseJumpUI.exe installs its own hotkey monitor (using the shared config file to determine what key sequence to register, and listens for activation. When it receives a Windows message that thee hotkey has been activated it displays the popup form, and hides it again when the user clicks or cancels the popup form.

Validation Steps Performed

@github-actions

This comment has been minimized.

@mikeclayton mikeclayton marked this pull request as draft July 20, 2023 20:12
@mikeclayton mikeclayton changed the title [MouseJump] long-lived executable with hotkey handler (#26073) [MouseJump] WIP - long-lived executable with hotkey handler (#26073) Jul 20, 2023
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@mikeclayton mikeclayton marked this pull request as ready for review July 25, 2023 22:40
@mikeclayton mikeclayton changed the title [MouseJump] WIP - long-lived executable with hotkey handler (#26073) [MouseJump] Ready For Review - long-lived executable with hotkey handler (#26073) Jul 25, 2023
@mikeclayton
Copy link
Contributor Author

mikeclayton commented Jul 25, 2023

There's a lot of files touched in this PR, so here's a summary in case it helps work though it quicker...

MouseJump

  • main.cpp - changed the module interface to remove the activation shortcut code and just stop / start a long-lived MouseJumpUI.exe when the module is enabled / disabled

MouseJumpUI

  • Helpers\ConfigHelper.cs - loads the config file and starts a FileSystemWatcher to reload it whenever it's changed (including activation shortcut)
  • HotKeys\** - implements a background message loop that registers a hotkey and listens for a WM_HOTKEY message on the configured shortcut
  • Models\** - added some classes and code to read the settings file
  • NativeMethods** - adds a bunch more WIn32 declarations to support Hotkeys and all the associated window / message functions

The remaining handful of changes are really just to integrate the ones above into the application.

@mikeclayton
Copy link
Contributor Author

mikeclayton commented Jul 25, 2023

Btw, I don't have any hard stats on performance, but it certainly feels a lot more responsive now that it's a long-lived background task rather than having to launch a whole new MouseJump.exe process every time the hotkey is activated.

Have a play with the current PowerToys release first to get a feel for how quickly the popup is displayed before you try the one in this PR, and see if it feels faster...

Copy link
Collaborator

@jaimecbernardo jaimecbernardo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @mikeclayton ,

Thank you for the contribution!
Sorry for the delay in reviewing the PR, since we had entered "wrap up release" mode last week.
Giving the PR a try, there's some shortcuts that are not supported anymore, but they were without the changes. I tested Win+J, for example. This is through the use of shortcuts instead of low level keyboard hooks. LLKH however shouldn't be used in C#.

I think a better approach here would be to let the module interface keep handling activation by using a event instead to signal actiavation to MouseJumpUI. No need to add all that boiler code to the project this way.
This is used, for example, by Peek, ColorPicker, TextExtractor(PowerOCR) and even PowerLauncher when using the centralized key hook.

TextExtractor might be the best one to look at to understand this, since it's pretty similar in what it does (running in the background to then show an overlay). If you look in the solution for the strings SHOW_POWEROCR_SHARED_EVENT and ShowPowerOCRSharedEvent it should give a good idea of the workflow. The modules interface registers the event and then the UI listens for it with NativeEventWaiter.WaitForEventLoop.

What do you think about this?

@jaimecbernardo
Copy link
Collaborator

Someone in the core team can help bring this method in if you want :)

@mikeclayton
Copy link
Contributor Author

@jaimecbernardo - No worries. I'm on leave at the moment, but I'll take a look when I'm back home - I'll give it a go myself and yell if I get stuck :-)..

I'll put this back to WIP in the meantime...

@mikeclayton mikeclayton changed the title [MouseJump] Ready For Review - long-lived executable with hotkey handler (#26073) [MouseJump] WIP - long-lived executable with hotkey handler (#26073) Aug 1, 2023
@mikeclayton
Copy link
Contributor Author

I'm going to close this PR as the proposed fix re Windows Events is probably easiest to just start on again from main...

@mikeclayton mikeclayton closed this Sep 1, 2023
@mikeclayton mikeclayton deleted the dev/mikeclayton/mousejump branch September 5, 2023 22:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants