Improve naming + documentation for transforms (row/column vectors)

Changes:
* Documentation + links to Godot API doc
* Transform
  * Remove from_axis_origin(), confusing semantics
  * Add from_basis_origin() instead
  * Add FLIP_X, FLIP_Y, FLIP_Z
* Transform2D
  * Rename from_axis_origin() -> from_basis_origin()
* Basis
  * Rename from_elements() -> from_rows()
  * Add from_basis_vectors()
  * Pass vectors by value
  * Make tdotx() etc. private for now (used internally by Transform)

Author: Bromeon

gdnative-core/src/core_types/geom/basis.rs

@@ -2,11 +2,21 @@ use crate::core_types::{IsEqualApprox, Quat, Vector3};
 use core::ops::Mul;
 use glam::Mat3;
 
-/// A 3x3 matrix, usually used as an orthogonal basis for [`Transform`].
+/// A 3x3 matrix, typically used as an orthogonal basis for [`Transform`][crate::core_types::Transform].
+///
+/// The basis vectors are the column vectors of the matrix, while the [`elements`][Self::elements]
+/// field represents the row vectors.
+///
+/// See also [Basis](https://docs.godotengine.org/en/stable/classes/class_basis.html) in the Godot API doc.
 #[repr(C)]
 #[derive(Copy, Clone, Debug, PartialEq)]
 #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
 pub struct Basis {
+    /// Matrix rows. These are **not** the basis vectors!
+    ///
+    /// This is a transposed view for performance. <br>
+    /// To read basis vectors, see [`x()`][Self::x], [`y()`][Self::y], [`z()`][Self::z]. <br>
+    /// To write them, see [`set_x()`][Self::set_x], [`set_y()`][Self::set_x], [`set_z()`][Self::set_x].
     pub elements: [Vector3; 3],
 }
 
@@ -18,10 +28,10 @@ impl Default for Basis {
 }
 
 impl Basis {
-    /// The identity basis.
+    /// The identity basis. Basis vectors are unit vectors along each axis X, Y and Z.
     ///
     /// Equivalent to calling [`Basis::default()`].
-    pub const IDENTITY: Basis = Self {
+    pub const IDENTITY: Self = Self {
         elements: [
             Vector3::new(1.0, 0.0, 0.0),
             Vector3::new(0.0, 1.0, 0.0),
@@ -30,30 +40,69 @@ impl Basis {
     };
 
     /// The basis that will flip something along the **X axis** when used in a transformation.
-    pub const FLIP_X: Self = Basis::from_diagonal(Vector3::new(-1.0, 1.0, 1.0));
+    pub const FLIP_X: Self = Self::from_diagonal(Vector3::new(-1.0, 1.0, 1.0));
 
     /// The basis that will flip something along the **Y axis** when used in a transformation.
-    pub const FLIP_Y: Self = Basis::from_diagonal(Vector3::new(1.0, -1.0, 1.0));
+    pub const FLIP_Y: Self = Self::from_diagonal(Vector3::new(1.0, -1.0, 1.0));
 
     /// The basis that will flip something along the **Z axis** when used in a transformation.
-    pub const FLIP_Z: Self = Basis::from_diagonal(Vector3::new(1.0, 1.0, -1.0));
+    pub const FLIP_Z: Self = Self::from_diagonal(Vector3::new(1.0, 1.0, -1.0));
 
-    /// Creates a Basis from the given [`Vector3`](./type.Vector3.html)
+    /// Constructs a basis matrix from 3 linearly independent basis vectors (matrix columns).
+    ///
+    /// This is the typical way to construct a basis. If you want to fill in the elements one-by-one,
+    /// consider using [`Self::from_rows()`] instead.
     #[inline]
-    pub const fn from_diagonal(p_diag: Vector3) -> Self {
+    pub const fn from_basis_vectors(a: Vector3, b: Vector3, c: Vector3) -> Self {
         Self {
             elements: [
-                Vector3::new(p_diag.x, 0.0, 0.0),
-                Vector3::new(0.0, p_diag.y, 0.0),
-                Vector3::new(0.0, 0.0, p_diag.z),
+                Vector3::new(a.x, b.x, c.x),
+                Vector3::new(a.y, b.y, c.y),
+                Vector3::new(a.z, b.z, c.z),
             ],
         }
     }
 
-    /// Creates a `Basis` from 3 vectors
+    /// Creates a basis from 3 row vectors. These are **not** basis vectors.
+    ///
+    /// This constructor is mostly useful if you want to write matrix elements
+    /// in matrix syntax:
+    /// ```
+    /// # use gdnative::core_types::{Vector3, Basis};
+    /// # let a = Vector3::RIGHT;
+    /// # let b = Vector3::UP;
+    /// # let c = Vector3::BACK;
+    /// let basis = Basis::from_rows(
+    ///     Vector3::new(a.x, b.x, c.x),
+    ///     Vector3::new(a.y, b.y, c.y),
+    ///     Vector3::new(a.z, b.z, c.z),
+    /// );
+    /// ```
+    /// In that case, the vectors `a`, `b` and `c` are the basis vectors.
+    #[inline]
+    pub const fn from_rows(
+        x_components: Vector3,
+        y_components: Vector3,
+        z_components: Vector3,
+    ) -> Self {
+        Self {
+            elements: [x_components, y_components, z_components],
+        }
+    }
+
+    /// Creates a diagonal matrix from the given vector.
+    ///
+    /// Can be used as a basis for a scaling transform.
+    /// Each component of `scale` represents the scale factor in the corresponding direction.
     #[inline]
-    pub const fn from_elements(elements: [Vector3; 3]) -> Self {
-        Self { elements }
+    pub const fn from_diagonal(scale: Vector3) -> Self {
+        Self {
+            elements: [
+                Vector3::new(scale.x, 0.0, 0.0),
+                Vector3::new(0.0, scale.y, 0.0),
+                Vector3::new(0.0, 0.0, scale.z),
+            ],
+        }
     }
 
     /// Creates a rotation matrix from Euler angles.
@@ -62,35 +111,35 @@ impl Basis {
     /// first **Z**, then **X**, and **Y** last.
     #[inline]
     pub fn from_euler(euler_angles: Vector3) -> Self {
-        let mut b = Basis::default();
-        b.set_euler_yxz(&euler_angles);
+        let mut b = Self::default();
+        b.set_euler_yxz(euler_angles);
         b
     }
 
     #[inline]
-    fn set_euler_yxz(&mut self, euler: &Vector3) {
+    fn set_euler_yxz(&mut self, euler: Vector3) {
         let c = euler.x.cos();
         let s = euler.x.sin();
-        let xmat = Basis::from_elements([
+        let xmat = Self::from_rows(
             Vector3::new(1.0, 0.0, 0.0),
             Vector3::new(0.0, c, -s),
             Vector3::new(0.0, s, c),
-        ]);
+        );
         let c = euler.y.cos();
         let s = euler.y.sin();
-        let ymat = Basis::from_elements([
+        let ymat = Self::from_rows(
             Vector3::new(c, 0.0, s),
             Vector3::new(0.0, 1.0, 0.0),
             Vector3::new(-s, 0.0, c),
-        ]);
+        );
 
         let c = euler.z.cos();
         let s = euler.z.sin();
-        let zmat = Basis::from_elements([
+        let zmat = Self::from_rows(
             Vector3::new(c, -s, 0.0),
             Vector3::new(s, c, 0.0),
             Vector3::new(0.0, 0.0, 1.0),
-        ]);
+        );
 
         *self = ymat * xmat * zmat;
     }
@@ -99,29 +148,29 @@ impl Basis {
     #[inline]
     pub fn from_quat(quat: Quat) -> Self {
         let basis = Mat3::from_quat(quat.glam()).to_cols_array_2d();
-        let basis = [
+
+        Self::from_rows(
             Vector3::new(basis[0][0], basis[1][0], basis[2][0]),
             Vector3::new(basis[0][1], basis[1][1], basis[2][1]),
             Vector3::new(basis[0][2], basis[1][2], basis[2][2]),
-        ];
-        Basis::from_elements(basis)
+        )
     }
 
     /// Rotation matrix from axis and angle.
     ///
-    /// See <https://en.wikipedia.org/wiki/Rotation_matrix#Rotation_matrix_from_axis_angle>
+    /// See <https://en.wikipedia.org/wiki/Rotation_matrix#Rotation_matrix_from_axis_and_angle>
     ///
     /// # Panics
     ///
     /// If `axis` is not normalized.
     #[inline]
-    pub fn from_axis_angle(axis: &Vector3, phi: f32) -> Self {
+    pub fn from_axis_angle(axis: Vector3, phi: f32) -> Self {
         assert!(
             axis.length().is_equal_approx(1.0),
             "The axis Vector3 must be normalized."
         );
 
-        let mut basis = Basis::default();
+        let mut basis = Self::default();
         let [x, y, z] = &mut basis.elements;
 
         let axis_sq = Vector3::new(axis.x * axis.x, axis.y * axis.y, axis.z * axis.z);
@@ -283,7 +332,7 @@ impl Basis {
     /// of the 3D object. `rotated()` here refers to rotation of the object (which is `R * self`), not the matrix itself.
     #[inline]
     pub fn rotated(&self, axis: Vector3, phi: f32) -> Self {
-        let rot = Basis::from_axis_angle(&axis, phi);
+        let rot = Basis::from_axis_angle(axis, phi);
         rot * (*self)
     }
 
@@ -316,7 +365,7 @@ impl Basis {
 
     /// Introduce an additional scaling specified by the given 3D scaling factor.
     #[inline]
-    pub fn scaled(&self, scale: &Vector3) -> Self {
+    pub fn scaled(&self, scale: Vector3) -> Self {
         let mut copy = *self;
         copy.scale_self(scale);
         copy
@@ -326,7 +375,7 @@ impl Basis {
     ///
     /// See the comment for [Basis::rotated](#method.rotated) for further explanation.
     #[inline]
-    fn scale_self(&mut self, s: &Vector3) {
+    fn scale_self(&mut self, s: Vector3) {
         self.elements[0] *= s.x;
         self.elements[1] *= s.y;
         self.elements[2] *= s.z;
@@ -349,7 +398,7 @@ impl Basis {
         let det = matrix.determinant();
         if det < 0.0 {
             // Ensure that the determinant is 1, such that result is a proper rotation matrix which can be represented by Euler angles.
-            matrix.scale_self(&Vector3::new(-1.0, -1.0, -1.0));
+            matrix.scale_self(Vector3::new(-1.0, -1.0, -1.0));
         }
 
         assert!(matrix.is_rotation(), "Basis must be normalized in order to be casted to a Quaternion. Use to_quat() or call orthonormalized() instead.");
@@ -469,59 +518,59 @@ impl Basis {
         )
     }
 
-    /// Transposed dot product with the **X axis** of the matrix.
+    /// Transposed dot product with the **X basis vector** of the matrix.
     #[inline]
-    pub fn tdotx(&self, v: Vector3) -> f32 {
-        self.elements[0].x * v.x + self.elements[1].x * v.y + self.elements[2].x * v.z
+    pub(crate) fn tdotx(&self, v: Vector3) -> f32 {
+        self.x().dot(v)
     }
 
-    /// Transposed dot product with the **Y axis** of the matrix.
+    /// Transposed dot product with the **Y basis vector** of the matrix.
     #[inline]
-    pub fn tdoty(&self, v: Vector3) -> f32 {
-        self.elements[0].y * v.x + self.elements[1].y * v.y + self.elements[2].y * v.z
+    pub(crate) fn tdoty(&self, v: Vector3) -> f32 {
+        self.y().dot(v)
     }
 
-    /// Transposed dot product with the **Z axis** of the matrix.
+    /// Transposed dot product with the **Z basis vector** of the matrix.
     #[inline]
-    pub fn tdotz(&self, v: Vector3) -> f32 {
-        self.elements[0].z * v.x + self.elements[1].z * v.y + self.elements[2].z * v.z
+    pub(crate) fn tdotz(&self, v: Vector3) -> f32 {
+        self.z().dot(v)
     }
 
-    /// Get the **X axis** of the matrix
+    /// Get the **X basis vector** (first column vector of the matrix).
     #[inline]
     pub fn x(&self) -> Vector3 {
         Vector3::new(self.elements[0].x, self.elements[1].x, self.elements[2].x)
     }
 
-    /// Set the **X axis** of the matrix
+    /// Set the **X basis vector** (first column vector of the matrix).
     #[inline]
     pub fn set_x(&mut self, v: Vector3) {
         self.elements[0].x = v.x;
         self.elements[1].x = v.y;
         self.elements[2].x = v.z;
     }
 
-    /// Get the **Y axis** of the matrix
+    /// Get the **Y basis vector** (second column vector of the matrix).
     #[inline]
     pub fn y(&self) -> Vector3 {
         Vector3::new(self.elements[0].y, self.elements[1].y, self.elements[2].y)
     }
 
-    /// Set the **Y axis** of the matrix
+    /// Set the **Y basis vector** (second column vector of the matrix).
     #[inline]
     pub fn set_y(&mut self, v: Vector3) {
         self.elements[0].y = v.x;
         self.elements[1].y = v.y;
         self.elements[2].y = v.z;
     }
 
-    /// Get the **Z axis** of the matrix
+    /// Get the **Z basis vector** (third column vector of the matrix).
     #[inline]
     pub fn z(&self) -> Vector3 {
         Vector3::new(self.elements[0].z, self.elements[1].z, self.elements[2].z)
     }
 
-    /// Set the **Z axis** of the matrix
+    /// Set the **Z basis vector** (third column vector of the matrix).
     #[inline]
     pub fn set_z(&mut self, v: Vector3) {
         self.elements[0].z = v.x;
@@ -547,7 +596,7 @@ impl Mul<Basis> for Basis {
 
     #[inline]
     fn mul(self, rhs: Self) -> Self {
-        Basis::from_elements([
+        Basis::from_rows(
             Vector3::new(
                 rhs.tdotx(self.elements[0]),
                 rhs.tdoty(self.elements[0]),
@@ -563,7 +612,7 @@ impl Mul<Basis> for Basis {
                 rhs.tdoty(self.elements[2]),
                 rhs.tdotz(self.elements[2]),
             ),
-        ])
+        )
     }
 }
 
@@ -657,36 +706,36 @@ mod tests {
     fn orthonormalized() {
         let (b, _bn) = test_inputs();
 
-        let expected = Basis::from_elements([
+        let expected = Basis::from_rows(
             Vector3::new(0.077431, -0.165055, 0.98324),
             Vector3::new(-0.288147, 0.94041, 0.180557),
             Vector3::new(-0.95445, -0.297299, 0.025257),
-        ]);
+        );
         assert!(expected.is_equal_approx(&b.orthonormalized()));
     }
 
     #[test]
     fn scaled() {
         let (b, _bn) = test_inputs();
 
-        let expected = Basis::from_elements([
+        let expected = Basis::from_rows(
             Vector3::new(0.052484, -0.111876, 0.666453),
             Vector3::new(0.012407, -0.040492, -0.007774),
             Vector3::new(-0.682131, -0.212475, 0.018051),
-        ]);
-        assert!(expected.is_equal_approx(&b.scaled(&Vector3::new(0.677813, -0.043058, 0.714685))));
+        );
+        assert!(expected.is_equal_approx(&b.scaled(Vector3::new(0.677813, -0.043058, 0.714685))));
     }
 
     #[test]
     fn rotated() {
         let (b, _bn) = test_inputs();
 
         let r = Vector3::new(-50.167156, 60.67781, -70.04305).normalized();
-        let expected = Basis::from_elements([
+        let expected = Basis::from_rows(
             Vector3::new(-0.676245, 0.113805, 0.727833),
             Vector3::new(-0.467094, 0.697765, -0.54309),
             Vector3::new(-0.569663, -0.707229, -0.418703),
-        ]);
+        );
         assert!(expected.is_equal_approx(&b.rotated(r, 1.0)));
     }
 
@@ -713,11 +762,11 @@ mod tests {
     #[test]
     fn transposed() {
         let (b, _bn) = test_inputs();
-        let expected = Basis::from_elements([
+        let expected = Basis::from_rows(
             Vector3::new(0.077431, -0.288147, -0.95445),
             Vector3::new(-0.165055, 0.94041, -0.297299),
             Vector3::new(0.98324, 0.180557, 0.025257),
-        ]);
+        );
         assert!(expected.is_equal_approx(&b.transposed()));
     }
 
@@ -741,11 +790,11 @@ mod tests {
     fn inverse() {
         let (b, _bn) = test_inputs();
 
-        let expected = Basis::from_elements([
+        let expected = Basis::from_rows(
             Vector3::new(0.077431, -0.288147, -0.95445),
             Vector3::new(-0.165055, 0.94041, -0.297299),
             Vector3::new(0.98324, 0.180557, 0.025257),
-        ]);
+        );
         assert!(expected.is_equal_approx(&b.inverted()));
     }
 }