From fb1c075a1272c5a57ab9fd80fcf051d1a2702160 Mon Sep 17 00:00:00 2001
From: Andrew Meadows <andrew@highfidelity.io>
Date: Mon, 16 Oct 2017 15:55:25 -0700
Subject: [PATCH] fix animating entities

---
 .../src/RenderableModelEntityItem.cpp         | 32 +++++++++----------
 libraries/render-utils/src/Model.cpp          | 10 ++++--
 libraries/render-utils/src/Model.h            |  1 +
 3 files changed, 25 insertions(+), 18 deletions(-)

diff --git a/libraries/entities-renderer/src/RenderableModelEntityItem.cpp b/libraries/entities-renderer/src/RenderableModelEntityItem.cpp
index e2afc6eac2..1e75b3bdf1 100644
--- a/libraries/entities-renderer/src/RenderableModelEntityItem.cpp
+++ b/libraries/entities-renderer/src/RenderableModelEntityItem.cpp
@@ -153,7 +153,7 @@ bool RenderableModelEntityItem::needsUpdateModelBounds() const {
         return true;
     }
 
-    if (isMovingRelativeToParent() || isAnimatingSomething()) {
+    if (isAnimatingSomething()) {
         return true;
     }
 
@@ -195,14 +195,10 @@ void RenderableModelEntityItem::updateModelBounds() {
         return;
     }
 
-    /* adebug TODO: figure out if we need to DO anything when isAnimatingSomething()
-    if (isAnimatingSomething()) {
-        return true;
-    }
-    */
-
+    bool updateRenderItems = false;
     if (model->needsReload()) {
         model->updateGeometry();
+        updateRenderItems = true;
     }
 
     if (model->getScaleToFitDimensions() != getDimensions() ||
@@ -217,23 +213,28 @@ void RenderableModelEntityItem::updateModelBounds() {
         // now recalculate the bounds and registration
         model->setScaleToFit(true, getDimensions());
         model->setSnapModelToRegistrationPoint(true, getRegistrationPoint());
+        updateRenderItems = true;
     }
 
     bool success;
     auto transform = getTransform(success);
-    if (success) {
-        if (model->getTranslation() != transform.getTranslation()) {
-            model->setTranslation(transform.getTranslation());
-        }
-        if (model->getRotation() != transform.getRotation()) {
-            model->setRotation(transform.getRotation());
-        }
+    if (success && (model->getTranslation() != transform.getTranslation() ||
+            model->getRotation() != transform.getRotation())) {
+        model->setTransformNoUpdateRenderItems(transform);
+        updateRenderItems = true;
     }
 
-    if (_needsInitialSimulation || _needsJointSimulation) {
+    if (_needsInitialSimulation || _needsJointSimulation || isAnimatingSomething()) {
+        // NOTE: on isAnimatingSomething() we need to call Model::simulate() which calls Rig::updateRig()
+        // TODO: there is opportunity to further optimize the isAnimatingSomething() case.
         model->simulate(0.0f);
         _needsInitialSimulation = false;
         _needsJointSimulation = false;
+        updateRenderItems = true;
+    }
+
+    if (updateRenderItems) {
+        model->updateRenderItems();
     }
 }
 
@@ -929,7 +930,6 @@ void RenderableModelEntityItem::copyAnimationJointDataToModel() {
         return;
     }
 
-
     // relay any inbound joint changes from scripts/animation/network to the model/rig
     _jointDataLock.withWriteLock([&] {
         for (int index = 0; index < _localJointData.size(); ++index) {
diff --git a/libraries/render-utils/src/Model.cpp b/libraries/render-utils/src/Model.cpp
index 4573b5aa78..fcda330f4a 100644
--- a/libraries/render-utils/src/Model.cpp
+++ b/libraries/render-utils/src/Model.cpp
@@ -121,8 +121,6 @@ bool Model::needsFixupInScene() const {
     return (_needsFixupInScene || !_addedToScene) && !_needsReload && isLoaded();
 }
 
-// TODO?: should we combine translation and rotation into single method to avoid double-work?
-// (figure out where we call these)
 void Model::setTranslation(const glm::vec3& translation) {
     _translation = translation;
     updateRenderItems();
@@ -133,6 +131,14 @@ void Model::setRotation(const glm::quat& rotation) {
     updateRenderItems();
 }
 
+// temporary HACK: set transform while avoiding implicit calls to updateRenderItems()
+// TODO: make setRotation() and friends set flag to be used later to decide to updateRenderItems()
+void Model::setTransformNoUpdateRenderItems(const Transform& transform) {
+    _translation = transform.getTranslation();
+    _rotation = transform.getRotation();
+    // DO NOT call updateRenderItems() here!
+}
+
 Transform Model::getTransform() const {
     if (_spatiallyNestableOverride) {
         bool success;
diff --git a/libraries/render-utils/src/Model.h b/libraries/render-utils/src/Model.h
index 0207a4871a..8bce976b4e 100644
--- a/libraries/render-utils/src/Model.h
+++ b/libraries/render-utils/src/Model.h
@@ -206,6 +206,7 @@ public:
 
     void setTranslation(const glm::vec3& translation);
     void setRotation(const glm::quat& rotation);
+    void setTransformNoUpdateRenderItems(const Transform& transform); // temporary HACK
 
     const glm::vec3& getTranslation() const { return _translation; }
     const glm::quat& getRotation() const { return _rotation; }