From 9ed418f7b612030cf6d2b825f043456319b99af1 Mon Sep 17 00:00:00 2001 From: Howard Stearns Date: Mon, 17 Aug 2015 19:19:11 -0700 Subject: [PATCH 01/22] Simple frequencey decoupling. Still same thread. --- interface/src/Application.cpp | 23 +++++++++++++++++------ interface/src/Application.h | 3 +++ 2 files changed, 20 insertions(+), 6 deletions(-) diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp index 621210cc4e..b89fc35f2c 100644 --- a/interface/src/Application.cpp +++ b/interface/src/Application.cpp @@ -2289,6 +2289,7 @@ QVector Application::pasteEntities(float x, float y, float z) { void Application::initDisplay() { } +static QTimer* avatarTimer = NULL; void Application::init() { // Make sure Login state is up to date DependencyManager::get()->toggleLoginDialog(); @@ -2351,6 +2352,11 @@ void Application::init() { // Make sure any new sounds are loaded as soon as know about them. connect(tree, &EntityTree::newCollisionSoundURL, DependencyManager::get().data(), &SoundCache::getSound); connect(_myAvatar, &MyAvatar::newCollisionSoundURL, DependencyManager::get().data(), &SoundCache::getSound); + + const qint64 AVATAR_UPDATE_INTERVAL_MSECS = 1000 / 55; + avatarTimer = new QTimer(this); + connect(avatarTimer, &QTimer::timeout, this, &Application::avatarUpdate); + avatarTimer->start(AVATAR_UPDATE_INTERVAL_MSECS); } void Application::closeMirrorView() { @@ -2415,6 +2421,16 @@ void Application::updateMouseRay() { } } +void Application::avatarUpdate() { + PerformanceTimer perfTimer("myAvatar"); + qint64 now = usecTimestampNow(); + float deltaTime = (now - _lastAvatarUpdate) / (1000.0f * 1000.0f); + _lastAvatarUpdate = now; + updateMyAvatarLookAtPosition(); + // Sample hardware, update view frustum if needed, and send avatar data to mixer/nodes + DependencyManager::get()->updateMyAvatar(deltaTime); +} + // Called during Application::update immediately before AvatarManager::updateMyAvatar, updating my data that is then sent to everyone. // (Maybe this code should be moved there?) // The principal result is to call updateLookAtTargetAvatar() and then setLookAtPosition(). @@ -2787,12 +2803,7 @@ void Application::update(float deltaTime) { _overlays.update(deltaTime); } - { - PerformanceTimer perfTimer("myAvatar"); - updateMyAvatarLookAtPosition(); - // Sample hardware, update view frustum if needed, and send avatar data to mixer/nodes - DependencyManager::get()->updateMyAvatar(deltaTime); - } + avatarUpdate(); { PerformanceTimer perfTimer("emitSimulating"); diff --git a/interface/src/Application.h b/interface/src/Application.h index e0d4fa559d..9e1e5e8239 100644 --- a/interface/src/Application.h +++ b/interface/src/Application.h @@ -681,6 +681,9 @@ private: EntityItemID _keyboardFocusedItem; quint64 _lastAcceptedKeyPress = 0; + + void avatarUpdate(); + quint64 _lastAvatarUpdate = 0; }; #endif // hifi_Application_h From 2819f3ac1bfae2a2e97327663b7e60a78233b178 Mon Sep 17 00:00:00 2001 From: Howard Stearns Date: Tue, 18 Aug 2015 16:39:30 -0700 Subject: [PATCH 02/22] constant --- interface/src/avatar/AvatarUpdate.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/interface/src/avatar/AvatarUpdate.cpp b/interface/src/avatar/AvatarUpdate.cpp index c14fcdd6f5..c997a76132 100644 --- a/interface/src/avatar/AvatarUpdate.cpp +++ b/interface/src/avatar/AvatarUpdate.cpp @@ -84,7 +84,7 @@ void AvatarUpdate::avatarUpdateIfSynchronous() { void AvatarUpdate::avatarUpdate() { PerformanceTimer perfTimer("AvatarUpdate"); quint64 now = usecTimestampNow(); - float deltaTime = (now - _lastAvatarUpdate) / (1000.0f * 1000.0f); + float deltaTime = (now - _lastAvatarUpdate) / (float) USECS_PER_SECOND; Application::getInstance()->setAvatarSimrateSample(1.0f / deltaTime); _lastAvatarUpdate = now; From b8ca604bf8b76b976a1c90bf22d9ba81c0cfb280 Mon Sep 17 00:00:00 2001 From: Howard Stearns Date: Tue, 25 Aug 2015 16:17:50 -0700 Subject: [PATCH 03/22] Checkpoint before GenericThread. --- interface/src/Application.cpp | 5 +- interface/src/avatar/AvatarUpdate.cpp | 109 ++++++++++++++------------ interface/src/avatar/AvatarUpdate.h | 43 ++++++++-- 3 files changed, 98 insertions(+), 59 deletions(-) diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp index 9bc395e168..919816a643 100644 --- a/interface/src/Application.cpp +++ b/interface/src/Application.cpp @@ -771,7 +771,7 @@ void Application::cleanupBeforeQuit() { // first stop all timers directly or by invokeMethod // depending on what thread they run in - _avatarUpdate->stop(); + _avatarUpdate->terminate(); locationUpdateTimer->stop(); balanceUpdateTimer->stop(); identityPacketTimer->stop(); @@ -2453,6 +2453,7 @@ void Application::init() { connect(_myAvatar, &MyAvatar::newCollisionSoundURL, DependencyManager::get().data(), &SoundCache::getSound); _avatarUpdate = new AvatarUpdate(); + _avatarUpdate->initialize(_avatarUpdate->isToBeThreaded()); } void Application::closeMirrorView() { @@ -2884,7 +2885,7 @@ void Application::update(float deltaTime) { _overlays.update(deltaTime); } - _avatarUpdate->avatarUpdateIfSynchronous(); + _avatarUpdate->synchronousProcess(); { PerformanceTimer perfTimer("emitSimulating"); diff --git a/interface/src/avatar/AvatarUpdate.cpp b/interface/src/avatar/AvatarUpdate.cpp index c997a76132..acab5a46ad 100644 --- a/interface/src/avatar/AvatarUpdate.cpp +++ b/interface/src/avatar/AvatarUpdate.cpp @@ -15,73 +15,69 @@ #include "AvatarUpdate.h" #include "InterfaceLogging.h" -enum UpdateType { - Synchronous = 1, - Timer, - Thread -}; - -AvatarUpdate::AvatarUpdate() : _lastAvatarUpdate(0) { +// GenericThread accepts an optional "parent" argument, defaulting to nullptr. +// This is odd, because the moveToThread() in GenericThread::initialize() will +// become a no-op if the instance ever inits QObject(someNonNullParentQObject). +// (The only clue will be a log message "QObject::moveToThread: Cannot move objects with a parent", +// and things will end up in the same thread that created this instance. Hillarity ensues.) +// As it turns out, all the other subclasses of GenericThread (at this time) do not init +// GenericThread(parent), so things work as expected. Here we explicitly init GenericThread(nullptr) +// so that there is no confusion and no chance for a hillarious thread debugging session. +AvatarUpdate::AvatarUpdate() : QObject(nullptr), _timer(nullptr), _lastAvatarUpdate(0), _thread(nullptr)/*FIXME remove*/ { + setObjectName("Avatar Update"); // GenericThread::initialize uses this to set the thread name. Settings settings; - int type = settings.value("AvatarUpdateType", UpdateType::Synchronous).toInt(); - _targetSimrate = settings.value("AvatarUpdateTargetSimrate", 60).toInt(); - qCDebug(interfaceapp) << "AvatarUpdate using" << type << "at" << _targetSimrate << "sims/second"; - switch (type) { - case UpdateType::Synchronous: - _timer = nullptr; - break; - case UpdateType::Timer: - initTimer(); - break; - default: - initThread(); - break; - } + const int DEFAULT_TARGET_AVATAR_SIMRATE = 60; + _targetSimrate = settings.value("AvatarUpdateTargetSimrate", DEFAULT_TARGET_AVATAR_SIMRATE).toInt(); + _type = settings.value("AvatarUpdateType", UpdateType::Synchronous).toInt(); + qCDebug(interfaceapp) << "AvatarUpdate using" << _type << "at" << _targetSimrate << "sims/second"; } -void AvatarUpdate::stop() { - if (!_timer) { - return; +// We could have the constructor call initialize(), but GenericThread::initialize can take parameters. +// Keeping it separately called by the client allows that client to pass those without our +// constructor needing to know about them. + +// GenericThread::terimate() calls terminating() only when _isThreaded, so it's not good enough +// to just override terminating(). Instead, we extend terminate(); +void AvatarUpdate::terminate() { + if (_timer) { + _timer->stop(); } - _timer->stop(); - if (_avatarThread) { - return; + // FIXME: GenericThread::terminate(); + if (_thread) { + _thread->quit(); } - _avatarThread->quit(); } +// QTimer runs in the thread that starts it. +// threadRoutine() is called once within the separate thread (if any), +// or on each main loop update via synchronousProcess(); +void AvatarUpdate::threadRoutine() { + if (!_timer && (_type != UpdateType::Synchronous)) { + initTimer(); + } + if (!_timer) { + process(); + } +} void AvatarUpdate::initTimer() { const qint64 AVATAR_UPDATE_INTERVAL_MSECS = 1000 / _targetSimrate; _timer = new QTimer(this); - connect(_timer, &QTimer::timeout, this, &AvatarUpdate::avatarUpdate); + connect(_timer, &QTimer::timeout, this, &AvatarUpdate::process); _timer->start(AVATAR_UPDATE_INTERVAL_MSECS); } -void AvatarUpdate::initThread() { - _avatarThread = new QThread(); - _avatarThread->setObjectName("Avatar Update Thread"); - this->moveToThread(_avatarThread); - connect(_avatarThread, &QThread::started, this, &AvatarUpdate::initTimer); - _avatarThread->start(); -} -// There are a couple of ways we could do this. -// Right now, the goals are: -// 1. allow development to switch between UpdateType -// 2. minimize changes everwhere, particularly outside of Avatars. -// As an example of the latter, we could make Application::isHMDMode() thread safe, but in this case -// we just made AvatarUpdate::isHMDMode() thread safe. -void AvatarUpdate::avatarUpdateIfSynchronous() { +void AvatarUpdate::synchronousProcess() { + // Keep our own updated value, so that our asynchronous code can consult it. _isHMDMode = Application::getInstance()->isHMDMode(); + if (_updateBillboard) { Application::getInstance()->getMyAvatar()->doUpdateBillboard(); } - if (_timer) { - return; - } - avatarUpdate(); + + threadRoutine(); } -void AvatarUpdate::avatarUpdate() { + bool AvatarUpdate::process() { PerformanceTimer perfTimer("AvatarUpdate"); quint64 now = usecTimestampNow(); float deltaTime = (now - _lastAvatarUpdate) / (float) USECS_PER_SECOND; @@ -95,4 +91,19 @@ void AvatarUpdate::avatarUpdate() { Application::getInstance()->updateMyAvatarLookAtPosition(); // Sample hardware, update view frustum if needed, and send avatar data to mixer/nodes DependencyManager::get()->updateMyAvatar(deltaTime); -} \ No newline at end of file + return true; +} + +// To be removed with GenericThread +void AvatarUpdate::initialize(bool isThreaded) { //fixme remove + if (isThreaded) { + initThread(); + } +} +void AvatarUpdate::initThread() { + _thread = new QThread(); + _thread->setObjectName(objectName()); + this->moveToThread(_thread); + connect(_thread, &QThread::started, this, &AvatarUpdate::threadRoutine); + _thread->start(); +} diff --git a/interface/src/avatar/AvatarUpdate.h b/interface/src/avatar/AvatarUpdate.h index af17ffdeab..6a70aeb1f9 100644 --- a/interface/src/avatar/AvatarUpdate.h +++ b/interface/src/avatar/AvatarUpdate.h @@ -15,27 +15,54 @@ #include #include +// There are a couple of ways we could do this. +// Right now, the goals are: +// 1. allow development to switch between UpdateType +// e.g.: see uses of UpdateType. +// 2. minimize changes everwhere, particularly outside of Avatars. +// e.g.: we could make Application::isHMDMode() thread safe, but for now we just made AvatarUpdate::isHMDMode() thread safe. + + // Home for the avatarUpdate operations (e.g., whether on a separate thread, pipelined in various ways, etc.) -// TODO: become GenericThread // This might get folded into AvatarManager. class AvatarUpdate : public QObject { Q_OBJECT public: AvatarUpdate(); - void avatarUpdateIfSynchronous(); - bool isHMDMode() { return _isHMDMode; } + void synchronousProcess(); void setRequestBillboardUpdate(bool needsUpdate) { _updateBillboard = needsUpdate; } - void stop(); + void terminate(); // Extends GenericThread::terminate to also kill timer. + private: + virtual bool process(); // No reason for other classes to invoke this. void initTimer(); - void initThread(); - void avatarUpdate(); int _targetSimrate; - bool _isHMDMode; bool _updateBillboard; QTimer* _timer; - QThread* _avatarThread; quint64 _lastAvatarUpdate; + + // Goes away when we get rid of the ability to switch back and forth in settings: + enum UpdateType { + Synchronous = 1, + Timer, + Thread + }; + int _type; +public: + bool isToBeThreaded() const { return _type == UpdateType::Thread; } // Currently set by constructor from settings. + void threadRoutine(); + + // Goes away when using GenericThread: + void initialize(bool isThreaded); +private: + QThread* _thread; + void initThread(); + + // Goes away if Applicaiton::isHMDMode() is made thread safe: +public: + bool isHMDMode() { return _isHMDMode; } +private: + bool _isHMDMode; }; From 38bc8634de226b4534e4801fceaf2a2d466a8f21 Mon Sep 17 00:00:00 2001 From: Howard Stearns Date: Tue, 25 Aug 2015 21:26:46 -0700 Subject: [PATCH 04/22] displayPlugin is not thread safe, and switch from simulate rate to interval in prep for sleep. --- interface/src/Application.cpp | 2 +- interface/src/avatar/AvatarUpdate.cpp | 9 +++++---- interface/src/avatar/AvatarUpdate.h | 6 ++++-- 3 files changed, 10 insertions(+), 7 deletions(-) diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp index 919816a643..50f39c87bb 100644 --- a/interface/src/Application.cpp +++ b/interface/src/Application.cpp @@ -2588,7 +2588,7 @@ void Application::updateMyAvatarLookAtPosition() { } else { // I am not looking at anyone else, so just look forward if (isHMD) { - glm::mat4 headPose = getActiveDisplayPlugin()->getHeadPose(); + glm::mat4 headPose = _avatarUpdate->getHeadPose(); glm::quat headRotation = glm::quat_cast(headPose); lookAtSpot = _myCamera.getPosition() + _myAvatar->getOrientation() * (headRotation * glm::vec3(0.0f, 0.0f, -TREE_SCALE)); diff --git a/interface/src/avatar/AvatarUpdate.cpp b/interface/src/avatar/AvatarUpdate.cpp index acab5a46ad..4335cbd16d 100644 --- a/interface/src/avatar/AvatarUpdate.cpp +++ b/interface/src/avatar/AvatarUpdate.cpp @@ -13,6 +13,7 @@ #include "Application.h" #include "AvatarManager.h" #include "AvatarUpdate.h" +#include #include "InterfaceLogging.h" // GenericThread accepts an optional "parent" argument, defaulting to nullptr. @@ -27,9 +28,9 @@ AvatarUpdate::AvatarUpdate() : QObject(nullptr), _timer(nullptr), _lastAvatarUp setObjectName("Avatar Update"); // GenericThread::initialize uses this to set the thread name. Settings settings; const int DEFAULT_TARGET_AVATAR_SIMRATE = 60; - _targetSimrate = settings.value("AvatarUpdateTargetSimrate", DEFAULT_TARGET_AVATAR_SIMRATE).toInt(); + _targetInterval = USECS_PER_SECOND / settings.value("AvatarUpdateTargetSimrate", DEFAULT_TARGET_AVATAR_SIMRATE).toInt(); _type = settings.value("AvatarUpdateType", UpdateType::Synchronous).toInt(); - qCDebug(interfaceapp) << "AvatarUpdate using" << _type << "at" << _targetSimrate << "sims/second"; + qCDebug(interfaceapp) << "AvatarUpdate using" << _type << "at" << _targetInterval << "microseconds"; } // We could have the constructor call initialize(), but GenericThread::initialize can take parameters. // Keeping it separately called by the client allows that client to pass those without our @@ -59,16 +60,16 @@ void AvatarUpdate::threadRoutine() { } } void AvatarUpdate::initTimer() { - const qint64 AVATAR_UPDATE_INTERVAL_MSECS = 1000 / _targetSimrate; _timer = new QTimer(this); connect(_timer, &QTimer::timeout, this, &AvatarUpdate::process); - _timer->start(AVATAR_UPDATE_INTERVAL_MSECS); + _timer->start(_targetInterval / USECS_PER_MSEC); } void AvatarUpdate::synchronousProcess() { // Keep our own updated value, so that our asynchronous code can consult it. _isHMDMode = Application::getInstance()->isHMDMode(); + _headPose = Application::getInstance()->getActiveDisplayPlugin()->getHeadPose(); if (_updateBillboard) { Application::getInstance()->getMyAvatar()->doUpdateBillboard(); diff --git a/interface/src/avatar/AvatarUpdate.h b/interface/src/avatar/AvatarUpdate.h index 6a70aeb1f9..93192ae50d 100644 --- a/interface/src/avatar/AvatarUpdate.h +++ b/interface/src/avatar/AvatarUpdate.h @@ -36,7 +36,7 @@ public: private: virtual bool process(); // No reason for other classes to invoke this. void initTimer(); - int _targetSimrate; + quint64 _targetInterval; bool _updateBillboard; QTimer* _timer; quint64 _lastAvatarUpdate; @@ -58,11 +58,13 @@ private: QThread* _thread; void initThread(); - // Goes away if Applicaiton::isHMDMode() is made thread safe: + // Goes away if Applicaiton::isHMDMode() and friends are made thread safe: public: bool isHMDMode() { return _isHMDMode; } + glm::mat4 getHeadPose() { return _headPose; } private: bool _isHMDMode; + glm::mat4 _headPose; }; From b0734a06f3b81fbf1649f06cae09261ed102f179 Mon Sep 17 00:00:00 2001 From: Howard Stearns Date: Wed, 26 Aug 2015 11:14:09 -0700 Subject: [PATCH 05/22] snapshot --- interface/src/avatar/AvatarUpdate.cpp | 36 +++++++++++++++++++-------- interface/src/avatar/AvatarUpdate.h | 6 ++--- 2 files changed, 28 insertions(+), 14 deletions(-) diff --git a/interface/src/avatar/AvatarUpdate.cpp b/interface/src/avatar/AvatarUpdate.cpp index 4335cbd16d..7f5131f94b 100644 --- a/interface/src/avatar/AvatarUpdate.cpp +++ b/interface/src/avatar/AvatarUpdate.cpp @@ -24,7 +24,7 @@ // As it turns out, all the other subclasses of GenericThread (at this time) do not init // GenericThread(parent), so things work as expected. Here we explicitly init GenericThread(nullptr) // so that there is no confusion and no chance for a hillarious thread debugging session. -AvatarUpdate::AvatarUpdate() : QObject(nullptr), _timer(nullptr), _lastAvatarUpdate(0), _thread(nullptr)/*FIXME remove*/ { +AvatarUpdate::AvatarUpdate() : QObject(nullptr), _lastAvatarUpdate(0), _timer(nullptr), _thread(nullptr)/*FIXME remove*/ { setObjectName("Avatar Update"); // GenericThread::initialize uses this to set the thread name. Settings settings; const int DEFAULT_TARGET_AVATAR_SIMRATE = 60; @@ -61,8 +61,11 @@ void AvatarUpdate::threadRoutine() { } void AvatarUpdate::initTimer() { _timer = new QTimer(this); - connect(_timer, &QTimer::timeout, this, &AvatarUpdate::process); - _timer->start(_targetInterval / USECS_PER_MSEC); + /*FIXME connect(_timer, &QTimer::timeout, this, &AvatarUpdate::process); + _timer->start(_targetInterval / USECS_PER_MSEC);*/ + while (true) { + process(); + } } void AvatarUpdate::synchronousProcess() { @@ -75,23 +78,34 @@ void AvatarUpdate::synchronousProcess() { Application::getInstance()->getMyAvatar()->doUpdateBillboard(); } - threadRoutine(); + //threadRoutine(); + //process(); //fixme } - bool AvatarUpdate::process() { +bool AvatarUpdate::process() { PerformanceTimer perfTimer("AvatarUpdate"); - quint64 now = usecTimestampNow(); - float deltaTime = (now - _lastAvatarUpdate) / (float) USECS_PER_SECOND; - Application::getInstance()->setAvatarSimrateSample(1.0f / deltaTime); - _lastAvatarUpdate = now; + quint64 start = usecTimestampNow(); + quint64 deltaMicroseconds = start - _lastAvatarUpdate; + float deltaSeconds = deltaMicroseconds / (float) USECS_PER_SECOND; + Application::getInstance()->setAvatarSimrateSample(1.0f / deltaSeconds); //loop through all the other avatars and simulate them... //gets current lookat data, removes missing avatars, etc. - DependencyManager::get()->updateOtherAvatars(deltaTime); + DependencyManager::get()->updateOtherAvatars(deltaSeconds); Application::getInstance()->updateMyAvatarLookAtPosition(); // Sample hardware, update view frustum if needed, and send avatar data to mixer/nodes - DependencyManager::get()->updateMyAvatar(deltaTime); + DependencyManager::get()->updateMyAvatar(deltaSeconds); + + if (isToBeThreaded()) { + int elapsed = (usecTimestampNow() - start); + int usecToSleep = _targetInterval - elapsed; + if (usecToSleep < 0) { + usecToSleep = 1; // always yield + } + usleep(usecToSleep); + } + _lastAvatarUpdate = start; return true; } diff --git a/interface/src/avatar/AvatarUpdate.h b/interface/src/avatar/AvatarUpdate.h index 93192ae50d..fc2f540111 100644 --- a/interface/src/avatar/AvatarUpdate.h +++ b/interface/src/avatar/AvatarUpdate.h @@ -36,10 +36,9 @@ public: private: virtual bool process(); // No reason for other classes to invoke this. void initTimer(); - quint64 _targetInterval; + quint64 _targetInterval; // microseconds bool _updateBillboard; - QTimer* _timer; - quint64 _lastAvatarUpdate; + quint64 _lastAvatarUpdate; // microsoeconds // Goes away when we get rid of the ability to switch back and forth in settings: enum UpdateType { @@ -55,6 +54,7 @@ public: // Goes away when using GenericThread: void initialize(bool isThreaded); private: + QTimer* _timer; QThread* _thread; void initThread(); From 54877cdd47d837950bd0b65f4abb225c4d6f9a65 Mon Sep 17 00:00:00 2001 From: Howard Stearns Date: Wed, 26 Aug 2015 11:59:12 -0700 Subject: [PATCH 06/22] Use GenericThread, and use usleep instead of QTimer (which is incompatible with GenericThread). --- interface/src/avatar/AvatarUpdate.cpp | 83 +++++++-------------------- interface/src/avatar/AvatarUpdate.h | 40 +++---------- 2 files changed, 29 insertions(+), 94 deletions(-) diff --git a/interface/src/avatar/AvatarUpdate.cpp b/interface/src/avatar/AvatarUpdate.cpp index 7f5131f94b..9c6f4a6f25 100644 --- a/interface/src/avatar/AvatarUpdate.cpp +++ b/interface/src/avatar/AvatarUpdate.cpp @@ -24,50 +24,22 @@ // As it turns out, all the other subclasses of GenericThread (at this time) do not init // GenericThread(parent), so things work as expected. Here we explicitly init GenericThread(nullptr) // so that there is no confusion and no chance for a hillarious thread debugging session. -AvatarUpdate::AvatarUpdate() : QObject(nullptr), _lastAvatarUpdate(0), _timer(nullptr), _thread(nullptr)/*FIXME remove*/ { +AvatarUpdate::AvatarUpdate() : GenericThread(nullptr), _lastAvatarUpdate(0)/*, _timer(nullptr), _thread(nullptr)FIXME remove*/ { setObjectName("Avatar Update"); // GenericThread::initialize uses this to set the thread name. Settings settings; const int DEFAULT_TARGET_AVATAR_SIMRATE = 60; _targetInterval = USECS_PER_SECOND / settings.value("AvatarUpdateTargetSimrate", DEFAULT_TARGET_AVATAR_SIMRATE).toInt(); - _type = settings.value("AvatarUpdateType", UpdateType::Synchronous).toInt(); - qCDebug(interfaceapp) << "AvatarUpdate using" << _type << "at" << _targetInterval << "microseconds"; + _isToBeThreaded = settings.value("AvatarUpdateIsThreaded", true).toBool(); + if (_isToBeThreaded) { + qCDebug(interfaceapp) << "AvatarUpdate threaded at" << _targetInterval << "microsecond interval."; + } else { + qCDebug(interfaceapp) << "AvatarUpdate unthreaded."; + } } // We could have the constructor call initialize(), but GenericThread::initialize can take parameters. // Keeping it separately called by the client allows that client to pass those without our // constructor needing to know about them. -// GenericThread::terimate() calls terminating() only when _isThreaded, so it's not good enough -// to just override terminating(). Instead, we extend terminate(); -void AvatarUpdate::terminate() { - if (_timer) { - _timer->stop(); - } - // FIXME: GenericThread::terminate(); - if (_thread) { - _thread->quit(); - } -} - -// QTimer runs in the thread that starts it. -// threadRoutine() is called once within the separate thread (if any), -// or on each main loop update via synchronousProcess(); -void AvatarUpdate::threadRoutine() { - if (!_timer && (_type != UpdateType::Synchronous)) { - initTimer(); - } - if (!_timer) { - process(); - } -} -void AvatarUpdate::initTimer() { - _timer = new QTimer(this); - /*FIXME connect(_timer, &QTimer::timeout, this, &AvatarUpdate::process); - _timer->start(_targetInterval / USECS_PER_MSEC);*/ - while (true) { - process(); - } -} - void AvatarUpdate::synchronousProcess() { // Keep our own updated value, so that our asynchronous code can consult it. @@ -78,14 +50,17 @@ void AvatarUpdate::synchronousProcess() { Application::getInstance()->getMyAvatar()->doUpdateBillboard(); } - //threadRoutine(); - //process(); //fixme + if (isThreaded()) { + return; + } + process(); } bool AvatarUpdate::process() { PerformanceTimer perfTimer("AvatarUpdate"); quint64 start = usecTimestampNow(); quint64 deltaMicroseconds = start - _lastAvatarUpdate; + _lastAvatarUpdate = start; float deltaSeconds = deltaMicroseconds / (float) USECS_PER_SECOND; Application::getInstance()->setAvatarSimrateSample(1.0f / deltaSeconds); @@ -96,29 +71,15 @@ bool AvatarUpdate::process() { Application::getInstance()->updateMyAvatarLookAtPosition(); // Sample hardware, update view frustum if needed, and send avatar data to mixer/nodes DependencyManager::get()->updateMyAvatar(deltaSeconds); - - if (isToBeThreaded()) { - int elapsed = (usecTimestampNow() - start); - int usecToSleep = _targetInterval - elapsed; - if (usecToSleep < 0) { - usecToSleep = 1; // always yield - } - usleep(usecToSleep); - } - _lastAvatarUpdate = start; + + if (!isThreaded()) { + return true; + } + int elapsed = (usecTimestampNow() - start); + int usecToSleep = _targetInterval - elapsed; + if (usecToSleep < 0) { + usecToSleep = 1; // always yield + } + usleep(usecToSleep); return true; } - -// To be removed with GenericThread -void AvatarUpdate::initialize(bool isThreaded) { //fixme remove - if (isThreaded) { - initThread(); - } -} -void AvatarUpdate::initThread() { - _thread = new QThread(); - _thread->setObjectName(objectName()); - this->moveToThread(_thread); - connect(_thread, &QThread::started, this, &AvatarUpdate::threadRoutine); - _thread->start(); -} diff --git a/interface/src/avatar/AvatarUpdate.h b/interface/src/avatar/AvatarUpdate.h index fc2f540111..8e3448c533 100644 --- a/interface/src/avatar/AvatarUpdate.h +++ b/interface/src/avatar/AvatarUpdate.h @@ -15,50 +15,24 @@ #include #include -// There are a couple of ways we could do this. -// Right now, the goals are: -// 1. allow development to switch between UpdateType -// e.g.: see uses of UpdateType. -// 2. minimize changes everwhere, particularly outside of Avatars. -// e.g.: we could make Application::isHMDMode() thread safe, but for now we just made AvatarUpdate::isHMDMode() thread safe. - - // Home for the avatarUpdate operations (e.g., whether on a separate thread, pipelined in various ways, etc.) // This might get folded into AvatarManager. -class AvatarUpdate : public QObject { +class AvatarUpdate : public GenericThread { Q_OBJECT public: AvatarUpdate(); void synchronousProcess(); void setRequestBillboardUpdate(bool needsUpdate) { _updateBillboard = needsUpdate; } - void terminate(); // Extends GenericThread::terminate to also kill timer. + bool isToBeThreaded() { return _isToBeThreaded; } private: virtual bool process(); // No reason for other classes to invoke this. - void initTimer(); - quint64 _targetInterval; // microseconds - bool _updateBillboard; quint64 _lastAvatarUpdate; // microsoeconds - - // Goes away when we get rid of the ability to switch back and forth in settings: - enum UpdateType { - Synchronous = 1, - Timer, - Thread - }; - int _type; -public: - bool isToBeThreaded() const { return _type == UpdateType::Thread; } // Currently set by constructor from settings. - void threadRoutine(); - - // Goes away when using GenericThread: - void initialize(bool isThreaded); -private: - QTimer* _timer; - QThread* _thread; - void initThread(); - - // Goes away if Applicaiton::isHMDMode() and friends are made thread safe: + quint64 _targetInterval; // microseconds + bool _isToBeThreaded; + bool _updateBillboard; + + // Goes away if Application::getActiveDisplayPlugin() and friends are made thread safe: public: bool isHMDMode() { return _isHMDMode; } glm::mat4 getHeadPose() { return _headPose; } From 475331a97df3a41ddeaf5358f561c0928136243a Mon Sep 17 00:00:00 2001 From: Howard Stearns Date: Wed, 26 Aug 2015 16:08:07 -0700 Subject: [PATCH 07/22] Sprinkle locks everywhere. --- interface/src/avatar/Avatar.cpp | 5 +++++ interface/src/avatar/AvatarManager.cpp | 6 +++++ interface/src/avatar/SkeletonModel.cpp | 7 ++++++ interface/src/avatar/SkeletonModel.h | 2 ++ libraries/avatars/src/AvatarData.h | 2 ++ .../src/DynamicCharacterController.cpp | 2 ++ libraries/render-utils/src/Model.cpp | 22 +++++++++++++++---- libraries/render-utils/src/Model.h | 3 +++ 8 files changed, 45 insertions(+), 4 deletions(-) diff --git a/interface/src/avatar/Avatar.cpp b/interface/src/avatar/Avatar.cpp index e7423336b1..7b40b5cc5b 100644 --- a/interface/src/avatar/Avatar.cpp +++ b/interface/src/avatar/Avatar.cpp @@ -318,6 +318,7 @@ void Avatar::removeFromScene(AvatarSharedPointer self, std::shared_ptrupdate(); } @@ -392,6 +393,7 @@ void Avatar::render(RenderArgs* renderArgs, const glm::vec3& cameraPosition) { } if (frustum->sphereInFrustum(getPosition(), boundingRadius) == ViewFrustum::OUTSIDE) { + avatarLock.unlock(); return; } @@ -542,6 +544,7 @@ void Avatar::render(RenderArgs* renderArgs, const glm::vec3& cameraPosition) { if (!isMyAvatar() || cameraMode != CAMERA_MODE_FIRST_PERSON) { renderDisplayName(batch, *renderArgs->_viewFrustum, renderArgs->_viewport); } + avatarLock.unlock(); } glm::quat Avatar::computeRotationFromBodyToWorldUp(float proportion) const { @@ -1019,6 +1022,7 @@ void Avatar::setBillboard(const QByteArray& billboard) { } int Avatar::parseDataFromBuffer(const QByteArray& buffer) { + avatarLock.lockForWrite(); if (!_initialized) { // now that we have data for this Avatar we are go for init init(); @@ -1034,6 +1038,7 @@ int Avatar::parseDataFromBuffer(const QByteArray& buffer) { if (_moving && _motionState) { _motionState->addDirtyFlags(EntityItem::DIRTY_POSITION); } + avatarLock.unlock(); return bytesRead; } diff --git a/interface/src/avatar/AvatarManager.cpp b/interface/src/avatar/AvatarManager.cpp index 1644f22b09..fefb774d66 100644 --- a/interface/src/avatar/AvatarManager.cpp +++ b/interface/src/avatar/AvatarManager.cpp @@ -93,7 +93,9 @@ void AvatarManager::updateMyAvatar(float deltaTime) { bool showWarnings = Menu::getInstance()->isOptionChecked(MenuOption::PipelineWarnings); PerformanceWarning warn(showWarnings, "AvatarManager::updateMyAvatar()"); + _myAvatar->avatarLock.lockForWrite(); _myAvatar->update(deltaTime); + _myAvatar->avatarLock.unlock(); quint64 now = usecTimestampNow(); quint64 dt = now - _lastSendAvatarDataTime; @@ -129,7 +131,9 @@ void AvatarManager::updateOtherAvatars(float deltaTime) { _avatarFades.push_back(avatarIterator.value()); avatarIterator = _avatarHash.erase(avatarIterator); } else { + avatar->avatarLock.lockForWrite(); avatar->simulate(deltaTime); + avatar->avatarLock.unlock(); ++avatarIterator; } } @@ -148,6 +152,7 @@ void AvatarManager::simulateAvatarFades(float deltaTime) { render::PendingChanges pendingChanges; while (fadingIterator != _avatarFades.end()) { auto avatar = std::static_pointer_cast(*fadingIterator); + avatar->avatarLock.lockForWrite(); avatar->setTargetScale(avatar->getScale() * SHRINK_RATE, true); if (avatar->getTargetScale() < MIN_FADE_SCALE) { avatar->removeFromScene(*fadingIterator, scene, pendingChanges); @@ -156,6 +161,7 @@ void AvatarManager::simulateAvatarFades(float deltaTime) { avatar->simulate(deltaTime); ++fadingIterator; } + avatar->avatarLock.unlock(); } scene->enqueuePendingChanges(pendingChanges); } diff --git a/interface/src/avatar/SkeletonModel.cpp b/interface/src/avatar/SkeletonModel.cpp index bd50215de6..973a9ea8c7 100644 --- a/interface/src/avatar/SkeletonModel.cpp +++ b/interface/src/avatar/SkeletonModel.cpp @@ -39,6 +39,13 @@ SkeletonModel::SkeletonModel(Avatar* owningAvatar, QObject* parent, RigPointer r SkeletonModel::~SkeletonModel() { } +void SkeletonModel::avatarLockForWriteIfApplicable() { + _owningAvatar->avatarLock.lockForWrite(); +} +void SkeletonModel::avatarLockReleaseIfApplicable() { + _owningAvatar->avatarLock.unlock(); +} + void SkeletonModel::initJointStates(QVector states) { const FBXGeometry& geometry = _geometry->getFBXGeometry(); glm::mat4 parentTransform = glm::scale(_scale) * glm::translate(_offset) * geometry.offset; diff --git a/interface/src/avatar/SkeletonModel.h b/interface/src/avatar/SkeletonModel.h index 4ae615eadd..f18895d674 100644 --- a/interface/src/avatar/SkeletonModel.h +++ b/interface/src/avatar/SkeletonModel.h @@ -105,6 +105,8 @@ public: float getHeadClipDistance() const { return _headClipDistance; } virtual void onInvalidate() override; + virtual void avatarLockForWriteIfApplicable() override; + virtual void avatarLockReleaseIfApplicable() override; signals: diff --git a/libraries/avatars/src/AvatarData.h b/libraries/avatars/src/AvatarData.h index 8bb874bc71..7356bc863b 100644 --- a/libraries/avatars/src/AvatarData.h +++ b/libraries/avatars/src/AvatarData.h @@ -305,6 +305,8 @@ public: bool shouldDie() const { return _owningAvatarMixer.isNull() || getUsecsSinceLastUpdate() > AVATAR_SILENCE_THRESHOLD_USECS; } + QReadWriteLock avatarLock; // Name is redundant, but it aids searches. + public slots: void sendAvatarDataPacket(); void sendIdentityPacket(); diff --git a/libraries/physics/src/DynamicCharacterController.cpp b/libraries/physics/src/DynamicCharacterController.cpp index 22f84eccec..7c1455c69e 100644 --- a/libraries/physics/src/DynamicCharacterController.cpp +++ b/libraries/physics/src/DynamicCharacterController.cpp @@ -405,6 +405,7 @@ void DynamicCharacterController::preSimulation(btScalar timeStep) { void DynamicCharacterController::postSimulation() { if (_enabled && _rigidBody) { + _avatarData->avatarLock.lockForWrite(); const btTransform& avatarTransform = _rigidBody->getWorldTransform(); glm::quat rotation = bulletToGLM(avatarTransform.getRotation()); glm::vec3 position = bulletToGLM(avatarTransform.getOrigin()); @@ -412,5 +413,6 @@ void DynamicCharacterController::postSimulation() { _avatarData->setOrientation(rotation); _avatarData->setPosition(position - rotation * _shapeLocalOffset); _avatarData->setVelocity(bulletToGLM(_rigidBody->getLinearVelocity())); + _avatarData->avatarLock.unlock(); } } diff --git a/libraries/render-utils/src/Model.cpp b/libraries/render-utils/src/Model.cpp index c2d723a323..039cd803c7 100644 --- a/libraries/render-utils/src/Model.cpp +++ b/libraries/render-utils/src/Model.cpp @@ -1285,7 +1285,9 @@ void Model::simulateInternal(float deltaTime) { const FBXGeometry& geometry = _geometry->getFBXGeometry(); glm::mat4 parentTransform = glm::scale(_scale) * glm::translate(_offset) * geometry.offset; updateRig(deltaTime, parentTransform); - +} +void Model::updateClusterMatrices() { + const FBXGeometry& geometry = _geometry->getFBXGeometry(); glm::mat4 zeroScale(glm::vec4(0.0f, 0.0f, 0.0f, 0.0f), glm::vec4(0.0f, 0.0f, 0.0f, 0.0f), glm::vec4(0.0f, 0.0f, 0.0f, 0.0f), @@ -1419,15 +1421,17 @@ AABox Model::getPartBounds(int meshIndex, int partIndex) { return AABox(); } + avatarLockForWriteIfApplicable(); if (meshIndex < _meshStates.size()) { const MeshState& state = _meshStates.at(meshIndex); bool isSkinned = state.clusterMatrices.size() > 1; if (isSkinned) { // if we're skinned return the entire mesh extents because we can't know for sure our clusters don't move us - return calculateScaledOffsetAABox(_geometry->getFBXGeometry().meshExtents); + AABox box = calculateScaledOffsetAABox(_geometry->getFBXGeometry().meshExtents); + avatarLockReleaseIfApplicable(); + return box; } } - if (_geometry->getFBXGeometry().meshes.size() > meshIndex) { // FIX ME! - This is currently a hack because for some mesh parts our efforts to calculate the bounding @@ -1443,8 +1447,11 @@ AABox Model::getPartBounds(int meshIndex, int partIndex) { // // If we not skinned use the bounds of the subMesh for all it's parts const FBXMesh& mesh = _geometry->getFBXGeometry().meshes.at(meshIndex); - return calculateScaledOffsetExtents(mesh.meshExtents); + AABox box = calculateScaledOffsetExtents(mesh.meshExtents); + avatarLockReleaseIfApplicable(); + return box; } + avatarLockReleaseIfApplicable(); return AABox(); } @@ -1482,6 +1489,9 @@ void Model::renderPart(RenderArgs* args, int meshIndex, int partIndex, bool tran return; } + avatarLockForWriteIfApplicable(); + updateClusterMatrices(); + const NetworkMesh& networkMesh = *(networkMeshes.at(meshIndex).get()); const FBXMesh& mesh = geometry.meshes.at(meshIndex); const MeshState& state = _meshStates.at(meshIndex); @@ -1536,6 +1546,7 @@ void Model::renderPart(RenderArgs* args, int meshIndex, int partIndex, bool tran _meshGroupsKnown = false; // regenerate these lists next time around. _readyWhenAdded = false; // in case any of our users are using scenes invalidCalculatedMeshBoxes(); // if we have to reload, we need to assume our mesh boxes are all invalid + avatarLockReleaseIfApplicable(); return; // FIXME! } @@ -1543,6 +1554,7 @@ void Model::renderPart(RenderArgs* args, int meshIndex, int partIndex, bool tran int vertexCount = mesh.vertices.size(); if (vertexCount == 0) { // sanity check + avatarLockReleaseIfApplicable(); return; // FIXME! } @@ -1587,6 +1599,7 @@ void Model::renderPart(RenderArgs* args, int meshIndex, int partIndex, bool tran // guard against partially loaded meshes if (partIndex >= (int)networkMesh._parts.size() || partIndex >= mesh.parts.size()) { + avatarLockReleaseIfApplicable(); return; } @@ -1703,6 +1716,7 @@ void Model::renderPart(RenderArgs* args, int meshIndex, int partIndex, bool tran args->_details._trianglesRendered += part.triangleIndices.size() / INDICES_PER_TRIANGLE; args->_details._quadsRendered += part.quadIndices.size() / INDICES_PER_QUAD; } + avatarLockReleaseIfApplicable(); } void Model::segregateMeshGroups() { diff --git a/libraries/render-utils/src/Model.h b/libraries/render-utils/src/Model.h index e55bff6aca..ca291450f7 100644 --- a/libraries/render-utils/src/Model.h +++ b/libraries/render-utils/src/Model.h @@ -113,6 +113,9 @@ public: bool getSnapModelToRegistrationPoint() { return _snapModelToRegistrationPoint; } virtual void simulate(float deltaTime, bool fullUpdate = true); + void updateClusterMatrices(); + virtual void avatarLockForWriteIfApplicable() {}; + virtual void avatarLockReleaseIfApplicable() {}; /// Returns a reference to the shared geometry. const QSharedPointer& getGeometry() const { return _geometry; } From 2429134b37d76a9e2a84eb5d8071c94ae0cfdf71 Mon Sep 17 00:00:00 2001 From: Howard Stearns Date: Thu, 27 Aug 2015 13:44:19 -0700 Subject: [PATCH 08/22] Lock both myavatar activities, not one. --- interface/src/avatar/AvatarManager.cpp | 2 -- interface/src/avatar/AvatarUpdate.cpp | 2 ++ 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/interface/src/avatar/AvatarManager.cpp b/interface/src/avatar/AvatarManager.cpp index fefb774d66..d4885d651e 100644 --- a/interface/src/avatar/AvatarManager.cpp +++ b/interface/src/avatar/AvatarManager.cpp @@ -93,9 +93,7 @@ void AvatarManager::updateMyAvatar(float deltaTime) { bool showWarnings = Menu::getInstance()->isOptionChecked(MenuOption::PipelineWarnings); PerformanceWarning warn(showWarnings, "AvatarManager::updateMyAvatar()"); - _myAvatar->avatarLock.lockForWrite(); _myAvatar->update(deltaTime); - _myAvatar->avatarLock.unlock(); quint64 now = usecTimestampNow(); quint64 dt = now - _lastSendAvatarDataTime; diff --git a/interface/src/avatar/AvatarUpdate.cpp b/interface/src/avatar/AvatarUpdate.cpp index 9c6f4a6f25..5f7999d61f 100644 --- a/interface/src/avatar/AvatarUpdate.cpp +++ b/interface/src/avatar/AvatarUpdate.cpp @@ -68,9 +68,11 @@ bool AvatarUpdate::process() { //gets current lookat data, removes missing avatars, etc. DependencyManager::get()->updateOtherAvatars(deltaSeconds); + Application::getInstance()->getMyAvatar()->avatarLock.lockForWrite(); Application::getInstance()->updateMyAvatarLookAtPosition(); // Sample hardware, update view frustum if needed, and send avatar data to mixer/nodes DependencyManager::get()->updateMyAvatar(deltaSeconds); + Application::getInstance()->getMyAvatar()->avatarLock.unlock(); if (!isThreaded()) { return true; From c0fec44f3253b8770725b02bd439a085485120ae Mon Sep 17 00:00:00 2001 From: Howard Stearns Date: Thu, 3 Sep 2015 19:34:25 -0700 Subject: [PATCH 09/22] Menu item to switch between threaded and not (without restart!). --- interface/src/Application.cpp | 14 +++++++++++--- interface/src/Application.h | 3 ++- interface/src/Menu.cpp | 2 ++ interface/src/Menu.h | 1 + interface/src/avatar/AvatarUpdate.cpp | 13 +++---------- interface/src/avatar/AvatarUpdate.h | 2 -- 6 files changed, 19 insertions(+), 16 deletions(-) diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp index 74788224ef..578a637198 100644 --- a/interface/src/Application.cpp +++ b/interface/src/Application.cpp @@ -2445,10 +2445,18 @@ void Application::init() { connect(tree, &EntityTree::newCollisionSoundURL, DependencyManager::get().data(), &SoundCache::getSound); connect(_myAvatar, &MyAvatar::newCollisionSoundURL, DependencyManager::get().data(), &SoundCache::getSound); - _avatarUpdate = new AvatarUpdate(); - _avatarUpdate->initialize(_avatarUpdate->isToBeThreaded()); + setAvatarUpdateThreading(Menu::getInstance()->isOptionChecked(MenuOption::EnableAvatarUpdateThreading)); } +void Application::setAvatarUpdateThreading(bool isThreaded) { + if (_avatarUpdate) { + _avatarUpdate->terminate(); + } + _avatarUpdate = new AvatarUpdate(); + _avatarUpdate->initialize(isThreaded); +} + + void Application::closeMirrorView() { if (Menu::getInstance()->isOptionChecked(MenuOption::Mirror)) { Menu::getInstance()->triggerOption(MenuOption::Mirror); @@ -2581,7 +2589,7 @@ void Application::updateMyAvatarLookAtPosition() { } else { // I am not looking at anyone else, so just look forward if (isHMD) { - glm::mat4 headPose = _avatarUpdate->getHeadPose(); + glm::mat4 headPose = _avatarUpdate->getHeadPose() ; glm::quat headRotation = glm::quat_cast(headPose); lookAtSpot = _myCamera.getPosition() + _myAvatar->getOrientation() * (headRotation * glm::vec3(0.0f, 0.0f, -TREE_SCALE)); diff --git a/interface/src/Application.h b/interface/src/Application.h index 739ad8fee1..b6f807dec3 100644 --- a/interface/src/Application.h +++ b/interface/src/Application.h @@ -412,6 +412,7 @@ public slots: void openUrl(const QUrl& url); void updateMyAvatarTransform(); + void setAvatarUpdateThreading(bool isThreaded); void domainSettingsReceived(const QJsonObject& domainSettingsObject); @@ -557,7 +558,7 @@ private: KeyboardMouseDevice* _keyboardMouseDevice{ nullptr }; // Default input device, the good old keyboard mouse and maybe touchpad MyAvatar* _myAvatar; // TODO: move this and relevant code to AvatarManager (or MyAvatar as the case may be) - AvatarUpdate* _avatarUpdate; + AvatarUpdate* _avatarUpdate = nullptr; SimpleMovingAverage _avatarSimsPerSecond{10}; int _avatarSimsPerSecondReport = 0; quint64 _lastAvatarSimsPerSecondUpdate = 0; diff --git a/interface/src/Menu.cpp b/interface/src/Menu.cpp index 18015804f8..b012057757 100644 --- a/interface/src/Menu.cpp +++ b/interface/src/Menu.cpp @@ -431,6 +431,8 @@ Menu::Menu() { addCheckableActionToQMenuAndActionHash(avatarDebugMenu, MenuOption::RenderFocusIndicator, 0, false); addCheckableActionToQMenuAndActionHash(avatarDebugMenu, MenuOption::ShowWhosLookingAtMe, 0, false); addCheckableActionToQMenuAndActionHash(avatarDebugMenu, MenuOption::FixGaze, 0, false); + addCheckableActionToQMenuAndActionHash(avatarDebugMenu, MenuOption::EnableAvatarUpdateThreading, 0, false, + qApp, SLOT(setAvatarUpdateThreading(bool))); addCheckableActionToQMenuAndActionHash(avatarDebugMenu, MenuOption::EnableRigAnimations, 0, false, avatar, SLOT(setEnableRigAnimations(bool))); addCheckableActionToQMenuAndActionHash(avatarDebugMenu, MenuOption::DisableEyelidAdjustment, 0, false); diff --git a/interface/src/Menu.h b/interface/src/Menu.h index f1fbb17895..476d6d20ae 100644 --- a/interface/src/Menu.h +++ b/interface/src/Menu.h @@ -185,6 +185,7 @@ namespace MenuOption { const QString EchoServerAudio = "Echo Server Audio"; const QString EditEntitiesHelp = "Edit Entities Help..."; const QString Enable3DTVMode = "Enable 3DTV Mode"; + const QString EnableAvatarUpdateThreading = "Enable Avatar Update Threading"; const QString EnableCharacterController = "Enable avatar collisions"; const QString EnableRigAnimations = "Enable Rig Animations"; const QString ExpandMyAvatarSimulateTiming = "Expand /myAvatar/simulation"; diff --git a/interface/src/avatar/AvatarUpdate.cpp b/interface/src/avatar/AvatarUpdate.cpp index 5f7999d61f..4121b95c82 100644 --- a/interface/src/avatar/AvatarUpdate.cpp +++ b/interface/src/avatar/AvatarUpdate.cpp @@ -24,17 +24,11 @@ // As it turns out, all the other subclasses of GenericThread (at this time) do not init // GenericThread(parent), so things work as expected. Here we explicitly init GenericThread(nullptr) // so that there is no confusion and no chance for a hillarious thread debugging session. -AvatarUpdate::AvatarUpdate() : GenericThread(nullptr), _lastAvatarUpdate(0)/*, _timer(nullptr), _thread(nullptr)FIXME remove*/ { +AvatarUpdate::AvatarUpdate() : GenericThread(nullptr), _lastAvatarUpdate(0) { setObjectName("Avatar Update"); // GenericThread::initialize uses this to set the thread name. Settings settings; const int DEFAULT_TARGET_AVATAR_SIMRATE = 60; _targetInterval = USECS_PER_SECOND / settings.value("AvatarUpdateTargetSimrate", DEFAULT_TARGET_AVATAR_SIMRATE).toInt(); - _isToBeThreaded = settings.value("AvatarUpdateIsThreaded", true).toBool(); - if (_isToBeThreaded) { - qCDebug(interfaceapp) << "AvatarUpdate threaded at" << _targetInterval << "microsecond interval."; - } else { - qCDebug(interfaceapp) << "AvatarUpdate unthreaded."; - } } // We could have the constructor call initialize(), but GenericThread::initialize can take parameters. // Keeping it separately called by the client allows that client to pass those without our @@ -50,10 +44,9 @@ void AvatarUpdate::synchronousProcess() { Application::getInstance()->getMyAvatar()->doUpdateBillboard(); } - if (isThreaded()) { - return; + if (!isThreaded()) { + process(); } - process(); } bool AvatarUpdate::process() { diff --git a/interface/src/avatar/AvatarUpdate.h b/interface/src/avatar/AvatarUpdate.h index 8e3448c533..08c90c0701 100644 --- a/interface/src/avatar/AvatarUpdate.h +++ b/interface/src/avatar/AvatarUpdate.h @@ -23,13 +23,11 @@ public: AvatarUpdate(); void synchronousProcess(); void setRequestBillboardUpdate(bool needsUpdate) { _updateBillboard = needsUpdate; } - bool isToBeThreaded() { return _isToBeThreaded; } private: virtual bool process(); // No reason for other classes to invoke this. quint64 _lastAvatarUpdate; // microsoeconds quint64 _targetInterval; // microseconds - bool _isToBeThreaded; bool _updateBillboard; // Goes away if Application::getActiveDisplayPlugin() and friends are made thread safe: From 0065c64b3143401de537fb83b9553c43d4a59c74 Mon Sep 17 00:00:00 2001 From: Howard Stearns Date: Mon, 7 Sep 2015 09:57:05 -0700 Subject: [PATCH 10/22] Snapshot of no-judder, before cleanup. --- interface/src/Application.cpp | 7 ++- interface/src/Application.h | 2 +- interface/src/avatar/Avatar.cpp | 6 +-- interface/src/avatar/Avatar.h | 1 + interface/src/avatar/AvatarManager.cpp | 8 ++-- interface/src/avatar/AvatarUpdate.cpp | 17 ++++--- interface/src/avatar/AvatarUpdate.h | 2 +- interface/src/avatar/MyAvatar.cpp | 6 ++- interface/src/avatar/SkeletonModel.cpp | 14 ++++-- interface/src/avatar/SkeletonModel.h | 1 + libraries/avatars/src/AvatarData.cpp | 48 +++++++++++++++++++ libraries/avatars/src/AvatarData.h | 13 +++++ .../src/DynamicCharacterController.cpp | 5 +- libraries/render-utils/src/Model.cpp | 10 ++-- 14 files changed, 108 insertions(+), 32 deletions(-) diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp index dc286dafae..92f64303c7 100644 --- a/interface/src/Application.cpp +++ b/interface/src/Application.cpp @@ -1019,6 +1019,8 @@ void Application::paintGL() { return; } _inPaint = true; + _myAvatar->captureAttitude(); + _myAvatar->startRender(); //FIXME Finally clearFlagLambda([this] { _inPaint = false; }); auto displayPlugin = getActiveDisplayPlugin(); @@ -1236,6 +1238,7 @@ void Application::paintGL() { gpu::Batch batch; batch.resetStages(); renderArgs._context->render(batch); + _myAvatar->endRender(); } void Application::runTests() { @@ -2152,7 +2155,7 @@ void Application::setAvatarSimrateSample(float sample) { } float Application::getAvatarSimrate() { uint64_t now = usecTimestampNow(); - + if (now - _lastAvatarSimsPerSecondUpdate > USECS_PER_SECOND) { _avatarSimsPerSecondReport = _avatarSimsPerSecond.getAverage(); _lastAvatarSimsPerSecondUpdate = now; @@ -2444,7 +2447,7 @@ void Application::init() { // Make sure any new sounds are loaded as soon as know about them. connect(tree, &EntityTree::newCollisionSoundURL, DependencyManager::get().data(), &SoundCache::getSound); connect(_myAvatar, &MyAvatar::newCollisionSoundURL, DependencyManager::get().data(), &SoundCache::getSound); - + setAvatarUpdateThreading(Menu::getInstance()->isOptionChecked(MenuOption::EnableAvatarUpdateThreading)); } diff --git a/interface/src/Application.h b/interface/src/Application.h index b6f807dec3..317832d9f5 100644 --- a/interface/src/Application.h +++ b/interface/src/Application.h @@ -335,7 +335,7 @@ public: gpu::ContextPointer getGPUContext() const { return _gpuContext; } const QRect& getMirrorViewRect() const { return _mirrorViewRect; } - + void updateMyAvatarLookAtPosition(); AvatarUpdate* getAvatarUpdater() { return _avatarUpdate; } MyAvatar* getMyAvatar() { return _myAvatar; } diff --git a/interface/src/avatar/Avatar.cpp b/interface/src/avatar/Avatar.cpp index 67d60073fe..fbc940078e 100644 --- a/interface/src/avatar/Avatar.cpp +++ b/interface/src/avatar/Avatar.cpp @@ -316,7 +316,7 @@ void Avatar::removeFromScene(AvatarSharedPointer self, std::shared_ptrupdate(); } @@ -391,7 +391,7 @@ void Avatar::render(RenderArgs* renderArgs, const glm::vec3& cameraPosition) { } if (frustum->sphereInFrustum(getPosition(), boundingRadius) == ViewFrustum::OUTSIDE) { - avatarLock.unlock(); + //FIXME endRender(); return; } @@ -542,7 +542,7 @@ void Avatar::render(RenderArgs* renderArgs, const glm::vec3& cameraPosition) { if (!isMyAvatar() || cameraMode != CAMERA_MODE_FIRST_PERSON) { renderDisplayName(batch, *renderArgs->_viewFrustum, renderArgs->_viewport); } - avatarLock.unlock(); + //FIXME endRender(); } glm::quat Avatar::computeRotationFromBodyToWorldUp(float proportion) const { diff --git a/interface/src/avatar/Avatar.h b/interface/src/avatar/Avatar.h index a7cece2b45..6cfbf939a2 100644 --- a/interface/src/avatar/Avatar.h +++ b/interface/src/avatar/Avatar.h @@ -156,6 +156,7 @@ public: void scaleVectorRelativeToPosition(glm::vec3 &positionToScale) const; void slamPosition(const glm::vec3& position); + virtual void updateAttitude() { _skeletonModel.updateAttitude(); } // Call this when updating Avatar position with a delta. This will allow us to // _accurately_ measure position changes and compute the resulting velocity diff --git a/interface/src/avatar/AvatarManager.cpp b/interface/src/avatar/AvatarManager.cpp index d4885d651e..67668a549d 100644 --- a/interface/src/avatar/AvatarManager.cpp +++ b/interface/src/avatar/AvatarManager.cpp @@ -129,9 +129,9 @@ void AvatarManager::updateOtherAvatars(float deltaTime) { _avatarFades.push_back(avatarIterator.value()); avatarIterator = _avatarHash.erase(avatarIterator); } else { - avatar->avatarLock.lockForWrite(); + avatar->startUpdate(); avatar->simulate(deltaTime); - avatar->avatarLock.unlock(); + avatar->endUpdate(); ++avatarIterator; } } @@ -150,7 +150,7 @@ void AvatarManager::simulateAvatarFades(float deltaTime) { render::PendingChanges pendingChanges; while (fadingIterator != _avatarFades.end()) { auto avatar = std::static_pointer_cast(*fadingIterator); - avatar->avatarLock.lockForWrite(); + avatar->startUpdate(); avatar->setTargetScale(avatar->getScale() * SHRINK_RATE, true); if (avatar->getTargetScale() < MIN_FADE_SCALE) { avatar->removeFromScene(*fadingIterator, scene, pendingChanges); @@ -159,7 +159,7 @@ void AvatarManager::simulateAvatarFades(float deltaTime) { avatar->simulate(deltaTime); ++fadingIterator; } - avatar->avatarLock.unlock(); + avatar->endUpdate(); } scene->enqueuePendingChanges(pendingChanges); } diff --git a/interface/src/avatar/AvatarUpdate.cpp b/interface/src/avatar/AvatarUpdate.cpp index 4121b95c82..c2240ef123 100644 --- a/interface/src/avatar/AvatarUpdate.cpp +++ b/interface/src/avatar/AvatarUpdate.cpp @@ -56,17 +56,20 @@ bool AvatarUpdate::process() { _lastAvatarUpdate = start; float deltaSeconds = deltaMicroseconds / (float) USECS_PER_SECOND; Application::getInstance()->setAvatarSimrateSample(1.0f / deltaSeconds); - + + QSharedPointer manager = DependencyManager::get(); + MyAvatar* myAvatar = manager->getMyAvatar(); + //loop through all the other avatars and simulate them... //gets current lookat data, removes missing avatars, etc. - DependencyManager::get()->updateOtherAvatars(deltaSeconds); - - Application::getInstance()->getMyAvatar()->avatarLock.lockForWrite(); + manager->updateOtherAvatars(deltaSeconds); + + myAvatar->startUpdate(); Application::getInstance()->updateMyAvatarLookAtPosition(); // Sample hardware, update view frustum if needed, and send avatar data to mixer/nodes - DependencyManager::get()->updateMyAvatar(deltaSeconds); - Application::getInstance()->getMyAvatar()->avatarLock.unlock(); - + manager->updateMyAvatar(deltaSeconds); + myAvatar->endUpdate(); + if (!isThreaded()) { return true; } diff --git a/interface/src/avatar/AvatarUpdate.h b/interface/src/avatar/AvatarUpdate.h index 08c90c0701..27c88b6617 100644 --- a/interface/src/avatar/AvatarUpdate.h +++ b/interface/src/avatar/AvatarUpdate.h @@ -29,7 +29,7 @@ private: quint64 _lastAvatarUpdate; // microsoeconds quint64 _targetInterval; // microseconds bool _updateBillboard; - + // Goes away if Application::getActiveDisplayPlugin() and friends are made thread safe: public: bool isHMDMode() { return _isHMDMode; } diff --git a/interface/src/avatar/MyAvatar.cpp b/interface/src/avatar/MyAvatar.cpp index f75aa5cba4..4e4043b28b 100644 --- a/interface/src/avatar/MyAvatar.cpp +++ b/interface/src/avatar/MyAvatar.cpp @@ -192,6 +192,9 @@ void MyAvatar::simulate(float deltaTime) { PerformanceTimer perfTimer("transform"); updateOrientation(deltaTime); updatePosition(deltaTime); + // The 2 updates set position, orientation, and all manner of physics stuff. + // Here we record the results. + nextAttitude(getPosition(), getOrientation()); } { @@ -266,8 +269,7 @@ void MyAvatar::updateFromHMDSensorMatrix(const glm::mat4& hmdSensorMatrix) { if (getStandingHMDSensorMode()) { // set the body position/orientation to reflect motion due to the head. auto worldMat = _sensorToWorldMatrix * _bodySensorMatrix; - setPosition(extractTranslation(worldMat)); - setOrientation(glm::quat_cast(worldMat)); + nextAttitude(extractTranslation(worldMat), glm::quat_cast(worldMat)); } } diff --git a/interface/src/avatar/SkeletonModel.cpp b/interface/src/avatar/SkeletonModel.cpp index 9bd6d35075..69aea5bb9d 100644 --- a/interface/src/avatar/SkeletonModel.cpp +++ b/interface/src/avatar/SkeletonModel.cpp @@ -40,10 +40,10 @@ SkeletonModel::~SkeletonModel() { } void SkeletonModel::avatarLockForWriteIfApplicable() { - _owningAvatar->avatarLock.lockForWrite(); + //FIXME _owningAvatar->avatarLock.lockForWrite(); } void SkeletonModel::avatarLockReleaseIfApplicable() { - _owningAvatar->avatarLock.unlock(); + //FIXME _owningAvatar->avatarLock.unlock(); } void SkeletonModel::initJointStates(QVector states) { @@ -154,13 +154,17 @@ void SkeletonModel::updateRig(float deltaTime, glm::mat4 parentTransform) { } } -// Called by Avatar::simulate after it has set the joint states (fullUpdate true if changed), -// but just before head has been simulated. -void SkeletonModel::simulate(float deltaTime, bool fullUpdate) { +void SkeletonModel::updateAttitude() { setTranslation(_owningAvatar->getSkeletonPosition()); static const glm::quat refOrientation = glm::angleAxis(PI, glm::vec3(0.0f, 1.0f, 0.0f)); setRotation(_owningAvatar->getOrientation() * refOrientation); setScale(glm::vec3(1.0f, 1.0f, 1.0f) * _owningAvatar->getScale()); +} + +// Called by Avatar::simulate after it has set the joint states (fullUpdate true if changed), +// but just before head has been simulated. +void SkeletonModel::simulate(float deltaTime, bool fullUpdate) { + updateAttitude(); setBlendshapeCoefficients(_owningAvatar->getHead()->getBlendshapeCoefficients()); Model::simulate(deltaTime, fullUpdate); diff --git a/interface/src/avatar/SkeletonModel.h b/interface/src/avatar/SkeletonModel.h index d3bf850c53..1481619a04 100644 --- a/interface/src/avatar/SkeletonModel.h +++ b/interface/src/avatar/SkeletonModel.h @@ -31,6 +31,7 @@ public: virtual void simulate(float deltaTime, bool fullUpdate = true); virtual void updateRig(float deltaTime, glm::mat4 parentTransform); + void updateAttitude(); void renderIKConstraints(gpu::Batch& batch); diff --git a/libraries/avatars/src/AvatarData.cpp b/libraries/avatars/src/AvatarData.cpp index 46a323733a..9d7f974a68 100644 --- a/libraries/avatars/src/AvatarData.cpp +++ b/libraries/avatars/src/AvatarData.cpp @@ -110,6 +110,54 @@ void AvatarData::setOrientation(const glm::quat& orientation, bool overideRefere } } +// There are a number of possible strategies, some more optimal than others in terms of using the latest info +// The current one does not update anything until captureAttitude, and then keeps that value until rendered. +void AvatarData::nextAttitude(glm::vec3 position, glm::quat orientation) { + setPosition(position, true); setOrientation(orientation, true); + _nextPending = 1; // FIXME type bool +} +void AvatarData::captureAttitude() { + if (!_nextAllowed) { // We haven't finished rendering the last one + return; + } + avatarLock.lockForWrite(); + if (_nextPending) { + _nextAllowed = false; + _nextPosition = getPosition(); + _nextOrientation = getOrientation(); + } else { + qCDebug(avatars) << "FIXME capture with nothing pending"; + } + avatarLock.unlock(); +} +void AvatarData::startUpdate() { + avatarLock.lockForWrite(); +} +void AvatarData::endUpdate() { + avatarLock.unlock(); +} +void AvatarData::startRender() { + avatarLock.lockForRead(); + if (!_nextPending) { + return; + } + glm::vec3 pos = getPosition(); + glm::quat rot = getOrientation(); + setPosition(_nextPosition, true); + //setOrientation(_nextOrientation, true); + updateAttitude(); + _nextPosition = pos; + _nextOrientation = rot; +} +void AvatarData::endRender() { + setPosition(_nextPosition, true); + //setOrientation(_nextOrientation, true); + updateAttitude(); + _nextPending = 0; + _nextAllowed = true; + avatarLock.unlock(); +} + float AvatarData::getTargetScale() const { if (_referential) { _referential->update(); diff --git a/libraries/avatars/src/AvatarData.h b/libraries/avatars/src/AvatarData.h index d4da9487f7..41d8cba997 100644 --- a/libraries/avatars/src/AvatarData.h +++ b/libraries/avatars/src/AvatarData.h @@ -198,6 +198,14 @@ public: glm::quat getOrientation() const; virtual void setOrientation(const glm::quat& orientation, bool overideReferential = false); + void nextAttitude(glm::vec3 position, glm::quat orientation); // Can be safely called at any time. + void captureAttitude(); // Indicates that the latest values are about to be captured for camera, etc. + void startUpdate(); // start/end of update iteration + void endUpdate(); + void startRender(); // start/end of rendering + void endRender(); + virtual void updateAttitude() {} // Tell skeleton mesh about changes + glm::quat getHeadOrientation() const { return _headData->getOrientation(); } void setHeadOrientation(const glm::quat& orientation) { _headData->setOrientation(orientation); } @@ -358,6 +366,11 @@ protected: float _bodyPitch; // degrees float _bodyRoll; // degrees + glm::vec3 _nextPosition {}; + glm::quat _nextOrientation {}; + int _nextPending = 0; + bool _nextAllowed = true; + // Body scale float _targetScale; diff --git a/libraries/physics/src/DynamicCharacterController.cpp b/libraries/physics/src/DynamicCharacterController.cpp index 7c1455c69e..163cba6aae 100644 --- a/libraries/physics/src/DynamicCharacterController.cpp +++ b/libraries/physics/src/DynamicCharacterController.cpp @@ -405,14 +405,11 @@ void DynamicCharacterController::preSimulation(btScalar timeStep) { void DynamicCharacterController::postSimulation() { if (_enabled && _rigidBody) { - _avatarData->avatarLock.lockForWrite(); const btTransform& avatarTransform = _rigidBody->getWorldTransform(); glm::quat rotation = bulletToGLM(avatarTransform.getRotation()); glm::vec3 position = bulletToGLM(avatarTransform.getOrigin()); - _avatarData->setOrientation(rotation); - _avatarData->setPosition(position - rotation * _shapeLocalOffset); + _avatarData->nextAttitude(position - rotation * _shapeLocalOffset, rotation); _avatarData->setVelocity(bulletToGLM(_rigidBody->getLinearVelocity())); - _avatarData->avatarLock.unlock(); } } diff --git a/libraries/render-utils/src/Model.cpp b/libraries/render-utils/src/Model.cpp index 0a785dfa3c..99a0c81d4b 100644 --- a/libraries/render-utils/src/Model.cpp +++ b/libraries/render-utils/src/Model.cpp @@ -1310,7 +1310,7 @@ void Model::updateClusterMatrices() { if (_showTrueJointTransforms) { for (int j = 0; j < mesh.clusters.size(); j++) { const FBXCluster& cluster = mesh.clusters.at(j); - auto jointMatrix =_rig->getJointTransform(cluster.jointIndex); + auto jointMatrix = _rig->getJointTransform(cluster.jointIndex); state.clusterMatrices[j] = modelToWorld * jointMatrix * cluster.inverseBindMatrix; // as an optimization, don't build cautrizedClusterMatrices if the boneSet is empty. @@ -1324,7 +1324,7 @@ void Model::updateClusterMatrices() { } else { for (int j = 0; j < mesh.clusters.size(); j++) { const FBXCluster& cluster = mesh.clusters.at(j); - auto jointMatrix = _rig->getJointVisibleTransform(cluster.jointIndex); + auto jointMatrix = _rig->getJointVisibleTransform(cluster.jointIndex); // differs from above only in using get...VisibleTransform state.clusterMatrices[j] = modelToWorld * jointMatrix * cluster.inverseBindMatrix; // as an optimization, don't build cautrizedClusterMatrices if the boneSet is empty. @@ -1472,6 +1472,10 @@ void Model::renderPart(RenderArgs* args, int meshIndex, int partIndex, bool tran return; // bail asap } + avatarLockForWriteIfApplicable(); + if (!_calculatedMeshPartOffsetValid) + qCDebug(renderutils) << "FIXME surprise!"; + _calculatedMeshPartOffsetValid = false; // FIXME // We need to make sure we have valid offsets calculated before we can render if (!_calculatedMeshPartOffsetValid) { _mutex.lock(); @@ -1496,10 +1500,10 @@ void Model::renderPart(RenderArgs* args, int meshIndex, int partIndex, bool tran // guard against partially loaded meshes if (meshIndex >= (int)networkMeshes.size() || meshIndex >= (int)geometry.meshes.size() || meshIndex >= (int)_meshStates.size() ) { + avatarLockReleaseIfApplicable(); return; } - avatarLockForWriteIfApplicable(); updateClusterMatrices(); const NetworkMesh& networkMesh = *(networkMeshes.at(meshIndex).get()); From efeaf213050e0be79e441099cbf167daf7608491 Mon Sep 17 00:00:00 2001 From: Howard Stearns Date: Mon, 7 Sep 2015 16:32:51 -0700 Subject: [PATCH 11/22] Checkpoint smoother. --- interface/src/Application.cpp | 19 +++++-- interface/src/avatar/Avatar.cpp | 10 ++-- interface/src/avatar/MyAvatar.cpp | 3 -- libraries/avatars/src/AvatarData.cpp | 78 ++++++++++++++++++---------- libraries/avatars/src/AvatarData.h | 7 ++- libraries/render-utils/src/Model.cpp | 3 -- 6 files changed, 76 insertions(+), 44 deletions(-) diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp index 92f64303c7..eeb6563bb4 100644 --- a/interface/src/Application.cpp +++ b/interface/src/Application.cpp @@ -1019,8 +1019,6 @@ void Application::paintGL() { return; } _inPaint = true; - _myAvatar->captureAttitude(); - _myAvatar->startRender(); //FIXME Finally clearFlagLambda([this] { _inPaint = false; }); auto displayPlugin = getActiveDisplayPlugin(); @@ -1028,6 +1026,8 @@ void Application::paintGL() { _offscreenContext->makeCurrent(); // update the avatar with a fresh HMD pose _myAvatar->updateFromHMDSensorMatrix(getHMDSensorPose()); + //_myAvatar->captureAttitude(); //FIXME + //_myAvatar->startRender(); //FIXME auto lodManager = DependencyManager::get(); @@ -1046,7 +1046,7 @@ void Application::paintGL() { // Before anything else, let's sync up the gpuContext with the true glcontext used in case anything happened renderArgs._context->syncCache(); - + if (Menu::getInstance()->isOptionChecked(MenuOption::Mirror)) { auto primaryFbo = DependencyManager::get()->getPrimaryFramebufferDepthColor(); @@ -1079,6 +1079,7 @@ void Application::paintGL() { _applicationOverlay.renderOverlay(&renderArgs); } + _myAvatar->startCapture(); if (_myCamera.getMode() == CAMERA_MODE_FIRST_PERSON || _myCamera.getMode() == CAMERA_MODE_THIRD_PERSON) { Menu::getInstance()->setIsOptionChecked(MenuOption::FirstPerson, _myAvatar->getBoomLength() <= MyAvatar::ZOOM_MIN); Menu::getInstance()->setIsOptionChecked(MenuOption::ThirdPerson, !(_myAvatar->getBoomLength() <= MyAvatar::ZOOM_MIN)); @@ -1128,7 +1129,7 @@ void Application::paintGL() { if (!isHMDMode()) { _myCamera.update(1.0f / _fps); } - + _myAvatar->endCapture(); // Primary rendering pass auto framebufferCache = DependencyManager::get(); @@ -1237,8 +1238,9 @@ void Application::paintGL() { // Back to the default framebuffer; gpu::Batch batch; batch.resetStages(); + //_myAvatar->startRender(); //FIXME renderArgs._context->render(batch); - _myAvatar->endRender(); + //_myAvatar->endRender(); //FIXME } void Application::runTests() { @@ -3473,7 +3475,10 @@ void Application::displaySide(RenderArgs* renderArgs, Camera& theCamera, bool se // FIXME: This preRender call is temporary until we create a separate render::scene for the mirror rendering. // Then we can move this logic into the Avatar::simulate call. + _myAvatar->startRender(); //FIXME _myAvatar->preRender(renderArgs); + _myAvatar->endRender(); //FIXME + activeRenderingThread = QThread::currentThread(); PROFILE_RANGE(__FUNCTION__); @@ -3587,7 +3592,9 @@ void Application::displaySide(RenderArgs* renderArgs, Camera& theCamera, bool se _renderEngine->setRenderContext(renderContext); // Before the deferred pass, let's try to use the render engine + _myAvatar->startRenderRun(); //FIXME _renderEngine->run(); + _myAvatar->endRenderRun(); //FIXME auto engineRC = _renderEngine->getRenderContext(); sceneInterface->setEngineFeedOpaqueItems(engineRC->_numFeedOpaqueItems); @@ -3619,6 +3626,7 @@ void Application::renderRearViewMirror(RenderArgs* renderArgs, const QRect& regi float fov = MIRROR_FIELD_OF_VIEW; // bool eyeRelativeCamera = false; + _myAvatar->startRenderCam(); //FIXME if (billboard) { fov = BILLBOARD_FIELD_OF_VIEW; // degees _mirrorCamera.setPosition(_myAvatar->getPosition() + @@ -3664,6 +3672,7 @@ void Application::renderRearViewMirror(RenderArgs* renderArgs, const QRect& regi viewport = gpu::Vec4i(0, 0, width, height); } renderArgs->_viewport = viewport; + _myAvatar->endRenderCam(); //FIXME // render rear mirror view displaySide(renderArgs, _mirrorCamera, true, billboard); diff --git a/interface/src/avatar/Avatar.cpp b/interface/src/avatar/Avatar.cpp index fbc940078e..913afbb2af 100644 --- a/interface/src/avatar/Avatar.cpp +++ b/interface/src/avatar/Avatar.cpp @@ -316,7 +316,7 @@ void Avatar::removeFromScene(AvatarSharedPointer self, std::shared_ptrupdate(); } @@ -391,7 +391,7 @@ void Avatar::render(RenderArgs* renderArgs, const glm::vec3& cameraPosition) { } if (frustum->sphereInFrustum(getPosition(), boundingRadius) == ViewFrustum::OUTSIDE) { - //FIXME endRender(); + endRenderAv(); //FIXME return; } @@ -542,7 +542,7 @@ void Avatar::render(RenderArgs* renderArgs, const glm::vec3& cameraPosition) { if (!isMyAvatar() || cameraMode != CAMERA_MODE_FIRST_PERSON) { renderDisplayName(batch, *renderArgs->_viewFrustum, renderArgs->_viewport); } - //FIXME endRender(); + endRenderAv(); //FIXME } glm::quat Avatar::computeRotationFromBodyToWorldUp(float proportion) const { @@ -1022,7 +1022,7 @@ void Avatar::setBillboard(const QByteArray& billboard) { } int Avatar::parseDataFromBuffer(const QByteArray& buffer) { - avatarLock.lockForWrite(); + startUpdate(); if (!_initialized) { // now that we have data for this Avatar we are go for init init(); @@ -1038,7 +1038,7 @@ int Avatar::parseDataFromBuffer(const QByteArray& buffer) { if (_moving && _motionState) { _motionState->addDirtyFlags(EntityItem::DIRTY_POSITION); } - avatarLock.unlock(); + endUpdate(); return bytesRead; } diff --git a/interface/src/avatar/MyAvatar.cpp b/interface/src/avatar/MyAvatar.cpp index 4e4043b28b..cc850eab9e 100644 --- a/interface/src/avatar/MyAvatar.cpp +++ b/interface/src/avatar/MyAvatar.cpp @@ -192,9 +192,6 @@ void MyAvatar::simulate(float deltaTime) { PerformanceTimer perfTimer("transform"); updateOrientation(deltaTime); updatePosition(deltaTime); - // The 2 updates set position, orientation, and all manner of physics stuff. - // Here we record the results. - nextAttitude(getPosition(), getOrientation()); } { diff --git a/libraries/avatars/src/AvatarData.cpp b/libraries/avatars/src/AvatarData.cpp index 9d7f974a68..db8837fb79 100644 --- a/libraries/avatars/src/AvatarData.cpp +++ b/libraries/avatars/src/AvatarData.cpp @@ -110,52 +110,78 @@ void AvatarData::setOrientation(const glm::quat& orientation, bool overideRefere } } -// There are a number of possible strategies, some more optimal than others in terms of using the latest info -// The current one does not update anything until captureAttitude, and then keeps that value until rendered. +// There are a number of possible strategies for this set of tools through endRender, below. void AvatarData::nextAttitude(glm::vec3 position, glm::quat orientation) { - setPosition(position, true); setOrientation(orientation, true); - _nextPending = 1; // FIXME type bool -} -void AvatarData::captureAttitude() { - if (!_nextAllowed) { // We haven't finished rendering the last one - return; - } - avatarLock.lockForWrite(); - if (_nextPending) { - _nextAllowed = false; - _nextPosition = getPosition(); - _nextOrientation = getOrientation(); - } else { - qCDebug(avatars) << "FIXME capture with nothing pending"; - } + avatarLock.lock(); + setPosition(position, true); + setOrientation(orientation, true); avatarLock.unlock(); } +void AvatarData::captureAttitude() { + avatarLock.lock(); + assert(_nextAllowed); + _nextAllowed = false; + _nextPosition = getPosition(); + _nextOrientation = getOrientation(); + avatarLock.unlock(); +} +void AvatarData::startCapture() { + avatarLock.lock(); + assert(_nextAllowed); + _nextAllowed = false; + _nextPosition = getPosition(); + _nextOrientation = getOrientation(); +} +void AvatarData::endCapture() { + avatarLock.unlock(); +} + void AvatarData::startUpdate() { - avatarLock.lockForWrite(); + avatarLock.lock(); } void AvatarData::endUpdate() { avatarLock.unlock(); } +void AvatarData::startRenderRun() { + _nextPending = true; // FIXME remove here and in .h + //startRender(); // when on: smooth when startRenderCam off; mini-mirror judder (only, both axes) when startRenderCam on + avatarLock.lock(); +} +void AvatarData::endRenderRun() { + _nextPending = false; // FIXME remove here and in .h + //endRender(); + avatarLock.unlock(); +} +void AvatarData::startRenderAv() { + startRender(); // when on: small rotate judder in all views when starRenderCam off; big rotate judder in all views (and mini-mirror forward judder) when startRenderCam on +} +void AvatarData::endRenderAv() { + endRender(); +} +void AvatarData::startRenderCam() { + //startRender(); +} +void AvatarData::endRenderCam() { + //endRender(); +} void AvatarData::startRender() { - avatarLock.lockForRead(); - if (!_nextPending) { - return; - } + //avatarLock.lock(); + _nextPending = true; // FIXME remove here and in .h glm::vec3 pos = getPosition(); glm::quat rot = getOrientation(); setPosition(_nextPosition, true); - //setOrientation(_nextOrientation, true); + setOrientation(_nextOrientation, true); updateAttitude(); _nextPosition = pos; _nextOrientation = rot; } void AvatarData::endRender() { + _nextPending = false; // FIXME remove here and in .h setPosition(_nextPosition, true); - //setOrientation(_nextOrientation, true); + setOrientation(_nextOrientation, true); updateAttitude(); - _nextPending = 0; _nextAllowed = true; - avatarLock.unlock(); + //avatarLock.unlock(); } float AvatarData::getTargetScale() const { diff --git a/libraries/avatars/src/AvatarData.h b/libraries/avatars/src/AvatarData.h index 41d8cba997..2b557e79b1 100644 --- a/libraries/avatars/src/AvatarData.h +++ b/libraries/avatars/src/AvatarData.h @@ -200,9 +200,12 @@ public: void nextAttitude(glm::vec3 position, glm::quat orientation); // Can be safely called at any time. void captureAttitude(); // Indicates that the latest values are about to be captured for camera, etc. + void startCapture(); // start/end of the period in which the latest values are about to be captured for camera, etc. + void endCapture(); void startUpdate(); // start/end of update iteration void endUpdate(); void startRender(); // start/end of rendering + void startRenderRun(); void endRenderRun(); void startRenderAv(); void endRenderAv(); void startRenderCam(); void endRenderCam(); void endRender(); virtual void updateAttitude() {} // Tell skeleton mesh about changes @@ -319,7 +322,7 @@ public: bool shouldDie() const { return _owningAvatarMixer.isNull() || getUsecsSinceLastUpdate() > AVATAR_SILENCE_THRESHOLD_USECS; } - QReadWriteLock avatarLock; // Name is redundant, but it aids searches. + QMutex avatarLock; // Name is redundant, but it aids searches. public slots: void sendAvatarDataPacket(); @@ -368,7 +371,7 @@ protected: glm::vec3 _nextPosition {}; glm::quat _nextOrientation {}; - int _nextPending = 0; + bool _nextPending = false; bool _nextAllowed = true; // Body scale diff --git a/libraries/render-utils/src/Model.cpp b/libraries/render-utils/src/Model.cpp index 99a0c81d4b..438bed437a 100644 --- a/libraries/render-utils/src/Model.cpp +++ b/libraries/render-utils/src/Model.cpp @@ -1473,9 +1473,6 @@ void Model::renderPart(RenderArgs* args, int meshIndex, int partIndex, bool tran } avatarLockForWriteIfApplicable(); - if (!_calculatedMeshPartOffsetValid) - qCDebug(renderutils) << "FIXME surprise!"; - _calculatedMeshPartOffsetValid = false; // FIXME // We need to make sure we have valid offsets calculated before we can render if (!_calculatedMeshPartOffsetValid) { _mutex.lock(); From d472fd66ffb5a95b1b8cb121c69e9c02d223389d Mon Sep 17 00:00:00 2001 From: Howard Stearns Date: Mon, 7 Sep 2015 17:26:22 -0700 Subject: [PATCH 12/22] Cleanup. --- interface/src/Application.cpp | 2 -- interface/src/avatar/Avatar.cpp | 6 +++--- libraries/avatars/src/AvatarData.cpp | 23 ++--------------------- libraries/avatars/src/AvatarData.h | 6 +++--- 4 files changed, 8 insertions(+), 29 deletions(-) diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp index eeb6563bb4..ada1260023 100644 --- a/interface/src/Application.cpp +++ b/interface/src/Application.cpp @@ -3626,7 +3626,6 @@ void Application::renderRearViewMirror(RenderArgs* renderArgs, const QRect& regi float fov = MIRROR_FIELD_OF_VIEW; // bool eyeRelativeCamera = false; - _myAvatar->startRenderCam(); //FIXME if (billboard) { fov = BILLBOARD_FIELD_OF_VIEW; // degees _mirrorCamera.setPosition(_myAvatar->getPosition() + @@ -3672,7 +3671,6 @@ void Application::renderRearViewMirror(RenderArgs* renderArgs, const QRect& regi viewport = gpu::Vec4i(0, 0, width, height); } renderArgs->_viewport = viewport; - _myAvatar->endRenderCam(); //FIXME // render rear mirror view displaySide(renderArgs, _mirrorCamera, true, billboard); diff --git a/interface/src/avatar/Avatar.cpp b/interface/src/avatar/Avatar.cpp index 913afbb2af..086178ec6c 100644 --- a/interface/src/avatar/Avatar.cpp +++ b/interface/src/avatar/Avatar.cpp @@ -316,7 +316,7 @@ void Avatar::removeFromScene(AvatarSharedPointer self, std::shared_ptrupdate(); } @@ -391,7 +391,7 @@ void Avatar::render(RenderArgs* renderArgs, const glm::vec3& cameraPosition) { } if (frustum->sphereInFrustum(getPosition(), boundingRadius) == ViewFrustum::OUTSIDE) { - endRenderAv(); //FIXME + endRender(); return; } @@ -542,7 +542,7 @@ void Avatar::render(RenderArgs* renderArgs, const glm::vec3& cameraPosition) { if (!isMyAvatar() || cameraMode != CAMERA_MODE_FIRST_PERSON) { renderDisplayName(batch, *renderArgs->_viewFrustum, renderArgs->_viewport); } - endRenderAv(); //FIXME + endRender(); } glm::quat Avatar::computeRotationFromBodyToWorldUp(float proportion) const { diff --git a/libraries/avatars/src/AvatarData.cpp b/libraries/avatars/src/AvatarData.cpp index db8837fb79..2bde964ff2 100644 --- a/libraries/avatars/src/AvatarData.cpp +++ b/libraries/avatars/src/AvatarData.cpp @@ -135,7 +135,6 @@ void AvatarData::startCapture() { void AvatarData::endCapture() { avatarLock.unlock(); } - void AvatarData::startUpdate() { avatarLock.lock(); } @@ -143,30 +142,14 @@ void AvatarData::endUpdate() { avatarLock.unlock(); } void AvatarData::startRenderRun() { - _nextPending = true; // FIXME remove here and in .h - //startRender(); // when on: smooth when startRenderCam off; mini-mirror judder (only, both axes) when startRenderCam on + // I'd like to get rid of this and just (un)lock at (end-)startRender. + // But somehow that causes judder in rotations. avatarLock.lock(); } void AvatarData::endRenderRun() { - _nextPending = false; // FIXME remove here and in .h - //endRender(); avatarLock.unlock(); } -void AvatarData::startRenderAv() { - startRender(); // when on: small rotate judder in all views when starRenderCam off; big rotate judder in all views (and mini-mirror forward judder) when startRenderCam on -} -void AvatarData::endRenderAv() { - endRender(); -} -void AvatarData::startRenderCam() { - //startRender(); -} -void AvatarData::endRenderCam() { - //endRender(); -} void AvatarData::startRender() { - //avatarLock.lock(); - _nextPending = true; // FIXME remove here and in .h glm::vec3 pos = getPosition(); glm::quat rot = getOrientation(); setPosition(_nextPosition, true); @@ -176,12 +159,10 @@ void AvatarData::startRender() { _nextOrientation = rot; } void AvatarData::endRender() { - _nextPending = false; // FIXME remove here and in .h setPosition(_nextPosition, true); setOrientation(_nextOrientation, true); updateAttitude(); _nextAllowed = true; - //avatarLock.unlock(); } float AvatarData::getTargetScale() const { diff --git a/libraries/avatars/src/AvatarData.h b/libraries/avatars/src/AvatarData.h index 2b557e79b1..15be97c22e 100644 --- a/libraries/avatars/src/AvatarData.h +++ b/libraries/avatars/src/AvatarData.h @@ -204,8 +204,9 @@ public: void endCapture(); void startUpdate(); // start/end of update iteration void endUpdate(); - void startRender(); // start/end of rendering - void startRenderRun(); void endRenderRun(); void startRenderAv(); void endRenderAv(); void startRenderCam(); void endRenderCam(); + void startRender(); // start/end of rendering of this object + void startRenderRun(); // start/end of entire scene. + void endRenderRun(); void endRender(); virtual void updateAttitude() {} // Tell skeleton mesh about changes @@ -371,7 +372,6 @@ protected: glm::vec3 _nextPosition {}; glm::quat _nextOrientation {}; - bool _nextPending = false; bool _nextAllowed = true; // Body scale From 27f4bca0a483316e72fdc3a4915455fc860c8921 Mon Sep 17 00:00:00 2001 From: Howard Stearns Date: Tue, 8 Sep 2015 14:49:58 -0700 Subject: [PATCH 13/22] Cleanup: This appears to be complete and working -- EXCEPT for loading animations on the update thread! Until I figure that out, the answer is to turn off Developer->Avatar->"Enable Avatar Update Threading", run through forward/back/left/right/strafeLeft/strafeRight, and then turn "Enable Avatar Update Threading" back on. --- interface/src/Application.cpp | 12 ++++-------- interface/src/avatar/SkeletonModel.cpp | 7 ------- interface/src/avatar/SkeletonModel.h | 2 -- libraries/avatars/src/AvatarData.cpp | 8 -------- libraries/avatars/src/AvatarData.h | 1 - libraries/render-utils/src/Model.cpp | 16 ++-------------- libraries/render-utils/src/Model.h | 2 -- 7 files changed, 6 insertions(+), 42 deletions(-) diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp index ada1260023..7da07ff64c 100644 --- a/interface/src/Application.cpp +++ b/interface/src/Application.cpp @@ -1026,8 +1026,6 @@ void Application::paintGL() { _offscreenContext->makeCurrent(); // update the avatar with a fresh HMD pose _myAvatar->updateFromHMDSensorMatrix(getHMDSensorPose()); - //_myAvatar->captureAttitude(); //FIXME - //_myAvatar->startRender(); //FIXME auto lodManager = DependencyManager::get(); @@ -1238,9 +1236,7 @@ void Application::paintGL() { // Back to the default framebuffer; gpu::Batch batch; batch.resetStages(); - //_myAvatar->startRender(); //FIXME renderArgs._context->render(batch); - //_myAvatar->endRender(); //FIXME } void Application::runTests() { @@ -3475,9 +3471,9 @@ void Application::displaySide(RenderArgs* renderArgs, Camera& theCamera, bool se // FIXME: This preRender call is temporary until we create a separate render::scene for the mirror rendering. // Then we can move this logic into the Avatar::simulate call. - _myAvatar->startRender(); //FIXME + _myAvatar->startRender(); _myAvatar->preRender(renderArgs); - _myAvatar->endRender(); //FIXME + _myAvatar->endRender(); activeRenderingThread = QThread::currentThread(); @@ -3592,9 +3588,9 @@ void Application::displaySide(RenderArgs* renderArgs, Camera& theCamera, bool se _renderEngine->setRenderContext(renderContext); // Before the deferred pass, let's try to use the render engine - _myAvatar->startRenderRun(); //FIXME + _myAvatar->startRenderRun(); _renderEngine->run(); - _myAvatar->endRenderRun(); //FIXME + _myAvatar->endRenderRun(); auto engineRC = _renderEngine->getRenderContext(); sceneInterface->setEngineFeedOpaqueItems(engineRC->_numFeedOpaqueItems); diff --git a/interface/src/avatar/SkeletonModel.cpp b/interface/src/avatar/SkeletonModel.cpp index 69aea5bb9d..7b5979e62f 100644 --- a/interface/src/avatar/SkeletonModel.cpp +++ b/interface/src/avatar/SkeletonModel.cpp @@ -39,13 +39,6 @@ SkeletonModel::SkeletonModel(Avatar* owningAvatar, QObject* parent, RigPointer r SkeletonModel::~SkeletonModel() { } -void SkeletonModel::avatarLockForWriteIfApplicable() { - //FIXME _owningAvatar->avatarLock.lockForWrite(); -} -void SkeletonModel::avatarLockReleaseIfApplicable() { - //FIXME _owningAvatar->avatarLock.unlock(); -} - void SkeletonModel::initJointStates(QVector states) { const FBXGeometry& geometry = _geometry->getFBXGeometry(); glm::mat4 rootTransform = glm::scale(_scale) * glm::translate(_offset) * geometry.offset; diff --git a/interface/src/avatar/SkeletonModel.h b/interface/src/avatar/SkeletonModel.h index 1481619a04..fc99f22ee9 100644 --- a/interface/src/avatar/SkeletonModel.h +++ b/interface/src/avatar/SkeletonModel.h @@ -105,8 +105,6 @@ public: float getHeadClipDistance() const { return _headClipDistance; } virtual void onInvalidate() override; - virtual void avatarLockForWriteIfApplicable() override; - virtual void avatarLockReleaseIfApplicable() override; void initAnimGraph(const QUrl& url, const FBXGeometry& fbxGeometry); diff --git a/libraries/avatars/src/AvatarData.cpp b/libraries/avatars/src/AvatarData.cpp index 2bde964ff2..190214017b 100644 --- a/libraries/avatars/src/AvatarData.cpp +++ b/libraries/avatars/src/AvatarData.cpp @@ -117,14 +117,6 @@ void AvatarData::nextAttitude(glm::vec3 position, glm::quat orientation) { setOrientation(orientation, true); avatarLock.unlock(); } -void AvatarData::captureAttitude() { - avatarLock.lock(); - assert(_nextAllowed); - _nextAllowed = false; - _nextPosition = getPosition(); - _nextOrientation = getOrientation(); - avatarLock.unlock(); -} void AvatarData::startCapture() { avatarLock.lock(); assert(_nextAllowed); diff --git a/libraries/avatars/src/AvatarData.h b/libraries/avatars/src/AvatarData.h index 15be97c22e..c4adae29e2 100644 --- a/libraries/avatars/src/AvatarData.h +++ b/libraries/avatars/src/AvatarData.h @@ -199,7 +199,6 @@ public: virtual void setOrientation(const glm::quat& orientation, bool overideReferential = false); void nextAttitude(glm::vec3 position, glm::quat orientation); // Can be safely called at any time. - void captureAttitude(); // Indicates that the latest values are about to be captured for camera, etc. void startCapture(); // start/end of the period in which the latest values are about to be captured for camera, etc. void endCapture(); void startUpdate(); // start/end of update iteration diff --git a/libraries/render-utils/src/Model.cpp b/libraries/render-utils/src/Model.cpp index 438bed437a..4c56556118 100644 --- a/libraries/render-utils/src/Model.cpp +++ b/libraries/render-utils/src/Model.cpp @@ -1431,15 +1431,12 @@ AABox Model::getPartBounds(int meshIndex, int partIndex) { return AABox(); } - avatarLockForWriteIfApplicable(); if (meshIndex < _meshStates.size()) { const MeshState& state = _meshStates.at(meshIndex); bool isSkinned = state.clusterMatrices.size() > 1; if (isSkinned) { // if we're skinned return the entire mesh extents because we can't know for sure our clusters don't move us - AABox box = calculateScaledOffsetAABox(_geometry->getFBXGeometry().meshExtents); - avatarLockReleaseIfApplicable(); - return box; + return calculateScaledOffsetAABox(_geometry->getFBXGeometry().meshExtents); } } if (_geometry->getFBXGeometry().meshes.size() > meshIndex) { @@ -1457,11 +1454,8 @@ AABox Model::getPartBounds(int meshIndex, int partIndex) { // // If we not skinned use the bounds of the subMesh for all it's parts const FBXMesh& mesh = _geometry->getFBXGeometry().meshes.at(meshIndex); - AABox box = calculateScaledOffsetExtents(mesh.meshExtents); - avatarLockReleaseIfApplicable(); - return box; + return calculateScaledOffsetExtents(mesh.meshExtents); } - avatarLockReleaseIfApplicable(); return AABox(); } @@ -1472,7 +1466,6 @@ void Model::renderPart(RenderArgs* args, int meshIndex, int partIndex, bool tran return; // bail asap } - avatarLockForWriteIfApplicable(); // We need to make sure we have valid offsets calculated before we can render if (!_calculatedMeshPartOffsetValid) { _mutex.lock(); @@ -1497,7 +1490,6 @@ void Model::renderPart(RenderArgs* args, int meshIndex, int partIndex, bool tran // guard against partially loaded meshes if (meshIndex >= (int)networkMeshes.size() || meshIndex >= (int)geometry.meshes.size() || meshIndex >= (int)_meshStates.size() ) { - avatarLockReleaseIfApplicable(); return; } @@ -1557,7 +1549,6 @@ void Model::renderPart(RenderArgs* args, int meshIndex, int partIndex, bool tran _meshGroupsKnown = false; // regenerate these lists next time around. _readyWhenAdded = false; // in case any of our users are using scenes invalidCalculatedMeshBoxes(); // if we have to reload, we need to assume our mesh boxes are all invalid - avatarLockReleaseIfApplicable(); return; // FIXME! } @@ -1565,7 +1556,6 @@ void Model::renderPart(RenderArgs* args, int meshIndex, int partIndex, bool tran int vertexCount = mesh.vertices.size(); if (vertexCount == 0) { // sanity check - avatarLockReleaseIfApplicable(); return; // FIXME! } @@ -1610,7 +1600,6 @@ void Model::renderPart(RenderArgs* args, int meshIndex, int partIndex, bool tran // guard against partially loaded meshes if (partIndex >= (int)networkMesh._parts.size() || partIndex >= mesh.parts.size()) { - avatarLockReleaseIfApplicable(); return; } @@ -1727,7 +1716,6 @@ void Model::renderPart(RenderArgs* args, int meshIndex, int partIndex, bool tran args->_details._trianglesRendered += part.triangleIndices.size() / INDICES_PER_TRIANGLE; args->_details._quadsRendered += part.quadIndices.size() / INDICES_PER_QUAD; } - avatarLockReleaseIfApplicable(); } void Model::segregateMeshGroups() { diff --git a/libraries/render-utils/src/Model.h b/libraries/render-utils/src/Model.h index d736cc972f..91b6b4c83b 100644 --- a/libraries/render-utils/src/Model.h +++ b/libraries/render-utils/src/Model.h @@ -113,8 +113,6 @@ public: virtual void simulate(float deltaTime, bool fullUpdate = true); void updateClusterMatrices(); - virtual void avatarLockForWriteIfApplicable() {}; - virtual void avatarLockReleaseIfApplicable() {}; /// Returns a reference to the shared geometry. const QSharedPointer& getGeometry() const { return _geometry; } From 2c856e4b08b103cfb3e4529d96a000d2686d155a Mon Sep 17 00:00:00 2001 From: Howard Stearns Date: Tue, 8 Sep 2015 16:22:06 -0700 Subject: [PATCH 14/22] Work around animation cache misbehavior wrt threads. --- interface/src/avatar/MyAvatar.cpp | 27 +++++++++++++++++++++++++++ interface/src/avatar/MyAvatar.h | 1 + 2 files changed, 28 insertions(+) diff --git a/interface/src/avatar/MyAvatar.cpp b/interface/src/avatar/MyAvatar.cpp index cc850eab9e..adfde2936b 100644 --- a/interface/src/avatar/MyAvatar.cpp +++ b/interface/src/avatar/MyAvatar.cpp @@ -704,19 +704,46 @@ float loadSetting(QSettings& settings, const char* name, float defaultValue) { return value; } +// Resource loading is not yet thread safe. If an animation is not loaded when requested by other than tha main thread, +// we block in AnimationHandle::setURL => AnimationCache::getAnimation. +// Meanwhile, the main thread will also eventually lock as it tries to render us. +// If we demand the animation from the update thread while we're locked, we'll deadlock. +// Until we untangle this, code puts the updates back on the main thread temporarilly and starts all the loading. +void MyAvatar::safelyLoadAnimations() { + qApp->setAvatarUpdateThreading(false); + _rig->addAnimationByRole("idle"); + _rig->addAnimationByRole("walk"); + _rig->addAnimationByRole("backup"); + _rig->addAnimationByRole("leftTurn"); + _rig->addAnimationByRole("rightTurn"); + _rig->addAnimationByRole("leftStrafe"); + _rig->addAnimationByRole("rightStrafe"); +} + void MyAvatar::setEnableRigAnimations(bool isEnabled) { + if (isEnabled) { + safelyLoadAnimations(); + } _rig->setEnableRig(isEnabled); if (!isEnabled) { _rig->deleteAnimations(); + } else if (Menu::getInstance()->isOptionChecked(MenuOption::EnableAvatarUpdateThreading)) { + qApp->setAvatarUpdateThreading(true); } } void MyAvatar::setEnableAnimGraph(bool isEnabled) { + if (isEnabled) { + safelyLoadAnimations(); + } _rig->setEnableAnimGraph(isEnabled); if (isEnabled) { if (_skeletonModel.readyToAddToScene()) { initAnimGraph(); } + if (Menu::getInstance()->isOptionChecked(MenuOption::EnableAvatarUpdateThreading)) { + qApp->setAvatarUpdateThreading(true); + } } else { destroyAnimGraph(); } diff --git a/interface/src/avatar/MyAvatar.h b/interface/src/avatar/MyAvatar.h index ffa93ed13c..5fdfe000e3 100644 --- a/interface/src/avatar/MyAvatar.h +++ b/interface/src/avatar/MyAvatar.h @@ -292,6 +292,7 @@ private: void initHeadBones(); void initAnimGraph(); void destroyAnimGraph(); + void safelyLoadAnimations(); // Avatar Preferences QUrl _fullAvatarURLFromPreferences; From 24f1387579cb5d61a8f9d4cec915229bdef328a8 Mon Sep 17 00:00:00 2001 From: Howard Stearns Date: Wed, 9 Sep 2015 10:34:08 -0700 Subject: [PATCH 15/22] Consistently use brace initialization in class headers. --- interface/src/Application.h | 8 ++++---- libraries/avatars/src/AvatarData.h | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/interface/src/Application.h b/interface/src/Application.h index 10901b4644..6056323aa9 100644 --- a/interface/src/Application.h +++ b/interface/src/Application.h @@ -558,10 +558,10 @@ private: KeyboardMouseDevice* _keyboardMouseDevice{ nullptr }; // Default input device, the good old keyboard mouse and maybe touchpad MyAvatar* _myAvatar; // TODO: move this and relevant code to AvatarManager (or MyAvatar as the case may be) - AvatarUpdate* _avatarUpdate = nullptr; - SimpleMovingAverage _avatarSimsPerSecond{10}; - int _avatarSimsPerSecondReport = 0; - quint64 _lastAvatarSimsPerSecondUpdate = 0; + AvatarUpdate* _avatarUpdate {nullptr}; + SimpleMovingAverage _avatarSimsPerSecond {10}; + int _avatarSimsPerSecondReport {0}; + quint64 _lastAvatarSimsPerSecondUpdate {0}; Camera _myCamera; // My view onto the world Camera _mirrorCamera; // Cammera for mirror view QRect _mirrorViewRect; diff --git a/libraries/avatars/src/AvatarData.h b/libraries/avatars/src/AvatarData.h index 53818cacab..1809704ce5 100644 --- a/libraries/avatars/src/AvatarData.h +++ b/libraries/avatars/src/AvatarData.h @@ -373,7 +373,7 @@ protected: glm::vec3 _nextPosition {}; glm::quat _nextOrientation {}; - bool _nextAllowed = true; + bool _nextAllowed {true}; // Body scale float _targetScale; From f081631075f2b4cef7c6690ca3e28cec88234913 Mon Sep 17 00:00:00 2001 From: Howard Stearns Date: Wed, 9 Sep 2015 10:36:52 -0700 Subject: [PATCH 16/22] Explicit cast. --- interface/src/avatar/AvatarUpdate.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/interface/src/avatar/AvatarUpdate.cpp b/interface/src/avatar/AvatarUpdate.cpp index c2240ef123..12836bb129 100644 --- a/interface/src/avatar/AvatarUpdate.cpp +++ b/interface/src/avatar/AvatarUpdate.cpp @@ -54,7 +54,7 @@ bool AvatarUpdate::process() { quint64 start = usecTimestampNow(); quint64 deltaMicroseconds = start - _lastAvatarUpdate; _lastAvatarUpdate = start; - float deltaSeconds = deltaMicroseconds / (float) USECS_PER_SECOND; + float deltaSeconds = (float) deltaMicroseconds / (float) USECS_PER_SECOND; Application::getInstance()->setAvatarSimrateSample(1.0f / deltaSeconds); QSharedPointer manager = DependencyManager::get(); From 5572840133ba75ef706abcc24fcd008be1b08cf4 Mon Sep 17 00:00:00 2001 From: Howard Stearns Date: Wed, 9 Sep 2015 10:39:36 -0700 Subject: [PATCH 17/22] Convert isHMD reference. --- interface/src/avatar/SkeletonModel.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/interface/src/avatar/SkeletonModel.cpp b/interface/src/avatar/SkeletonModel.cpp index d3b3c19c08..ffe15a6d04 100644 --- a/interface/src/avatar/SkeletonModel.cpp +++ b/interface/src/avatar/SkeletonModel.cpp @@ -111,7 +111,7 @@ void SkeletonModel::updateRig(float deltaTime, glm::mat4 parentTransform) { Rig::HeadParameters params; params.modelRotation = getRotation(); params.modelTranslation = getTranslation(); - params.enableLean = qApp->isHMDMode() && !myAvatar->getStandingHMDSensorMode(); + params.enableLean = qApp->getAvatarUpdater()->isHMDMode() && !myAvatar->getStandingHMDSensorMode(); params.leanSideways = head->getFinalLeanSideways(); params.leanForward = head->getFinalLeanForward(); params.torsoTwist = head->getTorsoTwist(); From 3f5744712f764c92a65b9b0886b22293484af8c7 Mon Sep 17 00:00:00 2001 From: Howard Stearns Date: Wed, 9 Sep 2015 11:04:17 -0700 Subject: [PATCH 18/22] Remove optional "parent" argument. GenericThread used to accept an optional "parent" argument, defaulting to nullptr. This was odd, because the moveToThread() in GenericThread::initialize() would become a no-op if the instance ever inits QObject(someNonNullParentQObject). (The only clue would be a log message "QObject::moveToThread: Cannot move objects with a parent", and things would end up in the same thread that created the instance.) As it turns out, all the subclasses of GenericThread do not init GenericThread(parent), so things worked as expected. --- interface/src/avatar/AvatarUpdate.cpp | 10 +--------- libraries/shared/src/GenericQueueThread.h | 2 +- libraries/shared/src/GenericThread.cpp | 4 ++-- libraries/shared/src/GenericThread.h | 2 +- 4 files changed, 5 insertions(+), 13 deletions(-) diff --git a/interface/src/avatar/AvatarUpdate.cpp b/interface/src/avatar/AvatarUpdate.cpp index 12836bb129..a4391172a7 100644 --- a/interface/src/avatar/AvatarUpdate.cpp +++ b/interface/src/avatar/AvatarUpdate.cpp @@ -16,15 +16,7 @@ #include #include "InterfaceLogging.h" -// GenericThread accepts an optional "parent" argument, defaulting to nullptr. -// This is odd, because the moveToThread() in GenericThread::initialize() will -// become a no-op if the instance ever inits QObject(someNonNullParentQObject). -// (The only clue will be a log message "QObject::moveToThread: Cannot move objects with a parent", -// and things will end up in the same thread that created this instance. Hillarity ensues.) -// As it turns out, all the other subclasses of GenericThread (at this time) do not init -// GenericThread(parent), so things work as expected. Here we explicitly init GenericThread(nullptr) -// so that there is no confusion and no chance for a hillarious thread debugging session. -AvatarUpdate::AvatarUpdate() : GenericThread(nullptr), _lastAvatarUpdate(0) { +AvatarUpdate::AvatarUpdate() : GenericThread(), _lastAvatarUpdate(0) { setObjectName("Avatar Update"); // GenericThread::initialize uses this to set the thread name. Settings settings; const int DEFAULT_TARGET_AVATAR_SIMRATE = 60; diff --git a/libraries/shared/src/GenericQueueThread.h b/libraries/shared/src/GenericQueueThread.h index 2a48ff7418..067d7a7989 100644 --- a/libraries/shared/src/GenericQueueThread.h +++ b/libraries/shared/src/GenericQueueThread.h @@ -24,7 +24,7 @@ class GenericQueueThread : public GenericThread { public: using Queue = QQueue; GenericQueueThread(QObject* parent = nullptr) - : GenericThread(parent) {} + : GenericThread() {} virtual ~GenericQueueThread() {} diff --git a/libraries/shared/src/GenericThread.cpp b/libraries/shared/src/GenericThread.cpp index 18f5224229..66af2e01c8 100644 --- a/libraries/shared/src/GenericThread.cpp +++ b/libraries/shared/src/GenericThread.cpp @@ -14,8 +14,8 @@ #include "GenericThread.h" -GenericThread::GenericThread(QObject* parent) : - QObject(parent), +GenericThread::GenericThread() : + QObject(), _stopThread(false), _isThreaded(false) // assume non-threaded, must call initialize() { diff --git a/libraries/shared/src/GenericThread.h b/libraries/shared/src/GenericThread.h index f261dc5b37..8362d3ba57 100644 --- a/libraries/shared/src/GenericThread.h +++ b/libraries/shared/src/GenericThread.h @@ -23,7 +23,7 @@ class GenericThread : public QObject { Q_OBJECT public: - GenericThread(QObject* parent = nullptr); + GenericThread(); virtual ~GenericThread(); /// Call to start the thread. From 83d14d33809a0702d50b938a9d1ab8e496eb6291 Mon Sep 17 00:00:00 2001 From: Howard Stearns Date: Wed, 9 Sep 2015 11:11:06 -0700 Subject: [PATCH 19/22] Protect avatarLock. --- libraries/avatars/src/AvatarData.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libraries/avatars/src/AvatarData.h b/libraries/avatars/src/AvatarData.h index 1809704ce5..81d252c622 100644 --- a/libraries/avatars/src/AvatarData.h +++ b/libraries/avatars/src/AvatarData.h @@ -324,8 +324,6 @@ public: bool shouldDie() const { return _owningAvatarMixer.isNull() || getUsecsSinceLastUpdate() > AVATAR_SILENCE_THRESHOLD_USECS; } - QMutex avatarLock; // Name is redundant, but it aids searches. - public slots: void sendAvatarDataPacket(); void sendIdentityPacket(); @@ -424,6 +422,8 @@ protected: SimpleMovingAverage _averageBytesReceived; + QMutex avatarLock; // Name is redundant, but it aids searches. + private: static QUrl _defaultFullAvatarModelUrl; // privatize the copy constructor and assignment operator so they cannot be called From ed2ed525b2958b8507071001c0c59efd4eb05c8f Mon Sep 17 00:00:00 2001 From: Howard Stearns Date: Wed, 9 Sep 2015 12:15:48 -0700 Subject: [PATCH 20/22] Tear down animGraph when turning off the avatar update thread. Otherwise, big time deltas accumulate. --- interface/src/Application.cpp | 1 + interface/src/avatar/MyAvatar.h | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp index b8ddf1c4c2..eb4fbe9607 100644 --- a/interface/src/Application.cpp +++ b/interface/src/Application.cpp @@ -2477,6 +2477,7 @@ void Application::init() { void Application::setAvatarUpdateThreading(bool isThreaded) { if (_avatarUpdate) { + getMyAvatar()->destroyAnimGraph(); _avatarUpdate->terminate(); } _avatarUpdate = new AvatarUpdate(); diff --git a/interface/src/avatar/MyAvatar.h b/interface/src/avatar/MyAvatar.h index cc26be9012..a3a7514778 100644 --- a/interface/src/avatar/MyAvatar.h +++ b/interface/src/avatar/MyAvatar.h @@ -152,6 +152,7 @@ public: bool getStandingHMDSensorMode() const { return _standingHMDSensorMode; } void doUpdateBillboard(); + void destroyAnimGraph(); public slots: void increaseSize(); @@ -291,7 +292,6 @@ private: void maybeUpdateBillboard(); void initHeadBones(); void initAnimGraph(); - void destroyAnimGraph(); void safelyLoadAnimations(); // Avatar Preferences From ec93fc1b05c1e49da3e01381110f7f61e519bc08 Mon Sep 17 00:00:00 2001 From: Brad Hefta-Gaub Date: Wed, 9 Sep 2015 14:24:52 -0700 Subject: [PATCH 21/22] added some debugging to breakdance toy --- examples/entityScripts/breakdanceEntity.js | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/examples/entityScripts/breakdanceEntity.js b/examples/entityScripts/breakdanceEntity.js index ece3e57062..332c381e39 100644 --- a/examples/entityScripts/breakdanceEntity.js +++ b/examples/entityScripts/breakdanceEntity.js @@ -45,14 +45,15 @@ // if the grabData says we're being grabbed, and the owner ID is our session, then we are being grabbed by this interface if (grabData.activated && grabData.avatarId == MyAvatar.sessionUUID) { - + //print("BreakdanceEntity.update() [I'm being grabbed] _this.entityID:" + _this.entityID); if (!_this.beingGrabbed) { + print("I'm was grabbed... _this.entityID:" + _this.entityID); + // remember we're being grabbed so we can detect being released _this.beingGrabbed = true; var props = Entities.getEntityProperties(entityID); var puppetPosition = getPuppetPosition(props); breakdanceStart(props.modelURL, puppetPosition); - print("I'm was grabbed..."); } else { breakdanceUpdate(); } @@ -62,7 +63,7 @@ // if we are not being grabbed, and we previously were, then we were just released, remember that // and print out a message _this.beingGrabbed = false; - print("I'm was released..."); + print("I'm was released... _this.entityID:" + _this.entityID); breakdanceEnd(); } }, @@ -73,6 +74,7 @@ // * connecting to the update signal so we can check our grabbed state preload: function(entityID) { this.entityID = entityID; + print("BreakdanceEntity.preload() this.entityID:" + this.entityID); Script.update.connect(this.update); }, @@ -80,6 +82,7 @@ // or because we've left the domain or quit the application. In all cases we want to unhook our connection // to the update signal unload: function(entityID) { + print("BreakdanceEntity.unload() this.entityID:" + this.entityID); Script.update.disconnect(this.update); }, From 62bf6c7c6fea7443ec5b121ba9a74818aa59bc19 Mon Sep 17 00:00:00 2001 From: Brad Hefta-Gaub Date: Wed, 9 Sep 2015 14:55:39 -0700 Subject: [PATCH 22/22] added some debugging to breakdance toy --- examples/entityScripts/breakdanceEntity.js | 1 + 1 file changed, 1 insertion(+) diff --git a/examples/entityScripts/breakdanceEntity.js b/examples/entityScripts/breakdanceEntity.js index 332c381e39..5d4d418fec 100644 --- a/examples/entityScripts/breakdanceEntity.js +++ b/examples/entityScripts/breakdanceEntity.js @@ -31,6 +31,7 @@ // if we're currently being grabbed and if the person grabbing us is the current interfaces avatar. // we will watch this for state changes and print out if we're being grabbed or released when it changes. update: function() { + //print("BreakdanceEntity.update() _this.entityID:" + _this.entityID); var GRAB_USER_DATA_KEY = "grabKey"; // because the update() signal doesn't have a valid this, we need to use our memorized _this to access our entityID