We've isolated the issue further, and have written a quick fix.
The problem is that when we add a fifth contact point to the persistent manifold, it will always replace one of the previous 4. Only that replacement can sometimes result in a contact point area that shrinks.
Our fix is to compute the current area of the contact points, and only accept the new candidate if it improves the situation.
Here is the patch that dramatically improves our situation:
Code:
--- src/BulletCollision/NarrowPhaseCollision/btPersistentManifold.cpp 2012-05-23 12:27:03.922020400 -0400
+++ src/BulletCollision/NarrowPhaseCollision/btPersistentManifold.cpp.fix 2012-05-23 12:27:16.203722800 -0400
@@ -23,7 +23,7 @@
ContactProcessedCallback gContactProcessedCallback = 0;
///gContactCalcArea3Points will approximate the convex hull area using 3 points
///when setting it to false, it will use 4 points to compute the area: it is more accurate but slower
-bool gContactCalcArea3Points = true;
+bool gContactCalcArea3Points = false;
btPersistentManifold::btPersistentManifold()
@@ -126,6 +126,7 @@
#endif //KEEP_DEEPEST_POINT
btScalar res0(btScalar(0.)),res1(btScalar(0.)),res2(btScalar(0.)),res3(btScalar(0.));
+ btScalar res;
if (gContactCalcArea3Points)
{
@@ -159,6 +160,11 @@
btVector3 cross = a3.cross(b3);
res3 = cross.length2();
}
+ // Compute current area
+ btVector3 a = m_pointCache[1].m_localPointA-m_pointCache[0].m_localPointA;
+ btVector3 b = m_pointCache[3].m_localPointA-m_pointCache[2].m_localPointA;
+ btVector3 cross = a.cross(b);
+ res = cross.length2();
}
else
{
@@ -177,11 +183,12 @@
if(maxPenetrationIndex != 3) {
res3 = calcArea4Points(pt.m_localPointA,m_pointCache[0].m_localPointA,m_pointCache[1].m_localPointA,m_pointCache[2].m_localPointA);
}
+ res = calcArea4Points(m_pointCache[0].m_localPointA,m_pointCache[1].m_localPointA,m_pointCache[2].m_localPointA,m_pointCache[3].m_localPointA);
}
btVector4 maxvec(res0,res1,res2,res3);
int biggestarea = maxvec.closestAxis4();
- return biggestarea;
-
+ //return biggestarea;
+ return maxvec[biggestarea] < res ? -1 : biggestarea;
}
@@ -218,6 +225,7 @@
#else
insertIndex = 0;
#endif
+ if( insertIndex == -1 ) return 0;
clearUserCache(m_pointCache[insertIndex]);
} else
We've switched to using calcArea4Points() instead of the 3-point path because the 3-point path can yield zero area when the 2 edges being used in the cross product are parallel to one another (a case that can happen when the 4 contact points converge to a square, for example). That said, we've coded sortCachedPoints() to work with gContactCalcArea3Points set to either true or false; we just feel setting it to false is warranted considering the improved quality versus performance impact.
In our code, sortCachedPoints() can now return -1 to indicate that the new candidate doesn't improve the situation (i.e. doesn't increase the area of the contact points). Since we were wary of such a change, we've decided to cut it short as soon as possible, which is right after sorting in addManifoldPoint(), where we return 0. Someone with a deeper understanding might prefer to propagate the -1 further up.
With this simple patch, we have noticeably improved stability on contacts of boxes on our btBvhTriangleMeshShape static scene. The contact points of simple boxes now very rapidly spread to the corners. We sometimes have the 4th contact point shift slightly, but since the other 3 are stable, we no longer observe the shakiness that we originally had.
We've looked at a few Bullet demos, and haven't noticed any difference in behavior, except for one: InternalEdgeDemo. That said, we do not quite understand why we would need to have the contact points follow an internal edge. Comparing internal edge enabled/disabled showed no noticeable difference, so we have to admit we don't understand what this test is meant to show in the first place.