From 33378ede5ccabcbfc044d3cf6a88c2812b6385b8 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Tue, 24 Jan 2017 16:34:57 -0800 Subject: [PATCH 1/6] force full scene send if json parameters changed --- assignment-client/src/octree/OctreeSendThread.cpp | 5 +++-- libraries/entities/src/EntityTreeElement.cpp | 7 ++++--- libraries/octree/src/OctreeQueryNode.cpp | 12 ++++++++++++ libraries/octree/src/OctreeQueryNode.h | 5 +++++ 4 files changed, 24 insertions(+), 5 deletions(-) diff --git a/assignment-client/src/octree/OctreeSendThread.cpp b/assignment-client/src/octree/OctreeSendThread.cpp index 7e88e47da8..afc17d71aa 100644 --- a/assignment-client/src/octree/OctreeSendThread.cpp +++ b/assignment-client/src/octree/OctreeSendThread.cpp @@ -316,8 +316,9 @@ int OctreeSendThread::packetDistributor(SharedNodePointer node, OctreeQueryNode* int truePacketsSent = 0; int trueBytesSent = 0; int packetsSentThisInterval = 0; - bool isFullScene = nodeData->getUsesFrustum() && - ((!viewFrustumChanged && nodeData->getViewFrustumJustStoppedChanging()) || nodeData->hasLodChanged()); + bool isFullScene = nodeData->haveJSONParametersChanged() || + (nodeData->getUsesFrustum() + && ((!viewFrustumChanged && nodeData->getViewFrustumJustStoppedChanging()) || nodeData->hasLodChanged())); bool somethingToSend = true; // assume we have something diff --git a/libraries/entities/src/EntityTreeElement.cpp b/libraries/entities/src/EntityTreeElement.cpp index 4cd82f223d..51d6d2b270 100644 --- a/libraries/entities/src/EntityTreeElement.cpp +++ b/libraries/entities/src/EntityTreeElement.cpp @@ -302,14 +302,15 @@ OctreeElement::AppendState EntityTreeElement::appendElementData(OctreePacketData includeThisEntity = false; } - if (!jsonFilters.isEmpty()) { + // if this entity has been updated since our last full send and there are json filters, check them + if (includeThisEntity && !jsonFilters.isEmpty()) { // if params include JSON filters, check if this entity matches bool entityMatchesFilters = entity->matchesJSONFilters(jsonFilters); if (entityMatchesFilters) { // we should include this entity unless it has already been excluded - includeThisEntity = includeThisEntity && true; + includeThisEntity = true; // make sure this entity is in the set of entities sent last frame entityNodeData->insertEntitySentLastFrame(entity->getID()); @@ -317,7 +318,7 @@ OctreeElement::AppendState EntityTreeElement::appendElementData(OctreePacketData } else { // we might include this entity if it matched in the previous frame if (entityNodeData->sentEntityLastFrame(entity->getID())) { - includeThisEntity = includeThisEntity && true; + includeThisEntity = true; entityNodeData->removeEntitySentLastFrame(entity->getID()); } else { diff --git a/libraries/octree/src/OctreeQueryNode.cpp b/libraries/octree/src/OctreeQueryNode.cpp index fe0c6c2a1f..cffba0bef4 100644 --- a/libraries/octree/src/OctreeQueryNode.cpp +++ b/libraries/octree/src/OctreeQueryNode.cpp @@ -320,3 +320,15 @@ void OctreeQueryNode::parseNackPacket(ReceivedMessage& message) { _nackedSequenceNumbers.enqueue(sequenceNumber); } } + +bool OctreeQueryNode::haveJSONParametersChanged() { + bool parametersChanged = false; + auto currentParameters = getJSONParameters(); + + if (_lastCheckJSONParameters != getJSONParameters()) { + parametersChanged = true; + _lastCheckJSONParameters = currentParameters; + } + + return parametersChanged; +} diff --git a/libraries/octree/src/OctreeQueryNode.h b/libraries/octree/src/OctreeQueryNode.h index 021b293804..10c5598b30 100644 --- a/libraries/octree/src/OctreeQueryNode.h +++ b/libraries/octree/src/OctreeQueryNode.h @@ -100,6 +100,9 @@ public: bool hasNextNackedPacket() const; const NLPacket* getNextNackedPacket(); + // call only from OctreeSendThread for the given node + bool haveJSONParametersChanged(); + private: OctreeQueryNode(const OctreeQueryNode &); OctreeQueryNode& operator= (const OctreeQueryNode&); @@ -143,6 +146,8 @@ private: quint64 _sceneSendStartTime = 0; std::array _lastOctreePayload; + + QJsonObject _lastCheckJSONParameters; }; #endif // hifi_OctreeQueryNode_h From 543d0ce77e36a86183a6786fc725d382aac1c70f Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Tue, 24 Jan 2017 16:36:44 -0800 Subject: [PATCH 2/6] add a comment about usesFrustum check in EntityTreeElement --- libraries/entities/src/EntityTreeElement.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/libraries/entities/src/EntityTreeElement.cpp b/libraries/entities/src/EntityTreeElement.cpp index 51d6d2b270..511aec43c0 100644 --- a/libraries/entities/src/EntityTreeElement.cpp +++ b/libraries/entities/src/EntityTreeElement.cpp @@ -332,6 +332,8 @@ OctreeElement::AppendState EntityTreeElement::appendElementData(OctreePacketData entityTreeElementExtraEncodeData->entities.contains(entity->getEntityItemID()); } + // we only check the bounds against our frustum and LOD if the query has asked us to check against the frustum + // which can sometimes not be the case when JSON filters are sent if (params.usesFrustum && (includeThisEntity || params.recurseEverything)) { // we want to use the maximum possible box for this, so that we don't have to worry about the nuance of From 67a3d97f81c9960f3b03ea1e135fd3a5ef4eb54d Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Tue, 24 Jan 2017 16:41:20 -0800 Subject: [PATCH 3/6] cleanup includeThisEntity check --- libraries/entities/src/EntityTreeElement.cpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/libraries/entities/src/EntityTreeElement.cpp b/libraries/entities/src/EntityTreeElement.cpp index 511aec43c0..5a7f0d2a87 100644 --- a/libraries/entities/src/EntityTreeElement.cpp +++ b/libraries/entities/src/EntityTreeElement.cpp @@ -327,9 +327,8 @@ OctreeElement::AppendState EntityTreeElement::appendElementData(OctreePacketData } } - if (hadElementExtraData) { - includeThisEntity = includeThisEntity && - entityTreeElementExtraEncodeData->entities.contains(entity->getEntityItemID()); + if (includeThisEntity && hadElementExtraData) { + includeThisEntity = entityTreeElementExtraEncodeData->entities.contains(entity->getEntityItemID()); } // we only check the bounds against our frustum and LOD if the query has asked us to check against the frustum From 69b47dcf50e564766253995dc47a66b4ac345bee Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Tue, 24 Jan 2017 16:49:17 -0800 Subject: [PATCH 4/6] use currentParameters instead of getJSONParameters() --- libraries/octree/src/OctreeQueryNode.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/octree/src/OctreeQueryNode.cpp b/libraries/octree/src/OctreeQueryNode.cpp index cffba0bef4..4ebe650f6a 100644 --- a/libraries/octree/src/OctreeQueryNode.cpp +++ b/libraries/octree/src/OctreeQueryNode.cpp @@ -325,7 +325,7 @@ bool OctreeQueryNode::haveJSONParametersChanged() { bool parametersChanged = false; auto currentParameters = getJSONParameters(); - if (_lastCheckJSONParameters != getJSONParameters()) { + if (_lastCheckJSONParameters != currentParameters) { parametersChanged = true; _lastCheckJSONParameters = currentParameters; } From fc42c1bc94e5a6717afa25dacfd64c60bff15e36 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Tue, 24 Jan 2017 17:08:13 -0800 Subject: [PATCH 5/6] don't tell ESS about script Agents that won't talk to it --- domain-server/src/DomainServer.cpp | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/domain-server/src/DomainServer.cpp b/domain-server/src/DomainServer.cpp index 5a5dd62c40..8a7ff7ab20 100644 --- a/domain-server/src/DomainServer.cpp +++ b/domain-server/src/DomainServer.cpp @@ -960,11 +960,14 @@ void DomainServer::sendDomainListToNode(const SharedNodePointer& node, const Hif // (1/19/17) Agents only need to connect to Entity Script Servers to perform administrative tasks // related to entity server scripts. Only agents with rez permissions should be doing that, so // if the agent does not have those permissions, we do not want them and the server to incur the - // overhead of connecting to one another. + // overhead of connecting to one another. Additionally we exclude agents that do not care about the + // Entity Script Server and won't attempt to connect to it. + auto otherNodeData = static_cast(otherNode->getLinkedData()); bool shouldNotConnect = (node->getType() == NodeType::Agent && otherNode->getType() == NodeType::EntityScriptServer && !node->getCanRez() && !node->getCanRezTmp()) || (node->getType() == NodeType::EntityScriptServer && otherNode->getType() == NodeType::Agent - && !otherNode->getCanRez() && !otherNode->getCanRezTmp()); + && (!otherNodeData->getNodeInterestSet().contains(NodeType::EntityServer) || + !otherNode->getCanRez() && !otherNode->getCanRezTmp())); if (!shouldNotConnect) { // since we're about to add a node to the packet we start a segment From e511b6e84c84f7534a1e3969e4b06a8ab69ca73f Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Tue, 24 Jan 2017 17:14:05 -0800 Subject: [PATCH 6/6] cleanup connect check in DomainServer --- domain-server/src/DomainServer.cpp | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/domain-server/src/DomainServer.cpp b/domain-server/src/DomainServer.cpp index 8a7ff7ab20..8e44922424 100644 --- a/domain-server/src/DomainServer.cpp +++ b/domain-server/src/DomainServer.cpp @@ -963,11 +963,17 @@ void DomainServer::sendDomainListToNode(const SharedNodePointer& node, const Hif // overhead of connecting to one another. Additionally we exclude agents that do not care about the // Entity Script Server and won't attempt to connect to it. auto otherNodeData = static_cast(otherNode->getLinkedData()); - bool shouldNotConnect = (node->getType() == NodeType::Agent && otherNode->getType() == NodeType::EntityScriptServer - && !node->getCanRez() && !node->getCanRezTmp()) - || (node->getType() == NodeType::EntityScriptServer && otherNode->getType() == NodeType::Agent - && (!otherNodeData->getNodeInterestSet().contains(NodeType::EntityServer) || - !otherNode->getCanRez() && !otherNode->getCanRezTmp())); + + bool isAgentWithoutRights = node->getType() == NodeType::Agent + && otherNode->getType() == NodeType::EntityScriptServer + && !node->getCanRez() && !node->getCanRezTmp(); + + bool isScriptServerForIneffectiveAgent = + (node->getType() == NodeType::EntityScriptServer && otherNode->getType() == NodeType::Agent) + && (!otherNodeData->getNodeInterestSet().contains(NodeType::EntityServer) + || (!otherNode->getCanRez() && !otherNode->getCanRezTmp())); + + bool shouldNotConnect = isAgentWithoutRights || isScriptServerForIneffectiveAgent; if (!shouldNotConnect) { // since we're about to add a node to the packet we start a segment