diff --git a/interface/src/avatar/AvatarManager.cpp b/interface/src/avatar/AvatarManager.cpp index 5f0ac435e0..9783590b05 100644 --- a/interface/src/avatar/AvatarManager.cpp +++ b/interface/src/avatar/AvatarManager.cpp @@ -77,7 +77,10 @@ AvatarManager::AvatarManager(QObject* parent) : void AvatarManager::init() { _myAvatar->init(); - _avatarHash.insert(MY_AVATAR_KEY, _myAvatar); + { + QWriteLocker locker(&_hashLock); + _avatarHash.insert(MY_AVATAR_KEY, _myAvatar); + } connect(DependencyManager::get().data(), &SceneScriptingInterface::shouldRenderAvatarsChanged, this, &AvatarManager::updateAvatarRenderStatus, Qt::QueuedConnection); @@ -127,6 +130,7 @@ void AvatarManager::updateOtherAvatars(float deltaTime) { } else if (avatar->shouldDie()) { removeAvatarMotionState(avatar); _avatarFades.push_back(avatarIterator.value()); + QWriteLocker locker(&_hashLock); avatarIterator = _avatarHash.erase(avatarIterator); } else { avatar->startUpdate(); @@ -202,6 +206,7 @@ void AvatarManager::removeAvatar(const QUuid& sessionUUID) { if (avatar != _myAvatar && avatar->isInitialized()) { removeAvatarMotionState(avatar); _avatarFades.push_back(avatarIterator.value()); + QWriteLocker locker(&_hashLock); _avatarHash.erase(avatarIterator); } } @@ -218,6 +223,7 @@ void AvatarManager::clearOtherAvatars() { } else { removeAvatarMotionState(avatar); _avatarFades.push_back(avatarIterator.value()); + QWriteLocker locker(&_hashLock); avatarIterator = _avatarHash.erase(avatarIterator); } } @@ -349,5 +355,6 @@ AvatarSharedPointer AvatarManager::getAvatarBySessionID(const QUuid& sessionID) if (sessionID == _myAvatar->getSessionUUID()) { return std::static_pointer_cast(_myAvatar); } - return getAvatarHash()[sessionID]; + QReadLocker locker(&_hashLock); + return _avatarHash[sessionID]; } diff --git a/interface/src/avatar/MyAvatar.cpp b/interface/src/avatar/MyAvatar.cpp index f7fad2bd2b..eba32e94b2 100644 --- a/interface/src/avatar/MyAvatar.cpp +++ b/interface/src/avatar/MyAvatar.cpp @@ -1020,7 +1020,11 @@ void MyAvatar::updateLookAtTargetAvatar() { const float KEEP_LOOKING_AT_CURRENT_ANGLE_FACTOR = 1.3f; const float GREATEST_LOOKING_AT_DISTANCE = 10.0f; - foreach (const AvatarSharedPointer& avatarPointer, DependencyManager::get()->getAvatarHash()) { + AvatarHash hash; + DependencyManager::get()->withAvatarHash([&] (const AvatarHash& locked) { + hash = locked; // make a shallow copy and operate on that, to minimize lock time + }); + foreach (const AvatarSharedPointer& avatarPointer, hash) { auto avatar = static_pointer_cast(avatarPointer); bool isCurrentTarget = avatar->getIsLookAtTarget(); float distanceTo = glm::length(avatar->getHead()->getEyePosition() - cameraPosition); @@ -1052,7 +1056,6 @@ void MyAvatar::updateLookAtTargetAvatar() { glm::vec3 humanLeftEye = humanSystem->getPosition() + (humanSystem->getOrientation() * leftEyeHeadLocal); glm::vec3 humanRightEye = humanSystem->getPosition() + (humanSystem->getOrientation() * rightEyeHeadLocal); - // First find out where (in world space) the person is looking relative to that bridge-of-the-avatar point. // (We will be adding that offset to the camera position, after making some other adjustments.) glm::vec3 gazeOffset = lookAtPosition - getHead()->getEyePosition(); @@ -1064,14 +1067,14 @@ void MyAvatar::updateLookAtTargetAvatar() { // If the camera is also not oriented with the head, adjust by getting the offset in head-space... /* Not needed (i.e., code is a no-op), but I'm leaving the example code here in case something like this is needed someday. - glm::quat avatarHeadOrientation = getHead()->getOrientation(); - glm::vec3 gazeOffsetLocalToHead = glm::inverse(avatarHeadOrientation) * gazeOffset; - // ... and treat that as though it were in camera space, bringing it back to world space. - // But camera is fudged to make the picture feel like the avatar's orientation. - glm::quat humanOrientation = humanSystem->getOrientation(); // or just avatar getOrienation() ? - gazeOffset = humanOrientation * gazeOffsetLocalToHead; - glm::vec3 corrected = humanSystem->getPosition() + gazeOffset; - */ + glm::quat avatarHeadOrientation = getHead()->getOrientation(); + glm::vec3 gazeOffsetLocalToHead = glm::inverse(avatarHeadOrientation) * gazeOffset; + // ... and treat that as though it were in camera space, bringing it back to world space. + // But camera is fudged to make the picture feel like the avatar's orientation. + glm::quat humanOrientation = humanSystem->getOrientation(); // or just avatar getOrienation() ? + gazeOffset = humanOrientation * gazeOffsetLocalToHead; + glm::vec3 corrected = humanSystem->getPosition() + gazeOffset; + */ // And now we can finally add that offset to the camera. glm::vec3 corrected = qApp->getViewFrustum()->getPosition() + gazeOffset; diff --git a/libraries/avatars/src/AvatarHashMap.cpp b/libraries/avatars/src/AvatarHashMap.cpp index 520bb34887..3f1e668cd0 100644 --- a/libraries/avatars/src/AvatarHashMap.cpp +++ b/libraries/avatars/src/AvatarHashMap.cpp @@ -22,7 +22,12 @@ AvatarHashMap::AvatarHashMap() { connect(DependencyManager::get().data(), &NodeList::uuidChanged, this, &AvatarHashMap::sessionUUIDChanged); } +void AvatarHashMap::withAvatarHash(std::function callback) { + QReadLocker locker(&_hashLock); + callback(_avatarHash); +} bool AvatarHashMap::isAvatarInRange(const glm::vec3& position, const float range) { + QReadLocker locker(&_hashLock); foreach(const AvatarSharedPointer& sharedAvatar, _avatarHash) { glm::vec3 avatarPosition = sharedAvatar->getPosition(); float distance = glm::distance(avatarPosition, position); @@ -43,6 +48,7 @@ AvatarSharedPointer AvatarHashMap::addAvatar(const QUuid& sessionUUID, const QWe AvatarSharedPointer avatar = newSharedAvatar(); avatar->setSessionUUID(sessionUUID); avatar->setOwningAvatarMixer(mixerWeakPointer); + QWriteLocker locker(&_hashLock); _avatarHash.insert(sessionUUID, avatar); return avatar; @@ -134,6 +140,7 @@ void AvatarHashMap::processKillAvatar(QSharedPointer packet, SharedNod } void AvatarHashMap::removeAvatar(const QUuid& sessionUUID) { + QWriteLocker locker(&_hashLock); _avatarHash.remove(sessionUUID); } diff --git a/libraries/avatars/src/AvatarHashMap.h b/libraries/avatars/src/AvatarHashMap.h index 804233b76a..93364eb1ef 100644 --- a/libraries/avatars/src/AvatarHashMap.h +++ b/libraries/avatars/src/AvatarHashMap.h @@ -16,6 +16,7 @@ #include #include +#include #include #include @@ -30,7 +31,7 @@ class AvatarHashMap : public QObject, public Dependency { SINGLETON_DEPENDENCY public: - const AvatarHash& getAvatarHash() { return _avatarHash; } + void withAvatarHash(std::function); int size() { return _avatarHash.size(); } public slots: @@ -52,6 +53,9 @@ protected: virtual void removeAvatar(const QUuid& sessionUUID); AvatarHash _avatarHash; + // "Case-based safety": Most access to the _avatarHash is on the same thread. Write access is protected by a write-lock. + // If you read from a different thread, you must read-lock the _hashLock. (Scripted write access is not supported). + QReadWriteLock _hashLock; private: QUuid _lastOwnerSessionUUID;