GraphNode setPositionAndRotation

[kpal81xd]

Fixes #6317

Adds a method to graph node class for apply both position and rotation together.

Performance Testing

Average of 5
1e6 Iterations
Random Positions [-5, 5]
Random Rotations [0, 360]

setPos + setRot	setTRS	setPosRot
178ms	18.6ms	105ms

Using entity.getWorldTransform().setTRS directly is the fastest however the local positions and rotations are not updated and so neither is the local transform.

[mvaligursky]

Should the table compare

• setPos + setRot times
• setPosRotCombined times?

I do not see any timing of setRot to conclude the combined functionality is not an improvement. Would you have some numbers here @kpal81xd ?

[kpal81xd]

Should the table compare

• setPos + setRot times
• setPosRotCombined times?

I do not see any timing of setRot to conclude the combined functionality is not an improvement. Would you have some numbers here @kpal81xd ?

The screenshot names are actuallly incorrect the setPos should be setPos + setRot. Ill updated the screenshot

[mvaligursky]

So in this case, the combined functionality has a nice performance benefit (34% faster), and we should definitely consider including this in the engine.

[kpal81xd]

So in this case, the combined functionality has a nice performance benefit (34% faster), and we should definitely consider including this in the engine.

This api of having setPositionAndRotation is similar to Unity's so should we also include getPositionAndRotation likewise? https://docs.unity3d.com/ScriptReference/Transform.GetPositionAndRotation.html

[mvaligursky]

By a quick look at the code, I'd say that this is not likely to win much performance, so perhaps not, as those are a simple functions.

[mvaligursky]

Looking at the new function, it does not seem to work the same way the two separate functions work though, which is undesirable?

[kpal81xd]

Looking at the new function, it does not seem to work the same way the two separate functions work though, which is undesirable?

Wdym? The end result of the transformation? Or the fact that the function uses the transform of the node as opposed to the parent transform directly?

[mvaligursky]

The individual functions transform the provided world space position and rotation to the parent space.
But the new function pretty much assigns those to local space position and rotation, ignoring the parent transform.
So the resulting local space position / rotation will be different between the functions.

Add a unit test for it and compare the results with old and new functions.

[mvaligursky]

And the performance benefit is smaller now, around 8.5%. Any way to optimize this?
One way would be to avoid doing the expensive Mat4 inverse, as these matrixes are affine and so the last column (or row?) is always (0, 0, 0, 1). Would could likely do something with the Mat3.invertMat4 and just handle the translation separately. Or add Mat4.invertAffine (similarly to mulAffine2 it has). But perhaps at least we can avoid inverting the rotation, and extract it from the inverted matrix that we already have?

But perhaps a separate PR for the affine part, if we ge to it.

[kpal81xd]

And the performance benefit is smaller now, around 8.5%. Any way to optimize this? One way would be to avoid doing the expensive Mat4 inverse, as these matrixes are affine and so the last column (or row?) is always (0, 0, 0, 1). Would could likely do something with the Mat3.invertMat4 and just handle the translation separately. Or add Mat4.invertAffine (similarly to mulAffine2 it has). But perhaps at least we can avoid inverting the rotation, and extract it from the inverted matrix that we already have?

But perhaps a separate PR for the affine part, if we ge to it.

I modified the code to use the existing inverse matrix instead of calling getRotation which re generates the parent world transform. Looks like a ~40% speedup

[mvaligursky]

Nice!

[willeastcott]

Presumably we can now swap out individual calls to setPosition and setRotation to this function across the engine codebase.

[AlexAPPi]

This method is not included in v1 of the engine, can you include it in the build?

[mvaligursky]

I just cherry picked it to v1, this will get released in the future.