From 3349093908f4791b1a393fd80219d15d60fcb2f5 Mon Sep 17 00:00:00 2001 From: Brad Davis Date: Thu, 7 Apr 2016 19:11:18 -0700 Subject: [PATCH 1/3] Fix vr menu login item update --- interface/src/ui/LoginDialog.cpp | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/interface/src/ui/LoginDialog.cpp b/interface/src/ui/LoginDialog.cpp index 196ba5d29e..60d19df355 100644 --- a/interface/src/ui/LoginDialog.cpp +++ b/interface/src/ui/LoginDialog.cpp @@ -34,16 +34,19 @@ void LoginDialog::toggleAction() { AccountManager& accountManager = AccountManager::getInstance(); QAction* loginAction = Menu::getInstance()->getActionForOption(MenuOption::Login); Q_CHECK_PTR(loginAction); - disconnect(loginAction, 0, 0, 0); + static QMetaObject::Connection connection; + if (connection) { + disconnect(connection); + } if (accountManager.isLoggedIn()) { // change the menu item to logout loginAction->setText("Logout " + accountManager.getAccountInfo().getUsername()); - connect(loginAction, &QAction::triggered, &accountManager, &AccountManager::logout); + connection = connect(loginAction, &QAction::triggered, &accountManager, &AccountManager::logout); } else { // change the menu item to login loginAction->setText("Login"); - connect(loginAction, &QAction::triggered, [] { + connection = connect(loginAction, &QAction::triggered, [] { LoginDialog::show(); }); } From 4b02e8437cafb0d56adccae1cfe43f75f7996559 Mon Sep 17 00:00:00 2001 From: Brad Davis Date: Thu, 7 Apr 2016 19:17:16 -0700 Subject: [PATCH 2/3] Fix menu items created by plugins --- libraries/ui/src/OffscreenUi.cpp | 16 +++- libraries/ui/src/OffscreenUi.h | 10 +++ libraries/ui/src/VrMenu.cpp | 142 ++++++++++++++----------------- libraries/ui/src/VrMenu.h | 6 +- libraries/ui/src/ui/Menu.cpp | 23 +++-- 5 files changed, 107 insertions(+), 90 deletions(-) diff --git a/libraries/ui/src/OffscreenUi.cpp b/libraries/ui/src/OffscreenUi.cpp index 3bf172dc6a..45413b6c35 100644 --- a/libraries/ui/src/OffscreenUi.cpp +++ b/libraries/ui/src/OffscreenUi.cpp @@ -338,6 +338,13 @@ QVariant OffscreenUi::inputDialog(const Icon icon, const QString& title, const Q return waitForInputDialogResult(createInputDialog(icon, title, label, current)); } +void OffscreenUi::addMenuInitializer(std::function f) { + if (!_vrMenu) { + _queuedMenuInitializers.push_back(f); + return; + } + f(_vrMenu); +} QQuickItem* OffscreenUi::createInputDialog(const Icon icon, const QString& title, const QString& label, const QVariant& current) { @@ -445,7 +452,10 @@ void OffscreenUi::createDesktop(const QUrl& url) { _toolWindow = _desktop->findChild("ToolWindow"); - new VrMenu(this); + _vrMenu = new VrMenu(this); + for (const auto& menuInitializer : _queuedMenuInitializers) { + menuInitializer(_vrMenu); + } new KeyboardFocusHack(); @@ -598,5 +608,9 @@ bool OffscreenUi::eventFilter(QObject* originalDestination, QEvent* event) { return result; } +unsigned int OffscreenUi::getMenuUserDataId() const { + return _vrMenu->_userDataId; +} + #include "OffscreenUi.moc" diff --git a/libraries/ui/src/OffscreenUi.h b/libraries/ui/src/OffscreenUi.h index f219d0fea8..0e87018c5c 100644 --- a/libraries/ui/src/OffscreenUi.h +++ b/libraries/ui/src/OffscreenUi.h @@ -13,7 +13,10 @@ #define hifi_OffscreenUi_h #include +#include + #include +#include #include #include #include @@ -23,10 +26,12 @@ #include "OffscreenQmlElement.h" +class VrMenu; class OffscreenUi : public OffscreenQmlSurface, public Dependency { Q_OBJECT + friend class VrMenu; public: OffscreenUi(); virtual void create(QOpenGLContext* context) override; @@ -39,6 +44,7 @@ public: void unfocusWindows(); void toggleMenu(const QPoint& screenCoordinates); bool eventFilter(QObject* originalDestination, QEvent* event) override; + void addMenuInitializer(std::function f); QQuickItem* getDesktop(); QQuickItem* getToolWindow(); @@ -125,6 +131,8 @@ public: static QString getText(const Icon icon, const QString & title, const QString & label, const QString & text = QString(), bool * ok = 0); static QString getItem(const Icon icon, const QString & title, const QString & label, const QStringList & items, int current = 0, bool editable = true, bool * ok = 0); + unsigned int getMenuUserDataId() const; + signals: void showDesktop(); @@ -134,6 +142,8 @@ private: QQuickItem* _desktop { nullptr }; QQuickItem* _toolWindow { nullptr }; std::unordered_map _pressedKeys; + VrMenu* _vrMenu { nullptr }; + QQueue> _queuedMenuInitializers; }; #endif diff --git a/libraries/ui/src/VrMenu.cpp b/libraries/ui/src/VrMenu.cpp index e6f0e92067..777709615c 100644 --- a/libraries/ui/src/VrMenu.cpp +++ b/libraries/ui/src/VrMenu.cpp @@ -9,38 +9,71 @@ // #include "VrMenu.h" + #include #include +#include "OffscreenUi.h" + +unsigned int USER_DATA_ID() { + return DependencyManager::get()->getMenuUserDataId(); +} + // Binds together a Qt Action or Menu with the QML Menu or MenuItem // // TODO On reflection, it may be pointless to use the UUID. Perhaps // simply creating the bidirectional link pointing to both the widget // and qml object and inject the pointer into both objects class MenuUserData : public QObjectUserData { - static const int USER_DATA_ID; - public: MenuUserData(QAction* action, QObject* qmlObject) { - init(action, qmlObject); - } - MenuUserData(QMenu* menu, QObject* qmlObject) { - init(menu, qmlObject); + _action = action; + _qml = qmlObject; + action->setUserData(USER_DATA_ID(), this); + qmlObject->setUserData(USER_DATA_ID(), this); + qmlObject->setObjectName(uuid.toString()); + // Make sure we can find it again in the future + updateQmlItemFromAction(); + auto connection = QObject::connect(action, &QAction::changed, [=] { + updateQmlItemFromAction(); + }); + QObject::connect(qApp, &QCoreApplication::aboutToQuit, [=] { + QObject::disconnect(connection); + }); } ~MenuUserData() { - _widget->setUserData(USER_DATA_ID, nullptr); - _qml->setUserData(USER_DATA_ID, nullptr); + _action->setUserData(USER_DATA_ID(), nullptr); + _qml->setUserData(USER_DATA_ID(), nullptr); } + void updateQmlItemFromAction() { + _qml->setProperty("checkable", _action->isCheckable()); + _qml->setProperty("enabled", _action->isEnabled()); + QString text = _action->text(); + _qml->setProperty("text", text); + _qml->setProperty("shortcut", _action->shortcut().toString()); + _qml->setProperty("checked", _action->isChecked()); + _qml->setProperty("visible", _action->isVisible()); + } + + const QUuid uuid{ QUuid::createUuid() }; - static MenuUserData* forObject(QObject* object) { + static bool hasData(QAction* object) { + if (!object) { + qWarning() << "Attempted to fetch MenuUserData for null object"; + return false; + } + return (nullptr != static_cast(object->userData(USER_DATA_ID()))); + } + + static MenuUserData* forObject(QAction* object) { if (!object) { qWarning() << "Attempted to fetch MenuUserData for null object"; return nullptr; } - auto result = static_cast(object->userData(USER_DATA_ID)); + auto result = static_cast(object->userData(USER_DATA_ID())); if (!result) { qWarning() << "Unable to find MenuUserData for object " << object; if (auto action = dynamic_cast(object)) { @@ -55,46 +88,15 @@ public: private: MenuUserData(const MenuUserData&); - void init(QObject* widgetObject, QObject* qmlObject) { - _widget = widgetObject; - _qml = qmlObject; - widgetObject->setUserData(USER_DATA_ID, this); - qmlObject->setUserData(USER_DATA_ID, this); - qmlObject->setObjectName(uuid.toString()); - // Make sure we can find it again in the future - Q_ASSERT(VrMenu::_instance->findMenuObject(uuid.toString())); - } - QObject* _widget { nullptr }; + QAction* _action { nullptr }; QObject* _qml { nullptr }; }; -const int MenuUserData::USER_DATA_ID = QObject::registerUserData(); -VrMenu* VrMenu::_instance{ nullptr }; -static QQueue> queuedLambdas; - -void VrMenu::executeOrQueue(std::function f) { - if (_instance) { - foreach(std::function priorLambda, queuedLambdas) { - priorLambda(_instance); - } - f(_instance); - } else { - queuedLambdas.push_back(f); - } -} - - -VrMenu::VrMenu(QObject* parent) : QObject(parent) { - _instance = this; - auto offscreenUi = DependencyManager::get(); - _rootMenu = offscreenUi->getRootItem()->findChild("rootMenu"); - offscreenUi->getRootContext()->setContextProperty("rootMenu", _rootMenu); - foreach(std::function f, queuedLambdas) { - f(this); - } - queuedLambdas.clear(); +VrMenu::VrMenu(OffscreenUi* parent) : QObject(parent) { + _rootMenu = parent->getRootItem()->findChild("rootMenu"); + parent->getRootContext()->setContextProperty("rootMenu", _rootMenu); } QObject* VrMenu::findMenuObject(const QString& menuOption) { @@ -105,21 +107,14 @@ QObject* VrMenu::findMenuObject(const QString& menuOption) { return result; } -void updateQmlItemFromAction(QObject* target, QAction* source) { - target->setProperty("checkable", source->isCheckable()); - target->setProperty("enabled", source->isEnabled()); - target->setProperty("text", source->text()); - target->setProperty("shortcut", source->shortcut().toString()); - target->setProperty("checked", source->isChecked()); - target->setProperty("visible", source->isVisible()); -} void VrMenu::addMenu(QMenu* menu) { - Q_ASSERT(!MenuUserData::forObject(menu)); + Q_ASSERT(!MenuUserData::hasData(menu->menuAction())); QObject* parent = menu->parent(); QObject* qmlParent = nullptr; - if (dynamic_cast(parent)) { - MenuUserData* userData = MenuUserData::forObject(parent); + QMenu* parentMenu = dynamic_cast(parent); + if (parentMenu) { + MenuUserData* userData = MenuUserData::forObject(parentMenu->menuAction()); if (!userData) { return; } @@ -143,28 +138,16 @@ void VrMenu::addMenu(QMenu* menu) { } // Bind the QML and Widget together - new MenuUserData(menu, result); - auto menuAction = menu->menuAction(); - updateQmlItemFromAction(result, menuAction); - auto connection = QObject::connect(menuAction, &QAction::changed, [=] { - updateQmlItemFromAction(result, menuAction); - }); - QObject::connect(qApp, &QCoreApplication::aboutToQuit, [=] { - QObject::disconnect(connection); - }); - + new MenuUserData(menu->menuAction(), result); } void bindActionToQmlAction(QObject* qmlAction, QAction* action) { - new MenuUserData(action, qmlAction); - updateQmlItemFromAction(qmlAction, action); - auto connection = QObject::connect(action, &QAction::changed, [=] { - updateQmlItemFromAction(qmlAction, action); - }); - QObject::connect(qApp, &QCoreApplication::aboutToQuit, [=] { - QObject::disconnect(connection); - }); + auto text = action->text(); + if (text == "Login") { + qDebug() << "Login action " << action; + } + new MenuUserData(action, qmlAction); QObject::connect(action, &QAction::toggled, [=](bool checked) { qmlAction->setProperty("checked", checked); }); @@ -174,9 +157,10 @@ void bindActionToQmlAction(QObject* qmlAction, QAction* action) { class QQuickMenuItem; void VrMenu::addAction(QMenu* menu, QAction* action) { - Q_ASSERT(!MenuUserData::forObject(action)); - Q_ASSERT(MenuUserData::forObject(menu)); - MenuUserData* userData = MenuUserData::forObject(menu); + Q_ASSERT(!MenuUserData::hasData(action)); + + Q_ASSERT(MenuUserData::hasData(menu->menuAction())); + MenuUserData* userData = MenuUserData::forObject(menu->menuAction()); if (!userData) { return; } @@ -197,8 +181,8 @@ void VrMenu::addAction(QMenu* menu, QAction* action) { } void VrMenu::addSeparator(QMenu* menu) { - Q_ASSERT(MenuUserData::forObject(menu)); - MenuUserData* userData = MenuUserData::forObject(menu); + Q_ASSERT(MenuUserData::hasData(menu->menuAction())); + MenuUserData* userData = MenuUserData::forObject(menu->menuAction()); if (!userData) { return; } diff --git a/libraries/ui/src/VrMenu.h b/libraries/ui/src/VrMenu.h index d03887821f..c2f9fd830b 100644 --- a/libraries/ui/src/VrMenu.h +++ b/libraries/ui/src/VrMenu.h @@ -24,8 +24,7 @@ class VrMenu : public QObject { Q_OBJECT public: - static void executeOrQueue(std::function f); - VrMenu(QObject* parent = nullptr); + VrMenu(OffscreenUi* parent = nullptr); void addMenu(QMenu* menu); void addAction(QMenu* parent, QAction* action); void addSeparator(QMenu* parent); @@ -36,8 +35,9 @@ protected: QObject* _rootMenu{ nullptr }; QObject* findMenuObject(const QString& name); - static VrMenu* _instance; friend class MenuUserData; + friend class OffscreenUi; + const unsigned int _userDataId { QObject::registerUserData() }; }; #endif // hifi_VrMenu_h diff --git a/libraries/ui/src/ui/Menu.cpp b/libraries/ui/src/ui/Menu.cpp index 1e6cfb1846..985d2f3a02 100644 --- a/libraries/ui/src/ui/Menu.cpp +++ b/libraries/ui/src/ui/Menu.cpp @@ -15,6 +15,8 @@ #include #include "../VrMenu.h" +#include "../OffscreenUi.h" + #include "Logging.h" using namespace ui; @@ -495,7 +497,8 @@ void Menu::removeActionGroup(const QString& groupName) { } MenuWrapper::MenuWrapper(ui::Menu& rootMenu, QMenu* menu) : _rootMenu(rootMenu), _realMenu(menu) { - VrMenu::executeOrQueue([=](VrMenu* vrMenu) { + auto offscreenUi = DependencyManager::get(); + offscreenUi->addMenuInitializer([=](VrMenu* vrMenu) { vrMenu->addMenu(menu); }); _rootMenu._backMap[menu] = this; @@ -515,7 +518,8 @@ void MenuWrapper::setEnabled(bool enabled) { QAction* MenuWrapper::addSeparator() { QAction* action = _realMenu->addSeparator(); - VrMenu::executeOrQueue([=](VrMenu* vrMenu) { + auto offscreenUi = DependencyManager::get(); + offscreenUi->addMenuInitializer([=](VrMenu* vrMenu) { vrMenu->addSeparator(_realMenu); }); return action; @@ -523,14 +527,16 @@ QAction* MenuWrapper::addSeparator() { void MenuWrapper::addAction(QAction* action) { _realMenu->addAction(action); - VrMenu::executeOrQueue([=](VrMenu* vrMenu) { + auto offscreenUi = DependencyManager::get(); + offscreenUi->addMenuInitializer([=](VrMenu* vrMenu) { vrMenu->addAction(_realMenu, action); }); } QAction* MenuWrapper::addAction(const QString& menuName) { QAction* action = _realMenu->addAction(menuName); - VrMenu::executeOrQueue([=](VrMenu* vrMenu) { + auto offscreenUi = DependencyManager::get(); + offscreenUi->addMenuInitializer([=](VrMenu* vrMenu) { vrMenu->addAction(_realMenu, action); }); return action; @@ -538,7 +544,8 @@ QAction* MenuWrapper::addAction(const QString& menuName) { QAction* MenuWrapper::addAction(const QString& menuName, const QObject* receiver, const char* member, const QKeySequence& shortcut) { QAction* action = _realMenu->addAction(menuName, receiver, member, shortcut); - VrMenu::executeOrQueue([=](VrMenu* vrMenu) { + auto offscreenUi = DependencyManager::get(); + offscreenUi->addMenuInitializer([=](VrMenu* vrMenu) { vrMenu->addAction(_realMenu, action); }); return action; @@ -546,14 +553,16 @@ QAction* MenuWrapper::addAction(const QString& menuName, const QObject* receiver void MenuWrapper::removeAction(QAction* action) { _realMenu->removeAction(action); - VrMenu::executeOrQueue([=](VrMenu* vrMenu) { + auto offscreenUi = DependencyManager::get(); + offscreenUi->addMenuInitializer([=](VrMenu* vrMenu) { vrMenu->removeAction(action); }); } void MenuWrapper::insertAction(QAction* before, QAction* action) { _realMenu->insertAction(before, action); - VrMenu::executeOrQueue([=](VrMenu* vrMenu) { + auto offscreenUi = DependencyManager::get(); + offscreenUi->addMenuInitializer([=](VrMenu* vrMenu) { vrMenu->insertAction(before, action); }); } From dfc1d896cd57bf826fb57a1e7c5a94b6825425db Mon Sep 17 00:00:00 2001 From: Brad Davis Date: Tue, 12 Apr 2016 11:49:55 -0700 Subject: [PATCH 3/3] Fix shutdown crash --- libraries/ui/src/VrMenu.cpp | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/libraries/ui/src/VrMenu.cpp b/libraries/ui/src/VrMenu.cpp index 777709615c..2d6113ad63 100644 --- a/libraries/ui/src/VrMenu.cpp +++ b/libraries/ui/src/VrMenu.cpp @@ -15,9 +15,7 @@ #include "OffscreenUi.h" -unsigned int USER_DATA_ID() { - return DependencyManager::get()->getMenuUserDataId(); -} +static unsigned int USER_DATA_ID = 0; // Binds together a Qt Action or Menu with the QML Menu or MenuItem // @@ -27,10 +25,13 @@ unsigned int USER_DATA_ID() { class MenuUserData : public QObjectUserData { public: MenuUserData(QAction* action, QObject* qmlObject) { + if (!USER_DATA_ID) { + USER_DATA_ID = DependencyManager::get()->getMenuUserDataId(); + } _action = action; _qml = qmlObject; - action->setUserData(USER_DATA_ID(), this); - qmlObject->setUserData(USER_DATA_ID(), this); + action->setUserData(USER_DATA_ID, this); + qmlObject->setUserData(USER_DATA_ID, this); qmlObject->setObjectName(uuid.toString()); // Make sure we can find it again in the future updateQmlItemFromAction(); @@ -43,8 +44,8 @@ public: } ~MenuUserData() { - _action->setUserData(USER_DATA_ID(), nullptr); - _qml->setUserData(USER_DATA_ID(), nullptr); + _action->setUserData(USER_DATA_ID, nullptr); + _qml->setUserData(USER_DATA_ID, nullptr); } void updateQmlItemFromAction() { @@ -65,7 +66,7 @@ public: qWarning() << "Attempted to fetch MenuUserData for null object"; return false; } - return (nullptr != static_cast(object->userData(USER_DATA_ID()))); + return (nullptr != static_cast(object->userData(USER_DATA_ID))); } static MenuUserData* forObject(QAction* object) { @@ -73,7 +74,7 @@ public: qWarning() << "Attempted to fetch MenuUserData for null object"; return nullptr; } - auto result = static_cast(object->userData(USER_DATA_ID())); + auto result = static_cast(object->userData(USER_DATA_ID)); if (!result) { qWarning() << "Unable to find MenuUserData for object " << object; if (auto action = dynamic_cast(object)) {