From 5378cbead6dca1394abcb54784802587a3a86428 Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Thu, 25 May 2017 13:47:03 -0700 Subject: [PATCH 1/5] make ObjectMotionState dtor virtual --- libraries/physics/src/ObjectMotionState.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/physics/src/ObjectMotionState.h b/libraries/physics/src/ObjectMotionState.h index 1e582ea854..ccdc0e0041 100644 --- a/libraries/physics/src/ObjectMotionState.h +++ b/libraries/physics/src/ObjectMotionState.h @@ -79,7 +79,7 @@ public: static ShapeManager* getShapeManager(); ObjectMotionState(const btCollisionShape* shape); - ~ObjectMotionState(); + virtual ~ObjectMotionState(); virtual void handleEasyChanges(uint32_t& flags); virtual bool handleHardAndEasyChanges(uint32_t& flags, PhysicsEngine* engine); From 3bd0c111d724fceba4bc6d205b5fbf77875eb3c7 Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Thu, 25 May 2017 13:47:40 -0700 Subject: [PATCH 2/5] verify empty ShapeManager in Application dtor --- interface/src/Application.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp index 5e3e09e778..2fe8677e1e 100644 --- a/interface/src/Application.cpp +++ b/interface/src/Application.cpp @@ -1682,6 +1682,10 @@ Application::~Application() { _physicsEngine->setCharacterController(nullptr); + // the _shapeManager should have zero references + _shapeManager.collectGarbage(); + assert(_shapeManager.getNumShapes() == 0); + // shutdown render engine _main3DScene = nullptr; _renderEngine = nullptr; From c22e4ef883eb3120de2bb29db8ccf152cdbd0595 Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Thu, 25 May 2017 13:48:19 -0700 Subject: [PATCH 3/5] rename method to clarify where it is called --- libraries/physics/src/PhysicalEntitySimulation.cpp | 2 +- libraries/physics/src/PhysicsEngine.cpp | 2 +- libraries/physics/src/PhysicsEngine.h | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/libraries/physics/src/PhysicalEntitySimulation.cpp b/libraries/physics/src/PhysicalEntitySimulation.cpp index 2e69ff987c..fe507ed1ee 100644 --- a/libraries/physics/src/PhysicalEntitySimulation.cpp +++ b/libraries/physics/src/PhysicalEntitySimulation.cpp @@ -130,7 +130,7 @@ void PhysicalEntitySimulation::clearEntitiesInternal() { } // then remove the objects (aka MotionStates) from physics - _physicsEngine->removeObjects(_physicalObjects); + _physicsEngine->removeSetOfObjects(_physicalObjects); // delete the MotionStates // TODO: after we invert the entities/physics lib dependencies we will let EntityItem delete diff --git a/libraries/physics/src/PhysicsEngine.cpp b/libraries/physics/src/PhysicsEngine.cpp index 3a02e95e7c..1d61e53fbb 100644 --- a/libraries/physics/src/PhysicsEngine.cpp +++ b/libraries/physics/src/PhysicsEngine.cpp @@ -207,7 +207,7 @@ void PhysicsEngine::removeObjects(const VectorOfMotionStates& objects) { } // Same as above, but takes a Set instead of a Vector. Should only be called during teardown. -void PhysicsEngine::removeObjects(const SetOfMotionStates& objects) { +void PhysicsEngine::removeSetOfObjects(const SetOfMotionStates& objects) { _contactMap.clear(); for (auto object : objects) { btRigidBody* body = object->getRigidBody(); diff --git a/libraries/physics/src/PhysicsEngine.h b/libraries/physics/src/PhysicsEngine.h index e9b29a43a4..c029d14a42 100644 --- a/libraries/physics/src/PhysicsEngine.h +++ b/libraries/physics/src/PhysicsEngine.h @@ -53,7 +53,7 @@ public: uint32_t getNumSubsteps(); void removeObjects(const VectorOfMotionStates& objects); - void removeObjects(const SetOfMotionStates& objects); // only called during teardown + void removeSetOfObjects(const SetOfMotionStates& objects); // only called during teardown void addObjects(const VectorOfMotionStates& objects); VectorOfMotionStates changeObjects(const VectorOfMotionStates& objects); From 971855e6bf91ca863c38e057bb01bb0102018753 Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Thu, 25 May 2017 14:29:14 -0700 Subject: [PATCH 4/5] add asserts to catch dangling pointers sooner --- libraries/physics/src/EntityMotionState.cpp | 2 ++ libraries/physics/src/ObjectMotionState.cpp | 2 ++ 2 files changed, 4 insertions(+) diff --git a/libraries/physics/src/EntityMotionState.cpp b/libraries/physics/src/EntityMotionState.cpp index fdd290bfca..adb7bbddde 100644 --- a/libraries/physics/src/EntityMotionState.cpp +++ b/libraries/physics/src/EntityMotionState.cpp @@ -114,6 +114,7 @@ void EntityMotionState::handleDeactivation() { // virtual void EntityMotionState::handleEasyChanges(uint32_t& flags) { + assert(_entity); assert(entityTreeIsLocked()); updateServerPhysicsVariables(); ObjectMotionState::handleEasyChanges(flags); @@ -170,6 +171,7 @@ void EntityMotionState::handleEasyChanges(uint32_t& flags) { // virtual bool EntityMotionState::handleHardAndEasyChanges(uint32_t& flags, PhysicsEngine* engine) { + assert(_entity); updateServerPhysicsVariables(); return ObjectMotionState::handleHardAndEasyChanges(flags, engine); } diff --git a/libraries/physics/src/ObjectMotionState.cpp b/libraries/physics/src/ObjectMotionState.cpp index a877887840..b11e21366e 100644 --- a/libraries/physics/src/ObjectMotionState.cpp +++ b/libraries/physics/src/ObjectMotionState.cpp @@ -202,6 +202,7 @@ void ObjectMotionState::setShape(const btCollisionShape* shape) { } void ObjectMotionState::handleEasyChanges(uint32_t& flags) { + assert(_body && _shape); if (flags & Simulation::DIRTY_POSITION) { btTransform worldTrans = _body->getWorldTransform(); btVector3 newPosition = glmToBullet(getObjectPosition()); @@ -282,6 +283,7 @@ void ObjectMotionState::handleEasyChanges(uint32_t& flags) { } bool ObjectMotionState::handleHardAndEasyChanges(uint32_t& flags, PhysicsEngine* engine) { + assert(_body && _shape); if (flags & Simulation::DIRTY_SHAPE) { // make sure the new shape is valid if (!isReadyToComputeShape()) { From fa003b0c14bddfef711f175dcd629525ce3b2f29 Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Fri, 26 May 2017 12:29:37 -0700 Subject: [PATCH 5/5] swap order of changes and deactvations --- interface/src/Application.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp index 2fe8677e1e..71efbd52f3 100644 --- a/interface/src/Application.cpp +++ b/interface/src/Application.cpp @@ -4485,12 +4485,13 @@ void Application::update(float deltaTime) { getEntities()->getTree()->withWriteLock([&] { PerformanceTimer perfTimer("handleOutgoingChanges"); - const VectorOfMotionStates& deactivations = _physicsEngine->getDeactivatedMotionStates(); - _entitySimulation->handleDeactivatedMotionStates(deactivations); const VectorOfMotionStates& outgoingChanges = _physicsEngine->getChangedMotionStates(); _entitySimulation->handleChangedMotionStates(outgoingChanges); avatarManager->handleChangedMotionStates(outgoingChanges); + + const VectorOfMotionStates& deactivations = _physicsEngine->getDeactivatedMotionStates(); + _entitySimulation->handleDeactivatedMotionStates(deactivations); }); if (!_aboutToQuit) {