From a0841c937cb79823c8ad536efc47f40ddef937be Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Wed, 1 May 2019 17:48:23 -0700 Subject: [PATCH] fix logic around updating off-thread shapes --- interface/src/avatar/AvatarManager.cpp | 4 +- interface/src/avatar/AvatarMotionState.cpp | 4 +- interface/src/avatar/AvatarMotionState.h | 2 +- interface/src/avatar/DetailedMotionState.cpp | 18 +++++- interface/src/avatar/DetailedMotionState.h | 2 +- libraries/physics/src/EntityMotionState.cpp | 4 +- libraries/physics/src/EntityMotionState.h | 2 +- libraries/physics/src/ObjectMotionState.cpp | 4 +- libraries/physics/src/ObjectMotionState.h | 2 +- .../physics/src/PhysicalEntitySimulation.cpp | 59 ++++++++----------- .../physics/src/PhysicalEntitySimulation.h | 3 - libraries/physics/src/PhysicsEngine.cpp | 2 - 12 files changed, 54 insertions(+), 52 deletions(-) diff --git a/interface/src/avatar/AvatarManager.cpp b/interface/src/avatar/AvatarManager.cpp index 8537217756..c49db0345f 100755 --- a/interface/src/avatar/AvatarManager.cpp +++ b/interface/src/avatar/AvatarManager.cpp @@ -507,13 +507,13 @@ void AvatarManager::buildPhysicsTransaction(PhysicsEngine::Transaction& transact } else { uint32_t flags = motionState->getIncomingDirtyFlags(); motionState->clearIncomingDirtyFlags(); - if (flags | EASY_DIRTY_PHYSICS_FLAGS) { + if (flags & EASY_DIRTY_PHYSICS_FLAGS) { motionState->handleEasyChanges(flags); } // NOTE: we don't call detailedMotionState->handleEasyChanges() here is because they are KINEMATIC // and Bullet will automagically call DetailedMotionState::getWorldTransform() on all that are active. - if (flags | (Simulation::DIRTY_MOTION_TYPE | Simulation::DIRTY_COLLISION_GROUP)) { + if (flags & (Simulation::DIRTY_MOTION_TYPE | Simulation::DIRTY_COLLISION_GROUP)) { transaction.objectsToReinsert.push_back(motionState); for (auto detailedMotionState : avatar->getDetailedMotionStates()) { transaction.objectsToReinsert.push_back(detailedMotionState); diff --git a/interface/src/avatar/AvatarMotionState.cpp b/interface/src/avatar/AvatarMotionState.cpp index ae229dc66f..97085c8443 100755 --- a/interface/src/avatar/AvatarMotionState.cpp +++ b/interface/src/avatar/AvatarMotionState.cpp @@ -39,9 +39,9 @@ uint32_t AvatarMotionState::getIncomingDirtyFlags() const { return _body ? _dirtyFlags : 0; } -void AvatarMotionState::clearIncomingDirtyFlags() { +void AvatarMotionState::clearIncomingDirtyFlags(uint32_t mask) { if (_body) { - _dirtyFlags = 0; + _dirtyFlags &= ~mask; } } diff --git a/interface/src/avatar/AvatarMotionState.h b/interface/src/avatar/AvatarMotionState.h index 5f26b5114d..a752d2c8fb 100644 --- a/interface/src/avatar/AvatarMotionState.h +++ b/interface/src/avatar/AvatarMotionState.h @@ -28,7 +28,7 @@ public: PhysicsMotionType getMotionType() const override { return _motionType; } uint32_t getIncomingDirtyFlags() const override; - void clearIncomingDirtyFlags() override; + void clearIncomingDirtyFlags(uint32_t mask = DIRTY_PHYSICS_FLAGS) override; PhysicsMotionType computePhysicsMotionType() const override; diff --git a/interface/src/avatar/DetailedMotionState.cpp b/interface/src/avatar/DetailedMotionState.cpp index 4cfbbf032c..02a2b9d425 100644 --- a/interface/src/avatar/DetailedMotionState.cpp +++ b/interface/src/avatar/DetailedMotionState.cpp @@ -43,9 +43,9 @@ uint32_t DetailedMotionState::getIncomingDirtyFlags() const { return _body ? _dirtyFlags : 0; } -void DetailedMotionState::clearIncomingDirtyFlags() { +void DetailedMotionState::clearIncomingDirtyFlags(uint32_t mask) { if (_body) { - _dirtyFlags = 0; + _dirtyFlags &= ~mask; } } @@ -157,7 +157,19 @@ void DetailedMotionState::setRigidBody(btRigidBody* body) { } void DetailedMotionState::setShape(const btCollisionShape* shape) { - ObjectMotionState::setShape(shape); + if (_shape != shape) { + if (_shape) { + getShapeManager()->releaseShape(_shape); + } + _shape = shape; + if (_body) { + assert(_shape); + _body->setCollisionShape(const_cast(_shape)); + } + } else if (shape) { + // we need to release unused reference to shape + getShapeManager()->releaseShape(shape); + } } void DetailedMotionState::forceActive() { diff --git a/interface/src/avatar/DetailedMotionState.h b/interface/src/avatar/DetailedMotionState.h index 95b0600cf9..de59f41310 100644 --- a/interface/src/avatar/DetailedMotionState.h +++ b/interface/src/avatar/DetailedMotionState.h @@ -28,7 +28,7 @@ public: PhysicsMotionType getMotionType() const override { return _motionType; } uint32_t getIncomingDirtyFlags() const override; - void clearIncomingDirtyFlags() override; + void clearIncomingDirtyFlags(uint32_t mask = DIRTY_PHYSICS_FLAGS) override; PhysicsMotionType computePhysicsMotionType() const override; diff --git a/libraries/physics/src/EntityMotionState.cpp b/libraries/physics/src/EntityMotionState.cpp index 24ce17d6fb..69da64b899 100644 --- a/libraries/physics/src/EntityMotionState.cpp +++ b/libraries/physics/src/EntityMotionState.cpp @@ -630,9 +630,9 @@ uint32_t EntityMotionState::getIncomingDirtyFlags() const { return dirtyFlags; } -void EntityMotionState::clearIncomingDirtyFlags() { +void EntityMotionState::clearIncomingDirtyFlags(uint32_t mask) { if (_body && _entity) { - _entity->clearDirtyFlags(DIRTY_PHYSICS_FLAGS); + _entity->clearDirtyFlags(mask); } } diff --git a/libraries/physics/src/EntityMotionState.h b/libraries/physics/src/EntityMotionState.h index 0d5a97eeb8..7d42c2fc7a 100644 --- a/libraries/physics/src/EntityMotionState.h +++ b/libraries/physics/src/EntityMotionState.h @@ -56,7 +56,7 @@ public: void sendUpdate(OctreeEditPacketSender* packetSender, uint32_t step); virtual uint32_t getIncomingDirtyFlags() const override; - virtual void clearIncomingDirtyFlags() override; + virtual void clearIncomingDirtyFlags(uint32_t mask = DIRTY_PHYSICS_FLAGS) override; virtual float getObjectRestitution() const override { return _entity->getRestitution(); } virtual float getObjectFriction() const override { return _entity->getFriction(); } diff --git a/libraries/physics/src/ObjectMotionState.cpp b/libraries/physics/src/ObjectMotionState.cpp index 92482437e2..ad7332cb15 100644 --- a/libraries/physics/src/ObjectMotionState.cpp +++ b/libraries/physics/src/ObjectMotionState.cpp @@ -198,7 +198,9 @@ void ObjectMotionState::setShape(const btCollisionShape* shape) { getShapeManager()->releaseShape(_shape); } _shape = shape; - if (_body && _type != MOTIONSTATE_TYPE_DETAILED) { + if (_body) { + assert(_shape); + _body->setCollisionShape(const_cast(_shape)); updateCCDConfiguration(); } } else if (shape) { diff --git a/libraries/physics/src/ObjectMotionState.h b/libraries/physics/src/ObjectMotionState.h index 7d204194b2..415b388e70 100644 --- a/libraries/physics/src/ObjectMotionState.h +++ b/libraries/physics/src/ObjectMotionState.h @@ -123,7 +123,7 @@ public: virtual glm::vec3 getObjectLinearVelocityChange() const; virtual uint32_t getIncomingDirtyFlags() const = 0; - virtual void clearIncomingDirtyFlags() = 0; + virtual void clearIncomingDirtyFlags(uint32_t mask = DIRTY_PHYSICS_FLAGS) = 0; virtual PhysicsMotionType computePhysicsMotionType() const = 0; diff --git a/libraries/physics/src/PhysicalEntitySimulation.cpp b/libraries/physics/src/PhysicalEntitySimulation.cpp index daf2ce0298..f1f356cef7 100644 --- a/libraries/physics/src/PhysicalEntitySimulation.cpp +++ b/libraries/physics/src/PhysicalEntitySimulation.cpp @@ -237,7 +237,6 @@ void PhysicalEntitySimulation::buildMotionStatesForEntitiesThatNeedThem() { _incomingChanges.insert(motionState); }; - QMutexLocker lock(&_mutex); uint32_t deliveryCount = ObjectMotionState::getShapeManager()->getWorkDeliveryCount(); if (deliveryCount != _lastWorkDeliveryCount) { // new off-thread shapes have arrived --> find adds whose shapes have arrived @@ -343,41 +342,22 @@ void PhysicalEntitySimulation::buildMotionStatesForEntitiesThatNeedThem() { } } -void PhysicalEntitySimulation::setObjectsToChange(const VectorOfMotionStates& objectsToChange) { - QMutexLocker lock(&_mutex); - for (auto object : objectsToChange) { - _incomingChanges.insert(static_cast(object)); - } -} - -void PhysicalEntitySimulation::getObjectsToChange(VectorOfMotionStates& result) { - result.clear(); - QMutexLocker lock(&_mutex); - for (auto stateItr : _incomingChanges) { - EntityMotionState* motionState = &(*stateItr); - result.push_back(motionState); - } - _incomingChanges.clear(); -} - void PhysicalEntitySimulation::buildPhysicsTransaction(PhysicsEngine::Transaction& transaction) { + QMutexLocker lock(&_mutex); buildMotionStatesForEntitiesThatNeedThem(); for (auto& object : _incomingChanges) { uint32_t flags = object->getIncomingDirtyFlags(); - object->clearIncomingDirtyFlags(); + uint32_t handledFlags = EASY_DIRTY_PHYSICS_FLAGS; bool isInPhysicsSimulation = object->isInPhysicsSimulation(); - if (isInPhysicsSimulation != object->shouldBeInPhysicsSimulation()) { - if (isInPhysicsSimulation) { - transaction.objectsToRemove.push_back(object); - continue; - } else { - transaction.objectsToAdd.push_back(object); - } + bool shouldBeInPhysicsSimulation = object->shouldBeInPhysicsSimulation(); + if (!shouldBeInPhysicsSimulation && isInPhysicsSimulation) { + transaction.objectsToRemove.push_back(object); + continue; } - bool reinsert = false; - if (object->needsNewShape()) { + bool needsNewShape = object->needsNewShape(); + if (needsNewShape) { ShapeType shapeType = object->getShapeType(); if (shapeType == SHAPE_TYPE_STATIC_MESH) { ShapeRequest shapeRequest(object->_entity); @@ -389,13 +369,15 @@ void PhysicalEntitySimulation::buildPhysicsTransaction(PhysicsEngine::Transactio btCollisionShape* shape = const_cast(ObjectMotionState::getShapeManager()->getShape(shapeInfo)); if (shape) { object->setShape(shape); - reinsert = true; + handledFlags |= Simulation::DIRTY_SHAPE; + needsNewShape = false; } else if (requestCount != ObjectMotionState::getShapeManager()->getWorkRequestCount()) { // shape doesn't exist but a new worker has been spawned to build it --> add to shapeRequests and wait shapeRequest.shapeHash = shapeInfo.getHash(); _shapeRequests.insert(shapeRequest); } else { // failed to build shape --> will not be added/updated + handledFlags |= Simulation::DIRTY_SHAPE; } } else { // continue waiting for shape request @@ -406,24 +388,35 @@ void PhysicalEntitySimulation::buildPhysicsTransaction(PhysicsEngine::Transactio btCollisionShape* shape = const_cast(ObjectMotionState::getShapeManager()->getShape(shapeInfo)); if (shape) { object->setShape(shape); - reinsert = true; + handledFlags |= Simulation::DIRTY_SHAPE; + needsNewShape = false; } else { // failed to build shape --> will not be added } } } if (!isInPhysicsSimulation) { - continue; + if (needsNewShape) { + // skip it + continue; + } else { + transaction.objectsToAdd.push_back(object); + handledFlags = DIRTY_PHYSICS_FLAGS; + } } - if (flags | EASY_DIRTY_PHYSICS_FLAGS) { + + if (flags & EASY_DIRTY_PHYSICS_FLAGS) { object->handleEasyChanges(flags); } - if (flags | (Simulation::DIRTY_MOTION_TYPE | Simulation::DIRTY_COLLISION_GROUP) || reinsert) { + if ((flags & (Simulation::DIRTY_MOTION_TYPE | Simulation::DIRTY_COLLISION_GROUP)) || (handledFlags & Simulation::DIRTY_SHAPE)) { transaction.objectsToReinsert.push_back(object); + handledFlags |= (Simulation::DIRTY_MOTION_TYPE | Simulation::DIRTY_COLLISION_GROUP); } else if (flags & Simulation::DIRTY_PHYSICS_ACTIVATION && object->getRigidBody()->isStaticObject()) { transaction.activeStaticObjects.push_back(object); } + object->clearIncomingDirtyFlags(handledFlags); } + _incomingChanges.clear(); } void PhysicalEntitySimulation::handleProcessedPhysicsTransaction(PhysicsEngine::Transaction& transaction) { diff --git a/libraries/physics/src/PhysicalEntitySimulation.h b/libraries/physics/src/PhysicalEntitySimulation.h index 956674c070..817f92cb3c 100644 --- a/libraries/physics/src/PhysicalEntitySimulation.h +++ b/libraries/physics/src/PhysicalEntitySimulation.h @@ -81,9 +81,6 @@ protected: // only called by EntitySimulation public: virtual void prepareEntityForDelete(EntityItemPointer entity) override; - void setObjectsToChange(const VectorOfMotionStates& objectsToChange); - void getObjectsToChange(VectorOfMotionStates& result); - void buildPhysicsTransaction(PhysicsEngine::Transaction& transaction); void handleProcessedPhysicsTransaction(PhysicsEngine::Transaction& transaction); diff --git a/libraries/physics/src/PhysicsEngine.cpp b/libraries/physics/src/PhysicsEngine.cpp index bae9ef2485..2e1a1a401e 100644 --- a/libraries/physics/src/PhysicsEngine.cpp +++ b/libraries/physics/src/PhysicsEngine.cpp @@ -178,8 +178,6 @@ void PhysicsEngine::addObjectToDynamicsWorld(ObjectMotionState* motionState) { int32_t group, mask; motionState->computeCollisionGroupAndMask(group, mask); _dynamicsWorld->addRigidBody(body, group, mask); - - motionState->clearIncomingDirtyFlags(); } QList PhysicsEngine::removeDynamicsForBody(btRigidBody* body) {