From 2d98f6a880a6d10b7bab9874feb60e9834cf4b86 Mon Sep 17 00:00:00 2001 From: fvbj Date: Tue, 6 Feb 2024 21:24:41 +0100 Subject: [PATCH] Fix identity matrix check (#5445) * Fix identity matrix check Adds an extra epsilon value to check the matrix4x4 identity. The method is also used to export to GLTF/GLTF2 format to check node transformation matrices. The epsilon value can be set using AI_CONFIG_CHECK_IDENTITY_MATRIX_EPSILON with the default value set to 10e-3f for backward compatibility of legacy code. * Fix type of float values in the unit test * Update matrix4x4.inl Fix typo * Update matrix4x4.inl Remove dead code. * Add isIdentity-Test * Update AssimpAPITest_aiMatrix4x4.cpp --------- Co-authored-by: Kim Kulling --- code/AssetLib/glTF/glTFExporter.cpp | 9 +++++++-- code/AssetLib/glTF/glTFExporter.h | 3 +++ code/AssetLib/glTF2/glTF2Exporter.cpp | 9 +++++++-- code/AssetLib/glTF2/glTF2Exporter.h | 2 ++ include/assimp/config.h.in | 15 +++++++++++++++ include/assimp/matrix4x4.h | 6 +++++- include/assimp/matrix4x4.inl | 11 ++++------- test/unit/AssimpAPITest_aiMatrix4x4.cpp | 11 +++++++---- test/unit/utMatrix4x4.cpp | 14 ++++++++++++++ test/unit/utglTF2ImportExport.cpp | 24 ++++++++++++++++++++++++ 10 files changed, 88 insertions(+), 16 deletions(-) diff --git a/code/AssetLib/glTF/glTFExporter.cpp b/code/AssetLib/glTF/glTFExporter.cpp index 91d88f1ae1..9b44578c03 100644 --- a/code/AssetLib/glTF/glTFExporter.cpp +++ b/code/AssetLib/glTF/glTFExporter.cpp @@ -56,6 +56,7 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. #include #include #include +#include // Header files, standard library. #include @@ -113,6 +114,10 @@ glTFExporter::glTFExporter(const char* filename, IOSystem* pIOSystem, const aiSc mAsset = std::make_shared(pIOSystem); + configEpsilon = mProperties->GetPropertyFloat( + AI_CONFIG_CHECK_IDENTITY_MATRIX_EPSILON, + (ai_real)AI_CONFIG_CHECK_IDENTITY_MATRIX_EPSILON_DEFAULT); + if (isBinary) { mAsset->SetAsBinary(); } @@ -824,7 +829,7 @@ unsigned int glTFExporter::ExportNodeHierarchy(const aiNode* n) { Ref node = mAsset->nodes.Create(mAsset->FindUniqueID(n->mName.C_Str(), "node")); - if (!n->mTransformation.IsIdentity()) { + if (!n->mTransformation.IsIdentity(configEpsilon)) { node->matrix.isPresent = true; CopyValue(n->mTransformation, node->matrix.value); } @@ -851,7 +856,7 @@ unsigned int glTFExporter::ExportNode(const aiNode* n, Ref& parent) node->parent = parent; - if (!n->mTransformation.IsIdentity()) { + if (!n->mTransformation.IsIdentity(configEpsilon)) { node->matrix.isPresent = true; CopyValue(n->mTransformation, node->matrix.value); } diff --git a/code/AssetLib/glTF/glTFExporter.h b/code/AssetLib/glTF/glTFExporter.h index a526954020..1d036509cd 100644 --- a/code/AssetLib/glTF/glTFExporter.h +++ b/code/AssetLib/glTF/glTFExporter.h @@ -50,6 +50,7 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. #include #include +#include #include #include @@ -98,6 +99,8 @@ class glTFExporter { std::vector mBodyData; + ai_real configEpsilon; + void WriteBinaryData(IOStream *outfile, std::size_t sceneLength); void GetTexSampler(const aiMaterial *mat, glTF::TexProperty &prop); diff --git a/code/AssetLib/glTF2/glTF2Exporter.cpp b/code/AssetLib/glTF2/glTF2Exporter.cpp index 17d1624669..20a1f7ae90 100644 --- a/code/AssetLib/glTF2/glTF2Exporter.cpp +++ b/code/AssetLib/glTF2/glTF2Exporter.cpp @@ -55,6 +55,7 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. #include #include #include +#include // Header files, standard library. #include @@ -90,6 +91,10 @@ glTF2Exporter::glTF2Exporter(const char *filename, IOSystem *pIOSystem, const ai // Always on as our triangulation process is aware of this type of encoding mAsset->extensionsUsed.FB_ngon_encoding = true; + configEpsilon = mProperties->GetPropertyFloat( + AI_CONFIG_CHECK_IDENTITY_MATRIX_EPSILON, + (ai_real)AI_CONFIG_CHECK_IDENTITY_MATRIX_EPSILON_DEFAULT); + if (isBinary) { mAsset->SetAsBinary(); } @@ -1455,7 +1460,7 @@ unsigned int glTF2Exporter::ExportNodeHierarchy(const aiNode *n) { node->name = n->mName.C_Str(); - if (!n->mTransformation.IsIdentity()) { + if (!n->mTransformation.IsIdentity(configEpsilon)) { node->matrix.isPresent = true; CopyValue(n->mTransformation, node->matrix.value); } @@ -1485,7 +1490,7 @@ unsigned int glTF2Exporter::ExportNode(const aiNode *n, Ref &parent) { ExportNodeExtras(n->mMetaData, node->extras); - if (!n->mTransformation.IsIdentity()) { + if (!n->mTransformation.IsIdentity(configEpsilon)) { if (mScene->mNumAnimations > 0 || (mProperties && mProperties->HasPropertyBool("GLTF2_NODE_IN_TRS"))) { aiQuaternion quaternion; n->mTransformation.Decompose(*reinterpret_cast(&node->scale.value), quaternion, *reinterpret_cast(&node->translation.value)); diff --git a/code/AssetLib/glTF2/glTF2Exporter.h b/code/AssetLib/glTF2/glTF2Exporter.h index 7bf57b5676..47b668de70 100644 --- a/code/AssetLib/glTF2/glTF2Exporter.h +++ b/code/AssetLib/glTF2/glTF2Exporter.h @@ -49,6 +49,7 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. #include #include +#include #include #include @@ -142,6 +143,7 @@ class glTF2Exporter { std::map mTexturesByPath; std::shared_ptr mAsset; std::vector mBodyData; + ai_real configEpsilon; }; } // namespace Assimp diff --git a/include/assimp/config.h.in b/include/assimp/config.h.in index cb46d80576..403ddb32a7 100644 --- a/include/assimp/config.h.in +++ b/include/assimp/config.h.in @@ -225,6 +225,21 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. #define AI_CONFIG_PP_PTV_ROOT_TRANSFORMATION \ "PP_PTV_ROOT_TRANSFORMATION" +// --------------------------------------------------------------------------- +/** @brief Set epsilon to check the identity of the matrix 4x4. + * + * This is used by aiMatrix4x4t::IsIdentity(const TReal epsilon). + * @note The default value is 10e-3f for backward compatibility of legacy code. + * Property type: Float. + */ +#define AI_CONFIG_CHECK_IDENTITY_MATRIX_EPSILON \ + "CHECK_IDENTITY_MATRIX_EPSILON" + +// default value for AI_CONFIG_CHECK_IDENTITY_MATRIX_EPSILON +#if (!defined AI_CONFIG_CHECK_IDENTITY_MATRIX_EPSILON_DEFAULT) +# define AI_CONFIG_CHECK_IDENTITY_MATRIX_EPSILON_DEFAULT 10e-3f +#endif + // --------------------------------------------------------------------------- /** @brief Configures the #aiProcess_FindDegenerates step to * remove degenerated primitives from the import - immediately. diff --git a/include/assimp/matrix4x4.h b/include/assimp/matrix4x4.h index 67e0e356ef..7de59c7060 100644 --- a/include/assimp/matrix4x4.h +++ b/include/assimp/matrix4x4.h @@ -136,9 +136,13 @@ class aiMatrix4x4t { // ------------------------------------------------------------------- /** @brief Returns true of the matrix is the identity matrix. + * @param epsilon Value of epsilon. Default value is 10e-3 for backward + * compatibility with legacy code. + * @return Returns true of the matrix is the identity matrix. * The check is performed against a not so small epsilon. */ - inline bool IsIdentity() const; + inline bool IsIdentity(const TReal + epsilon = AI_CONFIG_CHECK_IDENTITY_MATRIX_EPSILON_DEFAULT) const; // ------------------------------------------------------------------- /** @brief Decompose a trafo matrix into its original components diff --git a/include/assimp/matrix4x4.inl b/include/assimp/matrix4x4.inl index 54d176d183..56f7296e5b 100644 --- a/include/assimp/matrix4x4.inl +++ b/include/assimp/matrix4x4.inl @@ -3,7 +3,7 @@ Open Asset Import Library (assimp) --------------------------------------------------------------------------- -Copyright (c) 2006-2022, assimp team +Copyright (c) 2006-2024, assimp team All rights reserved. @@ -242,7 +242,7 @@ aiMatrix4x4t& aiMatrix4x4t::Inverse() { const TReal det = Determinant(); if(det == static_cast(0.0)) { - // Matrix not invertible. Setting all elements to nan is not really + // Matrix is not invertible. Setting all elements to nan is not really // correct in a mathematical sense but it is easy to debug for the // programmer. const TReal nan = std::numeric_limits::quiet_NaN(); @@ -545,10 +545,7 @@ aiMatrix4x4t& aiMatrix4x4t::FromEulerAnglesXYZ(TReal x, TReal y, T // ---------------------------------------------------------------------------------------- template AI_FORCE_INLINE -bool aiMatrix4x4t::IsIdentity() const { - // Use a small epsilon to solve floating-point inaccuracies - const static TReal epsilon = 10e-3f; - +bool aiMatrix4x4t::IsIdentity(const TReal epsilon) const { return (a2 <= epsilon && a2 >= -epsilon && a3 <= epsilon && a3 >= -epsilon && a4 <= epsilon && a4 >= -epsilon && @@ -657,7 +654,7 @@ aiMatrix4x4t& aiMatrix4x4t::Scaling( const aiVector3t& v, a /** A function for creating a rotation matrix that rotates a vector called * "from" into another vector called "to". * Input : from[3], to[3] which both must be *normalized* non-zero vectors - * Output: mtx[3][3] -- a 3x3 matrix in colum-major form + * Output: mtx[3][3] -- a 3x3 matrix in column-major form * Authors: Tomas Möller, John Hughes * "Efficiently Building a Matrix to Rotate One Vector to Another" * Journal of Graphics Tools, 4(4):1-4, 1999 diff --git a/test/unit/AssimpAPITest_aiMatrix4x4.cpp b/test/unit/AssimpAPITest_aiMatrix4x4.cpp index 9372bd9c4d..2f17ea2ad4 100644 --- a/test/unit/AssimpAPITest_aiMatrix4x4.cpp +++ b/test/unit/AssimpAPITest_aiMatrix4x4.cpp @@ -3,9 +3,7 @@ Open Asset Import Library (assimp) --------------------------------------------------------------------------- -Copyright (c) 2006-2022, assimp team - - +Copyright (c) 2006-2024, assimp team All rights reserved. @@ -47,7 +45,7 @@ using namespace Assimp; class AssimpAPITest_aiMatrix4x4 : public AssimpMathTest { protected: - virtual void SetUp() { + void SetUp() override { result_c = result_cpp = aiMatrix4x4(); } @@ -64,6 +62,11 @@ class AssimpAPITest_aiMatrix4x4 : public AssimpMathTest { aiMatrix4x4 result_c, result_cpp; }; +TEST_F(AssimpAPITest_aiMatrix4x4, isIdendityTest) { + aiMatrix4x4 m = aiMatrix4x4(1.001f, 0, 0, 0, 0, 1, 0, 0, 0, 0, 1, 0, 0, 0, 0, 1); + EXPECT_TRUE(m.IsIdentity(1e-3f)); +} + TEST_F(AssimpAPITest_aiMatrix4x4, aiIdentityMatrix4Test) { // Force a non-identity matrix. result_c = aiMatrix4x4(0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0); diff --git a/test/unit/utMatrix4x4.cpp b/test/unit/utMatrix4x4.cpp index 795971afcc..86a70bfe48 100644 --- a/test/unit/utMatrix4x4.cpp +++ b/test/unit/utMatrix4x4.cpp @@ -90,3 +90,17 @@ TEST_F(utMatrix4x4, indexOperatorTest) { ai_real *a15 = a12 + 3; EXPECT_FLOAT_EQ(1.0, *a15); } + +TEST_F(utMatrix4x4, identityMatrixTest) { + aiMatrix4x4 m1 = aiMatrix4x4(1.f,0,0,0, 0,1,0,0, 0,0,1,0, 0,0,0, 1); + EXPECT_TRUE(m1.IsIdentity()); + aiMatrix4x4 m2 = aiMatrix4x4(1.02f,0,0,0, 0,1,0,0, 0,0,1,0, 0,0,0, 1); + EXPECT_FALSE(m2.IsIdentity()); + aiMatrix4x4 m3 = aiMatrix4x4(1.009f,0,0,0, 0,1,0,0, 0,0,1,0, 0,0,0, 1); + EXPECT_TRUE(m3.IsIdentity()); + + EXPECT_TRUE(m1.IsIdentity(1e-3f)); + EXPECT_FALSE(m2.IsIdentity(1e-3f)); + EXPECT_TRUE(m2.IsIdentity(1e-1f)); + EXPECT_FALSE(m3.IsIdentity(1e-3f)); +} diff --git a/test/unit/utglTF2ImportExport.cpp b/test/unit/utglTF2ImportExport.cpp index 6778e26794..478f756667 100644 --- a/test/unit/utglTF2ImportExport.cpp +++ b/test/unit/utglTF2ImportExport.cpp @@ -988,3 +988,27 @@ TEST_F(utglTF2ImportExport, noSchemaFound) { EXPECT_NE(scene, nullptr); EXPECT_STREQ(importer.GetErrorString(), ""); } + +// ------------------------------------------------------------------------------------------------ +TEST_F(utglTF2ImportExport, testSetIdentityMatrixEpsilon) { +// Assimp::Exporter exporter; + Assimp::ExportProperties properties = Assimp::ExportProperties(); + EXPECT_EQ(AI_CONFIG_CHECK_IDENTITY_MATRIX_EPSILON_DEFAULT, + properties.GetPropertyFloat("CHECK_IDENTITY_MATRIX_EPSILON", + AI_CONFIG_CHECK_IDENTITY_MATRIX_EPSILON_DEFAULT)); + aiMatrix4x4 m; + m = aiMatrix4x4(1.02f, 0, 0, 0, 0, 1, 0, 0, 0, 0, 1, 0, 0, 0, 0, 1); + EXPECT_FALSE(m.IsIdentity()); + m = aiMatrix4x4(1.001f, 0, 0, 0, 0, 1, 0, 0, 0, 0, 1, 0, 0, 0, 0, 1); + EXPECT_TRUE(m.IsIdentity()); + + bool b = properties.SetPropertyFloat("CHECK_IDENTITY_MATRIX_EPSILON", 0.0001f); + EXPECT_TRUE(!b); + ai_real epsilon = properties.GetPropertyFloat("CHECK_IDENTITY_MATRIX_EPSILON", 0.01f); + EXPECT_EQ(0.0001f, epsilon); + m = aiMatrix4x4(1.0002f, 0, 0, 0, 0, 1, 0, 0, 0, 0, 1, 0, 0, 0, 0, 1); + EXPECT_FALSE(m.IsIdentity(epsilon)); + m = aiMatrix4x4(1.00009f, 0, 0, 0, 0, 1, 0, 0, 0, 0, 1, 0, 0, 0, 0, 1); + EXPECT_TRUE(m.IsIdentity(epsilon)); +} +