From 1ce3164bc24d345129384fc644ccbd2f65b573fa Mon Sep 17 00:00:00 2001 From: Kim Kulling Date: Mon, 23 Dec 2024 16:33:37 +0100 Subject: [PATCH] Bug/evaluate matrix4x4 access (#5936) * Add test for 3x3 matrices and 4x4 matrix access --- code/AssetLib/3DS/3DSExporter.cpp | 6 +- code/AssetLib/3DS/3DSExporter.h | 2 +- code/AssetLib/Collada/ColladaExporter.cpp | 9 +-- code/AssetLib/Collada/ColladaExporter.h | 2 +- include/assimp/matrix4x4.h | 67 ++++++++++++----------- test/unit/AssimpAPITest_aiMatrix3x3.cpp | 18 +++++- test/unit/AssimpAPITest_aiMatrix4x4.cpp | 18 ++++++ test/unit/TestModelFactory.h | 16 ++---- test/unit/utColladaExport.cpp | 2 +- test/unit/utIssues.cpp | 7 ++- 10 files changed, 85 insertions(+), 62 deletions(-) diff --git a/code/AssetLib/3DS/3DSExporter.cpp b/code/AssetLib/3DS/3DSExporter.cpp index 5341a69f1..326627218 100644 --- a/code/AssetLib/3DS/3DSExporter.cpp +++ b/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 &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 diff --git a/code/AssetLib/3DS/3DSExporter.h b/code/AssetLib/3DS/3DSExporter.h index 9e3e42911..9dda720b1 100644 --- a/code/AssetLib/3DS/3DSExporter.h +++ b/code/AssetLib/3DS/3DSExporter.h @@ -66,7 +66,7 @@ namespace Assimp { class Discreet3DSExporter { public: Discreet3DSExporter(std::shared_ptr &outfile, const aiScene* pScene); - ~Discreet3DSExporter(); + ~Discreet3DSExporter() = default; private: void WriteMeshes(); diff --git a/code/AssetLib/Collada/ColladaExporter.cpp b/code/AssetLib/Collada/ColladaExporter.cpp index 3fc3a6e15..f3297dee4 100644 --- a/code/AssetLib/Collada/ColladaExporter.cpp +++ b/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; diff --git a/code/AssetLib/Collada/ColladaExporter.h b/code/AssetLib/Collada/ColladaExporter.h index 05e076034..5277b1e92 100644 --- a/code/AssetLib/Collada/ColladaExporter.h +++ b/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 diff --git a/include/assimp/matrix4x4.h b/include/assimp/matrix4x4.h index 861a7acef..ff3d3e01f 100644 --- a/include/assimp/matrix4x4.h +++ b/include/assimp/matrix4x4.h @@ -73,11 +73,14 @@ template class aiQuaterniont; template 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& scaling, const aiQuaterniont& rotation, - const aiVector3t& position); + const aiVector3t& 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 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 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& scaling, aiQuaterniont& rotation, aiVector3t& position) const; - // ------------------------------------------------------------------- - /** @fn void Decompose(aiVector3t& pScaling, aiVector3t& pRotation, aiVector3t& 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& pScaling, aiVector3t& pRotation, aiVector3t& pPosition) const; - // ------------------------------------------------------------------- - /** @fn void Decompose(aiVector3t& pScaling, aiVector3t& pRotationAxis, TReal& pRotationAngle, aiVector3t& 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& pScaling, aiVector3t& pRotationAxis, TReal& pRotationAngle, aiVector3t& 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& 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 */ diff --git a/test/unit/AssimpAPITest_aiMatrix3x3.cpp b/test/unit/AssimpAPITest_aiMatrix3x3.cpp index 3a12b1e55..2d81ce63c 100644 --- a/test/unit/AssimpAPITest_aiMatrix3x3.cpp +++ b/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 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 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++; + } + } +} diff --git a/test/unit/AssimpAPITest_aiMatrix4x4.cpp b/test/unit/AssimpAPITest_aiMatrix4x4.cpp index d57026348..4e2e5f4bb 100644 --- a/test/unit/AssimpAPITest_aiMatrix4x4.cpp +++ b/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 +#include 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 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++; + } + } +} diff --git a/test/unit/TestModelFactory.h b/test/unit/TestModelFactory.h index 2ddf37e3e..78e22baae 100644 --- a/test/unit/TestModelFactory.h +++ b/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 }; diff --git a/test/unit/utColladaExport.cpp b/test/unit/utColladaExport.cpp index 5fcd48a9a..a7e34194a 100644 --- a/test/unit/utColladaExport.cpp +++ b/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 origFOV; + std::unique_ptr origFOV(new float[origNumCams]); std::unique_ptr orifClipPlaneNear(new float[origNumCams]); std::unique_ptr orifClipPlaneFar(new float[origNumCams]); diff --git a/test/unit/utIssues.cpp b/test/unit/utIssues.cpp index b153147f1..af538b7fd 100644 --- a/test/unit/utIssues.cpp +++ b/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()));