Scale in 3D and Quat()

[sezit]

Its bug or not, I dont know. When i use Quat() to set rotation and if obj have scale, he reset scale back to oiginal.

[reduz]

This is intended, as Godot uses transforms for 3D internally, the scale/loc/rot are mostly helpers. All this will probably change a little in 3.0 to make more sense though

[ghost]

There's a related issue. I have a testcube, and I scaled it along the x axis by 2. After rotating it along the Y-axis by 90 degrees with cube.set_transform(cube.get_transform().rotated(Vector3(0,1,0), PI/2)) instead of getting a rotated cube which is elongated along the Z-axis, I'm getting a rotated cube which is still scaled along the X-axis.

Right now, Transform::rotated essentially does basis = basis*R which pre-adds another rotation, and Transform::scaled scales the basis vectors. The transformation matrix M is typically split to translation, rotation and scaling as: M = T.R.S, but rotate actually breaks this since rotating M by R' gives (T.R.S).R' rather than T.(R.R').S. I'll check how this is handled in other engines, but I believe correct behavior requires storing the scaling vector independent from the basis vectors. (Edit: While this makes sense for composition/decomposition, it doesn't make any sense for transforming objects. So this part is fine.)

[ghost]

Alternatively, Transform::rotate can first orthonormalize the basis vectors and obtain the scaling vector in the process, apply rotation to the ortonormalized matrix, and then do the scaling. This doesn't require any changes to the data structure. Transform::scale is fine, since M = (T.R.S).S' = T.R.(S.S') is already the correct behavior.

We should do the same thing with quaternions, which will fix this issue completely.

[ghost]

As I mentioned in #6865, all these work for transformations, that is, matrices which are made of translation, rotation and scaling in the particular ordering M = T.R.S, and does not apply to 3x3 matrices (or 4x4 matrices, if you include the translation part) in general. So in a different PR, I will propose to remove rotate and scale functions from Matrix3 class (both C++ and GDScript, so this is breaking change which should be done before 3.0), and make use of Transform::rotate_basis and Transform::scale_basis (I believe I will need to add Transform::scale_basis as it doesn't exist at this moment). There should also be GDScript script bindings too.

[ghost]

I have a related question: why does rotate multiply the existing matrix from the right? I think the expected behavior is it should be to multiply from left, such that we add another rotation on top of the current one.

[ghost]

@sezit Can you post the snippet where you "use Quat() to set rotation" that you think is problematic? In particular, what do you mean by setting the rotation?

The only relevant functions which take Quat I see so far are constructors of Matrix3 or Transform, and since they create a matrix from scratch, that's the expected behavior.

[sezit]

@tagcup Ooo....its been a long time ago, i dont even remember how code looks :З

[ghost]

I've gone through the code and I'm not seeing any issues about it. If you're OK with this, I'd then recommend closing this issue.

Also I'm sorry for hijacking this issue with my later comments, but they will be addressed in an upcoming PR.

[ghost]

We can add set_rotation (changes the rotation part, while keeping the scale and translation parts fixed) and set_scale (similar), which would complement get_rotation and get_scale.