From ec9226c5b9788d505300dc65470068032ba5598d Mon Sep 17 00:00:00 2001 From: aramis_acg Date: Thu, 5 Feb 2009 16:16:04 +0000 Subject: [PATCH] Fixed nasty bug in FindInstancesProcess.cpp. Added a small epsilon to some functions in the API. git-svn-id: https://assimp.svn.sourceforge.net/svnroot/assimp/trunk@331 67173fc5-114c-0410-ac8e-9d2fd5bffc1f --- code/FindInstancesProcess.cpp | 55 ++++++++++++++++++++++++++++------- code/FindInstancesProcess.h | 6 ++++ include/aiTypes.h | 7 +++-- 3 files changed, 54 insertions(+), 14 deletions(-) diff --git a/code/FindInstancesProcess.cpp b/code/FindInstancesProcess.cpp index 400e57fa0..c1a145a47 100644 --- a/code/FindInstancesProcess.cpp +++ b/code/FindInstancesProcess.cpp @@ -51,6 +51,7 @@ using namespace Assimp; // ------------------------------------------------------------------------------------------------ // Constructor to be privately used by Importer FindInstancesProcess::FindInstancesProcess() +: configSpeedFlag (false) {} // ------------------------------------------------------------------------------------------------ @@ -62,7 +63,17 @@ FindInstancesProcess::~FindInstancesProcess() // Returns whether the processing step is present in the given flag field. bool FindInstancesProcess::IsActive( unsigned int pFlags) const { - return 0 != (pFlags & aiProcess_FindInstances); + // FindInstances makes absolutely no sense together with PreTransformVertices + // fixme: spawn error message somewhere else? + return 0 != (pFlags & aiProcess_FindInstances) && 0 == (pFlags & aiProcess_PreTransformVertices); +} + +// ------------------------------------------------------------------------------------------------ +// Setup properties for the step +void FindInstancesProcess::SetupProperties(const Importer* pImp) +{ + // AI_CONFIG_FAVOUR_SPEED + configSpeedFlag = (0 != pImp->GetPropertyInteger(AI_CONFIG_FAVOUR_SPEED,0)); } // ------------------------------------------------------------------------------------------------ @@ -112,7 +123,7 @@ void FindInstancesProcess::Execute( aiScene* pScene) // the ones which are possibly equal. This step is executed early // in the pipeline, so we could, depending on the file format, // have several thousand small meshes. That's too much for a brute - // everyone-against-everyone check involving up to 25 comparisons + // everyone-against-everyone check involving up to 10 comparisons // each. boost::scoped_array hashes (new uint64_t[pScene->mNumMeshes]); boost::scoped_array remapping (new unsigned int[pScene->mNumMeshes]); @@ -204,18 +215,40 @@ void FindInstancesProcess::Execute( aiScene* pScene) } } - // It seems to be strange, but we really need to check whether the - // bones are identical too. Although it's extremely unprobable - // that they're not if control reaches here, but we need to deal - // with unprobable cases, too. - if (!CompareBones(orig,inst)) - continue; + // These two checks are actually quite expensive and almost *never* required. + // Almost. That's why they're still here. But there's no reason to do them + // in speed-targeted imports. + if (!configSpeedFlag) { + + // It seems to be strange, but we really need to check whether the + // bones are identical too. Although it's extremely unprobable + // that they're not if control reaches here, we need to deal + // with unprobable cases, too. It could still be that there are + // equal shapes which are deformed differently. + if (!CompareBones(orig,inst)) + continue; + + // For completeness ... compare even the index buffers for equality + // face order & winding order doesn't care. Input data is in verbose format. + boost::scoped_array ftbl_orig(new unsigned int[orig->mNumVertices]); + boost::scoped_array ftbl_inst(new unsigned int[orig->mNumVertices]); + + for (unsigned int tt = 0; tt < orig->mNumFaces;++tt) { + aiFace& f = orig->mFaces[tt]; + for (unsigned int nn = 0; nn < f.mNumIndices;++nn) + ftbl_orig[f.mIndices[nn]] = tt; + + aiFace& f2 = inst->mFaces[tt]; + for (unsigned int nn = 0; nn < f2.mNumIndices;++nn) + ftbl_inst[f2.mIndices[nn]] = tt; + } + if (0 != ::memcmp(ftbl_inst.get(),ftbl_orig.get(),orig->mNumVertices*sizeof(unsigned int))) + continue; + } - // FIXME: Ignore the faces for the moment ... ok!? - // We're still here. Or in other words: 'inst' is an instance of 'orig'. // Place a marker in our list that we can easily update mesh indices. - remapping[i] = a; + remapping[i] = remapping[a]; // Delete the instanced mesh, we don't need it anymore delete inst; diff --git a/code/FindInstancesProcess.h b/code/FindInstancesProcess.h index abb330dfb..2a4de3ae3 100644 --- a/code/FindInstancesProcess.h +++ b/code/FindInstancesProcess.h @@ -126,8 +126,14 @@ public: // Execute step on a given scene void Execute( aiScene* pScene); + // ------------------------------------------------------------------- + // Setup properties prior to executing the process + void SetupProperties(const Importer* pImp); + private: + bool configSpeedFlag; + }; // ! end class FindInstancesProcess } // ! end namespace Assimp diff --git a/include/aiTypes.h b/include/aiTypes.h index f06cfa41b..31cdda8b8 100644 --- a/include/aiTypes.h +++ b/include/aiTypes.h @@ -188,10 +188,10 @@ struct aiColor3D float& operator[](unsigned int i) {return *(&r + i);} // Check whether a color is black - // TODO: add epsilon? bool IsBlack() const { - return !r && !g && !b; + static const float epsilon = 10e-3f; + return fabs( r ) < epsilon && fabs( g ) < epsilon && fabs( b ) < epsilon; } #endif // !__cplusplus @@ -235,7 +235,8 @@ struct aiColor4D inline bool IsBlack() const { // The alpha component doesn't care here. black is black. - return !r && !g && !b; + static const float epsilon = 10e-3f; + return fabs( r ) < epsilon && fabs( g ) < epsilon && fabs( b ) < epsilon; } #endif // !__cplusplus