From 2775b17a657a19353281e58fdcffe39260e665cc Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Tue, 4 Apr 2017 14:02:58 -0700 Subject: [PATCH 1/8] unravel one loop of spaghetti --- interface/src/avatar/AvatarManager.cpp | 37 +++++++------------------- interface/src/avatar/AvatarManager.h | 26 +++++++++--------- 2 files changed, 24 insertions(+), 39 deletions(-) diff --git a/interface/src/avatar/AvatarManager.cpp b/interface/src/avatar/AvatarManager.cpp index 48914908c6..03d7dd1c6c 100644 --- a/interface/src/avatar/AvatarManager.cpp +++ b/interface/src/avatar/AvatarManager.cpp @@ -68,7 +68,7 @@ void AvatarManager::registerMetaTypes(QScriptEngine* engine) { } AvatarManager::AvatarManager(QObject* parent) : - _avatarFades(), + _avatarsToFade(), _myAvatar(std::make_shared(std::make_shared())) { // register a meta type for the weak pointer we'll use for the owning avatar mixer for each avatar @@ -151,7 +151,7 @@ float AvatarManager::getAvatarSimulationRate(const QUuid& sessionID, const QStri void AvatarManager::updateOtherAvatars(float deltaTime) { // lock the hash for read to check the size QReadLocker lock(&_hashLock); - if (_avatarHash.size() < 2 && _avatarFades.isEmpty()) { + if (_avatarHash.size() < 2 && _avatarsToFade.isEmpty()) { return; } lock.unlock(); @@ -277,22 +277,22 @@ void AvatarManager::postUpdate(float deltaTime) { } void AvatarManager::simulateAvatarFades(float deltaTime) { - QVector::iterator fadingIterator = _avatarFades.begin(); + QVector::iterator fadingIterator = _avatarsToFade.begin(); const float SHRINK_RATE = 0.15f; const float MIN_FADE_SCALE = MIN_AVATAR_SCALE; render::ScenePointer scene = qApp->getMain3DScene(); render::Transaction transaction; - while (fadingIterator != _avatarFades.end()) { + while (fadingIterator != _avatarsToFade.end()) { auto avatar = std::static_pointer_cast(*fadingIterator); avatar->setTargetScale(avatar->getUniformScale() * SHRINK_RATE); avatar->animateScaleChanges(deltaTime); if (avatar->getTargetScale() <= MIN_FADE_SCALE) { avatar->removeFromScene(*fadingIterator, scene, transaction); - // only remove from _avatarFades if we're sure its motionState has been removed from PhysicsEngine + // only remove from _avatarsToFade if we're sure its motionState has been removed from PhysicsEngine if (_motionStatesToRemoveFromPhysics.empty()) { - fadingIterator = _avatarFades.erase(fadingIterator); + fadingIterator = _avatarsToFade.erase(fadingIterator); } else { ++fadingIterator; } @@ -306,26 +306,9 @@ void AvatarManager::simulateAvatarFades(float deltaTime) { } AvatarSharedPointer AvatarManager::newSharedAvatar() { - return std::make_shared(std::make_shared()); -} - -AvatarSharedPointer AvatarManager::addAvatar(const QUuid& sessionUUID, const QWeakPointer& mixerWeakPointer) { - auto newAvatar = AvatarHashMap::addAvatar(sessionUUID, mixerWeakPointer); - auto rawRenderableAvatar = std::static_pointer_cast(newAvatar); - - rawRenderableAvatar->addToScene(rawRenderableAvatar); - - return newAvatar; -} - -// virtual -void AvatarManager::removeAvatar(const QUuid& sessionUUID, KillAvatarReason removalReason) { - QWriteLocker locker(&_hashLock); - - auto removedAvatar = _avatarHash.take(sessionUUID); - if (removedAvatar) { - handleRemovedAvatar(removedAvatar, removalReason); - } + std::shared_ptr avatar = std::make_shared(std::make_shared()); + avatar->addToScene(avatar); + return avatar; } void AvatarManager::handleRemovedAvatar(const AvatarSharedPointer& removedAvatar, KillAvatarReason removalReason) { @@ -353,7 +336,7 @@ void AvatarManager::handleRemovedAvatar(const AvatarSharedPointer& removedAvatar DependencyManager::get()->removeFromIgnoreMuteSets(avatar->getSessionUUID()); DependencyManager::get()->avatarDisconnected(avatar->getSessionUUID()); } - _avatarFades.push_back(removedAvatar); + _avatarsToFade.push_back(removedAvatar); } void AvatarManager::clearOtherAvatars() { diff --git a/interface/src/avatar/AvatarManager.h b/interface/src/avatar/AvatarManager.h index b94f9e6a96..228fe7e92f 100644 --- a/interface/src/avatar/AvatarManager.h +++ b/interface/src/avatar/AvatarManager.h @@ -91,21 +91,27 @@ public slots: void setShouldShowReceiveStats(bool shouldShowReceiveStats) { _shouldShowReceiveStats = shouldShowReceiveStats; } void updateAvatarRenderStatus(bool shouldRenderAvatars); -private slots: - virtual void removeAvatar(const QUuid& sessionUUID, KillAvatarReason removalReason = KillAvatarReason::NoReason) override; - private: explicit AvatarManager(QObject* parent = 0); explicit AvatarManager(const AvatarManager& other); void simulateAvatarFades(float deltaTime); - // virtual overrides - virtual AvatarSharedPointer newSharedAvatar() override; - virtual AvatarSharedPointer addAvatar(const QUuid& sessionUUID, const QWeakPointer& mixerWeakPointer) override; - virtual void handleRemovedAvatar(const AvatarSharedPointer& removedAvatar, KillAvatarReason removalReason = KillAvatarReason::NoReason) override; + AvatarSharedPointer newSharedAvatar() override; + void handleRemovedAvatar(const AvatarSharedPointer& removedAvatar, KillAvatarReason removalReason = KillAvatarReason::NoReason) override; + + /* TODO: maintain these lists + QVector _avatarsToRemoveFromScene; + QVector _avatarsToAddToScene; + QVector _avatarsToRemoveFromPhysicsEngine; + QVector _avatarsToAddToPhysicsEngine; + */ + QVector _avatarsToFade; + + SetOfAvatarMotionStates _motionStatesThatMightUpdate; + VectorOfMotionStates _motionStatesToRemoveFromPhysics; + SetOfMotionStates _motionStatesToAddToPhysics; - QVector _avatarFades; std::shared_ptr _myAvatar; quint64 _lastSendAvatarDataTime = 0; // Controls MyAvatar send data rate. @@ -115,10 +121,6 @@ private: std::list> _collisionInjectors; - SetOfAvatarMotionStates _motionStatesThatMightUpdate; - SetOfMotionStates _motionStatesToAddToPhysics; - VectorOfMotionStates _motionStatesToRemoveFromPhysics; - RateCounter<> _myAvatarSendRate; int _numAvatarsUpdated { 0 }; int _numAvatarsNotUpdated { 0 }; From 40e223cb82e4e1bed70c6bad719170a75d19d9ec Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Tue, 4 Apr 2017 15:10:02 -0700 Subject: [PATCH 2/8] cleanup unused return values and redundant variables --- interface/src/avatar/Avatar.cpp | 8 ++------ interface/src/avatar/Avatar.h | 3 +-- 2 files changed, 3 insertions(+), 8 deletions(-) diff --git a/interface/src/avatar/Avatar.cpp b/interface/src/avatar/Avatar.cpp index b6ec45b308..a8fc4840fe 100644 --- a/interface/src/avatar/Avatar.cpp +++ b/interface/src/avatar/Avatar.cpp @@ -476,7 +476,7 @@ static TextRenderer3D* textRenderer(TextRendererType type) { return displayNameRenderer; } -bool Avatar::addToScene(AvatarSharedPointer self, std::shared_ptr scene, render::Transaction& transaction) { +void Avatar::addToScene(AvatarSharedPointer self, std::shared_ptr scene, render::Transaction& transaction) { auto avatarPayload = new render::Payload(self); auto avatarPayloadPointer = Avatar::PayloadPointer(avatarPayload); _renderItemID = scene->allocateID(); @@ -486,9 +486,6 @@ bool Avatar::addToScene(AvatarSharedPointer self, std::shared_ptr for (auto& attachmentModel : _attachmentModels) { attachmentModel->addToScene(scene, transaction); } - - _inScene = true; - return true; } void Avatar::removeFromScene(AvatarSharedPointer self, std::shared_ptr scene, render::Transaction& transaction) { @@ -498,7 +495,6 @@ void Avatar::removeFromScene(AvatarSharedPointer self, std::shared_ptrremoveFromScene(scene, transaction); } - _inScene = false; } void Avatar::updateRenderItem(render::Transaction& transaction) { @@ -1435,7 +1431,7 @@ void Avatar::addToScene(AvatarSharedPointer myHandle) { } } void Avatar::ensureInScene(AvatarSharedPointer self) { - if (!_inScene) { + if (!render::Item::isValidID(_renderItemID)) { addToScene(self); } } diff --git a/interface/src/avatar/Avatar.h b/interface/src/avatar/Avatar.h index 7fadf96e11..ae98c525c8 100644 --- a/interface/src/avatar/Avatar.h +++ b/interface/src/avatar/Avatar.h @@ -81,7 +81,7 @@ public: virtual void render(RenderArgs* renderArgs, const glm::vec3& cameraPosition); - bool addToScene(AvatarSharedPointer self, std::shared_ptr scene, + void addToScene(AvatarSharedPointer self, std::shared_ptr scene, render::Transaction& transaction); void removeFromScene(AvatarSharedPointer self, std::shared_ptr scene, @@ -310,7 +310,6 @@ private: int _nameRectGeometryID { 0 }; bool _initialized; bool _isLookAtTarget { false }; - bool _inScene { false }; bool _isAnimatingScale { false }; float getBoundingRadius() const; From 662c34c266c230619c5272b11866d830dbc7bb0a Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Thu, 6 Apr 2017 09:40:24 -0700 Subject: [PATCH 3/8] fix avatar scale animation and avatar render debug --- interface/src/avatar/Avatar.cpp | 9 +- interface/src/avatar/Avatar.h | 1 + interface/src/avatar/AvatarManager.cpp | 111 ++++++++++++++++-------- interface/src/avatar/AvatarManager.h | 10 +-- interface/src/avatar/MyAvatar.cpp | 3 +- libraries/avatars/src/AvatarData.cpp | 1 + libraries/avatars/src/AvatarHashMap.cpp | 41 +++++---- libraries/avatars/src/AvatarHashMap.h | 11 +-- 8 files changed, 117 insertions(+), 70 deletions(-) diff --git a/interface/src/avatar/Avatar.cpp b/interface/src/avatar/Avatar.cpp index a8fc4840fe..988d34e554 100644 --- a/interface/src/avatar/Avatar.cpp +++ b/interface/src/avatar/Avatar.cpp @@ -192,6 +192,7 @@ void Avatar::animateScaleChanges(float deltaTime) { _isAnimatingScale = false; } setScale(glm::vec3(animatedScale)); // avatar scale is uniform + _hasNewJointData = true; // TODO: rebuilding the shape constantly is somehwat expensive. // We should only rebuild after significant change. @@ -200,8 +201,12 @@ void Avatar::animateScaleChanges(float deltaTime) { } void Avatar::setTargetScale(float targetScale) { - AvatarData::setTargetScale(targetScale); - _isAnimatingScale = true; + float newValue = glm::clamp(targetScale, MIN_AVATAR_SCALE, MAX_AVATAR_SCALE); + if (_targetScale != newValue) { + _targetScale = newValue; + _scaleChanged = usecTimestampNow(); + _isAnimatingScale = true; + } } void Avatar::updateAvatarEntities() { diff --git a/interface/src/avatar/Avatar.h b/interface/src/avatar/Avatar.h index ae98c525c8..c4084fce9a 100644 --- a/interface/src/avatar/Avatar.h +++ b/interface/src/avatar/Avatar.h @@ -285,6 +285,7 @@ protected: void addToScene(AvatarSharedPointer self); void ensureInScene(AvatarSharedPointer self); + bool isInScene() const { return render::Item::isValidID(_renderItemID); } // Some rate tracking support RateCounter<> _simulationRate; diff --git a/interface/src/avatar/AvatarManager.cpp b/interface/src/avatar/AvatarManager.cpp index 03d7dd1c6c..0d9cd13d24 100644 --- a/interface/src/avatar/AvatarManager.cpp +++ b/interface/src/avatar/AvatarManager.cpp @@ -100,15 +100,16 @@ void AvatarManager::init() { _avatarHash.insert(MY_AVATAR_KEY, _myAvatar); } + _shouldRender = DependencyManager::get()->shouldRenderAvatars(); connect(DependencyManager::get().data(), &SceneScriptingInterface::shouldRenderAvatarsChanged, this, &AvatarManager::updateAvatarRenderStatus, Qt::QueuedConnection); - render::ScenePointer scene = qApp->getMain3DScene(); - render::Transaction transaction; - if (DependencyManager::get()->shouldRenderAvatars()) { + if (_shouldRender) { + render::ScenePointer scene = qApp->getMain3DScene(); + render::Transaction transaction; _myAvatar->addToScene(_myAvatar, scene, transaction); + scene->enqueueTransaction(transaction); } - scene->enqueueTransaction(transaction); } void AvatarManager::updateMyAvatar(float deltaTime) { @@ -181,30 +182,24 @@ void AvatarManager::updateOtherAvatars(float deltaTime) { // DO NOT update or fade out uninitialized Avatars return true; // ignore it } - if (avatar->shouldDie()) { - removeAvatar(avatar->getID()); - return true; // ignore it - } - if (avatar->isDead()) { - return true; // ignore it - } - return false; }); - render::Transaction transaction; uint64_t startTime = usecTimestampNow(); const uint64_t UPDATE_BUDGET = 2000; // usec uint64_t updateExpiry = startTime + UPDATE_BUDGET; - int numAvatarsUpdated = 0; int numAVatarsNotUpdated = 0; + + render::Transaction transaction; while (!sortedAvatars.empty()) { const AvatarPriority& sortData = sortedAvatars.top(); const auto& avatar = std::static_pointer_cast(sortData.avatar); // for ALL avatars... - avatar->ensureInScene(avatar); + if (_shouldRender) { + avatar->ensureInScene(avatar); + } if (!avatar->getMotionState()) { ShapeInfo shapeInfo; avatar->computeShapeInfo(shapeInfo); @@ -218,6 +213,10 @@ void AvatarManager::updateOtherAvatars(float deltaTime) { } } avatar->animateScaleChanges(deltaTime); + if (avatar->shouldDie()) { + avatar->die(); + removeAvatar(avatar->getID()); + } const float OUT_OF_VIEW_THRESHOLD = 0.5f * AvatarData::OUT_OF_VIEW_PENALTY; uint64_t now = usecTimestampNow(); @@ -259,10 +258,21 @@ void AvatarManager::updateOtherAvatars(float deltaTime) { sortedAvatars.pop(); } + if (_shouldRender) { + QVector::iterator itr = _avatarsToFade.begin(); + while (itr != _avatarsToFade.end() && usecTimestampNow() > updateExpiry) { + auto avatar = std::static_pointer_cast(*itr); + avatar->animateScaleChanges(deltaTime); + avatar->simulate(deltaTime, true); + avatar->updateRenderItem(transaction); + ++itr; + } + qApp->getMain3DScene()->enqueueTransaction(transaction); + } + _avatarSimulationTime = (float)(usecTimestampNow() - startTime) / (float)USECS_PER_MSEC; _numAvatarsUpdated = numAvatarsUpdated; _numAvatarsNotUpdated = numAVatarsNotUpdated; - qApp->getMain3DScene()->enqueueTransaction(transaction); simulateAvatarFades(deltaTime); } @@ -277,38 +287,68 @@ void AvatarManager::postUpdate(float deltaTime) { } void AvatarManager::simulateAvatarFades(float deltaTime) { - QVector::iterator fadingIterator = _avatarsToFade.begin(); + QVector::iterator itr = _avatarsToFade.begin(); const float SHRINK_RATE = 0.15f; const float MIN_FADE_SCALE = MIN_AVATAR_SCALE; - render::ScenePointer scene = qApp->getMain3DScene(); - render::Transaction transaction; - while (fadingIterator != _avatarsToFade.end()) { - auto avatar = std::static_pointer_cast(*fadingIterator); + while (itr != _avatarsToFade.end()) { + auto avatar = std::static_pointer_cast(*itr); avatar->setTargetScale(avatar->getUniformScale() * SHRINK_RATE); avatar->animateScaleChanges(deltaTime); if (avatar->getTargetScale() <= MIN_FADE_SCALE) { - avatar->removeFromScene(*fadingIterator, scene, transaction); + // fading to zero is such a rare event we push unique transaction for each one + removeAvatar(avatar->getID()); + if (avatar->isInScene()) { + render::ScenePointer scene = qApp->getMain3DScene(); + render::Transaction transaction; + avatar->removeFromScene(*itr, scene, transaction); + scene->enqueueTransaction(transaction); + } + // only remove from _avatarsToFade if we're sure its motionState has been removed from PhysicsEngine if (_motionStatesToRemoveFromPhysics.empty()) { - fadingIterator = _avatarsToFade.erase(fadingIterator); + itr = _avatarsToFade.erase(itr); } else { - ++fadingIterator; + ++itr; } } else { const bool inView = true; // HACK avatar->simulate(deltaTime, inView); - ++fadingIterator; + ++itr; } } - scene->enqueueTransaction(transaction); } AvatarSharedPointer AvatarManager::newSharedAvatar() { - std::shared_ptr avatar = std::make_shared(std::make_shared()); - avatar->addToScene(avatar); - return avatar; + return std::make_shared(std::make_shared()); +} + +void AvatarManager::processAvatarDataPacket(QSharedPointer message, SharedNodePointer sendingNode) { + PerformanceTimer perfTimer("receiveAvatar"); + // enumerate over all of the avatars in this packet + // only add them if mixerWeakPointer points to something (meaning that mixer is still around) + while (message->getBytesLeftToRead()) { + AvatarSharedPointer avatarData = parseAvatarData(message, sendingNode); + if (avatarData) { + auto avatar = std::static_pointer_cast(avatarData); + if (avatar->isInScene()) { + if (!_shouldRender) { + // rare transition so we process pending changes immediately + render::ScenePointer scene = qApp->getMain3DScene(); + render::Transaction transaction; + avatar->removeFromScene(avatar, scene, transaction); + scene->enqueueTransaction(transaction); + } + } else if (_shouldRender) { + // very rare transition so we process pending changes immediately + render::ScenePointer scene = qApp->getMain3DScene(); + render::Transaction transaction; + avatar->addToScene(avatar, scene, transaction); + scene->enqueueTransaction(transaction); + } + } + } } void AvatarManager::handleRemovedAvatar(const AvatarSharedPointer& removedAvatar, KillAvatarReason removalReason) { @@ -458,26 +498,23 @@ void AvatarManager::handleCollisionEvents(const CollisionEvents& collisionEvents } void AvatarManager::updateAvatarRenderStatus(bool shouldRenderAvatars) { - if (DependencyManager::get()->shouldRenderAvatars()) { + _shouldRender = shouldRenderAvatars; + render::ScenePointer scene = qApp->getMain3DScene(); + render::Transaction transaction; + if (_shouldRender) { for (auto avatarData : _avatarHash) { auto avatar = std::static_pointer_cast(avatarData); - render::ScenePointer scene = qApp->getMain3DScene(); - render::Transaction transaction; avatar->addToScene(avatar, scene, transaction); - scene->enqueueTransaction(transaction); } } else { for (auto avatarData : _avatarHash) { auto avatar = std::static_pointer_cast(avatarData); - render::ScenePointer scene = qApp->getMain3DScene(); - render::Transaction transaction; avatar->removeFromScene(avatar, scene, transaction); - scene->enqueueTransaction(transaction); } } + scene->enqueueTransaction(transaction); } - AvatarSharedPointer AvatarManager::getAvatarBySessionID(const QUuid& sessionID) const { if (sessionID == AVATAR_SELF_ID || sessionID == _myAvatar->getSessionUUID()) { return _myAvatar; diff --git a/interface/src/avatar/AvatarManager.h b/interface/src/avatar/AvatarManager.h index 228fe7e92f..3f053f43a6 100644 --- a/interface/src/avatar/AvatarManager.h +++ b/interface/src/avatar/AvatarManager.h @@ -91,6 +91,9 @@ public slots: void setShouldShowReceiveStats(bool shouldShowReceiveStats) { _shouldShowReceiveStats = shouldShowReceiveStats; } void updateAvatarRenderStatus(bool shouldRenderAvatars); +protected slots: + void processAvatarDataPacket(QSharedPointer message, SharedNodePointer sendingNode) override; + private: explicit AvatarManager(QObject* parent = 0); explicit AvatarManager(const AvatarManager& other); @@ -100,12 +103,6 @@ private: AvatarSharedPointer newSharedAvatar() override; void handleRemovedAvatar(const AvatarSharedPointer& removedAvatar, KillAvatarReason removalReason = KillAvatarReason::NoReason) override; - /* TODO: maintain these lists - QVector _avatarsToRemoveFromScene; - QVector _avatarsToAddToScene; - QVector _avatarsToRemoveFromPhysicsEngine; - QVector _avatarsToAddToPhysicsEngine; - */ QVector _avatarsToFade; SetOfAvatarMotionStates _motionStatesThatMightUpdate; @@ -125,6 +122,7 @@ private: int _numAvatarsUpdated { 0 }; int _numAvatarsNotUpdated { 0 }; float _avatarSimulationTime { 0.0f }; + bool _shouldRender { true }; }; Q_DECLARE_METATYPE(AvatarManager::LocalLight) diff --git a/interface/src/avatar/MyAvatar.cpp b/interface/src/avatar/MyAvatar.cpp index d1edf9d44e..2a616a7a2a 100644 --- a/interface/src/avatar/MyAvatar.cpp +++ b/interface/src/avatar/MyAvatar.cpp @@ -54,6 +54,7 @@ #include "DebugDraw.h" #include "EntityEditPacketSender.h" #include "MovingEntitiesOperator.h" +#include "SceneScriptingInterface.h" using namespace std; @@ -1634,7 +1635,7 @@ void MyAvatar::postUpdate(float deltaTime) { Avatar::postUpdate(deltaTime); render::ScenePointer scene = qApp->getMain3DScene(); - if (_skeletonModel->initWhenReady(scene)) { + if (DependencyManager::get()->shouldRenderAvatars() && _skeletonModel->initWhenReady(scene)) { initHeadBones(); _skeletonModel->setCauterizeBoneSet(_headBoneSet); _fstAnimGraphOverrideUrl = _skeletonModel->getGeometry()->getAnimGraphOverrideUrl(); diff --git a/libraries/avatars/src/AvatarData.cpp b/libraries/avatars/src/AvatarData.cpp index a1ea103edb..23aa0cd811 100644 --- a/libraries/avatars/src/AvatarData.cpp +++ b/libraries/avatars/src/AvatarData.cpp @@ -123,6 +123,7 @@ void AvatarData::setTargetScale(float targetScale) { if (_targetScale != newValue) { _targetScale = newValue; _scaleChanged = usecTimestampNow(); + _avatarScaleChanged = _scaleChanged; } } diff --git a/libraries/avatars/src/AvatarHashMap.cpp b/libraries/avatars/src/AvatarHashMap.cpp index 48e5d673c9..8708030190 100644 --- a/libraries/avatars/src/AvatarHashMap.cpp +++ b/libraries/avatars/src/AvatarHashMap.cpp @@ -80,13 +80,10 @@ AvatarSharedPointer AvatarHashMap::addAvatar(const QUuid& sessionUUID, const QWe AvatarSharedPointer AvatarHashMap::newOrExistingAvatar(const QUuid& sessionUUID, const QWeakPointer& mixerWeakPointer) { QWriteLocker locker(&_hashLock); - auto avatar = _avatarHash.value(sessionUUID); - if (!avatar) { avatar = addAvatar(sessionUUID, mixerWeakPointer); } - return avatar; } @@ -103,27 +100,33 @@ void AvatarHashMap::processAvatarDataPacket(QSharedPointer mess // enumerate over all of the avatars in this packet // only add them if mixerWeakPointer points to something (meaning that mixer is still around) while (message->getBytesLeftToRead()) { - QUuid sessionUUID = QUuid::fromRfc4122(message->readWithoutCopy(NUM_BYTES_RFC4122_UUID)); + parseAvatarData(message, sendingNode); + } +} - int positionBeforeRead = message->getPosition(); +AvatarSharedPointer AvatarHashMap::parseAvatarData(QSharedPointer message, SharedNodePointer sendingNode) { + QUuid sessionUUID = QUuid::fromRfc4122(message->readWithoutCopy(NUM_BYTES_RFC4122_UUID)); - QByteArray byteArray = message->readWithoutCopy(message->getBytesLeftToRead()); + int positionBeforeRead = message->getPosition(); - // make sure this isn't our own avatar data or for a previously ignored node - auto nodeList = DependencyManager::get(); + QByteArray byteArray = message->readWithoutCopy(message->getBytesLeftToRead()); - if (sessionUUID != _lastOwnerSessionUUID && (!nodeList->isIgnoringNode(sessionUUID) || nodeList->getRequestsDomainListData())) { - auto avatar = newOrExistingAvatar(sessionUUID, sendingNode); + // make sure this isn't our own avatar data or for a previously ignored node + auto nodeList = DependencyManager::get(); - // have the matching (or new) avatar parse the data from the packet - int bytesRead = avatar->parseDataFromBuffer(byteArray); - message->seek(positionBeforeRead + bytesRead); - } else { - // create a dummy AvatarData class to throw this data on the ground - AvatarData dummyData; - int bytesRead = dummyData.parseDataFromBuffer(byteArray); - message->seek(positionBeforeRead + bytesRead); - } + if (sessionUUID != _lastOwnerSessionUUID && (!nodeList->isIgnoringNode(sessionUUID) || nodeList->getRequestsDomainListData())) { + auto avatar = newOrExistingAvatar(sessionUUID, sendingNode); + + // have the matching (or new) avatar parse the data from the packet + int bytesRead = avatar->parseDataFromBuffer(byteArray); + message->seek(positionBeforeRead + bytesRead); + return avatar; + } else { + // create a dummy AvatarData class to throw this data on the ground + AvatarData dummyData; + int bytesRead = dummyData.parseDataFromBuffer(byteArray); + message->seek(positionBeforeRead + bytesRead); + return std::make_shared(); } } diff --git a/libraries/avatars/src/AvatarHashMap.h b/libraries/avatars/src/AvatarHashMap.h index 104ac83261..346cd36b60 100644 --- a/libraries/avatars/src/AvatarHashMap.h +++ b/libraries/avatars/src/AvatarHashMap.h @@ -49,11 +49,11 @@ signals: public slots: bool isAvatarInRange(const glm::vec3 & position, const float range); - -private slots: + +protected slots: void sessionUUIDChanged(const QUuid& sessionUUID, const QUuid& oldUUID); - - void processAvatarDataPacket(QSharedPointer message, SharedNodePointer sendingNode); + + virtual void processAvatarDataPacket(QSharedPointer message, SharedNodePointer sendingNode); void processAvatarIdentityPacket(QSharedPointer message, SharedNodePointer sendingNode); void processKillAvatar(QSharedPointer message, SharedNodePointer sendingNode); void processExitingSpaceBubble(QSharedPointer message, SharedNodePointer sendingNode); @@ -61,12 +61,13 @@ private slots: protected: AvatarHashMap(); + virtual AvatarSharedPointer parseAvatarData(QSharedPointer message, SharedNodePointer sendingNode); virtual AvatarSharedPointer newSharedAvatar(); virtual AvatarSharedPointer addAvatar(const QUuid& sessionUUID, const QWeakPointer& mixerWeakPointer); AvatarSharedPointer newOrExistingAvatar(const QUuid& sessionUUID, const QWeakPointer& mixerWeakPointer); virtual AvatarSharedPointer findAvatar(const QUuid& sessionUUID) const; // uses a QReadLocker on the hashLock virtual void removeAvatar(const QUuid& sessionUUID, KillAvatarReason removalReason = KillAvatarReason::NoReason); - + virtual void handleRemovedAvatar(const AvatarSharedPointer& removedAvatar, KillAvatarReason removalReason = KillAvatarReason::NoReason); AvatarHash _avatarHash; From 2f18e51f6ba20740c4ab0e56755a39214540db8e Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Thu, 6 Apr 2017 10:52:48 -0700 Subject: [PATCH 4/8] lock mutex around fading avatar list --- interface/src/avatar/AvatarManager.cpp | 24 +++++++++++++++--------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/interface/src/avatar/AvatarManager.cpp b/interface/src/avatar/AvatarManager.cpp index 0d9cd13d24..7f4bc1c953 100644 --- a/interface/src/avatar/AvatarManager.cpp +++ b/interface/src/avatar/AvatarManager.cpp @@ -259,13 +259,16 @@ void AvatarManager::updateOtherAvatars(float deltaTime) { } if (_shouldRender) { - QVector::iterator itr = _avatarsToFade.begin(); - while (itr != _avatarsToFade.end() && usecTimestampNow() > updateExpiry) { - auto avatar = std::static_pointer_cast(*itr); - avatar->animateScaleChanges(deltaTime); - avatar->simulate(deltaTime, true); - avatar->updateRenderItem(transaction); - ++itr; + if (!_avatarsToFade.empty()) { + QReadLocker lock(&_hashLock); + QVector::iterator itr = _avatarsToFade.begin(); + while (itr != _avatarsToFade.end() && usecTimestampNow() > updateExpiry) { + auto avatar = std::static_pointer_cast(*itr); + avatar->animateScaleChanges(deltaTime); + avatar->simulate(deltaTime, true); + avatar->updateRenderItem(transaction); + ++itr; + } } qApp->getMain3DScene()->enqueueTransaction(transaction); } @@ -287,18 +290,21 @@ void AvatarManager::postUpdate(float deltaTime) { } void AvatarManager::simulateAvatarFades(float deltaTime) { - QVector::iterator itr = _avatarsToFade.begin(); + if (_avatarsToFade.empty()) { + return; + } const float SHRINK_RATE = 0.15f; const float MIN_FADE_SCALE = MIN_AVATAR_SCALE; + QReadLocker locker(&_hashLock); + QVector::iterator itr = _avatarsToFade.begin(); while (itr != _avatarsToFade.end()) { auto avatar = std::static_pointer_cast(*itr); avatar->setTargetScale(avatar->getUniformScale() * SHRINK_RATE); avatar->animateScaleChanges(deltaTime); if (avatar->getTargetScale() <= MIN_FADE_SCALE) { // fading to zero is such a rare event we push unique transaction for each one - removeAvatar(avatar->getID()); if (avatar->isInScene()) { render::ScenePointer scene = qApp->getMain3DScene(); render::Transaction transaction; From 6d627aa9bc44916a647c4c6baea4777fb5d3c197 Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Thu, 6 Apr 2017 11:19:58 -0700 Subject: [PATCH 5/8] don't remove other avatars on sign-out --- interface/src/avatar/AvatarManager.cpp | 30 +++++++++++++++++--------- 1 file changed, 20 insertions(+), 10 deletions(-) diff --git a/interface/src/avatar/AvatarManager.cpp b/interface/src/avatar/AvatarManager.cpp index 7f4bc1c953..73e247bdd1 100644 --- a/interface/src/avatar/AvatarManager.cpp +++ b/interface/src/avatar/AvatarManager.cpp @@ -386,22 +386,32 @@ void AvatarManager::handleRemovedAvatar(const AvatarSharedPointer& removedAvatar } void AvatarManager::clearOtherAvatars() { - // clear any avatars that came from an avatar-mixer - QWriteLocker locker(&_hashLock); + // Remove other avatars from the world but don't actually remove them from _avatarHash + // each will either be removed on timeout or will re-added to the world on receipt of update. + render::ScenePointer scene = qApp->getMain3DScene(); + render::PendingChanges pendingChanges; + QReadLocker locker(&_hashLock); AvatarHash::iterator avatarIterator = _avatarHash.begin(); while (avatarIterator != _avatarHash.end()) { auto avatar = std::static_pointer_cast(avatarIterator.value()); - if (avatar == _myAvatar || !avatar->isInitialized()) { - // don't remove myAvatar or uninitialized avatars from the list - ++avatarIterator; - } else { - auto removedAvatar = avatarIterator.value(); - avatarIterator = _avatarHash.erase(avatarIterator); - - handleRemovedAvatar(removedAvatar); + if (avatar != _myAvatar) { + //auto removedAvatar = avatarIterator.value(); + //avatarIterator = _avatarHash.erase(avatarIterator); + //handleRemovedAvatar(removedAvatar); + if (avatar->isInScene()) { + avatar->removeFromScene(avatar, scene, pendingChanges); + } + AvatarMotionState* motionState = avatar->getMotionState(); + if (motionState) { + _motionStatesThatMightUpdate.remove(motionState); + _motionStatesToAddToPhysics.remove(motionState); + _motionStatesToRemoveFromPhysics.push_back(motionState); + } } + ++avatarIterator; } + scene->enqueuePendingChanges(pendingChanges); _myAvatar->clearLookAtTargetAvatar(); } From 1fb88a03152d72aa507f025abfe92a4669cb0fc9 Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Thu, 6 Apr 2017 12:59:25 -0700 Subject: [PATCH 6/8] add a helpful comment --- interface/src/avatar/Avatar.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/interface/src/avatar/Avatar.cpp b/interface/src/avatar/Avatar.cpp index 988d34e554..6ad389ccf6 100644 --- a/interface/src/avatar/Avatar.cpp +++ b/interface/src/avatar/Avatar.cpp @@ -192,6 +192,8 @@ void Avatar::animateScaleChanges(float deltaTime) { _isAnimatingScale = false; } setScale(glm::vec3(animatedScale)); // avatar scale is uniform + + // flag the joints as having changed for force update to RenderItem _hasNewJointData = true; // TODO: rebuilding the shape constantly is somehwat expensive. From 8773497f7e157052a943243178f2cf6ed578101d Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Thu, 6 Apr 2017 12:59:39 -0700 Subject: [PATCH 7/8] remvoe commented out cruft --- interface/src/avatar/AvatarManager.cpp | 3 --- 1 file changed, 3 deletions(-) diff --git a/interface/src/avatar/AvatarManager.cpp b/interface/src/avatar/AvatarManager.cpp index 73e247bdd1..15d1c60b5d 100644 --- a/interface/src/avatar/AvatarManager.cpp +++ b/interface/src/avatar/AvatarManager.cpp @@ -396,9 +396,6 @@ void AvatarManager::clearOtherAvatars() { while (avatarIterator != _avatarHash.end()) { auto avatar = std::static_pointer_cast(avatarIterator.value()); if (avatar != _myAvatar) { - //auto removedAvatar = avatarIterator.value(); - //avatarIterator = _avatarHash.erase(avatarIterator); - //handleRemovedAvatar(removedAvatar); if (avatar->isInScene()) { avatar->removeFromScene(avatar, scene, pendingChanges); } From 92495d9622e9a9f787a130f05d29a3b471978c98 Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Thu, 6 Apr 2017 15:22:04 -0700 Subject: [PATCH 8/8] delete in proper order on shutdown --- interface/src/Application.cpp | 3 +- interface/src/avatar/AvatarManager.cpp | 50 ++++++++++++++++---------- interface/src/avatar/AvatarManager.h | 2 +- 3 files changed, 34 insertions(+), 21 deletions(-) diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp index 342d325f58..87fcc77704 100644 --- a/interface/src/Application.cpp +++ b/interface/src/Application.cpp @@ -1800,10 +1800,11 @@ Application::~Application() { _physicsEngine->setCharacterController(nullptr); // remove avatars from physics engine - DependencyManager::get()->clearAllAvatars(); + DependencyManager::get()->clearOtherAvatars(); VectorOfMotionStates motionStates; DependencyManager::get()->getObjectsToRemoveFromPhysics(motionStates); _physicsEngine->removeObjects(motionStates); + DependencyManager::get()->deleteAllAvatars(); DependencyManager::destroy(); DependencyManager::destroy(); diff --git a/interface/src/avatar/AvatarManager.cpp b/interface/src/avatar/AvatarManager.cpp index 15d1c60b5d..f1170a7085 100644 --- a/interface/src/avatar/AvatarManager.cpp +++ b/interface/src/avatar/AvatarManager.cpp @@ -309,7 +309,9 @@ void AvatarManager::simulateAvatarFades(float deltaTime) { render::ScenePointer scene = qApp->getMain3DScene(); render::Transaction transaction; avatar->removeFromScene(*itr, scene, transaction); - scene->enqueueTransaction(transaction); + if (scene) { + scene->enqueueTransaction(transaction); + } } // only remove from _avatarsToFade if we're sure its motionState has been removed from PhysicsEngine @@ -340,18 +342,22 @@ void AvatarManager::processAvatarDataPacket(QSharedPointer mess auto avatar = std::static_pointer_cast(avatarData); if (avatar->isInScene()) { if (!_shouldRender) { - // rare transition so we process pending changes immediately + // rare transition so we process the transaction immediately render::ScenePointer scene = qApp->getMain3DScene(); - render::Transaction transaction; - avatar->removeFromScene(avatar, scene, transaction); - scene->enqueueTransaction(transaction); + if (scene) { + render::Transaction transaction; + avatar->removeFromScene(avatar, scene, transaction); + scene->enqueueTransaction(transaction); + } } } else if (_shouldRender) { - // very rare transition so we process pending changes immediately + // very rare transition so we process the transaction immediately render::ScenePointer scene = qApp->getMain3DScene(); - render::Transaction transaction; - avatar->addToScene(avatar, scene, transaction); - scene->enqueueTransaction(transaction); + if (scene) { + render::Transaction transaction; + avatar->addToScene(avatar, scene, transaction); + scene->enqueueTransaction(transaction); + } } } } @@ -389,7 +395,7 @@ void AvatarManager::clearOtherAvatars() { // Remove other avatars from the world but don't actually remove them from _avatarHash // each will either be removed on timeout or will re-added to the world on receipt of update. render::ScenePointer scene = qApp->getMain3DScene(); - render::PendingChanges pendingChanges; + render::Transaction transaction; QReadLocker locker(&_hashLock); AvatarHash::iterator avatarIterator = _avatarHash.begin(); @@ -397,7 +403,7 @@ void AvatarManager::clearOtherAvatars() { auto avatar = std::static_pointer_cast(avatarIterator.value()); if (avatar != _myAvatar) { if (avatar->isInScene()) { - avatar->removeFromScene(avatar, scene, pendingChanges); + avatar->removeFromScene(avatar, scene, transaction); } AvatarMotionState* motionState = avatar->getMotionState(); if (motionState) { @@ -408,16 +414,20 @@ void AvatarManager::clearOtherAvatars() { } ++avatarIterator; } - scene->enqueuePendingChanges(pendingChanges); + if (scene) { + scene->enqueueTransaction(transaction); + } _myAvatar->clearLookAtTargetAvatar(); } -void AvatarManager::clearAllAvatars() { - clearOtherAvatars(); - - QWriteLocker locker(&_hashLock); - - handleRemovedAvatar(_myAvatar); +void AvatarManager::deleteAllAvatars() { + QReadLocker locker(&_hashLock); + AvatarHash::iterator avatarIterator = _avatarHash.begin(); + while (avatarIterator != _avatarHash.end()) { + auto avatar = std::static_pointer_cast(avatarIterator.value()); + avatarIterator = _avatarHash.erase(avatarIterator); + avatar->die(); + } } void AvatarManager::setLocalLights(const QVector& localLights) { @@ -525,7 +535,9 @@ void AvatarManager::updateAvatarRenderStatus(bool shouldRenderAvatars) { avatar->removeFromScene(avatar, scene, transaction); } } - scene->enqueueTransaction(transaction); + if (scene) { + scene->enqueueTransaction(transaction); + } } AvatarSharedPointer AvatarManager::getAvatarBySessionID(const QUuid& sessionID) const { diff --git a/interface/src/avatar/AvatarManager.h b/interface/src/avatar/AvatarManager.h index 3f053f43a6..c8229115bd 100644 --- a/interface/src/avatar/AvatarManager.h +++ b/interface/src/avatar/AvatarManager.h @@ -53,7 +53,7 @@ public: void postUpdate(float deltaTime); void clearOtherAvatars(); - void clearAllAvatars(); + void deleteAllAvatars(); bool shouldShowReceiveStats() const { return _shouldShowReceiveStats; }