Add 2d snap transforms option

[lawnjelly]

This is a cut-down backport of reduz snapping PR #43194.
It just offers a global project setting for transform snapping.

Fixes #35606
Fixes #41491

Notes

• These are based on original idea in Fix for the sprite display position jumps when placed at subpixel locations and moving the camera (3.2) #41535 by @m6502
• This is a cutdown version of the changes in reduz PR, notably there are no options per viewport, and there is no refactoring to pixel snap
• If desired this can be fleshed out a bit more, but this may do the job

[m6502]

Thank you!

[akien-mga]

Thanks!

[AttackButton]

Edit: my fault.

[golddotasksquestions]

I just downloaded the automatically generated editor in the "Action" tab from this fix, and the issue as described in #35606 still exists for me (camera_smoothing enabled, pixelsnap enabled, Win 64, Nvidea.) I also tried "Nvidea Rect flicker workaround", which sounds to me like the solution, but is not.
Lastly I also tried to enable the new "Transform snap" and suddenly no jitter anymore.

Is there a reason this is optional, and worse, turned off by default? If I did not know there was a solution implemented recently and actively searching for it even more -- after -- I thought the fix did not work for me, I would have not guessed this is the checkbox I would have to click.

If there is not some dramatic performance loss, please remove that option and just make it the default. If there are some downsides when using it, still please enable it automatically when using camera smoothing.

@AttackButton I have a Minimal Project if you want to test here:
camera2D_jitter_test_Godot3.2.4.zip

[lawnjelly]

Hi. Is this already in the 3.2.4-beta 2? If so, I can confirm this is not working like #41535.

Just to make clear, this setting is optional: Rendering->Quality->2d->Use_Transform_Snap. I don't know whether you had this set in your test.

Is there a reason this is optional, and worse, turned off by default?

Typically it is a good idea when introducing new features to implement them as optional (at least at first), and then there can be consideration of changing the default. Sometimes new features can have unintended consequences in other areas.

However, at the risk of stating the obvious, consider that not everyone is making a transform snapped 2d game. Transform snapping has implications in the editor and all other 2d rendering. (It also default to off in @reduz PR on master which mine is a backport of.)

[AttackButton]

@lawnjelly Thanks man. It is working now. 👍

[golddotasksquestions]

Typically it is a good idea when introducing new features to implement them as optional (at least at first), and then there can be consideration of changing the default. Sometimes new features can have unintended consequences in other areas.

I very much agree. However in this case the new feature feels more like a long awaited bugfix. A bug that made one of the Engines most appreciated features (it's 2D pixel perfect rendering capability) seem like a fragile house of cards on a shaking table. If it is not on by default will lead to more people wasting their time looking for an assumed bug to fix, and users on the community forums repeating over and over again "you have to turn X on" (those few who know anyway).

So if there is really is no known apparent performance cost at this time or other downsides, can we please have this feature turned on by default for the next release and - if there is no known upside to turning it off - have the option removed in a stable release?

Edit: Here is another person who thinks this new feature or fix as I would call it is not working for them: https://www.reddit.com/r/godot/comments/jxjtgb/gdquest_asked_their_twitter_followers_about_the/gcxnzx3?utm_source=share&utm_medium=web2x&context=3

[AttackButton]

I don't know if I open or not a new issue for this. I think I will let other people notice it too. But in my platformer project the old jittering problem is completely fixed, however now is the Player that jitters a little when it jumps, getting a little blurry.

This can be tested in any platformer, just let the Player fall endlessly from an edge and keep moving to the right or to the left. You will notice that the Player keep jittering in the camera.

[golddotasksquestions]

@AttackButton Can you please share a minimal project?

[AttackButton]

@golddotasksquestions Here. In this one the jittering is there but a little subtle.

KinematicMoving-jitter.zip

[golddotasksquestions]

@AttackButton Thanks for the Minimal Project. I'm not sure I follow what you are saying. When jumping down from the platform there is this jerk every ~1.5 seconds. (Win64, Nvidia) Is that what you mean? I tried to screen record it, but the recording directly affects it, making the jerking look totally different, so there is no point.
Anyway if that's the issue you are describing, then I would believe it is something unrelated to this fix here, but I would not know for sure.

[AttackButton]

I think it is caused by this pr, because if you turn the transform snapping option off , the jittering stops. In my project it is more noticeable than in the uploaded MRP. But except for that, this pr is really good.

[lawnjelly]

Yes, transform snapping has the potential to affect things in multiple areas not initially envisaged. Hence making it optional.

Just off the top of my head you may get more pronounced effects from float error, jiggling when using different scales for different objects / layers, and in your example there may be jiggling if the camera is not being transform snapped and the objects are, etc etc.

[CritCorsac]

I'm also getting a problem similar to the one mentioned above. With this new setting there is a new kind of jittering issue, one that's more subtle and seems to occur in a way that it almost makes objects look blurred when a camera follows them as they're in motion. When running a low resolution pixel game that's been stretched out in fullscreen mode it becomes more noticeable. The moment the new setting is disabled this new jittering issue goes away.

[golddotasksquestions]

@CritCorsac @AttackButton I'm not seeing any blur. Not in my test project and not in the one AttackButton provided. It would be great if you guys could state what OS and graphics hardware you use?

[CritCorsac]

I have created a basic, low resolution pixel platformer using Godot 3.2.4beta2 that helps bring the issue I mentioned to light. In this minimal reproduction project, the jitter is very noticeable when the new transform snap is enabled, but is eliminated entirely when it is disabled.

It is also worth noting that the player appears to be one pixel above the ground when transform snap is enabled but is accurately placed on the ground when it is disabled.

I believe the blur effect is caused by this jittering issue too, as at higher resolutions the rapid one pixel jittering becomes less jarring but instead gives off the illusion of a slight blur to our eyes.

Minimal Reproduction Project
Jittering Camera Bug.zip

[CritCorsac]

I have made some additional discoveries.

First I'll openly admit I'm not very familiar with the Godot source code or C++ in general, but as I was checking through the changes in this pull request I noticed a lot of values were getting floored. So using the minimal reproduction project that I linked to in my previous comment as a base, I decided to make some changes to the camera.

Firstly I removed the camera from the player and instead added it to the main scene. I then gave it a script that simply set its global position to that of the players global position, every frame, then flooring that value.

This removed the jitter, even with transform snap enabled. This leads me to believe that the source code for the camera may also need to be tweaked to have certain values floored, similar to the changes given to the sprites and animated sprites in this pull request, but again, I'm not very familiar with the source code so this is just speculation.

However the other issue that I mentioned in my previous comment is still prevalent. The player is still visibly floating one pixel above the floor when transform snap is enabled.

[AttackButton]

It is also worth noting that the player appears to be one pixel above the ground when transform snap is enabled but is accurately placed on the ground when it is disabled.

I noticed this in my project too. But not only for the Player, but for other KnimaticBodies2D in the scene too. And if you turn transform snap off, they will return to their original positions.

@golddotasksquestions I'm using Windows 8.1/Nvidia.

[lawnjelly]

I spent a bit of time examining the jittering camera bug project.

First I removed all the physics to make it simpler, then reduced the frame and tick rate. Then moved the camera to not be a child of the player to avoid any effects from that:
Jittering Camera Bug.zip

The camera is set each frame to the position of the player, using floor as the same way as the rendering code does. But still there is a jitter. I figured out that I think this is a single frame delay between setting the position of the camera and it taking effect (or reading the position of the player is a frame delayed). When a target is on a boundary between whole numbers this leads to a jitter.

I tested this by artificially putting the camera a frame in advance.
If you change the line in Camera2D.gd from the bottom to the top one:

#	var ix = floor(pos.x + 0.33)
	var ix = floor(pos.x)

You no longer get the jitter.

So in short this is a problem (likely bug), but I'm not sure it is caused by the transform snapping, it may be that the transform snapping makes a pre-existing bug more apparent.

[lawnjelly]

Have found the bug causing a frame delay in updating a camera:

When you change the origin of a camera it sends a NOTIFICATION_TRANSFORM_CHANGED. The problem is here:

void Camera2D::_notification(int p_what) {
		case NOTIFICATION_TRANSFORM_CHANGED: {

			if (!is_processing_internal() && !is_physics_processing_internal())
				_update_scroll();
			else
			{
				print_line("camera missed update");
			}

When I added the debug print, it turns out the camera is not updating, because we have set it either during a physics tick or a frame process. This introduces a delay, presumably until the next NOTIFICATION_INTERNAL_PROCESS or NOTIFICATION_INTERNAL_PHYSICS_PROCESS of the camera.

I'm not 100% sure yet but I'm pretty sure this is the cause. This could be a major bug for 2d games.

Yes I can confirm, if I set it to _update_scroll in all cases, it cures the jitter! 👍 🥳
The question is now how to fix this properly.

And it turns out it is a known bug:
#28492

Can also confirm the workaround, putting:
force_update_scroll() after changing the Camera2D position works in my min reprod project. It may work in the physics case too.

YESS!! It cures the physics too. But to get it working I had to make the camera not a child of the Player, and to update it's position manually each frame, then call force_update_scroll. Works fantastic! 😄

The fact the camera needs to be updated manually, and not as a child of the player for it to work indicates there is also a similar delay involved in the physics causing a similar problem. This could be an order of operations issue, I'm not sure at what point the physics updates the positions of objects in relation to the _process and _physics_process in the scene tree.

[AttackButton]

About the float bug, it is affecting the CollisionShape2D in the Player scene itself (and probably in other object scenes too). However, there was apparently no change in the transform property. The bug seems to only happen on the y axis.

With transform snap disabled:
In the running game:

In the Player scene:

With transform snap enabled:
In the running game:

In the player scene (need to restart the engine to update and see the bug).

[lawnjelly]

@AttackButton Do you have a project for that? This seems to occur in a few projects, and is likely just a consequence of snapping, especially on a boundary. If something is just above 1 it will be 1, and even fractionally below it becomes 0. So if the platform is rounded down, and the player rounded up, you get a 1 pixel gap. This is the nature of snapping.

I'll have a look today at whether it might be more appropriate to use round() rather than floor() (or be able to switch between them), but neither will solve this problem in all situations. If you make pixel art on a whole number aligned grid, then round() may work better.

With the PR I just copied literally from m6502 and reduz (using floor), but now we are getting some feedback we may be able to improve it prior to 3.2.4 release.

[AttackButton]

@lawnjelly Here. In this MRP, the bug is not very noticeable in the main scene. But it is in the Player scene (need to restart the engine when the transform snap is enable).

float-KinematicMoving-jitter.zip

[lawnjelly]

I think the draw is at -23.185 which with floor rounds down to -24 which raises it up. But I can't really debug it easily because it is outside the drawable area. Because godot draws with y increasing downward, the floor will operate in the opposite direction that you expect.

That would be a reason to possibly use ceiling for y, however I've got a PR in to use round instead as it seems to work better overall.

[GhbSmwc]

Hey, uh, as of the current and future versions of godot, would enabling pixel-snap forces all-positioning related controls (margin, size, position, global position etc.) to be an integers? Like if set to 0.5, but returns 0?

[lawnjelly]

Hey, uh, as of the current and future versions of godot, would enabling pixel-snap forces all-positioning related controls (margin, size, position, global position etc.) to be an integers? Like if set to 0.5, but returns 0?

The idea is for the snap to affect the drawn positions of objects, but not the stored position. Pixel snapping (historical name) is really vertex snapping to pixels in the vertex shader. Transform snapping is snapping the transform on the CPU before sending to the GPU.

[GhbSmwc]

@lawnjelly Oh, I see, this is just the drawing, nothing to do with the storing of positions. So the stored positions of stuff is handled by the user's code and it is up to them to decide floats or ints.

[oeleo1]

@lawnjelly Interesting thread. Am I correct in deducing that by using the Camera2D force_update_scroll() workaround we can compensate for the potential jitter of the 1 frame camera delay even without using the pixel-snap machinery?

And if so, the following in Camera2D would be good enough:

    func _process(delta) -> void:
        transform.origin = player.transform.origin
        force_update_scroll()

[lawnjelly]

Haven't worked on this for a bit, but yes, as far as I can remember you may be best off using force_update_scroll and manually setting the camera, regardless of whether you are using snapping. That was the best results I could get.

As well as the projects I posted here, take a look at the example projects I posted in #44690.