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

Factor window scale in UserInputManager.mouseOutsideAllDisplays #6432

Merged
merged 2 commits into from
Nov 24, 2024

Conversation

Susko3
Copy link
Member

@Susko3 Susko3 commented Nov 22, 2024

The mouse position is in pixel/client coordinates, while window and displays positions are in window coordinates.

Proof of coordinate spaces for display position (on a 3456x2234 macbook):

image

(Image taken from https://discord.com/channels/188630481301012481/188630652340404224/1308162379045011506)


We should probably have IWindow.ClientToDisplay(Vector2) that takes in a client (game screen space) vector and converts it to display coordinates. Would the need for ISDLWindow type checks from this code.

The mouse position is in pixel/client coordinates, while
window and displays positions are in window coordinates.
@frenzibyte
Copy link
Member

frenzibyte commented Nov 23, 2024

We should probably have IWindow.ClientToDisplay(Vector2) that takes in a client (game screen space) vector and converts it to display coordinates. Would the need for ISDLWindow type checks from this code.

We already have PointToClient/PointToScreen but those are unimplemented on non-Windows platforms (and they heavily lack documentation and clarification on how they even work, and there's also this which is funny at this point).

@frenzibyte
Copy link
Member

Auto merge is borked (android CI is dead and it's marked as required), to be manually merged by anyone.

@peppy peppy disabled auto-merge November 24, 2024 14:27
@peppy peppy merged commit c274d81 into ppy:master Nov 24, 2024
11 of 14 checks passed
@Susko3 Susko3 deleted the factor-scale-in-mouseOutsideAllDisplays branch November 24, 2024 19:55
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.

3 participants