partId and triangleIndex when using GImpact

[AlexSilverman]

Hello,

I'm using Bullet with GImpact (the version distributed with Bullet 2.67) and implementing a system for per triangle materials when I ran into a problem that, after I found it, I saw it was referenced here as well, with no resolution. The problem is that when a btGImpactConvexDecompositionShape (I haven't tested with the non convex decomposition flavor of GImpact) collides with a btBvhTriangleMeshShape, the m_part and m_triface values for the btGImpactConvexDecompositionShape are both -1, which is fine (although it'd be nice to have accurate numbers for these as well), but the m_part and m_triface values for the btBvhTriangleMeshShape are 0 and -1 respectively, which is not correct, as topcomer noted in the other thread.

After a bit of digging, I found the cause. When any collision shape (except the btGImpactConvexDecompositionShape) collides with a btBvhTriangleMeshShape, the btManifoldResult->setShapeIdentifiers() method is called from the btConvexTriangleCallback::processTriangle() method, assigning the partId and triangleIndex values as they were passed in from btBvhTriangleMeshShape::processAllTriangles(). In the case of a collision with a btGImpactConvexDecompositionShape however, those values are passed into btGImpactTriangleCallback::processTriangle() but never used, because btGImpactCollisionAlgorithm::gimpact_vs_shape() is called to drill down and do more shape appropriate collision. btManifoldResult->setShapeIdentifiers() is called further down the line in btGImpactCollisionAlgorithm::addContactPoint, but it uses the btGImpactCollisionAlgorithm member variables as arguments, which have never been set, so obviously the values are incorrect.

As a fix, I added Get/Set methods to the btGImpactCollisionAlgorithm for m_part0/1 and m_triface0/1 and assign them in btGImpactTriangleCallback::processTriangle before the call to btGImpactCollisionAlgorithm::gimpact_vs_shape(). Since doing this, I haven't seen the problem anymore, but there's probably a better way to go about doing it, if there's concern about accessing those four variables.

- Alex

[Erwin Coumans]

As a fix, I added Get/Set methods to the btGImpactCollisionAlgorithm for m_part0/1 and m_triface0/1 and assign them in btGImpactTriangleCallback::processTriangle before the call to btGImpactCollisionAlgorithm::gimpact_vs_shape(). Since doing this, I haven't seen the problem anymore, but there's probably a better way to go about doing it, if there's concern about accessing those four variables.
- Alex

Thanks for figuring this out. Do you have a patch/fix for this?

Erwin

[AlexSilverman]

Erwin,

I've attached a patch to the Extras folder that changes btGImpactCollisionAlgorithm.h/cpp.

- Alex

[Erwin Coumans]

Thanks, the fix has been applied.

The coding standard was adapted to match Bullet coding style

[AlexSilverman]

Erwin,

Apologies. I found a mistake in my previous patch. Here's another patch that corrects it. I was applying the partID and triIndex to the GIMPACT object rather than the triangle from the static mesh. The attached patch only deals with the code file, not the header, and I've tried to adhere to the coding standard in the function calls, so hopefully you won't need to alter it.

- Alex

[Erwin Coumans]

The patch cannot be applied, did you make sure to use the very latest Subversion version of Bullet?

Note that your previous patch has already been applied, and btGImpactCollisionAlgorithm.cpp looks like this (around those lines):

Code: Select all

class btGImpactTriangleCallback: public btTriangleCallback
{
public:
	btGImpactCollisionAlgorithm * algorithm;
	btCollisionObject * body0;
	btCollisionObject * body1;
	btGImpactShapeInterface * gimpactshape0;
	bool swapped;
	btScalar margin;

	virtual void processTriangle(btVector3* triangle, int partId, int triangleIndex)
	{
		btTriangleShapeEx tri1(triangle[0],triangle[1],triangle[2]);
		tri1.setMargin(margin);
        algorithm->setPart1(partId);
        algorithm->setFace1(triangleIndex);
		algorithm->gimpact_vs_shape(
							body0,body1,gimpactshape0,&tri1,swapped);
	}
};

Can you .rar just entire btGImpactCollisionAlgorithm.cpp, as you think is correct?
Thanks for your help,
Erwin

[AlexSilverman]

Sorry for the confusion. That was definitely the case (not building the patch against an updated SVN). I had been assigning the values to the wrong body (1 rather than 0), but in fixing this, I found that I had to comment out a line, which may have some unintended consequences. Hopefully someone more familiar with the algorithm might be able to say more conclusively.

In addition to the change of setting the partID and triangleIndex earlier, I commented out line 654, which assigned btGImpactCollisionAlgorithm::m_triface0 to the i-th result of the collided pairs found by btGImpactCollisionAlgorithm::gimpact_vs_shape_find_pairs. I tried to test this in several situation and didn't see any detrimental effects, and thinking it through, from an admittedly uninformed position, I think it's a safe thing to do. If I understand what's going on correctly, there will be more than 1 pair returned (in a test where the triangle index is of any consequence, being a collision between a triangle shape and a GImpact shape) if a GImpact shape collides with one triangle at more than one point. If this is the case, then the triangleIndex will be the same across all collision points, so it does not need to be changed as the points are iterated through. I beleive these are the only times that triangleIndex will be paid attention to, so other cases don't matter.

I've attached my version of btGImpactCollisionAlgorithm.cpp. Sorry again for the mixup.

- Alex

[Erwin Coumans]

I am unfamiliar with most of the GImpact internals.

Why was it necessary to comment out this line 654? Unless you are sure, shall we first ask Francisco?
Thanks,
Erwin

[AlexSilverman]

I agree. Defering to Francisco is certainly called for.

The portion of the algorithm in question, to the best of my understanding, searches for indices that the GImpact shape is colliding with in the other shape via a call to btGImpactCollisionAlgorithm::gimpact_vs_shape_find_pairs(), and for each index value returned, the line in question reassigned btGImpactCollisionAlgorithm::m_triface0 to that index value, overwriting the correct index value assigned earlier.

As I said before, I'm not sure whether this fix will have any unintended consequences, so it's definitely better to have Francisco weigh in on this.

Thanks.

- Alex

[projectileman]

I know that is too late for responding this,
But about your issue, there is something to take in consideration about GIMPACT. btGImpactConvexDecompositionShape is a merely Compound Shape of Convex polyhedrons with a BVH tree attached. It doesn't give any triangle information.

If you want to get the collided features (triangles and part ID), you must use the btGImpactMeshShape. Or may you can try the SoftBody collision shape.

Thanks.