Bugfix: Contact removal callback was not called when the last body went to sleep

Added more comments on contact listener behavior and added a unit test to validate that contact removal callback is now called when the last body goes to sleep.

Jolt/Core/StringTools.h

@@ -5,7 +5,8 @@
 
 JPH_NAMESPACE_BEGIN
 
-/// Create a formatted text string
+/// Create a formatted text string for debugging purposes.
+/// Note that this function has an internal buffer of 1024 characters, so long strings will be trimmed.
 string StringFormat(const char *inFMT, ...);
 
 /// Convert type to string

Jolt/Physics/Collision/CollideShape.h

@@ -37,8 +37,8 @@ public:
 
 	using Face = StaticArray<Vec3, 32>;
 
-	Vec3					mContactPointOn1;			///< Contact point on shape 1 (in world space)
-	Vec3					mContactPointOn2;			///< Contact point on shape 2 (in world space)
+	Vec3					mContactPointOn1;			///< Contact point on the surface of shape 1 (in world space)
+	Vec3					mContactPointOn2;			///< Contact point on the surface of shape 2 (in world space). If the penetration depth is 0, this will be the same as mContactPointOn1.
 	Vec3					mPenetrationAxis;			///< Direction to move shape 2 out of collision along the shortest path (magnitude is meaningless, in world space). You can use -mPenetrationAxis.Normalized() as contact normal.
 	float					mPenetrationDepth;			///< Penetration depth (move shape 2 by this distance to resolve the collision)
 	SubShapeID				mSubShapeID1;				///< Sub shape ID that identifies the face on shape 1

Jolt/Physics/Collision/ContactListener.h

@@ -25,8 +25,8 @@ public:
 	float					mPenetrationDepth;					///< Penetration depth (move shape 2 by this distance to resolve the collision)
 	SubShapeID				mSubShapeID1;						///< Sub shapes that formed this manifold (note that when multiple manifolds are combined because they're coplanar, we lose some information here because we only keep track of one sub shape pair that we encounter)
 	SubShapeID				mSubShapeID2;
-	ContactPoints			mWorldSpaceContactPointsOn1;		///< Contact points on shape 1 in world space
-	ContactPoints			mWorldSpaceContactPointsOn2;		///< Contact points on shape 2 in world space
+	ContactPoints			mWorldSpaceContactPointsOn1;		///< Contact points on the surface of shape 1 in world space.
+	ContactPoints			mWorldSpaceContactPointsOn2;		///< Contact points on the surface of shape 2 in world space. If there's no penetration, this will be the same as mWorldSpaceContactPointsOn1. If there is penetration they will be different.
 };
 
 /// When a contact point is added or persisted, the callback gets a chance to override certain properties of the contact constraint.
@@ -68,18 +68,23 @@ public:
 
 	/// Called whenever a new contact point is detected.
 	/// Note that this callback is called when all bodies are locked, so don't use any locking functions!
-	/// Body 1 and 2 will be sorted such that body 1 ID < body 2 ID, so body 1 may not be dynamic
+	/// Body 1 and 2 will be sorted such that body 1 ID < body 2 ID, so body 1 may not be dynamic.
+	/// Note that only active bodies will report contacts, as soon as a body goes to sleep the contacts between that body and all other
+	/// bodies will receive an OnContactRemoved callback, if this is the case then Body::IsActive() will return false during the callback.
+	/// When contacts are added, the constraint solver has not run yet, so the collision impulse is unknown at that point.
+	/// The velocities of inBody1 and inBody2 are the velocities before the contact has been resolved, so you can use this to
+	/// estimate the collision impulse to e.g. determine the volume of the impact sound to play.
 	virtual void			OnContactAdded(const Body &inBody1, const Body &inBody2, const ContactManifold &inManifold, ContactSettings &ioSettings) { /* Do nothing */ }
 
 	/// Called whenever a contact is detected that was also detected last update.
 	/// Note that this callback is called when all bodies are locked, so don't use any locking functions!
-	/// Body 1 and 2 will be sorted such that body 1 ID < body 2 ID, so body 1 may not be dynamic
+	/// Body 1 and 2 will be sorted such that body 1 ID < body 2 ID, so body 1 may not be dynamic.
 	virtual void			OnContactPersisted(const Body &inBody1, const Body &inBody2, const ContactManifold &inManifold, ContactSettings &ioSettings) { /* Do nothing */ }
 
 	/// Called whenever a contact was detected last update but is not detected anymore.
 	/// Note that this callback is called when all bodies are locked, so don't use any locking functions!
 	/// Note that we're using BodyID's since the bodies may have been removed at the time of callback.
-	/// Body 1 and 2 will be sorted such that body 1 ID < body 2 ID, so body 1 may not be dynamic
+	/// Body 1 and 2 will be sorted such that body 1 ID < body 2 ID, so body 1 may not be dynamic.
 	virtual void			OnContactRemoved(const SubShapeIDPair &inSubShapePair) { /* Do nothing */ }
 };
 

Jolt/Physics/PhysicsSystem.cpp

@@ -130,6 +130,10 @@ void PhysicsSystem::Update(float inDeltaTime, int inCollisionSteps, int inIntegr
 		mBroadPhase->UpdateFinalize(update_state);
 		mBroadPhase->UnlockModifications();
 
+		// Call contact removal callbacks from contacts that existed in the previous update
+		mContactManager.ContactPointRemovedCallbacks();
+		mContactManager.FinalizeContactCache(0, 0);
+
 		mBodyManager.UnlockAllBodies();
 		return;
 	}

UnitTests/Physics/ContactListenerTests.cpp

@@ -116,6 +116,7 @@ TEST_SUITE("ContactListenerTests")
 
 		// Simulate one more step to process the collision
 		c.Simulate(c.GetDeltaTime());
+		CHECK_APPROX_EQUAL(body.GetPosition(), cFloorHitPos, cPenetrationSlop);
 
 		// We expect a validate and a contact point added message
 		CHECK(listener.GetEntryCount() == 2);
@@ -144,6 +145,7 @@ TEST_SUITE("ContactListenerTests")
 
 		// Simulate 10 steps
 		c.Simulate(10 * c.GetDeltaTime());
+		CHECK_APPROX_EQUAL(body.GetPosition(), cFloorHitPos, cPenetrationSlop);
 
 		// We're not moving, we should have persisted contacts only
 		CHECK(listener.GetEntryCount() == 10);
@@ -164,6 +166,42 @@ TEST_SUITE("ContactListenerTests")
 		}
 		listener.Clear();
 
+		// Make the body able to go to sleep
+		body.SetAllowSleeping(true);
+
+		// Let the body go to sleep
+		c.Simulate(1.0f);
+		CHECK_APPROX_EQUAL(body.GetPosition(), cFloorHitPos, cPenetrationSlop);
+
+		// Check it went to sleep and that we received a contact removal callback
+		CHECK(!body.IsActive());
+		CHECK(listener.GetEntryCount() > 0);
+		for (size_t i = 0; i < listener.GetEntryCount(); ++i)
+		{
+			// Check persist / removed callbacks
+			const LogEntry &entry = listener.GetEntry(i);
+			CHECK(entry.mBody1 == floor.GetID());
+			CHECK(entry.mBody2 == body.GetID());
+			CHECK(entry.mType == ((i == listener.GetEntryCount() - 1)? EType::Remove : EType::Persist)); // The last entry should remove the contact as the body went to sleep
+		}
+		listener.Clear();
+
+		// Wake the body up again
+		c.GetBodyInterface().ActivateBody(body.GetID());
+		CHECK(body.IsActive());
+
+		// Simulate 1 time step to detect the collision with the floor again
+		c.SimulateSingleStep();
+
+		// Check that the contact got readded
+		CHECK(listener.GetEntryCount() == 2);
+		CHECK(listener.Contains(EType::Validate, floor.GetID(), body.GetID()));
+		CHECK(listener.Contains(EType::Add, floor.GetID(), body.GetID()));
+		listener.Clear();
+
+		// Prevent body from going to sleep again
+		body.SetAllowSleeping(false);
+
 		// Make the sphere move horizontal
 		body.SetLinearVelocity(Vec3::sAxisX());
 
@@ -207,7 +245,7 @@ TEST_SUITE("ContactListenerTests")
 		listener.Clear();
 
 		// Move the sphere away from the floor
-		c.GetSystem()->GetBodyInterface().SetPosition(body.GetID(), cInitialPos, EActivation::Activate);
+		c.GetBodyInterface().SetPosition(body.GetID(), cInitialPos, EActivation::Activate);
 
 		// Simulate 10 steps
 		c.Simulate(10 * c.GetDeltaTime());