From 05d896f3de1945fd3713dbf335ba06f0c0442c6f Mon Sep 17 00:00:00 2001
From: Andrew Meadows <andrew@highfidelity.io>
Date: Wed, 10 Jul 2019 17:45:35 -0700
Subject: [PATCH 1/4] fix SafeLanding failure mode when first sequence number
not 0
---
interface/src/Application.cpp | 3 +-
.../src/octree/OctreePacketProcessor.cpp | 21 ++++--
interface/src/octree/OctreePacketProcessor.h | 2 +
interface/src/octree/SafeLanding.cpp | 68 +++++++++++--------
interface/src/octree/SafeLanding.h | 26 ++++---
5 files changed, 75 insertions(+), 45 deletions(-)
diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp
index d20574bdaa..cd7bb36763 100644
--- a/interface/src/Application.cpp
+++ b/interface/src/Application.cpp
@@ -6245,6 +6245,7 @@ void Application::tryToEnablePhysics() {
// We keep physics disabled until we've received a full scene and everything near the avatar in that
// scene is ready to compute its collision shape.
if (getMyAvatar()->isReadyForPhysics()) {
+ _octreeProcessor.resetSafeLanding();
_physicsEnabled = true;
setIsInterstitialMode(false);
getMyAvatar()->updateMotionBehaviorFromMenu();
@@ -7316,7 +7317,7 @@ void Application::nodeKilled(SharedNodePointer node) {
_octreeProcessor.nodeKilled(node);
_entityEditSender.nodeKilled(node);
-
+
if (node->getType() == NodeType::AudioMixer) {
QMetaObject::invokeMethod(DependencyManager::get<AudioClient>().data(), "audioMixerKilled");
} else if (node->getType() == NodeType::EntityServer) {
diff --git a/interface/src/octree/OctreePacketProcessor.cpp b/interface/src/octree/OctreePacketProcessor.cpp
index c6f908e039..a44774540a 100644
--- a/interface/src/octree/OctreePacketProcessor.cpp
+++ b/interface/src/octree/OctreePacketProcessor.cpp
@@ -114,7 +114,11 @@ void OctreePacketProcessor::processPacket(QSharedPointer<ReceivedMessage> messag
if (renderer) {
renderer->processDatagram(*message, sendingNode);
if (_safeLanding && _safeLanding->isTracking()) {
- _safeLanding->addToSequence(renderer->getLastOctreeMessageSequence());
+ OCTREE_PACKET_SEQUENCE thisSequence = renderer->getLastOctreeMessageSequence();
+ _safeLanding->addToSequence(thisSequence);
+ if (_safeLandingSequenceStart == SafeLanding::INVALID_SEQUENCE) {
+ _safeLandingSequenceStart = thisSequence;
+ }
}
}
}
@@ -122,10 +126,10 @@ void OctreePacketProcessor::processPacket(QSharedPointer<ReceivedMessage> messag
case PacketType::EntityQueryInitialResultsComplete: {
// Read sequence #
- OCTREE_PACKET_SEQUENCE completionNumber;
- message->readPrimitive(&completionNumber);
- if (_safeLanding) {
- _safeLanding->finishSequence(0, completionNumber);
+ if (_safeLanding && _safeLanding->isTracking()) {
+ OCTREE_PACKET_SEQUENCE completionNumber;
+ message->readPrimitive(&completionNumber);
+ _safeLanding->finishSequence(_safeLandingSequenceStart, completionNumber);
}
} break;
@@ -153,6 +157,13 @@ void OctreePacketProcessor::stopSafeLanding() {
}
}
+void OctreePacketProcessor::resetSafeLanding() {
+ if (_safeLanding) {
+ _safeLanding->reset();
+ }
+ _safeLandingSequenceStart = SafeLanding::INVALID_SEQUENCE;
+}
+
bool OctreePacketProcessor::safeLandingIsActive() const {
return _safeLanding && _safeLanding->isTracking();
}
diff --git a/interface/src/octree/OctreePacketProcessor.h b/interface/src/octree/OctreePacketProcessor.h
index eacc15f62d..d56b7e0e35 100644
--- a/interface/src/octree/OctreePacketProcessor.h
+++ b/interface/src/octree/OctreePacketProcessor.h
@@ -28,6 +28,7 @@ public:
void startSafeLanding();
void updateSafeLanding();
void stopSafeLanding();
+ void resetSafeLanding();
bool safeLandingIsActive() const;
bool safeLandingIsComplete() const;
@@ -43,6 +44,7 @@ private slots:
void handleOctreePacket(QSharedPointer<ReceivedMessage> message, SharedNodePointer senderNode);
private:
+ OCTREE_PACKET_SEQUENCE _safeLandingSequenceStart { SafeLanding::INVALID_SEQUENCE };
std::unique_ptr<SafeLanding> _safeLanding;
};
#endif // hifi_OctreePacketProcessor_h
diff --git a/interface/src/octree/SafeLanding.cpp b/interface/src/octree/SafeLanding.cpp
index 80ca0e5398..2e2a43859c 100644
--- a/interface/src/octree/SafeLanding.cpp
+++ b/interface/src/octree/SafeLanding.cpp
@@ -17,7 +17,6 @@
#include "InterfaceLogging.h"
#include "Application.h"
-const int SafeLanding::SEQUENCE_MODULO = std::numeric_limits<OCTREE_PACKET_SEQUENCE>::max() + 1;
CalculateEntityLoadingPriority SafeLanding::entityLoadingOperatorElevateCollidables = [](const EntityItem& entityItem) {
const int COLLIDABLE_ENTITY_PRIORITY = 10.0f;
@@ -25,8 +24,8 @@ CalculateEntityLoadingPriority SafeLanding::entityLoadingOperatorElevateCollidab
};
namespace {
- template<typename T> bool lessThanWraparound(int a, int b) {
- constexpr int MAX_T_VALUE = std::numeric_limits<T>::max();
+ template<typename T> bool lessThanWraparound(int32_t a, int32_t b) {
+ constexpr int32_t MAX_T_VALUE = std::numeric_limits<T>::max();
if (b <= a) {
b += MAX_T_VALUE;
}
@@ -34,7 +33,7 @@ namespace {
}
}
-bool SafeLanding::SequenceLessThan::operator()(const int& a, const int& b) const {
+bool SafeLanding::SequenceLessThan::operator()(const OCTREE_PACKET_SEQUENCE& a, const OCTREE_PACKET_SEQUENCE& b) const {
return lessThanWraparound<OCTREE_PACKET_SEQUENCE>(a, b);
}
@@ -46,8 +45,8 @@ void SafeLanding::startTracking(QSharedPointer<EntityTreeRenderer> entityTreeRen
_entityTreeRenderer = entityTreeRenderer;
_trackedEntities.clear();
_maxTrackedEntityCount = 0;
- _initialStart = INVALID_SEQUENCE;
- _initialEnd = INVALID_SEQUENCE;
+ _sequenceStart = SafeLanding::INVALID_SEQUENCE;
+ _sequenceEnd = SafeLanding::INVALID_SEQUENCE;
_sequenceNumbers.clear();
_trackingEntities = true;
_startTime = usecTimestampNow();
@@ -72,7 +71,7 @@ void SafeLanding::addTrackedEntity(const EntityItemID& entityID) {
if (entity && !entity->isLocalEntity() && entity->getCreated() < _startTime) {
_trackedEntities.emplace(entityID, entity);
- int trackedEntityCount = (int)_trackedEntities.size();
+ int32_t trackedEntityCount = (int32_t)_trackedEntities.size();
if (trackedEntityCount > _maxTrackedEntityCount) {
_maxTrackedEntityCount = trackedEntityCount;
_trackedEntityStabilityCount = 0;
@@ -87,15 +86,15 @@ void SafeLanding::deleteTrackedEntity(const EntityItemID& entityID) {
_trackedEntities.erase(entityID);
}
-void SafeLanding::finishSequence(int first, int last) {
+void SafeLanding::finishSequence(OCTREE_PACKET_SEQUENCE first, OCTREE_PACKET_SEQUENCE last) {
Locker lock(_lock);
if (_trackingEntities) {
- _initialStart = first;
- _initialEnd = last;
+ _sequenceStart = first;
+ _sequenceEnd = last;
}
}
-void SafeLanding::addToSequence(int sequenceNumber) {
+void SafeLanding::addToSequence(OCTREE_PACKET_SEQUENCE sequenceNumber) {
Locker lock(_lock);
_sequenceNumbers.insert(sequenceNumber);
}
@@ -135,14 +134,15 @@ void SafeLanding::updateTracking() {
if (_trackedEntities.empty()) {
// no more tracked entities --> check sequenceNumbers
- if (_initialStart != INVALID_SEQUENCE) {
+ if (_sequenceStart != SafeLanding::INVALID_SEQUENCE) {
bool shouldStop = false;
{
Locker lock(_lock);
- int sequenceSize = _initialStart <= _initialEnd ? _initialEnd - _initialStart:
- _initialEnd + SEQUENCE_MODULO - _initialStart;
- auto startIter = _sequenceNumbers.find(_initialStart);
- auto endIter = _sequenceNumbers.find(_initialEnd - 1);
+ int32_t sequenceSize = _sequenceStart < _sequenceEnd ?
+ (int32_t)(_sequenceEnd - _sequenceStart) :
+ (int32_t)((SafeLanding::MAX_SEQUENCE - _sequenceStart) + _sequenceEnd + 1); // with rollover
+ auto startIter = _sequenceNumbers.find(_sequenceStart);
+ auto endIter = _sequenceNumbers.find(_sequenceEnd - 1);
bool missingSequenceNumbers = qApp->isMissingSequenceNumbers();
shouldStop = (sequenceSize == 0 ||
@@ -171,19 +171,27 @@ void SafeLanding::stopTracking() {
EntityTreeRenderer::setEntityLoadingPriorityFunction(_prevEntityLoadingPriorityOperator);
}
+void SafeLanding::reset() {
+ _trackingEntities = false;
+ _trackedEntities.clear();
+ _maxTrackedEntityCount = 0;
+ _sequenceStart = SafeLanding::INVALID_SEQUENCE;
+ _sequenceEnd = SafeLanding::INVALID_SEQUENCE;
+}
+
bool SafeLanding::trackingIsComplete() const {
- return !_trackingEntities && (_initialStart != INVALID_SEQUENCE);
+ return !_trackingEntities && (_sequenceStart != SafeLanding::INVALID_SEQUENCE);
}
float SafeLanding::loadingProgressPercentage() {
Locker lock(_lock);
- static const int MINIMUM_TRACKED_ENTITY_STABILITY_COUNT = 15;
float entityReadyPercentage = 0.0f;
if (_maxTrackedEntityCount > 0) {
entityReadyPercentage = ((_maxTrackedEntityCount - _trackedEntities.size()) / (float)_maxTrackedEntityCount);
}
+ constexpr int32_t MINIMUM_TRACKED_ENTITY_STABILITY_COUNT = 15;
if (_trackedEntityStabilityCount < MINIMUM_TRACKED_ENTITY_STABILITY_COUNT) {
entityReadyPercentage *= 0.20f;
}
@@ -226,20 +234,24 @@ bool SafeLanding::isEntityPhysicsReady(const EntityItemPointer& entity) {
}
void SafeLanding::debugDumpSequenceIDs() const {
- int p = -1;
qCDebug(interfaceapp) << "Sequence set size:" << _sequenceNumbers.size();
- for (auto s: _sequenceNumbers) {
- if (p == -1) {
- p = s;
- qCDebug(interfaceapp) << "First:" << s;
- } else {
+
+ auto itr = _sequenceNumbers.begin();
+ OCTREE_PACKET_SEQUENCE p = SafeLanding::INVALID_SEQUENCE;
+ if (itr != _sequenceNumbers.end()) {
+ p = (*itr);
+ qCDebug(interfaceapp) << "First:" << (int32_t)p;
+ ++itr;
+ while (itr != _sequenceNumbers.end()) {
+ OCTREE_PACKET_SEQUENCE s = *itr;
if (s != p + 1) {
- qCDebug(interfaceapp) << "Gap from" << p << "to" << s << "(exclusive)";
+ qCDebug(interfaceapp) << "Gap from" << (int32_t)p << "to" << (int32_t)s << "(exclusive)";
p = s;
}
+ ++itr;
+ }
+ if (p != SafeLanding::INVALID_SEQUENCE) {
+ qCDebug(interfaceapp) << "Last:" << p;
}
}
- if (p != -1) {
- qCDebug(interfaceapp) << "Last:" << p;
- }
}
diff --git a/interface/src/octree/SafeLanding.h b/interface/src/octree/SafeLanding.h
index 87dac16ba3..00473f6fc8 100644
--- a/interface/src/octree/SafeLanding.h
+++ b/interface/src/octree/SafeLanding.h
@@ -17,6 +17,8 @@
#include <QtCore/QObject>
#include <QtCore/QSharedPointer>
+#include <set>
+
#include "EntityItem.h"
#include "EntityDynamicInterface.h"
@@ -27,14 +29,18 @@ class EntityItemID;
class SafeLanding : public QObject {
public:
+ static constexpr OCTREE_PACKET_SEQUENCE MAX_SEQUENCE = std::numeric_limits<OCTREE_PACKET_SEQUENCE>::max();
+ static constexpr OCTREE_PACKET_SEQUENCE INVALID_SEQUENCE = MAX_SEQUENCE; // not technically invalid, but close enough
+
void startTracking(QSharedPointer<EntityTreeRenderer> entityTreeRenderer);
void updateTracking();
void stopTracking();
+ void reset();
bool isTracking() const { return _trackingEntities; }
bool trackingIsComplete() const;
- void finishSequence(int first, int last); // 'last' exclusive.
- void addToSequence(int sequenceNumber);
+ void finishSequence(OCTREE_PACKET_SEQUENCE first, OCTREE_PACKET_SEQUENCE last); // 'last' exclusive.
+ void addToSequence(OCTREE_PACKET_SEQUENCE sequenceNumber);
float loadingProgressPercentage();
private slots:
@@ -52,24 +58,22 @@ private:
using EntityMap = std::map<EntityItemID, EntityItemPointer>;
EntityMap _trackedEntities;
- static constexpr int INVALID_SEQUENCE = -1;
- int _initialStart { INVALID_SEQUENCE };
- int _initialEnd { INVALID_SEQUENCE };
- int _maxTrackedEntityCount { 0 };
- int _trackedEntityStabilityCount { 0 };
+ OCTREE_PACKET_SEQUENCE _sequenceStart { INVALID_SEQUENCE };
+ OCTREE_PACKET_SEQUENCE _sequenceEnd { INVALID_SEQUENCE };
+ int32_t _maxTrackedEntityCount { 0 };
+ int32_t _trackedEntityStabilityCount { 0 };
quint64 _startTime { 0 };
struct SequenceLessThan {
- bool operator()(const int& a, const int& b) const;
+ bool operator()(const OCTREE_PACKET_SEQUENCE& a, const OCTREE_PACKET_SEQUENCE& b) const;
};
- std::set<int, SequenceLessThan> _sequenceNumbers;
+ using SequenceSet = std::set<OCTREE_PACKET_SEQUENCE, SequenceLessThan>;
+ SequenceSet _sequenceNumbers;
static CalculateEntityLoadingPriority entityLoadingOperatorElevateCollidables;
CalculateEntityLoadingPriority _prevEntityLoadingPriorityOperator { nullptr };
-
- static const int SEQUENCE_MODULO;
};
#endif // hifi_SafeLanding_h
From b0f297e696a9d49280710bd8e1522ff05900333b Mon Sep 17 00:00:00 2001
From: Andrew Meadows <andrew@highfidelity.io>
Date: Wed, 17 Jul 2019 11:20:38 -0700
Subject: [PATCH 2/4] wait for SafeLanding to start before starting octreeQuery
---
interface/src/Application.cpp | 12 +++++++-----
interface/src/octree/SafeLanding.cpp | 20 +++++++++++---------
2 files changed, 18 insertions(+), 14 deletions(-)
diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp
index cd7bb36763..ad38599dcf 100644
--- a/interface/src/Application.cpp
+++ b/interface/src/Application.cpp
@@ -5986,6 +5986,7 @@ void Application::resetPhysicsReadyInformation() {
_gpuTextureMemSizeStabilityCount = 0;
_gpuTextureMemSizeAtLastCheck = 0;
_physicsEnabled = false;
+ _octreeProcessor.stopSafeLanding();
}
void Application::reloadResourceCaches() {
@@ -6981,13 +6982,16 @@ int Application::sendNackPackets() {
}
void Application::queryOctree(NodeType_t serverType, PacketType packetType) {
-
if (!_settingsLoaded) {
return; // bail early if settings are not loaded
}
const bool isModifiedQuery = !_physicsEnabled;
if (isModifiedQuery) {
+ if (!_octreeProcessor.safeLandingIsActive()) {
+ // don't send the octreeQuery until SafeLanding knows it has started
+ return;
+ }
// Create modified view that is a simple sphere.
bool interstitialModeEnabled = DependencyManager::get<NodeList>()->getDomainHandler().getInterstitialModeEnabled();
@@ -7210,7 +7214,6 @@ void Application::clearDomainOctreeDetails(bool clearAll) {
void Application::domainURLChanged(QUrl domainURL) {
// disable physics until we have enough information about our new location to not cause craziness.
- resetPhysicsReadyInformation();
setIsServerlessMode(domainURL.scheme() != URL_SCHEME_HIFI);
if (isServerlessMode()) {
loadServerlessDomain(domainURL);
@@ -7220,7 +7223,6 @@ void Application::domainURLChanged(QUrl domainURL) {
void Application::goToErrorDomainURL(QUrl errorDomainURL) {
// disable physics until we have enough information about our new location to not cause craziness.
- resetPhysicsReadyInformation();
setIsServerlessMode(errorDomainURL.scheme() != URL_SCHEME_HIFI);
if (isServerlessMode()) {
loadErrorDomain(errorDomainURL);
@@ -7237,12 +7239,12 @@ void Application::resettingDomain() {
void Application::nodeAdded(SharedNodePointer node) {
if (node->getType() == NodeType::EntityServer) {
if (_failedToConnectToEntityServer && !_entityServerConnectionTimer.isActive()) {
- _failedToConnectToEntityServer = false;
_octreeProcessor.stopSafeLanding();
- _octreeProcessor.startSafeLanding();
+ _failedToConnectToEntityServer = false;
} else if (_entityServerConnectionTimer.isActive()) {
_entityServerConnectionTimer.stop();
}
+ _octreeProcessor.startSafeLanding();
_entityServerConnectionTimer.setInterval(ENTITY_SERVER_CONNECTION_TIMEOUT);
_entityServerConnectionTimer.start();
}
diff --git a/interface/src/octree/SafeLanding.cpp b/interface/src/octree/SafeLanding.cpp
index 2e2a43859c..6f715ea033 100644
--- a/interface/src/octree/SafeLanding.cpp
+++ b/interface/src/octree/SafeLanding.cpp
@@ -159,16 +159,18 @@ void SafeLanding::updateTracking() {
void SafeLanding::stopTracking() {
Locker lock(_lock);
- _trackingEntities = false;
- if (_entityTreeRenderer) {
- auto entityTree = _entityTreeRenderer->getTree();
- disconnect(std::const_pointer_cast<EntityTree>(entityTree).get(),
- &EntityTree::addingEntity, this, &SafeLanding::addTrackedEntity);
- disconnect(std::const_pointer_cast<EntityTree>(entityTree).get(),
- &EntityTree::deletingEntity, this, &SafeLanding::deleteTrackedEntity);
- _entityTreeRenderer.reset();
+ if (_trackingEntities) {
+ _trackingEntities = false;
+ if (_entityTreeRenderer) {
+ auto entityTree = _entityTreeRenderer->getTree();
+ disconnect(std::const_pointer_cast<EntityTree>(entityTree).get(),
+ &EntityTree::addingEntity, this, &SafeLanding::addTrackedEntity);
+ disconnect(std::const_pointer_cast<EntityTree>(entityTree).get(),
+ &EntityTree::deletingEntity, this, &SafeLanding::deleteTrackedEntity);
+ _entityTreeRenderer.reset();
+ }
+ EntityTreeRenderer::setEntityLoadingPriorityFunction(_prevEntityLoadingPriorityOperator);
}
- EntityTreeRenderer::setEntityLoadingPriorityFunction(_prevEntityLoadingPriorityOperator);
}
void SafeLanding::reset() {
From 63b3d666f950dcdde24bf74fef304a28ea24350f Mon Sep 17 00:00:00 2001
From: Andrew Meadows <andrew@highfidelity.io>
Date: Wed, 17 Jul 2019 13:57:08 -0700
Subject: [PATCH 3/4] simplify rollover logic for SafeLanding sequence size
calculation
---
interface/src/octree/SafeLanding.cpp | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/interface/src/octree/SafeLanding.cpp b/interface/src/octree/SafeLanding.cpp
index 6f715ea033..ed779787c9 100644
--- a/interface/src/octree/SafeLanding.cpp
+++ b/interface/src/octree/SafeLanding.cpp
@@ -138,9 +138,7 @@ void SafeLanding::updateTracking() {
bool shouldStop = false;
{
Locker lock(_lock);
- int32_t sequenceSize = _sequenceStart < _sequenceEnd ?
- (int32_t)(_sequenceEnd - _sequenceStart) :
- (int32_t)((SafeLanding::MAX_SEQUENCE - _sequenceStart) + _sequenceEnd + 1); // with rollover
+ auto sequenceSize = _sequenceEnd - _sequenceStart; // this works even in rollover case
auto startIter = _sequenceNumbers.find(_sequenceStart);
auto endIter = _sequenceNumbers.find(_sequenceEnd - 1);
From 7b7414dead363a6d130e134654cd9ee5c1855012 Mon Sep 17 00:00:00 2001
From: Andrew Meadows <andrew@highfidelity.io>
Date: Wed, 17 Jul 2019 17:04:12 -0700
Subject: [PATCH 4/4] revert optimization that would skip read from packet
---
interface/src/octree/OctreePacketProcessor.cpp | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/interface/src/octree/OctreePacketProcessor.cpp b/interface/src/octree/OctreePacketProcessor.cpp
index a44774540a..bc3c1afdd5 100644
--- a/interface/src/octree/OctreePacketProcessor.cpp
+++ b/interface/src/octree/OctreePacketProcessor.cpp
@@ -126,9 +126,9 @@ void OctreePacketProcessor::processPacket(QSharedPointer<ReceivedMessage> messag
case PacketType::EntityQueryInitialResultsComplete: {
// Read sequence #
+ OCTREE_PACKET_SEQUENCE completionNumber;
+ message->readPrimitive(&completionNumber);
if (_safeLanding && _safeLanding->isTracking()) {
- OCTREE_PACKET_SEQUENCE completionNumber;
- message->readPrimitive(&completionNumber);
_safeLanding->finishSequence(_safeLandingSequenceStart, completionNumber);
}
} break;