Bugfix/sparky kitty studios (#6623)

* Fixed regression that scrambled FBX blendshape order.

* Merge master to this branch

---------

Co-authored-by: Lux <lxw404@gmail.com>
Co-authored-by: Lux <52231149+lxw404@users.noreply.github.com>
Co-authored-by: Kim Kulling <kim.kulling@draeger.com>

code/AssetLib/Assbin/AssbinLoader.cpp

@@ -149,11 +149,18 @@ aiQuaternion Read<aiQuaternion>(IOStream *stream) {
 template <>
 aiString Read<aiString>(IOStream *stream) {
     aiString s;
-    stream->Read(&s.length, 4, 1);
+    uint32_t len;
-    if (s.length) {
+    if (stream->Read(&len, 4, 1) != 1) {
-        stream->Read(s.data, s.length, 1);
+        throw DeadlyImportError("ASSBIN: Unexpected EOF reading string length");
     }
-    s.data[s.length] = 0;
+    if (len >= AI_MAXLEN) {
+        throw DeadlyImportError("ASSBIN: String length too large, potential buffer overflow attempt");
+    }
+    s.length = len;
+    if ((s.length > 0) && (stream->Read(s.data, s.length, 1) != 1)) {
+        throw DeadlyImportError("ASSBIN: Unexpected EOF reading string data");
+    }
+    s.data[s.length] = '\0';
 
     return s;
 }

code/AssetLib/FBX/FBXDeformer.cpp

@@ -45,6 +45,8 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
 #ifndef ASSIMP_BUILD_NO_FBX_IMPORTER
 
+#include <algorithm>
+
 #include "FBXParser.h"
 #include "FBXDocument.h"
 #include "FBXMeshGeometry.h"
@@ -144,8 +146,10 @@ BlendShape::BlendShape(uint64_t id, const Element& element, const Document& doc,
     for (const Connection* con : conns) {
         const BlendShapeChannel* const bspc = ProcessSimpleConnection<BlendShapeChannel>(*con, false, "BlendShapeChannel -> BlendShape", element);
         if (bspc) {
-            auto pr = blendShapeChannels.insert(bspc);
+            // Only add a channel if it doesn't exist already
-            if (!pr.second) {
+            if (std::find(blendShapeChannels.begin(), blendShapeChannels.end(), bspc) == blendShapeChannels.end()) {
+                blendShapeChannels.push_back(bspc);
+            } else {
                 FBXImporter::LogWarn("there is the same blendShapeChannel id ", bspc->ID());
             }
         }
@@ -170,8 +174,10 @@ BlendShapeChannel::BlendShapeChannel(uint64_t id, const Element& element, const
     for (const Connection* con : conns) {
         const ShapeGeometry* const sg = ProcessSimpleConnection<ShapeGeometry>(*con, false, "Shape -> BlendShapeChannel", element);
         if (sg) {
-            auto pr = shapeGeometries.insert(sg);
+            // Only add a geometry if it doesn't exist already
-            if (!pr.second) {
+            if (std::find(shapeGeometries.begin(), shapeGeometries.end(), sg) == shapeGeometries.end()) {
+                shapeGeometries.push_back(sg);
+            } else {
                 FBXImporter::LogWarn("there is the same shapeGeometrie id ", sg->ID());
             }
         }

code/AssetLib/FBX/FBXDocument.h

@@ -865,14 +865,14 @@ public:
         return fullWeights;
     }
 
-    const std::unordered_set<const ShapeGeometry*>& GetShapeGeometries() const {
+    const std::vector<const ShapeGeometry*>& GetShapeGeometries() const {
         return shapeGeometries;
     }
 
 private:
     float percent;
     WeightArray fullWeights;
-    std::unordered_set<const ShapeGeometry*> shapeGeometries;
+    std::vector<const ShapeGeometry*> shapeGeometries;
 };
 
 /** DOM class for BlendShape deformers */
@@ -882,12 +882,12 @@ public:
 
     virtual ~BlendShape() = default;
 
-    const std::unordered_set<const BlendShapeChannel*>& BlendShapeChannels() const {
+    const std::vector<const BlendShapeChannel*>& BlendShapeChannels() const {
         return blendShapeChannels;
     }
 
 private:
-    std::unordered_set<const BlendShapeChannel*> blendShapeChannels;
+    std::vector<const BlendShapeChannel*> blendShapeChannels;
 };
 
 /** DOM class for skin deformer clusters (aka sub-deformers) */

code/AssetLib/FBX/FBXMeshGeometry.cpp

@@ -45,6 +45,7 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
 #ifndef ASSIMP_BUILD_NO_FBX_IMPORTER
 
+#include <algorithm>
 #include <functional>
 
 #include "FBXMeshGeometry.h"
@@ -69,8 +70,10 @@ Geometry::Geometry(uint64_t id, const Element& element, const std::string& name,
         }
         const BlendShape* const bsp = ProcessSimpleConnection<BlendShape>(*con, false, "BlendShape -> Geometry", element);
         if (bsp) {
-            auto pr = blendShapes.insert(bsp);
+            // Only add a blendshape if it doesn't exist already
-            if (!pr.second) {
+            if (std::find(blendShapes.begin(), blendShapes.end(), bsp) == blendShapes.end()) {
+                blendShapes.push_back(bsp);
+            } else {
                 FBXImporter::LogWarn("there is the same blendShape id ", bsp->ID());
             }
         }
@@ -78,7 +81,7 @@ Geometry::Geometry(uint64_t id, const Element& element, const std::string& name,
 }
 
 // ------------------------------------------------------------------------------------------------
-const std::unordered_set<const BlendShape*>& Geometry::GetBlendShapes() const {
+const std::vector<const BlendShape*>& Geometry::GetBlendShapes() const {
     return blendShapes;
 }
 

code/AssetLib/FBX/FBXMeshGeometry.h

@@ -72,11 +72,11 @@ public:
 
     /// @brief Get the BlendShape attached to this geometry or nullptr
     /// @return The blendshape arrays.
-    const std::unordered_set<const BlendShape*>& GetBlendShapes() const;
+    const std::vector<const BlendShape*>& GetBlendShapes() const;
 
 private:
     const Skin* skin;
-    std::unordered_set<const BlendShape*> blendShapes;
+    std::vector<const BlendShape*> blendShapes;
 
 };
 

code/PostProcessing/ImproveCacheLocality.cpp

@@ -58,11 +58,55 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 #include <stack>
 
 namespace Assimp {
+namespace {
+    ai_real calculateInputACMR(aiMesh *pMesh, const aiFace *const pcEnd,
+            unsigned int configCacheDepth, unsigned int meshNum) {
+        ai_real fACMR = 0.0f;
+        unsigned int *piFIFOStack = new unsigned int[configCacheDepth];
+        memset(piFIFOStack, 0xff, configCacheDepth * sizeof(unsigned int));
+        unsigned int *piCur = piFIFOStack;
+        const unsigned int *const piCurEnd = piFIFOStack + configCacheDepth;
+
+        // count the number of cache misses
+        unsigned int iCacheMisses = 0;
+        for (const aiFace *pcFace = pMesh->mFaces; pcFace != pcEnd; ++pcFace) {
+            for (unsigned int qq = 0; qq < 3; ++qq) {
+                bool bInCache = false;
+                for (unsigned int *pp = piFIFOStack; pp < piCurEnd; ++pp) {
+                    if (*pp == pcFace->mIndices[qq]) {
+                        // the vertex is in cache
+                        bInCache = true;
+                        break;
+                    }
+                }
+                if (!bInCache) {
+                    ++iCacheMisses;
+                    if (piCurEnd == piCur) {
+                        piCur = piFIFOStack;
+                    }
+                    *piCur++ = pcFace->mIndices[qq];
+                }
+            }
+        }
+        delete[] piFIFOStack;
+        fACMR = (ai_real)iCacheMisses / pMesh->mNumFaces;
+        if (3.0 == fACMR) {
+            char szBuff[128]; // should be sufficiently large in every case
+
+            // the JoinIdenticalVertices process has not been executed on this
+            // mesh, otherwise this value would normally be at least minimally
+            // smaller than 3.0 ...
+            ai_snprintf(szBuff, 128, "Mesh %u: Not suitable for vcache optimization", meshNum);
+            ASSIMP_LOG_WARN(szBuff);
+            return static_cast<ai_real>(0.f);
+        }
+        return fACMR;
+    }
+}
 
 // ------------------------------------------------------------------------------------------------
 // Constructor to be privately used by Importer
-ImproveCacheLocalityProcess::ImproveCacheLocalityProcess() :
+ImproveCacheLocalityProcess::ImproveCacheLocalityProcess() : mConfigCacheDepth(PP_ICL_PTCACHE_SIZE) {
-        mConfigCacheDepth(PP_ICL_PTCACHE_SIZE) {
     // empty
 }
 
@@ -107,51 +151,6 @@ void ImproveCacheLocalityProcess::Execute(aiScene *pScene) {
     }
 }
 
-// ------------------------------------------------------------------------------------------------
-static ai_real calculateInputACMR(aiMesh *pMesh, const aiFace *const pcEnd,
-        unsigned int configCacheDepth, unsigned int meshNum) {
-    ai_real fACMR = 0.0f;
-    unsigned int *piFIFOStack = new unsigned int[configCacheDepth];
-    memset(piFIFOStack, 0xff, configCacheDepth * sizeof(unsigned int));
-    unsigned int *piCur = piFIFOStack;
-    const unsigned int *const piCurEnd = piFIFOStack + configCacheDepth;
-
-    // count the number of cache misses
-    unsigned int iCacheMisses = 0;
-    for (const aiFace *pcFace = pMesh->mFaces; pcFace != pcEnd; ++pcFace) {
-        for (unsigned int qq = 0; qq < 3; ++qq) {
-            bool bInCache = false;
-            for (unsigned int *pp = piFIFOStack; pp < piCurEnd; ++pp) {
-                if (*pp == pcFace->mIndices[qq]) {
-                    // the vertex is in cache
-                    bInCache = true;
-                    break;
-                }
-            }
-            if (!bInCache) {
-                ++iCacheMisses;
-                if (piCurEnd == piCur) {
-                    piCur = piFIFOStack;
-                }
-                *piCur++ = pcFace->mIndices[qq];
-            }
-        }
-    }
-    delete[] piFIFOStack;
-    fACMR = (ai_real)iCacheMisses / pMesh->mNumFaces;
-    if (3.0 == fACMR) {
-        char szBuff[128]; // should be sufficiently large in every case
-
-        // the JoinIdenticalVertices process has not been executed on this
-        // mesh, otherwise this value would normally be at least minimally
-        // smaller than 3.0 ...
-        ai_snprintf(szBuff, 128, "Mesh %u: Not suitable for vcache optimization", meshNum);
-        ASSIMP_LOG_WARN(szBuff);
-        return static_cast<ai_real>(0.f);
-    }
-    return fACMR;
-}
-
 // ------------------------------------------------------------------------------------------------
 // Improves the cache coherency of a specific mesh
 ai_real ImproveCacheLocalityProcess::ProcessMesh(aiMesh *pMesh, unsigned int meshNum) {

include/assimp/StreamReader.h

@@ -182,7 +182,7 @@ public:
     // ---------------------------------------------------------------------
     /// Get the remaining stream size (to the end of the stream)
     size_t GetRemainingSize() const {
-        return (unsigned int)(mEnd - mCurrent);
+        return static_cast<size_t>(mEnd - mCurrent);
     }
 
     // ---------------------------------------------------------------------
@@ -190,16 +190,29 @@ public:
      *  return value is the remaining size of the stream if no custom
      *  read limit has been set. */
     size_t GetRemainingSizeToLimit() const {
-        return (unsigned int)(mLimit - mCurrent);
+        return static_cast<size_t>(mLimit - mCurrent);
     }
 
     // ---------------------------------------------------------------------
     /** Increase the file pointer (relative seeking)  */
     void IncPtr(intptr_t plus) {
-        mCurrent += plus;
+        // Ensure internal pointer invariants hold
-        if (mCurrent > mLimit) {
+        if (mCurrent < mBuffer || mCurrent > mLimit) {
-            throw DeadlyImportError("End of file or read limit was reached");
+            throw DeadlyImportError("StreamReader: Invalid internal pointer state");
         }
+
+        if (plus < 0) {
+            const size_t absPlus = static_cast<size_t>(-(plus + 1)) + 1;
+            if (absPlus > static_cast<size_t>(mCurrent - mBuffer)) {
+                throw DeadlyImportError("StreamReader: Attempted to seek outside buffer bounds");
+            }
+        } else if (plus > 0) {
+            if (static_cast<size_t>(plus) > static_cast<size_t>(mLimit - mCurrent)) {
+                throw DeadlyImportError("StreamReader: Attempted to seek outside buffer bounds");
+            }
+        }
+
+        mCurrent += plus;
     }
 
     // ---------------------------------------------------------------------
@@ -233,8 +246,9 @@ public:
     }
 
     /// @brief Get the current offset from the beginning of the file
+    /// @return The current offset from the beginning of the file.
     int GetCurrentPos() const {
-        return (unsigned int)(mCurrent - mBuffer);
+        return static_cast<int>(mCurrent - mBuffer);
     }
 
     void SetCurrentPos(size_t pos) {
@@ -244,10 +258,10 @@ public:
     // ---------------------------------------------------------------------
     /** Setup a temporary read limit
      *
-     *  @param limit Maximum number of bytes to be read from
+     *  @param _limit Maximum number of bytes to be read from
      *    the beginning of the file. Specifying UINT_MAX
      *    resets the limit to the original end of the stream.
-     *  Returns the previously set limit. */
+     *  @return The previously set limit. */
     unsigned int SetReadLimit(unsigned int _limit) {
         unsigned int prev = GetReadLimit();
         if (UINT_MAX == _limit) {
@@ -264,9 +278,10 @@ public:
 
     // ---------------------------------------------------------------------
     /** Get the current read limit in bytes. Reading over this limit
-     *  accidentally raises an exception.  */
+     *  accidentally raises an exception.
+     *  @return The current limit. */
     unsigned int GetReadLimit() const {
-        return (unsigned int)(mLimit - mBuffer);
+        return static_cast<unsigned int>(mLimit - mBuffer);
     }
 
     // ---------------------------------------------------------------------