Refactor pixel snapping. #43194
Refactor pixel snapping. #43194
Conversation
reduz
force-pushed
the
pixel-snap-refactor
branch
from
f988778 to
df4f610
Compare
|
@RamaStudio I am not exactly sure what I should be looking at. 4.0 prior to this PR did not have pixel snap implemented, so even if you turn on the setting it was the same as just disabling it (made no difference). If you are using filtered textures, you should probably not enable snapping. Snapping will be mostly useful for pixel art. This PR aims to solve the problems with motion (see the referenced issues/PRS0, not with scaling. Pixel (no filter) scaling will always be wobbly, not sure if anything can be done about that. |
|
Also you should most definitely not be using vertex to pixel snap for pixel art. Use the new transforms to pixel snap instead. |
|
Ok just so we are on the same page, there are two options here: The first one is meant to solve #41535. With it, the demos run ok, no jitter between objects reaching the next pixel at different times. When you turn it on, all reach the next pixel at the same time. This is definitely meant for pixel art, if your texture has filtering enabled you may not want to use this. The second is the same as the old "pixel snap" in Godot 3.x and should work the same. It wasn't being enabled but it should use the same code as 3.2 so I assume it works. If it does not, it's most likely a bug and it needs to be fixed. |
|
I would love to test this. It's a huge problem in my game. |
| @@ -3419,6 +3440,8 @@ void Viewport::_bind_methods() { | |||
| ADD_PROPERTY(PropertyInfo(Variant::OBJECT, "world_2d", PROPERTY_HINT_RESOURCE_TYPE, "World2D", 0), "set_world_2d", "get_world_2d"); | |||
| ADD_PROPERTY(PropertyInfo(Variant::BOOL, "transparent_bg"), "set_transparent_background", "has_transparent_background"); | |||
| ADD_PROPERTY(PropertyInfo(Variant::BOOL, "handle_input_locally"), "set_handle_input_locally", "is_handling_input_locally"); | |||
| ADD_PROPERTY(PropertyInfo(Variant::BOOL, "snap_2d_transforms_to_pixel"), "set_snap_2d_transforms_to_pixel", "is_snap_2d_transforms_to_pixel_enabled"); | |||
| ADD_PROPERTY(PropertyInfo(Variant::BOOL, "snap_2d_vertices_to_pixel"), "set_snap_2d_vertices_to_pixel", "is_snap_2d_vertices_to_pixel_enabled"); | |||
There was a problem hiding this comment.
Seems like you forgot the method bindings for those?
| @@ -3577,6 +3600,8 @@ Viewport::Viewport() { | |||
| debug_draw = DEBUG_DRAW_DISABLED; | |||
|
|
|||
| snap_controls_to_pixels = true; | |||
| snap_2d_transforms_to_pixel = false; | |||
There was a problem hiding this comment.
You forgot to initialize the other one.
|
@RPicster builds artifacts are available for this PR to test: Windows: https://github.com/godotengine/godot/actions/runs/336674241 Go into "Show all checks" → "Details" for each platform of interest → "Artifacts". |
-Rename pixel_snap to snap_2d_to_vertices -Added snap_2d_to_transforms which is more useful Fixes godotengine#41814 Solves proposal godotengine/godot-proposals#1666 Supersedes godotengine#35606, supersedes godotengine#41535, supersedes godotengine#41534
reduz
force-pushed
the
pixel-snap-refactor
branch
from
df4f610 to
0e66645
Compare
|
@RPicster This pull request is based on the |
|
Can the "viewport size = window size" requirement for pixel perfect get documented? |
| Transform2D xform = p_transform * ci->xform; | ||
| Transform2D xform = ci->xform; | ||
| if (snapping_2d_transforms_to_pixel) { | ||
| xform.elements[2].floor(); |
There was a problem hiding this comment.
I've just been backporting this to 3.2, and noticed, there is an error here which stops the whole method from working.
This line needs to be xform.elements[2] = xform.elements[2].floor();, as it stands, this line is a no-op. I've made this mistake more than once with godot funcs myself! 😁
|
@reduz I've pointed out small typo which may be needed for this to work in master branch. I can't currently run 4.0 so anyone on 4.0 (@akien-mga ?) can make a small PR to fix this. |
This is a cut back backport of reduz snapping PR godotengine#43194. It just offers a global project setting for transform snapping.
|
@reduz Should those issues/PRs you linked be closed? |
For the issues at least an equivalent PR still has to be finalized / merged for 3.2 first I think (this one can't be cherry picked super easily because there are some differences). |
This is a cut back backport of reduz snapping PR godotengine#43194. It just offers a global project setting for transform snapping.
-Rename pixel_snap to snap_2d_to_vertices
-Added snap_2d_to_transforms which is more useful
Fixes #41814
Solves proposal godotengine/godot-proposals#1666
Supersedes #35606, supersedes #41535, supersedes #41534