addressing review requests

interface/src/avatar/AvatarManager.cpp

@@ -84,7 +84,6 @@ AvatarManager::AvatarManager(QObject* parent) :
 
 AvatarSharedPointer AvatarManager::addAvatar(const QUuid& sessionUUID, const QWeakPointer<Node>& mixerWeakPointer) {
     AvatarSharedPointer avatar = AvatarHashMap::addAvatar(sessionUUID, mixerWeakPointer);
-
     const auto otherAvatar = std::static_pointer_cast<OtherAvatar>(avatar);
     if (otherAvatar && _space) {
         std::unique_lock<std::mutex> lock(_spaceLock);
@@ -210,7 +209,7 @@ void AvatarManager::updateOtherAvatars(float deltaTime) {
     {
         // lock the hash for read to check the size
         QReadLocker lock(&_hashLock);
-        if (_avatarHash.size() < 2 && _avatarsToFadeOut.empty()) {
+        if (_avatarHash.size() < 2) {
             return;
         }
     }
@@ -375,18 +374,12 @@ void AvatarManager::updateOtherAvatars(float deltaTime) {
         qApp->getMain3DScene()->enqueueTransaction(renderTransaction);
     }
 
-    if (!_spaceProxiesToDelete.empty() && _space) {
-        std::unique_lock<std::mutex> lock(_spaceLock);
-        workloadTransaction.remove(_spaceProxiesToDelete);
-        _spaceProxiesToDelete.clear();
-    }
     _space->enqueueTransaction(workloadTransaction);
 
     _numAvatarsUpdated = numAvatarsUpdated;
     _numAvatarsNotUpdated = numAvatarsNotUpdated;
     _numHeroAvatarsUpdated = numHerosUpdated;
 
-    removeFadedAvatars();
     _avatarSimulationTime = (float)(usecTimestampNow() - startTime) / (float)USECS_PER_MSEC;
 }
 
@@ -399,30 +392,6 @@ void AvatarManager::postUpdate(float deltaTime, const render::ScenePointer& scen
     }
 }
 
-void AvatarManager::removeFadedAvatars() {
-    if (_avatarsToFadeOut.empty()) {
-        return;
-    }
-
-    QReadLocker locker(&_hashLock);
-    auto avatarItr = _avatarsToFadeOut.begin();
-    const render::ScenePointer& scene = qApp->getMain3DScene();
-    render::Transaction transaction;
-    while (avatarItr != _avatarsToFadeOut.end()) {
-        auto avatar = std::static_pointer_cast<Avatar>(*avatarItr);
-        if (!avatar->isFading()) {
-            // fading to zero is such a rare event we push a unique transaction for each
-            if (avatar->isInScene()) {
-                avatar->removeFromScene(*avatarItr, scene, transaction);
-            }
-            avatarItr = _avatarsToFadeOut.erase(avatarItr);
-        } else {
-            ++avatarItr;
-        }
-    }
-    scene->enqueueTransaction(transaction);
-}
-
 AvatarSharedPointer AvatarManager::newSharedAvatar(const QUuid& sessionUUID) {
     auto otherAvatar = new OtherAvatar(qApp->thread());
     otherAvatar->setSessionUUID(sessionUUID);
@@ -517,10 +486,6 @@ void AvatarManager::removeDeadAvatarEntities(const SetOfEntities& deadEntities)
 
 void AvatarManager::handleRemovedAvatar(const AvatarSharedPointer& removedAvatar, KillAvatarReason removalReason) {
     auto avatar = std::static_pointer_cast<OtherAvatar>(removedAvatar);
-    {
-        std::unique_lock<std::mutex> lock(_spaceLock);
-        _spaceProxiesToDelete.push_back(avatar->getSpaceIndex());
-    }
     AvatarHashMap::handleRemovedAvatar(avatar, removalReason);
     avatar->tearDownGrabs();
 
@@ -534,6 +499,15 @@ void AvatarManager::handleRemovedAvatar(const AvatarSharedPointer& removedAvatar
     avatar->setIsFading(false);
     if (removalReason == KillAvatarReason::TheirAvatarEnteredYourBubble) {
         emit DependencyManager::get<UsersScriptingInterface>()->enteredIgnoreRadius();
+
+        workload::Transaction workloadTransaction;
+        workloadTransaction.remove(avatar->getSpaceIndex());
+        _space->enqueueTransaction(workloadTransaction);
+
+        const render::ScenePointer& scene = qApp->getMain3DScene();
+        render::Transaction transaction;
+        avatar->removeFromScene(avatar, scene, transaction);
+        scene->enqueueTransaction(transaction);
     } else if (removalReason == KillAvatarReason::AvatarDisconnected) {
         // remove from node sets, if present
         DependencyManager::get<NodeList>()->removeFromIgnoreMuteSets(avatar->getSessionUUID());
@@ -542,13 +516,20 @@ void AvatarManager::handleRemovedAvatar(const AvatarSharedPointer& removedAvatar
         auto scene = qApp->getMain3DScene();
         avatar->fadeOut(transaction, removalReason);
 
-        transaction.transitionFinishedOperator(avatar->getRenderItemID(), [avatar]() {
+        workload::SpacePointer space = _space;
+        transaction.transitionFinishedOperator(avatar->getRenderItemID(), [space, avatar]() {
             avatar->setIsFading(false);
+            const render::ScenePointer& scene = qApp->getMain3DScene();
+            render::Transaction transaction;
+            avatar->removeFromScene(avatar, scene, transaction);
+            scene->enqueueTransaction(transaction);
+
+            workload::Transaction workloadTransaction;
+            workloadTransaction.remove(avatar->getSpaceIndex());
+            space->enqueueTransaction(workloadTransaction);
         });
         scene->enqueueTransaction(transaction);
     }
-
-    _avatarsToFadeOut.push_back(removedAvatar);
 }
 
 void AvatarManager::clearOtherAvatars() {

interface/src/avatar/AvatarManager.h

@@ -222,8 +222,6 @@ private:
 
     AvatarSharedPointer newSharedAvatar(const QUuid& sessionUUID) override;
 
-    void removeFadedAvatars();
-
     // called only from the AvatarHashMap thread - cannot be called while this thread holds the
     // hash lock, since handleRemovedAvatar needs a write lock on the entity tree and the entity tree
     // frequently grabs a read lock on the hash to get a given avatar by ID
@@ -231,7 +229,6 @@ private:
                              KillAvatarReason removalReason = KillAvatarReason::NoReason) override;
     void handleTransitAnimations(AvatarTransit::Status status);
 
-    std::vector<AvatarSharedPointer> _avatarsToFadeOut;
     using SetOfOtherAvatars = std::set<OtherAvatarPointer>;
     SetOfOtherAvatars _avatarsToChangeInPhysics;
 
@@ -251,7 +248,6 @@ private:
 
     mutable std::mutex _spaceLock;
     workload::SpacePointer _space;
-    std::vector<int32_t> _spaceProxiesToDelete;
 
     AvatarTransit::TransitConfig  _transitConfig;
 };

libraries/avatars/src/AvatarHashMap.cpp

@@ -439,7 +439,6 @@ void AvatarHashMap::removeAvatar(const QUuid& sessionUUID, KillAvatarReason remo
         }
 
         auto removedAvatar = _avatarHash.take(sessionUUID);
-
         if (removedAvatar) {
             removedAvatars.push_back(removedAvatar);
         }

libraries/entities-renderer/src/EntityTreeRenderer.cpp

@@ -214,30 +214,6 @@ void EntityTreeRenderer::stopDomainAndNonOwnedEntities() {
     }
 }
 
-void EntityTreeRenderer::removeFadedRenderables() {
-    if (_entityRenderablesToFadeOut.empty()) {
-        return;
-    }
-
-    std::unique_lock<std::mutex> lock(_entitiesToFadeLock);
-    auto entityIter = _entityRenderablesToFadeOut.begin();
-    auto scene = _viewState->getMain3DScene();
-    render::Transaction transaction;
-
-    while (entityIter != _entityRenderablesToFadeOut.end()) {
-        auto entityRenderable = *entityIter;
-
-        if (!entityRenderable->getIsFading()) {
-            entityRenderable->removeFromScene(scene, transaction);
-            entityIter = _entityRenderablesToFadeOut.erase(entityIter);
-        } else {
-            ++entityIter;
-        }
-    }
-
-    scene->enqueueTransaction(transaction);
-}
-
 void EntityTreeRenderer::clearDomainAndNonOwnedEntities() {
     stopDomainAndNonOwnedEntities();
 
@@ -551,7 +527,6 @@ void EntityTreeRenderer::update(bool simulate) {
         }
 
     }
-    removeFadedRenderables();
 }
 
 void EntityTreeRenderer::handleSpaceUpdate(std::pair<int32_t, glm::vec4> proxyUpdate) {
@@ -1080,12 +1055,14 @@ void EntityTreeRenderer::fadeOutRenderable(const EntityRendererPointer& renderab
     auto scene = _viewState->getMain3DScene();
 
     renderable->setIsFading(true);
-    transaction.transitionFinishedOperator(renderable->getRenderItemID(), [renderable]() {
+    transaction.transitionFinishedOperator(renderable->getRenderItemID(), [scene, renderable]() {
         renderable->setIsFading(false);
+        render::Transaction transaction;
+        renderable->removeFromScene(scene, transaction);
+        scene->enqueueTransaction(transaction);
     });
 
     scene->enqueueTransaction(transaction);
-    _entityRenderablesToFadeOut.push_back(renderable);
 }
 
 void EntityTreeRenderer::playEntityCollisionSound(const EntityItemPointer& entity, const Collision& collision) {

libraries/entities-renderer/src/EntityTreeRenderer.h

@@ -259,8 +259,6 @@ private:
     std::unordered_map<EntityItemID, EntityRendererPointer> _entitiesInScene;
     std::unordered_map<EntityItemID, EntityItemWeakPointer> _entitiesToAdd;
 
-    std::mutex _entitiesToFadeLock;
-    std::vector<EntityRendererPointer> _entityRenderablesToFadeOut;
     // For Scene.shouldRenderEntities
     QList<EntityItemID> _entityIDsLastInScene;
 

libraries/render/src/render/Scene.cpp

@@ -559,7 +559,6 @@ void Scene::resetItemTransition(ItemID itemId) {
         auto transitionStage = getStage<TransitionStage>(TransitionStage::getName());
 
         auto finishedOperators = _transitionFinishedOperatorMap[transitionId];
-        qDebug() << "removing transition: " << transitionId;
         for (auto finishedOperator : finishedOperators) {
             if (finishedOperator) {
                 finishedOperator();