Fix Transform2D and Transform3D scaled and rotated methods

[madmiraal]

Currently, Transform and Transform2D's scaled() and rotated() methods are performed in the parent reference frame (i.e. left matrix multiplication). This may make sense from a mathematical point of view, but it's not intuitive from the user's point of view, who expects the transform to take place in the local reference frame. Furthermore, it prevents these methods from being chained. To make it even more confusing, the translated() method does the intuitive transform in the local reference frame (i.e. right matrix multiplication). The result is, there is currently no simply way to recreate an editor's Transfrom's Translation, Rotation and Scale using a series of translated(), rotated() and scaled() methods.

This PR makes the scaled() and rotated() perform a local transformation (i.e. right matrix multiplication) so they behave the same way as translated(). This not only makes them more intuitive, but it allows them to be chained.

Below are 2D and 3D demo projects showing how, with this PR, an object's Translation, Rotation and Scale can be easily recreated using chained translated(), rotated() and scaled() methods e.g.:

	var xform = Transform.IDENTITY
	xform = xform.translated(Vector3(-1.5, 0, 0))
	xform = xform.rotated(Vector3(0, 0, 1), deg2rad(20))
	xform = xform.scaled(Vector3(1, 1.5, 1))
	transform = xform

2DTransforms.zip
3DTransforms.zip

Properly fixes #34329
Closes godotengine/godot-proposals#1336

[Zireael07]

Will it change results/behavior for people's existing projects or is this just a mathematical fix that won't affect gameplays?

[madmiraal]

Will it change results/behavior for people's existing projects or is this just a mathematical fix that won't affect gameplays?

This change breaks compat. However, only projects that have tried to combine multiple translated(), scaled() and rotated() calls should be affected, because it affects the way rotated() and scaled() are combined with the existing Transform. If the existing Transform is the the default IDENTITY then no, there will not be a difference.

If you're trying to rotate or scale a Transform that has already been translated, rotated and/or scaled, then yes, it will affect the behaviour of an existing project. Obviously, since combining transforms is not working as expected at the moment, it's unlikely to affect many existing projects: only those projects where people have made a concerted effort to get it to work for them e.g. the one described in godotengine/godot-proposals#1336. For them, this change should allow that code to be a lot simpler.

[kleonc]

Transform and Transform2D's scaled() and rotated() methods perform left matrix multiplication on the transform. This makes sense from a mathematical point of view, but it's not intuitive from a user's point of view, and it prevents these methods from being chained.

These methods (plus translated() if changed to match the convention) can be chained whether they perform left or right multiplication. The only difference between left and right multiplication is the order in which consecutive transformations are being applied.

var S := Transform2D.IDENTITY.scaled(s)
var R := Transform2D.IDENTITY.rotated(r)
var T := Transform2D.IDENTITY.translated(t)

# If scaled(), rotated(), translated() perform LEFT MULTIPLICATION then these are equivalent:
var transform := Transform2D.IDENTITY.scaled(s).rotated(r).translated(t)
var transform := T * R * S

# If scaled(), rotated(), translated() perform RIGHT MULTIPLICATION then these are equivalent:
var transform := Transform2D.IDENTITY.scaled(s).rotated(r).translated(t)
var transform := S * R * T

So since multiplying transforms behaves the same as in math (factor transformations are applied from right to left):

• for left multiplication transformations corresponding to the chained method calls are applied in the method call order,
• for right multiplication transformations corresponding to the chained method calls are applied in the reversed method call order.

Thus I find left multiplication being way more intuitive.

Besides that I think left vs right multiplication is not a proper discussion. I do think the proper solution is providing all 6 methods as proposed in godotengine/godot-proposals#1336. Then whether left or right multiplication would be the default one (used by scaled(), rotated(), translated()) would be a minor problem.

[tagcup2]

This shouldn't be merged.

The premise is based on some misconceptions, which I'll try to clarify below. But the solution is very simple (the last item at the bottom).

1. Godot uses column vectors.

As such, matrices act on vectors as T2*T1*v, not v*T1*T2.

The action of a matrix on a column vector is u = M*v, or u_i = M_ij v_j explicitly (sum over repeated index j). It is equivalently possible to use the dual vector space of row vectors, in which case it becomes u = v*M which is u_i = v_j M_ji.

The way the data is stored is different in Basis and Transform2D (row vs column major), but this only affects the internal implementation details, and is transparent to the user and is irrelevant in this context (By the way, I offered to change the internals in the past, but reduz wanted to keep it this way for the convenience of easily getting the columns, so instead, this point lives on as a comment in transform2d.h).

This is why we type T*v instead of v*T, and why the following code

	var v = Vector2(1,2)
	var T = Transform2D(PI/3, Vector2(0,0))
	print(T*v)

returns (-1.232051, 1.866025), and not (2.23..., 0.13...).

2. The operation order on column vectors is read from right-to-left

There is only one ordering of operations that act on column vectors:

u = M2*M1*v

There is nothing counter-intuitive about this, this is just how operators (whether they are functions or matrices) work. When you type g(f(v)), it means f acts first, g acts second.

3. What is a transform?

A transform represents T*R*S, where S,R,T represent scaling, rotating and translation operations. The ordering of these operations matters, which means that TSR and TRS are different things. (barring trivial situations such as a rotation by a multiple of 180 degrees, or a uniform scaling as in #34329, which commute with everything since they are proportional to identity matrix).

Moreover, even though R2*S2*R1*S1 is a valid matrix (which you can create using the API), it is not a valid transform in general, and things will break. (There are a few checks behind MATH_CHECKS which may catch such mistakes.)

Internally, Godot uses a matrix to store the R*S part combined, and a vector to store the T part. Godot doesn't ensure that the stored data is a valid Transform.

You can "chain" arbitrary number of T, R and S operations in arbitrary order, but that results in garbage data and things will break. Only T*R*S is a valid Transform.

The following code from the original post

var M = Transform.IDENTITY\
    .scaled(... S_1 ...)\
    .translated(... T_1 ...)\
    .scaled(... S_2 ...)\
    .rotated(... R_1 ...)\
    .translated(... T_2 ...)\
    .rotated(... R_2 ...)

would produce an invalid transform in general, so there is no real use case for operations like that (regardless of the syntax, be it "chaining" or nested function calls).

4. Local and global transforms, not right or left multiplications

When we type a matrix or a vector, the numbers that go inside are coordinates with respect to some frame.

If we have do different frames, and know how these two are related to each other (for example "frame F is related to frame G by 60 degrees rotation and then 2x scaling" etc, which can be written as a Basis B), we can convert matrices and vectors between frames.

Vectors transform as vG = B*vF, and matrices transform as MG = B*MF*Inverse(B).

In Godot, a Transform is written in the global frame (or if the node has a parent, the local frame of the parent). In 3D, it sometimes makes sense to work in node's local frame (for example, when you want to tilt the characters head up/down in a FPS, that's a rotation around the "left-right" axis of the character). This can be done as follows. Suppose that the node has the basis B0 = R0*S0, and we would like to perform some rotation R (say, rotation around x, call it Rx) in the local frame.

The rotation in the local frame is given by B0*Rx*Inverse(B0), which performs a rotation about the axis that is aligned with the character's arms, if you imagine it being in a T-pose. Then, if you apply this on top of the existing B0, you get

B0*Rx*Inverse(B0) * B0 = B0*Rx

Although the end result effectively looks like we are doing a "right multiplication", that's not what we did at all. We applied a rotation on top (=multiplication from left) the existing basis, as usual.

There is no such thing as "more intuitive right multiplication". What we have is global or local transformations, and they are two different things.

Basis has both global and local versions of the operations (the latter has _local suffix). When I added those functions a few years ago, I also considered doing the same for Transform2D but ended up not doing it because all rotations in 2D commute. However, it can still be relevant in situations which involve nonuniform scales in conjunction with rotations.

5. What should be done?

The root of the real issue is the confusion caused by the following inconsistency: rotated and scaled functions are in global frame, whereas translated is in local frame.

To make the API consistent, _local versions of these functions should be added (similar to the way Basis works). In Transform2D and Transform, translated should be renamed as translated_local, and a global version (which does t.origin += v) should be added.

There should also be a piece of documentation which explains what global and local rotations are for those aren't familiar with those. (Once upon a time, I started a documentation, which included a section on this, but it was deemed to be too technical and removed. That might be in the git history of the docs repo somewhere and could be used as a starting point)

[madmiraal]

The root of the issue is that rotated() and scaled() do not currently behave the same was as Rotation and Scale do in the inspector. The behaviour in the inspector is the intuitive behaviour. Rotation should not be relative to the node’s position with respect to the parent or, in the simplest 2D case, the top left corner of the viewport. Rotation should be around the node’s centre. Similarly, scaling should be independent of the node's current position and rotation.

For example, for a Sprite2D positioned in the centre of the screen: transform = transform.rotated(deg2rad(30)) should result in the Sprite2D rotating around its centre (left image) not around the top, left corner of the viewport (right image):

Correct: Inspector Rotation and rotated() with this PR	Incorrect: Current unexpected rotated() result

Similarly, for a Sprite2D positioned in the centre of the screen and rotated around its centre: transform = transform.scaled(Vector2(1,1.5)) should result in the Sprite2D being made taller (left image) not shifted down and distorted (right image):

Correct: Inspector Scale and scaled() with this PR	Incorrect: Current unexpected scaled() result

[tagcup2]

What you are seeing is exactly what a transform followed by a global rotate/scale should do. What you want is object-local versions of them, which can be implemented in the same way as Basis::rotate_local.

The only issue here is, both global and local operations should be provided, they are both valid and different things.

And BTW, again, you're not supposed to do do transform followed by global rotate/scale, because it results in an invalid Transform which doesn't have the form TRS; various code-paths under core/math and physics rely on the definition that Transform is TSR matrix..

[bluenote10]

Moreover, even though R2S2R1*S1 is a valid matrix (which you can create using the API), it is not a valid transform in general.

It is still a valid affine transformation, right? Invertible affine transformation form a group, and an affine transformation does not require orthogonality. The whole point of using affine transformations / homogeneous coordinates in computer graphics is to have a unified framework for scale, rotation, translations and perspective transformations (which you'd have to call "invalid" as well?) to easily transition from model space <=> world space <=> camera space. Non-TRS transforms in user space may not be super common, but there are certainly valid use cases for sheared transforms.

various code-paths under core/math and physics rely on the definition that Transform is TSR matrix

Since affine transformation are in no way limited to TSR, these code paths should handle this implicit assumption pro-actively by checking if their pre-condition is satisfied.

To make the API consistent, _local versions of these functions should be added (similar to the way Basis works). In Transform2D and Transform, translated should be renamed as translated_local, and a global version (which does t.origin += v) should be added.

From all my experience with other libraries and working with students/teams in physics/robotics, I have the impression that our brains can much easier think in left/right matrix multiplication (or rather after/before the current transform) to put a chain of transforms together. Perhaps because most people first touch the topic of transform chaining by multiplying matrices in linear algebra courses. Conversely when discussing transform topics, I've seen many people get confused by speaking of local/global, perhaps because what "global" really means is vague when you are only partly through a transform chain. In other words: If you are thinking in camera space, world space, model space, submodel space, ... thinking in local/global is ambiguous.

I'd simply call it a day and offer all 6 variants like other libraries do, keeping the entry barrier low for people from other ecosystems.

[tagcup2]

Godot isn't a general-purpose geometry or linear algebra library, it's a game engine.

Godot's Transform (or Unity/Blender/Maya/Bullet/etc) doesn't implement general affine transforms, and valid elements of Transform are not closed under multiplication. As I explained above, it is designed to implement a TRS operation only. In fact, Bullet has additional restrictions on S (as well as some other code paths in Godot). This is a working trade-off for modeling and game physics, and allows various optimizations.

As I made it very clear above, "global" meant the local frame of the parent node, and if there is no parent, it's the global frame. In the docs, I referred to it as the parent-local frame. In Basis, this is default so is no suffix for it. Local means object-local. They are very well defined frames without ambiguity.

3D->2D camera projection (which is not even invertible) is different and not a part of Transform. There are functions for that in Camera nodes.

True global transformations aren't part in the API because they will often result in something which cannot be represented as TRS (unless the intermediate nodes up in the chain are uniform scaling matrices and/or the rotations are multiples of 180 degrees). Internally, such it is nonetheless used is various places internally in the form of manual transformations (skeleton code on top of my head) under various assumptions (that can be easily broken by the user), so maybe they can be added provided that the constraints are well documented.

[bluenote10]

... and elements of Transform are not closed under multiplication.

They are according to the type system.

[kleonc]

For the simplification, below I'm refering just to 2D.

The root of the issue is that rotated() and scaled() do not currently behave the same was as Rotation and Scale do in the inspector. (...)

@madmiraal Seems like there's a misconception in your argument. What you see in the editor inspector are Node2D's properties: position, rotation, scale (and skew in 4.0). Changing these properties results in recreating Node2D's transform based on these values (in 3.x result is a TRS transform), it doesn't result in calling any of the methods this PR is about. So you're not comparing equivalent operations. Moreover, you're referring to what is intuitive for Node2D, not for Transform2D. I guess utility (non-chainable) methods like Node2D.rotate(), Node2D.apply_scale(), Node2D.translate() are provided exactly for the use case you're talking about (they do what's intuitive from the Node2D's perspective).

Transform2D is (and should be) independent from Node2D and in my previous comment I've explained why I think doing right multiplication by default is not intuitive (chained transformations would be applied in the reversed call order).

Besides that this PR introduces new inconsistency (on the C++ side): now Transform2D's rotate()/scale() are no longer the mutating counterparts for rotated()/scaled(), now they perform different transformations. Also documentation is still vague.

[tagcup2]

The valid elements of Transform are not closed under multiplication, obviously, gosh what a nitpicker. As I repeatedly explained, yes, you can generate garbage Transform data that will break things.

Look at Basis docs, I documented it there that B = RS. Somebody else can do it for 2D.

In any case, game engines work with a specific subset of affine operations which don't form a group. It has worked fine for decades. You can complain all you want about it, but unless you submit an efficient replacement of core/math that does general affine transformations AND an efficient physics engine replacement that can handle those transformations, I doubt reduz or anyone will take it seriously. Talk is cheap, show me the code.

[bluenote10]

@madmiraal After thinking about it some more, only switching to right-multiplication semantics for scaled/rotated doesn't fully address godotengine/godot-proposals#1336. From a practical perspective there are use cases for all 6 variants whether we want to call them global vs local or left vs right multiplication. I also agree with @kleonc and @tagcup2 that it is better to keep the "unsuffixed" ones as global transforms (= left multiplications), because of the more intuitive order in terms of syntax and because it is less of a breaking change (only breaking translated instead of breaking scaled and rotated). Thanks anyway for investigating all options!

@tagcup2 After re-reading your proposal, I think it is exactly the same as my original proposal expect for the function naming convention. And after having a look at the existing code in Basis which already uses the _local convention it is definitely a nicer fit to go down that naming path. In fact, I could basically forward to identically named methods in Basis, which makes it quite nice. I've opened PR #55923 that implements your proposal, would be glad if you could have a look 😉 .

[reduz]

Even though I understand why we decided to go this way, I am feeling inclined to convalidate this PR just for the fact that the way proposed is more common in graphic APIs, from Logo back in the day to OpenGL functions like glRotate/glScale and glTranslate (and by the way OpenGL transrforms are also column based), all used local transforms by default, and rename the existing ones to pre_rotate, pre_scale and pre_translate.

In most cases I can think of, local transforms are preferred for ease of use.

[madmiraal]

Updated to include pre_translated(), pre_scaled and pre_rotated() which return transforms in the parent's reference frame: T * M, S * M and R * M, where M is the current transform matrix.
For reference, translated(), scaled() and rotated() are made in the local reference frame: M * T, M * S and M * R.

[kleonc]

and rename the existing ones to pre_rotate, pre_scale and pre_translate.

Updated to include pre_translated(), pre_scaled and pre_rotated() which return transforms in the parent's reference frame: T * M, S * M and R * M, where M is the current transform matrix.

With the current state of the changes I find the pre prefix confusing and counter-intuitive. It refers to the left-to-right order of the transform multiplication, not to the actual order of applying the transformations (which is reversed).

For example in some_transform.pre_translated(...).pre_scaled(...) I'd expect (because of the method naming) that the scaling would happen before the translation but with the current implementation it's actually the opposite. Meaning pre_* methods should rather be called something like post_*. Or maybe I'm misunderstanding what pre is supposed to mean (which would mean it's confusing anyway 🤔 ).

See also godotengine/godot-proposals#1336 (comment) comparing this PR and #55923.

[reduz]

@kleonc After discussing with many other contributors, opinions are pretty torn on this. I never expected it to be so divisive.

[bluenote10]

@reduz Everyone in the discussions seems to agree that we want to fix the inconsistency between translated and scaled/rotated for Godot 4.0. Looking at it pragmatically (#55923): We have the choice between breaking only 1 function (make translated behave like scaled/rotated) or breaking 2 (make both scaled/rotated behave like translated) plus further breaking changes, since Basis already uses the _local convention, and would need to change then as well for consistency.

In the long term, as long as we offer all 6 variants in a consistent manner and document their semantic difference well, it doesn't matter much which one gets the "short" name in my opinion, but we might as well minimizing the amount of breakage.

[madmiraal]

For example in some_transform.pre_translated(...).pre_scaled(...) I'd expect (because of the method naming) that the scaling would happen before the translation but with the current implementation it's actually the opposite. Meaning pre_* methods should rather be called something like post_*. Or maybe I'm misunderstanding what pre is supposed to mean (which would mean it's confusing anyway thinking ).

@kleonc, with this PR, some_transform.pre_translated(...).pre_scaled(...) the scaling does happen before doing the translation i.e. the translation will take place in the scaled reference frame. If, for example, we had pre_translated(Vector2(100, 100).pre_scaled(Vector2(2, 2)) then the translation would be scaled to Vector2(200, 200). Whereas if we used scaled(Vector2(2, 2)), the translation would be unaffected and the new position would be Vector(100, 100) away from some_transform's position.

Another way to think about it, and why I've explicitly stated it in the documentation, is pre_*() methods happen in the parent reference frame. If, as another example, some_transform was the identity matrix then pre_translated() and translated() would be the same; so it's just a translation. The difference then between scaled() and pre_scaled() would be whether the scaling happened in the parent reference frame or the local reference frame i.e. pre_scaled() is the same as scaling the parent (without actually scaling the parent).

[kleonc]

@madmiraal Thanks. I see how my confusion was a result of me mixing up the conventions. Even though this PR changes the convention to "think in the local reference frame", when I saw pre_* I was still thinking in the right-to-left manner. So things that are "post" in the right-to-left manner are indeed "pre" from the local reference frame perspective. Meaning these pre_* names in this PR indeed do make sense.

Of course, such naming will be confusing for people fixed up to thinking in the right-to-left manner (like me) but probably any naming would be confusing here as long as someone will be thinking in the convention opposite to the one being followed. Personally, I think I've already found "a switch in my head" so it's probably a matter of realizing what perspective to look from, and getting used to it.

I see how, comparing to #55923, this PR might be more average-user-friendly approach. Both of these PRs/approaches have upsides/downsides, both make sense and I don't have a strong opinion on which one I favor. At this moment #55923 seems way more intuitive for me but I see how this PR could get intuitive for me too. But there's also an average-user to consider...

To sum up: now I'm pretty torn between these two approaches myself. 😄

---

@madmiraal BTW seems like this PR still have this issue I've mentioned earlier:

Besides that this PR introduces new inconsistency (on the C++ side): now Transform2D's rotate()/scale() are no longer the mutating counterparts for rotated()/scaled(), now they perform different transformations.

Meaning scale() is now mutating counterpart for pre_scaled() etc. Such inconsistences would need to be fixed.

[andre-caldas]

This proposal is totally based on misconceptions. Not that it is right or wrong. What shall be discussed here is the meaning of "rotating" a Transform2D.

From the point of view of a Node2D, Transform2D is a coordinate system. If you are going to place a coordinate system L something inside another coordinate system G, Transform2D indicates, relative to G, where is the origin (let me call it LO), where is the x axis (LX) and the y axis (LY).

If someone that lives inside G wants to rotate an object o (with coordinate system L), keeping its position fixed (origin fixed), then it should call o.rotate(). The effect of this should be, IMO, keep the origin fixed and rotate the axis. So, calling o.rotate() should be applied to LX and LY. This has nothing to do to applying on the left or on the right. This is related to rotating about L's origin or rotating about G's origin.

Now, if you are treating, not the Node2D, but the Transform2D itself, what should be the meaning of rotated()? ANY CHOICE is very LEGITIMATE!!! I am not saying OP's proposal is good or bad.

So, when one calls Transform2D::rotated()... does this person wants to keep the origin fixed? Or does s/he wants to apply rotation to the local origin, about the global origin?

Important question 1

Suppose that Transform2D::rotated() allowed you to specify the point p where rotation should keep fixed. Is p given in local coordinates? Or in global coordinates? Why?

Again, we are taken to
godotengine/godot-proposals#1336

This has nothing to do with "doing on the left". It is a shame we keeping discussing on left and right.

Exercise (read this!!!)

Suppose you have a stretched axis with origin at (0,0)... the transform would have

x = (1,0)
y = (0,0)

(Edit: These two, above were wrong. They were x = (2,0) and y = (0,1). I have edited, because they should make the drawing "tall", not "wide".)

Inside this system of coordinates, you have a 1 x 1 house. Globally, this would be presented as a tall house. If you rotate this Transform2D, 90 degrees counterclockwise. What should you get?

1. A tall house turned 90 degrees?
2. A fat house turned 90 degrees?

Number 1 is what you get if you "multiply on the left". Number 2 is what you get if you "multiply on the right".

Shall you rotate the house and then apply the stretching (right multiplication), getting a fat house? Or shall you apply the stretching and then rotate the house (left multiplication), getting a tall house?

[madmiraal]

@andre-caldas I agree with you completely. With this PR, translated(), rotated() and scaled() are performed in the local frame of reference, which I believe is the expected behaviour, because translated().rotated().scaled() produces the same results seen in the Inspector.

To help visualise the combinations, I've created a test project: 2DTransforms.zip

Row 1 contains the rotated first combinations:

• rotated(deg2rad(90))
• rotated(deg2rad(90)).translated(Vector2(50,0))
• rotated(deg2rad(90)).scaled(Vector2(2,1))

Row 2 contains the scaled first combinations:

• scaled(Vector2(2,1))
• scaled(Vector2(2,1)).rotated(deg2rad(90))
• scaled(Vector2(2,1)).translated(Vector2(50,0))

Row 3 contains the translated first combinations:

• translated(Vector2(50,0))
• translated(Vector2(50,0)).rotated(deg2rad(90))
• translated(Vector2(50,0)).scaled(Vector2(2,1))

These are the results with this PR:

[andre-caldas]

@madmiraal,

I am very sorry, the post you are referring to had wrong x and y. I have edited it.

Second row, second column:

scaled(Vector2(2,1)).rotated(deg2rad(90))

This should, IMO, first scale. That is, the mascot should become wide (x coordinate multiplied by 2), not tall. And then, it should be rotated. But the central image shows a tall rotated mascot.

[madmiraal]

Second row, second column:

scaled(Vector2(2,1)).rotated(deg2rad(90))

This should, IMO, first scale. That is, the mascot should become wide (x coordinate multiplied by 2), not tall. And then, it should be rotated. But the central image shows a tall rotated mascot.

This was my first gut reaction too. However, it is correct as it is. It's easier if you first consider that it needs to be the opposite of rotated(deg2rad(90)).scaled(Vector2(2,1)) (first row, last column). In this case it is first rotated and then scaled in the new local, rotated coordinate system, where the x axis is now up and down. This correctly results in the fat mascot.

Therefore, scaled(Vector2(2,1)).rotated(deg2rad(90)). (second row, second column) must produce a tall mascot. The reason is that the scaling is along the unrotated, x-axis (left-right). The rotation needs to incorporate and keep this scaling along the x-axis.

Note: This is the same result seen when (in the Inspector) you scale the parent and rotate the child.

[andre-caldas]

I will list what I think methods should do...

Let me define things, beforehand.

The Transform2D structure is composed of:

• Vector2D x
• Vector2D y
• Vector2D origin

Another way to look at Transform2D, is as a "transform". It is composed of a translation (let me call it T and a linear transform M.

The linear transform takes (1,0) to x, and (0,1) to y. If you like matrices, the matrix corresponding to M is

[ x1  y1 ]
[        ]
[ x2  y2 ]

The x and y can be seen as a linear transform. If you like matrices, a 2x2 matrix. I will call this transform M.

Rotation

Rotate just x and y. Keep origin fixed. That is, denoting rotation by R:

x = R(x)
y = R(y)
origin = origin

There is no left, there is no right!!! You rotate x and y.

Although, I do not think talking about matrices very productive... in terms of matrix, this is done from the left:

[cos(a)  -sin(a)] [ x1  y1 ]
[               ] [        ]
[sin(a)   cos(a)] [ x2  y2 ]

Notice that those two operations above comute when scaling is uniform. That is, when x and y have the same size and are orthogonal. In this specific case, if you do it from the right, it also works.

The problem is when you want to represent it using "augumented matrices"!!! If you multiply augumented matrices on the left, then the origin also gets rotated. This is another reason why I find it so counterintuitive to talk about matrices in this case. Because you would have to rotate and then translate the origin back to its original position.

Scaling

I imagine that scaling means stretching the x and y axis. So, if you apply a scaled(a,b), this sould give:

x = a * x
y = a * y
origin = origin

Again, there is no left or right! But, of course, if you want to use matrix multiplication, then you would do it from the right:

[ x1  y1 ] [ a  0 ]
[        ] [      ]
[ x2  y2 ] [ 0  b ]

Translation

Translation shall not affect x and y. Just the origin should be shifted by a certain ammount v. There are two possible, very legitimate translations, IMO: v is in global coordinates or in local coordinates.

Gobal coordinates:

x = x
y = y
origin = origin + v

Local coordinates:

x = x
y = y
origin = origin + M(v).

Here, M is the linear transform mentioned on the begining of this post:

M(v) = v1 * x + v2 * y

Translation, rotation and scaling commute!!!

By the way, the way I describe, rotation and translatio commute! And this is the expected behaviour if you want rotation to keep positition fixed, and translation to keep rotation fixed.

Rotation and scaling also commute!!!

And so does translation and scalling!!! They all commute. I think this is the expected behaviour!!!

They all commute because translation only affects origin. Rotation affects x and y from the left. And scaling affects x and y from the right.

Scalling globally, but keeping origin fixed

One can also think of a global scalling:

[ a  0 ] [ x1  y1 ]
[      ] [        ]
[ 0  b ] [ x2  y2 ]

But this would generate tilted axis. It is also very legitimate! But Godot does not cope well with tilted axis. So I will not even get started with it.

Summary

Just to summarize, in terms of rotation, translation and scaling, I only see the need of:

1. Axis rotation.
2. Axis scaling.
3. Translation in local coordinates.
4. Translation in global coordinates.

[andre-caldas]

However, it is correct as it is.

What is supposed to happen with:

rotated(deg2rad(45)).scaled(Vector2(2,1))
scaled(Vector2(2,1)).rotated(deg2rad(45))

Should it become "tilted"? Godot does not cope well with "tilted"... Godot math library has problems when x and y are not orthogonal.

[madmiraal]

What is supposed to happen with:

rotated(deg2rad(45)).scaled(Vector2(2,1))
scaled(Vector2(2,1)).rotated(deg2rad(45))

Should it become "tilted"? Godot does not cope well with "tilted"... Godot math library has problems when x and y are not orthogonal.

It should do whatever a parent, child combination would do with the parent taking the first transformation and the child the second.

I've created a simple demo project for the two rotated() then scaled(), and scaled() then rotated() options:
RotateAndPreRotate.zip

I've animated the rotation in each case. This is the output with this PR:

[andre-caldas]

I barely see the uneven scaling on the second column.

It should do whatever a parent, child combination would do with the parent taking the first transformation and the child the second.

Even if it did, the order is swapped. If you do:

t = t.rotated()
t = t.scaled()

It is the same as:

t = t.rotated().scaled()

Rotation should be done first, and scaling, second. This is what happens if you, for example, rotate the child and scale the parent.

Anyway, I think it should simply be as Node2D works! As far as I can tell from the source code, it is as if there were three variables: scale, rotation and position. They are all independent and manipulating them should commute. This is simple, intuitive and very predictable.

At the end, what you have is an origin and two axis. Forgetting about matrices, natural operations to do with this is:

1. Move the origin.
2. Rotate the axis about the origin.
3. Scale each axis individually.

This is, if I am not missing anything, what Node2D does. Anyone that wants to do anything more complicated then that, should simply manipulate Transform2D::origin, Transform2D::x and Transform2D::y. Unfortunately, those variables do not exist... so, you have to use Transform2D::set_axis(). Now... this is a reason to complain! :-)

[andre-caldas]

I would like to suggest that we solve this (and others) issue once and for all. :-)

We could create one or two (or three) very precise proposals on exactly how Transform2D should look from the user's point of view. It is like if we start by writing the documentation. But taking care to be very very very precise.

1. An overview of what Transform2D represents and how it should behave or be used.
2. Exactly what public methods Transform2D should have.
3. Exactly what is the behaviour of each public method.
4. Examples for each public method showing how and when it should be used. And, optionally, how it should not be used.
5. List of compatibility issues.

Also, we could have a document listing "problems" and "use cases". Each proposal must address each listed problem and use case, one by one, in a separate section.

Then, some how, we could choose one. Or someone could chose one... I don't know how democratic we are. :-)

Right now, the specifications for how Transform2D works are not at all precise. I do believe this is the cause of all this confusion.

With this in hands, it would be very easy, IMO, to implement a very nice Transform2D I am tempted to say no one would complain about. But I know... we are humans. :-)

Can anyone suggest/setup/point out the necessary infrastructure? We need a place to discuss AND a place to produce a wiki-like document.

[bluenote10]

With this in hands, it would be very easy, IMO, to implement a very nice Transform2D

I've spend many many hours on this topic, and the result is exactly #55923. The public interface is complete and consistent and covers all use cases. It includes a thorough test suite covering the full API. It is also consistent with existing code of Basis and it is the least breaking change we can opt for.

I've also spend several days alone thinking about how to cover it in the documentation. I have explained it in the docs from different perspective so that people of various backgrounds have a chance of interpreting the semantics in their way, i.e., explaining it both in terms of (1) global vs local transformation, (2) in terms of mathematical equations, and (3) in terms of transformation order, because everyone has a preferred way of approaching the topic. The goal of all these hours was exactly to provide this "very very precise" specification / documentation...

[akien-mga]

Superseded by #55923 (for now at least).