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

[Mouse Jump] - new feature - #23216 #23566

Merged

Conversation

mikeclayton
Copy link
Contributor

@mikeclayton mikeclayton commented Jan 24, 2023

Summary of the Pull Request

Initial implementation of Mouse Jump as proposed in this issue:

#23216 - "Mouse Jump" for ultra-wide monitors

FancyMouse.mp4

(based on https://github.com/mikeclayton/FancyMouse)

Adds the core Mouse Jump functionality, a new settings card to enable / disable the feature and configure the activation shortcut, and includes some tests around the thumbnail sizing and form layout calculations.

It's functional and ready for an initial design review but will probably need some work before it's ready to release.

Some of the work still to do (or add as future improvements):

To Do

  • Initial implementation and basic smoke testing
  • Complete rest of the PR checklist :-)

Needs Review by PowerToys Team

  • Confirm default enabled / disabled state and activation shortcut
  • WiX installer package
  • Icon on the settings page is a bit non-descript - improve it?

Nice to Have / Future Work

  • Add logging?
  • Add telemetry?
  • Configurable preview size - min / max percentage of screen or pixel size (currently shows at 1600x1200)
  • Review performance - currently starts an *.exe every time the activation shortcut is triggered. Might be more responsive to run a background app and trigger an event inside that somehow
  • resize the desktop image before assigning to picture box - test perf diff
  • Convert WinForms -> WPF?
  • documentation - make notes or comments re
    • dependency on dpi awareness (per monitor v2) otherwise jump position gets incorrectly scaled
    • handling negative coordinates on non-primary monitors if higher or "lefter" than primary monitor
    • virtualbox with mouse integration enabled doesn't move the cursor? might be by design, but at least call it out as a compatibility issue
  • limit preview size to percentage of screen (on single screen desktops with a low resolution it can fill the entire monitor!)

PR Checklist

  • Closes: "Mouse Jump" for ultra-wide monitors #23216
  • Communication: I've discussed this with core contributors already. If work hasn't been agreed, this work might be rejected
  • Tests: Added/updated and all pass
    • added MouseJumpUI.UnitTests.csproj to sln
    • added MouseJumpUI.UnitTests.dll to "MS Tests" step in build-powertoys-steps.yml
    • checked tests are running - "Passed RunTestCases (MouseJumpUI.Helpers.Tests.LayoutHelperTests+CenterObjectTests+TestCase) [16 ms]" appears in build log
  • Localization: All end user facing strings can be localized
    • settingsui labels are localised
    • module ui has no text / labels
  • [wip] Dev docs: Added/updated
  • [wip] New binaries: Added on the required places
  • Documentation updated: If checked, please file a pull request on our docs repo and link it here: #xxx

Detailed Description of the Pull Request / Additional comments

Validation Steps Performed

@github-actions

This comment has been minimized.

@mikeclayton mikeclayton changed the title WIP: #23216 - new Mouse Jump feature - initial review requested WIP: #23216 - new Mouse Jump feature - not ready for review Jan 24, 2023
@htcfreek
Copy link
Collaborator

@mikeclayton
For your information: I tried v0.0.7 from the reverenced Repository and there are two bugs: Settings doesn't open and the preview shows an empty white window.

@mikeclayton
Copy link
Contributor Author

@htcfreek - oh, that's not very promising :-S.

I'll try to get the build working in this PR asap, and then maybe you could test it again in the PowerToys version? I'll ping you when it's ready to test...

@mikeclayton
Copy link
Contributor Author

mikeclayton commented Jan 25, 2023

@htcfreek - the latest commit in this PR builds successfully and passes all tests now. Are you able to test again and see if MouseJump works inside PowerToys or if you still get the white panel?

There's a pane on the "Mouse utils" tab in the PowerToys settings screen where you can enable the feature and set the activation shortcut - on my machine I've bound that combination to a spare button in my mouse software (Logitec SetPoint) so I can activate it without letting go of the mouse, which makes it a bit more seamless.

image

You'll need to run it from the source repo as I've not added the feature to the installer yet...

This will be the first real test outside of "works on my machine", so give me a yell if / when there's any issues...

@jaimecbernardo
Copy link
Collaborator

Hi @mikeclayton ,
Thanks for the contribution!
I'm now just gaving it a try.
I basically get a large white rectangle on my screen (I've got 3 screens lined horizontally)
image

I'm on Windows 10 by the way. Not sure if that might be the issue.

@jaimecbernardo
Copy link
Collaborator

@mikeclayton , I think I know what's happening:

This is my usual configuration of 3 screens:
image
Note that the middle screen is the "main monitor", meaning screen 3 will have negative coordinates. Mouse Jump shows everything white in this configuration.

If I make my main monitor the leftmost one, it will show correctly:
image

I just shifted one of the screens a bit up and we can see that the issue with the white screen seems to be about the coordinates where we're copying the screen contents to the bitmap. They likely need to be shifted because the top left coordinate of the the virtual screen can be negative and not 0,0.

image

Hope this helps.

@jaimecbernardo
Copy link
Collaborator

On another note, just noticed that the background application running is staying running even after PowerToys has exited.

@htcfreek
Copy link
Collaborator

Hi @mikeclayton , Thanks for the contribution! I'm now just gaving it a try. I basically get a large white rectangle on my screen (I've got 3 screens lined horizontally) image

I'm on Windows 10 by the way. Not sure if that might be the issue.

@jaimecbernardo
I had the same bug with the version 0.7 from the linked repository. Does it work for you when clicking within the white preview too?

@mikeclayton
Copy link
Contributor Author

@jaimecbernardo, @htcfreek - apologies - the white screen was a school-boy error...

graphics.CopyFromScreen(desktopBounds.Top, desktopBounds.Left, 0, 0, desktopBounds.Size);
//                                    ^^^                ^^^^

should have been

graphics.CopyFromScreen(desktopBounds.Left, desktopBounds.Top, 0, 0, desktopBounds.Size);
//                                    ^^^^                ^^^

It was working fine for me because both values were 0, but obviously not so much if they're different...

@mikeclayton
Copy link
Contributor Author

mikeclayton commented Jan 26, 2023

And it wasn't closing because I'd cut&pasted from my poc that sat in the system tray, so it was doing a this.Hide(); instead of a this.Close(); after receiving a click.

I've fixed both issues and pushed them into this PR...

@mikeclayton
Copy link
Contributor Author

@jaimecbernardo, @htcfreek - I think I've fixed the two reported issues, if you're able to try again?

I'm struggling to update the installer, but it would be good to know if the core feature is working for you now...

@htcfreek
Copy link
Collaborator

@jaimecbernardo, @htcfreek - I think I've fixed the two reported issues, if you're able to try again?

I'm struggling to update the installer, but it would be good to know if the core feature is working for you now...

@mikeclayton
VBox doesn't allow me to have more than one monitor in my dev vm. So I can't test.

@mikeclayton
Copy link
Contributor Author

VBox doesn't allow me to have more than one monitor in my dev vm. So I can't test.

@htcfreek - no worries.

Note the feature should still work if there's only one monitor though - if you've got a single ultra-wide on a desktop / tower pc for example it would still be useful. And even you've only got a small virtual screen it might not be much practical use, but it should still show the pop-up thumbnail correctly and move the mouse :-).

@htcfreek
Copy link
Collaborator

@mikeclayton
The preview is working now. But in one case the cursor is set to the wrong position.
image

And it doesn't works with VirtualBox and enabled Mouse pointer integration.

@mikeclayton
Copy link
Contributor Author

@htcfreak - I'll need some time to take a look. I'll install VirtualBox and test that as well and I'll revert once I've got an update - thanks for the continued feedback...

@htcfreek
Copy link
Collaborator

htcfreek commented Feb 1, 2023

And it doesn't works with VirtualBox and enabled Mouse pointer integration.

@mikeclayton
I think this is by design and caused in how the mouse integration works.

@jaimecbernardo
Copy link
Collaborator

Hi @mikeclayton , Gave it another try. It doesn't seem to be working when the screens have different dpis.
This can be changed here for one of the screens to verify:
image

In my case, I changed dpi value for the rightmost screen (not a primary one).

@jaimecbernardo
Copy link
Collaborator

Hi @mikeclayton ,
It's OK to add them here, Thank you!
I had to wrap up the Paste as Plain Text PR first, so I paused the wrap up here.
I imagine we won't conflict with each other on the changes we're making, so merges from/to should work well enough.

@jaimecbernardo
Copy link
Collaborator

Alright, feel like this one is pretty wrapped up now as a MVP.
@mikeclayton , we're looking to try and bring this in for 0.68, which we're planning for next week at this moment.
It might be better to do the optimization you mentioned in a different PR, since we're likely to bring this PR in tomorrow.

@mikeclayton
Copy link
Contributor Author

mikeclayton commented Feb 23, 2023

@jaimecbernardo - fab, yeah I was just thinking it would be better to leave any improvement work for the next iteration of the tool...

src/modules/MouseUtils/MouseJump/dllmain.cpp Outdated Show resolved Hide resolved
@jaimecbernardo
Copy link
Collaborator

I've pushed a commit with the suggested changes. Thank you for the reviews!

@jaimecbernardo jaimecbernardo merged commit 0524a4b into microsoft:main Feb 24, 2023
@jaimecbernardo
Copy link
Collaborator

Merged it in to be released on the next release.
Thanks a lot for the contribution @mikeclayton 🎉

@williamdean19
Copy link

I am having an issue where the mouse jump window is not appearing on top. Example:
image

BLM16 pushed a commit to BLM16/PowerToys that referenced this pull request Jun 22, 2023
* microsoft#23216 - initial MouseJump commit

* microsoft#23216 - Mouse Jump - fix spelling, removing Interop folder

* microsoft#23216 - Mouse Jump - removed orphaned project guids from PowerToys.sln

* microsoft#23216 - Mouse Jump - removed orphaned project guids from PowerToys.sln

* microsoft#23216 - Mouse Jump - switch MS Logger to NLog for nuget package allow-listing

* microsoft#23216 added MouseJumpUI.UnitTests.dll to "MS Tests" step in build-powertoys-steps.yml

* [MouseJump] fixed screenshot coords (x & y were transposed) (microsoft#23216)

* [MouseJump] close form rather than hide on deactivate (microsoft#23216)

* [MouseJump] added UI dll for signing (microsoft#23216)

* [MouseJump] close form rather than hide on deactivate (microsoft#23216)

* [MouseJump] removed redundant line

* [MouseJump] configure dpi awareness, add NLog.config (microsoft#23216)

* [MouseJump] fix spellchecker errors (microsoft#23216)

* [MouseJump] fixing comment style warning (microsoft#23216)

* [MouseJump] simplified dpi config (microsoft#23216)

* [MouseJump] fixed edge case issue with moving cursor (microsoft#23216)

* [MouseJump] fixed typo (microsoft#23216)

* [MouseJump] added attribution (microsoft#23216)

* [Mouse Jump] fix attribution link and spelling (microsoft#23216)

* Add MouseJump to installer

* Fix centralized version control

* Add Quick Access enable/disable entry

* Fix analyzer error in GPO

* Fix botched merge

* Disabled by default and remove boilerplate code

* Add GPO definitions

* Add GPO implications when starting standalone

* Update hotkey when it's changed in Settings

* Use standard Logger

* Add OOBE strings for Mouse Jump

* Add telemetry

* Update installer

* Add signing

* Add to bug report tool

* Address PR feedback
@mikeclayton mikeclayton deleted the dev/mclayton/23216-mouse-jump branch September 5, 2023 22:18
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.

None yet

5 participants