From d42052a299e7746ee7a230cca2e39a4e772f0a7c Mon Sep 17 00:00:00 2001 From: Brad Davis Date: Fri, 28 Jul 2017 14:56:21 -0700 Subject: [PATCH] Fix overlay event handling thread issues --- interface/src/ui/overlays/Overlays.cpp | 12 ++-- interface/src/ui/overlays/Web3DOverlay.cpp | 83 +++++++++------------- interface/src/ui/overlays/Web3DOverlay.h | 9 --- 3 files changed, 41 insertions(+), 63 deletions(-) diff --git a/interface/src/ui/overlays/Overlays.cpp b/interface/src/ui/overlays/Overlays.cpp index 72682fcb8c..7fb4722c7d 100644 --- a/interface/src/ui/overlays/Overlays.cpp +++ b/interface/src/ui/overlays/Overlays.cpp @@ -706,27 +706,27 @@ bool Overlays::isAddedOverlay(OverlayID id) { } void Overlays::sendMousePressOnOverlay(OverlayID overlayID, const PointerEvent& event) { - emit mousePressOnOverlay(overlayID, event); + QMetaObject::invokeMethod(this, "mousePressOnOverlay", Q_ARG(OverlayID, overlayID), Q_ARG(PointerEvent, event)); } void Overlays::sendMouseReleaseOnOverlay(OverlayID overlayID, const PointerEvent& event) { - emit mouseReleaseOnOverlay(overlayID, event); + QMetaObject::invokeMethod(this, "mouseReleaseOnOverlay", Q_ARG(OverlayID, overlayID), Q_ARG(PointerEvent, event)); } void Overlays::sendMouseMoveOnOverlay(OverlayID overlayID, const PointerEvent& event) { - emit mouseMoveOnOverlay(overlayID, event); + QMetaObject::invokeMethod(this, "mouseMoveOnOverlay", Q_ARG(OverlayID, overlayID), Q_ARG(PointerEvent, event)); } void Overlays::sendHoverEnterOverlay(OverlayID id, PointerEvent event) { - emit hoverEnterOverlay(id, event); + QMetaObject::invokeMethod(this, "hoverEnterOverlay", Q_ARG(OverlayID, id), Q_ARG(PointerEvent, event)); } void Overlays::sendHoverOverOverlay(OverlayID id, PointerEvent event) { - emit hoverOverOverlay(id, event); + QMetaObject::invokeMethod(this, "hoverOverOverlay", Q_ARG(OverlayID, id), Q_ARG(PointerEvent, event)); } void Overlays::sendHoverLeaveOverlay(OverlayID id, PointerEvent event) { - emit hoverLeaveOverlay(id, event); + QMetaObject::invokeMethod(this, "hoverLeaveOverlay", Q_ARG(OverlayID, id), Q_ARG(PointerEvent, event)); } OverlayID Overlays::getKeyboardFocusOverlay() { diff --git a/interface/src/ui/overlays/Web3DOverlay.cpp b/interface/src/ui/overlays/Web3DOverlay.cpp index acba15d2ec..a069b67d2b 100644 --- a/interface/src/ui/overlays/Web3DOverlay.cpp +++ b/interface/src/ui/overlays/Web3DOverlay.cpp @@ -100,30 +100,14 @@ Web3DOverlay::~Web3DOverlay() { } _webSurface->pause(); - _webSurface->disconnect(_connection); - - QObject::disconnect(_mousePressConnection); - _mousePressConnection = QMetaObject::Connection(); - QObject::disconnect(_mouseReleaseConnection); - _mouseReleaseConnection = QMetaObject::Connection(); - QObject::disconnect(_mouseMoveConnection); - _mouseMoveConnection = QMetaObject::Connection(); - QObject::disconnect(_hoverLeaveConnection); - _hoverLeaveConnection = QMetaObject::Connection(); - - QObject::disconnect(_emitScriptEventConnection); - _emitScriptEventConnection = QMetaObject::Connection(); - QObject::disconnect(_webEventReceivedConnection); - _webEventReceivedConnection = QMetaObject::Connection(); - - // The lifetime of the QML surface MUST be managed by the main thread - // Additionally, we MUST use local variables copied by value, rather than - // member variables, since they would implicitly refer to a this that - // is no longer valid - auto webSurface = _webSurface; - AbstractViewStateInterface::instance()->postLambdaEvent([webSurface] { - DependencyManager::get()->release(QML, webSurface); - }); + auto overlays = &(qApp->getOverlays()); + QObject::disconnect(overlays, &Overlays::mousePressOnOverlay, this, nullptr); + QObject::disconnect(overlays, &Overlays::mouseReleaseOnOverlay, this, nullptr); + QObject::disconnect(overlays, &Overlays::mouseMoveOnOverlay, this, nullptr); + QObject::disconnect(overlays, &Overlays::hoverLeaveOverlay, this, nullptr); + QObject::disconnect(this, &Web3DOverlay::scriptEventReceived, _webSurface.data(), &OffscreenQmlSurface::emitScriptEvent); + QObject::disconnect(_webSurface.data(), &OffscreenQmlSurface::webEventReceived, this, &Web3DOverlay::webEventReceived); + DependencyManager::get()->release(QML, _webSurface); _webSurface.reset(); } auto geometryCache = DependencyManager::get(); @@ -153,6 +137,9 @@ QString Web3DOverlay::pickURL() { void Web3DOverlay::loadSourceURL() { + if (!_webSurface) { + return; + } QUrl sourceUrl(_url); if (sourceUrl.scheme() == "http" || sourceUrl.scheme() == "https" || @@ -252,10 +239,11 @@ void Web3DOverlay::render(RenderArgs* args) { } }; - _mousePressConnection = connect(&(qApp->getOverlays()), &Overlays::mousePressOnOverlay, this, forwardPointerEvent, Qt::DirectConnection); - _mouseReleaseConnection = connect(&(qApp->getOverlays()), &Overlays::mouseReleaseOnOverlay, this, forwardPointerEvent, Qt::DirectConnection); - _mouseMoveConnection = connect(&(qApp->getOverlays()), &Overlays::mouseMoveOnOverlay, this, forwardPointerEvent, Qt::DirectConnection); - _hoverLeaveConnection = connect(&(qApp->getOverlays()), &Overlays::hoverLeaveOverlay, this, [=](OverlayID overlayID, const PointerEvent& event) { + auto overlays = &(qApp->getOverlays()); + QObject::connect(overlays, &Overlays::mousePressOnOverlay, this, forwardPointerEvent); + QObject::connect(overlays, &Overlays::mouseReleaseOnOverlay, this, forwardPointerEvent); + QObject::connect(overlays, &Overlays::mouseMoveOnOverlay, this, forwardPointerEvent); + QObject::connect(overlays, &Overlays::hoverLeaveOverlay, this, [=](OverlayID overlayID, const PointerEvent& event) { auto self = weakSelf.lock(); if (!self) { return; @@ -265,10 +253,10 @@ void Web3DOverlay::render(RenderArgs* args) { event.getButton(), event.getButtons(), event.getKeyboardModifiers()); forwardPointerEvent(overlayID, event); } - }, Qt::DirectConnection); + }); - _emitScriptEventConnection = connect(this, &Web3DOverlay::scriptEventReceived, _webSurface.data(), &OffscreenQmlSurface::emitScriptEvent); - _webEventReceivedConnection = connect(_webSurface.data(), &OffscreenQmlSurface::webEventReceived, this, &Web3DOverlay::webEventReceived); + QObject::connect(this, &Web3DOverlay::scriptEventReceived, _webSurface.data(), &OffscreenQmlSurface::emitScriptEvent); + QObject::connect(_webSurface.data(), &OffscreenQmlSurface::webEventReceived, this, &Web3DOverlay::webEventReceived); } else { if (_currentMaxFPS != _desiredMaxFPS) { setMaxFPS(_desiredMaxFPS); @@ -438,11 +426,11 @@ void Web3DOverlay::handlePointerEventAsTouch(const PointerEvent& event) { QList touchPoints; touchPoints.push_back(point); - QTouchEvent* touchEvent = new QTouchEvent(touchType, &_touchDevice, event.getKeyboardModifiers()); - touchEvent->setWindow(_webSurface->getWindow()); - touchEvent->setTarget(_webSurface->getRootItem()); - touchEvent->setTouchPoints(touchPoints); - touchEvent->setTouchPointStates(touchPointState); + QTouchEvent touchEvent(touchType, &_touchDevice, event.getKeyboardModifiers()); + touchEvent.setWindow(_webSurface->getWindow()); + touchEvent.setTarget(_webSurface->getRootItem()); + touchEvent.setTouchPoints(touchPoints); + touchEvent.setTouchPointStates(touchPointState); // Send mouse events to the Web surface so that HTML dialog elements work with mouse press and hover. // FIXME: Scroll bar dragging is a bit unstable in the tablet (content can jump up and down at times). @@ -452,16 +440,16 @@ void Web3DOverlay::handlePointerEventAsTouch(const PointerEvent& event) { // receive mouse events #if QT_VERSION >= QT_VERSION_CHECK(5, 9, 0) if (event.getType() == PointerEvent::Move) { - QMouseEvent* mouseEvent = new QMouseEvent(mouseType, windowPoint, windowPoint, windowPoint, button, buttons, Qt::NoModifier); - QCoreApplication::postEvent(_webSurface->getWindow(), mouseEvent); + QMouseEvent mouseEvent(mouseType, windowPoint, windowPoint, windowPoint, button, buttons, Qt::NoModifier); + QCoreApplication::sendEvent(_webSurface->getWindow(), &mouseEvent); } #endif - QCoreApplication::postEvent(_webSurface->getWindow(), touchEvent); + QCoreApplication::sendEvent(_webSurface->getWindow(), &touchEvent); #if QT_VERSION < QT_VERSION_CHECK(5, 9, 0) if (event.getType() == PointerEvent::Move) { - QMouseEvent* mouseEvent = new QMouseEvent(mouseType, windowPoint, windowPoint, windowPoint, button, buttons, Qt::NoModifier); - QCoreApplication::postEvent(_webSurface->getWindow(), mouseEvent); + QMouseEvent mouseEvent(mouseType, windowPoint, windowPoint, windowPoint, button, buttons, Qt::NoModifier); + QCoreApplication::sendEvent(_webSurface->getWindow(), &mouseEvent); } #endif } @@ -505,8 +493,8 @@ void Web3DOverlay::handlePointerEventAsMouse(const PointerEvent& event) { return; } - QMouseEvent* mouseEvent = new QMouseEvent(type, windowPoint, windowPoint, windowPoint, button, buttons, Qt::NoModifier); - QCoreApplication::postEvent(_webSurface->getWindow(), mouseEvent); + QMouseEvent mouseEvent(type, windowPoint, windowPoint, windowPoint, button, buttons, Qt::NoModifier); + QCoreApplication::sendEvent(_webSurface->getWindow(), &mouseEvent); } void Web3DOverlay::setProperties(const QVariantMap& properties) { @@ -608,6 +596,9 @@ void Web3DOverlay::setScriptURL(const QString& scriptURL) { _scriptURL = scriptURL; if (_webSurface) { AbstractViewStateInterface::instance()->postLambdaEvent([this, scriptURL] { + if (!_webSurface) { + return; + } _webSurface->getRootItem()->setProperty("scriptURL", scriptURL); }); } @@ -631,9 +622,5 @@ Web3DOverlay* Web3DOverlay::createClone() const { } void Web3DOverlay::emitScriptEvent(const QVariant& message) { - if (QThread::currentThread() != thread()) { - QMetaObject::invokeMethod(this, "emitScriptEvent", Qt::QueuedConnection, Q_ARG(QVariant, message)); - } else { - emit scriptEventReceived(message); - } + QMetaObject::invokeMethod(this, "scriptEventReceived", Q_ARG(QVariant, message)); } diff --git a/interface/src/ui/overlays/Web3DOverlay.h b/interface/src/ui/overlays/Web3DOverlay.h index 1e3706ed25..aba2ee8555 100644 --- a/interface/src/ui/overlays/Web3DOverlay.h +++ b/interface/src/ui/overlays/Web3DOverlay.h @@ -72,7 +72,6 @@ signals: private: InputMode _inputMode { Touch }; QSharedPointer _webSurface; - QMetaObject::Connection _connection; gpu::TexturePointer _texture; QString _url; QString _scriptURL; @@ -88,14 +87,6 @@ private: uint8_t _currentMaxFPS { 0 }; bool _mayNeedResize { false }; - - QMetaObject::Connection _mousePressConnection; - QMetaObject::Connection _mouseReleaseConnection; - QMetaObject::Connection _mouseMoveConnection; - QMetaObject::Connection _hoverLeaveConnection; - - QMetaObject::Connection _emitScriptEventConnection; - QMetaObject::Connection _webEventReceivedConnection; }; #endif // hifi_Web3DOverlay_h