gkjohnson / three-mesh-bvh

A BVH implementation to speed up raycasting and enable spatial queries against three.js meshes.
https://gkjohnson.github.io/three-mesh-bvh/example/bundle/raycast.html
MIT License
2.38k stars 247 forks source link

StaticGeometryGenerator shouldn't automatically traverse children meshes #442

Open minitoine opened 1 year ago

minitoine commented 1 year ago

Hello,

https://github.com/gkjohnson/three-mesh-bvh/blob/5b2b27f30dfd3a55ce5b05a7fcb63fac806eac3e/src/utils/StaticGeometryGenerator.js#L316-L328

I see in your code that when you create a StaticGeometryGenerator with a SkinnedMesh, for example, you traverse the object to gather children meshes. Shouldn't this behavior be optional? I mean, you could want to build a static geometry of a skinnedMesh without incorporating children meshes of the scene graph.

gkjohnson commented 1 year ago

Hello! Can you describe your use case? Meshes are not typically included as children of other meshes / skinned meshes.

minitoine commented 1 year ago

For my specific case, I’m developing a custom editor (such as three editor) where it is possible to manipulate SkinnedMeshes via an ik system. To ease the creation/manipulation of ik chains, I’m attaching small target objets to specific parts of the skinnedmeshes.

As I am also using a custom rendering engine to render the scene as SVG, I need the StaticGeometryGenerator (was using ThreeBufferUtils before your class) to apply the bones transform to the geometry and create a HalfEdgeStructure from that.

Since the geometries get merged in the generator, I do have artefacts in my HalfEdgeStructure.

Of course, I can detach the objects before generate and reattach them afterwards, but I would say that you should provide the option to generate a new buffer for the skinned mesh only, even if it’s not the default option.

I can create a PR if you want.

gkjohnson commented 1 year ago

Perhaps it makes sense to ignore any meshes that are set to visible = false, then, to support a use case like this? What do you think?

minitoine commented 1 year ago

Hello. That might be a solution, but don't you think it's going to make things more complex than a simple flag/option?

gkjohnson commented 1 year ago

Probably - I was just suggesting another alternative. I think we can add a StaticGeometryGenerator.recursive flag that toggles this behavior if you want to make a PR!