From e867ae86f441b6d14a4e6ccc521612e31f34c8d2 Mon Sep 17 00:00:00 2001 From: Simon Walton Date: Tue, 21 Aug 2018 14:44:55 -0700 Subject: [PATCH 01/44] Split readPendingDatagrams() into two slots --- libraries/networking/src/udt/Socket.cpp | 67 ++++++++++++++++++------- libraries/networking/src/udt/Socket.h | 14 ++++++ 2 files changed, 62 insertions(+), 19 deletions(-) diff --git a/libraries/networking/src/udt/Socket.cpp b/libraries/networking/src/udt/Socket.cpp index c378987cd0..d01b937b8b 100644 --- a/libraries/networking/src/udt/Socket.cpp +++ b/libraries/networking/src/udt/Socket.cpp @@ -37,6 +37,7 @@ Socket::Socket(QObject* parent, bool shouldChangeSocketOptions) : _shouldChangeSocketOptions(shouldChangeSocketOptions) { connect(&_udpSocket, &QUdpSocket::readyRead, this, &Socket::readPendingDatagrams); + connect(this, &Socket::pendingDatagrams, this, &Socket::processPendingDatagrams); // make sure we hear about errors and state changes from the underlying socket connect(&_udpSocket, SIGNAL(error(QAbstractSocket::SocketError)), @@ -315,55 +316,80 @@ void Socket::checkForReadyReadBackup() { } void Socket::readPendingDatagrams() { + int packetsRead = 0; + int packetSizeWithHeader = -1; - while (_udpSocket.hasPendingDatagrams() && (packetSizeWithHeader = _udpSocket.pendingDatagramSize()) != -1) { - - // we're reading a packet so re-start the readyRead backup timer - _readyReadBackupTimer->start(); - // grab a time point we can mark as the receive time of this packet auto receiveTime = p_high_resolution_clock::now(); - // setup a HifiSockAddr to read into - HifiSockAddr senderSockAddr; // setup a buffer to read the packet into auto buffer = std::unique_ptr(new char[packetSizeWithHeader]); + QHostAddress senderAddress; + quint16 senderPort; + // pull the datagram auto sizeRead = _udpSocket.readDatagram(buffer.get(), packetSizeWithHeader, - senderSockAddr.getAddressPointer(), senderSockAddr.getPortPointer()); + &senderAddress, &senderPort); - // save information for this packet, in case it is the one that sticks readyRead - _lastPacketSizeRead = sizeRead; - _lastPacketSockAddr = senderSockAddr; + // we either didn't pull anything for this packet or there was an error reading (this seems to trigger + // on windows even if there's not a packet available) - if (sizeRead <= 0) { - // we either didn't pull anything for this packet or there was an error reading (this seems to trigger - // on windows even if there's not a packet available) + if (packetSizeWithHeader < 0) { continue; } + _incomingDatagrams.push({ senderAddress, senderPort, packetSizeWithHeader, + std::move(buffer), receiveTime }); + ++packetsRead; + + } + + _maxDatagramsRead = std::max(_maxDatagramsRead, packetsRead); + emit pendingDatagrams(packetsRead); +} + +void Socket::processPendingDatagrams(int) { + // setup a HifiSockAddr to read into + HifiSockAddr senderSockAddr; + + while (!_incomingDatagrams.empty()) { + auto& datagram = _incomingDatagrams.front(); + senderSockAddr.setAddress(datagram._senderAddress); + senderSockAddr.setPort(datagram._senderPort); + int datagramSize = datagram._datagramLength; + auto receiveTime = datagram._receiveTime; + auto it = _unfilteredHandlers.find(senderSockAddr); if (it != _unfilteredHandlers.end()) { // we have a registered unfiltered handler for this HifiSockAddr - call that and return if (it->second) { - auto basePacket = BasePacket::fromReceivedPacket(std::move(buffer), packetSizeWithHeader, senderSockAddr); - basePacket->setReceiveTime(receiveTime); + auto basePacket = BasePacket::fromReceivedPacket(std::move(datagram._datagram), datagramSize, + senderSockAddr); + basePacket->setReceiveTime(datagram._receiveTime); it->second(std::move(basePacket)); } + _incomingDatagrams.pop(); continue; } + // we're reading a packet so re-start the readyRead backup timer + _readyReadBackupTimer->start(); + + // save information for this packet, in case it is the one that sticks readyRead + _lastPacketSizeRead = datagramSize; + _lastPacketSockAddr = senderSockAddr; + // check if this was a control packet or a data packet - bool isControlPacket = *reinterpret_cast(buffer.get()) & CONTROL_BIT_MASK; + bool isControlPacket = *reinterpret_cast(datagram._datagram.get()) & CONTROL_BIT_MASK; if (isControlPacket) { // setup a control packet from the data we just read - auto controlPacket = ControlPacket::fromReceivedPacket(std::move(buffer), packetSizeWithHeader, senderSockAddr); + auto controlPacket = ControlPacket::fromReceivedPacket(std::move(datagram._datagram), datagramSize, senderSockAddr); controlPacket->setReceiveTime(receiveTime); // move this control packet to the matching connection, if there is one @@ -375,7 +401,7 @@ void Socket::readPendingDatagrams() { } else { // setup a Packet from the data we just read - auto packet = Packet::fromReceivedPacket(std::move(buffer), packetSizeWithHeader, senderSockAddr); + auto packet = Packet::fromReceivedPacket(std::move(datagram._datagram), datagramSize, senderSockAddr); packet->setReceiveTime(receiveTime); // save the sequence number in case this is the packet that sticks readyRead @@ -395,6 +421,7 @@ void Socket::readPendingDatagrams() { qCDebug(networking) << "Can't process packet: version" << (unsigned int)NLPacket::versionInHeader(*packet) << ", type" << NLPacket::typeInHeader(*packet); #endif + _incomingDatagrams.pop(); continue; } } @@ -410,6 +437,8 @@ void Socket::readPendingDatagrams() { } } } + + _incomingDatagrams.pop(); } } diff --git a/libraries/networking/src/udt/Socket.h b/libraries/networking/src/udt/Socket.h index 1f28592c83..d23e0425a0 100644 --- a/libraries/networking/src/udt/Socket.h +++ b/libraries/networking/src/udt/Socket.h @@ -17,6 +17,7 @@ #include #include #include +#include #include #include @@ -94,6 +95,7 @@ public: signals: void clientHandshakeRequestComplete(const HifiSockAddr& sockAddr); + void pendingDatagrams(int datagramCount); public slots: void cleanupConnection(HifiSockAddr sockAddr); @@ -101,6 +103,7 @@ public slots: private slots: void readPendingDatagrams(); + void processPendingDatagrams(int datagramCount); void checkForReadyReadBackup(); void handleSocketError(QAbstractSocket::SocketError socketError); @@ -144,6 +147,17 @@ private: int _lastPacketSizeRead { 0 }; SequenceNumber _lastReceivedSequenceNumber; HifiSockAddr _lastPacketSockAddr; + + struct Datagram { + QHostAddress _senderAddress; + int _senderPort; + int _datagramLength; + std::unique_ptr _datagram; + std::chrono::time_point _receiveTime; + }; + + std::queue _incomingDatagrams; + int _maxDatagramsRead { 0 }; friend UDTTest; }; From 4084973cca6d1193e681f53c9acddbc9870a0e38 Mon Sep 17 00:00:00 2001 From: Simon Walton Date: Tue, 21 Aug 2018 16:04:08 -0700 Subject: [PATCH 02/44] Changes for linux compilation --- libraries/networking/src/udt/Socket.cpp | 2 +- libraries/networking/src/udt/Socket.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/libraries/networking/src/udt/Socket.cpp b/libraries/networking/src/udt/Socket.cpp index d01b937b8b..090beb2726 100644 --- a/libraries/networking/src/udt/Socket.cpp +++ b/libraries/networking/src/udt/Socket.cpp @@ -337,7 +337,7 @@ void Socket::readPendingDatagrams() { // we either didn't pull anything for this packet or there was an error reading (this seems to trigger // on windows even if there's not a packet available) - if (packetSizeWithHeader < 0) { + if (sizeRead < 0) { continue; } diff --git a/libraries/networking/src/udt/Socket.h b/libraries/networking/src/udt/Socket.h index d23e0425a0..078863663f 100644 --- a/libraries/networking/src/udt/Socket.h +++ b/libraries/networking/src/udt/Socket.h @@ -153,7 +153,7 @@ private: int _senderPort; int _datagramLength; std::unique_ptr _datagram; - std::chrono::time_point _receiveTime; + p_high_resolution_clock::time_point _receiveTime; }; std::queue _incomingDatagrams; From d8d940b06a26d4eeb0a800c20dfa4cbdcf69b764 Mon Sep 17 00:00:00 2001 From: Simon Walton Date: Wed, 22 Aug 2018 10:28:32 -0700 Subject: [PATCH 03/44] Cap datagrams-at-once processed; debugging output --- libraries/networking/src/udt/Socket.cpp | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/libraries/networking/src/udt/Socket.cpp b/libraries/networking/src/udt/Socket.cpp index 090beb2726..5b64cc0716 100644 --- a/libraries/networking/src/udt/Socket.cpp +++ b/libraries/networking/src/udt/Socket.cpp @@ -319,7 +319,11 @@ void Socket::readPendingDatagrams() { int packetsRead = 0; int packetSizeWithHeader = -1; - while (_udpSocket.hasPendingDatagrams() && (packetSizeWithHeader = _udpSocket.pendingDatagramSize()) != -1) { + // Max datagrams to read before processing: + static const int MAX_DATAGRAMS_CONSECUTIVELY = 10000; + while (_udpSocket.hasPendingDatagrams() + && (packetSizeWithHeader = _udpSocket.pendingDatagramSize()) != -1 + && packetsRead <= MAX_DATAGRAMS_CONSECUTIVELY) { // grab a time point we can mark as the receive time of this packet auto receiveTime = p_high_resolution_clock::now(); @@ -336,7 +340,6 @@ void Socket::readPendingDatagrams() { // we either didn't pull anything for this packet or there was an error reading (this seems to trigger // on windows even if there's not a packet available) - if (sizeRead < 0) { continue; } @@ -347,7 +350,10 @@ void Socket::readPendingDatagrams() { } - _maxDatagramsRead = std::max(_maxDatagramsRead, packetsRead); + if (packetsRead > _maxDatagramsRead) { + _maxDatagramsRead = packetsRead; + qCDebug(networking) << "readPendingDatagrams: Datagrams read:" << packetsRead; + } emit pendingDatagrams(packetsRead); } @@ -365,7 +371,7 @@ void Socket::processPendingDatagrams(int) { auto it = _unfilteredHandlers.find(senderSockAddr); if (it != _unfilteredHandlers.end()) { - // we have a registered unfiltered handler for this HifiSockAddr - call that and return + // we have a registered unfiltered handler for this HifiSockAddr (eg. STUN packet) - call that and return if (it->second) { auto basePacket = BasePacket::fromReceivedPacket(std::move(datagram._datagram), datagramSize, senderSockAddr); @@ -407,7 +413,7 @@ void Socket::processPendingDatagrams(int) { // save the sequence number in case this is the packet that sticks readyRead _lastReceivedSequenceNumber = packet->getSequenceNumber(); - // call our verification operator to see if this packet is verified + // call our hash verification operator to see if this packet is verified if (!_packetFilterOperator || _packetFilterOperator(*packet)) { if (packet->isReliable()) { // if this was a reliable packet then signal the matching connection with the sequence number From bf839ca2912b20f7a080be62c1786c4fda47a90d Mon Sep 17 00:00:00 2001 From: Simon Walton Date: Fri, 24 Aug 2018 16:53:38 -0700 Subject: [PATCH 04/44] Handle _unfilteredHandlers first upon processing datagrams --- libraries/networking/src/udt/Socket.cpp | 41 +++++++++++++++---------- libraries/networking/src/udt/Socket.h | 4 +-- 2 files changed, 27 insertions(+), 18 deletions(-) diff --git a/libraries/networking/src/udt/Socket.cpp b/libraries/networking/src/udt/Socket.cpp index 5b64cc0716..162b045a6e 100644 --- a/libraries/networking/src/udt/Socket.cpp +++ b/libraries/networking/src/udt/Socket.cpp @@ -344,7 +344,7 @@ void Socket::readPendingDatagrams() { continue; } - _incomingDatagrams.push({ senderAddress, senderPort, packetSizeWithHeader, + _incomingDatagrams.push_back({ senderAddress, senderPort, packetSizeWithHeader, std::move(buffer), receiveTime }); ++packetsRead; @@ -361,6 +361,28 @@ void Socket::processPendingDatagrams(int) { // setup a HifiSockAddr to read into HifiSockAddr senderSockAddr; + // Process unfiltered packets first. + for (auto datagramIter = _incomingDatagrams.begin(); datagramIter != _incomingDatagrams.end(); ++datagramIter) { + senderSockAddr.setAddress(datagramIter->_senderAddress); + senderSockAddr.setPort(datagramIter->_senderPort); + auto it = _unfilteredHandlers.find(senderSockAddr); + if (it != _unfilteredHandlers.end()) { + // we have a registered unfiltered handler for this HifiSockAddr (eg. STUN packet) - call that and return + if (it->second) { + auto basePacket = BasePacket::fromReceivedPacket(std::move(datagramIter->_datagram), + datagramIter->_datagramLength, + senderSockAddr); + basePacket->setReceiveTime(datagramIter->_receiveTime); + it->second(std::move(basePacket)); + } + + datagramIter = _incomingDatagrams.erase(datagramIter); + if (datagramIter == _incomingDatagrams.end()) { + break; + } + } + } + while (!_incomingDatagrams.empty()) { auto& datagram = _incomingDatagrams.front(); senderSockAddr.setAddress(datagram._senderAddress); @@ -370,19 +392,6 @@ void Socket::processPendingDatagrams(int) { auto it = _unfilteredHandlers.find(senderSockAddr); - if (it != _unfilteredHandlers.end()) { - // we have a registered unfiltered handler for this HifiSockAddr (eg. STUN packet) - call that and return - if (it->second) { - auto basePacket = BasePacket::fromReceivedPacket(std::move(datagram._datagram), datagramSize, - senderSockAddr); - basePacket->setReceiveTime(datagram._receiveTime); - it->second(std::move(basePacket)); - } - - _incomingDatagrams.pop(); - continue; - } - // we're reading a packet so re-start the readyRead backup timer _readyReadBackupTimer->start(); @@ -427,7 +436,7 @@ void Socket::processPendingDatagrams(int) { qCDebug(networking) << "Can't process packet: version" << (unsigned int)NLPacket::versionInHeader(*packet) << ", type" << NLPacket::typeInHeader(*packet); #endif - _incomingDatagrams.pop(); + _incomingDatagrams.pop_front(); continue; } } @@ -444,7 +453,7 @@ void Socket::processPendingDatagrams(int) { } } - _incomingDatagrams.pop(); + _incomingDatagrams.pop_front(); } } diff --git a/libraries/networking/src/udt/Socket.h b/libraries/networking/src/udt/Socket.h index 078863663f..ef4a457116 100644 --- a/libraries/networking/src/udt/Socket.h +++ b/libraries/networking/src/udt/Socket.h @@ -17,7 +17,7 @@ #include #include #include -#include +#include #include #include @@ -156,7 +156,7 @@ private: p_high_resolution_clock::time_point _receiveTime; }; - std::queue _incomingDatagrams; + std::list _incomingDatagrams; int _maxDatagramsRead { 0 }; friend UDTTest; From ab810f45050c0e03011cd384070968fe3b1e53ea Mon Sep 17 00:00:00 2001 From: Simon Walton Date: Fri, 24 Aug 2018 17:33:06 -0700 Subject: [PATCH 05/44] Try reducing use of shared pointers in O(n2) code --- .../src/avatars/AvatarMixerSlave.cpp | 35 ++++++++++--------- 1 file changed, 18 insertions(+), 17 deletions(-) diff --git a/assignment-client/src/avatars/AvatarMixerSlave.cpp b/assignment-client/src/avatars/AvatarMixerSlave.cpp index f347ff1f10..3e9755ed39 100644 --- a/assignment-client/src/avatars/AvatarMixerSlave.cpp +++ b/assignment-client/src/avatars/AvatarMixerSlave.cpp @@ -210,6 +210,7 @@ void AvatarMixerSlave::broadcastAvatarData(const SharedNodePointer& node) { } void AvatarMixerSlave::broadcastAvatarDataToAgent(const SharedNodePointer& node) { + const Node* nodeRaw = node.data(); auto nodeList = DependencyManager::get(); @@ -220,7 +221,7 @@ void AvatarMixerSlave::broadcastAvatarDataToAgent(const SharedNodePointer& node) _stats.nodesBroadcastedTo++; - AvatarMixerClientData* nodeData = reinterpret_cast(node->getLinkedData()); + AvatarMixerClientData* nodeData = reinterpret_cast(nodeRaw->getLinkedData()); nodeData->resetInViewStats(); @@ -260,7 +261,7 @@ void AvatarMixerSlave::broadcastAvatarDataToAgent(const SharedNodePointer& node) bool PALIsOpen = nodeData->getRequestsDomainListData(); // When this is true, the AvatarMixer will send Avatar data to a client about avatars that have ignored them - bool getsAnyIgnored = PALIsOpen && node->getCanKick(); + bool getsAnyIgnored = PALIsOpen && nodeRaw->getCanKick(); if (PALIsOpen) { // Increase minimumBytesPerAvatar if the PAL is open @@ -286,8 +287,8 @@ void AvatarMixerSlave::broadcastAvatarDataToAgent(const SharedNodePointer& node) // setup list of AvatarData as well as maps to map betweeen the AvatarData and the original nodes - std::vector avatarsToSort; - std::unordered_map avatarDataToNodes; + std::vector avatarsToSort; + std::unordered_map avatarDataToNodes; std::unordered_map avatarEncodeTimes; std::for_each(_begin, _end, [&](const SharedNodePointer& otherNode) { // make sure this is an agent that we have avatar data for before considering it for inclusion @@ -295,7 +296,7 @@ void AvatarMixerSlave::broadcastAvatarDataToAgent(const SharedNodePointer& node) && otherNode->getLinkedData()) { const AvatarMixerClientData* otherNodeData = reinterpret_cast(otherNode->getLinkedData()); - AvatarSharedPointer otherAvatar = otherNodeData->getAvatarSharedPointer(); + AvatarData* otherAvatar = otherNodeData->getAvatarSharedPointer().get(); avatarsToSort.push_back(otherAvatar); avatarDataToNodes[otherAvatar] = otherNode; QUuid id = otherAvatar->getSessionUUID(); @@ -306,7 +307,7 @@ void AvatarMixerSlave::broadcastAvatarDataToAgent(const SharedNodePointer& node) class SortableAvatar: public PrioritySortUtil::Sortable { public: SortableAvatar() = delete; - SortableAvatar(const AvatarSharedPointer& avatar, uint64_t lastEncodeTime) + SortableAvatar(const AvatarData* avatar, uint64_t lastEncodeTime) : _avatar(avatar), _lastEncodeTime(lastEncodeTime) {} glm::vec3 getPosition() const override { return _avatar->getWorldPosition(); } float getRadius() const override { @@ -316,10 +317,10 @@ void AvatarMixerSlave::broadcastAvatarDataToAgent(const SharedNodePointer& node) uint64_t getTimestamp() const override { return _lastEncodeTime; } - AvatarSharedPointer getAvatar() const { return _avatar; } + const AvatarData* getAvatar() const { return _avatar; } private: - AvatarSharedPointer _avatar; + const AvatarData* _avatar; uint64_t _lastEncodeTime; }; @@ -332,8 +333,8 @@ void AvatarMixerSlave::broadcastAvatarDataToAgent(const SharedNodePointer& node) // ignore or sort const AvatarSharedPointer& thisAvatar = nodeData->getAvatarSharedPointer(); - for (const auto& avatar : avatarsToSort) { - if (avatar == thisAvatar) { + for (const auto avatar : avatarsToSort) { + if (avatar == thisAvatar.get()) { // don't echo updates to self continue; } @@ -356,14 +357,14 @@ void AvatarMixerSlave::broadcastAvatarDataToAgent(const SharedNodePointer& node) // and isn't an avatar that the viewing node has ignored // or that has ignored the viewing node if (!avatarNode->getLinkedData() - || avatarNode->getUUID() == node->getUUID() - || (node->isIgnoringNodeWithID(avatarNode->getUUID()) && !PALIsOpen) - || (avatarNode->isIgnoringNodeWithID(node->getUUID()) && !getsAnyIgnored)) { + || avatarNode->getUUID() == nodeRaw->getUUID() + || (nodeRaw->isIgnoringNodeWithID(avatarNode->getUUID()) && !PALIsOpen) + || (avatarNode->isIgnoringNodeWithID(nodeRaw->getUUID()) && !getsAnyIgnored)) { shouldIgnore = true; } else { // Check to see if the space bubble is enabled // Don't bother with these checks if the other avatar has their bubble enabled and we're gettingAnyIgnored - if (node->isIgnoreRadiusEnabled() || (avatarNode->isIgnoreRadiusEnabled() && !getsAnyIgnored)) { + if (nodeRaw->isIgnoreRadiusEnabled() || (avatarNode->isIgnoreRadiusEnabled() && !getsAnyIgnored)) { float sensorToWorldScale = avatarNodeData->getAvatarSharedPointer()->getSensorToWorldScale(); // Define the scale of the box for the current other node glm::vec3 otherNodeBoxScale = (avatarNodeData->getPosition() - avatarNodeData->getGlobalBoundingBoxCorner()) * 2.0f * sensorToWorldScale; @@ -430,7 +431,7 @@ void AvatarMixerSlave::broadcastAvatarDataToAgent(const SharedNodePointer& node) int remainingAvatars = (int)sortedAvatars.size(); auto traitsPacketList = NLPacketList::create(PacketType::BulkAvatarTraits, QByteArray(), true, true); while (!sortedAvatars.empty()) { - const auto avatarData = sortedAvatars.top().getAvatar(); + const AvatarData* avatarData = sortedAvatars.top().getAvatar(); sortedAvatars.pop(); remainingAvatars--; @@ -565,7 +566,7 @@ void AvatarMixerSlave::broadcastAvatarDataToAgent(const SharedNodePointer& node) _stats.numBytesSent += numAvatarDataBytes; // send the avatar data PacketList - nodeList->sendPacketList(std::move(avatarPacketList), *node); + nodeList->sendPacketList(std::move(avatarPacketList), *nodeRaw); // record the bytes sent for other avatar data in the AvatarMixerClientData nodeData->recordSentAvatarData(numAvatarDataBytes); @@ -575,7 +576,7 @@ void AvatarMixerSlave::broadcastAvatarDataToAgent(const SharedNodePointer& node) if (traitsPacketList->getNumPackets() >= 1) { // send the traits packet list - nodeList->sendPacketList(std::move(traitsPacketList), *node); + nodeList->sendPacketList(std::move(traitsPacketList), *nodeRaw); } // record the number of avatars held back this frame From 861d1e26a99be1f9b4839cb25f6ff2fabeac24da Mon Sep 17 00:00:00 2001 From: Simon Walton Date: Fri, 24 Aug 2018 18:02:28 -0700 Subject: [PATCH 06/44] Remove unused variable --- libraries/networking/src/udt/Socket.cpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/libraries/networking/src/udt/Socket.cpp b/libraries/networking/src/udt/Socket.cpp index 162b045a6e..56b7521d7a 100644 --- a/libraries/networking/src/udt/Socket.cpp +++ b/libraries/networking/src/udt/Socket.cpp @@ -390,8 +390,6 @@ void Socket::processPendingDatagrams(int) { int datagramSize = datagram._datagramLength; auto receiveTime = datagram._receiveTime; - auto it = _unfilteredHandlers.find(senderSockAddr); - // we're reading a packet so re-start the readyRead backup timer _readyReadBackupTimer->start(); From 402ed4fb76532327066d2ea1448556d68c906e6e Mon Sep 17 00:00:00 2001 From: Simon Walton Date: Mon, 27 Aug 2018 10:35:17 -0700 Subject: [PATCH 07/44] More shared pointer tweaks --- assignment-client/src/avatars/AvatarMixerSlave.cpp | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/assignment-client/src/avatars/AvatarMixerSlave.cpp b/assignment-client/src/avatars/AvatarMixerSlave.cpp index 3e9755ed39..367e991a04 100644 --- a/assignment-client/src/avatars/AvatarMixerSlave.cpp +++ b/assignment-client/src/avatars/AvatarMixerSlave.cpp @@ -288,7 +288,8 @@ void AvatarMixerSlave::broadcastAvatarDataToAgent(const SharedNodePointer& node) // setup list of AvatarData as well as maps to map betweeen the AvatarData and the original nodes std::vector avatarsToSort; - std::unordered_map avatarDataToNodes; + std::unordered_map avatarDataToNodes; + std::unordered_map avatarDataToNodesShared; std::unordered_map avatarEncodeTimes; std::for_each(_begin, _end, [&](const SharedNodePointer& otherNode) { // make sure this is an agent that we have avatar data for before considering it for inclusion @@ -298,7 +299,8 @@ void AvatarMixerSlave::broadcastAvatarDataToAgent(const SharedNodePointer& node) AvatarData* otherAvatar = otherNodeData->getAvatarSharedPointer().get(); avatarsToSort.push_back(otherAvatar); - avatarDataToNodes[otherAvatar] = otherNode; + avatarDataToNodes[otherAvatar] = otherNode.data(); + avatarDataToNodesShared[otherAvatar] = otherNode; QUuid id = otherAvatar->getSessionUUID(); avatarEncodeTimes[id] = nodeData->getLastOtherAvatarEncodeTime(id); } @@ -332,9 +334,9 @@ void AvatarMixerSlave::broadcastAvatarDataToAgent(const SharedNodePointer& node) AvatarData::_avatarSortCoefficientAge); // ignore or sort - const AvatarSharedPointer& thisAvatar = nodeData->getAvatarSharedPointer(); + const AvatarData* thisAvatar = nodeData->getAvatarSharedPointer().get(); for (const auto avatar : avatarsToSort) { - if (avatar == thisAvatar.get()) { + if (avatar == thisAvatar) { // don't echo updates to self continue; } @@ -380,7 +382,7 @@ void AvatarMixerSlave::broadcastAvatarDataToAgent(const SharedNodePointer& node) // Perform the collision check between the two bounding boxes if (nodeBox.touches(otherNodeBox)) { - nodeData->ignoreOther(node, avatarNode); + nodeData->ignoreOther(node, avatarDataToNodesShared[avatar]); shouldIgnore = !getsAnyIgnored; } } From 44f253c482a9978c378e091d811e8358a785dc88 Mon Sep 17 00:00:00 2001 From: Simon Walton Date: Mon, 27 Aug 2018 14:54:37 -0700 Subject: [PATCH 08/44] Reduce use of shared pointers and maps --- .../src/avatars/AvatarMixerClientData.cpp | 4 +- .../src/avatars/AvatarMixerSlave.cpp | 60 ++++++++++--------- 2 files changed, 33 insertions(+), 31 deletions(-) diff --git a/assignment-client/src/avatars/AvatarMixerClientData.cpp b/assignment-client/src/avatars/AvatarMixerClientData.cpp index f524c071ec..e003fdd769 100644 --- a/assignment-client/src/avatars/AvatarMixerClientData.cpp +++ b/assignment-client/src/avatars/AvatarMixerClientData.cpp @@ -236,9 +236,7 @@ void AvatarMixerClientData::ignoreOther(SharedNodePointer self, SharedNodePointe } void AvatarMixerClientData::removeFromRadiusIgnoringSet(SharedNodePointer self, const QUuid& other) { - if (isRadiusIgnoring(other)) { - _radiusIgnoredOthers.erase(other); - } + _radiusIgnoredOthers.erase(other); } void AvatarMixerClientData::resetSentTraitData(Node::LocalID nodeLocalID) { diff --git a/assignment-client/src/avatars/AvatarMixerSlave.cpp b/assignment-client/src/avatars/AvatarMixerSlave.cpp index 367e991a04..2ba54bd6b6 100644 --- a/assignment-client/src/avatars/AvatarMixerSlave.cpp +++ b/assignment-client/src/avatars/AvatarMixerSlave.cpp @@ -287,30 +287,39 @@ void AvatarMixerSlave::broadcastAvatarDataToAgent(const SharedNodePointer& node) // setup list of AvatarData as well as maps to map betweeen the AvatarData and the original nodes - std::vector avatarsToSort; - std::unordered_map avatarDataToNodes; - std::unordered_map avatarDataToNodesShared; - std::unordered_map avatarEncodeTimes; + struct AvatarSortData { + SharedNodePointer _nodeShared; + Node* _node; + AvatarData* _avatarData; + quint64 _lastEncodeTime; + }; + std::vector avatarsToSort; + avatarsToSort.reserve(nodeList->size()); + //std::unordered_map avatarDataToNodes; + //std::unordered_map avatarDataToNodesShared; + //std::unordered_map avatarEncodeTimes; std::for_each(_begin, _end, [&](const SharedNodePointer& otherNode) { + Node* otherNodeRaw = otherNode.data(); // make sure this is an agent that we have avatar data for before considering it for inclusion - if (otherNode->getType() == NodeType::Agent - && otherNode->getLinkedData()) { - const AvatarMixerClientData* otherNodeData = reinterpret_cast(otherNode->getLinkedData()); + if (otherNodeRaw->getType() == NodeType::Agent + && otherNodeRaw->getLinkedData()) { + const AvatarMixerClientData* otherNodeData = reinterpret_cast(otherNodeRaw->getLinkedData()); + AvatarData* otherAvatar = otherNodeData->getAvatarSharedPointer().get(); - avatarsToSort.push_back(otherAvatar); - avatarDataToNodes[otherAvatar] = otherNode.data(); - avatarDataToNodesShared[otherAvatar] = otherNode; - QUuid id = otherAvatar->getSessionUUID(); - avatarEncodeTimes[id] = nodeData->getLastOtherAvatarEncodeTime(id); + //avatarsToSort.push_back(otherAvatar); + //avatarDataToNodes[otherAvatar] = otherNode.data(); + //avatarDataToNodesShared[otherAvatar] = otherNode; + auto lastEncodeTime = nodeData->getLastOtherAvatarEncodeTime(otherAvatar->getSessionUUID()); + avatarsToSort.emplace_back(AvatarSortData({ otherNode, otherNodeRaw, otherAvatar, lastEncodeTime })); } }); class SortableAvatar: public PrioritySortUtil::Sortable { public: SortableAvatar() = delete; - SortableAvatar(const AvatarData* avatar, uint64_t lastEncodeTime) - : _avatar(avatar), _lastEncodeTime(lastEncodeTime) {} + SortableAvatar(const AvatarData* avatar, const Node* avatarNode, uint64_t lastEncodeTime) + : _avatar(avatar), _node(avatarNode), _lastEncodeTime(lastEncodeTime) {} glm::vec3 getPosition() const override { return _avatar->getWorldPosition(); } float getRadius() const override { glm::vec3 nodeBoxHalfScale = (_avatar->getWorldPosition() - _avatar->getGlobalBoundingBoxCorner() * _avatar->getSensorToWorldScale()); @@ -320,9 +329,11 @@ void AvatarMixerSlave::broadcastAvatarDataToAgent(const SharedNodePointer& node) return _lastEncodeTime; } const AvatarData* getAvatar() const { return _avatar; } + const Node* getNode() const { return _node; } private: const AvatarData* _avatar; + const Node* _node; uint64_t _lastEncodeTime; }; @@ -335,8 +346,8 @@ void AvatarMixerSlave::broadcastAvatarDataToAgent(const SharedNodePointer& node) // ignore or sort const AvatarData* thisAvatar = nodeData->getAvatarSharedPointer().get(); - for (const auto avatar : avatarsToSort) { - if (avatar == thisAvatar) { + for (const auto& avatar : avatarsToSort) { + if (avatar._node == nodeRaw) { // don't echo updates to self continue; } @@ -348,7 +359,7 @@ void AvatarMixerSlave::broadcastAvatarDataToAgent(const SharedNodePointer& node) // happen if for example the avatar is connected on a desktop and sending // updates at ~30hz. So every 3 frames we skip a frame. - auto avatarNode = avatarDataToNodes[avatar]; + auto avatarNode = avatar._node; assert(avatarNode); // we can't have gotten here without the avatarData being a valid key in the map const AvatarMixerClientData* avatarNodeData = reinterpret_cast(avatarNode->getLinkedData()); @@ -358,9 +369,7 @@ void AvatarMixerSlave::broadcastAvatarDataToAgent(const SharedNodePointer& node) // make sure we have data for this avatar, that it isn't the same node, // and isn't an avatar that the viewing node has ignored // or that has ignored the viewing node - if (!avatarNode->getLinkedData() - || avatarNode->getUUID() == nodeRaw->getUUID() - || (nodeRaw->isIgnoringNodeWithID(avatarNode->getUUID()) && !PALIsOpen) + if ((nodeRaw->isIgnoringNodeWithID(avatarNode->getUUID()) && !PALIsOpen) || (avatarNode->isIgnoringNodeWithID(nodeRaw->getUUID()) && !getsAnyIgnored)) { shouldIgnore = true; } else { @@ -382,7 +391,7 @@ void AvatarMixerSlave::broadcastAvatarDataToAgent(const SharedNodePointer& node) // Perform the collision check between the two bounding boxes if (nodeBox.touches(otherNodeBox)) { - nodeData->ignoreOther(node, avatarDataToNodesShared[avatar]); + nodeData->ignoreOther(node, avatar._nodeShared); shouldIgnore = !getsAnyIgnored; } } @@ -419,12 +428,7 @@ void AvatarMixerSlave::broadcastAvatarDataToAgent(const SharedNodePointer& node) if (!shouldIgnore) { // sort this one for later - uint64_t lastEncodeTime = 0; - std::unordered_map::const_iterator itr = avatarEncodeTimes.find(avatar->getSessionUUID()); - if (itr != avatarEncodeTimes.end()) { - lastEncodeTime = itr->second; - } - sortedAvatars.push(SortableAvatar(avatar, lastEncodeTime)); + sortedAvatars.push(SortableAvatar(avatar._avatarData, avatar._node, avatar._lastEncodeTime)); } } @@ -434,10 +438,10 @@ void AvatarMixerSlave::broadcastAvatarDataToAgent(const SharedNodePointer& node) auto traitsPacketList = NLPacketList::create(PacketType::BulkAvatarTraits, QByteArray(), true, true); while (!sortedAvatars.empty()) { const AvatarData* avatarData = sortedAvatars.top().getAvatar(); + const Node* otherNode = sortedAvatars.top().getNode(); sortedAvatars.pop(); remainingAvatars--; - auto otherNode = avatarDataToNodes[avatarData]; assert(otherNode); // we can't have gotten here without the avatarData being a valid key in the map // NOTE: Here's where we determine if we are over budget and drop to bare minimum data From 027d11736446769867807294f6e842463ee4889a Mon Sep 17 00:00:00 2001 From: Simon Walton Date: Tue, 28 Aug 2018 09:41:09 -0700 Subject: [PATCH 09/44] Remove deadlock caused by trying readlock on NodeList Also just use refs to other SharedNodes as the NodeList is locked. --- assignment-client/src/avatars/AvatarMixerSlave.cpp | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/assignment-client/src/avatars/AvatarMixerSlave.cpp b/assignment-client/src/avatars/AvatarMixerSlave.cpp index 2ba54bd6b6..50ddcec92b 100644 --- a/assignment-client/src/avatars/AvatarMixerSlave.cpp +++ b/assignment-client/src/avatars/AvatarMixerSlave.cpp @@ -288,13 +288,19 @@ void AvatarMixerSlave::broadcastAvatarDataToAgent(const SharedNodePointer& node) // setup list of AvatarData as well as maps to map betweeen the AvatarData and the original nodes struct AvatarSortData { - SharedNodePointer _nodeShared; + AvatarSortData(const SharedNodePointer& nodeShared, AvatarData* avatarData, quint64 lastEncodeTime) + : _nodeShared(nodeShared) + , _node(nodeShared.data()) + , _avatarData(avatarData) + , _lastEncodeTime(lastEncodeTime) + { } + const SharedNodePointer& _nodeShared; Node* _node; AvatarData* _avatarData; quint64 _lastEncodeTime; }; + // Temporary info about the avatars we're sending: std::vector avatarsToSort; - avatarsToSort.reserve(nodeList->size()); //std::unordered_map avatarDataToNodes; //std::unordered_map avatarDataToNodesShared; //std::unordered_map avatarEncodeTimes; @@ -311,7 +317,7 @@ void AvatarMixerSlave::broadcastAvatarDataToAgent(const SharedNodePointer& node) //avatarDataToNodes[otherAvatar] = otherNode.data(); //avatarDataToNodesShared[otherAvatar] = otherNode; auto lastEncodeTime = nodeData->getLastOtherAvatarEncodeTime(otherAvatar->getSessionUUID()); - avatarsToSort.emplace_back(AvatarSortData({ otherNode, otherNodeRaw, otherAvatar, lastEncodeTime })); + avatarsToSort.emplace_back(AvatarSortData(otherNode, otherAvatar, lastEncodeTime)); } }); @@ -345,7 +351,6 @@ void AvatarMixerSlave::broadcastAvatarDataToAgent(const SharedNodePointer& node) AvatarData::_avatarSortCoefficientAge); // ignore or sort - const AvatarData* thisAvatar = nodeData->getAvatarSharedPointer().get(); for (const auto& avatar : avatarsToSort) { if (avatar._node == nodeRaw) { // don't echo updates to self @@ -437,7 +442,6 @@ void AvatarMixerSlave::broadcastAvatarDataToAgent(const SharedNodePointer& node) int remainingAvatars = (int)sortedAvatars.size(); auto traitsPacketList = NLPacketList::create(PacketType::BulkAvatarTraits, QByteArray(), true, true); while (!sortedAvatars.empty()) { - const AvatarData* avatarData = sortedAvatars.top().getAvatar(); const Node* otherNode = sortedAvatars.top().getNode(); sortedAvatars.pop(); remainingAvatars--; From 4b7f6a346fecaf0b8b9e508254dfff805498227f Mon Sep 17 00:00:00 2001 From: Simon Walton Date: Tue, 28 Aug 2018 16:21:47 -0700 Subject: [PATCH 10/44] Use AvatarData::getClientGlobalPosition() for position in priority queue It looks like it's the same as world position with the avatar mixer. Also use one time stamp for current time in priority queue; use std::chrono for some other timestamps in hope that it's faster. Fix priority-code logic bug. --- .../src/avatars/AvatarMixerSlave.cpp | 26 +++++++++---------- libraries/shared/src/PrioritySortUtil.h | 12 ++++++--- 2 files changed, 21 insertions(+), 17 deletions(-) diff --git a/assignment-client/src/avatars/AvatarMixerSlave.cpp b/assignment-client/src/avatars/AvatarMixerSlave.cpp index 50ddcec92b..f0c291b2c2 100644 --- a/assignment-client/src/avatars/AvatarMixerSlave.cpp +++ b/assignment-client/src/avatars/AvatarMixerSlave.cpp @@ -13,6 +13,7 @@ #include #include +#include #include #include @@ -33,6 +34,8 @@ #include "AvatarMixer.h" #include "AvatarMixerClientData.h" +namespace chrono = std::chrono; + void AvatarMixerSlave::configure(ConstIter begin, ConstIter end) { _begin = begin; _end = end; @@ -301,9 +304,7 @@ void AvatarMixerSlave::broadcastAvatarDataToAgent(const SharedNodePointer& node) }; // Temporary info about the avatars we're sending: std::vector avatarsToSort; - //std::unordered_map avatarDataToNodes; - //std::unordered_map avatarDataToNodesShared; - //std::unordered_map avatarEncodeTimes; + avatarsToSort.reserve(_end - _begin); std::for_each(_begin, _end, [&](const SharedNodePointer& otherNode) { Node* otherNodeRaw = otherNode.data(); // make sure this is an agent that we have avatar data for before considering it for inclusion @@ -313,9 +314,6 @@ void AvatarMixerSlave::broadcastAvatarDataToAgent(const SharedNodePointer& node) AvatarData* otherAvatar = otherNodeData->getAvatarSharedPointer().get(); - //avatarsToSort.push_back(otherAvatar); - //avatarDataToNodes[otherAvatar] = otherNode.data(); - //avatarDataToNodesShared[otherAvatar] = otherNode; auto lastEncodeTime = nodeData->getLastOtherAvatarEncodeTime(otherAvatar->getSessionUUID()); avatarsToSort.emplace_back(AvatarSortData(otherNode, otherAvatar, lastEncodeTime)); } @@ -326,7 +324,7 @@ void AvatarMixerSlave::broadcastAvatarDataToAgent(const SharedNodePointer& node) SortableAvatar() = delete; SortableAvatar(const AvatarData* avatar, const Node* avatarNode, uint64_t lastEncodeTime) : _avatar(avatar), _node(avatarNode), _lastEncodeTime(lastEncodeTime) {} - glm::vec3 getPosition() const override { return _avatar->getWorldPosition(); } + glm::vec3 getPosition() const override { return _avatar->getClientGlobalPosition(); } float getRadius() const override { glm::vec3 nodeBoxHalfScale = (_avatar->getWorldPosition() - _avatar->getGlobalBoundingBoxCorner() * _avatar->getSensorToWorldScale()); return glm::max(nodeBoxHalfScale.x, glm::max(nodeBoxHalfScale.y, nodeBoxHalfScale.z)); @@ -452,7 +450,7 @@ void AvatarMixerSlave::broadcastAvatarDataToAgent(const SharedNodePointer& node) int minimRemainingAvatarBytes = minimumBytesPerAvatar * remainingAvatars; bool overBudget = (identityBytesSent + numAvatarDataBytes + minimRemainingAvatarBytes) > maxAvatarBytesPerFrame; - quint64 startAvatarDataPacking = usecTimestampNow(); + auto startAvatarDataPacking = chrono::high_resolution_clock::now(); ++numOtherAvatars; @@ -504,12 +502,13 @@ void AvatarMixerSlave::broadcastAvatarDataToAgent(const SharedNodePointer& node) AvatarDataPacket::HasFlags hasFlagsOut; // the result of the toByteArray bool dropFaceTracking = false; - quint64 start = usecTimestampNow(); + auto startSerialize = chrono::high_resolution_clock::now(); QByteArray bytes = otherAvatar->toByteArray(detail, lastEncodeForOther, lastSentJointsForOther, hasFlagsOut, dropFaceTracking, distanceAdjust, viewerPosition, &lastSentJointsForOther); - quint64 end = usecTimestampNow(); - _stats.toByteArrayElapsedTime += (end - start); + auto endSerialize = chrono::high_resolution_clock::now(); + _stats.toByteArrayElapsedTime += + (quint64) chrono::duration_cast(endSerialize - startSerialize).count(); static auto maxAvatarDataBytes = avatarPacketList->getMaxSegmentSize() - NUM_BYTES_RFC4122_UUID; if (bytes.size() > maxAvatarDataBytes) { @@ -558,8 +557,9 @@ void AvatarMixerSlave::broadcastAvatarDataToAgent(const SharedNodePointer& node) avatarPacketList->endSegment(); - quint64 endAvatarDataPacking = usecTimestampNow(); - _stats.avatarDataPackingElapsedTime += (endAvatarDataPacking - startAvatarDataPacking); + auto endAvatarDataPacking = chrono::high_resolution_clock::now(); + _stats.avatarDataPackingElapsedTime += + (quint64) chrono::duration_cast(endAvatarDataPacking - startAvatarDataPacking).count(); // use helper to add any changed traits to our packet list traitBytesSent += addChangedTraitsToBulkPacket(nodeData, otherNodeData, *traitsPacketList); diff --git a/libraries/shared/src/PrioritySortUtil.h b/libraries/shared/src/PrioritySortUtil.h index 34ec074d45..a2be5fadcd 100644 --- a/libraries/shared/src/PrioritySortUtil.h +++ b/libraries/shared/src/PrioritySortUtil.h @@ -87,6 +87,7 @@ namespace PrioritySortUtil { PriorityQueue(const ConicalViewFrustums& views) : _views(views) { } PriorityQueue(const ConicalViewFrustums& views, float angularWeight, float centerWeight, float ageWeight) : _views(views), _angularWeight(angularWeight), _centerWeight(centerWeight), _ageWeight(ageWeight) + , _usecCurrentTime(usecTimestampNow()) { } void setViews(const ConicalViewFrustums& views) { _views = views; } @@ -95,6 +96,7 @@ namespace PrioritySortUtil { _angularWeight = angularWeight; _centerWeight = centerWeight; _ageWeight = ageWeight; + _usecCurrentTime = usecTimestampNow(); } size_t size() const { return _queue.size(); } @@ -129,16 +131,17 @@ namespace PrioritySortUtil { glm::vec3 offset = position - view.getPosition(); float distance = glm::length(offset) + 0.001f; // add 1mm to avoid divide by zero const float MIN_RADIUS = 0.1f; // WORKAROUND for zero size objects (we still want them to sort by distance) - float radius = glm::min(thing.getRadius(), MIN_RADIUS); + float radius = glm::max(thing.getRadius(), MIN_RADIUS); + // Other item's angle from view centre: float cosineAngle = (glm::dot(offset, view.getDirection()) / distance); - float age = (float)(usecTimestampNow() - thing.getTimestamp()); + float age = (float)(_usecCurrentTime - thing.getTimestamp()); - // we modulatate "age" drift rate by the cosineAngle term to make periphrial objects sort forward + // we modulate "age" drift rate by the cosineAngle term to make peripheral objects sort forward // at a reduced rate but we don't want the "age" term to go zero or negative so we clamp it const float MIN_COSINE_ANGLE_FACTOR = 0.1f; float cosineAngleFactor = glm::max(cosineAngle, MIN_COSINE_ANGLE_FACTOR); - float priority = _angularWeight * glm::max(radius, MIN_RADIUS) / distance + float priority = _angularWeight * radius / distance + _centerWeight * cosineAngle + _ageWeight * cosineAngleFactor * age; @@ -157,6 +160,7 @@ namespace PrioritySortUtil { float _angularWeight { DEFAULT_ANGULAR_COEF }; float _centerWeight { DEFAULT_CENTER_COEF }; float _ageWeight { DEFAULT_AGE_COEF }; + quint64 _usecCurrentTime { 0 }; }; } // namespace PrioritySortUtil From 9711076daea27342310275d1cffd3f6e193030a5 Mon Sep 17 00:00:00 2001 From: Simon Walton Date: Wed, 29 Aug 2018 10:49:29 -0700 Subject: [PATCH 11/44] Use a queued signal for pending datagrams --- libraries/networking/src/udt/Socket.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/networking/src/udt/Socket.cpp b/libraries/networking/src/udt/Socket.cpp index 56b7521d7a..d764b9e8b2 100644 --- a/libraries/networking/src/udt/Socket.cpp +++ b/libraries/networking/src/udt/Socket.cpp @@ -37,7 +37,7 @@ Socket::Socket(QObject* parent, bool shouldChangeSocketOptions) : _shouldChangeSocketOptions(shouldChangeSocketOptions) { connect(&_udpSocket, &QUdpSocket::readyRead, this, &Socket::readPendingDatagrams); - connect(this, &Socket::pendingDatagrams, this, &Socket::processPendingDatagrams); + connect(this, &Socket::pendingDatagrams, this, &Socket::processPendingDatagrams, Qt::QueuedConnection); // make sure we hear about errors and state changes from the underlying socket connect(&_udpSocket, SIGNAL(error(QAbstractSocket::SocketError)), From a186be014da18676776ae88865d3d45a9008d961 Mon Sep 17 00:00:00 2001 From: Simon Walton Date: Wed, 29 Aug 2018 17:35:28 -0700 Subject: [PATCH 12/44] Use std::chrono for usecTimestampNow() --- libraries/shared/src/SharedUtil.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/libraries/shared/src/SharedUtil.cpp b/libraries/shared/src/SharedUtil.cpp index bb22a1e753..3bef7f7e18 100644 --- a/libraries/shared/src/SharedUtil.cpp +++ b/libraries/shared/src/SharedUtil.cpp @@ -21,6 +21,7 @@ #include #include #include +#include #include @@ -132,6 +133,9 @@ static std::once_flag usecTimestampNowIsInitialized; static QElapsedTimer timestampTimer; quint64 usecTimestampNow(bool wantDebug) { + using namespace std::chrono; + return duration_cast(high_resolution_clock::now().time_since_epoch()).count(); +#if 0 std::call_once(usecTimestampNowIsInitialized, [&] { TIME_REFERENCE = QDateTime::currentMSecsSinceEpoch() * USECS_PER_MSEC; // ms to usec timestampTimer.start(); @@ -203,6 +207,7 @@ quint64 usecTimestampNow(bool wantDebug) { } return now; +#endif } float secTimestampNow() { From 7b7f369c394edd715d2e143cce321324f4d5dfff Mon Sep 17 00:00:00 2001 From: Simon Walton Date: Wed, 29 Aug 2018 18:25:10 -0700 Subject: [PATCH 13/44] Further reductions in shared pointers, getWorldPosition(). --- .../src/avatars/AvatarMixerClientData.cpp | 4 +++ .../src/avatars/AvatarMixerClientData.h | 1 + .../src/avatars/AvatarMixerSlave.cpp | 34 +++++++++---------- 3 files changed, 21 insertions(+), 18 deletions(-) diff --git a/assignment-client/src/avatars/AvatarMixerClientData.cpp b/assignment-client/src/avatars/AvatarMixerClientData.cpp index e003fdd769..1dd4cc769b 100644 --- a/assignment-client/src/avatars/AvatarMixerClientData.cpp +++ b/assignment-client/src/avatars/AvatarMixerClientData.cpp @@ -218,6 +218,10 @@ uint16_t AvatarMixerClientData::getLastBroadcastSequenceNumber(const QUuid& node } void AvatarMixerClientData::ignoreOther(SharedNodePointer self, SharedNodePointer other) { + ignoreOther(self.data(), other.data()); +} + +void AvatarMixerClientData::ignoreOther(const Node* self, const Node* other) { if (!isRadiusIgnoring(other->getUUID())) { addToRadiusIgnoringSet(other->getUUID()); auto killPacket = NLPacket::create(PacketType::KillAvatar, NUM_BYTES_RFC4122_UUID + sizeof(KillAvatarReason), true); diff --git a/assignment-client/src/avatars/AvatarMixerClientData.h b/assignment-client/src/avatars/AvatarMixerClientData.h index a892455fe3..7d203bd771 100644 --- a/assignment-client/src/avatars/AvatarMixerClientData.h +++ b/assignment-client/src/avatars/AvatarMixerClientData.h @@ -95,6 +95,7 @@ public: void addToRadiusIgnoringSet(const QUuid& other) { _radiusIgnoredOthers.insert(other); } void removeFromRadiusIgnoringSet(SharedNodePointer self, const QUuid& other); void ignoreOther(SharedNodePointer self, SharedNodePointer other); + void ignoreOther(const Node* self, const Node* other); void readViewFrustumPacket(const QByteArray& message); diff --git a/assignment-client/src/avatars/AvatarMixerSlave.cpp b/assignment-client/src/avatars/AvatarMixerSlave.cpp index f0c291b2c2..e772fa4d08 100644 --- a/assignment-client/src/avatars/AvatarMixerSlave.cpp +++ b/assignment-client/src/avatars/AvatarMixerSlave.cpp @@ -213,7 +213,7 @@ void AvatarMixerSlave::broadcastAvatarData(const SharedNodePointer& node) { } void AvatarMixerSlave::broadcastAvatarDataToAgent(const SharedNodePointer& node) { - const Node* nodeRaw = node.data(); + const Node* destinationNode = node.data(); auto nodeList = DependencyManager::get(); @@ -224,7 +224,7 @@ void AvatarMixerSlave::broadcastAvatarDataToAgent(const SharedNodePointer& node) _stats.nodesBroadcastedTo++; - AvatarMixerClientData* nodeData = reinterpret_cast(nodeRaw->getLinkedData()); + AvatarMixerClientData* nodeData = reinterpret_cast(destinationNode->getLinkedData()); nodeData->resetInViewStats(); @@ -264,7 +264,7 @@ void AvatarMixerSlave::broadcastAvatarDataToAgent(const SharedNodePointer& node) bool PALIsOpen = nodeData->getRequestsDomainListData(); // When this is true, the AvatarMixer will send Avatar data to a client about avatars that have ignored them - bool getsAnyIgnored = PALIsOpen && nodeRaw->getCanKick(); + bool getsAnyIgnored = PALIsOpen && destinationNode->getCanKick(); if (PALIsOpen) { // Increase minimumBytesPerAvatar if the PAL is open @@ -291,14 +291,12 @@ void AvatarMixerSlave::broadcastAvatarDataToAgent(const SharedNodePointer& node) // setup list of AvatarData as well as maps to map betweeen the AvatarData and the original nodes struct AvatarSortData { - AvatarSortData(const SharedNodePointer& nodeShared, AvatarData* avatarData, quint64 lastEncodeTime) - : _nodeShared(nodeShared) - , _node(nodeShared.data()) + AvatarSortData(const Node* node, AvatarData* avatarData, quint64 lastEncodeTime) + : _node(node) , _avatarData(avatarData) , _lastEncodeTime(lastEncodeTime) { } - const SharedNodePointer& _nodeShared; - Node* _node; + const Node* _node; AvatarData* _avatarData; quint64 _lastEncodeTime; }; @@ -315,7 +313,7 @@ void AvatarMixerSlave::broadcastAvatarDataToAgent(const SharedNodePointer& node) AvatarData* otherAvatar = otherNodeData->getAvatarSharedPointer().get(); auto lastEncodeTime = nodeData->getLastOtherAvatarEncodeTime(otherAvatar->getSessionUUID()); - avatarsToSort.emplace_back(AvatarSortData(otherNode, otherAvatar, lastEncodeTime)); + avatarsToSort.emplace_back(AvatarSortData(otherNodeRaw, otherAvatar, lastEncodeTime)); } }); @@ -326,7 +324,7 @@ 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 = (_avatar->getWorldPosition() - _avatar->getGlobalBoundingBoxCorner() * _avatar->getSensorToWorldScale()); + glm::vec3 nodeBoxHalfScale = (_avatar->getClientGlobalPosition() - _avatar->getGlobalBoundingBoxCorner() * _avatar->getSensorToWorldScale()); return glm::max(nodeBoxHalfScale.x, glm::max(nodeBoxHalfScale.y, nodeBoxHalfScale.z)); } uint64_t getTimestamp() const override { @@ -350,7 +348,8 @@ void AvatarMixerSlave::broadcastAvatarDataToAgent(const SharedNodePointer& node) // ignore or sort for (const auto& avatar : avatarsToSort) { - if (avatar._node == nodeRaw) { + auto avatarNode = avatar._node; + if (avatarNode == destinationNode) { // don't echo updates to self continue; } @@ -362,7 +361,6 @@ void AvatarMixerSlave::broadcastAvatarDataToAgent(const SharedNodePointer& node) // happen if for example the avatar is connected on a desktop and sending // updates at ~30hz. So every 3 frames we skip a frame. - auto avatarNode = avatar._node; assert(avatarNode); // we can't have gotten here without the avatarData being a valid key in the map const AvatarMixerClientData* avatarNodeData = reinterpret_cast(avatarNode->getLinkedData()); @@ -372,13 +370,13 @@ void AvatarMixerSlave::broadcastAvatarDataToAgent(const SharedNodePointer& node) // make sure we have data for this avatar, that it isn't the same node, // and isn't an avatar that the viewing node has ignored // or that has ignored the viewing node - if ((nodeRaw->isIgnoringNodeWithID(avatarNode->getUUID()) && !PALIsOpen) - || (avatarNode->isIgnoringNodeWithID(nodeRaw->getUUID()) && !getsAnyIgnored)) { + if ((destinationNode->isIgnoringNodeWithID(avatarNode->getUUID()) && !PALIsOpen) + || (avatarNode->isIgnoringNodeWithID(destinationNode->getUUID()) && !getsAnyIgnored)) { shouldIgnore = true; } else { // Check to see if the space bubble is enabled // Don't bother with these checks if the other avatar has their bubble enabled and we're gettingAnyIgnored - if (nodeRaw->isIgnoreRadiusEnabled() || (avatarNode->isIgnoreRadiusEnabled() && !getsAnyIgnored)) { + if (destinationNode->isIgnoreRadiusEnabled() || (avatarNode->isIgnoreRadiusEnabled() && !getsAnyIgnored)) { float sensorToWorldScale = avatarNodeData->getAvatarSharedPointer()->getSensorToWorldScale(); // Define the scale of the box for the current other node glm::vec3 otherNodeBoxScale = (avatarNodeData->getPosition() - avatarNodeData->getGlobalBoundingBoxCorner()) * 2.0f * sensorToWorldScale; @@ -394,7 +392,7 @@ void AvatarMixerSlave::broadcastAvatarDataToAgent(const SharedNodePointer& node) // Perform the collision check between the two bounding boxes if (nodeBox.touches(otherNodeBox)) { - nodeData->ignoreOther(node, avatar._nodeShared); + nodeData->ignoreOther(destinationNode, avatarNode); shouldIgnore = !getsAnyIgnored; } } @@ -576,7 +574,7 @@ void AvatarMixerSlave::broadcastAvatarDataToAgent(const SharedNodePointer& node) _stats.numBytesSent += numAvatarDataBytes; // send the avatar data PacketList - nodeList->sendPacketList(std::move(avatarPacketList), *nodeRaw); + nodeList->sendPacketList(std::move(avatarPacketList), *destinationNode); // record the bytes sent for other avatar data in the AvatarMixerClientData nodeData->recordSentAvatarData(numAvatarDataBytes); @@ -586,7 +584,7 @@ void AvatarMixerSlave::broadcastAvatarDataToAgent(const SharedNodePointer& node) if (traitsPacketList->getNumPackets() >= 1) { // send the traits packet list - nodeList->sendPacketList(std::move(traitsPacketList), *nodeRaw); + nodeList->sendPacketList(std::move(traitsPacketList), *destinationNode); } // record the number of avatars held back this frame From c2ac2b9ab0f56aae0e8e283d2508cce10f0df0b3 Mon Sep 17 00:00:00 2001 From: Simon Walton Date: Thu, 30 Aug 2018 10:59:55 -0700 Subject: [PATCH 14/44] Various tweaks; fix clang warning --- .../src/avatars/AvatarMixerClientData.cpp | 2 +- .../src/avatars/AvatarMixerClientData.h | 2 +- .../src/avatars/AvatarMixerSlave.cpp | 16 +++++++--------- libraries/shared/src/SharedUtil.cpp | 6 +++--- 4 files changed, 12 insertions(+), 14 deletions(-) diff --git a/assignment-client/src/avatars/AvatarMixerClientData.cpp b/assignment-client/src/avatars/AvatarMixerClientData.cpp index 1dd4cc769b..c54db90f5d 100644 --- a/assignment-client/src/avatars/AvatarMixerClientData.cpp +++ b/assignment-client/src/avatars/AvatarMixerClientData.cpp @@ -239,7 +239,7 @@ void AvatarMixerClientData::ignoreOther(const Node* self, const Node* other) { } } -void AvatarMixerClientData::removeFromRadiusIgnoringSet(SharedNodePointer self, const QUuid& other) { +void AvatarMixerClientData::removeFromRadiusIgnoringSet(const QUuid& other) { _radiusIgnoredOthers.erase(other); } diff --git a/assignment-client/src/avatars/AvatarMixerClientData.h b/assignment-client/src/avatars/AvatarMixerClientData.h index 7d203bd771..ee27f9bb0f 100644 --- a/assignment-client/src/avatars/AvatarMixerClientData.h +++ b/assignment-client/src/avatars/AvatarMixerClientData.h @@ -93,7 +93,7 @@ public: glm::vec3 getGlobalBoundingBoxCorner() const { return _avatar ? _avatar->getGlobalBoundingBoxCorner() : glm::vec3(0); } bool isRadiusIgnoring(const QUuid& other) const { return _radiusIgnoredOthers.find(other) != _radiusIgnoredOthers.end(); } void addToRadiusIgnoringSet(const QUuid& other) { _radiusIgnoredOthers.insert(other); } - void removeFromRadiusIgnoringSet(SharedNodePointer self, const QUuid& other); + void removeFromRadiusIgnoringSet(const QUuid& other); void ignoreOther(SharedNodePointer self, SharedNodePointer other); void ignoreOther(const Node* self, const Node* other); diff --git a/assignment-client/src/avatars/AvatarMixerSlave.cpp b/assignment-client/src/avatars/AvatarMixerSlave.cpp index e772fa4d08..34a429bea0 100644 --- a/assignment-client/src/avatars/AvatarMixerSlave.cpp +++ b/assignment-client/src/avatars/AvatarMixerSlave.cpp @@ -276,16 +276,16 @@ void AvatarMixerSlave::broadcastAvatarDataToAgent(const SharedNodePointer& node) auto avatarPacketList = NLPacketList::create(PacketType::BulkAvatarData); // Define the minimum bubble size - static const glm::vec3 minBubbleSize = avatar.getSensorToWorldScale() * glm::vec3(0.3f, 1.3f, 0.3f); + const glm::vec3 minBubbleSize = avatar.getSensorToWorldScale() * glm::vec3(0.3f, 1.3f, 0.3f); // Define the scale of the box for the current node - glm::vec3 nodeBoxScale = (nodeData->getPosition() - nodeData->getGlobalBoundingBoxCorner()) * 2.0f * avatar.getSensorToWorldScale(); + glm::vec3 nodeBoxScale = (avatar.getClientGlobalPosition() - avatar.getGlobalBoundingBoxCorner()) * 2.0f * avatar.getSensorToWorldScale(); // Set up the bounding box for the current node - AABox nodeBox(nodeData->getGlobalBoundingBoxCorner(), nodeBoxScale); + AABox nodeBox(avatar.getGlobalBoundingBoxCorner(), nodeBoxScale); // Clamp the size of the bounding box to a minimum scale if (glm::any(glm::lessThan(nodeBoxScale, minBubbleSize))) { nodeBox.setScaleStayCentered(minBubbleSize); } - // Quadruple the scale of both bounding boxes + // Quadruple the scale of first bounding box nodeBox.embiggen(4.0f); @@ -386,7 +386,7 @@ void AvatarMixerSlave::broadcastAvatarDataToAgent(const SharedNodePointer& node) if (glm::any(glm::lessThan(otherNodeBoxScale, minBubbleSize))) { otherNodeBox.setScaleStayCentered(minBubbleSize); } - // Change the scale of both bounding boxes + // Change the scale of other bounding box // (This is an arbitrary number determined empirically) otherNodeBox.embiggen(2.4f); @@ -398,7 +398,7 @@ void AvatarMixerSlave::broadcastAvatarDataToAgent(const SharedNodePointer& node) } // Not close enough to ignore if (!shouldIgnore) { - nodeData->removeFromRadiusIgnoringSet(node, avatarNode->getUUID()); + nodeData->removeFromRadiusIgnoringSet(avatarNode->getUUID()); } } @@ -439,6 +439,7 @@ void AvatarMixerSlave::broadcastAvatarDataToAgent(const SharedNodePointer& node) auto traitsPacketList = NLPacketList::create(PacketType::BulkAvatarTraits, QByteArray(), true, true); while (!sortedAvatars.empty()) { const Node* otherNode = sortedAvatars.top().getNode(); + auto lastEncodeForOther = sortedAvatars.top().getTimestamp(); sortedAvatars.pop(); remainingAvatars--; @@ -490,7 +491,6 @@ void AvatarMixerSlave::broadcastAvatarDataToAgent(const SharedNodePointer& node) } bool includeThisAvatar = true; - auto lastEncodeForOther = nodeData->getLastOtherAvatarEncodeTime(otherNode->getUUID()); QVector& lastSentJointsForOther = nodeData->getLastOtherAvatarSentJoints(otherNode->getUUID()); lastSentJointsForOther.resize(otherAvatar->getJointCount()); @@ -561,8 +561,6 @@ void AvatarMixerSlave::broadcastAvatarDataToAgent(const SharedNodePointer& node) // use helper to add any changed traits to our packet list traitBytesSent += addChangedTraitsToBulkPacket(nodeData, otherNodeData, *traitsPacketList); - - traitsPacketList->getDataSize(); } quint64 startPacketSending = usecTimestampNow(); diff --git a/libraries/shared/src/SharedUtil.cpp b/libraries/shared/src/SharedUtil.cpp index 3bef7f7e18..a36bdb061e 100644 --- a/libraries/shared/src/SharedUtil.cpp +++ b/libraries/shared/src/SharedUtil.cpp @@ -128,9 +128,9 @@ void usecTimestampNowForceClockSkew(qint64 clockSkew) { ::usecTimestampNowAdjust = clockSkew; } -static std::atomic TIME_REFERENCE { 0 }; // in usec -static std::once_flag usecTimestampNowIsInitialized; -static QElapsedTimer timestampTimer; +//static std::atomic TIME_REFERENCE { 0 }; // in usec +//static std::once_flag usecTimestampNowIsInitialized; +//static QElapsedTimer timestampTimer; quint64 usecTimestampNow(bool wantDebug) { using namespace std::chrono; From b45b0f54e4ebd58d6ebe87e67c10a1089902d8bf Mon Sep 17 00:00:00 2001 From: Simon Walton Date: Thu, 30 Aug 2018 11:59:53 -0700 Subject: [PATCH 15/44] Move unfilteredHandlers processing back into main loop --- libraries/networking/src/udt/Socket.cpp | 36 ++++++++++--------------- 1 file changed, 14 insertions(+), 22 deletions(-) diff --git a/libraries/networking/src/udt/Socket.cpp b/libraries/networking/src/udt/Socket.cpp index d764b9e8b2..44220df8f8 100644 --- a/libraries/networking/src/udt/Socket.cpp +++ b/libraries/networking/src/udt/Socket.cpp @@ -361,28 +361,6 @@ void Socket::processPendingDatagrams(int) { // setup a HifiSockAddr to read into HifiSockAddr senderSockAddr; - // Process unfiltered packets first. - for (auto datagramIter = _incomingDatagrams.begin(); datagramIter != _incomingDatagrams.end(); ++datagramIter) { - senderSockAddr.setAddress(datagramIter->_senderAddress); - senderSockAddr.setPort(datagramIter->_senderPort); - auto it = _unfilteredHandlers.find(senderSockAddr); - if (it != _unfilteredHandlers.end()) { - // we have a registered unfiltered handler for this HifiSockAddr (eg. STUN packet) - call that and return - if (it->second) { - auto basePacket = BasePacket::fromReceivedPacket(std::move(datagramIter->_datagram), - datagramIter->_datagramLength, - senderSockAddr); - basePacket->setReceiveTime(datagramIter->_receiveTime); - it->second(std::move(basePacket)); - } - - datagramIter = _incomingDatagrams.erase(datagramIter); - if (datagramIter == _incomingDatagrams.end()) { - break; - } - } - } - while (!_incomingDatagrams.empty()) { auto& datagram = _incomingDatagrams.front(); senderSockAddr.setAddress(datagram._senderAddress); @@ -397,6 +375,20 @@ void Socket::processPendingDatagrams(int) { _lastPacketSizeRead = datagramSize; _lastPacketSockAddr = senderSockAddr; + // Process unfiltered packets first. + auto it = _unfilteredHandlers.find(senderSockAddr); + if (it != _unfilteredHandlers.end()) { + // we have a registered unfiltered handler for this HifiSockAddr (eg. STUN packet) - call that and return + if (it->second) { + auto basePacket = BasePacket::fromReceivedPacket(std::move(datagram._datagram), + datagramSize, senderSockAddr); + basePacket->setReceiveTime(receiveTime); + it->second(std::move(basePacket)); + } + _incomingDatagrams.pop_front(); + continue; + } + // check if this was a control packet or a data packet bool isControlPacket = *reinterpret_cast(datagram._datagram.get()) & CONTROL_BIT_MASK; From 33db1394e6dde359fda68a131354f7a56066c42e Mon Sep 17 00:00:00 2001 From: Simon Walton Date: Thu, 30 Aug 2018 17:12:42 -0700 Subject: [PATCH 16/44] Use system_clock in usecTimestampNow for cross-platform goodness --- libraries/shared/src/SharedUtil.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/libraries/shared/src/SharedUtil.cpp b/libraries/shared/src/SharedUtil.cpp index a36bdb061e..6c79a23c20 100644 --- a/libraries/shared/src/SharedUtil.cpp +++ b/libraries/shared/src/SharedUtil.cpp @@ -134,7 +134,8 @@ void usecTimestampNowForceClockSkew(qint64 clockSkew) { quint64 usecTimestampNow(bool wantDebug) { using namespace std::chrono; - return duration_cast(high_resolution_clock::now().time_since_epoch()).count(); + static const auto unixEpoch = system_clock::from_time_t(0); + return duration_cast(system_clock::now() - unixEpoch).count(); #if 0 std::call_once(usecTimestampNowIsInitialized, [&] { TIME_REFERENCE = QDateTime::currentMSecsSinceEpoch() * USECS_PER_MSEC; // ms to usec From e1d51a6c425de1f91818bd98901665fcc64aeb8e Mon Sep 17 00:00:00 2001 From: Simon Walton Date: Fri, 31 Aug 2018 15:06:33 -0700 Subject: [PATCH 17/44] Avatar serializing: use memcpy where possible, take copy of joint data to avoid holding lock, etc --- libraries/avatars/src/AvatarData.cpp | 145 ++++++++++----------------- 1 file changed, 51 insertions(+), 94 deletions(-) diff --git a/libraries/avatars/src/AvatarData.cpp b/libraries/avatars/src/AvatarData.cpp index 262bf2a567..ca891eb246 100644 --- a/libraries/avatars/src/AvatarData.cpp +++ b/libraries/avatars/src/AvatarData.cpp @@ -363,13 +363,13 @@ QByteArray AvatarData::toByteArray(AvatarDataDetail dataDetail, quint64 lastSent memcpy(destinationBuffer, &packetStateFlags, sizeof(packetStateFlags)); destinationBuffer += sizeof(packetStateFlags); +#define AVATAR_MEMCPY(src) \ + memcpy(destinationBuffer, &(src), sizeof(src)); \ + destinationBuffer += sizeof(src); + if (hasAvatarGlobalPosition) { auto startSection = destinationBuffer; - auto data = reinterpret_cast(destinationBuffer); - data->globalPosition[0] = _globalPosition.x; - data->globalPosition[1] = _globalPosition.y; - data->globalPosition[2] = _globalPosition.z; - destinationBuffer += sizeof(AvatarDataPacket::AvatarGlobalPosition); + AVATAR_MEMCPY(_globalPosition); int numBytes = destinationBuffer - startSection; @@ -380,17 +380,8 @@ QByteArray AvatarData::toByteArray(AvatarDataDetail dataDetail, quint64 lastSent if (hasAvatarBoundingBox) { auto startSection = destinationBuffer; - auto data = reinterpret_cast(destinationBuffer); - - data->avatarDimensions[0] = _globalBoundingBoxDimensions.x; - data->avatarDimensions[1] = _globalBoundingBoxDimensions.y; - data->avatarDimensions[2] = _globalBoundingBoxDimensions.z; - - data->boundOriginOffset[0] = _globalBoundingBoxOffset.x; - data->boundOriginOffset[1] = _globalBoundingBoxOffset.y; - data->boundOriginOffset[2] = _globalBoundingBoxOffset.z; - - destinationBuffer += sizeof(AvatarDataPacket::AvatarBoundingBox); + AVATAR_MEMCPY(_globalBoundingBoxDimensions); + AVATAR_MEMCPY(_globalBoundingBoxOffset); int numBytes = destinationBuffer - startSection; if (outboundDataRateOut) { @@ -424,13 +415,7 @@ QByteArray AvatarData::toByteArray(AvatarDataDetail dataDetail, quint64 lastSent if (hasLookAtPosition) { auto startSection = destinationBuffer; - auto data = reinterpret_cast(destinationBuffer); - auto lookAt = _headData->getLookAtPosition(); - data->lookAtPosition[0] = lookAt.x; - data->lookAtPosition[1] = lookAt.y; - data->lookAtPosition[2] = lookAt.z; - destinationBuffer += sizeof(AvatarDataPacket::LookAtPosition); - + AVATAR_MEMCPY(_headData->getLookAtPosition()); int numBytes = destinationBuffer - startSection; if (outboundDataRateOut) { outboundDataRateOut->lookAtPositionRate.increment(numBytes); @@ -531,12 +516,7 @@ QByteArray AvatarData::toByteArray(AvatarDataDetail dataDetail, quint64 lastSent if (hasAvatarLocalPosition) { auto startSection = destinationBuffer; - auto data = reinterpret_cast(destinationBuffer); - auto localPosition = getLocalPosition(); - data->localPosition[0] = localPosition.x; - data->localPosition[1] = localPosition.y; - data->localPosition[2] = localPosition.z; - destinationBuffer += sizeof(AvatarDataPacket::AvatarLocalPosition); + AVATAR_MEMCPY(getLocalPosition()); int numBytes = destinationBuffer - startSection; if (outboundDataRateOut) { @@ -572,14 +552,17 @@ QByteArray AvatarData::toByteArray(AvatarDataDetail dataDetail, quint64 lastSent auto startSection = destinationBuffer; QReadLocker readLock(&_jointDataLock); + const QVector jointData(_jointData); + readLock.unlock(); // Unlock quickly. + // joint rotation data - int numJoints = _jointData.size(); + int numJoints = jointData.size(); *destinationBuffer++ = (uint8_t)numJoints; unsigned char* validityPosition = destinationBuffer; unsigned char validity = 0; int validityBit = 0; - int numValidityBytes = (int)std::ceil(numJoints / (float)BITS_IN_BYTE); + int numValidityBytes = (numJoints + BITS_IN_BYTE - 1) / BITS_IN_BYTE; #ifdef WANT_DEBUG int rotationSentCount = 0; @@ -595,24 +578,19 @@ QByteArray AvatarData::toByteArray(AvatarDataDetail dataDetail, quint64 lastSent localSentJointDataOut.resize(numJoints); // Make sure the destination is resized before using it } - float minRotationDOT = !distanceAdjust ? AVATAR_MIN_ROTATION_DOT : getDistanceBasedMinRotationDOT(viewerPosition); + float minRotationDOT = (distanceAdjust && cullSmallChanges) ? getDistanceBasedMinRotationDOT(viewerPosition) : AVATAR_MIN_ROTATION_DOT; - for (int i = 0; i < _jointData.size(); i++) { - const JointData& data = _jointData[i]; + for (int i = 0; i < jointData.size(); i++) { + const JointData& data = jointData[i]; const JointData& last = lastSentJointData[i]; if (!data.rotationIsDefaultPose) { bool mustSend = sendAll || last.rotationIsDefaultPose; if (mustSend || last.rotation != data.rotation) { - bool largeEnoughRotation = true; - if (cullSmallChanges) { - // The dot product for smaller rotations is a smaller number. - // So if the dot() is less than the value, then the rotation is a larger angle of rotation - largeEnoughRotation = fabsf(glm::dot(last.rotation, data.rotation)) < minRotationDOT; - } - - if (mustSend || !cullSmallChanges || largeEnoughRotation) { + // The dot product for larger rotations is a lower number. + // So if the dot() is less than the value, then the rotation is a larger angle of rotation + if (!cullSmallChanges || fabsf(glm::dot(last.rotation, data.rotation)) < minRotationDOT) { validity |= (1 << validityBit); #ifdef WANT_DEBUG rotationSentCount++; @@ -647,17 +625,17 @@ QByteArray AvatarData::toByteArray(AvatarDataDetail dataDetail, quint64 lastSent destinationBuffer += numValidityBytes; // Move pointer past the validity bytes - float minTranslation = !distanceAdjust ? AVATAR_MIN_TRANSLATION : getDistanceBasedMinTranslationDistance(viewerPosition); + float minTranslation = (distanceAdjust && cullSmallChanges) ? getDistanceBasedMinTranslationDistance(viewerPosition) : AVATAR_MIN_ROTATION_DOT; float maxTranslationDimension = 0.0; - for (int i = 0; i < _jointData.size(); i++) { - const JointData& data = _jointData[i]; + for (int i = 0; i < jointData.size(); i++) { + const JointData& data = jointData[i]; const JointData& last = lastSentJointData[i]; if (!data.translationIsDefaultPose) { bool mustSend = sendAll || last.translationIsDefaultPose; if (mustSend || last.translation != data.translation) { - if (mustSend || !cullSmallChanges || glm::distance(data.translation, lastSentJointData[i].translation) > minTranslation) { + if (!cullSmallChanges || glm::distance(data.translation, lastSentJointData[i].translation) > minTranslation) { validity |= (1 << validityBit); #ifdef WANT_DEBUG translationSentCount++; @@ -707,34 +685,12 @@ QByteArray AvatarData::toByteArray(AvatarDataDetail dataDetail, quint64 lastSent glm::vec3 mouseFarGrabPosition = extractTranslation(mouseFarGrabMatrix); glm::quat mouseFarGrabRotation = extractRotation(mouseFarGrabMatrix); - data->leftFarGrabPosition[0] = leftFarGrabPosition.x; - data->leftFarGrabPosition[1] = leftFarGrabPosition.y; - data->leftFarGrabPosition[2] = leftFarGrabPosition.z; - - data->leftFarGrabRotation[0] = leftFarGrabRotation.w; - data->leftFarGrabRotation[1] = leftFarGrabRotation.x; - data->leftFarGrabRotation[2] = leftFarGrabRotation.y; - data->leftFarGrabRotation[3] = leftFarGrabRotation.z; - - data->rightFarGrabPosition[0] = rightFarGrabPosition.x; - data->rightFarGrabPosition[1] = rightFarGrabPosition.y; - data->rightFarGrabPosition[2] = rightFarGrabPosition.z; - - data->rightFarGrabRotation[0] = rightFarGrabRotation.w; - data->rightFarGrabRotation[1] = rightFarGrabRotation.x; - data->rightFarGrabRotation[2] = rightFarGrabRotation.y; - data->rightFarGrabRotation[3] = rightFarGrabRotation.z; - - data->mouseFarGrabPosition[0] = mouseFarGrabPosition.x; - data->mouseFarGrabPosition[1] = mouseFarGrabPosition.y; - data->mouseFarGrabPosition[2] = mouseFarGrabPosition.z; - - data->mouseFarGrabRotation[0] = mouseFarGrabRotation.w; - data->mouseFarGrabRotation[1] = mouseFarGrabRotation.x; - data->mouseFarGrabRotation[2] = mouseFarGrabRotation.y; - data->mouseFarGrabRotation[3] = mouseFarGrabRotation.z; - - destinationBuffer += sizeof(AvatarDataPacket::FarGrabJoints); + AVATAR_MEMCPY(leftFarGrabPosition); + AVATAR_MEMCPY(leftFarGrabRotation); + AVATAR_MEMCPY(rightFarGrabPosition); + AVATAR_MEMCPY(rightFarGrabRotation); + AVATAR_MEMCPY(mouseFarGrabPosition); + AVATAR_MEMCPY(mouseFarGrabRotation); int numBytes = destinationBuffer - startSection; @@ -764,8 +720,8 @@ QByteArray AvatarData::toByteArray(AvatarDataDetail dataDetail, quint64 lastSent if (sentJointDataOut) { // Mark default poses in lastSentJointData, so when they become non-default we send them. - for (int i = 0; i < _jointData.size(); i++) { - const JointData& data = _jointData[i]; + for (int i = 0; i < jointData.size(); i++) { + const JointData& data = jointData[i]; JointData& local = localSentJointDataOut[i]; if (data.rotationIsDefaultPose) { local.rotationIsDefaultPose = true; @@ -778,30 +734,31 @@ QByteArray AvatarData::toByteArray(AvatarDataDetail dataDetail, quint64 lastSent // Push new sent joint data to sentJointDataOut sentJointDataOut->swap(localSentJointDataOut); } - } - if (hasJointDefaultPoseFlags) { - auto startSection = destinationBuffer; - QReadLocker readLock(&_jointDataLock); + // Always true, currently: + if (hasJointDefaultPoseFlags) { + auto startSection = destinationBuffer; - // write numJoints - int numJoints = _jointData.size(); - *destinationBuffer++ = (uint8_t)numJoints; + // write numJoints + int numJoints = jointData.size(); + *destinationBuffer++ = (uint8_t)numJoints; - // write rotationIsDefaultPose bits - destinationBuffer += writeBitVector(destinationBuffer, numJoints, [&](int i) { - return _jointData[i].rotationIsDefaultPose; - }); + // write rotationIsDefaultPose bits + destinationBuffer += writeBitVector(destinationBuffer, numJoints, [&](int i) { + return jointData[i].rotationIsDefaultPose; + }); - // write translationIsDefaultPose bits - destinationBuffer += writeBitVector(destinationBuffer, numJoints, [&](int i) { - return _jointData[i].translationIsDefaultPose; - }); + // write translationIsDefaultPose bits + destinationBuffer += writeBitVector(destinationBuffer, numJoints, [&](int i) { + return jointData[i].translationIsDefaultPose; + }); - if (outboundDataRateOut) { - size_t numBytes = destinationBuffer - startSection; - outboundDataRateOut->jointDefaultPoseFlagsRate.increment(numBytes); + if (outboundDataRateOut) { + size_t numBytes = destinationBuffer - startSection; + outboundDataRateOut->jointDefaultPoseFlagsRate.increment(numBytes); + } } + } int avatarDataSize = destinationBuffer - startPosition; From 2da533857475223df5f2e2c4159a01588ba8c406 Mon Sep 17 00:00:00 2001 From: Simon Walton Date: Fri, 31 Aug 2018 15:24:51 -0700 Subject: [PATCH 18/44] Fix couple of gcc issues --- libraries/avatars/src/AvatarData.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libraries/avatars/src/AvatarData.cpp b/libraries/avatars/src/AvatarData.cpp index ca891eb246..4363607e71 100644 --- a/libraries/avatars/src/AvatarData.cpp +++ b/libraries/avatars/src/AvatarData.cpp @@ -516,7 +516,8 @@ QByteArray AvatarData::toByteArray(AvatarDataDetail dataDetail, quint64 lastSent if (hasAvatarLocalPosition) { auto startSection = destinationBuffer; - AVATAR_MEMCPY(getLocalPosition()); + const auto localPosition = getLocalPosition(); + AVATAR_MEMCPY(localPosition); int numBytes = destinationBuffer - startSection; if (outboundDataRateOut) { @@ -677,7 +678,6 @@ QByteArray AvatarData::toByteArray(AvatarDataDetail dataDetail, quint64 lastSent if (hasGrabJoints) { // the far-grab joints may range further than 3 meters, so we can't use packFloatVec3ToSignedTwoByteFixed etc auto startSection = destinationBuffer; - auto data = reinterpret_cast(destinationBuffer); glm::vec3 leftFarGrabPosition = extractTranslation(leftFarGrabMatrix); glm::quat leftFarGrabRotation = extractRotation(leftFarGrabMatrix); glm::vec3 rightFarGrabPosition = extractTranslation(rightFarGrabMatrix); From 5a0de0f103cda82906cd1d47aa38b190b9d3c470 Mon Sep 17 00:00:00 2001 From: Simon Walton Date: Fri, 31 Aug 2018 16:45:39 -0700 Subject: [PATCH 19/44] Small fixes for joint logic --- libraries/avatars/src/AvatarData.cpp | 50 +++++++++++++--------------- 1 file changed, 23 insertions(+), 27 deletions(-) diff --git a/libraries/avatars/src/AvatarData.cpp b/libraries/avatars/src/AvatarData.cpp index 4363607e71..c7a506a782 100644 --- a/libraries/avatars/src/AvatarData.cpp +++ b/libraries/avatars/src/AvatarData.cpp @@ -586,22 +586,19 @@ QByteArray AvatarData::toByteArray(AvatarDataDetail dataDetail, quint64 lastSent const JointData& last = lastSentJointData[i]; if (!data.rotationIsDefaultPose) { - bool mustSend = sendAll || last.rotationIsDefaultPose; - if (mustSend || last.rotation != data.rotation) { - - // The dot product for larger rotations is a lower number. - // So if the dot() is less than the value, then the rotation is a larger angle of rotation - if (!cullSmallChanges || fabsf(glm::dot(last.rotation, data.rotation)) < minRotationDOT) { - validity |= (1 << validityBit); + // The dot product for larger rotations is a lower number. + // So if the dot() is less than the value, then the rotation is a larger angle of rotation + if (sendAll || last.rotationIsDefaultPose || (!cullSmallChanges && last.rotation != data.rotation) + || (cullSmallChanges && glm::dot(last.rotation, data.rotation) < minRotationDOT) ) { + validity |= (1 << validityBit); #ifdef WANT_DEBUG - rotationSentCount++; + rotationSentCount++; #endif - destinationBuffer += packOrientationQuatToSixBytes(destinationBuffer, data.rotation); + destinationBuffer += packOrientationQuatToSixBytes(destinationBuffer, data.rotation); - if (sentJointDataOut) { - localSentJointDataOut[i].rotation = data.rotation; - localSentJointDataOut[i].rotationIsDefaultPose = false; - } + if (sentJointDataOut) { + localSentJointDataOut[i].rotation = data.rotation; + localSentJointDataOut[i].rotationIsDefaultPose = false; } } } @@ -634,24 +631,23 @@ QByteArray AvatarData::toByteArray(AvatarDataDetail dataDetail, quint64 lastSent const JointData& last = lastSentJointData[i]; if (!data.translationIsDefaultPose) { - bool mustSend = sendAll || last.translationIsDefaultPose; - if (mustSend || last.translation != data.translation) { - if (!cullSmallChanges || glm::distance(data.translation, lastSentJointData[i].translation) > minTranslation) { - validity |= (1 << validityBit); + if (sendAll || last.translationIsDefaultPose || (!cullSmallChanges && last.translation != data.translation) + || (cullSmallChanges && glm::distance(data.translation, lastSentJointData[i].translation) > minTranslation)) { + + validity |= (1 << validityBit); #ifdef WANT_DEBUG - translationSentCount++; + translationSentCount++; #endif - maxTranslationDimension = glm::max(fabsf(data.translation.x), maxTranslationDimension); - maxTranslationDimension = glm::max(fabsf(data.translation.y), maxTranslationDimension); - maxTranslationDimension = glm::max(fabsf(data.translation.z), maxTranslationDimension); + maxTranslationDimension = glm::max(fabsf(data.translation.x), maxTranslationDimension); + maxTranslationDimension = glm::max(fabsf(data.translation.y), maxTranslationDimension); + maxTranslationDimension = glm::max(fabsf(data.translation.z), maxTranslationDimension); - destinationBuffer += - packFloatVec3ToSignedTwoByteFixed(destinationBuffer, data.translation, TRANSLATION_COMPRESSION_RADIX); + destinationBuffer += + packFloatVec3ToSignedTwoByteFixed(destinationBuffer, data.translation, TRANSLATION_COMPRESSION_RADIX); - if (sentJointDataOut) { - localSentJointDataOut[i].translation = data.translation; - localSentJointDataOut[i].translationIsDefaultPose = false; - } + if (sentJointDataOut) { + localSentJointDataOut[i].translation = data.translation; + localSentJointDataOut[i].translationIsDefaultPose = false; } } } From cf5f81ab95da956565f1d6035f1e38cec12bc48d Mon Sep 17 00:00:00 2001 From: Simon Walton Date: Fri, 31 Aug 2018 17:04:23 -0700 Subject: [PATCH 20/44] Don't memcpy quaternions as we don't use same component order on wire! --- libraries/avatars/src/AvatarData.cpp | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/libraries/avatars/src/AvatarData.cpp b/libraries/avatars/src/AvatarData.cpp index c7a506a782..d60a1a1310 100644 --- a/libraries/avatars/src/AvatarData.cpp +++ b/libraries/avatars/src/AvatarData.cpp @@ -666,6 +666,7 @@ QByteArray AvatarData::toByteArray(AvatarDataDetail dataDetail, quint64 lastSent destinationBuffer += packOrientationQuatToSixBytes(destinationBuffer, controllerLeftHandTransform.getRotation()); destinationBuffer += packFloatVec3ToSignedTwoByteFixed(destinationBuffer, controllerLeftHandTransform.getTranslation(), TRANSLATION_COMPRESSION_RADIX); + Transform controllerRightHandTransform = Transform(getControllerRightHandMatrix()); destinationBuffer += packOrientationQuatToSixBytes(destinationBuffer, controllerRightHandTransform.getRotation()); destinationBuffer += packFloatVec3ToSignedTwoByteFixed(destinationBuffer, controllerRightHandTransform.getTranslation(), @@ -674,6 +675,7 @@ QByteArray AvatarData::toByteArray(AvatarDataDetail dataDetail, quint64 lastSent if (hasGrabJoints) { // the far-grab joints may range further than 3 meters, so we can't use packFloatVec3ToSignedTwoByteFixed etc auto startSection = destinationBuffer; + auto data = reinterpret_cast(destinationBuffer); glm::vec3 leftFarGrabPosition = extractTranslation(leftFarGrabMatrix); glm::quat leftFarGrabRotation = extractRotation(leftFarGrabMatrix); glm::vec3 rightFarGrabPosition = extractTranslation(rightFarGrabMatrix); @@ -682,11 +684,25 @@ QByteArray AvatarData::toByteArray(AvatarDataDetail dataDetail, quint64 lastSent glm::quat mouseFarGrabRotation = extractRotation(mouseFarGrabMatrix); AVATAR_MEMCPY(leftFarGrabPosition); - AVATAR_MEMCPY(leftFarGrabRotation); + data->leftFarGrabRotation[0] = leftFarGrabRotation.w; + data->leftFarGrabRotation[1] = leftFarGrabRotation.x; + data->leftFarGrabRotation[2] = leftFarGrabRotation.y; + data->leftFarGrabRotation[3] = leftFarGrabRotation.z; + destinationBuffer += sizeof(data->leftFarGrabPosition); + AVATAR_MEMCPY(rightFarGrabPosition); - AVATAR_MEMCPY(rightFarGrabRotation); + data->rightFarGrabRotation[0] = rightFarGrabRotation.w; + data->rightFarGrabRotation[1] = rightFarGrabRotation.x; + data->rightFarGrabRotation[2] = rightFarGrabRotation.y; + data->rightFarGrabRotation[3] = rightFarGrabRotation.z; + destinationBuffer += sizeof(data->rightFarGrabRotation); + AVATAR_MEMCPY(mouseFarGrabPosition); - AVATAR_MEMCPY(mouseFarGrabRotation); + data->mouseFarGrabRotation[0] = mouseFarGrabRotation.w; + data->mouseFarGrabRotation[1] = mouseFarGrabRotation.x; + data->mouseFarGrabRotation[2] = mouseFarGrabRotation.y; + data->mouseFarGrabRotation[3] = mouseFarGrabRotation.z; + destinationBuffer += sizeof(data->mouseFarGrabRotation); int numBytes = destinationBuffer - startSection; From 11a563cb386290c254040e4a5a0f1a79326af3e2 Mon Sep 17 00:00:00 2001 From: Simon Walton Date: Fri, 31 Aug 2018 17:40:57 -0700 Subject: [PATCH 21/44] Don't use temp vector for outgoing joint status - doesn't seem to be necessary --- libraries/avatars/src/AvatarData.cpp | 34 ++++++++-------------------- 1 file changed, 9 insertions(+), 25 deletions(-) diff --git a/libraries/avatars/src/AvatarData.cpp b/libraries/avatars/src/AvatarData.cpp index d60a1a1310..ed1a8e5742 100644 --- a/libraries/avatars/src/AvatarData.cpp +++ b/libraries/avatars/src/AvatarData.cpp @@ -573,10 +573,8 @@ QByteArray AvatarData::toByteArray(AvatarDataDetail dataDetail, quint64 lastSent destinationBuffer += numValidityBytes; // Move pointer past the validity bytes // sentJointDataOut and lastSentJointData might be the same vector - // build sentJointDataOut locally and then swap it at the end. - QVector localSentJointDataOut; if (sentJointDataOut) { - localSentJointDataOut.resize(numJoints); // Make sure the destination is resized before using it + sentJointDataOut->resize(numJoints); // Make sure the destination is resized before using it } float minRotationDOT = (distanceAdjust && cullSmallChanges) ? getDistanceBasedMinRotationDOT(viewerPosition) : AVATAR_MIN_ROTATION_DOT; @@ -597,11 +595,13 @@ QByteArray AvatarData::toByteArray(AvatarDataDetail dataDetail, quint64 lastSent destinationBuffer += packOrientationQuatToSixBytes(destinationBuffer, data.rotation); if (sentJointDataOut) { - localSentJointDataOut[i].rotation = data.rotation; - localSentJointDataOut[i].rotationIsDefaultPose = false; + (*sentJointDataOut)[i].rotation = data.rotation; } } } + + (*sentJointDataOut)[i].rotationIsDefaultPose = data.rotationIsDefaultPose; + if (++validityBit == BITS_IN_BYTE) { *validityPosition++ = validity; validityBit = validity = 0; @@ -646,11 +646,13 @@ QByteArray AvatarData::toByteArray(AvatarDataDetail dataDetail, quint64 lastSent packFloatVec3ToSignedTwoByteFixed(destinationBuffer, data.translation, TRANSLATION_COMPRESSION_RADIX); if (sentJointDataOut) { - localSentJointDataOut[i].translation = data.translation; - localSentJointDataOut[i].translationIsDefaultPose = false; + (*sentJointDataOut)[i].translation = data.translation; } } } + + (*sentJointDataOut)[i].translationIsDefaultPose = data.translationIsDefaultPose; + if (++validityBit == BITS_IN_BYTE) { *validityPosition++ = validity; validityBit = validity = 0; @@ -729,24 +731,6 @@ QByteArray AvatarData::toByteArray(AvatarDataDetail dataDetail, quint64 lastSent outboundDataRateOut->jointDataRate.increment(numBytes); } - if (sentJointDataOut) { - - // Mark default poses in lastSentJointData, so when they become non-default we send them. - for (int i = 0; i < jointData.size(); i++) { - const JointData& data = jointData[i]; - JointData& local = localSentJointDataOut[i]; - if (data.rotationIsDefaultPose) { - local.rotationIsDefaultPose = true; - } - if (data.translationIsDefaultPose) { - local.translationIsDefaultPose = true; - } - } - - // Push new sent joint data to sentJointDataOut - sentJointDataOut->swap(localSentJointDataOut); - } - // Always true, currently: if (hasJointDefaultPoseFlags) { auto startSection = destinationBuffer; From 058115b79142c4b591c89cf9a9c6f64f0fcf0986 Mon Sep 17 00:00:00 2001 From: Simon Walton Date: Tue, 4 Sep 2018 10:38:18 -0700 Subject: [PATCH 22/44] Add guard for sentJointDataOut --- libraries/avatars/src/AvatarData.cpp | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/libraries/avatars/src/AvatarData.cpp b/libraries/avatars/src/AvatarData.cpp index f2f4e2fd7d..3e822fd17a 100644 --- a/libraries/avatars/src/AvatarData.cpp +++ b/libraries/avatars/src/AvatarData.cpp @@ -600,7 +600,9 @@ QByteArray AvatarData::toByteArray(AvatarDataDetail dataDetail, quint64 lastSent } } - (*sentJointDataOut)[i].rotationIsDefaultPose = data.rotationIsDefaultPose; + if (sentJointDataOut) { + (*sentJointDataOut)[i].rotationIsDefaultPose = data.rotationIsDefaultPose; + } if (++validityBit == BITS_IN_BYTE) { *validityPosition++ = validity; @@ -651,7 +653,9 @@ QByteArray AvatarData::toByteArray(AvatarDataDetail dataDetail, quint64 lastSent } } - (*sentJointDataOut)[i].translationIsDefaultPose = data.translationIsDefaultPose; + if (sentJointDataOut) { + (*sentJointDataOut)[i].translationIsDefaultPose = data.translationIsDefaultPose; + } if (++validityBit == BITS_IN_BYTE) { *validityPosition++ = validity; From 0738bcaebc1447d0e7689cb2fbeec6ae3bedf265 Mon Sep 17 00:00:00 2001 From: Simon Walton Date: Tue, 4 Sep 2018 18:08:34 -0700 Subject: [PATCH 23/44] Unused variable from merge --- assignment-client/src/avatars/AvatarMixerSlave.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/assignment-client/src/avatars/AvatarMixerSlave.cpp b/assignment-client/src/avatars/AvatarMixerSlave.cpp index 7d1ac25469..81ee9c18f8 100644 --- a/assignment-client/src/avatars/AvatarMixerSlave.cpp +++ b/assignment-client/src/avatars/AvatarMixerSlave.cpp @@ -440,7 +440,6 @@ void AvatarMixerSlave::broadcastAvatarDataToAgent(const SharedNodePointer& node) auto traitsPacketList = NLPacketList::create(PacketType::BulkAvatarTraits, QByteArray(), true, true); const auto& sortedAvatarVector = sortedAvatars.getSortedVector(); for (const auto& sortedAvatar : sortedAvatarVector) { - const auto& avatarData = sortedAvatar.getAvatar(); const Node* otherNode = sortedAvatar.getNode(); auto lastEncodeForOther = sortedAvatar.getTimestamp(); remainingAvatars--; From f7e84995b41961d08ac6ecdcffef170c8b6c3253 Mon Sep 17 00:00:00 2001 From: Simon Walton Date: Wed, 5 Sep 2018 10:58:37 -0700 Subject: [PATCH 24/44] Only build the sortable vector once, now we're using vectors for the priority sort --- .../src/avatars/AvatarMixerSlave.cpp | 60 ++++++------------- 1 file changed, 18 insertions(+), 42 deletions(-) diff --git a/assignment-client/src/avatars/AvatarMixerSlave.cpp b/assignment-client/src/avatars/AvatarMixerSlave.cpp index 81ee9c18f8..a61f65ffb0 100644 --- a/assignment-client/src/avatars/AvatarMixerSlave.cpp +++ b/assignment-client/src/avatars/AvatarMixerSlave.cpp @@ -288,35 +288,6 @@ void AvatarMixerSlave::broadcastAvatarDataToAgent(const SharedNodePointer& node) // Quadruple the scale of first bounding box nodeBox.embiggen(4.0f); - - // setup list of AvatarData as well as maps to map betweeen the AvatarData and the original nodes - struct AvatarSortData { - AvatarSortData(const Node* node, AvatarData* avatarData, quint64 lastEncodeTime) - : _node(node) - , _avatarData(avatarData) - , _lastEncodeTime(lastEncodeTime) - { } - const Node* _node; - AvatarData* _avatarData; - quint64 _lastEncodeTime; - }; - // Temporary info about the avatars we're sending: - std::vector avatarsToSort; - avatarsToSort.reserve(_end - _begin); - std::for_each(_begin, _end, [&](const SharedNodePointer& otherNode) { - Node* otherNodeRaw = otherNode.data(); - // make sure this is an agent that we have avatar data for before considering it for inclusion - if (otherNodeRaw->getType() == NodeType::Agent - && otherNodeRaw->getLinkedData()) { - const AvatarMixerClientData* otherNodeData = reinterpret_cast(otherNodeRaw->getLinkedData()); - - - AvatarData* otherAvatar = otherNodeData->getAvatarSharedPointer().get(); - auto lastEncodeTime = nodeData->getLastOtherAvatarEncodeTime(otherAvatar->getSessionUUID()); - avatarsToSort.emplace_back(AvatarSortData(otherNodeRaw, otherAvatar, lastEncodeTime)); - } - }); - class SortableAvatar: public PrioritySortUtil::Sortable { public: SortableAvatar() = delete; @@ -345,16 +316,18 @@ void AvatarMixerSlave::broadcastAvatarDataToAgent(const SharedNodePointer& node) AvatarData::_avatarSortCoefficientSize, AvatarData::_avatarSortCoefficientCenter, AvatarData::_avatarSortCoefficientAge); - sortedAvatars.reserve(avatarsToSort.size()); + sortedAvatars.reserve(_end - _begin); - // ignore or sort - for (const auto& avatar : avatarsToSort) { - auto avatarNode = avatar._node; - if (avatarNode == destinationNode) { - // don't echo updates to self + for (auto listedNode = _begin; listedNode != _end; ++listedNode) { + Node* otherNodeRaw = (*listedNode).data(); + if (otherNodeRaw->getType() != NodeType::Agent + || !otherNodeRaw->getLinkedData() + || otherNodeRaw == destinationNode) { continue; } + auto avatarNode = otherNodeRaw; + bool shouldIgnore = false; // We ignore other nodes for a couple of reasons: // 1) ignore bubbles and ignore specific node @@ -364,8 +337,8 @@ void AvatarMixerSlave::broadcastAvatarDataToAgent(const SharedNodePointer& node) assert(avatarNode); // we can't have gotten here without the avatarData being a valid key in the map - const AvatarMixerClientData* avatarNodeData = reinterpret_cast(avatarNode->getLinkedData()); - assert(avatarNodeData); // we can't have gotten here without avatarNode having valid data + const AvatarMixerClientData* avatarClientNodeData = reinterpret_cast(avatarNode->getLinkedData()); + assert(avatarClientNodeData); // we can't have gotten here without avatarNode having valid data quint64 startIgnoreCalculation = usecTimestampNow(); // make sure we have data for this avatar, that it isn't the same node, @@ -378,11 +351,11 @@ void AvatarMixerSlave::broadcastAvatarDataToAgent(const SharedNodePointer& node) // Check to see if the space bubble is enabled // Don't bother with these checks if the other avatar has their bubble enabled and we're gettingAnyIgnored if (destinationNode->isIgnoreRadiusEnabled() || (avatarNode->isIgnoreRadiusEnabled() && !getsAnyIgnored)) { - float sensorToWorldScale = avatarNodeData->getAvatarSharedPointer()->getSensorToWorldScale(); + float sensorToWorldScale = avatarClientNodeData->getAvatarSharedPointer()->getSensorToWorldScale(); // Define the scale of the box for the current other node - glm::vec3 otherNodeBoxScale = (avatarNodeData->getPosition() - avatarNodeData->getGlobalBoundingBoxCorner()) * 2.0f * sensorToWorldScale; + glm::vec3 otherNodeBoxScale = (avatarClientNodeData->getPosition() - avatarClientNodeData->getGlobalBoundingBoxCorner()) * 2.0f * sensorToWorldScale; // Set up the bounding box for the current other node - AABox otherNodeBox(avatarNodeData->getGlobalBoundingBoxCorner(), otherNodeBoxScale); + AABox otherNodeBox(avatarClientNodeData->getGlobalBoundingBoxCorner(), otherNodeBoxScale); // Clamp the size of the bounding box to a minimum scale if (glm::any(glm::lessThan(otherNodeBoxScale, minBubbleSize))) { otherNodeBox.setScaleStayCentered(minBubbleSize); @@ -405,7 +378,7 @@ void AvatarMixerSlave::broadcastAvatarDataToAgent(const SharedNodePointer& node) if (!shouldIgnore) { AvatarDataSequenceNumber lastSeqToReceiver = nodeData->getLastBroadcastSequenceNumber(avatarNode->getUUID()); - AvatarDataSequenceNumber lastSeqFromSender = avatarNodeData->getLastReceivedSequenceNumber(); + AvatarDataSequenceNumber lastSeqFromSender = avatarClientNodeData->getLastReceivedSequenceNumber(); // FIXME - This code does appear to be working. But it seems brittle. // It supports determining if the frame of data for this "other" @@ -430,7 +403,10 @@ void AvatarMixerSlave::broadcastAvatarDataToAgent(const SharedNodePointer& node) if (!shouldIgnore) { // sort this one for later - sortedAvatars.push(SortableAvatar(avatar._avatarData, avatar._node, avatar._lastEncodeTime)); + const AvatarData* avatarNodeData = avatarClientNodeData->getConstAvatarData(); + auto lastEncodeTime = nodeData->getLastOtherAvatarEncodeTime(avatarNodeData->getSessionUUID()); + + sortedAvatars.push(SortableAvatar(avatarNodeData, avatarNode, lastEncodeTime)); } } From d2650f7ede3cfafa2130d51016b30412d1c10f29 Mon Sep 17 00:00:00 2001 From: Simon Walton Date: Wed, 5 Sep 2018 17:22:07 -0700 Subject: [PATCH 25/44] Modified priority function from Andrew --- .../src/avatars/AvatarMixerSlave.cpp | 6 +- libraries/shared/src/PrioritySortUtil.h | 64 +++---------------- 2 files changed, 11 insertions(+), 59 deletions(-) diff --git a/assignment-client/src/avatars/AvatarMixerSlave.cpp b/assignment-client/src/avatars/AvatarMixerSlave.cpp index a61f65ffb0..9e38f465d3 100644 --- a/assignment-client/src/avatars/AvatarMixerSlave.cpp +++ b/assignment-client/src/avatars/AvatarMixerSlave.cpp @@ -313,9 +313,9 @@ void AvatarMixerSlave::broadcastAvatarDataToAgent(const SharedNodePointer& node) // prepare to sort const auto& cameraViews = nodeData->getViewFrustums(); PrioritySortUtil::PriorityQueue sortedAvatars(cameraViews, - AvatarData::_avatarSortCoefficientSize, - AvatarData::_avatarSortCoefficientCenter, - AvatarData::_avatarSortCoefficientAge); + /*AvatarData::_avatarSortCoefficientSize*/ 1.0f, // Suggested weights from Andrew M. + /*AvatarData::_avatarSortCoefficientCenter*/ 0.5f, + /*AvatarData::_avatarSortCoefficientAge*/ 0.25f); sortedAvatars.reserve(_end - _begin); for (auto listedNode = _begin; listedNode != _end; ++listedNode) { diff --git a/libraries/shared/src/PrioritySortUtil.h b/libraries/shared/src/PrioritySortUtil.h index 0a9bb45d46..ac2e6a6852 100644 --- a/libraries/shared/src/PrioritySortUtil.h +++ b/libraries/shared/src/PrioritySortUtil.h @@ -16,49 +16,7 @@ #include "NumericalConstants.h" #include "shared/ConicalViewFrustum.h" -/* PrioritySortUtil is a helper for sorting 3D things relative to a ViewFrustum. To use: - -(1) Derive a class from pure-virtual PrioritySortUtil::Sortable that wraps a copy of - the Thing you want to prioritize and sort: - - class SortableWrapper: public PrioritySortUtil::Sortable { - public: - SortableWrapper(const Thing& thing) : _thing(thing) { } - glm::vec3 getPosition() const override { return _thing->getPosition(); } - float getRadius() const override { return 0.5f * _thing->getBoundingRadius(); } - uint64_t getTimestamp() const override { return _thing->getLastTime(); } - Thing getThing() const { return _thing; } - private: - Thing _thing; - }; - -(2) Make a PrioritySortUtil::PriorityQueue and add them to the queue: - - PrioritySortUtil::PriorityQueue sortedThings(viewFrustum); - std::priority_queue< PrioritySortUtil::Sortable > sortedThings; - for (thing in things) { - sortedThings.push(SortableWrapper(thing)); - } - -(3) Loop over your priority queue and do timeboxed work: - - NOTE: Be careful using references to members of instances of T from std::priority_queue. - Under the hood std::priority_queue may re-use instances of T. - For example, after a pop() or a push() the top T may have the same memory address - as the top T before the pop() or push() (but point to a swapped instance of T). - This causes a reference to member variable of T to point to a different value - when operations taken on std::priority_queue shuffle around the instances of T. - - uint64_t cutoffTime = usecTimestampNow() + TIME_BUDGET; - while (!sortedThings.empty()) { - const Thing& thing = sortedThings.top(); - // ...do work on thing... - sortedThings.pop(); - if (usecTimestampNow() > cutoffTime) { - break; - } - } -*/ +// PrioritySortUtil is a helper for sorting 3D things relative to a ViewFrustum. namespace PrioritySortUtil { @@ -84,9 +42,9 @@ namespace PrioritySortUtil { PriorityQueue() = delete; PriorityQueue(const ConicalViewFrustums& views) : _views(views) { } PriorityQueue(const ConicalViewFrustums& views, float angularWeight, float centerWeight, float ageWeight) - : _views(views), _angularWeight(angularWeight), _centerWeight(centerWeight), _ageWeight(ageWeight) - , _usecCurrentTime(usecTimestampNow()) - { } + : _views(views), _angularWeight(angularWeight), _centerWeight(centerWeight), _ageWeight(ageWeight) + , _usecCurrentTime(usecTimestampNow()) { + } void setViews(const ConicalViewFrustums& views) { _views = views; } @@ -138,14 +96,9 @@ namespace PrioritySortUtil { float cosineAngle = (glm::dot(offset, view.getDirection()) / distance); float age = (float)(_usecCurrentTime - thing.getTimestamp()); - // we modulate "age" drift rate by the cosineAngle term to make peripheral objects sort forward - // at a reduced rate but we don't want the "age" term to go zero or negative so we clamp it - const float MIN_COSINE_ANGLE_FACTOR = 0.1f; - float cosineAngleFactor = glm::max(cosineAngle, MIN_COSINE_ANGLE_FACTOR); - - float priority = _angularWeight * radius / distance - + _centerWeight * cosineAngle - + _ageWeight * cosineAngleFactor * age; + // the "age" term accumulates at the sum of all weights + float angularSize = glm::max(radius, MIN_RADIUS) / distance; + float priority = (_angularWeight * angularSize + _centerWeight * cosineAngle) * (age + 1.0f) + _ageWeight * age; // decrement priority of things outside keyhole if (distance - radius > view.getRadius()) { @@ -166,9 +119,8 @@ namespace PrioritySortUtil { }; } // namespace PrioritySortUtil -// for now we're keeping hard-coded sorted time budgets in one spot + // 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 #endif // hifi_PrioritySortUtil_h - From adf0a9a4147df64c59d325be68a7834aa1cc7493 Mon Sep 17 00:00:00 2001 From: Simon Walton Date: Wed, 5 Sep 2018 17:41:00 -0700 Subject: [PATCH 26/44] Convert priority age from microseconds to seconds --- libraries/shared/src/PrioritySortUtil.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libraries/shared/src/PrioritySortUtil.h b/libraries/shared/src/PrioritySortUtil.h index ac2e6a6852..075de13d9d 100644 --- a/libraries/shared/src/PrioritySortUtil.h +++ b/libraries/shared/src/PrioritySortUtil.h @@ -22,7 +22,7 @@ namespace PrioritySortUtil { constexpr float DEFAULT_ANGULAR_COEF { 1.0f }; constexpr float DEFAULT_CENTER_COEF { 0.5f }; - constexpr float DEFAULT_AGE_COEF { 0.25f / (float)(USECS_PER_SECOND) }; + constexpr float DEFAULT_AGE_COEF { 0.25f }; class Sortable { public: @@ -94,7 +94,7 @@ namespace PrioritySortUtil { float radius = glm::max(thing.getRadius(), MIN_RADIUS); // Other item's angle from view centre: float cosineAngle = (glm::dot(offset, view.getDirection()) / distance); - float age = (float)(_usecCurrentTime - thing.getTimestamp()); + 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; From e1aba52c239ea552201429bff57b4d396d969307 Mon Sep 17 00:00:00 2001 From: Simon Walton Date: Thu, 6 Sep 2018 12:09:44 -0700 Subject: [PATCH 27/44] Revert to statics for priority sort weights; update defaults from Andrew --- assignment-client/src/avatars/AvatarMixerSlave.cpp | 6 +++--- libraries/avatars/src/AvatarData.cpp | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/assignment-client/src/avatars/AvatarMixerSlave.cpp b/assignment-client/src/avatars/AvatarMixerSlave.cpp index 9e38f465d3..a61f65ffb0 100644 --- a/assignment-client/src/avatars/AvatarMixerSlave.cpp +++ b/assignment-client/src/avatars/AvatarMixerSlave.cpp @@ -313,9 +313,9 @@ void AvatarMixerSlave::broadcastAvatarDataToAgent(const SharedNodePointer& node) // prepare to sort const auto& cameraViews = nodeData->getViewFrustums(); PrioritySortUtil::PriorityQueue sortedAvatars(cameraViews, - /*AvatarData::_avatarSortCoefficientSize*/ 1.0f, // Suggested weights from Andrew M. - /*AvatarData::_avatarSortCoefficientCenter*/ 0.5f, - /*AvatarData::_avatarSortCoefficientAge*/ 0.25f); + AvatarData::_avatarSortCoefficientSize, + AvatarData::_avatarSortCoefficientCenter, + AvatarData::_avatarSortCoefficientAge); sortedAvatars.reserve(_end - _begin); for (auto listedNode = _begin; listedNode != _end; ++listedNode) { diff --git a/libraries/avatars/src/AvatarData.cpp b/libraries/avatars/src/AvatarData.cpp index 3e822fd17a..6896883f74 100644 --- a/libraries/avatars/src/AvatarData.cpp +++ b/libraries/avatars/src/AvatarData.cpp @@ -2822,8 +2822,8 @@ void RayToAvatarIntersectionResultFromScriptValue(const QScriptValue& object, Ra const float AvatarData::OUT_OF_VIEW_PENALTY = -10.0f; -float AvatarData::_avatarSortCoefficientSize { 1.0f }; -float AvatarData::_avatarSortCoefficientCenter { 0.25 }; +float AvatarData::_avatarSortCoefficientSize { 4.0f }; +float AvatarData::_avatarSortCoefficientCenter { 4.0f }; float AvatarData::_avatarSortCoefficientAge { 1.0f }; QScriptValue AvatarEntityMapToScriptValue(QScriptEngine* engine, const AvatarEntityMap& value) { From 021146e4f0d6911d9c9ca57e88719932730c002a Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Thu, 6 Sep 2018 15:33:52 -0700 Subject: [PATCH 28/44] minor tweak to default avatar-update sort coefficients --- libraries/avatars/src/AvatarData.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/avatars/src/AvatarData.cpp b/libraries/avatars/src/AvatarData.cpp index 6896883f74..13adc803dd 100644 --- a/libraries/avatars/src/AvatarData.cpp +++ b/libraries/avatars/src/AvatarData.cpp @@ -2822,7 +2822,7 @@ void RayToAvatarIntersectionResultFromScriptValue(const QScriptValue& object, Ra const float AvatarData::OUT_OF_VIEW_PENALTY = -10.0f; -float AvatarData::_avatarSortCoefficientSize { 4.0f }; +float AvatarData::_avatarSortCoefficientSize { 8.0f }; float AvatarData::_avatarSortCoefficientCenter { 4.0f }; float AvatarData::_avatarSortCoefficientAge { 1.0f }; From 229e9624e62f2afe29b55cccef4fdf2fc4d7eb7c Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Thu, 6 Sep 2018 15:34:28 -0700 Subject: [PATCH 29/44] remove bad bounding box calculations in avatar-mixer --- .../src/avatars/AvatarMixerClientData.h | 1 + .../src/avatars/AvatarMixerSlave.cpp | 43 +++++++------------ 2 files changed, 17 insertions(+), 27 deletions(-) diff --git a/assignment-client/src/avatars/AvatarMixerClientData.h b/assignment-client/src/avatars/AvatarMixerClientData.h index ee27f9bb0f..81a4a58769 100644 --- a/assignment-client/src/avatars/AvatarMixerClientData.h +++ b/assignment-client/src/avatars/AvatarMixerClientData.h @@ -45,6 +45,7 @@ public: int parseData(ReceivedMessage& message) override; AvatarData& getAvatar() { return *_avatar; } + const AvatarData& getAvatar() const { return *_avatar; } const AvatarData* getConstAvatarData() const { return _avatar.get(); } AvatarSharedPointer getAvatarSharedPointer() const { return _avatar; } diff --git a/assignment-client/src/avatars/AvatarMixerSlave.cpp b/assignment-client/src/avatars/AvatarMixerSlave.cpp index a61f65ffb0..3e83a78341 100644 --- a/assignment-client/src/avatars/AvatarMixerSlave.cpp +++ b/assignment-client/src/avatars/AvatarMixerSlave.cpp @@ -212,6 +212,15 @@ void AvatarMixerSlave::broadcastAvatarData(const SharedNodePointer& node) { _stats.jobElapsedTime += (end - start); } +AABox computeBubbleBox(const AvatarData& avatar, float bubbleExpansionFactor) { + glm::vec3 position = avatar.getClientGlobalPosition(); + glm::vec3 scale = 2.0f * (avatar.getClientGlobalPosition()- avatar.getGlobalBoundingBoxCorner()); + scale *= bubbleExpansionFactor; + const glm::vec3 MIN_BUBBLE_SCALE(0.3f, 1.3f, 0.3); + scale = glm::max(scale, MIN_BUBBLE_SCALE); + return AABox(position - 0.5f * scale, scale); +} + void AvatarMixerSlave::broadcastAvatarDataToAgent(const SharedNodePointer& node) { const Node* destinationNode = node.data(); @@ -275,18 +284,9 @@ void AvatarMixerSlave::broadcastAvatarDataToAgent(const SharedNodePointer& node) // setup a PacketList for the avatarPackets auto avatarPacketList = NLPacketList::create(PacketType::BulkAvatarData); - // Define the minimum bubble size - const glm::vec3 minBubbleSize = avatar.getSensorToWorldScale() * glm::vec3(0.3f, 1.3f, 0.3f); - // Define the scale of the box for the current node - glm::vec3 nodeBoxScale = (avatar.getClientGlobalPosition() - avatar.getGlobalBoundingBoxCorner()) * 2.0f * avatar.getSensorToWorldScale(); - // Set up the bounding box for the current node - AABox nodeBox(avatar.getGlobalBoundingBoxCorner(), nodeBoxScale); - // Clamp the size of the bounding box to a minimum scale - if (glm::any(glm::lessThan(nodeBoxScale, minBubbleSize))) { - nodeBox.setScaleStayCentered(minBubbleSize); - } - // Quadruple the scale of first bounding box - nodeBox.embiggen(4.0f); + // 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 ); class SortableAvatar: public PrioritySortUtil::Sortable { public: @@ -295,7 +295,7 @@ 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 = (_avatar->getClientGlobalPosition() - _avatar->getGlobalBoundingBoxCorner() * _avatar->getSensorToWorldScale()); + glm::vec3 nodeBoxHalfScale = _avatar->getClientGlobalPosition() - _avatar->getGlobalBoundingBoxCorner(); return glm::max(nodeBoxHalfScale.x, glm::max(nodeBoxHalfScale.y, nodeBoxHalfScale.z)); } uint64_t getTimestamp() const override { @@ -351,20 +351,9 @@ void AvatarMixerSlave::broadcastAvatarDataToAgent(const SharedNodePointer& node) // Check to see if the space bubble is enabled // Don't bother with these checks if the other avatar has their bubble enabled and we're gettingAnyIgnored if (destinationNode->isIgnoreRadiusEnabled() || (avatarNode->isIgnoreRadiusEnabled() && !getsAnyIgnored)) { - float sensorToWorldScale = avatarClientNodeData->getAvatarSharedPointer()->getSensorToWorldScale(); - // Define the scale of the box for the current other node - glm::vec3 otherNodeBoxScale = (avatarClientNodeData->getPosition() - avatarClientNodeData->getGlobalBoundingBoxCorner()) * 2.0f * sensorToWorldScale; - // Set up the bounding box for the current other node - AABox otherNodeBox(avatarClientNodeData->getGlobalBoundingBoxCorner(), otherNodeBoxScale); - // Clamp the size of the bounding box to a minimum scale - if (glm::any(glm::lessThan(otherNodeBoxScale, minBubbleSize))) { - otherNodeBox.setScaleStayCentered(minBubbleSize); - } - // Change the scale of other bounding box - // (This is an arbitrary number determined empirically) - otherNodeBox.embiggen(2.4f); - // Perform the collision check between the two bounding boxes + const float OTHER_AVATAR_BUBBLE_EXPANSION_FACTOR = 2.4f; // magic number determined empirically + AABox otherNodeBox = computeBubbleBox(avatarClientNodeData->getAvatar(), OTHER_AVATAR_BUBBLE_EXPANSION_FACTOR); if (nodeBox.touches(otherNodeBox)) { nodeData->ignoreOther(destinationNode, avatarNode); shouldIgnore = !getsAnyIgnored; @@ -445,7 +434,7 @@ void AvatarMixerSlave::broadcastAvatarDataToAgent(const SharedNodePointer& node) // determine if avatar is in view which determines how much data to send glm::vec3 otherPosition = otherAvatar->getClientGlobalPosition(); - glm::vec3 otherNodeBoxScale = (otherPosition - otherNodeData->getGlobalBoundingBoxCorner()) * 2.0f * otherAvatar->getSensorToWorldScale(); + glm::vec3 otherNodeBoxScale = otherPosition - otherNodeData->getGlobalBoundingBoxCorner(); AABox otherNodeBox(otherNodeData->getGlobalBoundingBoxCorner(), otherNodeBoxScale); bool isInView = nodeData->otherAvatarInView(otherNodeBox); From 9e0c21065e0de0d3b75e0f213959d27ca61f6013 Mon Sep 17 00:00:00 2001 From: Simon Walton Date: Thu, 6 Sep 2018 15:47:23 -0700 Subject: [PATCH 30/44] Stop sending avatars when over bandwidth quota --- assignment-client/src/avatars/AvatarMixerSlave.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/assignment-client/src/avatars/AvatarMixerSlave.cpp b/assignment-client/src/avatars/AvatarMixerSlave.cpp index a61f65ffb0..5dd250fba7 100644 --- a/assignment-client/src/avatars/AvatarMixerSlave.cpp +++ b/assignment-client/src/avatars/AvatarMixerSlave.cpp @@ -425,6 +425,11 @@ void AvatarMixerSlave::broadcastAvatarDataToAgent(const SharedNodePointer& node) // 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; + overBudgetAvatars += remainingAvatars + 1; + break; + } auto startAvatarDataPacking = chrono::high_resolution_clock::now(); From f59168e1c8b61a143cd0b0de8b672b36d5b00493 Mon Sep 17 00:00:00 2001 From: Simon Walton Date: Thu, 6 Sep 2018 16:31:31 -0700 Subject: [PATCH 31/44] make sure ignoredNode is available before sending packet --- assignment-client/src/avatars/AvatarMixer.cpp | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/assignment-client/src/avatars/AvatarMixer.cpp b/assignment-client/src/avatars/AvatarMixer.cpp index edbba20dc7..561afee296 100644 --- a/assignment-client/src/avatars/AvatarMixer.cpp +++ b/assignment-client/src/avatars/AvatarMixer.cpp @@ -654,6 +654,15 @@ void AvatarMixer::handleNodeIgnoreRequestPacket(QSharedPointer if (addToIgnore) { senderNode->addIgnoredNode(ignoredUUID); + + if (ignoredNode) { + // send a reliable kill packet to remove the sending avatar for the ignored avatar + auto killPacket = NLPacket::create(PacketType::KillAvatar, + NUM_BYTES_RFC4122_UUID + sizeof(KillAvatarReason), true); + killPacket->write(senderNode->getUUID().toRfc4122()); + killPacket->writePrimitive(KillAvatarReason::AvatarDisconnected); + nodeList->sendPacket(std::move(killPacket), *ignoredNode); + } } else { senderNode->removeIgnoredNode(ignoredUUID); } From 92210f28b51403b9ff2a0783c3d9222a46638dd1 Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Thu, 6 Sep 2018 16:33:30 -0700 Subject: [PATCH 32/44] cleanup and use correct avatar bounding box --- .../src/avatars/AvatarMixerClientData.h | 1 - assignment-client/src/avatars/AvatarMixerSlave.cpp | 14 ++++++-------- libraries/avatars/src/AvatarData.h | 2 +- 3 files changed, 7 insertions(+), 10 deletions(-) diff --git a/assignment-client/src/avatars/AvatarMixerClientData.h b/assignment-client/src/avatars/AvatarMixerClientData.h index 81a4a58769..64b1ea3edf 100644 --- a/assignment-client/src/avatars/AvatarMixerClientData.h +++ b/assignment-client/src/avatars/AvatarMixerClientData.h @@ -91,7 +91,6 @@ public: void loadJSONStats(QJsonObject& jsonObject) const; glm::vec3 getPosition() const { return _avatar ? _avatar->getWorldPosition() : glm::vec3(0); } - glm::vec3 getGlobalBoundingBoxCorner() const { return _avatar ? _avatar->getGlobalBoundingBoxCorner() : glm::vec3(0); } bool isRadiusIgnoring(const QUuid& other) const { return _radiusIgnoredOthers.find(other) != _radiusIgnoredOthers.end(); } void addToRadiusIgnoringSet(const QUuid& other) { _radiusIgnoredOthers.insert(other); } void removeFromRadiusIgnoringSet(const QUuid& other); diff --git a/assignment-client/src/avatars/AvatarMixerSlave.cpp b/assignment-client/src/avatars/AvatarMixerSlave.cpp index 3e83a78341..411f77459f 100644 --- a/assignment-client/src/avatars/AvatarMixerSlave.cpp +++ b/assignment-client/src/avatars/AvatarMixerSlave.cpp @@ -213,12 +213,13 @@ void AvatarMixerSlave::broadcastAvatarData(const SharedNodePointer& node) { } AABox computeBubbleBox(const AvatarData& avatar, float bubbleExpansionFactor) { - glm::vec3 position = avatar.getClientGlobalPosition(); - glm::vec3 scale = 2.0f * (avatar.getClientGlobalPosition()- avatar.getGlobalBoundingBoxCorner()); + AABox box = avatar.getGlobalBoundingBox(); + glm::vec3 scale = box.getScale(); scale *= bubbleExpansionFactor; const glm::vec3 MIN_BUBBLE_SCALE(0.3f, 1.3f, 0.3); scale = glm::max(scale, MIN_BUBBLE_SCALE); - return AABox(position - 0.5f * scale, scale); + box.setScaleStayCentered(glm::max(scale, MIN_BUBBLE_SCALE)); + return box; } void AvatarMixerSlave::broadcastAvatarDataToAgent(const SharedNodePointer& node) { @@ -295,7 +296,7 @@ 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 = _avatar->getClientGlobalPosition() - _avatar->getGlobalBoundingBoxCorner(); + glm::vec3 nodeBoxHalfScale = 0.5f * _avatar->getGlobalBoundingBox().getScale(); return glm::max(nodeBoxHalfScale.x, glm::max(nodeBoxHalfScale.y, nodeBoxHalfScale.z)); } uint64_t getTimestamp() const override { @@ -433,10 +434,7 @@ void AvatarMixerSlave::broadcastAvatarDataToAgent(const SharedNodePointer& node) } // determine if avatar is in view which determines how much data to send - glm::vec3 otherPosition = otherAvatar->getClientGlobalPosition(); - glm::vec3 otherNodeBoxScale = otherPosition - otherNodeData->getGlobalBoundingBoxCorner(); - AABox otherNodeBox(otherNodeData->getGlobalBoundingBoxCorner(), otherNodeBoxScale); - bool isInView = nodeData->otherAvatarInView(otherNodeBox); + bool isInView = nodeData->otherAvatarInView(otherAvatar->getGlobalBoundingBox()); // start a new segment in the PacketList for this avatar avatarPacketList->startSegment(); diff --git a/libraries/avatars/src/AvatarData.h b/libraries/avatars/src/AvatarData.h index 0474d07acd..9146340c4d 100644 --- a/libraries/avatars/src/AvatarData.h +++ b/libraries/avatars/src/AvatarData.h @@ -1095,7 +1095,7 @@ public: void fromJson(const QJsonObject& json, bool useFrameSkeleton = true); glm::vec3 getClientGlobalPosition() const { return _globalPosition; } - glm::vec3 getGlobalBoundingBoxCorner() const { return _globalPosition + _globalBoundingBoxOffset - _globalBoundingBoxDimensions; } + AABox getGlobalBoundingBox() const { return AABox(_globalPosition + _globalBoundingBoxOffset - _globalBoundingBoxDimensions, _globalBoundingBoxDimensions); } /**jsdoc * @function MyAvatar.getAvatarEntityData From ed7f993c0dc257132c860d415247215d6349d198 Mon Sep 17 00:00:00 2001 From: SamGondelman Date: Thu, 6 Sep 2018 16:55:27 -0700 Subject: [PATCH 33/44] avatar mixer and manager perf improvements and cleanup --- .../src/avatars/AvatarMixerSlave.cpp | 45 +++++++++---------- interface/src/avatar/AvatarManager.cpp | 40 ++++++++--------- libraries/avatars/src/AvatarData.cpp | 17 +++---- libraries/avatars/src/AvatarData.h | 2 - libraries/avatars/src/AvatarHashMap.cpp | 1 - libraries/shared/src/PrioritySortUtil.h | 9 ++-- 6 files changed, 50 insertions(+), 64 deletions(-) diff --git a/assignment-client/src/avatars/AvatarMixerSlave.cpp b/assignment-client/src/avatars/AvatarMixerSlave.cpp index 8afd7d73ee..705d660a2e 100644 --- a/assignment-client/src/avatars/AvatarMixerSlave.cpp +++ b/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(); - return glm::max(nodeBoxHalfScale.x, glm::max(nodeBoxHalfScale.y, nodeBoxHalfScale.z)); + glm::vec3 nodeBoxScale = _avatar->getGlobalBoundingBox().getScale(); + 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; - overBudgetAvatars += remainingAvatars + 1; - break; + if (PALIsOpen) { + _stats.overBudgetAvatars++; + 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 - avatarPacketList->startSegment(); - - AvatarData::AvatarDataDetail detail; - - if (overBudget) { - overBudgetAvatars++; - _stats.overBudgetAvatars++; - detail = PALIsOpen ? AvatarData::PALMinimum : AvatarData::NoData; - } else if (!isInView) { + if (!isInView) { detail = PALIsOpen ? AvatarData::PALMinimum : AvatarData::MinimumData; nodeData->incrementAvatarOutOfView(); - } else { - detail = distribution(generator) < AVATAR_SEND_FULL_UPDATE_RATIO - ? AvatarData::SendAllData : AvatarData::CullSmallData; + } else if (!overBudget) { + detail = distribution(generator) < AVATAR_SEND_FULL_UPDATE_RATIO ? 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(endAvatarDataPacking - startAvatarDataPacking).count(); // use helper to add any changed traits to our packet list traitBytesSent += addChangedTraitsToBulkPacket(nodeData, otherNodeData, *traitsPacketList); + remainingAvatars--; } quint64 startPacketSending = usecTimestampNow(); diff --git a/interface/src/avatar/AvatarManager.cpp b/interface/src/avatar/AvatarManager.cpp index e9486b9def..1bc22ea98c 100644 --- a/interface/src/avatar/AvatarManager.cpp +++ b/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); - if (_avatarHash.size() < 2 && _avatarsToFade.isEmpty()) { - return; + { + // lock the hash for read to check the size + QReadLocker lock(&_hashLock); + 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) {} glm::vec3 getPosition() const override { return _avatar->getWorldPosition(); } - float getRadius() const override { return std::static_pointer_cast(_avatar)->getBoundingRadius(); } - uint64_t getTimestamp() const override { return std::static_pointer_cast(_avatar)->getLastRenderUpdateTime(); } - AvatarSharedPointer getAvatar() const { return _avatar; } + float getRadius() const override { return _avatar->getBoundingRadius(); } + uint64_t getTimestamp() const override { return _avatar->getLastRenderUpdateTime(); } + std::shared_ptr getAvatar() const { return _avatar; } private: - AvatarSharedPointer _avatar; + std::shared_ptr _avatar; }; auto avatarMap = getHashCopy(); - AvatarHash::iterator itr = avatarMap.begin(); const auto& views = qApp->getConicalViews(); PrioritySortUtil::PriorityQueue 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(); + AvatarHash::iterator itr = avatarMap.begin(); while (itr != avatarMap.end()) { const auto& avatar = std::static_pointer_cast(*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 + UPDATE_BUDGET; + uint64_t updateExpiry = startTime + MAX_UPDATE_AVATARS_TIME_BUDGET; int numAvatarsUpdated = 0; int numAVatarsNotUpdated = 0; @@ -231,18 +233,12 @@ void AvatarManager::updateOtherAvatars(float deltaTime) { avatar->updateOrbPosition(); } - bool ignoring = DependencyManager::get()->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(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) { diff --git a/libraries/avatars/src/AvatarData.cpp b/libraries/avatars/src/AvatarData.cpp index 13adc803dd..038d08145a 100644 --- a/libraries/avatars/src/AvatarData.cpp +++ b/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 }; diff --git a/libraries/avatars/src/AvatarData.h b/libraries/avatars/src/AvatarData.h index 9146340c4d..cd6440ae5d 100644 --- a/libraries/avatars/src/AvatarData.h +++ b/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; diff --git a/libraries/avatars/src/AvatarHashMap.cpp b/libraries/avatars/src/AvatarHashMap.cpp index c437b56f32..be2ce3d397 100644 --- a/libraries/avatars/src/AvatarHashMap.cpp +++ b/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; diff --git a/libraries/shared/src/PrioritySortUtil.h b/libraries/shared/src/PrioritySortUtil.h index 075de13d9d..27f6b193ba 100644 --- a/libraries/shared/src/PrioritySortUtil.h +++ b/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 From 22cfad248310788459d594c874d7c5fee1bce902 Mon Sep 17 00:00:00 2001 From: Simon Walton Date: Thu, 6 Sep 2018 18:32:37 -0700 Subject: [PATCH 34/44] Squelch (false) warning from gcc --- assignment-client/src/avatars/AvatarMixerSlave.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/assignment-client/src/avatars/AvatarMixerSlave.cpp b/assignment-client/src/avatars/AvatarMixerSlave.cpp index 705d660a2e..784205a2da 100644 --- a/assignment-client/src/avatars/AvatarMixerSlave.cpp +++ b/assignment-client/src/avatars/AvatarMixerSlave.cpp @@ -409,7 +409,7 @@ 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; + AvatarData::AvatarDataDetail detail = AvatarData::NoData; // NOTE: Here's where we determine if we are over budget and drop to bare minimum data int minimRemainingAvatarBytes = minimumBytesPerAvatar * remainingAvatars; From 3dc993c841566027c7409ca29b98cb35a7e08701 Mon Sep 17 00:00:00 2001 From: Simon Walton Date: Fri, 7 Sep 2018 09:46:21 -0700 Subject: [PATCH 35/44] Fix small error in translation delta --- libraries/avatars/src/AvatarData.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/avatars/src/AvatarData.cpp b/libraries/avatars/src/AvatarData.cpp index 905569cb65..04efcb242e 100644 --- a/libraries/avatars/src/AvatarData.cpp +++ b/libraries/avatars/src/AvatarData.cpp @@ -625,7 +625,7 @@ QByteArray AvatarData::toByteArray(AvatarDataDetail dataDetail, quint64 lastSent destinationBuffer += numValidityBytes; // Move pointer past the validity bytes - float minTranslation = (distanceAdjust && cullSmallChanges) ? getDistanceBasedMinTranslationDistance(viewerPosition) : AVATAR_MIN_ROTATION_DOT; + float minTranslation = (distanceAdjust && cullSmallChanges) ? getDistanceBasedMinTranslationDistance(viewerPosition) : AVATAR_MIN_TRANSLATION; float maxTranslationDimension = 0.0; for (int i = 0; i < jointData.size(); i++) { From 1a67176819d5171dba901442d07b7a76b4c2c198 Mon Sep 17 00:00:00 2001 From: Simon Walton Date: Fri, 7 Sep 2018 11:29:30 -0700 Subject: [PATCH 36/44] Fix a dangling merge conflict --- libraries/render-utils/src/SoftAttachmentModel.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/libraries/render-utils/src/SoftAttachmentModel.cpp b/libraries/render-utils/src/SoftAttachmentModel.cpp index 9ab30c179a..93ce8f595a 100644 --- a/libraries/render-utils/src/SoftAttachmentModel.cpp +++ b/libraries/render-utils/src/SoftAttachmentModel.cpp @@ -79,7 +79,6 @@ void SoftAttachmentModel::updateClusterMatrices(bool triggerBlendshapes) { // post the blender if we're not currently waiting for one to finish auto modelBlender = DependencyManager::get(); if (triggerBlendshapes && modelBlender->shouldComputeBlendshapes() && geometry.hasBlendedMeshes() && _blendshapeCoefficients != _blendedBlendshapeCoefficients) { - if (_blendedVertexBuffersInitialized && modelBlender->shouldComputeBlendshapes() && geometry.hasBlendedMeshes() && _blendshapeCoefficients != _blendedBlendshapeCoefficients) { _blendedBlendshapeCoefficients = _blendshapeCoefficients; modelBlender->noteRequiresBlend(getThisPointer()); } From c2fe2b60b3ee8e04b2b122f903de1b05591185f0 Mon Sep 17 00:00:00 2001 From: Simon Walton Date: Fri, 7 Sep 2018 15:49:47 -0700 Subject: [PATCH 37/44] Use std::vector for ignored avatars since the list will be small --- .../src/avatars/AvatarMixerClientData.cpp | 16 +++++++++++++++- .../src/avatars/AvatarMixerClientData.h | 8 ++++---- 2 files changed, 19 insertions(+), 5 deletions(-) diff --git a/assignment-client/src/avatars/AvatarMixerClientData.cpp b/assignment-client/src/avatars/AvatarMixerClientData.cpp index c54db90f5d..6c01e6e02b 100644 --- a/assignment-client/src/avatars/AvatarMixerClientData.cpp +++ b/assignment-client/src/avatars/AvatarMixerClientData.cpp @@ -11,6 +11,7 @@ #include "AvatarMixerClientData.h" +#include #include #include @@ -239,8 +240,21 @@ void AvatarMixerClientData::ignoreOther(const Node* self, const Node* other) { } } +bool AvatarMixerClientData::isRadiusIgnoring(const QUuid& other) const { + return std::find(_radiusIgnoredOthers.cbegin(), _radiusIgnoredOthers.cend(), other) != _radiusIgnoredOthers.cend(); +} + +void AvatarMixerClientData::addToRadiusIgnoringSet(const QUuid& other) { + if (!isRadiusIgnoring(other)) { + _radiusIgnoredOthers.push_back(other); + } +} + void AvatarMixerClientData::removeFromRadiusIgnoringSet(const QUuid& other) { - _radiusIgnoredOthers.erase(other); + auto ignoredOtherIter = std::find(_radiusIgnoredOthers.cbegin(), _radiusIgnoredOthers.cend(), other); + if (ignoredOtherIter != _radiusIgnoredOthers.cend()) { + _radiusIgnoredOthers.erase(ignoredOtherIter); + } } void AvatarMixerClientData::resetSentTraitData(Node::LocalID nodeLocalID) { diff --git a/assignment-client/src/avatars/AvatarMixerClientData.h b/assignment-client/src/avatars/AvatarMixerClientData.h index 64b1ea3edf..d38a90ef1f 100644 --- a/assignment-client/src/avatars/AvatarMixerClientData.h +++ b/assignment-client/src/avatars/AvatarMixerClientData.h @@ -15,7 +15,7 @@ #include #include #include -#include +#include #include #include @@ -91,8 +91,8 @@ public: void loadJSONStats(QJsonObject& jsonObject) const; glm::vec3 getPosition() const { return _avatar ? _avatar->getWorldPosition() : glm::vec3(0); } - bool isRadiusIgnoring(const QUuid& other) const { return _radiusIgnoredOthers.find(other) != _radiusIgnoredOthers.end(); } - void addToRadiusIgnoringSet(const QUuid& other) { _radiusIgnoredOthers.insert(other); } + bool isRadiusIgnoring(const QUuid& other) const; + void addToRadiusIgnoringSet(const QUuid& other); void removeFromRadiusIgnoringSet(const QUuid& other); void ignoreOther(SharedNodePointer self, SharedNodePointer other); void ignoreOther(const Node* self, const Node* other); @@ -167,7 +167,7 @@ private: int _numOutOfOrderSends = 0; SimpleMovingAverage _avgOtherAvatarDataRate; - std::unordered_set _radiusIgnoredOthers; + std::vector _radiusIgnoredOthers; ConicalViewFrustums _currentViewFrustums; int _recentOtherAvatarsInView { 0 }; From 6bb143da4273157bd6cfaa49f8e656829da72346 Mon Sep 17 00:00:00 2001 From: Simon Walton Date: Fri, 7 Sep 2018 18:14:39 -0700 Subject: [PATCH 38/44] Make maxAvatarBytesPerFrame int rather than float; other clean-up --- assignment-client/src/avatars/AvatarMixerSlave.cpp | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/assignment-client/src/avatars/AvatarMixerSlave.cpp b/assignment-client/src/avatars/AvatarMixerSlave.cpp index 784205a2da..8b69479a4e 100644 --- a/assignment-client/src/avatars/AvatarMixerSlave.cpp +++ b/assignment-client/src/avatars/AvatarMixerSlave.cpp @@ -256,7 +256,7 @@ void AvatarMixerSlave::broadcastAvatarDataToAgent(const SharedNodePointer& node) int traitBytesSent = 0; // max number of avatarBytes per frame - auto maxAvatarBytesPerFrame = (_maxKbpsPerNode * BYTES_PER_KILOBIT) / AVATAR_MIXER_BROADCAST_FRAMES_PER_SECOND; + int maxAvatarBytesPerFrame = int(_maxKbpsPerNode * BYTES_PER_KILOBIT / AVATAR_MIXER_BROADCAST_FRAMES_PER_SECOND); // FIXME - find a way to not send the sessionID for every avatar int minimumBytesPerAvatar = AvatarDataPacket::AVATAR_HAS_FLAGS_SIZE + NUM_BYTES_RFC4122_UUID; @@ -412,6 +412,8 @@ void AvatarMixerSlave::broadcastAvatarDataToAgent(const SharedNodePointer& node) AvatarData::AvatarDataDetail detail = AvatarData::NoData; // NOTE: Here's where we determine if we are over budget and drop to bare minimum data + // If we're going to just drop the over-budget avatars entirely the decision should + // just use numAvatarDataBytes, not minimRemainingAvatarBytes. int minimRemainingAvatarBytes = minimumBytesPerAvatar * remainingAvatars; bool overBudget = (identityBytesSent + numAvatarDataBytes + minimRemainingAvatarBytes) > maxAvatarBytesPerFrame; if (overBudget) { @@ -441,10 +443,10 @@ void AvatarMixerSlave::broadcastAvatarDataToAgent(const SharedNodePointer& node) nodeData->setLastBroadcastTime(otherNode->getUUID(), usecTimestampNow()); } - // determine if avatar is in view which determines how much data to send - bool isInView = sortedAvatar.getPriority() > OUT_OF_VIEW_THRESHOLD; + // Typically all out-of-view avatars but such avatars' priorities will rise with time: + bool isLowerPriority = sortedAvatar.getPriority() <= OUT_OF_VIEW_THRESHOLD; - if (!isInView) { + if (isLowerPriority) { detail = PALIsOpen ? AvatarData::PALMinimum : AvatarData::MinimumData; nodeData->incrementAvatarOutOfView(); } else if (!overBudget) { From 9628615c441c8987dddb50c91dccc01b1ba7ff40 Mon Sep 17 00:00:00 2001 From: Simon Walton Date: Mon, 10 Sep 2018 17:16:16 -0700 Subject: [PATCH 39/44] Fix bandwidth quota calculation now we're dropping excess --- .../src/avatars/AvatarMixerSlave.cpp | 19 +++++++------------ 1 file changed, 7 insertions(+), 12 deletions(-) diff --git a/assignment-client/src/avatars/AvatarMixerSlave.cpp b/assignment-client/src/avatars/AvatarMixerSlave.cpp index 8b69479a4e..7368db0c31 100644 --- a/assignment-client/src/avatars/AvatarMixerSlave.cpp +++ b/assignment-client/src/avatars/AvatarMixerSlave.cpp @@ -258,8 +258,6 @@ void AvatarMixerSlave::broadcastAvatarDataToAgent(const SharedNodePointer& node) // max number of avatarBytes per frame int maxAvatarBytesPerFrame = int(_maxKbpsPerNode * BYTES_PER_KILOBIT / AVATAR_MIXER_BROADCAST_FRAMES_PER_SECOND); - // FIXME - find a way to not send the sessionID for every avatar - int minimumBytesPerAvatar = AvatarDataPacket::AVATAR_HAS_FLAGS_SIZE + NUM_BYTES_RFC4122_UUID; // keep track of the number of other avatars held back in this frame int numAvatarsHeldBack = 0; @@ -274,14 +272,13 @@ void AvatarMixerSlave::broadcastAvatarDataToAgent(const SharedNodePointer& node) // When this is true, the AvatarMixer will send Avatar data to a client about avatars that have ignored them bool getsAnyIgnored = PALIsOpen && destinationNode->getCanKick(); - if (PALIsOpen) { - // Increase minimumBytesPerAvatar if the PAL is open - minimumBytesPerAvatar += sizeof(AvatarDataPacket::AvatarGlobalPosition) + - sizeof(AvatarDataPacket::AudioLoudness); - } + // Bandwidth allowance for data that must be sent. + int minimumBytesPerAvatar = PALIsOpen ? AvatarDataPacket::AVATAR_HAS_FLAGS_SIZE + NUM_BYTES_RFC4122_UUID + + sizeof(AvatarDataPacket::AvatarGlobalPosition) + sizeof(AvatarDataPacket::AudioLoudness) : 0; // setup a PacketList for the avatarPackets auto avatarPacketList = NLPacketList::create(PacketType::BulkAvatarData); + static auto maxAvatarDataBytes = avatarPacketList->getMaxSegmentSize() - NUM_BYTES_RFC4122_UUID; // compute node bounding box const float MY_AVATAR_BUBBLE_EXPANSION_FACTOR = 4.0f; // magic number determined emperically @@ -401,19 +398,18 @@ void AvatarMixerSlave::broadcastAvatarDataToAgent(const SharedNodePointer& node) int remainingAvatars = (int)sortedAvatars.size(); auto traitsPacketList = NLPacketList::create(PacketType::BulkAvatarTraits, QByteArray(), true, true); + const auto& sortedAvatarVector = sortedAvatars.getSortedVector(); for (const auto& sortedAvatar : sortedAvatarVector) { const Node* otherNode = sortedAvatar.getNode(); auto lastEncodeForOther = sortedAvatar.getTimestamp(); - remainingAvatars--; assert(otherNode); // we can't have gotten here without the avatarData being a valid key in the map AvatarData::AvatarDataDetail detail = AvatarData::NoData; - // NOTE: Here's where we determine if we are over budget and drop to bare minimum data - // If we're going to just drop the over-budget avatars entirely the decision should - // just use numAvatarDataBytes, not minimRemainingAvatarBytes. + // NOTE: Here's where we determine if we are over budget and drop remaining avatars, + // or send minimal avatar data in uncommon case of PALIsOpen. int minimRemainingAvatarBytes = minimumBytesPerAvatar * remainingAvatars; bool overBudget = (identityBytesSent + numAvatarDataBytes + minimRemainingAvatarBytes) > maxAvatarBytesPerFrame; if (overBudget) { @@ -472,7 +468,6 @@ void AvatarMixerSlave::broadcastAvatarDataToAgent(const SharedNodePointer& node) _stats.toByteArrayElapsedTime += (quint64) chrono::duration_cast(endSerialize - startSerialize).count(); - static auto maxAvatarDataBytes = avatarPacketList->getMaxSegmentSize() - NUM_BYTES_RFC4122_UUID; if (bytes.size() > maxAvatarDataBytes) { qCWarning(avatars) << "otherAvatar.toByteArray() for" << otherNode->getUUID() << "resulted in very large buffer of" << bytes.size() << "bytes - dropping facial data"; From 80b7e04bc678838532f8a6d95e7379ef0f9e1af0 Mon Sep 17 00:00:00 2001 From: Simon Walton Date: Tue, 11 Sep 2018 10:51:36 -0700 Subject: [PATCH 40/44] Revert change to chrono for usecTimestampNow() --- libraries/shared/src/SharedUtil.cpp | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/libraries/shared/src/SharedUtil.cpp b/libraries/shared/src/SharedUtil.cpp index 6c79a23c20..e52e815520 100644 --- a/libraries/shared/src/SharedUtil.cpp +++ b/libraries/shared/src/SharedUtil.cpp @@ -128,15 +128,11 @@ void usecTimestampNowForceClockSkew(qint64 clockSkew) { ::usecTimestampNowAdjust = clockSkew; } -//static std::atomic TIME_REFERENCE { 0 }; // in usec -//static std::once_flag usecTimestampNowIsInitialized; -//static QElapsedTimer timestampTimer; +static std::atomic TIME_REFERENCE { 0 }; // in usec +static std::once_flag usecTimestampNowIsInitialized; +static QElapsedTimer timestampTimer; quint64 usecTimestampNow(bool wantDebug) { - using namespace std::chrono; - static const auto unixEpoch = system_clock::from_time_t(0); - return duration_cast(system_clock::now() - unixEpoch).count(); -#if 0 std::call_once(usecTimestampNowIsInitialized, [&] { TIME_REFERENCE = QDateTime::currentMSecsSinceEpoch() * USECS_PER_MSEC; // ms to usec timestampTimer.start(); @@ -208,7 +204,6 @@ quint64 usecTimestampNow(bool wantDebug) { } return now; -#endif } float secTimestampNow() { From 2a0ee1c5326fbfceeada45e8d610b20ab31f603d Mon Sep 17 00:00:00 2001 From: Simon Walton Date: Tue, 11 Sep 2018 16:58:21 -0700 Subject: [PATCH 41/44] Cherry-pick of commit cde2dc2e to fix merge issue --- libraries/render-utils/src/CauterizedModel.cpp | 2 +- libraries/render-utils/src/CauterizedModel.h | 2 +- libraries/render-utils/src/Model.cpp | 4 ++-- libraries/render-utils/src/Model.h | 2 +- libraries/render-utils/src/SoftAttachmentModel.cpp | 4 ++-- libraries/render-utils/src/SoftAttachmentModel.h | 2 +- 6 files changed, 8 insertions(+), 8 deletions(-) diff --git a/libraries/render-utils/src/CauterizedModel.cpp b/libraries/render-utils/src/CauterizedModel.cpp index fe4574a696..c4631c3676 100644 --- a/libraries/render-utils/src/CauterizedModel.cpp +++ b/libraries/render-utils/src/CauterizedModel.cpp @@ -102,7 +102,7 @@ void CauterizedModel::createRenderItemSet() { } } -void CauterizedModel::updateClusterMatrices(bool triggerBlendshapes) { +void CauterizedModel::updateClusterMatrices() { PerformanceTimer perfTimer("CauterizedModel::updateClusterMatrices"); if (!_needsUpdateClusterMatrices || !isLoaded()) { diff --git a/libraries/render-utils/src/CauterizedModel.h b/libraries/render-utils/src/CauterizedModel.h index 12cf921e5b..36a96fb006 100644 --- a/libraries/render-utils/src/CauterizedModel.h +++ b/libraries/render-utils/src/CauterizedModel.h @@ -33,7 +33,7 @@ public: void createRenderItemSet() override; - virtual void updateClusterMatrices(bool triggerBlendshapes = true) override; + virtual void updateClusterMatrices() override; void updateRenderItems() override; const Model::MeshState& getCauterizeMeshState(int index) const; diff --git a/libraries/render-utils/src/Model.cpp b/libraries/render-utils/src/Model.cpp index 2d986168ba..ba2bd28852 100644 --- a/libraries/render-utils/src/Model.cpp +++ b/libraries/render-utils/src/Model.cpp @@ -976,7 +976,7 @@ bool Model::addToScene(const render::ScenePointer& scene, render::Transaction& transaction, render::Item::Status::Getters& statusGetters) { if (!_addedToScene && isLoaded()) { - updateClusterMatrices(false); + updateClusterMatrices(); if (_modelMeshRenderItems.empty()) { createRenderItemSet(); } @@ -1487,7 +1487,7 @@ void Model::computeMeshPartLocalBounds() { } // virtual -void Model::updateClusterMatrices(bool triggerBlendshapes) { +void Model::updateClusterMatrices() { DETAILED_PERFORMANCE_TIMER("Model::updateClusterMatrices"); if (!_needsUpdateClusterMatrices || !isLoaded()) { diff --git a/libraries/render-utils/src/Model.h b/libraries/render-utils/src/Model.h index 2244c94267..e7534f5b89 100644 --- a/libraries/render-utils/src/Model.h +++ b/libraries/render-utils/src/Model.h @@ -159,7 +159,7 @@ public: bool getSnapModelToRegistrationPoint() { return _snapModelToRegistrationPoint; } virtual void simulate(float deltaTime, bool fullUpdate = true); - virtual void updateClusterMatrices(bool triggerBlendshapes = true); + virtual void updateClusterMatrices(); /// Returns a reference to the shared geometry. const Geometry::Pointer& getGeometry() const { return _renderGeometry; } diff --git a/libraries/render-utils/src/SoftAttachmentModel.cpp b/libraries/render-utils/src/SoftAttachmentModel.cpp index 93ce8f595a..b9a6581f1d 100644 --- a/libraries/render-utils/src/SoftAttachmentModel.cpp +++ b/libraries/render-utils/src/SoftAttachmentModel.cpp @@ -31,7 +31,7 @@ int SoftAttachmentModel::getJointIndexOverride(int i) const { // virtual // use the _rigOverride matrices instead of the Model::_rig -void SoftAttachmentModel::updateClusterMatrices(bool triggerBlendshapes) { +void SoftAttachmentModel::updateClusterMatrices() { if (!_needsUpdateClusterMatrices) { return; } @@ -78,7 +78,7 @@ void SoftAttachmentModel::updateClusterMatrices(bool triggerBlendshapes) { // post the blender if we're not currently waiting for one to finish auto modelBlender = DependencyManager::get(); - if (triggerBlendshapes && modelBlender->shouldComputeBlendshapes() && geometry.hasBlendedMeshes() && _blendshapeCoefficients != _blendedBlendshapeCoefficients) { + if (_blendedVertexBuffersInitialized && modelBlender->shouldComputeBlendshapes() && geometry.hasBlendedMeshes() && _blendshapeCoefficients != _blendedBlendshapeCoefficients) { _blendedBlendshapeCoefficients = _blendshapeCoefficients; modelBlender->noteRequiresBlend(getThisPointer()); } diff --git a/libraries/render-utils/src/SoftAttachmentModel.h b/libraries/render-utils/src/SoftAttachmentModel.h index 03038c35f3..4335c1634e 100644 --- a/libraries/render-utils/src/SoftAttachmentModel.h +++ b/libraries/render-utils/src/SoftAttachmentModel.h @@ -27,7 +27,7 @@ public: ~SoftAttachmentModel(); void updateRig(float deltaTime, glm::mat4 parentTransform) override; - void updateClusterMatrices(bool triggerBlendshapes = true) override; + void updateClusterMatrices() override; protected: int getJointIndexOverride(int i) const; From 15f06264f92c52e15424ecfbf07d477cf9ece228 Mon Sep 17 00:00:00 2001 From: Simon Walton Date: Tue, 11 Sep 2018 17:03:39 -0700 Subject: [PATCH 42/44] Remove unneeded #include --- libraries/shared/src/SharedUtil.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/libraries/shared/src/SharedUtil.cpp b/libraries/shared/src/SharedUtil.cpp index e52e815520..bb22a1e753 100644 --- a/libraries/shared/src/SharedUtil.cpp +++ b/libraries/shared/src/SharedUtil.cpp @@ -21,7 +21,6 @@ #include #include #include -#include #include From 0d72e3808c17bd057e1c85d358f4ba467174630b Mon Sep 17 00:00:00 2001 From: Simon Walton Date: Wed, 12 Sep 2018 10:31:26 -0700 Subject: [PATCH 43/44] Small changes per review --- libraries/avatars/src/AvatarData.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/libraries/avatars/src/AvatarData.cpp b/libraries/avatars/src/AvatarData.cpp index 04efcb242e..cd1567af4d 100644 --- a/libraries/avatars/src/AvatarData.cpp +++ b/libraries/avatars/src/AvatarData.cpp @@ -563,7 +563,7 @@ QByteArray AvatarData::toByteArray(AvatarDataDetail dataDetail, quint64 lastSent unsigned char* validityPosition = destinationBuffer; unsigned char validity = 0; int validityBit = 0; - int numValidityBytes = (numJoints + BITS_IN_BYTE - 1) / BITS_IN_BYTE; + int numValidityBytes = calcBitVectorSize(numJoints); #ifdef WANT_DEBUG int rotationSentCount = 0; @@ -690,6 +690,7 @@ QByteArray AvatarData::toByteArray(AvatarDataDetail dataDetail, quint64 lastSent glm::quat mouseFarGrabRotation = extractRotation(mouseFarGrabMatrix); AVATAR_MEMCPY(leftFarGrabPosition); + // Can't do block copy as struct order is x, y, z, w. data->leftFarGrabRotation[0] = leftFarGrabRotation.w; data->leftFarGrabRotation[1] = leftFarGrabRotation.x; data->leftFarGrabRotation[2] = leftFarGrabRotation.y; From 7136698c6bf0ae1b0dc2379518c0bf5fc0267633 Mon Sep 17 00:00:00 2001 From: Simon Walton Date: Wed, 12 Sep 2018 10:49:26 -0700 Subject: [PATCH 44/44] Move default pose flags out of joint-positions condition --- libraries/avatars/src/AvatarData.cpp | 47 ++++++++++++++-------------- 1 file changed, 24 insertions(+), 23 deletions(-) diff --git a/libraries/avatars/src/AvatarData.cpp b/libraries/avatars/src/AvatarData.cpp index cd1567af4d..5be6631c59 100644 --- a/libraries/avatars/src/AvatarData.cpp +++ b/libraries/avatars/src/AvatarData.cpp @@ -548,13 +548,15 @@ QByteArray AvatarData::toByteArray(AvatarDataDetail dataDetail, quint64 lastSent } } + QVector jointData; + if (hasJointData || hasJointDefaultPoseFlags) { + QReadLocker readLock(&_jointDataLock); + jointData = _jointData; + } + // If it is connected, pack up the data if (hasJointData) { auto startSection = destinationBuffer; - QReadLocker readLock(&_jointDataLock); - - const QVector jointData(_jointData); - readLock.unlock(); // Unlock quickly. // joint rotation data int numJoints = jointData.size(); @@ -736,30 +738,29 @@ QByteArray AvatarData::toByteArray(AvatarDataDetail dataDetail, quint64 lastSent outboundDataRateOut->jointDataRate.increment(numBytes); } - // Always true, currently: - if (hasJointDefaultPoseFlags) { - auto startSection = destinationBuffer; + } - // write numJoints - int numJoints = jointData.size(); - *destinationBuffer++ = (uint8_t)numJoints; + if (hasJointDefaultPoseFlags) { + auto startSection = destinationBuffer; - // write rotationIsDefaultPose bits - destinationBuffer += writeBitVector(destinationBuffer, numJoints, [&](int i) { - return jointData[i].rotationIsDefaultPose; - }); + // write numJoints + int numJoints = jointData.size(); + *destinationBuffer++ = (uint8_t)numJoints; - // write translationIsDefaultPose bits - destinationBuffer += writeBitVector(destinationBuffer, numJoints, [&](int i) { - return jointData[i].translationIsDefaultPose; - }); + // write rotationIsDefaultPose bits + destinationBuffer += writeBitVector(destinationBuffer, numJoints, [&](int i) { + return jointData[i].rotationIsDefaultPose; + }); - if (outboundDataRateOut) { - size_t numBytes = destinationBuffer - startSection; - outboundDataRateOut->jointDefaultPoseFlagsRate.increment(numBytes); - } + // write translationIsDefaultPose bits + destinationBuffer += writeBitVector(destinationBuffer, numJoints, [&](int i) { + return jointData[i].translationIsDefaultPose; + }); + + if (outboundDataRateOut) { + size_t numBytes = destinationBuffer - startSection; + outboundDataRateOut->jointDefaultPoseFlagsRate.increment(numBytes); } - } int avatarDataSize = destinationBuffer - startPosition;