Skip to content

Fix mouse centering on high DPI displays with scaling enabled #2611

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 1 commit into from
Jun 19, 2023

Conversation

FeralChild64
Copy link
Collaborator

Fixes #2610

@FeralChild64 FeralChild64 added the bug Something isn't working label Jun 17, 2023
@FeralChild64 FeralChild64 requested review from johnnovak and kcgen June 17, 2023 20:43
@FeralChild64 FeralChild64 self-assigned this Jun 17, 2023
@johnnovak
Copy link
Member

Yeah, so this needs more work. Results on macOS:

image

So the pointer appears in the bottom-right corner, which makes it turn into the resize pointer shape.

I've done some debugging:

image image

SDL bug perhaps?

@FeralChild64 FeralChild64 force-pushed the fc/mouse-centering-fix-1 branch from d1bd706 to 7bef491 Compare June 18, 2023 12:54
@kcgen
Copy link
Member

kcgen commented Jun 18, 2023

Confirmed working OK here on Linux w/ a slightly raised DPI, using both output opengl and texture.

Edit: I was using X11 which @GranMinigun mentioned:

kcgen, no point in testing DPI-related stuff in X11 session. No such concept there.

Posting results from Wayland.

@FeralChild64 FeralChild64 force-pushed the fc/mouse-centering-fix-1 branch from 7bef491 to 92b72bd Compare June 18, 2023 16:59
@kcgen
Copy link
Member

kcgen commented Jun 18, 2023

@GranMinigun: You have to switch to Wayland session, yes, as DPI scaling concept exists only there.

PR is working great on XWayland with DPI scaling:

dpi-wayland

@kcgen
Copy link
Member

kcgen commented Jun 18, 2023

Using SDL_VIDEODRIVER=wayland, the released cursor appears in the original place it was captured (so perhaps, mouse warp doesn't work there):

Screenshot_20230618_121914

This is perfect fine; if that's how Wayland does things, then it's probably true for anything that captures the mouse for exclusive control as well.

@FeralChild64
Copy link
Collaborator Author

Using SDL_VIDEODRIVER=wayland, the released cursor appears in the original place it was captured (so perhaps, mouse warp doesn't work there):

Really strange - works for me. SDL2 v2.26.5, Plasma 5.27.5, KDE frameworks 5.106.0, Wayland 1.22.0

@kcgen
Copy link
Member

kcgen commented Jun 18, 2023

Using SDL_VIDEODRIVER=wayland, the released cursor appears in the original place it was captured (so perhaps, mouse warp doesn't work there):

Really strange - works for me. SDL2 v2.26.5, Plasma 5.27.5, KDE frameworks 5.106.0, Wayland 1.22.0

Looks like I'm behind everywhere: SDL2 2.26.3, Plasma 5.27.4, KDE frameworks 5.104.0, and Wayland 1.21.0.

These packages are still improving, and I guess there have been mouse position-related improvements made after the versions I'm running.

So long as the code on our side has the correct meaning, then we'll just let the user-space catch up eventually.

@FeralChild64
Copy link
Collaborator Author

@kcgen Sorry, I was wrong - indeed, it gets released at the place where it was captured. I must have misspelled something during my previous test.

@kcgen
Copy link
Member

kcgen commented Jun 18, 2023

@kcgen Sorry, I was wrong - indeed, it gets released at the place where it was captured. I must have misspelled something during my previous test.

Oh, interesting! Maybe this is going to be the intended behavior of SDL 2.x on Wayland (or it'll change in 3.x; looks like other Wayland mouse position tickets are now scheduled to be addressed in the 3.x timeframe like: libsdl-org/SDL#1836)

@johnnovak
Copy link
Member

So ifdef this out for Apple and possibly raise an SDL bug report?

@kcgen
Copy link
Member

kcgen commented Jun 18, 2023

So ifdef this out for Apple and possibly raise an SDL bug report?

yup - @FeralChild64's got the change isolated to WIN32 now.

I was checking in SDL2's repo and noticed this one with a possible solution, libsdl-org/SDL#7245, however that's for SDL3 :-)!

The 2.28 release (already in RC1) is their final release for the 2.x series, so I suspect we've missed the boat. Given it's a one-liner, not a big deal to carry until we can migrate to SDL3.

(But maybe it's worth reporting anyway; I don't feel like it though.. given they are migrating to SDL3 and have punted many DPI issues into 3.x already).

@johnnovak
Copy link
Member

So ifdef this out for Apple and possibly raise an SDL bug report?

yup - @FeralChild64's got the change isolated to WIN32 now.

I was checking in SDL2's repo and noticed this one with a possible solution, libsdl-org/SDL#7245, however that's for SDL3 :-)!

The 2.28 release (already in RC1) is their final release for the 2.x series, so I suspect we've missed the boat. Given it's a one-liner, not a big deal to carry until we can migrate to SDL3.

(But maybe it's worth reporting anyway; I don't feel like it though.. given they are migrating to SDL3 and have punted many DPI issues into 3.x already).

So we're planning to migrate SDL3 soon?

@kcgen
Copy link
Member

kcgen commented Jun 19, 2023

So we're planning to migrate SDL3 soon?

Not us.. (we still have to merge @GranMinigun 's branch post 0.81, then wait for things to stabilize in SDL 3, which might be some years).

But SDL devs are migrating! They've finalized the 2.x feature set and 2.28 will be their last release in the 2.x series.

Also, they have punted very old 2.x DPI-related issues (ones that have been open since 2016) into the 3.x series.. never to be fixed in 2.x.

It's a bit line someone reporting an issue against Staging 0.79. we won't be releasing 0.79.2.. likewise, I don't think we can expect fresh 2.x bugs to be fixed. Instead, we'll just wait for 3.x, which reworks DPI handling from the ground up.

@kcgen
Copy link
Member

kcgen commented Jun 19, 2023

Nice quick fix @FeralChild64.

@kcgen kcgen merged commit f989b9c into main Jun 19, 2023
@weirddan455
Copy link
Collaborator

For what it's worth, there's going to be an SDL2 -> SDL3 compatibility layer (similar to what they did for SDL1 -> SDL2 except this time it's going to available from day 1) so we're not necessarily "stuck" with any SDL bugs. When SDL 3 releases, we should just be able to link to the compat library and benefit from bug fixes (or have end users link dynamically, I believe it even works with the same binary).

@GranMinigun
Copy link
Contributor

@kcgen, bugs are meant to be reported, and SDL 2.28 will be supported for some time and will receive patch releases (2.28.x). Whether that specific behaviour will be addressed in SDL 2 or 3 is up to them, of course, but unless there's an open bug report already, we ought to let them know about inconsistency in behaviour between different platforms. We all want SDL to be better, don't we?

@johnnovak
Copy link
Member

@kcgen, bugs are meant to be reported, and SDL 2.28 will be supported for some time and will receive patch releases (2.28.x). Whether that specific behaviour will be addressed in SDL 2 or 3 is up to them, of course, but unless there's an open bug report already, we ought to let them know about inconsistency in behaviour between different platforms. We all want SDL to be better, don't we?

I agree, this should be reported... by someone else than me as I'm busy with other stuff :ducks:

@kcgen
Copy link
Member

kcgen commented Jun 19, 2023

Yup, I agree too.

( but I also won't be opening it because I'm not well versed in this area, and there's too much going on here to break out a side project and make a stand alone repro case, check if latest SDL 2.28 rc1 on a Win VM is affected, etc).

I'm also not about to (t)ask anyone to do it: but if it gets done, great and thank you 👏 !

@kcgen kcgen deleted the fc/mouse-centering-fix-1 branch June 20, 2023 02:56
@GranMinigun
Copy link
Contributor

Fine, I'll do it myself.

@FeralChild64
Copy link
Collaborator Author

@GranMinigun Thank you. It would be hard for me to provide support for the SDL team, as I’m a Linux-only user.

@kcgen
Copy link
Member

kcgen commented Jun 21, 2023

@GranMinigun , your ticket and repro code were so good, slouken was able to fix it immediately. 🤯 🥇

@johnnovak johnnovak added the input handling Issues related to handling any input (keyboard, mouse, joystick & game controllers) label Dec 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working input handling Issues related to handling any input (keyboard, mouse, joystick & game controllers)
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Center mouse cursor before releasing it doesn't work with windows scale of 200%
5 participants