Use RecastMeshBuilder once to create RecastMesh

This allows to move all data out of the object instead of copying.

apps/openmw_test_suite/detournavigator/recastmeshbuilder.cpp

@@ -49,7 +49,7 @@ namespace
     TEST_F(DetourNavigatorRecastMeshBuilderTest, create_for_empty_should_return_empty)
     {
         RecastMeshBuilder builder(mSettings, mBounds);
-        const auto recastMesh = builder.create(mGeneration, mRevision);
+        const auto recastMesh = std::move(builder).create(mGeneration, mRevision);
         EXPECT_EQ(recastMesh->getVertices(), std::vector<float>());
         EXPECT_EQ(recastMesh->getIndices(), std::vector<int>());
         EXPECT_EQ(recastMesh->getAreaTypes(), std::vector<AreaType>());
@@ -63,7 +63,7 @@ namespace
 
         RecastMeshBuilder builder(mSettings, mBounds);
         builder.addObject(static_cast<const btCollisionShape&>(shape), btTransform::getIdentity(), AreaType_ground);
-        const auto recastMesh = builder.create(mGeneration, mRevision);
+        const auto recastMesh = std::move(builder).create(mGeneration, mRevision);
         EXPECT_EQ(recastMesh->getVertices(), std::vector<float>({
             1, 0, -1,
             -1, 0, 1,
@@ -84,7 +84,7 @@ namespace
             btTransform(btMatrix3x3::getIdentity().scaled(btVector3(1, 2, 3)), btVector3(1, 2, 3)),
             AreaType_ground
         );
-        const auto recastMesh = builder.create(mGeneration, mRevision);
+        const auto recastMesh = std::move(builder).create(mGeneration, mRevision);
         EXPECT_EQ(recastMesh->getVertices(), std::vector<float>({
             2, 3, 0,
             0, 3, 4,
@@ -100,7 +100,7 @@ namespace
         btHeightfieldTerrainShape shape(2, 2, heightfieldData.data(), 1, 0, 0, 2, PHY_FLOAT, false);
         RecastMeshBuilder builder(mSettings, mBounds);
         builder.addObject(static_cast<const btCollisionShape&>(shape), btTransform::getIdentity(), AreaType_ground);
-        const auto recastMesh = builder.create(mGeneration, mRevision);
+        const auto recastMesh = std::move(builder).create(mGeneration, mRevision);
         EXPECT_EQ(recastMesh->getVertices(), std::vector<float>({
             -0.5, 0, -0.5,
             -0.5, 0, 0.5,
@@ -116,7 +116,7 @@ namespace
         btBoxShape shape(btVector3(1, 1, 2));
         RecastMeshBuilder builder(mSettings, mBounds);
         builder.addObject(static_cast<const btCollisionShape&>(shape), btTransform::getIdentity(), AreaType_ground);
-        const auto recastMesh = builder.create(mGeneration, mRevision);
+        const auto recastMesh = std::move(builder).create(mGeneration, mRevision);
         EXPECT_EQ(recastMesh->getVertices(), std::vector<float>({
             1, 2, 1,
             -1, 2, 1,
@@ -163,7 +163,7 @@ namespace
             btTransform::getIdentity(),
             AreaType_ground
         );
-        const auto recastMesh = builder.create(mGeneration, mRevision);
+        const auto recastMesh = std::move(builder).create(mGeneration, mRevision);
         EXPECT_EQ(recastMesh->getVertices(), std::vector<float>({
             -1, -2, -1,
             -1, -2, 1,
@@ -210,7 +210,7 @@ namespace
             btTransform(btMatrix3x3::getIdentity().scaled(btVector3(1, 2, 3)), btVector3(1, 2, 3)),
             AreaType_ground
         );
-        const auto recastMesh = builder.create(mGeneration, mRevision);
+        const auto recastMesh = std::move(builder).create(mGeneration, mRevision);
         EXPECT_EQ(recastMesh->getVertices(), std::vector<float>({
             2, 3, 0,
             0, 3, 4,
@@ -234,7 +234,7 @@ namespace
             btTransform(btMatrix3x3::getIdentity().scaled(btVector3(1, 2, 3)), btVector3(1, 2, 3)),
             AreaType_ground
         );
-        const auto recastMesh = builder.create(mGeneration, mRevision);
+        const auto recastMesh = std::move(builder).create(mGeneration, mRevision);
         EXPECT_EQ(recastMesh->getVertices(), std::vector<float>({
             3, 12, 2,
             1, 12, 10,
@@ -256,7 +256,7 @@ namespace
             btTransform::getIdentity(),
             AreaType_ground
         );
-        const auto recastMesh = builder.create(mGeneration, mRevision);
+        const auto recastMesh = std::move(builder).create(mGeneration, mRevision);
         EXPECT_EQ(recastMesh->getVertices(), std::vector<float>({
             1, 0, -1,
             -1, 0, 1,
@@ -284,7 +284,7 @@ namespace
             btTransform::getIdentity(),
             AreaType_ground
         );
-        const auto recastMesh = builder.create(mGeneration, mRevision);
+        const auto recastMesh = std::move(builder).create(mGeneration, mRevision);
         EXPECT_EQ(recastMesh->getVertices(), std::vector<float>({
             -0.2f, 0, -0.3f,
             -0.3f, 0, -0.2f,
@@ -309,7 +309,7 @@ namespace
             static_cast<btScalar>(-osg::PI_4))),
             AreaType_ground
         );
-        const auto recastMesh = builder.create(mGeneration, mRevision);
+        const auto recastMesh = std::move(builder).create(mGeneration, mRevision);
         EXPECT_THAT(recastMesh->getVertices(), Pointwise(FloatNear(1e-5), std::vector<float>({
             0, -0.70710659027099609375, -3.535533905029296875,
             0, 0.707107067108154296875, -3.535533905029296875,
@@ -334,7 +334,7 @@ namespace
             static_cast<btScalar>(osg::PI_4))),
             AreaType_ground
         );
-        const auto recastMesh = builder.create(mGeneration, mRevision);
+        const auto recastMesh = std::move(builder).create(mGeneration, mRevision);
         EXPECT_THAT(recastMesh->getVertices(), Pointwise(FloatNear(1e-5), std::vector<float>({
             -3.535533905029296875, -0.70710659027099609375, 0,
             -3.535533905029296875, 0.707107067108154296875, 0,
@@ -359,7 +359,7 @@ namespace
             static_cast<btScalar>(osg::PI_4))),
             AreaType_ground
         );
-        const auto recastMesh = builder.create(mGeneration, mRevision);
+        const auto recastMesh = std::move(builder).create(mGeneration, mRevision);
         EXPECT_THAT(recastMesh->getVertices(), Pointwise(FloatNear(1e-5), std::vector<float>({
             1.41421353816986083984375, 0, 1.1920928955078125e-07,
             -1.41421353816986083984375, 0, -1.1920928955078125e-07,
@@ -388,7 +388,7 @@ namespace
             btTransform::getIdentity(),
             AreaType_null
         );
-        const auto recastMesh = builder.create(mGeneration, mRevision);
+        const auto recastMesh = std::move(builder).create(mGeneration, mRevision);
         EXPECT_EQ(recastMesh->getVertices(), std::vector<float>({
             1, 0, -1,
             -1, 0, 1,
@@ -405,7 +405,7 @@ namespace
     {
         RecastMeshBuilder builder(mSettings, mBounds);
         builder.addWater(1000, btTransform(btMatrix3x3::getIdentity(), btVector3(100, 200, 300)));
-        const auto recastMesh = builder.create(mGeneration, mRevision);
+        const auto recastMesh = std::move(builder).create(mGeneration, mRevision);
         EXPECT_EQ(recastMesh->getWater(), std::vector<RecastMesh::Water>({
             RecastMesh::Water {1000, btTransform(btMatrix3x3::getIdentity(), btVector3(100, 200, 300))}
         }));
@@ -420,7 +420,7 @@ namespace
 
         RecastMeshBuilder builder(mSettings, mBounds);
         builder.addObject(static_cast<const btCollisionShape&>(shape), btTransform::getIdentity(), AreaType_ground);
-        const auto recastMesh = builder.create(mGeneration, mRevision);
+        const auto recastMesh = std::move(builder).create(mGeneration, mRevision);
         EXPECT_EQ(recastMesh->getVertices(), std::vector<float>({
             -1, 0, -1,
             -1, 0, 1,

components/detournavigator/recastmeshbuilder.cpp

@@ -152,19 +152,11 @@ namespace DetourNavigator
         mWater.push_back(RecastMesh::Water {cellSize, transform});
     }
 
-    std::shared_ptr<RecastMesh> RecastMeshBuilder::create(std::size_t generation, std::size_t revision)
+    std::shared_ptr<RecastMesh> RecastMeshBuilder::create(std::size_t generation, std::size_t revision) &&
     {
         optimizeRecastMesh(mIndices, mVertices);
         std::sort(mWater.begin(), mWater.end());
-        return std::make_shared<RecastMesh>(generation, revision, mIndices, mVertices, mAreaTypes, mWater);
-    }
-
-    void RecastMeshBuilder::reset()
-    {
-        mIndices.clear();
-        mVertices.clear();
-        mAreaTypes.clear();
-        mWater.clear();
+        return std::make_shared<RecastMesh>(generation, revision, std::move(mIndices), std::move(mVertices), std::move(mAreaTypes), std::move(mWater));
     }
 
     void RecastMeshBuilder::addObject(const btConcaveShape& shape, const btTransform& transform,

components/detournavigator/recastmeshbuilder.hpp

@@ -34,9 +34,7 @@ namespace DetourNavigator
 
         void addWater(const int mCellSize, const btTransform& transform);
 
-        std::shared_ptr<RecastMesh> create(std::size_t generation, std::size_t revision);
-
-        void reset();
+        std::shared_ptr<RecastMesh> create(std::size_t generation, std::size_t revision) &&;
 
     private:
         std::reference_wrapper<const Settings> mSettings;

components/detournavigator/recastmeshmanager.cpp

@@ -1,10 +1,11 @@
 #include "recastmeshmanager.hpp"
+#include "recastmeshbuilder.hpp"
 
 namespace DetourNavigator
 {
     RecastMeshManager::RecastMeshManager(const Settings& settings, const TileBounds& bounds, std::size_t generation)
-        : mGeneration(generation)
-        , mMeshBuilder(settings, bounds)
+        : mSettings(settings)
+        , mGeneration(generation)
         , mTileBounds(bounds)
     {
     }
@@ -74,8 +75,15 @@ namespace DetourNavigator
 
     std::shared_ptr<RecastMesh> RecastMeshManager::getMesh()
     {
-        rebuild();
-        return mMeshBuilder.create(mGeneration, mRevision);
+        RecastMeshBuilder builder(mSettings, mTileBounds);
+        for (const auto& v : mWaterOrder)
+            builder.addWater(v.mCellSize, v.mTransform);
+        for (const auto& object : mObjectsOrder)
+        {
+            const RecastMeshObject& v = object.getImpl();
+            builder.addObject(v.getShape(), v.getTransform(), v.getAreaType());
+        }
+        return std::move(builder).create(mGeneration, mRevision);
     }
 
     bool RecastMeshManager::isEmpty() const
@@ -99,16 +107,4 @@ namespace DetourNavigator
     {
         return Version {mGeneration, mRevision};
     }
-
-    void RecastMeshManager::rebuild()
-    {
-        mMeshBuilder.reset();
-        for (const auto& v : mWaterOrder)
-            mMeshBuilder.addWater(v.mCellSize, v.mTransform);
-        for (const auto& object : mObjectsOrder)
-        {
-            const RecastMeshObject& v = object.getImpl();
-            mMeshBuilder.addObject(v.getShape(), v.getTransform(), v.getAreaType());
-        }
-    }
 }

components/detournavigator/recastmeshmanager.hpp

@@ -1,7 +1,6 @@
 #ifndef OPENMW_COMPONENTS_DETOURNAVIGATOR_RECASTMESHMANAGER_H
 #define OPENMW_COMPONENTS_DETOURNAVIGATOR_RECASTMESHMANAGER_H
 
-#include "recastmeshbuilder.hpp"
 #include "oscillatingrecastmeshobject.hpp"
 #include "objectid.hpp"
 #include "version.hpp"
@@ -13,11 +12,15 @@
 #include <list>
 #include <map>
 #include <optional>
+#include <memory>
 
 class btCollisionShape;
 
 namespace DetourNavigator
 {
+    struct Settings;
+    class RecastMesh;
+
     struct RemovedRecastMeshObject
     {
         std::reference_wrapper<const btCollisionShape> mShape;
@@ -61,9 +64,9 @@ namespace DetourNavigator
             Version mNavMeshVersion;
         };
 
+        const Settings& mSettings;
         std::size_t mRevision = 0;
         std::size_t mGeneration;
-        RecastMeshBuilder mMeshBuilder;
         TileBounds mTileBounds;
         std::list<OscillatingRecastMeshObject> mObjectsOrder;
         std::map<ObjectId, std::list<OscillatingRecastMeshObject>::iterator> mObjects;
@@ -71,8 +74,6 @@ namespace DetourNavigator
         std::map<osg::Vec2i, std::list<Water>::iterator> mWater;
         std::optional<Report> mLastNavMeshReportedChange;
         std::optional<Report> mLastNavMeshReport;
-
-        void rebuild();
     };
 }