[C#] Fix Transform3D.InterpolateWith applying rotation before scale

[kleonc]

Seen a user confused by Transform3D transformC = transformA.InterpolateWith(transformB, 0.0f); resulting in a transformC being way different than transformA (specifically transformC.Scale not matching transformA.Scale). Turns out there's a bug in the C# implementation of the method.

Transform3D.InterpolateWith(Transform3D, real_t) has the same implementation as in the core:

godot/modules/mono/glue/GodotSharp/GodotSharp/Core/Transform3D.cs

Lines 129 to 146 in 99ff024

	public readonly Transform3D InterpolateWith(Transform3D transform, real_t weight)
	{
	Vector3 sourceScale = Basis.Scale;
	Quaternion sourceRotation = Basis.GetRotationQuaternion();
	Vector3 sourceLocation = Origin;
	Vector3 destinationScale = transform.Basis.Scale;
	Quaternion destinationRotation = transform.Basis.GetRotationQuaternion();
	Vector3 destinationLocation = transform.Origin;
	var interpolated = new Transform3D();
	Quaternion quaternion = sourceRotation.Slerp(destinationRotation, weight).Normalized();
	Vector3 scale = sourceScale.Lerp(destinationScale, weight);
	interpolated.Basis.SetQuaternionScale(quaternion, scale);
	interpolated.Origin = sourceLocation.Lerp(destinationLocation, weight);
	return interpolated;
	}

godot/core/math/transform_3d.cpp

Lines 97 to 112 in 99ff024

	Transform3D Transform3D::interpolate_with(const Transform3D &p_transform, real_t p_c) const {
	Transform3D interp;
	Vector3 src_scale = basis.get_scale();
	Quaternion src_rot = basis.get_rotation_quaternion();
	Vector3 src_loc = origin;
	Vector3 dst_scale = p_transform.basis.get_scale();
	Quaternion dst_rot = p_transform.basis.get_rotation_quaternion();
	Vector3 dst_loc = p_transform.origin;
	interp.basis.set_quaternion_scale(src_rot.slerp(dst_rot, p_c).normalized(), src_scale.lerp(dst_scale, p_c));
	interp.origin = src_loc.lerp(dst_loc, p_c);
	return interp;
	}

The issue is hidden in Basis.SetQuaternionScale(Quaternion, Vector3) which itself is the same as in the core, but the private Basis.Rotate(Quaternion) method used within is rotating locally instead of parentwise, as in the core:

godot/modules/mono/glue/GodotSharp/GodotSharp/Core/Basis.cs

Lines 207 to 216 in 99ff024

	internal void SetQuaternionScale(Quaternion quaternion, Vector3 scale)
	{
	SetDiagonal(scale);
	Rotate(quaternion);
	}
	private void Rotate(Quaternion quaternion)
	{
	this *= new Basis(quaternion);
	}

godot/core/math/basis.cpp

Lines 889 to 892 in 99ff024

	void Basis::set_quaternion_scale(const Quaternion &p_quaternion, const Vector3 &p_scale) {
	_set_diagonal(p_scale);
	rotate(p_quaternion);
	}

godot/core/math/basis.cpp

Lines 379 to 385 in 99ff024

	Basis Basis::rotated(const Quaternion &p_quaternion) const {
	return Basis(p_quaternion) * (*this);
	}
	void Basis::rotate(const Quaternion &p_quaternion) {
	*this = rotated(p_quaternion);
	}

This PR fixes this. Both private Basis.Rotate(Quaternion) and internal Basis.SetQuaternionScale(Quaternion, Vector3) affected by this change are not used anywhere else so shouldn't break anything else.

---

Example:

 transformA = [X: (-0.13499019, 0.5286249, 0.13893306), Y: (1.3693482, 0.5052223, -0.59182847), Z: (-0.19655085, 0.05662668, -0.40643105), O: (0, 0, 0)]
 transformA.basis.Scale = (0.563, 1.5749999, 0.455)

 transformB = [X: (-0.24048872, 0.94175977, 0.24751306), Y: (1.1998098, 0.44267097, -0.51855445), Z: (-0.12959397, 0.03733627, -0.26797652), O: (0, 0, 0)]
 transformB.basis.Scale = (1.003, 1.3799999, 0.3)

Before (v4.2.1.stable.mono.official [b09f793]):
(Transform3D transformC = transformA.InterpolateWith(transformB, 0.0f);)

 transformC = [X: (-0.13499022, 1.4788351, 0.1122816), Y: (0.4894877, 0.5052223, -0.17097268), Z: (-0.24320467, 0.1960154, -0.40643108), O: (0, 0, 0)]
 transformC.basis.Scale = (1.4892222, 0.72393334, 0.51259804)

After (this PR):

 transformC = [X: (-0.13499022, 0.5286249, 0.13893306), Y: (1.3693483, 0.5052223, -0.5918284), Z: (-0.19655085, 0.056626678, -0.40643108), O: (0, 0, 0)]
 transformC.basis.Scale = (0.563, 1.575, 0.45500004)

[TokageItLab]

I remember I saw a PR similar to this for 2D (for GDScript, not C#) somewhere, but I forget. It might be better to have @aaronfranke check it as well since we discussed there.

[aaronfranke]

This PR is correct, and it matches C++ as noted.

Aside from everything already mentioned, also note that in Godot it's the convention that "thing" is parent-relative (rotation * self) and "local thing" is relative to the object's own transform (self * rotation), which this fix is consistent with.

[kleonc]

AFAICT this seems to be bugged the same way in 3.5/3.x/4.1/4.2 and seems to be safe change, so I guess this PR should be cherry-picked to all of them? 🤔
(@akien-mga Please unlabel if not, I'm not sure e.g. if we still cherry-pick to 3.5)

[akien-mga]

Thanks!

[akien-mga]

Cherry-picked for 4.2.2.

[akien-mga]

Cherry-picked for 4.1.4.

[akien-mga]

Cherry-picked for 3.6.
Cherry-picked for 3.5.4.