From 0dbc83049b71f1207e2dfe30a8e4096cfd0ab656 Mon Sep 17 00:00:00 2001 From: Anthony Thibault Date: Wed, 6 Mar 2019 13:28:14 -0800 Subject: [PATCH 1/4] Make AnimPose from mat4 work better for matrices with negative determinants. Took part of this code from glm::decompose() which references https://opensource.apple.com/source/WebCore/WebCore-514/platform/graphics/transforms/TransformationMatrix.cpp --- libraries/animation/src/AnimPose.cpp | 50 +++++++++++++++++++++++ libraries/animation/src/AnimTwoBoneIK.cpp | 6 +-- tests/animation/CMakeLists.txt | 2 +- 3 files changed, 54 insertions(+), 4 deletions(-) diff --git a/libraries/animation/src/AnimPose.cpp b/libraries/animation/src/AnimPose.cpp index d77514e691..8b030fac39 100644 --- a/libraries/animation/src/AnimPose.cpp +++ b/libraries/animation/src/AnimPose.cpp @@ -13,12 +13,16 @@ #include #include #include "AnimUtil.h" +#include const AnimPose AnimPose::identity = AnimPose(glm::vec3(1.0f), glm::quat(), glm::vec3(0.0f)); +#define NEW_VERSION + AnimPose::AnimPose(const glm::mat4& mat) { +#if defined(ORIGINAL_VERSION) static const float EPSILON = 0.0001f; _scale = extractScale(mat); // quat_cast doesn't work so well with scaled matrices, so cancel it out. @@ -30,6 +34,52 @@ AnimPose::AnimPose(const glm::mat4& mat) { _rot = glm::quat(_rot.w * oneOverLength, _rot.x * oneOverLength, _rot.y * oneOverLength, _rot.z * oneOverLength); } _trans = extractTranslation(mat); +#elif defined(DECOMPOSE_VERSION) + // glm::decompose code + glm::vec3 scale; + glm::quat rotation; + glm::vec3 translation; + glm::vec3 skew; + glm::vec4 perspective; + bool result = glm::decompose(mat, scale, rotation, translation, skew, perspective); + _scale = scale; + _rot = rotation; + _trans = translation; + if (!result) { + // hack + const float HACK_FACTOR = 1000.0f; + glm::mat4 tmp = glm::scale(mat, HACK_FACTOR); + glm::decompose(tmp, scale, rotation, translation, skew, perspective); + _scale = scale / HACK_FACTOR; + _rot = rotation; + _trans = translation; + } +#elif defined(NEW_VERSION) + glm::mat3 m(mat); + _scale = glm::vec3(glm::length(m[0]), glm::length(m[1]), glm::length(m[2])); + float det = glm::determinant(m); + + glm::mat3 tmp; + if (det < 0.0f) { + _scale *= -1.0f; + } + + // quat_cast doesn't work so well with scaled matrices, so cancel out scale. + // also, as a side effect, multiply mirrored matrices by -1 to get the right rotation out. + tmp[0] = m[0] * (1.0f / _scale[0]); + tmp[1] = m[1] * (1.0f / _scale[1]); + tmp[2] = m[2] * (1.0f / _scale[2]); + _rot = glm::quat_cast(tmp); + + // normalize quat if necessary + float lengthSquared = glm::length2(_rot); + if (glm::abs(lengthSquared - 1.0f) > EPSILON) { + float oneOverLength = 1.0f / sqrtf(lengthSquared); + _rot = glm::quat(_rot.w * oneOverLength, _rot.x * oneOverLength, _rot.y * oneOverLength, _rot.z * oneOverLength); + } + + _trans = extractTranslation(mat); +#endif } glm::vec3 AnimPose::operator*(const glm::vec3& rhs) const { diff --git a/libraries/animation/src/AnimTwoBoneIK.cpp b/libraries/animation/src/AnimTwoBoneIK.cpp index 8960b15940..c91518d5db 100644 --- a/libraries/animation/src/AnimTwoBoneIK.cpp +++ b/libraries/animation/src/AnimTwoBoneIK.cpp @@ -156,7 +156,7 @@ const AnimPoseVec& AnimTwoBoneIK::evaluate(const AnimVariantMap& animVars, const glm::quat relMidRot = glm::angleAxis(midAngle, _midHingeAxis); // insert new relative pose into the chain and rebuild it. - ikChain.setRelativePoseAtJointIndex(_midJointIndex, AnimPose(relMidRot, underPoses[_midJointIndex].trans())); + ikChain.setRelativePoseAtJointIndex(_midJointIndex, AnimPose(underPoses[_midJointIndex].scale(), relMidRot, underPoses[_midJointIndex].trans())); ikChain.buildDirtyAbsolutePoses(); // recompute tip pose after mid joint has been rotated @@ -180,7 +180,7 @@ const AnimPoseVec& AnimTwoBoneIK::evaluate(const AnimVariantMap& animVars, const // transform result back into parent relative frame. glm::quat relBaseRot = glm::inverse(baseParentPose.rot()) * absRot; - ikChain.setRelativePoseAtJointIndex(_baseJointIndex, AnimPose(relBaseRot, underPoses[_baseJointIndex].trans())); + ikChain.setRelativePoseAtJointIndex(_baseJointIndex, AnimPose(underPoses[_baseJointIndex].scale(), relBaseRot, underPoses[_baseJointIndex].trans())); } // recompute midJoint pose after base has been rotated. @@ -189,7 +189,7 @@ const AnimPoseVec& AnimTwoBoneIK::evaluate(const AnimVariantMap& animVars, const // transform target rotation in to parent relative frame. glm::quat relTipRot = glm::inverse(midJointPose.rot()) * targetPose.rot(); - ikChain.setRelativePoseAtJointIndex(_tipJointIndex, AnimPose(relTipRot, underPoses[_tipJointIndex].trans())); + ikChain.setRelativePoseAtJointIndex(_tipJointIndex, AnimPose(underPoses[_tipJointIndex].scale(), relTipRot, underPoses[_tipJointIndex].trans())); // blend with the underChain ikChain.blend(underChain, alpha); diff --git a/tests/animation/CMakeLists.txt b/tests/animation/CMakeLists.txt index 2af4d5f2cd..e378750425 100644 --- a/tests/animation/CMakeLists.txt +++ b/tests/animation/CMakeLists.txt @@ -1,7 +1,7 @@ # Declare dependencies macro (setup_testcase_dependencies) # link in the shared libraries - link_hifi_libraries(shared animation gpu fbx hfm graphics networking test-utils) + link_hifi_libraries(shared animation gpu fbx hfm graphics networking test-utils image) package_libraries_for_deployment() endmacro () From 5fcfa265d2621a22d7696d8c21305a768fad4a4d Mon Sep 17 00:00:00 2001 From: Anthony Thibault Date: Wed, 6 Mar 2019 14:02:58 -0800 Subject: [PATCH 2/4] Remove alternate versions of AnimPose(mat4) constructor --- libraries/animation/src/AnimPose.cpp | 36 ---------------------------- 1 file changed, 36 deletions(-) diff --git a/libraries/animation/src/AnimPose.cpp b/libraries/animation/src/AnimPose.cpp index 8b030fac39..56692e763e 100644 --- a/libraries/animation/src/AnimPose.cpp +++ b/libraries/animation/src/AnimPose.cpp @@ -11,9 +11,7 @@ #include "AnimPose.h" #include #include -#include #include "AnimUtil.h" -#include const AnimPose AnimPose::identity = AnimPose(glm::vec3(1.0f), glm::quat(), @@ -22,39 +20,6 @@ const AnimPose AnimPose::identity = AnimPose(glm::vec3(1.0f), #define NEW_VERSION AnimPose::AnimPose(const glm::mat4& mat) { -#if defined(ORIGINAL_VERSION) - static const float EPSILON = 0.0001f; - _scale = extractScale(mat); - // quat_cast doesn't work so well with scaled matrices, so cancel it out. - glm::mat4 tmp = glm::scale(mat, 1.0f / _scale); - _rot = glm::quat_cast(tmp); - float lengthSquared = glm::length2(_rot); - if (glm::abs(lengthSquared - 1.0f) > EPSILON) { - float oneOverLength = 1.0f / sqrtf(lengthSquared); - _rot = glm::quat(_rot.w * oneOverLength, _rot.x * oneOverLength, _rot.y * oneOverLength, _rot.z * oneOverLength); - } - _trans = extractTranslation(mat); -#elif defined(DECOMPOSE_VERSION) - // glm::decompose code - glm::vec3 scale; - glm::quat rotation; - glm::vec3 translation; - glm::vec3 skew; - glm::vec4 perspective; - bool result = glm::decompose(mat, scale, rotation, translation, skew, perspective); - _scale = scale; - _rot = rotation; - _trans = translation; - if (!result) { - // hack - const float HACK_FACTOR = 1000.0f; - glm::mat4 tmp = glm::scale(mat, HACK_FACTOR); - glm::decompose(tmp, scale, rotation, translation, skew, perspective); - _scale = scale / HACK_FACTOR; - _rot = rotation; - _trans = translation; - } -#elif defined(NEW_VERSION) glm::mat3 m(mat); _scale = glm::vec3(glm::length(m[0]), glm::length(m[1]), glm::length(m[2])); float det = glm::determinant(m); @@ -79,7 +44,6 @@ AnimPose::AnimPose(const glm::mat4& mat) { } _trans = extractTranslation(mat); -#endif } glm::vec3 AnimPose::operator*(const glm::vec3& rhs) const { From c5e9a7d1ab2bdacd79602aa7354723370fa616ca Mon Sep 17 00:00:00 2001 From: Anthony Thibault Date: Wed, 6 Mar 2019 14:26:50 -0800 Subject: [PATCH 3/4] Added unit test for AnimPose() ctor using matrix with negative determinant --- tests/animation/src/AnimTests.cpp | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/tests/animation/src/AnimTests.cpp b/tests/animation/src/AnimTests.cpp index 0cd9571e22..a14ffcf967 100644 --- a/tests/animation/src/AnimTests.cpp +++ b/tests/animation/src/AnimTests.cpp @@ -443,6 +443,28 @@ void AnimTests::testAnimPose() { } } } + + + // test matrix that has a negative determiant. + glm::vec4 col0(-9.91782e-05f, -5.40349e-05f, 0.000724383f, 0.0f); + glm::vec4 col1(-0.000155237f, 0.00071579f, 3.21398e-05f, 0.0f); + glm::vec4 col2(0.000709614f, 0.000149036f, 0.000108273f, 0.0f); + glm::vec4 col3(0.117922f, 0.250457f, 0.102155f, 1.0f); + glm::mat4 m(col0, col1, col2, col3); + AnimPose p(m); + + glm::vec3 resultTrans = glm::vec3(col3); + glm::quat resultRot = glm::quat(0.0530394f, 0.751549f, 0.0949531f, -0.650649f); + glm::vec3 resultScale = glm::vec3(-0.000733135f, -0.000733135f, -0.000733135f); + + const float TEST_EPSILON2 = 0.00001f; + QCOMPARE_WITH_ABS_ERROR(p.trans(), resultTrans, TEST_EPSILON2); + + if (glm::dot(p.rot(), resultRot) < 0.0f) { + resultRot = -resultRot; + } + QCOMPARE_WITH_ABS_ERROR(p.rot(), resultRot, TEST_EPSILON2); + QCOMPARE_WITH_ABS_ERROR(p.scale(), resultScale, TEST_EPSILON2); } void AnimTests::testExpressionTokenizer() { From 0cf8f3e5c3d306545355ad3013e923cc9dca868f Mon Sep 17 00:00:00 2001 From: Anthony Thibault Date: Thu, 7 Mar 2019 14:11:01 -0800 Subject: [PATCH 4/4] Code review feed back remove NEW_VERSION ifdef --- libraries/animation/src/AnimPose.cpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/libraries/animation/src/AnimPose.cpp b/libraries/animation/src/AnimPose.cpp index 56692e763e..8649db8233 100644 --- a/libraries/animation/src/AnimPose.cpp +++ b/libraries/animation/src/AnimPose.cpp @@ -17,8 +17,6 @@ const AnimPose AnimPose::identity = AnimPose(glm::vec3(1.0f), glm::quat(), glm::vec3(0.0f)); -#define NEW_VERSION - AnimPose::AnimPose(const glm::mat4& mat) { glm::mat3 m(mat); _scale = glm::vec3(glm::length(m[0]), glm::length(m[1]), glm::length(m[2]));