btAssert(found) in btSequentialImpulseConstraintSolver::solveGroupCacheFriendlySetup function

[GoogleCodeExporter]

What steps will reproduce the problem?
1. Replace GenericJointDemo with attached file.
2. There is a small experiment with 3 boxes connected with 2 constraints. If 
the collision for these boxes is disabled or if the distance between them is 
large enough, the assert is reproducable grabbing the leftmost box after all 
the boxes are sleeping.

What is the expected output?
The leftmost box is grabbed and moved with the mouse, the rest 2 boxes are 
moving controlled by the constraints.

What do you see instead?
btAssert(found) in 
btSequentialImpulseConstraintSolver::solveGroupCacheFriendlySetup function.

What version of the product are you using? On what operating system?
2.81-rev2613 on Win7 64 bit

Please provide any additional information below.
Please also see forum issue about same topic:
http://www.bulletphysics.org/Bullet/phpBB3/viewtopic.php?f=9&t=9028
As stated there here is my best guess at what the problem is:
"For me it seems that if there is no collision and this way the island building 
is based on only the constraints, it is possible that such a "border" 
constraint is put to the island for which only RigidBodyA or RigidBodyB is in 
the island, but not both. Please see InplaceSolverIslandCallback::processIsland 
and btGetConstraintIslandId."

Original issue reported on code.google.com by nsark...@gmail.com on 22 Apr 2013 at 9:04

Attachments:

• GenericJointDemo.cpp

[GoogleCodeExporter]

We created a modification in btGetConstraintIslandId to avoid the above 
situation. The idea behind the modification is that if we find a constraint 
where the island ids of RigidBodyA and RigidBodyB of the constraint are 
different then this constraint is between 2 islands. With the current 
implementation of the solver (btSequentialImpulseConstraintSolver) the 2 
islands can be kept separated and we can say that the constraint does not 
belong to any of the 2 islands. For this reason the number of actual collision 
objects are used as the island id of those constraints not belonging to any 
island in the attached patch file, but I was warned that as passing this 
information (actual number of collision objects) to the level of 
btGetConstraintIslandId is too "circumstantial" or "cumbersome" so simply 
returning a big const int for constraints not belonging to any island might be 
better.

Please take a look at the changes.
Thank you.

Original comment by nsark...@gmail.com on 27 Aug 2013 at 9:33

Attachments:

• solver_island_btAssert.patch

[GoogleCodeExporter]

I cannot reproduce this in latest Bullet SVN trunk, perhaps it is fixed already?

Can you check please, and re-open the issue if it is still an issue?
Thanks a lot for the report!

Original comment by erwin.coumans on 10 Sep 2013 at 8:13

• Changed state: WontFix

[GoogleCodeExporter]

Unfortunately the problem is not fixed but the #ifdef statement guarding the 
code part that is responsible for checking the integrity of the island in 
btSequentialImpulseConstraintSolver::solveGroupCacheFriendlySetup is changed 
from
#ifdef BT_DEBUG
to
#ifdef BT_ADDITIONAL_DEBUG .
BT_DEBUG was originally defined in revision 2613 while BT_ADDITIONAL_DEBUG is 
not defined in revision 2669.

Please check the issue once more with BT_ADDITIONAL_DEBUG defined. Thank you.

Original comment by nsark...@gmail.com on 16 Sep 2013 at 11:31

[GoogleCodeExporter]

Thanks for the feedback, I'll have another look at it.

Original comment by erwin.coumans on 16 Sep 2013 at 4:56

• Changed state: Accepted

[GoogleCodeExporter]

It shouldn't happen, so it is likely a bug: a constraint should merge the 
islands of the attached rigid bodies.

Original comment by erwin.coumans on 16 Sep 2013 at 4:58

[GoogleCodeExporter]

The issue is because of this test in 
btDiscreteDynamicsWorld::calculateSimulationIslands. I need to check why this 
was ever added, it seems wrong:

if (colObj0->isActive() || colObj1->isActive())

Original comment by erwin.coumans on 16 Sep 2013 at 5:11

[GoogleCodeExporter]

It is fixed now, there should be no 'isActive' test for merging simulation 
islands.

By the way, the btGeneric6DofConstrains is very unstable, while the 
btSliderConstraint is much more stable. Can you try using the 
btSliderConstraint instead?

    btSliderConstraint* joint2 = new btSliderConstraint(*boxBody2, *boxBody3,localA, localB, true);
        joint2->setLowerLinLimit(-4);
        joint2->setUpperLinLimit(4);

Original comment by erwin.coumans on 16 Sep 2013 at 5:26

• Changed state: Fixed

[GoogleCodeExporter]

Thank you.

Your solution is the same that we used temporarily, but we thought this is not 
nice, because with the old version an island contained only active rigidbodies. 
(and of course those who were inactive but were connected to active rigidbody 
through a constraint or a collision) 
We know that in the setup phase of the solver a user constraint will probably 
not generate solver constraints between inactive elements (assuming they went 
to sleep naturally), but still it needs a little bit more calculation this way.
We accept the new logic (especially because in other physics simulation engine 
this is more common) we just wanted to highlight this.

Actually we only used the btGeneric6DofConstraint for this demo application 
because that is the type of constraint used in our real application as well. 
There we have a human character so it is used in a completely different way 
(not as a slider).
We know that 6dof has issues unfortunately. We actually have a task to create a 
better 6dof. (though we don't say that it will be an easy task)

Original comment by nsark...@gmail.com on 19 Sep 2013 at 11:02