godotengine / godot

Godot Engine – Multi-platform 2D and 3D game engine
https://godotengine.org
MIT License
88.94k stars 20.17k forks source link

Incorrect mouse event position y precision in maximized window with content_scale_factor #94780

Open lostminds opened 1 month ago

lostminds commented 1 month ago

Tested versions

Reproducible in 4.3bX and 4.3rc1

System information

Windows 10, macOS 14.5

Issue description

In a Window with content_scale_factor 2 (for HiDPI purposes) mouse event positions are expected to be even or half pixel units, which is also the case if the window is in windowed mode or fullscreen.

However, if you instead maximize the window the y coordinate of mouse events global_position start getting additional seemingly random decimals added as you can see in the example below.

https://github.com/user-attachments/assets/bca6132a-979e-4700-84b4-352a5cad741b

This happen on Windows, but only in Maximized window mode. It might also happen on linux in the same mode, but I've been unable to test it myself. Can't reproduce it on macOS, but this might be because there's no maximized window mode in the same sense there.

Steps to reproduce

Minimal reproduction project (MRP)

mouse-precision-test.zip

snokaru commented 1 month ago

Hello, new contributor here!

I've looked into this for a bit, I was able to also reproduce this on Linux (Ubuntu 23.10), it seems like the issue is not actually due to full-screen, but it is due to uneven screen resolutions.

Whenever the size of the window is changed, Window (main/window.cpp) updates the viewport size via _update_viewport_size. In this method, a 2D override size (2d_override_size) is computed based on the content_scale_factor and the actual window resolution (size). Due to the scaling of 2, the 2D override size is basically half of the original window size.

Once this size is computed, the window sets it on the Viewport (main/viewport.cpp) through _set_size. Here, a new scale is recomputed (stretch_transform.scale) by dividing size and 2d_override_size, this time for the stretch transform. This calculation is done on real numbers, resulting in something very close to 2 but not exactly 2 if the actual resolution is uneven on any axis.

I would be happy to work on a fix if I can get some guidance on what the expected behavior is. At the moment, I can see two ways it could go:

lostminds commented 1 month ago

I would be happy to work on a fix if I can get some guidance on what the expected behavior is.

That's great to hear! I assume you mean uneven viewport sizes? In my case the issue only manifested in maximized windows mode. But this could I guess result in an uneven height viewport area of the window in case the height of the windows title bar is uneven (which is a little unexpected)? And it could be that in windowed mode the size will never be uneven since when resizing it at 2x scale each pixel resize step will always result in an even window size.

If it's just a precision error of Vector2 I'd however expect that to result in additional decimals, but more like 128.000076 instead of things like 128.8772. But perhaps the precision error are compounded by multiple multiplications?

Regarding the transforms I think they will need to at least support non-integer scales as the content_scale_factor on windows can be any scale value theoretically. It's set as a float, so it's unexpected to hear it's validated to be an integer > 1 like you write above? And even if you base the scale on system dpi-scaling it might at least be values like 1.5 and 1.75 and not just 1 or 2.

snokaru commented 1 month ago

And it could be that in windowed mode the size will never be uneven since when resizing it at 2x scale each pixel resize step will always result in an even window size.

I was also able to get the issue in windowed mode on my end (Linux) by slightly resizing the window, however I'm not sure about Windows. Also, I can get both axes to manifest the issue by resizing in both directions, not just the Y axis. image

If it's just a precision error of Vector2 I'd however expect that to result in additional decimals, but more like 128.000076 instead of things like 128.8772. But perhaps the precision error are compounded by multiple multiplications?

Yes, that is what I noticed, the value goes into multiple multiplications before ending up as the displayed globalPosition.

Regarding the transforms I think they will need to at least support non-integer scales as the content_scale_factor on windows can be any scale value theoretically. It's set as a float, so it's unexpected to hear it's validated to be an integer > 1 like you write above? And even if you base the scale on system dpi-scaling it might at least be values like 1.5 and 1.75 and not just 1 or 2.

After checking this further, it was a mistake on my part, the logic I was mentioning is only executed when the setting display/window/stretch/stretch_mode is set to integer, but the default is fractional.

Then the only solution I can see currently is to not recompute the scale based on current and override resolutions, as you can't avoid possible precision errors otherwise.

lostminds commented 1 month ago

Then the only solution I can see currently is to not recompute the scale based on current and override resolutions, as you can't avoid possible precision errors otherwise.

Indeed. Why is this even done? As stretch_transform.scale should be the same as content_scale_factor, why recalculate it back and forth via 2d_override_size and introduce compounding precision errors? Why not set it to content_scale_factor directly in this case?

alvinhochun commented 1 month ago

This may be tangentially related to #93796