donmccurdy / three-pathfinding

Navigation mesh utilities for three.js, based on PatrolJS.
https://three-pathfinding.donmccurdy.com/
MIT License
1.2k stars 135 forks source link

Utils.mergeVertices generates distinct hashes for nearby values within tolerance. #221

Open closedcontour opened 8 months ago

closedcontour commented 8 months ago

In the course of getting strange, very long paths, I found that Utils.mergeVertices isn't merging vertices within tolerance distance of each other:

image

This debugging image shows the navmesh with vertices randomly colored. The discontinuity shows up because nearby but distinct vertices aren't being merged.

Just before I hit the "Submit" button, I searched a bit more and found that this is just a manifestation of https://github.com/mrdoob/three.js/issues/24621. It doesn't look like their mitigation was brought over. That said, I think I will use a rounding based approach before I hand over the BufferGeometry to three-pathfinding. All that said, I'm creating this issue in case anyone else runs into this.

Thank you for this and your other amazing three.js related libraries!

donmccurdy commented 8 months ago

Note that the newer implementation in three.js is not fully able to solve this situation either, though it does reduce the occurrences, see https://github.com/mrdoob/three.js/issues/24621#issuecomment-1246956856.

I've written a vertex merging implementation before that would avoid the issue entirely, I think, but it was very slow. I'm hesitant to drop that somewhat complex code into this project.

That said, I think I will use a rounding based approach before I hand over the BufferGeometry to three-pathfinding...

I think that's a great solution if it solves the problems for your meshes! Just note that you'll probably want to remove any vertex attributes other than position before doing this, so normals and UVs don't affect the merge.

I'd also be happy to review a PR upgrading Utils.mergeVertices to the newer approach in three.js, if that seems to be working better.