Skip to content

Commit

Permalink
PR #307 was incorrect, the array-based constructors are, in fact, (#309)
Browse files Browse the repository at this point in the history 
necessary.  Although constructing a matrix from a 2D-array variable
invokes the interop constructor:
```
    const float a[2][2] = {{1,0},{0,1}};
    M22f m(a);
```
constructing a matrix from a 2D array *parameter* uses the array
constructor, which #307 thought was never used, because it was never
invoked by the test suite:
```
    void foo (const float a[2][2])
    {
        M22f m(a);
    }
```
This restores the constructors and adds a test.

Signed-off-by: Cary Phillips <[email protected]>
  • Loading branch information
@cary-ilm
cary-ilm authored Mar 18, 2023
1 parent 0983bf6 commit f5b7ae6
Show file tree
Hide file tree
Showing 2 changed files with 59 additions and 34 deletions.
50 changes: 16 additions & 34 deletions src/Imath/ImathMatrix.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,11 @@ template <class T> class IMATH_EXPORT_TEMPLATE_TYPE Matrix22
/// a a
IMATH_HOSTDEVICE IMATH_CONSTEXPR14 Matrix22 (T a) IMATH_NOEXCEPT;

/// Construct from 2x2 array:
///
/// a[0][0] a[0][1]
/// a[1][0] a[1][1]
IMATH_HOSTDEVICE IMATH_CONSTEXPR14 Matrix22 (const T a[2][2]) IMATH_NOEXCEPT;
/// Construct from given scalar values:
///
/// a b
Expand Down Expand Up @@ -137,13 +142,6 @@ template <class T> class IMATH_EXPORT_TEMPLATE_TYPE Matrix22
return *this;
}
/// @}
#else
/// Construct from 2x2 array:
///
/// a[0][0] a[0][1]
/// a[1][0] a[1][1]
IMATH_HOSTDEVICE IMATH_CONSTEXPR14 Matrix22 (const T a[2][2])
IMATH_NOEXCEPT;
#endif

/// @{
Expand Down Expand Up @@ -407,6 +405,11 @@ template <class T> class IMATH_EXPORT_TEMPLATE_TYPE Matrix33
/// a a a
IMATH_HOSTDEVICE IMATH_CONSTEXPR14 Matrix33 (T a) IMATH_NOEXCEPT;

/// Construct from 3x3 array
/// a[0][0] a[0][1] a[0][2]
/// a[1][0] a[1][1] a[1][2]
/// a[2][0] a[2][1] a[2][2]
IMATH_HOSTDEVICE IMATH_CONSTEXPR14 Matrix33 (const T a[3][3]) IMATH_NOEXCEPT;
/// Construct from given scalar values
/// a b c
/// d e f
Expand Down Expand Up @@ -487,13 +490,6 @@ template <class T> class IMATH_EXPORT_TEMPLATE_TYPE Matrix33
return *this;
}
/// @}
#else
/// Construct from 3x3 array
/// a[0][0] a[0][1] a[0][2]
/// a[1][0] a[1][1] a[1][2]
/// a[2][0] a[2][1] a[2][2]
IMATH_HOSTDEVICE IMATH_CONSTEXPR14 Matrix33 (const T a[3][3])
IMATH_NOEXCEPT;
#endif

/// @{
Expand Down Expand Up @@ -838,6 +834,12 @@ template <class T> class IMATH_EXPORT_TEMPLATE_TYPE Matrix44
/// a a a a
IMATH_HOSTDEVICE IMATH_CONSTEXPR14 Matrix44 (T a) IMATH_NOEXCEPT;

/// Construct from 4x4 array
/// a[0][0] a[0][1] a[0][2] a[0][3]
/// a[1][0] a[1][1] a[1][2] a[1][3]
/// a[2][0] a[2][1] a[2][2] a[2][3]
/// a[3][0] a[3][1] a[3][2] a[3][3]
IMATH_HOSTDEVICE IMATH_CONSTEXPR14 Matrix44 (const T a[4][4]) IMATH_NOEXCEPT;
/// Construct from given scalar values
/// a b c d
/// e f g h
Expand Down Expand Up @@ -956,14 +958,6 @@ template <class T> class IMATH_EXPORT_TEMPLATE_TYPE Matrix44
return *this;
}
/// @}
#else
/// Construct from 4x4 array
/// a[0][0] a[0][1] a[0][2] a[0][3]
/// a[1][0] a[1][1] a[1][2] a[1][3]
/// a[2][0] a[2][1] a[2][2] a[2][3]
/// a[3][0] a[3][1] a[3][2] a[3][3]
IMATH_HOSTDEVICE IMATH_CONSTEXPR14 Matrix44 (const T a[4][4])
IMATH_NOEXCEPT;
#endif

/// @{
Expand Down Expand Up @@ -1445,8 +1439,6 @@ IMATH_HOSTDEVICE
x[1][1] = a;
}

#if !IMATH_FOREIGN_VECTOR_INTEROP

template <class T>
IMATH_HOSTDEVICE IMATH_CONSTEXPR14 inline Matrix22<T>::Matrix22 (
const T a[2][2]) IMATH_NOEXCEPT
Expand All @@ -1461,8 +1453,6 @@ IMATH_HOSTDEVICE IMATH_CONSTEXPR14 inline Matrix22<T>::Matrix22 (
x[1][1] = a[1][1];
}

#endif

template <class T>
IMATH_HOSTDEVICE IMATH_CONSTEXPR14 inline Matrix22<T>::Matrix22 (
T a, T b, T c, T d) IMATH_NOEXCEPT
Expand Down Expand Up @@ -2062,8 +2052,6 @@ IMATH_HOSTDEVICE
x[2][2] = a;
}

#if !IMATH_FOREIGN_VECTOR_INTEROP

template <class T>
IMATH_HOSTDEVICE IMATH_CONSTEXPR14 inline Matrix33<T>::Matrix33 (
const T a[3][3]) IMATH_NOEXCEPT
Expand All @@ -2083,8 +2071,6 @@ IMATH_HOSTDEVICE IMATH_CONSTEXPR14 inline Matrix33<T>::Matrix33 (
x[2][2] = a[2][2];
}

#endif

template <class T>
IMATH_HOSTDEVICE IMATH_CONSTEXPR14 inline Matrix33<T>::Matrix33 (
T a, T b, T c, T d, T e, T f, T g, T h, T i) IMATH_NOEXCEPT
Expand Down Expand Up @@ -3391,8 +3377,6 @@ IMATH_HOSTDEVICE
x[3][3] = a;
}

#if !IMATH_FOREIGN_VECTOR_INTEROP

template <class T>
IMATH_HOSTDEVICE IMATH_CONSTEXPR14 inline Matrix44<T>::Matrix44 (
const T a[4][4]) IMATH_NOEXCEPT
Expand All @@ -3415,8 +3399,6 @@ IMATH_HOSTDEVICE IMATH_CONSTEXPR14 inline Matrix44<T>::Matrix44 (
x[3][3] = a[3][3];
}

#endif

template <class T>
IMATH_HOSTDEVICE IMATH_CONSTEXPR14 inline Matrix44<T>::Matrix44 (
T a,
Expand Down
43 changes: 43 additions & 0 deletions src/ImathTest/testMatrix.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,28 @@ using IMATH_INTERNAL_NAMESPACE::Int64;
// or are more convenient to test from C++.
//

void
testMatrix22ArrayConstructor(const float a[2][2])
{
IMATH_INTERNAL_NAMESPACE::M22f m(a);
assert(m == IMATH_INTERNAL_NAMESPACE::M22f());
}

void
testMatrix33ArrayConstructor(const float a[3][3])
{
IMATH_INTERNAL_NAMESPACE::M33f m(a);
assert(m == IMATH_INTERNAL_NAMESPACE::M33f());
}

void
testMatrix44ArrayConstructor(const float a[4][4])
{
IMATH_INTERNAL_NAMESPACE::M44f m(a);
assert(m == IMATH_INTERNAL_NAMESPACE::M44f());
}


void
testMatrix ()
{
Expand Down Expand Up @@ -70,6 +92,12 @@ testMatrix ()
test3.makeIdentity ();
assert (test2 == test3);

const float a[2][2] = {
{ 1.0f, 0.0f },
{ 0.0f, 1.0f }
};
testMatrix22ArrayConstructor(a);

m1 = 42;
assert (m1[0][0] == 42 && m1[0][1] == 42 && m1[1][0] == 42 && m1[1][1] == 42);

Expand Down Expand Up @@ -118,6 +146,13 @@ testMatrix ()

assert (test5[1][0] == 3.0);
assert (test5[1][1] == 4.0);

const float a[3][3] = {
{ 1.0f, 0.0f, 0.0f },
{ 0.0f, 1.0f, 0.0f },
{ 0.0f, 0.0f, 1.0f }
};
testMatrix33ArrayConstructor(a);
}

{
Expand Down Expand Up @@ -325,6 +360,14 @@ testMatrix ()
test3.makeIdentity ();
assert (test2 == test3);

const float a[4][4] = {
{ 1.0f, 0.0f, 0.0f, 0.0f },
{ 0.0f, 1.0f, 0.0f, 0.0f },
{ 0.0f, 0.0f, 1.0f, 0.0f },
{ 0.0f, 0.0f, 0.0f, 1.0f }
};
testMatrix44ArrayConstructor(a);

//
// Test non-equality when a NAN is in the same
// place in two identical matrices
Expand Down

0 comments on commit f5b7ae6

Please sign in to comment.