CR feedback

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<PalmData>& palms) {
+    hand->modifyPalm(whichHand, [&](PalmData& palm) {
         auto myAvatar = DependencyManager::get<AvatarManager>()->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);
     });
 }
 

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) {

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<PalmData> 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<void(std::vector<PalmData>& 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<typename PalmModifierFunction> 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;
 };