From f95a731249a58bb9f0914fc0bcfc818cb8f5e8f3 Mon Sep 17 00:00:00 2001 From: Brad Hefta-Gaub Date: Wed, 26 Oct 2016 13:22:03 -0700 Subject: [PATCH 01/16] add app property when the app was launched from steam --- interface/src/main.cpp | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/interface/src/main.cpp b/interface/src/main.cpp index 5b0da4f578..afde066108 100644 --- a/interface/src/main.cpp +++ b/interface/src/main.cpp @@ -127,13 +127,17 @@ int main(int argc, const char* argv[]) { QCommandLineParser parser; QCommandLineOption runServerOption("runServer", "Whether to run the server"); QCommandLineOption serverContentPathOption("serverContentPath", "Where to find server content", "serverContentPath"); + QCommandLineOption launchedFromSteamOption("launchedFromSteam", "Whether we were launched from SteamVR."); parser.addOption(runServerOption); parser.addOption(serverContentPathOption); + parser.addOption(launchedFromSteamOption); parser.parse(arguments); bool runServer = parser.isSet(runServerOption); bool serverContentPathOptionIsSet = parser.isSet(serverContentPathOption); QString serverContentPathOptionValue = serverContentPathOptionIsSet ? parser.value(serverContentPathOption) : QString(); + bool launchedFromSteam = parser.isSet(launchedFromSteamOption); + QElapsedTimer startupTime; startupTime.start(); @@ -161,6 +165,9 @@ int main(int argc, const char* argv[]) { QSettings::setDefaultFormat(QSettings::IniFormat); Application app(argc, const_cast(argv), startupTime, runServer, serverContentPathOptionValue); + QVariant launchedFromSteamVariant(launchedFromSteam); + app.setProperty("com.highfidelity.launchedFromSteam", launchedFromSteamVariant); + // If we failed the OpenGLVersion check, log it. if (override) { auto accountManager = DependencyManager::get(); From 5f2b4c2a7f24111980b819a7252b0b64d8502ea6 Mon Sep 17 00:00:00 2001 From: Brad Hefta-Gaub Date: Wed, 26 Oct 2016 13:27:11 -0700 Subject: [PATCH 02/16] CR feedback --- interface/src/main.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/interface/src/main.cpp b/interface/src/main.cpp index afde066108..78cec16237 100644 --- a/interface/src/main.cpp +++ b/interface/src/main.cpp @@ -165,8 +165,7 @@ int main(int argc, const char* argv[]) { QSettings::setDefaultFormat(QSettings::IniFormat); Application app(argc, const_cast(argv), startupTime, runServer, serverContentPathOptionValue); - QVariant launchedFromSteamVariant(launchedFromSteam); - app.setProperty("com.highfidelity.launchedFromSteam", launchedFromSteamVariant); + app.setProperty("com.highfidelity.launchedFromSteam", launchedFromSteam); // If we failed the OpenGLVersion check, log it. if (override) { From e464858c4c4034cd6f5f7f78df414e18d62443f1 Mon Sep 17 00:00:00 2001 From: Brad Hefta-Gaub Date: Wed, 26 Oct 2016 14:08:18 -0700 Subject: [PATCH 03/16] use SteamClient::isRunning() instead of command line param --- interface/src/main.cpp | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/interface/src/main.cpp b/interface/src/main.cpp index 78cec16237..ab4ab689f7 100644 --- a/interface/src/main.cpp +++ b/interface/src/main.cpp @@ -127,17 +127,13 @@ int main(int argc, const char* argv[]) { QCommandLineParser parser; QCommandLineOption runServerOption("runServer", "Whether to run the server"); QCommandLineOption serverContentPathOption("serverContentPath", "Where to find server content", "serverContentPath"); - QCommandLineOption launchedFromSteamOption("launchedFromSteam", "Whether we were launched from SteamVR."); parser.addOption(runServerOption); parser.addOption(serverContentPathOption); - parser.addOption(launchedFromSteamOption); parser.parse(arguments); bool runServer = parser.isSet(runServerOption); bool serverContentPathOptionIsSet = parser.isSet(serverContentPathOption); QString serverContentPathOptionValue = serverContentPathOptionIsSet ? parser.value(serverContentPathOption) : QString(); - bool launchedFromSteam = parser.isSet(launchedFromSteamOption); - QElapsedTimer startupTime; startupTime.start(); @@ -165,6 +161,7 @@ int main(int argc, const char* argv[]) { QSettings::setDefaultFormat(QSettings::IniFormat); Application app(argc, const_cast(argv), startupTime, runServer, serverContentPathOptionValue); + bool launchedFromSteam = SteamClient::isRunning(); app.setProperty("com.highfidelity.launchedFromSteam", launchedFromSteam); // If we failed the OpenGLVersion check, log it. From 10e6157ab9517ac1f58cfb8d263dac041b5026ba Mon Sep 17 00:00:00 2001 From: Ryan Huffman Date: Wed, 26 Oct 2016 15:16:29 -0700 Subject: [PATCH 04/16] Fix race condition in BatchLoader --- libraries/script-engine/src/BatchLoader.cpp | 27 +++++++++++++-------- libraries/script-engine/src/BatchLoader.h | 11 ++++++++- 2 files changed, 27 insertions(+), 11 deletions(-) diff --git a/libraries/script-engine/src/BatchLoader.cpp b/libraries/script-engine/src/BatchLoader.cpp index 605d7e95bd..ef8cf90656 100644 --- a/libraries/script-engine/src/BatchLoader.cpp +++ b/libraries/script-engine/src/BatchLoader.cpp @@ -44,33 +44,40 @@ void BatchLoader::start() { return; } + for (const auto& rawURL : _urls) { QUrl url = expandScriptUrl(normalizeScriptURL(rawURL)); qCDebug(scriptengine) << "Loading script at " << url; - QPointer self = this; - DependencyManager::get()->getScriptContents(url.toString(), [this, self](const QString& url, const QString& contents, bool isURL, bool success) { - if (!self) { - return; - } + auto scriptCache = DependencyManager::get(); - // Because the ScriptCache may call this callback from differents threads, - // we need to make sure this is thread-safe. - std::lock_guard lock(_dataLock); + // Use a proxy callback to handle the call and emit the signal in a thread-safe way. + // If BatchLoader is deleted before the callback is called, the subsequent "emit" call will not do + // anything. + ScriptCacheSignalProxy* proxy = new ScriptCacheSignalProxy(scriptCache.data()); + scriptCache->getScriptContents(url.toString(), [proxy](const QString& url, const QString& contents, bool isURL, bool success) { + proxy->receivedContent(url, contents, isURL, success); + proxy->deleteLater(); + }, false); + connect(proxy, &ScriptCacheSignalProxy::contentAvailable, this, [this](const QString& url, const QString& contents, bool isURL, bool success) { if (isURL && success) { _data.insert(url, contents); qCDebug(scriptengine) << "Loaded: " << url; } else { _data.insert(url, QString()); - qCDebug(scriptengine) << "Could not load" << url; + qCDebug(scriptengine) << "Could not load: " << url; } if (!_finished && _urls.size() == _data.size()) { _finished = true; emit finished(_data); } - }, false); + }); } } + +void ScriptCacheSignalProxy::receivedContent(const QString& url, const QString& contents, bool isURL, bool success) { + emit contentAvailable(url, contents, isURL, success); +} diff --git a/libraries/script-engine/src/BatchLoader.h b/libraries/script-engine/src/BatchLoader.h index 40b43d23b6..55acc11c26 100644 --- a/libraries/script-engine/src/BatchLoader.h +++ b/libraries/script-engine/src/BatchLoader.h @@ -21,6 +21,16 @@ #include +class ScriptCacheSignalProxy : public QObject { + Q_OBJECT +public: + ScriptCacheSignalProxy(QObject* parent) : QObject(parent) { } + void receivedContent(const QString& url, const QString& contents, bool isURL, bool success); + +signals: + void contentAvailable(const QString& url, const QString& contents, bool isURL, bool success); +}; + class BatchLoader : public QObject { Q_OBJECT public: @@ -39,7 +49,6 @@ private: bool _finished; QSet _urls; QMap _data; - std::mutex _dataLock; }; #endif // hifi_BatchLoader_h From e7355e84f3c5d99ddc9f548c0abe74f4cbf830f8 Mon Sep 17 00:00:00 2001 From: Brad Davis Date: Tue, 25 Oct 2016 22:09:00 -0700 Subject: [PATCH 05/16] Disable OpenVR submit thread --- plugins/openvr/src/OpenVrDisplayPlugin.cpp | 1 + plugins/openvr/src/OpenVrDisplayPlugin.h | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/plugins/openvr/src/OpenVrDisplayPlugin.cpp b/plugins/openvr/src/OpenVrDisplayPlugin.cpp index b9a491a8a2..d54cc540a5 100644 --- a/plugins/openvr/src/OpenVrDisplayPlugin.cpp +++ b/plugins/openvr/src/OpenVrDisplayPlugin.cpp @@ -625,6 +625,7 @@ void OpenVrDisplayPlugin::hmdPresent() { vr::VRCompositor()->Submit(vr::Eye_Left, &vrTexture, &OPENVR_TEXTURE_BOUNDS_LEFT); vr::VRCompositor()->Submit(vr::Eye_Right, &vrTexture, &OPENVR_TEXTURE_BOUNDS_RIGHT); vr::VRCompositor()->PostPresentHandoff(); + _presentRate.increment(); #endif } diff --git a/plugins/openvr/src/OpenVrDisplayPlugin.h b/plugins/openvr/src/OpenVrDisplayPlugin.h index 025f879d84..dd8381b5ee 100644 --- a/plugins/openvr/src/OpenVrDisplayPlugin.h +++ b/plugins/openvr/src/OpenVrDisplayPlugin.h @@ -15,7 +15,7 @@ const float TARGET_RATE_OpenVr = 90.0f; // FIXME: get from sdk tracked device property? This number is vive-only. -#define OPENVR_THREADED_SUBMIT 1 +#define OPENVR_THREADED_SUBMIT 0 #if OPENVR_THREADED_SUBMIT namespace gl { From 857f5a69d6e35abf6974908296afd4f672873018 Mon Sep 17 00:00:00 2001 From: Brad Davis Date: Tue, 25 Oct 2016 22:10:50 -0700 Subject: [PATCH 06/16] Don't reserve additional thread when we're not using one --- plugins/openvr/src/OpenVrDisplayPlugin.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/plugins/openvr/src/OpenVrDisplayPlugin.h b/plugins/openvr/src/OpenVrDisplayPlugin.h index dd8381b5ee..9d7b4fbb19 100644 --- a/plugins/openvr/src/OpenVrDisplayPlugin.h +++ b/plugins/openvr/src/OpenVrDisplayPlugin.h @@ -58,8 +58,10 @@ public: void unsuppressKeyboard() override; bool isKeyboardVisible() override; +#if OPENVR_THREADED_SUBMIT // Needs an additional thread for VR submission int getRequiredThreadCount() const override { return Parent::getRequiredThreadCount() + 1; } +#endif protected: bool internalActivate() override; From 1c89fa22916422d24d6e47fcd5eb93653af0e26e Mon Sep 17 00:00:00 2001 From: Brad Davis Date: Wed, 26 Oct 2016 10:36:17 -0700 Subject: [PATCH 07/16] Make threaded submit in OpenVR controllable from menu --- interface/src/Menu.cpp | 3 + interface/src/Menu.h | 1 + plugins/openvr/src/OpenVrDisplayPlugin.cpp | 161 +++++++++++---------- plugins/openvr/src/OpenVrDisplayPlugin.h | 14 +- 4 files changed, 90 insertions(+), 89 deletions(-) diff --git a/interface/src/Menu.cpp b/interface/src/Menu.cpp index 68d1065f72..cd8d39b739 100644 --- a/interface/src/Menu.cpp +++ b/interface/src/Menu.cpp @@ -338,6 +338,9 @@ Menu::Menu() { // Developer > Render > Throttle FPS If Not Focus addCheckableActionToQMenuAndActionHash(renderOptionsMenu, MenuOption::ThrottleFPSIfNotFocus, 0, true); + // Developer > Render > OpenVR threaded submit + addCheckableActionToQMenuAndActionHash(renderOptionsMenu, MenuOption::OpenVrThreadedSubmit, 0, true); + // Developer > Render > Resolution MenuWrapper* resolutionMenu = renderOptionsMenu->addMenu(MenuOption::RenderResolution); QActionGroup* resolutionGroup = new QActionGroup(resolutionMenu); diff --git a/interface/src/Menu.h b/interface/src/Menu.h index 7cb3c47b43..640a3e05d2 100644 --- a/interface/src/Menu.h +++ b/interface/src/Menu.h @@ -136,6 +136,7 @@ namespace MenuOption { const QString OctreeStats = "Entity Statistics"; const QString OnePointCalibration = "1 Point Calibration"; const QString OnlyDisplayTopTen = "Only Display Top Ten"; + const QString OpenVrThreadedSubmit = "OpenVR Threaded Submit"; const QString OutputMenu = "Display"; const QString Overlays = "Overlays"; const QString PackageModel = "Package Model..."; diff --git a/plugins/openvr/src/OpenVrDisplayPlugin.cpp b/plugins/openvr/src/OpenVrDisplayPlugin.cpp index d54cc540a5..646c7fffd4 100644 --- a/plugins/openvr/src/OpenVrDisplayPlugin.cpp +++ b/plugins/openvr/src/OpenVrDisplayPlugin.cpp @@ -35,6 +35,7 @@ Q_DECLARE_LOGGING_CATEGORY(displayplugins) const QString OpenVrDisplayPlugin::NAME("OpenVR (Vive)"); const QString StandingHMDSensorMode = "Standing HMD Sensor Mode"; // this probably shouldn't be hardcoded here +const QString OpenVrThreadedSubmit = "OpenVR Threaded Submit"; // this probably shouldn't be hardcoded here PoseData _nextRenderPoseData; PoseData _nextSimPoseData; @@ -49,8 +50,6 @@ bool _openVrDisplayActive { false }; static vr::VRTextureBounds_t OPENVR_TEXTURE_BOUNDS_LEFT{ 0, 0, 0.5f, 1 }; static vr::VRTextureBounds_t OPENVR_TEXTURE_BOUNDS_RIGHT{ 0.5f, 0, 1, 1 }; -#if OPENVR_THREADED_SUBMIT - #define REPROJECTION_BINDING 1 static const char* HMD_REPROJECTION_VERT = R"SHADER( @@ -351,8 +350,6 @@ public: OpenVrDisplayPlugin& _plugin; }; -#endif - bool OpenVrDisplayPlugin::isSupported() const { return openVrSupported(); } @@ -394,6 +391,9 @@ bool OpenVrDisplayPlugin::internalActivate() { return false; } + _threadedSubmit = _container->isOptionChecked(OpenVrThreadedSubmit); + qDebug() << "OpenVR Threaded submit enabled: " << _threadedSubmit; + _openVrDisplayActive = true; _container->setIsOptionChecked(StandingHMDSensorMode, true); @@ -434,16 +434,16 @@ bool OpenVrDisplayPlugin::internalActivate() { #endif } -#if OPENVR_THREADED_SUBMIT - _submitThread = std::make_shared(*this); - if (!_submitCanvas) { - withMainThreadContext([&] { - _submitCanvas = std::make_shared(); - _submitCanvas->create(); - _submitCanvas->doneCurrent(); - }); + if (_threadedSubmit) { + _submitThread = std::make_shared(*this); + if (!_submitCanvas) { + withMainThreadContext([&] { + _submitCanvas = std::make_shared(); + _submitCanvas->create(); + _submitCanvas->doneCurrent(); + }); + } } -#endif return Parent::internalActivate(); } @@ -473,27 +473,27 @@ void OpenVrDisplayPlugin::customizeContext() { Parent::customizeContext(); -#if OPENVR_THREADED_SUBMIT - _compositeInfos[0].texture = _compositeFramebuffer->getRenderBuffer(0); - for (size_t i = 0; i < COMPOSITING_BUFFER_SIZE; ++i) { - if (0 != i) { - _compositeInfos[i].texture = gpu::TexturePointer(gpu::Texture::create2D(gpu::Element::COLOR_RGBA_32, _renderTargetSize.x, _renderTargetSize.y, gpu::Sampler(gpu::Sampler::FILTER_MIN_MAG_POINT))); + if (_threadedSubmit) { + _compositeInfos[0].texture = _compositeFramebuffer->getRenderBuffer(0); + for (size_t i = 0; i < COMPOSITING_BUFFER_SIZE; ++i) { + if (0 != i) { + _compositeInfos[i].texture = gpu::TexturePointer(gpu::Texture::create2D(gpu::Element::COLOR_RGBA_32, _renderTargetSize.x, _renderTargetSize.y, gpu::Sampler(gpu::Sampler::FILTER_MIN_MAG_POINT))); + } + _compositeInfos[i].textureID = getGLBackend()->getTextureID(_compositeInfos[i].texture, false); } - _compositeInfos[i].textureID = getGLBackend()->getTextureID(_compositeInfos[i].texture, false); + _submitThread->_canvas = _submitCanvas; + _submitThread->start(QThread::HighPriority); } - _submitThread->_canvas = _submitCanvas; - _submitThread->start(QThread::HighPriority); -#endif } void OpenVrDisplayPlugin::uncustomizeContext() { Parent::uncustomizeContext(); -#if OPENVR_THREADED_SUBMIT - _submitThread->_quit = true; - _submitThread->wait(); - _submitThread.reset(); -#endif + if (_threadedSubmit) { + _submitThread->_quit = true; + _submitThread->wait(); + _submitThread.reset(); + } } void OpenVrDisplayPlugin::resetSensors() { @@ -582,76 +582,77 @@ bool OpenVrDisplayPlugin::beginFrameRender(uint32_t frameIndex) { } void OpenVrDisplayPlugin::compositeLayers() { -#if OPENVR_THREADED_SUBMIT - ++_renderingIndex; - _renderingIndex %= COMPOSITING_BUFFER_SIZE; + if (_threadedSubmit) { + ++_renderingIndex; + _renderingIndex %= COMPOSITING_BUFFER_SIZE; - auto& newComposite = _compositeInfos[_renderingIndex]; - newComposite.pose = _currentPresentFrameInfo.presentPose; - _compositeFramebuffer->setRenderBuffer(0, newComposite.texture); -#endif + auto& newComposite = _compositeInfos[_renderingIndex]; + newComposite.pose = _currentPresentFrameInfo.presentPose; + _compositeFramebuffer->setRenderBuffer(0, newComposite.texture); + } Parent::compositeLayers(); -#if OPENVR_THREADED_SUBMIT - newComposite.fence = glFenceSync(GL_SYNC_GPU_COMMANDS_COMPLETE, 0); - // https://www.opengl.org/registry/specs/ARB/sync.txt: - // > The simple flushing behavior defined by - // > SYNC_FLUSH_COMMANDS_BIT will not help when waiting for a fence - // > command issued in another context's command stream to complete. - // > Applications which block on a fence sync object must take - // > additional steps to assure that the context from which the - // > corresponding fence command was issued has flushed that command - // > to the graphics pipeline. - glFlush(); + if (_threadedSubmit) { + auto& newComposite = _compositeInfos[_renderingIndex]; + newComposite.fence = glFenceSync(GL_SYNC_GPU_COMMANDS_COMPLETE, 0); + // https://www.opengl.org/registry/specs/ARB/sync.txt: + // > The simple flushing behavior defined by + // > SYNC_FLUSH_COMMANDS_BIT will not help when waiting for a fence + // > command issued in another context's command stream to complete. + // > Applications which block on a fence sync object must take + // > additional steps to assure that the context from which the + // > corresponding fence command was issued has flushed that command + // > to the graphics pipeline. + glFlush(); - if (!newComposite.textureID) { - newComposite.textureID = getGLBackend()->getTextureID(newComposite.texture, false); + if (!newComposite.textureID) { + newComposite.textureID = getGLBackend()->getTextureID(newComposite.texture, false); + } + withPresentThreadLock([&] { + _submitThread->update(newComposite); + }); } - withPresentThreadLock([&] { - _submitThread->update(newComposite); - }); -#endif } void OpenVrDisplayPlugin::hmdPresent() { PROFILE_RANGE_EX(__FUNCTION__, 0xff00ff00, (uint64_t)_currentFrame->frameIndex) -#if OPENVR_THREADED_SUBMIT - _submitThread->waitForPresent(); -#else - GLuint glTexId = getGLBackend()->getTextureID(_compositeFramebuffer->getRenderBuffer(0), false); - vr::Texture_t vrTexture{ (void*)glTexId, vr::API_OpenGL, vr::ColorSpace_Auto }; - vr::VRCompositor()->Submit(vr::Eye_Left, &vrTexture, &OPENVR_TEXTURE_BOUNDS_LEFT); - vr::VRCompositor()->Submit(vr::Eye_Right, &vrTexture, &OPENVR_TEXTURE_BOUNDS_RIGHT); - vr::VRCompositor()->PostPresentHandoff(); - _presentRate.increment(); -#endif + if (_threadedSubmit) { + _submitThread->waitForPresent(); + } else { + GLuint glTexId = getGLBackend()->getTextureID(_compositeFramebuffer->getRenderBuffer(0), false); + vr::Texture_t vrTexture { (void*)glTexId, vr::API_OpenGL, vr::ColorSpace_Auto }; + vr::VRCompositor()->Submit(vr::Eye_Left, &vrTexture, &OPENVR_TEXTURE_BOUNDS_LEFT); + vr::VRCompositor()->Submit(vr::Eye_Right, &vrTexture, &OPENVR_TEXTURE_BOUNDS_RIGHT); + vr::VRCompositor()->PostPresentHandoff(); + _presentRate.increment(); + } } void OpenVrDisplayPlugin::postPreview() { PROFILE_RANGE_EX(__FUNCTION__, 0xff00ff00, (uint64_t)_currentFrame->frameIndex) PoseData nextRender, nextSim; nextRender.frameIndex = presentCount(); -#if !OPENVR_THREADED_SUBMIT - vr::VRCompositor()->WaitGetPoses(nextRender.vrPoses, vr::k_unMaxTrackedDeviceCount, nextSim.vrPoses, vr::k_unMaxTrackedDeviceCount); + if (_threadedSubmit) { + _hmdActivityLevel = _system->GetTrackedDeviceActivityLevel(vr::k_unTrackedDeviceIndex_Hmd); + } else { + vr::VRCompositor()->WaitGetPoses(nextRender.vrPoses, vr::k_unMaxTrackedDeviceCount, nextSim.vrPoses, vr::k_unMaxTrackedDeviceCount); - glm::mat4 resetMat; - withPresentThreadLock([&] { - resetMat = _sensorResetMat; - }); - nextRender.update(resetMat); - nextSim.update(resetMat); - withPresentThreadLock([&] { - _nextSimPoseData = nextSim; - }); - _nextRenderPoseData = nextRender; + glm::mat4 resetMat; + withPresentThreadLock([&] { + resetMat = _sensorResetMat; + }); + nextRender.update(resetMat); + nextSim.update(resetMat); + withPresentThreadLock([&] { + _nextSimPoseData = nextSim; + }); + _nextRenderPoseData = nextRender; - // FIXME - this looks wrong! - _hmdActivityLevel = vr::k_EDeviceActivityLevel_UserInteraction; // _system->GetTrackedDeviceActivityLevel(vr::k_unTrackedDeviceIndex_Hmd); -#else - _hmdActivityLevel = _system->GetTrackedDeviceActivityLevel(vr::k_unTrackedDeviceIndex_Hmd); -#endif + // FIXME - this looks wrong! + _hmdActivityLevel = vr::k_EDeviceActivityLevel_UserInteraction; // _system->GetTrackedDeviceActivityLevel(vr::k_unTrackedDeviceIndex_Hmd); + } } bool OpenVrDisplayPlugin::isHmdMounted() const { @@ -685,3 +686,7 @@ void OpenVrDisplayPlugin::unsuppressKeyboard() { bool OpenVrDisplayPlugin::isKeyboardVisible() { return isOpenVrKeyboardShown(); } + +int OpenVrDisplayPlugin::getRequiredThreadCount() const { + return Parent::getRequiredThreadCount() + (_threadedSubmit ? 1 : 0); +} \ No newline at end of file diff --git a/plugins/openvr/src/OpenVrDisplayPlugin.h b/plugins/openvr/src/OpenVrDisplayPlugin.h index 9d7b4fbb19..d867c71cb6 100644 --- a/plugins/openvr/src/OpenVrDisplayPlugin.h +++ b/plugins/openvr/src/OpenVrDisplayPlugin.h @@ -15,9 +15,6 @@ const float TARGET_RATE_OpenVr = 90.0f; // FIXME: get from sdk tracked device property? This number is vive-only. -#define OPENVR_THREADED_SUBMIT 0 - -#if OPENVR_THREADED_SUBMIT namespace gl { class OffscreenContext; } @@ -34,7 +31,6 @@ struct CompositeInfo { glm::mat4 pose; GLsync fence{ 0 }; }; -#endif class OpenVrDisplayPlugin : public HmdDisplayPlugin { using Parent = HmdDisplayPlugin; @@ -58,10 +54,8 @@ public: void unsuppressKeyboard() override; bool isKeyboardVisible() override; -#if OPENVR_THREADED_SUBMIT - // Needs an additional thread for VR submission - int getRequiredThreadCount() const override { return Parent::getRequiredThreadCount() + 1; } -#endif + // Possibly needs an additional thread for VR submission + int getRequiredThreadCount() const override; protected: bool internalActivate() override; @@ -73,7 +67,6 @@ protected: bool isHmdMounted() const override; void postPreview() override; - private: vr::IVRSystem* _system { nullptr }; std::atomic _hmdActivityLevel { vr::k_EDeviceActivityLevel_Unknown }; @@ -82,12 +75,11 @@ private: vr::HmdMatrix34_t _lastGoodHMDPose; mat4 _sensorResetMat; + bool _threadedSubmit { true }; -#if OPENVR_THREADED_SUBMIT CompositeInfo::Array _compositeInfos; size_t _renderingIndex { 0 }; std::shared_ptr _submitThread; std::shared_ptr _submitCanvas; friend class OpenVrSubmitThread; -#endif }; From f4f1b04cf0509e14c75bc98ab459e780d616c521 Mon Sep 17 00:00:00 2001 From: Brad Davis Date: Wed, 26 Oct 2016 16:02:06 -0700 Subject: [PATCH 08/16] Use OpenVR to determine if async reprojection is enabled --- cmake/externals/openvr/CMakeLists.txt | 4 ++-- plugins/openvr/src/OpenVrDisplayPlugin.cpp | 11 ++++++++++- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/cmake/externals/openvr/CMakeLists.txt b/cmake/externals/openvr/CMakeLists.txt index 1cd4c071f1..19a9dd1f15 100644 --- a/cmake/externals/openvr/CMakeLists.txt +++ b/cmake/externals/openvr/CMakeLists.txt @@ -7,8 +7,8 @@ string(TOUPPER ${EXTERNAL_NAME} EXTERNAL_NAME_UPPER) ExternalProject_Add( ${EXTERNAL_NAME} - URL https://github.com/ValveSoftware/openvr/archive/v1.0.2.zip - URL_MD5 0d1cf5f579cf092e33f34759967b7046 + URL https://github.com/ValveSoftware/openvr/archive/v1.0.3.zip + URL_MD5 b484b12901917cc739e40389583c8b0d CONFIGURE_COMMAND "" BUILD_COMMAND "" INSTALL_COMMAND "" diff --git a/plugins/openvr/src/OpenVrDisplayPlugin.cpp b/plugins/openvr/src/OpenVrDisplayPlugin.cpp index 646c7fffd4..567d8f513b 100644 --- a/plugins/openvr/src/OpenVrDisplayPlugin.cpp +++ b/plugins/openvr/src/OpenVrDisplayPlugin.cpp @@ -373,6 +373,9 @@ void OpenVrDisplayPlugin::init() { emit deviceConnected(getName()); } +// FIXME remove once OpenVR header is updated +#define VRCompositor_ReprojectionAsync 0x04 + bool OpenVrDisplayPlugin::internalActivate() { if (!_system) { _system = acquireOpenVrSystem(); @@ -391,7 +394,13 @@ bool OpenVrDisplayPlugin::internalActivate() { return false; } - _threadedSubmit = _container->isOptionChecked(OpenVrThreadedSubmit); + vr::Compositor_FrameTiming timing; + memset(&timing, 0, sizeof(timing)); + timing.m_nSize = sizeof(vr::Compositor_FrameTiming); + vr::VRCompositor()->GetFrameTiming(&timing); + bool asyncReprojectionActive = timing.m_nReprojectionFlags & VRCompositor_ReprojectionAsync; + + _threadedSubmit = !asyncReprojectionActive; qDebug() << "OpenVR Threaded submit enabled: " << _threadedSubmit; _openVrDisplayActive = true; From 4abc4adde967bb27a4d36c62c95c6a20431ba4c5 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Wed, 26 Oct 2016 16:18:12 -0700 Subject: [PATCH 09/16] add a timer to check for readyRead backup --- libraries/networking/src/udt/Socket.cpp | 22 ++++++++++++++++++++++ libraries/networking/src/udt/Socket.h | 3 +++ 2 files changed, 25 insertions(+) diff --git a/libraries/networking/src/udt/Socket.cpp b/libraries/networking/src/udt/Socket.cpp index 7ddbd54593..43bd7dd973 100644 --- a/libraries/networking/src/udt/Socket.cpp +++ b/libraries/networking/src/udt/Socket.cpp @@ -32,6 +32,7 @@ using namespace udt; Socket::Socket(QObject* parent, bool shouldChangeSocketOptions) : QObject(parent), _synTimer(new QTimer(this)), + _readyReadBackupTimer(new QTimer(this)), _shouldChangeSocketOptions(shouldChangeSocketOptions) { connect(&_udpSocket, &QUdpSocket::readyRead, this, &Socket::readPendingDatagrams); @@ -46,6 +47,11 @@ Socket::Socket(QObject* parent, bool shouldChangeSocketOptions) : connect(&_udpSocket, SIGNAL(error(QAbstractSocket::SocketError)), this, SLOT(handleSocketError(QAbstractSocket::SocketError))); connect(&_udpSocket, &QAbstractSocket::stateChanged, this, &Socket::handleStateChanged); + + // in order to help track down the zombie server bug, add a timer to check if we missed a readyRead + const int READY_READ_BACKUP_CHECK_MSECS = 10 * 1000; + connect(_readyReadBackupTimer, &QTimer::timeout, this, &Socket::checkForReadyReadBackup); + _readyReadBackupTimer->start(READY_READ_BACKUP_CHECK_MSECS); } void Socket::bind(const QHostAddress& address, quint16 port) { @@ -296,9 +302,25 @@ void Socket::messageFailed(Connection* connection, Packet::MessageNumber message } } +void Socket::checkForReadyReadBackup() { + if (_udpSocket.hasPendingDatagrams()) { + qCDebug(networking) << "Socket::checkForReadyReadBackup() detected blocked readyRead signal. Flushing pending datagrams."; + + // drop all of the pending datagrams on the floor + while (_udpSocket.hasPendingDatagrams()) { + _udpSocket.readDatagram(nullptr, 0); + } + } +} + void Socket::readPendingDatagrams() { int packetSizeWithHeader = -1; + while ((packetSizeWithHeader = _udpSocket.pendingDatagramSize()) != -1) { + + // we're reading a packet so re-start the readyRead backup timer + _readyReadBackupTimer->start(); + // grab a time point we can mark as the receive time of this packet auto receiveTime = p_high_resolution_clock::now(); diff --git a/libraries/networking/src/udt/Socket.h b/libraries/networking/src/udt/Socket.h index d65cb1406c..a811d78958 100644 --- a/libraries/networking/src/udt/Socket.h +++ b/libraries/networking/src/udt/Socket.h @@ -101,6 +101,7 @@ public slots: private slots: void readPendingDatagrams(); + void checkForReadyReadBackup(); void rateControlSync(); void handleSocketError(QAbstractSocket::SocketError socketError); @@ -136,6 +137,8 @@ private: int _synInterval { 10 }; // 10ms QTimer* _synTimer { nullptr }; + QTimer* _readyReadBackupTimer { nullptr }; + int _maxBandwidth { -1 }; std::unique_ptr _ccFactory { new CongestionControlFactory() }; From a53049b18652d6e3c44c3a8a9f002abaefbc2dad Mon Sep 17 00:00:00 2001 From: Brad Davis Date: Wed, 26 Oct 2016 17:19:24 -0700 Subject: [PATCH 10/16] Consolidate activity tracking code --- plugins/openvr/src/OpenVrDisplayPlugin.cpp | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/plugins/openvr/src/OpenVrDisplayPlugin.cpp b/plugins/openvr/src/OpenVrDisplayPlugin.cpp index 567d8f513b..18a2ce99c3 100644 --- a/plugins/openvr/src/OpenVrDisplayPlugin.cpp +++ b/plugins/openvr/src/OpenVrDisplayPlugin.cpp @@ -643,9 +643,10 @@ void OpenVrDisplayPlugin::postPreview() { PROFILE_RANGE_EX(__FUNCTION__, 0xff00ff00, (uint64_t)_currentFrame->frameIndex) PoseData nextRender, nextSim; nextRender.frameIndex = presentCount(); - if (_threadedSubmit) { - _hmdActivityLevel = _system->GetTrackedDeviceActivityLevel(vr::k_unTrackedDeviceIndex_Hmd); - } else { + + _hmdActivityLevel = _system->GetTrackedDeviceActivityLevel(vr::k_unTrackedDeviceIndex_Hmd); + + if (!_threadedSubmit) { vr::VRCompositor()->WaitGetPoses(nextRender.vrPoses, vr::k_unMaxTrackedDeviceCount, nextSim.vrPoses, vr::k_unMaxTrackedDeviceCount); glm::mat4 resetMat; @@ -659,8 +660,6 @@ void OpenVrDisplayPlugin::postPreview() { }); _nextRenderPoseData = nextRender; - // FIXME - this looks wrong! - _hmdActivityLevel = vr::k_EDeviceActivityLevel_UserInteraction; // _system->GetTrackedDeviceActivityLevel(vr::k_unTrackedDeviceIndex_Hmd); } } From 1ea018db1d0a146bcbb9e2eb0ad52c693e9a452a Mon Sep 17 00:00:00 2001 From: "Anthony J. Thibault" Date: Wed, 26 Oct 2016 17:40:34 -0700 Subject: [PATCH 11/16] Fix for crash in openvr on startup pluginActivate can fail and return false, when steamvr is in a "Not Ready" state. In practice this can occur on a botched steamvr install, or when the vr compositor process is unable to start. When pluginActivate returns false, there's some sort of race condition where pluginUpdate will still be called. But it's only reproducible in release builds, without the debugger attached. This commit prevents pluginUpdate from crashing by making sure that the openvr _system ptr is valid first. --- plugins/openvr/src/ViveControllerManager.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/plugins/openvr/src/ViveControllerManager.cpp b/plugins/openvr/src/ViveControllerManager.cpp index 2d2720e388..ff8fc64474 100644 --- a/plugins/openvr/src/ViveControllerManager.cpp +++ b/plugins/openvr/src/ViveControllerManager.cpp @@ -210,6 +210,11 @@ void ViveControllerManager::renderHand(const controller::Pose& pose, gpu::Batch& void ViveControllerManager::pluginUpdate(float deltaTime, const controller::InputCalibrationData& inputCalibrationData) { + + if (!_system) { + return; + } + auto userInputMapper = DependencyManager::get(); handleOpenVrEvents(); if (openVrQuitRequested()) { From 7ac3a189a0763c6cc0aa141e714d34c4abc27c2b Mon Sep 17 00:00:00 2001 From: "Anthony J. Thibault" Date: Wed, 26 Oct 2016 17:48:28 -0700 Subject: [PATCH 12/16] generate pdb files for plugins as well I'm going to coordinate with marco to ensure that these pdb files are uploaded to bugsplat as well. It will make hard to reproduce crashes in display plugins easier to diagnose in the future. --- cmake/macros/SetupHifiPlugin.cmake | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/cmake/macros/SetupHifiPlugin.cmake b/cmake/macros/SetupHifiPlugin.cmake index e9c8688590..0db91cb9e6 100644 --- a/cmake/macros/SetupHifiPlugin.cmake +++ b/cmake/macros/SetupHifiPlugin.cmake @@ -17,6 +17,12 @@ macro(SETUP_HIFI_PLUGIN) set(PLUGIN_PATH "plugins") endif() + if (WIN32) + # produce PDB files for plugins as well + set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} /Zi") + set(CMAKE_SHARED_LINKER_FLAGS "${CMAKE_SHARED_LINKER_FLAGS} /DEBUG") + endif() + if (CMAKE_SYSTEM_NAME MATCHES "Linux" OR CMAKE_GENERATOR STREQUAL "Unix Makefiles") set(PLUGIN_FULL_PATH "${CMAKE_BINARY_DIR}/interface/${PLUGIN_PATH}/") else() From 76344f325334e4469e6c1fe7f2dbb57faca4fd86 Mon Sep 17 00:00:00 2001 From: Brad Hefta-Gaub Date: Wed, 26 Oct 2016 22:41:56 -0700 Subject: [PATCH 13/16] add a menu item to run audio stats script --- interface/src/Menu.cpp | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/interface/src/Menu.cpp b/interface/src/Menu.cpp index cd8d39b739..d357713bff 100644 --- a/interface/src/Menu.cpp +++ b/interface/src/Menu.cpp @@ -620,6 +620,14 @@ Menu::Menu() { // Developer > Audio >>> MenuWrapper* audioDebugMenu = developerMenu->addMenu("Audio"); + action = addActionToQMenuAndActionHash(audioDebugMenu, "Stats..."); + connect(action, &QAction::triggered, [] { + auto scriptEngines = DependencyManager::get(); + QUrl defaultScriptsLoc = defaultScriptsLocation(); + defaultScriptsLoc.setPath(defaultScriptsLoc.path() + "developer/utilities/audio/stats.js"); + scriptEngines->loadScript(defaultScriptsLoc.toString()); + }); + action = addActionToQMenuAndActionHash(audioDebugMenu, "Buffers..."); connect(action, &QAction::triggered, [] { DependencyManager::get()->toggle(QString("hifi/dialogs/AudioPreferencesDialog.qml"), "AudioPreferencesDialog"); From 946f3782f36c44b751c89a566eda56e637f13546 Mon Sep 17 00:00:00 2001 From: Ryan Huffman Date: Thu, 27 Oct 2016 09:59:17 -0700 Subject: [PATCH 14/16] Fix indentation in BatchLoader --- libraries/script-engine/src/BatchLoader.cpp | 24 ++++++++++----------- libraries/script-engine/src/BatchLoader.h | 10 ++++----- 2 files changed, 17 insertions(+), 17 deletions(-) diff --git a/libraries/script-engine/src/BatchLoader.cpp b/libraries/script-engine/src/BatchLoader.cpp index ef8cf90656..65c1bc5a2a 100644 --- a/libraries/script-engine/src/BatchLoader.cpp +++ b/libraries/script-engine/src/BatchLoader.cpp @@ -50,18 +50,18 @@ void BatchLoader::start() { qCDebug(scriptengine) << "Loading script at " << url; - auto scriptCache = DependencyManager::get(); + auto scriptCache = DependencyManager::get(); - // Use a proxy callback to handle the call and emit the signal in a thread-safe way. - // If BatchLoader is deleted before the callback is called, the subsequent "emit" call will not do - // anything. - ScriptCacheSignalProxy* proxy = new ScriptCacheSignalProxy(scriptCache.data()); - scriptCache->getScriptContents(url.toString(), [proxy](const QString& url, const QString& contents, bool isURL, bool success) { - proxy->receivedContent(url, contents, isURL, success); - proxy->deleteLater(); - }, false); + // Use a proxy callback to handle the call and emit the signal in a thread-safe way. + // If BatchLoader is deleted before the callback is called, the subsequent "emit" call will not do + // anything. + ScriptCacheSignalProxy* proxy = new ScriptCacheSignalProxy(scriptCache.data()); + scriptCache->getScriptContents(url.toString(), [proxy](const QString& url, const QString& contents, bool isURL, bool success) { + proxy->receivedContent(url, contents, isURL, success); + proxy->deleteLater(); + }, false); - connect(proxy, &ScriptCacheSignalProxy::contentAvailable, this, [this](const QString& url, const QString& contents, bool isURL, bool success) { + connect(proxy, &ScriptCacheSignalProxy::contentAvailable, this, [this](const QString& url, const QString& contents, bool isURL, bool success) { if (isURL && success) { _data.insert(url, contents); qCDebug(scriptengine) << "Loaded: " << url; @@ -74,10 +74,10 @@ void BatchLoader::start() { _finished = true; emit finished(_data); } - }); + }); } } void ScriptCacheSignalProxy::receivedContent(const QString& url, const QString& contents, bool isURL, bool success) { - emit contentAvailable(url, contents, isURL, success); + emit contentAvailable(url, contents, isURL, success); } diff --git a/libraries/script-engine/src/BatchLoader.h b/libraries/script-engine/src/BatchLoader.h index 55acc11c26..a03a8d80c6 100644 --- a/libraries/script-engine/src/BatchLoader.h +++ b/libraries/script-engine/src/BatchLoader.h @@ -22,19 +22,19 @@ #include class ScriptCacheSignalProxy : public QObject { - Q_OBJECT + Q_OBJECT public: - ScriptCacheSignalProxy(QObject* parent) : QObject(parent) { } - void receivedContent(const QString& url, const QString& contents, bool isURL, bool success); + ScriptCacheSignalProxy(QObject* parent) : QObject(parent) { } + void receivedContent(const QString& url, const QString& contents, bool isURL, bool success); signals: - void contentAvailable(const QString& url, const QString& contents, bool isURL, bool success); + void contentAvailable(const QString& url, const QString& contents, bool isURL, bool success); }; class BatchLoader : public QObject { Q_OBJECT public: - BatchLoader(const QList& urls) ; + BatchLoader(const QList& urls); void start(); bool isFinished() const { return _finished; }; From d778b3b4ce89c826937d736775952ca5506274c7 Mon Sep 17 00:00:00 2001 From: Brad Hefta-Gaub Date: Thu, 27 Oct 2016 10:00:48 -0700 Subject: [PATCH 15/16] fix the timing of the launchedFromSteam property --- interface/src/Application.cpp | 4 +++- interface/src/Application.h | 2 +- interface/src/main.cpp | 4 +--- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp index fcfbe0f3b1..a2473aa3b8 100644 --- a/interface/src/Application.cpp +++ b/interface/src/Application.cpp @@ -506,7 +506,7 @@ Q_GUI_EXPORT void qt_gl_set_global_share_context(QOpenGLContext *context); Setting::Handle sessionRunTime{ "sessionRunTime", 0 }; -Application::Application(int& argc, char** argv, QElapsedTimer& startupTimer, bool runServer, QString runServerPathOption) : +Application::Application(int& argc, char** argv, QElapsedTimer& startupTimer, bool runServer, QString runServerPathOption, bool launchedFromSteam) : QApplication(argc, argv), _shouldRunServer(runServer), _runServerPath(runServerPathOption), @@ -534,6 +534,8 @@ Application::Application(int& argc, char** argv, QElapsedTimer& startupTimer, bo _maxOctreePPS(maxOctreePacketsPerSecond.get()), _lastFaceTrackerUpdate(0) { + setProperty("com.highfidelity.launchedFromSteam", launchedFromSteam); + _runningMarker.startRunningMarker(); PluginContainer* pluginContainer = dynamic_cast(this); // set the container for any plugins that care diff --git a/interface/src/Application.h b/interface/src/Application.h index 1a0041223e..24e4d9be34 100644 --- a/interface/src/Application.h +++ b/interface/src/Application.h @@ -139,7 +139,7 @@ public: static void initPlugins(const QStringList& arguments); static void shutdownPlugins(); - Application(int& argc, char** argv, QElapsedTimer& startup_time, bool runServer, QString runServerPathOption); + Application(int& argc, char** argv, QElapsedTimer& startup_time, bool runServer, QString runServerPathOption, bool launchedFromSteam); ~Application(); void postLambdaEvent(std::function f) override; diff --git a/interface/src/main.cpp b/interface/src/main.cpp index ab4ab689f7..59cf4647b2 100644 --- a/interface/src/main.cpp +++ b/interface/src/main.cpp @@ -159,10 +159,8 @@ int main(int argc, const char* argv[]) { int exitCode; { QSettings::setDefaultFormat(QSettings::IniFormat); - Application app(argc, const_cast(argv), startupTime, runServer, serverContentPathOptionValue); - bool launchedFromSteam = SteamClient::isRunning(); - app.setProperty("com.highfidelity.launchedFromSteam", launchedFromSteam); + Application app(argc, const_cast(argv), startupTime, runServer, serverContentPathOptionValue, launchedFromSteam); // If we failed the OpenGLVersion check, log it. if (override) { From c6d89f181a061e228c6c1d9c38d0b795c5916087 Mon Sep 17 00:00:00 2001 From: Brad Hefta-Gaub Date: Thu, 27 Oct 2016 10:08:16 -0700 Subject: [PATCH 16/16] CR feedback --- interface/src/Application.cpp | 4 ++-- interface/src/Application.h | 2 +- interface/src/main.cpp | 3 +-- 3 files changed, 4 insertions(+), 5 deletions(-) diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp index a2473aa3b8..a32f50f25c 100644 --- a/interface/src/Application.cpp +++ b/interface/src/Application.cpp @@ -506,7 +506,7 @@ Q_GUI_EXPORT void qt_gl_set_global_share_context(QOpenGLContext *context); Setting::Handle sessionRunTime{ "sessionRunTime", 0 }; -Application::Application(int& argc, char** argv, QElapsedTimer& startupTimer, bool runServer, QString runServerPathOption, bool launchedFromSteam) : +Application::Application(int& argc, char** argv, QElapsedTimer& startupTimer, bool runServer, QString runServerPathOption) : QApplication(argc, argv), _shouldRunServer(runServer), _runServerPath(runServerPathOption), @@ -534,7 +534,7 @@ Application::Application(int& argc, char** argv, QElapsedTimer& startupTimer, bo _maxOctreePPS(maxOctreePacketsPerSecond.get()), _lastFaceTrackerUpdate(0) { - setProperty("com.highfidelity.launchedFromSteam", launchedFromSteam); + setProperty("com.highfidelity.launchedFromSteam", SteamClient::isRunning()); _runningMarker.startRunningMarker(); diff --git a/interface/src/Application.h b/interface/src/Application.h index 24e4d9be34..1a0041223e 100644 --- a/interface/src/Application.h +++ b/interface/src/Application.h @@ -139,7 +139,7 @@ public: static void initPlugins(const QStringList& arguments); static void shutdownPlugins(); - Application(int& argc, char** argv, QElapsedTimer& startup_time, bool runServer, QString runServerPathOption, bool launchedFromSteam); + Application(int& argc, char** argv, QElapsedTimer& startup_time, bool runServer, QString runServerPathOption); ~Application(); void postLambdaEvent(std::function f) override; diff --git a/interface/src/main.cpp b/interface/src/main.cpp index 59cf4647b2..5b0da4f578 100644 --- a/interface/src/main.cpp +++ b/interface/src/main.cpp @@ -159,8 +159,7 @@ int main(int argc, const char* argv[]) { int exitCode; { QSettings::setDefaultFormat(QSettings::IniFormat); - bool launchedFromSteam = SteamClient::isRunning(); - Application app(argc, const_cast(argv), startupTime, runServer, serverContentPathOptionValue, launchedFromSteam); + Application app(argc, const_cast(argv), startupTime, runServer, serverContentPathOptionValue); // If we failed the OpenGLVersion check, log it. if (override) {