From 29a772c33fcc3de36aaa1d0d9517ab1e38010a8b Mon Sep 17 00:00:00 2001 From: elsid Date: Thu, 28 Oct 2021 01:43:25 +0200 Subject: [PATCH 01/10] Rename Resource::BulletShape::CollisionBox fields according to styleguide --- apps/openmw/mwphysics/actor.cpp | 2 +- apps/openmw/mwphysics/physicssystem.cpp | 4 +-- .../nifloader/testbulletnifloader.cpp | 36 +++++++++---------- components/nifbullet/bulletnifloader.cpp | 8 ++--- components/resource/bulletshape.hpp | 4 +-- components/resource/bulletshapemanager.cpp | 8 ++--- 6 files changed, 31 insertions(+), 31 deletions(-) diff --git a/apps/openmw/mwphysics/actor.cpp b/apps/openmw/mwphysics/actor.cpp index 4857339228..9d02d310b0 100644 --- a/apps/openmw/mwphysics/actor.cpp +++ b/apps/openmw/mwphysics/actor.cpp @@ -22,7 +22,7 @@ namespace MWPhysics Actor::Actor(const MWWorld::Ptr& ptr, const Resource::BulletShape* shape, PhysicsTaskScheduler* scheduler, bool canWaterWalk) : mStandingOnPtr(nullptr), mCanWaterWalk(canWaterWalk), mWalkingOnWater(false) - , mMeshTranslation(shape->mCollisionBox.center), mOriginalHalfExtents(shape->mCollisionBox.extents) + , mMeshTranslation(shape->mCollisionBox.mCenter), mOriginalHalfExtents(shape->mCollisionBox.mExtents) , mStuckFrames(0), mLastStuckPosition{0, 0, 0} , mForce(0.f, 0.f, 0.f), mOnGround(true), mOnSlope(false) , mInternalCollisionMode(true) diff --git a/apps/openmw/mwphysics/physicssystem.cpp b/apps/openmw/mwphysics/physicssystem.cpp index 5b00677fce..636c3492f7 100644 --- a/apps/openmw/mwphysics/physicssystem.cpp +++ b/apps/openmw/mwphysics/physicssystem.cpp @@ -618,7 +618,7 @@ namespace MWPhysics osg::ref_ptr shape = mShapeManager->getShape(mesh); // Try to get shape from basic model as fallback for creatures - if (!ptr.getClass().isNpc() && shape && shape->mCollisionBox.extents.length2() == 0) + if (!ptr.getClass().isNpc() && shape && shape->mCollisionBox.mExtents.length2() == 0) { const std::string fallbackModel = ptr.getClass().getModel(ptr); if (fallbackModel != mesh) @@ -643,7 +643,7 @@ namespace MWPhysics { osg::ref_ptr shapeInstance = mShapeManager->getInstance(mesh); assert(shapeInstance); - float radius = computeRadius ? shapeInstance->mCollisionBox.extents.length() / 2.f : 1.f; + float radius = computeRadius ? shapeInstance->mCollisionBox.mExtents.length() / 2.f : 1.f; mProjectileId++; diff --git a/apps/openmw_test_suite/nifloader/testbulletnifloader.cpp b/apps/openmw_test_suite/nifloader/testbulletnifloader.cpp index a1f5d6091d..d0a5b3d2e5 100644 --- a/apps/openmw_test_suite/nifloader/testbulletnifloader.cpp +++ b/apps/openmw_test_suite/nifloader/testbulletnifloader.cpp @@ -162,8 +162,8 @@ namespace Resource { return compareObjects(lhs.mCollisionShape, rhs.mCollisionShape) && compareObjects(lhs.mAvoidCollisionShape, rhs.mAvoidCollisionShape) - && lhs.mCollisionBox.extents == rhs.mCollisionBox.extents - && lhs.mCollisionBox.center == rhs.mCollisionBox.center + && lhs.mCollisionBox.mExtents == rhs.mCollisionBox.mExtents + && lhs.mCollisionBox.mCenter == rhs.mCollisionBox.mCenter && lhs.mAnimatedShapes == rhs.mAnimatedShapes; } @@ -172,8 +172,8 @@ namespace Resource return stream << "Resource::BulletShape {" << value.mCollisionShape << ", " << value.mAvoidCollisionShape << ", " - << "osg::Vec3f {" << value.mCollisionBox.extents << "}" << ", " - << "osg::Vec3f {" << value.mCollisionBox.center << "}" << ", " + << "osg::Vec3f {" << value.mCollisionBox.mExtents << "}" << ", " + << "osg::Vec3f {" << value.mCollisionBox.mCenter << "}" << ", " << value.mAnimatedShapes << "}"; } @@ -441,8 +441,8 @@ namespace const auto result = mLoader.load(mNifFile); Resource::BulletShape expected; - expected.mCollisionBox.extents = osg::Vec3f(1, 2, 3); - expected.mCollisionBox.center = osg::Vec3f(-1, -2, -3); + expected.mCollisionBox.mExtents = osg::Vec3f(1, 2, 3); + expected.mCollisionBox.mCenter = osg::Vec3f(-1, -2, -3); std::unique_ptr box(new btBoxShape(btVector3(1, 2, 3))); std::unique_ptr shape(new btCompoundShape); shape->addChildShape(btTransform(btMatrix3x3::getIdentity(), btVector3(-1, -2, -3)), box.release()); @@ -466,8 +466,8 @@ namespace const auto result = mLoader.load(mNifFile); Resource::BulletShape expected; - expected.mCollisionBox.extents = osg::Vec3f(1, 2, 3); - expected.mCollisionBox.center = osg::Vec3f(-1, -2, -3); + expected.mCollisionBox.mExtents = osg::Vec3f(1, 2, 3); + expected.mCollisionBox.mCenter = osg::Vec3f(-1, -2, -3); std::unique_ptr box(new btBoxShape(btVector3(1, 2, 3))); std::unique_ptr shape(new btCompoundShape); shape->addChildShape(btTransform(btMatrix3x3::getIdentity(), btVector3(-1, -2, -3)), box.release()); @@ -496,8 +496,8 @@ namespace const auto result = mLoader.load(mNifFile); Resource::BulletShape expected; - expected.mCollisionBox.extents = osg::Vec3f(1, 2, 3); - expected.mCollisionBox.center = osg::Vec3f(-1, -2, -3); + expected.mCollisionBox.mExtents = osg::Vec3f(1, 2, 3); + expected.mCollisionBox.mCenter = osg::Vec3f(-1, -2, -3); std::unique_ptr box(new btBoxShape(btVector3(1, 2, 3))); std::unique_ptr shape(new btCompoundShape); shape->addChildShape(btTransform(btMatrix3x3::getIdentity(), btVector3(-1, -2, -3)), box.release()); @@ -531,8 +531,8 @@ namespace const auto result = mLoader.load(mNifFile); Resource::BulletShape expected; - expected.mCollisionBox.extents = osg::Vec3f(1, 2, 3); - expected.mCollisionBox.center = osg::Vec3f(-1, -2, -3); + expected.mCollisionBox.mExtents = osg::Vec3f(1, 2, 3); + expected.mCollisionBox.mCenter = osg::Vec3f(-1, -2, -3); std::unique_ptr box(new btBoxShape(btVector3(1, 2, 3))); std::unique_ptr shape(new btCompoundShape); shape->addChildShape(btTransform(btMatrix3x3::getIdentity(), btVector3(-1, -2, -3)), box.release()); @@ -566,8 +566,8 @@ namespace const auto result = mLoader.load(mNifFile); Resource::BulletShape expected; - expected.mCollisionBox.extents = osg::Vec3f(4, 5, 6); - expected.mCollisionBox.center = osg::Vec3f(-4, -5, -6); + expected.mCollisionBox.mExtents = osg::Vec3f(4, 5, 6); + expected.mCollisionBox.mCenter = osg::Vec3f(-4, -5, -6); std::unique_ptr box(new btBoxShape(btVector3(4, 5, 6))); std::unique_ptr shape(new btCompoundShape); shape->addChildShape(btTransform(btMatrix3x3::getIdentity(), btVector3(-4, -5, -6)), box.release()); @@ -589,8 +589,8 @@ namespace const auto result = mLoader.load(mNifFile); Resource::BulletShape expected; - expected.mCollisionBox.extents = osg::Vec3f(1, 2, 3); - expected.mCollisionBox.center = osg::Vec3f(-1, -2, -3); + expected.mCollisionBox.mExtents = osg::Vec3f(1, 2, 3); + expected.mCollisionBox.mCenter = osg::Vec3f(-1, -2, -3); EXPECT_EQ(*result, expected); } @@ -623,8 +623,8 @@ namespace const auto result = mLoader.load(mNifFile); Resource::BulletShape expected; - expected.mCollisionBox.extents = osg::Vec3f(1, 2, 3); - expected.mCollisionBox.center = osg::Vec3f(-1, -2, -3); + expected.mCollisionBox.mExtents = osg::Vec3f(1, 2, 3); + expected.mCollisionBox.mCenter = osg::Vec3f(-1, -2, -3); EXPECT_EQ(*result, expected); } diff --git a/components/nifbullet/bulletnifloader.cpp b/components/nifbullet/bulletnifloader.cpp index 6ae1759395..03ef63014b 100644 --- a/components/nifbullet/bulletnifloader.cpp +++ b/components/nifbullet/bulletnifloader.cpp @@ -143,8 +143,8 @@ osg::ref_ptr BulletNifLoader::load(const Nif::File& nif) { if (findBoundingBox(node, filename)) { - const btVector3 extents = Misc::Convert::toBullet(mShape->mCollisionBox.extents); - const btVector3 center = Misc::Convert::toBullet(mShape->mCollisionBox.center); + const btVector3 extents = Misc::Convert::toBullet(mShape->mCollisionBox.mExtents); + const btVector3 center = Misc::Convert::toBullet(mShape->mCollisionBox.mCenter); std::unique_ptr compound (new btCompoundShape); std::unique_ptr boxShape(new btBoxShape(extents)); btTransform transform = btTransform::getIdentity(); @@ -206,8 +206,8 @@ bool BulletNifLoader::findBoundingBox(const Nif::Node* node, const std::string& switch (type) { case Nif::NiBoundingVolume::Type::BOX_BV: - mShape->mCollisionBox.extents = node->bounds.box.extents; - mShape->mCollisionBox.center = node->bounds.box.center; + mShape->mCollisionBox.mExtents = node->bounds.box.extents; + mShape->mCollisionBox.mCenter = node->bounds.box.center; break; default: { diff --git a/components/resource/bulletshape.hpp b/components/resource/bulletshape.hpp index 6ac8064cb3..b40ab6b6a7 100644 --- a/components/resource/bulletshape.hpp +++ b/components/resource/bulletshape.hpp @@ -29,8 +29,8 @@ namespace Resource struct CollisionBox { - osg::Vec3f extents; - osg::Vec3f center; + osg::Vec3f mExtents; + osg::Vec3f mCenter; }; // Used for actors and projectiles. mCollisionShape is used for actors only when we need to autogenerate collision box for creatures. // For now, use one file <-> one resource for simplicity. diff --git a/components/resource/bulletshapemanager.cpp b/components/resource/bulletshapemanager.cpp index 5b6dce067c..d295265b5f 100644 --- a/components/resource/bulletshapemanager.cpp +++ b/components/resource/bulletshapemanager.cpp @@ -86,10 +86,10 @@ public: btBvhTriangleMeshShape* triangleMeshShape = new TriangleMeshShape(mTriangleMesh.release(), true); btVector3 aabbMin = triangleMeshShape->getLocalAabbMin(); btVector3 aabbMax = triangleMeshShape->getLocalAabbMax(); - shape->mCollisionBox.extents[0] = (aabbMax[0] - aabbMin[0]) / 2.0f; - shape->mCollisionBox.extents[1] = (aabbMax[1] - aabbMin[1]) / 2.0f; - shape->mCollisionBox.extents[2] = (aabbMax[2] - aabbMin[2]) / 2.0f; - shape->mCollisionBox.center = osg::Vec3f( (aabbMax[0] + aabbMin[0]) / 2.0f, + shape->mCollisionBox.mExtents[0] = (aabbMax[0] - aabbMin[0]) / 2.0f; + shape->mCollisionBox.mExtents[1] = (aabbMax[1] - aabbMin[1]) / 2.0f; + shape->mCollisionBox.mExtents[2] = (aabbMax[2] - aabbMin[2]) / 2.0f; + shape->mCollisionBox.mCenter = osg::Vec3f( (aabbMax[0] + aabbMin[0]) / 2.0f, (aabbMax[1] + aabbMin[1]) / 2.0f, (aabbMax[2] + aabbMin[2]) / 2.0f ); shape->mCollisionShape = triangleMeshShape; From ca8584f6f61ca277272000fad724506e55d7a45c Mon Sep 17 00:00:00 2001 From: elsid Date: Sat, 30 Oct 2021 02:58:40 +0200 Subject: [PATCH 02/10] Move functions independent from BulletShape into anonymous namespace --- components/resource/bulletshape.cpp | 112 +++++++++++++--------------- components/resource/bulletshape.hpp | 6 -- 2 files changed, 53 insertions(+), 65 deletions(-) diff --git a/components/resource/bulletshape.cpp b/components/resource/bulletshape.cpp index 798a6778e6..5e0415e59e 100644 --- a/components/resource/bulletshape.cpp +++ b/components/resource/bulletshape.cpp @@ -10,6 +10,53 @@ namespace Resource { +namespace +{ + btCollisionShape* duplicateCollisionShape(const btCollisionShape *shape) + { + if (shape == nullptr) + return nullptr; + + if (shape->isCompound()) + { + const btCompoundShape *comp = static_cast(shape); + btCompoundShape* newShape = new btCompoundShape; + + for (int i = 0, n = comp->getNumChildShapes(); i < n; ++i) + { + btCollisionShape* child = duplicateCollisionShape(comp->getChildShape(i)); + const btTransform& trans = comp->getChildTransform(i); + newShape->addChildShape(trans, child); + } + + return newShape; + } + + if (const btBvhTriangleMeshShape* trishape = dynamic_cast(shape)) + return new btScaledBvhTriangleMeshShape(const_cast(trishape), btVector3(1.f, 1.f, 1.f)); + + if (const btBoxShape* boxshape = dynamic_cast(shape)) + return new btBoxShape(*boxshape); + + if (shape->getShapeType() == TERRAIN_SHAPE_PROXYTYPE) + return new btHeightfieldTerrainShape(static_cast(*shape)); + + throw std::logic_error(std::string("Unhandled Bullet shape duplication: ") + shape->getName()); + } + + void deleteShape(btCollisionShape* shape) + { + if (shape->isCompound()) + { + btCompoundShape* compound = static_cast(shape); + for (int i = 0, n = compound->getNumChildShapes(); i < n; i++) + if (btCollisionShape* child = compound->getChildShape(i)) + deleteShape(child); + } + + delete shape; + } +} BulletShape::BulletShape() : mCollisionShape(nullptr) @@ -28,58 +75,10 @@ BulletShape::BulletShape(const BulletShape ©, const osg::CopyOp ©op) BulletShape::~BulletShape() { - deleteShape(mAvoidCollisionShape); - deleteShape(mCollisionShape); -} - -void BulletShape::deleteShape(btCollisionShape* shape) -{ - if(shape!=nullptr) - { - if(shape->isCompound()) - { - btCompoundShape* ms = static_cast(shape); - int a = ms->getNumChildShapes(); - for(int i=0; i getChildShape(i)); - } - delete shape; - } -} - -btCollisionShape* BulletShape::duplicateCollisionShape(const btCollisionShape *shape) const -{ - if(shape->isCompound()) - { - const btCompoundShape *comp = static_cast(shape); - btCompoundShape *newShape = new btCompoundShape; - - int numShapes = comp->getNumChildShapes(); - for(int i = 0;i < numShapes;++i) - { - btCollisionShape *child = duplicateCollisionShape(comp->getChildShape(i)); - const btTransform& trans = comp->getChildTransform(i); - newShape->addChildShape(trans, child); - } - - return newShape; - } - - if(const btBvhTriangleMeshShape* trishape = dynamic_cast(shape)) - { - btScaledBvhTriangleMeshShape* newShape = new btScaledBvhTriangleMeshShape(const_cast(trishape), btVector3(1.f, 1.f, 1.f)); - return newShape; - } - - if (const btBoxShape* boxshape = dynamic_cast(shape)) - { - return new btBoxShape(*boxshape); - } - - if (shape->getShapeType() == TERRAIN_SHAPE_PROXYTYPE) - return new btHeightfieldTerrainShape(static_cast(*shape)); - - throw std::logic_error(std::string("Unhandled Bullet shape duplication: ")+shape->getName()); + if (mAvoidCollisionShape != nullptr) + deleteShape(mAvoidCollisionShape); + if (mCollisionShape != nullptr) + deleteShape(mCollisionShape); } btCollisionShape *BulletShape::getCollisionShape() const @@ -115,14 +114,9 @@ BulletShapeInstance::BulletShapeInstance(osg::ref_ptr source) , mSource(source) { mCollisionBox = source->mCollisionBox; - mAnimatedShapes = source->mAnimatedShapes; - - if (source->mCollisionShape) - mCollisionShape = duplicateCollisionShape(source->mCollisionShape); - - if (source->mAvoidCollisionShape) - mAvoidCollisionShape = duplicateCollisionShape(source->mAvoidCollisionShape); + mCollisionShape = duplicateCollisionShape(source->mCollisionShape); + mAvoidCollisionShape = duplicateCollisionShape(source->mAvoidCollisionShape); } } diff --git a/components/resource/bulletshape.hpp b/components/resource/bulletshape.hpp index b40ab6b6a7..8b30464fe2 100644 --- a/components/resource/bulletshape.hpp +++ b/components/resource/bulletshape.hpp @@ -44,8 +44,6 @@ namespace Resource osg::ref_ptr makeInstance() const; - btCollisionShape* duplicateCollisionShape(const btCollisionShape* shape) const; - btCollisionShape* getCollisionShape() const; btCollisionShape* getAvoidCollisionShape() const; @@ -53,10 +51,6 @@ namespace Resource void setLocalScaling(const btVector3& scale); bool isAnimated() const; - - private: - - void deleteShape(btCollisionShape* shape); }; From 80e3623d9a3c77d6896e9c5787a49ad45256d450 Mon Sep 17 00:00:00 2001 From: elsid Date: Sat, 30 Oct 2021 03:04:54 +0200 Subject: [PATCH 03/10] Avoid dynamic cast in duplicateCollisionShape --- components/resource/bulletshape.cpp | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/components/resource/bulletshape.cpp b/components/resource/bulletshape.cpp index 5e0415e59e..3f599e5386 100644 --- a/components/resource/bulletshape.cpp +++ b/components/resource/bulletshape.cpp @@ -32,11 +32,17 @@ namespace return newShape; } - if (const btBvhTriangleMeshShape* trishape = dynamic_cast(shape)) + if (shape->getShapeType() == TRIANGLE_MESH_SHAPE_PROXYTYPE) + { + const btBvhTriangleMeshShape* trishape = static_cast(shape); return new btScaledBvhTriangleMeshShape(const_cast(trishape), btVector3(1.f, 1.f, 1.f)); + } - if (const btBoxShape* boxshape = dynamic_cast(shape)) + if (shape->getShapeType() == BOX_SHAPE_PROXYTYPE) + { + const btBoxShape* boxshape = static_cast(shape); return new btBoxShape(*boxshape); + } if (shape->getShapeType() == TERRAIN_SHAPE_PROXYTYPE) return new btHeightfieldTerrainShape(static_cast(*shape)); From b905dd17c3e27f9bee9143a27fafcfdf84e6902b Mon Sep 17 00:00:00 2001 From: elsid Date: Sat, 30 Oct 2021 03:06:22 +0200 Subject: [PATCH 04/10] Use unique_ptr to store btCollisionShape in BulletShape --- .../detournavigator/navigator.cpp | 6 +-- .../nifloader/testbulletnifloader.cpp | 42 +++++++++--------- components/nifbullet/bulletnifloader.cpp | 8 ++-- components/resource/bulletshape.cpp | 43 +++++++------------ components/resource/bulletshape.hpp | 14 ++++-- components/resource/bulletshapemanager.cpp | 2 +- 6 files changed, 55 insertions(+), 60 deletions(-) diff --git a/apps/openmw_test_suite/detournavigator/navigator.cpp b/apps/openmw_test_suite/detournavigator/navigator.cpp index 62d3f9de04..6579076778 100644 --- a/apps/openmw_test_suite/detournavigator/navigator.cpp +++ b/apps/openmw_test_suite/detournavigator/navigator.cpp @@ -117,7 +117,7 @@ namespace osg::ref_ptr makeBulletShapeInstance(std::unique_ptr&& shape) { osg::ref_ptr bulletShape(new Resource::BulletShape); - bulletShape->mCollisionShape = std::move(shape).release(); + bulletShape->mCollisionShape.reset(std::move(shape).release()); return new Resource::BulletShapeInstance(bulletShape); } @@ -466,7 +466,7 @@ namespace }}; std::unique_ptr shapePtr = makeSquareHeightfieldTerrainShape(heightfieldData); shapePtr->setLocalScaling(btVector3(128, 128, 1)); - bulletShape->mCollisionShape = shapePtr.release(); + bulletShape->mCollisionShape.reset(shapePtr.release()); std::array heightfieldDataAvoid {{ -25, -25, -25, -25, -25, @@ -477,7 +477,7 @@ namespace }}; std::unique_ptr shapeAvoidPtr = makeSquareHeightfieldTerrainShape(heightfieldDataAvoid); shapeAvoidPtr->setLocalScaling(btVector3(128, 128, 1)); - bulletShape->mAvoidCollisionShape = shapeAvoidPtr.release(); + bulletShape->mAvoidCollisionShape.reset(shapeAvoidPtr.release()); osg::ref_ptr instance(new Resource::BulletShapeInstance(bulletShape)); diff --git a/apps/openmw_test_suite/nifloader/testbulletnifloader.cpp b/apps/openmw_test_suite/nifloader/testbulletnifloader.cpp index d0a5b3d2e5..3d4628c267 100644 --- a/apps/openmw_test_suite/nifloader/testbulletnifloader.cpp +++ b/apps/openmw_test_suite/nifloader/testbulletnifloader.cpp @@ -160,8 +160,8 @@ namespace Resource { static bool operator ==(const Resource::BulletShape& lhs, const Resource::BulletShape& rhs) { - return compareObjects(lhs.mCollisionShape, rhs.mCollisionShape) - && compareObjects(lhs.mAvoidCollisionShape, rhs.mAvoidCollisionShape) + return compareObjects(lhs.mCollisionShape.get(), rhs.mCollisionShape.get()) + && compareObjects(lhs.mAvoidCollisionShape.get(), rhs.mAvoidCollisionShape.get()) && lhs.mCollisionBox.mExtents == rhs.mCollisionBox.mExtents && lhs.mCollisionBox.mCenter == rhs.mCollisionBox.mCenter && lhs.mAnimatedShapes == rhs.mAnimatedShapes; @@ -170,8 +170,8 @@ namespace Resource static std::ostream& operator <<(std::ostream& stream, const Resource::BulletShape& value) { return stream << "Resource::BulletShape {" - << value.mCollisionShape << ", " - << value.mAvoidCollisionShape << ", " + << value.mCollisionShape.get() << ", " + << value.mAvoidCollisionShape.get() << ", " << "osg::Vec3f {" << value.mCollisionBox.mExtents << "}" << ", " << "osg::Vec3f {" << value.mCollisionBox.mCenter << "}" << ", " << value.mAnimatedShapes @@ -446,7 +446,7 @@ namespace std::unique_ptr box(new btBoxShape(btVector3(1, 2, 3))); std::unique_ptr shape(new btCompoundShape); shape->addChildShape(btTransform(btMatrix3x3::getIdentity(), btVector3(-1, -2, -3)), box.release()); - expected.mCollisionShape = shape.release(); + expected.mCollisionShape.reset(shape.release()); EXPECT_EQ(*result, expected); } @@ -471,7 +471,7 @@ namespace std::unique_ptr box(new btBoxShape(btVector3(1, 2, 3))); std::unique_ptr shape(new btCompoundShape); shape->addChildShape(btTransform(btMatrix3x3::getIdentity(), btVector3(-1, -2, -3)), box.release()); - expected.mCollisionShape = shape.release(); + expected.mCollisionShape.reset(shape.release()); EXPECT_EQ(*result, expected); } @@ -501,7 +501,7 @@ namespace std::unique_ptr box(new btBoxShape(btVector3(1, 2, 3))); std::unique_ptr shape(new btCompoundShape); shape->addChildShape(btTransform(btMatrix3x3::getIdentity(), btVector3(-1, -2, -3)), box.release()); - expected.mCollisionShape = shape.release(); + expected.mCollisionShape.reset(shape.release()); EXPECT_EQ(*result, expected); } @@ -536,7 +536,7 @@ namespace std::unique_ptr box(new btBoxShape(btVector3(1, 2, 3))); std::unique_ptr shape(new btCompoundShape); shape->addChildShape(btTransform(btMatrix3x3::getIdentity(), btVector3(-1, -2, -3)), box.release()); - expected.mCollisionShape = shape.release(); + expected.mCollisionShape.reset(shape.release()); EXPECT_EQ(*result, expected); } @@ -571,7 +571,7 @@ namespace std::unique_ptr box(new btBoxShape(btVector3(4, 5, 6))); std::unique_ptr shape(new btCompoundShape); shape->addChildShape(btTransform(btMatrix3x3::getIdentity(), btVector3(-4, -5, -6)), box.release()); - expected.mCollisionShape = shape.release(); + expected.mCollisionShape.reset(shape.release()); EXPECT_EQ(*result, expected); } @@ -605,7 +605,7 @@ namespace std::unique_ptr triangles(new btTriangleMesh(false)); triangles->addTriangle(btVector3(0, 0, 0), btVector3(1, 0, 0), btVector3(1, 1, 0)); Resource::BulletShape expected; - expected.mCollisionShape = new Resource::TriangleMeshShape(triangles.release(), true); + expected.mCollisionShape.reset(new Resource::TriangleMeshShape(triangles.release(), true)); EXPECT_EQ(*result, expected); } @@ -641,7 +641,7 @@ namespace std::unique_ptr triangles(new btTriangleMesh(false)); triangles->addTriangle(btVector3(0, 0, 0), btVector3(1, 0, 0), btVector3(1, 1, 0)); Resource::BulletShape expected; - expected.mCollisionShape = new Resource::TriangleMeshShape(triangles.release(), true); + expected.mCollisionShape.reset(new Resource::TriangleMeshShape(triangles.release(), true)); EXPECT_EQ(*result, expected); } @@ -659,7 +659,7 @@ namespace std::unique_ptr triangles(new btTriangleMesh(false)); triangles->addTriangle(btVector3(0, 0, 0), btVector3(1, 0, 0), btVector3(1, 1, 0)); Resource::BulletShape expected; - expected.mCollisionShape = new Resource::TriangleMeshShape(triangles.release(), true); + expected.mCollisionShape.reset(new Resource::TriangleMeshShape(triangles.release(), true)); EXPECT_EQ(*result, expected); } @@ -680,7 +680,7 @@ namespace triangles->addTriangle(btVector3(0, 0, 1), btVector3(1, 0, 1), btVector3(1, 1, 1)); triangles->addTriangle(btVector3(0, 0, 0), btVector3(1, 0, 0), btVector3(1, 1, 0)); Resource::BulletShape expected; - expected.mCollisionShape = new Resource::TriangleMeshShape(triangles.release(), true); + expected.mCollisionShape.reset(new Resource::TriangleMeshShape(triangles.release(), true)); EXPECT_EQ(*result, expected); } @@ -698,7 +698,7 @@ namespace std::unique_ptr triangles(new btTriangleMesh(false)); triangles->addTriangle(btVector3(0, 0, 0), btVector3(1, 0, 0), btVector3(1, 1, 0)); Resource::BulletShape expected; - expected.mCollisionShape = new Resource::TriangleMeshShape(triangles.release(), true); + expected.mCollisionShape.reset(new Resource::TriangleMeshShape(triangles.release(), true)); EXPECT_EQ(*result, expected); } @@ -720,7 +720,7 @@ namespace std::unique_ptr shape(new btCompoundShape); shape->addChildShape(mResultTransform, mesh.release()); Resource::BulletShape expected; - expected.mCollisionShape = shape.release(); + expected.mCollisionShape.reset(shape.release()); expected.mAnimatedShapes = {{-1, 0}}; EXPECT_EQ(*result, expected); @@ -746,7 +746,7 @@ namespace std::unique_ptr shape(new btCompoundShape); shape->addChildShape(mResultTransform2, mesh.release()); Resource::BulletShape expected; - expected.mCollisionShape = shape.release(); + expected.mCollisionShape.reset(shape.release()); expected.mAnimatedShapes = {{-1, 0}}; EXPECT_EQ(*result, expected); @@ -784,7 +784,7 @@ namespace shape->addChildShape(mResultTransform, mesh.release()); shape->addChildShape(mResultTransform, mesh2.release()); Resource::BulletShape expected; - expected.mCollisionShape = shape.release(); + expected.mCollisionShape.reset(shape.release()); expected.mAnimatedShapes = {{-1, 0}}; EXPECT_EQ(*result, expected); @@ -813,7 +813,7 @@ namespace std::unique_ptr shape(new btCompoundShape); shape->addChildShape(mResultTransform2, mesh.release()); Resource::BulletShape expected; - expected.mCollisionShape = shape.release(); + expected.mCollisionShape.reset(shape.release()); expected.mAnimatedShapes = {{-1, 0}}; EXPECT_EQ(*result, expected); @@ -854,7 +854,7 @@ namespace shape->addChildShape(mResultTransform2, mesh2.release()); shape->addChildShape(btTransform::getIdentity(), mesh.release()); Resource::BulletShape expected; - expected.mCollisionShape = shape.release(); + expected.mCollisionShape.reset(shape.release()); expected.mAnimatedShapes = {{-1, 0}}; EXPECT_EQ(*result, expected); @@ -873,7 +873,7 @@ namespace std::unique_ptr triangles(new btTriangleMesh(false)); triangles->addTriangle(btVector3(0, 0, 0), btVector3(1, 0, 0), btVector3(1, 1, 0)); Resource::BulletShape expected; - expected.mAvoidCollisionShape = new Resource::TriangleMeshShape(triangles.release(), false); + expected.mAvoidCollisionShape.reset(new Resource::TriangleMeshShape(triangles.release(), false)); EXPECT_EQ(*result, expected); } @@ -979,7 +979,7 @@ namespace std::unique_ptr triangles(new btTriangleMesh(false)); triangles->addTriangle(btVector3(0, 0, 0), btVector3(1, 0, 0), btVector3(1, 1, 0)); Resource::BulletShape expected; - expected.mCollisionShape = new Resource::TriangleMeshShape(triangles.release(), true); + expected.mCollisionShape.reset(new Resource::TriangleMeshShape(triangles.release(), true)); EXPECT_EQ(*result, expected); } diff --git a/components/nifbullet/bulletnifloader.cpp b/components/nifbullet/bulletnifloader.cpp index 03ef63014b..685a80951a 100644 --- a/components/nifbullet/bulletnifloader.cpp +++ b/components/nifbullet/bulletnifloader.cpp @@ -152,7 +152,7 @@ osg::ref_ptr BulletNifLoader::load(const Nif::File& nif) compound->addChildShape(transform, boxShape.get()); boxShape.release(); - mShape->mCollisionShape = compound.release(); + mShape->mCollisionShape.reset(compound.release()); return mShape; } } @@ -179,17 +179,17 @@ osg::ref_ptr BulletNifLoader::load(const Nif::File& nif) child.release(); mStaticMesh.release(); } - mShape->mCollisionShape = mCompoundShape.release(); + mShape->mCollisionShape.reset(mCompoundShape.release()); } else if (mStaticMesh) { - mShape->mCollisionShape = new Resource::TriangleMeshShape(mStaticMesh.get(), true); + mShape->mCollisionShape.reset(new Resource::TriangleMeshShape(mStaticMesh.get(), true)); mStaticMesh.release(); } if (mAvoidStaticMesh) { - mShape->mAvoidCollisionShape = new Resource::TriangleMeshShape(mAvoidStaticMesh.get(), false); + mShape->mAvoidCollisionShape.reset(new Resource::TriangleMeshShape(mAvoidStaticMesh.get(), false)); mAvoidStaticMesh.release(); } diff --git a/components/resource/bulletshape.cpp b/components/resource/bulletshape.cpp index 3f599e5386..e52e68ca07 100644 --- a/components/resource/bulletshape.cpp +++ b/components/resource/bulletshape.cpp @@ -12,7 +12,7 @@ namespace Resource { namespace { - btCollisionShape* duplicateCollisionShape(const btCollisionShape *shape) + CollisionShapePtr duplicateCollisionShape(const btCollisionShape *shape) { if (shape == nullptr) return nullptr; @@ -20,13 +20,13 @@ namespace if (shape->isCompound()) { const btCompoundShape *comp = static_cast(shape); - btCompoundShape* newShape = new btCompoundShape; + std::unique_ptr newShape(new btCompoundShape); for (int i = 0, n = comp->getNumChildShapes(); i < n; ++i) { - btCollisionShape* child = duplicateCollisionShape(comp->getChildShape(i)); + auto child = duplicateCollisionShape(comp->getChildShape(i)); const btTransform& trans = comp->getChildTransform(i); - newShape->addChildShape(trans, child); + newShape->addChildShape(trans, child.release()); } return newShape; @@ -35,17 +35,17 @@ namespace if (shape->getShapeType() == TRIANGLE_MESH_SHAPE_PROXYTYPE) { const btBvhTriangleMeshShape* trishape = static_cast(shape); - return new btScaledBvhTriangleMeshShape(const_cast(trishape), btVector3(1.f, 1.f, 1.f)); + return CollisionShapePtr(new btScaledBvhTriangleMeshShape(const_cast(trishape), btVector3(1.f, 1.f, 1.f))); } if (shape->getShapeType() == BOX_SHAPE_PROXYTYPE) { const btBoxShape* boxshape = static_cast(shape); - return new btBoxShape(*boxshape); + return CollisionShapePtr(new btBoxShape(*boxshape)); } if (shape->getShapeType() == TERRAIN_SHAPE_PROXYTYPE) - return new btHeightfieldTerrainShape(static_cast(*shape)); + return CollisionShapePtr(new btHeightfieldTerrainShape(static_cast(*shape))); throw std::logic_error(std::string("Unhandled Bullet shape duplication: ") + shape->getName()); } @@ -64,37 +64,27 @@ namespace } } -BulletShape::BulletShape() - : mCollisionShape(nullptr) - , mAvoidCollisionShape(nullptr) +void DeleteCollisionShape::operator()(btCollisionShape* shape) const { - + deleteShape(shape); } BulletShape::BulletShape(const BulletShape ©, const osg::CopyOp ©op) - : mCollisionShape(duplicateCollisionShape(copy.mCollisionShape)) - , mAvoidCollisionShape(duplicateCollisionShape(copy.mAvoidCollisionShape)) + : mCollisionShape(duplicateCollisionShape(copy.mCollisionShape.get())) + , mAvoidCollisionShape(duplicateCollisionShape(copy.mAvoidCollisionShape.get())) , mCollisionBox(copy.mCollisionBox) , mAnimatedShapes(copy.mAnimatedShapes) { } -BulletShape::~BulletShape() -{ - if (mAvoidCollisionShape != nullptr) - deleteShape(mAvoidCollisionShape); - if (mCollisionShape != nullptr) - deleteShape(mCollisionShape); -} - btCollisionShape *BulletShape::getCollisionShape() const { - return mCollisionShape; + return mCollisionShape.get(); } btCollisionShape *BulletShape::getAvoidCollisionShape() const { - return mAvoidCollisionShape; + return mAvoidCollisionShape.get(); } void BulletShape::setLocalScaling(const btVector3& scale) @@ -116,13 +106,12 @@ osg::ref_ptr BulletShape::makeInstance() const } BulletShapeInstance::BulletShapeInstance(osg::ref_ptr source) - : BulletShape() - , mSource(source) + : mSource(source) { mCollisionBox = source->mCollisionBox; mAnimatedShapes = source->mAnimatedShapes; - mCollisionShape = duplicateCollisionShape(source->mCollisionShape); - mAvoidCollisionShape = duplicateCollisionShape(source->mAvoidCollisionShape); + mCollisionShape = duplicateCollisionShape(source->mCollisionShape.get()); + mAvoidCollisionShape = duplicateCollisionShape(source->mAvoidCollisionShape.get()); } } diff --git a/components/resource/bulletshape.hpp b/components/resource/bulletshape.hpp index 8b30464fe2..369aed18a0 100644 --- a/components/resource/bulletshape.hpp +++ b/components/resource/bulletshape.hpp @@ -2,6 +2,7 @@ #define OPENMW_COMPONENTS_RESOURCE_BULLETSHAPE_H #include +#include #include #include @@ -13,19 +14,24 @@ class btCollisionShape; namespace Resource { + struct DeleteCollisionShape + { + void operator()(btCollisionShape* shape) const; + }; + + using CollisionShapePtr = std::unique_ptr; class BulletShapeInstance; class BulletShape : public osg::Object { public: - BulletShape(); + BulletShape() = default; BulletShape(const BulletShape& copy, const osg::CopyOp& copyop); - virtual ~BulletShape(); META_Object(Resource, BulletShape) - btCollisionShape* mCollisionShape; - btCollisionShape* mAvoidCollisionShape; + CollisionShapePtr mCollisionShape; + CollisionShapePtr mAvoidCollisionShape; struct CollisionBox { diff --git a/components/resource/bulletshapemanager.cpp b/components/resource/bulletshapemanager.cpp index d295265b5f..cde069837a 100644 --- a/components/resource/bulletshapemanager.cpp +++ b/components/resource/bulletshapemanager.cpp @@ -92,7 +92,7 @@ public: shape->mCollisionBox.mCenter = osg::Vec3f( (aabbMax[0] + aabbMin[0]) / 2.0f, (aabbMax[1] + aabbMin[1]) / 2.0f, (aabbMax[2] + aabbMin[2]) / 2.0f ); - shape->mCollisionShape = triangleMeshShape; + shape->mCollisionShape.reset(triangleMeshShape); return shape; } From fc9a405dc5897f335c5317b36296cbffa2cf57b0 Mon Sep 17 00:00:00 2001 From: elsid Date: Sat, 30 Oct 2021 03:16:21 +0200 Subject: [PATCH 05/10] Make BulletShape::makeInstance free function --- components/resource/bulletshape.cpp | 5 ++--- components/resource/bulletshape.hpp | 5 ++--- components/resource/bulletshapemanager.cpp | 5 ++--- 3 files changed, 6 insertions(+), 9 deletions(-) diff --git a/components/resource/bulletshape.cpp b/components/resource/bulletshape.cpp index e52e68ca07..4b074ee146 100644 --- a/components/resource/bulletshape.cpp +++ b/components/resource/bulletshape.cpp @@ -99,10 +99,9 @@ bool BulletShape::isAnimated() const return !mAnimatedShapes.empty(); } -osg::ref_ptr BulletShape::makeInstance() const +osg::ref_ptr makeInstance(osg::ref_ptr source) { - osg::ref_ptr instance (new BulletShapeInstance(this)); - return instance; + return {new BulletShapeInstance(std::move(source))}; } BulletShapeInstance::BulletShapeInstance(osg::ref_ptr source) diff --git a/components/resource/bulletshape.hpp b/components/resource/bulletshape.hpp index 369aed18a0..bf249cb364 100644 --- a/components/resource/bulletshape.hpp +++ b/components/resource/bulletshape.hpp @@ -21,7 +21,6 @@ namespace Resource using CollisionShapePtr = std::unique_ptr; - class BulletShapeInstance; class BulletShape : public osg::Object { public: @@ -48,8 +47,6 @@ namespace Resource // we store the node's record index mapped to the child index of the shape in the btCompoundShape. std::map mAnimatedShapes; - osg::ref_ptr makeInstance() const; - btCollisionShape* getCollisionShape() const; btCollisionShape* getAvoidCollisionShape() const; @@ -71,6 +68,8 @@ namespace Resource osg::ref_ptr mSource; }; + osg::ref_ptr makeInstance(osg::ref_ptr source); + // Subclass btBhvTriangleMeshShape to auto-delete the meshInterface struct TriangleMeshShape : public btBvhTriangleMeshShape { diff --git a/components/resource/bulletshapemanager.cpp b/components/resource/bulletshapemanager.cpp index cde069837a..ae6f3659a5 100644 --- a/components/resource/bulletshapemanager.cpp +++ b/components/resource/bulletshapemanager.cpp @@ -193,9 +193,8 @@ osg::ref_ptr BulletShapeManager::createInstance(const std:: { osg::ref_ptr shape = getShape(name); if (shape) - return shape->makeInstance(); - else - return osg::ref_ptr(); + return makeInstance(std::move(shape)); + return osg::ref_ptr(); } void BulletShapeManager::updateCache(double referenceTime) From 8e71c246bfcae5c44907c4a06702f2403082b4da Mon Sep 17 00:00:00 2001 From: elsid Date: Sat, 30 Oct 2021 03:30:38 +0200 Subject: [PATCH 06/10] Remove redundant BulletShape getters --- apps/openmw/mwphysics/object.cpp | 6 +++--- apps/openmw/mwphysics/physicssystem.cpp | 2 +- apps/openmw/mwworld/scene.cpp | 2 +- apps/openmw/mwworld/worldimp.cpp | 2 +- apps/openmw_test_suite/detournavigator/navigator.cpp | 2 +- components/detournavigator/navigatorimpl.cpp | 8 ++++---- components/resource/bulletshape.cpp | 10 ---------- components/resource/bulletshape.hpp | 4 ---- 8 files changed, 11 insertions(+), 25 deletions(-) diff --git a/apps/openmw/mwphysics/object.cpp b/apps/openmw/mwphysics/object.cpp index 879c12124e..08fcc7e47d 100644 --- a/apps/openmw/mwphysics/object.cpp +++ b/apps/openmw/mwphysics/object.cpp @@ -24,7 +24,7 @@ namespace MWPhysics , mTaskScheduler(scheduler) { mPtr = ptr; - mCollisionObject = BulletHelpers::makeCollisionObject(mShapeInstance->getCollisionShape(), + mCollisionObject = BulletHelpers::makeCollisionObject(mShapeInstance->mCollisionShape.get(), Misc::Convert::toBullet(mPosition), Misc::Convert::toBullet(rotation)); mCollisionObject->setUserPointer(this); mShapeInstance->setLocalScaling(mScale); @@ -109,9 +109,9 @@ namespace MWPhysics if (mShapeInstance->mAnimatedShapes.empty()) return false; - assert (mShapeInstance->getCollisionShape()->isCompound()); + assert (mShapeInstance->mCollisionShape->isCompound()); - btCompoundShape* compound = static_cast(mShapeInstance->getCollisionShape()); + btCompoundShape* compound = static_cast(mShapeInstance->mCollisionShape.get()); for (const auto& [recIndex, shapeIndex] : mShapeInstance->mAnimatedShapes) { auto nodePathFound = mRecIndexToNodePath.find(recIndex); diff --git a/apps/openmw/mwphysics/physicssystem.cpp b/apps/openmw/mwphysics/physicssystem.cpp index 636c3492f7..9b5f41a5fb 100644 --- a/apps/openmw/mwphysics/physicssystem.cpp +++ b/apps/openmw/mwphysics/physicssystem.cpp @@ -481,7 +481,7 @@ namespace MWPhysics if (ptr.mRef->mData.mPhysicsPostponed) return; osg::ref_ptr shapeInstance = mShapeManager->getInstance(mesh); - if (!shapeInstance || !shapeInstance->getCollisionShape()) + if (!shapeInstance || !shapeInstance->mCollisionShape) return; assert(!getObject(ptr)); diff --git a/apps/openmw/mwworld/scene.cpp b/apps/openmw/mwworld/scene.cpp index d49e6c0e8b..f9e792c7d2 100644 --- a/apps/openmw/mwworld/scene.cpp +++ b/apps/openmw/mwworld/scene.cpp @@ -132,7 +132,7 @@ namespace { btVector3 aabbMin; btVector3 aabbMax; - object->getShapeInstance()->getCollisionShape()->getAabb(btTransform::getIdentity(), aabbMin, aabbMax); + object->getShapeInstance()->mCollisionShape->getAabb(btTransform::getIdentity(), aabbMin, aabbMax); const auto center = (aabbMax + aabbMin) * 0.5f; diff --git a/apps/openmw/mwworld/worldimp.cpp b/apps/openmw/mwworld/worldimp.cpp index 21e2d8380a..efb8b7f370 100644 --- a/apps/openmw/mwworld/worldimp.cpp +++ b/apps/openmw/mwworld/worldimp.cpp @@ -3948,7 +3948,7 @@ namespace MWWorld btVector3 aabbMin; btVector3 aabbMax; - object->getShapeInstance()->getCollisionShape()->getAabb(btTransform::getIdentity(), aabbMin, aabbMax); + object->getShapeInstance()->mCollisionShape->getAabb(btTransform::getIdentity(), aabbMin, aabbMax); const auto toLocal = object->getTransform().inverse(); const auto localFrom = toLocal(Misc::Convert::toBullet(position)); diff --git a/apps/openmw_test_suite/detournavigator/navigator.cpp b/apps/openmw_test_suite/detournavigator/navigator.cpp index 6579076778..d4bdcb13b8 100644 --- a/apps/openmw_test_suite/detournavigator/navigator.cpp +++ b/apps/openmw_test_suite/detournavigator/navigator.cpp @@ -482,7 +482,7 @@ namespace osg::ref_ptr instance(new Resource::BulletShapeInstance(bulletShape)); mNavigator->addAgent(mAgentHalfExtents); - mNavigator->addObject(ObjectId(instance->getCollisionShape()), ObjectShapes(instance), btTransform::getIdentity()); + mNavigator->addObject(ObjectId(instance->mCollisionShape.get()), ObjectShapes(instance), btTransform::getIdentity()); mNavigator->update(mPlayerPosition); mNavigator->wait(mListener, WaitConditionType::allJobsDone); diff --git a/components/detournavigator/navigatorimpl.cpp b/components/detournavigator/navigatorimpl.cpp index 9a59ecf6d7..44b42b22c2 100644 --- a/components/detournavigator/navigatorimpl.cpp +++ b/components/detournavigator/navigatorimpl.cpp @@ -34,9 +34,9 @@ namespace DetourNavigator bool NavigatorImpl::addObject(const ObjectId id, const ObjectShapes& shapes, const btTransform& transform) { - CollisionShape collisionShape {shapes.mShapeInstance, *shapes.mShapeInstance->getCollisionShape()}; + CollisionShape collisionShape {shapes.mShapeInstance, *shapes.mShapeInstance->mCollisionShape}; bool result = mNavMeshManager.addObject(id, collisionShape, transform, AreaType_ground); - if (const btCollisionShape* const avoidShape = shapes.mShapeInstance->getAvoidCollisionShape()) + if (const btCollisionShape* const avoidShape = shapes.mShapeInstance->mAvoidCollisionShape.get()) { const ObjectId avoidId(avoidShape); CollisionShape avoidCollisionShape {shapes.mShapeInstance, *avoidShape}; @@ -64,9 +64,9 @@ namespace DetourNavigator bool NavigatorImpl::updateObject(const ObjectId id, const ObjectShapes& shapes, const btTransform& transform) { - const CollisionShape collisionShape {shapes.mShapeInstance, *shapes.mShapeInstance->getCollisionShape()}; + const CollisionShape collisionShape {shapes.mShapeInstance, *shapes.mShapeInstance->mCollisionShape}; bool result = mNavMeshManager.updateObject(id, collisionShape, transform, AreaType_ground); - if (const btCollisionShape* const avoidShape = shapes.mShapeInstance->getAvoidCollisionShape()) + if (const btCollisionShape* const avoidShape = shapes.mShapeInstance->mAvoidCollisionShape.get()) { const ObjectId avoidId(avoidShape); const CollisionShape avoidCollisionShape {shapes.mShapeInstance, *avoidShape}; diff --git a/components/resource/bulletshape.cpp b/components/resource/bulletshape.cpp index 4b074ee146..d8315d08b6 100644 --- a/components/resource/bulletshape.cpp +++ b/components/resource/bulletshape.cpp @@ -77,16 +77,6 @@ BulletShape::BulletShape(const BulletShape ©, const osg::CopyOp ©op) { } -btCollisionShape *BulletShape::getCollisionShape() const -{ - return mCollisionShape.get(); -} - -btCollisionShape *BulletShape::getAvoidCollisionShape() const -{ - return mAvoidCollisionShape.get(); -} - void BulletShape::setLocalScaling(const btVector3& scale) { mCollisionShape->setLocalScaling(scale); diff --git a/components/resource/bulletshape.hpp b/components/resource/bulletshape.hpp index bf249cb364..1065f3893b 100644 --- a/components/resource/bulletshape.hpp +++ b/components/resource/bulletshape.hpp @@ -47,10 +47,6 @@ namespace Resource // we store the node's record index mapped to the child index of the shape in the btCompoundShape. std::map mAnimatedShapes; - btCollisionShape* getCollisionShape() const; - - btCollisionShape* getAvoidCollisionShape() const; - void setLocalScaling(const btVector3& scale); bool isAnimated() const; From ed5a4e195b1e4bb55c89444eaa5546d3dd5edc35 Mon Sep 17 00:00:00 2001 From: elsid Date: Sat, 30 Oct 2021 03:38:38 +0200 Subject: [PATCH 07/10] Use unique_ptr to avoid possible memory leak --- components/resource/bulletshapemanager.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/components/resource/bulletshapemanager.cpp b/components/resource/bulletshapemanager.cpp index ae6f3659a5..39ceb4fe7e 100644 --- a/components/resource/bulletshapemanager.cpp +++ b/components/resource/bulletshapemanager.cpp @@ -83,7 +83,8 @@ public: return osg::ref_ptr(); osg::ref_ptr shape (new BulletShape); - btBvhTriangleMeshShape* triangleMeshShape = new TriangleMeshShape(mTriangleMesh.release(), true); + + auto triangleMeshShape = std::make_unique(mTriangleMesh.release(), true); btVector3 aabbMin = triangleMeshShape->getLocalAabbMin(); btVector3 aabbMax = triangleMeshShape->getLocalAabbMax(); shape->mCollisionBox.mExtents[0] = (aabbMax[0] - aabbMin[0]) / 2.0f; @@ -92,7 +93,7 @@ public: shape->mCollisionBox.mCenter = osg::Vec3f( (aabbMax[0] + aabbMin[0]) / 2.0f, (aabbMax[1] + aabbMin[1]) / 2.0f, (aabbMax[2] + aabbMin[2]) / 2.0f ); - shape->mCollisionShape.reset(triangleMeshShape); + shape->mCollisionShape.reset(triangleMeshShape.release()); return shape; } From c83facd9d3d21a19319157edff209499062ec0fc Mon Sep 17 00:00:00 2001 From: elsid Date: Sat, 30 Oct 2021 03:39:27 +0200 Subject: [PATCH 08/10] Avoid redundant osg::ref_ptr copy --- components/resource/bulletshape.cpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/components/resource/bulletshape.cpp b/components/resource/bulletshape.cpp index d8315d08b6..1d4be1d14d 100644 --- a/components/resource/bulletshape.cpp +++ b/components/resource/bulletshape.cpp @@ -95,12 +95,12 @@ osg::ref_ptr makeInstance(osg::ref_ptr s } BulletShapeInstance::BulletShapeInstance(osg::ref_ptr source) - : mSource(source) + : mSource(std::move(source)) { - mCollisionBox = source->mCollisionBox; - mAnimatedShapes = source->mAnimatedShapes; - mCollisionShape = duplicateCollisionShape(source->mCollisionShape.get()); - mAvoidCollisionShape = duplicateCollisionShape(source->mAvoidCollisionShape.get()); + mCollisionBox = mSource->mCollisionBox; + mAnimatedShapes = mSource->mAnimatedShapes; + mCollisionShape = duplicateCollisionShape(mSource->mCollisionShape.get()); + mAvoidCollisionShape = duplicateCollisionShape(mSource->mAvoidCollisionShape.get()); } } From b731a981c4267929f2e0fdf055bb1a3e1f27c6a6 Mon Sep 17 00:00:00 2001 From: elsid Date: Sat, 30 Oct 2021 03:41:19 +0200 Subject: [PATCH 09/10] Make BulletShape::isAnimated inlined --- components/resource/bulletshape.cpp | 5 ----- components/resource/bulletshape.hpp | 2 +- 2 files changed, 1 insertion(+), 6 deletions(-) diff --git a/components/resource/bulletshape.cpp b/components/resource/bulletshape.cpp index 1d4be1d14d..52d639d272 100644 --- a/components/resource/bulletshape.cpp +++ b/components/resource/bulletshape.cpp @@ -84,11 +84,6 @@ void BulletShape::setLocalScaling(const btVector3& scale) mAvoidCollisionShape->setLocalScaling(scale); } -bool BulletShape::isAnimated() const -{ - return !mAnimatedShapes.empty(); -} - osg::ref_ptr makeInstance(osg::ref_ptr source) { return {new BulletShapeInstance(std::move(source))}; diff --git a/components/resource/bulletshape.hpp b/components/resource/bulletshape.hpp index 1065f3893b..7188165045 100644 --- a/components/resource/bulletshape.hpp +++ b/components/resource/bulletshape.hpp @@ -49,7 +49,7 @@ namespace Resource void setLocalScaling(const btVector3& scale); - bool isAnimated() const; + bool isAnimated() const { return !mAnimatedShapes.empty(); } }; From a851ac5fea0792f1ef6a4566d92afb16e9b971a9 Mon Sep 17 00:00:00 2001 From: elsid Date: Sat, 30 Oct 2021 03:46:29 +0200 Subject: [PATCH 10/10] Use custom deleter for btCompoundShape to delete children shapes --- components/nifbullet/bulletnifloader.cpp | 2 +- components/nifbullet/bulletnifloader.hpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/components/nifbullet/bulletnifloader.cpp b/components/nifbullet/bulletnifloader.cpp index 685a80951a..f808877a75 100644 --- a/components/nifbullet/bulletnifloader.cpp +++ b/components/nifbullet/bulletnifloader.cpp @@ -179,7 +179,7 @@ osg::ref_ptr BulletNifLoader::load(const Nif::File& nif) child.release(); mStaticMesh.release(); } - mShape->mCollisionShape.reset(mCompoundShape.release()); + mShape->mCollisionShape = std::move(mCompoundShape); } else if (mStaticMesh) { diff --git a/components/nifbullet/bulletnifloader.hpp b/components/nifbullet/bulletnifloader.hpp index 71c84566a0..3d6a95e09f 100644 --- a/components/nifbullet/bulletnifloader.hpp +++ b/components/nifbullet/bulletnifloader.hpp @@ -61,7 +61,7 @@ private: void handleNiTriShape(const Nif::Node *nifNode, int flags, const osg::Matrixf& transform, bool isAnimated, bool avoid); - std::unique_ptr mCompoundShape; + std::unique_ptr mCompoundShape; std::unique_ptr mStaticMesh;