Replace floor() with round() in pixel snap to transform

[KeyboardDanni]

Fixes #56793.
Fixes #84632.

Similar to #76277 except this PR also makes the necessary changes to renderer_viewport.cpp to avoid regressions in camera jitter. In fact, with these changes, the camera is actually more stable according to my testing.

Before:

2d_pixel_snap_before.mp4

After:

2d_pixel_snap_after.mp4

NOTE: You will still get camera jitter if you are using a script to make a camera follow an object that is moved in idle process. Camera movement is fine if the object is moved in physics process. This is an existing issue which will be addressed in a separate PR.

Test project download:

PixelPerfectTest.zip

Production edit: added fixes #84632

[lawnjelly]

Related #76277 , and #57221 (comment) .

[markdibarry]

Looks good! Results seem much more consistent. If there are issues I haven't found, the current implementation isn't usable as is, so this is still a big improvement.

[Cerno-b]

General thought/question: When transitioning from float to pixel coordinates, is Godot using a pixel-centered or top-left coordinates in general?

This PR seems to assume pixel-centered (rounding instead of flooring).

I am asking because I haven't found any specs on this in the docs and I think it should be defined somewhere for consistent behavior. All sorts of problems can occur if it's sometimes centered and sometimes top-left.

This question does not block the PR, but it may be worthwhile to check if there are other places where float to pixel coordinate transition occurs and change them accordingly.

[KeyboardDanni]

My understanding is that it's using top-left for pixel coordinates, but this PR just changes when rounding occurs. That is, an object at x = 1.8 gets rounded to 2.0 instead of 1.0. Objects still snap to 1.0 or 2.0; they don't snap to 1.5, for example, with this change. With this rounding you'll want to place your objects at whole pixel coordinates for best results instead of offsetting their position by half a pixel.

Of course I imagine in practice the pixel offset might not actually be half a screen pixel and this would come down to how the pixel center is handled in either the vertex shader, 3D API, or drivers. That's an area I'm less familiar with.

[carlOSx64]

I did a custom build using your fork and it fixed all the issues I've had with pixel perfect rendering. The options right now in Godot are not working properly and you're better off using custom code in _process functions. I hope this gets merged soon.

[Cerno-b]

@KeyboardDanni I experimented a bit with Godot and I agree that it's top-left (corner pixel's top left is [0.0, 0.0] and bottom right is [0.99, 0.99]). Under that regimen, flooring should be correct to get from float it int.

From what I've read so far, that's the way OpenGL, Direct3D 10 and Vulcan does it as well.

But don't let this hold up the PR: If rounding works here, I think it's pragmatic to merge this if it solves the current issues.

We can always go deeper into the whys and hows with a separate proposal. I noticed another weird inconsistency in float to int, so looking at this from first principles is probably a good idea, but outside of the scope of this PR.

[KeyboardDanni]

@KeyboardDanni I experimented a bit with Godot and I agree that it's top-left (corner pixel's top left is [0.0, 0.0] and bottom right is [0.99, 0.99]). Under that regimen, flooring should be correct to get from float it int.

I think you might be confusing two different concepts here. There's the rounding behavior, i.e. the criteria that determines where to round up or down. Then there's the offset that we're rounding to, i.e. whether we round to top-left or center pixel. This PR focuses on the former.

Without this PR, the current rounding behavior means that graphics snapped via top-left pixel are going to be put right up next to the rounding boundary. This means if you have an object at position 1.0, the moment it moves to 0.999 it'll suddenly snap to 0.0. I theorize that this floor snapping, combined with floating point precision errors, is responsible for the shakiness you see in the first video.

With this PR, an object at position 1.0 needs to travel at least half a pixel before it gets rounded away from 1.0, increasing stability. Visualized:

[Cerno-b]

@KeyboardDanni Thanks for the clarification. I think I may have lost myself a bit in the different kinds of coordinate systems:

https://www.realtimerendering.com/blog/the-center-of-the-pixel-is-0-50-5/
https://learn.microsoft.com/en-us/windows/win32/direct3d10/d3d10-graphics-programming-guide-resources-coordinates

From that I was under the assumption that you could either put your pixel centers at (0.5, 0.5) which would always require flooring or you could put your pixel centers at (0.0, 0.0) which would always require rounding. But this is probably a different concept from the problem that you are solving and I may have mixed up the two.

With the image examples, I see how flooring causes problems and how your rounding solves it.

[KeyboardDanni]

you could either put your pixel centers at (0.5, 0.5) which would always require flooring or you could put your pixel centers at (0.0, 0.0) which would always require rounding.

This sounds about right, yeah.

[adamscott]

Absolutely great! It seems a lot nicer with your PR, indeed.

But I tested it with @HybridEidolon's PixelPerfect.zip (from godotengine/godot-proposals#6389 (comment)) and the player's sprite falls through the ground with your PR.

I'll investigate on why changing the offset causes this side-effect.

[adamscott]

Nevermind, I rebased your PR and it seems to work with the PixelPerfect.zip project mentionned before.

@KeyboardDanni Can you rebase your PR?

[adamscott]

Your PR seems to fix Rubonnek's PixelPerfectJitter.zip (from godotengine/godot-proposals#6389 (comment)) too!!

[adamscott]

I'm wondering though if:

• it could break existing projects;
• it would be better to create a project setting for the pixel snapping method.

[adamscott]

@clayjohn I closed #86670 because this PR seems better suited. It already deals with the snapping options, without having to resort to round "low-level" in the driver.

[markdibarry]

@adamscott

I'm wondering though if it could break existing projects

I think the risk level is low since the code changes only affect blocks where it's been established that pixel snap is turned on.

it would be better to create a project setting for the pixel snapping method.

I believe it was decided that this is a better solution, for various reasons mentioned here: godotengine/godot-proposals#8318

[kleonc]

Looking at this PR I've realized a big issue with the current 2D transform snapping for Sprite2D and alike: it's... applied twice. 🤦‍♂️

• Firstly, it's applied when calculating the destination rect for the draw texture rect command.
• Secondly, it's applied to the Sprite2D's transform within the rendering server (transform snapping is also applied to the transform of each parent CanvasItem in the chain but it's the double-snapping for the Sprite2D itself which is absolutely wrong).

Meaning an already snapped draw texture rect command is being drawn according to a snapped Sprite2D's transform. Given that before this PR these were flooring it can very easily result in big discrepancies.

Simple example, a Sprite2D with 1x1 texture, v4.2.1.stable.official [b09f793] (so snapping by flooring):

snap_2d_transforms_to_pixel=false	snap_2d_transforms_to_pixel=true (by flooring)

Kinda surprising where it ended up being rendered, huh? 🙃 Nearly 2 pixels away on each axis.

---

Is this still an issue if snapping is done with rounding instead of flooring? Yes, it is. The difference is that for such single double-snapping:

• by flooring error < 2.0,
• by rounding error <= 1.0.

(by error I mean per axis distance to the original unsnapped rect)

Same 1x1 texture, but with this PR (so snapping by rounding):

snap_2d_transforms_to_pixel=false	snap_2d_transforms_to_pixel=true (by rounding)

Nearly 1 pixel away, yikes!

---

Regardless, just changing flooring to rounding seems like an improvement / good first step. So it's probably fine to merge this as is, and fix the mentioned double-snapping in a subsequent PR (unless the author wants to tackle it at once in here; either one is fine for me 🙂).

I'm wondering though if:

• it could break existing projects;

Let's keep it in 4.3+ (no backports) and I guess it should be fine? Still, it's more like a bugfix/improvement and I think potential breaking by bugfixing is expectable/acceptable. 🤔 Just because users found a workaround for an issue (and use it in their projects) doesn't mean the issue shouldn't be tackled/fixed.

[KeyboardDanni]

If I had to guess, the additional snapping done in the Sprite2D code is probably meant to address the corner case of having a sprite with an odd-number size that is centered on the Node2D position in space. This needs additional investigation, though.

[markdibarry]

@KeyboardDanni In case you missed it in the last few comments. Could you rebase your PR?

[adamscott]

I tested out your PR against Scalazard, an open-source pixel perfect game. (cc. @nicolasbize)

It seems to fix a weird issue where, when pushing a box, the player and the box would jitter a bit.

Without the PR (floor)

GodotRoundVsFloor-floor.mp4

With the PR (round)

GodotRoundVsFloor-round.mp4

[nicolasbize]

@adamscott that's really cool to see :)

[adamscott]

I would suggest you all to checkout #87058. It seems that a fix was introduced in 3.x that was not forward-ported to 4.x.

It supersedes this PR, as we don't need to round values as it removes the last transformations in the DisplayServer that did break everything.

Could you test your games/projects with that PR? Thanks!

[Proggle]

I would suggest you all to checkout #87058. It seems that a fix was introduced in 3.x that was not forward-ported to 4.x.

It supersedes this PR, as we don't need to round values as it removes the last transformations in the DisplayServer that did break everything.

Could you test your games/projects with that PR? Thanks!

Doesn't seem to fix my distortion, snapping modes don't fix (but cause different things to jitter), mode is integer scaling, viewport scaling, etc.

Note that the hill to the right and the grass lower than it are all on the same tilemap layer, so they really shouldn't have any way to be misaligned relative to each other. Was hoping it was just a rounding thing.

[akien-mga]

Superseded by #87297. Thanks for the contribution!