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

Add option to snap 2D transforms to rounded pixel (instead of using floor) #8318

Open
KeyboardDanni opened this issue Oct 31, 2023 · 4 comments

Comments

@KeyboardDanni
Copy link

Describe the project you are working on

2D game with pixel art

Describe the problem or limitation you are having in your project

When moving my character around with "Snap 2D Transforms to Pixel" enabled in the project settings, the sprite appears to float one pixel above the floor. This is happening because the collision margin is pushing the sprite just slightly above a whole pixel (i.e. 319.940979 instead of 320). It seems like the pixel snap is simply discarding the fraction via floor() instead of rounding up or down to the nearest whole pixel. I would expect a sprite placed at 319.940979 to appear to be at 320, not 319.

Issue doesn't occur when disabling the pixel snap, but then I get shimmering when rotated or scaled sprites move across the screen.

Describe the feature / enhancement and how it helps to overcome the problem or limitation

Add a new boolean project setting, rendering/2d/snap/offset_snap_by_half_pixel. When active, the pixel snap used for drawing 2D graphics will use round() instead of floor(). With this active, a sprite positioned at y = 319.9 will display at y = 320, and one positioned at y = 319.1 will display at y = 319.

Describe how your proposal will work, with code, pseudo-code, mock-ups, and/or diagrams

Mockup:

OffsetPixelSnap

If this enhancement will not be used often, can it be worked around with a few lines of script?

I can think of a few workarounds (shrinking the collision shape slightly, using a script to offset the sprites, offsetting the camera by a half pixel), though these can be awkward to deal with, and I feel like this issue would be prevalent enough that having to constantly apply workarounds would be obnoxious.

Is there a reason why this should be core and not an add-on in the asset library?

Since this touches on a limitation in core functionality, it should also be considered for core.

@KeyboardDanni
Copy link
Author

Related issue: godotengine/godot#56793

@KeyboardDanni KeyboardDanni changed the title Add option to snap 2D transforms to center of pixel (instead of top-left) Add option to snap 2D transforms to rounded pixel (instead of using floor) Oct 31, 2023
@Calinou
Copy link
Member

Calinou commented Nov 1, 2023

I suggest working on a proof of concept to ensure it'll actually resolve the issue you're trying to resolve. 2D pixel snapping is a well-known rabbit hole with plenty of whack-a-mole solutions 🙂

@KeyboardDanni
Copy link
Author

That is understandable. I'll see what I can come up with.

@KeyboardDanni
Copy link
Author

Addressed in godotengine/godot#84380. Decided against having it be a setting because it would probably cause extra code bloat and might also confuse users.

Tested with nearest/linear filtering, AA on/off. It all looks fine (PR doesn't change where it snaps to, just where the snapping boundary is). I saw that there was a previous attempt to make pixel snapping use round(). I'm not sure why this was reverted, because it's working just fine in my testing. If the issue is camera jitter, that's fixed by also updating the viewport snapping to use round().

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

No branches or pull requests

3 participants