Skip to content

Add location picking feature #68

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

Merged
merged 12 commits into from
Nov 19, 2022
Merged

Add location picking feature #68

merged 12 commits into from
Nov 19, 2022

Conversation

reloaded
Copy link
Contributor

@reloaded reloaded commented Sep 3, 2022

#1 Add location picking feature

  • Added a "Capture Mouse Coordinates" button next to the X, Y coordinate textboxes allowing the user to capture screen coordinates using the mouse. The captured coordinates will be automatically prefilled in the aforementioned textboxes.
  • Added a new CaptureMouseScreenCoordinatesWindow that overlays a partially transparent window across all available screens/monitors.
  • CaptureMouseScreenCoordinatesWindow exposes a public event that MainWindow hooks in to to receive the user's mouse coordinates upon mouse click.

image

The following scenarios have been tested with no issues.

  • 1 monitor.

  • 2 monitors, horizontally (see below).
    image

  • 2 monitors, vertically (see below).
    image

  • 2 monitors, horizontally, but one has a Y offset (see below).
    image

  • 2 monitors, vertically, but one has a X offset (see below).
    image

The following scenarios have been tested, but with some issues.

  • When a multi monitor setup's primary monitor is configured with a negative offset the capture window does not render fully and correctly for some reason. Still looking into this but I'd argue this scenario is not a common one and can be accepted as a buggy scenario which can be fixed further down the road.

The following scenarios have not been tested.

  • 3 monitors, where 2 are horizontal and the 3rd is vertically stacked above/under the others.

@reloaded
Copy link
Contributor Author

reloaded commented Sep 4, 2022

@oriash93 bringing your attention to this PR, hopefully it can get merged in relatively soon as it's a helpful feature when wanting to click in a specific spot on screen.

… where the primary monitor has a negative X coordinate.

Also added information logging.
@reloaded
Copy link
Contributor Author

reloaded commented Sep 4, 2022

Based on my testing the only bug I found is for the rare scenario where the user's primary screen is configured with a negative offset.

In the horizontal layout configured as seen below (screen 1 is primary).

image

The bug is that the capture window does not draw fully across screen 2. See below.

image

A note about the above image: For some reason the Snipping Tool and Print Screen functions capture things backwards. In the screenshot showing the bug the screen where the window was only rendered halfway across is Screen 2 in the display configuration screenshot...

In a vertical layout configured as seen below (screen 1 is primary).

image

The bug is that the capture window does not draw fully across screen 1. See below.

image

To provide further documentation on the oddity of this bug, I have two identical monitors. Same make/model, same resolution, same physical size. When specifing the primary screen with a negative offset and setting the correct Width/Height and Top/Left values of the mouse capture window, Windows or the .NET framework changes the window size at runtime when the window is rendered. I'm not sure why... See below. I have two screens that are 3840x2160 resolution, as you can see I calculated the correct total width of the window (7680px) yet the rendered window size is 5137.33px.

[21:08:45 INF] Opening window to capture mouse coordinates.
[21:08:45 DBG] Total screens detected: 2
[21:08:45 INF] Screen[Bounds={X=0,Y=0,Width=3840,Height=2160} WorkingArea={X=0,Y=0,Width=3840,Height=2160} Primary=True DeviceName=\\.\DISPLAY1
[21:08:45 INF] Screen[Bounds={X=-3840,Y=7,Width=3840,Height=2160} WorkingArea={X=-3840,Y=7,Width=3840,Height=2160} Primary=False DeviceName=\\.\DISPLAY2
[21:08:45 INF] Min Screen X: -3840
[21:08:45 INF] Max Screen X: 0
[21:08:45 INF] Min Screen Y: 0
[21:08:45 INF] Max Screen Y: 7
[21:08:45 INF] Set window size. Width: 7680, Height: 2167
[21:08:45 INF] Setting window position. Left: -3840, Top: 0
[21:08:45 INF] Opened window to capture mouse coordinates.
[21:08:45 INF] Rendered window size: Width: 5137.33333333333, Height: 1462
[21:08:45 INF] Rendered window position: Left:-3840, Height: 1462
[21:08:52 INF] Captured mouse position: 1707, 1836
[21:08:52 INF] Closing window to capture mouse coordinates.\

I went into the live tree viewer and manually changed the width to 7680 and nothing happened as far as the window size increasing in width. If I set the window value to something less than 5137.33 than the window width actually reduces in size in real time. Something is capping it and I don't know why.

image

image

image

@oriash93
Copy link
Owner

oriash93 commented Sep 4, 2022

Hey @reloaded! first of all, thanks a lot for the PR!
I will take a look at it ASAP

Copy link
Owner

@oriash93 oriash93 left a comment

Choose a reason for hiding this comment

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

Partial review in the meantime, great work so far!

@reloaded
Copy link
Contributor Author

@oriash93 I took care of your suggestions & requests. Go ahead and give it another look.

If Github gives you the option to do so, you should ensure this PR is merged in via a Squash merge. That way your main branch only has a single commit that brings in this feature and keeps your commit history clean.

@reloaded reloaded requested a review from oriash93 September 18, 2022 02:25
@reloaded
Copy link
Contributor Author

@oriash93 bump, do you have time to finish reviewing this PR?

@oriash93
Copy link
Owner

oriash93 commented Nov 9, 2022

Hey @reloaded sorry for the delay. I plan to review and merge by the end of this week :)

@reloaded
Copy link
Contributor Author

reloaded commented Nov 9, 2022

Great.

@oriash93 oriash93 merged commit b5b7df0 into oriash93:main Nov 19, 2022
@oriash93
Copy link
Owner

@reloaded Thank you so much for your contribution!

@oriash93 oriash93 added this to the v3.3.0 milestone Aug 25, 2024
@oriash93 oriash93 changed the title #1 Add location picking feature Add location picking feature Aug 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants