Apply instance scale to its matrix transform

[krispya]

This fixes https://github.com/pmndrs/use-cannon/issues/367

This is a proof of concept for a solution to the problem. I'm not convinced decomposing an instance matrix * the count of instances every frame is good for performance, but maybe it is good enough? A more robust solution might involve adding a scale Float32Array along with position and quaternion that auto updates or can be updated with a callback.

[vercel[bot]]

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/pmndrs/use-cannon/8aEysgfuJ78HmWQxFpruhqQSL2xN
✅ Preview: https://use-cannon-git-fork-krispya-exp-instance-scale-pmndrs.vercel.app

[isaac-mason]

Thanks for raising this POC PR! 🙌

Warning: I'm no expert on JS performance profiling! But this change appears to cause a bit of a performance drop.

I tried comparing these CubeHeap examples: https://use-cannon-git-fork-krispya-exp-instance-scale-pmndrs.vercel.app/#/demo/CubeHeap https://cannon.pmnd.rs/#/demo/CubeHeap

On the preview deployment, worker.onmessage takes around ~17ms-20ms On the live deployment, worker.onmessage takes around ~11-12ms

I'm not sure whether that's an acceptable performance hit or not. There seems to be some visible performance impact to me? But I'd need to have a more thorough look. I'm also not sure if the preview build is different to the prod build - need to check!

[krispya]

I ran my own tests and was getting similar results.

Just passing a scale vector3 of (1,1,1) got a worker.onmessage of around ~10.5ms. This means no calculation done and is equivalent to old behavior. Using decompose inside of getMatrix, I was getting times around ~16.66ms. So about a 58% increase in time. Not good! That tracks with what I was reading here where it was warned by Mugen that decompose can be an expensive operation. I also noticed it was calling setRotationFromMatrix as part of decompose, so there is a lot going on there.

So I optimized getScale to use setFromMatrixScale instead and now I am getting times around ~12.5ms. This is a 19% increase, so much more palatable, but again it's a 19% performance drop that you might not be benefiting from if you your instance scale is not changing per frame.

This exploration also showed me that frameHandler is being called multiple times per worker.onmessage. This is because it just loops on useFrame which goes at whatever the refresh rate of the target monitor is, while the Cannon physics world is on a fixed tick. A further optimization I'm going to look at will be locking the matrix updates on frameHandler to only happen once per physics step.

[isaac-mason]

Nice! That's a great improvement.

We could make instanced mesh scale handling opt-in if we don't see any further ops for improvement + people don't think the increase is palatable? Though that kind of sucks - I would find that surprising 🙃

e.g. some prop like

<Physics instancedMeshScale>
...
</Physics>

Would be good to hear opinions on the ~19% increase, and whether anyone sees any other performance opportunities.

[krispya]

It's a tough pill to swallow when the majority of use cases with instancing likely won't have the scale change dynamically. Maybe we can have method for the api that is updateInstancedMeshScale. Set to false it always passes [1,1,1] which is previous behavior. Set to true, it always updates which is the new behavior. And then optionally you can pass a callback where the return value updates an array of InstancedMesh scales held by use-cannon. This way you can only perform the update when you know you need to. In my demo, I would call this once during the initial loop where I set all the scales on mount and I should then have no performance issues.

It's not a set it and forget it solution but it should solve the issue for people who need it.

[isaac-mason]

Having an option configured per body sounds better than enabling it globally for everything.

Maybe this could be a body prop passed to hooks?

const [ref, api] = useBox((i) => ({
  args: [1, 1, 1],
  position: [0, i, 0],
  updateInstanceMeshScale: true,
});

Do you see a use case for changing updateInstanceMeshScale after creating a body?

@bjornstar - would be keen to hear your thoughts on this

[krispya]

Yes, it would be necessary if the user has any kind of scale animation for an InstancedMesh. For example, this has scaling of the InstancedMesh on hover: https://codesandbox.io/s/instanced-vertex-colors-8fo01

How about splitting it up. The prop on the body can be instancedMeshScaleAutoUpdate that is true or false like three. Then the api can have a updateInstancedMeshScale((index) => { }) method that allows you to manually update the array of scales. This would only do something if instancedMeshScaleAutoUpdate is set to false, otherwise it is auto updating anyway.

[bjornstar]

Cannon does not support scaling so I'm not sure what our options are here.

I agree that Cannon overriding your scale is not desired, but doing matrix copies to fetch the scale on every instance on every frame is not performant. If we had a scale property on the instance we could use that, but we don't.

I think the best approach here is to add a scaleOverride to our API that is totally client side. We store the scale overrides on the client and when we get a frame from the worker, we check to see if there is a scaleOverride and use that when applying our updates.

[krispya]

I think so. The only alternative I can think of is you add the scale as a buffer attribute to the instance so it can be read without being calculated. I don't know what that interface would look like though.

[bjornstar]

I went ahead and implemented a scaleOverrides property on the WorkerAPI in #372

I'm not convinced that it's a good idea to pretend to support scaling because it has no reflection in the physics world.

[bjornstar]

I'll go ahead and release with that solution. Please give it a try and let me know how it goes. I'll close this one in the meantime.