reactphysics3d icon indicating copy to clipboard operation
reactphysics3d copied to clipboard

Off-by-one error in TriangleMesh::removeUnusedVertices

Open glhrmfrts opened this issue 1 year ago • 2 comments

Hello, I download latest master just today.

I was trying to create a TriangleMesh to use with ConcaveMeshShape and I kept getting a {0,0,0} normal at index 0, which in turn crashed the application with an assertion failure.

My mesh doesn't use all the vertices provided because I have a shared vertex/normal buffer and several sub-meshes with different indices. This caused the TriangleMesh to have a lot of unused vertices.

The problem is in here: https://github.com/DanielChappuis/reactphysics3d/blob/master/src/collision/TriangleMesh.cpp#L220

Because of the i > 0 condition in the for loop, if the first vertex is not used, it is not removed from the mesh, which causes a zero-length normal to be leftover and causes more problems down the line.

I fixed by changing the type of i to int64 and using a i >= 0 condition:

for (int64 i = mVertices.size() - 1; i >= 0; i--)

Ofc you can also keep the uint32 type and use a i < mVertices.size() condition, but I didn't like to rely on the unsigned number wrapping around.

If you want I can open a PR.

Thanks for the great library!

glhrmfrts avatar Oct 23 '24 05:10 glhrmfrts

Thanks a lot for taking the time to report this.

Do you have an example of this issue with a very small triangle mesh? It would me reproduce the issue on my side.

DanielChappuis avatar Jan 08 '25 22:01 DanielChappuis

I have fixed the issue with this commit in the develop branch. This will be available in the next release of the library.

Thanks a lot for taking the time to report this issue.

DanielChappuis avatar Feb 16 '25 16:02 DanielChappuis