Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 42 additions & 6 deletions src/matrix.rs
Original file line number Diff line number Diff line change
Expand Up @@ -580,14 +580,26 @@ impl<S: BaseFloat> Matrix for Matrix2<S> {

#[inline]
fn swap_columns(&mut self, a: usize, b: usize) {
unsafe { ptr::swap(&mut self[a], &mut self[b]) };
if a == b {
return;
}
// S: BaseFloat implies S: Copy, so column vectors are Copy — no unsafe needed.
let tmp = self[a];
self[a] = self[b];
self[b] = tmp;
}

#[inline]
fn swap_elements(&mut self, a: (usize, usize), b: (usize, usize)) {
if a == b {
return;
}
let (ac, ar) = a;
let (bc, br) = b;
unsafe { ptr::swap(&mut self[ac][ar], &mut self[bc][br]) };
// S: BaseFloat implies S: Copy — swap by value without unsafe.
let tmp = self[ac][ar];
self[ac][ar] = self[bc][br];
self[bc][br] = tmp;
}

fn transpose(&self) -> Matrix2<S> {
Expand Down Expand Up @@ -679,14 +691,26 @@ impl<S: BaseFloat> Matrix for Matrix3<S> {

#[inline]
fn swap_columns(&mut self, a: usize, b: usize) {
unsafe { ptr::swap(&mut self[a], &mut self[b]) };
if a == b {
return;
}
// S: BaseFloat implies S: Copy, so column vectors are Copy — no unsafe needed.
let tmp = self[a];
self[a] = self[b];
self[b] = tmp;
}

#[inline]
fn swap_elements(&mut self, a: (usize, usize), b: (usize, usize)) {
if a == b {
return;
}
let (ac, ar) = a;
let (bc, br) = b;
unsafe { ptr::swap(&mut self[ac][ar], &mut self[bc][br]) };
// S: BaseFloat implies S: Copy — swap by value without unsafe.
let tmp = self[ac][ar];
self[ac][ar] = self[bc][br];
self[bc][br] = tmp;
}

fn transpose(&self) -> Matrix3<S> {
Expand Down Expand Up @@ -795,14 +819,26 @@ impl<S: BaseFloat> Matrix for Matrix4<S> {

#[inline]
fn swap_columns(&mut self, a: usize, b: usize) {
unsafe { ptr::swap(&mut self[a], &mut self[b]) };
if a == b {
return;
}
// S: BaseFloat implies S: Copy, so column vectors are Copy — no unsafe needed.
let tmp = self[a];
self[a] = self[b];
self[b] = tmp;
}

#[inline]
fn swap_elements(&mut self, a: (usize, usize), b: (usize, usize)) {
if a == b {
return;
}
let (ac, ar) = a;
let (bc, br) = b;
unsafe { ptr::swap(&mut self[ac][ar], &mut self[bc][br]) };
// S: BaseFloat implies S: Copy — swap by value without unsafe.
let tmp = self[ac][ar];
self[ac][ar] = self[bc][br];
self[bc][br] = tmp;
}

fn transpose(&self) -> Matrix4<S> {
Expand Down
11 changes: 7 additions & 4 deletions src/structure.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,10 +72,13 @@ where
/// Swap the elements at indices `i` and `j` in-place.
#[inline]
fn swap_elements(&mut self, i: usize, j: usize) {
use std::ptr;

// Yeah, ok borrow checker – I know what I'm doing here
unsafe { ptr::swap(&mut self[i], &mut self[j]) };
if i == j {
return;
}
// Element: Copy is a supertrait bound, so we can swap without unsafe.
let tmp = self[i];
self[i] = self[j];
self[j] = tmp;
}

/// The sum of the elements of the array.
Expand Down
128 changes: 128 additions & 0 deletions tests/matrix.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,134 @@
extern crate approx;
extern crate cgmath;

/// Tests for swap_columns, swap_rows, and swap_elements correctness and soundness
/// (see https://github.com/rustgd/cgmath/issues/565).
pub mod swap {
use cgmath::*;

#[test]
fn matrix2_swap_columns_same_index() {
let original = Matrix2::new(1.0f64, 2.0, 3.0, 4.0);
let mut m = original;
m.swap_columns(0, 0);
assert_eq!(m, original);
m.swap_columns(1, 1);
assert_eq!(m, original);
}

#[test]
fn matrix2_swap_columns_different_indices() {
let mut m = Matrix2::new(1.0f64, 2.0, 3.0, 4.0);
m.swap_columns(0, 1);
assert_eq!(m, Matrix2::new(3.0, 4.0, 1.0, 2.0));
// swapping back gives original
m.swap_columns(1, 0);
assert_eq!(m, Matrix2::new(1.0, 2.0, 3.0, 4.0));
}

#[test]
fn matrix2_swap_elements_same_position() {
let original = Matrix2::new(1.0f64, 2.0, 3.0, 4.0);
let mut m = original;
m.swap_elements((0, 0), (0, 0));
assert_eq!(m, original);
m.swap_elements((1, 1), (1, 1));
assert_eq!(m, original);
}

#[test]
fn matrix2_swap_elements_same_column_different_row() {
// column 0 has elements [1.0 (row0), 2.0 (row1)]
let mut m = Matrix2::new(1.0f64, 2.0, 3.0, 4.0);
m.swap_elements((0, 0), (0, 1));
assert_eq!(m, Matrix2::new(2.0, 1.0, 3.0, 4.0));
}

#[test]
fn matrix3_swap_columns_same_index() {
let original = Matrix3::new(1.0f64, 2.0, 3.0, 4.0, 5.0, 6.0, 7.0, 8.0, 9.0);
let mut m = original;
m.swap_columns(0, 0);
assert_eq!(m, original);
m.swap_columns(1, 1);
assert_eq!(m, original);
m.swap_columns(2, 2);
assert_eq!(m, original);
}

#[test]
fn matrix3_swap_columns_different_indices() {
let mut m = Matrix3::new(1.0f64, 2.0, 3.0, 4.0, 5.0, 6.0, 7.0, 8.0, 9.0);
let col0_before = m[0];
let col2_before = m[2];
m.swap_columns(0, 2);
assert_eq!(m[0], col2_before);
assert_eq!(m[2], col0_before);
}

#[test]
fn matrix3_swap_elements_same_position() {
let original = Matrix3::new(1.0f64, 2.0, 3.0, 4.0, 5.0, 6.0, 7.0, 8.0, 9.0);
let mut m = original;
m.swap_elements((1, 1), (1, 1));
assert_eq!(m, original);
}

#[test]
fn matrix3_swap_elements_same_column_different_row() {
let mut m = Matrix3::new(1.0f64, 2.0, 3.0, 4.0, 5.0, 6.0, 7.0, 8.0, 9.0);
m.swap_elements((1, 0), (1, 2));
// column 1 had [4, 5, 6]; after swap row0 and row2: [6, 5, 4]
assert_eq!(m[1], Vector3::new(6.0, 5.0, 4.0));
}

#[test]
fn matrix4_swap_columns_same_index() {
let original = Matrix4::new(
1.0f64, 2.0, 3.0, 4.0, 5.0, 6.0, 7.0, 8.0, 9.0, 10.0, 11.0, 12.0, 13.0, 14.0, 15.0,
16.0,
);
let mut m = original;
m.swap_columns(2, 2);
assert_eq!(m, original);
}

#[test]
fn matrix4_swap_columns_different_indices() {
let mut m = Matrix4::new(
1.0f64, 2.0, 3.0, 4.0, 5.0, 6.0, 7.0, 8.0, 9.0, 10.0, 11.0, 12.0, 13.0, 14.0, 15.0,
16.0,
);
let col1_before = m[1];
let col3_before = m[3];
m.swap_columns(1, 3);
assert_eq!(m[1], col3_before);
assert_eq!(m[3], col1_before);
}

#[test]
fn matrix4_swap_elements_same_position() {
let original = Matrix4::new(
1.0f64, 2.0, 3.0, 4.0, 5.0, 6.0, 7.0, 8.0, 9.0, 10.0, 11.0, 12.0, 13.0, 14.0, 15.0,
16.0,
);
let mut m = original;
m.swap_elements((2, 2), (2, 2));
assert_eq!(m, original);
}

#[test]
fn matrix4_swap_elements_same_column_different_row() {
let mut m = Matrix4::new(
1.0f64, 2.0, 3.0, 4.0, 5.0, 6.0, 7.0, 8.0, 9.0, 10.0, 11.0, 12.0, 13.0, 14.0, 15.0,
16.0,
);
// column 2 has rows [9, 10, 11, 12]; swap row 0 and row 3
m.swap_elements((2, 0), (2, 3));
assert_eq!(m[2], Vector4::new(12.0, 10.0, 11.0, 9.0));
}
}

pub mod matrix2 {
use std::f64;

Expand Down