Browse Source

Prefer to use (col, row) instead of [col][row] on matrices.

They should be in theory both compiled to the same code, but in practice
[][] sometimes fails during optimization.
vectorfields
Vladimír Vondruš 14 years ago
parent
commit
2b62cc4012
  1. 36
      src/Math/Matrix.h
  2. 13
      src/Math/Test/MatrixTest.cpp

36
src/Math/Matrix.h

@ -125,7 +125,12 @@ template<size_t size, class T> class Matrix {
inline T* data() { return _data; } inline T* data() { return _data; }
inline constexpr const T* data() const { return _data; } /**< @overload */ inline constexpr const T* data() const { return _data; } /**< @overload */
/** @brief %Matrix column */ /**
* @brief %Matrix column
*
* For accessing individual elements prefer to use operator(), as it
* is guaranteed to not involve unnecessary conversions.
*/
inline Vector<size, T>& operator[](size_t col) { inline Vector<size, T>& operator[](size_t col) {
return Vector<size, T>::from(_data+col*size); return Vector<size, T>::from(_data+col*size);
} }
@ -134,6 +139,20 @@ template<size_t size, class T> class Matrix {
return Vector<size, T>::from(_data+col*size); return Vector<size, T>::from(_data+col*size);
} }
/**
* @brief Element on given position
*
* Prefer this instead of using `[][]`.
* @see operator[]
*/
inline T& operator()(size_t col, size_t row) {
return _data[col*size+row];
}
/** @overload */
inline constexpr const T& operator()(size_t col, size_t row) const {
return _data[col*size+row];
}
/** @brief Equality operator */ /** @brief Equality operator */
inline bool operator==(const Matrix<size, T>& other) const { inline bool operator==(const Matrix<size, T>& other) const {
for(size_t i = 0; i != size*size; ++i) for(size_t i = 0; i != size*size; ++i)
@ -243,15 +262,6 @@ template<size_t size, class T> class Matrix {
return Matrix<size, T>(first, next...); return Matrix<size, T>(first, next...);
} }
/* Used internally instead of [][], because GCC does some heavy
optimalization in release mode which breaks it */
inline T& operator()(size_t col, size_t row) {
return _data[col*size+row];
}
inline constexpr const T& operator()(size_t col, size_t row) const {
return _data[col*size+row];
}
T _data[size*size]; T _data[size*size];
}; };
@ -318,7 +328,7 @@ template<size_t size, class T> class MatrixDeterminant {
T out(0); T out(0);
for(size_t col = 0; col != size; ++col) for(size_t col = 0; col != size; ++col)
out += ((col & 1) ? -1 : 1)*m[col][0]*m.ij(col, 0).determinant(); out += ((col & 1) ? -1 : 1)*m(col, 0)*m.ij(col, 0).determinant();
return out; return out;
} }
@ -328,7 +338,7 @@ template<class T> class MatrixDeterminant<2, T> {
public: public:
/** @brief Functor */ /** @brief Functor */
inline constexpr T operator()(const Matrix<2, T>& m) { inline constexpr T operator()(const Matrix<2, T>& m) {
return m[0][0]*m[1][1] - m[1][0]*m[0][1]; return m(0, 0)*m(1, 1) - m(1, 0)*m(0, 1);
} }
}; };
@ -336,7 +346,7 @@ template<class T> class MatrixDeterminant<1, T> {
public: public:
/** @brief Functor */ /** @brief Functor */
inline constexpr T operator()(const Matrix<1, T>& m) { inline constexpr T operator()(const Matrix<1, T>& m) {
return m[0][0]; return m(0, 0);
} }
}; };

13
src/Math/Test/MatrixTest.cpp

@ -124,9 +124,10 @@ void MatrixTest::data() {
m[3] = vector; m[3] = vector;
m[2][1] = 1.0f; m[2][1] = 1.0f;
m[1][2] = 1.5f; m(1, 2) = 1.5f;
CORRADE_COMPARE(m[2][1], 1.0f); CORRADE_COMPARE(m(2, 1), 1.0f);
CORRADE_COMPARE(m[1][2], 1.5f);
CORRADE_COMPARE(m[3], vector); CORRADE_COMPARE(m[3], vector);
Matrix4 expected( Matrix4 expected(
@ -142,20 +143,20 @@ void MatrixTest::data() {
void MatrixTest::copy() { void MatrixTest::copy() {
Matrix4 m1(Matrix4::Zero); Matrix4 m1(Matrix4::Zero);
m1[2][3] = 1.0f; m1(2, 3) = 1.0f;
/* Copy */ /* Copy */
Matrix4 m2(m1); Matrix4 m2(m1);
Matrix4 m3; Matrix4 m3;
m3[0][0] = 1.0f; /* this line is here so it's not optimized to Matrix4 m3(m1) */ m3(0, 0) = 1.0f; /* this line is here so it's not optimized to Matrix4 m3(m1) */
m3 = m1; m3 = m1;
/* Change original */ /* Change original */
m1[3][2] = 1.0f; m1(3, 2) = 1.0f;
/* Verify the copy is the same as original */ /* Verify the copy is the same as original */
Matrix4 original(Matrix4::Zero); Matrix4 original(Matrix4::Zero);
original[2][3] = 1.0f; original(2, 3) = 1.0f;
CORRADE_COMPARE(m2, original); CORRADE_COMPARE(m2, original);
CORRADE_COMPARE(m3, original); CORRADE_COMPARE(m3, original);

Loading…
Cancel
Save