NVIDIAGameWorks / PhysX-3.4

NVIDIA PhysX SDK 3.4
https://www.nvidia.com/
2.35k stars 274 forks source link

Assert during overlap query #22

Closed bdumesnil closed 7 years ago

bdumesnil commented 7 years ago

Hi,

We are having an assert in function SphereAABBTest_SIMD::operator() during an overlap query. When passed an empty box ( mCenter{x=0.000000000 y=0.000000000 z=0.000000000 }, mExtents {x=-8.50705867e+037 y=-8.50705867e+037 z=-8.50705867e+037 }), the function produces an INF value and triggers the assert.

Based on comments in BucketPrunerCore::removeObject :

"Queries against empty boxes will never return a hit"

So it seems to be valid to test an empty box. When ignoring the assert the function return the correct result ( false ).

Since this seems a valid case and it returns a correct result, I'd say this assert should not trigger.

Here is our callstack :

PhysX64_d.dll!physx::shdfnd::aos::FIsGrtrOrEq(const __m128 a, const __m128 b) Line 671  C++
PhysX64_d.dll!SphereAABBTest_SIMD::operator()(const physx::Sq::BucketBox & box) Line 1741   C++
PhysX64_d.dll!processBucket<1,SphereAABBTest_SIMD>(unsigned int nb, const physx::Sq::BucketBox * baseBoxes, physx::Sq::PrunerPayload * baseObjects, unsigned int offset, unsigned int totalAllocated, const SphereAABBTest_SIMD & test, physx::Sq::PrunerCallback & pcb, unsigned int minLimitInt, unsigned int maxLimitInt) Line 1621  C++
PhysX64_d.dll!BucketPrunerOverlapTraversal<SphereAABBTest_SIMD,1>::operator()(const physx::Sq::BucketPrunerCore & core, const SphereAABBTest_SIMD & test, physx::Sq::PrunerCallback & pcb, const physx::PxBounds3 & cullBox) Line 1696  C++
PhysX64_d.dll!physx::Sq::BucketPrunerCore::overlap(const physx::Gu::ShapeData & queryVolume, physx::Sq::PrunerCallback & pcb) Line 2071 C++
PhysX64_d.dll!physx::Sq::ExtendedBucketPruner::overlap(const physx::Gu::ShapeData & queryVolume, physx::Sq::PrunerCallback & prunerCallback) Line 683   C++
PhysX64_d.dll!physx::Sq::AABBPruner::overlap(const physx::Gu::ShapeData & queryVolume, physx::Sq::PrunerCallback & pcb) Line 311    C++
PhysX64_d.dll!physx::NpSceneQueries::multiQuery<physx::PxOverlapHit>(const physx::MultiQueryInput & input, physx::PxHitCallback<physx::PxOverlapHit> & hits, physx::PxFlags<enum physx::PxHitFlag::Enum,unsigned short> hitFlags, const physx::PxQueryCache * cache, const physx::PxQueryFilterData & filterData, physx::PxQueryFilterCallback * filterCall, physx::BatchQueryFilterData * bfd) Line 802    C++
PhysX64_d.dll!physx::NpSceneQueries::overlap(const physx::PxGeometry & geometry, const physx::PxTransform & pose, physx::PxHitCallback<physx::PxOverlapHit> & hits, const physx::PxQueryFilterData & filterData, physx::PxQueryFilterCallback * filterCall) Line 115    C++
PierreTerdiman commented 7 years ago

Yes, I fixed this bug just some days ago.

The fix should appear in GitHub eventually.

Meanwhile you can try these changes:

In SqAABBTree.cpp, around line 620, do this:

// Might happen after a node has been invalidated //const float max = 0.25f 1e33f; // ### const float max = PxSqrt(0.25f 1e33f); // ### resultMinV = V4Load(max); resultMaxV = V4Load(-max);

In SqBucketPruner.cpp, around line 690, do this:

// Invalidating the box does not invalidate the sorting, since it's now captured in mData0/mData1. // That is, mData0/mData1 keep their previous integer-encoded values, as if the box/object was still here. // PxBounds3 empty; // empty.setEmpty(); mSortedWorldBoxes[sortedIndex].mCenter = PxVec3(0.0f); mSortedWorldBoxes[sortedIndex].mExtents = PxVec3(-2.0fPxSqrt(0.25f 1e33f)); // PT: TODO: refactor value with similar one in SqAABBTree.cpp

bdumesnil commented 7 years ago

Thanks, it works fine

I just had to also apply the modification at line 739 in SqBucketPruner.cpp :

//PxBounds3 empty;
//empty.setEmpty();
//const PxVec3 emptyCenter = empty.getCenter();
//const PxVec3 emptyExtents = empty.getExtents();
const PxVec3 emptyCenter = PxVec3(0.0f);
const PxVec3 emptyExtents = PxVec3(-2.0f*PxSqrt(0.25f * 1e33f));
PierreTerdiman commented 7 years ago

You should apply both changes, otherwise there's a risk this assert will happen again with a slightly different sequence of operations.

bdumesnil commented 7 years ago

Of course, Thanks for your answers !