From 2d4180f06503870304b88bbb6183e6564e66fca9 Mon Sep 17 00:00:00 2001 From: "Anthony J. Thibault" Date: Wed, 16 Dec 2015 13:15:16 -0800 Subject: [PATCH 1/5] Make Avatar palm position/rotation accessors thread safe These are called directly from script threads and can result in out of bounds array accesses when called while the main thread is initializing the rig. I was able to reproduce this in debug builds. --- interface/src/avatar/Avatar.cpp | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/interface/src/avatar/Avatar.cpp b/interface/src/avatar/Avatar.cpp index 7a234f2b47..a33c3975b6 100644 --- a/interface/src/avatar/Avatar.cpp +++ b/interface/src/avatar/Avatar.cpp @@ -1106,6 +1106,12 @@ void Avatar::rebuildSkeletonBody() { } glm::vec3 Avatar::getLeftPalmPosition() { + if (QThread::currentThread() != thread()) { + glm::vec3 result; + QMetaObject::invokeMethod(const_cast(this), "getLeftPalmPosition", Qt::BlockingQueuedConnection, + Q_RETURN_ARG(glm::vec3, result)); + return result; + } glm::vec3 leftHandPosition; getSkeletonModel().getLeftHandPosition(leftHandPosition); glm::quat leftRotation; @@ -1115,12 +1121,24 @@ glm::vec3 Avatar::getLeftPalmPosition() { } glm::quat Avatar::getLeftPalmRotation() { + if (QThread::currentThread() != thread()) { + glm::quat result; + QMetaObject::invokeMethod(const_cast(this), "getLeftPalmRotation", Qt::BlockingQueuedConnection, + Q_RETURN_ARG(glm::quat, result)); + return result; + } glm::quat leftRotation; getSkeletonModel().getJointRotationInWorldFrame(getSkeletonModel().getLeftHandJointIndex(), leftRotation); return leftRotation; } glm::vec3 Avatar::getRightPalmPosition() { + if (QThread::currentThread() != thread()) { + glm::vec3 result; + QMetaObject::invokeMethod(const_cast(this), "getRightPalmPosition", Qt::BlockingQueuedConnection, + Q_RETURN_ARG(glm::vec3, result)); + return result; + } glm::vec3 rightHandPosition; getSkeletonModel().getRightHandPosition(rightHandPosition); glm::quat rightRotation; @@ -1130,6 +1148,12 @@ glm::vec3 Avatar::getRightPalmPosition() { } glm::quat Avatar::getRightPalmRotation() { + if (QThread::currentThread() != thread()) { + glm::quat result; + QMetaObject::invokeMethod(const_cast(this), "getRightPalmRotation", Qt::BlockingQueuedConnection, + Q_RETURN_ARG(glm::quat, result)); + return result; + } glm::quat rightRotation; getSkeletonModel().getJointRotationInWorldFrame(getSkeletonModel().getRightHandJointIndex(), rightRotation); return rightRotation; From e7f3f549a593c8c73d00704e7cfcd883648bd5c2 Mon Sep 17 00:00:00 2001 From: "Anthony J. Thibault" Date: Tue, 29 Dec 2015 15:10:38 -0800 Subject: [PATCH 2/5] Avatar getPalm* methods are thread-safe and non-blocking Uses newly added ThreadSafeValue template class. --- interface/src/avatar/Avatar.cpp | 85 +++++++++++++------------- interface/src/avatar/Avatar.h | 8 +++ interface/src/avatar/MyAvatar.cpp | 7 +++ interface/src/avatar/MyAvatar.h | 6 +- libraries/shared/src/ThreadSafeValue.h | 49 +++++++++++++++ 5 files changed, 111 insertions(+), 44 deletions(-) create mode 100644 libraries/shared/src/ThreadSafeValue.h diff --git a/interface/src/avatar/Avatar.cpp b/interface/src/avatar/Avatar.cpp index 95085f62d6..e26499db71 100644 --- a/interface/src/avatar/Avatar.cpp +++ b/interface/src/avatar/Avatar.cpp @@ -257,6 +257,8 @@ void Avatar::simulate(float deltaTime) { // until velocity is included in AvatarData update message. //_position += _velocity * deltaTime; measureMotionDerivatives(deltaTime); + + updatePalms(); } bool Avatar::isLookingAtMe(AvatarSharedPointer avatar) const { @@ -1126,58 +1128,24 @@ void Avatar::rebuildCollisionShape() { } } +// thread-safe glm::vec3 Avatar::getLeftPalmPosition() { - if (QThread::currentThread() != thread()) { - glm::vec3 result; - QMetaObject::invokeMethod(const_cast(this), "getLeftPalmPosition", Qt::BlockingQueuedConnection, - Q_RETURN_ARG(glm::vec3, result)); - return result; - } - glm::vec3 leftHandPosition; - getSkeletonModel().getLeftHandPosition(leftHandPosition); - glm::quat leftRotation; - getSkeletonModel().getJointRotationInWorldFrame(getSkeletonModel().getLeftHandJointIndex(), leftRotation); - leftHandPosition += HAND_TO_PALM_OFFSET * glm::inverse(leftRotation); - return leftHandPosition; + return _leftPalmPosition.get(); } +// thread-safe glm::quat Avatar::getLeftPalmRotation() { - if (QThread::currentThread() != thread()) { - glm::quat result; - QMetaObject::invokeMethod(const_cast(this), "getLeftPalmRotation", Qt::BlockingQueuedConnection, - Q_RETURN_ARG(glm::quat, result)); - return result; - } - glm::quat leftRotation; - getSkeletonModel().getJointRotationInWorldFrame(getSkeletonModel().getLeftHandJointIndex(), leftRotation); - return leftRotation; + return _leftPalmRotation.get(); } +// thread-safe glm::vec3 Avatar::getRightPalmPosition() { - if (QThread::currentThread() != thread()) { - glm::vec3 result; - QMetaObject::invokeMethod(const_cast(this), "getRightPalmPosition", Qt::BlockingQueuedConnection, - Q_RETURN_ARG(glm::vec3, result)); - return result; - } - glm::vec3 rightHandPosition; - getSkeletonModel().getRightHandPosition(rightHandPosition); - glm::quat rightRotation; - getSkeletonModel().getJointRotationInWorldFrame(getSkeletonModel().getRightHandJointIndex(), rightRotation); - rightHandPosition += HAND_TO_PALM_OFFSET * glm::inverse(rightRotation); - return rightHandPosition; + return _rightPalmPosition.get(); } +// thread-safe glm::quat Avatar::getRightPalmRotation() { - if (QThread::currentThread() != thread()) { - glm::quat result; - QMetaObject::invokeMethod(const_cast(this), "getRightPalmRotation", Qt::BlockingQueuedConnection, - Q_RETURN_ARG(glm::quat, result)); - return result; - } - glm::quat rightRotation; - getSkeletonModel().getJointRotationInWorldFrame(getSkeletonModel().getRightHandJointIndex(), rightRotation); - return rightRotation; + return _rightPalmRotation.get(); } void Avatar::setPosition(const glm::vec3& position) { @@ -1189,3 +1157,36 @@ void Avatar::setOrientation(const glm::quat& orientation) { AvatarData::setOrientation(orientation); updateAttitude(); } + +void Avatar::updatePalms() { + + // get palm rotations + glm::quat leftPalmRotation, rightPalmRotation; + getSkeletonModel().getJointRotationInWorldFrame(getSkeletonModel().getLeftHandJointIndex(), leftPalmRotation); + getSkeletonModel().getJointRotationInWorldFrame(getSkeletonModel().getLeftHandJointIndex(), rightPalmRotation); + + // get palm positions + glm::vec3 leftPalmPosition, rightPalmPosition; + getSkeletonModel().getLeftHandPosition(leftPalmPosition); + getSkeletonModel().getRightHandPosition(rightPalmPosition); + leftPalmPosition += HAND_TO_PALM_OFFSET * glm::inverse(leftPalmRotation); + rightPalmPosition += HAND_TO_PALM_OFFSET * glm::inverse(rightPalmRotation); + + // update thread-safe values + _leftPalmRotation.update([&](glm::quat& value, bool hasPending, const glm::quat& pendingValue) { + value = leftPalmRotation; + return false; + }); + _rightPalmRotation.update([&](glm::quat& value, bool hasPending, const glm::quat& pendingValue) { + value = rightPalmRotation; + return false; + }); + _leftPalmPosition.update([&](glm::vec3& value, bool hasPending, const glm::vec3& pendingValue) { + value = rightPalmPosition; + return false; + }); + _rightPalmPosition.update([&](glm::vec3& value, bool hasPending, const glm::vec3& pendingValue) { + value = leftPalmPosition; + return false; + }); +} diff --git a/interface/src/avatar/Avatar.h b/interface/src/avatar/Avatar.h index 4926212b4d..27bd6c9e24 100644 --- a/interface/src/avatar/Avatar.h +++ b/interface/src/avatar/Avatar.h @@ -27,6 +27,7 @@ #include "SkeletonModel.h" #include "world.h" #include "Rig.h" +#include namespace render { template <> const ItemKey payloadGetKey(const AvatarSharedPointer& avatar); @@ -225,8 +226,15 @@ protected: virtual void updateJointMappings() override; + virtual void updatePalms(); + render::ItemID _renderItemID; + ThreadSafeValue _leftPalmPosition { glm::vec3() }; + ThreadSafeValue _leftPalmRotation { glm::quat() }; + ThreadSafeValue _rightPalmPosition { glm::vec3() }; + ThreadSafeValue _rightPalmRotation { glm::quat() }; + private: bool _initialized; NetworkTexturePointer _billboardTexture; diff --git a/interface/src/avatar/MyAvatar.cpp b/interface/src/avatar/MyAvatar.cpp index c989f1dc71..fb4c93a748 100644 --- a/interface/src/avatar/MyAvatar.cpp +++ b/interface/src/avatar/MyAvatar.cpp @@ -412,6 +412,8 @@ void MyAvatar::updateSensorToWorldMatrix() { // position when driven from the head. glm::mat4 desiredMat = createMatFromQuatAndPos(getOrientation(), getPosition()); _sensorToWorldMatrix = desiredMat * glm::inverse(_bodySensorMatrix); + + lateUpdatePalms(); } // Update avatar head rotation with sensor data @@ -1829,3 +1831,8 @@ QScriptValue audioListenModeToScriptValue(QScriptEngine* engine, const AudioList void audioListenModeFromScriptValue(const QScriptValue& object, AudioListenerMode& audioListenerMode) { audioListenerMode = (AudioListenerMode)object.toUInt16(); } + + +void MyAvatar::lateUpdatePalms() { + Avatar::updatePalms(); +} diff --git a/interface/src/avatar/MyAvatar.h b/interface/src/avatar/MyAvatar.h index 019ba0f992..cf4c9f6b93 100644 --- a/interface/src/avatar/MyAvatar.h +++ b/interface/src/avatar/MyAvatar.h @@ -309,12 +309,14 @@ private: void setVisibleInSceneIfReady(Model* model, render::ScenePointer scene, bool visiblity); - PalmData getActivePalmData(int palmIndex) const; - // derive avatar body position and orientation from the current HMD Sensor location. // results are in HMD frame glm::mat4 deriveBodyFromHMDSensor() const; + virtual void updatePalms() override {} + void lateUpdatePalms(); + + float _driveKeys[MAX_DRIVE_KEYS]; bool _wasPushing; bool _isPushing; diff --git a/libraries/shared/src/ThreadSafeValue.h b/libraries/shared/src/ThreadSafeValue.h new file mode 100644 index 0000000000..3992cfd492 --- /dev/null +++ b/libraries/shared/src/ThreadSafeValue.h @@ -0,0 +1,49 @@ +// +// ThreadSafeValue.h +// interface/src/avatar +// +// Copyright 2012 High Fidelity, Inc. +// +// Distributed under the Apache License, Version 2.0. +// See the accompanying file LICENSE or http://www.apache.org/licenses/LICENSE-2.0.html +// + +#ifndef hifi_ThreadSafeValue_h +#define hifi_ThreadSafeValue_h + +#include + +template +class ThreadSafeValue { +public: + ThreadSafeValue(const T& v) : _value { v }, _pending { v }, _hasPending { false } {} + + template + void update(const F& callback) { + std::lock_guard guard(_mutex); + _hasPending = callback((T&)_value, _hasPending, (const T&)_pending); + } + + T get() const { + std::lock_guard guard(_mutex); + return _value; + } + + void set(const T& v) { + std::lock_guard guard(_mutex); + _hasPending = true; + _pending = v; + } + +private: + mutable std::mutex _mutex; + T _value; + T _pending; + bool _hasPending; + + // no copies + ThreadSafeValue(const ThreadSafeValue&) = delete; + ThreadSafeValue& operator=(const ThreadSafeValue&) = delete; +}; + +#endif // #define hifi_ThreadSafeValue_h From 17e6a04aa8d7b63cd6205221d6d29cfc1d4d73bb Mon Sep 17 00:00:00 2001 From: "Anthony J. Thibault" Date: Tue, 29 Dec 2015 16:35:31 -0800 Subject: [PATCH 3/5] Renamed ThreadSafeValue to ThreadSafeValueCache Also bug fixes where I was incorrectly swapping left and right hands. --- interface/src/avatar/Avatar.cpp | 26 +++++++++---------- interface/src/avatar/Avatar.h | 10 +++---- ...readSafeValue.h => ThreadSafeValueCache.h} | 22 +++++++++------- 3 files changed, 31 insertions(+), 27 deletions(-) rename libraries/shared/src/{ThreadSafeValue.h => ThreadSafeValueCache.h} (60%) diff --git a/interface/src/avatar/Avatar.cpp b/interface/src/avatar/Avatar.cpp index e26499db71..cb956d917a 100644 --- a/interface/src/avatar/Avatar.cpp +++ b/interface/src/avatar/Avatar.cpp @@ -1130,22 +1130,22 @@ void Avatar::rebuildCollisionShape() { // thread-safe glm::vec3 Avatar::getLeftPalmPosition() { - return _leftPalmPosition.get(); + return _leftPalmPositionCache.get(); } // thread-safe glm::quat Avatar::getLeftPalmRotation() { - return _leftPalmRotation.get(); + return _leftPalmRotationCache.get(); } // thread-safe glm::vec3 Avatar::getRightPalmPosition() { - return _rightPalmPosition.get(); + return _rightPalmPositionCache.get(); } // thread-safe glm::quat Avatar::getRightPalmRotation() { - return _rightPalmRotation.get(); + return _rightPalmRotationCache.get(); } void Avatar::setPosition(const glm::vec3& position) { @@ -1163,7 +1163,7 @@ void Avatar::updatePalms() { // get palm rotations glm::quat leftPalmRotation, rightPalmRotation; getSkeletonModel().getJointRotationInWorldFrame(getSkeletonModel().getLeftHandJointIndex(), leftPalmRotation); - getSkeletonModel().getJointRotationInWorldFrame(getSkeletonModel().getLeftHandJointIndex(), rightPalmRotation); + getSkeletonModel().getJointRotationInWorldFrame(getSkeletonModel().getRightHandJointIndex(), rightPalmRotation); // get palm positions glm::vec3 leftPalmPosition, rightPalmPosition; @@ -1172,21 +1172,21 @@ void Avatar::updatePalms() { leftPalmPosition += HAND_TO_PALM_OFFSET * glm::inverse(leftPalmRotation); rightPalmPosition += HAND_TO_PALM_OFFSET * glm::inverse(rightPalmRotation); - // update thread-safe values - _leftPalmRotation.update([&](glm::quat& value, bool hasPending, const glm::quat& pendingValue) { + // update thread-safe cache values + _leftPalmRotationCache.update([&](glm::quat& value, bool hasPending, const glm::quat& pendingValue) { value = leftPalmRotation; return false; }); - _rightPalmRotation.update([&](glm::quat& value, bool hasPending, const glm::quat& pendingValue) { + _rightPalmRotationCache.update([&](glm::quat& value, bool hasPending, const glm::quat& pendingValue) { value = rightPalmRotation; return false; }); - _leftPalmPosition.update([&](glm::vec3& value, bool hasPending, const glm::vec3& pendingValue) { - value = rightPalmPosition; - return false; - }); - _rightPalmPosition.update([&](glm::vec3& value, bool hasPending, const glm::vec3& pendingValue) { + _leftPalmPositionCache.update([&](glm::vec3& value, bool hasPending, const glm::vec3& pendingValue) { value = leftPalmPosition; return false; }); + _rightPalmPositionCache.update([&](glm::vec3& value, bool hasPending, const glm::vec3& pendingValue) { + value = rightPalmPosition; + return false; + }); } diff --git a/interface/src/avatar/Avatar.h b/interface/src/avatar/Avatar.h index 27bd6c9e24..815d9cb455 100644 --- a/interface/src/avatar/Avatar.h +++ b/interface/src/avatar/Avatar.h @@ -27,7 +27,7 @@ #include "SkeletonModel.h" #include "world.h" #include "Rig.h" -#include +#include namespace render { template <> const ItemKey payloadGetKey(const AvatarSharedPointer& avatar); @@ -230,10 +230,10 @@ protected: render::ItemID _renderItemID; - ThreadSafeValue _leftPalmPosition { glm::vec3() }; - ThreadSafeValue _leftPalmRotation { glm::quat() }; - ThreadSafeValue _rightPalmPosition { glm::vec3() }; - ThreadSafeValue _rightPalmRotation { glm::quat() }; + ThreadSafeValueCache _leftPalmPositionCache { glm::vec3() }; + ThreadSafeValueCache _leftPalmRotationCache { glm::quat() }; + ThreadSafeValueCache _rightPalmPositionCache { glm::vec3() }; + ThreadSafeValueCache _rightPalmRotationCache { glm::quat() }; private: bool _initialized; diff --git a/libraries/shared/src/ThreadSafeValue.h b/libraries/shared/src/ThreadSafeValueCache.h similarity index 60% rename from libraries/shared/src/ThreadSafeValue.h rename to libraries/shared/src/ThreadSafeValueCache.h index 3992cfd492..0e8be477b7 100644 --- a/libraries/shared/src/ThreadSafeValue.h +++ b/libraries/shared/src/ThreadSafeValueCache.h @@ -1,5 +1,5 @@ // -// ThreadSafeValue.h +// ThreadSafeValueCache.h // interface/src/avatar // // Copyright 2012 High Fidelity, Inc. @@ -8,15 +8,15 @@ // See the accompanying file LICENSE or http://www.apache.org/licenses/LICENSE-2.0.html // -#ifndef hifi_ThreadSafeValue_h -#define hifi_ThreadSafeValue_h +#ifndef hifi_ThreadSafeValueCache_h +#define hifi_ThreadSafeValueCache_h #include template -class ThreadSafeValue { +class ThreadSafeValueCache { public: - ThreadSafeValue(const T& v) : _value { v }, _pending { v }, _hasPending { false } {} + ThreadSafeValueCache(const T& v) : _value { v }, _pending { v }, _hasPending { false } {} template void update(const F& callback) { @@ -26,7 +26,11 @@ public: T get() const { std::lock_guard guard(_mutex); - return _value; + if (_hasPending) { + return _pending; + } else { + return _value; + } } void set(const T& v) { @@ -42,8 +46,8 @@ private: bool _hasPending; // no copies - ThreadSafeValue(const ThreadSafeValue&) = delete; - ThreadSafeValue& operator=(const ThreadSafeValue&) = delete; + ThreadSafeValueCache(const ThreadSafeValueCache&) = delete; + ThreadSafeValueCache& operator=(const ThreadSafeValueCache&) = delete; }; -#endif // #define hifi_ThreadSafeValue_h +#endif // #define hifi_ThreadSafeValueCache_h From feb63772f39b6ff4c544b5d340ab38d9688f4917 Mon Sep 17 00:00:00 2001 From: "Anthony J. Thibault" Date: Tue, 29 Dec 2015 17:08:17 -0800 Subject: [PATCH 4/5] ThreadSafeValueCache added class and method descriptions Also, changed callback prototype. --- interface/src/avatar/Avatar.cpp | 22 ++++++++----------- libraries/shared/src/ThreadSafeValueCache.h | 24 +++++++++++++++++++-- 2 files changed, 31 insertions(+), 15 deletions(-) diff --git a/interface/src/avatar/Avatar.cpp b/interface/src/avatar/Avatar.cpp index cb956d917a..f30f00f079 100644 --- a/interface/src/avatar/Avatar.cpp +++ b/interface/src/avatar/Avatar.cpp @@ -1172,21 +1172,17 @@ void Avatar::updatePalms() { leftPalmPosition += HAND_TO_PALM_OFFSET * glm::inverse(leftPalmRotation); rightPalmPosition += HAND_TO_PALM_OFFSET * glm::inverse(rightPalmRotation); - // update thread-safe cache values - _leftPalmRotationCache.update([&](glm::quat& value, bool hasPending, const glm::quat& pendingValue) { - value = leftPalmRotation; - return false; + // update thread-safe caches + _leftPalmRotationCache.merge([&](const glm::quat& value, bool hasPending, const glm::quat& pendingValue) { + return leftPalmRotation; }); - _rightPalmRotationCache.update([&](glm::quat& value, bool hasPending, const glm::quat& pendingValue) { - value = rightPalmRotation; - return false; + _rightPalmRotationCache.merge([&](const glm::quat& value, bool hasPending, const glm::quat& pendingValue) { + return rightPalmRotation; }); - _leftPalmPositionCache.update([&](glm::vec3& value, bool hasPending, const glm::vec3& pendingValue) { - value = leftPalmPosition; - return false; + _leftPalmPositionCache.merge([&](const glm::vec3& value, bool hasPending, const glm::vec3& pendingValue) { + return leftPalmPosition; }); - _rightPalmPositionCache.update([&](glm::vec3& value, bool hasPending, const glm::vec3& pendingValue) { - value = rightPalmPosition; - return false; + _rightPalmPositionCache.merge([&](const glm::vec3& value, bool hasPending, const glm::vec3& pendingValue) { + return rightPalmPosition; }); } diff --git a/libraries/shared/src/ThreadSafeValueCache.h b/libraries/shared/src/ThreadSafeValueCache.h index 0e8be477b7..5e9a7e8c4f 100644 --- a/libraries/shared/src/ThreadSafeValueCache.h +++ b/libraries/shared/src/ThreadSafeValueCache.h @@ -13,17 +13,36 @@ #include +// Helper class for for sharing a value type between threads. +// It allows many threads to get or set a value atomically. +// This provides cache semantics, any get will return the last set value. +// +// It also provides a mechanism for the owner of the cached value to reconcile +// the cached value with it's own internal values, via the merge method. +// +// For example: This can be used to copy values between C++ code running on the application thread +// and JavaScript which is running on a different thread. + template class ThreadSafeValueCache { public: ThreadSafeValueCache(const T& v) : _value { v }, _pending { v }, _hasPending { false } {} + // The callback function should have the following prototype. + // T func(const T& value, bool hasPending, const T& pendingValue); + // It will be called synchronously on the current thread possibly blocking it for a short time. + // it gives thread-safe access to the internal cache value, as well as the pending cache value + // that was set via the last set call. This gives the cache's owner the opportunity to update + // the cached value by resolving it with it's own internal state. The owner should then return + // the resolved value which will be atomically reflected into the cache. template - void update(const F& callback) { + void merge(F func) { std::lock_guard guard(_mutex); - _hasPending = callback((T&)_value, _hasPending, (const T&)_pending); + _value = func((const T&)_value, _hasPending, (const T&)_pending); + _hasPending = false; } + // returns atomic copy of the cached value. T get() const { std::lock_guard guard(_mutex); if (_hasPending) { @@ -33,6 +52,7 @@ public: } } + // will reflect copy of value into the cache. void set(const T& v) { std::lock_guard guard(_mutex); _hasPending = true; From a21b6047460a28e5def9df3b6fda48aed385b39c Mon Sep 17 00:00:00 2001 From: "Anthony J. Thibault" Date: Tue, 5 Jan 2016 11:08:08 -0800 Subject: [PATCH 5/5] Simplified implementation of ThreadSafeValueCache --- interface/src/avatar/Avatar.cpp | 16 +++-------- libraries/shared/src/ThreadSafeValueCache.h | 30 +++------------------ 2 files changed, 7 insertions(+), 39 deletions(-) diff --git a/interface/src/avatar/Avatar.cpp b/interface/src/avatar/Avatar.cpp index 5405540e4c..5c69ad100c 100644 --- a/interface/src/avatar/Avatar.cpp +++ b/interface/src/avatar/Avatar.cpp @@ -1198,16 +1198,8 @@ void Avatar::updatePalms() { rightPalmPosition += HAND_TO_PALM_OFFSET * glm::inverse(rightPalmRotation); // update thread-safe caches - _leftPalmRotationCache.merge([&](const glm::quat& value, bool hasPending, const glm::quat& pendingValue) { - return leftPalmRotation; - }); - _rightPalmRotationCache.merge([&](const glm::quat& value, bool hasPending, const glm::quat& pendingValue) { - return rightPalmRotation; - }); - _leftPalmPositionCache.merge([&](const glm::vec3& value, bool hasPending, const glm::vec3& pendingValue) { - return leftPalmPosition; - }); - _rightPalmPositionCache.merge([&](const glm::vec3& value, bool hasPending, const glm::vec3& pendingValue) { - return rightPalmPosition; - }); + _leftPalmRotationCache.set(leftPalmRotation); + _rightPalmRotationCache.set(rightPalmRotation); + _leftPalmPositionCache.set(leftPalmPosition); + _rightPalmPositionCache.set(rightPalmPosition); } diff --git a/libraries/shared/src/ThreadSafeValueCache.h b/libraries/shared/src/ThreadSafeValueCache.h index 5e9a7e8c4f..e4e78ca3d7 100644 --- a/libraries/shared/src/ThreadSafeValueCache.h +++ b/libraries/shared/src/ThreadSafeValueCache.h @@ -17,53 +17,29 @@ // It allows many threads to get or set a value atomically. // This provides cache semantics, any get will return the last set value. // -// It also provides a mechanism for the owner of the cached value to reconcile -// the cached value with it's own internal values, via the merge method. -// // For example: This can be used to copy values between C++ code running on the application thread // and JavaScript which is running on a different thread. template class ThreadSafeValueCache { public: - ThreadSafeValueCache(const T& v) : _value { v }, _pending { v }, _hasPending { false } {} - - // The callback function should have the following prototype. - // T func(const T& value, bool hasPending, const T& pendingValue); - // It will be called synchronously on the current thread possibly blocking it for a short time. - // it gives thread-safe access to the internal cache value, as well as the pending cache value - // that was set via the last set call. This gives the cache's owner the opportunity to update - // the cached value by resolving it with it's own internal state. The owner should then return - // the resolved value which will be atomically reflected into the cache. - template - void merge(F func) { - std::lock_guard guard(_mutex); - _value = func((const T&)_value, _hasPending, (const T&)_pending); - _hasPending = false; - } + ThreadSafeValueCache(const T& v) : _value { v } {} // returns atomic copy of the cached value. T get() const { std::lock_guard guard(_mutex); - if (_hasPending) { - return _pending; - } else { - return _value; - } + return _value; } // will reflect copy of value into the cache. void set(const T& v) { std::lock_guard guard(_mutex); - _hasPending = true; - _pending = v; + _value = v; } private: mutable std::mutex _mutex; T _value; - T _pending; - bool _hasPending; // no copies ThreadSafeValueCache(const ThreadSafeValueCache&) = delete;