search

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.5k stars 260 forks source link

Fix `InstancedMesh` raycast #673

Closed agargaro closed 3 months ago

agargaro commented 3 months ago

I fixed the problem related to getScaleWorld and added tests.

I need to clean up the code, but can you tell me if the InstancedMesh tests are okay?

Shall I add tests on BatchedMesh too? (after https://github.com/mrdoob/three.js/pull/28462)

related #671

gkjohnson commented 3 months ago

Shall I add tests on BatchedMesh too? (after https://github.com/mrdoob/three.js/pull/28462)

If you don't mind I think this would be good to have

agargaro commented 3 months ago

Accelerated raycasting does not seem to work on BatchedMesh. It is currently ignored because the temporary mesh geometry used for raycasting does not have the boundsTree property (unlike InstancedMesh, it is not replaced by another geometry).

https://github.com/mrdoob/three.js/blob/master/src/objects/BatchedMesh.js#L876

I don't think it is a good idea to create a BVH from the merged geometry (containing the repeated geometries), so I would say to wait until your new changes are released.

I have some ideas in mind, but it may be better to talk about them later, in any case I think it will be necessary to add the possibility of creating separate BVH based on a range of indexes.

I hope I am not wrong 😂

gkjohnson commented 3 months ago

Accelerated raycasting does not seem to work on BatchedMesh. It is currently ignored because the temporary mesh geometry used for raycasting does not have the boundsTree property (unlike InstancedMesh, it is not replaced by another geometry).

Oh ha yes I should have known this 😅

I don't think it is a good idea to create a BVH from the merged geometry

Definitely agree.

I have some ideas in mind, but it may be better to talk about them later

Yeah I think this is separate from this PR. #660 was opened in order to afford the ability to create a BVH from the subrange of a geometry specifically for BatchedMesh but it remains unfinished. We can ping or finish up that PR if we'd like to move it along. I think in the long run we'd want to keep a list of BVH's per batched geometry and then we'd have to add a special case in the acceleratedRaycast function to take advantage of it.

agargaro commented 3 months ago

We can ping or finish up that PR if we'd like to move it along.

Okay, if he cannot I will continue his work.