From a0d80b9508fe4ebd79dc030f832a410d3e25e73f Mon Sep 17 00:00:00 2001 From: Zach Pomerantz Date: Mon, 9 May 2016 16:23:59 -0700 Subject: [PATCH 01/39] Fix hang on new script on shutdown --- libraries/script-engine/src/ScriptEngines.cpp | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/libraries/script-engine/src/ScriptEngines.cpp b/libraries/script-engine/src/ScriptEngines.cpp index c6070e0598..d78fd9fe0c 100644 --- a/libraries/script-engine/src/ScriptEngines.cpp +++ b/libraries/script-engine/src/ScriptEngines.cpp @@ -118,9 +118,9 @@ void ScriptEngines::registerScriptInitializer(ScriptInitializer initializer) { } void ScriptEngines::addScriptEngine(ScriptEngine* engine) { - _allScriptsMutex.lock(); - _allKnownScriptEngines.insert(engine); - _allScriptsMutex.unlock(); + if (!_stoppingAllScripts) { + _allKnownScriptEngines.insert(engine); + } } void ScriptEngines::removeScriptEngine(ScriptEngine* engine) { @@ -128,16 +128,15 @@ void ScriptEngines::removeScriptEngine(ScriptEngine* engine) { // from the list of running scripts. We don't do this if we're in the process of stopping all scripts // because that method removes scripts from its list as it iterates them if (!_stoppingAllScripts) { - _allScriptsMutex.lock(); + QMutexLocker locker(&_allScriptsMutex); _allKnownScriptEngines.remove(engine); - _allScriptsMutex.unlock(); } } void ScriptEngines::shutdownScripting() { - _allScriptsMutex.lock(); _stoppingAllScripts = true; ScriptEngine::_stoppingAllScripts = true; + QMutexLocker locker(&_allScriptsMutex); qCDebug(scriptengine) << "Stopping all scripts.... currently known scripts:" << _allKnownScriptEngines.size(); QMutableSetIterator i(_allKnownScriptEngines); @@ -173,8 +172,6 @@ void ScriptEngines::shutdownScripting() { i.remove(); } } - _stoppingAllScripts = false; - _allScriptsMutex.unlock(); qCDebug(scriptengine) << "DONE Stopping all scripts...."; } From 694dc1bbf6e0cf4676da89aa7076079cb9c37b99 Mon Sep 17 00:00:00 2001 From: Zach Pomerantz Date: Mon, 9 May 2016 17:28:27 -0700 Subject: [PATCH 02/39] Delay Overlays cleanup for scripting --- interface/src/Application.cpp | 3 +++ interface/src/ui/overlays/Overlays.cpp | 11 ++--------- interface/src/ui/overlays/Overlays.h | 4 ++-- 3 files changed, 7 insertions(+), 11 deletions(-) diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp index 5b0d5c65ce..77e266bdb8 100644 --- a/interface/src/Application.cpp +++ b/interface/src/Application.cpp @@ -1189,6 +1189,9 @@ void Application::cleanupBeforeQuit() { DependencyManager::get()->shutdownScripting(); // stop all currently running global scripts DependencyManager::destroy(); + // Cleanup all overlays after the scripts, as scripts might add more + _overlays.cleanupAllOverlays(); + // first stop all timers directly or by invokeMethod // depending on what thread they run in locationUpdateTimer.stop(); diff --git a/interface/src/ui/overlays/Overlays.cpp b/interface/src/ui/overlays/Overlays.cpp index 1179fbaa50..9ff7f6268f 100644 --- a/interface/src/ui/overlays/Overlays.cpp +++ b/interface/src/ui/overlays/Overlays.cpp @@ -36,15 +36,8 @@ #include -Overlays::Overlays() : _nextOverlayID(1) { - connect(qApp, &Application::beforeAboutToQuit, [=] { - cleanupAllOverlays(); - }); -} - -Overlays::~Overlays() { -} - +Overlays::Overlays() : + _nextOverlayID(1) {} void Overlays::cleanupAllOverlays() { { diff --git a/interface/src/ui/overlays/Overlays.h b/interface/src/ui/overlays/Overlays.h index 25ba00fdf0..f47f8de153 100644 --- a/interface/src/ui/overlays/Overlays.h +++ b/interface/src/ui/overlays/Overlays.h @@ -62,7 +62,6 @@ class Overlays : public QObject { public: Overlays(); - ~Overlays(); void init(); void update(float deltatime); @@ -73,6 +72,8 @@ public: Overlay::Pointer getOverlay(unsigned int id) const; OverlayPanel::Pointer getPanel(unsigned int id) const { return _panels[id]; } + void cleanupAllOverlays(); + public slots: /// adds an overlay with the specific properties unsigned int addOverlay(const QString& type, const QVariant& properties); @@ -145,7 +146,6 @@ signals: private: void cleanupOverlaysToDelete(); - void cleanupAllOverlays(); QMap _overlaysHUD; QMap _overlaysWorld; From 2d820221dc83ffb42ac2070793393334e8f28d15 Mon Sep 17 00:00:00 2001 From: Zach Pomerantz Date: Mon, 9 May 2016 17:30:44 -0700 Subject: [PATCH 03/39] Clear queued processing on quit --- interface/src/Application.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp index 77e266bdb8..de955bb5b7 100644 --- a/interface/src/Application.cpp +++ b/interface/src/Application.cpp @@ -1185,6 +1185,9 @@ void Application::cleanupBeforeQuit() { getEntities()->shutdown(); // tell the entities system we're shutting down, so it will stop running scripts + // Clear any queued processing (I/O, FBX/OBJ/Texture parsing) + QThreadPool::globalInstance()->clear(); + DependencyManager::get()->saveScripts(); DependencyManager::get()->shutdownScripting(); // stop all currently running global scripts DependencyManager::destroy(); From 68731973d8f8288635b2e2aa2bc611bdd74c26e0 Mon Sep 17 00:00:00 2001 From: Zach Pomerantz Date: Mon, 9 May 2016 18:04:38 -0700 Subject: [PATCH 04/39] Keep locker when adding script engine --- libraries/script-engine/src/ScriptEngines.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/libraries/script-engine/src/ScriptEngines.cpp b/libraries/script-engine/src/ScriptEngines.cpp index d78fd9fe0c..61220bc00b 100644 --- a/libraries/script-engine/src/ScriptEngines.cpp +++ b/libraries/script-engine/src/ScriptEngines.cpp @@ -119,6 +119,7 @@ void ScriptEngines::registerScriptInitializer(ScriptInitializer initializer) { void ScriptEngines::addScriptEngine(ScriptEngine* engine) { if (!_stoppingAllScripts) { + QMutexLocker locker(&_allScriptsMutex); _allKnownScriptEngines.insert(engine); } } From c3f41cdd89a1c84a45cbe0595c821eea38607425 Mon Sep 17 00:00:00 2001 From: Zach Pomerantz Date: Tue, 10 May 2016 09:41:44 -0700 Subject: [PATCH 05/39] Add PAINT_DELAY_DEBUG log --- interface/src/Application.cpp | 14 ++++++++++++++ libraries/plugins/src/plugins/DisplayPlugin.cpp | 12 ++++++++++++ libraries/plugins/src/plugins/DisplayPlugin.h | 6 +++++- 3 files changed, 31 insertions(+), 1 deletion(-) diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp index aa98724a7d..0568b2f20e 100644 --- a/interface/src/Application.cpp +++ b/interface/src/Application.cpp @@ -2624,6 +2624,20 @@ void Application::idle(uint64_t now) { return; // bail early, we're throttled and not enough time has elapsed } +#ifdef DEBUG_PAINT_DELAY + static uint64_t paintDelaySamples{ 0 }; + static uint64_t paintDelayUsecs{ 0 }; + + paintDelayUsecs += displayPlugin->getPaintDelayUsecs(); + + static const int PAINT_DELAY_THROTTLE = 1000; + if (++paintDelaySamples % PAINT_DELAY_THROTTLE == 0) { + qCDebug(interfaceapp).nospace() << + "Paint delay (" << paintDelaySamples << " samples): " << + (float)paintDelaySamples / paintDelayUsecs << "us"; + } +#endif + auto presentCount = displayPlugin->presentCount(); if (presentCount < _renderedFrameIndex) { _renderedFrameIndex = INVALID_FRAME; diff --git a/libraries/plugins/src/plugins/DisplayPlugin.cpp b/libraries/plugins/src/plugins/DisplayPlugin.cpp index c7fa5f5671..f946547ebe 100644 --- a/libraries/plugins/src/plugins/DisplayPlugin.cpp +++ b/libraries/plugins/src/plugins/DisplayPlugin.cpp @@ -1,5 +1,6 @@ #include "DisplayPlugin.h" +#include #include #include "PluginContainer.h" @@ -23,4 +24,15 @@ void DisplayPlugin::deactivate() { Parent::deactivate(); } +int64_t DisplayPlugin::getPaintDelayUsecs() const { + return _paintDelayTimer.isValid() ? _paintDelayTimer.nsecsElapsed() / NSECS_PER_USEC : 0; +} +void DisplayPlugin::incrementPresentCount() { +#ifdef DEBUG_PAINT_DELAY + // Avoid overhead if we are not debugging + _paintDelayTimer.start(); +#endif + + ++_presentedFrameIndex; +} diff --git a/libraries/plugins/src/plugins/DisplayPlugin.h b/libraries/plugins/src/plugins/DisplayPlugin.h index 91dcf9398f..03a7737c3e 100644 --- a/libraries/plugins/src/plugins/DisplayPlugin.h +++ b/libraries/plugins/src/plugins/DisplayPlugin.h @@ -15,6 +15,7 @@ #include #include +#include class QImage; #include @@ -156,6 +157,8 @@ public: // Rate at which rendered frames are being skipped virtual float droppedFrameRate() const { return -1.0f; } uint32_t presentCount() const { return _presentedFrameIndex; } + // Time since last call to incrementPresentCount. Only valid if DEBUG_PAINT_DELAY is defined + int64_t getPaintDelayUsecs() const; virtual void cycleDebugOutput() {} @@ -165,9 +168,10 @@ signals: void recommendedFramebufferSizeChanged(const QSize & size); protected: - void incrementPresentCount() { ++_presentedFrameIndex; } + void incrementPresentCount(); private: std::atomic _presentedFrameIndex; + QElapsedTimer _paintDelayTimer; }; From 783be531254e4b0e4e7eb5df7162ca880b9020ac Mon Sep 17 00:00:00 2001 From: Zach Pomerantz Date: Tue, 10 May 2016 11:20:08 -0700 Subject: [PATCH 06/39] Trigger Idle from present Paint --- interface/src/Application.cpp | 117 +++++------------- interface/src/Application.h | 10 +- .../plugins/src/plugins/DisplayPlugin.cpp | 3 + libraries/plugins/src/plugins/DisplayPlugin.h | 4 + 4 files changed, 49 insertions(+), 85 deletions(-) diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp index 0568b2f20e..bce0b77096 100644 --- a/interface/src/Application.cpp +++ b/interface/src/Application.cpp @@ -347,19 +347,14 @@ public: }; #endif -enum CustomEventTypes { - Lambda = QEvent::User + 1, - Paint = Lambda + 1, -}; - class LambdaEvent : public QEvent { std::function _fun; public: LambdaEvent(const std::function & fun) : - QEvent(static_cast(Lambda)), _fun(fun) { + QEvent(static_cast(Application::Lambda)), _fun(fun) { } LambdaEvent(std::function && fun) : - QEvent(static_cast(Lambda)), _fun(fun) { + QEvent(static_cast(Application::Lambda)), _fun(fun) { } void call() const { _fun(); } }; @@ -1060,18 +1055,6 @@ Application::Application(int& argc, char** argv, QElapsedTimer& startupTimer) : connect(this, &Application::applicationStateChanged, this, &Application::activeChanged); qCDebug(interfaceapp, "Startup time: %4.2f seconds.", (double)startupTimer.elapsed() / 1000.0); - - _idleTimer = new QTimer(this); - connect(_idleTimer, &QTimer::timeout, [=] { - idle(usecTimestampNow()); - }); - connect(this, &Application::beforeAboutToQuit, [=] { - disconnect(_idleTimer); - }); - // Setting the interval to zero forces this to get called whenever there are no messages - // in the queue, which can be pretty damn frequent. Hence the idle function has a bunch - // of logic to abort early if it's being called too often. - _idleTimer->start(0); } @@ -1437,23 +1420,15 @@ void Application::initializeUi() { }); } - void Application::paintGL() { - updateHeartbeat(); - // Some plugins process message events, potentially leading to - // re-entering a paint event. don't allow further processing if this - // happens - if (_inPaint) { + // Some plugins process message events, allowing paintGL to be called reentrantly. + if (_inPaint || _aboutToQuit) { return; } - _inPaint = true; - Finally clearFlagLambda([this] { _inPaint = false; }); - // paintGL uses a queued connection, so we can get messages from the queue even after we've quit - // and the plugins have shutdown - if (_aboutToQuit) { - return; - } + _inPaint = true; + Finally clearFlag([this] { _inPaint = false; }); + _frameCount++; _frameCounter.increment(); @@ -1811,14 +1786,16 @@ bool Application::event(QEvent* event) { return false; } - if ((int)event->type() == (int)Lambda) { - static_cast(event)->call(); + if ((int)event->type() == (int)Idle) { + idle(); + removePostedEvents(this, Idle); // clear pending idles so we don't clog the pipes return true; - } - - if ((int)event->type() == (int)Paint) { + } else if ((int)event->type() == (int)Paint) { paintGL(); return true; + } else if ((int)event->type() == (int)Lambda) { + static_cast(event)->call(); + return true; } if (!_keyboardFocusedItem.isInvalidID()) { @@ -2595,34 +2572,13 @@ bool Application::acceptSnapshot(const QString& urlString) { static uint32_t _renderedFrameIndex { INVALID_FRAME }; -void Application::idle(uint64_t now) { - // NOTICE NOTICE NOTICE NOTICE - // Do not insert new code between here and the PROFILE_RANGE declaration - // unless you know exactly what you're doing. This idle function can be - // called thousands per second or more, so any additional work that's done - // here will have a serious impact on CPU usage. Only add code after all - // the thottling logic, i.e. after PROFILE_RANGE - // NOTICE NOTICE NOTICE NOTICE - updateHeartbeat(); - - if (_aboutToQuit || _inPaint) { - return; // bail early, nothing to do here. +void Application::idle() { + // idle is called on a queued connection, so make sure we should be here. + if (_inPaint || _aboutToQuit) { + return; } auto displayPlugin = getActiveDisplayPlugin(); - // depending on whether we're throttling or not. - // Once rendering is off on another thread we should be able to have Application::idle run at start(0) in - // perpetuity and not expect events to get backed up. - bool isThrottled = displayPlugin->isThrottled(); - // Only run simulation code if more than the targetFramePeriod have passed since last time we ran - // This attempts to lock the simulation at 60 updates per second, regardless of framerate - float timeSinceLastUpdateUs = (float)_lastTimeUpdated.nsecsElapsed() / NSECS_PER_USEC; - float secondsSinceLastUpdate = timeSinceLastUpdateUs / USECS_PER_SECOND; - - if (isThrottled && (timeSinceLastUpdateUs / USECS_PER_MSEC) < THROTTLED_SIM_FRAME_PERIOD_MS) { - // Throttling both rendering and idle - return; // bail early, we're throttled and not enough time has elapsed - } #ifdef DEBUG_PAINT_DELAY static uint64_t paintDelaySamples{ 0 }; @@ -2638,43 +2594,38 @@ void Application::idle(uint64_t now) { } #endif - auto presentCount = displayPlugin->presentCount(); - if (presentCount < _renderedFrameIndex) { - _renderedFrameIndex = INVALID_FRAME; - } + float msecondsSinceLastUpdate = (float)_lastTimeUpdated.nsecsElapsed() / NSECS_PER_USEC / USECS_PER_MSEC; - // Don't saturate the main thread with rendering and simulation, - // unless display plugin has increased by at least one frame - if (_renderedFrameIndex == INVALID_FRAME || presentCount > _renderedFrameIndex) { - // Record what present frame we're on - _renderedFrameIndex = presentCount; - - // request a paint, get to it as soon as possible: high priority - postEvent(this, new QEvent(static_cast(Paint)), Qt::HighEventPriority); - } else { - // there's no use in simulating or rendering faster then the present rate. + // Throttle if requested + if (displayPlugin->isThrottled() && (msecondsSinceLastUpdate < THROTTLED_SIM_FRAME_PERIOD_MS)) { return; } - // NOTICE NOTICE NOTICE NOTICE - // do NOT add new code above this line unless you want it to be executed potentially - // thousands of times per second - // NOTICE NOTICE NOTICE NOTICE + // Sync up the _renderedFrameIndex + _renderedFrameIndex = displayPlugin->presentCount(); - PROFILE_RANGE(__FUNCTION__); + // Request a paint ASAP + postEvent(this, new QEvent(static_cast(Paint)), Qt::HighEventPriority); + + // Update the deadlock watchdog + updateHeartbeat(); + + auto offscreenUi = DependencyManager::get(); // These tasks need to be done on our first idle, because we don't want the showing of // overlay subwindows to do a showDesktop() until after the first time through static bool firstIdle = true; if (firstIdle) { firstIdle = false; - auto offscreenUi = DependencyManager::get(); connect(offscreenUi.data(), &OffscreenUi::showDesktop, this, &Application::showDesktop); _overlayConductor.setEnabled(Menu::getInstance()->isOptionChecked(MenuOption::Overlays)); } + PROFILE_RANGE(__FUNCTION__); + + float secondsSinceLastUpdate = msecondsSinceLastUpdate / MSECS_PER_SECOND; + // If the offscreen Ui has something active that is NOT the root, then assume it has keyboard focus. - auto offscreenUi = DependencyManager::get(); if (_keyboardDeviceHasFocus && offscreenUi && offscreenUi->getWindow()->activeFocusItem() != offscreenUi->getRootItem()) { _keyboardMouseDevice->pluginFocusOutEvent(); _keyboardDeviceHasFocus = false; diff --git a/interface/src/Application.h b/interface/src/Application.h index f52dfc8c07..e13dffbcf1 100644 --- a/interface/src/Application.h +++ b/interface/src/Application.h @@ -33,6 +33,7 @@ #include #include #include +#include #include #include #include @@ -93,6 +94,12 @@ class Application : public QApplication, public AbstractViewStateInterface, publ friend class PluginContainerProxy; public: + enum Event { + Idle = DisplayPlugin::Paint, + Paint = Idle + 1, + Lambda = Paint + 1 + }; + // FIXME? Empty methods, do we still need them? static void initPlugins(); static void shutdownPlugins(); @@ -282,7 +289,6 @@ public slots: private slots: void showDesktop(); void clearDomainOctreeDetails(); - void idle(uint64_t now); void aboutToQuit(); void resettingDomain(); @@ -321,6 +327,7 @@ private: void cleanupBeforeQuit(); + void idle(); void update(float deltaTime); // Various helper functions called during update() @@ -498,7 +505,6 @@ private: int _avatarAttachmentRequest = 0; bool _settingsLoaded { false }; - QTimer* _idleTimer { nullptr }; bool _fakedMouseEvent { false }; diff --git a/libraries/plugins/src/plugins/DisplayPlugin.cpp b/libraries/plugins/src/plugins/DisplayPlugin.cpp index f946547ebe..f52910b952 100644 --- a/libraries/plugins/src/plugins/DisplayPlugin.cpp +++ b/libraries/plugins/src/plugins/DisplayPlugin.cpp @@ -35,4 +35,7 @@ void DisplayPlugin::incrementPresentCount() { #endif ++_presentedFrameIndex; + + // Alert the app that it needs to paint a new presentation frame + qApp->postEvent(qApp, new QEvent(static_cast(Paint)), Qt::HighEventPriority); } diff --git a/libraries/plugins/src/plugins/DisplayPlugin.h b/libraries/plugins/src/plugins/DisplayPlugin.h index 03a7737c3e..1e9b16eeac 100644 --- a/libraries/plugins/src/plugins/DisplayPlugin.h +++ b/libraries/plugins/src/plugins/DisplayPlugin.h @@ -60,6 +60,10 @@ class DisplayPlugin : public Plugin { Q_OBJECT using Parent = Plugin; public: + enum Event { + Paint = QEvent::User + 1 + }; + bool activate() override; void deactivate() override; virtual bool isHmd() const { return false; } From 4711c23d9d5b612214689c9711dcd7a8709128fe Mon Sep 17 00:00:00 2001 From: Zach Pomerantz Date: Tue, 10 May 2016 14:34:12 -0700 Subject: [PATCH 07/39] Delay AvatarInputs instantiation to first paint --- interface/src/Application.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp index bce0b77096..4ae206795d 100644 --- a/interface/src/Application.cpp +++ b/interface/src/Application.cpp @@ -2619,6 +2619,9 @@ void Application::idle() { firstIdle = false; connect(offscreenUi.data(), &OffscreenUi::showDesktop, this, &Application::showDesktop); _overlayConductor.setEnabled(Menu::getInstance()->isOptionChecked(MenuOption::Overlays)); + } else { + // FIXME: AvatarInputs are positioned incorrectly if instantiated before the first paint + AvatarInputs::getInstance()->update(); } PROFILE_RANGE(__FUNCTION__); @@ -2641,7 +2644,6 @@ void Application::idle() { checkChangeCursor(); Stats::getInstance()->updateStats(); - AvatarInputs::getInstance()->update(); _simCounter.increment(); From dd6a4dd091b87262716db6ece18dc8a8dd77f665 Mon Sep 17 00:00:00 2001 From: Zach Pomerantz Date: Tue, 10 May 2016 16:06:33 -0700 Subject: [PATCH 08/39] Make getPaintDelayUsecs threadsafe --- libraries/plugins/src/plugins/DisplayPlugin.cpp | 6 +++++- libraries/plugins/src/plugins/DisplayPlugin.h | 3 ++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/libraries/plugins/src/plugins/DisplayPlugin.cpp b/libraries/plugins/src/plugins/DisplayPlugin.cpp index f52910b952..430da00061 100644 --- a/libraries/plugins/src/plugins/DisplayPlugin.cpp +++ b/libraries/plugins/src/plugins/DisplayPlugin.cpp @@ -25,13 +25,17 @@ void DisplayPlugin::deactivate() { } int64_t DisplayPlugin::getPaintDelayUsecs() const { + std::lock_guard lock(_paintDelayMutex); return _paintDelayTimer.isValid() ? _paintDelayTimer.nsecsElapsed() / NSECS_PER_USEC : 0; } void DisplayPlugin::incrementPresentCount() { #ifdef DEBUG_PAINT_DELAY // Avoid overhead if we are not debugging - _paintDelayTimer.start(); + { + std::lock_guard lock(_paintDelayMutex); + _paintDelayTimer.start(); + } #endif ++_presentedFrameIndex; diff --git a/libraries/plugins/src/plugins/DisplayPlugin.h b/libraries/plugins/src/plugins/DisplayPlugin.h index 1e9b16eeac..3a668f27d7 100644 --- a/libraries/plugins/src/plugins/DisplayPlugin.h +++ b/libraries/plugins/src/plugins/DisplayPlugin.h @@ -161,7 +161,7 @@ public: // Rate at which rendered frames are being skipped virtual float droppedFrameRate() const { return -1.0f; } uint32_t presentCount() const { return _presentedFrameIndex; } - // Time since last call to incrementPresentCount. Only valid if DEBUG_PAINT_DELAY is defined + // Time since last call to incrementPresentCount (only valid if DEBUG_PAINT_DELAY is defined) int64_t getPaintDelayUsecs() const; virtual void cycleDebugOutput() {} @@ -176,6 +176,7 @@ protected: private: std::atomic _presentedFrameIndex; + mutable std::mutex _paintDelayMutex; QElapsedTimer _paintDelayTimer; }; From 2d4fd783bd7f808dc9c718092f45b8b25c0de327 Mon Sep 17 00:00:00 2001 From: Zach Pomerantz Date: Tue, 10 May 2016 16:09:04 -0700 Subject: [PATCH 09/39] Rename Paint/Idle events to Present --- interface/src/Application.cpp | 4 ++-- interface/src/Application.h | 4 ++-- libraries/plugins/src/plugins/DisplayPlugin.cpp | 2 +- libraries/plugins/src/plugins/DisplayPlugin.h | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp index 4ae206795d..3ba362e56d 100644 --- a/interface/src/Application.cpp +++ b/interface/src/Application.cpp @@ -1786,9 +1786,9 @@ bool Application::event(QEvent* event) { return false; } - if ((int)event->type() == (int)Idle) { + if ((int)event->type() == (int)Present) { idle(); - removePostedEvents(this, Idle); // clear pending idles so we don't clog the pipes + removePostedEvents(this, Present); // clear pending presents so we don't clog the pipes return true; } else if ((int)event->type() == (int)Paint) { paintGL(); diff --git a/interface/src/Application.h b/interface/src/Application.h index e13dffbcf1..0d88352b2f 100644 --- a/interface/src/Application.h +++ b/interface/src/Application.h @@ -95,8 +95,8 @@ class Application : public QApplication, public AbstractViewStateInterface, publ public: enum Event { - Idle = DisplayPlugin::Paint, - Paint = Idle + 1, + Present = DisplayPlugin::Present, + Paint = Present + 1, Lambda = Paint + 1 }; diff --git a/libraries/plugins/src/plugins/DisplayPlugin.cpp b/libraries/plugins/src/plugins/DisplayPlugin.cpp index 430da00061..a217041f4e 100644 --- a/libraries/plugins/src/plugins/DisplayPlugin.cpp +++ b/libraries/plugins/src/plugins/DisplayPlugin.cpp @@ -41,5 +41,5 @@ void DisplayPlugin::incrementPresentCount() { ++_presentedFrameIndex; // Alert the app that it needs to paint a new presentation frame - qApp->postEvent(qApp, new QEvent(static_cast(Paint)), Qt::HighEventPriority); + qApp->postEvent(qApp, new QEvent(static_cast(Present)), Qt::HighEventPriority); } diff --git a/libraries/plugins/src/plugins/DisplayPlugin.h b/libraries/plugins/src/plugins/DisplayPlugin.h index 3a668f27d7..41f380aa86 100644 --- a/libraries/plugins/src/plugins/DisplayPlugin.h +++ b/libraries/plugins/src/plugins/DisplayPlugin.h @@ -61,7 +61,7 @@ class DisplayPlugin : public Plugin { using Parent = Plugin; public: enum Event { - Paint = QEvent::User + 1 + Present = QEvent::User + 1 }; bool activate() override; From 25deaf5bda523ef53c574e997dae4b515be42931 Mon Sep 17 00:00:00 2001 From: Zach Pomerantz Date: Wed, 11 May 2016 18:27:32 -0700 Subject: [PATCH 10/39] Repost missed Presents with low priority --- interface/src/Application.cpp | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp index 3ba362e56d..3599303239 100644 --- a/interface/src/Application.cpp +++ b/interface/src/Application.cpp @@ -1786,14 +1786,29 @@ bool Application::event(QEvent* event) { return false; } + static bool justPresented = false; if ((int)event->type() == (int)Present) { + if (justPresented) { + justPresented = false; + + // If presentation is hogging the main thread, repost as low priority to avoid hanging the GUI. + // This has the effect of allowing presentation to exceed the paint budget by X times and + // only dropping every (1/X) frames, instead of every ceil(X) frames. + // (e.g. at a 60FPS target, painting for 17us would fall to 58.82FPS instead of 30FPS). + removePostedEvents(this, Present); + postEvent(this, new QEvent(static_cast(Present)), Qt::LowEventPriority); + return true; + } + idle(); - removePostedEvents(this, Present); // clear pending presents so we don't clog the pipes return true; } else if ((int)event->type() == (int)Paint) { + justPresented = true; paintGL(); return true; - } else if ((int)event->type() == (int)Lambda) { + } + + if ((int)event->type() == (int)Lambda) { static_cast(event)->call(); return true; } @@ -2605,7 +2620,7 @@ void Application::idle() { _renderedFrameIndex = displayPlugin->presentCount(); // Request a paint ASAP - postEvent(this, new QEvent(static_cast(Paint)), Qt::HighEventPriority); + postEvent(this, new QEvent(static_cast(Paint)), Qt::HighEventPriority + 1); // Update the deadlock watchdog updateHeartbeat(); From 820fdf09e2be812feef4cedc601d84e8ed5da2ec Mon Sep 17 00:00:00 2001 From: Zach Pomerantz Date: Thu, 12 May 2016 12:42:29 -0700 Subject: [PATCH 11/39] Remove unused ScriptEngine::_wantSignals --- libraries/script-engine/src/ScriptEngine.cpp | 72 ++++++------------- libraries/script-engine/src/ScriptEngine.h | 5 +- libraries/script-engine/src/ScriptEngines.cpp | 2 +- 3 files changed, 24 insertions(+), 55 deletions(-) diff --git a/libraries/script-engine/src/ScriptEngine.cpp b/libraries/script-engine/src/ScriptEngine.cpp index f7ac7894ff..53ffd19869 100644 --- a/libraries/script-engine/src/ScriptEngine.cpp +++ b/libraries/script-engine/src/ScriptEngine.cpp @@ -138,10 +138,9 @@ static bool hadUncaughtExceptions(QScriptEngine& engine, const QString& fileName return false; } -ScriptEngine::ScriptEngine(const QString& scriptContents, const QString& fileNameString, bool wantSignals) : +ScriptEngine::ScriptEngine(const QString& scriptContents, const QString& fileNameString) : _scriptContents(scriptContents), _timerFunctionMap(), - _wantSignals(wantSignals), _fileNameString(fileNameString), _arrayBufferClass(new ArrayBufferClass(this)) { @@ -231,15 +230,14 @@ void ScriptEngine::runDebuggable() { return; } stopAllTimers(); // make sure all our timers are stopped if the script is ending - if (_wantSignals) { - emit scriptEnding(); - emit finished(_fileNameString, this); - } + + emit scriptEnding(); + emit finished(_fileNameString, this); _isRunning = false; - if (_wantSignals) { - emit runningStateChanged(); - emit doneRunning(); - } + + emit runningStateChanged(); + emit doneRunning(); + timer->deleteLater(); return; } @@ -249,9 +247,7 @@ void ScriptEngine::runDebuggable() { if (_lastUpdate < now) { float deltaTime = (float)(now - _lastUpdate) / (float)USECS_PER_SECOND; if (!_isFinished) { - if (_wantSignals) { - emit update(deltaTime); - } + emit update(deltaTime); } } _lastUpdate = now; @@ -358,17 +354,13 @@ void ScriptEngine::scriptContentsAvailable(const QUrl& url, const QString& scrip if (QRegularExpression(DEBUG_FLAG).match(scriptContents).hasMatch()) { _debuggable = true; } - if (_wantSignals) { - emit scriptLoaded(url.toString()); - } + emit scriptLoaded(url.toString()); } // FIXME - switch this to the new model of ScriptCache callbacks void ScriptEngine::errorInLoadingScript(const QUrl& url) { qCDebug(scriptengine) << "ERROR Loading file:" << url.toString() << "line:" << __LINE__; - if (_wantSignals) { - emit errorLoadingScript(_fileNameString); // ?? - } + emit errorLoadingScript(_fileNameString); // ?? } // Even though we never pass AnimVariantMap directly to and from javascript, the queued invokeMethod of @@ -785,9 +777,7 @@ QScriptValue ScriptEngine::evaluate(const QString& sourceCode, const QString& fi --_evaluatesPending; const auto hadUncaughtException = hadUncaughtExceptions(*this, program.fileName()); - if (_wantSignals) { - emit evaluationFinished(result, hadUncaughtException); - } + emit evaluationFinished(result, hadUncaughtException); return result; } @@ -801,9 +791,7 @@ void ScriptEngine::run() { } _isRunning = true; - if (_wantSignals) { - emit runningStateChanged(); - } + emit runningStateChanged(); QScriptValue result = evaluate(_scriptContents, _fileNameString); @@ -872,9 +860,7 @@ void ScriptEngine::run() { if (_lastUpdate < now) { float deltaTime = (float) (now - _lastUpdate) / (float) USECS_PER_SECOND; if (!_isFinished) { - if (_wantSignals) { - emit update(deltaTime); - } + emit update(deltaTime); } } _lastUpdate = now; @@ -884,9 +870,7 @@ void ScriptEngine::run() { } stopAllTimers(); // make sure all our timers are stopped if the script is ending - if (_wantSignals) { - emit scriptEnding(); - } + emit scriptEnding(); if (entityScriptingInterface->getEntityPacketSender()->serversExist()) { // release the queue of edit entity messages. @@ -904,15 +888,11 @@ void ScriptEngine::run() { } } - if (_wantSignals) { - emit finished(_fileNameString, this); - } + emit finished(_fileNameString, this); _isRunning = false; - if (_wantSignals) { - emit runningStateChanged(); - emit doneRunning(); - } + emit runningStateChanged(); + emit doneRunning(); } // NOTE: This is private because it must be called on the same thread that created the timers, which is why @@ -950,9 +930,7 @@ void ScriptEngine::stop() { return; } _isFinished = true; - if (_wantSignals) { - emit runningStateChanged(); - } + emit runningStateChanged(); } } @@ -1076,9 +1054,7 @@ QUrl ScriptEngine::resolvePath(const QString& include) const { } void ScriptEngine::print(const QString& message) { - if (_wantSignals) { - emit printedMessage(message); - } + emit printedMessage(message); } // If a callback is specified, the included files will be loaded asynchronously and the callback will be called @@ -1214,13 +1190,9 @@ void ScriptEngine::load(const QString& loadFile) { if (_isReloading) { auto scriptCache = DependencyManager::get(); scriptCache->deleteScript(url.toString()); - if (_wantSignals) { - emit reloadScript(url.toString(), false); - } + emit reloadScript(url.toString(), false); } else { - if (_wantSignals) { - emit loadScript(url.toString(), false); - } + emit loadScript(url.toString(), false); } } diff --git a/libraries/script-engine/src/ScriptEngine.h b/libraries/script-engine/src/ScriptEngine.h index d37e3eb177..cc23e57fda 100644 --- a/libraries/script-engine/src/ScriptEngine.h +++ b/libraries/script-engine/src/ScriptEngine.h @@ -67,9 +67,7 @@ public: class ScriptEngine : public QScriptEngine, public ScriptUser, public EntitiesScriptEngineProvider { Q_OBJECT public: - ScriptEngine(const QString& scriptContents = NO_SCRIPT, - const QString& fileNameString = QString(""), - bool wantSignals = true); + ScriptEngine(const QString& scriptContents = NO_SCRIPT, const QString& fileNameString = QString("")); ~ScriptEngine(); @@ -191,7 +189,6 @@ protected: bool _isInitialized { false }; QHash _timerFunctionMap; QSet _includedURLs; - bool _wantSignals { true }; QHash _entityScripts; bool _isThreaded { false }; QScriptEngineDebugger* _debugger { nullptr }; diff --git a/libraries/script-engine/src/ScriptEngines.cpp b/libraries/script-engine/src/ScriptEngines.cpp index 330a94cf0b..f91d7df9e6 100644 --- a/libraries/script-engine/src/ScriptEngines.cpp +++ b/libraries/script-engine/src/ScriptEngines.cpp @@ -428,7 +428,7 @@ ScriptEngine* ScriptEngines::loadScript(const QUrl& scriptFilename, bool isUserL return scriptEngine; } - scriptEngine = new ScriptEngine(NO_SCRIPT, "", true); + scriptEngine = new ScriptEngine(NO_SCRIPT, ""); scriptEngine->setUserLoaded(isUserLoaded); connect(scriptEngine, &ScriptEngine::doneRunning, this, [scriptEngine] { scriptEngine->deleteLater(); From 13d602487ff4c57c2b4b2b2da7be6ff2392d14e9 Mon Sep 17 00:00:00 2001 From: Zach Pomerantz Date: Thu, 12 May 2016 12:43:16 -0700 Subject: [PATCH 12/39] Remove guard over atomic in ScriptEngine::stop --- libraries/script-engine/src/ScriptEngine.cpp | 4 ---- 1 file changed, 4 deletions(-) diff --git a/libraries/script-engine/src/ScriptEngine.cpp b/libraries/script-engine/src/ScriptEngine.cpp index 53ffd19869..0ad5087478 100644 --- a/libraries/script-engine/src/ScriptEngine.cpp +++ b/libraries/script-engine/src/ScriptEngine.cpp @@ -925,10 +925,6 @@ void ScriptEngine::stopAllTimersForEntityScript(const EntityItemID& entityID) { void ScriptEngine::stop() { if (!_isFinished) { - if (QThread::currentThread() != thread()) { - QMetaObject::invokeMethod(this, "stop"); - return; - } _isFinished = true; emit runningStateChanged(); } From edf82c57ba1104773fdddbc96985fbd9a4a97053 Mon Sep 17 00:00:00 2001 From: Zach Pomerantz Date: Thu, 12 May 2016 12:45:02 -0700 Subject: [PATCH 13/39] Clean scripting thread deletion --- libraries/script-engine/src/ScriptEngine.cpp | 54 +++++++++++--------- 1 file changed, 30 insertions(+), 24 deletions(-) diff --git a/libraries/script-engine/src/ScriptEngine.cpp b/libraries/script-engine/src/ScriptEngine.cpp index 0ad5087478..a27dd7eb10 100644 --- a/libraries/script-engine/src/ScriptEngine.cpp +++ b/libraries/script-engine/src/ScriptEngine.cpp @@ -162,16 +162,15 @@ ScriptEngine::~ScriptEngine() { } else { qCWarning(scriptengine) << "Script destroyed after ScriptEngines!"; } - - waitTillDoneRunning(); } void ScriptEngine::disconnectNonEssentialSignals() { disconnect(); - QThread* receiver; + QThread* workerThread; // Ensure the thread should be running, and does exist - if (_isRunning && _isThreaded && (receiver = thread())) { - connect(this, &ScriptEngine::doneRunning, receiver, &QThread::quit); + if (_isRunning && _isThreaded && (workerThread = thread())) { + connect(this, &ScriptEngine::doneRunning, workerThread, &QThread::quit); + connect(workerThread, &QThread::finished, workerThread, &QObject::deleteLater); } } @@ -268,30 +267,33 @@ void ScriptEngine::runInThread() { } _isThreaded = true; - QThread* workerThread = new QThread(); - QString scriptEngineName = QString("Script Thread:") + getFilename(); - workerThread->setObjectName(scriptEngineName); + // The thread interface cannot live on itself, and we want to move this into the thread, so + // the thread cannot have this as a parent. + QThread* workerThread = new QThread(); + workerThread->setObjectName(QString("Script Thread:") + getFilename()); + moveToThread(workerThread); + // NOTE: If you connect any essential signals for proper shutdown or cleanup of // the script engine, make sure to add code to "reconnect" them to the // disconnectNonEssentialSignals() method - - // when the worker thread is started, call our engine's run.. connect(workerThread, &QThread::started, this, &ScriptEngine::run); - - // tell the thread to stop when the script engine is done connect(this, &ScriptEngine::doneRunning, workerThread, &QThread::quit); + connect(workerThread, &QThread::finished, workerThread, &QObject::deleteLater); - moveToThread(workerThread); - - // Starts an event loop, and emits workerThread->started() workerThread->start(); } void ScriptEngine::waitTillDoneRunning() { // If the script never started running or finished running before we got here, we don't need to wait for it auto workerThread = thread(); + if (_isThreaded && workerThread) { + // We should never be waiting (blocking) on our own thread + assert(workerThread != QThread::currentThread()); + + stop(); + QString scriptName = getFilename(); auto startedWaiting = usecTimestampNow(); @@ -301,24 +303,28 @@ void ScriptEngine::waitTillDoneRunning() { // the scripts will likely need to marshall messages across to the main thread, e.g. // if they access Settings or Menu in any of their shutdown code. So: // Process events for the main application thread, allowing invokeMethod calls to pass between threads. - QCoreApplication::processEvents(); // thread-safe :) + QCoreApplication::processEvents(); - // If we've been waiting a second or more, then tell the script engine to stop evaluating + // If the final evaluation takes too long, then tell the script engine to stop evaluating static const auto MAX_SCRIPT_EVALUATION_TIME = USECS_PER_SECOND; + static const auto MAX_SCRIPT_QUITTING_TIME = 0.5 * MAX_SCRIPT_EVALUATION_TIME; auto elapsedUsecs = usecTimestampNow() - startedWaiting; if (elapsedUsecs > MAX_SCRIPT_EVALUATION_TIME) { - qCDebug(scriptengine) << - "Script " << scriptName << " has been running too long [" << elapsedUsecs << " usecs] quitting."; - abortEvaluation(); // to allow the thread to quit - workerThread->quit(); - break; + qCDebug(scriptengine).nospace() << + "Script " << scriptName << " has been running too long [" << elapsedUsecs << "usecs] quitting."; + abortEvaluation(); // break out of current evaluation + + // Wait for the scripting thread to stop running + if (!workerThread->wait(MAX_SCRIPT_QUITTING_TIME)) { + qCWarning(scriptengine).nospace() << + "Script " << scriptName << " has been quitting too long [" << elapsedUsecs << "usecs] terminating."; + workerThread->terminate(); + } } // Avoid a pure busy wait QThread::yieldCurrentThread(); } - - workerThread->deleteLater(); } } From 991f7fe873564c4bbe10db504bcc35bb0478a864 Mon Sep 17 00:00:00 2001 From: David Rowe Date: Fri, 13 May 2016 10:20:50 +1200 Subject: [PATCH 14/39] Fix up syntax per JSLint --- scripts/system/users.js | 45 ++++++++++++++++++++++++----------------- 1 file changed, 27 insertions(+), 18 deletions(-) diff --git a/scripts/system/users.js b/scripts/system/users.js index 9612a19eee..55729726f1 100644 --- a/scripts/system/users.js +++ b/scripts/system/users.js @@ -9,7 +9,7 @@ // See the accompanying file LICENSE or http://www.apache.org/licenses/LICENSE-2.0.html // -var PopUpMenu = function(properties) { +var PopUpMenu = function (properties) { var value = properties.value, promptOverlay, valueOverlay, @@ -21,9 +21,8 @@ var PopUpMenu = function(properties) { MIN_MAX_BUTTON_SVG_WIDTH = 17.1, MIN_MAX_BUTTON_SVG_HEIGHT = 32.5, MIN_MAX_BUTTON_WIDTH = 14, - MIN_MAX_BUTTON_HEIGHT = MIN_MAX_BUTTON_WIDTH; - - MIN_MAX_BUTTON_SVG = Script.resolvePath("assets/images/tools/min-max-toggle.svg"); + MIN_MAX_BUTTON_HEIGHT = MIN_MAX_BUTTON_WIDTH, + MIN_MAX_BUTTON_SVG = Script.resolvePath("assets/images/tools/min-max-toggle.svg"); function positionDisplayOptions() { var y, @@ -218,11 +217,10 @@ var PopUpMenu = function(properties) { }; }; -var usersWindow = (function() { +var usersWindow = (function () { - var baseURL = Script.resolvePath("assets/images/tools/"); - - var WINDOW_WIDTH = 260, + var baseURL = Script.resolvePath("assets/images/tools/"), + WINDOW_WIDTH = 260, WINDOW_MARGIN = 12, WINDOW_BASE_MARGIN = 6, // A little less is needed in order look correct WINDOW_FONT = { @@ -383,7 +381,9 @@ var usersWindow = (function() { } // Reserve space for title, friends button, and option controls - nonUsersHeight = WINDOW_MARGIN + windowLineHeight + FRIENDS_BUTTON_SPACER + FRIENDS_BUTTON_HEIGHT + DISPLAY_SPACER + windowLineHeight + VISIBILITY_SPACER + windowLineHeight + WINDOW_BASE_MARGIN; + nonUsersHeight = WINDOW_MARGIN + windowLineHeight + FRIENDS_BUTTON_SPACER + FRIENDS_BUTTON_HEIGHT + DISPLAY_SPACER + + windowLineHeight + VISIBILITY_SPACER + + windowLineHeight + WINDOW_BASE_MARGIN; // Limit window to height of viewport minus VU meter and mirror if displayed windowHeight = linesOfUsers.length * windowLineHeight - windowLineSpacing + nonUsersHeight; @@ -422,12 +422,15 @@ var usersWindow = (function() { Overlays.editOverlay(scrollbarBackground, { y: scrollbarBackgroundPosition.y }); - scrollbarBarPosition.y = scrollbarBackgroundPosition.y + 1 + scrollbarValue * (scrollbarBackgroundHeight - scrollbarBarHeight - 2); + scrollbarBarPosition.y = scrollbarBackgroundPosition.y + 1 + + scrollbarValue * (scrollbarBackgroundHeight - scrollbarBarHeight - 2); Overlays.editOverlay(scrollbarBar, { y: scrollbarBarPosition.y }); - y = viewportHeight - FRIENDS_BUTTON_HEIGHT - DISPLAY_SPACER - windowLineHeight - VISIBILITY_SPACER - windowLineHeight - WINDOW_BASE_MARGIN; + y = viewportHeight - FRIENDS_BUTTON_HEIGHT - DISPLAY_SPACER + - windowLineHeight - VISIBILITY_SPACER + - windowLineHeight - WINDOW_BASE_MARGIN; Overlays.editOverlay(friendsButton, { y: y }); @@ -512,7 +515,7 @@ var usersWindow = (function() { usersRequest.send(); } - processUsers = function() { + processUsers = function () { var response, myUsername, user, @@ -565,7 +568,7 @@ var usersWindow = (function() { } }; - pollUsersTimedOut = function() { + pollUsersTimedOut = function () { print("Error: Request for users status timed out"); usersTimer = Script.setTimeout(pollUsers, HTTP_GET_TIMEOUT); // Try again after a longer delay. }; @@ -683,7 +686,8 @@ var usersWindow = (function() { userClicked = firstUserToDisplay + lineClicked; - if (0 <= userClicked && userClicked < linesOfUsers.length && 0 <= overlayX && overlayX <= usersOnline[linesOfUsers[userClicked]].textWidth) { + if (0 <= userClicked && userClicked < linesOfUsers.length && 0 <= overlayX + && overlayX <= usersOnline[linesOfUsers[userClicked]].textWidth) { //print("Go to " + usersOnline[linesOfUsers[userClicked]].username); location.goToUser(usersOnline[linesOfUsers[userClicked]].username); } @@ -740,8 +744,12 @@ var usersWindow = (function() { function onMouseMoveEvent(event) { if (isMovingScrollbar) { - if (scrollbarBackgroundPosition.x - WINDOW_MARGIN <= event.x && event.x <= scrollbarBackgroundPosition.x + SCROLLBAR_BACKGROUND_WIDTH + WINDOW_MARGIN && scrollbarBackgroundPosition.y - WINDOW_MARGIN <= event.y && event.y <= scrollbarBackgroundPosition.y + scrollbarBackgroundHeight + WINDOW_MARGIN) { - scrollbarValue = (event.y - scrollbarBarClickedAt * scrollbarBarHeight - scrollbarBackgroundPosition.y) / (scrollbarBackgroundHeight - scrollbarBarHeight - 2); + if (scrollbarBackgroundPosition.x - WINDOW_MARGIN <= event.x + && event.x <= scrollbarBackgroundPosition.x + SCROLLBAR_BACKGROUND_WIDTH + WINDOW_MARGIN + && scrollbarBackgroundPosition.y - WINDOW_MARGIN <= event.y + && event.y <= scrollbarBackgroundPosition.y + scrollbarBackgroundHeight + WINDOW_MARGIN) { + scrollbarValue = (event.y - scrollbarBarClickedAt * scrollbarBarHeight - scrollbarBackgroundPosition.y) + / (scrollbarBackgroundHeight - scrollbarBarHeight - 2); scrollbarValue = Math.min(Math.max(scrollbarValue, 0.0), 1.0); firstUserToDisplay = Math.floor(scrollbarValue * (linesOfUsers.length - numUsersToDisplay)); updateOverlayPositions(); @@ -773,7 +781,8 @@ var usersWindow = (function() { isMirrorDisplay = Menu.isOptionChecked(MIRROR_MENU_ITEM); isFullscreenMirror = Menu.isOptionChecked(FULLSCREEN_MIRROR_MENU_ITEM); - if (viewportHeight !== oldViewportHeight || isMirrorDisplay !== oldIsMirrorDisplay || isFullscreenMirror !== oldIsFullscreenMirror) { + if (viewportHeight !== oldViewportHeight || isMirrorDisplay !== oldIsMirrorDisplay + || isFullscreenMirror !== oldIsFullscreenMirror) { calculateWindowHeight(); updateUsersDisplay(); updateOverlayPositions(); @@ -991,4 +1000,4 @@ var usersWindow = (function() { setUp(); Script.scriptEnding.connect(tearDown); -}()); \ No newline at end of file +}()); From 54d658798aca0021278f368553f927eddfa15ca1 Mon Sep 17 00:00:00 2001 From: David Rowe Date: Fri, 13 May 2016 11:22:10 +1200 Subject: [PATCH 15/39] Add border overlay --- scripts/system/users.js | 42 ++++++++++++++++++++++++++++++++++++++++- 1 file changed, 41 insertions(+), 1 deletion(-) diff --git a/scripts/system/users.js b/scripts/system/users.js index 55729726f1..6b8040fae8 100644 --- a/scripts/system/users.js +++ b/scripts/system/users.js @@ -202,7 +202,7 @@ var PopUpMenu = function (properties) { width: MIN_MAX_BUTTON_SVG_WIDTH, height: MIN_MAX_BUTTON_SVG_HEIGHT / 2 }, - color: properties.buttonColor, + //color: properties.buttonColor, alpha: properties.buttonAlpha, visible: properties.visible }); @@ -246,6 +246,17 @@ var usersWindow = (function () { WINDOW_BACKGROUND_ALPHA = 0.8, windowPane, windowHeading, + + // Window border is similar to that of edit.js. + WINDOW_BORDER_WIDTH = WINDOW_WIDTH + 2 * WINDOW_BASE_MARGIN, + WINDOW_BORDER_TOP_MARGIN = 2 * WINDOW_BASE_MARGIN, + WINDOW_BORDER_BOTTOM_MARGIN = WINDOW_BASE_MARGIN, + WINDOW_BORDER_LEFT_MARGIN = WINDOW_BASE_MARGIN, + WINDOW_BORDER_RADIUS = 4, + WINDOW_BORDER_COLOR = { red: 255, green: 255, blue: 255 }, + WINDOW_BORDER_ALPHA = 0.5, + windowBorder, + MIN_MAX_BUTTON_SVG = baseURL + "min-max-toggle.svg", MIN_MAX_BUTTON_SVG_WIDTH = 17.1, MIN_MAX_BUTTON_SVG_HEIGHT = 32.5, @@ -329,6 +340,7 @@ var usersWindow = (function () { visibilityControl, windowHeight, + windowBorderHeight, windowTextHeight, windowLineSpacing, windowLineHeight, // = windowTextHeight + windowLineSpacing @@ -357,6 +369,7 @@ var usersWindow = (function () { isVisible = true, isMinimized = false, + isBorderVisible = false, viewportHeight, isMirrorDisplay = false, @@ -377,6 +390,7 @@ var usersWindow = (function () { if (isMinimized) { windowHeight = windowTextHeight + WINDOW_MARGIN + WINDOW_BASE_MARGIN; + windowBorderHeight = windowHeight + WINDOW_BORDER_TOP_MARGIN + WINDOW_BORDER_BOTTOM_MARGIN; return; } @@ -392,6 +406,7 @@ var usersWindow = (function () { maxWindowHeight -= MIRROR_HEIGHT; } windowHeight = Math.max(Math.min(windowHeight, maxWindowHeight), nonUsersHeight); + windowBorderHeight = windowHeight + WINDOW_BORDER_TOP_MARGIN + WINDOW_BORDER_BOTTOM_MARGIN; // Corresponding number of users to actually display numUsersToDisplay = Math.max(Math.round((windowHeight - nonUsersHeight) / windowLineHeight), 0); @@ -407,6 +422,9 @@ var usersWindow = (function () { function updateOverlayPositions() { var y; + Overlays.editOverlay(windowBorder, { + y: viewportHeight - windowHeight - WINDOW_BORDER_TOP_MARGIN + }); Overlays.editOverlay(windowPane, { y: viewportHeight - windowHeight }); @@ -490,6 +508,10 @@ var usersWindow = (function () { }); } + Overlays.editOverlay(windowBorder, { + height: windowBorderHeight + }); + Overlays.editOverlay(windowPane, { height: windowHeight, text: displayText @@ -574,6 +596,10 @@ var usersWindow = (function () { }; function updateOverlayVisibility() { + // TODO + //Overlays.editOverlay(windowBorder, { + // visible: isVisible && isBorderVisible + //}); Overlays.editOverlay(windowPane, { visible: isVisible }); @@ -805,6 +831,19 @@ var usersWindow = (function () { calculateWindowHeight(); + windowBorder = Overlays.addOverlay("rectangle", { + x: -WINDOW_BORDER_LEFT_MARGIN, + y: viewportHeight, + width: WINDOW_BORDER_WIDTH, + height: windowBorderHeight, + radius: WINDOW_BORDER_RADIUS, + color: WINDOW_BORDER_COLOR, + alpha: WINDOW_BORDER_ALPHA, + // TODO + //visible: isVisible && isBorderVisible + visible: true + }); + windowPane = Overlays.addOverlay("text", { x: 0, y: viewportHeight, // Start up off-screen @@ -988,6 +1027,7 @@ var usersWindow = (function () { Menu.removeMenuItem(MENU_NAME, MENU_ITEM); Script.clearTimeout(usersTimer); + Overlays.deleteOverlay(windowBorder); Overlays.deleteOverlay(windowPane); Overlays.deleteOverlay(windowHeading); Overlays.deleteOverlay(minimizeButton); From aa226739d31e0fc5b216d39e36cd5786227189ae Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Sat, 30 Apr 2016 19:41:04 -0700 Subject: [PATCH 16/39] remove billboard code from Agent --- assignment-client/src/Agent.cpp | 20 +------------------- assignment-client/src/Agent.h | 2 -- 2 files changed, 1 insertion(+), 21 deletions(-) diff --git a/assignment-client/src/Agent.cpp b/assignment-client/src/Agent.cpp index f8f0f7904a..65e193dec6 100644 --- a/assignment-client/src/Agent.cpp +++ b/assignment-client/src/Agent.cpp @@ -290,7 +290,6 @@ void Agent::executeScript() { packetReceiver.registerListener(PacketType::BulkAvatarData, avatarHashMap.data(), "processAvatarDataPacket"); packetReceiver.registerListener(PacketType::KillAvatar, avatarHashMap.data(), "processKillAvatar"); packetReceiver.registerListener(PacketType::AvatarIdentity, avatarHashMap.data(), "processAvatarIdentityPacket"); - packetReceiver.registerListener(PacketType::AvatarBillboard, avatarHashMap.data(), "processAvatarBillboardPacket"); // register ourselves to the script engine _scriptEngine->registerGlobalObject("Agent", this); @@ -341,15 +340,12 @@ void Agent::setIsAvatar(bool isAvatar) { if (_isAvatar && !_avatarIdentityTimer) { // set up the avatar timers _avatarIdentityTimer = new QTimer(this); - _avatarBillboardTimer = new QTimer(this); // connect our slot connect(_avatarIdentityTimer, &QTimer::timeout, this, &Agent::sendAvatarIdentityPacket); - connect(_avatarBillboardTimer, &QTimer::timeout, this, &Agent::sendAvatarBillboardPacket); // start the timers _avatarIdentityTimer->start(AVATAR_IDENTITY_PACKET_SEND_INTERVAL_MSECS); - _avatarBillboardTimer->start(AVATAR_BILLBOARD_PACKET_SEND_INTERVAL_MSECS); } if (!_isAvatar) { @@ -359,12 +355,6 @@ void Agent::setIsAvatar(bool isAvatar) { delete _avatarIdentityTimer; _avatarIdentityTimer = nullptr; } - - if (_avatarBillboardTimer) { - _avatarBillboardTimer->stop(); - delete _avatarBillboardTimer; - _avatarBillboardTimer = nullptr; - } } } @@ -375,14 +365,6 @@ void Agent::sendAvatarIdentityPacket() { } } -void Agent::sendAvatarBillboardPacket() { - if (_isAvatar) { - auto scriptedAvatar = DependencyManager::get(); - scriptedAvatar->sendBillboardPacket(); - } -} - - void Agent::processAgentAvatarAndAudio(float deltaTime) { if (!_scriptEngine->isFinished() && _isAvatar) { auto scriptedAvatar = DependencyManager::get(); @@ -491,7 +473,7 @@ void Agent::processAgentAvatarAndAudio(float deltaTime) { } void Agent::aboutToFinish() { - setIsAvatar(false);// will stop timers for sending billboards and identity packets + setIsAvatar(false);// will stop timers for sending identity packets if (_scriptEngine) { _scriptEngine->stop(); diff --git a/assignment-client/src/Agent.h b/assignment-client/src/Agent.h index 2b0d22385d..63d4cfa4d6 100644 --- a/assignment-client/src/Agent.h +++ b/assignment-client/src/Agent.h @@ -82,7 +82,6 @@ private: void setAvatarSound(SharedSoundPointer avatarSound) { _avatarSound = avatarSound; } void sendAvatarIdentityPacket(); - void sendAvatarBillboardPacket(); QString _scriptContents; QTimer* _scriptRequestTimeout { nullptr }; @@ -92,7 +91,6 @@ private: int _numAvatarSoundSentBytes = 0; bool _isAvatar = false; QTimer* _avatarIdentityTimer = nullptr; - QTimer* _avatarBillboardTimer = nullptr; QHash _outgoingScriptAudioSequenceNumbers; }; From 2cf943b2b16e9ed618d8e3704b8bde763388b35f Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Sat, 30 Apr 2016 19:44:00 -0700 Subject: [PATCH 17/39] remove billboard code from AvatarData --- interface/src/avatar/AvatarManager.cpp | 1 - libraries/avatars/src/AvatarData.cpp | 50 ------------------------- libraries/avatars/src/AvatarData.h | 15 -------- libraries/avatars/src/AvatarHashMap.cpp | 11 ------ libraries/avatars/src/AvatarHashMap.h | 1 - 5 files changed, 78 deletions(-) diff --git a/interface/src/avatar/AvatarManager.cpp b/interface/src/avatar/AvatarManager.cpp index 41bcc0332a..ddadcb3909 100644 --- a/interface/src/avatar/AvatarManager.cpp +++ b/interface/src/avatar/AvatarManager.cpp @@ -73,7 +73,6 @@ AvatarManager::AvatarManager(QObject* parent) : packetReceiver.registerListener(PacketType::BulkAvatarData, this, "processAvatarDataPacket"); packetReceiver.registerListener(PacketType::KillAvatar, this, "processKillAvatar"); packetReceiver.registerListener(PacketType::AvatarIdentity, this, "processAvatarIdentityPacket"); - packetReceiver.registerListener(PacketType::AvatarBillboard, this, "processAvatarBillboardPacket"); } AvatarManager::~AvatarManager() { diff --git a/libraries/avatars/src/AvatarData.cpp b/libraries/avatars/src/AvatarData.cpp index fd13f8c370..a44d1715c9 100644 --- a/libraries/avatars/src/AvatarData.cpp +++ b/libraries/avatars/src/AvatarData.cpp @@ -58,7 +58,6 @@ AvatarData::AvatarData() : _headData(NULL), _displayNameTargetAlpha(1.0f), _displayNameAlpha(1.0f), - _billboard(), _errorLogExpiry(0), _owningAvatarMixer(), _targetVelocity(0.0f), @@ -999,13 +998,6 @@ QByteArray AvatarData::identityByteArray() { return identityData; } -bool AvatarData::hasBillboardChangedAfterParsing(const QByteArray& data) { - if (data == _billboard) { - return false; - } - _billboard = data; - return true; -} void AvatarData::setSkeletonModelURL(const QUrl& skeletonModelURL) { const QUrl& expanded = skeletonModelURL.isEmpty() ? AvatarData::defaultFullAvatarModelUrl() : skeletonModelURL; @@ -1103,33 +1095,6 @@ void AvatarData::detachAll(const QString& modelURL, const QString& jointName) { setAttachmentData(attachmentData); } -void AvatarData::setBillboard(const QByteArray& billboard) { - _billboard = billboard; - - qCDebug(avatars) << "Changing billboard for avatar."; -} - -void AvatarData::setBillboardFromURL(const QString &billboardURL) { - _billboardURL = billboardURL; - - - qCDebug(avatars) << "Changing billboard for avatar to PNG at" << qPrintable(billboardURL); - - QNetworkRequest billboardRequest; - billboardRequest.setHeader(QNetworkRequest::UserAgentHeader, HIGH_FIDELITY_USER_AGENT); - billboardRequest.setUrl(QUrl(billboardURL)); - - QNetworkAccessManager& networkAccessManager = NetworkAccessManager::getInstance(); - QNetworkReply* networkReply = networkAccessManager.get(billboardRequest); - connect(networkReply, SIGNAL(finished()), this, SLOT(setBillboardFromNetworkReply())); -} - -void AvatarData::setBillboardFromNetworkReply() { - QNetworkReply* networkReply = static_cast(sender()); - setBillboard(networkReply->readAll()); - networkReply->deleteLater(); -} - void AvatarData::setJointMappingsFromNetworkReply() { QNetworkReply* networkReply = static_cast(sender()); @@ -1204,21 +1169,6 @@ void AvatarData::sendIdentityPacket() { }); } -void AvatarData::sendBillboardPacket() { - if (!_billboard.isEmpty()) { - auto nodeList = DependencyManager::get(); - - // This makes sure the billboard won't be too large to send. - // Once more protocol changes are done and we can send blocks of data we can support sending > MTU sized billboards. - if (_billboard.size() <= NLPacket::maxPayloadSize(PacketType::AvatarBillboard)) { - auto billboardPacket = NLPacket::create(PacketType::AvatarBillboard, _billboard.size()); - billboardPacket->write(_billboard); - - nodeList->broadcastToNodes(std::move(billboardPacket), NodeSet() << NodeType::AvatarMixer); - } - } -} - void AvatarData::updateJointMappings() { _jointIndices.clear(); _jointNames.clear(); diff --git a/libraries/avatars/src/AvatarData.h b/libraries/avatars/src/AvatarData.h index 900da38ffa..a7b97ef4c0 100644 --- a/libraries/avatars/src/AvatarData.h +++ b/libraries/avatars/src/AvatarData.h @@ -107,7 +107,6 @@ static const float MIN_AVATAR_SCALE = .005f; const float MAX_AUDIO_LOUDNESS = 1000.0f; // close enough for mouth animation const int AVATAR_IDENTITY_PACKET_SEND_INTERVAL_MSECS = 1000; -const int AVATAR_BILLBOARD_PACKET_SEND_INTERVAL_MSECS = 5000; // See also static AvatarData::defaultFullAvatarModelUrl(). const QString DEFAULT_FULL_AVATAR_MODEL_NAME = QString("Default"); @@ -160,7 +159,6 @@ class AvatarData : public QObject, public SpatiallyNestable { Q_PROPERTY(QString displayName READ getDisplayName WRITE setDisplayName) Q_PROPERTY(QString skeletonModelURL READ getSkeletonModelURLFromScript WRITE setSkeletonModelURLFromScript) Q_PROPERTY(QVector attachmentData READ getAttachmentData WRITE setAttachmentData) - Q_PROPERTY(QString billboardURL READ getBillboardURL WRITE setBillboardFromURL) Q_PROPERTY(QStringList jointNames READ getJointNames) @@ -285,8 +283,6 @@ public: bool hasIdentityChangedAfterParsing(const QByteArray& data); QByteArray identityByteArray(); - bool hasBillboardChangedAfterParsing(const QByteArray& data); - const QUrl& getSkeletonModelURL() const { return _skeletonModelURL; } const QString& getDisplayName() const { return _displayName; } virtual void setSkeletonModelURL(const QUrl& skeletonModelURL); @@ -304,12 +300,6 @@ public: Q_INVOKABLE void detachOne(const QString& modelURL, const QString& jointName = QString()); Q_INVOKABLE void detachAll(const QString& modelURL, const QString& jointName = QString()); - virtual void setBillboard(const QByteArray& billboard); - const QByteArray& getBillboard() const { return _billboard; } - - void setBillboardFromURL(const QString& billboardURL); - const QString& getBillboardURL() { return _billboardURL; } - QString getSkeletonModelURLFromScript() const { return _skeletonModelURL.toString(); } void setSkeletonModelURLFromScript(const QString& skeletonModelString) { setSkeletonModelURL(QUrl(skeletonModelString)); } @@ -336,9 +326,7 @@ public: public slots: void sendAvatarDataPacket(); void sendIdentityPacket(); - void sendBillboardPacket(); - void setBillboardFromNetworkReply(); void setJointMappingsFromNetworkReply(); void setSessionUUID(const QUuid& sessionUUID) { setID(sessionUUID); } @@ -377,9 +365,6 @@ protected: float _displayNameTargetAlpha; float _displayNameAlpha; - QByteArray _billboard; - QString _billboardURL; - QHash _jointIndices; ///< 1-based, since zero is returned for missing keys QStringList _jointNames; ///< in order of depth-first traversal diff --git a/libraries/avatars/src/AvatarHashMap.cpp b/libraries/avatars/src/AvatarHashMap.cpp index 75fb5e6028..f14e2b0ad3 100644 --- a/libraries/avatars/src/AvatarHashMap.cpp +++ b/libraries/avatars/src/AvatarHashMap.cpp @@ -136,17 +136,6 @@ void AvatarHashMap::processAvatarIdentityPacket(QSharedPointer } } -void AvatarHashMap::processAvatarBillboardPacket(QSharedPointer message, SharedNodePointer sendingNode) { - QUuid sessionUUID = QUuid::fromRfc4122(message->readWithoutCopy(NUM_BYTES_RFC4122_UUID)); - - auto avatar = newOrExistingAvatar(sessionUUID, sendingNode); - - QByteArray billboard = message->read(message->getBytesLeftToRead()); - if (avatar->getBillboard() != billboard) { - avatar->setBillboard(billboard); - } -} - void AvatarHashMap::processKillAvatar(QSharedPointer message, SharedNodePointer sendingNode) { // read the node id QUuid sessionUUID = QUuid::fromRfc4122(message->readWithoutCopy(NUM_BYTES_RFC4122_UUID)); diff --git a/libraries/avatars/src/AvatarHashMap.h b/libraries/avatars/src/AvatarHashMap.h index ee1197367c..5f58074427 100644 --- a/libraries/avatars/src/AvatarHashMap.h +++ b/libraries/avatars/src/AvatarHashMap.h @@ -53,7 +53,6 @@ private slots: void processAvatarDataPacket(QSharedPointer message, SharedNodePointer sendingNode); void processAvatarIdentityPacket(QSharedPointer message, SharedNodePointer sendingNode); - void processAvatarBillboardPacket(QSharedPointer message, SharedNodePointer sendingNode); void processKillAvatar(QSharedPointer message, SharedNodePointer sendingNode); protected: From 2cc788f98dce5d0f53e162edffb49a5d3e5ff419 Mon Sep 17 00:00:00 2001 From: Zach Pomerantz Date: Thu, 12 May 2016 12:45:35 -0700 Subject: [PATCH 18/39] Rename ScriptEngine::wait to match std threading --- libraries/script-engine/src/ScriptEngine.cpp | 3 +-- libraries/script-engine/src/ScriptEngine.h | 5 ++++- libraries/script-engine/src/ScriptEngines.cpp | 5 +++-- 3 files changed, 8 insertions(+), 5 deletions(-) diff --git a/libraries/script-engine/src/ScriptEngine.cpp b/libraries/script-engine/src/ScriptEngine.cpp index a27dd7eb10..c52848ad11 100644 --- a/libraries/script-engine/src/ScriptEngine.cpp +++ b/libraries/script-engine/src/ScriptEngine.cpp @@ -284,8 +284,7 @@ void ScriptEngine::runInThread() { workerThread->start(); } -void ScriptEngine::waitTillDoneRunning() { - // If the script never started running or finished running before we got here, we don't need to wait for it +void ScriptEngine::wait() { auto workerThread = thread(); if (_isThreaded && workerThread) { diff --git a/libraries/script-engine/src/ScriptEngine.h b/libraries/script-engine/src/ScriptEngine.h index cc23e57fda..2c0812210c 100644 --- a/libraries/script-engine/src/ScriptEngine.h +++ b/libraries/script-engine/src/ScriptEngine.h @@ -197,7 +197,10 @@ protected: void init(); QString getFilename() const; - void waitTillDoneRunning(); + + // Stop any evaluating scripts and wait for the scripting thread to finish. + void wait(); + bool evaluatePending() const { return _evaluatesPending > 0; } void timerFired(); void stopAllTimers(); diff --git a/libraries/script-engine/src/ScriptEngines.cpp b/libraries/script-engine/src/ScriptEngines.cpp index f91d7df9e6..ba18a5019b 100644 --- a/libraries/script-engine/src/ScriptEngines.cpp +++ b/libraries/script-engine/src/ScriptEngines.cpp @@ -149,6 +149,7 @@ void ScriptEngines::shutdownScripting() { // NOTE: typically all script engines are running. But there's at least one known exception to this, the // "entities sandbox" which is only used to evaluate entities scripts to test their validity before using // them. We don't need to stop scripts that aren't running. + // TODO: Scripts could be shut down faster if we spread them across a threadpool. if (scriptEngine->isRunning()) { qCDebug(scriptengine) << "about to shutdown script:" << scriptName; @@ -165,12 +166,12 @@ void ScriptEngines::shutdownScripting() { // want any of the scripts final "scriptEnding()" or pending "update()" methods from accessing // any application state after we leave this stopAllScripts() method qCDebug(scriptengine) << "waiting on script:" << scriptName; - scriptEngine->waitTillDoneRunning(); + scriptEngine->wait(); qCDebug(scriptengine) << "done waiting on script:" << scriptName; scriptEngine->deleteLater(); - // If the script is stopped, we can remove it from our set + // Once the script is stopped, we can remove it from our set i.remove(); } } From f40fe88ee7275b5008da01085aec084fb2e72f52 Mon Sep 17 00:00:00 2001 From: Zach Pomerantz Date: Thu, 12 May 2016 13:04:49 -0700 Subject: [PATCH 19/39] Clean up entity script engine deletion --- .../src/EntityTreeRenderer.cpp | 25 ++++++++++--------- .../src/EntityTreeRenderer.h | 2 +- libraries/script-engine/src/ScriptEngines.cpp | 6 +++-- 3 files changed, 18 insertions(+), 15 deletions(-) diff --git a/libraries/entities-renderer/src/EntityTreeRenderer.cpp b/libraries/entities-renderer/src/EntityTreeRenderer.cpp index 814faa8874..1dfe2ab53c 100644 --- a/libraries/entities-renderer/src/EntityTreeRenderer.cpp +++ b/libraries/entities-renderer/src/EntityTreeRenderer.cpp @@ -77,9 +77,17 @@ EntityTreeRenderer::~EntityTreeRenderer() { int EntityTreeRenderer::_entitiesScriptEngineCount = 0; -void EntityTreeRenderer::setupEntitiesScriptEngine() { - QSharedPointer oldEngine = _entitiesScriptEngine; // save the old engine through this function, so the EntityScriptingInterface doesn't have problems with it. - _entitiesScriptEngine = QSharedPointer(new ScriptEngine(NO_SCRIPT, QString("Entities %1").arg(++_entitiesScriptEngineCount)), &QObject::deleteLater); +void EntityTreeRenderer::resetEntitiesScriptEngine() { + // Keep a ref to oldEngine until newEngine is ready so EntityScriptingInterface has something to use + auto oldEngine = _entitiesScriptEngine; + + auto newEngine = new ScriptEngine(NO_SCRIPT, QString("Entities %1").arg(++_entitiesScriptEngineCount)); + _entitiesScriptEngine = QSharedPointer(newEngine, [](ScriptEngine* engine){ + engine->unloadAllEntityScripts(); + engine->stop(); + engine->deleteLater(); + }); + _scriptingServices->registerScriptEngineWithApplicationServices(_entitiesScriptEngine.data()); _entitiesScriptEngine->runInThread(); DependencyManager::get()->setEntitiesScriptEngine(_entitiesScriptEngine.data()); @@ -87,16 +95,9 @@ void EntityTreeRenderer::setupEntitiesScriptEngine() { void EntityTreeRenderer::clear() { leaveAllEntities(); - if (_entitiesScriptEngine) { - _entitiesScriptEngine->unloadAllEntityScripts(); - _entitiesScriptEngine->stop(); - } if (_wantScripts && !_shuttingDown) { - // NOTE: you can't actually need to delete it here because when we call setupEntitiesScriptEngine it will - // assign a new instance to our shared pointer, which will deref the old instance and ultimately call - // the custom deleter which calls deleteLater - setupEntitiesScriptEngine(); + resetEntitiesScriptEngine(); } auto scene = _viewState->getMain3DScene(); @@ -125,7 +126,7 @@ void EntityTreeRenderer::init() { entityTree->setFBXService(this); if (_wantScripts) { - setupEntitiesScriptEngine(); + resetEntitiesScriptEngine(); } forceRecheckEntities(); // setup our state to force checking our inside/outsideness of entities diff --git a/libraries/entities-renderer/src/EntityTreeRenderer.h b/libraries/entities-renderer/src/EntityTreeRenderer.h index a6fc58e5f1..5c06c5f5cc 100644 --- a/libraries/entities-renderer/src/EntityTreeRenderer.h +++ b/libraries/entities-renderer/src/EntityTreeRenderer.h @@ -126,7 +126,7 @@ protected: } private: - void setupEntitiesScriptEngine(); + void resetEntitiesScriptEngine(); void addEntityToScene(EntityItemPointer entity); bool findBestZoneAndMaybeContainingEntities(const glm::vec3& avatarPosition, QVector* entitiesContainingAvatar); diff --git a/libraries/script-engine/src/ScriptEngines.cpp b/libraries/script-engine/src/ScriptEngines.cpp index ba18a5019b..6e7c2bb839 100644 --- a/libraries/script-engine/src/ScriptEngines.cpp +++ b/libraries/script-engine/src/ScriptEngines.cpp @@ -158,8 +158,10 @@ void ScriptEngines::shutdownScripting() { // and stop. We can safely short circuit this because we know we're in the "quitting" process scriptEngine->disconnect(this); - // Calling stop on the script engine will set it's internal _isFinished state to true, and result - // in the ScriptEngine gracefully ending it's run() method. + // If this is an entity script, we need to unload any entities + scriptEngine->unloadAllEntityScripts(); + + // Gracefully stop the engine's scripting thread scriptEngine->stop(); // We need to wait for the engine to be done running before we proceed, because we don't From f2f89ca062c50fcb187c211e3b9ce8179946ab85 Mon Sep 17 00:00:00 2001 From: Zach Pomerantz Date: Thu, 12 May 2016 13:39:43 -0700 Subject: [PATCH 20/39] Add logging to ScriptEngine lifetime --- libraries/script-engine/src/ScriptEngine.cpp | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/libraries/script-engine/src/ScriptEngine.cpp b/libraries/script-engine/src/ScriptEngine.cpp index c52848ad11..6ce8c56eb0 100644 --- a/libraries/script-engine/src/ScriptEngine.cpp +++ b/libraries/script-engine/src/ScriptEngine.cpp @@ -154,7 +154,7 @@ ScriptEngine::ScriptEngine(const QString& scriptContents, const QString& fileNam } ScriptEngine::~ScriptEngine() { - qCDebug(scriptengine) << "Script Engine shutting down (destructor) for script:" << getFilename(); + qCDebug(scriptengine) << "Script Engine shutting down:" << getFilename(); auto scriptEngines = DependencyManager::get(); if (scriptEngines) { @@ -291,11 +291,10 @@ void ScriptEngine::wait() { // We should never be waiting (blocking) on our own thread assert(workerThread != QThread::currentThread()); + // Engine should be stopped already, but be defensive stop(); - QString scriptName = getFilename(); auto startedWaiting = usecTimestampNow(); - while (workerThread->isRunning()) { // NOTE: This will be called on the main application thread from stopAllScripts. // The application thread will need to continue to process events, because @@ -310,13 +309,13 @@ void ScriptEngine::wait() { auto elapsedUsecs = usecTimestampNow() - startedWaiting; if (elapsedUsecs > MAX_SCRIPT_EVALUATION_TIME) { qCDebug(scriptengine).nospace() << - "Script " << scriptName << " has been running too long [" << elapsedUsecs << "usecs] quitting."; + "Script Engine has been running too long, aborting:" << getFilename(); abortEvaluation(); // break out of current evaluation // Wait for the scripting thread to stop running if (!workerThread->wait(MAX_SCRIPT_QUITTING_TIME)) { qCWarning(scriptengine).nospace() << - "Script " << scriptName << " has been quitting too long [" << elapsedUsecs << "usecs] terminating."; + "Script Engine has been aborting too long, terminating:" << getFilename(); workerThread->terminate(); } } @@ -324,6 +323,8 @@ void ScriptEngine::wait() { // Avoid a pure busy wait QThread::yieldCurrentThread(); } + + qCDebug(scriptengine) << "Script Engine has stopped:" << getFilename(); } } @@ -874,6 +875,8 @@ void ScriptEngine::run() { hadUncaughtExceptions(*this, _fileNameString); } + qCDebug(scriptengine) << "Script Engine stopping:" << getFilename(); + stopAllTimers(); // make sure all our timers are stopped if the script is ending emit scriptEnding(); From 7e82494a663b63d19f43caf2d1e8d1f42ab70fc5 Mon Sep 17 00:00:00 2001 From: Zach Pomerantz Date: Thu, 12 May 2016 13:40:10 -0700 Subject: [PATCH 21/39] Add cap on entities scripting thread stop time --- libraries/entities-renderer/src/EntityTreeRenderer.cpp | 5 +++++ libraries/script-engine/src/ScriptEngine.cpp | 1 - libraries/script-engine/src/ScriptEngine.h | 3 +++ 3 files changed, 8 insertions(+), 1 deletion(-) diff --git a/libraries/entities-renderer/src/EntityTreeRenderer.cpp b/libraries/entities-renderer/src/EntityTreeRenderer.cpp index 1dfe2ab53c..c73a7ae6c7 100644 --- a/libraries/entities-renderer/src/EntityTreeRenderer.cpp +++ b/libraries/entities-renderer/src/EntityTreeRenderer.cpp @@ -83,8 +83,13 @@ void EntityTreeRenderer::resetEntitiesScriptEngine() { auto newEngine = new ScriptEngine(NO_SCRIPT, QString("Entities %1").arg(++_entitiesScriptEngineCount)); _entitiesScriptEngine = QSharedPointer(newEngine, [](ScriptEngine* engine){ + // Gracefully exit engine->unloadAllEntityScripts(); engine->stop(); + + // Disgracefully exit, if necessary + QTimer::singleShot(ScriptEngine::MAX_SCRIPT_EVALUATION_TIME, engine, &ScriptEngine::abort); + engine->deleteLater(); }); diff --git a/libraries/script-engine/src/ScriptEngine.cpp b/libraries/script-engine/src/ScriptEngine.cpp index 6ce8c56eb0..dab921e962 100644 --- a/libraries/script-engine/src/ScriptEngine.cpp +++ b/libraries/script-engine/src/ScriptEngine.cpp @@ -304,7 +304,6 @@ void ScriptEngine::wait() { QCoreApplication::processEvents(); // If the final evaluation takes too long, then tell the script engine to stop evaluating - static const auto MAX_SCRIPT_EVALUATION_TIME = USECS_PER_SECOND; static const auto MAX_SCRIPT_QUITTING_TIME = 0.5 * MAX_SCRIPT_EVALUATION_TIME; auto elapsedUsecs = usecTimestampNow() - startedWaiting; if (elapsedUsecs > MAX_SCRIPT_EVALUATION_TIME) { diff --git a/libraries/script-engine/src/ScriptEngine.h b/libraries/script-engine/src/ScriptEngine.h index 2c0812210c..bb4814eb94 100644 --- a/libraries/script-engine/src/ScriptEngine.h +++ b/libraries/script-engine/src/ScriptEngine.h @@ -67,6 +67,8 @@ public: class ScriptEngine : public QScriptEngine, public ScriptUser, public EntitiesScriptEngineProvider { Q_OBJECT public: + static const auto MAX_SCRIPT_EVALUATION_TIME = USECS_PER_SECOND; + ScriptEngine(const QString& scriptContents = NO_SCRIPT, const QString& fileNameString = QString("")); ~ScriptEngine(); @@ -164,6 +166,7 @@ public: public slots: void callAnimationStateHandler(QScriptValue callback, AnimVariantMap parameters, QStringList names, bool useNames, AnimVariantResultHandler resultHandler); void updateMemoryCost(const qint64&); + void abort() { abortEvaluation(); } signals: void scriptLoaded(const QString& scriptFilename); From 1107882be288f8be23a27892c7087e108b89be0f Mon Sep 17 00:00:00 2001 From: Zach Pomerantz Date: Thu, 12 May 2016 17:48:14 -0700 Subject: [PATCH 22/39] Throw to stop non-evaluating scripts --- libraries/script-engine/src/ScriptEngine.cpp | 24 +++++++++++++------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/libraries/script-engine/src/ScriptEngine.cpp b/libraries/script-engine/src/ScriptEngine.cpp index dab921e962..b5077db240 100644 --- a/libraries/script-engine/src/ScriptEngine.cpp +++ b/libraries/script-engine/src/ScriptEngine.cpp @@ -304,17 +304,25 @@ void ScriptEngine::wait() { QCoreApplication::processEvents(); // If the final evaluation takes too long, then tell the script engine to stop evaluating - static const auto MAX_SCRIPT_QUITTING_TIME = 0.5 * MAX_SCRIPT_EVALUATION_TIME; auto elapsedUsecs = usecTimestampNow() - startedWaiting; if (elapsedUsecs > MAX_SCRIPT_EVALUATION_TIME) { - qCDebug(scriptengine).nospace() << - "Script Engine has been running too long, aborting:" << getFilename(); - abortEvaluation(); // break out of current evaluation + workerThread->quit(); - // Wait for the scripting thread to stop running - if (!workerThread->wait(MAX_SCRIPT_QUITTING_TIME)) { - qCWarning(scriptengine).nospace() << - "Script Engine has been aborting too long, terminating:" << getFilename(); + if (isEvaluating()) { + qCWarning(scriptengine) << "Script Engine has been running too long, aborting:" << getFilename(); + abortEvaluation(); + } else { + qCWarning(scriptengine) << "Script Engine has been running too long, throwing:" << getFilename(); + auto context = currentContext(); + if (context) { + context->throwError("Timed out during shutdown"); + } + } + + // Wait for the scripting thread to stop running, as + // flooding it with aborts/exceptions will persist it longer + static const auto MAX_SCRIPT_QUITTING_TIME = 0.5 * MSECS_PER_SECOND; + if (workerThread->wait(MAX_SCRIPT_QUITTING_TIME)) { workerThread->terminate(); } } From e1c130d02fe9fcb22e7c47dae143b6f2c3392a2f Mon Sep 17 00:00:00 2001 From: Zach Pomerantz Date: Thu, 12 May 2016 17:48:34 -0700 Subject: [PATCH 23/39] Timeout long sandbox scripts --- libraries/script-engine/src/ScriptEngine.cpp | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/libraries/script-engine/src/ScriptEngine.cpp b/libraries/script-engine/src/ScriptEngine.cpp index b5077db240..6ed7f7f684 100644 --- a/libraries/script-engine/src/ScriptEngine.cpp +++ b/libraries/script-engine/src/ScriptEngine.cpp @@ -1292,8 +1292,23 @@ void ScriptEngine::entityScriptContentAvailable(const EntityItemID& entityID, co setParentURL(scriptOrURL); } + const int SANDBOX_TIMEOUT = 0.25 * MSECS_PER_SECOND; QScriptEngine sandbox; - QScriptValue testConstructor = sandbox.evaluate(program); + sandbox.setProcessEventsInterval(SANDBOX_TIMEOUT); + QScriptValue testConstructor; + { + QTimer timeout; + timeout.setSingleShot(true); + timeout.start(SANDBOX_TIMEOUT); + connect(&timeout, &QTimer::timeout, [&sandbox, SANDBOX_TIMEOUT]{ + auto context = sandbox.currentContext(); + if (context) { + // Guard against infinite loops and non-performant code + context->throwError(QString("Timed out (entity constructors are limited to %1ms)").arg(SANDBOX_TIMEOUT)); + } + }); + testConstructor = sandbox.evaluate(program); + } if (hadUncaughtExceptions(sandbox, program.fileName())) { return; } From 93ead174aaf04d45e8b11926f4e5172e70859112 Mon Sep 17 00:00:00 2001 From: David Rowe Date: Fri, 13 May 2016 12:51:39 +1200 Subject: [PATCH 24/39] Click and drag border to move users window --- scripts/system/users.js | 122 +++++++++++++++++++++++++++++----------- 1 file changed, 90 insertions(+), 32 deletions(-) diff --git a/scripts/system/users.js b/scripts/system/users.js index 6b8040fae8..420f6a6bfe 100644 --- a/scripts/system/users.js +++ b/scripts/system/users.js @@ -375,6 +375,10 @@ var usersWindow = (function () { isMirrorDisplay = false, isFullscreenMirror = false, + windowPosition = { }, // Bottom left corner of window pane. + isMovingWindow = false, + movingClickOffset = { x: 0, y: 0 }, + isUsingScrollbars = false, isMovingScrollbar = false, scrollbarBackgroundPosition = {}, @@ -420,44 +424,57 @@ var usersWindow = (function () { } function updateOverlayPositions() { - var y; + // Overlay positions are all relative to windowPosition; windowPosition is the position of the windowPane overlay. + var windowLeft = windowPosition.x, + windowTop = windowPosition.y - windowHeight, + x, + y; Overlays.editOverlay(windowBorder, { - y: viewportHeight - windowHeight - WINDOW_BORDER_TOP_MARGIN + x: windowPosition.x - WINDOW_BORDER_LEFT_MARGIN, + y: windowTop - WINDOW_BORDER_TOP_MARGIN }); Overlays.editOverlay(windowPane, { - y: viewportHeight - windowHeight + x: windowLeft, + y: windowTop }); Overlays.editOverlay(windowHeading, { - y: viewportHeight - windowHeight + WINDOW_MARGIN + x: windowLeft + WINDOW_MARGIN, + y: windowTop + WINDOW_MARGIN }); Overlays.editOverlay(minimizeButton, { - y: viewportHeight - windowHeight + WINDOW_MARGIN / 2 + x: windowLeft + WINDOW_WIDTH - WINDOW_MARGIN / 2 - MIN_MAX_BUTTON_WIDTH, + y: windowTop + WINDOW_MARGIN / 2 }); - scrollbarBackgroundPosition.y = viewportHeight - windowHeight + WINDOW_MARGIN + windowTextHeight; + scrollbarBackgroundPosition.x = windowLeft + WINDOW_WIDTH - 0.5 * WINDOW_MARGIN - SCROLLBAR_BACKGROUND_WIDTH; + scrollbarBackgroundPosition.y = windowTop + WINDOW_MARGIN + windowTextHeight; Overlays.editOverlay(scrollbarBackground, { + x: scrollbarBackgroundPosition.x, y: scrollbarBackgroundPosition.y }); scrollbarBarPosition.y = scrollbarBackgroundPosition.y + 1 + scrollbarValue * (scrollbarBackgroundHeight - scrollbarBarHeight - 2); Overlays.editOverlay(scrollbarBar, { + x: scrollbarBackgroundPosition.x + 1, y: scrollbarBarPosition.y }); - y = viewportHeight - FRIENDS_BUTTON_HEIGHT - DISPLAY_SPACER + x = windowLeft + WINDOW_MARGIN; + y = windowPosition.y - FRIENDS_BUTTON_HEIGHT - DISPLAY_SPACER - windowLineHeight - VISIBILITY_SPACER - windowLineHeight - WINDOW_BASE_MARGIN; Overlays.editOverlay(friendsButton, { + x: x, y: y }); y += FRIENDS_BUTTON_HEIGHT + DISPLAY_SPACER; - displayControl.updatePosition(WINDOW_MARGIN, y); + displayControl.updatePosition(x, y); y += windowLineHeight + VISIBILITY_SPACER; - visibilityControl.updatePosition(WINDOW_MARGIN, y); + visibilityControl.updatePosition(x, y); } function updateUsersDisplay() { @@ -596,10 +613,9 @@ var usersWindow = (function () { }; function updateOverlayVisibility() { - // TODO - //Overlays.editOverlay(windowBorder, { - // visible: isVisible && isBorderVisible - //}); + Overlays.editOverlay(windowBorder, { + visible: isVisible && isBorderVisible + }); Overlays.editOverlay(windowPane, { visible: isVisible }); @@ -765,10 +781,22 @@ var usersWindow = (function () { friendsWindow.setURL(FRIENDS_WINDOW_URL); friendsWindow.setVisible(true); friendsWindow.raise(); + return; + } + + if (clickedOverlay === windowBorder) { + movingClickOffset = { + x: event.x - windowPosition.x, + y: event.y - windowPosition.y + }; + + isMovingWindow = true; } } function onMouseMoveEvent(event) { + var isVisible; + if (isMovingScrollbar) { if (scrollbarBackgroundPosition.x - WINDOW_MARGIN <= event.x && event.x <= scrollbarBackgroundPosition.x + SCROLLBAR_BACKGROUND_WIDTH + WINDOW_MARGIN @@ -787,13 +815,44 @@ var usersWindow = (function () { isMovingScrollbar = false; } } + + + if (isMovingWindow) { + windowPosition = { + x: event.x - movingClickOffset.x, + y: event.y - movingClickOffset.y + }; + updateOverlayPositions(); + + } else { + + isVisible = isBorderVisible; + if (isVisible) { + isVisible = windowPosition.x - WINDOW_BORDER_LEFT_MARGIN <= event.x + && event.x <= windowPosition.x - WINDOW_BORDER_LEFT_MARGIN + WINDOW_BORDER_WIDTH + && windowPosition.y - windowHeight - WINDOW_BORDER_TOP_MARGIN <= event.y + && event.y <= windowPosition.y + WINDOW_BORDER_BOTTOM_MARGIN; + } else { + isVisible = windowPosition.x <= event.x && event.x <= windowPosition.x + WINDOW_WIDTH + && windowPosition.y - windowHeight <= event.y && event.y <= windowPosition.y; + } + if (isVisible !== isBorderVisible) { + isBorderVisible = isVisible; + Overlays.editOverlay(windowBorder, { + visible: isBorderVisible + }); + } + } } function onMouseReleaseEvent() { - Overlays.editOverlay(scrollbarBar, { - backgroundAlpha: SCROLLBAR_BAR_ALPHA - }); - isMovingScrollbar = false; + if (isMovingScrollbar) { + Overlays.editOverlay(scrollbarBar, { + backgroundAlpha: SCROLLBAR_BAR_ALPHA + }); + isMovingScrollbar = false; + } + isMovingWindow = false; } function onScriptUpdate() { @@ -828,25 +887,24 @@ var usersWindow = (function () { Overlays.deleteOverlay(textSizeOverlay); viewportHeight = Controller.getViewportDimensions().y; + windowPosition = { x: 0, y: viewportHeight }; calculateWindowHeight(); windowBorder = Overlays.addOverlay("rectangle", { - x: -WINDOW_BORDER_LEFT_MARGIN, - y: viewportHeight, + x: 0, + y: viewportHeight, // Start up off-screen width: WINDOW_BORDER_WIDTH, height: windowBorderHeight, radius: WINDOW_BORDER_RADIUS, color: WINDOW_BORDER_COLOR, alpha: WINDOW_BORDER_ALPHA, - // TODO - //visible: isVisible && isBorderVisible - visible: true + visible: isVisible && isBorderVisible }); windowPane = Overlays.addOverlay("text", { x: 0, - y: viewportHeight, // Start up off-screen + y: viewportHeight, width: WINDOW_WIDTH, height: windowHeight, topMargin: WINDOW_MARGIN + windowLineHeight, @@ -861,7 +919,7 @@ var usersWindow = (function () { }); windowHeading = Overlays.addOverlay("text", { - x: WINDOW_MARGIN, + x: 0, y: viewportHeight, width: WINDOW_WIDTH - 2 * WINDOW_MARGIN, height: windowTextHeight, @@ -876,7 +934,7 @@ var usersWindow = (function () { }); minimizeButton = Overlays.addOverlay("image", { - x: WINDOW_WIDTH - WINDOW_MARGIN / 2 - MIN_MAX_BUTTON_WIDTH, + x: 0, y: viewportHeight, width: MIN_MAX_BUTTON_WIDTH, height: MIN_MAX_BUTTON_HEIGHT, @@ -893,11 +951,11 @@ var usersWindow = (function () { }); scrollbarBackgroundPosition = { - x: WINDOW_WIDTH - 0.5 * WINDOW_MARGIN - SCROLLBAR_BACKGROUND_WIDTH, + x: 0, y: viewportHeight }; scrollbarBackground = Overlays.addOverlay("text", { - x: scrollbarBackgroundPosition.x, + x: 0, y: scrollbarBackgroundPosition.y, width: SCROLLBAR_BACKGROUND_WIDTH, height: windowTextHeight, @@ -908,11 +966,11 @@ var usersWindow = (function () { }); scrollbarBarPosition = { - x: WINDOW_WIDTH - 0.5 * WINDOW_MARGIN - SCROLLBAR_BACKGROUND_WIDTH + 1, + x: 0, y: viewportHeight }; scrollbarBar = Overlays.addOverlay("text", { - x: scrollbarBarPosition.x, + x: 0, y: scrollbarBarPosition.y, width: SCROLLBAR_BACKGROUND_WIDTH - 2, height: windowTextHeight, @@ -923,7 +981,7 @@ var usersWindow = (function () { }); friendsButton = Overlays.addOverlay("image", { - x: WINDOW_MARGIN, + x: 0, y: viewportHeight, width: FRIENDS_BUTTON_WIDTH, height: FRIENDS_BUTTON_HEIGHT, @@ -943,7 +1001,7 @@ var usersWindow = (function () { value: DISPLAY_VALUES[0], values: DISPLAY_VALUES, displayValues: DISPLAY_DISPLAY_VALUES, - x: WINDOW_MARGIN, + x: 0, y: viewportHeight, width: WINDOW_WIDTH - 1.5 * WINDOW_MARGIN, promptWidth: DISPLAY_PROMPT_WIDTH, @@ -976,7 +1034,7 @@ var usersWindow = (function () { value: myVisibility, values: VISIBILITY_VALUES, displayValues: VISIBILITY_DISPLAY_VALUES, - x: WINDOW_MARGIN, + x: 0, y: viewportHeight, width: WINDOW_WIDTH - 1.5 * WINDOW_MARGIN, promptWidth: VISIBILITY_PROMPT_WIDTH, From 806d06b5529104c6f41f1748381204c1f4c6b9fc Mon Sep 17 00:00:00 2001 From: Zach Pomerantz Date: Thu, 12 May 2016 17:49:47 -0700 Subject: [PATCH 25/39] Wait on old entity script engines in threadpool --- .../src/EntityTreeRenderer.cpp | 21 +++++++++++++------ libraries/script-engine/src/ScriptEngine.h | 14 ++++++------- 2 files changed, 22 insertions(+), 13 deletions(-) diff --git a/libraries/entities-renderer/src/EntityTreeRenderer.cpp b/libraries/entities-renderer/src/EntityTreeRenderer.cpp index c73a7ae6c7..7643e446ec 100644 --- a/libraries/entities-renderer/src/EntityTreeRenderer.cpp +++ b/libraries/entities-renderer/src/EntityTreeRenderer.cpp @@ -13,6 +13,7 @@ #include #include +#include #include #include @@ -83,14 +84,22 @@ void EntityTreeRenderer::resetEntitiesScriptEngine() { auto newEngine = new ScriptEngine(NO_SCRIPT, QString("Entities %1").arg(++_entitiesScriptEngineCount)); _entitiesScriptEngine = QSharedPointer(newEngine, [](ScriptEngine* engine){ - // Gracefully exit + class WaitRunnable : public QRunnable { + public: + WaitRunnable(ScriptEngine* engine) : _engine(engine) {} + virtual void run() override { + _engine->wait(); + _engine->deleteLater(); + } + + private: + ScriptEngine* _engine; + }; + engine->unloadAllEntityScripts(); engine->stop(); - - // Disgracefully exit, if necessary - QTimer::singleShot(ScriptEngine::MAX_SCRIPT_EVALUATION_TIME, engine, &ScriptEngine::abort); - - engine->deleteLater(); + // Wait for the scripting thread from the thread pool to avoid hanging the main thread + QThreadPool::globalInstance()->start(new WaitRunnable(engine)); }); _scriptingServices->registerScriptEngineWithApplicationServices(_entitiesScriptEngine.data()); diff --git a/libraries/script-engine/src/ScriptEngine.h b/libraries/script-engine/src/ScriptEngine.h index bb4814eb94..b107e741f4 100644 --- a/libraries/script-engine/src/ScriptEngine.h +++ b/libraries/script-engine/src/ScriptEngine.h @@ -83,6 +83,13 @@ public: /// run the script in the callers thread, exit when stop() is called. void run(); + //////////////////////////////////////////////////////////////////////////////////////////////////////////////////// + // NOTE - this is intended to be a public interface for Agent scripts, and local scripts, but not for EntityScripts + Q_INVOKABLE void stop(); + + // Stop any evaluating scripts and wait for the scripting thread to finish. + void wait(); + //////////////////////////////////////////////////////////////////////////////////////////////////////////////////// // NOTE - these are NOT intended to be public interfaces available to scripts, the are only Q_INVOKABLE so we can // properly ensure they are only called on the correct thread @@ -138,10 +145,6 @@ public: Q_INVOKABLE void requestGarbageCollection() { collectGarbage(); } - //////////////////////////////////////////////////////////////////////////////////////////////////////////////////// - // NOTE - this is intended to be a public interface for Agent scripts, and local scripts, but not for EntityScripts - Q_INVOKABLE void stop(); - bool isFinished() const { return _isFinished; } // used by Application and ScriptWidget bool isRunning() const { return _isRunning; } // used by ScriptWidget @@ -201,9 +204,6 @@ protected: void init(); QString getFilename() const; - // Stop any evaluating scripts and wait for the scripting thread to finish. - void wait(); - bool evaluatePending() const { return _evaluatesPending > 0; } void timerFired(); void stopAllTimers(); From 8445c9fbc7bef4ffe9fe7d5b3a5f1c5c48214fa7 Mon Sep 17 00:00:00 2001 From: David Rowe Date: Fri, 13 May 2016 13:08:45 +1200 Subject: [PATCH 26/39] Update max window height before it scrolls, according to its position --- scripts/system/users.js | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/scripts/system/users.js b/scripts/system/users.js index 420f6a6bfe..f3f74e303a 100644 --- a/scripts/system/users.js +++ b/scripts/system/users.js @@ -403,9 +403,9 @@ var usersWindow = (function () { + windowLineHeight + VISIBILITY_SPACER + windowLineHeight + WINDOW_BASE_MARGIN; - // Limit window to height of viewport minus VU meter and mirror if displayed + // Limit window to height of viewport above window position minus VU meter and mirror if displayed windowHeight = linesOfUsers.length * windowLineHeight - windowLineSpacing + nonUsersHeight; - maxWindowHeight = viewportHeight - AUDIO_METER_HEIGHT; + maxWindowHeight = windowPosition.y - AUDIO_METER_HEIGHT; if (isMirrorDisplay && !isFullscreenMirror) { maxWindowHeight -= MIRROR_HEIGHT; } @@ -715,7 +715,7 @@ var usersWindow = (function () { if (clickedOverlay === windowPane) { overlayX = event.x - WINDOW_MARGIN; - overlayY = event.y - viewportHeight + windowHeight - WINDOW_MARGIN - windowLineHeight; + overlayY = event.y - windowPosition.y + windowHeight - WINDOW_MARGIN - windowLineHeight; numLinesBefore = Math.round(overlayY / windowLineHeight); minY = numLinesBefore * windowLineHeight; @@ -822,7 +822,9 @@ var usersWindow = (function () { x: event.x - movingClickOffset.x, y: event.y - movingClickOffset.y }; + calculateWindowHeight(); updateOverlayPositions(); + updateUsersDisplay(); } else { From ec74cb4aec53d51ba25379e29f84419a2a6a1497 Mon Sep 17 00:00:00 2001 From: David Rowe Date: Fri, 13 May 2016 13:22:03 +1200 Subject: [PATCH 27/39] Maintain offset of bottom with nearest display edge if Interface resized --- scripts/system/users.js | 47 +++++++++++++++++++++++++++-------------- 1 file changed, 31 insertions(+), 16 deletions(-) diff --git a/scripts/system/users.js b/scripts/system/users.js index f3f74e303a..3b97b9d685 100644 --- a/scripts/system/users.js +++ b/scripts/system/users.js @@ -371,7 +371,7 @@ var usersWindow = (function () { isMinimized = false, isBorderVisible = false, - viewportHeight, + viewport, isMirrorDisplay = false, isFullscreenMirror = false, @@ -858,22 +858,37 @@ var usersWindow = (function () { } function onScriptUpdate() { - var oldViewportHeight = viewportHeight, + var oldViewport = viewport, oldIsMirrorDisplay = isMirrorDisplay, oldIsFullscreenMirror = isFullscreenMirror, MIRROR_MENU_ITEM = "Mirror", FULLSCREEN_MIRROR_MENU_ITEM = "Fullscreen Mirror"; - viewportHeight = Controller.getViewportDimensions().y; + viewport = Controller.getViewportDimensions(); isMirrorDisplay = Menu.isOptionChecked(MIRROR_MENU_ITEM); isFullscreenMirror = Menu.isOptionChecked(FULLSCREEN_MIRROR_MENU_ITEM); - if (viewportHeight !== oldViewportHeight || isMirrorDisplay !== oldIsMirrorDisplay + if (viewport.y !== oldViewport.y || isMirrorDisplay !== oldIsMirrorDisplay || isFullscreenMirror !== oldIsFullscreenMirror) { calculateWindowHeight(); updateUsersDisplay(); - updateOverlayPositions(); } + + if (viewport.y !== oldViewport.y) { + if (windowPosition.y > oldViewport.y / 2) { + // Maintain position w.r.t. bottom of window. + windowPosition.y = viewport.y - (oldViewport.y - windowPosition.y); + } + } + + if (viewport.x !== oldViewport.x) { + if (windowPosition.x + (WINDOW_WIDTH / 2) > oldViewport.x / 2) { + // Maintain position w.r.t. right of window. + windowPosition.x = viewport.x - (oldViewport.x - windowPosition.x); + } + } + + updateOverlayPositions(); } function setUp() { @@ -888,14 +903,14 @@ var usersWindow = (function () { windowLineHeight = windowTextHeight + windowLineSpacing; Overlays.deleteOverlay(textSizeOverlay); - viewportHeight = Controller.getViewportDimensions().y; - windowPosition = { x: 0, y: viewportHeight }; + viewport = Controller.getViewportDimensions(); + windowPosition = { x: 0, y: viewport.y }; calculateWindowHeight(); windowBorder = Overlays.addOverlay("rectangle", { x: 0, - y: viewportHeight, // Start up off-screen + y: viewport.y, // Start up off-screen width: WINDOW_BORDER_WIDTH, height: windowBorderHeight, radius: WINDOW_BORDER_RADIUS, @@ -906,7 +921,7 @@ var usersWindow = (function () { windowPane = Overlays.addOverlay("text", { x: 0, - y: viewportHeight, + y: viewport.y, width: WINDOW_WIDTH, height: windowHeight, topMargin: WINDOW_MARGIN + windowLineHeight, @@ -922,7 +937,7 @@ var usersWindow = (function () { windowHeading = Overlays.addOverlay("text", { x: 0, - y: viewportHeight, + y: viewport.y, width: WINDOW_WIDTH - 2 * WINDOW_MARGIN, height: windowTextHeight, topMargin: 0, @@ -937,7 +952,7 @@ var usersWindow = (function () { minimizeButton = Overlays.addOverlay("image", { x: 0, - y: viewportHeight, + y: viewport.y, width: MIN_MAX_BUTTON_WIDTH, height: MIN_MAX_BUTTON_HEIGHT, imageURL: MIN_MAX_BUTTON_SVG, @@ -954,7 +969,7 @@ var usersWindow = (function () { scrollbarBackgroundPosition = { x: 0, - y: viewportHeight + y: viewport.y }; scrollbarBackground = Overlays.addOverlay("text", { x: 0, @@ -969,7 +984,7 @@ var usersWindow = (function () { scrollbarBarPosition = { x: 0, - y: viewportHeight + y: viewport.y }; scrollbarBar = Overlays.addOverlay("text", { x: 0, @@ -984,7 +999,7 @@ var usersWindow = (function () { friendsButton = Overlays.addOverlay("image", { x: 0, - y: viewportHeight, + y: viewport.y, width: FRIENDS_BUTTON_WIDTH, height: FRIENDS_BUTTON_HEIGHT, imageURL: FRIENDS_BUTTON_SVG, @@ -1004,7 +1019,7 @@ var usersWindow = (function () { values: DISPLAY_VALUES, displayValues: DISPLAY_DISPLAY_VALUES, x: 0, - y: viewportHeight, + y: viewport.y, width: WINDOW_WIDTH - 1.5 * WINDOW_MARGIN, promptWidth: DISPLAY_PROMPT_WIDTH, lineHeight: windowLineHeight, @@ -1037,7 +1052,7 @@ var usersWindow = (function () { values: VISIBILITY_VALUES, displayValues: VISIBILITY_DISPLAY_VALUES, x: 0, - y: viewportHeight, + y: viewport.y, width: WINDOW_WIDTH - 1.5 * WINDOW_MARGIN, promptWidth: VISIBILITY_PROMPT_WIDTH, lineHeight: windowLineHeight, From 1928f51b3e61f2c53ab6198de494b722b18d0485 Mon Sep 17 00:00:00 2001 From: David Rowe Date: Fri, 13 May 2016 13:56:31 +1200 Subject: [PATCH 28/39] Restore window position at script start --- scripts/system/users.js | 31 +++++++++++++++++++++++++++---- 1 file changed, 27 insertions(+), 4 deletions(-) diff --git a/scripts/system/users.js b/scripts/system/users.js index 3b97b9d685..08c354be87 100644 --- a/scripts/system/users.js +++ b/scripts/system/users.js @@ -366,6 +366,8 @@ var usersWindow = (function () { MENU_ITEM_AFTER = "Chat...", SETTING_USERS_WINDOW_MINIMIZED = "UsersWindow.Minimized", + SETINGS_USERS_WINDOW_OFFSET = "UsersWindow.Offset", + // +ve x, y values are offset from left, top of screen; -ve from right, bottom. isVisible = true, isMinimized = false, @@ -816,7 +818,6 @@ var usersWindow = (function () { } } - if (isMovingWindow) { windowPosition = { x: event.x - movingClickOffset.x, @@ -848,13 +849,22 @@ var usersWindow = (function () { } function onMouseReleaseEvent() { + var offset = {}; + if (isMovingScrollbar) { Overlays.editOverlay(scrollbarBar, { backgroundAlpha: SCROLLBAR_BAR_ALPHA }); isMovingScrollbar = false; } - isMovingWindow = false; + + if (isMovingWindow) { + // Save offset of bottom of window to nearest edge of the window. + offset.x = (windowPosition.x + WINDOW_WIDTH / 2 < viewport.x / 2) ? windowPosition.x : windowPosition.x - viewport.x; + offset.y = (windowPosition.y < viewport.y / 2) ? windowPosition.y : windowPosition.y - viewport.y; + Settings.setValue(SETINGS_USERS_WINDOW_OFFSET, JSON.stringify(offset)); + isMovingWindow = false; + } } function onScriptUpdate() { @@ -892,7 +902,9 @@ var usersWindow = (function () { } function setUp() { - var textSizeOverlay; + var textSizeOverlay, + offsetSetting, + offset = {}; textSizeOverlay = Overlays.addOverlay("text", { font: WINDOW_FONT, @@ -904,7 +916,18 @@ var usersWindow = (function () { Overlays.deleteOverlay(textSizeOverlay); viewport = Controller.getViewportDimensions(); - windowPosition = { x: 0, y: viewport.y }; + + offsetSetting = Settings.getValue(SETINGS_USERS_WINDOW_OFFSET); + if (offsetSetting !== "") { + offset = JSON.parse(Settings.getValue(SETINGS_USERS_WINDOW_OFFSET)); + } + if (offset.hasOwnProperty("x") && offset.hasOwnProperty("y")) { + windowPosition.x = offset.x < 0 ? viewport.x + offset.x : offset.x; + windowPosition.y = offset.y <= 0 ? viewport.y + offset.y : offset.y; + + } else { + windowPosition = { x: 0, y: viewport.y }; + } calculateWindowHeight(); From 70d0ebb91c94e59f47631f1e01ba3d89ddbf8c54 Mon Sep 17 00:00:00 2001 From: Zach Pomerantz Date: Fri, 13 May 2016 15:24:15 -0700 Subject: [PATCH 29/39] Consolidate stoppingAllScripts to ScriptEngines::_stopped --- libraries/script-engine/src/ScriptEngine.cpp | 16 +++++++--------- libraries/script-engine/src/ScriptEngine.h | 3 --- libraries/script-engine/src/ScriptEngines.cpp | 7 +++---- libraries/script-engine/src/ScriptEngines.h | 3 +-- 4 files changed, 11 insertions(+), 18 deletions(-) diff --git a/libraries/script-engine/src/ScriptEngine.cpp b/libraries/script-engine/src/ScriptEngine.cpp index 6c7739c784..48635df324 100644 --- a/libraries/script-engine/src/ScriptEngine.cpp +++ b/libraries/script-engine/src/ScriptEngine.cpp @@ -52,8 +52,6 @@ #include "MIDIEvent.h" -std::atomic ScriptEngine::_stoppingAllScripts { false }; - static const QString SCRIPT_EXCEPTION_FORMAT = "[UncaughtException] %1 in %2:%3"; Q_DECLARE_METATYPE(QScriptEngine::FunctionSignature) @@ -655,7 +653,7 @@ void ScriptEngine::addEventHandler(const EntityItemID& entityID, const QString& QScriptValue ScriptEngine::evaluate(const QString& sourceCode, const QString& fileName, int lineNumber) { - if (_stoppingAllScripts) { + if (DependencyManager::get()->_stopped) { return QScriptValue(); // bail early } @@ -691,7 +689,7 @@ QScriptValue ScriptEngine::evaluate(const QString& sourceCode, const QString& fi } void ScriptEngine::run() { - if (_stoppingAllScripts) { + if (DependencyManager::get()->_stopped) { return; // bail early - avoid setting state in init(), as evaluate() will bail too } @@ -901,7 +899,7 @@ QObject* ScriptEngine::setupTimerWithInterval(const QScriptValue& function, int } QObject* ScriptEngine::setInterval(const QScriptValue& function, int intervalMS) { - if (_stoppingAllScripts) { + if (DependencyManager::get()->_stopped) { qCDebug(scriptengine) << "Script.setInterval() while shutting down is ignored... parent script:" << getFilename(); return NULL; // bail early } @@ -910,7 +908,7 @@ QObject* ScriptEngine::setInterval(const QScriptValue& function, int intervalMS) } QObject* ScriptEngine::setTimeout(const QScriptValue& function, int timeoutMS) { - if (_stoppingAllScripts) { + if (DependencyManager::get()->_stopped) { qCDebug(scriptengine) << "Script.setTimeout() while shutting down is ignored... parent script:" << getFilename(); return NULL; // bail early } @@ -962,7 +960,7 @@ void ScriptEngine::print(const QString& message) { // If no callback is specified, the included files will be loaded synchronously and will block execution until // all of the files have finished loading. void ScriptEngine::include(const QStringList& includeFiles, QScriptValue callback) { - if (_stoppingAllScripts) { + if (DependencyManager::get()->_stopped) { qCDebug(scriptengine) << "Script.include() while shutting down is ignored..." << "includeFiles:" << includeFiles << "parent script:" << getFilename(); return; // bail early @@ -1063,7 +1061,7 @@ void ScriptEngine::include(const QStringList& includeFiles, QScriptValue callbac } void ScriptEngine::include(const QString& includeFile, QScriptValue callback) { - if (_stoppingAllScripts) { + if (DependencyManager::get()->_stopped) { qCDebug(scriptengine) << "Script.include() while shutting down is ignored... " << "includeFile:" << includeFile << "parent script:" << getFilename(); return; // bail early @@ -1078,7 +1076,7 @@ void ScriptEngine::include(const QString& includeFile, QScriptValue callback) { // as a stand-alone script. To accomplish this, the ScriptEngine class just emits a signal which // the Application or other context will connect to in order to know to actually load the script void ScriptEngine::load(const QString& loadFile) { - if (_stoppingAllScripts) { + if (DependencyManager::get()->_stopped) { qCDebug(scriptengine) << "Script.load() while shutting down is ignored... " << "loadFile:" << loadFile << "parent script:" << getFilename(); return; // bail early diff --git a/libraries/script-engine/src/ScriptEngine.h b/libraries/script-engine/src/ScriptEngine.h index 175a3f1f1c..abd5ed51f4 100644 --- a/libraries/script-engine/src/ScriptEngine.h +++ b/libraries/script-engine/src/ScriptEngine.h @@ -222,9 +222,6 @@ protected: QUrl currentSandboxURL {}; // The toplevel url string for the entity script that loaded the code being executed, else empty. void doWithEnvironment(const EntityItemID& entityID, const QUrl& sandboxURL, std::function operation); void callWithEnvironment(const EntityItemID& entityID, const QUrl& sandboxURL, QScriptValue function, QScriptValue thisObject, QScriptValueList args); - - friend class ScriptEngines; - static std::atomic _stoppingAllScripts; }; #endif // hifi_ScriptEngine_h diff --git a/libraries/script-engine/src/ScriptEngines.cpp b/libraries/script-engine/src/ScriptEngines.cpp index 61220bc00b..d519af1bce 100644 --- a/libraries/script-engine/src/ScriptEngines.cpp +++ b/libraries/script-engine/src/ScriptEngines.cpp @@ -118,7 +118,7 @@ void ScriptEngines::registerScriptInitializer(ScriptInitializer initializer) { } void ScriptEngines::addScriptEngine(ScriptEngine* engine) { - if (!_stoppingAllScripts) { + if (!_stopped) { QMutexLocker locker(&_allScriptsMutex); _allKnownScriptEngines.insert(engine); } @@ -128,15 +128,14 @@ void ScriptEngines::removeScriptEngine(ScriptEngine* engine) { // If we're not already in the middle of stopping all scripts, then we should remove ourselves // from the list of running scripts. We don't do this if we're in the process of stopping all scripts // because that method removes scripts from its list as it iterates them - if (!_stoppingAllScripts) { + if (!_stopped) { QMutexLocker locker(&_allScriptsMutex); _allKnownScriptEngines.remove(engine); } } void ScriptEngines::shutdownScripting() { - _stoppingAllScripts = true; - ScriptEngine::_stoppingAllScripts = true; + _stopped = true; QMutexLocker locker(&_allScriptsMutex); qCDebug(scriptengine) << "Stopping all scripts.... currently known scripts:" << _allKnownScriptEngines.size(); diff --git a/libraries/script-engine/src/ScriptEngines.h b/libraries/script-engine/src/ScriptEngines.h index 0963b21600..5b4a21be66 100644 --- a/libraries/script-engine/src/ScriptEngines.h +++ b/libraries/script-engine/src/ScriptEngines.h @@ -87,17 +87,16 @@ protected: void onScriptEngineError(const QString& scriptFilename); void launchScriptEngine(ScriptEngine* engine); - Setting::Handle _firstRun { "firstRun", true }; QReadWriteLock _scriptEnginesHashLock; QHash _scriptEnginesHash; QSet _allKnownScriptEngines; QMutex _allScriptsMutex; - std::atomic _stoppingAllScripts { false }; std::list _scriptInitializers; mutable Setting::Handle _scriptsLocationHandle; ScriptsModel _scriptsModel; ScriptsModelFilter _scriptsModelFilter; + std::atomic _stopped { false }; }; QUrl normalizeScriptURL(const QUrl& rawScriptURL); From 13610b1220cc54e261596f02e36ff05ec5fbfc21 Mon Sep 17 00:00:00 2001 From: Zach Pomerantz Date: Fri, 13 May 2016 15:24:31 -0700 Subject: [PATCH 30/39] Delete late-added script engines --- libraries/script-engine/src/ScriptEngines.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/libraries/script-engine/src/ScriptEngines.cpp b/libraries/script-engine/src/ScriptEngines.cpp index d519af1bce..fc2e7a3eaf 100644 --- a/libraries/script-engine/src/ScriptEngines.cpp +++ b/libraries/script-engine/src/ScriptEngines.cpp @@ -118,7 +118,9 @@ void ScriptEngines::registerScriptInitializer(ScriptInitializer initializer) { } void ScriptEngines::addScriptEngine(ScriptEngine* engine) { - if (!_stopped) { + if (_stopped) { + engine->deleteLater(); + } else { QMutexLocker locker(&_allScriptsMutex); _allKnownScriptEngines.insert(engine); } From 36565598a786b40f0b9de5f76347efa418361ad1 Mon Sep 17 00:00:00 2001 From: Zach Pomerantz Date: Fri, 13 May 2016 15:52:53 -0700 Subject: [PATCH 31/39] Hide script stop behind accessor and fix friendship --- libraries/script-engine/src/ScriptEngine.cpp | 14 +++++++------- libraries/script-engine/src/ScriptEngine.h | 6 ++++-- libraries/script-engine/src/ScriptEngines.cpp | 9 ++++++--- libraries/script-engine/src/ScriptEngines.h | 3 ++- 4 files changed, 19 insertions(+), 13 deletions(-) diff --git a/libraries/script-engine/src/ScriptEngine.cpp b/libraries/script-engine/src/ScriptEngine.cpp index 90434c977c..df31ad59f1 100644 --- a/libraries/script-engine/src/ScriptEngine.cpp +++ b/libraries/script-engine/src/ScriptEngine.cpp @@ -754,7 +754,7 @@ void ScriptEngine::addEventHandler(const EntityItemID& entityID, const QString& QScriptValue ScriptEngine::evaluate(const QString& sourceCode, const QString& fileName, int lineNumber) { - if (DependencyManager::get()->_stopped) { + if (DependencyManager::get()->isStopped()) { return QScriptValue(); // bail early } @@ -790,7 +790,7 @@ QScriptValue ScriptEngine::evaluate(const QString& sourceCode, const QString& fi } void ScriptEngine::run() { - if (DependencyManager::get()->_stopped) { + if (DependencyManager::get()->isStopped()) { return; // bail early - avoid setting state in init(), as evaluate() will bail too } @@ -1023,7 +1023,7 @@ QObject* ScriptEngine::setupTimerWithInterval(const QScriptValue& function, int } QObject* ScriptEngine::setInterval(const QScriptValue& function, int intervalMS) { - if (DependencyManager::get()->_stopped) { + if (DependencyManager::get()->isStopped()) { qCDebug(scriptengine) << "Script.setInterval() while shutting down is ignored... parent script:" << getFilename(); return NULL; // bail early } @@ -1032,7 +1032,7 @@ QObject* ScriptEngine::setInterval(const QScriptValue& function, int intervalMS) } QObject* ScriptEngine::setTimeout(const QScriptValue& function, int timeoutMS) { - if (DependencyManager::get()->_stopped) { + if (DependencyManager::get()->isStopped()) { qCDebug(scriptengine) << "Script.setTimeout() while shutting down is ignored... parent script:" << getFilename(); return NULL; // bail early } @@ -1084,7 +1084,7 @@ void ScriptEngine::print(const QString& message) { // If no callback is specified, the included files will be loaded synchronously and will block execution until // all of the files have finished loading. void ScriptEngine::include(const QStringList& includeFiles, QScriptValue callback) { - if (DependencyManager::get()->_stopped) { + if (DependencyManager::get()->isStopped()) { qCDebug(scriptengine) << "Script.include() while shutting down is ignored..." << "includeFiles:" << includeFiles << "parent script:" << getFilename(); return; // bail early @@ -1182,7 +1182,7 @@ void ScriptEngine::include(const QStringList& includeFiles, QScriptValue callbac } void ScriptEngine::include(const QString& includeFile, QScriptValue callback) { - if (DependencyManager::get()->_stopped) { + if (DependencyManager::get()->isStopped()) { qCDebug(scriptengine) << "Script.include() while shutting down is ignored... " << "includeFile:" << includeFile << "parent script:" << getFilename(); return; // bail early @@ -1197,7 +1197,7 @@ void ScriptEngine::include(const QString& includeFile, QScriptValue callback) { // as a stand-alone script. To accomplish this, the ScriptEngine class just emits a signal which // the Application or other context will connect to in order to know to actually load the script void ScriptEngine::load(const QString& loadFile) { - if (DependencyManager::get()->_stopped) { + if (DependencyManager::get()->isStopped()) { qCDebug(scriptengine) << "Script.load() while shutting down is ignored... " << "loadFile:" << loadFile << "parent script:" << getFilename(); return; // bail early diff --git a/libraries/script-engine/src/ScriptEngine.h b/libraries/script-engine/src/ScriptEngine.h index 8a3acb0fcb..993b4e1e8a 100644 --- a/libraries/script-engine/src/ScriptEngine.h +++ b/libraries/script-engine/src/ScriptEngine.h @@ -83,6 +83,10 @@ public: /// run the script in the callers thread, exit when stop() is called. void run(); + void waitTillDoneRunning(); + + QString getFilename() const; + //////////////////////////////////////////////////////////////////////////////////////////////////////////////////// // NOTE - these are NOT intended to be public interfaces available to scripts, the are only Q_INVOKABLE so we can // properly ensure they are only called on the correct thread @@ -199,8 +203,6 @@ protected: qint64 _lastUpdate; void init(); - QString getFilename() const; - void waitTillDoneRunning(); bool evaluatePending() const { return _evaluatesPending > 0; } void timerFired(); void stopAllTimers(); diff --git a/libraries/script-engine/src/ScriptEngines.cpp b/libraries/script-engine/src/ScriptEngines.cpp index f2b5f1cd01..2dce6457f1 100644 --- a/libraries/script-engine/src/ScriptEngines.cpp +++ b/libraries/script-engine/src/ScriptEngines.cpp @@ -119,7 +119,7 @@ void ScriptEngines::registerScriptInitializer(ScriptInitializer initializer) { } void ScriptEngines::addScriptEngine(ScriptEngine* engine) { - if (_stopped) { + if (_isStopped) { engine->deleteLater(); } else { QMutexLocker locker(&_allScriptsMutex); @@ -131,14 +131,14 @@ void ScriptEngines::removeScriptEngine(ScriptEngine* engine) { // If we're not already in the middle of stopping all scripts, then we should remove ourselves // from the list of running scripts. We don't do this if we're in the process of stopping all scripts // because that method removes scripts from its list as it iterates them - if (!_stopped) { + if (!_isStopped) { QMutexLocker locker(&_allScriptsMutex); _allKnownScriptEngines.remove(engine); } } void ScriptEngines::shutdownScripting() { - _stopped = true; + _isStopped = true; QMutexLocker locker(&_allScriptsMutex); qCDebug(scriptengine) << "Stopping all scripts.... currently known scripts:" << _allKnownScriptEngines.size(); @@ -498,6 +498,9 @@ void ScriptEngines::launchScriptEngine(ScriptEngine* scriptEngine) { } } +bool ScriptEngines::isStopped() const { + return _isStopped; +} void ScriptEngines::onScriptFinished(const QString& rawScriptURL, ScriptEngine* engine) { bool removed = false; diff --git a/libraries/script-engine/src/ScriptEngines.h b/libraries/script-engine/src/ScriptEngines.h index 8a395052a0..9a0a52bb52 100644 --- a/libraries/script-engine/src/ScriptEngines.h +++ b/libraries/script-engine/src/ScriptEngines.h @@ -86,6 +86,7 @@ protected: void onScriptEngineLoaded(const QString& scriptFilename); void onScriptEngineError(const QString& scriptFilename); void launchScriptEngine(ScriptEngine* engine); + bool isStopped() const; QReadWriteLock _scriptEnginesHashLock; QHash _scriptEnginesHash; @@ -95,7 +96,7 @@ protected: mutable Setting::Handle _scriptsLocationHandle; ScriptsModel _scriptsModel; ScriptsModelFilter _scriptsModelFilter; - std::atomic _stopped { false }; + std::atomic _isStopped { false }; }; QUrl normalizeScriptURL(const QUrl& rawScriptURL); From 2140dc77b3ccbb284c1c018e30d23589ae12378e Mon Sep 17 00:00:00 2001 From: Zach Pomerantz Date: Fri, 13 May 2016 16:14:22 -0700 Subject: [PATCH 32/39] Rename wait and unload in best thread --- .../entities-renderer/src/EntityTreeRenderer.cpp | 12 ++++++++---- libraries/script-engine/src/ScriptEngine.cpp | 5 +++-- libraries/script-engine/src/ScriptEngine.h | 6 +----- libraries/script-engine/src/ScriptEngines.cpp | 5 +---- 4 files changed, 13 insertions(+), 15 deletions(-) diff --git a/libraries/entities-renderer/src/EntityTreeRenderer.cpp b/libraries/entities-renderer/src/EntityTreeRenderer.cpp index 7643e446ec..d7f9f1343b 100644 --- a/libraries/entities-renderer/src/EntityTreeRenderer.cpp +++ b/libraries/entities-renderer/src/EntityTreeRenderer.cpp @@ -88,16 +88,13 @@ void EntityTreeRenderer::resetEntitiesScriptEngine() { public: WaitRunnable(ScriptEngine* engine) : _engine(engine) {} virtual void run() override { - _engine->wait(); + _engine->waitTillDoneRunning(); _engine->deleteLater(); } private: ScriptEngine* _engine; }; - - engine->unloadAllEntityScripts(); - engine->stop(); // Wait for the scripting thread from the thread pool to avoid hanging the main thread QThreadPool::globalInstance()->start(new WaitRunnable(engine)); }); @@ -110,6 +107,13 @@ void EntityTreeRenderer::resetEntitiesScriptEngine() { void EntityTreeRenderer::clear() { leaveAllEntities(); + if (_entitiesScriptEngine) { + // Unload and stop the engine here (instead of in its deleter) to + // avoid marshalling unload signals back to this thread + _entitiesScriptEngine->unloadAllEntityScripts(); + _entitiesScriptEngine->stop(); + } + if (_wantScripts && !_shuttingDown) { resetEntitiesScriptEngine(); } diff --git a/libraries/script-engine/src/ScriptEngine.cpp b/libraries/script-engine/src/ScriptEngine.cpp index 6ed7f7f684..843d714973 100644 --- a/libraries/script-engine/src/ScriptEngine.cpp +++ b/libraries/script-engine/src/ScriptEngine.cpp @@ -284,7 +284,7 @@ void ScriptEngine::runInThread() { workerThread->start(); } -void ScriptEngine::wait() { +void ScriptEngine::waitTillDoneRunning() { auto workerThread = thread(); if (_isThreaded && workerThread) { @@ -303,8 +303,9 @@ void ScriptEngine::wait() { // Process events for the main application thread, allowing invokeMethod calls to pass between threads. QCoreApplication::processEvents(); - // If the final evaluation takes too long, then tell the script engine to stop evaluating + // If the final evaluation takes too long, then tell the script engine to stop running auto elapsedUsecs = usecTimestampNow() - startedWaiting; + static const auto MAX_SCRIPT_EVALUATION_TIME = USECS_PER_SECOND; if (elapsedUsecs > MAX_SCRIPT_EVALUATION_TIME) { workerThread->quit(); diff --git a/libraries/script-engine/src/ScriptEngine.h b/libraries/script-engine/src/ScriptEngine.h index b107e741f4..789958d57a 100644 --- a/libraries/script-engine/src/ScriptEngine.h +++ b/libraries/script-engine/src/ScriptEngine.h @@ -67,10 +67,7 @@ public: class ScriptEngine : public QScriptEngine, public ScriptUser, public EntitiesScriptEngineProvider { Q_OBJECT public: - static const auto MAX_SCRIPT_EVALUATION_TIME = USECS_PER_SECOND; - ScriptEngine(const QString& scriptContents = NO_SCRIPT, const QString& fileNameString = QString("")); - ~ScriptEngine(); /// run the script in a dedicated thread. This will have the side effect of evalulating @@ -88,7 +85,7 @@ public: Q_INVOKABLE void stop(); // Stop any evaluating scripts and wait for the scripting thread to finish. - void wait(); + void waitTillDoneRunning(); //////////////////////////////////////////////////////////////////////////////////////////////////////////////////// // NOTE - these are NOT intended to be public interfaces available to scripts, the are only Q_INVOKABLE so we can @@ -169,7 +166,6 @@ public: public slots: void callAnimationStateHandler(QScriptValue callback, AnimVariantMap parameters, QStringList names, bool useNames, AnimVariantResultHandler resultHandler); void updateMemoryCost(const qint64&); - void abort() { abortEvaluation(); } signals: void scriptLoaded(const QString& scriptFilename); diff --git a/libraries/script-engine/src/ScriptEngines.cpp b/libraries/script-engine/src/ScriptEngines.cpp index 6e7c2bb839..b31c7ac07b 100644 --- a/libraries/script-engine/src/ScriptEngines.cpp +++ b/libraries/script-engine/src/ScriptEngines.cpp @@ -158,9 +158,6 @@ void ScriptEngines::shutdownScripting() { // and stop. We can safely short circuit this because we know we're in the "quitting" process scriptEngine->disconnect(this); - // If this is an entity script, we need to unload any entities - scriptEngine->unloadAllEntityScripts(); - // Gracefully stop the engine's scripting thread scriptEngine->stop(); @@ -168,7 +165,7 @@ void ScriptEngines::shutdownScripting() { // want any of the scripts final "scriptEnding()" or pending "update()" methods from accessing // any application state after we leave this stopAllScripts() method qCDebug(scriptengine) << "waiting on script:" << scriptName; - scriptEngine->wait(); + scriptEngine->waitTillDoneRunning(); qCDebug(scriptengine) << "done waiting on script:" << scriptName; scriptEngine->deleteLater(); From 33e2cde266c0cf814ce7b1d994f6a9b096be0a14 Mon Sep 17 00:00:00 2001 From: David Rowe Date: Sat, 14 May 2016 12:08:19 +1200 Subject: [PATCH 33/39] Change default position to be bottom left of HMD recommended rectangle --- scripts/system/users.js | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/scripts/system/users.js b/scripts/system/users.js index 08c354be87..d935dd23ca 100644 --- a/scripts/system/users.js +++ b/scripts/system/users.js @@ -904,7 +904,8 @@ var usersWindow = (function () { function setUp() { var textSizeOverlay, offsetSetting, - offset = {}; + offset = {}, + hmdViewport; textSizeOverlay = Overlays.addOverlay("text", { font: WINDOW_FONT, @@ -926,7 +927,11 @@ var usersWindow = (function () { windowPosition.y = offset.y <= 0 ? viewport.y + offset.y : offset.y; } else { - windowPosition = { x: 0, y: viewport.y }; + hmdViewport = Controller.getRecommendedOverlayRect(); + windowPosition = { + x: (viewport.x - hmdViewport.width) / 2, // HMD viewport is narrower than screen. + y: hmdViewport.height // HMD viewport starts at top of screen but only extends down so far. + }; } calculateWindowHeight(); From 4e62d7ff6114e7c529cd6307d8229545f2d279bc Mon Sep 17 00:00:00 2001 From: Zach Pomerantz Date: Fri, 13 May 2016 17:28:21 -0700 Subject: [PATCH 34/39] Define ScriptEngines::isStopped inline --- libraries/script-engine/src/ScriptEngines.cpp | 4 ---- libraries/script-engine/src/ScriptEngines.h | 2 +- 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/libraries/script-engine/src/ScriptEngines.cpp b/libraries/script-engine/src/ScriptEngines.cpp index 2dce6457f1..805ddfdada 100644 --- a/libraries/script-engine/src/ScriptEngines.cpp +++ b/libraries/script-engine/src/ScriptEngines.cpp @@ -498,10 +498,6 @@ void ScriptEngines::launchScriptEngine(ScriptEngine* scriptEngine) { } } -bool ScriptEngines::isStopped() const { - return _isStopped; -} - void ScriptEngines::onScriptFinished(const QString& rawScriptURL, ScriptEngine* engine) { bool removed = false; { diff --git a/libraries/script-engine/src/ScriptEngines.h b/libraries/script-engine/src/ScriptEngines.h index 9a0a52bb52..72bf7d529e 100644 --- a/libraries/script-engine/src/ScriptEngines.h +++ b/libraries/script-engine/src/ScriptEngines.h @@ -86,7 +86,7 @@ protected: void onScriptEngineLoaded(const QString& scriptFilename); void onScriptEngineError(const QString& scriptFilename); void launchScriptEngine(ScriptEngine* engine); - bool isStopped() const; + bool isStopped() const { return _isStopped; } QReadWriteLock _scriptEnginesHashLock; QHash _scriptEnginesHash; From 84f4945840c49c3e97f32613fbe28b9d10db6a3b Mon Sep 17 00:00:00 2001 From: Brad Davis Date: Sun, 15 May 2016 13:05:50 -0700 Subject: [PATCH 35/39] Fix sampler mode lookup --- libraries/gpu/src/gpu/GLBackendTexture.cpp | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/libraries/gpu/src/gpu/GLBackendTexture.cpp b/libraries/gpu/src/gpu/GLBackendTexture.cpp index 609451bd13..24b9544168 100755 --- a/libraries/gpu/src/gpu/GLBackendTexture.cpp +++ b/libraries/gpu/src/gpu/GLBackendTexture.cpp @@ -535,13 +535,12 @@ void GLBackend::syncSampler(const Sampler& sampler, Texture::Type type, const GL GLint minFilter; GLint magFilter; }; - static const GLFilterMode filterModes[] = { + static const GLFilterMode filterModes[Sampler::NUM_FILTERS] = { { GL_NEAREST, GL_NEAREST }, //FILTER_MIN_MAG_POINT, { GL_NEAREST, GL_LINEAR }, //FILTER_MIN_POINT_MAG_LINEAR, { GL_LINEAR, GL_NEAREST }, //FILTER_MIN_LINEAR_MAG_POINT, { GL_LINEAR, GL_LINEAR }, //FILTER_MIN_MAG_LINEAR, - { GL_NEAREST_MIPMAP_NEAREST, GL_NEAREST }, //FILTER_MIN_MAG_MIP_POINT, { GL_NEAREST_MIPMAP_NEAREST, GL_NEAREST }, //FILTER_MIN_MAG_MIP_POINT, { GL_NEAREST_MIPMAP_LINEAR, GL_NEAREST }, //FILTER_MIN_MAG_POINT_MIP_LINEAR, { GL_NEAREST_MIPMAP_NEAREST, GL_LINEAR }, //FILTER_MIN_POINT_MAG_LINEAR_MIP_POINT, @@ -557,7 +556,7 @@ void GLBackend::syncSampler(const Sampler& sampler, Texture::Type type, const GL glTexParameteri(object->_target, GL_TEXTURE_MIN_FILTER, fm.minFilter); glTexParameteri(object->_target, GL_TEXTURE_MAG_FILTER, fm.magFilter); - static const GLenum comparisonFuncs[] = { + static const GLenum comparisonFuncs[NUM_COMPARISON_FUNCS] = { GL_NEVER, GL_LESS, GL_EQUAL, @@ -574,7 +573,7 @@ void GLBackend::syncSampler(const Sampler& sampler, Texture::Type type, const GL glTexParameteri(object->_target, GL_TEXTURE_COMPARE_MODE, GL_NONE); } - static const GLenum wrapModes[] = { + static const GLenum wrapModes[Sampler::NUM_WRAP_MODES] = { GL_REPEAT, // WRAP_REPEAT, GL_MIRRORED_REPEAT, // WRAP_MIRROR, GL_CLAMP_TO_EDGE, // WRAP_CLAMP, From 61fbb7022c1b251e6a8f784a475f38814fccf664 Mon Sep 17 00:00:00 2001 From: Zach Pomerantz Date: Mon, 16 May 2016 10:40:11 -0700 Subject: [PATCH 36/39] Mv entities script engine deleter to fn --- .../src/EntityTreeRenderer.cpp | 25 +++++++++++-------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/libraries/entities-renderer/src/EntityTreeRenderer.cpp b/libraries/entities-renderer/src/EntityTreeRenderer.cpp index d7f9f1343b..bec2fa9b8d 100644 --- a/libraries/entities-renderer/src/EntityTreeRenderer.cpp +++ b/libraries/entities-renderer/src/EntityTreeRenderer.cpp @@ -78,13 +78,8 @@ EntityTreeRenderer::~EntityTreeRenderer() { int EntityTreeRenderer::_entitiesScriptEngineCount = 0; -void EntityTreeRenderer::resetEntitiesScriptEngine() { - // Keep a ref to oldEngine until newEngine is ready so EntityScriptingInterface has something to use - auto oldEngine = _entitiesScriptEngine; - - auto newEngine = new ScriptEngine(NO_SCRIPT, QString("Entities %1").arg(++_entitiesScriptEngineCount)); - _entitiesScriptEngine = QSharedPointer(newEngine, [](ScriptEngine* engine){ - class WaitRunnable : public QRunnable { +void entitiesScriptEngineDeleter(ScriptEngine* engine) { + class WaitRunnable : public QRunnable { public: WaitRunnable(ScriptEngine* engine) : _engine(engine) {} virtual void run() override { @@ -94,10 +89,18 @@ void EntityTreeRenderer::resetEntitiesScriptEngine() { private: ScriptEngine* _engine; - }; - // Wait for the scripting thread from the thread pool to avoid hanging the main thread - QThreadPool::globalInstance()->start(new WaitRunnable(engine)); - }); + }; + + // Wait for the scripting thread from the thread pool to avoid hanging the main thread + QThreadPool::globalInstance()->start(new WaitRunnable(engine)); +} + +void EntityTreeRenderer::resetEntitiesScriptEngine() { + // Keep a ref to oldEngine until newEngine is ready so EntityScriptingInterface has something to use + auto oldEngine = _entitiesScriptEngine; + + auto newEngine = new ScriptEngine(NO_SCRIPT, QString("Entities %1").arg(++_entitiesScriptEngineCount)); + _entitiesScriptEngine = QSharedPointer(newEngine, entitiesScriptEngineDeleter); _scriptingServices->registerScriptEngineWithApplicationServices(_entitiesScriptEngine.data()); _entitiesScriptEngine->runInThread(); From d77b3bf59a6edec6b557628c15e33cc83f3d8fa2 Mon Sep 17 00:00:00 2001 From: Seth Alves Date: Mon, 16 May 2016 14:12:05 -0700 Subject: [PATCH 37/39] fix some comments and debug pritns --- libraries/entities/src/UpdateEntityOperator.cpp | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/libraries/entities/src/UpdateEntityOperator.cpp b/libraries/entities/src/UpdateEntityOperator.cpp index a48f3f7198..84f801b059 100644 --- a/libraries/entities/src/UpdateEntityOperator.cpp +++ b/libraries/entities/src/UpdateEntityOperator.cpp @@ -45,23 +45,17 @@ UpdateEntityOperator::UpdateEntityOperator(EntityTreePointer tree, _newEntityCube = newQueryAACube; _newEntityBox = _newEntityCube.clamp((float)-HALF_TREE_SCALE, (float)HALF_TREE_SCALE); // clamp to domain bounds - // If our new properties don't have bounds details (no change to position, etc) or if this containing element would - // be the best fit for our new properties, then just do the new portion of the store pass, since the change path will - // be the same for both parts of the update + // set oldElementBestFit true if the entity was in the correct element before this operator was run. bool oldElementBestFit = _containingElement->bestFitBounds(_oldEntityBox); // For some reason we've seen a case where the original containing element isn't a best fit for the old properties // in this case we want to move it, even if the properties haven't changed. - if (oldElementBestFit) { - if (_wantDebug) { - qCDebug(entities) << " **** TYPICAL NO MOVE CASE **** oldElementBestFit:" << oldElementBestFit; - } - } else { + if (!oldElementBestFit) { _oldEntityBox = _existingEntity->getElement()->getAACube(); _removeOld = true; // our properties are going to move us, so remember this for later processing if (_wantDebug) { - qCDebug(entities) << " **** UNUSUAL CASE **** no changes, but not best fit... consider it a move.... **"; + qCDebug(entities) << " **** UNUSUAL CASE **** not best fit.... **"; } } From eb8a764f42b0321ef413b00e393c3de267d912de Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Mon, 16 May 2016 14:45:04 -0700 Subject: [PATCH 38/39] update LICENSE to include "and other platform" --- LICENSE | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/LICENSE b/LICENSE index 53c5ccf39a..60e86a1cc7 100644 --- a/LICENSE +++ b/LICENSE @@ -6,7 +6,7 @@ Licensed under the Apache License version 2.0 (the "License"); You may not use this software except in compliance with the License. You may obtain a copy of the License at: http://www.apache.org/licenses/LICENSE-2.0 -This software includes third-party software. +This software includes third-party and other platform software. Please see each individual software license for additional details. This software is distributed "as-is" without any warranties, conditions, or representations whether express or implied, including without limitation the implied warranties and conditions of merchantability, merchantable quality, fitness for a particular purpose, performance, durability, title, non-infringement, and those arising from statute or from custom or usage of trade or course of dealing. From 8ec280535bb9d6d97ab2b80eb58cc4eb98322169 Mon Sep 17 00:00:00 2001 From: samcake Date: Tue, 17 May 2016 11:13:34 -0700 Subject: [PATCH 39/39] FIx bad roughness value read from the deferred buffer --- libraries/render-utils/src/DeferredBufferRead.slh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/render-utils/src/DeferredBufferRead.slh b/libraries/render-utils/src/DeferredBufferRead.slh index fa166300ae..569063955d 100644 --- a/libraries/render-utils/src/DeferredBufferRead.slh +++ b/libraries/render-utils/src/DeferredBufferRead.slh @@ -101,7 +101,7 @@ DeferredFragment unpackDeferredFragmentNoPosition(vec2 texcoord) { // Unpack the normal from the map frag.normal = normalize(frag.normalVal.xyz * 2.0 - vec3(1.0)); - frag.roughness = 2.0 * frag.normalVal.a; + frag.roughness = frag.normalVal.a; // Diffuse color and unpack the mode and the metallicness frag.diffuse = frag.diffuseVal.xyz;