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

Snap 2D Transforms to Pixel for subpixel value of 0.5 is inconsistent between positive and negative coordinates #93731

Closed
alvinhochun opened this issue Jun 29, 2024 · 3 comments · Fixed by #93740

Comments

@alvinhochun
Copy link
Contributor

alvinhochun commented Jun 29, 2024

Tested versions

System information

Godot v4.3.beta2 - Windows 10.0.19045

Issue description

#87297 (comment):

[...] instead of rounding, it should have been floor(x + 0.5) instead. This is because rounding to nearest has different behaviour for positive and negative values when the fractional part is 0.5.

Consider this: We position a sprite at x=0.5, which is snapped to x=1 on screen. We move it to the left by 1px. Where should the sprite be at on screen?

The expectation should be x=0. However, since the subpixel position is now x=-0.5, the sprite is actually snapped to x=-1 on screen thanks to rounding. Which means the supposedly 1px move becomes a 2px move on screen.

Fixing this may affect the behaviour of #93712.

CC @markdibarry

Steps to reproduce

  • Run MRP
  • The 5 squares are 16 px apart, all having a subpixel x position of 0.5
  • Label shows the coordinates of the centre red square
  • Arrow key moves the squares by 0.125 px, Ctrl + arrow key moves by 1 px
    • Hold Shift to move one step at a time
  • Use Ctrl + arrow keys to move: Observe that there is a 1px gap between the squares crossing the origin (centre of screen)
  • Use arrow keys to move: Observe that the movement is inconsistent when the subpixel position changes from/to 0.5

Minimal reproduction project (MRP)

pixel-snapping-negative-bug.zip

@markdibarry
Copy link
Contributor

markdibarry commented Jun 29, 2024

I've tested it by replacing the rounding with floor(x + 0.5) and it only seems to make the wrong behavior more consistent. I don't think this would be a noticeable issue if your other bug ticket were solved, but I think you're right that the floor(x + 0.5) is the right call in the long run once the other ticket is resolved.

I wouldn't necessarily say this is a regression, because the behavior is much much more consistent than before, and there's now a workaround where there wasn't any before, but it could definitely be better.

@alvinhochun
Copy link
Contributor Author

In this issue I am focusing on the inconsistent snapping of canvas items, so please forget about the camera for a second. I would consider this a regression because, in 4.2, the behaviour is actually consistent in the way that all values are rounded towards negative infinity, so there is no subpixel value that would round differently in the negative range. On master however, the midway point 0.5 is being rounded away from zero.

@markdibarry
Copy link
Contributor

I mean, I guess I can see your perspective. I can't see any noticeable change better or worse in any of our pixel-perfect test projects after making this change, so I guess it couldn't hurt.

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.

3 participants