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.

Sign up for GitHub

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

Jump to bottom

fix: scale cursor_position #712

Merged
merged 3 commits into from
Mar 8, 2023
Merged

fix: scale cursor_position #712

merged 3 commits into from
Mar 8, 2023

Conversation

amrbashir
Copy link
Member

@amrbashir amrbashir commented Mar 7, 2023 •
edited
Loading

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Docs
  • Code style update
  • Refactor
  • Build-related changes
  • Other, please describe:

Does this PR introduce a breaking change?

  • Yes
  • No

Checklist

Other information

@amrbashir amrbashir requested a review from a team as a code owner March 7, 2023 12:47
@amrbashir amrbashir changed the title fix: scale cursor_position, closes #708 fix: scale cursor_position Mar 7, 2023
@amrbashir
Copy link
Member Author

@wusyong I fixed the linux impl, feel free to take macOS. Windows should be working fine iirc but I will test later today once I switch back to windows.

@wusyong
Copy link
Member

wusyong commented Mar 7, 2023 •
edited
Loading

@amrbashir I tested with event_loop.cursor_position() and it'll return error.
And I feel like this is tricky when two monitors have different scale factors. Not sure if this could be the case.

Edit: Linux should be easier because I believe it's all one display one screen now. They should share same scale factor.

@amrbashir
Copy link
Member Author

I tested with event_loop.cursor_position() and it'll return error.

I haven't been able to get it to error at all, using one screen or two.

And I feel like this is tricky when two monitors have different scale factors. Not sure if this could be the case.
Edit: Linux should be easier because I believe it's all one display one screen now. They should share same scale factor.

yeah they share the same scale_factor at least on my KDE.

@amrbashir
Copy link
Member Author

just tested Windows and it works as expected with single and multi-monitor setup

@wusyong
Copy link
Member

wusyong commented Mar 8, 2023

macOS added

@amrbashir
Copy link
Member Author

can't approve my own PR so feel free to approve and merge

@wusyong
Copy link
Member

wusyong commented Mar 8, 2023

but where's Windows?

@amrbashir
Copy link
Member Author

Windows works fine

@amrbashir
Copy link
Member Author

Tested it yesterday

wusyong
wusyong approved these changes Mar 8, 2023
View reviewed changes
Copy link
Member

@wusyong wusyong left a comment

Choose a reason for hiding this comment

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

I also tested on Windows myself and you are right.

@wusyong wusyong merged commit dc913cd into dev Mar 8, 2023
@wusyong wusyong deleted the fix/cursor-position-scale branch March 8, 2023 11:47
@github-actions github-actions bot mentioned this pull request Mar 8, 2023
Merged
oscartbeaumont pushed a commit to spacedriveapp/tao that referenced this pull request Dec 22, 2023
@amrbashir @wusyong @oscartbeaumont
fix: scale cursor_position (tauri-apps#712)
72d584b 
* fix: scale cursor_position, closes tauri-apps#708

* adjust for wayland

* Add macOS fix

---------

Co-authored-by: Wu Wayne <yuweiwu@pm.me>
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wusyong wusyong wusyong approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

window.cursor_position() is actually LogicalPosition
2 participants
@amrbashir @wusyong