From eb20181b6086c487542ac56e1773da8ef0d19d91 Mon Sep 17 00:00:00 2001 From: Brad Davis Date: Tue, 27 Mar 2018 14:18:01 -0700 Subject: [PATCH 1/7] Shut down the overlay interface on exit --- interface/src/ui/overlays/Overlays.cpp | 34 +++++++++++++++++++++++++- interface/src/ui/overlays/Overlays.h | 1 + 2 files changed, 34 insertions(+), 1 deletion(-) diff --git a/interface/src/ui/overlays/Overlays.cpp b/interface/src/ui/overlays/Overlays.cpp index 35274e4fbe..ee2f62c6bb 100644 --- a/interface/src/ui/overlays/Overlays.cpp +++ b/interface/src/ui/overlays/Overlays.cpp @@ -54,6 +54,7 @@ Overlays::Overlays() { } void Overlays::cleanupAllOverlays() { + _shuttingDown = true; QMap overlaysHUD; QMap overlaysWorld; { @@ -147,6 +148,10 @@ void Overlays::enable() { // Note, can't be invoked by scripts, but can be called by the InterfaceParentFinder // class on packet processing threads Overlay::Pointer Overlays::getOverlay(OverlayID id) const { + if (_shuttingDown) { + return nullptr; + } + QMutexLocker locker(&_mutex); if (_overlaysHUD.contains(id)) { return _overlaysHUD[id]; @@ -157,6 +162,10 @@ Overlay::Pointer Overlays::getOverlay(OverlayID id) const { } OverlayID Overlays::addOverlay(const QString& type, const QVariant& properties) { + if (_shuttingDown) { + return UNKNOWN_OVERLAY_ID; + } + if (QThread::currentThread() != thread()) { OverlayID result; PROFILE_RANGE(script, __FUNCTION__); @@ -261,6 +270,10 @@ OverlayID Overlays::addOverlay(const QString& type, const QVariant& properties) } OverlayID Overlays::addOverlay(const Overlay::Pointer& overlay) { + if (_shuttingDown) { + return UNKNOWN_OVERLAY_ID; + } + OverlayID thisID = OverlayID(QUuid::createUuid()); overlay->setOverlayID(thisID); overlay->setStackOrder(_stackOrder++); @@ -283,6 +296,10 @@ OverlayID Overlays::addOverlay(const Overlay::Pointer& overlay) { } OverlayID Overlays::cloneOverlay(OverlayID id) { + if (_shuttingDown) { + return UNKNOWN_OVERLAY_ID; + } + if (QThread::currentThread() != thread()) { OverlayID result; PROFILE_RANGE(script, __FUNCTION__); @@ -301,6 +318,10 @@ OverlayID Overlays::cloneOverlay(OverlayID id) { } bool Overlays::editOverlay(OverlayID id, const QVariant& properties) { + if (_shuttingDown) { + return false; + } + auto thisOverlay = getOverlay(id); if (!thisOverlay) { return false; @@ -320,6 +341,10 @@ bool Overlays::editOverlay(OverlayID id, const QVariant& properties) { } bool Overlays::editOverlays(const QVariant& propertiesById) { + if (_shuttingDown) { + return false; + } + bool defer2DOverlays = QThread::currentThread() != thread(); QVariantMap deferrred; @@ -351,6 +376,10 @@ bool Overlays::editOverlays(const QVariant& propertiesById) { } void Overlays::deleteOverlay(OverlayID id) { + if (_shuttingDown) { + return; + } + if (QThread::currentThread() != thread()) { QMetaObject::invokeMethod(this, "deleteOverlay", Q_ARG(OverlayID, id)); return; @@ -374,6 +403,9 @@ void Overlays::deleteOverlay(OverlayID id) { } QString Overlays::getOverlayType(OverlayID overlayId) { + if (_shuttingDown) { + return ""; + } if (QThread::currentThread() != thread()) { QString result; PROFILE_RANGE(script, __FUNCTION__); @@ -389,7 +421,7 @@ QString Overlays::getOverlayType(OverlayID overlayId) { } OverlayID Overlays::getOverlayAtPoint(const glm::vec2& point) { - if (!_enabled) { + if (_shuttingDown || !_enabled) { return UNKNOWN_OVERLAY_ID; } diff --git a/interface/src/ui/overlays/Overlays.h b/interface/src/ui/overlays/Overlays.h index c3d87642f1..8eccbfa4c5 100644 --- a/interface/src/ui/overlays/Overlays.h +++ b/interface/src/ui/overlays/Overlays.h @@ -680,6 +680,7 @@ private: unsigned int _stackOrder { 1 }; bool _enabled = true; + std::atomic _shuttingDown{ false }; PointerEvent calculateOverlayPointerEvent(OverlayID overlayID, PickRay ray, RayToOverlayIntersectionResult rayPickResult, QMouseEvent* event, PointerEvent::EventType eventType); From 29106fbbbe422527fed7b5f10a6f57bea2528124 Mon Sep 17 00:00:00 2001 From: Brad Davis Date: Tue, 27 Mar 2018 14:18:19 -0700 Subject: [PATCH 2/7] Prevent potential double delete of QML overlays --- interface/src/ui/overlays/QmlOverlay.cpp | 32 ++++++++++++------------ interface/src/ui/overlays/QmlOverlay.h | 3 ++- 2 files changed, 18 insertions(+), 17 deletions(-) diff --git a/interface/src/ui/overlays/QmlOverlay.cpp b/interface/src/ui/overlays/QmlOverlay.cpp index f4a9034187..2a583e0450 100644 --- a/interface/src/ui/overlays/QmlOverlay.cpp +++ b/interface/src/ui/overlays/QmlOverlay.cpp @@ -39,18 +39,20 @@ void QmlOverlay::buildQmlElement(const QUrl& url) { auto offscreenUi = DependencyManager::get(); offscreenUi->load(url, [=](QQmlContext* context, QObject* object) { - QQuickItem* rawPtr = dynamic_cast(object); - // Create a shared ptr with a custom deleter lambda, that calls deleteLater - _qmlElement = std::shared_ptr(rawPtr, [](QQuickItem* ptr) { - if (ptr) { - ptr->deleteLater(); - } - }); + _qmlElement = dynamic_cast(object); + connect(_qmlElement, &QObject::destroyed, this, &QmlOverlay::qmlElementDestroyed); }); } +void QmlOverlay::qmlElementDestroyed() { + _qmlElement = nullptr; +} + QmlOverlay::~QmlOverlay() { - _qmlElement.reset(); + if (_qmlElement) { + _qmlElement->deleteLater(); + } + _qmlElement = nullptr; } // QmlOverlay replaces Overlay's properties with those defined in the QML file used but keeps Overlay2D's properties. @@ -62,15 +64,13 @@ void QmlOverlay::setProperties(const QVariantMap& properties) { Overlay2D::setProperties(properties); auto bounds = _bounds; - std::weak_ptr weakQmlElement = _qmlElement; // check to see if qmlElement still exists - auto qmlElement = weakQmlElement.lock(); - if (qmlElement) { - qmlElement->setX(bounds.left()); - qmlElement->setY(bounds.top()); - qmlElement->setWidth(bounds.width()); - qmlElement->setHeight(bounds.height()); - QMetaObject::invokeMethod(qmlElement.get(), "updatePropertiesFromScript", Qt::DirectConnection, Q_ARG(QVariant, properties)); + if (_qmlElement) { + _qmlElement->setX(bounds.left()); + _qmlElement->setY(bounds.top()); + _qmlElement->setWidth(bounds.width()); + _qmlElement->setHeight(bounds.height()); + QMetaObject::invokeMethod(_qmlElement, "updatePropertiesFromScript", Qt::DirectConnection, Q_ARG(QVariant, properties)); } } diff --git a/interface/src/ui/overlays/QmlOverlay.h b/interface/src/ui/overlays/QmlOverlay.h index ced2b6fa1f..7f2cf5a918 100644 --- a/interface/src/ui/overlays/QmlOverlay.h +++ b/interface/src/ui/overlays/QmlOverlay.h @@ -32,10 +32,11 @@ public: void render(RenderArgs* args) override; private: + Q_INVOKABLE void qmlElementDestroyed(); Q_INVOKABLE void buildQmlElement(const QUrl& url); protected: - std::shared_ptr _qmlElement; + QQuickItem* _qmlElement{ nullptr }; }; #endif // hifi_QmlOverlay_h From aacfe927518d941ac8e024602f1266471e0ac6d3 Mon Sep 17 00:00:00 2001 From: Brad Davis Date: Wed, 28 Mar 2018 09:09:56 -0700 Subject: [PATCH 3/7] Don't trigger crashes from timers while waiting for some operation on exit --- interface/src/Application.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp index 968af3e298..56068fba68 100644 --- a/interface/src/Application.cpp +++ b/interface/src/Application.cpp @@ -7488,7 +7488,7 @@ DisplayPluginPointer Application::getActiveDisplayPlugin() const { return _displayPlugin; } - if (!_displayPlugin) { + if (!_aboutToQuit && !_displayPlugin) { const_cast(this)->updateDisplayMode(); Q_ASSERT(_displayPlugin); } From 9be162de95293d911cbaddfb1e772c53e4599ddf Mon Sep 17 00:00:00 2001 From: Brad Davis Date: Wed, 28 Mar 2018 09:13:46 -0700 Subject: [PATCH 4/7] Synchronously shut down QML rendering threads --- libraries/qml/src/qml/impl/SharedObject.cpp | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/libraries/qml/src/qml/impl/SharedObject.cpp b/libraries/qml/src/qml/impl/SharedObject.cpp index f2af5bd036..d66f0f1dab 100644 --- a/libraries/qml/src/qml/impl/SharedObject.cpp +++ b/libraries/qml/src/qml/impl/SharedObject.cpp @@ -69,6 +69,7 @@ SharedObject::SharedObject() { _quickWindow->setColor(QColor(255, 255, 255, 0)); _quickWindow->setClearBeforeRendering(true); + QObject::connect(qApp, &QCoreApplication::aboutToQuit, this, &SharedObject::onAboutToQuit); } @@ -124,6 +125,7 @@ void SharedObject::setRootItem(QQuickItem* rootItem) { _renderThread->setObjectName(objectName()); _renderThread->start(); + // Create event handler for the render thread _renderObject = new RenderEventHandler(this, _renderThread); QCoreApplication::postEvent(this, new OffscreenEvent(OffscreenEvent::Initialize)); @@ -152,9 +154,16 @@ void SharedObject::destroy() { QObject::disconnect(_renderControl); QObject::disconnect(qApp); - QMutexLocker lock(&_mutex); - _quit = true; - QCoreApplication::postEvent(_renderObject, new OffscreenEvent(OffscreenEvent::Quit)); + { + QMutexLocker lock(&_mutex); + _quit = true; + QCoreApplication::postEvent(_renderObject, new OffscreenEvent(OffscreenEvent::Quit), Qt::HighEventPriority); + } + // Block until the rendering thread has stopped + // FIXME this is undesirable because this is blocking the main thread, + // but I haven't found a reliable way to do this only at application + // shutdown + _renderThread->wait(); } From 35ee1bec09963cc27135512eaa2a82a085fd295d Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Wed, 28 Mar 2018 14:47:09 -0700 Subject: [PATCH 5/7] fix cmake changes for script copy for dev builds --- interface/CMakeLists.txt | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/interface/CMakeLists.txt b/interface/CMakeLists.txt index e316556d29..fe00d86c3a 100644 --- a/interface/CMakeLists.txt +++ b/interface/CMakeLists.txt @@ -312,7 +312,7 @@ if (APPLE) COMPONENT ${CLIENT_COMPONENT} ) - set(RESOURCES_INSTALL_DIR "${INTERFACE_INSTALL_APP_PATH}/Contents/Resources") + set(SCRIPTS_INSTALL_DIR "${INTERFACE_INSTALL_APP_PATH}/Contents/Resources") set(RESOURCES_DEV_DIR "$/../Resources") # copy script files beside the executable @@ -326,13 +326,14 @@ if (APPLE) fixup_interface() else() - set(RESOURCES_DEV_DIR "$/resources") + set(INTERFACE_EXEC_DIR "$") + set(RESOURCES_DEV_DIR "${INTERFACE_EXEC_DIR}/resources") # copy the resources files beside the executable add_custom_command(TARGET ${TARGET_NAME} POST_BUILD COMMAND "${CMAKE_COMMAND}" -E copy_if_different "${RESOURCES_RCC}" - "$" + "${INTERFACE_EXEC_DIR}" # FIXME, the edit script code loads HTML from the scripts folder # which in turn relies on CSS that refers to the fonts. In theory # we should be able to modify the CSS to reference the QRC path to @@ -340,13 +341,13 @@ else() # so we have to retain a copy of the fonts outside of the resources binary COMMAND "${CMAKE_COMMAND}" -E copy_directory "${PROJECT_SOURCE_DIR}/resources/fonts" - "$/resources/fonts" + "${RESOURCES_DEV_DIR}/fonts" COMMAND "${CMAKE_COMMAND}" -E copy_directory "${CMAKE_SOURCE_DIR}/scripts" - "${RESOURCES_DEV_DIR}/scripts" + "${INTERFACE_EXEC_DIR}/scripts" COMMAND "${CMAKE_COMMAND}" -E copy_if_different "${PROJECT_SOURCE_DIR}/resources/serverless/tutorial.json" - "$/resources/serverless/tutorial.json" + "${RESOURCES_DEV_DIR}/serverless/tutorial.json" ) # link target to external libraries @@ -363,7 +364,7 @@ else() PATTERN "*.exp" EXCLUDE ) - set(RESOURCES_INSTALL_DIR "${INTERFACE_INSTALL_DIR}") + set(SCRIPTS_INSTALL_DIR "${INTERFACE_INSTALL_DIR}") set(EXECUTABLE_COMPONENT ${CLIENT_COMPONENT}) @@ -371,11 +372,11 @@ else() endif() endif() -if (RESOURCES_INSTALL_DIR) +if (SCRIPTS_INSTALL_DIR) # setup install of scripts beside interface executable install( DIRECTORY "${CMAKE_SOURCE_DIR}/scripts/" - DESTINATION ${RESOURCES_INSTALL_DIR}/scripts + DESTINATION ${SCRIPTS_INSTALL_DIR}/scripts COMPONENT ${CLIENT_COMPONENT} ) endif() From afe39aba46b9c69e3ba6a4c6981f8f8da37c5ac1 Mon Sep 17 00:00:00 2001 From: SamGondelman Date: Wed, 28 Mar 2018 13:47:31 -0700 Subject: [PATCH 6/7] fix octal code char issue --- libraries/entities/src/EntityTree.cpp | 6 +++--- libraries/entities/src/UpdateEntityOperator.cpp | 2 +- libraries/shared/src/OctalCode.cpp | 6 +++--- libraries/shared/src/OctalCode.h | 2 +- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/libraries/entities/src/EntityTree.cpp b/libraries/entities/src/EntityTree.cpp index 75f024d0b9..2cf66911a4 100644 --- a/libraries/entities/src/EntityTree.cpp +++ b/libraries/entities/src/EntityTree.cpp @@ -425,8 +425,8 @@ bool EntityTree::updateEntity(EntityItemPointer entity, const EntityItemProperti if (!childEntity) { continue; } - EntityTreeElementPointer containingElement = childEntity->getElement(); - if (!containingElement) { + EntityTreeElementPointer childContainingElement = childEntity->getElement(); + if (!childContainingElement) { continue; } @@ -440,7 +440,7 @@ bool EntityTree::updateEntity(EntityItemPointer entity, const EntityItemProperti addToNeedsParentFixupList(childEntity); } - UpdateEntityOperator theChildOperator(getThisPointer(), containingElement, childEntity, queryCube); + UpdateEntityOperator theChildOperator(getThisPointer(), childContainingElement, childEntity, queryCube); recurseTreeWithOperator(&theChildOperator); foreach (SpatiallyNestablePointer childChild, childEntity->getChildren()) { if (childChild && childChild->getNestableType() == NestableType::Entity) { diff --git a/libraries/entities/src/UpdateEntityOperator.cpp b/libraries/entities/src/UpdateEntityOperator.cpp index fa7e5ca38f..32bd2f06ba 100644 --- a/libraries/entities/src/UpdateEntityOperator.cpp +++ b/libraries/entities/src/UpdateEntityOperator.cpp @@ -288,7 +288,7 @@ OctreeElementPointer UpdateEntityOperator::possiblyCreateChildAt(const OctreeEle int indexOfChildContainingNewEntity = element->getMyChildContaining(_newEntityBox); if (childIndex == indexOfChildContainingNewEntity) { - return element->addChildAtIndex(childIndex);; + return element->addChildAtIndex(childIndex); } } } diff --git a/libraries/shared/src/OctalCode.cpp b/libraries/shared/src/OctalCode.cpp index 1a0a19bf44..ae4338be6f 100644 --- a/libraries/shared/src/OctalCode.cpp +++ b/libraries/shared/src/OctalCode.cpp @@ -46,7 +46,7 @@ void printOctalCode(const unsigned char* octalCode) { } char sectionValue(const unsigned char* startByte, char startIndexInByte) { - char rightShift = 8 - startIndexInByte - 3; + int8_t rightShift = 8 - startIndexInByte - 3; if (rightShift < 0) { return ((startByte[0] << -rightShift) & 7) + (startByte[1] >> (8 + rightShift)); @@ -73,7 +73,7 @@ int branchIndexWithDescendant(const unsigned char* ancestorOctalCode, const unsi return sectionValue(descendantOctalCode + 1 + (branchStartBit / 8), branchStartBit % 8); } -unsigned char* childOctalCode(const unsigned char* parentOctalCode, char childNumber) { +unsigned char* childOctalCode(const unsigned char* parentOctalCode, int childNumber) { // find the length (in number of three bit code sequences) // in the parent @@ -111,7 +111,7 @@ unsigned char* childOctalCode(const unsigned char* parentOctalCode, char childNu // calculate the amount of left shift required // this will be -1 or -2 if there's wrap - char leftShift = 8 - (startBit % 8) - 3; + int8_t leftShift = 8 - (startBit % 8) - 3; if (leftShift < 0) { // we have a wrap-around to accomodate diff --git a/libraries/shared/src/OctalCode.h b/libraries/shared/src/OctalCode.h index a0d86f32d2..89c5e6d74e 100644 --- a/libraries/shared/src/OctalCode.h +++ b/libraries/shared/src/OctalCode.h @@ -30,7 +30,7 @@ using OctalCodePtrList = std::vector; void printOctalCode(const unsigned char* octalCode); size_t bytesRequiredForCodeLength(unsigned char threeBitCodes); int branchIndexWithDescendant(const unsigned char* ancestorOctalCode, const unsigned char* descendantOctalCode); -unsigned char* childOctalCode(const unsigned char* parentOctalCode, char childNumber); +unsigned char* childOctalCode(const unsigned char* parentOctalCode, int childNumber); const int OVERFLOWED_OCTCODE_BUFFER = -1; const int UNKNOWN_OCTCODE_LENGTH = -2; From 5a7e9d8e3eb185e086f674b5d5984adf3d5e8457 Mon Sep 17 00:00:00 2001 From: SamGondelman Date: Fri, 30 Mar 2018 09:48:04 -0700 Subject: [PATCH 7/7] fix shadows on primitives --- libraries/entities-renderer/src/RenderableShapeEntityItem.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/libraries/entities-renderer/src/RenderableShapeEntityItem.cpp b/libraries/entities-renderer/src/RenderableShapeEntityItem.cpp index b05854da4e..feb88bed4b 100644 --- a/libraries/entities-renderer/src/RenderableShapeEntityItem.cpp +++ b/libraries/entities-renderer/src/RenderableShapeEntityItem.cpp @@ -131,6 +131,8 @@ ItemKey ShapeEntityRenderer::getKey() { withReadLock([&] { if (isTransparent()) { builder.withTransparent(); + } else if (_canCastShadow) { + builder.withShadowCaster(); } });