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

Ignore Bullet collision contact points with distance = 0 #44726

Merged
merged 1 commit into from
Nov 18, 2021

Conversation

madmiraal
Copy link
Contributor

Currently, Godot processes all Bullet collision manifolds as long as the distance is less than or equal to 0 i.e. a collision has occurred. When CCD is enabled, Bullet creates an additional collision manifold with the CCD results. This manifold sets the collision point distance to 0 if a collision occurs. Therefore, when a CCD collision is detected, this manifold is processed too, when it shouldn't.

This PR ensures that Godot doesn't process Bullet collision manifolds' contact points with a distance of 0; so it won't process the additional collision manifold created by CCD.

Fixes #44644.

@madmiraal madmiraal added bug topic:physics regression cherrypick:3.x Considered for cherry-picking into a future 3.x release labels Dec 27, 2020
@madmiraal madmiraal added this to the 4.0 milestone Dec 27, 2020
@AndreaCatania
Copy link
Contributor

In case a collision has 0 distance, we would have both the manifolds excluded. This may lead to some flickering, even if the collisions are still detected. I wonder if exist another way to know if a manifold is generated because of CCD. Do you know?

@madmiraal
Copy link
Contributor Author

In case a collision has 0 distance, we would have both the manifolds excluded. This may lead to some flickering, even if the collisions are still detected. I wonder if exist another way to know if a manifold is generated because of CCD. Do you know?

The only thing that changes is whether or not a distance of exactly 0.0 is considered a collision and a contact should be reported. There should be no flickering if the distance remains 0.0, because it is never added to the list of contacts. If there is any movement, then it doesn't matter whether or not we consider a distance of exactly 0.0 a collision or not as it will change during the next time step anyway.

@AndreaCatania
Copy link
Contributor

What I mean is that, let's suppose there is a collision and the penetrations between these two objects oscillates between 0.0 - 0.01. The contact report on godot side would appear broken, while Bullet is detecting and keeping that contact. I wonder if exist a better way to detect duplicate manifolds and avoid the above case, if it would became too expensive, it's fine merge this.

@madmiraal
Copy link
Contributor Author

Bullet requires the penetration depth to be greater than the margin before considering it a collision; so Bullet wouldn't consider a distance of exactly 0.0 to be a collision either.

Do you have a scenario where two objects would oscillate between exactly 0.0 and some small penetration? My expectation is that, if it were to oscillate at all, it would be more likely to oscillate between some number greater that 0.0 and some number less than 0.0, but very rarely be exactly 0.0. Therefore, I wouldn't expect this change to make a noticeable difference.

@AndreaCatania
Copy link
Contributor

Bullet requires the penetration depth to be greater than the margin before considering it a collision; so Bullet wouldn't consider a distance of exactly 0.0 to be a collision either.

Well, it depends on the shapes pair, since each shape margin behave differently.

Do you have a scenario where two objects would oscillate between exactly 0.0 and some small penetration? [...]

From time to time it may become 0.0, but I agree that it's not a big deal lose that info, considering that fixing it otherwise would be too expensive since the distance is the only difference between the standard manifold and the CCD manifold.

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!!
cc @akien-mga

@pouleyKetchoupp pouleyKetchoupp merged commit b7260e0 into godotengine:master Nov 18, 2021
@akien-mga
Copy link
Member

Cherry-picked for 3.5.

@akien-mga akien-mga removed the cherrypick:3.x Considered for cherry-picking into a future 3.x release label Nov 18, 2021
@madmiraal madmiraal deleted the fix-44644 branch November 19, 2021 07:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants