From 8ae05d8490f3c00c3b22089e63ade4b0a2213efe Mon Sep 17 00:00:00 2001 From: Howard Stearns Date: Wed, 2 Dec 2015 16:40:41 -0800 Subject: [PATCH 01/19] Plugins target frame rate and period. --- .../display-plugins/Basic2DWindowOpenGLDisplayPlugin.cpp | 3 ++- .../src/display-plugins/Basic2DWindowOpenGLDisplayPlugin.h | 6 ++++++ .../src/display-plugins/openvr/OpenVrDisplayPlugin.h | 5 +++++ libraries/plugins/src/plugins/DisplayPlugin.h | 2 ++ plugins/oculus/src/OculusDisplayPlugin.h | 5 +++++ plugins/oculusLegacy/src/OculusLegacyDisplayPlugin.h | 5 +++++ 6 files changed, 25 insertions(+), 1 deletion(-) diff --git a/libraries/display-plugins/src/display-plugins/Basic2DWindowOpenGLDisplayPlugin.cpp b/libraries/display-plugins/src/display-plugins/Basic2DWindowOpenGLDisplayPlugin.cpp index 9366ec4403..b0dffbb7bc 100644 --- a/libraries/display-plugins/src/display-plugins/Basic2DWindowOpenGLDisplayPlugin.cpp +++ b/libraries/display-plugins/src/display-plugins/Basic2DWindowOpenGLDisplayPlugin.cpp @@ -139,7 +139,8 @@ void Basic2DWindowOpenGLDisplayPlugin::updateFramerate() { } else if (FRAMERATE_30 == actionText) { _framerateTarget = 30; } - } + } + _inverseFrameRate = _framerateTarget ? 1.0f / (float) _framerateTarget : 1.0f / TARGET_FRAMERATE_Basic2DWindowOpenGL; // not truncated int newInterval = getDesiredInterval(); qDebug() << newInterval; diff --git a/libraries/display-plugins/src/display-plugins/Basic2DWindowOpenGLDisplayPlugin.h b/libraries/display-plugins/src/display-plugins/Basic2DWindowOpenGLDisplayPlugin.h index f4655ab79f..f6052f82ca 100644 --- a/libraries/display-plugins/src/display-plugins/Basic2DWindowOpenGLDisplayPlugin.h +++ b/libraries/display-plugins/src/display-plugins/Basic2DWindowOpenGLDisplayPlugin.h @@ -9,6 +9,8 @@ #include "WindowOpenGLDisplayPlugin.h" +#define TARGET_FRAMERATE_Basic2DWindowOpenGL 60.0f + class QScreen; class Basic2DWindowOpenGLDisplayPlugin : public WindowOpenGLDisplayPlugin { Q_OBJECT @@ -16,6 +18,9 @@ class Basic2DWindowOpenGLDisplayPlugin : public WindowOpenGLDisplayPlugin { public: virtual const QString & getName() const override; + virtual float getTargetFrameRate() { return _framerateTarget ? (float) _framerateTarget : TARGET_FRAMERATE_Basic2DWindowOpenGL; } + virtual float getTargetFramePeriod() { return _inverseFrameRate; } + virtual void activate() override; virtual void deactivate() override; @@ -33,4 +38,5 @@ private: QScreen* getFullscreenTarget(); uint32_t _framerateTarget{ 0 }; int _fullscreenTarget{ -1 }; + float _inverseFrameRate{ 1.0f }; //seconds }; diff --git a/libraries/display-plugins/src/display-plugins/openvr/OpenVrDisplayPlugin.h b/libraries/display-plugins/src/display-plugins/openvr/OpenVrDisplayPlugin.h index 15d37d9de8..6116cd2283 100644 --- a/libraries/display-plugins/src/display-plugins/openvr/OpenVrDisplayPlugin.h +++ b/libraries/display-plugins/src/display-plugins/openvr/OpenVrDisplayPlugin.h @@ -14,12 +14,17 @@ #include "../WindowOpenGLDisplayPlugin.h" +#define TARGET_RATE_OpenVr 90.0f; // FIXME: get from sdk tracked device property? This number is vive-only. + class OpenVrDisplayPlugin : public WindowOpenGLDisplayPlugin { public: virtual bool isSupported() const override; virtual const QString & getName() const override; virtual bool isHmd() const override { return true; } + virtual float getTargetFrameRate() { return TARGET_RATE_OpenVr; } + virtual float getTargetFramePeriod() { return 1.0f / TARGET_RATE_OpenVr; } + virtual void activate() override; virtual void deactivate() override; diff --git a/libraries/plugins/src/plugins/DisplayPlugin.h b/libraries/plugins/src/plugins/DisplayPlugin.h index 5b00391f09..ea6060010e 100644 --- a/libraries/plugins/src/plugins/DisplayPlugin.h +++ b/libraries/plugins/src/plugins/DisplayPlugin.h @@ -57,6 +57,8 @@ public: /// By default, all HMDs are stereo virtual bool isStereo() const { return isHmd(); } virtual bool isThrottled() const { return false; } + virtual float getTargetFrameRate() { return 0.0f; } + virtual float getTargetFramePeriod() { return 0.0f; } // Rendering support diff --git a/plugins/oculus/src/OculusDisplayPlugin.h b/plugins/oculus/src/OculusDisplayPlugin.h index c1224ecf3a..62cdf35fc7 100644 --- a/plugins/oculus/src/OculusDisplayPlugin.h +++ b/plugins/oculus/src/OculusDisplayPlugin.h @@ -12,12 +12,17 @@ struct SwapFramebufferWrapper; using SwapFboPtr = QSharedPointer; +#define TARGET_RATE_Oculus 75.0f; + class OculusDisplayPlugin : public OculusBaseDisplayPlugin { public: virtual void activate() override; virtual void deactivate() override; virtual const QString & getName() const override; + virtual float getTargetFrameRate() { return TARGET_RATE_Oculus; } + virtual float getTargetFramePeriod() { return 1.0f / TARGET_RATE_Oculus; } + protected: virtual void display(GLuint finalTexture, const glm::uvec2& sceneSize) override; virtual void customizeContext() override; diff --git a/plugins/oculusLegacy/src/OculusLegacyDisplayPlugin.h b/plugins/oculusLegacy/src/OculusLegacyDisplayPlugin.h index 6e3f864aee..ca45e201f4 100644 --- a/plugins/oculusLegacy/src/OculusLegacyDisplayPlugin.h +++ b/plugins/oculusLegacy/src/OculusLegacyDisplayPlugin.h @@ -13,6 +13,8 @@ #include +#define TARGET_RATE_OculusLegacy 75.0f; + class OculusLegacyDisplayPlugin : public WindowOpenGLDisplayPlugin { public: OculusLegacyDisplayPlugin(); @@ -25,6 +27,9 @@ public: virtual bool eventFilter(QObject* receiver, QEvent* event) override; virtual int getHmdScreen() const override; + virtual float getTargetFrameRate() { return TARGET_RATE_OculusLegacy; } + virtual float getTargetFramePeriod() { return 1.0f / TARGET_RATE_OculusLegacy; } + // Stereo specific methods virtual bool isHmd() const override { return true; } virtual glm::mat4 getProjection(Eye eye, const glm::mat4& baseProjection) const override; From 53bfcd17e2537c7930faa3221ab1c74de5a2a482 Mon Sep 17 00:00:00 2001 From: Howard Stearns Date: Wed, 2 Dec 2015 16:41:16 -0800 Subject: [PATCH 02/19] Use plugins data for Application frame rate/period. --- interface/src/Application.cpp | 2 ++ interface/src/Application.h | 6 ++---- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp index ef10ad4464..94aa3949cc 100644 --- a/interface/src/Application.cpp +++ b/interface/src/Application.cpp @@ -3246,6 +3246,8 @@ void Application::queryOctree(NodeType_t serverType, PacketType packetType, Node bool Application::isHMDMode() const { return getActiveDisplayPlugin()->isHmd(); } +float Application::getTargetFrameRate() { return getActiveDisplayPlugin()->getTargetFrameRate(); } +float Application::getTargetFramePeriod() { return getActiveDisplayPlugin()->getTargetFramePeriod(); } QRect Application::getDesirableApplicationGeometry() { QRect applicationGeometry = getWindow()->geometry(); diff --git a/interface/src/Application.h b/interface/src/Application.h index 730158c689..adef152ac5 100644 --- a/interface/src/Application.h +++ b/interface/src/Application.h @@ -159,10 +159,8 @@ public: bool isForeground() const { return _isForeground; } float getFps() const { return _fps; } - float const HMD_TARGET_FRAME_RATE = 75.0f; - float const DESKTOP_TARGET_FRAME_RATE = 60.0f; - float getTargetFrameRate() { return isHMDMode() ? HMD_TARGET_FRAME_RATE : DESKTOP_TARGET_FRAME_RATE; } - float getTargetFramePeriod() { return isHMDMode() ? 1.0f / HMD_TARGET_FRAME_RATE : 1.0f / DESKTOP_TARGET_FRAME_RATE; } // same as 1/getTargetFrameRate, but w/compile-time division + float getTargetFrameRate(); // frames/second + float getTargetFramePeriod(); // seconds float getLastInstanteousFps() const { return _lastInstantaneousFps; } float getLastPaintWait() const { return _lastPaintWait; }; float getLastDeducedNonVSyncFps() const { return _lastDeducedNonVSyncFps; } From ed86a880832d988e05b33b2c8e2b7739e309274b Mon Sep 17 00:00:00 2001 From: Howard Stearns Date: Wed, 2 Dec 2015 21:48:06 -0800 Subject: [PATCH 03/19] Make various forms of throttling more consistent and give it a chance of actually reaching target. --- .../Basic2DWindowOpenGLDisplayPlugin.cpp | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/libraries/display-plugins/src/display-plugins/Basic2DWindowOpenGLDisplayPlugin.cpp b/libraries/display-plugins/src/display-plugins/Basic2DWindowOpenGLDisplayPlugin.cpp index b0dffbb7bc..ea8ffe1e2f 100644 --- a/libraries/display-plugins/src/display-plugins/Basic2DWindowOpenGLDisplayPlugin.cpp +++ b/libraries/display-plugins/src/display-plugins/Basic2DWindowOpenGLDisplayPlugin.cpp @@ -90,12 +90,8 @@ void Basic2DWindowOpenGLDisplayPlugin::display(GLuint sceneTexture, const glm::u int Basic2DWindowOpenGLDisplayPlugin::getDesiredInterval() const { - static const int THROTTLED_PAINT_TIMER_DELAY_MS = MSECS_PER_SECOND / 15; static const int ULIMIITED_PAINT_TIMER_DELAY_MS = 1; int result = ULIMIITED_PAINT_TIMER_DELAY_MS; - if (_isThrottled) { - result = THROTTLED_PAINT_TIMER_DELAY_MS; - } if (0 != _framerateTarget) { result = MSECS_PER_SECOND / _framerateTarget; } @@ -139,12 +135,18 @@ void Basic2DWindowOpenGLDisplayPlugin::updateFramerate() { } else if (FRAMERATE_30 == actionText) { _framerateTarget = 30; } + } else if (_isThrottled) { + _framerateTarget = 15; } _inverseFrameRate = _framerateTarget ? 1.0f / (float) _framerateTarget : 1.0f / TARGET_FRAMERATE_Basic2DWindowOpenGL; // not truncated int newInterval = getDesiredInterval(); + if (_framerateTarget) { // For any target other than vsync, we have little hope of achieving it with timer alone: + const int ALLOWANCE_FOR_DISPLAY_FINISHFRAME_AND_TIMER = 3; // ideally a windowed average of qApp->getLastPaintWait and then some, but not worth the complexity + newInterval -= ALLOWANCE_FOR_DISPLAY_FINISHFRAME_AND_TIMER; // Otherwise, any controller expecting us to hit "target" will always be disappointed. + } qDebug() << newInterval; - _timer.start(getDesiredInterval()); + _timer.start(newInterval); } // FIXME target the screen the window is currently on From a989b926b054fce73e694221646c2c8c385a5db0 Mon Sep 17 00:00:00 2001 From: Howard Stearns Date: Thu, 3 Dec 2015 11:28:26 -0800 Subject: [PATCH 04/19] isVSynchronized, and proper override declarations. --- .../display-plugins/Basic2DWindowOpenGLDisplayPlugin.cpp | 9 +++++---- .../display-plugins/Basic2DWindowOpenGLDisplayPlugin.h | 5 +++-- .../src/display-plugins/openvr/OpenVrDisplayPlugin.h | 4 ++-- libraries/plugins/src/plugins/DisplayPlugin.h | 1 + plugins/oculus/src/OculusDisplayPlugin.h | 4 ++-- plugins/oculusLegacy/src/OculusLegacyDisplayPlugin.h | 4 ++-- 6 files changed, 15 insertions(+), 12 deletions(-) diff --git a/libraries/display-plugins/src/display-plugins/Basic2DWindowOpenGLDisplayPlugin.cpp b/libraries/display-plugins/src/display-plugins/Basic2DWindowOpenGLDisplayPlugin.cpp index ea8ffe1e2f..c0af5609ee 100644 --- a/libraries/display-plugins/src/display-plugins/Basic2DWindowOpenGLDisplayPlugin.cpp +++ b/libraries/display-plugins/src/display-plugins/Basic2DWindowOpenGLDisplayPlugin.cpp @@ -113,6 +113,9 @@ bool Basic2DWindowOpenGLDisplayPlugin::isThrottled() const { return shouldThrottle; } +bool Basic2DWindowOpenGLDisplayPlugin::isVSynchronized() const { + return (_framerateTarget == 0) && (!_vsyncAction || _vsyncAction->isChecked()); +} void Basic2DWindowOpenGLDisplayPlugin::updateFramerate() { QAction* checkedFramerate{ nullptr }; @@ -141,10 +144,8 @@ void Basic2DWindowOpenGLDisplayPlugin::updateFramerate() { _inverseFrameRate = _framerateTarget ? 1.0f / (float) _framerateTarget : 1.0f / TARGET_FRAMERATE_Basic2DWindowOpenGL; // not truncated int newInterval = getDesiredInterval(); - if (_framerateTarget) { // For any target other than vsync, we have little hope of achieving it with timer alone: - const int ALLOWANCE_FOR_DISPLAY_FINISHFRAME_AND_TIMER = 3; // ideally a windowed average of qApp->getLastPaintWait and then some, but not worth the complexity - newInterval -= ALLOWANCE_FOR_DISPLAY_FINISHFRAME_AND_TIMER; // Otherwise, any controller expecting us to hit "target" will always be disappointed. - } + // Note: when not isVSynchronized, we are often not likely to hit target with a newInterval timer. + // We could try subtracting an allowance for qApp->getLastPaintWait() and qt timer machinery, but that starts getting complicated. qDebug() << newInterval; _timer.start(newInterval); } diff --git a/libraries/display-plugins/src/display-plugins/Basic2DWindowOpenGLDisplayPlugin.h b/libraries/display-plugins/src/display-plugins/Basic2DWindowOpenGLDisplayPlugin.h index f6052f82ca..45d2545a94 100644 --- a/libraries/display-plugins/src/display-plugins/Basic2DWindowOpenGLDisplayPlugin.h +++ b/libraries/display-plugins/src/display-plugins/Basic2DWindowOpenGLDisplayPlugin.h @@ -18,8 +18,8 @@ class Basic2DWindowOpenGLDisplayPlugin : public WindowOpenGLDisplayPlugin { public: virtual const QString & getName() const override; - virtual float getTargetFrameRate() { return _framerateTarget ? (float) _framerateTarget : TARGET_FRAMERATE_Basic2DWindowOpenGL; } - virtual float getTargetFramePeriod() { return _inverseFrameRate; } + virtual float getTargetFrameRate() override { return _framerateTarget ? (float) _framerateTarget : TARGET_FRAMERATE_Basic2DWindowOpenGL; } + virtual float getTargetFramePeriod() override { return _inverseFrameRate; } virtual void activate() override; virtual void deactivate() override; @@ -27,6 +27,7 @@ public: virtual void display(GLuint sceneTexture, const glm::uvec2& sceneSize) override; virtual bool isThrottled() const override; + virtual bool isVSynchronized() const override; protected: int getDesiredInterval() const; diff --git a/libraries/display-plugins/src/display-plugins/openvr/OpenVrDisplayPlugin.h b/libraries/display-plugins/src/display-plugins/openvr/OpenVrDisplayPlugin.h index 6116cd2283..390ab4b539 100644 --- a/libraries/display-plugins/src/display-plugins/openvr/OpenVrDisplayPlugin.h +++ b/libraries/display-plugins/src/display-plugins/openvr/OpenVrDisplayPlugin.h @@ -22,8 +22,8 @@ public: virtual const QString & getName() const override; virtual bool isHmd() const override { return true; } - virtual float getTargetFrameRate() { return TARGET_RATE_OpenVr; } - virtual float getTargetFramePeriod() { return 1.0f / TARGET_RATE_OpenVr; } + virtual float getTargetFrameRate() override { return TARGET_RATE_OpenVr; } + virtual float getTargetFramePeriod() override { return 1.0f / TARGET_RATE_OpenVr; } virtual void activate() override; virtual void deactivate() override; diff --git a/libraries/plugins/src/plugins/DisplayPlugin.h b/libraries/plugins/src/plugins/DisplayPlugin.h index ea6060010e..729b62f124 100644 --- a/libraries/plugins/src/plugins/DisplayPlugin.h +++ b/libraries/plugins/src/plugins/DisplayPlugin.h @@ -57,6 +57,7 @@ public: /// By default, all HMDs are stereo virtual bool isStereo() const { return isHmd(); } virtual bool isThrottled() const { return false; } + virtual bool isVSynchronized() const { return true; } // false when throttled or run by non vsync timer virtual float getTargetFrameRate() { return 0.0f; } virtual float getTargetFramePeriod() { return 0.0f; } diff --git a/plugins/oculus/src/OculusDisplayPlugin.h b/plugins/oculus/src/OculusDisplayPlugin.h index 62cdf35fc7..febfd5b08e 100644 --- a/plugins/oculus/src/OculusDisplayPlugin.h +++ b/plugins/oculus/src/OculusDisplayPlugin.h @@ -20,8 +20,8 @@ public: virtual void deactivate() override; virtual const QString & getName() const override; - virtual float getTargetFrameRate() { return TARGET_RATE_Oculus; } - virtual float getTargetFramePeriod() { return 1.0f / TARGET_RATE_Oculus; } + virtual float getTargetFrameRate() override { return TARGET_RATE_Oculus; } + virtual float getTargetFramePeriod() override { return 1.0f / TARGET_RATE_Oculus; } protected: virtual void display(GLuint finalTexture, const glm::uvec2& sceneSize) override; diff --git a/plugins/oculusLegacy/src/OculusLegacyDisplayPlugin.h b/plugins/oculusLegacy/src/OculusLegacyDisplayPlugin.h index ca45e201f4..d0840d3a72 100644 --- a/plugins/oculusLegacy/src/OculusLegacyDisplayPlugin.h +++ b/plugins/oculusLegacy/src/OculusLegacyDisplayPlugin.h @@ -27,8 +27,8 @@ public: virtual bool eventFilter(QObject* receiver, QEvent* event) override; virtual int getHmdScreen() const override; - virtual float getTargetFrameRate() { return TARGET_RATE_OculusLegacy; } - virtual float getTargetFramePeriod() { return 1.0f / TARGET_RATE_OculusLegacy; } + virtual float getTargetFrameRate() override { return TARGET_RATE_OculusLegacy; } + virtual float getTargetFramePeriod() override { return 1.0f / TARGET_RATE_OculusLegacy; } // Stereo specific methods virtual bool isHmd() const override { return true; } From 375211e13b454d64fc7b2d8a33c915119cc5b043 Mon Sep 17 00:00:00 2001 From: Howard Stearns Date: Thu, 3 Dec 2015 11:29:13 -0800 Subject: [PATCH 05/19] Use isVSynchronized in controller. --- interface/src/Application.cpp | 1 + interface/src/Application.h | 1 + interface/src/avatar/AvatarManager.cpp | 22 +++++++++++++++------- 3 files changed, 17 insertions(+), 7 deletions(-) diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp index 94aa3949cc..921c126ce6 100644 --- a/interface/src/Application.cpp +++ b/interface/src/Application.cpp @@ -3248,6 +3248,7 @@ bool Application::isHMDMode() const { } float Application::getTargetFrameRate() { return getActiveDisplayPlugin()->getTargetFrameRate(); } float Application::getTargetFramePeriod() { return getActiveDisplayPlugin()->getTargetFramePeriod(); } +bool Application::isVSynchronized() const { return getActiveDisplayPlugin()->isVSynchronized(); } QRect Application::getDesirableApplicationGeometry() { QRect applicationGeometry = getWindow()->geometry(); diff --git a/interface/src/Application.h b/interface/src/Application.h index adef152ac5..5c952f462e 100644 --- a/interface/src/Application.h +++ b/interface/src/Application.h @@ -124,6 +124,7 @@ public: PickRay computePickRay() const; bool isThrottleRendering() const; + bool isVSynchronized() const; Camera* getCamera() { return &_myCamera; } // Represents the current view frustum of the avatar. diff --git a/interface/src/avatar/AvatarManager.cpp b/interface/src/avatar/AvatarManager.cpp index e094f2e263..0ee07d15ec 100644 --- a/interface/src/avatar/AvatarManager.cpp +++ b/interface/src/avatar/AvatarManager.cpp @@ -146,13 +146,21 @@ void AvatarManager::updateOtherAvatars(float deltaTime) { PerformanceTimer perfTimer("otherAvatars"); - _renderDistanceController.setMeasuredValueSetpoint(qApp->getTargetFrameRate()); // No problem updating in flight. - // The PID controller raises the controlled value when the measured value goes up. - // The measured value is frame rate. When the controlled value (1 / render cutoff distance) - // goes up, the render cutoff distance gets closer, the number of rendered avatars is less, and frame rate - // goes up. - const float deduced = qApp->getLastDeducedNonVSyncFps(); - const float distance = 1.0f / _renderDistanceController.update(deduced, deltaTime); + float distance; + if (qApp->isVSynchronized()) { + _renderDistanceController.setMeasuredValueSetpoint(qApp->getTargetFrameRate()); // No problem updating in flight. + // The PID controller raises the controlled value when the measured value goes up. + // The measured value is frame rate. When the controlled value (1 / render cutoff distance) + // goes up, the render cutoff distance gets closer, the number of rendered avatars is less, and frame rate + // goes up. + const float deduced = qApp->getLastDeducedNonVSyncFps(); + distance = 1.0f / _renderDistanceController.update(deduced, deltaTime); + } else { + // We could keep the controller running when not vsync'd, if getLastDeducedNonVSyncFps is still meaningful. + // But the basic 2d controller doesn't try to adjust the timer for qt load or getLastPaintWait, so running the + // Here we choose to just use the maximum render cutoff distance if: throttled, running without vsync, or 30-60 "fixed" targets. + distance = 1.0f / _renderDistanceController.getControlledValueLowLimit(); + } _renderDistanceAverage.updateAverage(distance); _renderDistance = _renderDistanceAverage.getAverage(); int renderableCount = 0; From 30bbc82c245ddd357bb3f54abd9046f0c9f63a05 Mon Sep 17 00:00:00 2001 From: Howard Stearns Date: Thu, 3 Dec 2015 13:00:07 -0800 Subject: [PATCH 06/19] Restore throttling interval code. --- .../display-plugins/Basic2DWindowOpenGLDisplayPlugin.cpp | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/libraries/display-plugins/src/display-plugins/Basic2DWindowOpenGLDisplayPlugin.cpp b/libraries/display-plugins/src/display-plugins/Basic2DWindowOpenGLDisplayPlugin.cpp index c0af5609ee..8633628fef 100644 --- a/libraries/display-plugins/src/display-plugins/Basic2DWindowOpenGLDisplayPlugin.cpp +++ b/libraries/display-plugins/src/display-plugins/Basic2DWindowOpenGLDisplayPlugin.cpp @@ -88,10 +88,15 @@ void Basic2DWindowOpenGLDisplayPlugin::display(GLuint sceneTexture, const glm::u WindowOpenGLDisplayPlugin::display(sceneTexture, sceneSize); } - +#define THROTTLED_FRAMERATE 15 int Basic2DWindowOpenGLDisplayPlugin::getDesiredInterval() const { static const int ULIMIITED_PAINT_TIMER_DELAY_MS = 1; int result = ULIMIITED_PAINT_TIMER_DELAY_MS; + // This test wouldn't be necessary if we could depend on updateFramerate setting _framerateTarget. + // Alas, that gets complicated: isThrottled() is const and other stuff depends on it. + if (_isThrottled) { + result = MSECS_PER_SECOND / THROTTLED_FRAMERATE; + } if (0 != _framerateTarget) { result = MSECS_PER_SECOND / _framerateTarget; } @@ -139,7 +144,7 @@ void Basic2DWindowOpenGLDisplayPlugin::updateFramerate() { _framerateTarget = 30; } } else if (_isThrottled) { - _framerateTarget = 15; + _framerateTarget = (float) THROTTLED_FRAMERATE; } _inverseFrameRate = _framerateTarget ? 1.0f / (float) _framerateTarget : 1.0f / TARGET_FRAMERATE_Basic2DWindowOpenGL; // not truncated From f5b25791f466402549cdaaa066d1519854ad0532 Mon Sep 17 00:00:00 2001 From: howard-stearns Date: Thu, 3 Dec 2015 13:39:41 -0800 Subject: [PATCH 07/19] When throttling starts, we won't have noticed, so change test. --- .../src/display-plugins/Basic2DWindowOpenGLDisplayPlugin.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/display-plugins/src/display-plugins/Basic2DWindowOpenGLDisplayPlugin.cpp b/libraries/display-plugins/src/display-plugins/Basic2DWindowOpenGLDisplayPlugin.cpp index 8633628fef..b3719265ea 100644 --- a/libraries/display-plugins/src/display-plugins/Basic2DWindowOpenGLDisplayPlugin.cpp +++ b/libraries/display-plugins/src/display-plugins/Basic2DWindowOpenGLDisplayPlugin.cpp @@ -119,7 +119,7 @@ bool Basic2DWindowOpenGLDisplayPlugin::isThrottled() const { } bool Basic2DWindowOpenGLDisplayPlugin::isVSynchronized() const { - return (_framerateTarget == 0) && (!_vsyncAction || _vsyncAction->isChecked()); + return (_framerateTarget == 0) && !_isThrottled; } void Basic2DWindowOpenGLDisplayPlugin::updateFramerate() { From 4dbfd3bfd936702e58ac90cc102d7df8bd4fe5a8 Mon Sep 17 00:00:00 2001 From: Ryan Huffman Date: Tue, 8 Dec 2015 10:49:10 -0800 Subject: [PATCH 08/19] Fix concurrency issues for bools in ScriptEngine Several member variables were unsafely being accessed from multiple threads. Making these variables std::atomic makes this safe. --- libraries/script-engine/src/ScriptEngine.cpp | 2 +- libraries/script-engine/src/ScriptEngine.h | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/libraries/script-engine/src/ScriptEngine.cpp b/libraries/script-engine/src/ScriptEngine.cpp index 3306659c80..daadb0e394 100644 --- a/libraries/script-engine/src/ScriptEngine.cpp +++ b/libraries/script-engine/src/ScriptEngine.cpp @@ -183,7 +183,7 @@ void ScriptEngine::runInThread() { QSet ScriptEngine::_allKnownScriptEngines; QMutex ScriptEngine::_allScriptsMutex; -bool ScriptEngine::_stoppingAllScripts = false; +std::atomic ScriptEngine::_stoppingAllScripts = false; void ScriptEngine::stopAllScripts(QObject* application) { _allScriptsMutex.lock(); diff --git a/libraries/script-engine/src/ScriptEngine.h b/libraries/script-engine/src/ScriptEngine.h index 90b99d46fd..82b91aed2e 100644 --- a/libraries/script-engine/src/ScriptEngine.h +++ b/libraries/script-engine/src/ScriptEngine.h @@ -166,8 +166,8 @@ signals: protected: QString _scriptContents; QString _parentURL; - bool _isFinished { false }; - bool _isRunning { false }; + std::atomic _isFinished { false }; + std::atomic _isRunning { false }; int _evaluatesPending { 0 }; bool _isInitialized { false }; QHash _timerFunctionMap; @@ -206,7 +206,7 @@ protected: static QSet _allKnownScriptEngines; static QMutex _allScriptsMutex; - static bool _stoppingAllScripts; + static std::atomic _stoppingAllScripts; }; #endif // hifi_ScriptEngine_h \ No newline at end of file From 1d65cbce882525243d5be7f54e70484058493e93 Mon Sep 17 00:00:00 2001 From: howard-stearns Date: Tue, 8 Dec 2015 18:08:57 -0800 Subject: [PATCH 09/19] Remove unneeded getTargetFramePeriod and isVSynchronized. --- interface/src/Application.cpp | 2 -- interface/src/Application.h | 1 - interface/src/avatar/AvatarManager.cpp | 6 ++---- .../display-plugins/Basic2DWindowOpenGLDisplayPlugin.cpp | 7 ------- .../src/display-plugins/Basic2DWindowOpenGLDisplayPlugin.h | 3 --- .../src/display-plugins/openvr/OpenVrDisplayPlugin.h | 1 - libraries/plugins/src/plugins/DisplayPlugin.h | 2 -- plugins/oculus/src/OculusDisplayPlugin.h | 1 - plugins/oculusLegacy/src/OculusLegacyDisplayPlugin.h | 1 - 9 files changed, 2 insertions(+), 22 deletions(-) diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp index 24fd1d1ac9..e01899726a 100644 --- a/interface/src/Application.cpp +++ b/interface/src/Application.cpp @@ -3235,8 +3235,6 @@ bool Application::isHMDMode() const { return getActiveDisplayPlugin()->isHmd(); } float Application::getTargetFrameRate() { return getActiveDisplayPlugin()->getTargetFrameRate(); } -float Application::getTargetFramePeriod() { return getActiveDisplayPlugin()->getTargetFramePeriod(); } -bool Application::isVSynchronized() const { return getActiveDisplayPlugin()->isVSynchronized(); } QRect Application::getDesirableApplicationGeometry() { QRect applicationGeometry = getWindow()->geometry(); diff --git a/interface/src/Application.h b/interface/src/Application.h index 0678477aaa..122b6ea3ae 100644 --- a/interface/src/Application.h +++ b/interface/src/Application.h @@ -124,7 +124,6 @@ public: PickRay computePickRay() const; bool isThrottleRendering() const; - bool isVSynchronized() const; Camera* getCamera() { return &_myCamera; } // Represents the current view frustum of the avatar. diff --git a/interface/src/avatar/AvatarManager.cpp b/interface/src/avatar/AvatarManager.cpp index dff6893bef..7c1a52f1b3 100644 --- a/interface/src/avatar/AvatarManager.cpp +++ b/interface/src/avatar/AvatarManager.cpp @@ -146,7 +146,7 @@ void AvatarManager::updateOtherAvatars(float deltaTime) { PerformanceTimer perfTimer("otherAvatars"); float distance; - if (qApp->isVSynchronized()) { + if (!qApp->isThrottleRendering()) { _renderDistanceController.setMeasuredValueSetpoint(qApp->getTargetFrameRate()); // No problem updating in flight. // The PID controller raises the controlled value when the measured value goes up. // The measured value is frame rate. When the controlled value (1 / render cutoff distance) @@ -155,9 +155,7 @@ void AvatarManager::updateOtherAvatars(float deltaTime) { const float deduced = qApp->getLastUnsynchronizedFps(); distance = 1.0f / _renderDistanceController.update(deduced, deltaTime); } else { - // We could keep the controller running when not vsync'd, if getLastDeducedNonVSyncFps is still meaningful. - // But the basic 2d controller doesn't try to adjust the timer for qt load or getLastPaintWait, so running the - // Here we choose to just use the maximum render cutoff distance if: throttled, running without vsync, or 30-60 "fixed" targets. + // Here we choose to just use the maximum render cutoff distance if throttled. distance = 1.0f / _renderDistanceController.getControlledValueLowLimit(); } _renderDistanceAverage.updateAverage(distance); diff --git a/libraries/display-plugins/src/display-plugins/Basic2DWindowOpenGLDisplayPlugin.cpp b/libraries/display-plugins/src/display-plugins/Basic2DWindowOpenGLDisplayPlugin.cpp index 97e12643a9..9139a9dcc9 100644 --- a/libraries/display-plugins/src/display-plugins/Basic2DWindowOpenGLDisplayPlugin.cpp +++ b/libraries/display-plugins/src/display-plugins/Basic2DWindowOpenGLDisplayPlugin.cpp @@ -113,10 +113,6 @@ bool Basic2DWindowOpenGLDisplayPlugin::isThrottled() const { return shouldThrottle; } -bool Basic2DWindowOpenGLDisplayPlugin::isVSynchronized() const { - return (_framerateTarget == 0) && !_isThrottled; -} - void Basic2DWindowOpenGLDisplayPlugin::updateFramerate() { QAction* checkedFramerate{ nullptr }; foreach(auto action, _framerateActions) { @@ -141,11 +137,8 @@ void Basic2DWindowOpenGLDisplayPlugin::updateFramerate() { } else if (_isThrottled) { _framerateTarget = (float) THROTTLED_FRAMERATE; } - _inverseFrameRate = _framerateTarget ? 1.0f / (float) _framerateTarget : 1.0f / TARGET_FRAMERATE_Basic2DWindowOpenGL; // not truncated int newInterval = getDesiredInterval(); - // Note: when not isVSynchronized, we are often not likely to hit target with a newInterval timer. - // We could try subtracting an allowance for qApp->getLastPaintWait() and qt timer machinery, but that starts getting complicated. qDebug() << newInterval; _timer.start(newInterval); } diff --git a/libraries/display-plugins/src/display-plugins/Basic2DWindowOpenGLDisplayPlugin.h b/libraries/display-plugins/src/display-plugins/Basic2DWindowOpenGLDisplayPlugin.h index bd5359e141..59c0e4e759 100644 --- a/libraries/display-plugins/src/display-plugins/Basic2DWindowOpenGLDisplayPlugin.h +++ b/libraries/display-plugins/src/display-plugins/Basic2DWindowOpenGLDisplayPlugin.h @@ -21,7 +21,6 @@ public: virtual const QString & getName() const override; virtual float getTargetFrameRate() override { return _framerateTarget ? (float) _framerateTarget : TARGET_FRAMERATE_Basic2DWindowOpenGL; } - virtual float getTargetFramePeriod() override { return _inverseFrameRate; } virtual void activate() override; @@ -30,7 +29,6 @@ public: virtual void internalPresent() override; virtual bool isThrottled() const override; - virtual bool isVSynchronized() const override; protected: int getDesiredInterval() const; @@ -44,6 +42,5 @@ private: QAction* _vsyncAction { nullptr }; uint32_t _framerateTarget { 0 }; int _fullscreenTarget{ -1 }; - float _inverseFrameRate{ 1.0f }; //seconds bool _wantVsync { true }; }; diff --git a/libraries/display-plugins/src/display-plugins/openvr/OpenVrDisplayPlugin.h b/libraries/display-plugins/src/display-plugins/openvr/OpenVrDisplayPlugin.h index feb5c0716c..2b51c53d58 100644 --- a/libraries/display-plugins/src/display-plugins/openvr/OpenVrDisplayPlugin.h +++ b/libraries/display-plugins/src/display-plugins/openvr/OpenVrDisplayPlugin.h @@ -23,7 +23,6 @@ public: virtual bool isHmd() const override { return true; } virtual float getTargetFrameRate() override { return TARGET_RATE_OpenVr; } - virtual float getTargetFramePeriod() override { return 1.0f / TARGET_RATE_OpenVr; } virtual void activate() override; virtual void deactivate() override; diff --git a/libraries/plugins/src/plugins/DisplayPlugin.h b/libraries/plugins/src/plugins/DisplayPlugin.h index 5352cf9a87..7f3b38e4a2 100644 --- a/libraries/plugins/src/plugins/DisplayPlugin.h +++ b/libraries/plugins/src/plugins/DisplayPlugin.h @@ -58,9 +58,7 @@ public: /// By default, all HMDs are stereo virtual bool isStereo() const { return isHmd(); } virtual bool isThrottled() const { return false; } - virtual bool isVSynchronized() const { return true; } // false when throttled or run by non vsync timer virtual float getTargetFrameRate() { return 0.0f; } - virtual float getTargetFramePeriod() { return 0.0f; } // Rendering support diff --git a/plugins/oculus/src/OculusDisplayPlugin.h b/plugins/oculus/src/OculusDisplayPlugin.h index 47b41b08b6..071dca0258 100644 --- a/plugins/oculus/src/OculusDisplayPlugin.h +++ b/plugins/oculus/src/OculusDisplayPlugin.h @@ -21,7 +21,6 @@ public: virtual void setEyeRenderPose(uint32_t frameIndex, Eye eye, const glm::mat4& pose) override final; virtual float getTargetFrameRate() override { return TARGET_RATE_Oculus; } - virtual float getTargetFramePeriod() override { return 1.0f / TARGET_RATE_Oculus; } protected: virtual void internalPresent() override; diff --git a/plugins/oculusLegacy/src/OculusLegacyDisplayPlugin.h b/plugins/oculusLegacy/src/OculusLegacyDisplayPlugin.h index 67efcc6575..7fe721fc45 100644 --- a/plugins/oculusLegacy/src/OculusLegacyDisplayPlugin.h +++ b/plugins/oculusLegacy/src/OculusLegacyDisplayPlugin.h @@ -28,7 +28,6 @@ public: virtual int getHmdScreen() const override; virtual float getTargetFrameRate() override { return TARGET_RATE_OculusLegacy; } - virtual float getTargetFramePeriod() override { return 1.0f / TARGET_RATE_OculusLegacy; } // Stereo specific methods virtual bool isHmd() const override { return true; } From 4bfce768ca68951d44d2f973d1bd408f46b5133c Mon Sep 17 00:00:00 2001 From: Ryan Huffman Date: Wed, 9 Dec 2015 14:48:33 -0800 Subject: [PATCH 10/19] Add locks around read/writes of _scriptEnginesHash --- interface/src/Application.cpp | 113 +++++++++++++++++++++++----------- interface/src/Application.h | 1 + 2 files changed, 77 insertions(+), 37 deletions(-) diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp index 0b29cdc892..8655d6b77e 100644 --- a/interface/src/Application.cpp +++ b/interface/src/Application.cpp @@ -4259,10 +4259,13 @@ ScriptEngine* Application::loadScript(const QString& scriptFilename, bool isUser QUrl scriptUrl(scriptFilename); const QString& scriptURLString = scriptUrl.toString(); - if (_scriptEnginesHash.contains(scriptURLString) && loadScriptFromEditor - && !_scriptEnginesHash[scriptURLString]->isFinished()) { + { + QReadLocker lock(&_scriptEnginesHashLock); + if (_scriptEnginesHash.contains(scriptURLString) && loadScriptFromEditor + && !_scriptEnginesHash[scriptURLString]->isFinished()) { - return _scriptEnginesHash[scriptURLString]; + return _scriptEnginesHash[scriptURLString]; + } } ScriptEngine* scriptEngine = new ScriptEngine(NO_SCRIPT, "", &_controllerScriptingInterface); @@ -4302,7 +4305,11 @@ void Application::reloadScript(const QString& scriptName, bool isUserLoaded) { void Application::handleScriptEngineLoaded(const QString& scriptFilename) { ScriptEngine* scriptEngine = qobject_cast(sender()); - _scriptEnginesHash.insertMulti(scriptFilename, scriptEngine); + { + QWriteLocker lock(&_scriptEnginesHashLock); + _scriptEnginesHash.insertMulti(scriptFilename, scriptEngine); + } + _runningScriptsWidget->setRunningScripts(getRunningScripts()); UserActivityLogger::getInstance().loadedScript(scriptFilename); @@ -4317,55 +4324,86 @@ void Application::handleScriptLoadError(const QString& scriptFilename) { QMessageBox::warning(getWindow(), "Error Loading Script", scriptFilename + " failed to load."); } -void Application::scriptFinished(const QString& scriptName) { - const QString& scriptURLString = QUrl(scriptName).toString(); - QHash::iterator it = _scriptEnginesHash.find(scriptURLString); - if (it != _scriptEnginesHash.end()) { - _scriptEnginesHash.erase(it); - _runningScriptsWidget->scriptStopped(scriptName); - _runningScriptsWidget->setRunningScripts(getRunningScripts()); - } +QStringList Application::getRunningScripts() { + QReadLocker lock(&_scriptEnginesHashLock); + return _scriptEnginesHash.keys(); } -void Application::stopAllScripts(bool restart) { - if (restart) { - // Delete all running scripts from cache so that they are re-downloaded when they are restarted - auto scriptCache = DependencyManager::get(); - for (QHash::const_iterator it = _scriptEnginesHash.constBegin(); - it != _scriptEnginesHash.constEnd(); it++) { - if (!it.value()->isFinished()) { - scriptCache->deleteScript(it.key()); +ScriptEngine* Application::getScriptEngine(const QString& scriptHash) { + QReadLocker lock(&_scriptEnginesHashLock); + return _scriptEnginesHash.value(scriptHash, nullptr); +} + +void Application::scriptFinished(const QString& scriptName, ScriptEngine* engine) { + bool removed = false; + { + QWriteLocker lock(&_scriptEnginesHashLock); + const QString& scriptURLString = QUrl(scriptName).toString(); + for (auto it = _scriptEnginesHash.find(scriptURLString); it != _scriptEnginesHash.end(); ++it) { + if (it.value() == engine) { + _scriptEnginesHash.erase(it); + removed = true; + break; } } } + postLambdaEvent([this, scriptName]() { + _runningScriptsWidget->scriptStopped(scriptName); + _runningScriptsWidget->setRunningScripts(getRunningScripts()); + }); +} - // Stop and possibly restart all currently running scripts - for (QHash::const_iterator it = _scriptEnginesHash.constBegin(); +void Application::stopAllScripts(bool restart) { + { + QReadLocker lock(&_scriptEnginesHashLock); + + if (restart) { + // Delete all running scripts from cache so that they are re-downloaded when they are restarted + auto scriptCache = DependencyManager::get(); + for (QHash::const_iterator it = _scriptEnginesHash.constBegin(); it != _scriptEnginesHash.constEnd(); it++) { - if (it.value()->isFinished()) { - continue; + if (!it.value()->isFinished()) { + scriptCache->deleteScript(it.key()); + } + } } - if (restart && it.value()->isUserLoaded()) { - connect(it.value(), SIGNAL(finished(const QString&)), SLOT(reloadScript(const QString&))); + + // Stop and possibly restart all currently running scripts + for (QHash::const_iterator it = _scriptEnginesHash.constBegin(); + it != _scriptEnginesHash.constEnd(); it++) { + if (it.value()->isFinished()) { + continue; + } + if (restart && it.value()->isUserLoaded()) { + connect(it.value(), &ScriptEngine::finished, this, [this](QString scriptName, ScriptEngine* engine) { + reloadScript(scriptName); + }); + } + QMetaObject::invokeMethod(it.value(), "stop"); + //it.value()->stop(); + qCDebug(interfaceapp) << "stopping script..." << it.key(); } - it.value()->stop(); - qCDebug(interfaceapp) << "stopping script..." << it.key(); } getMyAvatar()->clearScriptableSettings(); } bool Application::stopScript(const QString& scriptHash, bool restart) { bool stoppedScript = false; - if (_scriptEnginesHash.contains(scriptHash)) { - ScriptEngine* scriptEngine = _scriptEnginesHash[scriptHash]; - if (restart) { - auto scriptCache = DependencyManager::get(); - scriptCache->deleteScript(QUrl(scriptHash)); - connect(scriptEngine, SIGNAL(finished(const QString&)), SLOT(reloadScript(const QString&))); + { + QReadLocker lock(&_scriptEnginesHashLock); + if (_scriptEnginesHash.contains(scriptHash)) { + ScriptEngine* scriptEngine = _scriptEnginesHash[scriptHash]; + if (restart) { + auto scriptCache = DependencyManager::get(); + scriptCache->deleteScript(QUrl(scriptHash)); + connect(scriptEngine, &ScriptEngine::finished, this, [this](QString scriptName, ScriptEngine* engine) { + reloadScript(scriptName); + }); + } + scriptEngine->stop(); + stoppedScript = true; + qCDebug(interfaceapp) << "stopping script..." << scriptHash; } - scriptEngine->stop(); - stoppedScript = true; - qCDebug(interfaceapp) << "stopping script..." << scriptHash; } if (_scriptEnginesHash.empty()) { getMyAvatar()->clearScriptableSettings(); @@ -4384,6 +4422,7 @@ void Application::reloadOneScript(const QString& scriptName) { } void Application::loadDefaultScripts() { + QReadLocker lock(&_scriptEnginesHashLock); if (!_scriptEnginesHash.contains(DEFAULT_SCRIPTS_JS_URL)) { loadScript(DEFAULT_SCRIPTS_JS_URL); } diff --git a/interface/src/Application.h b/interface/src/Application.h index ce33f051ef..2f4e33e52d 100644 --- a/interface/src/Application.h +++ b/interface/src/Application.h @@ -505,6 +505,7 @@ private: TouchEvent _lastTouchEvent; + QReadWriteLock _scriptEnginesHashLock; RunningScriptsWidget* _runningScriptsWidget; QHash _scriptEnginesHash; bool _runningScriptsWidgetWasVisible; From fc6a78d9ef74483a861aea122fee0885c674628f Mon Sep 17 00:00:00 2001 From: Ryan Huffman Date: Wed, 9 Dec 2015 14:49:05 -0800 Subject: [PATCH 11/19] Update scriptFinished signal to be a DirectConnection --- interface/src/Application.cpp | 2 +- interface/src/Application.h | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp index 8655d6b77e..740b0062f0 100644 --- a/interface/src/Application.cpp +++ b/interface/src/Application.cpp @@ -4013,7 +4013,7 @@ void Application::registerScriptEngineWithApplicationServices(ScriptEngine* scri scriptEngine->registerGlobalObject("Clipboard", clipboardScriptable); connect(scriptEngine, SIGNAL(finished(const QString&)), clipboardScriptable, SLOT(deleteLater())); - connect(scriptEngine, SIGNAL(finished(const QString&)), this, SLOT(scriptFinished(const QString&))); + connect(scriptEngine, &ScriptEngine::finished, this, &Application::scriptFinished, Qt::DirectConnection); connect(scriptEngine, SIGNAL(loadScript(const QString&, bool)), this, SLOT(loadScript(const QString&, bool))); connect(scriptEngine, SIGNAL(reloadScript(const QString&, bool)), this, SLOT(reloadScript(const QString&, bool))); diff --git a/interface/src/Application.h b/interface/src/Application.h index 2f4e33e52d..0d5b920c10 100644 --- a/interface/src/Application.h +++ b/interface/src/Application.h @@ -202,8 +202,8 @@ public: NodeToJurisdictionMap& getEntityServerJurisdictions() { return _entityServerJurisdictions; } - QStringList getRunningScripts() { return _scriptEnginesHash.keys(); } - ScriptEngine* getScriptEngine(const QString& scriptHash) { return _scriptEnginesHash.value(scriptHash, NULL); } + QStringList getRunningScripts(); + ScriptEngine* getScriptEngine(const QString& scriptHash); float getRenderResolutionScale() const; @@ -336,7 +336,7 @@ private slots: void loadSettings(); void saveSettings(); - void scriptFinished(const QString& scriptName); + void scriptFinished(const QString& scriptName, ScriptEngine* engine); void saveScripts(); void reloadScript(const QString& scriptName, bool isUserLoaded = true); From e7a1550b64e831a9b0eb820d277c72e324c8b189 Mon Sep 17 00:00:00 2001 From: Ryan Huffman Date: Wed, 9 Dec 2015 14:50:09 -0800 Subject: [PATCH 12/19] Update ScriptEngine::finished to include --- libraries/script-engine/src/ScriptEngine.cpp | 4 ++-- libraries/script-engine/src/ScriptEngine.h | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/libraries/script-engine/src/ScriptEngine.cpp b/libraries/script-engine/src/ScriptEngine.cpp index daadb0e394..42287bbea7 100644 --- a/libraries/script-engine/src/ScriptEngine.cpp +++ b/libraries/script-engine/src/ScriptEngine.cpp @@ -183,7 +183,7 @@ void ScriptEngine::runInThread() { QSet ScriptEngine::_allKnownScriptEngines; QMutex ScriptEngine::_allScriptsMutex; -std::atomic ScriptEngine::_stoppingAllScripts = false; +std::atomic ScriptEngine::_stoppingAllScripts { false }; void ScriptEngine::stopAllScripts(QObject* application) { _allScriptsMutex.lock(); @@ -752,7 +752,7 @@ void ScriptEngine::run() { } if (_wantSignals) { - emit finished(_fileNameString); + emit finished(_fileNameString, this); } _isRunning = false; diff --git a/libraries/script-engine/src/ScriptEngine.h b/libraries/script-engine/src/ScriptEngine.h index 82b91aed2e..f8da05135a 100644 --- a/libraries/script-engine/src/ScriptEngine.h +++ b/libraries/script-engine/src/ScriptEngine.h @@ -153,7 +153,7 @@ signals: void errorLoadingScript(const QString& scriptFilename); void update(float deltaTime); void scriptEnding(); - void finished(const QString& fileNameString); + void finished(const QString& fileNameString, ScriptEngine* engine); void cleanupMenuItem(const QString& menuItemString); void printedMessage(const QString& message); void errorMessage(const QString& message); From a21e07327d4da97256411cb6be691f13cedb8af2 Mon Sep 17 00:00:00 2001 From: Ryan Huffman Date: Wed, 9 Dec 2015 14:50:30 -0800 Subject: [PATCH 13/19] Update ScriptEngine::stop() to always run on its thread --- libraries/script-engine/src/ScriptEngine.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/libraries/script-engine/src/ScriptEngine.cpp b/libraries/script-engine/src/ScriptEngine.cpp index 42287bbea7..e06fe2b209 100644 --- a/libraries/script-engine/src/ScriptEngine.cpp +++ b/libraries/script-engine/src/ScriptEngine.cpp @@ -775,6 +775,10 @@ void ScriptEngine::stopAllTimers() { void ScriptEngine::stop() { if (!_isFinished) { + if (QThread::currentThread() != thread()) { + QMetaObject::invokeMethod(this, "stop"); + return; + } _isFinished = true; if (_wantSignals) { emit runningStateChanged(); From 5f60aeef2217e5fc1e3be0da0754a80982f933a4 Mon Sep 17 00:00:00 2001 From: Ryan Huffman Date: Wed, 9 Dec 2015 14:50:48 -0800 Subject: [PATCH 14/19] Update ScriptEngine::_isUserLoaded to be atomic --- libraries/script-engine/src/ScriptEngine.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/script-engine/src/ScriptEngine.h b/libraries/script-engine/src/ScriptEngine.h index f8da05135a..67a206d673 100644 --- a/libraries/script-engine/src/ScriptEngine.h +++ b/libraries/script-engine/src/ScriptEngine.h @@ -193,7 +193,7 @@ protected: Quat _quatLibrary; Vec3 _vec3Library; ScriptUUID _uuidLibrary; - bool _isUserLoaded { false }; + std::atomic _isUserLoaded { false }; bool _isReloading { false }; ArrayBufferClass* _arrayBufferClass; From ad0865171b53ca9030cf6875ae8afced602a2b39 Mon Sep 17 00:00:00 2001 From: Ryan Huffman Date: Wed, 9 Dec 2015 15:52:30 -0800 Subject: [PATCH 15/19] Fix unused variable in scriptFinished --- interface/src/Application.cpp | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp index 740b0062f0..d648537155 100644 --- a/interface/src/Application.cpp +++ b/interface/src/Application.cpp @@ -4347,10 +4347,12 @@ void Application::scriptFinished(const QString& scriptName, ScriptEngine* engine } } } - postLambdaEvent([this, scriptName]() { - _runningScriptsWidget->scriptStopped(scriptName); - _runningScriptsWidget->setRunningScripts(getRunningScripts()); - }); + if (removed) { + postLambdaEvent([this, scriptName]() { + _runningScriptsWidget->scriptStopped(scriptName); + _runningScriptsWidget->setRunningScripts(getRunningScripts()); + }); + } } void Application::stopAllScripts(bool restart) { From acb43e18d0a2c6b18b71c2081dd331a10fcfb68e Mon Sep 17 00:00:00 2001 From: Howard Stearns Date: Wed, 9 Dec 2015 15:54:49 -0800 Subject: [PATCH 16/19] coding standards --- .../src/display-plugins/Basic2DWindowOpenGLDisplayPlugin.h | 2 +- .../src/display-plugins/openvr/OpenVrDisplayPlugin.h | 2 +- plugins/oculus/src/OculusDisplayPlugin.h | 2 +- plugins/oculusLegacy/src/OculusLegacyDisplayPlugin.h | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/libraries/display-plugins/src/display-plugins/Basic2DWindowOpenGLDisplayPlugin.h b/libraries/display-plugins/src/display-plugins/Basic2DWindowOpenGLDisplayPlugin.h index 59c0e4e759..e3633b5fe8 100644 --- a/libraries/display-plugins/src/display-plugins/Basic2DWindowOpenGLDisplayPlugin.h +++ b/libraries/display-plugins/src/display-plugins/Basic2DWindowOpenGLDisplayPlugin.h @@ -9,7 +9,7 @@ #include "WindowOpenGLDisplayPlugin.h" -#define TARGET_FRAMERATE_Basic2DWindowOpenGL 60.0f +const float TARGET_FRAMERATE_Basic2DWindowOpenGL = 60.0f; class QScreen; class QAction; diff --git a/libraries/display-plugins/src/display-plugins/openvr/OpenVrDisplayPlugin.h b/libraries/display-plugins/src/display-plugins/openvr/OpenVrDisplayPlugin.h index 2b51c53d58..d07add7ea5 100644 --- a/libraries/display-plugins/src/display-plugins/openvr/OpenVrDisplayPlugin.h +++ b/libraries/display-plugins/src/display-plugins/openvr/OpenVrDisplayPlugin.h @@ -14,7 +14,7 @@ #include "../WindowOpenGLDisplayPlugin.h" -#define TARGET_RATE_OpenVr 90.0f; // FIXME: get from sdk tracked device property? This number is vive-only. +const float TARGET_RATE_OpenVr = 90.0f; // FIXME: get from sdk tracked device property? This number is vive-only. class OpenVrDisplayPlugin : public WindowOpenGLDisplayPlugin { public: diff --git a/plugins/oculus/src/OculusDisplayPlugin.h b/plugins/oculus/src/OculusDisplayPlugin.h index 071dca0258..03c9ba7511 100644 --- a/plugins/oculus/src/OculusDisplayPlugin.h +++ b/plugins/oculus/src/OculusDisplayPlugin.h @@ -12,7 +12,7 @@ struct SwapFramebufferWrapper; using SwapFboPtr = QSharedPointer; -#define TARGET_RATE_Oculus 75.0f; +const float TARGET_RATE_Oculus = 75.0f; class OculusDisplayPlugin : public OculusBaseDisplayPlugin { public: diff --git a/plugins/oculusLegacy/src/OculusLegacyDisplayPlugin.h b/plugins/oculusLegacy/src/OculusLegacyDisplayPlugin.h index 7fe721fc45..d3e68585df 100644 --- a/plugins/oculusLegacy/src/OculusLegacyDisplayPlugin.h +++ b/plugins/oculusLegacy/src/OculusLegacyDisplayPlugin.h @@ -13,7 +13,7 @@ #include -#define TARGET_RATE_OculusLegacy 75.0f; +const float TARGET_RATE_OculusLegacy = 75.0f; class OculusLegacyDisplayPlugin : public WindowOpenGLDisplayPlugin { public: From 3b372fd897aa9b4c011216037201c6c35ff92491 Mon Sep 17 00:00:00 2001 From: Howard Stearns Date: Wed, 9 Dec 2015 16:12:34 -0800 Subject: [PATCH 17/19] coding standards --- .../Basic2DWindowOpenGLDisplayPlugin.cpp | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/libraries/display-plugins/src/display-plugins/Basic2DWindowOpenGLDisplayPlugin.cpp b/libraries/display-plugins/src/display-plugins/Basic2DWindowOpenGLDisplayPlugin.cpp index 9139a9dcc9..5beadffb1e 100644 --- a/libraries/display-plugins/src/display-plugins/Basic2DWindowOpenGLDisplayPlugin.cpp +++ b/libraries/display-plugins/src/display-plugins/Basic2DWindowOpenGLDisplayPlugin.cpp @@ -83,17 +83,16 @@ void Basic2DWindowOpenGLDisplayPlugin::internalPresent() { } WindowOpenGLDisplayPlugin::internalPresent(); } -#define THROTTLED_FRAMERATE 15 +const uint32_t THROTTLED_FRAMERATE = 15; int Basic2DWindowOpenGLDisplayPlugin::getDesiredInterval() const { static const int ULIMIITED_PAINT_TIMER_DELAY_MS = 1; int result = ULIMIITED_PAINT_TIMER_DELAY_MS; - // This test wouldn't be necessary if we could depend on updateFramerate setting _framerateTarget. - // Alas, that gets complicated: isThrottled() is const and other stuff depends on it. - if (_isThrottled) { - result = MSECS_PER_SECOND / THROTTLED_FRAMERATE; - } if (0 != _framerateTarget) { result = MSECS_PER_SECOND / _framerateTarget; + } else if (_isThrottled) { + // This test wouldn't be necessary if we could depend on updateFramerate setting _framerateTarget. + // Alas, that gets complicated: isThrottled() is const and other stuff depends on it. + result = MSECS_PER_SECOND / THROTTLED_FRAMERATE; } qDebug() << "New interval " << result; @@ -135,7 +134,7 @@ void Basic2DWindowOpenGLDisplayPlugin::updateFramerate() { _framerateTarget = 30; } } else if (_isThrottled) { - _framerateTarget = (float) THROTTLED_FRAMERATE; + _framerateTarget = THROTTLED_FRAMERATE; } int newInterval = getDesiredInterval(); From d731875c5b8ac454115255bb266e6cb3ab4e36e2 Mon Sep 17 00:00:00 2001 From: Howard Stearns Date: Thu, 10 Dec 2015 09:58:53 -0800 Subject: [PATCH 18/19] Fix dependencies for Linux builds. --- libraries/input-plugins/CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/input-plugins/CMakeLists.txt b/libraries/input-plugins/CMakeLists.txt index af7cc3daa0..101a6c8016 100644 --- a/libraries/input-plugins/CMakeLists.txt +++ b/libraries/input-plugins/CMakeLists.txt @@ -1,6 +1,6 @@ set(TARGET_NAME input-plugins) setup_hifi_library() -link_hifi_libraries(shared plugins controllers) +link_hifi_libraries(shared plugins controllers script-engine) GroupSources("src/input-plugins") From dd011e8bc4c72ecb9578666f7d369bba0793cb00 Mon Sep 17 00:00:00 2001 From: Howard Stearns Date: Thu, 10 Dec 2015 10:05:01 -0800 Subject: [PATCH 19/19] Freakin' xcode ^S. --- libraries/input-plugins/CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/input-plugins/CMakeLists.txt b/libraries/input-plugins/CMakeLists.txt index 101a6c8016..d2cf96c7f8 100644 --- a/libraries/input-plugins/CMakeLists.txt +++ b/libraries/input-plugins/CMakeLists.txt @@ -1,6 +1,6 @@ set(TARGET_NAME input-plugins) setup_hifi_library() -link_hifi_libraries(shared plugins controllers script-engine) +link_hifi_libraries(shared plugins controllers script-engine) GroupSources("src/input-plugins")