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] Better damping implementation for Bullet rigid bodies #39084

Merged
merged 2 commits into from
Jul 2, 2020

Conversation

madmiraal
Copy link
Contributor

Backport of #37314.

Includes a separate commit, required to comply with new black formatter rules.

@madmiraal madmiraal requested a review from AndreaCatania as a code owner May 27, 2020 11:15
@akien-mga
Copy link
Member

akien-mga commented May 27, 2020

This only works if bullet is updated too, no? It depends on #38253.

See #37314 (comment). The added compilation flag does nothing in stable Bullet 2.89 which we use in the 3.2 branch.

@pouleyKetchoupp
Copy link
Contributor

I wouldn't recommend porting the whole Bullet update to 3.2 since we're currently not using a stable version on 4.0 and it could have unexpected results. In this case it would be better to only backport the damping flag change (I don't mind making a PR for that if we do want the damping changes in 3.2).

Also, in order to make things consistent between Bullet & Godot Physics, #37880 would be needed as well (it's not merged to 4.0 yet).

Next time I'd like to be mentioned when one of my PRs is being backported.

madmiraal added 2 commits May 27, 2020 17:50
make it easier to tweak and consistent with Godot Physics.

Include patch applied to Bullet master to enable dampings greater than 1.
@madmiraal madmiraal requested a review from akien-mga as a code owner May 27, 2020 17:04
@madmiraal
Copy link
Contributor Author

I’ve updated this PR to include the patch that enables the use of the define.

Note: #37314 fixes #19182 and #30991, but it also fixes the fact that the project’s default damping settings were not being used.

To fix #19182 and #30991 required using a different damping algorithm in Bullet physics. Technically a breaking change, but users using the default settings are unlikely to notice a 0.4% difference per second. However to separate the breaking and non-breaking parts I’ve split this PR into two commits. Therefore, the first commit fixes the fact that the project’s default damping settings were not being used. The second commit fixes #19182 and #30991 by forcing Bullet to use the same algorithm as Godot physics.

To make Bullet and Godot physics damping the same required using Bullet’s old damping algorithm, and allowing damping values greater than 1 – see below. #37314 required Bullet’s master branch, because it includes bulletphysics/bullet3#2748, which allows damping values greater than 1. I’ve applied this patch separately, and included the patch in the second commit.

For completeness, I’ve looked into why Bullet changed their damping algorithm. Bullet has a comment to explain the move to the new method by referring the reader to a discussion on issue 74.

Godot and Bullet’s old damping algorithm is:

velocity *= MAX(1 - delta * damping, 0);

In this algorithm, damping is supposed to represent the amount the velocity reduces by in 1 second. Setting damping to 1 should reduce the velocity to 0 in 1 second. The problem is that the calculation is done in discrete steps, with delta of 1/60, after 60 steps instead of reducing the velocity to 0, the velocity is only reduced to 36%. The other problem is that 1 second may be too long. The origin of issue #19182.

Note, there is an implicitly assumption in the algorithm that damping is a value between 0 and 1. Bullet enforced this constraint. Godot only documented this constraint, in reality, it allows values greater than 1. By using values greater than 1, any level of damping can be achieved. Note: Setting damping at 60 (or greater) with 60 frames per second will set velocity to zero in the next time step.

Bullet’s approach was to fix the algorithm to ensure that the damping value determined the amount the velocity is reduced by in 1 second:

velocity *= POW(1 – damping, delta);

Note: this algorithm requires damping to be clamped between 0 and 1, and high levels of damping can be achieved by setting values ever closer to 1, with infinite damping occurring at 1 (and all values greater than 1), which explains the apparent step change seen in #19182. Note: The remaining motion exists, because damping is only applied on the second time interval; so gravity is applied for one time interval before velocity is once again set to 0.

@madmiraal
Copy link
Contributor Author

madmiraal commented Jun 26, 2020

Dropped 3758492, because superseded by 7bf9787.

Copy link
Contributor

@AndreaCatania AndreaCatania left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, Thanks!

@akien-mga akien-mga merged commit 44a5169 into godotengine:3.2 Jul 2, 2020
@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.

5 participants