Merge pull request #2 from SamGondelman/otherAvatars

Avatar mixer and manager perf improvements and cleanup

assignment-client/src/avatars/AvatarMixerSlave.cpp

@@ -261,8 +261,6 @@ void AvatarMixerSlave::broadcastAvatarDataToAgent(const SharedNodePointer& node)
     // FIXME - find a way to not send the sessionID for every avatar
     int minimumBytesPerAvatar = AvatarDataPacket::AVATAR_HAS_FLAGS_SIZE + NUM_BYTES_RFC4122_UUID;
 
-    int overBudgetAvatars = 0;
-
     // keep track of the number of other avatars held back in this frame
     int numAvatarsHeldBack = 0;
 
@@ -287,7 +285,7 @@ void AvatarMixerSlave::broadcastAvatarDataToAgent(const SharedNodePointer& node)
 
     // compute node bounding box
     const float MY_AVATAR_BUBBLE_EXPANSION_FACTOR = 4.0f; // magic number determined emperically
-    AABox nodeBox = computeBubbleBox(avatar, MY_AVATAR_BUBBLE_EXPANSION_FACTOR );
+    AABox nodeBox = computeBubbleBox(avatar, MY_AVATAR_BUBBLE_EXPANSION_FACTOR);
 
     class SortableAvatar: public PrioritySortUtil::Sortable {
     public:
@@ -296,13 +294,12 @@ void AvatarMixerSlave::broadcastAvatarDataToAgent(const SharedNodePointer& node)
             : _avatar(avatar), _node(avatarNode), _lastEncodeTime(lastEncodeTime) {}
         glm::vec3 getPosition() const override { return _avatar->getClientGlobalPosition(); }
         float getRadius() const override {
-            glm::vec3 nodeBoxHalfScale = 0.5f * _avatar->getGlobalBoundingBox().getScale();
+            glm::vec3 nodeBoxScale = _avatar->getGlobalBoundingBox().getScale();
-            return glm::max(nodeBoxHalfScale.x, glm::max(nodeBoxHalfScale.y, nodeBoxHalfScale.z));
+            return 0.5f * glm::max(nodeBoxScale.x, glm::max(nodeBoxScale.y, nodeBoxScale.z));
         }
         uint64_t getTimestamp() const override {
             return _lastEncodeTime;
         }
-        const AvatarData* getAvatar() const { return _avatar; }
         const Node* getNode() const { return _node; }
 
     private:
@@ -412,13 +409,19 @@ void AvatarMixerSlave::broadcastAvatarDataToAgent(const SharedNodePointer& node)
 
         assert(otherNode); // we can't have gotten here without the avatarData being a valid key in the map
 
+        AvatarData::AvatarDataDetail detail;
+
         // NOTE: Here's where we determine if we are over budget and drop to bare minimum data
         int minimRemainingAvatarBytes = minimumBytesPerAvatar * remainingAvatars;
         bool overBudget = (identityBytesSent + numAvatarDataBytes + minimRemainingAvatarBytes) > maxAvatarBytesPerFrame;
         if (overBudget) {
-            _stats.overBudgetAvatars += remainingAvatars + 1;
+            if (PALIsOpen) {
-            overBudgetAvatars += remainingAvatars + 1;
+                _stats.overBudgetAvatars++;
-            break;
+                detail = AvatarData::PALMinimum;
+            } else {
+                _stats.overBudgetAvatars += remainingAvatars;
+                break;
+            }
         }
 
         auto startAvatarDataPacking = chrono::high_resolution_clock::now();
@@ -439,23 +442,13 @@ void AvatarMixerSlave::broadcastAvatarDataToAgent(const SharedNodePointer& node)
         }
 
         // determine if avatar is in view which determines how much data to send
-        bool isInView = nodeData->otherAvatarInView(otherAvatar->getGlobalBoundingBox());
+        bool isInView = sortedAvatar.getPriority() > OUT_OF_VIEW_THRESHOLD;
 
-        // start a new segment in the PacketList for this avatar
+        if (!isInView) {
-        avatarPacketList->startSegment();
-
-        AvatarData::AvatarDataDetail detail;
-
-        if (overBudget) {
-            overBudgetAvatars++;
-            _stats.overBudgetAvatars++;
-            detail = PALIsOpen ? AvatarData::PALMinimum : AvatarData::NoData;
-        } else if (!isInView) {
             detail = PALIsOpen ? AvatarData::PALMinimum : AvatarData::MinimumData;
             nodeData->incrementAvatarOutOfView();
-        } else {
+        } else if (!overBudget) {
-            detail = distribution(generator) < AVATAR_SEND_FULL_UPDATE_RATIO
+            detail = distribution(generator) < AVATAR_SEND_FULL_UPDATE_RATIO ? AvatarData::SendAllData : AvatarData::CullSmallData;
-            ? AvatarData::SendAllData : AvatarData::CullSmallData;
             nodeData->incrementAvatarInView();
         }
 
@@ -503,8 +496,11 @@ void AvatarMixerSlave::broadcastAvatarDataToAgent(const SharedNodePointer& node)
         }
 
         if (includeThisAvatar) {
+            // start a new segment in the PacketList for this avatar
+            avatarPacketList->startSegment();
             numAvatarDataBytes += avatarPacketList->write(otherNode->getUUID().toRfc4122());
             numAvatarDataBytes += avatarPacketList->write(bytes);
+            avatarPacketList->endSegment();
 
             if (detail != AvatarData::NoData) {
                 _stats.numOthersIncluded++;
@@ -522,14 +518,13 @@ void AvatarMixerSlave::broadcastAvatarDataToAgent(const SharedNodePointer& node)
             // It would be nice if we could tweak its future sort priority to put it at the back of the list.
         }
 
-        avatarPacketList->endSegment();
-
         auto endAvatarDataPacking = chrono::high_resolution_clock::now();
         _stats.avatarDataPackingElapsedTime +=
             (quint64) chrono::duration_cast<chrono::microseconds>(endAvatarDataPacking - startAvatarDataPacking).count();
 
         // use helper to add any changed traits to our packet list
         traitBytesSent += addChangedTraitsToBulkPacket(nodeData, otherNodeData, *traitsPacketList);
+        remainingAvatars--;
     }
 
     quint64 startPacketSending = usecTimestampNow();

interface/src/avatar/AvatarManager.cpp

@@ -166,29 +166,29 @@ 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);
+        // lock the hash for read to check the size
-    if (_avatarHash.size() < 2 && _avatarsToFade.isEmpty()) {
+        QReadLocker lock(&_hashLock);
-        return;
+        if (_avatarHash.size() < 2 && _avatarsToFade.isEmpty()) {
+            return;
+        }
     }
-    lock.unlock();
 
     PerformanceTimer perfTimer("otherAvatars");
 
     class SortableAvatar: public PrioritySortUtil::Sortable {
     public:
         SortableAvatar() = delete;
-        SortableAvatar(const AvatarSharedPointer& avatar) : _avatar(avatar) {}
+        SortableAvatar(const std::shared_ptr<Avatar>& avatar) : _avatar(avatar) {}
         glm::vec3 getPosition() const override { return _avatar->getWorldPosition(); }
-        float getRadius() const override { return std::static_pointer_cast<Avatar>(_avatar)->getBoundingRadius(); }
+        float getRadius() const override { return _avatar->getBoundingRadius(); }
-        uint64_t getTimestamp() const override { return std::static_pointer_cast<Avatar>(_avatar)->getLastRenderUpdateTime(); }
+        uint64_t getTimestamp() const override { return _avatar->getLastRenderUpdateTime(); }
-        AvatarSharedPointer getAvatar() const { return _avatar; }
+        std::shared_ptr<Avatar> getAvatar() const { return _avatar; }
     private:
-        AvatarSharedPointer _avatar;
+        std::shared_ptr<Avatar> _avatar;
     };
 
     auto avatarMap = getHashCopy();
-    AvatarHash::iterator itr = avatarMap.begin();
 
     const auto& views = qApp->getConicalViews();
     PrioritySortUtil::PriorityQueue<SortableAvatar> sortedAvatars(views,
@@ -197,22 +197,24 @@ void AvatarManager::updateOtherAvatars(float deltaTime) {
             AvatarData::_avatarSortCoefficientAge);
     sortedAvatars.reserve(avatarMap.size() - 1); // don't include MyAvatar
 
-    // sort
+    // Build vector and compute priorities
+    auto nodeList = DependencyManager::get<NodeList>();
+    AvatarHash::iterator itr = avatarMap.begin();
     while (itr != avatarMap.end()) {
         const auto& avatar = std::static_pointer_cast<Avatar>(*itr);
         // DO NOT update _myAvatar!  Its update has already been done earlier in the main loop.
         // DO NOT update or fade out uninitialized Avatars
-        if (avatar != _myAvatar && avatar->isInitialized()) {
+        if (avatar != _myAvatar && avatar->isInitialized() && !nodeList->isPersonalMutingNode(avatar->getID())) {
             sortedAvatars.push(SortableAvatar(avatar));
         }
         ++itr;
     }
+    // Sort
     const auto& sortedAvatarVector = sortedAvatars.getSortedVector();
 
     // process in sorted order
     uint64_t startTime = usecTimestampNow();
-    const uint64_t UPDATE_BUDGET = 2000; // usec
+    uint64_t updateExpiry = startTime + MAX_UPDATE_AVATARS_TIME_BUDGET;
-    uint64_t updateExpiry = startTime + UPDATE_BUDGET;
     int numAvatarsUpdated = 0;
     int numAVatarsNotUpdated = 0;
 
@@ -231,18 +233,12 @@ void AvatarManager::updateOtherAvatars(float deltaTime) {
             avatar->updateOrbPosition();
         }
 
-        bool ignoring = DependencyManager::get<NodeList>()->isPersonalMutingNode(avatar->getID());
-        if (ignoring) {
-            continue;
-        }
-
         // for ALL avatars...
         if (_shouldRender) {
             avatar->ensureInScene(avatar, qApp->getMain3DScene());
         }
         avatar->animateScaleChanges(deltaTime);
 
-        const float OUT_OF_VIEW_THRESHOLD = 0.5f * AvatarData::OUT_OF_VIEW_PENALTY;
         uint64_t now = usecTimestampNow();
         if (now < updateExpiry) {
             // we're within budget
@@ -263,7 +259,7 @@ void AvatarManager::updateOtherAvatars(float deltaTime) {
             // no time to simulate, but we take the time to count how many were tragically missed
             while (it != sortedAvatarVector.end()) {
                 const SortableAvatar& newSortData = *it;
-                const auto newAvatar = std::static_pointer_cast<Avatar>(newSortData.getAvatar());
+                const auto& newAvatar = newSortData.getAvatar();
                 bool inView = newSortData.getPriority() > OUT_OF_VIEW_THRESHOLD;
                 // Once we reach an avatar that's not in view, all avatars after it will also be out of view
                 if (!inView) {

libraries/avatars/src/AvatarData.cpp

@@ -837,7 +837,6 @@ const unsigned char* unpackFauxJoint(const unsigned char* sourceBuffer, ThreadSa
 
 // read data in packet starting at byte offset and return number of bytes parsed
 int AvatarData::parseDataFromBuffer(const QByteArray& buffer) {
-
     // lazily allocate memory for HeadData in case we're not an Avatar instance
     lazyInitHeadData();
 
@@ -889,7 +888,7 @@ int AvatarData::parseDataFromBuffer(const QByteArray& buffer) {
         auto newValue = glm::vec3(data->globalPosition[0], data->globalPosition[1], data->globalPosition[2]) + offset;
         if (_globalPosition != newValue) {
             _globalPosition = newValue;
-            _globalPositionChanged = usecTimestampNow();
+            _globalPositionChanged = now;
         }
         sourceBuffer += sizeof(AvatarDataPacket::AvatarGlobalPosition);
         int numBytesRead = sourceBuffer - startSection;
@@ -913,11 +912,11 @@ int AvatarData::parseDataFromBuffer(const QByteArray& buffer) {
 
         if (_globalBoundingBoxDimensions != newDimensions) {
             _globalBoundingBoxDimensions = newDimensions;
-            _avatarBoundingBoxChanged = usecTimestampNow();
+            _avatarBoundingBoxChanged = now;
         }
         if (_globalBoundingBoxOffset != newOffset) {
             _globalBoundingBoxOffset = newOffset;
-            _avatarBoundingBoxChanged = usecTimestampNow();
+            _avatarBoundingBoxChanged = now;
         }
 
         sourceBuffer += sizeof(AvatarDataPacket::AvatarBoundingBox);
@@ -1018,7 +1017,7 @@ int AvatarData::parseDataFromBuffer(const QByteArray& buffer) {
         glm::mat4 sensorToWorldMatrix = createMatFromScaleQuatAndPos(glm::vec3(sensorToWorldScale), sensorToWorldQuat, sensorToWorldTrans);
         if (_sensorToWorldMatrixCache.get() != sensorToWorldMatrix) {
             _sensorToWorldMatrixCache.set(sensorToWorldMatrix);
-            _sensorToWorldMatrixChanged = usecTimestampNow();
+            _sensorToWorldMatrixChanged = now;
         }
         sourceBuffer += sizeof(AvatarDataPacket::SensorToWorldMatrix);
         int numBytesRead = sourceBuffer - startSection;
@@ -1075,7 +1074,7 @@ int AvatarData::parseDataFromBuffer(const QByteArray& buffer) {
         sourceBuffer += sizeof(AvatarDataPacket::AdditionalFlags);
 
         if (somethingChanged) {
-            _additionalFlagsChanged = usecTimestampNow();
+            _additionalFlagsChanged = now;
         }
         int numBytesRead = sourceBuffer - startSection;
         _additionalFlagsRate.increment(numBytesRead);
@@ -1095,7 +1094,7 @@ int AvatarData::parseDataFromBuffer(const QByteArray& buffer) {
         if ((getParentID() != newParentID) || (getParentJointIndex() != parentInfo->parentJointIndex)) {
             SpatiallyNestable::setParentID(newParentID);
             SpatiallyNestable::setParentJointIndex(parentInfo->parentJointIndex);
-            _parentChanged = usecTimestampNow();
+            _parentChanged = now;
         }
 
         int numBytesRead = sourceBuffer - startSection;
@@ -1144,8 +1143,6 @@ int AvatarData::parseDataFromBuffer(const QByteArray& buffer) {
         int numBytesRead = sourceBuffer - startSection;
         _faceTrackerRate.increment(numBytesRead);
         _faceTrackerUpdateRate.increment();
-    } else {
-        _headData->_blendshapeCoefficients.fill(0, _headData->_blendshapeCoefficients.size());
     }
 
     if (hasJointData) {
@@ -2820,8 +2817,6 @@ void RayToAvatarIntersectionResultFromScriptValue(const QScriptValue& object, Ra
     value.extraInfo = object.property("extraInfo").toVariant().toMap();
 }
 
-const float AvatarData::OUT_OF_VIEW_PENALTY = -10.0f;
-
 float AvatarData::_avatarSortCoefficientSize { 8.0f };
 float AvatarData::_avatarSortCoefficientCenter { 4.0f };
 float AvatarData::_avatarSortCoefficientAge { 1.0f };

libraries/avatars/src/AvatarData.h

@@ -1167,8 +1167,6 @@ public:
     // A method intended to be overriden by MyAvatar for polling orientation for network transmission.
     virtual glm::quat getOrientationOutbound() const;
 
-    static const float OUT_OF_VIEW_PENALTY;
-
     // TODO: remove this HACK once we settle on optimal sort coefficients
     // These coefficients exposed for fine tuning the sort priority for transfering new _jointData to the render pipeline.
     static float _avatarSortCoefficientSize;

libraries/avatars/src/AvatarHashMap.cpp

@@ -21,7 +21,6 @@
 #include "AvatarLogging.h"
 #include "AvatarTraits.h"
 
-
 void AvatarReplicas::addReplica(const QUuid& parentID, AvatarSharedPointer replica) {
     if (parentID == QUuid()) {
         return;

libraries/shared/src/PrioritySortUtil.h

@@ -18,6 +18,9 @@
 
 //   PrioritySortUtil is a helper for sorting 3D things relative to a ViewFrustum.
 
+const float OUT_OF_VIEW_PENALTY = -10.0f;
+const float OUT_OF_VIEW_THRESHOLD = 0.5f * OUT_OF_VIEW_PENALTY;
+
 namespace PrioritySortUtil {
 
     constexpr float DEFAULT_ANGULAR_COEF { 1.0f };
@@ -93,17 +96,16 @@ namespace PrioritySortUtil {
             const float MIN_RADIUS = 0.1f; // WORKAROUND for zero size objects (we still want them to sort by distance)
             float radius = glm::max(thing.getRadius(), MIN_RADIUS);
             // Other item's angle from view centre:
-            float cosineAngle = (glm::dot(offset, view.getDirection()) / distance);
+            float cosineAngle = glm::dot(offset, view.getDirection()) / distance;
             float age = float((_usecCurrentTime - thing.getTimestamp()) / USECS_PER_SECOND);
 
             // the "age" term accumulates at the sum of all weights
-            float angularSize = glm::max(radius, MIN_RADIUS) / distance;
+            float angularSize = radius / distance;
             float priority = (_angularWeight * angularSize + _centerWeight * cosineAngle) * (age + 1.0f) + _ageWeight * age;
 
             // decrement priority of things outside keyhole
             if (distance - radius > view.getRadius()) {
                 if (!view.intersects(offset, distance, radius)) {
-                    constexpr float OUT_OF_VIEW_PENALTY = -10.0f;
                     priority += OUT_OF_VIEW_PENALTY;
                 }
             }
@@ -122,5 +124,6 @@ namespace PrioritySortUtil {
   // for now we're keeping hard-coded sorted time budgets in one spot
 const uint64_t MAX_UPDATE_RENDERABLES_TIME_BUDGET = 2000; // usec
 const uint64_t MIN_SORTED_UPDATE_RENDERABLES_TIME_BUDGET = 1000; // usec
+const uint64_t MAX_UPDATE_AVATARS_TIME_BUDGET = 2000; // usec
 
 #endif // hifi_PrioritySortUtil_h