Skip to content

Commit 16aa18e

Browse files
committed
Rename basis vectors x, y, z -> a, b, c
There is some confusion between matrix row and column vectors. Columns are the basis vectors of the transform, while rows represent the X/Y/Z coordinates of each vector. Although basis vectors are the _transformed_ unit vectors of X/Y/Z axes, they have no direct relation to those axes in the _transformed_ coordinate system. Thus, an independent notion a, b, c does not suggest such a relation. Furthermore, there are sometimes expressions such as x.x, x.y, y.x etc. They are typically hard to read and error-prone to write. Having a.x, a.y, b.x makes things more understandable.
1 parent 38719eb commit 16aa18e

File tree

3 files changed

+86
-93
lines changed

3 files changed

+86
-93
lines changed

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

Lines changed: 48 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,9 @@ use glam::Mat3;
1414
pub struct Basis {
1515
/// Matrix rows. These are **not** the basis vectors!
1616
///
17-
/// This is a transposed view for performance. <br>
18-
/// To read basis vectors, see [`x()`][Self::x], [`y()`][Self::y], [`z()`][Self::z]. <br>
19-
/// To write them, see [`set_x()`][Self::set_x], [`set_y()`][Self::set_x], [`set_z()`][Self::set_x].
17+
/// This is a transposed view for performance.<br>
18+
/// To read basis vectors, see [`a()`][Self::a], [`b()`][Self::b], [`c()`][Self::c].<br>
19+
/// To write them, see [`set_a()`][Self::set_a], [`set_b()`][Self::set_b], [`set_c()`][Self::set_c].
2020
pub elements: [Vector3; 3],
2121
}
2222

@@ -51,7 +51,7 @@ impl Basis {
5151
/// Constructs a basis matrix from 3 linearly independent basis vectors (matrix columns).
5252
///
5353
/// This is the typical way to construct a basis. If you want to fill in the elements one-by-one,
54-
/// consider using [`Self::from_rows()`] instead.
54+
/// consider using [`Self::from_rows()`].
5555
#[inline]
5656
pub const fn from_basis_vectors(a: Vector3, b: Vector3, c: Vector3) -> Self {
5757
Self {
@@ -78,7 +78,8 @@ impl Basis {
7878
/// Vector3::new(a.z, b.z, c.z),
7979
/// );
8080
/// ```
81-
/// In that case, the vectors `a`, `b` and `c` are the basis vectors.
81+
/// The vectors `a`, `b` and `c` are the basis vectors.<br>
82+
/// In this particular case, you could also use [`Self::from_basis_vectors(a, b, c)`][Self::from_basis_vectors()] instead.
8283
#[inline]
8384
pub const fn from_rows(
8485
x_components: Vector3,
@@ -221,13 +222,13 @@ impl Basis {
221222

222223
let s: f32 = 1.0 / det;
223224

224-
self.set_x(Vector3::new(co[0] * s, co[1] * s, co[2] * s));
225-
self.set_y(Vector3::new(
225+
self.set_a(Vector3::new(co[0] * s, co[1] * s, co[2] * s));
226+
self.set_b(Vector3::new(
226227
(x.z * z.y - x.y * z.z) * s,
227228
(x.x * z.z - x.z * z.x) * s,
228229
(x.y * z.x - x.x * z.y) * s,
229230
));
230-
self.set_z(Vector3::new(
231+
self.set_c(Vector3::new(
231232
(x.y * y.z - x.z * y.y) * s,
232233
(x.z * y.x - x.x * y.z) * s,
233234
(x.x * y.y - x.y * y.x) * s,
@@ -287,19 +288,19 @@ impl Basis {
287288
);
288289

289290
// Gram-Schmidt Process
290-
let mut x = self.x();
291-
let mut y = self.y();
292-
let mut z = self.z();
291+
let mut x = self.a();
292+
let mut y = self.b();
293+
let mut z = self.c();
293294

294295
x = x.normalized();
295296
y = y - x * (x.dot(y));
296297
y = y.normalized();
297298
z = z - x * (x.dot(z)) - y * (y.dot(z));
298299
z = z.normalized();
299300

300-
self.set_x(x);
301-
self.set_y(y);
302-
self.set_z(z);
301+
self.set_a(x);
302+
self.set_b(y);
303+
self.set_c(z);
303304
}
304305

305306
#[inline]
@@ -308,9 +309,7 @@ impl Basis {
308309
m.is_equal_approx(&Self::IDENTITY)
309310
}
310311

311-
/// Returns an orthonormalized version of the matrix.
312-
///
313-
/// See [`Basis::orthonormalize()`](#method.orthonormalize)
312+
/// Returns an orthonormalized version of the matrix: 3 orthogonal basis vectors of unit length.
314313
#[inline]
315314
pub fn orthonormalized(&self) -> Self {
316315
let mut copy = *self;
@@ -326,25 +325,27 @@ impl Basis {
326325
&& self.elements[2].is_equal_approx(other.elements[2])
327326
}
328327

329-
/// Multiplies the matrix from left by the rotation matrix: M -> R.M
328+
/// Multiplies the matrix from left with the rotation matrix: M -> R·M
330329
///
331330
/// The main use of `Basis` is as a `Transform.basis`, which is used as the transformation matrix
332331
/// of the 3D object. `rotated()` here refers to rotation of the object (which is `R * self`), not the matrix itself.
333332
#[inline]
334333
pub fn rotated(&self, axis: Vector3, phi: f32) -> Self {
335-
let rot = Basis::from_axis_angle(axis, phi);
336-
rot * (*self)
334+
let mut copy = *self;
335+
copy.rotate(axis, phi);
336+
copy
337337
}
338338

339339
/// Rotates the matrix.
340340
///
341341
/// If object rotation is needed, see [`Basis::rotated()`]
342342
#[inline]
343-
#[allow(unused)] // useful to have around, if more methods are added
344343
fn rotate(&mut self, axis: Vector3, phi: f32) {
345-
*self = self.rotated(axis, phi);
344+
let rot = Self::from_axis_angle(axis, phi);
345+
*self = rot * *self;
346346
}
347347

348+
/// Returns true if this basis represents a rotation matrix (orthogonal and no scaling).
348349
#[inline]
349350
fn is_rotation(&self) -> bool {
350351
let det = self.determinant();
@@ -356,11 +357,8 @@ impl Basis {
356357
pub fn scale(&self) -> Vector3 {
357358
let det = self.determinant();
358359
let det_sign = if det < 0.0 { -1.0 } else { 1.0 };
359-
Vector3::new(
360-
Vector3::new(self.elements[0].x, self.elements[1].x, self.elements[2].x).length(),
361-
Vector3::new(self.elements[0].y, self.elements[1].y, self.elements[2].y).length(),
362-
Vector3::new(self.elements[0].z, self.elements[1].z, self.elements[2].z).length(),
363-
) * det_sign
360+
361+
Vector3::new(self.a().length(), self.b().length(), self.c().length()) * det_sign
364362
}
365363

366364
/// Introduce an additional scaling specified by the given 3D scaling factor.
@@ -371,7 +369,7 @@ impl Basis {
371369
copy
372370
}
373371

374-
/// Multiplies the matrix from left by the scaIling matrix: M -> S.M
372+
/// Multiplies the matrix from left with the scaling matrix: M -> S·M
375373
///
376374
/// See the comment for [Basis::rotated](#method.rotated) for further explanation.
377375
#[inline]
@@ -511,68 +509,64 @@ impl Basis {
511509
/// Note: This results in a multiplication by the inverse of the matrix only if it represents a rotation-reflection.
512510
#[inline]
513511
pub fn xform_inv(&self, v: Vector3) -> Vector3 {
514-
Vector3::new(
515-
(self.elements[0].x * v.x) + (self.elements[1].x * v.y) + (self.elements[2].x * v.z),
516-
(self.elements[0].y * v.x) + (self.elements[1].y * v.y) + (self.elements[2].y * v.z),
517-
(self.elements[0].z * v.x) + (self.elements[1].z * v.y) + (self.elements[2].z * v.z),
518-
)
512+
Vector3::new(self.a().dot(v), self.b().dot(v), self.c().dot(v))
519513
}
520514

521515
/// Transposed dot product with the **X basis vector** of the matrix.
522516
#[inline]
523517
pub(crate) fn tdotx(&self, v: Vector3) -> f32 {
524-
self.x().dot(v)
518+
self.a().dot(v)
525519
}
526520

527521
/// Transposed dot product with the **Y basis vector** of the matrix.
528522
#[inline]
529523
pub(crate) fn tdoty(&self, v: Vector3) -> f32 {
530-
self.y().dot(v)
524+
self.b().dot(v)
531525
}
532526

533527
/// Transposed dot product with the **Z basis vector** of the matrix.
534528
#[inline]
535529
pub(crate) fn tdotz(&self, v: Vector3) -> f32 {
536-
self.z().dot(v)
530+
self.c().dot(v)
537531
}
538532

539-
/// Get the **X basis vector** (first column vector of the matrix).
533+
/// Get the **1st basis vector** (first column vector of the matrix).
540534
#[inline]
541-
pub fn x(&self) -> Vector3 {
535+
pub fn a(&self) -> Vector3 {
542536
Vector3::new(self.elements[0].x, self.elements[1].x, self.elements[2].x)
543537
}
544538

545-
/// Set the **X basis vector** (first column vector of the matrix).
539+
/// Set the **1st basis vector** (first column vector of the matrix).
546540
#[inline]
547-
pub fn set_x(&mut self, v: Vector3) {
541+
pub fn set_a(&mut self, v: Vector3) {
548542
self.elements[0].x = v.x;
549543
self.elements[1].x = v.y;
550544
self.elements[2].x = v.z;
551545
}
552546

553-
/// Get the **Y basis vector** (second column vector of the matrix).
547+
/// Get the **2nd basis vector** (second column vector of the matrix).
554548
#[inline]
555-
pub fn y(&self) -> Vector3 {
549+
pub fn b(&self) -> Vector3 {
556550
Vector3::new(self.elements[0].y, self.elements[1].y, self.elements[2].y)
557551
}
558552

559-
/// Set the **Y basis vector** (second column vector of the matrix).
553+
/// Set the **2nd basis vector** (second column vector of the matrix).
560554
#[inline]
561-
pub fn set_y(&mut self, v: Vector3) {
555+
pub fn set_b(&mut self, v: Vector3) {
562556
self.elements[0].y = v.x;
563557
self.elements[1].y = v.y;
564558
self.elements[2].y = v.z;
565559
}
566560

567-
/// Get the **Z basis vector** (third column vector of the matrix).
561+
/// Get the **3rd basis vector** (third column vector of the matrix).
568562
#[inline]
569-
pub fn z(&self) -> Vector3 {
563+
pub fn c(&self) -> Vector3 {
570564
Vector3::new(self.elements[0].z, self.elements[1].z, self.elements[2].z)
571565
}
572566

573-
/// Set the **Z basis vector** (third column vector of the matrix).
567+
/// Set the **3rd basis vector** (third column vector of the matrix).
574568
#[inline]
575-
pub fn set_z(&mut self, v: Vector3) {
569+
pub fn set_c(&mut self, v: Vector3) {
576570
self.elements[0].z = v.x;
577571
self.elements[1].z = v.y;
578572
self.elements[2].z = v.z;
@@ -657,9 +651,9 @@ mod tests {
657651
],
658652
};
659653

660-
assert!(basis.x() == Vector3::new(1.0, 4.0, 7.0));
661-
assert!(basis.y() == Vector3::new(2.0, 5.0, 8.0));
662-
assert!(basis.z() == Vector3::new(3.0, 6.0, 9.0));
654+
assert!(basis.a() == Vector3::new(1.0, 4.0, 7.0));
655+
assert!(basis.b() == Vector3::new(2.0, 5.0, 8.0));
656+
assert!(basis.c() == Vector3::new(3.0, 6.0, 9.0));
663657
}
664658

665659
#[test]
@@ -668,9 +662,9 @@ mod tests {
668662
elements: [Vector3::ZERO, Vector3::ZERO, Vector3::ZERO],
669663
};
670664

671-
basis.set_x(Vector3::new(1.0, 4.0, 7.0));
672-
basis.set_y(Vector3::new(2.0, 5.0, 8.0));
673-
basis.set_z(Vector3::new(3.0, 6.0, 9.0));
665+
basis.set_a(Vector3::new(1.0, 4.0, 7.0));
666+
basis.set_b(Vector3::new(2.0, 5.0, 8.0));
667+
basis.set_c(Vector3::new(3.0, 6.0, 9.0));
674668

675669
assert!(basis.elements[0] == Vector3::new(1.0, 2.0, 3.0));
676670
assert!(basis.elements[1] == Vector3::new(4.0, 5.0, 6.0));

0 commit comments

Comments
 (0)