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

Use Instanced Meshes in XRHandPrimitiveModel.js #21702

Merged
merged 1 commit into from
Apr 23, 2021

Conversation

zalo
Copy link
Contributor

@zalo zalo commented Apr 22, 2021

This PR switches the simple XRHandPrimitiveModel script to use the InstancedMesh class to draw the hand in a single drawcall.

Since OpenXR hands are only supported on the Oculus Quest, every drawcall is precious.

I've enjoyed a noticeable framerate improvement by switching to this rendering mode.

https://raw.githack.com/zalo/three.js/feat-instanced-hands/examples/?q=handinput

@mrdoob mrdoob added this to the r128 milestone Apr 23, 2021
@mrdoob
Copy link
Owner

mrdoob commented Apr 23, 2021

Merging this so it gets in r128 👍

@mrdoob mrdoob merged commit ea2e0f0 into mrdoob:dev Apr 23, 2021
}

this.handMesh.count = count;
if ( this.handMesh.instanceMatrix ) {
Copy link
Owner

@mrdoob mrdoob Apr 23, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't this always be true?

Copy link
Contributor Author

@zalo zalo Apr 23, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, yes; good spot!

That must have been a bit of vestigial superstition creeping in :-)

(EDIT: Aha, it was because I was burnt by unintialized color arrays before, but it seems like the matrix array is always initialized in the constructor, so it will always return true.)

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay! I'll change it 👍


}
this.handMesh = new InstancedMesh( geometry, jointMaterial, 30 );
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I honestly do not know how much this does, but I guess it would be good practice to add this?

this.handMesh.instanceMatrix.setUsage( THREE.DynamicDrawUsage ); // will be updated every frame

Copy link
Contributor Author

@zalo zalo Apr 23, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't know about this one; that does seem appropriate here. Thank you for letting me know about it!

I just tested it in a Quest 1 and I don't notice any additional perceptual framerate benefits, but perhaps it's something subtle like segregating the arrays to more volatile GPU memory, or reducing contention when interacting with other drawcalls somehow...

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's still a mystery to me 😁

@mrdoob
Copy link
Owner

mrdoob commented Apr 23, 2021

Here's the clean up: 8e50d5c
Hope I didn't break anything. I'll test on the Quest now.

@mrdoob
Copy link
Owner

mrdoob commented Apr 23, 2021

I broke it...

@mrdoob
Copy link
Owner

mrdoob commented Apr 23, 2021

Well, that was silly... c179013

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.

2 participants