From 97b09d680ca7bbff215a7765d082f909deed5d73 Mon Sep 17 00:00:00 2001 From: SamGondelman Date: Mon, 2 Oct 2017 13:17:53 -0700 Subject: [PATCH] fix and improve laser pointer locking --- interface/src/raypick/LaserPointer.h | 3 + interface/src/raypick/LaserPointerManager.cpp | 105 ++++++++++-------- interface/src/raypick/LaserPointerManager.h | 2 - interface/src/raypick/RayPick.h | 5 + interface/src/raypick/RayPickManager.cpp | 76 +++++++------ interface/src/raypick/RayPickManager.h | 2 - 6 files changed, 108 insertions(+), 85 deletions(-) diff --git a/interface/src/raypick/LaserPointer.h b/interface/src/raypick/LaserPointer.h index 5467a8233e..fa7d396ae8 100644 --- a/interface/src/raypick/LaserPointer.h +++ b/interface/src/raypick/LaserPointer.h @@ -75,6 +75,8 @@ public: void setLockEndUUID(QUuid objectID, const bool isOverlay) { _objectLockEnd = std::pair(objectID, isOverlay); } + QReadWriteLock* getLock() { return &_lock; } + void update(); private: @@ -89,6 +91,7 @@ private: std::pair _objectLockEnd { std::pair(QUuid(), false)}; QUuid _rayPickUID; + QReadWriteLock _lock; void updateRenderStateOverlay(const OverlayID& id, const QVariant& props); void updateRenderState(const RenderState& renderState, const IntersectionType type, const float distance, const QUuid& objectID, const PickRay& pickRay, const bool defaultState); diff --git a/interface/src/raypick/LaserPointerManager.cpp b/interface/src/raypick/LaserPointerManager.cpp index b19ecc14f0..387f88724e 100644 --- a/interface/src/raypick/LaserPointerManager.cpp +++ b/interface/src/raypick/LaserPointerManager.cpp @@ -17,7 +17,6 @@ QUuid LaserPointerManager::createLaserPointer(const QVariant& rayProps, const La QWriteLocker containsLock(&_containsLock); QUuid id = QUuid::createUuid(); _laserPointers[id] = laserPointer; - _laserPointerLocks[id] = std::make_shared(); return id; } return QUuid(); @@ -26,46 +25,50 @@ QUuid LaserPointerManager::createLaserPointer(const QVariant& rayProps, const La void LaserPointerManager::removeLaserPointer(const QUuid uid) { QWriteLocker lock(&_containsLock); _laserPointers.remove(uid); - _laserPointerLocks.remove(uid); } void LaserPointerManager::enableLaserPointer(const QUuid uid) { QReadLocker lock(&_containsLock); - if (_laserPointers.contains(uid)) { - QWriteLocker laserLock(_laserPointerLocks[uid].get()); - _laserPointers[uid]->enable(); + auto laserPointer = _laserPointers.find(uid); + if (laserPointer != _laserPointers.end()) { + QWriteLocker laserLock(laserPointer.value()->getLock()); + laserPointer.value()->enable(); } } void LaserPointerManager::disableLaserPointer(const QUuid uid) { QReadLocker lock(&_containsLock); - if (_laserPointers.contains(uid)) { - QWriteLocker laserLock(_laserPointerLocks[uid].get()); - _laserPointers[uid]->disable(); + auto laserPointer = _laserPointers.find(uid); + if (laserPointer != _laserPointers.end()) { + QWriteLocker laserLock(laserPointer.value()->getLock()); + laserPointer.value()->disable(); } } void LaserPointerManager::setRenderState(QUuid uid, const std::string& renderState) { QReadLocker lock(&_containsLock); - if (_laserPointers.contains(uid)) { - QWriteLocker laserLock(_laserPointerLocks[uid].get()); - _laserPointers[uid]->setRenderState(renderState); + auto laserPointer = _laserPointers.find(uid); + if (laserPointer != _laserPointers.end()) { + QWriteLocker laserLock(laserPointer.value()->getLock()); + laserPointer.value()->setRenderState(renderState); } } void LaserPointerManager::editRenderState(QUuid uid, const std::string& state, const QVariant& startProps, const QVariant& pathProps, const QVariant& endProps) { QReadLocker lock(&_containsLock); - if (_laserPointers.contains(uid)) { - QWriteLocker laserLock(_laserPointerLocks[uid].get()); - _laserPointers[uid]->editRenderState(state, startProps, pathProps, endProps); + auto laserPointer = _laserPointers.find(uid); + if (laserPointer != _laserPointers.end()) { + QWriteLocker laserLock(laserPointer.value()->getLock()); + laserPointer.value()->editRenderState(state, startProps, pathProps, endProps); } } const RayPickResult LaserPointerManager::getPrevRayPickResult(const QUuid uid) { QReadLocker lock(&_containsLock); - if (_laserPointers.contains(uid)) { - QReadLocker laserLock(_laserPointerLocks[uid].get()); - return _laserPointers[uid]->getPrevRayPickResult(); + auto laserPointer = _laserPointers.find(uid); + if (laserPointer != _laserPointers.end()) { + QReadLocker laserLock(laserPointer.value()->getLock()); + return laserPointer.value()->getPrevRayPickResult(); } return RayPickResult(); } @@ -74,79 +77,89 @@ void LaserPointerManager::update() { QReadLocker lock(&_containsLock); for (QUuid& uid : _laserPointers.keys()) { // This only needs to be a read lock because update won't change any of the properties that can be modified from scripts - QReadLocker laserLock(_laserPointerLocks[uid].get()); - _laserPointers[uid]->update(); + auto laserPointer = _laserPointers.find(uid); + QReadLocker laserLock(laserPointer.value()->getLock()); + laserPointer.value()->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); + auto laserPointer = _laserPointers.find(uid); + if (laserPointer != _laserPointers.end()) { + QWriteLocker laserLock(laserPointer.value()->getLock()); + laserPointer.value()->setPrecisionPicking(precisionPicking); } } void LaserPointerManager::setLaserLength(QUuid uid, const float laserLength) { QReadLocker lock(&_containsLock); - if (_laserPointers.contains(uid)) { - QWriteLocker laserLock(_laserPointerLocks[uid].get()); - _laserPointers[uid]->setLaserLength(laserLength); + auto laserPointer = _laserPointers.find(uid); + if (laserPointer != _laserPointers.end()) { + QWriteLocker laserLock(laserPointer.value()->getLock()); + laserPointer.value()->setLaserLength(laserLength); } } void LaserPointerManager::setIgnoreEntities(QUuid uid, const QScriptValue& ignoreEntities) { QReadLocker lock(&_containsLock); - if (_laserPointers.contains(uid)) { - QWriteLocker laserLock(_laserPointerLocks[uid].get()); - _laserPointers[uid]->setIgnoreEntities(ignoreEntities); + auto laserPointer = _laserPointers.find(uid); + if (laserPointer != _laserPointers.end()) { + QWriteLocker laserLock(laserPointer.value()->getLock()); + laserPointer.value()->setIgnoreEntities(ignoreEntities); } } void LaserPointerManager::setIncludeEntities(QUuid uid, const QScriptValue& includeEntities) { QReadLocker lock(&_containsLock); - if (_laserPointers.contains(uid)) { - QWriteLocker laserLock(_laserPointerLocks[uid].get()); - _laserPointers[uid]->setIncludeEntities(includeEntities); + auto laserPointer = _laserPointers.find(uid); + if (laserPointer != _laserPointers.end()) { + QWriteLocker laserLock(laserPointer.value()->getLock()); + laserPointer.value()->setIncludeEntities(includeEntities); } } void LaserPointerManager::setIgnoreOverlays(QUuid uid, const QScriptValue& ignoreOverlays) { QReadLocker lock(&_containsLock); - if (_laserPointers.contains(uid)) { - QWriteLocker laserLock(_laserPointerLocks[uid].get()); - _laserPointers[uid]->setIgnoreOverlays(ignoreOverlays); + auto laserPointer = _laserPointers.find(uid); + if (laserPointer != _laserPointers.end()) { + QWriteLocker laserLock(laserPointer.value()->getLock()); + laserPointer.value()->setIgnoreOverlays(ignoreOverlays); } } void LaserPointerManager::setIncludeOverlays(QUuid uid, const QScriptValue& includeOverlays) { QReadLocker lock(&_containsLock); - if (_laserPointers.contains(uid)) { - QWriteLocker laserLock(_laserPointerLocks[uid].get()); - _laserPointers[uid]->setIncludeOverlays(includeOverlays); + auto laserPointer = _laserPointers.find(uid); + if (laserPointer != _laserPointers.end()) { + QWriteLocker laserLock(laserPointer.value()->getLock()); + laserPointer.value()->setIncludeOverlays(includeOverlays); } } void LaserPointerManager::setIgnoreAvatars(QUuid uid, const QScriptValue& ignoreAvatars) { QReadLocker lock(&_containsLock); - if (_laserPointers.contains(uid)) { - QWriteLocker laserLock(_laserPointerLocks[uid].get()); - _laserPointers[uid]->setIgnoreAvatars(ignoreAvatars); + auto laserPointer = _laserPointers.find(uid); + if (laserPointer != _laserPointers.end()) { + QWriteLocker laserLock(laserPointer.value()->getLock()); + laserPointer.value()->setIgnoreAvatars(ignoreAvatars); } } void LaserPointerManager::setIncludeAvatars(QUuid uid, const QScriptValue& includeAvatars) { QReadLocker lock(&_containsLock); - if (_laserPointers.contains(uid)) { - QWriteLocker laserLock(_laserPointerLocks[uid].get()); - _laserPointers[uid]->setIncludeAvatars(includeAvatars); + auto laserPointer = _laserPointers.find(uid); + if (laserPointer != _laserPointers.end()) { + QWriteLocker laserLock(laserPointer.value()->getLock()); + laserPointer.value()->setIncludeAvatars(includeAvatars); } } void LaserPointerManager::setLockEndUUID(QUuid uid, QUuid objectID, const bool isOverlay) { QReadLocker lock(&_containsLock); - if (_laserPointers.contains(uid)) { - QWriteLocker laserLock(_laserPointerLocks[uid].get()); - _laserPointers[uid]->setLockEndUUID(objectID, isOverlay); + auto laserPointer = _laserPointers.find(uid); + if (laserPointer != _laserPointers.end()) { + QWriteLocker laserLock(laserPointer.value()->getLock()); + laserPointer.value()->setLockEndUUID(objectID, isOverlay); } } diff --git a/interface/src/raypick/LaserPointerManager.h b/interface/src/raypick/LaserPointerManager.h index 6494bb7056..b841877578 100644 --- a/interface/src/raypick/LaserPointerManager.h +++ b/interface/src/raypick/LaserPointerManager.h @@ -13,7 +13,6 @@ #include #include -#include #include "LaserPointer.h" @@ -46,7 +45,6 @@ public: private: QHash> _laserPointers; - QHash> _laserPointerLocks; QReadWriteLock _containsLock; }; diff --git a/interface/src/raypick/RayPick.h b/interface/src/raypick/RayPick.h index 0686a05718..428e44d670 100644 --- a/interface/src/raypick/RayPick.h +++ b/interface/src/raypick/RayPick.h @@ -16,6 +16,7 @@ #include "EntityItemID.h" #include "ui/overlays/Overlay.h" +#include class RayPickFilter { public: @@ -127,6 +128,8 @@ public: void setIgnoreAvatars(const QScriptValue& ignoreAvatars) { _ignoreAvatars = qVectorEntityItemIDFromScriptValue(ignoreAvatars); } void setIncludeAvatars(const QScriptValue& includeAvatars) { _includeAvatars = qVectorEntityItemIDFromScriptValue(includeAvatars); } + QReadWriteLock* getLock() { return &_lock; } + private: RayPickFilter _filter; float _maxDistance; @@ -139,6 +142,8 @@ private: QVector _includeOverlays; QVector _ignoreAvatars; QVector _includeAvatars; + + QReadWriteLock _lock; }; #endif // hifi_RayPick_h diff --git a/interface/src/raypick/RayPickManager.cpp b/interface/src/raypick/RayPickManager.cpp index bfc6e3fcb2..65f82dcd5f 100644 --- a/interface/src/raypick/RayPickManager.cpp +++ b/interface/src/raypick/RayPickManager.cpp @@ -47,6 +47,7 @@ void RayPickManager::update() { RayPickCache results; for (auto& uid : _rayPicks.keys()) { std::shared_ptr rayPick = _rayPicks[uid]; + QWriteLocker lock(rayPick->getLock()); if (!rayPick->isEnabled() || rayPick->getFilter().doesPickNothing() || rayPick->getMaxDistance() < 0.0f) { continue; } @@ -114,7 +115,6 @@ void RayPickManager::update() { } } - QWriteLocker lock(_rayPickLocks[uid].get()); if (rayPick->getMaxDistance() == 0.0f || (rayPick->getMaxDistance() > 0.0f && res.distance < rayPick->getMaxDistance())) { rayPick->setRayPickResult(res); } else { @@ -127,7 +127,6 @@ QUuid RayPickManager::createRayPick(const std::string& jointName, const glm::vec QWriteLocker lock(&_containsLock); QUuid id = QUuid::createUuid(); _rayPicks[id] = std::make_shared(jointName, posOffset, dirOffset, filter, maxDistance, enabled); - _rayPickLocks[id] = std::make_shared(); return id; } @@ -135,7 +134,6 @@ QUuid RayPickManager::createRayPick(const RayPickFilter& filter, const float max QWriteLocker lock(&_containsLock); QUuid id = QUuid::createUuid(); _rayPicks[id] = std::make_shared(filter, maxDistance, enabled); - _rayPickLocks[id] = std::make_shared(); return id; } @@ -143,93 +141,101 @@ QUuid RayPickManager::createRayPick(const glm::vec3& position, const glm::vec3& QWriteLocker lock(&_containsLock); QUuid id = QUuid::createUuid(); _rayPicks[id] = std::make_shared(position, direction, filter, maxDistance, enabled); - _rayPickLocks[id] = std::make_shared(); return id; } void RayPickManager::removeRayPick(const QUuid uid) { QWriteLocker lock(&_containsLock); _rayPicks.remove(uid); - _rayPickLocks.remove(uid); } void RayPickManager::enableRayPick(const QUuid uid) { QReadLocker containsLock(&_containsLock); - if (_rayPicks.contains(uid)) { - QWriteLocker rayPickLock(_rayPickLocks[uid].get()); - _rayPicks[uid]->enable(); + auto rayPick = _rayPicks.find(uid); + if (rayPick != _rayPicks.end()) { + QWriteLocker rayPickLock(rayPick.value()->getLock()); + rayPick.value()->enable(); } } void RayPickManager::disableRayPick(const QUuid uid) { QReadLocker containsLock(&_containsLock); - if (_rayPicks.contains(uid)) { - QWriteLocker rayPickLock(_rayPickLocks[uid].get()); - _rayPicks[uid]->disable(); + auto rayPick = _rayPicks.find(uid); + if (rayPick != _rayPicks.end()) { + QWriteLocker rayPickLock(rayPick.value()->getLock()); + rayPick.value()->disable(); } } const RayPickResult RayPickManager::getPrevRayPickResult(const QUuid uid) { QReadLocker containsLock(&_containsLock); - if (_rayPicks.contains(uid)) { - QReadLocker lock(_rayPickLocks[uid].get()); - return _rayPicks[uid]->getPrevRayPickResult(); + auto rayPick = _rayPicks.find(uid); + if (rayPick != _rayPicks.end()) { + QReadLocker lock(rayPick.value()->getLock()); + return rayPick.value()->getPrevRayPickResult(); } 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); + auto rayPick = _rayPicks.find(uid); + if (rayPick != _rayPicks.end()) { + QWriteLocker lock(rayPick.value()->getLock()); + rayPick.value()->setPrecisionPicking(precisionPicking); } } void RayPickManager::setIgnoreEntities(QUuid uid, const QScriptValue& ignoreEntities) { QReadLocker containsLock(&_containsLock); - if (_rayPicks.contains(uid)) { - QWriteLocker lock(_rayPickLocks[uid].get()); - _rayPicks[uid]->setIgnoreEntities(ignoreEntities); + auto rayPick = _rayPicks.find(uid); + if (rayPick != _rayPicks.end()) { + QWriteLocker lock(rayPick.value()->getLock()); + rayPick.value()->setIgnoreEntities(ignoreEntities); } } void RayPickManager::setIncludeEntities(QUuid uid, const QScriptValue& includeEntities) { QReadLocker containsLock(&_containsLock); - if (_rayPicks.contains(uid)) { - QWriteLocker lock(_rayPickLocks[uid].get()); - _rayPicks[uid]->setIncludeEntities(includeEntities); + auto rayPick = _rayPicks.find(uid); + if (rayPick != _rayPicks.end()) { + QWriteLocker lock(rayPick.value()->getLock()); + rayPick.value()->setIncludeEntities(includeEntities); } } void RayPickManager::setIgnoreOverlays(QUuid uid, const QScriptValue& ignoreOverlays) { QReadLocker containsLock(&_containsLock); - if (_rayPicks.contains(uid)) { - QWriteLocker lock(_rayPickLocks[uid].get()); - _rayPicks[uid]->setIgnoreOverlays(ignoreOverlays); + auto rayPick = _rayPicks.find(uid); + if (rayPick != _rayPicks.end()) { + QWriteLocker lock(rayPick.value()->getLock()); + rayPick.value()->setIgnoreOverlays(ignoreOverlays); } } void RayPickManager::setIncludeOverlays(QUuid uid, const QScriptValue& includeOverlays) { QReadLocker containsLock(&_containsLock); - if (_rayPicks.contains(uid)) { - QWriteLocker lock(_rayPickLocks[uid].get()); - _rayPicks[uid]->setIncludeOverlays(includeOverlays); + auto rayPick = _rayPicks.find(uid); + if (rayPick != _rayPicks.end()) { + QWriteLocker lock(rayPick.value()->getLock()); + rayPick.value()->setIncludeOverlays(includeOverlays); } } void RayPickManager::setIgnoreAvatars(QUuid uid, const QScriptValue& ignoreAvatars) { QReadLocker containsLock(&_containsLock); - if (_rayPicks.contains(uid)) { - QWriteLocker lock(_rayPickLocks[uid].get()); - _rayPicks[uid]->setIgnoreAvatars(ignoreAvatars); + auto rayPick = _rayPicks.find(uid); + if (rayPick != _rayPicks.end()) { + QWriteLocker lock(rayPick.value()->getLock()); + rayPick.value()->setIgnoreAvatars(ignoreAvatars); } } void RayPickManager::setIncludeAvatars(QUuid uid, const QScriptValue& includeAvatars) { QReadLocker containsLock(&_containsLock); - if (_rayPicks.contains(uid)) { - QWriteLocker lock(_rayPickLocks[uid].get()); - _rayPicks[uid]->setIncludeAvatars(includeAvatars); + auto rayPick = _rayPicks.find(uid); + if (rayPick != _rayPicks.end()) { + QWriteLocker lock(rayPick.value()->getLock()); + rayPick.value()->setIncludeAvatars(includeAvatars); } } \ No newline at end of file diff --git a/interface/src/raypick/RayPickManager.h b/interface/src/raypick/RayPickManager.h index 9717767f19..974022eb4d 100644 --- a/interface/src/raypick/RayPickManager.h +++ b/interface/src/raypick/RayPickManager.h @@ -15,7 +15,6 @@ #include #include -#include #include "RegisteredMetaTypes.h" @@ -47,7 +46,6 @@ public: private: QHash> _rayPicks; - QHash> _rayPickLocks; QReadWriteLock _containsLock; typedef QHash, std::unordered_map> RayPickCache;