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 <kimkulling@users.noreply.github.com>
pull/5397/head^2
fvbj 2024-02-06 21:24:41 +01:00 committed by GitHub
parent c877a15140
commit 2d98f6a880
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
10 changed files with 88 additions and 16 deletions

View File

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

View File

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

View File

@ -55,6 +55,7 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
#include <assimp/version.h> #include <assimp/version.h>
#include <assimp/Exporter.hpp> #include <assimp/Exporter.hpp>
#include <assimp/IOSystem.hpp> #include <assimp/IOSystem.hpp>
#include <assimp/config.h>
// Header files, standard library. // Header files, standard library.
#include <cinttypes> #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 // Always on as our triangulation process is aware of this type of encoding
mAsset->extensionsUsed.FB_ngon_encoding = true; 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) { if (isBinary) {
mAsset->SetAsBinary(); mAsset->SetAsBinary();
} }
@ -1455,7 +1460,7 @@ unsigned int glTF2Exporter::ExportNodeHierarchy(const aiNode *n) {
node->name = n->mName.C_Str(); node->name = n->mName.C_Str();
if (!n->mTransformation.IsIdentity()) { if (!n->mTransformation.IsIdentity(configEpsilon)) {
node->matrix.isPresent = true; node->matrix.isPresent = true;
CopyValue(n->mTransformation, node->matrix.value); 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); 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"))) { if (mScene->mNumAnimations > 0 || (mProperties && mProperties->HasPropertyBool("GLTF2_NODE_IN_TRS"))) {
aiQuaternion quaternion; aiQuaternion quaternion;
n->mTransformation.Decompose(*reinterpret_cast<aiVector3D *>(&node->scale.value), quaternion, *reinterpret_cast<aiVector3D *>(&node->translation.value)); n->mTransformation.Decompose(*reinterpret_cast<aiVector3D *>(&node->scale.value), quaternion, *reinterpret_cast<aiVector3D *>(&node->translation.value));

View File

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

View File

@ -225,6 +225,21 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
#define AI_CONFIG_PP_PTV_ROOT_TRANSFORMATION \ #define AI_CONFIG_PP_PTV_ROOT_TRANSFORMATION \
"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 /** @brief Configures the #aiProcess_FindDegenerates step to
* remove degenerated primitives from the import - immediately. * remove degenerated primitives from the import - immediately.

View File

@ -136,9 +136,13 @@ public:
// ------------------------------------------------------------------- // -------------------------------------------------------------------
/** @brief Returns true of the matrix is the identity matrix. /** @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. * 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 /** @brief Decompose a trafo matrix into its original components

View File

@ -3,7 +3,7 @@
Open Asset Import Library (assimp) Open Asset Import Library (assimp)
--------------------------------------------------------------------------- ---------------------------------------------------------------------------
Copyright (c) 2006-2022, assimp team Copyright (c) 2006-2024, assimp team
All rights reserved. All rights reserved.
@ -242,7 +242,7 @@ aiMatrix4x4t<TReal>& aiMatrix4x4t<TReal>::Inverse() {
const TReal det = Determinant(); const TReal det = Determinant();
if(det == static_cast<TReal>(0.0)) 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 // correct in a mathematical sense but it is easy to debug for the
// programmer. // programmer.
const TReal nan = std::numeric_limits<TReal>::quiet_NaN(); 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> template <typename TReal>
AI_FORCE_INLINE AI_FORCE_INLINE
bool aiMatrix4x4t<TReal>::IsIdentity() const { bool aiMatrix4x4t<TReal>::IsIdentity(const TReal epsilon) const {
// Use a small epsilon to solve floating-point inaccuracies
const static TReal epsilon = 10e-3f;
return (a2 <= epsilon && a2 >= -epsilon && return (a2 <= epsilon && a2 >= -epsilon &&
a3 <= epsilon && a3 >= -epsilon && a3 <= epsilon && a3 >= -epsilon &&
a4 <= epsilon && a4 >= -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 /** A function for creating a rotation matrix that rotates a vector called
* "from" into another vector called "to". * "from" into another vector called "to".
* Input : from[3], to[3] which both must be *normalized* non-zero vectors * 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 * Authors: Tomas Möller, John Hughes
* "Efficiently Building a Matrix to Rotate One Vector to Another" * "Efficiently Building a Matrix to Rotate One Vector to Another"
* Journal of Graphics Tools, 4(4):1-4, 1999 * Journal of Graphics Tools, 4(4):1-4, 1999

View File

@ -3,9 +3,7 @@
Open Asset Import Library (assimp) Open Asset Import Library (assimp)
--------------------------------------------------------------------------- ---------------------------------------------------------------------------
Copyright (c) 2006-2022, assimp team Copyright (c) 2006-2024, assimp team
All rights reserved. All rights reserved.
@ -47,7 +45,7 @@ using namespace Assimp;
class AssimpAPITest_aiMatrix4x4 : public AssimpMathTest { class AssimpAPITest_aiMatrix4x4 : public AssimpMathTest {
protected: protected:
virtual void SetUp() { void SetUp() override {
result_c = result_cpp = aiMatrix4x4(); result_c = result_cpp = aiMatrix4x4();
} }
@ -64,6 +62,11 @@ protected:
aiMatrix4x4 result_c, result_cpp; 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) { TEST_F(AssimpAPITest_aiMatrix4x4, aiIdentityMatrix4Test) {
// Force a non-identity matrix. // Force a non-identity matrix.
result_c = aiMatrix4x4(0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0); result_c = aiMatrix4x4(0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0);

View File

@ -90,3 +90,17 @@ TEST_F(utMatrix4x4, indexOperatorTest) {
ai_real *a15 = a12 + 3; ai_real *a15 = a12 + 3;
EXPECT_FLOAT_EQ(1.0, *a15); 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));
}

View File

@ -988,3 +988,27 @@ TEST_F(utglTF2ImportExport, noSchemaFound) {
EXPECT_NE(scene, nullptr); EXPECT_NE(scene, nullptr);
EXPECT_STREQ(importer.GetErrorString(), ""); 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));
}