Assert with unified heightfields

[bdumesnil]

Hi,

When passing from legacy heightfields to unified heightfields we stumbled on the assert PX_ASSERT(inds[0] == vertIndices[a] || inds[1] == vertIndices[a] || inds[2] == vertIndices[a]); in PCMHeightfieldContactGenerationCallback::onEvent Here is the call stack :

PhysXCommon64_d.dll!physx::Gu::PCMHeightfieldContactGenerationCallback<physx::PCMConvexVsHeightfieldContactGenerationCallback>::onEvent(unsigned int nb, unsigned int * indices) Line 164   C++
PhysXCommon64_d.dll!physx::Gu::HeightFieldUtil::overlapAABBTriangles(const physx::PxTransform & pose, const physx::PxBounds3 & bounds, unsigned int flags, physx::Gu::EntityReport<unsigned int> * callback) Line 929   C++
PhysXCommon64_d.dll!physx::Gu::PCMContactConvexHeightfield(const physx::Gu::PolygonalData & polyData, physx::Gu::SupportLocal * polyMap, const __m128 & minMargin, const physx::PxBounds3 & hullAABB, const physx::PxHeightFieldGeometry & shapeHeightfield, const physx::PxTransform & transform0, const physx::PxTransform & transform1, float contactDistance, physx::Gu::ContactBuffer & contactBuffer, const physx::Cm::FastVertex2ShapeScaling & convexScaling, bool idtConvexScale, physx::Gu::MultiplePersistentContactManifold & multiManifold, physx::Cm::RenderOutput * renderOutput) Line 164   C++
PhysXCommon64_d.dll!physx::Gu::pcmContactConvexHeightField(const physx::Gu::GeometryUnion & shape0, const physx::Gu::GeometryUnion & shape1, const physx::PxTransform & transform0, const physx::PxTransform & transform1, const physx::Gu::NarrowPhaseParams & params, physx::Gu::Cache & cache, physx::Gu::ContactBuffer & contactBuffer, physx::Cm::RenderOutput * renderOutput) Line 224    C++
PhysX64_d.dll!physx::PxcPCMContactConvexHeightField(const physx::Gu::GeometryUnion & shape0, const physx::Gu::GeometryUnion & shape1, const physx::PxTransform & transform0, const physx::PxTransform & transform1, const physx::Gu::NarrowPhaseParams & params, physx::Gu::Cache & cache, physx::Gu::ContactBuffer & contactBuffer, physx::Cm::RenderOutput * renderOutput) Line 282   C++
PhysX64_d.dll!discreteNarrowPhase<0>(physx::PxcNpThreadContext & context, const physx::PxcNpWorkUnit & input, physx::Gu::Cache & cache, physx::PxsContactManagerOutput & output) Line 398   C++
PhysX64_d.dll!physx::PxcDiscreteNarrowPhasePCM(physx::PxcNpThreadContext & context, const physx::PxcNpWorkUnit & input, physx::Gu::Cache & cache, physx::PxsContactManagerOutput & output) Line 436 C++
PhysX64_d.dll!PxsCMDiscreteUpdateTask::processCms<&physx::PxcDiscreteNarrowPhasePCM>(physx::PxcNpThreadContext * threadContext) Line 446    C++
PhysX64_d.dll!PxsCMDiscreteUpdateTask::runInternal() Line 519   C++
PhysX64_d.dll!physx::Cm::Task::run() Line 67    C++

Our investigation lead us to functions Gu::HeightField::getTriangleVertexIndices and Gu::HeightField::getTriangleAdjacencyIndices. There seems to be a confusion between vertexIds, cellIds and trianglesIds.

Let's say for a ( nbRow*nbColumn ) heightfield there is :

- VerticesPerRow = nbColumn
- CellsPerRow = ( nbColumn - 1 )
- TrianglesPerRow = 2 * ( nbColumn - 1) 

But in getTriangleAdjacencyIndices adjacent triangles are calculated by adding/subtracting VerticesPerRow instead of CellsPerRow, making one of the adjacent index wrong by one. For example : adjacencyIndex2 = ((cell + mData.columns ) * 2) + 1; Instead of adjacencyIndex2 = ((cell + ( mData.columns - 1 ) ) * 2) + 1;

Similarly in getTriangleVertexIndices it is assumed that cellId == VertexId of first vertex in cell for instance :

vertexIndex0 = cell;
vertexIndex1 = cell + 1;
vertexIndex2 = cell + mData.columns;

Instead it should be :

const PxU32 row = cell / ( mData.columns - 1 );
vertexIndex0 = row + cell;
vertexIndex1 = row + cell + 1;
vertexIndex2 = row + cell + mData.columns;

because there is one more vertex per row than there is cells.

There is a few other places I have seen similar errors, you'll find an attached file containing a patch of some of the errors I found ( I only fixed those who triggered when testing there are probably others ).

0001-Heightfield-fixes.zip

It seems that unified heightfields are not safe to use yet, or maybe we missed something in our calculations.

[michellelu-nvidia]

Could you provide a repro for the geometry that causes this assert to fire?

Are you making use of features like holes in the heightfield?

It would be useful to know exactly what the issue is. There are several games that have successfully used unified heightfields over the past few years as it generally produces better behaviour than the legacy heightfield implementation.

[rextimmy]

I get the exact same assert as the OP does using unified heightfields, we had to revert back to using the legacy heightfields