Change 2d transform snapping from floor to round #43813

lawnjelly · 2020-11-24T10:39:43Z

Two common problems have emerged as a result of transform snapping:

Camera jitter with a camera following a snapped object
Pixel gaps between e.g. a platform and a player, where a platform rounds down and a player rounds up

Using round seems to greatly reduce problems due to camera jitter. It also may prove better for pixel gaps because pixel art is often designed on a grid, so whole numbers are too expected, which are unstable with floor().

Fixes @CritCorsac camera jitter in #43554
Fixes worst effects of #43800
Fixes #44048
Fixes #46504

Notes

I started by making a PR which enabled switching between floor and round for transform snapping
While testing this switchable PR it became apparent that round() seemed to work much better in all tests, especially reducing camera jitter

See first section of this gif to see camera jitter, and 2nd half shows the effect this PR would have:

So instead this PR simply replaces the existing floor method. Alternatively I could push the PR with selectable method if preferred, but in tests so far, I haven't found a situation where floor was better, and it might save on some confusion for users.

reduz · 2020-11-24T13:03:44Z

This does not make much sense to me, afaik the camera (canvas transform) should also be floor()ed, so it may be possible this is what is failing?

AttackButton · 2020-11-24T14:58:44Z

Ok. Apparently, with this PR and Camera Smoothing disabled, and rounding x and y again and using the force_update_scroll() in the camera script solved the problem completely in my project.

I think the Camera Smoothing is a problem a part from the transform snap.

MRP with the round positions.
KinematicMoving-jitter-round2.zip

AttackButton · 2020-11-24T17:08:20Z

The workaround from the previous post with this PR looks to work for the Player jittering + floating "bug", but even with the #43554 version of the transform snap, the situation below occurs.

With transform snap enable objects (not only platforms) that are close and at the same speed as the Player keep jittering.

This PR with transform snap enable and round x and y.

This PR with transform snap disabled and camera x and y = with no round x and y. It seems to be working just like godot-3.2-stable.

The MRP:
MovingPlatforms-jitter-round3.zip

lawnjelly · 2020-11-25T09:56:53Z

Yes the camera / canvas snapping being left separate up to now how been partly intentional, I mentioned the need in comments on the original PR (although I did miss copying the version from your PR). I'm still not entirely convinced whether it should be fixed on at all times as your suggestion, or whether we might lose some flexibility by doing that, perhaps having an option in the 2d camera or something. We could certainly fix it to start with though.

I've hence been doing the camera snapping manually in the test projects I have been posting (in gdscript), but it is clear the users don't all understand this bit, so it will be nice to automate.

Unfortunately with the camera / canvas snapping there is however another bug which seems to affect it, shown in the gif here:
#43800 (comment)

I suspect this is an entirely separate issue caused by order of processing, which may be beyond me to fix easily (outside my knowledge area of godot).

As an aside though, irrespective of the canvas snapping, I still suspect there may be practical advantages to using round over floor (in terms of users building on whole number grids). Using floor on a whole number coordinate afaik will be the most unstable configuration, and round() the most stable. Although round or floor shouldn't really matter if users were building levels freeform rather than on whole number grid. However we can revisit this later if necessary.

In general I can foresee minor physics vibrations being a real problem with snapping as objects cross boundaries. It will probably be more suited to games where people are doing movement code themselves rather than using physics.

lawnjelly · 2020-11-25T09:57:39Z

Will close this for now, can always be reopened if we revisit.

lawnjelly · 2020-11-25T10:09:25Z

@AttackButton Yes I haven't looked in detail, but transform snapping will be expected to produce unwanted vibrations with physics when objects cross boundaries. This is one of the downsides of snapping. Generally I don't think it will be a good match with physics, especially for that type of camera.

It is important to realise, despite the wishes and hopes of users, transform snapping will not automatically solve all problems in 2d games, and it should not be expected to. Although it helps in some ways, it introduces problems of its own too.

Your comparison version with snapping off you won't see it because these vibrations may only be very small in floating point units, and hence not show without snapping.

lawnjelly · 2021-02-28T10:17:31Z

Reopening because I think this needs a second look, I'm just rebasing it.

This is taking some time as there have been quite a few PRs / issues since, and I'm going over all the various test projects and issues since to try and make sure everything works fine, marking as WIP until it is ready.

Just for reference am adding the PRs related to snapping here, and issues:

Issues

#35606
#41491
#44686
#43800
#44048
#46504
#28492

PRs

#43554
#44690
#43995

Two common problems have emerged as a result of transform snapping: 1) Camera jitter with a camera following a snapped object 2) Pixel gaps between e.g. a platform and a player, where a platform rounds down and a player rounds up Using round seems to greatly reduce problems due to camera jitter. It also may prove better for pixel gaps because pixel art is often designed on a grid, so whole numbers are too expected, which are unstable with floor().

lawnjelly · 2021-02-28T14:39:17Z

It seems like we are all agreed to test this for the next RC. 👍

The only bit I'm slightly unsure about is the using floor when pixel_snap (snap in the vertex shader, name preserved for compatibility). I've left it the same as before when snap_transforms is not in use, in case it was needed for historical reasons. We could perhaps try removing the pixel_snap version at a later date, it may not perform any useful function.

I've also standardized all these bits to use the same snippet of code as it seemed to be a bit haphazard (used some places and not others).

akien-mga · 2021-02-28T22:13:47Z

scene/2d/animated_sprite.cpp

 			if (Engine::get_singleton()->get_snap_2d_transforms()) {
+				ofs = ofs.round();
+			} else if (Engine::get_singleton()->get_use_pixel_snap()) {
 				ofs = ofs.floor();


Is the current documentation clear enough about how 2d transform and pixel snapping will behave differently, including this new difference? (And the fact that 2d transform has precedence.)

To be honest I'm not convinced that we even need the floor with the pixel snap (which is actually done in the vertex shader). But I didn't want to remove it in this PR, just in case there is an actual need for it, we can evaluate removing it in a separate PR.

Perhaps I should take the opportunity to rename pixel_snap, at least internally because it seems to be causing confusion. The name is historical, but really vertex_shader_snap or something might be better. Or the name reduz used in his PR I will check that.

In either case I don't think mentioning it in the docs is wise at this stage until it is finalized. snap_2d_transforms does not totally take precedence, only here on CPU. On the GPU, pixel_snap still acts alongside the CPU snapping.

akien-mga · 2021-03-01T07:54:46Z

Thanks!

m6502 · 2021-03-02T01:58:03Z

Wouldn't using round instead of floor make something that moves from 0.0 to 0.1 be drawn at 1.0? Doesn't sound right to me.

lawnjelly · 2021-03-02T07:33:19Z

Wouldn't using round instead of floor make something that moves from 0.0 to 0.1 be drawn at 1.0? Doesn't sound right to me.

Round rounds 0.1 to 0.0.

https://www.programiz.com/cpp-programming/library-function/cmath/round

m6502 · 2021-03-02T09:14:10Z

Round rounds 0.1 to 0.0.

Oh, was thinking about ceil(), you are right.

akien-mga added bug topic:rendering labels Nov 24, 2020

akien-mga added this to the 3.2 milestone Nov 24, 2020

akien-mga requested review from clayjohn and reduz November 24, 2020 11:03

lawnjelly closed this Nov 25, 2020

akien-mga added the archived label Nov 25, 2020

This was referenced Feb 28, 2021

Fix for the sprite display position jumps when placed at subpixel locations and moving the camera (3.2) #41535

Closed

transform snap yields 1px offset below and to the right of kinematicbody2d for pixel platformer #46504

Closed

lawnjelly reopened this Feb 28, 2021

lawnjelly force-pushed the snap_2d_round branch from cd1b81d to 003d973 Compare February 28, 2021 11:17

lawnjelly requested review from a team as code owners February 28, 2021 11:17

lawnjelly changed the title ~~Change 2d transform snapping from floor to round~~ [WIP] Change 2d transform snapping from floor to round Feb 28, 2021

lawnjelly mentioned this pull request Feb 28, 2021

Pixel snap interferes with Sprite offset rounding on Y axis #44048

Closed

lawnjelly force-pushed the snap_2d_round branch from 003d973 to 847a37b Compare February 28, 2021 14:35

lawnjelly changed the title ~~[WIP] Change 2d transform snapping from floor to round~~ Change 2d transform snapping from floor to round Feb 28, 2021

akien-mga reviewed Feb 28, 2021

View reviewed changes

akien-mga removed the archived label Mar 1, 2021

akien-mga approved these changes Mar 1, 2021

View reviewed changes

akien-mga merged commit 4fcd848 into godotengine:3.2 Mar 1, 2021

akien-mga mentioned this pull request Mar 1, 2021

2D physics objects have delay to their positions #43800

Closed

lawnjelly deleted the snap_2d_round branch March 1, 2021 08:34

akien-mga modified the milestones: 3.2, 3.3 Apr 20, 2021

Calinou mentioned this pull request Jan 25, 2022

Rotation plus motion causes jitter in pixel-perfect mode #57221

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Change 2d transform snapping from floor to round #43813

Change 2d transform snapping from floor to round #43813

lawnjelly commented Nov 24, 2020 •

edited

Loading

reduz commented Nov 24, 2020

AttackButton commented Nov 24, 2020 •

edited

Loading

AttackButton commented Nov 24, 2020 •

edited

Loading

lawnjelly commented Nov 25, 2020

lawnjelly commented Nov 25, 2020

lawnjelly commented Nov 25, 2020

lawnjelly commented Feb 28, 2021 •

edited

Loading

lawnjelly commented Feb 28, 2021

akien-mga Feb 28, 2021

lawnjelly Mar 1, 2021

akien-mga commented Mar 1, 2021

m6502 commented Mar 2, 2021 •

edited

Loading

lawnjelly commented Mar 2, 2021

m6502 commented Mar 2, 2021

Change 2d transform snapping from floor to round #43813

Change 2d transform snapping from floor to round #43813

Conversation

lawnjelly commented Nov 24, 2020 • edited Loading

Notes

reduz commented Nov 24, 2020

AttackButton commented Nov 24, 2020 • edited Loading

AttackButton commented Nov 24, 2020 • edited Loading

lawnjelly commented Nov 25, 2020

lawnjelly commented Nov 25, 2020

lawnjelly commented Nov 25, 2020

lawnjelly commented Feb 28, 2021 • edited Loading

Issues

PRs

lawnjelly commented Feb 28, 2021

akien-mga Feb 28, 2021

Choose a reason for hiding this comment

lawnjelly Mar 1, 2021

Choose a reason for hiding this comment

akien-mga commented Mar 1, 2021

m6502 commented Mar 2, 2021 • edited Loading

lawnjelly commented Mar 2, 2021

m6502 commented Mar 2, 2021

lawnjelly commented Nov 24, 2020 •

edited

Loading

AttackButton commented Nov 24, 2020 •

edited

Loading

AttackButton commented Nov 24, 2020 •

edited

Loading

lawnjelly commented Feb 28, 2021 •

edited

Loading

m6502 commented Mar 2, 2021 •

edited

Loading