Merge branch 'refactor/6677-2' into 'master'

Introduce IndexedVector

See merge request OpenMW/openmw!1733

apps/openmw/mwmechanics/actors.cpp

@@ -1057,9 +1057,11 @@ namespace MWMechanics
         MWRender::Animation *anim = MWBase::Environment::get().getWorld()->getAnimation(ptr);
         if (!anim)
             return;
-        mActors.emplace(ptr, new Actor(ptr, anim));
 
-        CharacterController* ctrl = mActors[ptr]->getCharacterController();
+        auto* newActor = new Actor(ptr, anim);
+        mActors.emplace(ptr, newActor);
+
+        CharacterController* ctrl = newActor->getCharacterController();
         if (updateImmediately)
             ctrl->update(0);
 
@@ -1160,7 +1162,7 @@ namespace MWMechanics
             mActors.erase(iter);
 
             actor->updatePtr(ptr);
-            mActors.insert(std::make_pair(ptr, actor));
+            mActors.emplace(ptr, actor);
         }
     }
 
@@ -1173,7 +1175,7 @@ namespace MWMechanics
             {
                 removeTemporaryEffects(iter->first);
                 delete iter->second;
-                mActors.erase(iter++);
+                iter = mActors.erase(iter);
             }
             else
                 ++iter;

apps/openmw/mwmechanics/actors.hpp

@@ -6,6 +6,7 @@
 #include <string>
 #include <list>
 #include <map>
+#include <components/misc/indexedvector.hpp>
 
 #include "../mwmechanics/actorutil.hpp"
 
@@ -62,7 +63,7 @@ namespace MWMechanics
             Actors();
             ~Actors();
 
-            typedef std::map<MWWorld::Ptr,Actor*> PtrActorMap;
+            using PtrActorMap = Misc::IndexedVector<MWWorld::Ptr, Actor*, MWWorld::PtrHash>;
 
             PtrActorMap::const_iterator begin() { return mActors.begin(); }
             PtrActorMap::const_iterator end() { return mActors.end(); }

apps/openmw/mwphysics/physicssystem.hpp

@@ -11,6 +11,8 @@
 #include <optional>
 #include <functional>
 
+#include <components/misc/indexedvector.hpp>
+
 #include <osg/Quat>
 #include <osg/BoundingBox>
 #include <osg/ref_ptr>
@@ -55,7 +57,7 @@ namespace MWPhysics
     class PhysicsTaskScheduler;
     class Projectile;
 
-    using ActorMap = std::unordered_map<const MWWorld::LiveCellRefBase*, std::shared_ptr<Actor>>;
+    using ActorMap = Misc::IndexedVector<const MWWorld::LiveCellRefBase*, std::shared_ptr<Actor>>;
 
     struct ContactPoint
     {
@@ -300,7 +302,8 @@ namespace MWPhysics
             using ObjectMap = std::unordered_map<const MWWorld::LiveCellRefBase*, std::shared_ptr<Object>>;
             ObjectMap mObjects;
             
-            std::map<Object*, bool> mAnimatedObjects; // stores pointers to elements in mObjects
+            using AnimatedObjectMap = Misc::IndexedVector<Object*, bool>;
+            AnimatedObjectMap mAnimatedObjects; // stores pointers to elements in mObjects
 
             ActorMap mActors;
 

apps/openmw/mwworld/ptr.hpp

@@ -6,6 +6,7 @@
 #include <string>
 #include <string_view>
 #include <sstream>
+#include <functional>
 
 #include "livecellref.hpp"
 
@@ -161,6 +162,20 @@ namespace MWWorld
         ConstPtr(const LiveCellRefBase *liveCellRef=nullptr, const CellStoreType *cell=nullptr) : PtrBase(liveCellRef, cell, nullptr) {}
     };
 
+    struct PtrHash
+    {
+        size_t operator()(const Ptr& ptr) const noexcept
+        {
+            const void* p = ptr;
+            return std::hash<const void*>{}(p);
+        }
+        size_t operator()(const ConstPtr& ptr) const noexcept
+        {
+            const void* p = ptr;
+            return std::hash<const void*>{}(p);
+        }
+    };
+
 }
 
 #endif

apps/openmw_test_suite/CMakeLists.txt

@@ -30,6 +30,7 @@ if (GTEST_FOUND AND GMOCK_FOUND)
         misc/test_stringops.cpp
         misc/test_endianness.cpp
         misc/test_resourcehelpers.cpp
+        misc/test_indexedvector.cpp
         misc/progressreporter.cpp
         misc/compression.cpp
 

apps/openmw_test_suite/misc/test_indexedvector.cpp

@@ -0,0 +1,88 @@
+#include <components/misc/indexedvector.hpp>
+
+#include <gtest/gtest.h>
+#include <gmock/gmock.h>
+
+#include <cstdint>
+#include <vector>
+
+namespace
+{
+    TEST(IndexedVectorTests, basicInsert)
+    {
+        std::vector<std::pair<int, int>> vec;
+        Misc::IndexedVector<int, int> map;
+
+        for (int i = 0; i < 10000; i++)
+        {
+            vec.emplace_back(i, i << 13);
+            map.emplace(i, i << 13);
+        }
+
+        ASSERT_EQ(vec.size(), map.size());
+
+        auto itVec = vec.begin();
+        auto itMap = map.begin();
+        while (itMap != map.end() && itVec != vec.end())
+        {
+            ASSERT_EQ(itVec->first, itMap->first);
+            ASSERT_EQ(itVec->second, itMap->second);
+            itMap++;
+            itVec++;
+        }
+    }
+
+    TEST(IndexedVectorTests, duplicateInsert)
+    {
+        Misc::IndexedVector<int, int> map;
+
+        auto pairVal = map.emplace(1, 5);
+        ASSERT_EQ(map.size(), 1);
+        ASSERT_EQ(pairVal.first, map.begin());
+        ASSERT_EQ(pairVal.first->second, 5);
+        ASSERT_EQ(pairVal.second, true);
+
+        pairVal = map.emplace(1, 10);
+        ASSERT_EQ(map.size(), 1);
+        ASSERT_EQ(pairVal.first, map.begin());
+        ASSERT_EQ(pairVal.first->second, 5);
+        ASSERT_EQ(pairVal.second, false);
+    }
+
+    TEST(IndexedVectorTests, testErase)
+    {
+        Misc::IndexedVector<int, int> map;
+        for (int i = 0; i < 10000; i++)
+        {
+            map.emplace(i, i);
+        }
+
+        auto itA = map.find(100);
+        ASSERT_NE(itA, map.end());
+        ASSERT_EQ(itA->second, 100);
+
+        itA = map.erase(itA);
+        ASSERT_EQ(map.size(), 10000 - 1);
+        ASSERT_NE(itA, map.end());
+        ASSERT_EQ(itA->second, 101);
+    }
+
+    TEST(IndexedVectorTests, testLookup)
+    {
+        Misc::IndexedVector<int, int> map;
+        for (int i = 0; i < 10000; i++)
+        {
+            map.emplace(i, i);
+        }
+
+        auto itA = map.find(100);
+        ASSERT_NE(itA, map.end());
+        ASSERT_EQ(itA->second, 100);
+
+        map.erase(itA);
+        ASSERT_EQ(map.size(), 10000 - 1);
+        itA = map.find(101);
+        ASSERT_NE(itA, map.end());
+        ASSERT_EQ(itA->second, 101);
+    }
+}

components/CMakeLists.txt

@@ -189,7 +189,7 @@ add_component_dir (esm4
 
 add_component_dir (misc
     constants utf8stream stringops resourcehelpers rng messageformatparser weakcache thread
-    compression osguservalues errorMarker color
+    compression osguservalues errorMarker color indexedvector
     )
 
 add_component_dir (debug

components/misc/indexedvector.hpp

@@ -0,0 +1,186 @@
+#ifndef OPENMW_COMPONENTS_FLATMAP_HPP
+#define OPENMW_COMPONENTS_FLATMAP_HPP
+
+#include <memory>
+#include <list>
+#include <unordered_map>
+#include <algorithm>
+#include <utility>
+
+namespace Misc
+{
+    /// \class IndexedVector
+    /// Works like std::unordered_map but storage is std::vector and uses insertion order.
+    template <typename Key, typename T, typename THasher = std::hash<Key>>
+    class IndexedVector
+    {
+        using ElementType = std::pair<Key, T>;
+
+    private:
+        using Storage = std::vector<ElementType>;
+        using LookupTable = std::unordered_map<Key, typename Storage::difference_type, THasher>;
+
+    public:
+        using iterator = typename Storage::iterator;
+        using const_iterator = typename Storage::const_iterator;
+        using value_type = typename Storage::value_type;
+
+        void clear()
+        {
+            mData.clear();
+            mLookup.clear();
+        }
+
+        size_t size() const noexcept
+        {
+            return mData.size();
+        }
+
+        /// Inserts the element at the back with move semantics.
+        std::pair<iterator, bool> insert(const value_type& value)
+        {
+            auto it = find(value.first);
+            if (it != std::end(mData))
+            {
+                return { it, false };
+            }
+            else
+            {
+                const auto& key = value.first;
+                it = mData.emplace(std::end(mData), value);
+                mLookup.emplace(key, std::distance(std::begin(mData), it));
+                return { it, true };
+            }
+        }
+
+        std::pair<iterator, bool> insert(value_type&& value)
+        {
+            auto it = find(value.first);
+            if (it != std::end(mData))
+            {
+                return { it, false };
+            }
+            else
+            {
+                const auto& key = value.first;
+                it = mData.emplace(std::end(mData), std::move(value));
+                mLookup.emplace(key, std::distance(std::begin(mData), it));
+                return { it, true };
+            }
+        }
+
+        /// Inserts the element at the back
+        template<typename ...TArgs>
+        std::pair<iterator, bool> emplace(TArgs&& ...args)
+        {
+            value_type value{ std::forward<TArgs>(args)... };
+            auto it = find(value.first);
+            if (it != std::end(mData))
+            {
+                return { it, false };
+            }
+            else
+            {
+                const auto& key = value.first;
+                it = mData.emplace(std::end(mData), std::move(value));
+                mLookup.emplace(key, std::distance(std::begin(mData), it));
+                return { it, true };
+            }
+        }
+
+        /// Erases a single element, iterators are invalidated after this call.
+        /// The returned iterator points to the value after the removed element.
+        iterator erase(iterator it) noexcept
+        {
+            return eraseImpl(it);
+        }
+
+        iterator erase(const_iterator it) noexcept
+        {
+            return eraseImpl(it);
+        }
+
+        /// Erases a single element by key, iterators are invalidated after this call.
+        /// The returned iterator points to the value after the removed element.
+        iterator erase(const Key& key) noexcept
+        {
+            auto it = find(key);
+            if (it == end())
+                return it;
+            return erase(it);
+        }
+
+        template<class K>
+        iterator find(const K& key)
+        {
+            auto it = mLookup.find(key);
+            if (it == std::end(mLookup))
+                return end();
+            return std::begin(mData) + it->second;
+        }
+
+        template<class K>
+        const_iterator find(const K& key) const
+        {
+            auto it = mLookup.find(key);
+            if (it == std::end(mLookup))
+                return end();
+            return std::begin(mData) + it->second;
+        }
+
+        iterator begin()
+        {
+            return mData.begin();
+        }
+
+        const_iterator begin() const
+        {
+            return mData.begin();
+        }
+
+        const_iterator cbegin() const
+        {
+            return mData.cbegin();
+        }
+
+        iterator end()
+        {
+            return mData.end();
+        }
+
+        const_iterator end() const
+        {
+            return mData.end();
+        }
+
+        const_iterator cend() const
+        {
+            return mData.cend();
+        }
+
+    private:
+        template<typename TIterator>
+        iterator eraseImpl(TIterator&& it) noexcept
+        {
+            const auto lookupIt = mLookup.find(it->first);
+            if (lookupIt == std::end(mLookup))
+                return end();
+            const auto index = lookupIt->second;
+            mLookup.erase(lookupIt);
+            // Adjust indices by one.
+            for (auto& [_, idx] : mLookup)
+            {
+                if (idx > index)
+                    idx--;
+            }
+            return mData.erase(it);
+        }
+
+    private:
+        Storage mData;
+        LookupTable mLookup;
+    };
+
+}
+
+#endif