From 8e201f4801dbfdd95486d1e28dce02a8fbb85acb Mon Sep 17 00:00:00 2001 From: Oren Hurvitz Date: Wed, 27 Feb 2019 10:23:57 +0200 Subject: [PATCH 1/3] Fixed race condition during initialization of the Desktop. The Application constructor must setup event handlers for events emitted when the Desktop is constructed *before* calling initializeUi(). Otherwise, sometimes these event handlers won't be called. When this problem happened (usually on slow machines), Interface would start but neither the Desktop nor the Login Screen would appear. --- interface/src/Application.cpp | 56 +++++++++++++++++------------------ 1 file changed, 28 insertions(+), 28 deletions(-) diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp index 635932ea1c..1efd9cc461 100644 --- a/interface/src/Application.cpp +++ b/interface/src/Application.cpp @@ -1446,6 +1446,34 @@ Application::Application(int& argc, char** argv, QElapsedTimer& startupTimer, bo _overlays.init(); // do this before scripts load DependencyManager::set(); + auto offscreenUi = getOffscreenUI(); + connect(offscreenUi.data(), &OffscreenUi::desktopReady, []() { + // Now that we've loaded the menu and thus switched to the previous display plugin + // we can unlock the desktop repositioning code, since all the positions will be + // relative to the desktop size for this plugin + auto offscreenUi = getOffscreenUI(); + auto desktop = offscreenUi->getDesktop(); + if (desktop) { + desktop->setProperty("repositionLocked", false); + } + }); + + connect(offscreenUi.data(), &OffscreenUi::keyboardFocusActive, [this]() { +#if !defined(Q_OS_ANDROID) && !defined(DISABLE_QML) + // Do not show login dialog if requested not to on the command line + QString hifiNoLoginCommandLineKey = QString("--").append(HIFI_NO_LOGIN_COMMAND_LINE_KEY); + int index = arguments().indexOf(hifiNoLoginCommandLineKey); + if (index != -1) { + resumeAfterLoginDialogActionTaken(); + return; + } + + showLoginScreen(); +#else + resumeAfterLoginDialogActionTaken(); +#endif + }); + // Initialize the user interface and menu system // Needs to happen AFTER the render engine initialization to access its configuration initializeUi(); @@ -1791,34 +1819,6 @@ Application::Application(int& argc, char** argv, QElapsedTimer& startupTimer, bo updateVerboseLogging(); - // Now that we've loaded the menu and thus switched to the previous display plugin - // we can unlock the desktop repositioning code, since all the positions will be - // relative to the desktop size for this plugin - auto offscreenUi = getOffscreenUI(); - connect(offscreenUi.data(), &OffscreenUi::desktopReady, []() { - auto offscreenUi = getOffscreenUI(); - auto desktop = offscreenUi->getDesktop(); - if (desktop) { - desktop->setProperty("repositionLocked", false); - } - }); - - connect(offscreenUi.data(), &OffscreenUi::keyboardFocusActive, [this]() { -#if !defined(Q_OS_ANDROID) && !defined(DISABLE_QML) - // Do not show login dialog if requested not to on the command line - QString hifiNoLoginCommandLineKey = QString("--").append(HIFI_NO_LOGIN_COMMAND_LINE_KEY); - int index = arguments().indexOf(hifiNoLoginCommandLineKey); - if (index != -1) { - resumeAfterLoginDialogActionTaken(); - return; - } - - showLoginScreen(); -#else - resumeAfterLoginDialogActionTaken(); -#endif - }); - // Make sure we don't time out during slow operations at startup updateHeartbeat(); QTimer* settingsTimer = new QTimer(); From 49165056c9e7de4dbb7b394ac5ad9918b4f4b880 Mon Sep 17 00:00:00 2001 From: Oren Hurvitz Date: Fri, 1 Mar 2019 15:39:06 +0200 Subject: [PATCH 2/3] Fixed a race condition that sometimes caused the main menus not to appear. When Interface starts, it first calls pauseUntilLoginDetermined(), and later resumeAfterLoginDialogActionTaken(). But on rare occasions these functions are called in the reverse order, and this caused Interface to remain in the "paused" state. Now we check for this case, and abort pauseUntilLoginDetermined() if it happens. This has only happened to me when running Interface immediately after rebuilding it (in Release mode). --- interface/src/Application.cpp | 9 +++++++++ interface/src/Application.h | 3 +++ 2 files changed, 12 insertions(+) diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp index 1efd9cc461..a6ab9c9782 100644 --- a/interface/src/Application.cpp +++ b/interface/src/Application.cpp @@ -5414,6 +5414,13 @@ void Application::pauseUntilLoginDetermined() { return; } + if (_resumeAfterLoginDialogActionTakenWasCalled) { + // This happens occasionally (though not often): resumeAfterLoginDialogActionTaken() has already been called. + // We must abort this method, otherwise Interface will remain in the "Paused" state permanently. + // E.g., the menus "Edit", "View", etc. will not appear. + return; + } + auto myAvatar = getMyAvatar(); _previousAvatarTargetScale = myAvatar->getTargetScale(); _previousAvatarSkeletonModel = myAvatar->getSkeletonModelURL().toString(); @@ -5528,6 +5535,8 @@ void Application::resumeAfterLoginDialogActionTaken() { menu->getMenu("Developer")->setVisible(_developerMenuVisible); _myCamera.setMode(_previousCameraMode); cameraModeChanged(); + + _resumeAfterLoginDialogActionTakenWasCalled = true; } void Application::loadAvatarScripts(const QVector& urls) { diff --git a/interface/src/Application.h b/interface/src/Application.h index 762ac9585a..421797d3a7 100644 --- a/interface/src/Application.h +++ b/interface/src/Application.h @@ -802,5 +802,8 @@ private: bool _showTrackedObjects { false }; bool _prevShowTrackedObjects { false }; + + // Whether resumeAfterLoginDialogActionTaken() has been called + bool _resumeAfterLoginDialogActionTakenWasCalled { false }; }; #endif // hifi_Application_h From 7a8b7c095bd139b413f2f23259a9c9759413d585 Mon Sep 17 00:00:00 2001 From: Oren Hurvitz Date: Sun, 10 Mar 2019 15:20:59 +0200 Subject: [PATCH 3/3] Fixed a race condition related to the keyboard when starting Interface. These two things need to happen in the following order: 1. Initialize the input plugins (including the keyboard) 2. Start the scripts That's because the scripts try to use the keyboard (e.g., in edit.js), and if it's not ready yet then they fail, and then Interface doesn't work right (e.g., the Create button doesn't work). In order to ensure that these things happen in the correct order, we must make sure that resumeAfterLoginDialogActionTaken() (which starts the scripts) is called only after everything else in the Interface constructor has finished. Usually this happens correctly, but occasionally resumeAfterLoginDialogActionTaken() is called too soon. This commit makes sure that even in that case, we'll postpone calling resumeAfterLoginDialogActionTaken() until the Interface constructor has finished. --- interface/src/Application.cpp | 21 ++++++++++++--------- interface/src/Application.h | 6 +++--- 2 files changed, 15 insertions(+), 12 deletions(-) diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp index a6ab9c9782..f6af20e180 100644 --- a/interface/src/Application.cpp +++ b/interface/src/Application.cpp @@ -5414,13 +5414,6 @@ void Application::pauseUntilLoginDetermined() { return; } - if (_resumeAfterLoginDialogActionTakenWasCalled) { - // This happens occasionally (though not often): resumeAfterLoginDialogActionTaken() has already been called. - // We must abort this method, otherwise Interface will remain in the "Paused" state permanently. - // E.g., the menus "Edit", "View", etc. will not appear. - return; - } - auto myAvatar = getMyAvatar(); _previousAvatarTargetScale = myAvatar->getTargetScale(); _previousAvatarSkeletonModel = myAvatar->getSkeletonModelURL().toString(); @@ -5459,6 +5452,13 @@ void Application::pauseUntilLoginDetermined() { // disconnect domain handler. nodeList->getDomainHandler().disconnect(); + // From now on, it's permissible to call resumeAfterLoginDialogActionTaken() + _resumeAfterLoginDialogActionTaken_SafeToRun = true; + + if (_resumeAfterLoginDialogActionTaken_WasPostponed) { + // resumeAfterLoginDialogActionTaken() was already called, but it aborted. Now it's safe to call it again. + resumeAfterLoginDialogActionTaken(); + } } void Application::resumeAfterLoginDialogActionTaken() { @@ -5467,6 +5467,11 @@ void Application::resumeAfterLoginDialogActionTaken() { return; } + if (!_resumeAfterLoginDialogActionTaken_SafeToRun) { + _resumeAfterLoginDialogActionTaken_WasPostponed = true; + return; + } + if (!isHMDMode() && getDesktopTabletBecomesToolbarSetting()) { auto toolbar = DependencyManager::get()->getToolbar("com.highfidelity.interface.toolbar.system"); toolbar->writeProperty("visible", true); @@ -5535,8 +5540,6 @@ void Application::resumeAfterLoginDialogActionTaken() { menu->getMenu("Developer")->setVisible(_developerMenuVisible); _myCamera.setMode(_previousCameraMode); cameraModeChanged(); - - _resumeAfterLoginDialogActionTakenWasCalled = true; } void Application::loadAvatarScripts(const QVector& urls) { diff --git a/interface/src/Application.h b/interface/src/Application.h index 421797d3a7..a674e313a9 100644 --- a/interface/src/Application.h +++ b/interface/src/Application.h @@ -1,4 +1,4 @@ -// +// // Application.h // interface/src // @@ -803,7 +803,7 @@ private: bool _showTrackedObjects { false }; bool _prevShowTrackedObjects { false }; - // Whether resumeAfterLoginDialogActionTaken() has been called - bool _resumeAfterLoginDialogActionTakenWasCalled { false }; + bool _resumeAfterLoginDialogActionTaken_WasPostponed { false }; + bool _resumeAfterLoginDialogActionTaken_SafeToRun { false }; }; #endif // hifi_Application_h