Divide by zero in ConvexFeature::testEdge()
by Tom Spilman · in Torque Game Engine · 09/05/2006 (2:47 am) · 6 replies
I found a division by zero situation in ConvexFeature::testEdge()...
I've caught this in a debugger a few times and it causes the normal to be set to -1.#INF. This then gets passed into collision resolvers like Vehicle::resolveCollision().
To fix it i did this...
It seems the vehicles properly deal with zero length normals, but maybe i should be generating a proper normal in that case... need some domain expert on the collision system to step in and help here.
Also note that mIsZero is a function in math/mSolver.cpp that i moved and inlined in math/mMathFn.h. mIsZero is really useful... it should be moved there.
Don't know if this fixes any of the collision issues people have encountered, i still see penetrations and stuff in my build, but you still should never push infinite floats thru the simulation.
// Get the distance and closest points
Point3F i1,i2;
F32 distance = sqrDistanceEdges(s1, e1, s2, e2, &i1, &i2);
if (distance > tolSquared)
continue;
distance = mSqrt(distance);
// Need to figure out how to orient the collision normal.
// The current test involves checking to see whether the collision
// points are contained within the convex volumes, which is slow.
if (inVolume(i1) || cf->inVolume(i2))
distance = -distance;
// Contact normal
VectorF normal = i1 - i2;
normal *= 1 / distance; // DIVIDE BY ZERO!I've caught this in a debugger a few times and it causes the normal to be set to -1.#INF. This then gets passed into collision resolvers like Vehicle::resolveCollision().
To fix it i did this...
// Contact normal
VectorF normal = i1 - i2;
if ( mIsZero( distance ) )
normal.zero();
else
normal *= 1 / distance;
AssertFinite( normal );It seems the vehicles properly deal with zero length normals, but maybe i should be generating a proper normal in that case... need some domain expert on the collision system to step in and help here.
Also note that mIsZero is a function in math/mSolver.cpp that i moved and inlined in math/mMathFn.h. mIsZero is really useful... it should be moved there.
Don't know if this fixes any of the collision issues people have encountered, i still see penetrations and stuff in my build, but you still should never push infinite floats thru the simulation.
About the author
Tom is a programmer and co-owner of Sickhead Games, LLC.
#2
do you happen to have some geometry you could share which reproduces the problem with a non-vehicle game ?
I'd like to try to convince myself that the fix is good. For the time being i just added an AssertFatal just after sqrDistanceEdges().
edit: oh wait, never mind.
it looks to me like this whole code-path is only called from Vehicle::updateCollision().
09/05/2006 (9:24 am)
Tom,do you happen to have some geometry you could share which reproduces the problem with a non-vehicle game ?
I'd like to try to convince myself that the fix is good. For the time being i just added an AssertFatal just after sqrDistanceEdges().
edit: oh wait, never mind.
it looks to me like this whole code-path is only called from Vehicle::updateCollision().
#3
Moving the test up to the tolSquared line is possible if it's ok to not return those collisions. A zero distance collision is still a collision. So it seems to me that if you don't return these you may fall thru surfaces. Someone with more expertise in this area will have to enlighten us on this.
09/05/2006 (9:55 am)
@Orion - It's possible that it only effects vehicles as that's the area i was working in. Moving the test up to the tolSquared line is possible if it's ok to not return those collisions. A zero distance collision is still a collision. So it seems to me that if you don't return these you may fall thru surfaces. Someone with more expertise in this area will have to enlighten us on this.
#4
09/05/2006 (11:52 pm)
Tom I treid to implement this but is says neither mIsZero or AssertFinite can be found. Any ideas?
#5
It's too useful to be stuck as a static in solver.
UPDATE: Also note that i've passed this issue to GG and it's being looked at. They mention that 0 distance collisions shouldn't go thru the system... just a waste of cycles. So Orion's fix is most likely correct, but i'm gonna keep an eye out for GG's final fix which may be a bit more complex.
09/05/2006 (11:55 pm)
@Ron - The AssertFinite can be removed... it's something i added for testing. mIsZero can be found in math/mSolver.cpp. I made a slight change to it and moved it into math/mMathFn.h...//--------------------------------------
// Inlines
inline bool mIsZero(const F32 val, const F32 epsilon = 1e-8f )
{
return (val > -epsilon) && (val < epsilon);
}
inline F32 mFloor(const F32 val)
{
return (F32) floor(val);
}It's too useful to be stuck as a static in solver.
UPDATE: Also note that i've passed this issue to GG and it's being looked at. They mention that 0 distance collisions shouldn't go thru the system... just a waste of cycles. So Orion's fix is most likely correct, but i'm gonna keep an eye out for GG's final fix which may be a bit more complex.
#6
09/06/2006 (1:27 pm)
Thanks Tom 
Associate Orion Elenzil
Real Life Plus
What about moving the mIsZero() test up into the tolSquared test,
and thus just skip creating the collision altogether ? (and skipping a square root of course)
ie
F32 distance = sqrDistanceEdges(s1, e1, s2, e2, &i1, &i2); if (distance > tolSquared [b]|| mIsZero(distance)[/b]) continue; distance = mSqrt(distance);