From 6b02cbb9112923e90fd92baa91304f1bbd3ebe93 Mon Sep 17 00:00:00 2001 From: SamGondelman Date: Thu, 5 Oct 2017 13:17:57 -0700 Subject: [PATCH] move locking inside laserpointer and raypick --- interface/src/raypick/LaserPointer.cpp | 56 +++++++++++++++++++ interface/src/raypick/LaserPointer.h | 20 +++---- interface/src/raypick/LaserPointerManager.cpp | 16 ------ interface/src/raypick/RayPick.cpp | 44 +++++++++++++++ interface/src/raypick/RayPick.h | 18 +++--- interface/src/raypick/RayPickManager.cpp | 10 ---- 6 files changed, 119 insertions(+), 45 deletions(-) diff --git a/interface/src/raypick/LaserPointer.cpp b/interface/src/raypick/LaserPointer.cpp index 55ddd01123..0e0f13cd6c 100644 --- a/interface/src/raypick/LaserPointer.cpp +++ b/interface/src/raypick/LaserPointer.cpp @@ -49,11 +49,13 @@ LaserPointer::~LaserPointer() { } void LaserPointer::enable() { + QWriteLocker lock(getLock()); DependencyManager::get()->enableRayPick(_rayPickUID); _renderingEnabled = true; } void LaserPointer::disable() { + QWriteLocker lock(getLock()); DependencyManager::get()->disableRayPick(_rayPickUID); _renderingEnabled = false; if (!_currentRenderState.empty()) { @@ -67,6 +69,7 @@ void LaserPointer::disable() { } void LaserPointer::setRenderState(const std::string& state) { + QWriteLocker lock(getLock()); if (!_currentRenderState.empty() && state != _currentRenderState) { if (_renderStates.find(_currentRenderState) != _renderStates.end()) { disableRenderState(_renderStates[_currentRenderState]); @@ -79,6 +82,7 @@ void LaserPointer::setRenderState(const std::string& state) { } void LaserPointer::editRenderState(const std::string& state, const QVariant& startProps, const QVariant& pathProps, const QVariant& endProps) { + QWriteLocker lock(getLock()); updateRenderStateOverlay(_renderStates[state].getStartID(), startProps); updateRenderStateOverlay(_renderStates[state].getPathID(), pathProps); updateRenderStateOverlay(_renderStates[state].getEndID(), endProps); @@ -92,6 +96,11 @@ void LaserPointer::updateRenderStateOverlay(const OverlayID& id, const QVariant& } } +const RayPickResult LaserPointer::getPrevRayPickResult() { + QReadLocker lock(getLock()); + return DependencyManager::get()->getPrevRayPickResult(_rayPickUID); +} + void LaserPointer::updateRenderState(const RenderState& renderState, const IntersectionType type, const float distance, const QUuid& objectID, const PickRay& pickRay, const bool defaultState) { if (!renderState.getStartID().isNull()) { QVariantMap startProps; @@ -183,6 +192,8 @@ void LaserPointer::disableRenderState(const RenderState& renderState) { } void LaserPointer::update() { + // This only needs to be a read lock because update won't change any of the properties that can be modified from scripts + QReadLocker lock(getLock()); RayPickResult prevRayPickResult = DependencyManager::get()->getPrevRayPickResult(_rayPickUID); if (_renderingEnabled && !_currentRenderState.empty() && _renderStates.find(_currentRenderState) != _renderStates.end() && (prevRayPickResult.type != IntersectionType::NONE || _laserLength > 0.0f || !_objectLockEnd.first.isNull())) { @@ -198,6 +209,51 @@ void LaserPointer::update() { } } +void LaserPointer::setPrecisionPicking(const bool precisionPicking) { + QWriteLocker lock(getLock()); + DependencyManager::get()->setPrecisionPicking(_rayPickUID, precisionPicking); +} + +void LaserPointer::setLaserLength(const float laserLength) { + QWriteLocker lock(getLock()); + _laserLength = laserLength; +} + +void LaserPointer::setLockEndUUID(QUuid objectID, const bool isOverlay) { + QWriteLocker lock(getLock()); + _objectLockEnd = std::pair(objectID, isOverlay); +} + +void LaserPointer::setIgnoreEntities(const QScriptValue& ignoreEntities) { + QWriteLocker lock(getLock()); + DependencyManager::get()->setIgnoreEntities(_rayPickUID, ignoreEntities); +} + +void LaserPointer::setIncludeEntities(const QScriptValue& includeEntities) { + QWriteLocker lock(getLock()); + DependencyManager::get()->setIncludeEntities(_rayPickUID, includeEntities); +} + +void LaserPointer::setIgnoreOverlays(const QScriptValue& ignoreOverlays) { + QWriteLocker lock(getLock()); + DependencyManager::get()->setIgnoreOverlays(_rayPickUID, ignoreOverlays); +} + +void LaserPointer::setIncludeOverlays(const QScriptValue& includeOverlays) { + QWriteLocker lock(getLock()); + DependencyManager::get()->setIncludeOverlays(_rayPickUID, includeOverlays); +} + +void LaserPointer::setIgnoreAvatars(const QScriptValue& ignoreAvatars) { + QWriteLocker lock(getLock()); + DependencyManager::get()->setIgnoreAvatars(_rayPickUID, ignoreAvatars); +} + +void LaserPointer::setIncludeAvatars(const QScriptValue& includeAvatars) { + QWriteLocker lock(getLock()); + DependencyManager::get()->setIncludeAvatars(_rayPickUID, includeAvatars); +} + RenderState::RenderState(const OverlayID& startID, const OverlayID& pathID, const OverlayID& endID) : _startID(startID), _pathID(pathID), _endID(endID) { diff --git a/interface/src/raypick/LaserPointer.h b/interface/src/raypick/LaserPointer.h index fa7d396ae8..01dfe01cfd 100644 --- a/interface/src/raypick/LaserPointer.h +++ b/interface/src/raypick/LaserPointer.h @@ -58,22 +58,22 @@ public: QUuid getRayUID() { return _rayPickUID; } void enable(); void disable(); - const RayPickResult getPrevRayPickResult() { return DependencyManager::get()->getPrevRayPickResult(_rayPickUID); } + const RayPickResult getPrevRayPickResult(); void setRenderState(const std::string& state); // 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 setLaserLength(const float laserLength) { _laserLength = laserLength; } - 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); } - void setIncludeOverlays(const QScriptValue& includeOverlays) { DependencyManager::get()->setIncludeOverlays(_rayPickUID, includeOverlays); } - void setIgnoreAvatars(const QScriptValue& ignoreAvatars) { DependencyManager::get()->setIgnoreAvatars(_rayPickUID, ignoreAvatars); } - void setIncludeAvatars(const QScriptValue& includeAvatars) { DependencyManager::get()->setIncludeAvatars(_rayPickUID, includeAvatars); } + void setPrecisionPicking(const bool precisionPicking); + void setLaserLength(const float laserLength); + void setLockEndUUID(QUuid objectID, const bool isOverlay); - void setLockEndUUID(QUuid objectID, const bool isOverlay) { _objectLockEnd = std::pair(objectID, isOverlay); } + void setIgnoreEntities(const QScriptValue& ignoreEntities); + void setIncludeEntities(const QScriptValue& includeEntities); + void setIgnoreOverlays(const QScriptValue& ignoreOverlays); + void setIncludeOverlays(const QScriptValue& includeOverlays); + void setIgnoreAvatars(const QScriptValue& ignoreAvatars); + void setIncludeAvatars(const QScriptValue& includeAvatars); QReadWriteLock* getLock() { return &_lock; } diff --git a/interface/src/raypick/LaserPointerManager.cpp b/interface/src/raypick/LaserPointerManager.cpp index 387f88724e..8615a96c3f 100644 --- a/interface/src/raypick/LaserPointerManager.cpp +++ b/interface/src/raypick/LaserPointerManager.cpp @@ -31,7 +31,6 @@ void LaserPointerManager::enableLaserPointer(const QUuid uid) { QReadLocker lock(&_containsLock); auto laserPointer = _laserPointers.find(uid); if (laserPointer != _laserPointers.end()) { - QWriteLocker laserLock(laserPointer.value()->getLock()); laserPointer.value()->enable(); } } @@ -40,7 +39,6 @@ void LaserPointerManager::disableLaserPointer(const QUuid uid) { QReadLocker lock(&_containsLock); auto laserPointer = _laserPointers.find(uid); if (laserPointer != _laserPointers.end()) { - QWriteLocker laserLock(laserPointer.value()->getLock()); laserPointer.value()->disable(); } } @@ -49,7 +47,6 @@ void LaserPointerManager::setRenderState(QUuid uid, const std::string& renderSta QReadLocker lock(&_containsLock); auto laserPointer = _laserPointers.find(uid); if (laserPointer != _laserPointers.end()) { - QWriteLocker laserLock(laserPointer.value()->getLock()); laserPointer.value()->setRenderState(renderState); } } @@ -58,7 +55,6 @@ void LaserPointerManager::editRenderState(QUuid uid, const std::string& state, c QReadLocker lock(&_containsLock); auto laserPointer = _laserPointers.find(uid); if (laserPointer != _laserPointers.end()) { - QWriteLocker laserLock(laserPointer.value()->getLock()); laserPointer.value()->editRenderState(state, startProps, pathProps, endProps); } } @@ -67,7 +63,6 @@ const RayPickResult LaserPointerManager::getPrevRayPickResult(const QUuid uid) { QReadLocker lock(&_containsLock); auto laserPointer = _laserPointers.find(uid); if (laserPointer != _laserPointers.end()) { - QReadLocker laserLock(laserPointer.value()->getLock()); return laserPointer.value()->getPrevRayPickResult(); } return RayPickResult(); @@ -76,9 +71,7 @@ const RayPickResult LaserPointerManager::getPrevRayPickResult(const QUuid uid) { 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 auto laserPointer = _laserPointers.find(uid); - QReadLocker laserLock(laserPointer.value()->getLock()); laserPointer.value()->update(); } } @@ -87,7 +80,6 @@ void LaserPointerManager::setPrecisionPicking(QUuid uid, const bool precisionPic QReadLocker lock(&_containsLock); auto laserPointer = _laserPointers.find(uid); if (laserPointer != _laserPointers.end()) { - QWriteLocker laserLock(laserPointer.value()->getLock()); laserPointer.value()->setPrecisionPicking(precisionPicking); } } @@ -96,7 +88,6 @@ void LaserPointerManager::setLaserLength(QUuid uid, const float laserLength) { QReadLocker lock(&_containsLock); auto laserPointer = _laserPointers.find(uid); if (laserPointer != _laserPointers.end()) { - QWriteLocker laserLock(laserPointer.value()->getLock()); laserPointer.value()->setLaserLength(laserLength); } } @@ -105,7 +96,6 @@ void LaserPointerManager::setIgnoreEntities(QUuid uid, const QScriptValue& ignor QReadLocker lock(&_containsLock); auto laserPointer = _laserPointers.find(uid); if (laserPointer != _laserPointers.end()) { - QWriteLocker laserLock(laserPointer.value()->getLock()); laserPointer.value()->setIgnoreEntities(ignoreEntities); } } @@ -114,7 +104,6 @@ void LaserPointerManager::setIncludeEntities(QUuid uid, const QScriptValue& incl QReadLocker lock(&_containsLock); auto laserPointer = _laserPointers.find(uid); if (laserPointer != _laserPointers.end()) { - QWriteLocker laserLock(laserPointer.value()->getLock()); laserPointer.value()->setIncludeEntities(includeEntities); } } @@ -123,7 +112,6 @@ void LaserPointerManager::setIgnoreOverlays(QUuid uid, const QScriptValue& ignor QReadLocker lock(&_containsLock); auto laserPointer = _laserPointers.find(uid); if (laserPointer != _laserPointers.end()) { - QWriteLocker laserLock(laserPointer.value()->getLock()); laserPointer.value()->setIgnoreOverlays(ignoreOverlays); } } @@ -132,7 +120,6 @@ void LaserPointerManager::setIncludeOverlays(QUuid uid, const QScriptValue& incl QReadLocker lock(&_containsLock); auto laserPointer = _laserPointers.find(uid); if (laserPointer != _laserPointers.end()) { - QWriteLocker laserLock(laserPointer.value()->getLock()); laserPointer.value()->setIncludeOverlays(includeOverlays); } } @@ -141,7 +128,6 @@ void LaserPointerManager::setIgnoreAvatars(QUuid uid, const QScriptValue& ignore QReadLocker lock(&_containsLock); auto laserPointer = _laserPointers.find(uid); if (laserPointer != _laserPointers.end()) { - QWriteLocker laserLock(laserPointer.value()->getLock()); laserPointer.value()->setIgnoreAvatars(ignoreAvatars); } } @@ -150,7 +136,6 @@ void LaserPointerManager::setIncludeAvatars(QUuid uid, const QScriptValue& inclu QReadLocker lock(&_containsLock); auto laserPointer = _laserPointers.find(uid); if (laserPointer != _laserPointers.end()) { - QWriteLocker laserLock(laserPointer.value()->getLock()); laserPointer.value()->setIncludeAvatars(includeAvatars); } } @@ -159,7 +144,6 @@ void LaserPointerManager::setLockEndUUID(QUuid uid, QUuid objectID, const bool i QReadLocker lock(&_containsLock); 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/RayPick.cpp b/interface/src/raypick/RayPick.cpp index 70170a8f85..a5b1299210 100644 --- a/interface/src/raypick/RayPick.cpp +++ b/interface/src/raypick/RayPick.cpp @@ -16,3 +16,47 @@ RayPick::RayPick(const RayPickFilter& filter, const float maxDistance, const boo _enabled(enabled) { } +void RayPick::enable() { + QWriteLocker lock(getLock()); + _enabled = true; +} + +void RayPick::disable() { + QWriteLocker lock(getLock()); + _enabled = false; +} + +const RayPickResult& RayPick::getPrevRayPickResult() { + QReadLocker lock(getLock()); + return _prevResult; +} + +void RayPick::setIgnoreEntities(const QScriptValue& ignoreEntities) { + QWriteLocker lock(getLock()); + _ignoreEntities = qVectorEntityItemIDFromScriptValue(ignoreEntities); +} + +void RayPick::setIncludeEntities(const QScriptValue& includeEntities) { + QWriteLocker lock(getLock()); + _includeEntities = qVectorEntityItemIDFromScriptValue(includeEntities); +} + +void RayPick::setIgnoreOverlays(const QScriptValue& ignoreOverlays) { + QWriteLocker lock(getLock()); + _ignoreOverlays = qVectorOverlayIDFromScriptValue(ignoreOverlays); +} + +void RayPick::setIncludeOverlays(const QScriptValue& includeOverlays) { + QWriteLocker lock(getLock()); + _includeOverlays = qVectorOverlayIDFromScriptValue(includeOverlays); +} + +void RayPick::setIgnoreAvatars(const QScriptValue& ignoreAvatars) { + QWriteLocker lock(getLock()); + _ignoreAvatars = qVectorEntityItemIDFromScriptValue(ignoreAvatars); +} + +void RayPick::setIncludeAvatars(const QScriptValue& includeAvatars) { + QWriteLocker lock(getLock()); + _includeAvatars = qVectorEntityItemIDFromScriptValue(includeAvatars); +} \ No newline at end of file diff --git a/interface/src/raypick/RayPick.h b/interface/src/raypick/RayPick.h index 428e44d670..6dacc084b4 100644 --- a/interface/src/raypick/RayPick.h +++ b/interface/src/raypick/RayPick.h @@ -103,13 +103,13 @@ public: virtual const PickRay getPickRay(bool& valid) const = 0; - void enable() { _enabled = true; } - void disable() { _enabled = false; } + void enable(); + void disable(); const RayPickFilter& getFilter() { return _filter; } float getMaxDistance() { return _maxDistance; } bool isEnabled() { return _enabled; } - const RayPickResult& getPrevRayPickResult() { return _prevResult; } + const RayPickResult& getPrevRayPickResult(); void setPrecisionPicking(bool precisionPicking) { _filter.setFlag(RayPickFilter::PICK_COURSE, !precisionPicking); } @@ -121,12 +121,12 @@ public: const QVector& getIncludeOverlays() { return _includeOverlays; } const QVector& getIgnoreAvatars() { return _ignoreAvatars; } const QVector& getIncludeAvatars() { return _includeAvatars; } - void setIgnoreEntities(const QScriptValue& ignoreEntities) { _ignoreEntities = qVectorEntityItemIDFromScriptValue(ignoreEntities); } - void setIncludeEntities(const QScriptValue& includeEntities) { _includeEntities = qVectorEntityItemIDFromScriptValue(includeEntities); } - void setIgnoreOverlays(const QScriptValue& ignoreOverlays) { _ignoreOverlays = qVectorOverlayIDFromScriptValue(ignoreOverlays); } - void setIncludeOverlays(const QScriptValue& includeOverlays) { _includeOverlays = qVectorOverlayIDFromScriptValue(includeOverlays); } - void setIgnoreAvatars(const QScriptValue& ignoreAvatars) { _ignoreAvatars = qVectorEntityItemIDFromScriptValue(ignoreAvatars); } - void setIncludeAvatars(const QScriptValue& includeAvatars) { _includeAvatars = qVectorEntityItemIDFromScriptValue(includeAvatars); } + void setIgnoreEntities(const QScriptValue& ignoreEntities); + void setIncludeEntities(const QScriptValue& includeEntities); + void setIgnoreOverlays(const QScriptValue& ignoreOverlays); + void setIncludeOverlays(const QScriptValue& includeOverlays); + void setIgnoreAvatars(const QScriptValue& ignoreAvatars); + void setIncludeAvatars(const QScriptValue& includeAvatars); QReadWriteLock* getLock() { return &_lock; } diff --git a/interface/src/raypick/RayPickManager.cpp b/interface/src/raypick/RayPickManager.cpp index 65f82dcd5f..1728ecd01a 100644 --- a/interface/src/raypick/RayPickManager.cpp +++ b/interface/src/raypick/RayPickManager.cpp @@ -153,7 +153,6 @@ void RayPickManager::enableRayPick(const QUuid uid) { QReadLocker containsLock(&_containsLock); auto rayPick = _rayPicks.find(uid); if (rayPick != _rayPicks.end()) { - QWriteLocker rayPickLock(rayPick.value()->getLock()); rayPick.value()->enable(); } } @@ -162,7 +161,6 @@ void RayPickManager::disableRayPick(const QUuid uid) { QReadLocker containsLock(&_containsLock); auto rayPick = _rayPicks.find(uid); if (rayPick != _rayPicks.end()) { - QWriteLocker rayPickLock(rayPick.value()->getLock()); rayPick.value()->disable(); } } @@ -171,7 +169,6 @@ const RayPickResult RayPickManager::getPrevRayPickResult(const QUuid uid) { QReadLocker containsLock(&_containsLock); auto rayPick = _rayPicks.find(uid); if (rayPick != _rayPicks.end()) { - QReadLocker lock(rayPick.value()->getLock()); return rayPick.value()->getPrevRayPickResult(); } return RayPickResult(); @@ -181,7 +178,6 @@ void RayPickManager::setPrecisionPicking(QUuid uid, const bool precisionPicking) QReadLocker containsLock(&_containsLock); auto rayPick = _rayPicks.find(uid); if (rayPick != _rayPicks.end()) { - QWriteLocker lock(rayPick.value()->getLock()); rayPick.value()->setPrecisionPicking(precisionPicking); } } @@ -190,7 +186,6 @@ void RayPickManager::setIgnoreEntities(QUuid uid, const QScriptValue& ignoreEnti QReadLocker containsLock(&_containsLock); auto rayPick = _rayPicks.find(uid); if (rayPick != _rayPicks.end()) { - QWriteLocker lock(rayPick.value()->getLock()); rayPick.value()->setIgnoreEntities(ignoreEntities); } } @@ -199,7 +194,6 @@ void RayPickManager::setIncludeEntities(QUuid uid, const QScriptValue& includeEn QReadLocker containsLock(&_containsLock); auto rayPick = _rayPicks.find(uid); if (rayPick != _rayPicks.end()) { - QWriteLocker lock(rayPick.value()->getLock()); rayPick.value()->setIncludeEntities(includeEntities); } } @@ -208,7 +202,6 @@ void RayPickManager::setIgnoreOverlays(QUuid uid, const QScriptValue& ignoreOver QReadLocker containsLock(&_containsLock); auto rayPick = _rayPicks.find(uid); if (rayPick != _rayPicks.end()) { - QWriteLocker lock(rayPick.value()->getLock()); rayPick.value()->setIgnoreOverlays(ignoreOverlays); } } @@ -217,7 +210,6 @@ void RayPickManager::setIncludeOverlays(QUuid uid, const QScriptValue& includeOv QReadLocker containsLock(&_containsLock); auto rayPick = _rayPicks.find(uid); if (rayPick != _rayPicks.end()) { - QWriteLocker lock(rayPick.value()->getLock()); rayPick.value()->setIncludeOverlays(includeOverlays); } } @@ -226,7 +218,6 @@ void RayPickManager::setIgnoreAvatars(QUuid uid, const QScriptValue& ignoreAvata QReadLocker containsLock(&_containsLock); auto rayPick = _rayPicks.find(uid); if (rayPick != _rayPicks.end()) { - QWriteLocker lock(rayPick.value()->getLock()); rayPick.value()->setIgnoreAvatars(ignoreAvatars); } } @@ -235,7 +226,6 @@ void RayPickManager::setIncludeAvatars(QUuid uid, const QScriptValue& includeAva QReadLocker containsLock(&_containsLock); auto rayPick = _rayPicks.find(uid); if (rayPick != _rayPicks.end()) { - QWriteLocker lock(rayPick.value()->getLock()); rayPick.value()->setIncludeAvatars(includeAvatars); } } \ No newline at end of file