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 Input under Wayland misses slow movement #88246

Closed
Tracked by #88346
pracedru opened this issue Feb 12, 2024 · 17 comments · Fixed by #88343
Closed
Tracked by #88346

Mouse Input under Wayland misses slow movement #88246

pracedru opened this issue Feb 12, 2024 · 17 comments · Fixed by #88343

Comments

@pracedru
Copy link

pracedru commented Feb 12, 2024

Tested versions

  • Reproducible in: Godot 4.3 dev3 on Gnome Wayland in Fedora 39

System information

Godot v4.3.dev3.mono - Fedora Linux 39 (Workstation Edition) - Wayland - Vulkan (Forward+) - dedicated AMD Radeon RX 6700 XT (RADV NAVI22) () - Intel(R) Core(TM) i7-6700K CPU @ 4.00GHz (8 Threads)

Issue description

  • I have a Corsair high DPI mouse that is set to high DPI and the movement speed in the OS is reduced.
  • When holding right mouse button to nevigate the editor scene and moving the mouse slowly the movement seems to be filtered, like if an float to int conversion is missing all values bellow 1.0.

Steps to reproduce

  1. Setup Mouse with high dpi settings and reduce movement speed in OS.
  2. Open Godot with --display-driver wayland.
  3. Right click on scene to navigate
  4. move mouse slowly.
  5. Then move mouse fast to see that some movement is registered if its fast enough.

Minimal reproduction project (MRP)

No project is needed to reproduce this issue.

@Riteo
Copy link
Contributor

Riteo commented Feb 13, 2024

like if an float to int conversion is missing all values bellow 1.0.

Good catch, that might be it!

It looks like for some reason when we receive a motion event we handle them all as ints, converting the wl_fixed type to int instead of double. Since the unit of measure is pixels (and is thus very small) it slipped out of testing.

Although, and this is very weird, relative motion seems to be (correctly) fractional. You said that this happens when you hold right click, which captures your mouse and thus switches to it, which should not cause the issue you described.

I'll switch motion events to Vector2 ASAP. In the meantime, could you attach a log of the editor where you replicate the issue with WAYLAND_DEBUG=1? It should dump us all Wayland events, including pointer motion.

@akien-mga akien-mga added this to the 4.3 milestone Feb 13, 2024
@pracedru
Copy link
Author

like if an float to int conversion is missing all values bellow 1.0.

Good catch, that might be it!

It looks like for some reason when we receive a motion event we handle them all as ints, converting the wl_fixed type to int instead of double. Since the unit of measure is pixels (and is thus very small) it slipped out of testing.

Although, and this is very weird, relative motion seems to be (correctly) fractional. You said that this happens when you hold right click, which captures your mouse and thus switches to it, which should not cause the issue you described.

I'll switch motion events to Vector2 ASAP. In the meantime, could you attach a log of the editor where you replicate the issue with WAYLAND_DEBUG=1? It should dump us all Wayland events, including pointer motion.

Yes. This happens during capture. It works fine when say moving an object and other mouse related features. So maybe you have found an unrelated bug there.

@Riteo
Copy link
Contributor

Riteo commented Feb 14, 2024

@pracedru I think you're right, the integer thing is unrelated as the "accumulation" is done compositor-side, so even if the thing gets truncated we still get a new pixel position later anyways. Just noticed that while switching to vectors (I'll still push the thing though as it's a correctness fix).

A WAYLAND_DEBUG=1 log would help tremendously here, as I'm not really sure what's going on.

@Riteo
Copy link
Contributor

Riteo commented Feb 14, 2024

Update (sorry for the DP): After some very careful finger slipping, I can replicate this even on my touchpad. It's indeed missing sub-pixel relative motions, which is very weird. I'll investigate further.

A WAYLAND_DEBUG=1 log is not needed anymore, sorry 😅

@Riteo
Copy link
Contributor

Riteo commented Feb 14, 2024

Bingo! I'm very proud to say that my backend was not the issue 8)

Jokes aside, the issue is actually a weird (leftover?) limitation in the way that a very specific part of the input is handled, namely Input::warp_mouse_motion and Node3DEditorViewport::_get_warped_mouse_motion worked in and returned Point2i (integer vector2 basically) while everything else uses Point2, including the wayland backend.

Really weird stuff. I'm setting up a PR as we speak.

@pracedru
Copy link
Author

This is great news @Riteo. Keep up the great work!!

@Riteo
Copy link
Contributor

Riteo commented Feb 14, 2024

Done! @pracedru could you test #88343 and tell me if it fixes the issue for you?

@pracedru
Copy link
Author

Done! @pracedru could you test #88343 and tell me if it fixes the issue for you?

I've recently got a new desktop and i've only now pulled your branch.
I am build right now.

@pracedru
Copy link
Author

@Riteo
I've just built your branch and i get:
image
Does one need to pass an argument to get wayland support ?

@akien-mga
Copy link
Member

You might be missing the wayland-scanner build dependency.

@pracedru
Copy link
Author

pracedru commented Feb 15, 2024

You might be missing the wayland-scanner build dependency.

Yeah. How do i get that? I am on Fedora 39.

EDIT: ok i found it by installing wayland-devel

@pracedru
Copy link
Author

pracedru commented Feb 15, 2024

Done! @pracedru could you test #88343 and tell me if it fixes the issue for you?

Hi @Riteo I have build your branch and can confirm that this branch fixes the issue with captured mouse navigation in the editor under Wayland.

@Riteo
Copy link
Contributor

Riteo commented Feb 15, 2024

@pracedru great! Thanks for testing and for your patience! <3

@vvvvvvitor
Copy link

Apparently this was fixed, why is it still open?

@akien-mga
Copy link
Member

The PR which purports fixing this hasn't been reviewed/merged yet (see the green "Open" label, it means it's not merged). So the bug isn't fixed yet.

@mournguard
Copy link

I'm having a similar issue just trying to use Input.warp_mouse directly with a new slightly different position. Would fixing this allow me to make <1 changes or does it just not make sense to warp_mouse to a subpixel ?

@Riteo
Copy link
Contributor

Riteo commented Apr 21, 2024

@mournguard just checked, and it takes a Point2i indeed. I suppose that it's possible to make it subpixel with a similar fix, yeah, as there's pretty much the same reason behind.

The only annoying part is that it'd require yet another API change from Point2i to Point2 but I suppose that it wouldn't too bad. Might be worth filing an issue ticket for that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants