From 47e99cdeacbc2023fd0762669e22bb644ee3df18 Mon Sep 17 00:00:00 2001 From: Simon Walton Date: Tue, 24 Jul 2018 16:50:37 -0700 Subject: [PATCH 01/25] Initial code for AC side of Enabling Physics plan When interface sets query flag send only entities close to main frustrum origin, until no more available, then send unreliabe sequence # reliably. --- .../src/entities/EntityTreeSendThread.cpp | 9 ++++ .../src/entities/EntityTreeSendThread.h | 2 + .../src/octree/OctreeSendThread.cpp | 30 +++++++---- .../src/octree/OctreeSendThread.h | 1 + interface/src/Application.cpp | 1 + .../networking/src/udt/PacketHeaders.cpp | 2 +- libraries/networking/src/udt/PacketHeaders.h | 1 + libraries/octree/src/OctreeQuery.cpp | 19 ++++++- libraries/octree/src/OctreeQuery.h | 8 +++ .../shared/src/shared/ConicalViewFrustum.cpp | 5 ++ .../shared/src/shared/ConicalViewFrustum.h | 3 ++ tools/dissectors/1-hfudt.lua | 51 +++++++++++++++++-- 12 files changed, 116 insertions(+), 16 deletions(-) diff --git a/assignment-client/src/entities/EntityTreeSendThread.cpp b/assignment-client/src/entities/EntityTreeSendThread.cpp index f008ef9925..af0c09aec7 100644 --- a/assignment-client/src/entities/EntityTreeSendThread.cpp +++ b/assignment-client/src/entities/EntityTreeSendThread.cpp @@ -17,6 +17,8 @@ #include "EntityServer.h" +// Initially just send all items within this distance. +const float EntityTreeSendThread::INITIAL_RADIUS = 50.0f; EntityTreeSendThread::EntityTreeSendThread(OctreeServer* myServer, const SharedNodePointer& node) : OctreeSendThread(myServer, node) @@ -112,6 +114,13 @@ void EntityTreeSendThread::traverseTreeAndSendContents(SharedNodePointer node, O int32_t lodLevelOffset = nodeData->getBoundaryLevelAdjust() + (viewFrustumChanged ? LOW_RES_MOVING_ADJUST : NO_BOUNDARY_ADJUST); newView.lodScaleFactor = powf(2.0f, lodLevelOffset); + if (nodeData->wantReportInitialResult() && !newView.viewFrustums.empty()) { + auto& mainView = newView.viewFrustums[0]; + // Force acceptance within INITIAL_RADIUS. + mainView.setSimpleRadius(INITIAL_RADIUS); + newView.lodScaleFactor = 0.0f; + } + startNewTraversal(newView, root); // When the viewFrustum changed the sort order may be incorrect, so we re-sort diff --git a/assignment-client/src/entities/EntityTreeSendThread.h b/assignment-client/src/entities/EntityTreeSendThread.h index 1305d7bfc7..8c66f47aea 100644 --- a/assignment-client/src/entities/EntityTreeSendThread.h +++ b/assignment-client/src/entities/EntityTreeSendThread.h @@ -58,6 +58,8 @@ private: int32_t _numEntitiesOffset { 0 }; uint16_t _numEntities { 0 }; + const static float INITIAL_RADIUS; + private slots: void editingEntityPointer(const EntityItemPointer& entity); void deletingEntityPointer(EntityItem* entity); diff --git a/assignment-client/src/octree/OctreeSendThread.cpp b/assignment-client/src/octree/OctreeSendThread.cpp index e9aa44b970..98b06d6344 100644 --- a/assignment-client/src/octree/OctreeSendThread.cpp +++ b/assignment-client/src/octree/OctreeSendThread.cpp @@ -195,6 +195,7 @@ int OctreeSendThread::handlePacketSend(SharedNodePointer node, OctreeQueryNode* // actually send it OctreeServer::didCallWriteDatagram(this); DependencyManager::get()->sendUnreliablePacket(statsPacket, *node); + _lastSequenceNumber = (decltype(_lastSequenceNumber)) statsPacket.getSequenceNumber(); } else { // not enough room in the packet, send two packets @@ -211,10 +212,9 @@ int OctreeSendThread::handlePacketSend(SharedNodePointer node, OctreeQueryNode* //_totalWastedBytes += 0; _trueBytesSent += numBytes; numPackets++; + NLPacket& sentPacket = nodeData->getPacket(); if (debug) { - NLPacket& sentPacket = nodeData->getPacket(); - sentPacket.seek(sizeof(OCTREE_PACKET_FLAGS)); OCTREE_PACKET_SEQUENCE sequence; @@ -231,9 +231,10 @@ int OctreeSendThread::handlePacketSend(SharedNodePointer node, OctreeQueryNode* // second packet OctreeServer::didCallWriteDatagram(this); - DependencyManager::get()->sendUnreliablePacket(nodeData->getPacket(), *node); + DependencyManager::get()->sendUnreliablePacket(sentPacket, *node); + _lastSequenceNumber = (decltype(_lastSequenceNumber)) sentPacket.getSequenceNumber(); - numBytes = nodeData->getPacket().getDataSize(); + numBytes = sentPacket.getDataSize(); _totalBytes += numBytes; _totalPackets++; // we count wasted bytes here because we were unable to fit the stats packet @@ -243,8 +244,6 @@ int OctreeSendThread::handlePacketSend(SharedNodePointer node, OctreeQueryNode* numPackets++; if (debug) { - NLPacket& sentPacket = nodeData->getPacket(); - sentPacket.seek(sizeof(OCTREE_PACKET_FLAGS)); OCTREE_PACKET_SEQUENCE sequence; @@ -265,9 +264,11 @@ int OctreeSendThread::handlePacketSend(SharedNodePointer node, OctreeQueryNode* if (nodeData->isPacketWaiting() && !nodeData->isShuttingDown()) { // just send the octree packet OctreeServer::didCallWriteDatagram(this); - DependencyManager::get()->sendUnreliablePacket(nodeData->getPacket(), *node); + NLPacket& sentPacket = nodeData->getPacket(); + DependencyManager::get()->sendUnreliablePacket(sentPacket, *node); + _lastSequenceNumber = (decltype(_lastSequenceNumber)) sentPacket.getSequenceNumber(); - int numBytes = nodeData->getPacket().getDataSize(); + int numBytes = sentPacket.getDataSize(); _totalBytes += numBytes; _totalPackets++; int thisWastedBytes = udt::MAX_PACKET_SIZE - numBytes; @@ -276,8 +277,6 @@ int OctreeSendThread::handlePacketSend(SharedNodePointer node, OctreeQueryNode* _trueBytesSent += numBytes; if (debug) { - NLPacket& sentPacket = nodeData->getPacket(); - sentPacket.seek(sizeof(OCTREE_PACKET_FLAGS)); OCTREE_PACKET_SEQUENCE sequence; @@ -512,6 +511,17 @@ void OctreeSendThread::traverseTreeAndSendContents(SharedNodePointer node, Octre OctreeServer::trackInsideTime((float)(usecTimestampNow() - startInside)); } + if (params.stopReason == EncodeBitstreamParams::FINISHED && nodeData->wantReportInitialResult()) { + // Dealt with nearby entities. + nodeData->setReportInitialResult(false); + + // send EntityQueryInitialResultsComplete reliable packet ... + auto initialCompletion = NLPacket::create(PacketType::EntityQueryInitialResultsComplete, -1, true); + QDataStream initialCompletionStream(initialCompletion.get()); + initialCompletionStream << _lastSequenceNumber; + DependencyManager::get()->sendPacket(std::move(initialCompletion), *node.data()); + } + if (somethingToSend && _myServer->wantsVerboseDebug()) { qCDebug(octree) << "Hit PPS Limit, packetsSentThisInterval =" << _packetsSentThisInterval << " maxPacketsPerInterval = " << maxPacketsPerInterval diff --git a/assignment-client/src/octree/OctreeSendThread.h b/assignment-client/src/octree/OctreeSendThread.h index 91c0ec7adc..58d2e3a160 100644 --- a/assignment-client/src/octree/OctreeSendThread.h +++ b/assignment-client/src/octree/OctreeSendThread.h @@ -60,6 +60,7 @@ protected: QWeakPointer _node; OctreeServer* _myServer { nullptr }; QUuid _nodeUuid; + udt::SequenceNumber::Type _lastSequenceNumber { 0 }; private: /// Called before a packetDistributor pass to allow for pre-distribution processing diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp index 311c08b858..55e634275d 100644 --- a/interface/src/Application.cpp +++ b/interface/src/Application.cpp @@ -6134,6 +6134,7 @@ void Application::queryOctree(NodeType_t serverType, PacketType packetType) { auto lodManager = DependencyManager::get(); _octreeQuery.setOctreeSizeScale(lodManager->getOctreeSizeScale()); _octreeQuery.setBoundaryLevelAdjust(lodManager->getBoundaryLevelAdjust()); + _octreeQuery.setReportInitialResult(true); auto nodeList = DependencyManager::get(); diff --git a/libraries/networking/src/udt/PacketHeaders.cpp b/libraries/networking/src/udt/PacketHeaders.cpp index 13ffcb5120..bb9666ee37 100644 --- a/libraries/networking/src/udt/PacketHeaders.cpp +++ b/libraries/networking/src/udt/PacketHeaders.cpp @@ -95,7 +95,7 @@ PacketVersion versionForPacketType(PacketType packetType) { case PacketType::AvatarIdentityRequest: return 22; default: - return 21; + return 22; } } diff --git a/libraries/networking/src/udt/PacketHeaders.h b/libraries/networking/src/udt/PacketHeaders.h index 6e1aca83e5..7cba3baaf0 100644 --- a/libraries/networking/src/udt/PacketHeaders.h +++ b/libraries/networking/src/udt/PacketHeaders.h @@ -126,6 +126,7 @@ public: EntityScriptCallMethod, ChallengeOwnershipRequest, ChallengeOwnershipReply, + EntityQueryInitialResultsComplete, OctreeDataFileRequest, OctreeDataFileReply, diff --git a/libraries/octree/src/OctreeQuery.cpp b/libraries/octree/src/OctreeQuery.cpp index 0d56dbb88f..5fb886df10 100644 --- a/libraries/octree/src/OctreeQuery.cpp +++ b/libraries/octree/src/OctreeQuery.cpp @@ -27,6 +27,10 @@ OctreeQuery::OctreeQuery(bool randomizeConnectionID) { } } +const OctreeQuery::OctreeQueryFlags operator|=(OctreeQuery::OctreeQueryFlags& lhs, int rhs) { + return lhs = OctreeQuery::OctreeQueryFlags(lhs | rhs); +} + int OctreeQuery::getBroadcastData(unsigned char* destinationBuffer) { unsigned char* bufferStart = destinationBuffer; @@ -76,7 +80,12 @@ int OctreeQuery::getBroadcastData(unsigned char* destinationBuffer) { memcpy(destinationBuffer, binaryParametersDocument.data(), binaryParametersBytes); destinationBuffer += binaryParametersBytes; } - + + OctreeQueryFlags queryFlags { NoFlags }; + queryFlags |= (_reportInitialResult ? OctreeQuery::WantInitialResult : 0); + memcpy(destinationBuffer, &queryFlags, sizeof(queryFlags)); + destinationBuffer += sizeof(queryFlags); + return destinationBuffer - bufferStart; } @@ -150,6 +159,12 @@ int OctreeQuery::parseData(ReceivedMessage& message) { QWriteLocker jsonParameterLocker { &_jsonParametersLock }; _jsonParameters = newJsonDocument.object(); } - + + OctreeQueryFlags queryFlags; + memcpy(&queryFlags, sourceBuffer, sizeof(queryFlags)); + sourceBuffer += sizeof(queryFlags); + + _reportInitialResult = bool(queryFlags & OctreeQueryFlags::WantInitialResult); + return sourceBuffer - startPosition; } diff --git a/libraries/octree/src/OctreeQuery.h b/libraries/octree/src/OctreeQuery.h index 0ca75bdeb0..711a22e9b5 100644 --- a/libraries/octree/src/OctreeQuery.h +++ b/libraries/octree/src/OctreeQuery.h @@ -52,6 +52,10 @@ public: bool hasReceivedFirstQuery() const { return _hasReceivedFirstQuery; } + // Want a report when the initial query is complete. + bool wantReportInitialResult() const { return _reportInitialResult; } + void setReportInitialResult(bool reportInitialResult) { _reportInitialResult = reportInitialResult; } + signals: void incomingConnectionIDChanged(); @@ -73,8 +77,12 @@ protected: QJsonObject _jsonParameters; QReadWriteLock _jsonParametersLock; + + enum OctreeQueryFlags : uint16_t { NoFlags = 0x0, WantInitialResult = 0x1 }; + friend const OctreeQuery::OctreeQueryFlags operator|=(OctreeQuery::OctreeQueryFlags& lhs, const int rhs); bool _hasReceivedFirstQuery { false }; + bool _reportInitialResult { false }; }; #endif // hifi_OctreeQuery_h diff --git a/libraries/shared/src/shared/ConicalViewFrustum.cpp b/libraries/shared/src/shared/ConicalViewFrustum.cpp index 2ef096e3a8..3d56683c82 100644 --- a/libraries/shared/src/shared/ConicalViewFrustum.cpp +++ b/libraries/shared/src/shared/ConicalViewFrustum.cpp @@ -144,3 +144,8 @@ int ConicalViewFrustum::deserialize(const unsigned char* sourceBuffer) { return sourceBuffer - startPosition; } + +void ConicalViewFrustum::setSimpleRadius(float radius) { + _radius = radius; + _farClip = radius / 2.0f; +} diff --git a/libraries/shared/src/shared/ConicalViewFrustum.h b/libraries/shared/src/shared/ConicalViewFrustum.h index dc372d560e..2180516441 100644 --- a/libraries/shared/src/shared/ConicalViewFrustum.h +++ b/libraries/shared/src/shared/ConicalViewFrustum.h @@ -54,6 +54,9 @@ public: int serialize(unsigned char* destinationBuffer) const; int deserialize(const unsigned char* sourceBuffer); + // Just test for within radius. + void setSimpleRadius(float radius); + private: glm::vec3 _position { 0.0f, 0.0f, 0.0f }; glm::vec3 _direction { 0.0f, 0.0f, 1.0f }; diff --git a/tools/dissectors/1-hfudt.lua b/tools/dissectors/1-hfudt.lua index 137bee659b..9bed892885 100644 --- a/tools/dissectors/1-hfudt.lua +++ b/tools/dissectors/1-hfudt.lua @@ -86,12 +86,12 @@ local packet_types = { [22] = "ICEServerPeerInformation", [23] = "ICEServerQuery", [24] = "OctreeStats", - [25] = "Jurisdiction", + [25] = "UNUSED_PACKET_TYPE_1", [26] = "AvatarIdentityRequest", [27] = "AssignmentClientStatus", [28] = "NoisyMute", [29] = "AvatarIdentity", - [30] = "AvatarBillboard", + [30] = "NodeIgnoreRequest", [31] = "DomainConnectRequest", [32] = "DomainServerRequireDTLS", [33] = "NodeJsonStats", @@ -115,7 +115,52 @@ local packet_types = { [51] = "AssetUpload", [52] = "AssetUploadReply", [53] = "AssetGetInfo", - [54] = "AssetGetInfoReply" + [54] = "AssetGetInfoReply", + [55] = "DomainDisconnectRequest", + [56] = "DomainServerRemovedNode", + [57] = "MessagesData", + [58] = "MessagesSubscribe", + [59] = "MessagesUnsubscribe", + [60] = "ICEServerHeartbeatDenied", + [61] = "AssetMappingOperation", + [62] = "AssetMappingOperationReply", + [63] = "ICEServerHeartbeatACK", + [64] = "NegotiateAudioFormat", + [65] = "SelectedAudioFormat", + [66] = "MoreEntityShapes", + [67] = "NodeKickRequest", + [68] = "NodeMuteRequest", + [69] = "RadiusIgnoreRequest", + [70] = "UsernameFromIDRequest", + [71] = "UsernameFromIDReply", + [72] = "AvatarQuery", + [73] = "RequestsDomainListData", + [74] = "PerAvatarGainSet", + [75] = "EntityScriptGetStatus", + [76] = "EntityScriptGetStatusReply", + [77] = "ReloadEntityServerScript", + [78] = "EntityPhysics", + [79] = "EntityServerScriptLog", + [80] = "AdjustAvatarSorting", + [81] = "OctreeFileReplacement", + [82] = "CollisionEventChanges", + [83] = "ReplicatedMicrophoneAudioNoEcho", + [84] = "ReplicatedMicrophoneAudioWithEcho", + [85] = "ReplicatedInjectAudio", + [86] = "ReplicatedSilentAudioFrame", + [87] = "ReplicatedAvatarIdentity", + [88] = "ReplicatedKillAvatar", + [89] = "ReplicatedBulkAvatarData", + [90] = "DomainContentReplacementFromUrl", + [91] = "ChallengeOwnership", + [92] = "EntityScriptCallMethod", + [93] = "ChallengeOwnershipRequest", + [94] = "ChallengeOwnershipReply", + [95] = "EntityQueryInitialResultsComplete", + [96] = "OctreeDataFileRequest", + [97] = "OctreeDataFileReply", + [98] = "OctreeDataPersist", + [99] = "EntityClone" } local unsourced_packet_types = { From 0cc96f2be14163f7bee0650781e5ce4df1b977a5 Mon Sep 17 00:00:00 2001 From: Simon Walton Date: Tue, 17 Jul 2018 11:04:25 -0700 Subject: [PATCH 02/25] Raise posix descriptor limit if necessary --- .../src/AssignmentClientMonitor.cpp | 24 +++++++++++++++++++ .../src/AssignmentClientMonitor.h | 2 ++ 2 files changed, 26 insertions(+) diff --git a/assignment-client/src/AssignmentClientMonitor.cpp b/assignment-client/src/AssignmentClientMonitor.cpp index 2847d4ebf1..163e5229bd 100644 --- a/assignment-client/src/AssignmentClientMonitor.cpp +++ b/assignment-client/src/AssignmentClientMonitor.cpp @@ -25,6 +25,9 @@ #include "AssignmentClientChildData.h" #include "SharedUtil.h" #include +#ifdef POSIX_SOURCE +#include +#endif const QString ASSIGNMENT_CLIENT_MONITOR_TARGET_NAME = "assignment-client-monitor"; const int WAIT_FOR_CHILD_MSECS = 1000; @@ -71,6 +74,7 @@ AssignmentClientMonitor::AssignmentClientMonitor(const unsigned int numAssignmen auto& packetReceiver = DependencyManager::get()->getPacketReceiver(); packetReceiver.registerListener(PacketType::AssignmentClientStatus, this, "handleChildStatusPacket"); + adjustOsResources(_numAssignmentClientForks); // use QProcess to fork off a process for each of the child assignment clients for (unsigned int i = 0; i < _numAssignmentClientForks; i++) { spawnChildClient(); @@ -372,3 +376,23 @@ bool AssignmentClientMonitor::handleHTTPRequest(HTTPConnection* connection, cons return true; } + +void AssignmentClientMonitor::adjustOsResources(unsigned int numForks) const +{ +#ifdef _POSIX_SOURCE + // QProcess on Unix uses six descriptors, some temporarily, for each child proc. + unsigned requiredDescriptors = 30 + 6 * numForks; + struct rlimit descLimits; + if (getrlimit(RLIMIT_NOFILE, &descLimits) == 0) { + if (descLimits.rlim_cur < requiredDescriptors) { + descLimits.rlim_cur = requiredDescriptors; + if (setrlimit(RLIMIT_NOFILE, &descLimits) == 0) { + qDebug() << "Resetting descriptor limit to" << requiredDescriptors; + } else { + const char *const errorString = strerror(errno); + qDebug() << "Failed to reset descriptor limit to" << requiredDescriptors << ":" << errorString; + } + } + } +#endif +} diff --git a/assignment-client/src/AssignmentClientMonitor.h b/assignment-client/src/AssignmentClientMonitor.h index 8848d503ae..331eed4599 100644 --- a/assignment-client/src/AssignmentClientMonitor.h +++ b/assignment-client/src/AssignmentClientMonitor.h @@ -23,6 +23,7 @@ #include "AssignmentClientChildData.h" #include #include +#include extern const char* NUM_FORKS_PARAMETER; @@ -55,6 +56,7 @@ public slots: private: void spawnChildClient(); void simultaneousWaitOnChildren(int waitMsecs); + void adjustOsResources(unsigned int numForks) const; QTimer _checkSparesTimer; // every few seconds see if it need fewer or more spare children From 6098c53d32641b73dc4d2f50c6d908857c5afa36 Mon Sep 17 00:00:00 2001 From: Simon Walton Date: Tue, 17 Jul 2018 11:29:31 -0700 Subject: [PATCH 03/25] Remove #include from header --- assignment-client/src/AssignmentClientMonitor.h | 1 - 1 file changed, 1 deletion(-) diff --git a/assignment-client/src/AssignmentClientMonitor.h b/assignment-client/src/AssignmentClientMonitor.h index 331eed4599..659ff4b001 100644 --- a/assignment-client/src/AssignmentClientMonitor.h +++ b/assignment-client/src/AssignmentClientMonitor.h @@ -23,7 +23,6 @@ #include "AssignmentClientChildData.h" #include #include -#include extern const char* NUM_FORKS_PARAMETER; From 7181c3ad487fd39f8760e5333e3d097798db33e5 Mon Sep 17 00:00:00 2001 From: Simon Walton Date: Tue, 17 Jul 2018 12:11:22 -0700 Subject: [PATCH 04/25] Fix typo in preprocessor symbol --- assignment-client/src/AssignmentClientMonitor.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/assignment-client/src/AssignmentClientMonitor.cpp b/assignment-client/src/AssignmentClientMonitor.cpp index 163e5229bd..9809c1b7d6 100644 --- a/assignment-client/src/AssignmentClientMonitor.cpp +++ b/assignment-client/src/AssignmentClientMonitor.cpp @@ -25,7 +25,7 @@ #include "AssignmentClientChildData.h" #include "SharedUtil.h" #include -#ifdef POSIX_SOURCE +#ifdef _POSIX_SOURCE #include #endif From 06d1602c0dd42495acd671fe0105c5cefc719c4c Mon Sep 17 00:00:00 2001 From: Simon Walton Date: Tue, 17 Jul 2018 14:39:21 -0700 Subject: [PATCH 05/25] Use max of --max & --n for resources; other clean-up --- assignment-client/src/AssignmentClientMonitor.cpp | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/assignment-client/src/AssignmentClientMonitor.cpp b/assignment-client/src/AssignmentClientMonitor.cpp index 9809c1b7d6..eb764a128c 100644 --- a/assignment-client/src/AssignmentClientMonitor.cpp +++ b/assignment-client/src/AssignmentClientMonitor.cpp @@ -74,7 +74,7 @@ AssignmentClientMonitor::AssignmentClientMonitor(const unsigned int numAssignmen auto& packetReceiver = DependencyManager::get()->getPacketReceiver(); packetReceiver.registerListener(PacketType::AssignmentClientStatus, this, "handleChildStatusPacket"); - adjustOsResources(_numAssignmentClientForks); + adjustOsResources(std::max(_numAssignmentClientForks, _maxAssignmentClientForks)); // use QProcess to fork off a process for each of the child assignment clients for (unsigned int i = 0; i < _numAssignmentClientForks; i++) { spawnChildClient(); @@ -380,7 +380,8 @@ bool AssignmentClientMonitor::handleHTTPRequest(HTTPConnection* connection, cons void AssignmentClientMonitor::adjustOsResources(unsigned int numForks) const { #ifdef _POSIX_SOURCE - // QProcess on Unix uses six descriptors, some temporarily, for each child proc. + // QProcess on Unix uses six (I think) descriptors, some temporarily, for each child proc. + // Formula based on tests with a Ubuntu 16.04 VM. unsigned requiredDescriptors = 30 + 6 * numForks; struct rlimit descLimits; if (getrlimit(RLIMIT_NOFILE, &descLimits) == 0) { @@ -393,6 +394,9 @@ void AssignmentClientMonitor::adjustOsResources(unsigned int numForks) const qDebug() << "Failed to reset descriptor limit to" << requiredDescriptors << ":" << errorString; } } + } else { + const char *const errorString = strerror(errno); + qDebug() << "Failed to read descriptor limit:" << errorString; } #endif } From 187259c838cfc950192a86ab8488501294c0caf6 Mon Sep 17 00:00:00 2001 From: Simon Walton Date: Tue, 17 Jul 2018 11:04:25 -0700 Subject: [PATCH 06/25] Raise posix descriptor limit if necessary --- assignment-client/src/AssignmentClientMonitor.h | 1 + 1 file changed, 1 insertion(+) diff --git a/assignment-client/src/AssignmentClientMonitor.h b/assignment-client/src/AssignmentClientMonitor.h index 659ff4b001..331eed4599 100644 --- a/assignment-client/src/AssignmentClientMonitor.h +++ b/assignment-client/src/AssignmentClientMonitor.h @@ -23,6 +23,7 @@ #include "AssignmentClientChildData.h" #include #include +#include extern const char* NUM_FORKS_PARAMETER; From 2d653db0640832bc0875fcf01f4fc4afc7e214ea Mon Sep 17 00:00:00 2001 From: Simon Walton Date: Tue, 17 Jul 2018 11:29:31 -0700 Subject: [PATCH 07/25] Remove #include from header --- assignment-client/src/AssignmentClientMonitor.h | 1 - 1 file changed, 1 deletion(-) diff --git a/assignment-client/src/AssignmentClientMonitor.h b/assignment-client/src/AssignmentClientMonitor.h index 331eed4599..659ff4b001 100644 --- a/assignment-client/src/AssignmentClientMonitor.h +++ b/assignment-client/src/AssignmentClientMonitor.h @@ -23,7 +23,6 @@ #include "AssignmentClientChildData.h" #include #include -#include extern const char* NUM_FORKS_PARAMETER; From 7cb917f7355bdfea0dfb3d4fd0cb8441a3c1a2eb Mon Sep 17 00:00:00 2001 From: Simon Walton Date: Wed, 25 Jul 2018 14:45:26 -0700 Subject: [PATCH 08/25] Angles aren't boolean! --- libraries/shared/src/shared/ConicalViewFrustum.cpp | 6 +++--- libraries/shared/src/shared/ConicalViewFrustum.h | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/libraries/shared/src/shared/ConicalViewFrustum.cpp b/libraries/shared/src/shared/ConicalViewFrustum.cpp index 3d56683c82..78f4f7d1bc 100644 --- a/libraries/shared/src/shared/ConicalViewFrustum.cpp +++ b/libraries/shared/src/shared/ConicalViewFrustum.cpp @@ -69,7 +69,7 @@ bool ConicalViewFrustum::intersects(const AABox& box) const { return intersects(position, distance, radius); } -bool ConicalViewFrustum::getAngularSize(const AACube& cube) const { +float ConicalViewFrustum::getAngularSize(const AACube& cube) const { auto radius = 0.5f * SQRT_THREE * cube.getScale(); // radius of bounding sphere auto position = cube.calcCenter() - _position; // position of bounding sphere in view-frame float distance = glm::length(position); @@ -77,7 +77,7 @@ bool ConicalViewFrustum::getAngularSize(const AACube& cube) const { return getAngularSize(distance, radius); } -bool ConicalViewFrustum::getAngularSize(const AABox& box) const { +float ConicalViewFrustum::getAngularSize(const AABox& box) const { auto radius = 0.5f * glm::length(box.getScale()); // radius of bounding sphere auto position = box.calcCenter() - _position; // position of bounding sphere in view-frame float distance = glm::length(position); @@ -107,7 +107,7 @@ bool ConicalViewFrustum::intersects(const glm::vec3& relativePosition, float dis sqrtf(distance * distance - radius * radius) * _cosAngle - radius * _sinAngle; } -bool ConicalViewFrustum::getAngularSize(float distance, float radius) const { +float ConicalViewFrustum::getAngularSize(float distance, float radius) const { const float AVOID_DIVIDE_BY_ZERO = 0.001f; float angularSize = radius / (distance + AVOID_DIVIDE_BY_ZERO); return angularSize; diff --git a/libraries/shared/src/shared/ConicalViewFrustum.h b/libraries/shared/src/shared/ConicalViewFrustum.h index 2180516441..6a2cc53f03 100644 --- a/libraries/shared/src/shared/ConicalViewFrustum.h +++ b/libraries/shared/src/shared/ConicalViewFrustum.h @@ -45,11 +45,11 @@ public: bool intersects(const AACube& cube) const; bool intersects(const AABox& box) const; - bool getAngularSize(const AACube& cube) const; - bool getAngularSize(const AABox& box) const; + float getAngularSize(const AACube& cube) const; + float getAngularSize(const AABox& box) const; bool intersects(const glm::vec3& relativePosition, float distance, float radius) const; - bool getAngularSize(float distance, float radius) const; + float getAngularSize(float distance, float radius) const; int serialize(unsigned char* destinationBuffer) const; int deserialize(const unsigned char* sourceBuffer); From 8f0283444719a88fe853f327e35cc3e74cb30dfa Mon Sep 17 00:00:00 2001 From: Simon Walton Date: Wed, 25 Jul 2018 14:46:47 -0700 Subject: [PATCH 09/25] Set params.stopReason in another place --- assignment-client/src/entities/EntityTreeSendThread.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/assignment-client/src/entities/EntityTreeSendThread.cpp b/assignment-client/src/entities/EntityTreeSendThread.cpp index af0c09aec7..5ca4c3588e 100644 --- a/assignment-client/src/entities/EntityTreeSendThread.cpp +++ b/assignment-client/src/entities/EntityTreeSendThread.cpp @@ -310,6 +310,7 @@ void EntityTreeSendThread::startNewTraversal(const DiffTraversal::View& view, En bool EntityTreeSendThread::traverseTreeAndBuildNextPacketPayload(EncodeBitstreamParams& params, const QJsonObject& jsonFilters) { if (_sendQueue.empty()) { + params.stopReason = EncodeBitstreamParams::FINISHED; OctreeServer::trackEncodeTime(OctreeServer::SKIP_TIME); return false; } From 2c06487df086d9bab9682152939ecbb7324f6bd3 Mon Sep 17 00:00:00 2001 From: Simon Walton Date: Wed, 25 Jul 2018 17:05:05 -0700 Subject: [PATCH 10/25] Move sending of completion packet up to EntityTreeSendThread I hadn't appreciated the continuous nature of the scene traversal. Looks good now. Also changed some identifier names. --- .../src/entities/EntityTreeSendThread.cpp | 21 +++++++++++++++---- .../src/entities/EntityTreeSendThread.h | 2 +- .../src/octree/OctreeSendThread.cpp | 15 +++---------- .../src/octree/OctreeSendThread.h | 2 +- libraries/octree/src/OctreeQuery.cpp | 4 ++-- libraries/octree/src/OctreeQuery.h | 8 +++---- 6 files changed, 28 insertions(+), 24 deletions(-) diff --git a/assignment-client/src/entities/EntityTreeSendThread.cpp b/assignment-client/src/entities/EntityTreeSendThread.cpp index 5ca4c3588e..19dc92686a 100644 --- a/assignment-client/src/entities/EntityTreeSendThread.cpp +++ b/assignment-client/src/entities/EntityTreeSendThread.cpp @@ -18,7 +18,7 @@ #include "EntityServer.h" // Initially just send all items within this distance. -const float EntityTreeSendThread::INITIAL_RADIUS = 50.0f; +const float EntityTreeSendThread::INITIAL_RADIUS = 10.0f; EntityTreeSendThread::EntityTreeSendThread(OctreeServer* myServer, const SharedNodePointer& node) : OctreeSendThread(myServer, node) @@ -102,7 +102,7 @@ void EntityTreeSendThread::preDistributionProcessing() { } } -void EntityTreeSendThread::traverseTreeAndSendContents(SharedNodePointer node, OctreeQueryNode* nodeData, +bool EntityTreeSendThread::traverseTreeAndSendContents(SharedNodePointer node, OctreeQueryNode* nodeData, bool viewFrustumChanged, bool isFullScene) { if (viewFrustumChanged || _traversal.finished()) { EntityTreeElementPointer root = std::dynamic_pointer_cast(_myServer->getOctree()->getRoot()); @@ -114,7 +114,7 @@ void EntityTreeSendThread::traverseTreeAndSendContents(SharedNodePointer node, O int32_t lodLevelOffset = nodeData->getBoundaryLevelAdjust() + (viewFrustumChanged ? LOW_RES_MOVING_ADJUST : NO_BOUNDARY_ADJUST); newView.lodScaleFactor = powf(2.0f, lodLevelOffset); - if (nodeData->wantReportInitialResult() && !newView.viewFrustums.empty()) { + if (nodeData->wantReportInitialCompletion() && !newView.viewFrustums.empty()) { auto& mainView = newView.viewFrustums[0]; // Force acceptance within INITIAL_RADIUS. mainView.setSimpleRadius(INITIAL_RADIUS); @@ -165,7 +165,20 @@ void EntityTreeSendThread::traverseTreeAndSendContents(SharedNodePointer node, O OctreeServer::trackTreeTraverseTime((float)(usecTimestampNow() - startTime)); } - OctreeSendThread::traverseTreeAndSendContents(node, nodeData, viewFrustumChanged, isFullScene); + bool sendComplete = OctreeSendThread::traverseTreeAndSendContents(node, nodeData, viewFrustumChanged, isFullScene); + + if (sendComplete && nodeData->wantReportInitialCompletion() && _traversal.finished()) { + // Dealt with all nearby entities. + nodeData->setReportInitialCompletion(false); + + // Send EntityQueryInitialResultsComplete reliable packet ... + auto initialCompletion = NLPacket::create(PacketType::EntityQueryInitialResultsComplete, -1, true); + QDataStream initialCompletionStream(initialCompletion.get()); + initialCompletionStream << _lastSequenceNumber; + DependencyManager::get()->sendPacket(std::move(initialCompletion), *node.data()); + } + + return sendComplete; } bool EntityTreeSendThread::addAncestorsToExtraFlaggedEntities(const QUuid& filteredEntityID, diff --git a/assignment-client/src/entities/EntityTreeSendThread.h b/assignment-client/src/entities/EntityTreeSendThread.h index 8c66f47aea..c9f4d06164 100644 --- a/assignment-client/src/entities/EntityTreeSendThread.h +++ b/assignment-client/src/entities/EntityTreeSendThread.h @@ -31,7 +31,7 @@ public: EntityTreeSendThread(OctreeServer* myServer, const SharedNodePointer& node); protected: - void traverseTreeAndSendContents(SharedNodePointer node, OctreeQueryNode* nodeData, + bool traverseTreeAndSendContents(SharedNodePointer node, OctreeQueryNode* nodeData, bool viewFrustumChanged, bool isFullScene) override; private slots: diff --git a/assignment-client/src/octree/OctreeSendThread.cpp b/assignment-client/src/octree/OctreeSendThread.cpp index 98b06d6344..54a7bbfa2d 100644 --- a/assignment-client/src/octree/OctreeSendThread.cpp +++ b/assignment-client/src/octree/OctreeSendThread.cpp @@ -433,7 +433,7 @@ int OctreeSendThread::packetDistributor(SharedNodePointer node, OctreeQueryNode* return _truePacketsSent; } -void OctreeSendThread::traverseTreeAndSendContents(SharedNodePointer node, OctreeQueryNode* nodeData, bool viewFrustumChanged, bool isFullScene) { +bool OctreeSendThread::traverseTreeAndSendContents(SharedNodePointer node, OctreeQueryNode* nodeData, bool viewFrustumChanged, bool isFullScene) { // calculate max number of packets that can be sent during this interval int clientMaxPacketsPerInterval = std::max(1, (nodeData->getMaxQueryPacketsPerSecond() / INTERVALS_PER_SECOND)); int maxPacketsPerInterval = std::min(clientMaxPacketsPerInterval, _myServer->getPacketsPerClientPerInterval()); @@ -511,20 +511,11 @@ void OctreeSendThread::traverseTreeAndSendContents(SharedNodePointer node, Octre OctreeServer::trackInsideTime((float)(usecTimestampNow() - startInside)); } - if (params.stopReason == EncodeBitstreamParams::FINISHED && nodeData->wantReportInitialResult()) { - // Dealt with nearby entities. - nodeData->setReportInitialResult(false); - - // send EntityQueryInitialResultsComplete reliable packet ... - auto initialCompletion = NLPacket::create(PacketType::EntityQueryInitialResultsComplete, -1, true); - QDataStream initialCompletionStream(initialCompletion.get()); - initialCompletionStream << _lastSequenceNumber; - DependencyManager::get()->sendPacket(std::move(initialCompletion), *node.data()); - } - if (somethingToSend && _myServer->wantsVerboseDebug()) { qCDebug(octree) << "Hit PPS Limit, packetsSentThisInterval =" << _packetsSentThisInterval << " maxPacketsPerInterval = " << maxPacketsPerInterval << " clientMaxPacketsPerInterval = " << clientMaxPacketsPerInterval; } + + return params.stopReason == EncodeBitstreamParams::FINISHED; } diff --git a/assignment-client/src/octree/OctreeSendThread.h b/assignment-client/src/octree/OctreeSendThread.h index 58d2e3a160..fc529c3e3a 100644 --- a/assignment-client/src/octree/OctreeSendThread.h +++ b/assignment-client/src/octree/OctreeSendThread.h @@ -52,7 +52,7 @@ protected: /// Implements generic processing behavior for this thread. virtual bool process() override; - virtual void traverseTreeAndSendContents(SharedNodePointer node, OctreeQueryNode* nodeData, + virtual bool traverseTreeAndSendContents(SharedNodePointer node, OctreeQueryNode* nodeData, bool viewFrustumChanged, bool isFullScene); virtual bool traverseTreeAndBuildNextPacketPayload(EncodeBitstreamParams& params, const QJsonObject& jsonFilters) = 0; diff --git a/libraries/octree/src/OctreeQuery.cpp b/libraries/octree/src/OctreeQuery.cpp index 5fb886df10..77977bf01e 100644 --- a/libraries/octree/src/OctreeQuery.cpp +++ b/libraries/octree/src/OctreeQuery.cpp @@ -82,7 +82,7 @@ int OctreeQuery::getBroadcastData(unsigned char* destinationBuffer) { } OctreeQueryFlags queryFlags { NoFlags }; - queryFlags |= (_reportInitialResult ? OctreeQuery::WantInitialResult : 0); + queryFlags |= (_reportInitialCompletion ? OctreeQuery::WantInitialCompletion : 0); memcpy(destinationBuffer, &queryFlags, sizeof(queryFlags)); destinationBuffer += sizeof(queryFlags); @@ -164,7 +164,7 @@ int OctreeQuery::parseData(ReceivedMessage& message) { memcpy(&queryFlags, sourceBuffer, sizeof(queryFlags)); sourceBuffer += sizeof(queryFlags); - _reportInitialResult = bool(queryFlags & OctreeQueryFlags::WantInitialResult); + _reportInitialCompletion = bool(queryFlags & OctreeQueryFlags::WantInitialCompletion); return sourceBuffer - startPosition; } diff --git a/libraries/octree/src/OctreeQuery.h b/libraries/octree/src/OctreeQuery.h index 711a22e9b5..04d6793158 100644 --- a/libraries/octree/src/OctreeQuery.h +++ b/libraries/octree/src/OctreeQuery.h @@ -53,8 +53,8 @@ public: bool hasReceivedFirstQuery() const { return _hasReceivedFirstQuery; } // Want a report when the initial query is complete. - bool wantReportInitialResult() const { return _reportInitialResult; } - void setReportInitialResult(bool reportInitialResult) { _reportInitialResult = reportInitialResult; } + bool wantReportInitialCompletion() const { return _reportInitialCompletion; } + void setReportInitialCompletion(bool reportInitialCompletion) { _reportInitialCompletion = reportInitialCompletion; } signals: void incomingConnectionIDChanged(); @@ -78,11 +78,11 @@ protected: QJsonObject _jsonParameters; QReadWriteLock _jsonParametersLock; - enum OctreeQueryFlags : uint16_t { NoFlags = 0x0, WantInitialResult = 0x1 }; + enum OctreeQueryFlags : uint16_t { NoFlags = 0x0, WantInitialCompletion = 0x1 }; friend const OctreeQuery::OctreeQueryFlags operator|=(OctreeQuery::OctreeQueryFlags& lhs, const int rhs); bool _hasReceivedFirstQuery { false }; - bool _reportInitialResult { false }; + bool _reportInitialCompletion { false }; }; #endif // hifi_OctreeQuery_h From a15e78a0f4420b5e51a73b487ba3f126a87f96ee Mon Sep 17 00:00:00 2001 From: Simon Walton Date: Wed, 25 Jul 2018 17:36:53 -0700 Subject: [PATCH 11/25] Missed an identifier change --- interface/src/Application.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp index 55e634275d..94f89a66ad 100644 --- a/interface/src/Application.cpp +++ b/interface/src/Application.cpp @@ -6134,7 +6134,7 @@ void Application::queryOctree(NodeType_t serverType, PacketType packetType) { auto lodManager = DependencyManager::get(); _octreeQuery.setOctreeSizeScale(lodManager->getOctreeSizeScale()); _octreeQuery.setBoundaryLevelAdjust(lodManager->getBoundaryLevelAdjust()); - _octreeQuery.setReportInitialResult(true); + _octreeQuery.setReportInitialCompletion(true); auto nodeList = DependencyManager::get(); From 0cec9a72d629b3888273e8ef7c169f0e5e7314c2 Mon Sep 17 00:00:00 2001 From: Simon Walton Date: Thu, 26 Jul 2018 18:10:38 -0700 Subject: [PATCH 12/25] Implement client-side enable physics once indicated EntityData has arrived Also now use Entity Server Protocol sequence numbers. --- .../src/entities/EntityTreeSendThread.cpp | 2 +- .../src/octree/OctreeSendThread.cpp | 3 -- .../src/octree/OctreeSendThread.h | 1 - interface/src/Application.cpp | 8 +++-- .../src/octree/OctreePacketProcessor.cpp | 34 +++++++++++++++++-- interface/src/octree/OctreePacketProcessor.h | 10 ++++++ libraries/octree/src/OctreeProcessor.cpp | 2 ++ libraries/octree/src/OctreeProcessor.h | 3 ++ 8 files changed, 52 insertions(+), 11 deletions(-) diff --git a/assignment-client/src/entities/EntityTreeSendThread.cpp b/assignment-client/src/entities/EntityTreeSendThread.cpp index 19dc92686a..154d22f253 100644 --- a/assignment-client/src/entities/EntityTreeSendThread.cpp +++ b/assignment-client/src/entities/EntityTreeSendThread.cpp @@ -174,7 +174,7 @@ bool EntityTreeSendThread::traverseTreeAndSendContents(SharedNodePointer node, O // Send EntityQueryInitialResultsComplete reliable packet ... auto initialCompletion = NLPacket::create(PacketType::EntityQueryInitialResultsComplete, -1, true); QDataStream initialCompletionStream(initialCompletion.get()); - initialCompletionStream << _lastSequenceNumber; + initialCompletionStream << OCTREE_PACKET_SEQUENCE(nodeData->getSequenceNumber() - 1U); DependencyManager::get()->sendPacket(std::move(initialCompletion), *node.data()); } diff --git a/assignment-client/src/octree/OctreeSendThread.cpp b/assignment-client/src/octree/OctreeSendThread.cpp index 54a7bbfa2d..ab357f4146 100644 --- a/assignment-client/src/octree/OctreeSendThread.cpp +++ b/assignment-client/src/octree/OctreeSendThread.cpp @@ -195,7 +195,6 @@ int OctreeSendThread::handlePacketSend(SharedNodePointer node, OctreeQueryNode* // actually send it OctreeServer::didCallWriteDatagram(this); DependencyManager::get()->sendUnreliablePacket(statsPacket, *node); - _lastSequenceNumber = (decltype(_lastSequenceNumber)) statsPacket.getSequenceNumber(); } else { // not enough room in the packet, send two packets @@ -232,7 +231,6 @@ int OctreeSendThread::handlePacketSend(SharedNodePointer node, OctreeQueryNode* // second packet OctreeServer::didCallWriteDatagram(this); DependencyManager::get()->sendUnreliablePacket(sentPacket, *node); - _lastSequenceNumber = (decltype(_lastSequenceNumber)) sentPacket.getSequenceNumber(); numBytes = sentPacket.getDataSize(); _totalBytes += numBytes; @@ -266,7 +264,6 @@ int OctreeSendThread::handlePacketSend(SharedNodePointer node, OctreeQueryNode* OctreeServer::didCallWriteDatagram(this); NLPacket& sentPacket = nodeData->getPacket(); DependencyManager::get()->sendUnreliablePacket(sentPacket, *node); - _lastSequenceNumber = (decltype(_lastSequenceNumber)) sentPacket.getSequenceNumber(); int numBytes = sentPacket.getDataSize(); _totalBytes += numBytes; diff --git a/assignment-client/src/octree/OctreeSendThread.h b/assignment-client/src/octree/OctreeSendThread.h index fc529c3e3a..bdf0f03364 100644 --- a/assignment-client/src/octree/OctreeSendThread.h +++ b/assignment-client/src/octree/OctreeSendThread.h @@ -60,7 +60,6 @@ protected: QWeakPointer _node; OctreeServer* _myServer { nullptr }; QUuid _nodeUuid; - udt::SequenceNumber::Type _lastSequenceNumber { 0 }; private: /// Called before a packetDistributor pass to allow for pre-distribution processing diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp index 94f89a66ad..ed67b7464c 100644 --- a/interface/src/Application.cpp +++ b/interface/src/Application.cpp @@ -5476,8 +5476,9 @@ void Application::update(float deltaTime) { // for nearby entities before starting bullet up. quint64 now = usecTimestampNow(); const int PHYSICS_CHECK_TIMEOUT = 2 * USECS_PER_SECOND; - - if (now - _lastPhysicsCheckTime > PHYSICS_CHECK_TIMEOUT || _fullSceneReceivedCounter > _fullSceneCounterAtLastPhysicsCheck) { + auto entityTreeRenderer = getEntities(); + if (entityTreeRenderer && _octreeProcessor.octreeSequenceIsComplete(entityTreeRenderer->getLastOctreeMessageSequence()) ) { + /*if (now - _lastPhysicsCheckTime > PHYSICS_CHECK_TIMEOUT || _fullSceneReceivedCounter > _fullSceneCounterAtLastPhysicsCheck) {*/ // we've received a new full-scene octree stats packet, or it's been long enough to try again anyway _lastPhysicsCheckTime = now; _fullSceneCounterAtLastPhysicsCheck = _fullSceneReceivedCounter; @@ -6134,7 +6135,7 @@ void Application::queryOctree(NodeType_t serverType, PacketType packetType) { auto lodManager = DependencyManager::get(); _octreeQuery.setOctreeSizeScale(lodManager->getOctreeSizeScale()); _octreeQuery.setBoundaryLevelAdjust(lodManager->getBoundaryLevelAdjust()); - _octreeQuery.setReportInitialCompletion(true); + _octreeQuery.setReportInitialCompletion(!_physicsEnabled); auto nodeList = DependencyManager::get(); @@ -6279,6 +6280,7 @@ void Application::clearDomainOctreeDetails() { _octreeServerSceneStats.clear(); }); + _octreeProcessor.resetCompletionSequenceNumber(); // reset the model renderer getEntities()->clear(); diff --git a/interface/src/octree/OctreePacketProcessor.cpp b/interface/src/octree/OctreePacketProcessor.cpp index 7d38e29710..5ab2218f67 100644 --- a/interface/src/octree/OctreePacketProcessor.cpp +++ b/interface/src/octree/OctreePacketProcessor.cpp @@ -21,9 +21,9 @@ OctreePacketProcessor::OctreePacketProcessor() { setObjectName("Octree Packet Processor"); auto& packetReceiver = DependencyManager::get()->getPacketReceiver(); - - packetReceiver.registerDirectListenerForTypes({ PacketType::OctreeStats, PacketType::EntityData, PacketType::EntityErase }, - this, "handleOctreePacket"); + const PacketReceiver::PacketTypeList octreePackets = + { PacketType::OctreeStats, PacketType::EntityData, PacketType::EntityErase, PacketType::EntityQueryInitialResultsComplete }; + packetReceiver.registerDirectListenerForTypes(octreePackets, this, "handleOctreePacket"); } void OctreePacketProcessor::handleOctreePacket(QSharedPointer message, SharedNodePointer senderNode) { @@ -111,8 +111,36 @@ void OctreePacketProcessor::processPacket(QSharedPointer messag } } break; + case PacketType::EntityQueryInitialResultsComplete: { + // Read sequence # + OCTREE_PACKET_SEQUENCE completionNumber; + message->readPrimitive(&completionNumber); + _completionSequenceNumber = completionNumber; + _completionSequenceNumberValid = true; + } break; + default: { // nothing to do } break; } } + +void OctreePacketProcessor::resetCompletionSequenceNumber() { + _completionSequenceNumber = false; +} + +namespace { + template constexpr bool lessThanWraparound(int a, int b) { + static const int MAX_T_VALUE = std::numeric_limits::max(); + if (b < a) { + b += MAX_T_VALUE; + } + return (b - a) < (MAX_T_VALUE / 2); + } +} + +bool OctreePacketProcessor::octreeSequenceIsComplete(int sequenceNumber) const { + const int completionSequenceNumber = _completionSequenceNumber; + return _completionSequenceNumberValid && + !lessThanWraparound(completionSequenceNumber, sequenceNumber); +} diff --git a/interface/src/octree/OctreePacketProcessor.h b/interface/src/octree/OctreePacketProcessor.h index d04cab3584..f7e001fbde 100644 --- a/interface/src/octree/OctreePacketProcessor.h +++ b/interface/src/octree/OctreePacketProcessor.h @@ -22,13 +22,23 @@ class OctreePacketProcessor : public ReceivedPacketProcessor { public: OctreePacketProcessor(); + bool octreeSequenceIsComplete(int sequenceNumber) const; + signals: void packetVersionMismatch(); +public slots: + void resetCompletionSequenceNumber(); + protected: virtual void processPacket(QSharedPointer message, SharedNodePointer sendingNode) override; private slots: void handleOctreePacket(QSharedPointer message, SharedNodePointer senderNode); + +private: + bool _completionSequenceNumberValid { false }; + std::atomic _completionSequenceNumber { 0 }; + }; #endif // hifi_OctreePacketProcessor_h diff --git a/libraries/octree/src/OctreeProcessor.cpp b/libraries/octree/src/OctreeProcessor.cpp index beaac1198c..206ff399d9 100644 --- a/libraries/octree/src/OctreeProcessor.cpp +++ b/libraries/octree/src/OctreeProcessor.cpp @@ -192,6 +192,8 @@ void OctreeProcessor::processDatagram(ReceivedMessage& message, SharedNodePointe _elementsInLastWindow = 0; _entitiesInLastWindow = 0; } + + _lastOctreeMessageSequence = sequence; } } diff --git a/libraries/octree/src/OctreeProcessor.h b/libraries/octree/src/OctreeProcessor.h index 325b33cd15..8085682fa5 100644 --- a/libraries/octree/src/OctreeProcessor.h +++ b/libraries/octree/src/OctreeProcessor.h @@ -56,6 +56,8 @@ public: float getAverageUncompressPerPacket() const { return _uncompressPerPacket.getAverage(); } float getAverageReadBitstreamPerPacket() const { return _readBitstreamPerPacket.getAverage(); } + OCTREE_PACKET_SEQUENCE getLastOctreeMessageSequence() const { return _lastOctreeMessageSequence; } + protected: virtual OctreePointer createTree() = 0; @@ -77,6 +79,7 @@ protected: int _packetsInLastWindow = 0; int _elementsInLastWindow = 0; int _entitiesInLastWindow = 0; + std::atomic _lastOctreeMessageSequence = 0; }; From 4d559bbaeeb59063515fc7c4c8ab60528d1acbfc Mon Sep 17 00:00:00 2001 From: Simon Walton Date: Fri, 27 Jul 2018 10:05:34 -0700 Subject: [PATCH 13/25] Don't initialize an atomic; squelch gcc warning --- libraries/octree/src/OctreeProcessor.h | 2 +- libraries/octree/src/OctreeQuery.cpp | 2 +- libraries/octree/src/OctreeQuery.h | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/libraries/octree/src/OctreeProcessor.h b/libraries/octree/src/OctreeProcessor.h index 8085682fa5..1bc3bd10f9 100644 --- a/libraries/octree/src/OctreeProcessor.h +++ b/libraries/octree/src/OctreeProcessor.h @@ -79,7 +79,7 @@ protected: int _packetsInLastWindow = 0; int _elementsInLastWindow = 0; int _entitiesInLastWindow = 0; - std::atomic _lastOctreeMessageSequence = 0; + std::atomic _lastOctreeMessageSequence; }; diff --git a/libraries/octree/src/OctreeQuery.cpp b/libraries/octree/src/OctreeQuery.cpp index 77977bf01e..8c3685dc69 100644 --- a/libraries/octree/src/OctreeQuery.cpp +++ b/libraries/octree/src/OctreeQuery.cpp @@ -27,7 +27,7 @@ OctreeQuery::OctreeQuery(bool randomizeConnectionID) { } } -const OctreeQuery::OctreeQueryFlags operator|=(OctreeQuery::OctreeQueryFlags& lhs, int rhs) { +OctreeQuery::OctreeQueryFlags operator|=(OctreeQuery::OctreeQueryFlags& lhs, int rhs) { return lhs = OctreeQuery::OctreeQueryFlags(lhs | rhs); } diff --git a/libraries/octree/src/OctreeQuery.h b/libraries/octree/src/OctreeQuery.h index 04d6793158..2c3c00ef05 100644 --- a/libraries/octree/src/OctreeQuery.h +++ b/libraries/octree/src/OctreeQuery.h @@ -79,7 +79,7 @@ protected: QReadWriteLock _jsonParametersLock; enum OctreeQueryFlags : uint16_t { NoFlags = 0x0, WantInitialCompletion = 0x1 }; - friend const OctreeQuery::OctreeQueryFlags operator|=(OctreeQuery::OctreeQueryFlags& lhs, const int rhs); + friend OctreeQuery::OctreeQueryFlags operator|=(OctreeQuery::OctreeQueryFlags& lhs, const int rhs); bool _hasReceivedFirstQuery { false }; bool _reportInitialCompletion { false }; From 3d390203e20fa88f73d4a2a35132177e0fae6b48 Mon Sep 17 00:00:00 2001 From: Simon Walton Date: Fri, 27 Jul 2018 11:34:08 -0700 Subject: [PATCH 14/25] More fixes for clang & gcc C++14 issues, I think. --- interface/src/Application.cpp | 2 +- interface/src/octree/OctreePacketProcessor.cpp | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp index ed67b7464c..1c4de2165a 100644 --- a/interface/src/Application.cpp +++ b/interface/src/Application.cpp @@ -5475,7 +5475,7 @@ void Application::update(float deltaTime) { // we haven't yet enabled physics. we wait until we think we have all the collision information // for nearby entities before starting bullet up. quint64 now = usecTimestampNow(); - const int PHYSICS_CHECK_TIMEOUT = 2 * USECS_PER_SECOND; + //const int PHYSICS_CHECK_TIMEOUT = 2 * USECS_PER_SECOND; auto entityTreeRenderer = getEntities(); if (entityTreeRenderer && _octreeProcessor.octreeSequenceIsComplete(entityTreeRenderer->getLastOctreeMessageSequence()) ) { /*if (now - _lastPhysicsCheckTime > PHYSICS_CHECK_TIMEOUT || _fullSceneReceivedCounter > _fullSceneCounterAtLastPhysicsCheck) {*/ diff --git a/interface/src/octree/OctreePacketProcessor.cpp b/interface/src/octree/OctreePacketProcessor.cpp index 5ab2218f67..696fa4ffd4 100644 --- a/interface/src/octree/OctreePacketProcessor.cpp +++ b/interface/src/octree/OctreePacketProcessor.cpp @@ -130,8 +130,8 @@ void OctreePacketProcessor::resetCompletionSequenceNumber() { } namespace { - template constexpr bool lessThanWraparound(int a, int b) { - static const int MAX_T_VALUE = std::numeric_limits::max(); + template bool lessThanWraparound(int a, int b) { + constexpr int MAX_T_VALUE = std::numeric_limits::max(); if (b < a) { b += MAX_T_VALUE; } From 5315b020ec2ec2087c7484b882a77a3610c9ff55 Mon Sep 17 00:00:00 2001 From: Simon Walton Date: Fri, 27 Jul 2018 14:51:24 -0700 Subject: [PATCH 15/25] Use locking for the OctreePacketProcessor additions --- interface/src/octree/OctreePacketProcessor.cpp | 8 ++++++-- interface/src/octree/OctreePacketProcessor.h | 4 +++- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/interface/src/octree/OctreePacketProcessor.cpp b/interface/src/octree/OctreePacketProcessor.cpp index 696fa4ffd4..62dcd056e9 100644 --- a/interface/src/octree/OctreePacketProcessor.cpp +++ b/interface/src/octree/OctreePacketProcessor.cpp @@ -115,6 +115,8 @@ void OctreePacketProcessor::processPacket(QSharedPointer messag // Read sequence # OCTREE_PACKET_SEQUENCE completionNumber; message->readPrimitive(&completionNumber); + + Locker lock(_completionMutex); _completionSequenceNumber = completionNumber; _completionSequenceNumberValid = true; } break; @@ -126,6 +128,7 @@ void OctreePacketProcessor::processPacket(QSharedPointer messag } void OctreePacketProcessor::resetCompletionSequenceNumber() { + Locker lock(_completionMutex); _completionSequenceNumber = false; } @@ -140,7 +143,8 @@ namespace { } bool OctreePacketProcessor::octreeSequenceIsComplete(int sequenceNumber) const { - const int completionSequenceNumber = _completionSequenceNumber; + Locker lock(_completionMutex); + // If we've received the flagged seq # and the current one is >= it. return _completionSequenceNumberValid && - !lessThanWraparound(completionSequenceNumber, sequenceNumber); + !lessThanWraparound(_completionSequenceNumber, sequenceNumber); } diff --git a/interface/src/octree/OctreePacketProcessor.h b/interface/src/octree/OctreePacketProcessor.h index f7e001fbde..ad1ab6c36c 100644 --- a/interface/src/octree/OctreePacketProcessor.h +++ b/interface/src/octree/OctreePacketProcessor.h @@ -37,8 +37,10 @@ private slots: void handleOctreePacket(QSharedPointer message, SharedNodePointer senderNode); private: + mutable std::mutex _completionMutex; + using Locker = std::lock_guard; bool _completionSequenceNumberValid { false }; - std::atomic _completionSequenceNumber { 0 }; + int _completionSequenceNumber { 0 }; }; #endif // hifi_OctreePacketProcessor_h From 2ac6ca3291015d0b5ebdcc676ada0bf9413f31bc Mon Sep 17 00:00:00 2001 From: Simon Walton Date: Fri, 27 Jul 2018 17:32:35 -0700 Subject: [PATCH 16/25] Remove old physics-enabling code --- interface/src/Application.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp index 1c4de2165a..6c5efee807 100644 --- a/interface/src/Application.cpp +++ b/interface/src/Application.cpp @@ -5475,10 +5475,9 @@ void Application::update(float deltaTime) { // we haven't yet enabled physics. we wait until we think we have all the collision information // for nearby entities before starting bullet up. quint64 now = usecTimestampNow(); - //const int PHYSICS_CHECK_TIMEOUT = 2 * USECS_PER_SECOND; + // Check for flagged EntityData having arrived. auto entityTreeRenderer = getEntities(); if (entityTreeRenderer && _octreeProcessor.octreeSequenceIsComplete(entityTreeRenderer->getLastOctreeMessageSequence()) ) { - /*if (now - _lastPhysicsCheckTime > PHYSICS_CHECK_TIMEOUT || _fullSceneReceivedCounter > _fullSceneCounterAtLastPhysicsCheck) {*/ // we've received a new full-scene octree stats packet, or it's been long enough to try again anyway _lastPhysicsCheckTime = now; _fullSceneCounterAtLastPhysicsCheck = _fullSceneReceivedCounter; From 3d1fe7da9f09e86118e844c6c33d0b38c5e4506e Mon Sep 17 00:00:00 2001 From: Simon Walton Date: Fri, 27 Jul 2018 18:22:36 -0700 Subject: [PATCH 17/25] Reset known entity-state when leaving initial send state --- assignment-client/src/entities/EntityTreeSendThread.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/assignment-client/src/entities/EntityTreeSendThread.cpp b/assignment-client/src/entities/EntityTreeSendThread.cpp index 154d22f253..6079f03c26 100644 --- a/assignment-client/src/entities/EntityTreeSendThread.cpp +++ b/assignment-client/src/entities/EntityTreeSendThread.cpp @@ -170,6 +170,7 @@ bool EntityTreeSendThread::traverseTreeAndSendContents(SharedNodePointer node, O if (sendComplete && nodeData->wantReportInitialCompletion() && _traversal.finished()) { // Dealt with all nearby entities. nodeData->setReportInitialCompletion(false); + resetState(); // Send EntityQueryInitialResultsComplete reliable packet ... auto initialCompletion = NLPacket::create(PacketType::EntityQueryInitialResultsComplete, -1, true); From be1bbc07c26de990ea3040a0a77269c5f7afec4b Mon Sep 17 00:00:00 2001 From: Simon Walton Date: Mon, 30 Jul 2018 10:20:55 -0700 Subject: [PATCH 18/25] Don't wait for (nonexistant) ES when in serverless mode --- interface/src/Application.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp index 6c5efee807..0037c9500f 100644 --- a/interface/src/Application.cpp +++ b/interface/src/Application.cpp @@ -5477,7 +5477,8 @@ void Application::update(float deltaTime) { quint64 now = usecTimestampNow(); // Check for flagged EntityData having arrived. auto entityTreeRenderer = getEntities(); - if (entityTreeRenderer && _octreeProcessor.octreeSequenceIsComplete(entityTreeRenderer->getLastOctreeMessageSequence()) ) { + if (isServerlessMode() || + (entityTreeRenderer && _octreeProcessor.octreeSequenceIsComplete(entityTreeRenderer->getLastOctreeMessageSequence()) )) { // we've received a new full-scene octree stats packet, or it's been long enough to try again anyway _lastPhysicsCheckTime = now; _fullSceneCounterAtLastPhysicsCheck = _fullSceneReceivedCounter; From 8c512ec19bd8c13443faa388f48e0ec5e9781d23 Mon Sep 17 00:00:00 2001 From: Simon Walton Date: Mon, 30 Jul 2018 18:03:45 -0700 Subject: [PATCH 19/25] Send spherical view from Interface rather than creating in ES --- .../src/entities/EntityTreeSendThread.cpp | 12 +--------- .../src/entities/EntityTreeSendThread.h | 2 -- interface/src/Application.cpp | 22 ++++++++++++++----- 3 files changed, 18 insertions(+), 18 deletions(-) diff --git a/assignment-client/src/entities/EntityTreeSendThread.cpp b/assignment-client/src/entities/EntityTreeSendThread.cpp index 6079f03c26..2e777ae0f1 100644 --- a/assignment-client/src/entities/EntityTreeSendThread.cpp +++ b/assignment-client/src/entities/EntityTreeSendThread.cpp @@ -17,9 +17,6 @@ #include "EntityServer.h" -// Initially just send all items within this distance. -const float EntityTreeSendThread::INITIAL_RADIUS = 10.0f; - EntityTreeSendThread::EntityTreeSendThread(OctreeServer* myServer, const SharedNodePointer& node) : OctreeSendThread(myServer, node) { @@ -113,14 +110,7 @@ bool EntityTreeSendThread::traverseTreeAndSendContents(SharedNodePointer node, O int32_t lodLevelOffset = nodeData->getBoundaryLevelAdjust() + (viewFrustumChanged ? LOW_RES_MOVING_ADJUST : NO_BOUNDARY_ADJUST); newView.lodScaleFactor = powf(2.0f, lodLevelOffset); - - if (nodeData->wantReportInitialCompletion() && !newView.viewFrustums.empty()) { - auto& mainView = newView.viewFrustums[0]; - // Force acceptance within INITIAL_RADIUS. - mainView.setSimpleRadius(INITIAL_RADIUS); - newView.lodScaleFactor = 0.0f; - } - + startNewTraversal(newView, root); // When the viewFrustum changed the sort order may be incorrect, so we re-sort diff --git a/assignment-client/src/entities/EntityTreeSendThread.h b/assignment-client/src/entities/EntityTreeSendThread.h index c9f4d06164..199769ca09 100644 --- a/assignment-client/src/entities/EntityTreeSendThread.h +++ b/assignment-client/src/entities/EntityTreeSendThread.h @@ -58,8 +58,6 @@ private: int32_t _numEntitiesOffset { 0 }; uint16_t _numEntities { 0 }; - const static float INITIAL_RADIUS; - private slots: void editingEntityPointer(const EntityItemPointer& entity); void deletingEntityPointer(EntityItem* entity); diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp index 0037c9500f..bbb8dc8161 100644 --- a/interface/src/Application.cpp +++ b/interface/src/Application.cpp @@ -374,6 +374,7 @@ static const int THROTTLED_SIM_FRAME_PERIOD_MS = MSECS_PER_SECOND / THROTTLED_SI static const uint32_t INVALID_FRAME = UINT32_MAX; static const float PHYSICS_READY_RANGE = 3.0f; // how far from avatar to check for entities that aren't ready for simulation +static const float INITIAL_QUERY_RADIUS = 10.0f; // priority radius for entities before physics enabled static const QString DESKTOP_LOCATION = QStandardPaths::writableLocation(QStandardPaths::DesktopLocation); @@ -6130,12 +6131,23 @@ void Application::queryOctree(NodeType_t serverType, PacketType packetType) { return; // bail early if settings are not loaded } - _octreeQuery.setConicalViews(_conicalViews); + const bool isModifiedQuery = !_physicsEnabled; + if (isModifiedQuery) { + // Create modified view that is a simple sphere. + ConicalViewFrustum sphericalView; + sphericalView.setSimpleRadius(INITIAL_QUERY_RADIUS); + _octreeQuery.setConicalViews({ sphericalView }); + _octreeQuery.setOctreeSizeScale(DEFAULT_OCTREE_SIZE_SCALE); + static constexpr float MIN_LOD_ADJUST = -20.0f; + _octreeQuery.setBoundaryLevelAdjust(MIN_LOD_ADJUST); + } else { + _octreeQuery.setConicalViews(_conicalViews); + auto lodManager = DependencyManager::get(); + _octreeQuery.setOctreeSizeScale(lodManager->getOctreeSizeScale()); + _octreeQuery.setBoundaryLevelAdjust(lodManager->getBoundaryLevelAdjust()); + } + _octreeQuery.setReportInitialCompletion(isModifiedQuery); - auto lodManager = DependencyManager::get(); - _octreeQuery.setOctreeSizeScale(lodManager->getOctreeSizeScale()); - _octreeQuery.setBoundaryLevelAdjust(lodManager->getBoundaryLevelAdjust()); - _octreeQuery.setReportInitialCompletion(!_physicsEnabled); auto nodeList = DependencyManager::get(); From a320308eaf7a8b1a9f94a962c2200e7a2e65c859 Mon Sep 17 00:00:00 2001 From: Simon Walton Date: Tue, 31 Jul 2018 13:54:22 -0700 Subject: [PATCH 20/25] Fix sequence number encoding issue Don't mix QDataStream and readPrimitive. Also fix logic issues in completion check; move new packet type; don't reset known sent entities upon initial completion; other reviewer recommendations. --- .../src/entities/EntityTreeSendThread.cpp | 7 +++---- interface/src/Application.cpp | 1 + interface/src/octree/OctreePacketProcessor.cpp | 4 ++-- libraries/networking/src/udt/PacketHeaders.cpp | 2 +- libraries/networking/src/udt/PacketHeaders.h | 2 +- tools/dissectors/1-hfudt.lua | 10 +++++----- 6 files changed, 13 insertions(+), 13 deletions(-) diff --git a/assignment-client/src/entities/EntityTreeSendThread.cpp b/assignment-client/src/entities/EntityTreeSendThread.cpp index 2e777ae0f1..7e4ccc88cd 100644 --- a/assignment-client/src/entities/EntityTreeSendThread.cpp +++ b/assignment-client/src/entities/EntityTreeSendThread.cpp @@ -160,12 +160,11 @@ bool EntityTreeSendThread::traverseTreeAndSendContents(SharedNodePointer node, O if (sendComplete && nodeData->wantReportInitialCompletion() && _traversal.finished()) { // Dealt with all nearby entities. nodeData->setReportInitialCompletion(false); - resetState(); // Send EntityQueryInitialResultsComplete reliable packet ... - auto initialCompletion = NLPacket::create(PacketType::EntityQueryInitialResultsComplete, -1, true); - QDataStream initialCompletionStream(initialCompletion.get()); - initialCompletionStream << OCTREE_PACKET_SEQUENCE(nodeData->getSequenceNumber() - 1U); + auto initialCompletion = NLPacket::create(PacketType::EntityQueryInitialResultsComplete, + sizeof(OCTREE_PACKET_SEQUENCE), true); + initialCompletion->writePrimitive(OCTREE_PACKET_SEQUENCE(nodeData->getSequenceNumber() - 1U)); DependencyManager::get()->sendPacket(std::move(initialCompletion), *node.data()); } diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp index bbb8dc8161..3969e7617e 100644 --- a/interface/src/Application.cpp +++ b/interface/src/Application.cpp @@ -5483,6 +5483,7 @@ void Application::update(float deltaTime) { // we've received a new full-scene octree stats packet, or it's been long enough to try again anyway _lastPhysicsCheckTime = now; _fullSceneCounterAtLastPhysicsCheck = _fullSceneReceivedCounter; + _lastQueriedViews.clear(); // Force new view. // process octree stats packets are sent in between full sends of a scene (this isn't currently true). // We keep physics disabled until we've received a full scene and everything near the avatar in that diff --git a/interface/src/octree/OctreePacketProcessor.cpp b/interface/src/octree/OctreePacketProcessor.cpp index 62dcd056e9..76f62bcc4e 100644 --- a/interface/src/octree/OctreePacketProcessor.cpp +++ b/interface/src/octree/OctreePacketProcessor.cpp @@ -135,7 +135,7 @@ void OctreePacketProcessor::resetCompletionSequenceNumber() { namespace { template bool lessThanWraparound(int a, int b) { constexpr int MAX_T_VALUE = std::numeric_limits::max(); - if (b < a) { + if (b <= a) { b += MAX_T_VALUE; } return (b - a) < (MAX_T_VALUE / 2); @@ -146,5 +146,5 @@ bool OctreePacketProcessor::octreeSequenceIsComplete(int sequenceNumber) const { Locker lock(_completionMutex); // If we've received the flagged seq # and the current one is >= it. return _completionSequenceNumberValid && - !lessThanWraparound(_completionSequenceNumber, sequenceNumber); + !lessThanWraparound(sequenceNumber, _completionSequenceNumber); } diff --git a/libraries/networking/src/udt/PacketHeaders.cpp b/libraries/networking/src/udt/PacketHeaders.cpp index bb9666ee37..13ffcb5120 100644 --- a/libraries/networking/src/udt/PacketHeaders.cpp +++ b/libraries/networking/src/udt/PacketHeaders.cpp @@ -95,7 +95,7 @@ PacketVersion versionForPacketType(PacketType packetType) { case PacketType::AvatarIdentityRequest: return 22; default: - return 22; + return 21; } } diff --git a/libraries/networking/src/udt/PacketHeaders.h b/libraries/networking/src/udt/PacketHeaders.h index 7cba3baaf0..073f3f7c42 100644 --- a/libraries/networking/src/udt/PacketHeaders.h +++ b/libraries/networking/src/udt/PacketHeaders.h @@ -126,13 +126,13 @@ public: EntityScriptCallMethod, ChallengeOwnershipRequest, ChallengeOwnershipReply, - EntityQueryInitialResultsComplete, OctreeDataFileRequest, OctreeDataFileReply, OctreeDataPersist, EntityClone, + EntityQueryInitialResultsComplete, NUM_PACKET_TYPE }; diff --git a/tools/dissectors/1-hfudt.lua b/tools/dissectors/1-hfudt.lua index 9bed892885..26494bb515 100644 --- a/tools/dissectors/1-hfudt.lua +++ b/tools/dissectors/1-hfudt.lua @@ -156,11 +156,11 @@ local packet_types = { [92] = "EntityScriptCallMethod", [93] = "ChallengeOwnershipRequest", [94] = "ChallengeOwnershipReply", - [95] = "EntityQueryInitialResultsComplete", - [96] = "OctreeDataFileRequest", - [97] = "OctreeDataFileReply", - [98] = "OctreeDataPersist", - [99] = "EntityClone" + [95] = "OctreeDataFileRequest", + [96] = "OctreeDataFileReply", + [97] = "OctreeDataPersist", + [98] = "EntityClone", + [99] = "EntityQueryInitialResultsComplete" } local unsourced_packet_types = { From ed75fe673e82b1efdc15356c6ba714bfee0a9b4a Mon Sep 17 00:00:00 2001 From: Simon Walton Date: Tue, 31 Jul 2018 18:28:14 -0700 Subject: [PATCH 21/25] Replace int+bool with single atomic int --- interface/src/octree/OctreePacketProcessor.cpp | 8 ++------ interface/src/octree/OctreePacketProcessor.h | 5 +---- 2 files changed, 3 insertions(+), 10 deletions(-) diff --git a/interface/src/octree/OctreePacketProcessor.cpp b/interface/src/octree/OctreePacketProcessor.cpp index 76f62bcc4e..9196699473 100644 --- a/interface/src/octree/OctreePacketProcessor.cpp +++ b/interface/src/octree/OctreePacketProcessor.cpp @@ -116,9 +116,7 @@ void OctreePacketProcessor::processPacket(QSharedPointer messag OCTREE_PACKET_SEQUENCE completionNumber; message->readPrimitive(&completionNumber); - Locker lock(_completionMutex); _completionSequenceNumber = completionNumber; - _completionSequenceNumberValid = true; } break; default: { @@ -128,8 +126,7 @@ void OctreePacketProcessor::processPacket(QSharedPointer messag } void OctreePacketProcessor::resetCompletionSequenceNumber() { - Locker lock(_completionMutex); - _completionSequenceNumber = false; + _completionSequenceNumber = -1; } namespace { @@ -143,8 +140,7 @@ namespace { } bool OctreePacketProcessor::octreeSequenceIsComplete(int sequenceNumber) const { - Locker lock(_completionMutex); // If we've received the flagged seq # and the current one is >= it. - return _completionSequenceNumberValid && + return _completionSequenceNumber != -1 && !lessThanWraparound(sequenceNumber, _completionSequenceNumber); } diff --git a/interface/src/octree/OctreePacketProcessor.h b/interface/src/octree/OctreePacketProcessor.h index ad1ab6c36c..9c5eaca8ec 100644 --- a/interface/src/octree/OctreePacketProcessor.h +++ b/interface/src/octree/OctreePacketProcessor.h @@ -37,10 +37,7 @@ private slots: void handleOctreePacket(QSharedPointer message, SharedNodePointer senderNode); private: - mutable std::mutex _completionMutex; - using Locker = std::lock_guard; - bool _completionSequenceNumberValid { false }; - int _completionSequenceNumber { 0 }; + std::atomic _completionSequenceNumber { -1 }; }; #endif // hifi_OctreePacketProcessor_h From b18c1f0e1eed058b9bc30dc36588c82e25334bcb Mon Sep 17 00:00:00 2001 From: Simon Walton Date: Wed, 1 Aug 2018 10:10:45 -0700 Subject: [PATCH 22/25] Use defined value for invalid sequence number --- interface/src/octree/OctreePacketProcessor.cpp | 4 ++-- interface/src/octree/OctreePacketProcessor.h | 3 ++- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/interface/src/octree/OctreePacketProcessor.cpp b/interface/src/octree/OctreePacketProcessor.cpp index 9196699473..063b07f699 100644 --- a/interface/src/octree/OctreePacketProcessor.cpp +++ b/interface/src/octree/OctreePacketProcessor.cpp @@ -126,7 +126,7 @@ void OctreePacketProcessor::processPacket(QSharedPointer messag } void OctreePacketProcessor::resetCompletionSequenceNumber() { - _completionSequenceNumber = -1; + _completionSequenceNumber = INVALID_SEQUENCE; } namespace { @@ -141,6 +141,6 @@ namespace { bool OctreePacketProcessor::octreeSequenceIsComplete(int sequenceNumber) const { // If we've received the flagged seq # and the current one is >= it. - return _completionSequenceNumber != -1 && + return _completionSequenceNumber != INVALID_SEQUENCE && !lessThanWraparound(sequenceNumber, _completionSequenceNumber); } diff --git a/interface/src/octree/OctreePacketProcessor.h b/interface/src/octree/OctreePacketProcessor.h index 9c5eaca8ec..edba48a238 100644 --- a/interface/src/octree/OctreePacketProcessor.h +++ b/interface/src/octree/OctreePacketProcessor.h @@ -37,7 +37,8 @@ private slots: void handleOctreePacket(QSharedPointer message, SharedNodePointer senderNode); private: - std::atomic _completionSequenceNumber { -1 }; + static constexpr int INVALID_SEQUENCE = -1; + std::atomic _completionSequenceNumber { INVALID_SEQUENCE }; }; #endif // hifi_OctreePacketProcessor_h From 9d42a6e08fbbbb9eab4ba52335621eac2beefa02 Mon Sep 17 00:00:00 2001 From: Simon Walton Date: Wed, 1 Aug 2018 10:37:28 -0700 Subject: [PATCH 23/25] Change method name per coding standards --- assignment-client/src/AssignmentClientMonitor.cpp | 4 ++-- assignment-client/src/AssignmentClientMonitor.h | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/assignment-client/src/AssignmentClientMonitor.cpp b/assignment-client/src/AssignmentClientMonitor.cpp index eb764a128c..330023dae0 100644 --- a/assignment-client/src/AssignmentClientMonitor.cpp +++ b/assignment-client/src/AssignmentClientMonitor.cpp @@ -74,7 +74,7 @@ AssignmentClientMonitor::AssignmentClientMonitor(const unsigned int numAssignmen auto& packetReceiver = DependencyManager::get()->getPacketReceiver(); packetReceiver.registerListener(PacketType::AssignmentClientStatus, this, "handleChildStatusPacket"); - adjustOsResources(std::max(_numAssignmentClientForks, _maxAssignmentClientForks)); + adjustOSResources(std::max(_numAssignmentClientForks, _maxAssignmentClientForks)); // use QProcess to fork off a process for each of the child assignment clients for (unsigned int i = 0; i < _numAssignmentClientForks; i++) { spawnChildClient(); @@ -377,7 +377,7 @@ bool AssignmentClientMonitor::handleHTTPRequest(HTTPConnection* connection, cons return true; } -void AssignmentClientMonitor::adjustOsResources(unsigned int numForks) const +void AssignmentClientMonitor::adjustOSResources(unsigned int numForks) const { #ifdef _POSIX_SOURCE // QProcess on Unix uses six (I think) descriptors, some temporarily, for each child proc. diff --git a/assignment-client/src/AssignmentClientMonitor.h b/assignment-client/src/AssignmentClientMonitor.h index 659ff4b001..5e32c50e0d 100644 --- a/assignment-client/src/AssignmentClientMonitor.h +++ b/assignment-client/src/AssignmentClientMonitor.h @@ -55,7 +55,7 @@ public slots: private: void spawnChildClient(); void simultaneousWaitOnChildren(int waitMsecs); - void adjustOsResources(unsigned int numForks) const; + void adjustOSResources(unsigned int numForks) const; QTimer _checkSparesTimer; // every few seconds see if it need fewer or more spare children From 67528cb64636c70bcef7d8ab09471b2019b06ab5 Mon Sep 17 00:00:00 2001 From: Zach Fox Date: Wed, 1 Aug 2018 15:34:53 -0700 Subject: [PATCH 24/25] Fix MS17215: Auto-show Confirmed txns in Recent Activity when many Pending txns present --- .../qml/hifi/commerce/wallet/WalletHome.qml | 50 +++++++++++++++---- .../qml/hifi/models/PSFListModel.qml | 33 ++++++++++++ 2 files changed, 72 insertions(+), 11 deletions(-) diff --git a/interface/resources/qml/hifi/commerce/wallet/WalletHome.qml b/interface/resources/qml/hifi/commerce/wallet/WalletHome.qml index a0c6057b3b..50208793fe 100644 --- a/interface/resources/qml/hifi/commerce/wallet/WalletHome.qml +++ b/interface/resources/qml/hifi/commerce/wallet/WalletHome.qml @@ -142,7 +142,7 @@ Item { Timer { id: refreshTimer; - interval: 4000; + interval: 6000; onTriggered: { if (transactionHistory.atYBeginning) { console.log("Refreshing 1st Page of Recent Activity..."); @@ -211,6 +211,7 @@ Item { HifiModels.PSFListModel { id: transactionHistoryModel; + property int lastPendingCount: 0; listModelName: "transaction history"; // For debugging. Alternatively, we could specify endpoint for that purpose, even though it's not used directly. listView: transactionHistory; itemsPerPage: 6; @@ -221,8 +222,26 @@ Item { processPage: function (data) { console.debug('processPage', transactionHistoryModel.listModelName, JSON.stringify(data)); var result, pending; // Set up or get the accumulator for pending. - if (transactionHistoryModel.currentPageToRetrieve == 1) { - pending = {transaction_type: "pendingCount", count: 0}; + if (transactionHistoryModel.currentPageToRetrieve === 1) { + // The initial data elements inside the ListModel MUST contain all keys + // that will be used in future data. + pending = { + transaction_type: "pendingCount", + count: 0, + created_at: 0, + hfc_text: "", + id: "", + message: "", + place_name: "", + received_certs: 0, + received_money: 0, + recipient_name: "", + sender_name: "", + sent_certs: 0, + sent_money: 0, + status: "", + transaction_text: "" + }; result = [pending]; } else { pending = transactionHistoryModel.get(0); @@ -239,6 +258,15 @@ Item { } }); + if (lastPendingCount === 0) { + lastPendingCount = pending.count; + } else { + if (lastPendingCount !== pending.count) { + transactionHistoryModel.getNextPageIfNotEnoughVerticalResults(); + } + lastPendingCount = pending.count; + } + // Only auto-refresh if the user hasn't scrolled // and there is more data to grab if (transactionHistory.atYBeginning && data.history.length) { @@ -257,13 +285,13 @@ Item { ListView { id: transactionHistory; ScrollBar.vertical: ScrollBar { - policy: transactionHistory.contentHeight > parent.parent.height ? ScrollBar.AlwaysOn : ScrollBar.AsNeeded; - parent: transactionHistory.parent; - anchors.top: transactionHistory.top; - anchors.left: transactionHistory.right; - anchors.leftMargin: 4; - anchors.bottom: transactionHistory.bottom; - width: 20; + policy: transactionHistory.contentHeight > parent.parent.height ? ScrollBar.AlwaysOn : ScrollBar.AsNeeded; + parent: transactionHistory.parent; + anchors.top: transactionHistory.top; + anchors.left: transactionHistory.right; + anchors.leftMargin: 4; + anchors.bottom: transactionHistory.bottom; + width: 20; } anchors.centerIn: parent; width: parent.width - 12; @@ -340,7 +368,7 @@ Item { } HifiControlsUit.Separator { - colorScheme: 1; + colorScheme: 1; anchors.left: parent.left; anchors.right: parent.right; anchors.bottom: parent.bottom; diff --git a/interface/resources/qml/hifi/models/PSFListModel.qml b/interface/resources/qml/hifi/models/PSFListModel.qml index 542145904f..ad9fbcc8ef 100644 --- a/interface/resources/qml/hifi/models/PSFListModel.qml +++ b/interface/resources/qml/hifi/models/PSFListModel.qml @@ -38,6 +38,16 @@ ListModel { onSearchFilterChanged: if (initialized) { getFirstPage('delayClear'); } onTagsFilterChanged: if (initialized) { getFirstPage('delayClear'); } + // When considering a value for `itemsPerPage` in YOUR model, consider the following: + // - If your ListView delegates are of variable width/height, ensure you select + // an `itemsPerPage` value that would be sufficient to show one full page of data + // if all of the delegates were at their minimum heights. + // - If your first ListView delegate contains some special data (as in WalletHome's + // "Recent Activity" view), beware that your `itemsPerPage` value may _never_ reasonably be + // high enough such that the first page of data causes the view to be one-screen in height + // after retrieving the first page. This means data will automatically pop-in (after a short delay) + // until the combined heights of your View's delegates reach one-screen in height OR there is + // no more data to retrieve. See "needsMoreVerticalResults()" below. property int itemsPerPage: 100; // State. @@ -81,12 +91,35 @@ ListModel { function getNextPageIfVerticalScroll() { if (needsEarlyYFetch()) { getNextPage(); } } + function needsMoreHorizontalResults() { + return flickable + && currentPageToRetrieve > 0 + && flickable.contentWidth < flickable.width; + } + function needsMoreVerticalResults() { + return flickable + && currentPageToRetrieve > 0 + && flickable.contentHeight < flickable.height; + } + function getNextPageIfNotEnoughHorizontalResults() { + if (needsMoreHorizontalResults()) { + getNextPage(); + } + } + function getNextPageIfNotEnoughVerticalResults() { + if (needsMoreVerticalResults()) { + getNextPage(); + } + } + Component.onCompleted: { initialized = true; if (flickable && pageAhead > 0.0) { // Pun: Scrollers are usually one direction or another, such that only one of the following will actually fire. flickable.contentXChanged.connect(getNextPageIfHorizontalScroll); flickable.contentYChanged.connect(getNextPageIfVerticalScroll); + flickable.contentWidthChanged.connect(getNextPageIfNotEnoughHorizontalResults); + flickable.contentHeightChanged.connect(getNextPageIfNotEnoughVerticalResults); } } From 6f1487bbf8f779f5876c812e2543001eed228c56 Mon Sep 17 00:00:00 2001 From: Simon Walton Date: Wed, 1 Aug 2018 17:11:26 -0700 Subject: [PATCH 25/25] Don't explicitly use data method of shared node pointer --- assignment-client/src/entities/EntityTreeSendThread.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/assignment-client/src/entities/EntityTreeSendThread.cpp b/assignment-client/src/entities/EntityTreeSendThread.cpp index 7e4ccc88cd..f7ca05fbf2 100644 --- a/assignment-client/src/entities/EntityTreeSendThread.cpp +++ b/assignment-client/src/entities/EntityTreeSendThread.cpp @@ -165,7 +165,7 @@ bool EntityTreeSendThread::traverseTreeAndSendContents(SharedNodePointer node, O auto initialCompletion = NLPacket::create(PacketType::EntityQueryInitialResultsComplete, sizeof(OCTREE_PACKET_SEQUENCE), true); initialCompletion->writePrimitive(OCTREE_PACKET_SEQUENCE(nodeData->getSequenceNumber() - 1U)); - DependencyManager::get()->sendPacket(std::move(initialCompletion), *node.data()); + DependencyManager::get()->sendPacket(std::move(initialCompletion), *node); } return sendComplete;