From 2422c7e1bb73e76b3a1a53bb106aff74b4c17d03 Mon Sep 17 00:00:00 2001 From: "Anthony J. Thibault" Date: Fri, 2 Jun 2017 15:47:51 -0700 Subject: [PATCH] code review feedback --- .../animation/src/AnimInverseKinematics.cpp | 31 +++++++++++-------- .../animation/src/TranslationAccumulator.h | 10 +++--- libraries/shared/src/CubicHermiteSpline.h | 6 ++-- 3 files changed, 26 insertions(+), 21 deletions(-) diff --git a/libraries/animation/src/AnimInverseKinematics.cpp b/libraries/animation/src/AnimInverseKinematics.cpp index 12d480a578..c155f4733f 100644 --- a/libraries/animation/src/AnimInverseKinematics.cpp +++ b/libraries/animation/src/AnimInverseKinematics.cpp @@ -540,16 +540,21 @@ void AnimInverseKinematics::solveTargetWithSpline(const AnimContext& context, co if (splineJointInfo.jointIndex != _hipsIndex) { // constrain the amount the spine can stretch or compress float length = glm::length(relPose.trans()); - float defaultLength = glm::length(_skeleton->getRelativeDefaultPose(splineJointInfo.jointIndex).trans()); - const float STRETCH_COMPRESS_PERCENTAGE = 0.15f; - const float MAX_LENGTH = defaultLength * (1.0f + STRETCH_COMPRESS_PERCENTAGE); - const float MIN_LENGTH = defaultLength * (1.0f - STRETCH_COMPRESS_PERCENTAGE); - if (length > MAX_LENGTH) { - relPose.trans() = (relPose.trans() / length) * MAX_LENGTH; - constrained = true; - } else if (length < MIN_LENGTH) { - relPose.trans() = (relPose.trans() / length) * MIN_LENGTH; - constrained = true; + const float EPSILON = 0.0001f; + if (length > EPSILON) { + float defaultLength = glm::length(_skeleton->getRelativeDefaultPose(splineJointInfo.jointIndex).trans()); + const float STRETCH_COMPRESS_PERCENTAGE = 0.15f; + const float MAX_LENGTH = defaultLength * (1.0f + STRETCH_COMPRESS_PERCENTAGE); + const float MIN_LENGTH = defaultLength * (1.0f - STRETCH_COMPRESS_PERCENTAGE); + if (length > MAX_LENGTH) { + relPose.trans() = (relPose.trans() / length) * MAX_LENGTH; + constrained = true; + } else if (length < MIN_LENGTH) { + relPose.trans() = (relPose.trans() / length) * MIN_LENGTH; + constrained = true; + } + } else { + relPose.trans() = glm::vec3(0.0f); } } @@ -1521,10 +1526,10 @@ void AnimInverseKinematics::debugDrawSpineSplines(const AnimContext& context, co // draw red and white stripped spline, parameterized by arc length. // i.e. each stripe should be the same length. AnimPose geomToWorldPose = AnimPose(context.getRigToWorldMatrix() * context.getGeometryToRigMatrix()); - int NUM_SUBDIVISIONS = 20; - const float dArcLength = totalArcLength / NUM_SUBDIVISIONS; + const int NUM_SEGMENTS = 20; + const float dArcLength = totalArcLength / NUM_SEGMENTS; float arcLength = 0.0f; - for (int i = 0; i < NUM_SUBDIVISIONS; i++) { + for (int i = 0; i < NUM_SEGMENTS; i++) { float prevT = spline.arcLengthInverse(arcLength); float nextT = spline.arcLengthInverse(arcLength + dArcLength); DebugDraw::getInstance().drawRay(geomToWorldPose.xformPoint(spline(prevT)), geomToWorldPose.xformPoint(spline(nextT)), (i % 2) == 0 ? RED : WHITE); diff --git a/libraries/animation/src/TranslationAccumulator.h b/libraries/animation/src/TranslationAccumulator.h index 65f8a0dc97..18cac5ec7a 100644 --- a/libraries/animation/src/TranslationAccumulator.h +++ b/libraries/animation/src/TranslationAccumulator.h @@ -18,19 +18,19 @@ public: int size() const { return _totalWeight > 0.0f; } - /// \param rotation rotation to add - /// \param weight contribution factor of this rotation to total accumulation + /// \param translation translation to add + /// \param weight contribution factor of this translation to total accumulation void add(const glm::vec3& translation, float weight = 1.0f); glm::vec3 getAverage(); - /// \return true if any rotations were accumulated + /// \return true if any translation were accumulated bool isDirty() const { return _isDirty; } - /// \brief clear accumulated rotation but don't change _isDirty + /// \brief clear accumulated translation but don't change _isDirty void clear(); - /// \brief clear accumulated rotation and set _isDirty to false + /// \brief clear accumulated translation and set _isDirty to false void clearAndClean(); private: diff --git a/libraries/shared/src/CubicHermiteSpline.h b/libraries/shared/src/CubicHermiteSpline.h index d03136867a..a393e4c51f 100644 --- a/libraries/shared/src/CubicHermiteSpline.h +++ b/libraries/shared/src/CubicHermiteSpline.h @@ -23,8 +23,8 @@ public: // evalute the hermite curve at parameter t (0..1) glm::vec3 operator()(float t) const { - float t3 = t * t * t; float t2 = t * t; + float t3 = t2 * t; float w0 = 2.0f * t3 - 3.0f * t2 + 1.0f; float w1 = t3 - 2.0f * t2 + t; float w2 = -2.0f * t3 + 3.0f * t2; @@ -63,7 +63,7 @@ public: enum Constants { NUM_SUBDIVISIONS = 30 }; CubicHermiteSplineFunctorWithArcLength() : CubicHermiteSplineFunctor() { - memset(_values, 0, sizeof(float) * NUM_SUBDIVISIONS + 1); + memset(_values, 0, sizeof(float) * (NUM_SUBDIVISIONS + 1)); } CubicHermiteSplineFunctorWithArcLength(const glm::vec3& p0, const glm::vec3& m0, const glm::vec3& p1, const glm::vec3& m1) : CubicHermiteSplineFunctor(p0, m0, p1, m1) { // initialize _values with the accumulated arcLength along the spline. @@ -80,7 +80,7 @@ public: } CubicHermiteSplineFunctorWithArcLength(const CubicHermiteSplineFunctorWithArcLength& orig) : CubicHermiteSplineFunctor(orig) { - memcpy(_values, orig._values, sizeof(float) * NUM_SUBDIVISIONS + 1); + memcpy(_values, orig._values, sizeof(float) * (NUM_SUBDIVISIONS + 1)); } // given the spline parameter (0..1) output the arcLength of the spline up to that point.