From dd739d73a01d8d4a178c4a7659a04914262d9e28 Mon Sep 17 00:00:00 2001 From: David Rowe Date: Tue, 2 May 2017 12:01:55 +1200 Subject: [PATCH 1/6] Don't warn that recording didn't start playing if you stop it --- scripts/system/record.js | 26 +++++++++++++++++++------- 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/scripts/system/record.js b/scripts/system/record.js index 9b231f64f1..3db82696ef 100644 --- a/scripts/system/record.js +++ b/scripts/system/record.js @@ -273,10 +273,11 @@ PLAYER_COMMAND_PLAY = "play", PLAYER_COMMAND_STOP = "stop", - playerIDs = [], // UUIDs of AC player scripts. - playerIsPlayings = [], // True if AC player script is playing a recording. - playerRecordings = [], // Assignment client mappings of recordings being played. - playerTimestamps = [], // Timestamps of last heartbeat update from player script. + playerIDs = [], // UUIDs of AC player scripts. + playerIsPlayings = [], // True if AC player script is playing a recording. + playerRecordings = [], // Assignment client mappings of recordings being played. + playerTimestamps = [], // Timestamps of last heartbeat update from player script. + playerStartupTimeouts = [], // Timers that check that recording has started playing. updateTimer, UPDATE_INTERVAL = 5000; // Must be > player's HEARTBEAT_INTERVAL. @@ -297,6 +298,7 @@ playerIsPlayings.splice(i, 1); playerRecordings.splice(i, 1); playerTimestamps.splice(i, 1); + playerStartupTimeouts.splice(i, 1); } } @@ -333,16 +335,25 @@ orientation: orientation })); - Script.setTimeout(function () { - if (!playerIsPlayings[index] || playerRecordings[index] !== recording) { + playerStartupTimeouts[index] = Script.setTimeout(function () { + if ((!playerIsPlayings[index] || playerRecordings[index] !== recording) && playerStartupTimeouts[index]) { error("Didn't start playing recording " + recording.slice(4) + "!"); // Remove leading "atp:" from recording. } + playerStartupTimeouts[index] = null; }, CHECK_PLAYING_TIMEOUT); - } function stopPlayingRecording(playerID) { + var index; + + // Cancel check that recording started playing. + index = playerIDs.indexOf(playerID); + if (index !== -1 && playerStartupTimeouts[index] !== null) { + // Cannot clearTimeout() without program log error, so just set null. + playerStartupTimeouts[index] = null; + } + Messages.sendMessage(HIFI_PLAYER_CHANNEL, JSON.stringify({ player: playerID, command: PLAYER_COMMAND_STOP @@ -375,6 +386,7 @@ playerIsPlayings = []; playerRecordings = []; playerTimestamps = []; + playerStartupTimeouts = []; Dialog.updatePlayerDetails(playerIsPlayings, playerRecordings, playerIDs); } From cb8f75394d51282ebea2508de615573742a51fbe Mon Sep 17 00:00:00 2001 From: David Rowe Date: Tue, 2 May 2017 12:08:39 +1200 Subject: [PATCH 2/6] Don't sort recordings --- scripts/system/html/js/record.js | 6 ------ 1 file changed, 6 deletions(-) diff --git a/scripts/system/html/js/record.js b/scripts/system/html/js/record.js index c78500307d..aae4d1c89a 100644 --- a/scripts/system/html/js/record.js +++ b/scripts/system/html/js/record.js @@ -53,10 +53,6 @@ function updatePlayersUnused() { elPlayersUnused.innerHTML = numberOfPlayers - recordingsBeingPlayed.length; } -function orderRecording(a, b) { - return a.filename > b.filename ? 1 : -1; -} - function updateRecordings() { var tbody, tr, @@ -68,8 +64,6 @@ function updateRecordings() { i, HIFI_GLYPH_CLOSE = "w"; - recordingsBeingPlayed.sort(orderRecording); - tbody = document.createElement("tbody"); tbody.id = "recordings-list"; From 0353940d9ebe721cb17e51f1ea1fb7d494bde917 Mon Sep 17 00:00:00 2001 From: Zach Pomerantz Date: Tue, 2 May 2017 17:42:23 -0400 Subject: [PATCH 3/6] decouple initialization from storage in Table.getTablet() --- .../src/TabletScriptingInterface.cpp | 71 +++++++++++-------- .../src/TabletScriptingInterface.h | 8 +-- 2 files changed, 47 insertions(+), 32 deletions(-) diff --git a/libraries/script-engine/src/TabletScriptingInterface.cpp b/libraries/script-engine/src/TabletScriptingInterface.cpp index d4eeecc82e..aa9840b226 100644 --- a/libraries/script-engine/src/TabletScriptingInterface.cpp +++ b/libraries/script-engine/src/TabletScriptingInterface.cpp @@ -21,6 +21,8 @@ #include #include "SoundEffect.h" +const QString SYSTEM_TOOLBAR = "com.highfidelity.interface.toolbar.system"; +const QString SYSTEM_TABLET = "com.highfidelity.interface.tablet.system"; QScriptValue tabletToScriptValue(QScriptEngine* engine, TabletProxy* const &in) { return engine->newQObject(in, QScriptEngine::QtOwnership, QScriptEngine::ExcludeDeleteLater | QScriptEngine::ExcludeChildObjects); @@ -35,11 +37,11 @@ TabletScriptingInterface::TabletScriptingInterface() { } QObject* TabletScriptingInterface::getSystemToolbarProxy() { - const QString SYSTEM_TOOLBAR = "com.highfidelity.interface.toolbar.system"; Qt::ConnectionType connectionType = Qt::AutoConnection; if (QThread::currentThread() != _toolbarScriptingInterface->thread()) { connectionType = Qt::BlockingQueuedConnection; } + QObject* toolbarProxy = nullptr; bool hasResult = QMetaObject::invokeMethod(_toolbarScriptingInterface, "getToolbar", connectionType, Q_RETURN_ARG(QObject*, toolbarProxy), Q_ARG(QString, SYSTEM_TOOLBAR)); if (hasResult) { @@ -51,28 +53,38 @@ QObject* TabletScriptingInterface::getSystemToolbarProxy() { } TabletProxy* TabletScriptingInterface::getTablet(const QString& tabletId) { + TabletProxy* tabletProxy = nullptr; + { + // the only thing guarded should be map mutation + // this avoids a deadlock with the Main thread + // from Qt::BlockingQueuedEvent invocations later in the call-tree + std::lock_guard guard(_mapMutex); - std::lock_guard guard(_mutex); - - // look up tabletId in the map. - auto iter = _tabletProxies.find(tabletId); - if (iter != _tabletProxies.end()) { - // tablet already exists, just return it. - return iter->second; - } else { - // allocate a new tablet, add it to the map then return it. - auto tabletProxy = new TabletProxy(tabletId); - tabletProxy->setParent(this); - _tabletProxies[tabletId] = tabletProxy; - tabletProxy->setToolbarMode(_toolbarMode); - return tabletProxy; + auto iter = _tabletProxies.find(tabletId); + if (iter != _tabletProxies.end()) { + // tablet already exists + return iter->second; + } else { + // tablet must be created + tabletProxy = new TabletProxy(this, tabletId); + _tabletProxies[tabletId] = tabletProxy; + } } + + assert(tabletProxy); + // initialize new tablet + tabletProxy->setToolbarMode(_toolbarMode); + return tabletProxy; } void TabletScriptingInterface::setToolbarMode(bool toolbarMode) { - std::lock_guard guard(_mutex); - - _toolbarMode = toolbarMode; + { + // the only thing guarded should be _toolbarMode + // this avoids a deadlock with the Main thread + // from Qt::BlockingQueuedEvent invocations later in the call-tree + std::lock_guard guard(_mapMutex); + _toolbarMode = toolbarMode; + } for (auto& iter : _tabletProxies) { iter.second->setToolbarMode(toolbarMode); @@ -89,8 +101,9 @@ void TabletScriptingInterface::setQmlTabletRoot(QString tabletId, QQuickItem* qm } QQuickWindow* TabletScriptingInterface::getTabletWindow() { - TabletProxy* tablet = qobject_cast(getTablet("com.highfidelity.interface.tablet.system")); + TabletProxy* tablet = qobject_cast(getTablet(SYSTEM_TABLET)); QObject* qmlSurface = tablet->getTabletSurface(); + OffscreenQmlSurface* surface = dynamic_cast(qmlSurface); if (!surface) { @@ -156,7 +169,7 @@ void TabletScriptingInterface::processTabletEvents(QObject* object, const QKeyEv void TabletScriptingInterface::processEvent(const QKeyEvent* event) { - TabletProxy* tablet = qobject_cast(getTablet("com.highfidelity.interface.tablet.system")); + TabletProxy* tablet = qobject_cast(getTablet(SYSTEM_TABLET)); QObject* qmlTablet = tablet->getQmlTablet(); QObject* qmlMenu = tablet->getQmlMenu(); @@ -185,10 +198,12 @@ class TabletRootWindow : public QmlWindowClass { virtual QString qmlSource() const override { return "hifi/tablet/WindowRoot.qml"; } }; -TabletProxy::TabletProxy(QString name) : _name(name) { +TabletProxy::TabletProxy(QObject* parent, QString name) : QObject(parent), _name(name) { } void TabletProxy::setToolbarMode(bool toolbarMode) { + std::lock_guard guard(_tabletMutex); + if (toolbarMode == _toolbarMode) { return; } @@ -288,7 +303,7 @@ bool TabletProxy::isPathLoaded(QVariant path) { return path.toString() == _currentPathLoaded.toString(); } void TabletProxy::setQmlTabletRoot(QQuickItem* qmlTabletRoot, QObject* qmlOffscreenSurface) { - std::lock_guard guard(_mutex); + std::lock_guard guard(_tabletMutex); _qmlOffscreenSurface = qmlOffscreenSurface; _qmlTabletRoot = qmlTabletRoot; if (_qmlTabletRoot && _qmlOffscreenSurface) { @@ -519,7 +534,7 @@ void TabletProxy::gotoWebScreen(const QString& url, const QString& injectedJavaS QObject* TabletProxy::addButton(const QVariant& properties) { auto tabletButtonProxy = QSharedPointer(new TabletButtonProxy(properties.toMap())); - std::lock_guard guard(_mutex); + std::lock_guard guard(_tabletMutex); _tabletButtonProxies.push_back(tabletButtonProxy); if (!_toolbarMode && _qmlTabletRoot) { auto tablet = getQmlTablet(); @@ -555,7 +570,7 @@ bool TabletProxy::onHomeScreen() { } void TabletProxy::removeButton(QObject* tabletButtonProxy) { - std::lock_guard guard(_mutex); + std::lock_guard guard(_tabletMutex); auto tablet = getQmlTablet(); if (!tablet) { @@ -743,12 +758,12 @@ TabletButtonProxy::TabletButtonProxy(const QVariantMap& properties) : } void TabletButtonProxy::setQmlButton(QQuickItem* qmlButton) { - std::lock_guard guard(_mutex); + std::lock_guard guard(_buttonMutex); _qmlButton = qmlButton; } void TabletButtonProxy::setToolbarButtonProxy(QObject* toolbarButtonProxy) { - std::lock_guard guard(_mutex); + std::lock_guard guard(_buttonMutex); _toolbarButtonProxy = toolbarButtonProxy; if (_toolbarButtonProxy) { QObject::connect(_toolbarButtonProxy, SIGNAL(clicked()), this, SLOT(clickedSlot())); @@ -756,12 +771,12 @@ void TabletButtonProxy::setToolbarButtonProxy(QObject* toolbarButtonProxy) { } QVariantMap TabletButtonProxy::getProperties() const { - std::lock_guard guard(_mutex); + std::lock_guard guard(_buttonMutex); return _properties; } void TabletButtonProxy::editProperties(QVariantMap properties) { - std::lock_guard guard(_mutex); + std::lock_guard guard(_buttonMutex); QVariantMap::const_iterator iter = properties.constBegin(); while (iter != properties.constEnd()) { diff --git a/libraries/script-engine/src/TabletScriptingInterface.h b/libraries/script-engine/src/TabletScriptingInterface.h index aff14128db..85c1fdaf9a 100644 --- a/libraries/script-engine/src/TabletScriptingInterface.h +++ b/libraries/script-engine/src/TabletScriptingInterface.h @@ -71,7 +71,7 @@ private: void processTabletEvents(QObject* object, const QKeyEvent* event); protected: - std::mutex _mutex; + std::mutex _mapMutex; std::map _tabletProxies; QObject* _toolbarScriptingInterface { nullptr }; bool _toolbarMode { false }; @@ -90,7 +90,7 @@ class TabletProxy : public QObject { Q_PROPERTY(bool landscape READ getLandscape WRITE setLandscape) Q_PROPERTY(bool tabletShown MEMBER _tabletShown NOTIFY tabletShownChanged) public: - TabletProxy(QString name); + TabletProxy(QObject* parent, QString name); void setQmlTabletRoot(QQuickItem* qmlTabletRoot, QObject* qmlOffscreenSurface); @@ -248,7 +248,7 @@ protected: QVariant _initialPath { "" }; QVariant _currentPathLoaded { "" }; QString _name; - std::mutex _mutex; + std::mutex _tabletMutex; std::vector> _tabletButtonProxies; QQuickItem* _qmlTabletRoot { nullptr }; QObject* _qmlOffscreenSurface { nullptr }; @@ -309,7 +309,7 @@ signals: protected: QUuid _uuid; int _stableOrder; - mutable std::mutex _mutex; + mutable std::mutex _buttonMutex; QQuickItem* _qmlButton { nullptr }; QObject* _toolbarButtonProxy { nullptr }; QVariantMap _properties; From 7b387c52c37cd0a777e9a408869811c2d6a908bd Mon Sep 17 00:00:00 2001 From: David Rowe Date: Wed, 3 May 2017 14:42:06 +1200 Subject: [PATCH 4/6] Fix reliability of keyboard enabling in tablet Web pages --- interface/resources/qml/controls/TabletWebView.qml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/interface/resources/qml/controls/TabletWebView.qml b/interface/resources/qml/controls/TabletWebView.qml index 04e784e2ba..333a79e509 100644 --- a/interface/resources/qml/controls/TabletWebView.qml +++ b/interface/resources/qml/controls/TabletWebView.qml @@ -19,7 +19,7 @@ Item { property alias address: displayUrl.text //for compatibility property string scriptURL property alias eventBridge: eventBridgeWrapper.eventBridge - property bool keyboardEnabled: HMD.active + property bool keyboardEnabled: false property bool keyboardRaised: false property bool punctuationMode: false property bool isDesktop: false @@ -307,6 +307,7 @@ Item { Component.onCompleted: { web.isDesktop = (typeof desktop !== "undefined"); + keyboardEnabled = HMD.active; address = url; } From 9d92ed2b9bf1e3be72955b6b1aae08b690aceb79 Mon Sep 17 00:00:00 2001 From: David Rowe Date: Thu, 4 May 2017 16:55:10 +1200 Subject: [PATCH 5/6] Fix keyboard not appearing in social media snapshot sharing dialogs --- interface/resources/qml/controls/TabletWebView.qml | 1 + 1 file changed, 1 insertion(+) diff --git a/interface/resources/qml/controls/TabletWebView.qml b/interface/resources/qml/controls/TabletWebView.qml index ee0f73c259..51b6c16a4c 100644 --- a/interface/resources/qml/controls/TabletWebView.qml +++ b/interface/resources/qml/controls/TabletWebView.qml @@ -281,6 +281,7 @@ Item { web.initialPage = webview.url; startingUp = false; } + webview.forceActiveFocus(); } } From 30604ab901d23196820cba121fb75b5af37c95d8 Mon Sep 17 00:00:00 2001 From: David Rowe Date: Thu, 4 May 2017 16:56:18 +1200 Subject: [PATCH 6/6] Fix some typos noticed in passing --- interface/resources/qml/controls/TabletWebView.qml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/interface/resources/qml/controls/TabletWebView.qml b/interface/resources/qml/controls/TabletWebView.qml index 51b6c16a4c..d288872289 100644 --- a/interface/resources/qml/controls/TabletWebView.qml +++ b/interface/resources/qml/controls/TabletWebView.qml @@ -238,7 +238,7 @@ Item { worldId: WebEngineScript.MainWorld } - property string urlTag: "noDownload=false"; + property string urlTag: "noDownload=false"; userScripts: [ createGlobalEventBridge, raiseAndLowerKeyboard, userScript ] property string newUrl: "" @@ -264,7 +264,7 @@ Item { keyboard.resetShiftMode(false); // Required to support clicking on "hifi://" links if (WebEngineView.LoadStartedStatus == loadRequest.status) { - var url = loadRequest.url.toString(); + var url = loadRequest.url.toString(); if (urlHandler.canHandleUrl(url)) { if (urlHandler.handleUrl(url)) { root.stop(); @@ -273,7 +273,7 @@ Item { } if (WebEngineView.LoadFailedStatus == loadRequest.status) { - console.log(" Tablet WebEngineView failed to laod url: " + loadRequest.url.toString()); + console.log(" Tablet WebEngineView failed to load url: " + loadRequest.url.toString()); } if (WebEngineView.LoadSucceededStatus == loadRequest.status) {