From 572e208a1ef86078755325e12388e93d5645f47c Mon Sep 17 00:00:00 2001 From: Joshua Chamberlain Date: Tue, 21 Nov 2017 17:19:09 -0800 Subject: [PATCH 01/30] Create BUILD_LINUX_CHEATSHEET.md --- BUILD_LINUX_CHEATSHEET.md | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) create mode 100644 BUILD_LINUX_CHEATSHEET.md diff --git a/BUILD_LINUX_CHEATSHEET.md b/BUILD_LINUX_CHEATSHEET.md new file mode 100644 index 0000000000..7d77f5d685 --- /dev/null +++ b/BUILD_LINUX_CHEATSHEET.md @@ -0,0 +1,27 @@ + # this guide is specific to Ubuntu 16.04. + # deb packages of High Fidelity domain server and assignment client are stored on debian.highfidelity.com +sudo su - +apt-get -y update +apt-get install -y software-properties-common +apt-key adv --keyserver keyserver.ubuntu.com --recv-keys 15FF1AAE +add-apt-repository "deb http://debian.highfidelity.com stable main" +apt-get -y update +apt-get install -y hifi-domain-server +apt-get install -y hifi-assignment-client + + # When installing master/dev builds, the packages are slightly different and you just need to change the last 2 steps to: +apt-get install -y hifi-dev-domain-server +apt-get install -y hifi-dev-assignment-client + + # domain server and assignment clients should already be running. The processes are controlled via: +systemctl start hifi-domain-server +systemctl stop hifi-domain-server + + # Once the machine is setup and processes are running you should ensure that your firewall exposes port 40100 on TCP and all UDP ports. This will get your domain up and running and you could connect to it (for now) by using High Fidelity Interface and typing in the IP for the place name. (further customizations can be done via http://IPAddress:40100). + + # The server always depends on both hifi-domain-server and hifi-assignment-client running at the same time. + # As an additional step, you should ensure that your packages are automatically updated when a new version goes out. You could, for example, set the automatic update checks to happen every hour (though this could potentially result in the domain being unreachable for a whole hour by new clients when they are released - adjust the update checks accordingly). +To do this you can modify /etc/crontab by adding the following lines +0 */1 * * * root apt-get update +1 */1 * * * root apt-get install --only-upgrade -y hifi-domain-server +2 */1 * * * root apt-get install --only-upgrade -y hifi-assignment-client From 4374b4f9a83c4775a99719b6fd6c7193cdb402dc Mon Sep 17 00:00:00 2001 From: humbletim Date: Mon, 11 Dec 2017 00:39:23 -0500 Subject: [PATCH 02/30] forward MIDI_CONTROL_CHANGE events --- libraries/midi/src/Midi.cpp | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/libraries/midi/src/Midi.cpp b/libraries/midi/src/Midi.cpp index ad650cf067..6c2471f680 100644 --- a/libraries/midi/src/Midi.cpp +++ b/libraries/midi/src/Midi.cpp @@ -177,9 +177,10 @@ void Midi::MidiCleanup() { #endif void Midi::noteReceived(int status, int note, int velocity) { - if (((status & MIDI_STATUS_MASK) != MIDI_NOTE_OFF) && - ((status & MIDI_STATUS_MASK) != MIDI_NOTE_ON)) { - return; // NOTE: only sending note-on and note-off to Javascript + if (((status & MIDI_STATUS_MASK) != MIDI_NOTE_OFF) && + ((status & MIDI_STATUS_MASK) != MIDI_NOTE_ON) && + ((status & MIDI_STATUS_MASK) != MIDI_CONTROL_CHANGE)) { + return; // NOTE: only sending note-on, note-off, and control-change to Javascript } QVariantMap eventData; From 3a45907df078bdc20bc915ac7103e810fbf28d5c Mon Sep 17 00:00:00 2001 From: Atlante45 Date: Tue, 12 Dec 2017 17:16:07 -0800 Subject: [PATCH 03/30] Keep connection alive as long as the node is connected --- libraries/networking/src/udt/Connection.cpp | 82 ++++++++------------- libraries/networking/src/udt/Connection.h | 9 +-- libraries/networking/src/udt/SendQueue.cpp | 52 ++++++------- libraries/networking/src/udt/SendQueue.h | 8 +- libraries/networking/src/udt/Socket.cpp | 3 - 5 files changed, 57 insertions(+), 97 deletions(-) diff --git a/libraries/networking/src/udt/Connection.cpp b/libraries/networking/src/udt/Connection.cpp index f42049f107..7a31bbeedc 100644 --- a/libraries/networking/src/udt/Connection.cpp +++ b/libraries/networking/src/udt/Connection.cpp @@ -11,6 +11,8 @@ #include "Connection.h" +#include + #include #include @@ -60,6 +62,15 @@ Connection::Connection(Socket* parentSocket, HifiSockAddr destination, std::uniq _ack2Packet = ControlPacket::create(ControlPacket::ACK2, ACK2_PAYLOAD_BYTES); _lossReport = ControlPacket::create(ControlPacket::NAK, NAK_PACKET_PAYLOAD_BYTES); _handshakeACK = ControlPacket::create(ControlPacket::HandshakeACK, HANDSHAKE_ACK_PAYLOAD_BYTES); + + + // setup psuedo-random number generation shared by all connections + static std::random_device rd; + static std::mt19937 generator(rd()); + static std::uniform_int_distribution<> distribution(0, SequenceNumber::MAX); + + // randomize the intial sequence number + _initialSequenceNumber = SequenceNumber(distribution(generator)); } Connection::~Connection() { @@ -81,9 +92,6 @@ void Connection::stopSendQueue() { sendQueue->stop(); sendQueue->deleteLater(); - // since we're stopping the send queue we should consider our handshake ACK not receieved - _hasReceivedHandshakeACK = false; - // wait on the send queue thread so we know the send queue is gone sendQueueThread->quit(); sendQueueThread->wait(); @@ -101,13 +109,22 @@ void Connection::setMaxBandwidth(int maxBandwidth) { SendQueue& Connection::getSendQueue() { if (!_sendQueue) { - // we may have a sequence number from the previous inactive queue - re-use that so that the // receiver is getting the sequence numbers it expects (given that the connection must still be active) // Lasily create send queue - _sendQueue = SendQueue::create(_parentSocket, _destination); - _lastReceivedACK = _sendQueue->getCurrentSequenceNumber(); + + if (!_hasReceivedHandshakeACK) { + // First time creating a send queue for this connection + _sendQueue = SendQueue::create(_parentSocket, _destination, _initialSequenceNumber - 1); + _lastReceivedACK = _sendQueue->getCurrentSequenceNumber(); + } else { + // Connection already has a handshake from a previous send queue + _sendQueue = SendQueue::create(_parentSocket, _destination, _lastReceivedACK); + // This connection has already gone through the handshake + // bypass it in the send queue + _sendQueue->handshakeACK(); + } #ifdef UDT_CONNECTION_DEBUG qCDebug(networking) << "Created SendQueue for connection to" << _destination; @@ -142,14 +159,6 @@ void Connection::queueInactive() { #ifdef UDT_CONNECTION_DEBUG qCDebug(networking) << "Connection to" << _destination << "has stopped its SendQueue."; #endif - - if (!_hasReceivedHandshake || !_isReceivingData) { -#ifdef UDT_CONNECTION_DEBUG - qCDebug(networking) << "Connection SendQueue to" << _destination << "stopped and no data is being received - stopping connection."; -#endif - - deactivate(); - } } void Connection::queueTimeout() { @@ -208,19 +217,6 @@ void Connection::sync() { && duration_cast(sincePacketReceive).count() >= MIN_SECONDS_BEFORE_EXPIRY ) { // the receive side of this connection is expired _isReceivingData = false; - - // if we don't have a send queue that means the whole connection has expired and we can emit our signal - // otherwise we'll wait for it to also timeout before cleaning up - if (!_sendQueue) { - -#ifdef UDT_CONNECTION_DEBUG - qCDebug(networking) << "Connection to" << _destination << "no longer receiving any data and there is currently no send queue - stopping connection."; -#endif - - deactivate(); - - return; - } } // reset the number of light ACKs or non SYN ACKs during this sync interval @@ -242,26 +238,6 @@ void Connection::sync() { sendTimeoutNAK(); } } - } else if (!_sendQueue) { - // we haven't received a packet and we're not sending - // this most likely means we were started erroneously - // check the start time for this connection and auto expire it after 5 seconds of not receiving or sending any data - static const int CONNECTION_NOT_USED_EXPIRY_SECONDS = 5; - auto secondsSinceStart = duration_cast(p_high_resolution_clock::now() - _connectionStart).count(); - - if (secondsSinceStart >= CONNECTION_NOT_USED_EXPIRY_SECONDS) { - // it's been CONNECTION_NOT_USED_EXPIRY_SECONDS and nothing has actually happened with this connection - // consider it inactive and emit our inactivity signal - -#ifdef UDT_CONNECTION_DEBUG - qCDebug(networking) << "Connection to" << _destination << "did not receive or send any data in last" - << CONNECTION_NOT_USED_EXPIRY_SECONDS << "seconds - stopping connection."; -#endif - - deactivate(); - - return; - } } } @@ -827,11 +803,13 @@ void Connection::processHandshakeACK(ControlPacketPointer controlPacket) { SequenceNumber initialSequenceNumber; controlPacket->readPrimitive(&initialSequenceNumber); - // hand off this handshake ACK to the send queue so it knows it can start sending - getSendQueue().handshakeACK(initialSequenceNumber); - - // indicate that handshake ACK was received - _hasReceivedHandshakeACK = true; + if (initialSequenceNumber == _initialSequenceNumber) { + // hand off this handshake ACK to the send queue so it knows it can start sending + getSendQueue().handshakeACK(); + + // indicate that handshake ACK was received + _hasReceivedHandshakeACK = true; + } } } diff --git a/libraries/networking/src/udt/Connection.h b/libraries/networking/src/udt/Connection.h index c134081dde..a13c29adc8 100644 --- a/libraries/networking/src/udt/Connection.h +++ b/libraries/networking/src/udt/Connection.h @@ -72,8 +72,6 @@ public: void queueReceivedMessagePacket(std::unique_ptr packet); ConnectionStats::Stats sampleStats() { return _stats.sample(); } - - bool isActive() const { return _isActive; } HifiSockAddr getDestination() const { return _destination; } @@ -83,7 +81,6 @@ public: signals: void packetSent(); - void connectionInactive(const HifiSockAddr& sockAddr); void receiverHandshakeRequestComplete(const HifiSockAddr& sockAddr); private slots: @@ -112,8 +109,6 @@ private: void resetReceiveState(); void resetRTT(); - void deactivate() { _isActive = false; emit connectionInactive(_destination); } - SendQueue& getSendQueue(); SequenceNumber nextACK() const; void updateRTT(int rtt); @@ -138,9 +133,9 @@ private: p_high_resolution_clock::time_point _lastReceiveTime; // holds the last time we received anything from sender bool _isReceivingData { false }; // flag used for expiry of receipt portion of connection - bool _isActive { true }; // flag used for inactivity of connection - SequenceNumber _initialReceiveSequenceNumber; // Randomized by peer SendQueue on creation, identifies connection during re-connect requests + SequenceNumber _initialSequenceNumber; // Randomized on Connection creation, identifies connection during re-connect requests + SequenceNumber _initialReceiveSequenceNumber; // Randomized by peer Connection on creation, identifies connection during re-connect requests LossList _lossList; // List of all missing packets SequenceNumber _lastReceivedSequenceNumber; // The largest sequence number received from the peer diff --git a/libraries/networking/src/udt/SendQueue.cpp b/libraries/networking/src/udt/SendQueue.cpp index 0c029751aa..ef249b8f75 100644 --- a/libraries/networking/src/udt/SendQueue.cpp +++ b/libraries/networking/src/udt/SendQueue.cpp @@ -12,7 +12,6 @@ #include "SendQueue.h" #include -#include #include #include @@ -62,10 +61,10 @@ private: Mutex2& _mutex2; }; -std::unique_ptr SendQueue::create(Socket* socket, HifiSockAddr destination) { +std::unique_ptr SendQueue::create(Socket* socket, HifiSockAddr destination, SequenceNumber currentSequenceNumber) { Q_ASSERT_X(socket, "SendQueue::create", "Must be called with a valid Socket*"); - auto queue = std::unique_ptr(new SendQueue(socket, destination)); + auto queue = std::unique_ptr(new SendQueue(socket, destination, currentSequenceNumber)); // Setup queue private thread QThread* thread = new QThread; @@ -84,20 +83,12 @@ std::unique_ptr SendQueue::create(Socket* socket, HifiSockAddr destin return queue; } -SendQueue::SendQueue(Socket* socket, HifiSockAddr dest) : +SendQueue::SendQueue(Socket* socket, HifiSockAddr dest, SequenceNumber currentSequenceNumber) : _socket(socket), _destination(dest) { - // setup psuedo-random number generation for all instances of SendQueue - static std::random_device rd; - static std::mt19937 generator(rd()); - static std::uniform_int_distribution<> distribution(0, SequenceNumber::MAX); - - // randomize the intial sequence number - _initialSequenceNumber = SequenceNumber(distribution(generator)); - - // set our member variables from randomized initial number - _currentSequenceNumber = _initialSequenceNumber - 1; + // set our member variables from current sequence number + _currentSequenceNumber = currentSequenceNumber; _atomicCurrentSequenceNumber = uint32_t(_currentSequenceNumber); _lastACKSequenceNumber = uint32_t(_currentSequenceNumber) - 1; @@ -114,8 +105,8 @@ void SendQueue::queuePacket(std::unique_ptr packet) { // call notify_one on the condition_variable_any in case the send thread is sleeping waiting for packets _emptyCondition.notify_one(); - if (!this->thread()->isRunning() && _state == State::NotStarted) { - this->thread()->start(); + if (!thread()->isRunning() && _state == State::NotStarted) { + thread()->start(); } } @@ -125,8 +116,8 @@ void SendQueue::queuePacketList(std::unique_ptr packetList) { // call notify_one on the condition_variable_any in case the send thread is sleeping waiting for packets _emptyCondition.notify_one(); - if (!this->thread()->isRunning() && _state == State::NotStarted) { - this->thread()->start(); + if (!thread()->isRunning() && _state == State::NotStarted) { + thread()->start(); } } @@ -225,8 +216,11 @@ void SendQueue::sendHandshake() { std::unique_lock handshakeLock { _handshakeMutex }; if (!_hasReceivedHandshakeACK) { // we haven't received a handshake ACK from the client, send another now + // if the handshake hasn't been completed, then the initial sequence number + // should be the current sequence number + 1 + SequenceNumber initialSequenceNumber = _currentSequenceNumber + 1; auto handshakePacket = ControlPacket::create(ControlPacket::Handshake, sizeof(SequenceNumber)); - handshakePacket->writePrimitive(_initialSequenceNumber); + handshakePacket->writePrimitive(initialSequenceNumber); _socket->writeBasePacket(*handshakePacket, _destination); // we wait for the ACK or the re-send interval to expire @@ -235,18 +229,16 @@ void SendQueue::sendHandshake() { } } -void SendQueue::handshakeACK(SequenceNumber initialSequenceNumber) { - if (initialSequenceNumber == _initialSequenceNumber) { - { - std::lock_guard locker { _handshakeMutex }; - _hasReceivedHandshakeACK = true; - } - - _lastReceiverResponse = QDateTime::currentMSecsSinceEpoch(); - - // Notify on the handshake ACK condition - _handshakeACKCondition.notify_one(); +void SendQueue::handshakeACK() { + { + std::lock_guard locker { _handshakeMutex }; + _hasReceivedHandshakeACK = true; } + + _lastReceiverResponse = QDateTime::currentMSecsSinceEpoch(); + + // Notify on the handshake ACK condition + _handshakeACKCondition.notify_one(); } SequenceNumber SendQueue::getNextSequenceNumber() { diff --git a/libraries/networking/src/udt/SendQueue.h b/libraries/networking/src/udt/SendQueue.h index 484afcb88e..b6cec15ffd 100644 --- a/libraries/networking/src/udt/SendQueue.h +++ b/libraries/networking/src/udt/SendQueue.h @@ -50,7 +50,7 @@ public: Stopped }; - static std::unique_ptr create(Socket* socket, HifiSockAddr destination); + static std::unique_ptr create(Socket* socket, HifiSockAddr destination, SequenceNumber currentSequenceNumber); virtual ~SendQueue(); @@ -76,7 +76,7 @@ public slots: void nak(SequenceNumber start, SequenceNumber end); void fastRetransmit(SequenceNumber ack); void overrideNAKListFromPacket(ControlPacket& packet); - void handshakeACK(SequenceNumber initialSequenceNumber); + void handshakeACK(); signals: void packetSent(int wireSize, int payloadSize, SequenceNumber seqNum, p_high_resolution_clock::time_point timePoint); @@ -91,7 +91,7 @@ private slots: void run(); private: - SendQueue(Socket* socket, HifiSockAddr dest); + SendQueue(Socket* socket, HifiSockAddr dest, SequenceNumber currentSequenceNumber); SendQueue(SendQueue& other) = delete; SendQueue(SendQueue&& other) = delete; @@ -115,8 +115,6 @@ private: Socket* _socket { nullptr }; // Socket to send packet on HifiSockAddr _destination; // Destination addr - - SequenceNumber _initialSequenceNumber; // Randomized on SendQueue creation, identifies connection during re-connect requests std::atomic _lastACKSequenceNumber { 0 }; // Last ACKed sequence number diff --git a/libraries/networking/src/udt/Socket.cpp b/libraries/networking/src/udt/Socket.cpp index a3374a0f47..55643985c8 100644 --- a/libraries/networking/src/udt/Socket.cpp +++ b/libraries/networking/src/udt/Socket.cpp @@ -257,9 +257,6 @@ Connection* Socket::findOrCreateConnection(const HifiSockAddr& sockAddr) { congestionControl->setMaxBandwidth(_maxBandwidth); auto connection = std::unique_ptr(new Connection(this, sockAddr, std::move(congestionControl))); - // we queue the connection to cleanup connection in case it asks for it during its own rate control sync - QObject::connect(connection.get(), &Connection::connectionInactive, this, &Socket::cleanupConnection); - // allow higher-level classes to find out when connections have completed a handshake QObject::connect(connection.get(), &Connection::receiverHandshakeRequestComplete, this, &Socket::clientHandshakeRequestComplete); From 44190f16c09fa3aae2330318c426364bbe0e8c47 Mon Sep 17 00:00:00 2001 From: Atlante45 Date: Wed, 13 Dec 2017 16:55:08 -0800 Subject: [PATCH 04/30] Reset ACK in ctor --- libraries/networking/src/udt/Connection.cpp | 7 ++----- libraries/networking/src/udt/SendQueue.cpp | 15 +++++++++++---- libraries/networking/src/udt/SendQueue.h | 4 ++-- 3 files changed, 15 insertions(+), 11 deletions(-) diff --git a/libraries/networking/src/udt/Connection.cpp b/libraries/networking/src/udt/Connection.cpp index 7a31bbeedc..5f632fa25f 100644 --- a/libraries/networking/src/udt/Connection.cpp +++ b/libraries/networking/src/udt/Connection.cpp @@ -116,14 +116,11 @@ SendQueue& Connection::getSendQueue() { if (!_hasReceivedHandshakeACK) { // First time creating a send queue for this connection - _sendQueue = SendQueue::create(_parentSocket, _destination, _initialSequenceNumber - 1); + _sendQueue = SendQueue::create(_parentSocket, _destination, _initialSequenceNumber - 1, _hasReceivedHandshakeACK); _lastReceivedACK = _sendQueue->getCurrentSequenceNumber(); } else { // Connection already has a handshake from a previous send queue - _sendQueue = SendQueue::create(_parentSocket, _destination, _lastReceivedACK); - // This connection has already gone through the handshake - // bypass it in the send queue - _sendQueue->handshakeACK(); + _sendQueue = SendQueue::create(_parentSocket, _destination, _lastReceivedACK, _hasReceivedHandshakeACK); } #ifdef UDT_CONNECTION_DEBUG diff --git a/libraries/networking/src/udt/SendQueue.cpp b/libraries/networking/src/udt/SendQueue.cpp index ef249b8f75..3dbfb35fbe 100644 --- a/libraries/networking/src/udt/SendQueue.cpp +++ b/libraries/networking/src/udt/SendQueue.cpp @@ -61,10 +61,10 @@ private: Mutex2& _mutex2; }; -std::unique_ptr SendQueue::create(Socket* socket, HifiSockAddr destination, SequenceNumber currentSequenceNumber) { +std::unique_ptr SendQueue::create(Socket* socket, HifiSockAddr destination, SequenceNumber currentSequenceNumber, bool hasReceivedHandshakeACK) { Q_ASSERT_X(socket, "SendQueue::create", "Must be called with a valid Socket*"); - auto queue = std::unique_ptr(new SendQueue(socket, destination, currentSequenceNumber)); + auto queue = std::unique_ptr(new SendQueue(socket, destination, currentSequenceNumber, hasReceivedHandshakeACK)); // Setup queue private thread QThread* thread = new QThread; @@ -83,14 +83,21 @@ std::unique_ptr SendQueue::create(Socket* socket, HifiSockAddr destin return queue; } -SendQueue::SendQueue(Socket* socket, HifiSockAddr dest, SequenceNumber currentSequenceNumber) : +SendQueue::SendQueue(Socket* socket, HifiSockAddr dest, SequenceNumber currentSequenceNumber, bool hasReceivedHandshakeACK) : _socket(socket), _destination(dest) { // set our member variables from current sequence number _currentSequenceNumber = currentSequenceNumber; _atomicCurrentSequenceNumber = uint32_t(_currentSequenceNumber); - _lastACKSequenceNumber = uint32_t(_currentSequenceNumber) - 1; + + if (hasReceivedHandshakeACK) { + _lastACKSequenceNumber = uint32_t(_currentSequenceNumber); + } else { + _lastACKSequenceNumber = uint32_t(_currentSequenceNumber - 1); + } + + _hasReceivedHandshakeACK = hasReceivedHandshakeACK; // default the last receiver response to the current time _lastReceiverResponse = QDateTime::currentMSecsSinceEpoch(); diff --git a/libraries/networking/src/udt/SendQueue.h b/libraries/networking/src/udt/SendQueue.h index b6cec15ffd..29fe6b35ff 100644 --- a/libraries/networking/src/udt/SendQueue.h +++ b/libraries/networking/src/udt/SendQueue.h @@ -50,7 +50,7 @@ public: Stopped }; - static std::unique_ptr create(Socket* socket, HifiSockAddr destination, SequenceNumber currentSequenceNumber); + static std::unique_ptr create(Socket* socket, HifiSockAddr destination, SequenceNumber currentSequenceNumber, bool hasReceivedHandshakeACK); virtual ~SendQueue(); @@ -91,7 +91,7 @@ private slots: void run(); private: - SendQueue(Socket* socket, HifiSockAddr dest, SequenceNumber currentSequenceNumber); + SendQueue(Socket* socket, HifiSockAddr dest, SequenceNumber currentSequenceNumber, bool hasReceivedHandshakeACK); SendQueue(SendQueue& other) = delete; SendQueue(SendQueue&& other) = delete; From ad547f483d76cb488f929dbc95874795e144e503 Mon Sep 17 00:00:00 2001 From: Atlante45 Date: Wed, 13 Dec 2017 19:25:24 -0800 Subject: [PATCH 05/30] Keep track of message numbers --- libraries/networking/src/udt/Connection.cpp | 10 ++++++---- libraries/networking/src/udt/Connection.h | 2 ++ libraries/networking/src/udt/PacketQueue.cpp | 2 +- libraries/networking/src/udt/PacketQueue.h | 4 +++- libraries/networking/src/udt/SendQueue.cpp | 19 +++++++++---------- libraries/networking/src/udt/SendQueue.h | 8 ++++++-- 6 files changed, 27 insertions(+), 18 deletions(-) diff --git a/libraries/networking/src/udt/Connection.cpp b/libraries/networking/src/udt/Connection.cpp index 5f632fa25f..fc590c3a66 100644 --- a/libraries/networking/src/udt/Connection.cpp +++ b/libraries/networking/src/udt/Connection.cpp @@ -90,6 +90,9 @@ void Connection::stopSendQueue() { // tell the send queue to stop and be deleted sendQueue->stop(); + + _lastMessageNumber = sendQueue->getCurrentMessageNumber(); + sendQueue->deleteLater(); // wait on the send queue thread so we know the send queue is gone @@ -116,11 +119,11 @@ SendQueue& Connection::getSendQueue() { if (!_hasReceivedHandshakeACK) { // First time creating a send queue for this connection - _sendQueue = SendQueue::create(_parentSocket, _destination, _initialSequenceNumber - 1, _hasReceivedHandshakeACK); + _sendQueue = SendQueue::create(_parentSocket, _destination, _initialSequenceNumber - 1, _lastMessageNumber, _hasReceivedHandshakeACK); _lastReceivedACK = _sendQueue->getCurrentSequenceNumber(); } else { // Connection already has a handshake from a previous send queue - _sendQueue = SendQueue::create(_parentSocket, _destination, _lastReceivedACK, _hasReceivedHandshakeACK); + _sendQueue = SendQueue::create(_parentSocket, _destination, _lastReceivedACK, _lastMessageNumber, _hasReceivedHandshakeACK); } #ifdef UDT_CONNECTION_DEBUG @@ -417,7 +420,6 @@ void Connection::sendHandshakeRequest() { } bool Connection::processReceivedSequenceNumber(SequenceNumber sequenceNumber, int packetSize, int payloadSize) { - if (!_hasReceivedHandshake) { // Refuse to process any packets until we've received the handshake // Send handshake request to re-request a handshake @@ -509,7 +511,7 @@ bool Connection::processReceivedSequenceNumber(SequenceNumber sequenceNumber, in } else { _stats.recordReceivedPackets(payloadSize, packetSize); } - + return !wasDuplicate; } diff --git a/libraries/networking/src/udt/Connection.h b/libraries/networking/src/udt/Connection.h index a13c29adc8..4f979750f9 100644 --- a/libraries/networking/src/udt/Connection.h +++ b/libraries/networking/src/udt/Connection.h @@ -137,6 +137,8 @@ private: SequenceNumber _initialSequenceNumber; // Randomized on Connection creation, identifies connection during re-connect requests SequenceNumber _initialReceiveSequenceNumber; // Randomized by peer Connection on creation, identifies connection during re-connect requests + MessageNumber _lastMessageNumber; + LossList _lossList; // List of all missing packets SequenceNumber _lastReceivedSequenceNumber; // The largest sequence number received from the peer SequenceNumber _lastReceivedACK; // The last ACK received diff --git a/libraries/networking/src/udt/PacketQueue.cpp b/libraries/networking/src/udt/PacketQueue.cpp index 9560f2f187..0560855ecb 100644 --- a/libraries/networking/src/udt/PacketQueue.cpp +++ b/libraries/networking/src/udt/PacketQueue.cpp @@ -15,7 +15,7 @@ using namespace udt; -PacketQueue::PacketQueue() { +PacketQueue::PacketQueue(MessageNumber messageNumber) : _currentMessageNumber(messageNumber) { _channels.emplace_back(new std::list()); } diff --git a/libraries/networking/src/udt/PacketQueue.h b/libraries/networking/src/udt/PacketQueue.h index 2b3d3a4b5b..bc4c5e3432 100644 --- a/libraries/networking/src/udt/PacketQueue.h +++ b/libraries/networking/src/udt/PacketQueue.h @@ -34,7 +34,7 @@ class PacketQueue { using Channels = std::vector; public: - PacketQueue(); + PacketQueue(MessageNumber messageNumber = 0); void queuePacket(PacketPointer packet); void queuePacketList(PacketListPointer packetList); @@ -42,6 +42,8 @@ public: PacketPointer takePacket(); Mutex& getLock() { return _packetsLock; } + + MessageNumber getCurrentMessageNumber() const { return _currentMessageNumber; } private: MessageNumber getNextMessageNumber(); diff --git a/libraries/networking/src/udt/SendQueue.cpp b/libraries/networking/src/udt/SendQueue.cpp index 3dbfb35fbe..43477e4562 100644 --- a/libraries/networking/src/udt/SendQueue.cpp +++ b/libraries/networking/src/udt/SendQueue.cpp @@ -61,10 +61,12 @@ private: Mutex2& _mutex2; }; -std::unique_ptr SendQueue::create(Socket* socket, HifiSockAddr destination, SequenceNumber currentSequenceNumber, bool hasReceivedHandshakeACK) { +std::unique_ptr SendQueue::create(Socket* socket, HifiSockAddr destination, SequenceNumber currentSequenceNumber, + MessageNumber currentMessageNumber, bool hasReceivedHandshakeACK) { Q_ASSERT_X(socket, "SendQueue::create", "Must be called with a valid Socket*"); - auto queue = std::unique_ptr(new SendQueue(socket, destination, currentSequenceNumber, hasReceivedHandshakeACK)); + auto queue = std::unique_ptr(new SendQueue(socket, destination, currentSequenceNumber, + currentMessageNumber, hasReceivedHandshakeACK)); // Setup queue private thread QThread* thread = new QThread; @@ -83,19 +85,16 @@ std::unique_ptr SendQueue::create(Socket* socket, HifiSockAddr destin return queue; } -SendQueue::SendQueue(Socket* socket, HifiSockAddr dest, SequenceNumber currentSequenceNumber, bool hasReceivedHandshakeACK) : +SendQueue::SendQueue(Socket* socket, HifiSockAddr dest, SequenceNumber currentSequenceNumber, + MessageNumber currentMessageNumber, bool hasReceivedHandshakeACK) : _socket(socket), - _destination(dest) + _destination(dest), + _packets(currentMessageNumber) { // set our member variables from current sequence number _currentSequenceNumber = currentSequenceNumber; _atomicCurrentSequenceNumber = uint32_t(_currentSequenceNumber); - - if (hasReceivedHandshakeACK) { - _lastACKSequenceNumber = uint32_t(_currentSequenceNumber); - } else { - _lastACKSequenceNumber = uint32_t(_currentSequenceNumber - 1); - } + _lastACKSequenceNumber = uint32_t(_currentSequenceNumber); _hasReceivedHandshakeACK = hasReceivedHandshakeACK; diff --git a/libraries/networking/src/udt/SendQueue.h b/libraries/networking/src/udt/SendQueue.h index 29fe6b35ff..b33180c1e2 100644 --- a/libraries/networking/src/udt/SendQueue.h +++ b/libraries/networking/src/udt/SendQueue.h @@ -50,7 +50,9 @@ public: Stopped }; - static std::unique_ptr create(Socket* socket, HifiSockAddr destination, SequenceNumber currentSequenceNumber, bool hasReceivedHandshakeACK); + static std::unique_ptr create(Socket* socket, HifiSockAddr destination, + SequenceNumber currentSequenceNumber, MessageNumber currentMessageNumber, + bool hasReceivedHandshakeACK); virtual ~SendQueue(); @@ -58,6 +60,7 @@ public: void queuePacketList(std::unique_ptr packetList); SequenceNumber getCurrentSequenceNumber() const { return SequenceNumber(_atomicCurrentSequenceNumber); } + MessageNumber getCurrentMessageNumber() const { return _packets.getCurrentMessageNumber(); } void setFlowWindowSize(int flowWindowSize) { _flowWindowSize = flowWindowSize; } @@ -91,7 +94,8 @@ private slots: void run(); private: - SendQueue(Socket* socket, HifiSockAddr dest, SequenceNumber currentSequenceNumber, bool hasReceivedHandshakeACK); + SendQueue(Socket* socket, HifiSockAddr dest, SequenceNumber currentSequenceNumber, + MessageNumber currentMessageNumber, bool hasReceivedHandshakeACK); SendQueue(SendQueue& other) = delete; SendQueue(SendQueue&& other) = delete; From 77b102669e454b59f394e768b1752ff831719ee9 Mon Sep 17 00:00:00 2001 From: Seth Alves Date: Tue, 12 Dec 2017 12:38:01 -0800 Subject: [PATCH 06/30] try harder to keep setTimeout loop going in controller-dispatcher --- scripts/system/controllers/controllerDispatcher.js | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/scripts/system/controllers/controllerDispatcher.js b/scripts/system/controllers/controllerDispatcher.js index 57f3b4fd8b..af3b1d2ed6 100644 --- a/scripts/system/controllers/controllerDispatcher.js +++ b/scripts/system/controllers/controllerDispatcher.js @@ -154,6 +154,15 @@ Script.include("/~/system/libraries/controllerDispatcherUtils.js"); }; this.update = function () { + try { + this.updateInternal(); + } catch (e) { + print(e); + } + Script.setTimeout(_this.update, BASIC_TIMER_INTERVAL_MS); + }; + + this.updateInternal = function () { if (PROFILE) { Script.beginProfileRange("dispatch.pre"); } @@ -376,7 +385,6 @@ Script.include("/~/system/libraries/controllerDispatcherUtils.js"); if (PROFILE) { Script.endProfileRange("dispatch.run"); } - Script.setTimeout(_this.update, BASIC_TIMER_INTERVAL_MS); }; this.setBlacklist = function() { From 9863458fef38ca0243c6d7945e16ba7a1192d6c5 Mon Sep 17 00:00:00 2001 From: Seth Alves Date: Tue, 12 Dec 2017 12:59:35 -0800 Subject: [PATCH 07/30] oops --- scripts/system/controllers/controllerDispatcher.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/system/controllers/controllerDispatcher.js b/scripts/system/controllers/controllerDispatcher.js index af3b1d2ed6..915b3b3680 100644 --- a/scripts/system/controllers/controllerDispatcher.js +++ b/scripts/system/controllers/controllerDispatcher.js @@ -155,7 +155,7 @@ Script.include("/~/system/libraries/controllerDispatcherUtils.js"); this.update = function () { try { - this.updateInternal(); + _this.updateInternal(); } catch (e) { print(e); } From 7f948586dd09afd464f7b5bb53756a29f45abd59 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Thu, 14 Dec 2017 12:07:32 -0800 Subject: [PATCH 08/30] fix message erase in conn, SendQueue ctor order --- libraries/networking/src/udt/Connection.cpp | 13 +++++++++---- libraries/networking/src/udt/Connection.h | 1 - libraries/networking/src/udt/SendQueue.cpp | 4 ++-- 3 files changed, 11 insertions(+), 7 deletions(-) diff --git a/libraries/networking/src/udt/Connection.cpp b/libraries/networking/src/udt/Connection.cpp index fc590c3a66..2f57523f79 100644 --- a/libraries/networking/src/udt/Connection.cpp +++ b/libraries/networking/src/udt/Connection.cpp @@ -193,11 +193,16 @@ void Connection::queueReceivedMessagePacket(std::unique_ptr packet) { while (pendingMessage.hasAvailablePackets()) { auto packet = pendingMessage.removeNextPacket(); - _parentSocket->messageReceived(std::move(packet)); - } - if (pendingMessage.isComplete()) { - _pendingReceivedMessages.erase(messageNumber); + auto packetPosition = packet->getPacketPosition(); + + _parentSocket->messageReceived(std::move(packet)); + + // if this was the last or only packet, then we can remove the pending message from our hash + if (packetPosition == Packet::PacketPosition::LAST || + packetPosition == Packet::PacketPosition::ONLY) { + _pendingReceivedMessages.erase(messageNumber); + } } } diff --git a/libraries/networking/src/udt/Connection.h b/libraries/networking/src/udt/Connection.h index 4f979750f9..8a96b1ded3 100644 --- a/libraries/networking/src/udt/Connection.h +++ b/libraries/networking/src/udt/Connection.h @@ -37,7 +37,6 @@ class Socket; class PendingReceivedMessage { public: void enqueuePacket(std::unique_ptr packet); - bool isComplete() const { return _hasLastPacket && _numPackets == _packets.size(); } bool hasAvailablePackets() const; std::unique_ptr removeNextPacket(); diff --git a/libraries/networking/src/udt/SendQueue.cpp b/libraries/networking/src/udt/SendQueue.cpp index 43477e4562..e7d1ab104a 100644 --- a/libraries/networking/src/udt/SendQueue.cpp +++ b/libraries/networking/src/udt/SendQueue.cpp @@ -87,9 +87,9 @@ std::unique_ptr SendQueue::create(Socket* socket, HifiSockAddr destin SendQueue::SendQueue(Socket* socket, HifiSockAddr dest, SequenceNumber currentSequenceNumber, MessageNumber currentMessageNumber, bool hasReceivedHandshakeACK) : + _packets(currentMessageNumber), _socket(socket), - _destination(dest), - _packets(currentMessageNumber) + _destination(dest) { // set our member variables from current sequence number _currentSequenceNumber = currentSequenceNumber; From 1d4801adf5edad026571b84dae96c3bce0a938b4 Mon Sep 17 00:00:00 2001 From: Nissim Hadar Date: Thu, 14 Dec 2017 13:48:31 -0800 Subject: [PATCH 09/30] Fix for Haze crash (bug 10537) --- libraries/render-utils/src/DrawHaze.cpp | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/libraries/render-utils/src/DrawHaze.cpp b/libraries/render-utils/src/DrawHaze.cpp index 4431c1bbc3..da07f5bd9b 100644 --- a/libraries/render-utils/src/DrawHaze.cpp +++ b/libraries/render-utils/src/DrawHaze.cpp @@ -169,7 +169,12 @@ void DrawHaze::run(const render::RenderContextPointer& renderContext, const Inpu auto hazeStage = args->_scene->getStage(); if (hazeStage && hazeStage->_currentFrame._hazes.size() > 0) { model::HazePointer hazePointer = hazeStage->getHaze(hazeStage->_currentFrame._hazes.front()); - batch.setUniformBuffer(HazeEffect_ParamsSlot, hazePointer->getHazeParametersBuffer()); + if (hazePointer) { + batch.setUniformBuffer(HazeEffect_ParamsSlot, hazePointer->getHazeParametersBuffer()); + } else { + // Something is wrong, so just quit Haze + return; + } } batch.setUniformBuffer(HazeEffect_TransformBufferSlot, transformBuffer->getFrameTransformBuffer()); @@ -178,7 +183,7 @@ void DrawHaze::run(const render::RenderContextPointer& renderContext, const Inpu if (lightStage) { model::LightPointer keyLight; keyLight = lightStage->getCurrentKeyLight(); - if (keyLight != nullptr) { + if (keyLight) { batch.setUniformBuffer(HazeEffect_LightingMapSlot, keyLight->getLightSchemaBuffer()); } } From 8b5e8f8d0843ead1df30949ad2900390de938a00 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Thu, 14 Dec 2017 15:30:25 -0800 Subject: [PATCH 10/30] only de-activate the SendQueue when it has nothing to send --- libraries/networking/src/udt/SendQueue.cpp | 22 ---------------------- 1 file changed, 22 deletions(-) diff --git a/libraries/networking/src/udt/SendQueue.cpp b/libraries/networking/src/udt/SendQueue.cpp index e7d1ab104a..4a0500f642 100644 --- a/libraries/networking/src/udt/SendQueue.cpp +++ b/libraries/networking/src/udt/SendQueue.cpp @@ -538,28 +538,6 @@ bool SendQueue::maybeResendPacket() { bool SendQueue::isInactive(bool attemptedToSendPacket) { // check for connection timeout first - // that will be the case if we have had 16 timeouts since hearing back from the client, and it has been - // at least 5 seconds - static const int NUM_TIMEOUTS_BEFORE_INACTIVE = 16; - static const int MIN_MS_BEFORE_INACTIVE = 5 * 1000; - - auto sinceLastResponse = (QDateTime::currentMSecsSinceEpoch() - _lastReceiverResponse); - - if (sinceLastResponse > 0 && - sinceLastResponse >= int64_t(NUM_TIMEOUTS_BEFORE_INACTIVE * (_estimatedTimeout / USECS_PER_MSEC)) && - sinceLastResponse > MIN_MS_BEFORE_INACTIVE) { - // If the flow window has been full for over CONSIDER_INACTIVE_AFTER, - // then signal the queue is inactive and return so it can be cleaned up - -#ifdef UDT_CONNECTION_DEBUG - qCDebug(networking) << "SendQueue to" << _destination << "reached" << NUM_TIMEOUTS_BEFORE_INACTIVE << "timeouts" - << "and" << MIN_MS_BEFORE_INACTIVE << "milliseconds before receiving any ACK/NAK and is now inactive. Stopping."; -#endif - - deactivate(); - return true; - } - if (!attemptedToSendPacket) { // During our processing above we didn't send any packets From da56d4730747bfa5ff8073249694cb3f9adc0a16 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Thu, 14 Dec 2017 15:30:51 -0800 Subject: [PATCH 11/30] make the _lastMessageNumber in Connection default to 0 --- libraries/networking/src/udt/Connection.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/networking/src/udt/Connection.h b/libraries/networking/src/udt/Connection.h index 8a96b1ded3..0017eb204a 100644 --- a/libraries/networking/src/udt/Connection.h +++ b/libraries/networking/src/udt/Connection.h @@ -136,7 +136,7 @@ private: SequenceNumber _initialSequenceNumber; // Randomized on Connection creation, identifies connection during re-connect requests SequenceNumber _initialReceiveSequenceNumber; // Randomized by peer Connection on creation, identifies connection during re-connect requests - MessageNumber _lastMessageNumber; + MessageNumber _lastMessageNumber { 0 }; LossList _lossList; // List of all missing packets SequenceNumber _lastReceivedSequenceNumber; // The largest sequence number received from the peer From a4696cc2b3ac311739b43266aa4f876b2b056e2d Mon Sep 17 00:00:00 2001 From: Alexander Ivash Date: Fri, 8 Dec 2017 00:20:35 +0300 Subject: [PATCH 12/30] 8992 Login window is small in tablet --- interface/resources/qml/LoginDialog/LinkAccountBody.qml | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/interface/resources/qml/LoginDialog/LinkAccountBody.qml b/interface/resources/qml/LoginDialog/LinkAccountBody.qml index 300bcd46f0..0e2f2a5282 100644 --- a/interface/resources/qml/LoginDialog/LinkAccountBody.qml +++ b/interface/resources/qml/LoginDialog/LinkAccountBody.qml @@ -52,7 +52,11 @@ Item { targetHeight += hifi.dimensions.contentSpacing.y + additionalInformation.height } - parent.width = root.width = Math.max(d.minWidth, Math.min(d.maxWidth, targetWidth)); + var newWidth = Math.max(d.minWidth, Math.min(d.maxWidth, targetWidth)); + if(!isNaN(newWidth)) { + parent.width = root.width = newWidth; + } + parent.height = root.height = Math.max(d.minHeight, Math.min(d.maxHeight, targetHeight)) + (keyboardEnabled && keyboardRaised ? (200 + 2 * hifi.dimensions.contentSpacing.y) : hifi.dimensions.contentSpacing.y); } From 355ceddcc6a0b839bb20a515fc329d6939b98e68 Mon Sep 17 00:00:00 2001 From: SamGondelman Date: Fri, 15 Dec 2017 16:23:53 -0800 Subject: [PATCH 13/30] keep _cacheFile alive --- libraries/gpu/src/gpu/Texture.h | 5 ++--- libraries/gpu/src/gpu/Texture_ktx.cpp | 29 +++++---------------------- 2 files changed, 7 insertions(+), 27 deletions(-) diff --git a/libraries/gpu/src/gpu/Texture.h b/libraries/gpu/src/gpu/Texture.h index 1877b494cf..26de5a0804 100755 --- a/libraries/gpu/src/gpu/Texture.h +++ b/libraries/gpu/src/gpu/Texture.h @@ -324,9 +324,8 @@ public: protected: std::shared_ptr maybeOpenFile() const; - mutable std::mutex _cacheFileCreateMutex; - mutable std::mutex _cacheFileWriteMutex; - mutable std::weak_ptr _cacheFile; + mutable std::mutex _cacheFileMutex; + mutable std::shared_ptr _cacheFile; std::string _filename; cache::FilePointer _cacheEntry; diff --git a/libraries/gpu/src/gpu/Texture_ktx.cpp b/libraries/gpu/src/gpu/Texture_ktx.cpp index 08fc4ec101..49e18aaf78 100644 --- a/libraries/gpu/src/gpu/Texture_ktx.cpp +++ b/libraries/gpu/src/gpu/Texture_ktx.cpp @@ -188,35 +188,17 @@ KtxStorage::KtxStorage(const std::string& filename) : _filename(filename) { } std::shared_ptr KtxStorage::maybeOpenFile() const { - // 1. Try to get the shared ptr - // 2. If it doesn't exist, grab the mutex around its creation - // 3. If it was created before we got the mutex, return it - // 4. Otherwise, create it - - std::shared_ptr file = _cacheFile.lock(); - if (file) { - return file; + if (!_cacheFile) { + _cacheFile = std::make_shared(_filename.c_str()); } - - { - std::lock_guard lock{ _cacheFileCreateMutex }; - - file = _cacheFile.lock(); - if (file) { - return file; - } - - file = std::make_shared(_filename.c_str()); - _cacheFile = file; - } - - return file; + return _cacheFile; } PixelsPointer KtxStorage::getMipFace(uint16 level, uint8 face) const { auto faceOffset = _ktxDescriptor->getMipFaceTexelsOffset(level, face); auto faceSize = _ktxDescriptor->getMipFaceTexelsSize(level, face); if (faceSize != 0 && faceOffset != 0) { + std::lock_guard lock(_cacheFileMutex); auto file = maybeOpenFile(); if (file) { auto storageView = file->createView(faceSize, faceOffset); @@ -262,6 +244,7 @@ void KtxStorage::assignMipData(uint16 level, const storage::StoragePointer& stor return; } + std::lock_guard lock(_cacheFileMutex); auto file = maybeOpenFile(); if (!file) { qWarning() << "Failed to open file to assign mip data " << QString::fromStdString(_filename); @@ -279,8 +262,6 @@ void KtxStorage::assignMipData(uint16 level, const storage::StoragePointer& stor imageData += ktx::IMAGE_SIZE_WIDTH; { - std::lock_guard lock { _cacheFileWriteMutex }; - if (level != _minMipLevelAvailable - 1) { qWarning() << "Invalid level to be stored"; return; From 90b97aba59214e9d5b5a70dd94141f95c0fb82ed Mon Sep 17 00:00:00 2001 From: SamGondelman Date: Fri, 15 Dec 2017 16:36:07 -0800 Subject: [PATCH 14/30] remove extra recycle --- .../src/display-plugins/OpenGLDisplayPlugin.cpp | 5 ----- 1 file changed, 5 deletions(-) diff --git a/libraries/display-plugins/src/display-plugins/OpenGLDisplayPlugin.cpp b/libraries/display-plugins/src/display-plugins/OpenGLDisplayPlugin.cpp index 0993daaa8b..cb9d06dce1 100644 --- a/libraries/display-plugins/src/display-plugins/OpenGLDisplayPlugin.cpp +++ b/libraries/display-plugins/src/display-plugins/OpenGLDisplayPlugin.cpp @@ -685,11 +685,6 @@ void OpenGLDisplayPlugin::present() { } incrementPresentCount(); - { - PROFILE_RANGE_EX(render, "recycle", 0xff00ff00, frameId) - _gpuContext->recycle(); - } - if (_currentFrame) { { withPresentThreadLock([&] { From 9bdcf34c9162cb3651d6eabbb32903fdec0cfa15 Mon Sep 17 00:00:00 2001 From: SamGondelman Date: Fri, 15 Dec 2017 17:52:48 -0800 Subject: [PATCH 15/30] clear all _cacheFiles on recycle --- libraries/gpu-gl/src/gpu/gl/GLBackend.cpp | 2 +- libraries/gpu/src/gpu/Texture.h | 7 ++++++- libraries/gpu/src/gpu/Texture_ktx.cpp | 18 ++++++++++++++++-- 3 files changed, 23 insertions(+), 4 deletions(-) diff --git a/libraries/gpu-gl/src/gpu/gl/GLBackend.cpp b/libraries/gpu-gl/src/gpu/gl/GLBackend.cpp index f138244fa2..0a71ee513a 100644 --- a/libraries/gpu-gl/src/gpu/gl/GLBackend.cpp +++ b/libraries/gpu-gl/src/gpu/gl/GLBackend.cpp @@ -776,7 +776,7 @@ void GLBackend::recycle() const { GLVariableAllocationSupport::manageMemory(); GLVariableAllocationSupport::_frameTexturesCreated = 0; - + Texture::KtxStorage::clearKtxFiles(); } void GLBackend::setCameraCorrection(const Mat4& correction) { diff --git a/libraries/gpu/src/gpu/Texture.h b/libraries/gpu/src/gpu/Texture.h index 26de5a0804..10edd895c0 100755 --- a/libraries/gpu/src/gpu/Texture.h +++ b/libraries/gpu/src/gpu/Texture.h @@ -321,10 +321,15 @@ public: void reset() override { } + // Don't keep files open forever. We close them at the beginning of each frame (GLBackend::recycle) + static std::vector, std::shared_ptr>> _cachedKtxFiles; + static std::mutex _cachedKtxFilesMutex; + static void clearKtxFiles(); + protected: std::shared_ptr maybeOpenFile() const; - mutable std::mutex _cacheFileMutex; + mutable std::shared_ptr _cacheFileMutex { std::make_shared() }; mutable std::shared_ptr _cacheFile; std::string _filename; diff --git a/libraries/gpu/src/gpu/Texture_ktx.cpp b/libraries/gpu/src/gpu/Texture_ktx.cpp index 49e18aaf78..46804b4b44 100644 --- a/libraries/gpu/src/gpu/Texture_ktx.cpp +++ b/libraries/gpu/src/gpu/Texture_ktx.cpp @@ -23,6 +23,9 @@ using namespace gpu; using PixelsPointer = Texture::PixelsPointer; using KtxStorage = Texture::KtxStorage; +std::vector, std::shared_ptr>> KtxStorage::_cachedKtxFiles; +std::mutex KtxStorage::_cachedKtxFilesMutex; + struct GPUKTXPayload { using Version = uint8; @@ -190,15 +193,26 @@ KtxStorage::KtxStorage(const std::string& filename) : _filename(filename) { std::shared_ptr KtxStorage::maybeOpenFile() const { if (!_cacheFile) { _cacheFile = std::make_shared(_filename.c_str()); + std::lock_guard lock(KtxStorage::_cachedKtxFilesMutex); + _cachedKtxFiles.emplace_back(_cacheFile, _cacheFileMutex); } return _cacheFile; } +void KtxStorage::clearKtxFiles() { + std::lock_guard lock(KtxStorage::_cachedKtxFilesMutex); + for (auto& cacheFileAndMutex : KtxStorage::_cachedKtxFiles) { + std::lock_guard lock(*(cacheFileAndMutex.second)); + cacheFileAndMutex.first.reset(); + } + _cachedKtxFiles.clear(); +} + PixelsPointer KtxStorage::getMipFace(uint16 level, uint8 face) const { auto faceOffset = _ktxDescriptor->getMipFaceTexelsOffset(level, face); auto faceSize = _ktxDescriptor->getMipFaceTexelsSize(level, face); if (faceSize != 0 && faceOffset != 0) { - std::lock_guard lock(_cacheFileMutex); + std::lock_guard lock(*_cacheFileMutex); auto file = maybeOpenFile(); if (file) { auto storageView = file->createView(faceSize, faceOffset); @@ -244,7 +258,7 @@ void KtxStorage::assignMipData(uint16 level, const storage::StoragePointer& stor return; } - std::lock_guard lock(_cacheFileMutex); + std::lock_guard lock(*_cacheFileMutex); auto file = maybeOpenFile(); if (!file) { qWarning() << "Failed to open file to assign mip data " << QString::fromStdString(_filename); From a39eea5a34812497203d42be1b885c504356cdce Mon Sep 17 00:00:00 2001 From: Sam Gondelman Date: Sun, 17 Dec 2017 15:56:34 -0800 Subject: [PATCH 16/30] cr and cleanup --- libraries/gpu-gl/src/gpu/gl/GLBackend.cpp | 2 +- libraries/gpu/src/gpu/Texture.h | 7 ++++--- libraries/gpu/src/gpu/Texture_ktx.cpp | 13 ++++++++----- 3 files changed, 13 insertions(+), 9 deletions(-) diff --git a/libraries/gpu-gl/src/gpu/gl/GLBackend.cpp b/libraries/gpu-gl/src/gpu/gl/GLBackend.cpp index 0a71ee513a..a36a6e0fb5 100644 --- a/libraries/gpu-gl/src/gpu/gl/GLBackend.cpp +++ b/libraries/gpu-gl/src/gpu/gl/GLBackend.cpp @@ -776,7 +776,7 @@ void GLBackend::recycle() const { GLVariableAllocationSupport::manageMemory(); GLVariableAllocationSupport::_frameTexturesCreated = 0; - Texture::KtxStorage::clearKtxFiles(); + Texture::KtxStorage::releaseOpenKtxFiles(); } void GLBackend::setCameraCorrection(const Mat4& correction) { diff --git a/libraries/gpu/src/gpu/Texture.h b/libraries/gpu/src/gpu/Texture.h index 10edd895c0..7c087fec2b 100755 --- a/libraries/gpu/src/gpu/Texture.h +++ b/libraries/gpu/src/gpu/Texture.h @@ -322,9 +322,7 @@ public: void reset() override { } // Don't keep files open forever. We close them at the beginning of each frame (GLBackend::recycle) - static std::vector, std::shared_ptr>> _cachedKtxFiles; - static std::mutex _cachedKtxFilesMutex; - static void clearKtxFiles(); + static void releaseOpenKtxFiles(); protected: std::shared_ptr maybeOpenFile() const; @@ -332,6 +330,9 @@ public: mutable std::shared_ptr _cacheFileMutex { std::make_shared() }; mutable std::shared_ptr _cacheFile; + static std::vector, std::shared_ptr>> _cachedKtxFiles; + static std::mutex _cachedKtxFilesMutex; + std::string _filename; cache::FilePointer _cacheEntry; std::atomic _minMipLevelAvailable; diff --git a/libraries/gpu/src/gpu/Texture_ktx.cpp b/libraries/gpu/src/gpu/Texture_ktx.cpp index 46804b4b44..8b054816bb 100644 --- a/libraries/gpu/src/gpu/Texture_ktx.cpp +++ b/libraries/gpu/src/gpu/Texture_ktx.cpp @@ -193,19 +193,22 @@ KtxStorage::KtxStorage(const std::string& filename) : _filename(filename) { std::shared_ptr KtxStorage::maybeOpenFile() const { if (!_cacheFile) { _cacheFile = std::make_shared(_filename.c_str()); - std::lock_guard lock(KtxStorage::_cachedKtxFilesMutex); + std::lock_guard lock(_cachedKtxFilesMutex); _cachedKtxFiles.emplace_back(_cacheFile, _cacheFileMutex); } return _cacheFile; } -void KtxStorage::clearKtxFiles() { - std::lock_guard lock(KtxStorage::_cachedKtxFilesMutex); - for (auto& cacheFileAndMutex : KtxStorage::_cachedKtxFiles) { +void KtxStorage::releaseOpenKtxFiles() { + std::vector, std::shared_ptr>> localKtxFiles; + { + std::lock_guard lock(_cachedKtxFilesMutex); + localKtxFiles.swap(_cachedKtxFiles); + } + for (auto& cacheFileAndMutex : localKtxFiles) { std::lock_guard lock(*(cacheFileAndMutex.second)); cacheFileAndMutex.first.reset(); } - _cachedKtxFiles.clear(); } PixelsPointer KtxStorage::getMipFace(uint16 level, uint8 face) const { From c39da5094ba883d3c5edcce1c1adb6207f66477e Mon Sep 17 00:00:00 2001 From: Sam Gondelman Date: Sun, 17 Dec 2017 22:09:20 -0800 Subject: [PATCH 17/30] weak_ptr _cacheFile --- libraries/gpu/src/gpu/Texture.h | 2 +- libraries/gpu/src/gpu/Texture_ktx.cpp | 22 +++++++++++++++++----- 2 files changed, 18 insertions(+), 6 deletions(-) diff --git a/libraries/gpu/src/gpu/Texture.h b/libraries/gpu/src/gpu/Texture.h index 7c087fec2b..06208179e0 100755 --- a/libraries/gpu/src/gpu/Texture.h +++ b/libraries/gpu/src/gpu/Texture.h @@ -328,7 +328,7 @@ public: std::shared_ptr maybeOpenFile() const; mutable std::shared_ptr _cacheFileMutex { std::make_shared() }; - mutable std::shared_ptr _cacheFile; + mutable std::weak_ptr _cacheFile; static std::vector, std::shared_ptr>> _cachedKtxFiles; static std::mutex _cachedKtxFilesMutex; diff --git a/libraries/gpu/src/gpu/Texture_ktx.cpp b/libraries/gpu/src/gpu/Texture_ktx.cpp index 8b054816bb..883d9abf15 100644 --- a/libraries/gpu/src/gpu/Texture_ktx.cpp +++ b/libraries/gpu/src/gpu/Texture_ktx.cpp @@ -190,13 +190,25 @@ KtxStorage::KtxStorage(const std::string& filename) : _filename(filename) { } } +// maybeOpenFile should be called with _cacheFileMutex already held to avoid modifying the file from multiple threads std::shared_ptr KtxStorage::maybeOpenFile() const { - if (!_cacheFile) { - _cacheFile = std::make_shared(_filename.c_str()); - std::lock_guard lock(_cachedKtxFilesMutex); - _cachedKtxFiles.emplace_back(_cacheFile, _cacheFileMutex); + // Try to get the shared_ptr + std::shared_ptr file = _cacheFile.lock(); + if (file) { + return file; } - return _cacheFile; + + // If the file isn't open, create it and save a weak_ptr to it + file = std::make_shared(_filename.c_str()); + _cacheFile = file; + + { + // Add the shared_ptr to the global list of open KTX files, to be released at the beginning of the next present thread frame + std::lock_guard lock(_cachedKtxFilesMutex); + _cachedKtxFiles.emplace_back(file, _cacheFileMutex); + } + + return file; } void KtxStorage::releaseOpenKtxFiles() { From b0ecaa10d4511a70ed6a80e2a659df46d84fc604 Mon Sep 17 00:00:00 2001 From: SamGondelman Date: Mon, 18 Dec 2017 12:38:25 -0800 Subject: [PATCH 18/30] fix equirectangular skybox loading --- libraries/image/src/image/Image.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/image/src/image/Image.cpp b/libraries/image/src/image/Image.cpp index 9e4ab2e43f..f78ed1a583 100644 --- a/libraries/image/src/image/Image.cpp +++ b/libraries/image/src/image/Image.cpp @@ -981,7 +981,7 @@ public: static QImage extractEquirectangularFace(const QImage& source, gpu::Texture::CubeFace face, int faceWidth) { QImage image(faceWidth, faceWidth, source.format()); - glm::vec2 dstInvSize(1.0f / (float)source.width(), 1.0f / (float)source.height()); + glm::vec2 dstInvSize(1.0f / faceWidth); struct CubeToXYZ { gpu::Texture::CubeFace _face; From 63ac426bf27c9782c0406224275862790aed1c3f Mon Sep 17 00:00:00 2001 From: Seth Alves Date: Mon, 18 Dec 2017 17:30:56 -0800 Subject: [PATCH 19/30] fix logic that resets own-avatar collisions after a parenting grab is released --- libraries/entities/src/EntityItem.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/libraries/entities/src/EntityItem.cpp b/libraries/entities/src/EntityItem.cpp index 48370b02fd..ddc5e9795f 100644 --- a/libraries/entities/src/EntityItem.cpp +++ b/libraries/entities/src/EntityItem.cpp @@ -1626,6 +1626,10 @@ void EntityItem::setParentID(const QUuid& value) { newParentNoBootstrapping |= Simulation::NO_BOOTSTRAPPING; } + if (!oldParentID.isNull() && (oldParentID == Physics::getSessionUUID() || oldParentID == AVATAR_SELF_ID)) { + oldParentNoBootstrapping |= Simulation::NO_BOOTSTRAPPING; + } + if ((bool)(oldParentNoBootstrapping ^ newParentNoBootstrapping)) { if ((bool)(newParentNoBootstrapping & Simulation::NO_BOOTSTRAPPING)) { markDirtyFlags(Simulation::NO_BOOTSTRAPPING); From 5da49d156ebdeaac49de2e6e753d06894ea4f295 Mon Sep 17 00:00:00 2001 From: Zach Fox Date: Tue, 19 Dec 2017 16:26:58 -0800 Subject: [PATCH 20/30] Commerce: Fix the behavior of the 'cancel' button in Wallet Setup --- .../qml/hifi/commerce/checkout/Checkout.qml | 4 +++- .../qml/hifi/commerce/wallet/WalletSetup.qml | 2 +- scripts/system/commerce/wallet.js | 16 +++++++++++++++- scripts/system/html/js/marketplacesInject.js | 11 +++++++---- scripts/system/marketplaces/marketplaces.js | 2 +- 5 files changed, 27 insertions(+), 8 deletions(-) diff --git a/interface/resources/qml/hifi/commerce/checkout/Checkout.qml b/interface/resources/qml/hifi/commerce/checkout/Checkout.qml index c9e5d2aaec..79332d08fa 100644 --- a/interface/resources/qml/hifi/commerce/checkout/Checkout.qml +++ b/interface/resources/qml/hifi/commerce/checkout/Checkout.qml @@ -41,6 +41,7 @@ Rectangle { property bool debugCheckoutSuccess: false; property bool canRezCertifiedItems: Entities.canRezCertified() || Entities.canRezTmpCertified(); property bool isWearable; + property string referrer; // Style color: hifi.colors.white; Hifi.QmlCommerce { @@ -131,7 +132,7 @@ Rectangle { id: notSetUpTimer; interval: 200; onTriggered: { - sendToScript({method: 'checkout_walletNotSetUp', itemId: itemId}); + sendToScript({method: 'checkout_walletNotSetUp', itemId: itemId, referrer: referrer}); } } @@ -877,6 +878,7 @@ Rectangle { itemName = message.params.itemName; root.itemPrice = message.params.itemPrice; itemHref = message.params.itemHref; + referrer = message.params.referrer; setBuyText(); break; default: diff --git a/interface/resources/qml/hifi/commerce/wallet/WalletSetup.qml b/interface/resources/qml/hifi/commerce/wallet/WalletSetup.qml index 1f9b56ca22..5d05b8fba3 100644 --- a/interface/resources/qml/hifi/commerce/wallet/WalletSetup.qml +++ b/interface/resources/qml/hifi/commerce/wallet/WalletSetup.qml @@ -252,7 +252,7 @@ Item { height: 50; text: "Cancel"; onClicked: { - sendSignalToWallet({method: 'walletSetup_cancelClicked'}); + sendSignalToWallet({method: 'walletSetup_cancelClicked', referrer: root.referrer ? root.referrer : "" }); } } } diff --git a/scripts/system/commerce/wallet.js b/scripts/system/commerce/wallet.js index a4cdc5b097..a37f2bfe91 100644 --- a/scripts/system/commerce/wallet.js +++ b/scripts/system/commerce/wallet.js @@ -61,10 +61,24 @@ function fromQml(message) { switch (message.method) { case 'passphrasePopup_cancelClicked': - case 'walletSetup_cancelClicked': case 'needsLogIn_cancelClicked': tablet.gotoHomeScreen(); break; + case 'walletSetup_cancelClicked': + switch (message.referrer) { + case '': // User clicked "Wallet" app + tablet.gotoHomeScreen(); + break; + case 'purchases': + case 'marketplace cta': + case 'mainPage': + tablet.gotoWebScreen(MARKETPLACE_URL, MARKETPLACES_INJECT_SCRIPT_URL); + break; + default: // User needs to return to an individual marketplace item URL + tablet.gotoWebScreen(MARKETPLACE_URL + '/items/' + message.referrer, MARKETPLACES_INJECT_SCRIPT_URL); + break; + } + break; case 'needsLogIn_loginClicked': openLoginWindow(); break; diff --git a/scripts/system/html/js/marketplacesInject.js b/scripts/system/html/js/marketplacesInject.js index 63d36f1a72..698dd93f29 100644 --- a/scripts/system/html/js/marketplacesInject.js +++ b/scripts/system/html/js/marketplacesInject.js @@ -243,13 +243,14 @@ }); } - function buyButtonClicked(id, name, author, price, href) { + function buyButtonClicked(id, name, author, price, href, referrer) { EventBridge.emitWebEvent(JSON.stringify({ type: "CHECKOUT", itemId: id, itemName: name, itemPrice: price ? parseInt(price, 10) : 0, - itemHref: href + itemHref: href, + referrer: referrer })); } @@ -316,7 +317,8 @@ $(this).closest('.grid-item').find('.item-title').text(), $(this).closest('.grid-item').find('.creator').find('.value').text(), $(this).closest('.grid-item').find('.item-cost').text(), - $(this).attr('data-href')); + $(this).attr('data-href'), + "mainPage"); }); } @@ -420,7 +422,8 @@ $('#top-center').find('h1').text(), $('#creator').find('.value').text(), cost, - href); + href, + "itemPage"); } }); maybeAddPurchasesButton(); diff --git a/scripts/system/marketplaces/marketplaces.js b/scripts/system/marketplaces/marketplaces.js index 004375bff7..a365730cd5 100644 --- a/scripts/system/marketplaces/marketplaces.js +++ b/scripts/system/marketplaces/marketplaces.js @@ -437,7 +437,7 @@ var selectionDisplay = null; // for gridTool.js to ignore wireEventBridge(true); tablet.sendToQml({ method: 'updateWalletReferrer', - referrer: message.itemId + referrer: message.referrer === "itemPage" ? message.itemId : message.referrer }); openWallet(); break; From a72ed27a25c5faf8034a7aa73544e5b877e75c47 Mon Sep 17 00:00:00 2001 From: Zach Fox Date: Tue, 19 Dec 2017 16:57:48 -0800 Subject: [PATCH 21/30] CR feedback --- interface/resources/qml/hifi/commerce/wallet/WalletSetup.qml | 2 +- scripts/system/commerce/wallet.js | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/interface/resources/qml/hifi/commerce/wallet/WalletSetup.qml b/interface/resources/qml/hifi/commerce/wallet/WalletSetup.qml index 5d05b8fba3..36f29b3f31 100644 --- a/interface/resources/qml/hifi/commerce/wallet/WalletSetup.qml +++ b/interface/resources/qml/hifi/commerce/wallet/WalletSetup.qml @@ -252,7 +252,7 @@ Item { height: 50; text: "Cancel"; onClicked: { - sendSignalToWallet({method: 'walletSetup_cancelClicked', referrer: root.referrer ? root.referrer : "" }); + sendSignalToWallet({method: 'walletSetup_cancelClicked', referrer: root.referrer }); } } } diff --git a/scripts/system/commerce/wallet.js b/scripts/system/commerce/wallet.js index a37f2bfe91..4ad043f71e 100644 --- a/scripts/system/commerce/wallet.js +++ b/scripts/system/commerce/wallet.js @@ -67,6 +67,8 @@ case 'walletSetup_cancelClicked': switch (message.referrer) { case '': // User clicked "Wallet" app + case undefined: + case null: tablet.gotoHomeScreen(); break; case 'purchases': From e02e6790fea7397befa2da9d624830904ab55a4b Mon Sep 17 00:00:00 2001 From: Zach Fox Date: Wed, 20 Dec 2017 13:46:02 -0800 Subject: [PATCH 22/30] Fix error state for Commerce passphrase TextInputs --- .../resources/qml/hifi/commerce/wallet/PassphraseModal.qml | 3 +++ .../resources/qml/hifi/commerce/wallet/PassphraseSelection.qml | 3 +++ 2 files changed, 6 insertions(+) diff --git a/interface/resources/qml/hifi/commerce/wallet/PassphraseModal.qml b/interface/resources/qml/hifi/commerce/wallet/PassphraseModal.qml index 379a217adb..87430246f3 100644 --- a/interface/resources/qml/hifi/commerce/wallet/PassphraseModal.qml +++ b/interface/resources/qml/hifi/commerce/wallet/PassphraseModal.qml @@ -50,9 +50,11 @@ Item { submitPassphraseInputButton.enabled = true; if (!isAuthenticated) { errorText.text = "Authentication failed - please try again."; + passphraseField.error = true; UserActivityLogger.commercePassphraseAuthenticationStatus("auth failure"); } else { sendSignalToParent({method: 'authSuccess'}); + passphraseField.error = false; UserActivityLogger.commercePassphraseAuthenticationStatus("auth success"); } } @@ -72,6 +74,7 @@ Item { // TODO: Fix this unlikely bug onVisibleChanged: { if (visible) { + passphraseField.error = false; passphraseField.focus = true; sendSignalToParent({method: 'disableHmdPreview'}); } else { diff --git a/interface/resources/qml/hifi/commerce/wallet/PassphraseSelection.qml b/interface/resources/qml/hifi/commerce/wallet/PassphraseSelection.qml index 3ed66f703b..50e58f8cc4 100644 --- a/interface/resources/qml/hifi/commerce/wallet/PassphraseSelection.qml +++ b/interface/resources/qml/hifi/commerce/wallet/PassphraseSelection.qml @@ -54,6 +54,9 @@ Item { // TODO: Fix this unlikely bug onVisibleChanged: { if (visible) { + passphraseField.error = false; + passphraseFieldAgain.error = false; + currentPassphraseField.error = false; if (root.shouldImmediatelyFocus) { focusFirstTextField(); } From 7a9d566e023dd4b7080bfc1f8d4758ef281fe56d Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Mon, 18 Dec 2017 13:56:20 -0800 Subject: [PATCH 23/30] remove unnecessary whitespace --- interface/src/LODManager.cpp | 42 ++++++++++++++++++------------------ 1 file changed, 21 insertions(+), 21 deletions(-) diff --git a/interface/src/LODManager.cpp b/interface/src/LODManager.cpp index 01ccbd0d9a..7b99c89cee 100644 --- a/interface/src/LODManager.cpp +++ b/interface/src/LODManager.cpp @@ -64,11 +64,11 @@ void LODManager::autoAdjustLOD(float batchTime, float engineRunTime, float delta // translate into fps for legacy implementation float currentFPS = (float)MSECS_PER_SECOND / _avgRenderTime; - + _fpsAverageStartWindow.updateAverage(currentFPS); _fpsAverageDownWindow.updateAverage(currentFPS); _fpsAverageUpWindow.updateAverage(currentFPS); - + quint64 now = usecTimestampNow(); quint64 elapsedSinceDownShift = now - _lastDownShift; @@ -76,22 +76,22 @@ void LODManager::autoAdjustLOD(float batchTime, float engineRunTime, float delta quint64 lastStableOrUpshift = glm::max(_lastUpShift, _lastStable); quint64 elapsedSinceStableOrUpShift = now - lastStableOrUpshift; - + if (_automaticLODAdjust) { bool changed = false; - - // LOD Downward adjustment + + // LOD Downward adjustment // If we've been downshifting, we watch a shorter downshift window so that we will quickly move toward our // target frame rate. But if we haven't just done a downshift (either because our last shift was an upshift, // or because we've just started out) then we look at a much longer window to consider whether or not to start // downshifting. - bool doDownShift = false; + bool doDownShift = false; if (_isDownshifting) { // only consider things if our DOWN_SHIFT time has elapsed... if (elapsedSinceDownShift > DOWN_SHIFT_ELPASED) { doDownShift = _fpsAverageDownWindow.getAverage() < getLODDecreaseFPS(); - + if (!doDownShift) { qCDebug(interfaceapp) << "---- WE APPEAR TO BE DONE DOWN SHIFTING -----"; _isDownshifting = false; @@ -99,10 +99,10 @@ void LODManager::autoAdjustLOD(float batchTime, float engineRunTime, float delta } } } else { - doDownShift = (elapsedSinceStableOrUpShift > START_SHIFT_ELPASED + doDownShift = (elapsedSinceStableOrUpShift > START_SHIFT_ELPASED && _fpsAverageStartWindow.getAverage() < getLODDecreaseFPS()); } - + if (doDownShift) { // Octree items... stepwise adjustment @@ -118,17 +118,17 @@ void LODManager::autoAdjustLOD(float batchTime, float engineRunTime, float delta if (_isDownshifting) { // subsequent downshift qCDebug(interfaceapp) << "adjusting LOD DOWN..." - << "average fps for last "<< DOWN_SHIFT_WINDOW_IN_SECS <<"seconds was " - << _fpsAverageDownWindow.getAverage() - << "minimum is:" << getLODDecreaseFPS() + << "average fps for last "<< DOWN_SHIFT_WINDOW_IN_SECS <<"seconds was " + << _fpsAverageDownWindow.getAverage() + << "minimum is:" << getLODDecreaseFPS() << "elapsedSinceDownShift:" << elapsedSinceDownShift << " NEW _octreeSizeScale=" << _octreeSizeScale; } else { // first downshift qCDebug(interfaceapp) << "adjusting LOD DOWN after initial delay..." - << "average fps for last "<< START_DELAY_WINDOW_IN_SECS <<"seconds was " - << _fpsAverageStartWindow.getAverage() - << "minimum is:" << getLODDecreaseFPS() + << "average fps for last "<< START_DELAY_WINDOW_IN_SECS <<"seconds was " + << _fpsAverageStartWindow.getAverage() + << "minimum is:" << getLODDecreaseFPS() << "elapsedSinceUpShift:" << elapsedSinceUpShift << " NEW _octreeSizeScale=" << _octreeSizeScale; } @@ -139,10 +139,10 @@ void LODManager::autoAdjustLOD(float batchTime, float engineRunTime, float delta emit LODDecreased(); } } else { - + // LOD Upward adjustment if (elapsedSinceUpShift > UP_SHIFT_ELPASED) { - + if (_fpsAverageUpWindow.getAverage() > getLODIncreaseFPS()) { // Octee items... stepwise adjustment @@ -158,11 +158,11 @@ void LODManager::autoAdjustLOD(float batchTime, float engineRunTime, float delta changed = true; } } - + if (changed) { - qCDebug(interfaceapp) << "adjusting LOD UP... average fps for last "<< UP_SHIFT_WINDOW_IN_SECS <<"seconds was " + qCDebug(interfaceapp) << "adjusting LOD UP... average fps for last "<< UP_SHIFT_WINDOW_IN_SECS <<"seconds was " << _fpsAverageUpWindow.getAverage() - << "upshift point is:" << getLODIncreaseFPS() + << "upshift point is:" << getLODIncreaseFPS() << "elapsedSinceUpShift:" << elapsedSinceUpShift << " NEW _octreeSizeScale=" << _octreeSizeScale; @@ -173,7 +173,7 @@ void LODManager::autoAdjustLOD(float batchTime, float engineRunTime, float delta } } } - + if (changed) { auto lodToolsDialog = DependencyManager::get()->getLodToolsDialog(); if (lodToolsDialog) { From 5c84c5f84572e1882b6cc42f552b924a3a6d2e02 Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Tue, 19 Dec 2017 13:03:36 -0800 Subject: [PATCH 24/30] measure average time of 'present' --- .../src/display-plugins/OpenGLDisplayPlugin.cpp | 2 ++ libraries/plugins/src/plugins/DisplayPlugin.h | 5 +++++ 2 files changed, 7 insertions(+) diff --git a/libraries/display-plugins/src/display-plugins/OpenGLDisplayPlugin.cpp b/libraries/display-plugins/src/display-plugins/OpenGLDisplayPlugin.cpp index cb9d06dce1..e646ba27f5 100644 --- a/libraries/display-plugins/src/display-plugins/OpenGLDisplayPlugin.cpp +++ b/libraries/display-plugins/src/display-plugins/OpenGLDisplayPlugin.cpp @@ -679,6 +679,7 @@ void OpenGLDisplayPlugin::internalPresent() { void OpenGLDisplayPlugin::present() { auto frameId = (uint64_t)presentCount(); PROFILE_RANGE_EX(render, __FUNCTION__, 0xffffff00, frameId) + uint64_t startPresent = usecTimestampNow(); { PROFILE_RANGE_EX(render, "updateFrameData", 0xff00ff00, frameId) updateFrameData(); @@ -713,6 +714,7 @@ void OpenGLDisplayPlugin::present() { gpu::Backend::freeGPUMemSize.set(gpu::gl::getFreeDedicatedMemory()); } + _movingAveragePresent.addSample((float)(usecTimestampNow() - startPresent)); } float OpenGLDisplayPlugin::newFramePresentRate() const { diff --git a/libraries/plugins/src/plugins/DisplayPlugin.h b/libraries/plugins/src/plugins/DisplayPlugin.h index efce158864..2c717f629c 100644 --- a/libraries/plugins/src/plugins/DisplayPlugin.h +++ b/libraries/plugins/src/plugins/DisplayPlugin.h @@ -21,8 +21,10 @@ #include #include +#include #include #include +#include #include #include "Plugin.h" @@ -203,6 +205,7 @@ public: virtual void cycleDebugOutput() {} void waitForPresent(); + float getAveragePresentTime() { return _movingAveragePresent.average / (float)USECS_PER_MSEC; } // in msec std::function getHUDOperator(); @@ -220,6 +223,8 @@ protected: std::function _hudOperator { std::function() }; + MovingAverage _movingAveragePresent; + private: QMutex _presentMutex; QWaitCondition _presentCondition; From 8e0d367a2d7fa0a8a6636dfd82153794c7aa96ea Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Tue, 19 Dec 2017 13:13:13 -0800 Subject: [PATCH 25/30] use full 'present' time for FPS calculation also, refactor LOD adjust logic --- interface/src/Application.cpp | 4 +- interface/src/LODManager.cpp | 189 ++++++++++------------------------ interface/src/LODManager.h | 42 ++------ 3 files changed, 68 insertions(+), 167 deletions(-) diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp index 81ed5a0578..174424f4ca 100644 --- a/interface/src/Application.cpp +++ b/interface/src/Application.cpp @@ -4323,9 +4323,9 @@ void Application::updateLOD(float deltaTime) const { PerformanceTimer perfTimer("LOD"); // adjust it unless we were asked to disable this feature, or if we're currently in throttleRendering mode if (!isThrottleRendering()) { - float batchTime = (float)_gpuContext->getFrameTimerBatchAverage(); + float presentTime = getActiveDisplayPlugin()->getAveragePresentTime(); float engineRunTime = (float)(_renderEngine->getConfiguration().get()->getCPURunTime()); - DependencyManager::get()->autoAdjustLOD(batchTime, engineRunTime, deltaTime); + DependencyManager::get()->autoAdjustLOD(presentTime, engineRunTime, deltaTime); } else { DependencyManager::get()->resetLODAdjust(); } diff --git a/interface/src/LODManager.cpp b/interface/src/LODManager.cpp index 7b99c89cee..c416e6a6c7 100644 --- a/interface/src/LODManager.cpp +++ b/interface/src/LODManager.cpp @@ -19,6 +19,11 @@ #include "LODManager.h" +const uint64_t LOD_AUTO_ADJUST_PERIOD = 500 * USECS_PER_MSEC; +const float LOD_AUTO_ADJUST_DECREMENT_FACTOR = 0.8f; +const float LOD_AUTO_ADJUST_INCREMENT_FACTOR = 1.2f; + + Setting::Handle desktopLODDecreaseFPS("desktopLODDecreaseFPS", DEFAULT_DESKTOP_LOD_DOWN_FPS); Setting::Handle hmdLODDecreaseFPS("hmdLODDecreaseFPS", DEFAULT_HMD_LOD_DOWN_FPS); @@ -39,156 +44,74 @@ float LODManager::getLODIncreaseFPS() { return getDesktopLODIncreaseFPS(); } -void LODManager::autoAdjustLOD(float batchTime, float engineRunTime, float deltaTimeSec) { - - // NOTE: our first ~100 samples at app startup are completely all over the place, and we don't - // really want to count them in our average, so we will ignore the real frame rates and stuff - // our moving average with simulated good data - const int IGNORE_THESE_SAMPLES = 100; - if (_fpsAverageUpWindow.getSampleCount() < IGNORE_THESE_SAMPLES) { - _lastStable = _lastUpShift = _lastDownShift = usecTimestampNow(); - } - +void LODManager::autoAdjustLOD(float presentTime, float engineRunTime, float deltaTimeSec) { // compute time-weighted running average renderTime - const float OVERLAY_AND_SWAP_TIME_BUDGET = 2.0f; // msec - float renderTime = batchTime + OVERLAY_AND_SWAP_TIME_BUDGET; - float maxTime = glm::max(renderTime, engineRunTime); - const float BLEND_TIMESCALE = 0.3f; // sec - const float MIN_DELTA_TIME = 0.001f; - const float safeDeltaTime = glm::max(deltaTimeSec, MIN_DELTA_TIME); - float blend = BLEND_TIMESCALE / safeDeltaTime; - if (blend > 1.0f) { - blend = 1.0f; - } + float maxTime = glm::max(presentTime, engineRunTime); + const float LOD_ADJUST_TIMESCALE = 0.1f; // sec + float blend = (deltaTimeSec < LOD_ADJUST_TIMESCALE) ? deltaTimeSec / LOD_ADJUST_TIMESCALE : 1.0f; _avgRenderTime = (1.0f - blend) * _avgRenderTime + blend * maxTime; // msec + if (!_automaticLODAdjust) { + // early exit + return; + } - // translate into fps for legacy implementation + float oldOctreeSizeScale = _octreeSizeScale; float currentFPS = (float)MSECS_PER_SECOND / _avgRenderTime; - - _fpsAverageStartWindow.updateAverage(currentFPS); - _fpsAverageDownWindow.updateAverage(currentFPS); - _fpsAverageUpWindow.updateAverage(currentFPS); - - quint64 now = usecTimestampNow(); - - quint64 elapsedSinceDownShift = now - _lastDownShift; - quint64 elapsedSinceUpShift = now - _lastUpShift; - - quint64 lastStableOrUpshift = glm::max(_lastUpShift, _lastStable); - quint64 elapsedSinceStableOrUpShift = now - lastStableOrUpshift; - - if (_automaticLODAdjust) { - bool changed = false; - - // LOD Downward adjustment - // If we've been downshifting, we watch a shorter downshift window so that we will quickly move toward our - // target frame rate. But if we haven't just done a downshift (either because our last shift was an upshift, - // or because we've just started out) then we look at a much longer window to consider whether or not to start - // downshifting. - bool doDownShift = false; - - if (_isDownshifting) { - // only consider things if our DOWN_SHIFT time has elapsed... - if (elapsedSinceDownShift > DOWN_SHIFT_ELPASED) { - doDownShift = _fpsAverageDownWindow.getAverage() < getLODDecreaseFPS(); - - if (!doDownShift) { - qCDebug(interfaceapp) << "---- WE APPEAR TO BE DONE DOWN SHIFTING -----"; - _isDownshifting = false; - _lastStable = now; - } - } - } else { - doDownShift = (elapsedSinceStableOrUpShift > START_SHIFT_ELPASED - && _fpsAverageStartWindow.getAverage() < getLODDecreaseFPS()); - } - - if (doDownShift) { - - // Octree items... stepwise adjustment + uint64_t now = usecTimestampNow(); + if (currentFPS < getLODDecreaseFPS()) { + if (now > _decreaseFPSExpiry) { + _decreaseFPSExpiry = now + LOD_AUTO_ADJUST_PERIOD; if (_octreeSizeScale > ADJUST_LOD_MIN_SIZE_SCALE) { - _octreeSizeScale *= ADJUST_LOD_DOWN_BY; + _octreeSizeScale *= LOD_AUTO_ADJUST_DECREMENT_FACTOR; if (_octreeSizeScale < ADJUST_LOD_MIN_SIZE_SCALE) { _octreeSizeScale = ADJUST_LOD_MIN_SIZE_SCALE; } - changed = true; + qCDebug(interfaceapp) << "adjusting LOD UP" + << "fps =" << currentFPS + << "targetFPS =" << getLODDecreaseFPS(); } - - if (changed) { - if (_isDownshifting) { - // subsequent downshift - qCDebug(interfaceapp) << "adjusting LOD DOWN..." - << "average fps for last "<< DOWN_SHIFT_WINDOW_IN_SECS <<"seconds was " - << _fpsAverageDownWindow.getAverage() - << "minimum is:" << getLODDecreaseFPS() - << "elapsedSinceDownShift:" << elapsedSinceDownShift - << " NEW _octreeSizeScale=" << _octreeSizeScale; + _decreaseFPSExpiry = now + LOD_AUTO_ADJUST_PERIOD; + } + _increaseFPSExpiry = now + LOD_AUTO_ADJUST_PERIOD; + } else if (currentFPS > getLODIncreaseFPS()) { + if (now > _increaseFPSExpiry) { + _increaseFPSExpiry = now + LOD_AUTO_ADJUST_PERIOD; + if (_octreeSizeScale < ADJUST_LOD_MAX_SIZE_SCALE) { + if (_octreeSizeScale < ADJUST_LOD_MIN_SIZE_SCALE) { + _octreeSizeScale = ADJUST_LOD_MIN_SIZE_SCALE; } else { - // first downshift - qCDebug(interfaceapp) << "adjusting LOD DOWN after initial delay..." - << "average fps for last "<< START_DELAY_WINDOW_IN_SECS <<"seconds was " - << _fpsAverageStartWindow.getAverage() - << "minimum is:" << getLODDecreaseFPS() - << "elapsedSinceUpShift:" << elapsedSinceUpShift - << " NEW _octreeSizeScale=" << _octreeSizeScale; - } - - _lastDownShift = now; - _isDownshifting = true; - - emit LODDecreased(); - } - } else { - - // LOD Upward adjustment - if (elapsedSinceUpShift > UP_SHIFT_ELPASED) { - - if (_fpsAverageUpWindow.getAverage() > getLODIncreaseFPS()) { - - // Octee items... stepwise adjustment - if (_octreeSizeScale < ADJUST_LOD_MAX_SIZE_SCALE) { - if (_octreeSizeScale < ADJUST_LOD_MIN_SIZE_SCALE) { - _octreeSizeScale = ADJUST_LOD_MIN_SIZE_SCALE; - } else { - _octreeSizeScale *= ADJUST_LOD_UP_BY; - } - if (_octreeSizeScale > ADJUST_LOD_MAX_SIZE_SCALE) { - _octreeSizeScale = ADJUST_LOD_MAX_SIZE_SCALE; - } - changed = true; - } - } - - if (changed) { - qCDebug(interfaceapp) << "adjusting LOD UP... average fps for last "<< UP_SHIFT_WINDOW_IN_SECS <<"seconds was " - << _fpsAverageUpWindow.getAverage() - << "upshift point is:" << getLODIncreaseFPS() - << "elapsedSinceUpShift:" << elapsedSinceUpShift - << " NEW _octreeSizeScale=" << _octreeSizeScale; - - _lastUpShift = now; - _isDownshifting = false; - - emit LODIncreased(); + _octreeSizeScale *= LOD_AUTO_ADJUST_INCREMENT_FACTOR; } + if (_octreeSizeScale > ADJUST_LOD_MAX_SIZE_SCALE) { + _octreeSizeScale = ADJUST_LOD_MAX_SIZE_SCALE; + } + qCDebug(interfaceapp) << "adjusting LOD DOWN" + << "fps =" << currentFPS + << "targetFPS =" << getLODDecreaseFPS(); } + _increaseFPSExpiry = now + LOD_AUTO_ADJUST_PERIOD; } + _decreaseFPSExpiry = now + LOD_AUTO_ADJUST_PERIOD; + } else { + _increaseFPSExpiry = now + LOD_AUTO_ADJUST_PERIOD; + _decreaseFPSExpiry = _increaseFPSExpiry; + } - if (changed) { - auto lodToolsDialog = DependencyManager::get()->getLodToolsDialog(); - if (lodToolsDialog) { - lodToolsDialog->reloadSliders(); - } + if (oldOctreeSizeScale != _octreeSizeScale) { + auto lodToolsDialog = DependencyManager::get()->getLodToolsDialog(); + if (lodToolsDialog) { + lodToolsDialog->reloadSliders(); } + // Assuming the LOD adjustment will work: we optimistically reset _avgRenderTime + // to be at middle of target zone. It will drift close to its true value within + // about three few LOD_ADJUST_TIMESCALEs and we'll adjust again as necessary. + float expectedFPS = 0.5f * (getLODIncreaseFPS() + getLODDecreaseFPS()); + _avgRenderTime = MSECS_PER_SECOND / expectedFPS; } } void LODManager::resetLODAdjust() { - _fpsAverageStartWindow.reset(); - _fpsAverageDownWindow.reset(); - _fpsAverageUpWindow.reset(); - _lastUpShift = _lastDownShift = usecTimestampNow(); - _isDownshifting = false; + _decreaseFPSExpiry = _increaseFPSExpiry = usecTimestampNow() + LOD_AUTO_ADJUST_PERIOD; } const float MIN_DECREASE_FPS = 0.5f; @@ -206,7 +129,7 @@ float LODManager::getDesktopLODDecreaseFPS() const { } float LODManager::getDesktopLODIncreaseFPS() const { - return glm::max(((float)MSECS_PER_SECOND / _desktopMaxRenderTime) + INCREASE_LOD_GAP, MAX_LIKELY_DESKTOP_FPS); + return glm::max(((float)MSECS_PER_SECOND / _desktopMaxRenderTime) + INCREASE_LOD_GAP_FPS, MAX_LIKELY_DESKTOP_FPS); } void LODManager::setHMDLODDecreaseFPS(float fps) { @@ -222,7 +145,7 @@ float LODManager::getHMDLODDecreaseFPS() const { } float LODManager::getHMDLODIncreaseFPS() const { - return glm::max(((float)MSECS_PER_SECOND / _hmdMaxRenderTime) + INCREASE_LOD_GAP, MAX_LIKELY_HMD_FPS); + return glm::max(((float)MSECS_PER_SECOND / _hmdMaxRenderTime) + INCREASE_LOD_GAP_FPS, MAX_LIKELY_HMD_FPS); } QString LODManager::getLODFeedbackText() { diff --git a/interface/src/LODManager.h b/interface/src/LODManager.h index 1b3797a0ca..4ec36e613b 100644 --- a/interface/src/LODManager.h +++ b/interface/src/LODManager.h @@ -19,29 +19,13 @@ #include #include -const float DEFAULT_DESKTOP_LOD_DOWN_FPS = 20.0; -const float DEFAULT_HMD_LOD_DOWN_FPS = 20.0; +const float DEFAULT_DESKTOP_LOD_DOWN_FPS = 20.0f; +const float DEFAULT_HMD_LOD_DOWN_FPS = 45.0f; const float DEFAULT_DESKTOP_MAX_RENDER_TIME = (float)MSECS_PER_SECOND / DEFAULT_DESKTOP_LOD_DOWN_FPS; // msec const float DEFAULT_HMD_MAX_RENDER_TIME = (float)MSECS_PER_SECOND / DEFAULT_HMD_LOD_DOWN_FPS; // msec -const float MAX_LIKELY_DESKTOP_FPS = 59.0; // this is essentially, V-synch - 1 fps -const float MAX_LIKELY_HMD_FPS = 74.0; // this is essentially, V-synch - 1 fps -const float INCREASE_LOD_GAP = 15.0f; - -const float START_DELAY_WINDOW_IN_SECS = 3.0f; // wait at least this long after steady state/last upshift to consider downshifts -const float DOWN_SHIFT_WINDOW_IN_SECS = 0.5f; -const float UP_SHIFT_WINDOW_IN_SECS = 2.5f; - -const int ASSUMED_FPS = 60; -const quint64 START_SHIFT_ELPASED = USECS_PER_SECOND * START_DELAY_WINDOW_IN_SECS; -const quint64 DOWN_SHIFT_ELPASED = USECS_PER_SECOND * DOWN_SHIFT_WINDOW_IN_SECS; // Consider adjusting LOD down after half a second -const quint64 UP_SHIFT_ELPASED = USECS_PER_SECOND * UP_SHIFT_WINDOW_IN_SECS; - -const int START_DELAY_SAMPLES_OF_FRAMES = ASSUMED_FPS * START_DELAY_WINDOW_IN_SECS; -const int DOWN_SHIFT_SAMPLES_OF_FRAMES = ASSUMED_FPS * DOWN_SHIFT_WINDOW_IN_SECS; -const int UP_SHIFT_SAMPLES_OF_FRAMES = ASSUMED_FPS * UP_SHIFT_WINDOW_IN_SECS; - -const float ADJUST_LOD_DOWN_BY = 0.9f; -const float ADJUST_LOD_UP_BY = 1.1f; +const float MAX_LIKELY_DESKTOP_FPS = 59.0f; // this is essentially, V-synch - 1 fps +const float MAX_LIKELY_HMD_FPS = 74.0f; // this is essentially, V-synch - 1 fps +const float INCREASE_LOD_GAP_FPS = 15.0f; // fps // The default value DEFAULT_OCTREE_SIZE_SCALE means you can be 400 meters away from a 1 meter object in order to see it (which is ~20:20 vision). const float ADJUST_LOD_MAX_SIZE_SCALE = DEFAULT_OCTREE_SIZE_SCALE; @@ -78,7 +62,7 @@ public: Q_INVOKABLE float getLODIncreaseFPS(); static bool shouldRender(const RenderArgs* args, const AABox& bounds); - void autoAdjustLOD(float batchTime, float engineRunTime, float deltaTimeSec); + void autoAdjustLOD(float presentTime, float engineRunTime, float deltaTimeSec); void loadSettings(); void saveSettings(); @@ -92,21 +76,15 @@ private: LODManager(); bool _automaticLODAdjust = true; - float _avgRenderTime { 0.0 }; + float _avgRenderTime { 0.0f }; float _desktopMaxRenderTime { DEFAULT_DESKTOP_MAX_RENDER_TIME }; float _hmdMaxRenderTime { DEFAULT_HMD_MAX_RENDER_TIME }; float _octreeSizeScale = DEFAULT_OCTREE_SIZE_SCALE; int _boundaryLevelAdjust = 0; - - quint64 _lastDownShift = 0; - quint64 _lastUpShift = 0; - quint64 _lastStable = 0; - bool _isDownshifting = false; // start out as if we're not downshifting - - SimpleMovingAverage _fpsAverageStartWindow = START_DELAY_SAMPLES_OF_FRAMES; - SimpleMovingAverage _fpsAverageDownWindow = DOWN_SHIFT_SAMPLES_OF_FRAMES; - SimpleMovingAverage _fpsAverageUpWindow = UP_SHIFT_SAMPLES_OF_FRAMES; + + uint64_t _decreaseFPSExpiry { 0 }; + uint64_t _increaseFPSExpiry { 0 }; }; #endif // hifi_LODManager_h From 9c4a0c8c702b6f3565c2c1c1945aa058503d48e4 Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Tue, 19 Dec 2017 14:12:46 -0800 Subject: [PATCH 26/30] tweak LOD adjustment logging --- interface/src/LODManager.cpp | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/interface/src/LODManager.cpp b/interface/src/LODManager.cpp index c416e6a6c7..77aabccb78 100644 --- a/interface/src/LODManager.cpp +++ b/interface/src/LODManager.cpp @@ -66,9 +66,10 @@ void LODManager::autoAdjustLOD(float presentTime, float engineRunTime, float del if (_octreeSizeScale < ADJUST_LOD_MIN_SIZE_SCALE) { _octreeSizeScale = ADJUST_LOD_MIN_SIZE_SCALE; } - qCDebug(interfaceapp) << "adjusting LOD UP" + qCDebug(interfaceapp) << "adjusting LOD DOWN" << "fps =" << currentFPS - << "targetFPS =" << getLODDecreaseFPS(); + << "targetFPS =" << getLODDecreaseFPS() + << "octreeSizeScale =" << _octreeSizeScale; } _decreaseFPSExpiry = now + LOD_AUTO_ADJUST_PERIOD; } @@ -85,9 +86,10 @@ void LODManager::autoAdjustLOD(float presentTime, float engineRunTime, float del if (_octreeSizeScale > ADJUST_LOD_MAX_SIZE_SCALE) { _octreeSizeScale = ADJUST_LOD_MAX_SIZE_SCALE; } - qCDebug(interfaceapp) << "adjusting LOD DOWN" + qCDebug(interfaceapp) << "adjusting LOD UP" << "fps =" << currentFPS - << "targetFPS =" << getLODDecreaseFPS(); + << "targetFPS =" << getLODDecreaseFPS() + << "octreeSizeScale =" << _octreeSizeScale; } _increaseFPSExpiry = now + LOD_AUTO_ADJUST_PERIOD; } From 4fb9680f3b96654675cff9fcec91b6efcb64755f Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Tue, 19 Dec 2017 16:43:49 -0800 Subject: [PATCH 27/30] also consider GPU render time for LOD adjust --- interface/src/Application.cpp | 4 +++- interface/src/LODManager.cpp | 5 ++--- interface/src/LODManager.h | 2 +- 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp index 174424f4ca..f17e06cb35 100644 --- a/interface/src/Application.cpp +++ b/interface/src/Application.cpp @@ -4325,7 +4325,9 @@ void Application::updateLOD(float deltaTime) const { if (!isThrottleRendering()) { float presentTime = getActiveDisplayPlugin()->getAveragePresentTime(); float engineRunTime = (float)(_renderEngine->getConfiguration().get()->getCPURunTime()); - DependencyManager::get()->autoAdjustLOD(presentTime, engineRunTime, deltaTime); + float gpuTime = getGPUContext()->getFrameTimerGPUAverage(); + float maxRenderTime = glm::max(gpuTime, glm::max(presentTime, engineRunTime)); + DependencyManager::get()->autoAdjustLOD(maxRenderTime, deltaTime); } else { DependencyManager::get()->resetLODAdjust(); } diff --git a/interface/src/LODManager.cpp b/interface/src/LODManager.cpp index 77aabccb78..c369920cf3 100644 --- a/interface/src/LODManager.cpp +++ b/interface/src/LODManager.cpp @@ -44,12 +44,11 @@ float LODManager::getLODIncreaseFPS() { return getDesktopLODIncreaseFPS(); } -void LODManager::autoAdjustLOD(float presentTime, float engineRunTime, float deltaTimeSec) { +void LODManager::autoAdjustLOD(float renderTime, float deltaTimeSec) { // compute time-weighted running average renderTime - float maxTime = glm::max(presentTime, engineRunTime); const float LOD_ADJUST_TIMESCALE = 0.1f; // sec float blend = (deltaTimeSec < LOD_ADJUST_TIMESCALE) ? deltaTimeSec / LOD_ADJUST_TIMESCALE : 1.0f; - _avgRenderTime = (1.0f - blend) * _avgRenderTime + blend * maxTime; // msec + _avgRenderTime = (1.0f - blend) * _avgRenderTime + blend * renderTime; // msec if (!_automaticLODAdjust) { // early exit return; diff --git a/interface/src/LODManager.h b/interface/src/LODManager.h index 4ec36e613b..eadb3f2e5c 100644 --- a/interface/src/LODManager.h +++ b/interface/src/LODManager.h @@ -62,7 +62,7 @@ public: Q_INVOKABLE float getLODIncreaseFPS(); static bool shouldRender(const RenderArgs* args, const AABox& bounds); - void autoAdjustLOD(float presentTime, float engineRunTime, float deltaTimeSec); + void autoAdjustLOD(float renderTime, float deltaTimeSec); void loadSettings(); void saveSettings(); From 5c11ae346962639c57fb8a89fc1b4979cec5a079 Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Tue, 19 Dec 2017 16:50:31 -0800 Subject: [PATCH 28/30] bump desktop min FPS from 20 to 30 msec --- interface/src/LODManager.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/interface/src/LODManager.h b/interface/src/LODManager.h index eadb3f2e5c..374f6a2e73 100644 --- a/interface/src/LODManager.h +++ b/interface/src/LODManager.h @@ -19,7 +19,7 @@ #include #include -const float DEFAULT_DESKTOP_LOD_DOWN_FPS = 20.0f; +const float DEFAULT_DESKTOP_LOD_DOWN_FPS = 30.0f; const float DEFAULT_HMD_LOD_DOWN_FPS = 45.0f; const float DEFAULT_DESKTOP_MAX_RENDER_TIME = (float)MSECS_PER_SECOND / DEFAULT_DESKTOP_LOD_DOWN_FPS; // msec const float DEFAULT_HMD_MAX_RENDER_TIME = (float)MSECS_PER_SECOND / DEFAULT_HMD_LOD_DOWN_FPS; // msec From 721e9d426aa3462c63e141ff55ebe5c874bde722 Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Wed, 20 Dec 2017 09:14:53 -0800 Subject: [PATCH 29/30] restore signals on LOD change --- interface/src/LODManager.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/interface/src/LODManager.cpp b/interface/src/LODManager.cpp index c369920cf3..714cc31f13 100644 --- a/interface/src/LODManager.cpp +++ b/interface/src/LODManager.cpp @@ -69,6 +69,7 @@ void LODManager::autoAdjustLOD(float renderTime, float deltaTimeSec) { << "fps =" << currentFPS << "targetFPS =" << getLODDecreaseFPS() << "octreeSizeScale =" << _octreeSizeScale; + emit LODDecreased(); } _decreaseFPSExpiry = now + LOD_AUTO_ADJUST_PERIOD; } @@ -89,6 +90,7 @@ void LODManager::autoAdjustLOD(float renderTime, float deltaTimeSec) { << "fps =" << currentFPS << "targetFPS =" << getLODDecreaseFPS() << "octreeSizeScale =" << _octreeSizeScale; + emit LODIncreased(); } _increaseFPSExpiry = now + LOD_AUTO_ADJUST_PERIOD; } From c1906215ba455113fc7c24ad3d89f21b76782b62 Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Wed, 20 Dec 2017 11:36:51 -0800 Subject: [PATCH 30/30] add comments --- interface/src/LODManager.cpp | 28 +++++++++++++++++++++------- interface/src/LODManager.h | 2 +- 2 files changed, 22 insertions(+), 8 deletions(-) diff --git a/interface/src/LODManager.cpp b/interface/src/LODManager.cpp index 714cc31f13..9e6fabd439 100644 --- a/interface/src/LODManager.cpp +++ b/interface/src/LODManager.cpp @@ -19,10 +19,6 @@ #include "LODManager.h" -const uint64_t LOD_AUTO_ADJUST_PERIOD = 500 * USECS_PER_MSEC; -const float LOD_AUTO_ADJUST_DECREMENT_FACTOR = 0.8f; -const float LOD_AUTO_ADJUST_INCREMENT_FACTOR = 1.2f; - Setting::Handle desktopLODDecreaseFPS("desktopLODDecreaseFPS", DEFAULT_DESKTOP_LOD_DOWN_FPS); Setting::Handle hmdLODDecreaseFPS("hmdLODDecreaseFPS", DEFAULT_HMD_LOD_DOWN_FPS); @@ -44,10 +40,28 @@ float LODManager::getLODIncreaseFPS() { return getDesktopLODIncreaseFPS(); } -void LODManager::autoAdjustLOD(float renderTime, float deltaTimeSec) { +// We use a "time-weighted running average" of the renderTime and compare it against min/max thresholds +// to determine if we should adjust the level of detail (LOD). +// +// A time-weighted running average has a timescale which determines how fast the average tracks the measured +// value in real-time. Given a step-function in the mesured value, and assuming measurements happen +// faster than the runningAverage is computed, the error between the value and its runningAverage will be +// reduced by 1/e every timescale of real-time that passes. +const float LOD_ADJUST_RUNNING_AVG_TIMESCALE = 0.1f; // sec +// +// Assuming the measured value is affected by logic invoked by the runningAverage bumping up against its +// thresholds, we expect the adjustment to introduce a step-function. We want the runningAverage settle +// to the new value BEFORE we test it aginst its thresholds again. Hence we test on a period that is a few +// multiples of the running average timescale: +const uint64_t LOD_AUTO_ADJUST_PERIOD = 5 * (uint64_t)(LOD_ADJUST_RUNNING_AVG_TIMESCALE * (float)USECS_PER_MSEC); // usec + +const float LOD_AUTO_ADJUST_DECREMENT_FACTOR = 0.8f; +const float LOD_AUTO_ADJUST_INCREMENT_FACTOR = 1.2f; + +void LODManager::autoAdjustLOD(float renderTime, float realTimeDelta) { // compute time-weighted running average renderTime - const float LOD_ADJUST_TIMESCALE = 0.1f; // sec - float blend = (deltaTimeSec < LOD_ADJUST_TIMESCALE) ? deltaTimeSec / LOD_ADJUST_TIMESCALE : 1.0f; + // Note: we MUST clamp the blend to 1.0 for stability + float blend = (realTimeDelta < LOD_ADJUST_RUNNING_AVG_TIMESCALE) ? realTimeDelta / LOD_ADJUST_RUNNING_AVG_TIMESCALE : 1.0f; _avgRenderTime = (1.0f - blend) * _avgRenderTime + blend * renderTime; // msec if (!_automaticLODAdjust) { // early exit diff --git a/interface/src/LODManager.h b/interface/src/LODManager.h index 374f6a2e73..cf38342db0 100644 --- a/interface/src/LODManager.h +++ b/interface/src/LODManager.h @@ -62,7 +62,7 @@ public: Q_INVOKABLE float getLODIncreaseFPS(); static bool shouldRender(const RenderArgs* args, const AABox& bounds); - void autoAdjustLOD(float renderTime, float deltaTimeSec); + void autoAdjustLOD(float renderTime, float realTimeDelta); void loadSettings(); void saveSettings();