From 9b41fa20c65cbcab83a82eac5dc3eb9bd179401a Mon Sep 17 00:00:00 2001
From: sabrina-shanman <sabrina@highfidelity.io>
Date: Wed, 30 Oct 2019 14:08:02 -0700
Subject: [PATCH] Clean up hfm joint geometric offset definition

---
 libraries/fbx/src/FBXSerializer.cpp | 20 ++++++++------------
 libraries/hfm/src/hfm/HFM.cpp       |  6 ++----
 libraries/hfm/src/hfm/HFM.h         |  5 +----
 3 files changed, 11 insertions(+), 20 deletions(-)

diff --git a/libraries/fbx/src/FBXSerializer.cpp b/libraries/fbx/src/FBXSerializer.cpp
index 33b9457928..33ffdb5714 100644
--- a/libraries/fbx/src/FBXSerializer.cpp
+++ b/libraries/fbx/src/FBXSerializer.cpp
@@ -1321,10 +1321,9 @@ HFMModel* FBXSerializer::extractHFMModel(const hifi::VariantHash& mapping, const
         joint.rotationMin = fbxModel.rotationMin;
         joint.rotationMax = fbxModel.rotationMax;
 
-        joint.hasGeometricOffset = fbxModel.hasGeometricOffset;
-        joint.geometricTranslation = fbxModel.geometricTranslation;
-        joint.geometricRotation = fbxModel.geometricRotation;
-        joint.geometricScaling = fbxModel.geometricScaling;
+        if (fbxModel.hasGeometricOffset) {
+            joint.geometricOffset = createMatFromScaleQuatAndPos(fbxModel.geometricScaling, fbxModel.geometricRotation, fbxModel.geometricTranslation);
+        }
         joint.isSkeletonJoint = fbxModel.isLimbNode;
         hfmModel.hasSkeletonJoints = (hfmModel.hasSkeletonJoints || joint.isSkeletonJoint);
 
@@ -1341,9 +1340,8 @@ HFMModel* FBXSerializer::extractHFMModel(const hifi::VariantHash& mapping, const
             joint.rotation *= upAxisZRotation;
             joint.translation = upAxisZRotation * joint.translation;
         }
-        if (joint.hasGeometricOffset) {
-            glm::mat4 geometricOffset = createMatFromScaleQuatAndPos(joint.geometricScaling, joint.geometricRotation, joint.geometricTranslation);
-            joint.postTransform *= geometricOffset;
+        if (fbxModel.hasGeometricOffset) {
+            joint.postTransform *= joint.geometricOffset;
         }
 
         glm::quat combinedRotation = joint.preRotation * joint.rotation * joint.postRotation;
@@ -1361,13 +1359,12 @@ HFMModel* FBXSerializer::extractHFMModel(const hifi::VariantHash& mapping, const
             joint.inverseDefaultRotation = glm::inverse(combinedRotation) * parentJoint.inverseDefaultRotation;
             joint.distanceToParent = glm::distance(extractTranslation(parentJoint.transform), extractTranslation(joint.transform));
 
-            if (parentJoint.hasGeometricOffset) {
+            if (fbxModel.hasGeometricOffset) {
                 // Per the FBX standard, geometric offset should not propagate to children.
                 // However, we must be careful when modifying the behavior of FBXSerializer.
                 // So, we leave this here, as a breakpoint for debugging, or stub for implementation.
                 // qCDebug(modelformat) << "Geometric offset encountered on non-leaf node. jointIndex: " << jointIndex << ", modelURL: " << url;
-                // glm::mat4 parentGeometricOffset = createMatFromScaleQuatAndPos(parentJoint.geometricScaling, parentJoint.geometricRotation, parentJoint.geometricTranslation);
-                // joint.preTransform = glm::inverse(parentGeometricOffset) * joint.preTransform;
+                // joint.preTransform = glm::inverse(parentJoint.geometricOffset) * joint.preTransform;
             }
         }
         joint.inverseBindRotation = joint.inverseDefaultRotation;
@@ -1385,8 +1382,7 @@ HFMModel* FBXSerializer::extractHFMModel(const hifi::VariantHash& mapping, const
                 transformForCluster = localTransformForCluster;
             }
             if (fbxModel.hasGeometricOffset) {
-                glm::mat4 geometricOffset = createMatFromScaleQuatAndPos(fbxModel.geometricScaling, fbxModel.geometricRotation, fbxModel.geometricTranslation);
-                transformForCluster = transformForCluster * geometricOffset;
+                transformForCluster = transformForCluster * joint.geometricOffset;
             }
         } else {
             transformForCluster = joint.transform;
diff --git a/libraries/hfm/src/hfm/HFM.cpp b/libraries/hfm/src/hfm/HFM.cpp
index 5d57ef2c98..500aaaa842 100644
--- a/libraries/hfm/src/hfm/HFM.cpp
+++ b/libraries/hfm/src/hfm/HFM.cpp
@@ -348,13 +348,11 @@ void hfm::Model::debugDump() const {
         qCDebug(modelformat) << "    rotationMax" << joint.rotationMax;
         qCDebug(modelformat) << "    inverseDefaultRotation" << joint.inverseDefaultRotation;
         qCDebug(modelformat) << "    inverseBindRotation" << joint.inverseBindRotation;
+        qCDebug(modelformat) << "    bindTransformFoundInCluster" << joint.bindTransformFoundInCluster;
         qCDebug(modelformat) << "    bindTransform" << joint.bindTransform;
         qCDebug(modelformat) << "    name" << joint.name;
         qCDebug(modelformat) << "    isSkeletonJoint" << joint.isSkeletonJoint;
-        qCDebug(modelformat) << "    bindTransformFoundInCluster" << joint.hasGeometricOffset;
-        qCDebug(modelformat) << "    bindTransformFoundInCluster" << joint.geometricTranslation;
-        qCDebug(modelformat) << "    bindTransformFoundInCluster" << joint.geometricRotation;
-        qCDebug(modelformat) << "    bindTransformFoundInCluster" << joint.geometricScaling;
+        qCDebug(modelformat) << "    geometricOffset" << joint.geometricOffset;
         qCDebug(modelformat) << "\n";
     }
 
diff --git a/libraries/hfm/src/hfm/HFM.h b/libraries/hfm/src/hfm/HFM.h
index 87263cb72f..11b32dc4b3 100644
--- a/libraries/hfm/src/hfm/HFM.h
+++ b/libraries/hfm/src/hfm/HFM.h
@@ -114,10 +114,7 @@ public:
     bool bindTransformFoundInCluster;
 
     // geometric offset is applied in local space but does NOT affect children.
-    bool hasGeometricOffset;
-    glm::vec3 geometricTranslation;
-    glm::quat geometricRotation;
-    glm::vec3 geometricScaling;
+    glm::mat4 geometricOffset;
 
     // globalTransform is the transform of the joint with all parent transforms applied, plus the geometric offset
     glm::mat4 localTransform;