Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

InstancedMesh raycast fix #685

Merged
merged 6 commits into from
Jul 2, 2024
Merged

InstancedMesh raycast fix #685

merged 6 commits into from
Jul 2, 2024

Conversation

gkjohnson
Copy link
Owner

Fix #671
Based on #673

cc @agargaro I made some of the remaining changes requested from #673 - let me know how it looks and I'll merge and publish!

@agargaro
Copy link
Contributor

agargaro commented Jul 1, 2024

Just a small consideration:

without using a scene bvh, a user usually needs to iterate over all the meshes in the scene to check which one is intersecting the mouse, this would mean calling the raycast function (and also decompose) N times.

If a user has 50k objects, decompose takes 2ms, instead of 0.5ms with custom method.
https://stackblitz.com/edit/getworldscaleper?file=index.ts

if it's okay for you, we can go ahead with the merge (after removing those 2 var from the tests) :)

@gkjohnson
Copy link
Owner Author

without using a scene bvh, a user usually needs to iterate over all the meshes in the scene to check which one is intersecting the mouse, this would mean calling the raycast function (and also decompose) N times.

Thanks! Good catch. It would be nice if three.js provided this scale function internally considering there's already Matrix4.copyPosition and Matrix4.extractRotation functions, as well. Adding Matrix4.extractScale( target ) might be an easy add to three.js if you'd like to give a PR a go. But in the mean time we can keep the function here.

@gkjohnson gkjohnson merged commit e7f3256 into master Jul 2, 2024
4 checks passed
@gkjohnson gkjohnson deleted the instanced-mesh-raycast-fix branch July 2, 2024 01:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

InstancedMesh regression in 0.7.5
2 participants