Fix identity matrix check (assimp#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 <[email protected]>

code/AssetLib/glTF/glTFExporter.cpp

@@ -56,6 +56,7 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 #include <assimp/Exporter.hpp>
 #include <assimp/material.h>
 #include <assimp/scene.h>
+#include <assimp/config.h>
 
 // Header files, standard library.
 #include <memory>
@@ -113,6 +114,10 @@ glTFExporter::glTFExporter(const char* filename, IOSystem* pIOSystem, const aiSc
 
     mAsset = std::make_shared<glTF::Asset>(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> 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<Node>& parent)
 
     node->parent = parent;
 
-    if (!n->mTransformation.IsIdentity()) {
+    if (!n->mTransformation.IsIdentity(configEpsilon)) {
         node->matrix.isPresent = true;
         CopyValue(n->mTransformation, node->matrix.value);
     }

code/AssetLib/glTF/glTFExporter.h

@@ -50,6 +50,7 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
 #include <assimp/material.h>
 #include <assimp/types.h>
+#include <assimp/defs.h>
 
 #include <map>
 #include <memory>
@@ -98,6 +99,8 @@ class glTFExporter {
 
     std::vector<unsigned char> mBodyData;
 
+    ai_real configEpsilon;
+
     void WriteBinaryData(IOStream *outfile, std::size_t sceneLength);
 
     void GetTexSampler(const aiMaterial *mat, glTF::TexProperty &prop);

code/AssetLib/glTF2/glTF2Exporter.cpp

@@ -55,6 +55,7 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 #include <assimp/version.h>
 #include <assimp/Exporter.hpp>
 #include <assimp/IOSystem.hpp>
+#include <assimp/config.h>
 
 // Header files, standard library.
 #include <cinttypes>
@@ -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<Node> &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<aiVector3D *>(&node->scale.value), quaternion, *reinterpret_cast<aiVector3D *>(&node->translation.value));

code/AssetLib/glTF2/glTF2Exporter.h

@@ -49,6 +49,7 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
 #include <assimp/material.h>
 #include <assimp/types.h>
+#include <assimp/defs.h>
 
 #include <map>
 #include <memory>
@@ -142,6 +143,7 @@ class glTF2Exporter {
     std::map<std::string, unsigned int> mTexturesByPath;
     std::shared_ptr<glTF2::Asset> mAsset;
     std::vector<unsigned char> mBodyData;
+    ai_real configEpsilon;
 };
 
 } // namespace Assimp

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<TReal>::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.

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

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<TReal>& aiMatrix4x4t<TReal>::Inverse() {
     const TReal det = Determinant();
     if(det == static_cast<TReal>(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<TReal>::quiet_NaN();
@@ -545,10 +545,7 @@ aiMatrix4x4t<TReal>& aiMatrix4x4t<TReal>::FromEulerAnglesXYZ(TReal x, TReal y, T
 // ----------------------------------------------------------------------------------------
 template <typename TReal>
 AI_FORCE_INLINE
-bool aiMatrix4x4t<TReal>::IsIdentity() const {
-    // Use a small epsilon to solve floating-point inaccuracies
-    const static TReal epsilon = 10e-3f;
-
+bool aiMatrix4x4t<TReal>::IsIdentity(const TReal epsilon) const {
     return (a2 <= epsilon && a2 >= -epsilon &&
             a3 <= epsilon && a3 >= -epsilon &&
             a4 <= epsilon && a4 >= -epsilon &&
@@ -657,7 +654,7 @@ aiMatrix4x4t<TReal>& aiMatrix4x4t<TReal>::Scaling( const aiVector3t<TReal>& 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

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);

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));
+}

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));
+}
+