Address performance issues introduced with this PR.

* Prevent clusterMatrices from being invalidated and re-computed in each updateItem lambda.
  We do this by not setting _model->_needsUpdateClusterMatrices = true;
* Prevent redundant work if Model::enqueueLocationChange is called multiple times per frame.
  We do this by introducing a preRenderLambdas map in the Application class.
  Instead of adding work directly to the scene PendingChanges queue Model::enqueueLocationChange
  adds a lambda to the Application preRenderLambdas map.  The Application ensures that only one lambda will
  be invoked for each model per frame.

interface/src/Application.cpp

@@ -2985,6 +2985,11 @@ void Application::updateLOD() {
     }
 }
 
+void Application::pushPreRenderLambda(void* key, std::function<void()> func) {
+    std::unique_lock<std::mutex> guard(_preRenderLambdasLock);
+    _preRenderLambdas[key] = func;
+}
+
 // Called during Application::update immediately before AvatarManager::updateMyAvatar, updating my data that is then sent to everyone.
 // (Maybe this code should be moved there?)
 // The principal result is to call updateLookAtTargetAvatar() and then setLookAtPosition().
@@ -3461,6 +3466,16 @@ void Application::update(float deltaTime) {
             QMetaObject::invokeMethod(DependencyManager::get<AudioClient>().data(), "sendDownstreamAudioStatsPacket", Qt::QueuedConnection);
         }
     }
+
+    {
+        PROFILE_RANGE_EX("PreRenderLambdas", 0xffff0000, (uint64_t)0);
+
+        std::unique_lock<std::mutex> guard(_preRenderLambdasLock);
+        for (auto& iter : _preRenderLambdas) {
+            iter.second();
+        }
+        _preRenderLambdas.clear();
+    }
 }
 
 

interface/src/Application.h

@@ -211,6 +211,8 @@ public:
     render::EnginePointer getRenderEngine() override { return _renderEngine; }
     gpu::ContextPointer getGPUContext() const { return _gpuContext; }
 
+    virtual void pushPreRenderLambda(void* key, std::function<void()> func) override;
+
     const QRect& getMirrorViewRect() const { return _mirrorViewRect; }
 
     void updateMyAvatarLookAtPosition();
@@ -510,6 +512,9 @@ private:
     bool _cursorNeedsChanging { false };
 
     QThread* _deadlockWatchdogThread;
+
+    std::map<void*, std::function<void()>> _preRenderLambdas;
+    std::mutex _preRenderLambdasLock;
 };
 
 #endif // hifi_Application_h

libraries/render-utils/src/AbstractViewStateInterface.h

@@ -46,6 +46,8 @@ public:
     virtual render::ScenePointer getMain3DScene() = 0;
     virtual render::EnginePointer getRenderEngine() = 0;
 
+    virtual void pushPreRenderLambda(void* key, std::function<void()> func) = 0;
+
     // FIXME - we shouldn't assume that there's a single instance of an AbstractViewStateInterface
     static AbstractViewStateInterface* instance();
     static void setInstance(AbstractViewStateInterface* instance);

libraries/render-utils/src/MeshPartPayload.cpp

@@ -347,7 +347,7 @@ void ModelMeshPartPayload::initCache() {
 
 
 void ModelMeshPartPayload::notifyLocationChanged() {
-    _model->_needsUpdateClusterMatrices = true;
+
 }
 
 void ModelMeshPartPayload::updateTransformForSkinnedMesh(const Transform& transform, const Transform& offsetTransform, const glm::mat4* clusterMatrices, size_t numClusterMatrices) {

libraries/render-utils/src/Model.cpp

@@ -128,45 +128,61 @@ void Model::setOffset(const glm::vec3& offset) {
 }
 
 void Model::enqueueLocationChange() {
-    render::ScenePointer scene = AbstractViewStateInterface::instance()->getMain3DScene();
 
-    Transform modelTransform;
-    modelTransform.setScale(_scale);
-    modelTransform.setTranslation(_translation);
-    modelTransform.setRotation(_rotation);
+    // queue up this work for later processing, at the end of update and just before rendering.
+    // the application will ensure only the last lambda is actually invoked.
+    void* key = (void*)this;
+    std::weak_ptr<Model> weakSelf = shared_from_this();
+    AbstractViewStateInterface::instance()->pushPreRenderLambda(key, [weakSelf]() {
 
-    Transform modelMeshOffset;
-    if (_geometry && _geometry->isLoaded()) {
-        modelMeshOffset = Transform(_rig->getGeometryToRigTransform());
-    } else {
-        modelMeshOffset.postTranslate(_offset);
-    }
+        // do nothing, if the model has already been destroyed.
+        auto self = weakSelf.lock();
+        if (!self) {
+            return;
+        }
 
-    Transform collisionMeshOffset;
-    collisionMeshOffset.postTranslate(_offset);
+        render::ScenePointer scene = AbstractViewStateInterface::instance()->getMain3DScene();
 
+        Transform modelTransform;
+        modelTransform.setScale(self->_scale);
+        modelTransform.setTranslation(self->_translation);
+        modelTransform.setRotation(self->_rotation);
 
-    render::PendingChanges pendingChanges;
-    foreach (auto itemID, _modelMeshRenderItems.keys()) {
-        pendingChanges.updateItem<ModelMeshPartPayload>(itemID, [modelTransform, modelMeshOffset](ModelMeshPartPayload& data) {
+        Transform modelMeshOffset;
+        if (self->_geometry && self->_geometry->isLoaded()) {
+            // includes model offset and unitScale.
+            modelMeshOffset = Transform(self->_rig->getGeometryToRigTransform());
+        } else {
+            modelMeshOffset.postTranslate(self->_offset);
+        }
 
-            data._model->updateClusterMatrices(modelTransform.getTranslation(), modelTransform.getRotation());
-            const Model::MeshState& state = data._model->_meshStates.at(data._meshIndex);
-            size_t numClusterMatrices = data._model->getGeometry()->getFBXGeometry().meshes.at(data._meshIndex).clusters.size();
+        // only apply offset only, collision mesh does not share the same unit scale as the FBX file's mesh.
+        Transform collisionMeshOffset;
+        collisionMeshOffset.postTranslate(self->_offset);
 
-            data.updateTransformForSkinnedMesh(modelTransform, modelMeshOffset, &state.clusterMatrices[0], numClusterMatrices);
-            data.notifyLocationChanged();
-        });
-    }
+        render::PendingChanges pendingChanges;
+        foreach (auto itemID, self->_modelMeshRenderItems.keys()) {
+            pendingChanges.updateItem<ModelMeshPartPayload>(itemID, [modelTransform, modelMeshOffset](ModelMeshPartPayload& data) {
 
-    foreach (auto itemID, _collisionRenderItems.keys()) {
-        pendingChanges.updateItem<MeshPartPayload>(itemID, [modelTransform, collisionMeshOffset](MeshPartPayload& data) {
-            data.updateTransform(modelTransform, collisionMeshOffset);
-            data.notifyLocationChanged();
-       });
-    }
+                // lazy update of cluster matrices used for rendering.  We need to update them here, so we can correctly update the bounding box.
+                data._model->updateClusterMatrices(modelTransform.getTranslation(), modelTransform.getRotation());
 
-    scene->enqueuePendingChanges(pendingChanges);
+                // update the model transform and bounding box for this render item.
+                const Model::MeshState& state = data._model->_meshStates.at(data._meshIndex);
+                size_t numClusterMatrices = data._model->getGeometry()->getFBXGeometry().meshes.at(data._meshIndex).clusters.size();
+                data.updateTransformForSkinnedMesh(modelTransform, modelMeshOffset, &state.clusterMatrices[0], numClusterMatrices);
+            });
+        }
+
+        foreach (auto itemID, self->_collisionRenderItems.keys()) {
+            pendingChanges.updateItem<MeshPartPayload>(itemID, [modelTransform, collisionMeshOffset](MeshPartPayload& data) {
+                // update the model transform for this render item.
+                data.updateTransform(modelTransform, collisionMeshOffset);
+            });
+        }
+
+        scene->enqueuePendingChanges(pendingChanges);
+    });
 }
 
 void Model::initJointTransforms() {