Aloush154 / bullet

Automatically exported from code.google.com/p/bullet
Other
0 stars 0 forks source link

Assert in btSequentialImpulseConstraintSolver::solveGroupCacheFriendlySetup. btAssert(fsum > SIMD_EPSILON); #666

Closed GoogleCodeExporter closed 8 years ago

GoogleCodeExporter commented 8 years ago
This was introduced in 2.81 I belive, pretty sure its a merge error or accident 
introduced in this check in...
https://code.google.com/p/bullet/source/diff?spec=svn2617&r=2609&format=side&pat
h=/trunk/src/BulletDynamics/ConstraintSolver/btSequentialImpulseConstraintSolver
.cpp&old_path=/trunk/src/BulletDynamics/ConstraintSolver/btSequentialImpulseCons
traintSolver.cpp&old=2604

We were periodically getting this assert after switching to 2.81 (from 2.78). 
It was cacking on one of our btConeTwistConstraints bound to a static point 
(only one RB attached). The debugger showed that the assert was being caused 
because it was treating the constaint as if it was between 2 active RBs. The 
assert was triggering on line 1172...

btScalar fsum = btFabs(sum);
btAssert(fsum > SIMD_EPSILON);

Suspecting this was caused by a recent change, I checked the history on that 
file and guessed that the change at 1050-1052 on the last check in was 
accidental. Deleting those 3 lines fixed the problem, havn't seen the assert 
since. Included a patch.

Original issue reported on code.google.com by cont...@vicariousentertainment.com on 1 Nov 2012 at 11:45

Attachments:

GoogleCodeExporter commented 8 years ago
Oh too cool, I submitted issue 666 lol

Original comment by cont...@vicariousentertainment.com on 1 Nov 2012 at 11:45

GoogleCodeExporter commented 8 years ago
[deleted comment]
GoogleCodeExporter commented 8 years ago

Well ignore my fix, it didn't work and my theory was wrong. Still getting this 
assert in our game. And it is still occuring on a btConeTwistConstraint where 
rbA is a valid rigid body, and rbB is not used. As in this constraint was 
initialized with this constructor...

btConeTwistConstraint::btConeTwistConstraint(btRigidBody& rbA,const 
btTransform& rbAFrame)

fsum is 1.#QNAN00, due to iMJaA & iMJaB being 1.#QNAN00 also for every 
parameter. Above that bodyBPtr is pointing to an empty btSolverBody structure.

This worked perfectly in 2.78. I dug around a bit more and there's been a ton 
of changes to this file just in recent months so I have no idea when this 
broke. Looking through the code thats asserting, I don't see any that deals 
with the case where the constraint is bound to a static position like this one 
is. Is this func new, rewritten since 2.78 and was this scenario forgotten? I'm 
at a loss and in desperate need of a fix, don't think we can wait for another 
release of bullet before launch.

Original comment by cont...@vicariousentertainment.com on 2 Nov 2012 at 2:15

GoogleCodeExporter commented 8 years ago
[deleted comment]
GoogleCodeExporter commented 8 years ago

Can you reproduce this in a Bullet demo, such as the 
Bullet/Demos/ConstraintDemo?

The assert is new, and previously things could have gone wrong, unnoticed.
http://code.google.com/p/bullet/source/browse/trunk/src/BulletDynamics/Constrain
tSolver/btSequentialImpulseConstraintSolver.cpp#1174

Original comment by erwin.coumans on 5 Nov 2012 at 5:41

GoogleCodeExporter commented 8 years ago
I tried switching the btContTwistConstraint to a single RB attachment as the 
one that is failing on our game is and I havn't been able to cause the assert 
yet. Thats part of the problem, its not a frequent occurance in our game even. 
Now that I think about it, obviously that code that I changed in the patch 
(although probably correct now I'm guessing?) isn't going to change anything. 
But like I said, should the solver even get to the point where that assert is 
if its handling a constraint that only uses one RB? Looking at that code, I 
don't see anything that checks to see if the 2nd RB is active.

Original comment by cont...@vicariousentertainment.com on 5 Nov 2012 at 10:40

GoogleCodeExporter commented 8 years ago

The assert fires if BOTH objects are static or have a zero jacobian sum.

Can you please reproduce this in a Bullet demo (not using some single RB 
attachment workaround)?

Otherwise we assume it was a user error and we simply close the issue.
Thanks,
Erwin

Original comment by erwin.coumans on 5 Nov 2012 at 10:59

GoogleCodeExporter commented 8 years ago
There was no workaround, I was trying to modify the ConstraintDemo so that it 
could cause the assert. Havn't had any luck getting it to fire yet. And like I 
said, this assert doesn't trigger that often in our game, there isn't anything 
I've found so far that triggers it everytime. We just get it every now and then 
and have never seen it fire before we upgraded to 2.81 (from 2.78).

But its always dealing with the same constraint in our game when it does fire. 
So once again, should a constraint with only 1 active RB ever get to that area 
of the code where the assert is? Seems to me like that code doesn't handle the 
case where the constraint is only bound to one unique active RB.

Original comment by cont...@vicariousentertainment.com on 6 Nov 2012 at 1:22

GoogleCodeExporter commented 8 years ago
There should be at least one active rigid body with non-zero mass. It doesn't 
matter if you use one or two rigid bodies to initialize the constraint: Bullet 
with automatically create a second static (mass=0) body. So the solver always 
gets two bodies anyway.

Again, the assert fires if a constraint attaches two static (mass=0) bodies OR 
the transforms are invalid (NAN/INF etc) due to some other bug.

Original comment by erwin.coumans on 6 Nov 2012 at 1:31

GoogleCodeExporter commented 8 years ago
Yeah I've managed to catch it again in my game. Its a btConeTwist with 0 set 
for swingSpan2, and .26 for swingSpan1. Looks like I've discovered another case 
where m_swingAxis can be initialized to {0, -1.#IND, -1.#IND, -1.#IND}. I'm 
going to put some more asserts into btConeTwistConstraint.cpp in the 2 places 
where m_swingAxis gets initialized in this scenario so I can debug the problem 
at the source. I don't know why this problem didn't show up in an earlier 
build, I don't see anything in the history that should cause this. I'm guessing 
my problem is here...
http://code.google.com/p/bullet/source/browse/trunk/src/BulletDynamics/Constrain
tSolver/btConeTwistConstraint.cpp#750

This constraint would go through that branch if this func is called for it, and 
that if statement with the fuzzy check would just continue on to set 
m_swingAxis with the bad z value

Original comment by cont...@vicariousentertainment.com on 6 Nov 2012 at 3:47

GoogleCodeExporter commented 8 years ago
Ok, I've caught it. Not entirely sure what the best solution is. I was right 
about where it would trigger though, m_swingAxis goes bad on line 776.

http://code.google.com/p/bullet/source/browse/trunk/src/BulletDynamics/Constrain
tSolver/btConeTwistConstraint.cpp#776

See the attached file for debugger output. Basiclly all of the calculated 
vectors above are good, but "target" and ivB have evaluated out to the exact 
same vector, thus producing a nill crossproduct and an invalid number on the 
normalize.

Now are these "if(!btFuzzyZero(z))" statements above correct? You sure they're 
not supposed to be the opposite? btFuzzyZero returns true when a number is less 
then the epsilon. In all of those if statements the number being tested gets 
reset to 0, and then the other two components (x&y etc) get recalculated. Right 
off the bat, the way this constraint is setup the z value is 0 and the cross 
product from line 774 is very small, but large enough to produce a valid normal 
after normalizing.

Again, this constraint has valid A & B frame axis set, .26 for SwingSpan1, 0 
for SwingSpan2 (it has twist as well, though not a factor here).

Original comment by cont...@vicariousentertainment.com on 6 Nov 2012 at 5:02

Attachments:

GoogleCodeExporter commented 8 years ago
If any swing limit is smaller than CONETWIST_DEF_FIX_THRESH (0.05), it is 
considered fixed.

It looks like the cone twist constraint doesn't work well if one (or more) 
swing limits is too small (or even zero). From the source code comments:
// or you're trying to set at least one of the swing limits too small. (if so, 
do you really want a conetwist constraint?)

Can you try to plug all the numbers of the rigid bodies/constraint in a 
Bullet/Demos/ConstraintDemo to try to reproduce it?

Original comment by erwin.coumans on 6 Nov 2012 at 5:30

GoogleCodeExporter commented 8 years ago
[deleted comment]
GoogleCodeExporter commented 8 years ago
SwingSpan 1 was .26, it was fine, and I had TwistSpan set to the same. I didn't 
need fore\aft movement in this case but needed the twist and lateral movement. 
As I recall I tried other types of constraints first but wasen't happy with the 
motion, the twist behaviour of the btConeTwistConstraint worked well for this 
case.

Ok, I've plugged my numbers into the ConstaintDemo and have gotten the same bad 
m_swingAxis to generate. Grab the attached cpp with a few new lines and break 
point line 774 in btConeTwistConstraint.cpp to see it go bad. So at this point 
I'm just wondering if the various if(!btFuzzyZero()) statements are all 
reversed or if this case was just never handled.

Oh and we should probably add asserts wherever m_swing_Axis is calculated in 
btConeTwistConstraint as its not very clear where this problem originates when 
the assert in the first post fires. I set btAssert(m_swingAxis.length() > 
0.5f); on line 777 to catch this case in our game, but there are are least 3 
other places in that file where m_swingAxis gets calculated.

Original comment by cont...@vicariousentertainment.com on 7 Nov 2012 at 12:48

Attachments:

GoogleCodeExporter commented 8 years ago
Can you check if Roman's fix helps?

http://code.google.com/p/bullet/source/detail?r=2619

Original comment by erwin.coumans on 10 Nov 2012 at 12:57

GoogleCodeExporter commented 8 years ago
It helps at first, but does not fix the problem. I tested it in the modifed 
ConstraintDemo app with my btAssert(m_swingAxis.length() > 0.5f); added to line 
777. It does go into those if statements now on the first few or so passes, and 
this avoids m_swingAxis from being corrupted, but the x,y,z numbers keep 
changing untill it hits a case again where target and ivB have the same value 
and of course the crossp produces an invalid result and the assert triggers. 
This all happens before a single frame is rendered.

Can Roman test it with that modified ConstraintDemo.cpp? Add that assert and it 
will catch the corruption right away in the right spot.

Original comment by cont...@vicariousentertainment.com on 10 Nov 2012 at 2:53

GoogleCodeExporter commented 8 years ago
[deleted comment]
GoogleCodeExporter commented 8 years ago
Well I've played with this a bit more. That check in of Roman's did not fix the 
problem, though it does not appear to have broken anything that wasn't already 
broken (though this assert triggers more often for the same case now). I can't 
afford to spend much more time on this so I've just set the smallest value for 
SwingSpan2 that I can get away with to get by the MinThreshold comparison 
(which is about 4 degrees). This of course completely makes the assert go away 
in our game but the problem is still in Bullet for others to run into.

I've played around in the Constraint Demo a little more. Reversing the Limit 
parameters and Setting 0 for SwingSpan 1 and something valid for Span 2 
triggers the same assert right away as well. If I change line 669...

if (m_swingSpan1 >= m_fixThresh && m_swingSpan2 >= m_fixThresh)

to be an || instead of the &&, the original case evalutes perfectly as it 
doesn't go into the code where the problem lies. However with the parameters 
reversed for swing span 1 and 2, I get no effective swing span on either of 
those axis. The branch that re-calculates the SwingAxis if SwingSpan 1 or 2 is 
0 is certainly needed, our game asserts elsewhere with that code disabled. FYI 
passing 0 for both swing spans works correctly. There is still an error in that 
code but I don't understand it well enough to offer a proper fix I'm afraid.

Original comment by cont...@vicariousentertainment.com on 27 Nov 2012 at 12:40

GoogleCodeExporter commented 8 years ago
https://github.com/bulletphysics/bullet3/issues/109

Original comment by erwin.coumans on 30 Mar 2014 at 7:18

GoogleCodeExporter commented 8 years ago
I have (had?) the same problem for btSliderConstraint where body is made out of 
btShapeHull (I don't observe assertion failure in case of primitive collision 
shapes). I pulled the last revision (063a0344acdae57df0321d898f1375d68a152aaf) 
and saw, that faulty part is (as was written before) :

1439  btScalar fsum = btFabs(sum);
1440  btAssert(fsum > SIMD_EPSILON);
1441  solverConstraint.m_jacDiagABInv = fsum>SIMD_EPSILON?btScalar(1.)/sum : 
0.f;

Although I don't understand the reason for all those dot products being 
computed as "sum", one can see that the fsum is checked for being bigger than 
SIMD_EPSILON properly at line 1441 which is _after_ assert. This must be the 
correct behavior in case of fsum being "0". So i simply commented out the line 
1440 and everything works as it supposed to (assertion is just a raised finger 
for developer, that there is a situation that should not happen, but it looks 
that code after counts with it and behaves as it supposed to). Hope it helps to 
somebody.

Original comment by Vladko.F...@gmail.com on 30 Jun 2014 at 3:13

GoogleCodeExporter commented 8 years ago
please continue discussion in github, not here

Original comment by erwin.coumans on 1 Jul 2014 at 11:26