[4.3beta3] Sprite pivot changed since 4.3beta2

[JUSTCAMH]

Tested versions

• Reproducible in 4.3.beta3
• Not reproducible in 4.2.1-stable
• Not reproducible in 4.3.beta2
• Not reproducible in 4.3.beta1

System information

Godot v4.3.beta3 - Windows 10.0.19045 - GLES3 (Compatibility) - NVIDIA GeForce RTX 3070 (NVIDIA; 31.0.15.3619) - 11th Gen Intel(R) Core(TM) i7-11700K @ 3.60GHz (16 Threads)

Issue description

Since upgrading to 4.3.beta3 a lot of the sprites in the game I've been working on have been visually offset. The data for the sprite has not changed or corrupted, you can freely switch between 4.3.beta2 and 4.3.beta3 to see the sprite offset.

Below is what this looks like in 4.3.beta2 (and in 4.2.1-stable):

And below is how it looks in 4.3.beta3, note that it is no longer centred on the xy axis.

Steps to reproduce

• Open the MRP below in 4.2.beta2. In the test_scene.tscn is a sprite which is centred on the xy axis.
• Open the MRP in 4.3.beta3. The sprite is no longer centred.

Minimal reproduction project (MRP)

Sprite Pivot Bug.zip

[Chaosus]

Bisected to 6b17d51

[alvinhochun]

The issue is with rounding of the sprite offset for both Sprite2D and AnimatedSprite2D. Since the sprite is 1px in size, the calculated offset when centred is -0.5, which in theory means the sprite's anchor should be at the centre of the sprite. But because of pixel snapping, the code decides it needs to snap the offset to an integer value, so it either snaps left or right (considering the x-axis). Mind that this snapping is local to the Sprite2D, which means it does not consider the sprite transform's scale.

In 4.2 and earlier, this snapping used floor(x), which means even a -0.1 offset would snap the sprite to the left. In #87297 it was changed to round(x) which makes more sense. Crucially, the halfway value -0.5 is still being snapped to the left.

#93740 aimed to address the inconsistent behaviour of snapping the halfway values of canvas items, but at the same time it also changed the snapping of the sprite offset. This changed the snapping of -0.5, which is what caused this issue. (Side note, #93712 is another issue caused by inconsistent snapping of halfway values.)

Consider the simpler case with no scaling: Say we have a 3x3 sprite at the origin, with the default centre offset by -1.5,-1.5, how do we want the sprite to be snapped?

Left is if we fix this bug, right is the current (bugged) behaviour.
Now there may not be an obvious answer at first. It'll be clearer if we move the sprite to 0.5,0.5:

Again, left is if we fix this bug, right is the current (bugged) behaviour.
I think the left is indeed more desirable.

Edit: Oh no, apparently I mixed up the two screenshots for the first case...

[markdibarry]

@alvinhochun I'm off work today so I'll look at this a little too. I'd first like to get a good grasp of how this is reproduced. The MRP is very specific and has a lot of odd project settings for this, with the specific scene scaled 20x and offset. I'm going to make a cleaner MRP because I suspect all of this isn't necessary to reproduce. I'll then test it against 4.2 and 4.3 and your fix against 4.2 and 4.3 for the pixel snap tests to make sure there's no regressions.

[markdibarry]

Okay, the MRP is actually very simple to make. I just made a 1x1 sprite from a PlaceholderTexture2D and turned on pixel snap.
sprite-snap-mrp.zip

If you run this in 4.2, you'll see that half values round down (actually all values round down in 4.2, but for the sake of the issue...), and in 4.3 half values round up. This is more apparent when you take a 1x1 pixel sprite and scale it 20x and then offset it like in the images in the issue description, but that's a very specific case. You can see this also just as clear by changing the placeholder texture from 1x1 to 2x2 or 3x3 etc and turning the centering on and off.

It's hard for me to say that this is a regression any more than the fact that 0.6 rounds up instead of down now, because it's an arbitrary tie-breaking rule for exact half values. This is expected behavior and one of the problems the pixel snapping changes intended to fix. In most cases in the wild you'll see round() methods round up for half values. Another common one is "banker's rounding" which rounds to the nearest even number which also may be a good choice for centering, but I'm not sure. Either way, it sounds like Godot's internal rounding oddly rounds down for half values?

We could change all floors to ceils but the intent of #87297 was to change how snapping works to be more consistent and just personally I think rounding up is the more common tie-breaker for 0.5. It doesn't seem like any of this is critical, but if Godot's internal round() and GDScript's round() both round down for half values, I guess we should make it consistent with that. I'll have to check though.

EDIT: I checked how GDScript and Godot internally handles half value rounding and it looks like they both round up, so this would be the odd one out if we change it to round down.

EDIT2: Actually it's round away from zero, so either way it's inconsistent.

[markdibarry]

Discussed the other day and agreed this is expected behavior with the intentional changes to snapping for 4.3.

4.2 and before with pixel snapping:

 0.9999 ->  0.0
 0.5000 ->  0.0
 0.4999 ->  0.0
-0.0001 -> -1.0
-0.4999 -> -1.0
-0.5000 -> -1.0

4.3 before floor(x + 0.5) fix:

 0.9999 ->  1.0
 0.5000 ->  1.0
 0.4999 ->  0.0
-0.0001 ->  0.0
-0.4999 ->  0.0
-0.5000 -> -1.0

4.3 after fix:

 0.9999 -> 1.0
 0.5000 -> 1.0
 0.4999 -> 0.0
-0.0001 -> 0.0
-0.4999 -> 0.0
-0.5000 -> 0.0

[alvinhochun]

After some discussions and considerations I do incline to consider this not a bug, because the previous changes intentionally fixed some inconsistent behaviour. This can be worked around by disabling "centered" and manually setting an offset with integer values.

[KeyboardDanni]

Hmm, for cases where numbers cause offsets in reverse (like pivots), should we replace floor(x + 0.5) with ceil(x - 0.5)? This would allow -0.5 to round to -1 specifically for those scenarios. It would also maintain the consistent behavior across both positive and negative signs that the "floor and offset by half" fix was designed to address.

[alvinhochun]

Hmm, for cases where numbers cause offsets in reverse (like pivots), should we replace floor(x + 0.5) with ceil(x - 0.5)? This would allow -0.5 to round to -1 specifically for those scenarios. It would also maintain the consistent behavior across both positive and negative signs that the "floor and offset by half" fix was designed to address.

This is what #94305 tried to do. The problem is that the offset is not setting the pivot point with respect to the top-left corner of the sprite, but actually moving the top-left corner of the sprite with respect to the node origin. Changing how it is rounded makes the offset rounding inconsistent with the canvas item position rounding.

[adamscott]

After noticing that all the 2D team agree on this issue, I'm closing this issue as "as intended". We deeply refactored (and fixed) "Snap 2D Transforms to Pixel", so these kind of issues are expected.

To me, the new behavior is behaving correctly. The old behavior was a bug.