From aef19c6f9794da3c7be83553812ee88b3790109e Mon Sep 17 00:00:00 2001 From: "Anthony J. Thibault" Date: Tue, 15 Aug 2017 18:28:51 -0700 Subject: [PATCH 01/11] Rig: Use a registry to prevent use after free crashes/corruption Create a global registry to hold all the currently active Rig instances. Use this registry and it's mutex to prevent accessing the rig after it has already been destroyed, or is in the process of being destroyed on the main thread. --- libraries/animation/src/Rig.cpp | 39 +++++++++++++++++++++++++++++++-- libraries/animation/src/Rig.h | 6 +++-- 2 files changed, 41 insertions(+), 4 deletions(-) diff --git a/libraries/animation/src/Rig.cpp b/libraries/animation/src/Rig.cpp index 3a31ccd25f..6222850d52 100644 --- a/libraries/animation/src/Rig.cpp +++ b/libraries/animation/src/Rig.cpp @@ -32,6 +32,10 @@ #include "AnimUtil.h" #include "IKTarget.h" +static int nextRigId = 1; +static std::map rigRegistry; +static std::mutex rigRegistryMutex; + static bool isEqual(const glm::vec3& u, const glm::vec3& v) { const float EPSILON = 0.0001f; return glm::length(u - v) / glm::length(u) <= EPSILON; @@ -49,6 +53,26 @@ const glm::vec3 DEFAULT_RIGHT_EYE_POS(-0.3f, 0.9f, 0.0f); const glm::vec3 DEFAULT_LEFT_EYE_POS(0.3f, 0.9f, 0.0f); const glm::vec3 DEFAULT_HEAD_POS(0.0f, 0.75f, 0.0f); +Rig::Rig() { + // Ensure thread-safe access to the rigRegistry. + std::lock_guard guard(rigRegistryMutex); + + // Insert this newly allocated rig into the rig registry + _rigId = nextRigId; + rigRegistry[_rigId] = this; + nextRigId++; +} + +Rig::~Rig() { + // Ensure thread-safe access to the rigRegstry, but also prevent the rig from being deleted + // while Rig::animationStateHandlerResult is being invoked on a script thread. + std::lock_guard guard(rigRegistryMutex); + auto iter = rigRegistry.find(_rigId); + if (iter != rigRegistry.end()) { + rigRegistry.erase(iter); + } +} + void Rig::overrideAnimation(const QString& url, float fps, bool loop, float firstFrame, float lastFrame) { UserAnimState::ClipNodeEnum clipNodeEnum; @@ -933,8 +957,19 @@ void Rig::updateAnimationStateHandlers() { // called on avatar update thread (wh int identifier = data.key(); StateHandler& value = data.value(); QScriptValue& function = value.function; - auto handleResult = [this, identifier](QScriptValue result) { // called in script thread to get the result back to us. - animationStateHandlerResult(identifier, result); + int rigId = _rigId; + auto handleResult = [rigId, identifier](QScriptValue result) { // called in script thread to get the result back to us. + // Hold the rigRegistryMutex to ensure thread-safe access to the rigRegistry, but + // also to prevent the rig from being deleted while this lambda is being executed. + std::lock_guard guard(rigRegistryMutex); + + // if the rig pointer is in the registry, it has not been deleted yet. + auto iter = rigRegistry.find(rigId); + if (iter != rigRegistry.end()) { + Rig* rig = iter->second; + assert(rig); + rig->animationStateHandlerResult(identifier, result); + } }; // invokeMethod makes a copy of the args, and copies of AnimVariantMap do copy the underlying map, so this will correctly capture // the state of _animVars and allow continued changes to _animVars in this thread without conflict. diff --git a/libraries/animation/src/Rig.h b/libraries/animation/src/Rig.h index ca55635250..eabc62ab75 100644 --- a/libraries/animation/src/Rig.h +++ b/libraries/animation/src/Rig.h @@ -97,8 +97,8 @@ public: Hover }; - Rig() {} - virtual ~Rig() {} + Rig(); + virtual ~Rig(); void destroyAnimGraph(); @@ -372,6 +372,8 @@ protected: glm::vec3 _prevLeftHandPoleVector { -Vectors::UNIT_Z }; bool _prevLeftHandPoleVectorValid { false }; + + int _rigId; }; #endif /* defined(__hifi__Rig__) */ From f56e95225311528b8786d66da33714ecce9b16d2 Mon Sep 17 00:00:00 2001 From: SamGondelman Date: Fri, 18 Aug 2017 14:21:46 -0700 Subject: [PATCH 02/11] properly update image3d render item --- interface/src/ui/overlays/Image3DOverlay.cpp | 17 +++++++++++++++++ interface/src/ui/overlays/Image3DOverlay.h | 1 + 2 files changed, 18 insertions(+) diff --git a/interface/src/ui/overlays/Image3DOverlay.cpp b/interface/src/ui/overlays/Image3DOverlay.cpp index 7dfee2c491..38f1517222 100644 --- a/interface/src/ui/overlays/Image3DOverlay.cpp +++ b/interface/src/ui/overlays/Image3DOverlay.cpp @@ -19,6 +19,7 @@ #include "GeometryUtil.h" +#include "Application.h" QString const Image3DOverlay::TYPE = "image3d"; @@ -58,12 +59,28 @@ void Image3DOverlay::render(RenderArgs* args) { if (!_isLoaded) { _isLoaded = true; _texture = DependencyManager::get()->getTexture(_url); + _textureIsLoaded = false; } if (!_visible || !getParentVisible() || !_texture || !_texture->isLoaded()) { return; } + // Once the texture has loaded, check if we need to update the render item because of transparency + if (!_textureIsLoaded && _texture && _texture->getGPUTexture()) { + _textureIsLoaded = true; + if (_texture->getGPUTexture()->getUsage().isAlpha()) { + auto itemID = getRenderItemID(); + setAlpha(0.5f); + if (render::Item::isValidID(itemID)) { + render::ScenePointer scene = qApp->getMain3DScene(); + render::Transaction transaction; + transaction.updateItem(itemID); + scene->enqueueTransaction(transaction); + } + } + } + Q_ASSERT(args->_batch); gpu::Batch* batch = args->_batch; diff --git a/interface/src/ui/overlays/Image3DOverlay.h b/interface/src/ui/overlays/Image3DOverlay.h index 2e5f74749c..2f4a666058 100644 --- a/interface/src/ui/overlays/Image3DOverlay.h +++ b/interface/src/ui/overlays/Image3DOverlay.h @@ -47,6 +47,7 @@ public: private: QString _url; + bool _textureIsLoaded; NetworkTexturePointer _texture; bool _emissive { false }; From da20eac9955b8d8c7bea06b58b03b6e714076c4c Mon Sep 17 00:00:00 2001 From: "Anthony J. Thibault" Date: Mon, 21 Aug 2017 17:01:21 -0700 Subject: [PATCH 03/11] Bug fix for deadlock between EntitySimulation and EntityItem locks. This is a classic deadlock between the main thread and the OctreeProcessor network thread. On the main thread, the EntitySimulation lock is taken before the EntityItem read lock is taken. On the network thread, the EntityItem write lock is taken before the EntitySimulation lock is taken. To work around this issue the network thread no longer takes the EntitySimulation lock when calling PhysicalEntitySimulation::addDynamic(). Instead, a fine grained lock around the EntitySimulation's dynamic lists is used instead. --- libraries/entities/src/EntitySimulation.cpp | 8 ++++---- libraries/entities/src/EntitySimulation.h | 1 + libraries/physics/src/PhysicalEntitySimulation.cpp | 8 ++++---- 3 files changed, 9 insertions(+), 8 deletions(-) diff --git a/libraries/entities/src/EntitySimulation.cpp b/libraries/entities/src/EntitySimulation.cpp index fbbc1bde71..6a1c359b5a 100644 --- a/libraries/entities/src/EntitySimulation.cpp +++ b/libraries/entities/src/EntitySimulation.cpp @@ -280,24 +280,24 @@ void EntitySimulation::moveSimpleKinematics(const quint64& now) { } void EntitySimulation::addDynamic(EntityDynamicPointer dynamic) { - QMutexLocker lock(&_mutex); + QMutexLocker lock(&_dynamicsMutex); _dynamicsToAdd += dynamic; } void EntitySimulation::removeDynamic(const QUuid dynamicID) { - QMutexLocker lock(&_mutex); + QMutexLocker lock(&_dynamicsMutex); _dynamicsToRemove += dynamicID; } void EntitySimulation::removeDynamics(QList dynamicIDsToRemove) { - QMutexLocker lock(&_mutex); + QMutexLocker lock(&_dynamicsMutex); foreach(QUuid uuid, dynamicIDsToRemove) { _dynamicsToRemove.insert(uuid); } } void EntitySimulation::applyDynamicChanges() { - QMutexLocker lock(&_mutex); + QMutexLocker lock(&_dynamicsMutex); _dynamicsToAdd.clear(); _dynamicsToRemove.clear(); } diff --git a/libraries/entities/src/EntitySimulation.h b/libraries/entities/src/EntitySimulation.h index 84d30c495d..1c633aa9dc 100644 --- a/libraries/entities/src/EntitySimulation.h +++ b/libraries/entities/src/EntitySimulation.h @@ -105,6 +105,7 @@ protected: SetOfEntities _simpleKinematicEntities; // entities undergoing non-colliding kinematic motion QList _dynamicsToAdd; QSet _dynamicsToRemove; + QMutex _dynamicsMutex { QMutex::Recursive }; protected: SetOfEntities _entitiesToDelete; // entities simulation decided needed to be deleted (EntityTree will actually delete) diff --git a/libraries/physics/src/PhysicalEntitySimulation.cpp b/libraries/physics/src/PhysicalEntitySimulation.cpp index fe507ed1ee..b6f460cf39 100644 --- a/libraries/physics/src/PhysicalEntitySimulation.cpp +++ b/libraries/physics/src/PhysicalEntitySimulation.cpp @@ -348,8 +348,7 @@ void PhysicalEntitySimulation::addDynamic(EntityDynamicPointer dynamic) { void PhysicalEntitySimulation::applyDynamicChanges() { QList dynamicsFailedToAdd; if (_physicsEngine) { - // FIXME put fine grain locking into _physicsEngine - QMutexLocker lock(&_mutex); + QMutexLocker lock(&_dynamicsMutex); foreach(QUuid dynamicToRemove, _dynamicsToRemove) { _physicsEngine->removeDynamic(dynamicToRemove); } @@ -360,9 +359,10 @@ void PhysicalEntitySimulation::applyDynamicChanges() { } } } + // applyDynamicChanges will clear _dynamicsToRemove and _dynamicsToAdd + EntitySimulation::applyDynamicChanges(); } - // applyDynamicChanges will clear _dynamicsToRemove and _dynamicsToAdd - EntitySimulation::applyDynamicChanges(); + // put back the ones that couldn't yet be added foreach (EntityDynamicPointer dynamicFailedToAdd, dynamicsFailedToAdd) { addDynamic(dynamicFailedToAdd); From 3a80c6cbe24c2c03c6a8750ba30202c3fd480565 Mon Sep 17 00:00:00 2001 From: SamGondelman Date: Mon, 21 Aug 2017 18:09:25 -0700 Subject: [PATCH 04/11] fix alpha issues, pipeline issue, clamping issue --- interface/src/ui/overlays/Circle3DOverlay.cpp | 2 +- interface/src/ui/overlays/Cube3DOverlay.cpp | 2 +- interface/src/ui/overlays/Image3DOverlay.cpp | 17 +++++++++-------- interface/src/ui/overlays/Image3DOverlay.h | 4 +++- interface/src/ui/overlays/Line3DOverlay.cpp | 15 ++++++++++++--- interface/src/ui/overlays/Line3DOverlay.h | 1 + interface/src/ui/overlays/Overlay.h | 1 + interface/src/ui/overlays/OverlaysPayload.cpp | 2 +- .../src/ui/overlays/Rectangle3DOverlay.cpp | 2 +- interface/src/ui/overlays/Shape3DOverlay.cpp | 2 +- interface/src/ui/overlays/Sphere3DOverlay.cpp | 2 +- interface/src/ui/overlays/Text3DOverlay.cpp | 2 +- interface/src/ui/overlays/Web3DOverlay.cpp | 4 ++-- 13 files changed, 35 insertions(+), 21 deletions(-) diff --git a/interface/src/ui/overlays/Circle3DOverlay.cpp b/interface/src/ui/overlays/Circle3DOverlay.cpp index 52a3d7a929..242021d698 100644 --- a/interface/src/ui/overlays/Circle3DOverlay.cpp +++ b/interface/src/ui/overlays/Circle3DOverlay.cpp @@ -264,7 +264,7 @@ void Circle3DOverlay::render(RenderArgs* args) { const render::ShapeKey Circle3DOverlay::getShapeKey() { auto builder = render::ShapeKey::Builder().withoutCullFace().withUnlit(); - if (getAlpha() != 1.0f) { + if (isTransparent()) { builder.withTranslucent(); } if (!getIsSolid() || shouldDrawHUDLayer()) { diff --git a/interface/src/ui/overlays/Cube3DOverlay.cpp b/interface/src/ui/overlays/Cube3DOverlay.cpp index 31cbe5e822..5ab32d21fe 100644 --- a/interface/src/ui/overlays/Cube3DOverlay.cpp +++ b/interface/src/ui/overlays/Cube3DOverlay.cpp @@ -117,7 +117,7 @@ void Cube3DOverlay::render(RenderArgs* args) { const render::ShapeKey Cube3DOverlay::getShapeKey() { auto builder = render::ShapeKey::Builder(); - if (getAlpha() != 1.0f) { + if (isTransparent()) { builder.withTranslucent(); } if (!getIsSolid() || shouldDrawHUDLayer()) { diff --git a/interface/src/ui/overlays/Image3DOverlay.cpp b/interface/src/ui/overlays/Image3DOverlay.cpp index e6e4e43881..76a82d0268 100644 --- a/interface/src/ui/overlays/Image3DOverlay.cpp +++ b/interface/src/ui/overlays/Image3DOverlay.cpp @@ -19,7 +19,7 @@ #include "GeometryUtil.h" -#include "Application.h" +#include "AbstractViewStateInterface.h" QString const Image3DOverlay::TYPE = "image3d"; @@ -69,11 +69,12 @@ void Image3DOverlay::render(RenderArgs* args) { // Once the texture has loaded, check if we need to update the render item because of transparency if (!_textureIsLoaded && _texture && _texture->getGPUTexture()) { _textureIsLoaded = true; - if (_texture->getGPUTexture()->getUsage().isAlpha()) { + bool prevAlphaTexture = _alphaTexture; + _alphaTexture = _texture->getGPUTexture()->getUsage().isAlpha(); + if (_alphaTexture != prevAlphaTexture) { auto itemID = getRenderItemID(); - setAlpha(0.5f); if (render::Item::isValidID(itemID)) { - render::ScenePointer scene = qApp->getMain3DScene(); + render::ScenePointer scene = AbstractViewStateInterface::instance()->getMain3DScene(); render::Transaction transaction; transaction.updateItem(itemID); scene->enqueueTransaction(transaction); @@ -109,9 +110,9 @@ void Image3DOverlay::render(RenderArgs* args) { glm::vec2 topLeft(-x, -y); glm::vec2 bottomRight(x, y); - glm::vec2 texCoordTopLeft(fromImage.x() / imageWidth, fromImage.y() / imageHeight); - glm::vec2 texCoordBottomRight((fromImage.x() + fromImage.width()) / imageWidth, - (fromImage.y() + fromImage.height()) / imageHeight); + glm::vec2 texCoordTopLeft((fromImage.x() + 0.5f) / imageWidth, (fromImage.y() + 0.5f) / imageHeight); + glm::vec2 texCoordBottomRight((fromImage.x() + fromImage.width() - 0.5f) / imageWidth, + (fromImage.y() + fromImage.height() - 0.5f) / imageHeight); const float MAX_COLOR = 255.0f; xColor color = getColor(); @@ -143,7 +144,7 @@ const render::ShapeKey Image3DOverlay::getShapeKey() { if (_emissive || shouldDrawHUDLayer()) { builder.withUnlit(); } - if (getAlpha() != 1.0f) { + if (isTransparent()) { builder.withTranslucent(); } return builder.build(); diff --git a/interface/src/ui/overlays/Image3DOverlay.h b/interface/src/ui/overlays/Image3DOverlay.h index 2f4a666058..4f813e7368 100644 --- a/interface/src/ui/overlays/Image3DOverlay.h +++ b/interface/src/ui/overlays/Image3DOverlay.h @@ -39,6 +39,7 @@ public: void setProperties(const QVariantMap& properties) override; QVariant getProperty(const QString& property) override; + bool isTransparent() override { return Base3DOverlay::isTransparent() || _alphaTexture; } virtual bool findRayIntersection(const glm::vec3& origin, const glm::vec3& direction, float& distance, BoxFace& face, glm::vec3& surfaceNormal) override; @@ -47,8 +48,9 @@ public: private: QString _url; - bool _textureIsLoaded; NetworkTexturePointer _texture; + bool _textureIsLoaded { false }; + bool _alphaTexture { false }; bool _emissive { false }; QRect _fromImage; // where from in the image to sample diff --git a/interface/src/ui/overlays/Line3DOverlay.cpp b/interface/src/ui/overlays/Line3DOverlay.cpp index d8fe0e6bb8..cc8ed8e1a8 100644 --- a/interface/src/ui/overlays/Line3DOverlay.cpp +++ b/interface/src/ui/overlays/Line3DOverlay.cpp @@ -13,6 +13,7 @@ #include #include +#include "AbstractViewStateInterface.h" QString const Line3DOverlay::TYPE = "line3d"; @@ -149,7 +150,7 @@ void Line3DOverlay::render(RenderArgs* args) { const render::ShapeKey Line3DOverlay::getShapeKey() { auto builder = render::ShapeKey::Builder().withOwnPipeline(); - if (getAlpha() != 1.0f || _glow > 0.0f) { + if (isTransparent()) { builder.withTranslucent(); } return builder.build(); @@ -222,9 +223,17 @@ void Line3DOverlay::setProperties(const QVariantMap& originalProperties) { auto glow = properties["glow"]; if (glow.isValid()) { + float prevGlow = _glow; setGlow(glow.toFloat()); - if (_glow > 0.0f) { - _alpha = 0.5f; + // Update our payload key if necessary to handle transparency + if ((prevGlow <= 0.0f && _glow > 0.0f) || (prevGlow > 0.0f && _glow <= 0.0f)) { + auto itemID = getRenderItemID(); + if (render::Item::isValidID(itemID)) { + render::ScenePointer scene = AbstractViewStateInterface::instance()->getMain3DScene(); + render::Transaction transaction; + transaction.updateItem(itemID); + scene->enqueueTransaction(transaction); + } } } diff --git a/interface/src/ui/overlays/Line3DOverlay.h b/interface/src/ui/overlays/Line3DOverlay.h index c9ceac55a9..9abc2f1a8d 100644 --- a/interface/src/ui/overlays/Line3DOverlay.h +++ b/interface/src/ui/overlays/Line3DOverlay.h @@ -45,6 +45,7 @@ public: void setProperties(const QVariantMap& properties) override; QVariant getProperty(const QString& property) override; + bool isTransparent() override { return Base3DOverlay::isTransparent() || _glow > 0.0f; } virtual Line3DOverlay* createClone() const override; diff --git a/interface/src/ui/overlays/Overlay.h b/interface/src/ui/overlays/Overlay.h index a9774eea06..db2979b4d5 100644 --- a/interface/src/ui/overlays/Overlay.h +++ b/interface/src/ui/overlays/Overlay.h @@ -59,6 +59,7 @@ public: bool isLoaded() { return _isLoaded; } bool getVisible() const { return _visible; } bool shouldDrawHUDLayer() const { return _drawHUDLayer; } + virtual bool isTransparent() { return getAlphaPulse() != 0.0f || getAlpha() != 1.0f; }; xColor getColor(); float getAlpha(); Anchor getAnchor() const { return _anchor; } diff --git a/interface/src/ui/overlays/OverlaysPayload.cpp b/interface/src/ui/overlays/OverlaysPayload.cpp index 8b7100205a..f2684a4368 100644 --- a/interface/src/ui/overlays/OverlaysPayload.cpp +++ b/interface/src/ui/overlays/OverlaysPayload.cpp @@ -37,7 +37,7 @@ namespace render { if (std::static_pointer_cast(overlay)->getDrawInFront()) { builder.withLayered(); } - if (overlay->getAlphaPulse() != 0.0f || overlay->getAlpha() != 1.0f) { + if (overlay->isTransparent()) { builder.withTransparent(); } } else { diff --git a/interface/src/ui/overlays/Rectangle3DOverlay.cpp b/interface/src/ui/overlays/Rectangle3DOverlay.cpp index f981fe1462..22124a0a97 100644 --- a/interface/src/ui/overlays/Rectangle3DOverlay.cpp +++ b/interface/src/ui/overlays/Rectangle3DOverlay.cpp @@ -110,7 +110,7 @@ void Rectangle3DOverlay::render(RenderArgs* args) { const render::ShapeKey Rectangle3DOverlay::getShapeKey() { auto builder = render::ShapeKey::Builder().withOwnPipeline(); - if (getAlpha() != 1.0f) { + if (isTransparent()) { builder.withTranslucent(); } return builder.build(); diff --git a/interface/src/ui/overlays/Shape3DOverlay.cpp b/interface/src/ui/overlays/Shape3DOverlay.cpp index 7126f7bde4..df0ecba307 100644 --- a/interface/src/ui/overlays/Shape3DOverlay.cpp +++ b/interface/src/ui/overlays/Shape3DOverlay.cpp @@ -62,7 +62,7 @@ void Shape3DOverlay::render(RenderArgs* args) { const render::ShapeKey Shape3DOverlay::getShapeKey() { auto builder = render::ShapeKey::Builder(); - if (getAlpha() != 1.0f) { + if (isTransparent()) { builder.withTranslucent(); } if (!getIsSolid() || shouldDrawHUDLayer()) { diff --git a/interface/src/ui/overlays/Sphere3DOverlay.cpp b/interface/src/ui/overlays/Sphere3DOverlay.cpp index ee3f9b9784..9309316d6e 100644 --- a/interface/src/ui/overlays/Sphere3DOverlay.cpp +++ b/interface/src/ui/overlays/Sphere3DOverlay.cpp @@ -59,7 +59,7 @@ void Sphere3DOverlay::render(RenderArgs* args) { const render::ShapeKey Sphere3DOverlay::getShapeKey() { auto builder = render::ShapeKey::Builder(); - if (getAlpha() != 1.0f) { + if (isTransparent()) { builder.withTranslucent(); } if (!getIsSolid() || shouldDrawHUDLayer()) { diff --git a/interface/src/ui/overlays/Text3DOverlay.cpp b/interface/src/ui/overlays/Text3DOverlay.cpp index 4b110b8099..bb8c24aa11 100644 --- a/interface/src/ui/overlays/Text3DOverlay.cpp +++ b/interface/src/ui/overlays/Text3DOverlay.cpp @@ -143,7 +143,7 @@ void Text3DOverlay::render(RenderArgs* args) { const render::ShapeKey Text3DOverlay::getShapeKey() { auto builder = render::ShapeKey::Builder(); - if (getAlpha() != 1.0f) { + if (isTransparent()) { builder.withTranslucent(); } return builder.build(); diff --git a/interface/src/ui/overlays/Web3DOverlay.cpp b/interface/src/ui/overlays/Web3DOverlay.cpp index 40af679a13..5acb4d2ceb 100644 --- a/interface/src/ui/overlays/Web3DOverlay.cpp +++ b/interface/src/ui/overlays/Web3DOverlay.cpp @@ -318,8 +318,8 @@ void Web3DOverlay::render(RenderArgs* args) { } const render::ShapeKey Web3DOverlay::getShapeKey() { - auto builder = render::ShapeKey::Builder().withoutCullFace().withDepthBias(); - if (getAlpha() != 1.0f) { + auto builder = render::ShapeKey::Builder().withoutCullFace().withDepthBias().withOwnPipeline(); + if (isTransparent()) { builder.withTranslucent(); } return builder.build(); From a904ae1a8ad74e87c78fea9818ba333b0abedba8 Mon Sep 17 00:00:00 2001 From: SamGondelman Date: Tue, 22 Aug 2017 11:41:20 -0700 Subject: [PATCH 05/11] can set precision picking on lasers/raypicks, EntityTreeRender uses ray pick API --- interface/src/Application.cpp | 19 ++++++++ interface/src/raypick/LaserPointer.h | 1 + interface/src/raypick/LaserPointerManager.cpp | 8 ++++ interface/src/raypick/LaserPointerManager.h | 1 + .../raypick/LaserPointerScriptingInterface.h | 1 + interface/src/raypick/RayPick.h | 4 ++ interface/src/raypick/RayPickManager.cpp | 14 +++++- interface/src/raypick/RayPickManager.h | 1 + .../src/raypick/RayPickScriptingInterface.cpp | 4 ++ .../src/raypick/RayPickScriptingInterface.h | 2 +- .../src/EntityTreeRenderer.cpp | 43 +++---------------- .../src/EntityTreeRenderer.h | 15 ++++--- 12 files changed, 68 insertions(+), 45 deletions(-) diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp index 398b2dbdb4..8d7892c4cc 100644 --- a/interface/src/Application.cpp +++ b/interface/src/Application.cpp @@ -1735,6 +1735,25 @@ Application::Application(int& argc, char** argv, QElapsedTimer& startupTimer, bo connect(&_myCamera, &Camera::modeUpdated, this, &Application::cameraModeChanged); + // Setup the mouse ray pick and related operators + DependencyManager::get()->setMouseRayPickID(_rayPickManager.createRayPick(RayPickFilter(RayPickFilter::PICK_ENTITIES), 0.0f, true)); + DependencyManager::get()->setMouseRayPickResultOperator([&](QUuid rayPickID) { + RayToEntityIntersectionResult entityResult; + RayPickResult result = _rayPickManager.getPrevRayPickResult(rayPickID); + entityResult.intersects = result.type != DependencyManager::get()->INTERSECTED_NONE(); + if (entityResult.intersects) { + entityResult.intersection = result.intersection; + entityResult.distance = result.distance; + entityResult.surfaceNormal = result.surfaceNormal; + entityResult.entityID = result.objectID; + entityResult.entity = DependencyManager::get()->getTree()->findEntityByID(entityResult.entityID); + } + return entityResult; + }); + DependencyManager::get()->setSetPrecisionPickingOperator([&](QUuid rayPickID, bool value) { + _rayPickManager.setPrecisionPicking(rayPickID, value); + }); + qCDebug(interfaceapp) << "Metaverse session ID is" << uuidStringWithoutCurlyBraces(accountManager->getSessionID()); } diff --git a/interface/src/raypick/LaserPointer.h b/interface/src/raypick/LaserPointer.h index d901d12cf4..23f023cf7a 100644 --- a/interface/src/raypick/LaserPointer.h +++ b/interface/src/raypick/LaserPointer.h @@ -64,6 +64,7 @@ public: // You cannot use editRenderState to change the overlay type of any part of the laser pointer. You can only edit the properties of the existing overlays. void editRenderState(const std::string& state, const QVariant& startProps, const QVariant& pathProps, const QVariant& endProps); + void setPrecisionPicking(const bool precisionPicking) { DependencyManager::get()->setPrecisionPicking(_rayPickUID, precisionPicking); } void setIgnoreEntities(const QScriptValue& ignoreEntities) { DependencyManager::get()->setIgnoreEntities(_rayPickUID, ignoreEntities); } void setIncludeEntities(const QScriptValue& includeEntities) { DependencyManager::get()->setIncludeEntities(_rayPickUID, includeEntities); } void setIgnoreOverlays(const QScriptValue& ignoreOverlays) { DependencyManager::get()->setIgnoreOverlays(_rayPickUID, ignoreOverlays); } diff --git a/interface/src/raypick/LaserPointerManager.cpp b/interface/src/raypick/LaserPointerManager.cpp index 908bcc39f1..1fd0fe9e88 100644 --- a/interface/src/raypick/LaserPointerManager.cpp +++ b/interface/src/raypick/LaserPointerManager.cpp @@ -97,6 +97,14 @@ void LaserPointerManager::update() { } } +void LaserPointerManager::setPrecisionPicking(QUuid uid, const bool precisionPicking) { + QReadLocker lock(&_containsLock); + if (_laserPointers.contains(uid)) { + QWriteLocker laserLock(_laserPointerLocks[uid].get()); + _laserPointers[uid]->setPrecisionPicking(precisionPicking); + } +} + void LaserPointerManager::setIgnoreEntities(QUuid uid, const QScriptValue& ignoreEntities) { QReadLocker lock(&_containsLock); if (_laserPointers.contains(uid)) { diff --git a/interface/src/raypick/LaserPointerManager.h b/interface/src/raypick/LaserPointerManager.h index 020b778983..b573410fe7 100644 --- a/interface/src/raypick/LaserPointerManager.h +++ b/interface/src/raypick/LaserPointerManager.h @@ -31,6 +31,7 @@ public: void editRenderState(QUuid uid, const std::string& state, const QVariant& startProps, const QVariant& pathProps, const QVariant& endProps); const RayPickResult getPrevRayPickResult(const QUuid uid); + void setPrecisionPicking(QUuid uid, const bool precisionPicking); void setIgnoreEntities(QUuid uid, const QScriptValue& ignoreEntities); void setIncludeEntities(QUuid uid, const QScriptValue& includeEntities); void setIgnoreOverlays(QUuid uid, const QScriptValue& ignoreOverlays); diff --git a/interface/src/raypick/LaserPointerScriptingInterface.h b/interface/src/raypick/LaserPointerScriptingInterface.h index 8ae263b0ec..d65eb335b3 100644 --- a/interface/src/raypick/LaserPointerScriptingInterface.h +++ b/interface/src/raypick/LaserPointerScriptingInterface.h @@ -30,6 +30,7 @@ public slots: Q_INVOKABLE void setRenderState(QUuid uid, const QString& renderState) { qApp->getLaserPointerManager().setRenderState(uid, renderState.toStdString()); } Q_INVOKABLE RayPickResult getPrevRayPickResult(QUuid uid) { return qApp->getLaserPointerManager().getPrevRayPickResult(uid); } + Q_INVOKABLE void setPrecisionPicking(QUuid uid, const bool precisionPicking) { qApp->getLaserPointerManager().setPrecisionPicking(uid, precisionPicking); } Q_INVOKABLE void setIgnoreEntities(QUuid uid, const QScriptValue& ignoreEntities) { qApp->getLaserPointerManager().setIgnoreEntities(uid, ignoreEntities); } Q_INVOKABLE void setIncludeEntities(QUuid uid, const QScriptValue& includeEntities) { qApp->getLaserPointerManager().setIncludeEntities(uid, includeEntities); } Q_INVOKABLE void setIgnoreOverlays(QUuid uid, const QScriptValue& ignoreOverlays) { qApp->getLaserPointerManager().setIgnoreOverlays(uid, ignoreOverlays); } diff --git a/interface/src/raypick/RayPick.h b/interface/src/raypick/RayPick.h index e108145d55..3b973358e9 100644 --- a/interface/src/raypick/RayPick.h +++ b/interface/src/raypick/RayPick.h @@ -102,6 +102,9 @@ public: const bool& isEnabled() { return _enabled; } const RayPickResult& getPrevRayPickResult() { return _prevResult; } + void setPrecisionPicking(bool precisionPicking) { _precisionPicking = precisionPicking; } + const bool& doesPrecisionPicking() { return _precisionPicking; } + void setRayPickResult(const RayPickResult& rayPickResult) { _prevResult = rayPickResult; } const QVector& getIgnoreEntites() { return _ignoreEntities; } @@ -123,6 +126,7 @@ private: bool _enabled; RayPickResult _prevResult; + bool _precisionPicking { true }; QVector _ignoreEntities; QVector _includeEntities; QVector _ignoreOverlays; diff --git a/interface/src/raypick/RayPickManager.cpp b/interface/src/raypick/RayPickManager.cpp index 8b639d4a81..971b08c82e 100644 --- a/interface/src/raypick/RayPickManager.cpp +++ b/interface/src/raypick/RayPickManager.cpp @@ -67,7 +67,8 @@ void RayPickManager::update() { bool noncollidable = rayPick->getFilter().doesPickNonCollidable(); RayPickFilter::Flags entityMask = rayPick->getFilter().getEntityFlags(); if (!checkAndCompareCachedResults(rayKey, results, res, entityMask)) { - entityRes = DependencyManager::get()->findRayIntersectionVector(ray, true, rayPick->getIncludeEntites(), rayPick->getIgnoreEntites(), !invisible, !noncollidable); + entityRes = DependencyManager::get()->findRayIntersectionVector(ray, rayPick->doesPrecisionPicking(), + rayPick->getIncludeEntites(), rayPick->getIgnoreEntites(), !invisible, !noncollidable); fromCache = false; } @@ -84,7 +85,8 @@ void RayPickManager::update() { bool noncollidable = rayPick->getFilter().doesPickNonCollidable(); RayPickFilter::Flags overlayMask = rayPick->getFilter().getOverlayFlags(); if (!checkAndCompareCachedResults(rayKey, results, res, overlayMask)) { - overlayRes = qApp->getOverlays().findRayIntersectionVector(ray, true, rayPick->getIncludeOverlays(), rayPick->getIgnoreOverlays(), !invisible, !noncollidable); + overlayRes = qApp->getOverlays().findRayIntersectionVector(ray, rayPick->doesPrecisionPicking(), + rayPick->getIncludeOverlays(), rayPick->getIgnoreOverlays(), !invisible, !noncollidable); fromCache = false; } @@ -204,6 +206,14 @@ const RayPickResult RayPickManager::getPrevRayPickResult(const QUuid uid) { return RayPickResult(); } +void RayPickManager::setPrecisionPicking(QUuid uid, const bool precisionPicking) { + QReadLocker containsLock(&_containsLock); + if (_rayPicks.contains(uid)) { + QWriteLocker lock(_rayPickLocks[uid].get()); + _rayPicks[uid]->setPrecisionPicking(precisionPicking); + } +} + void RayPickManager::setIgnoreEntities(QUuid uid, const QScriptValue& ignoreEntities) { QReadLocker containsLock(&_containsLock); if (_rayPicks.contains(uid)) { diff --git a/interface/src/raypick/RayPickManager.h b/interface/src/raypick/RayPickManager.h index 8ef2ceebe0..f2b1ff4ae4 100644 --- a/interface/src/raypick/RayPickManager.h +++ b/interface/src/raypick/RayPickManager.h @@ -38,6 +38,7 @@ public: void disableRayPick(const QUuid uid); const RayPickResult getPrevRayPickResult(const QUuid uid); + void setPrecisionPicking(QUuid uid, const bool precisionPicking); void setIgnoreEntities(QUuid uid, const QScriptValue& ignoreEntities); void setIncludeEntities(QUuid uid, const QScriptValue& includeEntities); void setIgnoreOverlays(QUuid uid, const QScriptValue& ignoreOverlays); diff --git a/interface/src/raypick/RayPickScriptingInterface.cpp b/interface/src/raypick/RayPickScriptingInterface.cpp index aa1613d696..015f8fd7d1 100644 --- a/interface/src/raypick/RayPickScriptingInterface.cpp +++ b/interface/src/raypick/RayPickScriptingInterface.cpp @@ -82,6 +82,10 @@ RayPickResult RayPickScriptingInterface::getPrevRayPickResult(QUuid uid) { return qApp->getRayPickManager().getPrevRayPickResult(uid); } +void RayPickScriptingInterface::setPrecisionPicking(QUuid uid, const bool precisionPicking) { + qApp->getRayPickManager().setPrecisionPicking(uid, precisionPicking); +} + void RayPickScriptingInterface::setIgnoreEntities(QUuid uid, const QScriptValue& ignoreEntities) { qApp->getRayPickManager().setIgnoreEntities(uid, ignoreEntities); } diff --git a/interface/src/raypick/RayPickScriptingInterface.h b/interface/src/raypick/RayPickScriptingInterface.h index e576b5d076..f7ed2e6fa6 100644 --- a/interface/src/raypick/RayPickScriptingInterface.h +++ b/interface/src/raypick/RayPickScriptingInterface.h @@ -43,6 +43,7 @@ public slots: Q_INVOKABLE void removeRayPick(QUuid uid); Q_INVOKABLE RayPickResult getPrevRayPickResult(QUuid uid); + Q_INVOKABLE void setPrecisionPicking(QUuid uid, const bool precisionPicking); Q_INVOKABLE void setIgnoreEntities(QUuid uid, const QScriptValue& ignoreEntities); Q_INVOKABLE void setIncludeEntities(QUuid uid, const QScriptValue& includeEntities); Q_INVOKABLE void setIgnoreOverlays(QUuid uid, const QScriptValue& ignoreOverlays); @@ -50,7 +51,6 @@ public slots: Q_INVOKABLE void setIgnoreAvatars(QUuid uid, const QScriptValue& ignoreAvatars); Q_INVOKABLE void setIncludeAvatars(QUuid uid, const QScriptValue& includeAvatars); -private: unsigned int PICK_NOTHING() { return RayPickFilter::getBitMask(RayPickFilter::FlagBit::PICK_NOTHING); } unsigned int PICK_ENTITIES() { return RayPickFilter::getBitMask(RayPickFilter::FlagBit::PICK_ENTITIES); } unsigned int PICK_OVERLAYS() { return RayPickFilter::getBitMask(RayPickFilter::FlagBit::PICK_OVERLAYS); } diff --git a/libraries/entities-renderer/src/EntityTreeRenderer.cpp b/libraries/entities-renderer/src/EntityTreeRenderer.cpp index a8eca41077..a2d2d34837 100644 --- a/libraries/entities-renderer/src/EntityTreeRenderer.cpp +++ b/libraries/entities-renderer/src/EntityTreeRenderer.cpp @@ -53,7 +53,6 @@ EntityTreeRenderer::EntityTreeRenderer(bool wantScripts, AbstractViewStateInterf _viewState(viewState), _scriptingServices(scriptingServices), _displayModelBounds(false), - _dontDoPrecisionPicking(false), _layeredZones(this) { REGISTER_ENTITY_TYPE_WITH_FACTORY(Model, RenderableModelEntityItem::factory) @@ -428,28 +427,6 @@ void EntityTreeRenderer::deleteReleasedModels() { } } -RayToEntityIntersectionResult EntityTreeRenderer::findRayIntersectionWorker(const PickRay& ray, Octree::lockType lockType, - bool precisionPicking, const QVector& entityIdsToInclude, - const QVector& entityIdsToDiscard, bool visibleOnly, bool collidableOnly) { - RayToEntityIntersectionResult result; - if (_tree) { - EntityTreePointer entityTree = std::static_pointer_cast(_tree); - - OctreeElementPointer element; - EntityItemPointer intersectedEntity = NULL; - result.intersects = entityTree->findRayIntersection(ray.origin, ray.direction, - entityIdsToInclude, entityIdsToDiscard, visibleOnly, collidableOnly, precisionPicking, - element, result.distance, result.face, result.surfaceNormal, - (void**)&intersectedEntity, lockType, &result.accurate); - if (result.intersects && intersectedEntity) { - result.entityID = intersectedEntity->getEntityItemID(); - result.intersection = ray.origin + (ray.direction * result.distance); - result.entity = intersectedEntity; - } - } - return result; -} - void EntityTreeRenderer::connectSignalsToSlots(EntityScriptingInterface* entityScriptingInterface) { connect(this, &EntityTreeRenderer::mousePressOnEntity, entityScriptingInterface, &EntityScriptingInterface::mousePressOnEntity); @@ -530,11 +507,10 @@ void EntityTreeRenderer::mousePressEvent(QMouseEvent* event) { if (!_tree || _shuttingDown) { return; } + PerformanceTimer perfTimer("EntityTreeRenderer::mousePressEvent"); PickRay ray = _viewState->computePickRay(event->x(), event->y()); - - bool precisionPicking = !_dontDoPrecisionPicking; - RayToEntityIntersectionResult rayPickResult = findRayIntersectionWorker(ray, Octree::Lock, precisionPicking); + RayToEntityIntersectionResult rayPickResult = _getPrevRayPickResultOperator(_mouseRayPickID); if (rayPickResult.intersects) { //qCDebug(entitiesrenderer) << "mousePressEvent over entity:" << rayPickResult.entityID; @@ -579,11 +555,10 @@ void EntityTreeRenderer::mouseDoublePressEvent(QMouseEvent* event) { if (!_tree || _shuttingDown) { return; } + PerformanceTimer perfTimer("EntityTreeRenderer::mouseDoublePressEvent"); PickRay ray = _viewState->computePickRay(event->x(), event->y()); - - bool precisionPicking = !_dontDoPrecisionPicking; - RayToEntityIntersectionResult rayPickResult = findRayIntersectionWorker(ray, Octree::Lock, precisionPicking); + RayToEntityIntersectionResult rayPickResult = _getPrevRayPickResultOperator(_mouseRayPickID); if (rayPickResult.intersects) { //qCDebug(entitiesrenderer) << "mouseDoublePressEvent over entity:" << rayPickResult.entityID; @@ -622,8 +597,7 @@ void EntityTreeRenderer::mouseReleaseEvent(QMouseEvent* event) { PerformanceTimer perfTimer("EntityTreeRenderer::mouseReleaseEvent"); PickRay ray = _viewState->computePickRay(event->x(), event->y()); - bool precisionPicking = !_dontDoPrecisionPicking; - RayToEntityIntersectionResult rayPickResult = findRayIntersectionWorker(ray, Octree::Lock, precisionPicking); + RayToEntityIntersectionResult rayPickResult = _getPrevRayPickResultOperator(_mouseRayPickID); if (rayPickResult.intersects) { //qCDebug(entitiesrenderer) << "mouseReleaseEvent over entity:" << rayPickResult.entityID; @@ -671,14 +645,11 @@ void EntityTreeRenderer::mouseMoveEvent(QMouseEvent* event) { if (!_tree || _shuttingDown) { return; } + PerformanceTimer perfTimer("EntityTreeRenderer::mouseMoveEvent"); - PickRay ray = _viewState->computePickRay(event->x(), event->y()); - - bool precisionPicking = false; // for mouse moves we do not do precision picking - RayToEntityIntersectionResult rayPickResult = findRayIntersectionWorker(ray, Octree::TryLock, precisionPicking); + RayToEntityIntersectionResult rayPickResult = _getPrevRayPickResultOperator(_mouseRayPickID); if (rayPickResult.intersects) { - glm::vec2 pos2D = projectOntoEntityXYPlane(rayPickResult.entity, ray, rayPickResult); PointerEvent pointerEvent(PointerEvent::Move, MOUSE_POINTER_ID, pos2D, rayPickResult.intersection, diff --git a/libraries/entities-renderer/src/EntityTreeRenderer.h b/libraries/entities-renderer/src/EntityTreeRenderer.h index f4909a2036..b70252d68c 100644 --- a/libraries/entities-renderer/src/EntityTreeRenderer.h +++ b/libraries/entities-renderer/src/EntityTreeRenderer.h @@ -59,6 +59,10 @@ public: float getEntityLoadingPriority(const EntityItem& item) const { return _calculateEntityLoadingPriorityFunc(item); } void setEntityLoadingPriorityFunction(CalculateEntityLoadingPriority fn) { this->_calculateEntityLoadingPriorityFunc = fn; } + void setMouseRayPickID(QUuid rayPickID) { _mouseRayPickID = rayPickID; } + void setMouseRayPickResultOperator(std::function getPrevRayPickResultOperator) { _getPrevRayPickResultOperator = getPrevRayPickResultOperator; } + void setSetPrecisionPickingOperator(std::function setPrecisionPickingOperator) { _setPrecisionPickingOperator = setPrecisionPickingOperator; } + void shutdown(); void update(); @@ -130,7 +134,7 @@ public slots: // optional slots that can be wired to menu items void setDisplayModelBounds(bool value) { _displayModelBounds = value; } - void setDontDoPrecisionPicking(bool value) { _dontDoPrecisionPicking = value; } + void setPrecisionPicking(bool value) { _setPrecisionPickingOperator(_mouseRayPickID, value); } protected: virtual OctreePointer createTree() override { @@ -150,10 +154,6 @@ private: void checkAndCallPreload(const EntityItemID& entityID, bool reload = false, bool unloadFirst = false); QList _releasedModels; - RayToEntityIntersectionResult findRayIntersectionWorker(const PickRay& ray, Octree::lockType lockType, - bool precisionPicking, const QVector& entityIdsToInclude = QVector(), - const QVector& entityIdsToDiscard = QVector(), bool visibleOnly=false, - bool collidableOnly = false); EntityItemID _currentHoverOverEntityID; EntityItemID _currentClickingOnEntityID; @@ -176,12 +176,15 @@ private: AbstractViewStateInterface* _viewState; AbstractScriptingServicesInterface* _scriptingServices; bool _displayModelBounds; - bool _dontDoPrecisionPicking; bool _shuttingDown { false }; QMultiMap _waitingOnPreload; + QUuid _mouseRayPickID; + std::function _getPrevRayPickResultOperator; + std::function _setPrecisionPickingOperator; + class LayeredZone { public: LayeredZone(std::shared_ptr zone, QUuid id, float volume) : zone(zone), id(id), volume(volume) {} From b8a7fce2011adcdff1077f4b66614fa4e6925aff Mon Sep 17 00:00:00 2001 From: SamGondelman Date: Tue, 22 Aug 2017 11:54:55 -0700 Subject: [PATCH 06/11] boop --- interface/src/raypick/RayPick.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/interface/src/raypick/RayPick.h b/interface/src/raypick/RayPick.h index 3b973358e9..17969fd9a6 100644 --- a/interface/src/raypick/RayPick.h +++ b/interface/src/raypick/RayPick.h @@ -98,12 +98,12 @@ public: void disable() { _enabled = false; } const RayPickFilter& getFilter() { return _filter; } - const float& getMaxDistance() { return _maxDistance; } - const bool& isEnabled() { return _enabled; } + float getMaxDistance() { return _maxDistance; } + bool isEnabled() { return _enabled; } const RayPickResult& getPrevRayPickResult() { return _prevResult; } void setPrecisionPicking(bool precisionPicking) { _precisionPicking = precisionPicking; } - const bool& doesPrecisionPicking() { return _precisionPicking; } + bool doesPrecisionPicking() { return _precisionPicking; } void setRayPickResult(const RayPickResult& rayPickResult) { _prevResult = rayPickResult; } From d7b3686364b0995d690cb67c72e74c1b88947b38 Mon Sep 17 00:00:00 2001 From: SamGondelman Date: Tue, 22 Aug 2017 17:37:51 -0700 Subject: [PATCH 07/11] include noncollidable entities --- interface/src/Application.cpp | 4 +++- interface/src/raypick/RayPick.h | 6 +++--- interface/src/raypick/RayPickManager.cpp | 12 ++++++------ scripts/system/controllers/handControllerGrab.js | 4 ++-- 4 files changed, 14 insertions(+), 12 deletions(-) diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp index 8d7892c4cc..d3b547a54c 100644 --- a/interface/src/Application.cpp +++ b/interface/src/Application.cpp @@ -1736,7 +1736,9 @@ Application::Application(int& argc, char** argv, QElapsedTimer& startupTimer, bo connect(&_myCamera, &Camera::modeUpdated, this, &Application::cameraModeChanged); // Setup the mouse ray pick and related operators - DependencyManager::get()->setMouseRayPickID(_rayPickManager.createRayPick(RayPickFilter(RayPickFilter::PICK_ENTITIES), 0.0f, true)); + DependencyManager::get()->setMouseRayPickID(_rayPickManager.createRayPick( + RayPickFilter(DependencyManager::get()->PICK_ENTITIES() | DependencyManager::get()->PICK_INCLUDE_NONCOLLIDABLE()), + 0.0f, true)); DependencyManager::get()->setMouseRayPickResultOperator([&](QUuid rayPickID) { RayToEntityIntersectionResult entityResult; RayPickResult result = _rayPickManager.getPrevRayPickResult(rayPickID); diff --git a/interface/src/raypick/RayPick.h b/interface/src/raypick/RayPick.h index 17969fd9a6..188779bb83 100644 --- a/interface/src/raypick/RayPick.h +++ b/interface/src/raypick/RayPick.h @@ -47,6 +47,8 @@ public: bool operator== (const RayPickFilter& rhs) const { return _flags == rhs._flags; } bool operator!= (const RayPickFilter& rhs) const { return _flags != rhs._flags; } + void setFlag(FlagBit flag, bool value) { _flags[flag] = value; } + bool doesPickNothing() const { return _flags[PICK_NOTHING]; } bool doesPickEntities() const { return _flags[PICK_ENTITIES]; } bool doesPickOverlays() const { return _flags[PICK_OVERLAYS]; } @@ -102,8 +104,7 @@ public: bool isEnabled() { return _enabled; } const RayPickResult& getPrevRayPickResult() { return _prevResult; } - void setPrecisionPicking(bool precisionPicking) { _precisionPicking = precisionPicking; } - bool doesPrecisionPicking() { return _precisionPicking; } + void setPrecisionPicking(bool precisionPicking) { _filter.setFlag(RayPickFilter::PICK_COURSE, !precisionPicking); } void setRayPickResult(const RayPickResult& rayPickResult) { _prevResult = rayPickResult; } @@ -126,7 +127,6 @@ private: bool _enabled; RayPickResult _prevResult; - bool _precisionPicking { true }; QVector _ignoreEntities; QVector _includeEntities; QVector _ignoreOverlays; diff --git a/interface/src/raypick/RayPickManager.cpp b/interface/src/raypick/RayPickManager.cpp index 971b08c82e..1ec8efc331 100644 --- a/interface/src/raypick/RayPickManager.cpp +++ b/interface/src/raypick/RayPickManager.cpp @@ -64,11 +64,11 @@ void RayPickManager::update() { RayToEntityIntersectionResult entityRes; bool fromCache = true; bool invisible = rayPick->getFilter().doesPickInvisible(); - bool noncollidable = rayPick->getFilter().doesPickNonCollidable(); + bool nonCollidable = rayPick->getFilter().doesPickNonCollidable(); RayPickFilter::Flags entityMask = rayPick->getFilter().getEntityFlags(); if (!checkAndCompareCachedResults(rayKey, results, res, entityMask)) { - entityRes = DependencyManager::get()->findRayIntersectionVector(ray, rayPick->doesPrecisionPicking(), - rayPick->getIncludeEntites(), rayPick->getIgnoreEntites(), !invisible, !noncollidable); + entityRes = DependencyManager::get()->findRayIntersectionVector(ray, !rayPick->getFilter().doesPickCourse(), + rayPick->getIncludeEntites(), rayPick->getIgnoreEntites(), !invisible, !nonCollidable); fromCache = false; } @@ -82,11 +82,11 @@ void RayPickManager::update() { RayToOverlayIntersectionResult overlayRes; bool fromCache = true; bool invisible = rayPick->getFilter().doesPickInvisible(); - bool noncollidable = rayPick->getFilter().doesPickNonCollidable(); + bool nonCollidable = rayPick->getFilter().doesPickNonCollidable(); RayPickFilter::Flags overlayMask = rayPick->getFilter().getOverlayFlags(); if (!checkAndCompareCachedResults(rayKey, results, res, overlayMask)) { - overlayRes = qApp->getOverlays().findRayIntersectionVector(ray, rayPick->doesPrecisionPicking(), - rayPick->getIncludeOverlays(), rayPick->getIgnoreOverlays(), !invisible, !noncollidable); + overlayRes = qApp->getOverlays().findRayIntersectionVector(ray, !rayPick->getFilter().doesPickCourse(), + rayPick->getIncludeOverlays(), rayPick->getIgnoreOverlays(), !invisible, !nonCollidable); fromCache = false; } diff --git a/scripts/system/controllers/handControllerGrab.js b/scripts/system/controllers/handControllerGrab.js index 9318873f64..f6eec7ab76 100644 --- a/scripts/system/controllers/handControllerGrab.js +++ b/scripts/system/controllers/handControllerGrab.js @@ -1182,7 +1182,7 @@ function MyController(hand) { this.fullEnd = fullEnd; this.laserPointer = LaserPointers.createLaserPointer({ joint: (hand == RIGHT_HAND) ? "_CAMERA_RELATIVE_CONTROLLER_RIGHTHAND" : "_CAMERA_RELATIVE_CONTROLLER_LEFTHAND", - filter: RayPick.PICK_ENTITIES | RayPick.PICK_OVERLAYS, + filter: RayPick.PICK_ENTITIES | RayPick.PICK_OVERLAYS | RayPick.PICK_INCLUDE_NONCOLLIDABLE, maxDistance: PICK_MAX_DISTANCE, posOffset: getGrabPointSphereOffset(this.handToController()), renderStates: renderStates, @@ -1191,7 +1191,7 @@ function MyController(hand) { }); this.headLaserPointer = LaserPointers.createLaserPointer({ joint: "Avatar", - filter: RayPick.PICK_ENTITIES | RayPick.PICK_OVERLAYS, + filter: RayPick.PICK_ENTITIES | RayPick.PICK_OVERLAYS | RayPick.PICK_INCLUDE_NONCOLLIDABLE, maxDistance: PICK_MAX_DISTANCE, renderStates: headRenderStates, faceAvatar: true, From 206862a90f813d343907c881501e0dd7fb026ace Mon Sep 17 00:00:00 2001 From: SamGondelman Date: Wed, 23 Aug 2017 12:20:30 -0700 Subject: [PATCH 08/11] fix flag bits --- interface/src/raypick/RayPick.h | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/interface/src/raypick/RayPick.h b/interface/src/raypick/RayPick.h index 188779bb83..04045b2116 100644 --- a/interface/src/raypick/RayPick.h +++ b/interface/src/raypick/RayPick.h @@ -41,7 +41,7 @@ public: // The key is the Flags Flags _flags; - RayPickFilter() : _flags(PICK_NOTHING) {} + RayPickFilter() : _flags(getBitMask(PICK_NOTHING)) {} RayPickFilter(const Flags& flags) : _flags(flags) {} bool operator== (const RayPickFilter& rhs) const { return _flags == rhs._flags; } @@ -63,27 +63,27 @@ public: // Helpers for RayPickManager Flags getEntityFlags() const { - Flags toReturn(PICK_ENTITIES); + unsigned int toReturn = getBitMask(PICK_ENTITIES); if (doesPickInvisible()) { - toReturn |= Flags(PICK_INCLUDE_INVISIBLE); + toReturn |= getBitMask(PICK_INCLUDE_INVISIBLE); } if (doesPickNonCollidable()) { - toReturn |= Flags(PICK_INCLUDE_NONCOLLIDABLE); + toReturn |= getBitMask(PICK_INCLUDE_NONCOLLIDABLE); } - return toReturn; + return Flags(toReturn); } Flags getOverlayFlags() const { - Flags toReturn(PICK_OVERLAYS); + unsigned int toReturn = getBitMask(PICK_OVERLAYS); if (doesPickInvisible()) { - toReturn |= Flags(PICK_INCLUDE_INVISIBLE); + toReturn |= getBitMask(PICK_INCLUDE_INVISIBLE); } if (doesPickNonCollidable()) { - toReturn |= Flags(PICK_INCLUDE_NONCOLLIDABLE); + toReturn |= getBitMask(PICK_INCLUDE_NONCOLLIDABLE); } - return toReturn; + return Flags(toReturn); } - Flags getAvatarFlags() const { return Flags(PICK_AVATARS); } - Flags getHUDFlags() const { return Flags(PICK_HUD); } + Flags getAvatarFlags() const { return Flags(getBitMask(PICK_AVATARS)); } + Flags getHUDFlags() const { return Flags(getBitMask(PICK_HUD)); } static unsigned int getBitMask(FlagBit bit) { return 1 << bit; } From 9cd784990cbb445a7afb9d1bb4a9ca16c83ee42f Mon Sep 17 00:00:00 2001 From: beholder Date: Wed, 23 Aug 2017 23:27:02 +0300 Subject: [PATCH 09/11] 5830: Go-To menu shows up blank except for address bar --- interface/resources/qml/hifi/Feed.qml | 7 ++----- .../qml/hifi/tablet/TabletAddressDialog.qml | 13 +++++++++++++ scripts/system/tablet-goto.js | 4 ++++ 3 files changed, 19 insertions(+), 5 deletions(-) diff --git a/interface/resources/qml/hifi/Feed.qml b/interface/resources/qml/hifi/Feed.qml index 960b62c169..8576e26fcd 100644 --- a/interface/resources/qml/hifi/Feed.qml +++ b/interface/resources/qml/hifi/Feed.qml @@ -35,7 +35,6 @@ Column { property string metaverseServerUrl: ''; property string actions: 'snapshot'; // sendToScript doesn't get wired until after everything gets created. So we have to queue fillDestinations on nextTick. - Component.onCompleted: delay.start(); property string labelText: actions; property string filter: ''; onFilterChanged: filterChoicesByText(); @@ -125,11 +124,9 @@ Column { cb(); }); } - property var delay: Timer { // No setTimeout or nextTick in QML. - interval: 0; - onTriggered: fillDestinations(); - } function fillDestinations() { // Public + console.debug('Feed::fillDestinations()') + function report(label, error) { console.log(label, actions, error || 'ok', allStories.length, 'filtered to', suggestions.count); } diff --git a/interface/resources/qml/hifi/tablet/TabletAddressDialog.qml b/interface/resources/qml/hifi/tablet/TabletAddressDialog.qml index 8bf13bad76..e8752b01e7 100644 --- a/interface/resources/qml/hifi/tablet/TabletAddressDialog.qml +++ b/interface/resources/qml/hifi/tablet/TabletAddressDialog.qml @@ -39,11 +39,24 @@ StackView { property var rpcCounter: 0; signal sendToScript(var message); function rpc(method, parameters, callback) { + console.debug('TabletAddressDialog: rpc: method = ', method, 'parameters = ', parameters, 'callback = ', callback) + rpcCalls[rpcCounter] = callback; var message = {method: method, params: parameters, id: rpcCounter++, jsonrpc: "2.0"}; sendToScript(message); } function fromScript(message) { + if (message.method === 'refreshFeeds') { + var feeds = [happeningNow, places, snapshots]; + console.debug('TabletAddressDialog::fromScript: refreshFeeds', 'feeds = ', feeds); + + feeds.forEach(function(feed) { + Qt.callLater(feed.fillDestinations); + }); + + return; + } + var callback = rpcCalls[message.id]; if (!callback) { console.log('No callback for message fromScript', JSON.stringify(message)); diff --git a/scripts/system/tablet-goto.js b/scripts/system/tablet-goto.js index fb842d1314..96ed3e3f59 100644 --- a/scripts/system/tablet-goto.js +++ b/scripts/system/tablet-goto.js @@ -42,6 +42,8 @@ }); function fromQml(message) { + console.debug('tablet-goto::fromQml: message = ', JSON.stringify(message)); + var response = {id: message.id, jsonrpc: "2.0"}; switch (message.method) { case 'request': @@ -98,6 +100,8 @@ button.editProperties({isActive: shouldActivateButton}); wireEventBridge(true); messagesWaiting(false); + tablet.sendToQml({ method: 'refreshFeeds' }) + } else { shouldActivateButton = false; onGotoScreen = false; From c122b22dfc7c1af8d0b5c82831433f73017c2359 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Wed, 23 Aug 2017 15:32:25 -0700 Subject: [PATCH 10/11] add additional IP check to DS packet filter operator --- domain-server/src/DomainGatekeeper.cpp | 1 + domain-server/src/DomainServer.cpp | 50 ++++++++++++++++++-- domain-server/src/DomainServer.h | 4 +- libraries/networking/src/HifiSockAddr.cpp | 12 +++++ libraries/networking/src/HifiSockAddr.h | 2 + libraries/networking/src/LimitedNodeList.cpp | 22 +++++---- libraries/networking/src/LimitedNodeList.h | 6 ++- 7 files changed, 80 insertions(+), 17 deletions(-) diff --git a/domain-server/src/DomainGatekeeper.cpp b/domain-server/src/DomainGatekeeper.cpp index 6951a90261..620d593ebc 100644 --- a/domain-server/src/DomainGatekeeper.cpp +++ b/domain-server/src/DomainGatekeeper.cpp @@ -54,6 +54,7 @@ void DomainGatekeeper::processConnectRequestPacket(QSharedPointergetSize() == 0) { return; } + QDataStream packetStream(message->getMessage()); // read a NodeConnectionData object from the packet so we can pass around this data while we're inspecting it diff --git a/domain-server/src/DomainServer.cpp b/domain-server/src/DomainServer.cpp index 163bd48f1b..3171fc066d 100644 --- a/domain-server/src/DomainServer.cpp +++ b/domain-server/src/DomainServer.cpp @@ -458,7 +458,7 @@ const QString DISABLED_AUTOMATIC_NETWORKING_VALUE = "disabled"; -bool DomainServer::packetVersionMatch(const udt::Packet& packet) { +bool DomainServer::isPacketVerified(const udt::Packet& packet) { PacketType headerType = NLPacket::typeInHeader(packet); PacketVersion headerVersion = NLPacket::versionInHeader(packet); @@ -471,7 +471,50 @@ bool DomainServer::packetVersionMatch(const udt::Packet& packet) { DomainGatekeeper::sendProtocolMismatchConnectionDenial(packet.getSenderSockAddr()); } - // let the normal nodeList implementation handle all other packets. + if (!PacketTypeEnum::getNonSourcedPackets().contains(headerType)) { + // this is a sourced packet - first check if we have a node that matches + QUuid sourceID = NLPacket::sourceIDInHeader(packet); + SharedNodePointer sourceNode = nodeList->nodeWithUUID(sourceID); + + if (sourceNode) { + // unverified DS packets (due to a lack of connection secret between DS + node) + // must come either from the same public IP address or a local IP address (set by RFC 1918) + + DomainServerNodeData* nodeData = static_cast(sourceNode->getLinkedData()); + + bool exactAddressMatch = nodeData->getSendingSockAddr() == packet.getSenderSockAddr(); + bool bothPrivateAddresses = nodeData->getSendingSockAddr().hasPrivateAddress() + && packet.getSenderSockAddr().hasPrivateAddress(); + + qDebug() << exactAddressMatch << bothPrivateAddresses; + + if (nodeData && (exactAddressMatch || bothPrivateAddresses)) { + // to the best of our ability we've verified that this packet comes from the right place + // let the NodeList do its checks now (but pass it the sourceNode so it doesn't need to look it up again) + return nodeList->isPacketVerifiedWithSource(packet, sourceNode.data()); + } else { + static const QString UNKNOWN_REGEX = "Packet of type \\d+ \\([\\sa-zA-Z:]+\\) received from unmatched IP for UUID"; + static QString repeatedMessage + = LogHandler::getInstance().addRepeatedMessageRegex(UNKNOWN_REGEX); + + qDebug() << "Packet of type" << headerType + << "received from unmatched IP for UUID" << uuidStringWithoutCurlyBraces(sourceID); + + return false; + } + } else { + static const QString UNKNOWN_REGEX = "Packet of type \\d+ \\([\\sa-zA-Z:]+\\) received from unknown node with UUID"; + static QString repeatedMessage + = LogHandler::getInstance().addRepeatedMessageRegex(UNKNOWN_REGEX); + + qDebug() << "Packet of type" << headerType + << "received from unknown node with UUID" << uuidStringWithoutCurlyBraces(sourceID); + + return false; + } + } + + // fallback to allow the normal NodeList implementation to verify packets return nodeList->isPacketVerified(packet); } @@ -570,7 +613,7 @@ void DomainServer::setupNodeListAndAssignments() { addStaticAssignmentsToQueue(); // set a custom packetVersionMatch as the verify packet operator for the udt::Socket - nodeList->setPacketFilterOperator(&DomainServer::packetVersionMatch); + nodeList->setPacketFilterOperator(&DomainServer::isPacketVerified); } const QString ACCESS_TOKEN_KEY_PATH = "metaverse.access_token"; @@ -853,7 +896,6 @@ void DomainServer::populateDefaultStaticAssignmentsExcludingTypes(const QSet message, SharedNodePointer sendingNode) { - QDataStream packetStream(message->getMessage()); NodeConnectionData nodeRequestData = NodeConnectionData::fromDataStream(packetStream, message->getSenderSockAddr(), false); diff --git a/domain-server/src/DomainServer.h b/domain-server/src/DomainServer.h index 4808297c89..03ad76d313 100644 --- a/domain-server/src/DomainServer.h +++ b/domain-server/src/DomainServer.h @@ -71,6 +71,7 @@ public slots: void restart(); +private slots: void processRequestAssignmentPacket(QSharedPointer packet); void processListRequestPacket(QSharedPointer packet, SharedNodePointer sendingNode); void processNodeJSONStatsPacket(QSharedPointer packetList, SharedNodePointer sendingNode); @@ -79,7 +80,6 @@ public slots: void processICEServerHeartbeatDenialPacket(QSharedPointer message); void processICEServerHeartbeatACK(QSharedPointer message); -private slots: void setupPendingAssignmentCredits(); void sendPendingTransactionsToServer(); @@ -129,7 +129,7 @@ private: void getTemporaryName(bool force = false); - static bool packetVersionMatch(const udt::Packet& packet); + static bool isPacketVerified(const udt::Packet& packet); bool resetAccountManagerAccessToken(); diff --git a/libraries/networking/src/HifiSockAddr.cpp b/libraries/networking/src/HifiSockAddr.cpp index 217d3096f5..a053610df0 100644 --- a/libraries/networking/src/HifiSockAddr.cpp +++ b/libraries/networking/src/HifiSockAddr.cpp @@ -113,6 +113,18 @@ QString HifiSockAddr::toString() const { return _address.toString() + ":" + QString::number(_port); } +bool HifiSockAddr::hasPrivateAddress() const { + // an address is private if it falls in any of the RFC1918 address spaces + const QPair TWENTY_FOUR_BIT_BLOCK = { QHostAddress("10.0.0.0"), 8 }; + const QPair TWENTY_BIT_BLOCK = { QHostAddress("172.16.0.0") , 12 }; + const QPair SIXTEEN_BIT_BLOCK = { QHostAddress("192.168.0.0"), 16 }; + + return _address.isLoopback() + || _address.isInSubnet(TWENTY_FOUR_BIT_BLOCK) + || _address.isInSubnet(TWENTY_BIT_BLOCK) + || _address.isInSubnet(SIXTEEN_BIT_BLOCK); +} + QDebug operator<<(QDebug debug, const HifiSockAddr& sockAddr) { debug.nospace() << sockAddr._address.toString().toLocal8Bit().constData() << ":" << sockAddr._port; return debug.space(); diff --git a/libraries/networking/src/HifiSockAddr.h b/libraries/networking/src/HifiSockAddr.h index af939de736..b582198139 100644 --- a/libraries/networking/src/HifiSockAddr.h +++ b/libraries/networking/src/HifiSockAddr.h @@ -55,6 +55,8 @@ public: QString toString() const; + bool hasPrivateAddress() const; // checks if the address behind this sock addr is private per RFC 1918 + friend QDebug operator<<(QDebug debug, const HifiSockAddr& sockAddr); friend QDataStream& operator<<(QDataStream& dataStream, const HifiSockAddr& sockAddr); friend QDataStream& operator>>(QDataStream& dataStream, HifiSockAddr& sockAddr); diff --git a/libraries/networking/src/LimitedNodeList.cpp b/libraries/networking/src/LimitedNodeList.cpp index 1d094d923c..954b9685af 100644 --- a/libraries/networking/src/LimitedNodeList.cpp +++ b/libraries/networking/src/LimitedNodeList.cpp @@ -202,12 +202,12 @@ QUdpSocket& LimitedNodeList::getDTLSSocket() { return *_dtlsSocket; } -bool LimitedNodeList::isPacketVerified(const udt::Packet& packet) { +bool LimitedNodeList::isPacketVerifiedWithSource(const udt::Packet& packet, Node* sourceNode) { // We track bandwidth when doing packet verification to avoid needing to do a node lookup // later when we already do it in packetSourceAndHashMatchAndTrackBandwidth. A node lookup // incurs a lock, so it is ideal to avoid needing to do it 2+ times for each packet // received. - return packetVersionMatch(packet) && packetSourceAndHashMatchAndTrackBandwidth(packet); + return packetVersionMatch(packet) && packetSourceAndHashMatchAndTrackBandwidth(packet, sourceNode); } bool LimitedNodeList::packetVersionMatch(const udt::Packet& packet) { @@ -256,7 +256,7 @@ bool LimitedNodeList::packetVersionMatch(const udt::Packet& packet) { } } -bool LimitedNodeList::packetSourceAndHashMatchAndTrackBandwidth(const udt::Packet& packet) { +bool LimitedNodeList::packetSourceAndHashMatchAndTrackBandwidth(const udt::Packet& packet, Node* sourceNode) { PacketType headerType = NLPacket::typeInHeader(packet); @@ -298,14 +298,18 @@ bool LimitedNodeList::packetSourceAndHashMatchAndTrackBandwidth(const udt::Packe } else { QUuid sourceID = NLPacket::sourceIDInHeader(packet); - // figure out which node this is from - SharedNodePointer matchingNode = nodeWithUUID(sourceID); + // check if we were passed a sourceNode hint or if we need to look it up + if (!sourceNode) { + // figure out which node this is from + SharedNodePointer matchingNode = nodeWithUUID(sourceID); + sourceNode = matchingNode.data(); + } - if (matchingNode) { + if (sourceNode) { if (!PacketTypeEnum::getNonVerifiedPackets().contains(headerType)) { QByteArray packetHeaderHash = NLPacket::verificationHashInHeader(packet); - QByteArray expectedHash = NLPacket::hashForPacketAndSecret(packet, matchingNode->getConnectionSecret()); + QByteArray expectedHash = NLPacket::hashForPacketAndSecret(packet, sourceNode->getConnectionSecret()); // check if the md5 hash in the header matches the hash we would expect if (packetHeaderHash != expectedHash) { @@ -323,9 +327,9 @@ bool LimitedNodeList::packetSourceAndHashMatchAndTrackBandwidth(const udt::Packe // No matter if this packet is handled or not, we update the timestamp for the last time we heard // from this sending node - matchingNode->setLastHeardMicrostamp(usecTimestampNow()); + sourceNode->setLastHeardMicrostamp(usecTimestampNow()); - emit dataReceived(matchingNode->getType(), packet.getPayloadSize()); + emit dataReceived(sourceNode->getType(), packet.getPayloadSize()); return true; diff --git a/libraries/networking/src/LimitedNodeList.h b/libraries/networking/src/LimitedNodeList.h index f4ec47636b..f730bcfa17 100644 --- a/libraries/networking/src/LimitedNodeList.h +++ b/libraries/networking/src/LimitedNodeList.h @@ -286,7 +286,9 @@ public: void setPacketFilterOperator(udt::PacketFilterOperator filterOperator) { _nodeSocket.setPacketFilterOperator(filterOperator); } bool packetVersionMatch(const udt::Packet& packet); - bool isPacketVerified(const udt::Packet& packet); + + bool isPacketVerifiedWithSource(const udt::Packet& packet, Node* sourceNode = nullptr); + bool isPacketVerified(const udt::Packet& packet) { return isPacketVerifiedWithSource(packet); } static void makeSTUNRequestPacket(char* stunRequestPacket); @@ -352,7 +354,7 @@ protected: void setLocalSocket(const HifiSockAddr& sockAddr); - bool packetSourceAndHashMatchAndTrackBandwidth(const udt::Packet& packet); + bool packetSourceAndHashMatchAndTrackBandwidth(const udt::Packet& packet, Node* sourceNode = nullptr); void processSTUNResponse(std::unique_ptr packet); void handleNodeKill(const SharedNodePointer& node); From 133b7e66b763d6712c7f460a1a89b6b412a68fc6 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Wed, 23 Aug 2017 16:38:23 -0700 Subject: [PATCH 11/11] adjust a comment and remove some debug --- domain-server/src/DomainServer.cpp | 2 -- libraries/networking/src/HifiSockAddr.cpp | 2 +- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/domain-server/src/DomainServer.cpp b/domain-server/src/DomainServer.cpp index 3171fc066d..5bd6020c92 100644 --- a/domain-server/src/DomainServer.cpp +++ b/domain-server/src/DomainServer.cpp @@ -486,8 +486,6 @@ bool DomainServer::isPacketVerified(const udt::Packet& packet) { bool bothPrivateAddresses = nodeData->getSendingSockAddr().hasPrivateAddress() && packet.getSenderSockAddr().hasPrivateAddress(); - qDebug() << exactAddressMatch << bothPrivateAddresses; - if (nodeData && (exactAddressMatch || bothPrivateAddresses)) { // to the best of our ability we've verified that this packet comes from the right place // let the NodeList do its checks now (but pass it the sourceNode so it doesn't need to look it up again) diff --git a/libraries/networking/src/HifiSockAddr.cpp b/libraries/networking/src/HifiSockAddr.cpp index a053610df0..e2a3e79c79 100644 --- a/libraries/networking/src/HifiSockAddr.cpp +++ b/libraries/networking/src/HifiSockAddr.cpp @@ -114,7 +114,7 @@ QString HifiSockAddr::toString() const { } bool HifiSockAddr::hasPrivateAddress() const { - // an address is private if it falls in any of the RFC1918 address spaces + // an address is private if it is loopback or falls in any of the RFC1918 address spaces const QPair TWENTY_FOUR_BIT_BLOCK = { QHostAddress("10.0.0.0"), 8 }; const QPair TWENTY_BIT_BLOCK = { QHostAddress("172.16.0.0") , 12 }; const QPair SIXTEEN_BIT_BLOCK = { QHostAddress("192.168.0.0"), 16 };