Adding QoL extensions to Transforms

[MarekLg]

I'd like Transforms to have some QoL methods (examples will follow), and would like to hear what the sentiment towards that would be (I know we want to be careful with implementing too many new features).

Here is my wishlist (much of these are influenced by Unity's Transforms:

• Transform::forward() -> Vec3 Get the normalized direction pointing towards in model-space
• Transform::look_at(&mut self, &Vec3) Rotate the Transform, so that it looks at a certain position
• General methods to rotate the Transform e.g. around a certain axis, or orbit around a certain point. Those could be better kept as constructors for Quat, though.
• Transform::face_towards(&mut self, ..) -> Self same as Transform::new(Mat4::face_towards(..)) but minimizes the amount of Mat4s the average user has to deal with
• Transform::apply(&mut self, other: &Transform) transform your transform by another transform ;)
• I'm sure more will come to mind later

The mutable methods would also only be implemented on Transform but not on GlobalTransform (although refining the difference between Global- and Transform could be another Issue).

If everyone's on board I'd like to use this issue to collect wanted features for Transforms and open a pull request to implement them.

[GrantMoyer]

There should definitely be both apply_before and apply_after. Also, I think rotation interpolation functions would be helpful such as rotate_angle_toward(angle: f32, target: &Quat) and rotate_fraction_toward(frac: f32, target: &Quat).

[nickbryan]

Hey, I was looking to add some QoL methods to the Transform component too but then I saw your PR.

I think it would be nice to have the following (taken from Unity):

I know this one is on a Vec3 in unity but I figure it could just as easily be on the Transform?

// Vector3.MoveTowards
 public static Vector3 MoveTowards(Vector3 current, Vector3 target, float maxDistanceDelta)
 {
     Vector3 a = target - current;
     float magnitude = a.magnitude;
     if (magnitude <= maxDistanceDelta || magnitude == 0f)
     {
         return target;
     }
     return current + a / magnitude * maxDistanceDelta;
 }

Transform.TransformDirection(Vec3);

transform_direction(&mut self, directon: Vec3) {
   self.transform.rotation().mul_vec3(direction);
}

[MarekLg]

There should definitely be both apply_before and apply_after.

@GrantMoyer Could you elaborate what would be the difference?

[GrantMoyer]

@MarekLg Tranform composition is not commutative, so in generally A*B != B*A. For example, translating by 10, then scaling by 2 is not the same as scaling by 2, then translating by 10, since the scaling is always relative to the global origin.

[MarekLg]

You're right, but that's not why I'm asking. Transform::apply(..), in the original sense, was meant to come down to this:

pub fn apply(&mut self, other: &Transform) {
  self.value = *other.value() * self.value;
}

So I'm assuming Transform::apply_before(..) would be like this?

pub fn apply(&mut self, other: &Transform) {
  self.value = self.value * *other.value();
}

If I got that right: what would be the use of apply_before()?

[GrantMoyer]

Apply before could be used to, for instance, scale a model to half size before applying the current transform. Since these ops are in place, doing that with apply (apply_after) would require an extra copy.

[MarekLg]

I understand, thanks for the clarification! I'll put in both of the apply methods.

[MarekLg]

Another question: should transform.translate(translation) translate with or without the rotation of transform applied to translation?

[0xFlugel]

I'm gonna add my two cents because I just ran into the ordering issue with Transform::rotate() for a Camera.

The naming of these applying methods seem to maximize the chance for misunderstandings. I would either remove the method when the multiply-left version is not allowed to exist or add an in-place multiply-left method.
In my opinion, though, the Transform::rotate method should be turned into multiply-left. That would be correct for the meaning of "rotate this current state".

Another question: should transform.translate(translation) translate with or without the rotation of transform applied to translation?

The "correct" way of doing this really depends on the context of application. For most things, I would expect a sequence of Translate -> Rotate and Rotate -> Translate to have differing effects. But in the context of a transform component that e.g. controls the camera, I want to iteratively change rotation and translation independent of each other. The latter use case may be the typical one for this framework. It should just be clearly communicated.

[mockersf]

most of those methods now exist on Transform