From 55a025b827ab5184e3d1311bf225f1004d0f25ee Mon Sep 17 00:00:00 2001 From: SamGondelman Date: Tue, 18 Jul 2017 12:43:34 -0700 Subject: [PATCH] initial code review --- interface/src/Application.cpp | 12 ++++--- interface/src/raypick/JointRayPick.cpp | 3 +- interface/src/raypick/LaserPointer.cpp | 14 ++++---- interface/src/raypick/LaserPointer.h | 4 ++- interface/src/raypick/LaserPointerManager.cpp | 5 --- interface/src/raypick/LaserPointerManager.h | 6 ++-- .../LaserPointerScriptingInterface.cpp | 9 ++--- .../raypick/LaserPointerScriptingInterface.h | 17 +++++---- interface/src/raypick/RayPickManager.cpp | 13 ++----- interface/src/raypick/RayPickManager.h | 35 ++++++++++--------- 10 files changed, 55 insertions(+), 63 deletions(-) diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp index 44f15ff16e..69835caff0 100644 --- a/interface/src/Application.cpp +++ b/interface/src/Application.cpp @@ -591,6 +591,10 @@ bool setupEssentials(int& argc, char** argv, bool runningMarkerExisted) { DependencyManager::set(); DependencyManager::set(); + DependencyManager::set(); + DependencyManager::set(); + DependencyManager::set(); + return previousSessionCrashed; } @@ -4922,12 +4926,12 @@ void Application::update(float deltaTime) { { PROFILE_RANGE(app, "RayPick"); - RayPickManager::getInstance().update(); + DependencyManager::get()->update(); } { PROFILE_RANGE(app, "LaserPointerManager"); - LaserPointerManager::getInstance().update(); + DependencyManager::get()->update(); } // Update _viewFrustum with latest camera and view frustum data... @@ -5775,7 +5779,7 @@ void Application::registerScriptEngineWithApplicationServices(ScriptEngine* scri scriptEngine->registerGlobalObject("AudioScope", DependencyManager::get().data()); scriptEngine->registerGlobalObject("AvatarBookmarks", DependencyManager::get().data()); scriptEngine->registerGlobalObject("LocationBookmarks", DependencyManager::get().data()); - scriptEngine->registerGlobalObject("LaserPointers", LaserPointerScriptingInterface::getInstance()); + scriptEngine->registerGlobalObject("LaserPointers", DependencyManager::get().data()); // Caches scriptEngine->registerGlobalObject("AnimationCache", DependencyManager::get().data()); @@ -5828,7 +5832,7 @@ void Application::registerScriptEngineWithApplicationServices(ScriptEngine* scri scriptEngine->registerGlobalObject("EntityScriptServerLog", entityScriptServerLog.data()); scriptEngine->registerGlobalObject("AvatarInputs", AvatarInputs::getInstance()); - scriptEngine->registerGlobalObject("RayPick", &RayPickManager::getInstance()); + scriptEngine->registerGlobalObject("RayPick", DependencyManager::get().data()); qScriptRegisterMetaType(scriptEngine, OverlayIDtoScriptValue, OverlayIDfromScriptValue); diff --git a/interface/src/raypick/JointRayPick.cpp b/interface/src/raypick/JointRayPick.cpp index 3281d307d9..f6be58437f 100644 --- a/interface/src/raypick/JointRayPick.cpp +++ b/interface/src/raypick/JointRayPick.cpp @@ -24,7 +24,8 @@ JointRayPick::JointRayPick(const QString& jointName, const glm::vec3& posOffset, const PickRay JointRayPick::getPickRay(bool& valid) const { auto myAvatar = DependencyManager::get()->getMyAvatar(); int jointIndex = myAvatar->getJointIndex(_jointName); - if (jointIndex != -1) { + const int INVALID_JOINT = -1; + if (jointIndex != INVALID_JOINT) { glm::vec3 jointPos = myAvatar->getAbsoluteJointTranslationInObjectFrame(jointIndex); glm::quat jointRot = myAvatar->getAbsoluteJointRotationInObjectFrame(jointIndex); glm::vec3 avatarPos = myAvatar->getPosition(); diff --git a/interface/src/raypick/LaserPointer.cpp b/interface/src/raypick/LaserPointer.cpp index ed5fbd2ece..930e72a6fe 100644 --- a/interface/src/raypick/LaserPointer.cpp +++ b/interface/src/raypick/LaserPointer.cpp @@ -23,7 +23,7 @@ LaserPointer::LaserPointer(const QString& jointName, const glm::vec3& posOffset, _faceAvatar(faceAvatar), _centerEndY(centerEndY) { - _rayPickUID = RayPickManager::getInstance().addRayPick(std::make_shared(jointName, posOffset, dirOffset, filter, maxDistance, enabled)); + _rayPickUID = DependencyManager::get()->addRayPick(std::make_shared(jointName, posOffset, dirOffset, filter, maxDistance, enabled)); for (auto& state : _renderStates.keys()) { if (!enabled || state != _currentRenderState) { @@ -39,7 +39,7 @@ LaserPointer::LaserPointer(const glm::vec3& position, const glm::vec3& direction _faceAvatar(faceAvatar), _centerEndY(centerEndY) { - _rayPickUID = RayPickManager::getInstance().addRayPick(std::make_shared(position, direction, filter, maxDistance, enabled)); + _rayPickUID = DependencyManager::get()->addRayPick(std::make_shared(position, direction, filter, maxDistance, enabled)); for (auto& state : _renderStates.keys()) { if (!enabled || state != _currentRenderState) { @@ -49,7 +49,7 @@ LaserPointer::LaserPointer(const glm::vec3& position, const glm::vec3& direction } LaserPointer::~LaserPointer() { - RayPickManager::getInstance().removeRayPick(_rayPickUID); + DependencyManager::get()->removeRayPick(_rayPickUID); for (RenderState& renderState : _renderStates) { if (!renderState.getStartID().isNull()) { qApp->getOverlays().deleteOverlay(renderState.getStartID()); @@ -64,12 +64,12 @@ LaserPointer::~LaserPointer() { } void LaserPointer::enable() { - RayPickManager::getInstance().enableRayPick(_rayPickUID); + DependencyManager::get()->enableRayPick(_rayPickUID); _renderingEnabled = true; } void LaserPointer::disable() { - RayPickManager::getInstance().disableRayPick(_rayPickUID); + DependencyManager::get()->disableRayPick(_rayPickUID); _renderingEnabled = false; if (!_currentRenderState.isEmpty() && _renderStates.contains(_currentRenderState)) { disableRenderState(_currentRenderState); @@ -105,9 +105,9 @@ void LaserPointer::disableRenderState(const QString& renderState) { } void LaserPointer::update() { - RayPickResult prevRayPickResult = RayPickManager::getInstance().getPrevRayPickResult(_rayPickUID); + RayPickResult prevRayPickResult = DependencyManager::get()->getPrevRayPickResult(_rayPickUID); if (_renderingEnabled && !_currentRenderState.isEmpty() && _renderStates.contains(_currentRenderState) && prevRayPickResult.type != IntersectionType::NONE) { - PickRay pickRay = RayPickManager::getInstance().getPickRay(_rayPickUID); + PickRay pickRay = DependencyManager::get()->getPickRay(_rayPickUID); if (!_renderStates[_currentRenderState].getStartID().isNull()) { QVariantMap startProps; startProps.insert("position", vec3toVariant(pickRay.origin)); diff --git a/interface/src/raypick/LaserPointer.h b/interface/src/raypick/LaserPointer.h index 075ec5e7f7..ba9a876556 100644 --- a/interface/src/raypick/LaserPointer.h +++ b/interface/src/raypick/LaserPointer.h @@ -14,6 +14,8 @@ #include #include "glm/glm.hpp" #include "ui/overlays/Overlay.h" + +#include #include "RayPickManager.h" class RayPickResult; @@ -53,7 +55,7 @@ public: unsigned int getUID() { return _rayPickUID; } void enable(); void disable(); - const RayPickResult getPrevRayPickResult() { return RayPickManager::getInstance().getPrevRayPickResult(_rayPickUID); } + const RayPickResult getPrevRayPickResult() { return DependencyManager::get()->getPrevRayPickResult(_rayPickUID); } void setRenderState(const QString& state); void disableRenderState(const QString& renderState); diff --git a/interface/src/raypick/LaserPointerManager.cpp b/interface/src/raypick/LaserPointerManager.cpp index e4c9e18127..b8086c2f89 100644 --- a/interface/src/raypick/LaserPointerManager.cpp +++ b/interface/src/raypick/LaserPointerManager.cpp @@ -12,11 +12,6 @@ #include "LaserPointer.h" #include "RayPick.h" -LaserPointerManager& LaserPointerManager::getInstance() { - static LaserPointerManager instance; - return instance; -} - unsigned int LaserPointerManager::createLaserPointer(const QString& jointName, const glm::vec3& posOffset, const glm::vec3& dirOffset, const uint16_t filter, const float maxDistance, const QHash& renderStates, const bool faceAvatar, const bool centerEndY, const bool enabled) { std::shared_ptr laserPointer = std::make_shared(jointName, posOffset, dirOffset, filter, maxDistance, renderStates, faceAvatar, centerEndY, enabled); diff --git a/interface/src/raypick/LaserPointerManager.h b/interface/src/raypick/LaserPointerManager.h index f5a8484054..fc043a9685 100644 --- a/interface/src/raypick/LaserPointerManager.h +++ b/interface/src/raypick/LaserPointerManager.h @@ -17,14 +17,14 @@ #include #include "LaserPointer.h" +#include "DependencyManager.h" class RayPickResult; -class LaserPointerManager { +class LaserPointerManager : public Dependency { + SINGLETON_DEPENDENCY public: - static LaserPointerManager& getInstance(); - unsigned int createLaserPointer(const QString& jointName, const glm::vec3& posOffset, const glm::vec3& dirOffset, const uint16_t filter, const float maxDistance, const QHash& renderStates, const bool faceAvatar, const bool centerEndY, const bool enabled); unsigned int createLaserPointer(const glm::vec3& position, const glm::vec3& direction, const uint16_t filter, const float maxDistance, diff --git a/interface/src/raypick/LaserPointerScriptingInterface.cpp b/interface/src/raypick/LaserPointerScriptingInterface.cpp index cf0289c44b..a53b3dbf01 100644 --- a/interface/src/raypick/LaserPointerScriptingInterface.cpp +++ b/interface/src/raypick/LaserPointerScriptingInterface.cpp @@ -17,11 +17,6 @@ #include "Application.h" -LaserPointerScriptingInterface* LaserPointerScriptingInterface::getInstance() { - static LaserPointerScriptingInterface instance; - return &instance; -} - uint32_t LaserPointerScriptingInterface::createLaserPointer(const QVariant& properties) { QVariantMap propertyMap = properties.toMap(); @@ -104,7 +99,7 @@ uint32_t LaserPointerScriptingInterface::createLaserPointer(const QVariant& prop dirOffset = vec3FromVariant(propertyMap["dirOffset"]); } - return LaserPointerManager::getInstance().createLaserPointer(jointName, posOffset, dirOffset, filter, maxDistance, renderStates, faceAvatar, centerEndY, enabled); + return DependencyManager::get()->createLaserPointer(jointName, posOffset, dirOffset, filter, maxDistance, renderStates, faceAvatar, centerEndY, enabled); } else if (propertyMap["position"].isValid()) { glm::vec3 position = vec3FromVariant(propertyMap["position"]); @@ -113,7 +108,7 @@ uint32_t LaserPointerScriptingInterface::createLaserPointer(const QVariant& prop direction = vec3FromVariant(propertyMap["direction"]); } - return LaserPointerManager::getInstance().createLaserPointer(position, direction, filter, maxDistance, renderStates, faceAvatar, centerEndY, enabled); + return DependencyManager::get()->createLaserPointer(position, direction, filter, maxDistance, renderStates, faceAvatar, centerEndY, enabled); } return 0; diff --git a/interface/src/raypick/LaserPointerScriptingInterface.h b/interface/src/raypick/LaserPointerScriptingInterface.h index b16daedda6..708c378abb 100644 --- a/interface/src/raypick/LaserPointerScriptingInterface.h +++ b/interface/src/raypick/LaserPointerScriptingInterface.h @@ -15,20 +15,19 @@ #include "LaserPointerManager.h" #include "RegisteredMetaTypes.h" +#include "DependencyManager.h" -class LaserPointerScriptingInterface : public QObject { +class LaserPointerScriptingInterface : public QObject, public Dependency { Q_OBJECT - -public: - static LaserPointerScriptingInterface* getInstance(); + SINGLETON_DEPENDENCY public slots: Q_INVOKABLE unsigned int createLaserPointer(const QVariant& properties); - Q_INVOKABLE void enableLaserPointer(unsigned int uid) { LaserPointerManager::getInstance().enableLaserPointer(uid); } - Q_INVOKABLE void disableLaserPointer(unsigned int uid) { LaserPointerManager::getInstance().disableLaserPointer(uid); } - Q_INVOKABLE void removeLaserPointer(unsigned int uid) { LaserPointerManager::getInstance().removeLaserPointer(uid); } - Q_INVOKABLE void setRenderState(unsigned int uid, const QString& renderState) { LaserPointerManager::getInstance().setRenderState(uid, renderState); } - Q_INVOKABLE RayPickResult getPrevRayPickResult(unsigned int uid) { return LaserPointerManager::getInstance().getPrevRayPickResult(uid); } + Q_INVOKABLE void enableLaserPointer(unsigned int uid) { DependencyManager::get()->enableLaserPointer(uid); } + Q_INVOKABLE void disableLaserPointer(unsigned int uid) { DependencyManager::get()->disableLaserPointer(uid); } + Q_INVOKABLE void removeLaserPointer(unsigned int uid) { DependencyManager::get()->removeLaserPointer(uid); } + Q_INVOKABLE void setRenderState(unsigned int uid, const QString& renderState) { DependencyManager::get()->setRenderState(uid, renderState); } + Q_INVOKABLE RayPickResult getPrevRayPickResult(unsigned int uid) { return DependencyManager::get()->getPrevRayPickResult(uid); } }; diff --git a/interface/src/raypick/RayPickManager.cpp b/interface/src/raypick/RayPickManager.cpp index 8ca503114c..ff4fca3f50 100644 --- a/interface/src/raypick/RayPickManager.cpp +++ b/interface/src/raypick/RayPickManager.cpp @@ -19,13 +19,7 @@ #include "scripting/HMDScriptingInterface.h" #include "DependencyManager.h" -RayPickManager& RayPickManager::getInstance() { - static RayPickManager instance; - return instance; -} - -// Returns true if this ray exists in the cache, and if it does, update res if the cached result is closer -bool RayPickManager::checkAndCompareCachedResults(QPair& ray, QHash, QHash>& cache, RayPickResult& res, unsigned int mask) { +bool RayPickManager::checkAndCompareCachedResults(QPair& ray, RayPickCache& cache, RayPickResult& res, unsigned int mask) { if (cache.contains(ray) && cache[ray].contains(mask)) { if (cache[ray][mask].distance < res.distance) { res = cache[ray][mask]; @@ -35,8 +29,7 @@ bool RayPickManager::checkAndCompareCachedResults(QPair& r return false; } -void RayPickManager::cacheResult(const bool intersects, const RayPickResult& resTemp, unsigned int mask, RayPickResult& res, - QPair& ray, QHash, QHash>& cache) { +void RayPickManager::cacheResult(const bool intersects, const RayPickResult& resTemp, unsigned int mask, RayPickResult& res, QPair& ray, RayPickCache& cache) { if (intersects) { cache[ray][mask] = resTemp; if (resTemp.distance < res.distance) { @@ -48,7 +41,7 @@ void RayPickManager::cacheResult(const bool intersects, const RayPickResult& res } void RayPickManager::update() { - QHash, QHash> results; + RayPickCache results; for (auto& uid : _rayPicks.keys()) { std::shared_ptr rayPick = _rayPicks[uid]; if (!rayPick->isEnabled() || rayPick->getFilter() == RayPickMask::PICK_NOTHING || rayPick->getMaxDistance() < 0.0f) { diff --git a/interface/src/raypick/RayPickManager.h b/interface/src/raypick/RayPickManager.h index ad4fb3f286..7ecb028d29 100644 --- a/interface/src/raypick/RayPickManager.h +++ b/interface/src/raypick/RayPickManager.h @@ -11,33 +11,34 @@ #ifndef hifi_RayPickManager_h #define hifi_RayPickManager_h -#include "RegisteredMetaTypes.h" - #include #include #include #include #include +#include "RegisteredMetaTypes.h" +#include "DependencyManager.h" + class RayPick; class RayPickResult; enum RayPickMask { PICK_NOTHING = 0, - PICK_ENTITIES = 1, - PICK_OVERLAYS = 2, - PICK_AVATARS = 4, - PICK_HUD = 8, + PICK_ENTITIES = 1 << 0, + PICK_OVERLAYS = 1 << 1, + PICK_AVATARS = 1 << 2, + PICK_HUD = 1 << 3, - PICK_BOUNDING_BOX = 16, // if not set, picks again physics mesh (can't pick against graphics mesh, yet) + PICK_BOUNDING_BOX = 1 << 4, // if not set, picks again physics mesh (can't pick against graphics mesh, yet) - PICK_INCLUDE_INVISIBLE = 32, // if not set, will not intersect invisible elements, otherwise, intersects both visible and invisible elements - PICK_INCLUDE_NONCOLLIDABLE = 64, // if not set, will not intersect noncollidable elements, otherwise, intersects both collidable and noncollidable elements + PICK_INCLUDE_INVISIBLE = 1 << 5, // if not set, will not intersect invisible elements, otherwise, intersects both visible and invisible elements + PICK_INCLUDE_NONCOLLIDABLE = 1 << 6, // if not set, will not intersect noncollidable elements, otherwise, intersects both collidable and noncollidable elements - PICK_ALL_INTERSECTIONS = 128 // if not set, returns closest intersection, otherwise, returns list of all intersections + PICK_ALL_INTERSECTIONS = 1 << 7 // if not set, returns closest intersection, otherwise, returns list of all intersections }; -class RayPickManager : public QObject { +class RayPickManager : public QObject, public Dependency { Q_OBJECT Q_PROPERTY(unsigned int PICK_NOTHING READ PICK_NOTHING CONSTANT) Q_PROPERTY(unsigned int PICK_ENTITIES READ PICK_ENTITIES CONSTANT) @@ -53,14 +54,10 @@ class RayPickManager : public QObject { Q_PROPERTY(unsigned int INTERSECTED_OVERLAY READ INTERSECTED_OVERLAY CONSTANT) Q_PROPERTY(unsigned int INTERSECTED_AVATAR READ INTERSECTED_AVATAR CONSTANT) Q_PROPERTY(unsigned int INTERSECTED_HUD READ INTERSECTED_HUD CONSTANT) + SINGLETON_DEPENDENCY public: - static RayPickManager& getInstance(); - void update(); - bool checkAndCompareCachedResults(QPair& ray, QHash, QHash>& cache, RayPickResult& res, unsigned int mask); - void cacheResult(const bool intersects, const RayPickResult& resTemp, unsigned int mask, RayPickResult& res, - QPair& ray, QHash, QHash>& cache); unsigned int addRayPick(std::shared_ptr rayPick); void removeRayPick(const unsigned int uid); void enableRayPick(const unsigned int uid); @@ -78,6 +75,12 @@ private: QQueue _rayPicksToRemove; QReadWriteLock _containsLock; + typedef QHash, QHash> RayPickCache; + + // Returns true if this ray exists in the cache, and if it does, update res if the cached result is closer + bool checkAndCompareCachedResults(QPair& ray, RayPickCache& cache, RayPickResult& res, unsigned int mask); + void cacheResult(const bool intersects, const RayPickResult& resTemp, unsigned int mask, RayPickResult& res, QPair& ray, RayPickCache& cache); + unsigned int PICK_NOTHING() { return RayPickMask::PICK_NOTHING; } unsigned int PICK_ENTITIES() { return RayPickMask::PICK_ENTITIES; } unsigned int PICK_OVERLAYS() { return RayPickMask::PICK_OVERLAYS; }