Merge pull request #7617 from jherico/vr_menu

VR menu fixes

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();
         });
     }

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<void(VrMenu*)> 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<QQuickItem*>("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"
 

libraries/ui/src/OffscreenUi.h

@@ -13,7 +13,10 @@
 #define hifi_OffscreenUi_h
 
 #include <unordered_map>
+#include <functional>
+
 #include <QtCore/QVariant>
+#include <QtCore/QQueue>
 #include <QtWidgets/QFileDialog>
 #include <QtWidgets/QMessageBox>
 #include <QtWidgets/QInputDialog>
@@ -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<void(VrMenu*)> 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<int, bool> _pressedKeys;
+    VrMenu* _vrMenu { nullptr };
+    QQueue<std::function<void(VrMenu*)>> _queuedMenuInitializers; 
 };
 
 #endif

libraries/ui/src/VrMenu.cpp

@@ -9,33 +9,67 @@
 //
 
 #include "VrMenu.h"
+
 #include <QtQml>
 #include <QMenuBar>
 
+#include "OffscreenUi.h"
+
+static unsigned int USER_DATA_ID = 0;
+
 // 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);
+        if (!USER_DATA_ID) {
-    }
+            USER_DATA_ID = DependencyManager::get<OffscreenUi>()->getMenuUserDataId();
-    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);
+        _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<MenuUserData*>(object->userData(USER_DATA_ID)));
+    }
+
+    static MenuUserData* forObject(QAction* object) {
         if (!object) {
             qWarning() << "Attempted to fetch MenuUserData for null object";
             return nullptr;
@@ -55,46 +89,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 };
+VrMenu::VrMenu(OffscreenUi* parent) : QObject(parent) {
-static QQueue<std::function<void(VrMenu*)>> queuedLambdas;
+    _rootMenu = parent->getRootItem()->findChild<QObject*>("rootMenu");
-
+    parent->getRootContext()->setContextProperty("rootMenu", _rootMenu);
-void VrMenu::executeOrQueue(std::function<void(VrMenu*)> f) {
-    if (_instance) {
-        foreach(std::function<void(VrMenu*)> priorLambda, queuedLambdas) {
-            priorLambda(_instance);
-        }
-        f(_instance);
-    } else {
-        queuedLambdas.push_back(f);
-    }
-}
-
-
-VrMenu::VrMenu(QObject* parent) : QObject(parent) {
-    _instance = this;
-    auto offscreenUi = DependencyManager::get<OffscreenUi>();
-    _rootMenu = offscreenUi->getRootItem()->findChild<QObject*>("rootMenu");
-    offscreenUi->getRootContext()->setContextProperty("rootMenu", _rootMenu);
-    foreach(std::function<void(VrMenu*)> f, queuedLambdas) {
-        f(this);
-    }
-    queuedLambdas.clear();
 }
 
 QObject* VrMenu::findMenuObject(const QString& menuOption) {
@@ -105,21 +108,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<QMenu*>(parent)) {
+    QMenu* parentMenu = dynamic_cast<QMenu*>(parent);
-        MenuUserData* userData = MenuUserData::forObject(parent);
+    if (parentMenu) {
+        MenuUserData* userData = MenuUserData::forObject(parentMenu->menuAction());
         if (!userData) {
             return;
         }
@@ -143,28 +139,16 @@ void VrMenu::addMenu(QMenu* menu) {
     }
 
     // Bind the QML and Widget together
-    new MenuUserData(menu, result);
+    new MenuUserData(menu->menuAction(), 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);
-    });
-
 }
 
 void bindActionToQmlAction(QObject* qmlAction, QAction* action) {
-    new MenuUserData(action, qmlAction);
+    auto text = action->text();
-    updateQmlItemFromAction(qmlAction, action);
+    if (text == "Login") {
-    auto connection = QObject::connect(action, &QAction::changed, [=] {
+        qDebug() << "Login action " << action;
-        updateQmlItemFromAction(qmlAction, action);
+    }
-    });
-    QObject::connect(qApp, &QCoreApplication::aboutToQuit, [=] {
-        QObject::disconnect(connection);
-    });
 
+    new MenuUserData(action, qmlAction);
     QObject::connect(action, &QAction::toggled, [=](bool checked) {
         qmlAction->setProperty("checked", checked);
     });
@@ -174,9 +158,10 @@ void bindActionToQmlAction(QObject* qmlAction, QAction* action) {
 class QQuickMenuItem;
 
 void VrMenu::addAction(QMenu* menu, QAction* action) {
-    Q_ASSERT(!MenuUserData::forObject(action));
+    Q_ASSERT(!MenuUserData::hasData(action));
-    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;
     }
@@ -197,8 +182,8 @@ void VrMenu::addAction(QMenu* menu, QAction* action) {
 }
 
 void VrMenu::addSeparator(QMenu* menu) {
-    Q_ASSERT(MenuUserData::forObject(menu));
+    Q_ASSERT(MenuUserData::hasData(menu->menuAction()));
-    MenuUserData* userData = MenuUserData::forObject(menu);
+    MenuUserData* userData = MenuUserData::forObject(menu->menuAction());
     if (!userData) {
         return;
     }

libraries/ui/src/VrMenu.h

@@ -24,8 +24,7 @@
 class VrMenu : public QObject {
     Q_OBJECT
 public:
-    static void executeOrQueue(std::function<void(VrMenu*)> f);
+    VrMenu(OffscreenUi* parent = nullptr);
-    VrMenu(QObject* 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

libraries/ui/src/ui/Menu.cpp

@@ -15,6 +15,8 @@
 #include <SettingHandle.h>
 
 #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>();
+    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>();
+    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>();
+    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>();
+    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>();
+    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>();
+    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>();
+    offscreenUi->addMenuInitializer([=](VrMenu* vrMenu) {
         vrMenu->insertAction(before, action);
     });
 }