Revert transform back to using Mat4

[lassade]

The article on #596 miss interpreted and there is no compelling reason to use TRS for global transforms, here is what I found:

• Mat4 is need to apply any transformations, this means way more TRS to Mat4 conversions will be need than Mat4 to TRS. So keeping a Mat4 is faster and more convenient
• The article mentioned in the issue Moving transform data out of Mat4 #596 shows that matrices drifts when too many multiplications are made over it. But what the transform system actually do is to calculate the global matrices by multiplying the parent matrix with local TRS which then will be carried down to the next child. So the drift will be only carried away through the hierarchy chain, meaning that, according with the article, the 200 child will have a drift of 0.000001 and having such deep hierarchy isn't realistic in a game context
• Mat4 multiplication is ~1.9 times faster than TRS, this affects directly the transform_propagation_system
• Mat4 point transform is ~5.3 times faster than TRS and that is a very common operation!
• Mat4 vector multiplication is ~2.4 times faster than TRS
• LocalToWorld translation is trivially accessible, it's just field like in TRS so nothing changes!
• LocalToWorld access to right, up, forward vectors is marginally cheaper than TRS so nothing changes!
• LocalToWorld shouldn't be modified directly, because the changes won't propagate
• Why name it LocalToWorld, because GlobalTransfrom confuses people into thinking they should modify it, the ecs approach doesnt allow for transform.position = ... like in traditional engines, because the transformations WONT propagate; Later on we want to add a WorldToLocal it can be useful in some situations.
• We can have a LocalToWorld2D that 2D and 2.5D games will only pay for what they use, a mixed hierarchy using both types is sort of simple to understand (see a WIP in here)
• Mat4 can encode shear and other types of deformations

A few disadvantages

• TRS to Mat4 is ~4.5 times faster than Mat4 to TRS
• Mat4 inverse is not always possible
• TRS inverse is ~1.7 times faster than Mat4 inverse
• TRS any direction is ~1.2 times faster than Mat4 and that is a very common operation!

Sorry for wanting to break everyone else code

[lassade]

Converting a Mat4 to TRS my result in a double cover Quaternion which may mess with some interpolation code that don't account for this, which isn't very common.

Not a really big problem

[CptPotato]

👋 A related issue came up on discord earlier today. I'll try to give some context and my opinion:

As part of a PR (#2049) I double checked if child-entities inherit the scale of their parents, which could cause some issues with my algorithm through shearing. At first I thought it didn't because GlobalTransform can't encode that properly (as @lassade also points out above). We then found that the scale is inherited anyway but simply multiplied (src) without taking rotation into account (possibly by accident in #596).

This can lead to unexpected behaviour if parent-child hierarchies, rotation and non-uniform scaling are mixed together at the same time. More specifically, scaling the parent along its x-axis would scale the children each along their own (local) x-axis. The change @lassade proposed here would potentially fix that (which is how all that ties into this specific issue).

In general I see different ways to approach this, each with its own quirks and limitations, and I feel like all of them have their own values. Note that this is just my personal view, opinions of more experienced users are welcome:

1. inherit scaling correctly (=> this issue, also my preference)
   • requires to change the type of GlobalTransform into something that can propagate scales (Mat4)
   • usually this is the expected behaviour
   • things like physics can get tricky if users rely on shearing
2. don't inherit scaling at all
   • will not cause shearing (can allow for simpler & more optimized code)
   • works with the current GlobalTransform representation
   • probably not what users expect
   • gltf import requires hacks (applying transforms on import)
3. keep the current approach
   • it works for most common cases
   • ..but causes unexpected results otherwise

---

Apparently there were discussions similar to this before, so sorry if I brought up a problem that has already been solved / dismissed.

I guess all in all I'd like to bring some awareness to this issue, weighing the possible options - even if it's only confirming that the current approach is preferred - and documenting the limitations of that at some point.

[alice-i-cecile]

Adding the high-impact and help-wanted tags: the Transform bugs are reasonably serious and this seems to be the place where that discussion around fixing that should occur, at least to start.

[olivia-fl]

The article mentioned in the issue #596 shows that matrices drifts when too many multiplications are made over it. But what the transform system actually do is to calculate the global matrices by multiplying the parent matrix with local TRS which then will be carried down to the next child. So the drift will be only carried away through the hierarchy chain, meaning that, according with the article, the 200 child will have a drift of 0.000001 and having such deep hierarchy isn't realistic in a game context

I don't think the issue is really with propagating transforms from parents through the hierarchy, it's with incremental updates between frames. If we want to use only the Mat4 representation for transformations, that means that every time the rotation of the local transformation is changed (via a set_rotation method or such), it will accumulate a small error, which will compound over multiple frames. Because of this, I don't think using Mat4s for the "source of truth" representation is an option.

There's a problem with our current TRS representation, which I don't think has been discussed outside of #2055, which is the order of application for different components. Our current implementation applies rotation, then scale, then translation (it also generates matrices by incorrectly applying SRT instead of RST, but that's a solvable problem). The RST ordering is pretty uninituitive, and is uncommon in graphics and game development. Unfortunately, this is actually the only representation we can use if we want to support nonuniform scale. The reason is that two affine transforms without a shear component can compose to generate a transform with a shear component. Shear cannot be represented with SRT ordering, but it can be represented with RST (because 'S' here is really more a combination of scale and shear than it is pure scale). Note that with uniform scale, shear is not an issue, and SRT and RST are equivalent.

There is no real clean solution to this problem, and I think the right choice is to either keep the RST ordering and just fix all the bugs, or to split into separate components for uniform and nonuniform scaling with different semantics.

Either way I think it makes sense to have a TransformationMatrix and GlobalTransformationMatrix component and a system to generate these every frame, since recalculating the matrix representation on the fly every time you need it is not efficient.

[cart]

@Benjamin-L

Because of this, I don't think using Mat4s for the "source of truth" representation is an option.

Yup this is one of main reasons we opted for "similarities" over mat4 as the source of truth.

There is no real clean solution to this problem, and I think the right choice is to either keep the RST ordering and just fix all the bugs, or to split into separate components for uniform and nonuniform scaling with different semantics.

My personal preference is to keep the current Transform representation(*) and to fix the transform op bugs. Separate components introduces extra complexity to entity construction and makes it way harder to do operations that require the values of multiple "source of truth" components (ex: transforming a point using Translation, Rotation, and Scale components requires querying for all three, then using some abstraction to group them and do the transform op).

I don't yet have an opinion on SRT vs RST. I don't fully grok the implications of each yet.

(*) including non-uniform scale ... it creates number of problems (physics, more expensive to calculate, etc) but in practice people want to squash and stretch stuff. I want that to work by default. Later we can internally optimize for uniform scales if it becomes a major concern (ex: consider making scale an enum Scale { Uniform(f32), NonUniform(Vec3) }, although that has ergonomics implications and maybe perf implications due to more branching) .

Either way I think it makes sense to have a TransformationMatrix and GlobalTransformationMatrix component and a system to generate these every frame, since recalculating the matrix representation on the fly every time you need it is not efficient.

My main problem with making these separate components is that they can't have "up to date" values during arbitrary points in the UPDATE stage, so they won't be useful for game logic unless the user manually inserts hard TransformMatrix sync points between GAME_LOGIC_A and GAME_LOGIC_B (which currently means adding multiple stages). They might save one or two conversions by "internal systems" that run after the UPDATE stage, but generally the only thing that needs the final Mat4 is shader uniforms. And we generally only generate those uniforms once per entity, so caching the Mat4 wouldn't save us much. Most game-related requirements for Transforms (editing translation/rotation/scale, transforming a point or other transform, etc) can be done without recalculating the Mat4. Things that do require Mat4s could create their own caches if perf becomes a problem.

We could consider embedding Mat4s into the Transform component and incrementally keeping them up to date, but that has a number of negative implications:

1. Naively, each transform op gets more expensive: rotation now involves updating the Mat4 and the rotation Quat (and the drift problem is re-introduced). Instead we'd probably need a "dirty" bit that gets flipped and we would lazily recalculate the Mat4 when it is requested.
2. Adding a Mat4 to Transform makes it less cache friendly. We want Transform to be as small as possible.

@lassade

Mat4 vector multiplication is ~2.4 times faster than TRS

Can you link to sources / benchmarks for these numbers?

[lassade]

@cart the benches are here

[olivia-fl]

Note that this benchmark is with the current (broken) composition implementation, which is going to be faster than the correct implementation.

[olivia-fl]

My main problem with making these separate components is that they can't have "up to date" values during arbitrary points in the UPDATE stage, so they won't be useful for game logic unless the user manually inserts hard TransformMatrix sync points between GAME_LOGIC_A and GAME_LOGIC_B (which currently means adding multiple stages).

yeah, this is a good point. I don't think using the raw matrices is going to be a particularly common requirement anyway, especially if we flesh out the Transformation API to add more useful operations. I also agree that storing a Mat4 alongside the TRS values would be bad, for the same reasons you stated.

For SRT vs RST, if we do want to support nonuniform scale by default, we effectively have the choice between the relatively unintuitive RST representation, which is closed under composition, and the intuitive SRT, which is not. I think it makes sense here to go with RST to avoid introducing complexity to users that only care about uniform scaling, since it is much more common. No matter what, we need to document this carefully.

[lassade]

Local transform should be kept the same, I only want to change the GlobalTransform to Mat4 witch shouldn't be changed directly