Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[3.2] Improve 2d snapping #44690

Merged
merged 1 commit into from
Jan 5, 2021
Merged

Conversation

lawnjelly
Copy link
Member

@lawnjelly lawnjelly commented Dec 25, 2020

Partially revert change allowing sprite get_rect snapping to be controlled by pixel_snap again rather than transform_snap (to prevent breaking compatibility). Adds a final use_camera_snap project setting to allow snapping viewports as in reduz original PR.

Based on feedback these are some improvements to #43554 (which is a backport of #43194).

Fixes #44686

This user was unable to reproduce the same behaviour as 3.2.3. This is a valid complaint.

Although moving the get_rect snapping to the transform_snap project setting was logical (in #43194), it loses some flexibility. Ultimate flexibility would be to have a setting for each, but in practice I currently believe most people using transform_snap will also be using pixel_snap, so this will save an extra project setting.

Viewport (Camera) Snapping

This is a little more controversial in my opinion, and I accidentally / on purpose missed it out of the original PR. But I guess it is ok here with a separate project setting.

There are current two problems with camera snapping:

  1. There is currently another bug (likely in the update order) which can cause jittering between camera position and target position (may be the force_update_scroll() bug Camera2d has a one frame delay #28492)
    2D physics objects have delay to their positions #43800 (comment)
    99997767-8a722900-2db5-11eb-84f3-4e70e0988fe9

  2. Using the camera_snap option means you cannot smoothly interpolate the camera in 'sub texel' movement (assuming blocky pixels, which I will refer to as texels here). This means you can't have smoothly interpolating cameras or smooth camera 'shock' effects.

You can achieve (2) by doing camera snapping manually however (see #43554 (comment)), and also bypass the bug of (1). However this is more advanced, and may be beyond beginner users to understand and pull off.

See this example project for an example of manual camera snapping:
Jittering Camera Bug Workaround.zip

In this same project if you change the active camera to the camera hanging off the player, and switch on camera snapping, you will likely see bug (1) exhibiting. This bug is more apparent when the physics tick rate diverges from the refresh rate.

Also the very act of merging camera_snapping may also expedite fixing bug (1). It is a chicken and egg situation, as few are aware of the bug at the moment and the need to fix it. I haven't fixed it as part of this PR because I'm unsure of the exact cause as yet.

Extra Demo

This shows (2) nicely. By doing manual camera snapping, it allows the camera to exactly follow a player, but apply a float offset. Note that if you turn on the camera_snapping in the project settings of this project, you will see the limitation (you lose the ability to have float offsets). This is a large part of the reasoning to have a separate setting for camera_snapping from transform_snapping.

Jittering Camera Bug Workaround2.zip
smooth_cameraB

Partially revert change allowing sprite get_rect snapping to be controlled by `pixel_snap` again rather than `transform_snap` (to prevent breaking compatibility). Adds a final `use_camera_snap` project setting to allow snapping viewports as in reduz original PR.
@ghost
Copy link

ghost commented Dec 26, 2020

For the #28492 bug, we have been using our solution since we opened the pull request #43995

It is safe in our case and it didn't caused any problems.

We hope to get review from reduz soon.
Thanks.

TechnoArt Studio

@akien-mga akien-mga merged commit 3032b38 into godotengine:3.2 Jan 5, 2021
@akien-mga
Copy link
Member

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants