Merge pull request #15066 from SamGondelman/2d

Case 19188: Potential 2D overlay + Tablet threading fixes

interface/src/ui/overlays/Overlay.cpp

@@ -16,8 +16,7 @@
 #include "Application.h"
 
 Overlay::Overlay() :
-    _renderItemID(render::Item::INVALID_ITEM_ID),
-    _visible(true)
+    _renderItemID(render::Item::INVALID_ITEM_ID)
 {
 }
 
@@ -34,20 +33,6 @@ void Overlay::setProperties(const QVariantMap& properties) {
     }
 }
 
-QVariant Overlay::getProperty(const QString& property) {
-    if (property == "type") {
-        return QVariant(getType());
-    }
-    if (property == "id") {
-        return getID();
-    }
-    if (property == "visible") {
-        return _visible;
-    }
-
-    return QVariant();
-}
-
 bool Overlay::addToScene(Overlay::Pointer overlay, const render::ScenePointer& scene, render::Transaction& transaction) {
     _renderItemID = scene->allocateID();
     transaction.resetItem(_renderItemID, std::make_shared<Overlay::Payload>(overlay));
@@ -65,7 +50,7 @@ render::ItemKey Overlay::getKey() {
     builder.withViewSpace();
     builder.withLayer(render::hifi::LAYER_2D);
 
-    if (!getVisible()) {
+    if (!_visible) {
         builder.withInvisible();
     }
 

interface/src/ui/overlays/Overlay.h

@@ -31,7 +31,6 @@ public:
 
     virtual render::ItemKey getKey();
     virtual AABox getBounds() const = 0;
-    virtual bool supportsGetProperty() const { return true; }
 
     virtual bool addToScene(Overlay::Pointer overlay, const render::ScenePointer& scene, render::Transaction& transaction);
     virtual void removeFromScene(Overlay::Pointer overlay, const render::ScenePointer& scene, render::Transaction& transaction);
@@ -42,17 +41,15 @@ public:
 
     // getters
     virtual QString getType() const = 0;
-    bool isLoaded() { return true; }
     bool getVisible() const { return _visible; }
 
     // setters
-    virtual void setVisible(bool visible) { _visible = visible; }
+    void setVisible(bool visible) { _visible = visible; }
     unsigned int getStackOrder() const { return _stackOrder; }
     void setStackOrder(unsigned int stackOrder) { _stackOrder = stackOrder; }
 
-    Q_INVOKABLE virtual void setProperties(const QVariantMap& properties);
+    Q_INVOKABLE virtual void setProperties(const QVariantMap& properties) = 0;
     Q_INVOKABLE virtual Overlay* createClone() const = 0;
-    Q_INVOKABLE virtual QVariant getProperty(const QString& property);
 
     render::ItemID getRenderItemID() const { return _renderItemID; }
     void setRenderItemID(render::ItemID renderItemID) { _renderItemID = renderItemID; }
@@ -60,7 +57,7 @@ public:
 protected:
     render::ItemID _renderItemID { render::Item::INVALID_ITEM_ID };
 
-    bool _visible;
+    bool _visible { true };
     unsigned int _stackOrder { 0 };
 
 private:

interface/src/ui/overlays/Overlay2D.cpp

@@ -65,24 +65,4 @@ void Overlay2D::setProperties(const QVariantMap& properties) {
         }
         setBounds(newBounds);
     }
-}
-
-QVariant Overlay2D::getProperty(const QString& property) {
-    if (property == "bounds") {
-        return qRectToVariant(_bounds);
-    }
-    if (property == "x") {
-        return _bounds.x();
-    }
-    if (property == "y") {
-        return _bounds.y();
-    }
-    if (property == "width") {
-        return _bounds.width();
-    }
-    if (property == "height") {
-        return _bounds.height();
-    }
-
-    return Overlay::getProperty(property);
-}
+}

interface/src/ui/overlays/Overlay2D.h

@@ -26,10 +26,6 @@ public:
     virtual uint32_t fetchMetaSubItems(render::ItemIDs& subItems) const override { subItems.push_back(getRenderItemID()); return 1; }
 
     // getters
-    int getX() const { return _bounds.x(); }
-    int getY() const { return _bounds.y(); }
-    int getWidth() const { return _bounds.width(); }
-    int getHeight() const { return _bounds.height(); }
     const QRect& getBoundingRect() const { return _bounds; }
 
     // setters
@@ -40,7 +36,6 @@ public:
     void setBounds(const QRect& bounds) { _bounds = bounds; }
 
     void setProperties(const QVariantMap& properties) override;
-    QVariant getProperty(const QString& property) override;
 
 protected:
     QRect _bounds; // where on the screen to draw

interface/src/ui/overlays/Overlays.cpp

@@ -772,29 +772,29 @@ QUuid Overlays::addOverlay(const QString& type, const QVariant& properties) {
         return UNKNOWN_ENTITY_ID;
     }
 
-    if (QThread::currentThread() != thread()) {
-        QUuid result;
-        PROFILE_RANGE(script, __FUNCTION__);
-        BLOCKING_INVOKE_METHOD(this, "addOverlay", Q_RETURN_ARG(QUuid, result), Q_ARG(const QString&, type), Q_ARG(const QVariant&, properties));
-        return result;
-    }
-
-    Overlay::Pointer overlay;
-    if (type == ImageOverlay::TYPE) {
+    if (type == ImageOverlay::TYPE || type == TextOverlay::TYPE || type == RectangleOverlay::TYPE) {
 #if !defined(DISABLE_QML)
-        overlay = Overlay::Pointer(new ImageOverlay(), [](Overlay* ptr) { ptr->deleteLater(); });
-#endif
-    } else if (type == TextOverlay::TYPE) {
-#if !defined(DISABLE_QML)
-        overlay = Overlay::Pointer(new TextOverlay(), [](Overlay* ptr) { ptr->deleteLater(); });
-#endif
-    } else if (type == RectangleOverlay::TYPE) {
-        overlay = Overlay::Pointer(new RectangleOverlay(), [](Overlay* ptr) { ptr->deleteLater(); });
-    }
+        if (QThread::currentThread() != thread()) {
+            QUuid result;
+            PROFILE_RANGE(script, __FUNCTION__);
+            BLOCKING_INVOKE_METHOD(this, "addOverlay", Q_RETURN_ARG(QUuid, result), Q_ARG(const QString&, type), Q_ARG(const QVariant&, properties));
+            return result;
+        }
 
-    if (overlay) {
-        overlay->setProperties(properties.toMap());
-        return add2DOverlay(overlay);
+        Overlay::Pointer overlay;
+        if (type == ImageOverlay::TYPE) {
+            overlay = Overlay::Pointer(new ImageOverlay(), [](Overlay* ptr) { ptr->deleteLater(); });
+        } else if (type == TextOverlay::TYPE) {
+            overlay = Overlay::Pointer(new TextOverlay(), [](Overlay* ptr) { ptr->deleteLater(); });
+        } else if (type == RectangleOverlay::TYPE) {
+            overlay = Overlay::Pointer(new RectangleOverlay(), [](Overlay* ptr) { ptr->deleteLater(); });
+        }
+        if (overlay) {
+            overlay->setProperties(properties.toMap());
+            return add2DOverlay(overlay);
+        }
+#endif
+        return QUuid();
     }
 
     QString entityType = overlayToEntityType(type);
@@ -835,15 +835,14 @@ QUuid Overlays::cloneOverlay(const QUuid& id) {
         return UNKNOWN_ENTITY_ID;
     }
 
-    if (QThread::currentThread() != thread()) {
-        QUuid result;
-        PROFILE_RANGE(script, __FUNCTION__);
-        BLOCKING_INVOKE_METHOD(this, "cloneOverlay", Q_RETURN_ARG(QUuid, result), Q_ARG(const QUuid&, id));
-        return result;
-    }
-
     Overlay::Pointer overlay = get2DOverlay(id);
     if (overlay) {
+        if (QThread::currentThread() != thread()) {
+            QUuid result;
+            PROFILE_RANGE(script, __FUNCTION__);
+            BLOCKING_INVOKE_METHOD(this, "cloneOverlay", Q_RETURN_ARG(QUuid, result), Q_ARG(const QUuid&, id));
+            return result;
+        }
         return add2DOverlay(Overlay::Pointer(overlay->createClone(), [](Overlay* ptr) { ptr->deleteLater(); }));
     }
 
@@ -919,6 +918,11 @@ void Overlays::deleteOverlay(const QUuid& id) {
 
     Overlay::Pointer overlay = take2DOverlay(id);
     if (overlay) {
+        if (QThread::currentThread() != thread()) {
+            QMetaObject::invokeMethod(this, "deleteOverlay", Q_ARG(const QUuid&, id));
+            return;
+        }
+
         _overlaysToDelete.push_back(overlay);
         emit overlayDeleted(id);
         return;
@@ -933,15 +937,14 @@ QString Overlays::getOverlayType(const QUuid& id) {
         return "";
     }
 
-    if (QThread::currentThread() != thread()) {
-        QString result;
-        PROFILE_RANGE(script, __FUNCTION__);
-        BLOCKING_INVOKE_METHOD(this, "getOverlayType", Q_RETURN_ARG(QString, result), Q_ARG(const QUuid&, id));
-        return result;
-    }
-
     Overlay::Pointer overlay = get2DOverlay(id);
     if (overlay) {
+        if (QThread::currentThread() != thread()) {
+            QString result;
+            PROFILE_RANGE(script, __FUNCTION__);
+            BLOCKING_INVOKE_METHOD(this, "getOverlayType", Q_RETURN_ARG(QString, result), Q_ARG(const QUuid&, id));
+            return result;
+        }
         return overlay->getType();
     }
 
@@ -949,15 +952,14 @@ QString Overlays::getOverlayType(const QUuid& id) {
 }
 
 QObject* Overlays::getOverlayObject(const QUuid& id) {
-    if (QThread::currentThread() != thread()) {
-        QObject* result;
-        PROFILE_RANGE(script, __FUNCTION__);
-        BLOCKING_INVOKE_METHOD(this, "getOverlayObject", Q_RETURN_ARG(QObject*, result), Q_ARG(const QUuid&, id));
-        return result;
-    }
-
     Overlay::Pointer overlay = get2DOverlay(id);
     if (overlay) {
+        if (QThread::currentThread() != thread()) {
+            QObject* result;
+            PROFILE_RANGE(script, __FUNCTION__);
+            BLOCKING_INVOKE_METHOD(this, "getOverlayObject", Q_RETURN_ARG(QObject*, result), Q_ARG(const QUuid&, id));
+            return result;
+        }
         return qobject_cast<QObject*>(&(*overlay));
     }
 
@@ -969,6 +971,12 @@ QUuid Overlays::getOverlayAtPoint(const glm::vec2& point) {
         return UNKNOWN_ENTITY_ID;
     }
 
+    if (QThread::currentThread() != thread()) {
+        QUuid result;
+        BLOCKING_INVOKE_METHOD(this, "getOverlayAtPoint", Q_RETURN_ARG(QUuid, result), Q_ARG(const glm::vec2&, point));
+        return result;
+    }
+
     QMutexLocker locker(&_mutex);
     QMapIterator<QUuid, Overlay::Pointer> i(_overlays);
     unsigned int bestStackOrder = 0;
@@ -976,8 +984,7 @@ QUuid Overlays::getOverlayAtPoint(const glm::vec2& point) {
     while (i.hasNext()) {
         i.next();
         auto thisOverlay = std::dynamic_pointer_cast<Overlay2D>(i.value());
-        if (thisOverlay && thisOverlay->getVisible() && thisOverlay->isLoaded() &&
-            thisOverlay->getBoundingRect().contains(point.x, point.y, false)) {
+        if (thisOverlay && thisOverlay->getVisible() && thisOverlay->getBoundingRect().contains(point.x, point.y, false)) {
             if (thisOverlay->getStackOrder() > bestStackOrder) {
                 bestID = i.key();
                 bestStackOrder = thisOverlay->getStackOrder();
@@ -991,9 +998,7 @@ QUuid Overlays::getOverlayAtPoint(const glm::vec2& point) {
 QVariant Overlays::getProperty(const QUuid& id, const QString& property) {
     Overlay::Pointer overlay = get2DOverlay(id);
     if (overlay) {
-        if (overlay->supportsGetProperty()) {
-            return overlay->getProperty(property);
-        }
+        // We don't support getting properties from QML Overlays right now
         return QVariant();
     }
 
@@ -1009,12 +1014,8 @@ QVariantMap Overlays::getProperties(const QUuid& id, const QStringList& properti
     Overlay::Pointer overlay = get2DOverlay(id);
     QVariantMap result;
     if (overlay) {
-        if (overlay->supportsGetProperty()) {
-            for (const auto& property : properties) {
-                result.insert(property, overlay->getProperty(property));
-            }
-        }
-        return result;
+        // We don't support getting properties from QML Overlays right now
+        return QVariantMap();
     }
 
     QVariantMap overlayProperties = convertEntityToOverlayProperties(DependencyManager::get<EntityScriptingInterface>()->getEntityProperties(id));
@@ -1141,38 +1142,30 @@ void RayToOverlayIntersectionResultFromScriptValue(const QScriptValue& object, R
 }
 
 bool Overlays::isLoaded(const QUuid& id) {
-    if (QThread::currentThread() != thread()) {
-        bool result;
-        PROFILE_RANGE(script, __FUNCTION__);
-        BLOCKING_INVOKE_METHOD(this, "isLoaded", Q_RETURN_ARG(bool, result), Q_ARG(const QUuid&, id));
-        return result;
-    }
-
     Overlay::Pointer overlay = get2DOverlay(id);
     if (overlay) {
-        return overlay->isLoaded();
+        return true;
     }
 
     return DependencyManager::get<EntityScriptingInterface>()->isLoaded(id);
 }
 
 QSizeF Overlays::textSize(const QUuid& id, const QString& text) {
-    if (QThread::currentThread() != thread()) {
-        QSizeF result;
-        PROFILE_RANGE(script, __FUNCTION__);
-        BLOCKING_INVOKE_METHOD(this, "textSize", Q_RETURN_ARG(QSizeF, result), Q_ARG(const QUuid&, id), Q_ARG(QString, text));
-        return result;
-    }
-
     Overlay::Pointer overlay = get2DOverlay(id);
     if (overlay) {
+        if (QThread::currentThread() != thread()) {
+            QSizeF result;
+            PROFILE_RANGE(script, __FUNCTION__);
+            BLOCKING_INVOKE_METHOD(this, "textSize", Q_RETURN_ARG(QSizeF, result), Q_ARG(const QUuid&, id), Q_ARG(QString, text));
+            return result;
+        }
         if (auto textOverlay = std::dynamic_pointer_cast<TextOverlay>(overlay)) {
             return textOverlay->textSize(text);
         }
         return QSizeF(0.0f, 0.0f);
-    } else {
-        return DependencyManager::get<EntityScriptingInterface>()->textSize(id, text);
     }
+
+    return DependencyManager::get<EntityScriptingInterface>()->textSize(id, text);
 }
 
 bool Overlays::isAddedOverlay(const QUuid& id) {

interface/src/ui/overlays/QmlOverlay.cpp

@@ -57,29 +57,15 @@ QmlOverlay::~QmlOverlay() {
 
 // QmlOverlay replaces Overlay's properties with those defined in the QML file used but keeps Overlay2D's properties.
 void QmlOverlay::setProperties(const QVariantMap& properties) {
-    if (QThread::currentThread() != thread()) {
-        QMetaObject::invokeMethod(this, "setProperties", Q_ARG(QVariantMap, properties));
-        return;
-    }
-
     Overlay2D::setProperties(properties);
-    auto bounds = _bounds;
+
     // check to see if qmlElement still exists
     if (_qmlElement) {
-        _qmlElement->setX(bounds.left());
-        _qmlElement->setY(bounds.top());
-        _qmlElement->setWidth(bounds.width());
-        _qmlElement->setHeight(bounds.height());
+        _qmlElement->setX(_bounds.left());
+        _qmlElement->setY(_bounds.top());
+        _qmlElement->setWidth(_bounds.width());
+        _qmlElement->setHeight(_bounds.height());
+        _qmlElement->setVisible(_visible);
         QMetaObject::invokeMethod(_qmlElement, "updatePropertiesFromScript", Qt::DirectConnection, Q_ARG(QVariant, properties));
     }
-}
-
-void QmlOverlay::render(RenderArgs* args) {
-    if (!_qmlElement) {
-        return;
-    }
-
-    if (_visible != _qmlElement->isVisible()) {
-        _qmlElement->setVisible(_visible);
-    }
-}
+}

interface/src/ui/overlays/QmlOverlay.h

@@ -25,10 +25,8 @@ public:
     QmlOverlay(const QUrl& url, const QmlOverlay* overlay);
     ~QmlOverlay();
 
-    bool supportsGetProperty() const override { return false; }
-
     void setProperties(const QVariantMap& properties) override;
-    void render(RenderArgs* args) override;
+    void render(RenderArgs* args) override {}
 
 private:
     Q_INVOKABLE void qmlElementDestroyed();

libraries/ui/src/ui/TabletScriptingInterface.cpp

@@ -458,6 +458,11 @@ void TabletProxy::emitWebEvent(const QVariant& msg) {
 }
 
 void TabletProxy::onTabletShown() {
+    if (QThread::currentThread() != thread()) {
+        QMetaObject::invokeMethod(this, "onTabletShown");
+        return;
+    }
+
     if (_tabletShown) {
         Setting::Handle<bool> notificationSounds{ QStringLiteral("play_notification_sounds"), true};
         Setting::Handle<bool> notificationSoundTablet{ QStringLiteral("play_notification_sounds_tablet"), true};
@@ -485,7 +490,11 @@ bool TabletProxy::isPathLoaded(const QVariant& path) {
 }
 
 void TabletProxy::setQmlTabletRoot(OffscreenQmlSurface* qmlOffscreenSurface) {
-    Q_ASSERT(QThread::currentThread() == qApp->thread());
+    if (QThread::currentThread() != thread()) {
+        QMetaObject::invokeMethod(this, "setQmlTabletRoot", Q_ARG(OffscreenQmlSurface*, qmlOffscreenSurface));
+        return;
+    }
+
     _qmlOffscreenSurface = qmlOffscreenSurface;
     _qmlTabletRoot = qmlOffscreenSurface ? qmlOffscreenSurface->getRootItem() : nullptr;
     if (_qmlTabletRoot && _qmlOffscreenSurface) {
@@ -654,6 +663,11 @@ void TabletProxy::loadQMLSource(const QVariant& path, bool resizable) {
 }
 
 void TabletProxy::stopQMLSource() {
+    if (QThread::currentThread() != thread()) {
+        QMetaObject::invokeMethod(this, "stopQMLSource");
+        return;
+    }
+
     // For desktop toolbar mode dialogs.
     if (!_toolbarMode || !_desktopWindow) {
         qCDebug(uiLogging) << "tablet cannot clear QML because not desktop toolbar mode";
@@ -879,6 +893,12 @@ void TabletProxy::sendToQml(const QVariant& msg) {
 
 
 OffscreenQmlSurface* TabletProxy::getTabletSurface() {
+    if (QThread::currentThread() != thread()) {
+        OffscreenQmlSurface* result = nullptr;
+        BLOCKING_INVOKE_METHOD(this, "getTabletSurface", Q_RETURN_ARG(OffscreenQmlSurface*, result));
+        return result;
+    }
+
     return _qmlOffscreenSurface;
 }
 
@@ -888,6 +908,11 @@ void TabletProxy::desktopWindowClosed() {
 }
 
 void TabletProxy::unfocus() {
+    if (QThread::currentThread() != thread()) {
+        QMetaObject::invokeMethod(this, "unfocus");
+        return;
+    }
+
     if (_qmlOffscreenSurface) {
         _qmlOffscreenSurface->lowerKeyboard();
     }