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

MeshBVHHelper: Update BVH Helper for instances and batching #703

Merged
merged 1 commit into from
Sep 2, 2024

Conversation

gkjohnson
Copy link
Owner

@gkjohnson gkjohnson commented Sep 1, 2024

Fix #701

Adds MeshBVHHelper.objectIndex to support displaying a BVH for an individual BVH in an InstancedMesh or BatchedMesh.

cc @agargaro

@gkjohnson gkjohnson added this to the v0.7.7 milestone Sep 1, 2024
@agargaro
Copy link
Contributor

agargaro commented Sep 1, 2024

After you merge this PR, I can update BatchedMesh example to show it.

@gkjohnson
Copy link
Owner Author

That would be great! Thanks.

@gkjohnson gkjohnson merged commit be976e6 into master Sep 2, 2024
4 checks passed
@gkjohnson gkjohnson deleted the batched-bvh-helper branch September 2, 2024 03:21
@gkjohnson
Copy link
Owner Author

I'm trying to decide what needs to be added to BatchedMesh to support what we're doing here with official, stable APIs. Use _drawInfo isn't great in the long term since it's intended to be internal. I suppose it could be made public? Or we could add some functions:

batchedMesh.getInstanceVisible( instanceIndex ); // returns visible && active
batchedMesh.getGeometryIndex( instanceIndex ); // returns drawInfo.geometryIndex
batchedMesh.getGeometryRange( geometryIndex ); // returns geometry drawRange object

What do you think?

@agargaro
Copy link
Contributor

agargaro commented Sep 2, 2024

For me it would be better to have the methods, easier to use.

Is this already present?
batchedMesh.getInstanceVisible( instanceIndex ); // returns visible && active
https://github.com/mrdoob/three.js/blob/master/src/objects/BatchedMesh.js#L821

@gkjohnson
Copy link
Owner Author

So it is! We should adjust the extension function to use it in this line, then:

if ( ! drawInfo[ i ].visible || ! drawInfo[ i ].active ) {

If you'd like to make a three.js PR with the other two functions that would be great otherwise I can do so when I get a chance.

@agargaro
Copy link
Contributor

agargaro commented Sep 2, 2024

All right, I'll do it 😁

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.

Update MeshBVHHelper to display BatchedMesh, InstancedMesh BVHs
2 participants