fix logic around updating off-thread shapes

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);

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;
     }
 }
 

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;
 

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<btCollisionShape*>(_shape));
+        }
+    } else if (shape) {
+        // we need to release unused reference to shape
+        getShapeManager()->releaseShape(shape);
+    }
 }
 
 void DetailedMotionState::forceActive() {

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;
 

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);
     }
 }
 

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(); }

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<btCollisionShape*>(_shape));
             updateCCDConfiguration();
         }
     } else if (shape) {

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;
 

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<EntityMotionState*>(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()) {
+        bool shouldBeInPhysicsSimulation = object->shouldBeInPhysicsSimulation();
-            if (isInPhysicsSimulation) {
+        if (!shouldBeInPhysicsSimulation && isInPhysicsSimulation) {
-                transaction.objectsToRemove.push_back(object);
+            transaction.objectsToRemove.push_back(object);
-                continue;
+            continue;
-            } else {
-                transaction.objectsToAdd.push_back(object);
-            }
         }
 
-        bool reinsert = false;
+        bool needsNewShape = object->needsNewShape();
-        if (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<btCollisionShape*>(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<btCollisionShape*>(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) {

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);
 

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<EntityDynamicPointer> PhysicsEngine::removeDynamicsForBody(btRigidBody* body) {