Skip to content

Commit

Permalink
PR AcademySoftwareFoundation#307 was incorrect, the array-based const…
Browse files Browse the repository at this point in the history 
…ructors are, in fact, (AcademySoftwareFoundation#309)

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 AcademySoftwareFoundation#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 committed May 19, 2023
1 parent bbc0f7a commit 5c2ebbe
Show file tree
Hide file tree
Showing 2 changed files with 60 additions and 35 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 @@ -78,6 +78,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 @@ -128,13 +133,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 @@ -355,6 +353,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 @@ -411,13 +414,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 @@ -705,6 +701,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 @@ -773,14 +775,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 @@ -1185,8 +1179,6 @@ template <class T> IMATH_HOSTDEVICE IMATH_CONSTEXPR14 inline Matrix22<T>::Matrix
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 @@ -1201,8 +1193,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 @@ -1790,8 +1780,6 @@ template <class T> IMATH_HOSTDEVICE IMATH_CONSTEXPR14 inline Matrix33<T>::Matrix
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 @@ -1811,8 +1799,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 @@ -3074,8 +3060,6 @@ template <class T> IMATH_HOSTDEVICE IMATH_CONSTEXPR14 inline Matrix44<T>::Matrix
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 @@ -3098,8 +3082,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, T b, T c, T d, T e, T f, T g, T h, T i, T j, T k, T l, T m, T n, T o, T p) IMATH_NOEXCEPT
Expand Down
45 changes: 44 additions & 1 deletion src/ImathTest/testMatrix.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,29 @@ using IMATH_INTERNAL_NAMESPACE::Int64;
//

void
testMatrix()
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 ()
{
cout << "Testing functions in ImathMatrix.h" << endl;

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 @@ -117,6 +145,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 @@ -314,6 +349,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 5c2ebbe

Please sign in to comment.