From 133d48ebee8c24cfd1fba989037da20b361573e7 Mon Sep 17 00:00:00 2001 From: Brad Hefta-Gaub Date: Tue, 27 Oct 2015 15:53:48 -0700 Subject: [PATCH] CR feedback --- interface/src/Application.cpp | 53 +++++++++--------------------- libraries/avatars/src/HandData.cpp | 19 +++++------ libraries/avatars/src/HandData.h | 28 +++++++++++----- 3 files changed, 44 insertions(+), 56 deletions(-) diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp index 910cbb19e2..29f4bd6ece 100644 --- a/interface/src/Application.cpp +++ b/interface/src/Application.cpp @@ -4811,30 +4811,9 @@ void Application::setPalmData(Hand* hand, const controller::Pose& pose, float de // reading or writing to the Palms. This is definitely not the best way of handling this, and I'd like to see more // of this palm manipulation in the Hand class itself. But unfortunately the Hand and Palm don't knbow about // controller::Pose. More work is needed to clean this up. - hand->modifyPalms([&](std::vector& palms) { + hand->modifyPalm(whichHand, [&](PalmData& palm) { auto myAvatar = DependencyManager::get()->getMyAvatar(); - PalmData* palm; - bool foundHand = false; - - // FIXME - this little chuck of code is a hot mess. It's basically searching the palms - // for the one that matches the "sixenseID" that is the "index" parameter. If it - // doesn't find it, then it creates a new palm and sets that palm's ID this really - // can and should be inside the HandData class - for (size_t j = 0; j < palms.size(); j++) { - if (palms[j].whichHand() == whichHand) { - palm = &(palms[j]); - foundHand = true; - break; - } - } - if (!foundHand) { - PalmData newPalm(hand); - palms.push_back(newPalm); - palm = &(palms[palms.size() - 1]); // FIXME - lame - palm->setHand(whichHand); - } - - palm->setActive(pose.isValid()); + palm.setActive(pose.isValid()); // transform from sensor space, to world space, to avatar model space. glm::mat4 poseMat = createMatFromQuatAndPos(pose.getRotation(), pose.getTranslation()); @@ -4848,46 +4827,46 @@ void Application::setPalmData(Hand* hand, const controller::Pose& pose, float de // Compute current velocity from position change glm::vec3 rawVelocity; if (deltaTime > 0.0f) { - rawVelocity = (position - palm->getRawPosition()) / deltaTime; + rawVelocity = (position - palm.getRawPosition()) / deltaTime; } else { rawVelocity = glm::vec3(0.0f); } - palm->setRawVelocity(rawVelocity); // meters/sec + palm.setRawVelocity(rawVelocity); // meters/sec // Angular Velocity of Palm - glm::quat deltaRotation = rotation * glm::inverse(palm->getRawRotation()); + glm::quat deltaRotation = rotation * glm::inverse(palm.getRawRotation()); glm::vec3 angularVelocity(0.0f); float rotationAngle = glm::angle(deltaRotation); if ((rotationAngle > EPSILON) && (deltaTime > 0.0f)) { angularVelocity = glm::normalize(glm::axis(deltaRotation)); angularVelocity *= (rotationAngle / deltaTime); - palm->setRawAngularVelocity(angularVelocity); + palm.setRawAngularVelocity(angularVelocity); } else { - palm->setRawAngularVelocity(glm::vec3(0.0f)); + palm.setRawAngularVelocity(glm::vec3(0.0f)); } if (controller::InputDevice::getLowVelocityFilter()) { // Use a velocity sensitive filter to damp small motions and preserve large ones with // no latency. float velocityFilter = glm::clamp(1.0f - glm::length(rawVelocity), 0.0f, 1.0f); - position = palm->getRawPosition() * velocityFilter + position * (1.0f - velocityFilter); - rotation = safeMix(palm->getRawRotation(), rotation, 1.0f - velocityFilter); + position = palm.getRawPosition() * velocityFilter + position * (1.0f - velocityFilter); + rotation = safeMix(palm.getRawRotation(), rotation, 1.0f - velocityFilter); } - palm->setRawPosition(position); - palm->setRawRotation(rotation); + palm.setRawPosition(position); + palm.setRawRotation(rotation); // Store the one fingertip in the palm structure so we can track velocity const float FINGER_LENGTH = 0.3f; // meters const glm::vec3 FINGER_VECTOR(0.0f, FINGER_LENGTH, 0.0f); const glm::vec3 newTipPosition = position + rotation * FINGER_VECTOR; - glm::vec3 oldTipPosition = palm->getTipRawPosition(); + glm::vec3 oldTipPosition = palm.getTipRawPosition(); if (deltaTime > 0.0f) { - palm->setTipVelocity((newTipPosition - oldTipPosition) / deltaTime); + palm.setTipVelocity((newTipPosition - oldTipPosition) / deltaTime); } else { - palm->setTipVelocity(glm::vec3(0.0f)); + palm.setTipVelocity(glm::vec3(0.0f)); } - palm->setTipPosition(newTipPosition); - palm->setTrigger(triggerValue); + palm.setTipPosition(newTipPosition); + palm.setTrigger(triggerValue); }); } diff --git a/libraries/avatars/src/HandData.cpp b/libraries/avatars/src/HandData.cpp index 4e08f5190f..0237f8ecd9 100644 --- a/libraries/avatars/src/HandData.cpp +++ b/libraries/avatars/src/HandData.cpp @@ -21,20 +21,17 @@ HandData::HandData(AvatarData* owningAvatar) : _owningAvatarData(owningAvatar) { - // FIXME - this is likely the source of the fact that with Hydras and other input plugins with hand controllers - // we end up with 4 palms... because we end up adding palms once we know the SixenseIDs - // Start with two palms - addNewPalm(); - addNewPalm(); + addNewPalm(LeftHand); + addNewPalm(RightHand); } glm::vec3 HandData::worldToLocalVector(const glm::vec3& worldVector) const { return glm::inverse(getBaseOrientation()) * worldVector / getBaseScale(); } -PalmData& HandData::addNewPalm() { +PalmData& HandData::addNewPalm(Hand whichHand) { QWriteLocker locker(&_palmsLock); - _palms.push_back(PalmData(this)); + _palms.push_back(PalmData(this, whichHand)); return _palms.back(); } @@ -48,7 +45,8 @@ PalmData HandData::getCopyOfPalmData(Hand hand) const { return palm; } } - return PalmData(nullptr); // invalid hand + PalmData noData; + return noData; // invalid hand } void HandData::getLeftRightPalmIndices(int& leftPalmIndex, int& rightPalmIndex) const { @@ -68,7 +66,7 @@ void HandData::getLeftRightPalmIndices(int& leftPalmIndex, int& rightPalmIndex) } } -PalmData::PalmData(HandData* owningHandData) : +PalmData::PalmData(HandData* owningHandData, HandData::Hand hand) : _rawRotation(0.0f, 0.0f, 0.0f, 1.0f), _rawPosition(0.0f), _rawVelocity(0.0f), @@ -76,7 +74,8 @@ _rawAngularVelocity(0.0f), _totalPenetration(0.0f), _isActive(false), _numFramesWithoutData(0), -_owningHandData(owningHandData) { +_owningHandData(owningHandData), +_hand(hand) { } void PalmData::addToPosition(const glm::vec3& delta) { diff --git a/libraries/avatars/src/HandData.h b/libraries/avatars/src/HandData.h index ca2aa9836a..10292daccc 100644 --- a/libraries/avatars/src/HandData.h +++ b/libraries/avatars/src/HandData.h @@ -30,9 +30,9 @@ class PalmData; class HandData { public: enum Hand { - UnknownHand, - RightHand, LeftHand, + RightHand, + UnknownHand, NUMBER_OF_HANDS }; @@ -52,7 +52,6 @@ public: PalmData getCopyOfPalmData(Hand hand) const; - PalmData& addNewPalm(); std::vector getCopyOfPalms() const { QReadLocker locker(&_palmsLock); return _palms; } /// Finds the indices of the left and right palms according to their locations, or -1 if either or @@ -70,9 +69,17 @@ public: glm::quat getBaseOrientation() const; - /// Allows a lamda function write access to the palms for this Hand - void modifyPalms(std::function& palms)> callback) - { QWriteLocker locker(&_palmsLock); callback(_palms);} + /// Allows a lamda function write access to the specific palm for this Hand, this might + /// modify the _palms vector + template void modifyPalm(Hand whichHand, PalmModifierFunction callback) { + QReadLocker locker(&_palmsLock); + for (auto& palm : _palms) { + if (palm.whichHand() == whichHand && palm.isValid()) { + callback(palm); + return; + } + } + } friend class AvatarData; protected: @@ -82,7 +89,10 @@ protected: glm::vec3 getBasePosition() const; float getBaseScale() const; - + + PalmData& addNewPalm(Hand whichHand); + PalmData& getPalmData(Hand hand); + private: // privatize copy ctor and assignment operator so copies of this object cannot be made HandData(const HandData&); @@ -92,7 +102,7 @@ private: class PalmData { public: - PalmData(HandData* owningHandData); + PalmData(HandData* owningHandData = nullptr, HandData::Hand hand = HandData::UnknownHand); glm::vec3 getPosition() const { return _owningHandData->localToWorldPosition(_rawPosition); } glm::vec3 getVelocity() const { return _owningHandData->localToWorldDirection(_rawVelocity); } @@ -158,7 +168,7 @@ private: float _trigger; bool _isActive; /// This has current valid data - HandData::Hand _hand = HandData::UnknownHand; + HandData::Hand _hand; int _numFramesWithoutData; /// after too many frames without data, this tracked object assumed lost. HandData* _owningHandData; };