While playing around with a Skeleton I noticed that it’s movements were alarmingly strange, and my investigation lead me to the inner workings of the Transform object. I was shocked to discover that its idea of the parent-child relationship seems quite different from what I expected it to be. I must be misunderstanding something because Transform is used for the scene graph and the entire scene graph couldn’t be broken without people noticing.
I expected that the local transformations of a child would be totally independent of the parent’s transformations, so you could rotate, scale, and transform the child however you like, and in the end it would just as if those transformations were pre-applied to the meshes when it is time for the parent transformations to take effect. In other words, I expect the overall transform should be no different than first applying the child transform to a vertex, then applying the parent transform.
I expected result1 and result2 to be equal for any pair of Transforms, but my experiments show it’s not true and I’m not sure how to deal with that. The effect works best when t2 is a scaling and t1 is a rotation. To my eyes my Skeleton looks strangely distorted when a parent is scaled and a child is rotated, and surely it is due to this same effect, but short of creating my own modified Skeleton system I don’t know how to make it work the way I want it to.
The scene graph doesn’t handle non uniform scale except on leaves of the scene graph, maybe that’s your issue. The reason is… I never got the actual reason but something along the line of “we use quaternion for rotations, and quaternion assume orthogonal matrix. Only way to fix it would be to use rotation matrices”…
That sounds like the source of the problem. The scaling that I used to create the problem was non-uniform. That’s awkward for anyone who wants to do non-uniform scaling. Is there a work-around?
I would love to know more about the relevant math. Is it an inevitable fact of math that certain transformations cannot be represented as (translation, quaternion, scale)? Or is this just a matter of not working hard enough to figure out the proper translation, quaternion, and scale?
No workaround I’m afraid. For the Math explanation I will defer to my brillant teammate @pspeed, because even if he will state otherwise, he’s the math guy in here
will do t1.scale * t2.scale, t1.rotation * t2.rotation, t1.translation + t2.translation in such order.
Affine transformations are not always commutative. For example, if you rotate and then translate object, results will be different than if you translate and then rotate. In your example, 2nd scenario uses cumulative rotation, but 1st scenario uses rotations, which are alternated with t1.translation.
Non-uniform scaling can’t be handled the way JME does transforms. (Though one could argue that skeleton transformations should probably support it…)
This is because JME splits the rotation, translation, and scaling into separate ops and applies them on the way down. A Quaternion is a compact form of rotation but it’s a rotation of orthogonal axes. Non-uniform scaling can cause axes to be non-orthogonal. (Imagine if you’ve rotated your coordinate system by 20 degrees and then scale down only one direction… now your rotated axes are not 90 degrees from each other anymore.)
JME could only support non-uniform scaling by accumulating things in a transformation matrix. There are some technical issues and tradeoffs with this approach that seemed too onerous to support the relatively rare need to have true non-uniform scaling.
As such, scaling is accumulated and only applied to the leaf Geometry of the scene graph. And in that case, it doesn’t make sense to put non-uniform scaling anywhere but on a Geometry… unless you already know your Geometry are all aligned to the same coordinate system axes.
I would love to know more about the issues. I see now how using quaternions makes certain transformations impossible, so that seems like a fairly big issue, but the issues on the other side are still a mystery.
As I understand quaternions, the standard motivation for using them is to avoid gimbal lock during successive rotations, but that doesn’t seem like it could be an issue when relating parents to children. The parents don’t care if some child’s rotations have become gimbal locked since the parents should never care about rotating along the child’s local axes. In other words, when you want to rotate along the child’s local axes, you would surely use a local rotation, not a rotation of the parent.
Surely it wouldn’t be so onerous to add a fourth field to Transform that would hold a Matrix4f. The matrix would be identity initially, and could only be set by calling combineWithParent. After that point I wouldn’t expect any calls to the getters and setters of translation, rotation, and scaling, so I would be tempted to make them throw an exception, but if we must preserve them then we could simply have the matrix hold the difference between the standard 3-transform representation and the actual transformation, so the matrix does only the part of the transformation that an ordinary Transform can’t do, and if the person using Transform is following the rules then the matrix will always be identity.
All we’d need to do in combineWithParent would be to calculate the combined transformation as both a matrix and a 3-transform, then multiply the matrix by the inverse of the 3-transform. That way when transforming a vertex get the total transformation by first applying the matrix and then applying the 3-transform. Surely that is not so onerous. We can save on computation and memory by using null for an identity matrix and checking for non-uniform scaling in the parent before computing the matrix.
The trade-off I see is that getRotation, getScale, and getTranslation will no longer provide exact and total information about the transformation in cases with nonuniform parent scaling, but in exchange we get a combineWithParent that exactly applies the child transformation followed by the parent transformation in all cases. Plus, we’d use slightly more time and memory.
We’d still have to keep the Quaternions and other things around because Matrix4f accumulates error over time and the axes become out-of-skew. In addition, quaternion concatenation is much faster over all.
Essentially, we’d be more than doubling the memory requirements for each and every standard spatial’s book-keeping to cover a use-case that has come up twice in 6 years. (And in my case where I original discovered all of this, I fixed it in the shader because I needed warped space.)
Edit: none of this has anything at all to do with gymbal lock, by the way. That’s an issue with Euler angles which are stupid to use in this case for 100 other reasons.
Okay, but surely that is resolved with the second part of my suggestion. I wasn’t suggesting that we get rid of the Quaternions. I was just suggesting that we include a matrix when we need one. It wouldn’t double the memory requirements so long as we only include the matrix when we need it and let the matrix be null the rest of the time. The only cost would be a single additional reference in the Transform.
Essentially what I’m suggesting is that instead of representing transformations as (translation)(rotation)(scale) we represent them as (translation)(rotation)(scale)(matrix) where the matrix is left null and ignored until someone puts a nonuniform scale in a parent. That way nothing has to change until someone breaks the rules, and in that case they pay whatever price in memory and accumulated error.
I look forward to your patches and continued maintenance in perpetuity for this feature that no one has needed so far.
…else, I don’t think it’s going to happen.
Edit: and note, it’s not just that change. You’d effectively have to create two entire transform pipelines for world state. One that worked with Matrix4f and one that works the way it does now. And any time a child starts to use a Matrix4 then its parents will also. Else every time something moves you have to recalculate the matrix of everything’s local transform in addition to the multiplication for accumulating the world transform.
Also, you’ll still have to provide a Quaternion for all of the code that expects world rotation to be a quaternion.
I must admit that I don’t know the system very well, but I’ve been poking around in Spatial.java and so far it seems that the entire transfer pipeline is based on Transform objects and combineWithParent calls. There is quite a bit of sophistication in Spatial, but none of that should matter if just changing Transform and combineWithParent will effectively change all the rest of it.
It’s not as if I’m proposing to have a new class called MatrixTransform to sit alongside the current Transform class. That would be a nightmare. I’m just suggesting that Transform be modified to eliminate the distortions that it currently causes when faced with nonuniform scaling in a parent. Unless the rest of the pipeline is doing something rather surprising, that one change ought to be both harmless, effective, and easy to maintain.
I was just thinking the opposite. Since the world transformation is calculated downward from the parents to the children, until the calculation gets to a point where matrices start to become necessary each worldTransform object can have a null matrix reference. We only need to create a matrix in combineWithParent under two conditions: either the parent has a matrix, or the parent has a nonuniform scale and the child has a rotation. This should mean that all the floating-point errors and memory bloat of matrices will be restricted to just those parts of the scene graph that actually need them.
A child would only need to use a matrix because something in its parents demand it. The idea isn’t to optionally put a matrix into the local transformation; that can always just be the usual (translation)(rotation)(scale). The matrix would only come in when its necessary to correct for the distortions caused by the transformations of the parent, so there would only ever be a matrix in the world transformation, and only in the Spatials that would have some distortion to correct.
If that is such a problem then we could just rule out having matrices in local transforms. They would serve no purpose that I can think of at the moment. The fromTransformMatrix method currently creates distortions if given the wrong sort of matrix and we can just allow it to continue doing that. That way the only way to introduce matrices would be through combineWithParent.
I’m trying to make it clear that I’m not suggesting we get rid of the Quaternion. We’d still have the Quaternion and it would still serve exactly the same purpose so that getRotation can still return exactly the same value that it always has and setRotation will still have exactly the same effect it always has. The only difference is that now Transform can contain an optional fourth transformation that corrects the distortion that can come from only using translation, rotation, and scale. The only code that ought to be broken by that is code that is expecting the distortion.
My particular immediate application is to have a skeleton-based animation with arms that change lengths, so I was scaling them down just their lengths and then expecting to be able to do rotations at the elbows. Of course I know that a skeleton is not the same thing as the scene graph and Bones don’t use Transform objects, so even if Transform were modified to allow nonuniform scaling it wouldn’t directly help me.
Even so, since I’m faced with implementing my own simplified skeleton system it would be nice to have Transform available as a tool to help me manage all the various transformations I will need to deal with. I think I shall make my modified version of Transform just to see if it works and use it in my skeleton system.
This is roughly what I have in mind. I’ve done some testing and this sort of thing seems to produce much more precise results, but it shouldn’t add any significant extra cost as long as we stay within the current restrictions on Transforms.
Naturally there a bunch of other little changes that I had to make to get it all to work, but I think the idea is clear from this part.
The point is that we know that Transform will do distorted transformations in certain situations, but we can correct that by including an additional transformation. We only need to compute and store it where it is actually necessary. The only time we pay a price is when Transform would otherwise fail, and in that case it is better to get the right answer late than get the wrong answer early.
So, first let me reiterate that this is a feature no one has really needed so far… in the six+ years I’ve been active here.
Potential use-cases fall into two mostly two large categories:
(the one I needed way back) “I want to deform a whole subgraph to warp space”. Better implemented in shader using a simple modification of the .vert shader.
“I want to reuse this box mesh and just resize it.” Better just to deform the mesh as you will want to be batching things for performance anyway… and such scaling will be lost in the batching process.
So, for a game engine, there are better ways to do this in the scene graph. For a CAD application, maybe. But for a game engine, it’s almost always a bad idea to make the scene graph do this. Whenever this has come up before, redirection was all that was needed and the results were better.
Now let’s go through the implementation.
Every time a non-uniform object moves, the proposed code will do all of the regular transform stuff that happens for normal spatials. It will ALSO create three new matrix objects (from components, not cheap) and perform additional math on those matrix objects… to include a matrix inversion.
Performing worldToLocal will also have to do some additional matrix inversion else we return bad results.
So the cost break down:
-additional code complexity to forever support two paths of processing
-additional GC overhead for objects that use this feature
-additional performance overhead for objects that use this feature (3x or 4x slowdown)… plus overhead for worldToLocal calls
-possibility of all kinds of edge cases that are missed and also require similar changes (for example, collideWith() will now return incorrect results for these objects)
-additional support burden in the form of forum threads that start “Why is my scene so slow?” and take 5 posts in before we figure out they accidentally set non-uniform scaling somewhere… versus instantly getting bad visuals when they do that.
Personally, I think the way easier solution is just to throw an IllegalArgumentException if Node.setLocalScale() is called with non-uniform scaling. ie: we’d only allow it on Geometry where it still makes sense in some isolated prototyping level use-cases.
The irony is that mesh deformation is exactly what I’m trying to do through Skeleton and lack of this feature is blocking me.
The Transform class is a class dedicated to one very specific job and this processing perfectly falls within that job, so it’s not as if the complexity were being mixed into a class with some other jobs so one might be confused over what it is trying to do. For example if we did something like this in the Bone class I would be concerned. In Transform, it seems quite simple. With a little testing we should be able to rest assured that it works perfectly and will never need to change.
Costly is better than impossible. The important comparison is how much overhead it adds to objects that don’t use this feature, and I think that’s a fairly small amount.
I was afraid of that sort of thing. Why does computeWorldMatrix not make use of worldTransform.toTransformMatrix()? Is there supposed to be some subtle difference between the matrices that they are computing, or is it just code duplication?
I like that solution. If something is going to be illegal, then it should at least throw an exception as if it is illegal. A similar exception should probably also be thrown in combineWithParent, since Spatial isn’t guaranteed to be the only place that Transform is used. Additionally the rule could be mentioned in the Javadoc everywhere it might be relevant, such as Spatial.setLocalScale and the main comments for Transform and Bone, and so on. This is a surprising rule and we should try to ease the blow of that surprise as much as possible.
Overall I suppose you’re right. It is frustrating that Skeleton can’t do what I want and that I can’t use Transform in implementing my own version of Skeleton, but for my use case my modified version of Transform really doesn’t do anything useful. It’s just going to be a massively slower version of using a Matrix4f directly. So I’ll just use Matrix4f instead of Transform.
