Bug/evaluate matrix4x4 access (#5936)

* Add test for 3x3 matrices and 4x4 matrix access

code/AssetLib/3DS/3DSExporter.cpp

@@ -4,7 +4,6 @@ Open Asset Import Library (assimp)
 
 Copyright (c) 2006-2024, assimp team
 
-
 All rights reserved.
 
 Redistribution and use of this software in source and binary forms,
@@ -103,7 +102,7 @@ private:
 // preserves the mesh's given name if it has one. |index| is the index
 // of the mesh in |aiScene::mMeshes|.
 std::string GetMeshName(const aiMesh &mesh, unsigned int index, const aiNode &node) {
-    static const char underscore = '_';
+    static constexpr char underscore = '_';
     char postfix[10] = { 0 };
     ASSIMP_itoa10(postfix, index);
 
@@ -209,9 +208,6 @@ Discreet3DSExporter::Discreet3DSExporter(std::shared_ptr<IOStream> &outfile, con
     }
 }
 
-// ------------------------------------------------------------------------------------------------
-Discreet3DSExporter::~Discreet3DSExporter() = default;
-
 // ------------------------------------------------------------------------------------------------
 int Discreet3DSExporter::WriteHierarchy(const aiNode &node, int seq, int sibling_level) {
     // 3DS scene hierarchy is serialized as in http://www.martinreddy.net/gfx/3d/3DS.spec

code/AssetLib/3DS/3DSExporter.h

@@ -66,7 +66,7 @@ namespace Assimp {
 class Discreet3DSExporter {
 public:
     Discreet3DSExporter(std::shared_ptr<IOStream> &outfile, const aiScene* pScene);
-    ~Discreet3DSExporter();
+    ~Discreet3DSExporter() = default;
 
 private:
     void WriteMeshes();

code/AssetLib/Collada/ColladaExporter.cpp

@@ -4,7 +4,6 @@ Open Asset Import Library (assimp)
 
 Copyright (c) 2006-2024, assimp team
 
-
 All rights reserved.
 
 Redistribution and use of this software in source and binary forms,
@@ -36,7 +35,6 @@ DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
 THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
 (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
 OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
-
 ----------------------------------------------------------------------
 */
 
@@ -152,10 +150,6 @@ ColladaExporter::ColladaExporter(const aiScene *pScene, IOSystem *pIOSystem, con
     WriteFile();
 }
 
-// ------------------------------------------------------------------------------------------------
-// Destructor
-ColladaExporter::~ColladaExporter() = default;
-
 // ------------------------------------------------------------------------------------------------
 // Starts writing the contents
 void ColladaExporter::WriteFile() {
@@ -558,7 +552,8 @@ void ColladaExporter::WriteAmbientLight(const aiLight *const light) {
 
 // ------------------------------------------------------------------------------------------------
 // Reads a single surface entry from the given material keys
-bool ColladaExporter::ReadMaterialSurface(Surface &poSurface, const aiMaterial &pSrcMat, aiTextureType pTexture, const char *pKey, size_t pType, size_t pIndex) {
+bool ColladaExporter::ReadMaterialSurface(Surface &poSurface, const aiMaterial &pSrcMat, aiTextureType pTexture,
+        const char *pKey, size_t pType, size_t pIndex) {
     if (pSrcMat.GetTextureCount(pTexture) > 0) {
         aiString texfile;
         unsigned int uvChannel = 0;

code/AssetLib/Collada/ColladaExporter.h

@@ -72,7 +72,7 @@ public:
     ColladaExporter(const aiScene *pScene, IOSystem *pIOSystem, const std::string &path, const std::string &file);
 
     /// Destructor
-    virtual ~ColladaExporter();
+    virtual ~ColladaExporter() = default;
 
 protected:
     /// Starts writing the contents

include/assimp/matrix4x4.h

@@ -73,11 +73,14 @@ template<typename TReal> class aiQuaterniont;
 template<typename TReal>
 class aiMatrix4x4t {
 public:
-
-    /** set to identity */
+    /** 
+     * @brief Set to identity 
+     * */
     aiMatrix4x4t() AI_NO_EXCEPT;
 
-    /** construction from single values */
+    /** 
+     * @brief Construction from single values 
+     * */
     aiMatrix4x4t (  TReal _a1, TReal _a2, TReal _a3, TReal _a4,
                     TReal _b1, TReal _b2, TReal _b3, TReal _b4,
                     TReal _c1, TReal _c2, TReal _c3, TReal _c4,
@@ -93,18 +96,18 @@ public:
      * @param position The position for the x,y,z axes
      */
     aiMatrix4x4t(const aiVector3t<TReal>& scaling, const aiQuaterniont<TReal>& rotation,
-        const aiVector3t<TReal>& position);
+          const aiVector3t<TReal>& position);
 
     // array access operators
-	/** @fn TReal* operator[] (unsigned int p_iIndex)
-	 *  @param [in] p_iIndex - index of the row.
-	 *  @return pointer to pointed row.
-	 */
-    TReal* operator[]       (unsigned int p_iIndex);
-
-	/** @fn const TReal* operator[] (unsigned int p_iIndex) const
-	 *  @overload TReal* operator[] (unsigned int p_iIndex)
-	 */
+    /** @fn TReal* operator[] (unsigned int p_iIndex)
+     *  @param [in] p_iIndex - index of the row.
+     *  @return pointer to pointed row.
+     */
+      TReal* operator[] (unsigned int p_iIndex);
+
+    /** @fn const TReal* operator[] (unsigned int p_iIndex) const
+     *  @overload TReal* operator[] (unsigned int p_iIndex)
+     */
     const TReal* operator[] (unsigned int p_iIndex) const;
 
     // comparison operators
@@ -132,8 +135,12 @@ public:
      *  Beware, use (f != f) to check whether a TReal f is qnan.
      */
     aiMatrix4x4t& Inverse();
-    TReal Determinant() const;
 
+    // -------------------------------------------------------------------
+    /**
+     * @brief Inverts the matrix if it is invertible.
+     */
+    TReal Determinant() const;
 
     // -------------------------------------------------------------------
     /** @brief Returns true of the matrix is the identity matrix.
@@ -147,40 +154,38 @@ public:
 
     // -------------------------------------------------------------------
     /** @brief Decompose a trafo matrix into its original components
-     *  @param scaling Receives the output scaling for the x,y,z axes
-     *  @param rotation Receives the output rotation as a hamilton
-     *   quaternion
+     *  @param scaling  Receives the output scaling for the x,y,z axes
+     *  @param rotation Receives the output rotation as a hamilton quaternion
      *  @param position Receives the output position for the x,y,z axes
      */
     void Decompose (aiVector3t<TReal>& scaling, aiQuaterniont<TReal>& rotation,
         aiVector3t<TReal>& position) const;
 
-	// -------------------------------------------------------------------
-	/** @fn void Decompose(aiVector3t<TReal>& pScaling, aiVector3t<TReal>& pRotation, aiVector3t<TReal>& pPosition) const
+    // -------------------------------------------------------------------
+    /**
      *  @brief Decompose a trafo matrix into its original components.
-     * Thx to good FAQ at http://www.gamedev.ru/code/articles/faq_matrix_quat
-     *  @param [out] pScaling - Receives the output scaling for the x,y,z axes.
+     *  Thx to good FAQ at http://www.gamedev.ru/code/articles/faq_matrix_quat
+     *  @param [out] pScaling  - Receives the output scaling for the x,y,z axes.
      *  @param [out] pRotation - Receives the output rotation as a Euler angles.
      *  @param [out] pPosition - Receives the output position for the x,y,z axes.
      */
     void Decompose(aiVector3t<TReal>& pScaling, aiVector3t<TReal>& pRotation, aiVector3t<TReal>& pPosition) const;
 
-	// -------------------------------------------------------------------
-	/** @fn void Decompose(aiVector3t<TReal>& pScaling, aiVector3t<TReal>& pRotationAxis, TReal& pRotationAngle, aiVector3t<TReal>& pPosition) const
+    // -------------------------------------------------------------------
+    /** 
      *  @brief Decompose a trafo matrix into its original components
-	 * Thx to good FAQ at http://www.gamedev.ru/code/articles/faq_matrix_quat
-     *  @param [out] pScaling - Receives the output scaling for the x,y,z axes.
-     *  @param [out] pRotationAxis - Receives the output rotation axis.
-	 *  @param [out] pRotationAngle - Receives the output rotation angle for @ref pRotationAxis.
-     *  @param [out] pPosition - Receives the output position for the x,y,z axes.
+     * Thx to good FAQ at http://www.gamedev.ru/code/articles/faq_matrix_quat
+     *  @param [out] pScaling       - Receives the output scaling for the x,y,z axes.
+     *  @param [out] pRotationAxis  - Receives the output rotation axis.
+     *  @param [out] pRotationAngle - Receives the output rotation angle for @ref pRotationAxis.
+     *  @param [out] pPosition      - Receives the output position for the x,y,z axes.
      */
     void Decompose(aiVector3t<TReal>& pScaling, aiVector3t<TReal>& pRotationAxis, TReal& pRotationAngle, aiVector3t<TReal>& pPosition) const;
 
     // -------------------------------------------------------------------
     /** @brief Decompose a trafo matrix with no scaling into its
      *    original components
-     *  @param rotation Receives the output rotation as a hamilton
-     *    quaternion
+     *  @param rotation Receives the output rotation as a hamilton quaternion
      *  @param position Receives the output position for the x,y,z axes
      */
     void DecomposeNoScaling (aiQuaterniont<TReal>& rotation,
@@ -197,7 +202,7 @@ public:
 
     // -------------------------------------------------------------------
     /** @brief Returns a rotation matrix for a rotation around the x axis
-     *  @param a Rotation angle, in radians
+     *  @param a   Rotation angle, in radians
      *  @param out Receives the output matrix
      *  @return Reference to the output matrix
      */
@@ -205,7 +210,7 @@ public:
 
     // -------------------------------------------------------------------
     /** @brief Returns a rotation matrix for a rotation around the y axis
-     *  @param a Rotation angle, in radians
+     *  @param a   Rotation angle, in radians
      *  @param out Receives the output matrix
      *  @return Reference to the output matrix
      */

test/unit/AssimpAPITest_aiMatrix3x3.cpp

@@ -5,8 +5,6 @@ Open Asset Import Library (assimp)
 
 Copyright (c) 2006-2024, assimp team
 
-
-
 All rights reserved.
 
 Redistribution and use of this software in source and binary forms,
@@ -42,6 +40,7 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 */
 #include "UnitTestPCH.h"
 #include "MathTest.h"
+#include <array>
 
 using namespace Assimp;
 
@@ -158,3 +157,18 @@ TEST_F(AssimpAPITest_aiMatrix3x3, aiMatrix3FromToTest) {
     aiMatrix3FromTo(&result_c, &from, &to);
     EXPECT_EQ(result_cpp, result_c);
 }
+
+TEST_F(AssimpAPITest_aiMatrix3x3, operatorTest) {
+    std::array<ai_real, 9> value = { 1, 2, 3, 4, 5, 6, 7, 8,9};
+    result_cpp = aiMatrix3x3( value[0], value[1], value[2], value[3],
+                              value[4], value[5], value[6], value[7],
+                              value[8]);
+    size_t idx=0;
+    for (unsigned int i = 0; i < 3; ++i) {
+       for (unsigned int j = 0; j < 3; ++j) {
+            ai_real curValue = result_cpp[i][j];
+            EXPECT_EQ(curValue, value[idx]);
+            idx++;
+       }
+    }
+}

test/unit/AssimpAPITest_aiMatrix4x4.cpp

@@ -41,6 +41,7 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 #include "UnitTestPCH.h"
 #include "MathTest.h"
 #include <assimp/MathFunctions.h>
+#include <array>
 
 using namespace Assimp;
 
@@ -262,3 +263,20 @@ TEST_F(AssimpAPITest_aiMatrix4x4, aiMatrix4FromToTest) {
     aiMatrix4FromTo(&result_c, &from, &to);
     EXPECT_EQ(result_cpp, result_c);
 }
+
+TEST_F(AssimpAPITest_aiMatrix4x4, operatorTest) {
+    std::array<ai_real, 16> value = { 1, 2, 3, 4, 5, 6, 7, 8,
+                        9, 10, 11, 12, 13, 14, 15, 16 };
+    result_cpp = aiMatrix4x4( value[0], value[1], value[2], value[3],
+                              value[4], value[5], value[6], value[7],
+                              value[8], value[9], value[10], value[11],
+                              value[12], value[13], value[14], value[15] );
+    size_t idx=0;
+    for (unsigned int i = 0; i < 4; ++i) {
+       for (unsigned int j = 0; j < 4; ++j) {
+            ai_real curValue = result_cpp[i][j];
+            EXPECT_EQ(curValue, value[idx]);
+            idx++;
+       }
+    }
+}

test/unit/TestModelFactory.h

@@ -5,8 +5,6 @@ Open Asset Import Library (assimp)
 
 Copyright (c) 2006-2024, assimp team
 
-
-
 All rights reserved.
 
 Redistribution and use of this software in source and binary forms,
@@ -49,18 +47,14 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
 namespace Assimp {
 
-class TestModelFacttory {
+class TestModelFactory {
 public:
-    TestModelFacttory() {
-        // empty
-    }
+    TestModelFactory() = default;
 
-    ~TestModelFacttory() {
-        // empty
-    }
+    ~TestModelFactory() = default;
 
     static aiScene *createDefaultTestModel( float &opacity ) {
-        aiScene *scene( new aiScene );
+        auto *scene = new aiScene;
         scene->mNumMaterials = 1;
         scene->mMaterials = new aiMaterial*[scene->mNumMaterials];
         scene->mMaterials[ 0 ] = new aiMaterial;
@@ -89,7 +83,7 @@ public:
         scene->mMeshes[ 0 ]->mFaces[ 0 ].mIndices[ 1 ] = 1;
         scene->mMeshes[ 0 ]->mFaces[ 0 ].mIndices[ 2 ] = 2;
 
-        scene->mRootNode = new aiNode();
+        scene->mRootNode = new aiNode;
         scene->mRootNode->mNumMeshes = 1;
         scene->mRootNode->mMeshes = new unsigned int[1]{ 0 };
 

test/unit/utColladaExport.cpp

@@ -79,7 +79,7 @@ TEST_F(utColladaExport, testExportCamera) {
 
     EXPECT_EQ(AI_SUCCESS, ex->Export(pTest, "collada", file));
     const unsigned int origNumCams(pTest->mNumCameras);
-    //std::vector<float> origFOV;
+
     std::unique_ptr<float[]> origFOV(new float[origNumCams]);
     std::unique_ptr<float[]> orifClipPlaneNear(new float[origNumCams]);
     std::unique_ptr<float[]> orifClipPlaneFar(new float[origNumCams]);

test/unit/utIssues.cpp

@@ -55,7 +55,7 @@ class utIssues : public ::testing::Test {};
 
 TEST_F( utIssues, OpacityBugWhenExporting_727 ) {
     float opacity;
-    aiScene *scene = TestModelFacttory::createDefaultTestModel(opacity);
+    aiScene *scene = TestModelFactory::createDefaultTestModel(opacity);
     Assimp::Importer importer;
     Assimp::Exporter exporter;
 
@@ -69,11 +69,12 @@ TEST_F( utIssues, OpacityBugWhenExporting_727 ) {
     const aiScene *newScene( importer.ReadFile( path, aiProcess_ValidateDataStructure ) );
     ASSERT_NE( nullptr, newScene );
     float newOpacity;
-    if ( newScene->mNumMaterials > 0 ) {
+    if (newScene->mNumMaterials > 0 ) {
         EXPECT_EQ( AI_SUCCESS, newScene->mMaterials[ 0 ]->Get( AI_MATKEY_OPACITY, newOpacity ) );
         EXPECT_FLOAT_EQ( opacity, newOpacity );
     }
-    delete scene;
+    
+    TestModelFactory::releaseDefaultTestModel(&scene);
 
     // Cleanup. Delete exported dae.dae file
     EXPECT_EQ(0, std::remove(path.c_str()));