From 115fd607a0912d8c48e2a82e60271607378c3be3 Mon Sep 17 00:00:00 2001 From: "Anthony J. Thibault" Date: Mon, 28 Mar 2016 19:47:30 -0700 Subject: [PATCH] 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 | 15 ++++ interface/src/Application.h | 5 ++ .../src/AbstractViewStateInterface.h | 2 + .../render-utils/src/MeshPartPayload.cpp | 2 +- libraries/render-utils/src/Model.cpp | 76 +++++++++++-------- 5 files changed, 69 insertions(+), 31 deletions(-) diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp index 381816e81f..84794f7c31 100644 --- a/interface/src/Application.cpp +++ b/interface/src/Application.cpp @@ -2985,6 +2985,11 @@ void Application::updateLOD() { } } +void Application::pushPreRenderLambda(void* key, std::function func) { + std::unique_lock 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().data(), "sendDownstreamAudioStatsPacket", Qt::QueuedConnection); } } + + { + PROFILE_RANGE_EX("PreRenderLambdas", 0xffff0000, (uint64_t)0); + + std::unique_lock guard(_preRenderLambdasLock); + for (auto& iter : _preRenderLambdas) { + iter.second(); + } + _preRenderLambdas.clear(); + } } diff --git a/interface/src/Application.h b/interface/src/Application.h index d21e647bc7..9124755011 100644 --- a/interface/src/Application.h +++ b/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 func) override; + const QRect& getMirrorViewRect() const { return _mirrorViewRect; } void updateMyAvatarLookAtPosition(); @@ -510,6 +512,9 @@ private: bool _cursorNeedsChanging { false }; QThread* _deadlockWatchdogThread; + + std::map> _preRenderLambdas; + std::mutex _preRenderLambdasLock; }; #endif // hifi_Application_h diff --git a/libraries/render-utils/src/AbstractViewStateInterface.h b/libraries/render-utils/src/AbstractViewStateInterface.h index 815cb45423..7c7c263562 100644 --- a/libraries/render-utils/src/AbstractViewStateInterface.h +++ b/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 func) = 0; + // FIXME - we shouldn't assume that there's a single instance of an AbstractViewStateInterface static AbstractViewStateInterface* instance(); static void setInstance(AbstractViewStateInterface* instance); diff --git a/libraries/render-utils/src/MeshPartPayload.cpp b/libraries/render-utils/src/MeshPartPayload.cpp index 8d738e44d8..41352f1532 100644 --- a/libraries/render-utils/src/MeshPartPayload.cpp +++ b/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) { diff --git a/libraries/render-utils/src/Model.cpp b/libraries/render-utils/src/Model.cpp index 55aa000ef4..87534ffac1 100644 --- a/libraries/render-utils/src/Model.cpp +++ b/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 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(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(itemID, [modelTransform, modelMeshOffset](ModelMeshPartPayload& data) { - foreach (auto itemID, _collisionRenderItems.keys()) { - pendingChanges.updateItem(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(itemID, [modelTransform, collisionMeshOffset](MeshPartPayload& data) { + // update the model transform for this render item. + data.updateTransform(modelTransform, collisionMeshOffset); + }); + } + + scene->enqueuePendingChanges(pendingChanges); + }); } void Model::initJointTransforms() {