NVIDIAGameWorks / PhysX-3.4

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

!overflow assert in PxMeshOverlapUtil::findOverlap #14

Closed davidkalloc closed 7 years ago

davidkalloc commented 7 years ago

In PxMeshOverlapUtil::findOverlap I was hitting the assert PX_ASSERT(!overflow);

I tracked this down to GuBV4_BoxOverlap.cpp:158:

OBBParamsAll* ParamsAll = static_cast<OBBParamsAll*>(params);
ParamsAll->mHits[ParamsAll->mNbHits] = primIndex;
ParamsAll->mNbHits++;
if(ParamsAll->mNbHits==ParamsAll->mMaxNbHits)
    return 1;

In this case, "return 1" means an overflow occured. As you can see, it detects an overflow too early, as it can't know if there's going to be another one added.

I'd recommend changing this to something like:

OBBParamsAll* ParamsAll = static_cast<OBBParamsAll*>(params);
if(ParamsAll->mNbHits<ParamsAll->mMaxNbHits)
{
    ParamsAll->mHits[ParamsAll->mNbHits] = primIndex;
    ParamsAll->mNbHits++;
}
else
{
    return 1;
}

This way, it only reports an overflow when it tries to add a new hit and it doesn't fit into the array.

There's another instance of this mistake in GuBV4_BoxOverlap.cpp:378. Not sure about other files.

PierreTerdiman commented 7 years ago

Thank you for the report. It looks like a bug indeed. I will have a look asap. Meanwhile your fix looks correct. There is another instance of this in GuBV4_SphereOverlap.cpp.

PierreTerdiman commented 7 years ago

I reproduced the bug in a unit test. To fix it I just switched the lines this way:

if(ParamsAll->mNbHits==ParamsAll->mMaxNbHits)
    return 1;
ParamsAll->mHits[ParamsAll->mNbHits] = primIndex;
ParamsAll->mNbHits++;

Going to submit to our trunk, and it will make its way to GitHub "asap". Thank you for tracking this one down to the BV4 files.