From 1a36693161424a82d2e103c96258701a22bde004 Mon Sep 17 00:00:00 2001 From: SamGondelman Date: Tue, 11 Sep 2018 16:58:38 -0700 Subject: [PATCH 1/6] I hate blendshapes --- .../render-utils/src/CauterizedModel.cpp | 3 +- libraries/render-utils/src/Model.cpp | 43 ++++++++++--------- libraries/render-utils/src/Model.h | 2 - 3 files changed, 25 insertions(+), 23 deletions(-) diff --git a/libraries/render-utils/src/CauterizedModel.cpp b/libraries/render-utils/src/CauterizedModel.cpp index c4631c3676..ffb652f923 100644 --- a/libraries/render-utils/src/CauterizedModel.cpp +++ b/libraries/render-utils/src/CauterizedModel.cpp @@ -86,7 +86,7 @@ void CauterizedModel::createRenderItemSet() { // Create the render payloads int numParts = (int)mesh->getNumParts(); for (int partIndex = 0; partIndex < numParts; partIndex++) { - if (!fbxGeometry.meshes[i].blendshapes.empty()) { + if (!fbxGeometry.meshes[i].blendshapes.empty() && _blendedVertexBuffers.find(i) == _blendedVertexBuffers.end()) { initializeBlendshapes(fbxGeometry.meshes[i], i); } auto ptr = std::make_shared(shared_from_this(), i, partIndex, shapeID, transform, offset); @@ -97,6 +97,7 @@ void CauterizedModel::createRenderItemSet() { shapeID++; } } + _blendedVertexBuffersInitialized = true; } else { Model::createRenderItemSet(); } diff --git a/libraries/render-utils/src/Model.cpp b/libraries/render-utils/src/Model.cpp index ba2bd28852..2feae1fabb 100644 --- a/libraries/render-utils/src/Model.cpp +++ b/libraries/render-utils/src/Model.cpp @@ -302,12 +302,18 @@ bool Model::updateGeometry() { assert(_meshStates.empty()); const FBXGeometry& fbxGeometry = getFBXGeometry(); + int i = 0; foreach (const FBXMesh& mesh, fbxGeometry.meshes) { MeshState state; state.clusterDualQuaternions.resize(mesh.clusters.size()); state.clusterMatrices.resize(mesh.clusters.size()); _meshStates.push_back(state); + if (!mesh.blendshapes.empty() && _blendedVertexBuffers.find(i) == _blendedVertexBuffers.end()) { + initializeBlendshapes(mesh, i); + } + i++; } + _blendedVertexBuffersInitialized = true; needFullUpdate = true; emit rigReady(); } @@ -1273,8 +1279,7 @@ QStringList Model::getJointNames() const { class Blender : public QRunnable { public: - Blender(ModelPointer model, int blendNumber, const Geometry::WeakPointer& geometry, - const QVector& meshes, const QVector& blendshapeCoefficients); + Blender(ModelPointer model, int blendNumber, const Geometry::WeakPointer& geometry, const QVector& blendshapeCoefficients); virtual void run() override; @@ -1283,16 +1288,13 @@ private: ModelPointer _model; int _blendNumber; Geometry::WeakPointer _geometry; - QVector _meshes; QVector _blendshapeCoefficients; }; -Blender::Blender(ModelPointer model, int blendNumber, const Geometry::WeakPointer& geometry, - const QVector& meshes, const QVector& blendshapeCoefficients) : +Blender::Blender(ModelPointer model, int blendNumber, const Geometry::WeakPointer& geometry, const QVector& blendshapeCoefficients) : _model(model), _blendNumber(blendNumber), _geometry(geometry), - _meshes(meshes), _blendshapeCoefficients(blendshapeCoefficients) { } @@ -1300,10 +1302,12 @@ void Blender::run() { DETAILED_PROFILE_RANGE_EX(simulation_animation, __FUNCTION__, 0xFFFF0000, 0, { { "url", _model->getURL().toString() } }); QVector vertices; QVector normalsAndTangents; - if (_model) { + auto geometry = _geometry.lock(); + if (_model && geometry) { int offset = 0; int normalsAndTangentsOffset = 0; - foreach (const FBXMesh& mesh, _meshes) { + auto meshes = geometry->getFBXGeometry().meshes; + foreach (const FBXMesh& mesh, meshes) { if (mesh.blendshapes.isEmpty()) { continue; } @@ -1526,8 +1530,7 @@ bool Model::maybeStartBlender() { if (isLoaded()) { const FBXGeometry& fbxGeometry = getFBXGeometry(); if (fbxGeometry.hasBlendedMeshes()) { - QThreadPool::globalInstance()->start(new Blender(getThisPointer(), ++_blendNumber, _renderGeometry, - fbxGeometry.meshes, _blendshapeCoefficients)); + QThreadPool::globalInstance()->start(new Blender(getThisPointer(), ++_blendNumber, _renderGeometry, _blendshapeCoefficients)); return true; } } @@ -1537,11 +1540,11 @@ bool Model::maybeStartBlender() { void Model::setBlendedVertices(int blendNumber, const Geometry::WeakPointer& geometry, const QVector& vertices, const QVector& normalsAndTangents) { auto geometryRef = geometry.lock(); - if (!geometryRef || _renderGeometry != geometryRef || blendNumber < _appliedBlendNumber) { + if (!geometryRef || _renderGeometry != geometryRef || blendNumber < _appliedBlendNumber || !_blendedVertexBuffersInitialized) { return; } _appliedBlendNumber = blendNumber; - const FBXGeometry& fbxGeometry = getFBXGeometry(); + const FBXGeometry& fbxGeometry = geometryRef->getFBXGeometry(); int index = 0; int normalAndTangentIndex = 0; for (int i = 0; i < fbxGeometry.meshes.size(); i++) { @@ -1552,11 +1555,11 @@ void Model::setBlendedVertices(int blendNumber, const Geometry::WeakPointer& geo const auto vertexCount = mesh.vertices.size(); const auto verticesSize = vertexCount * sizeof(glm::vec3); - const auto& buffer = _blendedVertexBuffers[i]; - assert(buffer && _blendedVertexBuffersInitialized); - buffer->resize(mesh.vertices.size() * sizeof(glm::vec3) + mesh.normalsAndTangents.size() * sizeof(NormalType)); - buffer->setSubData(0, verticesSize, (gpu::Byte*) vertices.constData() + index * sizeof(glm::vec3)); - buffer->setSubData(verticesSize, mesh.normalsAndTangents.size() * sizeof(NormalType), (const gpu::Byte*) normalsAndTangents.data() + normalAndTangentIndex * sizeof(NormalType)); + const auto& buffer = _blendedVertexBuffers.find(i); + assert(buffer != _blendedVertexBuffers.end()); + buffer->second->resize(mesh.vertices.size() * sizeof(glm::vec3) + mesh.normalsAndTangents.size() * sizeof(NormalType)); + buffer->second->setSubData(0, verticesSize, (gpu::Byte*) vertices.constData() + index * sizeof(glm::vec3)); + buffer->second->setSubData(verticesSize, mesh.normalsAndTangents.size() * sizeof(NormalType), (const gpu::Byte*) normalsAndTangents.data() + normalAndTangentIndex * sizeof(NormalType)); index += vertexCount; normalAndTangentIndex += mesh.normalsAndTangents.size(); @@ -1599,6 +1602,7 @@ const render::ItemIDs& Model::fetchRenderItemIDs() const { } void Model::initializeBlendshapes(const FBXMesh& mesh, int index) { + _blendedVertexBuffers[index] = std::make_shared(); QVector normalsAndTangents; normalsAndTangents.resize(2 * mesh.normals.size()); @@ -1627,12 +1631,10 @@ void Model::initializeBlendshapes(const FBXMesh& mesh, int index) { } }); const auto verticesSize = mesh.vertices.size() * sizeof(glm::vec3); - _blendedVertexBuffers[index] = std::make_shared(); _blendedVertexBuffers[index]->resize(mesh.vertices.size() * sizeof(glm::vec3) + normalsAndTangents.size() * sizeof(NormalType)); _blendedVertexBuffers[index]->setSubData(0, verticesSize, (const gpu::Byte*) mesh.vertices.constData()); _blendedVertexBuffers[index]->setSubData(verticesSize, normalsAndTangents.size() * sizeof(NormalType), (const gpu::Byte*) normalsAndTangents.data()); mesh.normalsAndTangents = normalsAndTangents; - _blendedVertexBuffersInitialized = true; } void Model::createRenderItemSet() { @@ -1673,7 +1675,7 @@ void Model::createRenderItemSet() { // Create the render payloads int numParts = (int)mesh->getNumParts(); for (int partIndex = 0; partIndex < numParts; partIndex++) { - if (!fbxGeometry.meshes[i].blendshapes.empty()) { + if (!fbxGeometry.meshes[i].blendshapes.empty() && _blendedVertexBuffers.find(i) == _blendedVertexBuffers.end()) { initializeBlendshapes(fbxGeometry.meshes[i], i); } _modelMeshRenderItems << std::make_shared(shared_from_this(), i, partIndex, shapeID, transform, offset); @@ -1683,6 +1685,7 @@ void Model::createRenderItemSet() { shapeID++; } } + _blendedVertexBuffersInitialized = true; } bool Model::isRenderable() const { diff --git a/libraries/render-utils/src/Model.h b/libraries/render-utils/src/Model.h index e7534f5b89..f35048aa2f 100644 --- a/libraries/render-utils/src/Model.h +++ b/libraries/render-utils/src/Model.h @@ -345,8 +345,6 @@ public: void addMaterial(graphics::MaterialLayer material, const std::string& parentMaterialName); void removeMaterial(graphics::MaterialPointer material, const std::string& parentMaterialName); - bool areBlendedVertexBuffersInitialized(int index) { return _blendedVertexBuffersInitialized; } - public slots: void loadURLFinished(bool success); From 4b93f9f5cda5c5233b2a5ba89319ac6975678530 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Wed, 12 Sep 2018 12:20:03 -0700 Subject: [PATCH 2/6] Revert "use different XORed instance ID for replicas" This reverts commit a37a19da1e4e7657548fb29ad6cf4e130a5eda76. --- libraries/avatars/src/AvatarHashMap.cpp | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/libraries/avatars/src/AvatarHashMap.cpp b/libraries/avatars/src/AvatarHashMap.cpp index 284bb20f16..6fc53fcc59 100644 --- a/libraries/avatars/src/AvatarHashMap.cpp +++ b/libraries/avatars/src/AvatarHashMap.cpp @@ -86,8 +86,7 @@ void AvatarReplicas::processDeletedTraitInstance(const QUuid& parentID, AvatarTr if (_replicasMap.find(parentID) != _replicasMap.end()) { auto &replicas = _replicasMap[parentID]; for (auto avatar : replicas) { - avatar->processDeletedTraitInstance(traitType, - AvatarTraits::xoredInstanceID(instanceID, avatar->getTraitInstanceXORID())); + avatar->processDeletedTraitInstance(traitType, instanceID); } } } @@ -96,9 +95,7 @@ void AvatarReplicas::processTraitInstance(const QUuid& parentID, AvatarTraits::T if (_replicasMap.find(parentID) != _replicasMap.end()) { auto &replicas = _replicasMap[parentID]; for (auto avatar : replicas) { - avatar->processTraitInstance(traitType, - AvatarTraits::xoredInstanceID(instanceID, avatar->getTraitInstanceXORID()), - traitBinaryData); + avatar->processTraitInstance(traitType, instanceID, traitBinaryData); } } } @@ -364,11 +361,11 @@ void AvatarHashMap::processBulkAvatarTraits(QSharedPointer mess // in order to handle re-connections to the avatar mixer when the other if (traitBinarySize == AvatarTraits::DELETED_TRAIT_SIZE) { avatar->processDeletedTraitInstance(traitType, xoredInstanceID); - _replicas.processDeletedTraitInstance(avatarID, traitType, traitInstanceID); + _replicas.processDeletedTraitInstance(avatarID, traitType, xoredInstanceID); } else { auto traitData = message->read(traitBinarySize); avatar->processTraitInstance(traitType, xoredInstanceID, traitData); - _replicas.processTraitInstance(avatarID, traitType, traitInstanceID, traitData); + _replicas.processTraitInstance(avatarID, traitType, xoredInstanceID, traitData); } processedInstanceVersion = packetTraitVersion; } else { From 6d3e6a661f1bc5d3650c6d11e502d4072c78826e Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Wed, 12 Sep 2018 12:20:31 -0700 Subject: [PATCH 3/6] Revert "XOR incoming trait instance IDs for other avatars" This reverts commit 9b3d9dd0f319ad2964e54a72bd17934765a91c02. --- libraries/avatars/src/AvatarData.h | 5 ----- libraries/avatars/src/AvatarHashMap.cpp | 20 ++++-------------- libraries/avatars/src/AvatarTraits.h | 21 ++----------------- libraries/avatars/src/ClientTraitsHandler.cpp | 21 ++++++++++++------- libraries/avatars/src/ClientTraitsHandler.h | 4 ++++ 5 files changed, 24 insertions(+), 47 deletions(-) diff --git a/libraries/avatars/src/AvatarData.h b/libraries/avatars/src/AvatarData.h index e9f1f5f6c3..b010a9f924 100644 --- a/libraries/avatars/src/AvatarData.h +++ b/libraries/avatars/src/AvatarData.h @@ -1193,9 +1193,6 @@ public: void setReplicaIndex(int replicaIndex) { _replicaIndex = replicaIndex; } int getReplicaIndex() { return _replicaIndex; } - const AvatarTraits::TraitInstanceID getTraitInstanceXORID() const { return _traitInstanceXORID; } - void cycleTraitInstanceXORID() { _traitInstanceXORID = QUuid::createUuid(); } - signals: /**jsdoc @@ -1502,8 +1499,6 @@ private: // privatize the copy constructor and assignment operator so they cannot be called AvatarData(const AvatarData&); AvatarData& operator= (const AvatarData&); - - AvatarTraits::TraitInstanceID _traitInstanceXORID { QUuid::createUuid() }; }; Q_DECLARE_METATYPE(AvatarData*) diff --git a/libraries/avatars/src/AvatarHashMap.cpp b/libraries/avatars/src/AvatarHashMap.cpp index 6fc53fcc59..d205a915f8 100644 --- a/libraries/avatars/src/AvatarHashMap.cpp +++ b/libraries/avatars/src/AvatarHashMap.cpp @@ -343,29 +343,17 @@ void AvatarHashMap::processBulkAvatarTraits(QSharedPointer mess AvatarTraits::TraitInstanceID traitInstanceID = QUuid::fromRfc4122(message->readWithoutCopy(NUM_BYTES_RFC4122_UUID)); - // XOR the incoming trait instance ID with this avatar object's personal XOR ID - - // this ensures that we have separate entity instances in the local tree - // if we briefly end up with two Avatar objects for this node - - // (which can occur if the shared pointer for the - // previous instance of an avatar hasn't yet gone out of scope before the - // new instance is created) - - auto xoredInstanceID = AvatarTraits::xoredInstanceID(traitInstanceID, avatar->getTraitInstanceXORID()); - message->readPrimitive(&traitBinarySize); auto& processedInstanceVersion = lastProcessedVersions.getInstanceValueRef(traitType, traitInstanceID); if (packetTraitVersion > processedInstanceVersion) { - // in order to handle re-connections to the avatar mixer when the other if (traitBinarySize == AvatarTraits::DELETED_TRAIT_SIZE) { - avatar->processDeletedTraitInstance(traitType, xoredInstanceID); - _replicas.processDeletedTraitInstance(avatarID, traitType, xoredInstanceID); + avatar->processDeletedTraitInstance(traitType, traitInstanceID); + _replicas.processDeletedTraitInstance(avatarID, traitType, traitInstanceID); } else { auto traitData = message->read(traitBinarySize); - avatar->processTraitInstance(traitType, xoredInstanceID, traitData); - _replicas.processTraitInstance(avatarID, traitType, xoredInstanceID, traitData); + avatar->processTraitInstance(traitType, traitInstanceID, traitData); + _replicas.processTraitInstance(avatarID, traitType, traitInstanceID, traitData); } processedInstanceVersion = packetTraitVersion; } else { diff --git a/libraries/avatars/src/AvatarTraits.h b/libraries/avatars/src/AvatarTraits.h index 47be0d6111..5866caf234 100644 --- a/libraries/avatars/src/AvatarTraits.h +++ b/libraries/avatars/src/AvatarTraits.h @@ -41,8 +41,7 @@ namespace AvatarTraits { const TraitWireSize DELETED_TRAIT_SIZE = -1; inline qint64 packInstancedTraitDelete(TraitType traitType, TraitInstanceID instanceID, ExtendedIODevice& destination, - TraitVersion traitVersion = NULL_TRAIT_VERSION, - TraitInstanceID xoredInstanceID = TraitInstanceID()) { + TraitVersion traitVersion = NULL_TRAIT_VERSION) { qint64 bytesWritten = 0; bytesWritten += destination.writePrimitive(traitType); @@ -51,28 +50,12 @@ namespace AvatarTraits { bytesWritten += destination.writePrimitive(traitVersion); } - if (xoredInstanceID.isNull()) { - bytesWritten += destination.write(instanceID.toRfc4122()); - } else { - bytesWritten += destination.write(xoredInstanceID.toRfc4122()); - } + bytesWritten += destination.write(instanceID.toRfc4122()); bytesWritten += destination.writePrimitive(DELETED_TRAIT_SIZE); return bytesWritten; } - - inline TraitInstanceID xoredInstanceID(TraitInstanceID localInstanceID, TraitInstanceID xorKeyID) { - QByteArray xoredInstanceID { NUM_BYTES_RFC4122_UUID, 0 }; - auto xorKeyIDBytes = xorKeyID.toRfc4122(); - auto localInstanceIDBytes = localInstanceID.toRfc4122(); - - for (auto i = 0; i < localInstanceIDBytes.size(); ++i) { - xoredInstanceID[i] = localInstanceIDBytes[i] ^ xorKeyIDBytes[i]; - } - - return QUuid::fromRfc4122(xoredInstanceID); - } }; #endif // hifi_AvatarTraits_h diff --git a/libraries/avatars/src/ClientTraitsHandler.cpp b/libraries/avatars/src/ClientTraitsHandler.cpp index 479852cf9a..94823610dc 100644 --- a/libraries/avatars/src/ClientTraitsHandler.cpp +++ b/libraries/avatars/src/ClientTraitsHandler.cpp @@ -45,7 +45,7 @@ void ClientTraitsHandler::resetForNewMixer() { _owningAvatar->prepareResetTraitInstances(); // reset the trait XOR ID since we're resetting for a new avatar mixer - _owningAvatar->cycleTraitInstanceXORID(); + _sessionXORID = QUuid::createUuid().toRfc4122(); } void ClientTraitsHandler::sendChangedTraitsToMixer() { @@ -100,15 +100,11 @@ void ClientTraitsHandler::sendChangedTraitsToMixer() { // since this is going to the mixer, use the XORed instance ID (to anonymize trait instance IDs // that would typically persist across sessions) _owningAvatar->packTraitInstance(instancedIt->traitType, instanceIDValuePair.id, *traitsPacketList, - AvatarTraits::NULL_TRAIT_VERSION, - AvatarTraits::xoredInstanceID(instanceIDValuePair.id, - _owningAvatar->getTraitInstanceXORID())); + AvatarTraits::NULL_TRAIT_VERSION, xorInstanceID(instanceIDValuePair.id)); } else if (!_shouldPerformInitialSend && instanceIDValuePair.value == Deleted) { // pack delete for this trait instance AvatarTraits::packInstancedTraitDelete(instancedIt->traitType, instanceIDValuePair.id, - *traitsPacketList, AvatarTraits::NULL_TRAIT_VERSION, - AvatarTraits::xoredInstanceID(instanceIDValuePair.id, - _owningAvatar->getTraitInstanceXORID())); + *traitsPacketList); } } @@ -122,6 +118,17 @@ void ClientTraitsHandler::sendChangedTraitsToMixer() { } } +AvatarTraits::TraitInstanceID ClientTraitsHandler::xorInstanceID(AvatarTraits::TraitInstanceID localInstanceID) { + QByteArray xorInstanceID { NUM_BYTES_RFC4122_UUID, 0 }; + auto localInstanceIDBytes = localInstanceID.toRfc4122(); + + for (auto i = 0; i < localInstanceIDBytes.size(); ++i) { + xorInstanceID[i] = localInstanceIDBytes[i] ^ _sessionXORID[i]; + } + + return QUuid::fromRfc4122(xorInstanceID); +} + void ClientTraitsHandler::processTraitOverride(QSharedPointer message, SharedNodePointer sendingNode) { if (sendingNode->getType() == NodeType::AvatarMixer) { while (message->getBytesLeftToRead()) { diff --git a/libraries/avatars/src/ClientTraitsHandler.h b/libraries/avatars/src/ClientTraitsHandler.h index 6d1592ba74..2319061502 100644 --- a/libraries/avatars/src/ClientTraitsHandler.h +++ b/libraries/avatars/src/ClientTraitsHandler.h @@ -37,6 +37,8 @@ public: void resetForNewMixer(); + AvatarTraits::TraitInstanceID xorInstanceID(AvatarTraits::TraitInstanceID localInstanceID); + public slots: void processTraitOverride(QSharedPointer message, SharedNodePointer sendingNode); @@ -53,6 +55,8 @@ private: AvatarTraits::TraitVersion _currentTraitVersion { AvatarTraits::DEFAULT_TRAIT_VERSION }; AvatarTraits::TraitVersion _currentSkeletonVersion { AvatarTraits::NULL_TRAIT_VERSION }; + + QByteArray _sessionXORID { NUM_BYTES_RFC4122_UUID, 0}; bool _shouldPerformInitialSend { false }; bool _hasChangedTraits { false }; From 1789d51d6e15ab7982579f8bbc6a03104e15d0a0 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Wed, 12 Sep 2018 12:22:07 -0700 Subject: [PATCH 4/6] Revert "cycle avatar entity IDs for new avatar mixer" This reverts commit 20912349a4f0b8854bda467ee1e1bd04d9bb67ef. --- .../src/avatars-renderer/Avatar.cpp | 1 + libraries/avatars/src/AvatarData.cpp | 10 ++-------- libraries/avatars/src/AvatarData.h | 3 +-- libraries/avatars/src/AvatarTraits.h | 2 +- libraries/avatars/src/ClientTraitsHandler.cpp | 20 +------------------ libraries/avatars/src/ClientTraitsHandler.h | 8 ++------ 6 files changed, 8 insertions(+), 36 deletions(-) diff --git a/libraries/avatars-renderer/src/avatars-renderer/Avatar.cpp b/libraries/avatars-renderer/src/avatars-renderer/Avatar.cpp index ac25f65576..cadc3b31e3 100644 --- a/libraries/avatars-renderer/src/avatars-renderer/Avatar.cpp +++ b/libraries/avatars-renderer/src/avatars-renderer/Avatar.cpp @@ -141,6 +141,7 @@ Avatar::~Avatar() { } }); } + auto geometryCache = DependencyManager::get(); if (geometryCache) { geometryCache->releaseID(_nameRectGeometryID); diff --git a/libraries/avatars/src/AvatarData.cpp b/libraries/avatars/src/AvatarData.cpp index e7aaa18576..ea6dbd7074 100644 --- a/libraries/avatars/src/AvatarData.cpp +++ b/libraries/avatars/src/AvatarData.cpp @@ -1861,9 +1861,7 @@ qint64 AvatarData::packTrait(AvatarTraits::TraitType traitType, ExtendedIODevice } qint64 AvatarData::packTraitInstance(AvatarTraits::TraitType traitType, AvatarTraits::TraitInstanceID traitInstanceID, - ExtendedIODevice& destination, AvatarTraits::TraitVersion traitVersion, - AvatarTraits::TraitInstanceID wireInstanceID) { - + ExtendedIODevice& destination, AvatarTraits::TraitVersion traitVersion) { qint64 bytesWritten = 0; bytesWritten += destination.writePrimitive(traitType); @@ -1872,11 +1870,7 @@ qint64 AvatarData::packTraitInstance(AvatarTraits::TraitType traitType, AvatarTr bytesWritten += destination.writePrimitive(traitVersion); } - if (!wireInstanceID.isNull()) { - bytesWritten += destination.write(wireInstanceID.toRfc4122()); - } else { - bytesWritten += destination.write(traitInstanceID.toRfc4122()); - } + bytesWritten += destination.write(traitInstanceID.toRfc4122()); if (traitType == AvatarTraits::AvatarEntity) { // grab a read lock on the avatar entities and check for entity data for the given ID diff --git a/libraries/avatars/src/AvatarData.h b/libraries/avatars/src/AvatarData.h index b010a9f924..db2b82b0b7 100644 --- a/libraries/avatars/src/AvatarData.h +++ b/libraries/avatars/src/AvatarData.h @@ -962,8 +962,7 @@ public: qint64 packTrait(AvatarTraits::TraitType traitType, ExtendedIODevice& destination, AvatarTraits::TraitVersion traitVersion = AvatarTraits::NULL_TRAIT_VERSION); qint64 packTraitInstance(AvatarTraits::TraitType traitType, AvatarTraits::TraitInstanceID instanceID, - ExtendedIODevice& destination, AvatarTraits::TraitVersion traitVersion = AvatarTraits::NULL_TRAIT_VERSION, - AvatarTraits::TraitInstanceID wireInstanceID = AvatarTraits::TraitInstanceID()); + ExtendedIODevice& destination, AvatarTraits::TraitVersion traitVersion = AvatarTraits::NULL_TRAIT_VERSION); void prepareResetTraitInstances(); diff --git a/libraries/avatars/src/AvatarTraits.h b/libraries/avatars/src/AvatarTraits.h index 5866caf234..f0c807a432 100644 --- a/libraries/avatars/src/AvatarTraits.h +++ b/libraries/avatars/src/AvatarTraits.h @@ -41,7 +41,7 @@ namespace AvatarTraits { const TraitWireSize DELETED_TRAIT_SIZE = -1; inline qint64 packInstancedTraitDelete(TraitType traitType, TraitInstanceID instanceID, ExtendedIODevice& destination, - TraitVersion traitVersion = NULL_TRAIT_VERSION) { + TraitVersion traitVersion = NULL_TRAIT_VERSION) { qint64 bytesWritten = 0; bytesWritten += destination.writePrimitive(traitType); diff --git a/libraries/avatars/src/ClientTraitsHandler.cpp b/libraries/avatars/src/ClientTraitsHandler.cpp index 94823610dc..a06b53da7c 100644 --- a/libraries/avatars/src/ClientTraitsHandler.cpp +++ b/libraries/avatars/src/ClientTraitsHandler.cpp @@ -43,9 +43,6 @@ void ClientTraitsHandler::resetForNewMixer() { // pre-fill the instanced statuses that we will need to send next frame _owningAvatar->prepareResetTraitInstances(); - - // reset the trait XOR ID since we're resetting for a new avatar mixer - _sessionXORID = QUuid::createUuid().toRfc4122(); } void ClientTraitsHandler::sendChangedTraitsToMixer() { @@ -96,11 +93,7 @@ void ClientTraitsHandler::sendChangedTraitsToMixer() { || instanceIDValuePair.value == Updated) { // this is a changed trait we need to send or we haven't send out trait information yet // ask the owning avatar to pack it - - // since this is going to the mixer, use the XORed instance ID (to anonymize trait instance IDs - // that would typically persist across sessions) - _owningAvatar->packTraitInstance(instancedIt->traitType, instanceIDValuePair.id, *traitsPacketList, - AvatarTraits::NULL_TRAIT_VERSION, xorInstanceID(instanceIDValuePair.id)); + _owningAvatar->packTraitInstance(instancedIt->traitType, instanceIDValuePair.id, *traitsPacketList); } else if (!_shouldPerformInitialSend && instanceIDValuePair.value == Deleted) { // pack delete for this trait instance AvatarTraits::packInstancedTraitDelete(instancedIt->traitType, instanceIDValuePair.id, @@ -118,17 +111,6 @@ void ClientTraitsHandler::sendChangedTraitsToMixer() { } } -AvatarTraits::TraitInstanceID ClientTraitsHandler::xorInstanceID(AvatarTraits::TraitInstanceID localInstanceID) { - QByteArray xorInstanceID { NUM_BYTES_RFC4122_UUID, 0 }; - auto localInstanceIDBytes = localInstanceID.toRfc4122(); - - for (auto i = 0; i < localInstanceIDBytes.size(); ++i) { - xorInstanceID[i] = localInstanceIDBytes[i] ^ _sessionXORID[i]; - } - - return QUuid::fromRfc4122(xorInstanceID); -} - void ClientTraitsHandler::processTraitOverride(QSharedPointer message, SharedNodePointer sendingNode) { if (sendingNode->getType() == NodeType::AvatarMixer) { while (message->getBytesLeftToRead()) { diff --git a/libraries/avatars/src/ClientTraitsHandler.h b/libraries/avatars/src/ClientTraitsHandler.h index 2319061502..27ba58d46b 100644 --- a/libraries/avatars/src/ClientTraitsHandler.h +++ b/libraries/avatars/src/ClientTraitsHandler.h @@ -30,15 +30,13 @@ public: void markTraitUpdated(AvatarTraits::TraitType updatedTrait) { _traitStatuses[updatedTrait] = Updated; _hasChangedTraits = true; } - void markInstancedTraitUpdated(AvatarTraits::TraitType traitType, AvatarTraits::TraitInstanceID updatedInstanceID) + void markInstancedTraitUpdated(AvatarTraits::TraitType traitType, QUuid updatedInstanceID) { _traitStatuses.instanceInsert(traitType, updatedInstanceID, Updated); _hasChangedTraits = true; } - void markInstancedTraitDeleted(AvatarTraits::TraitType traitType, AvatarTraits::TraitInstanceID deleteInstanceID) + void markInstancedTraitDeleted(AvatarTraits::TraitType traitType, QUuid deleteInstanceID) { _traitStatuses.instanceInsert(traitType, deleteInstanceID, Deleted); _hasChangedTraits = true; } void resetForNewMixer(); - AvatarTraits::TraitInstanceID xorInstanceID(AvatarTraits::TraitInstanceID localInstanceID); - public slots: void processTraitOverride(QSharedPointer message, SharedNodePointer sendingNode); @@ -55,8 +53,6 @@ private: AvatarTraits::TraitVersion _currentTraitVersion { AvatarTraits::DEFAULT_TRAIT_VERSION }; AvatarTraits::TraitVersion _currentSkeletonVersion { AvatarTraits::NULL_TRAIT_VERSION }; - - QByteArray _sessionXORID { NUM_BYTES_RFC4122_UUID, 0}; bool _shouldPerformInitialSend { false }; bool _hasChangedTraits { false }; From b1f44d2ad7c82f085881f8e243f75cf06f272167 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Wed, 12 Sep 2018 12:30:15 -0700 Subject: [PATCH 5/6] perform avatar entity cleanup in AvatarManager::handleRemovedAvatar --- interface/src/avatar/AvatarManager.cpp | 5 ++++ .../src/avatars-renderer/Avatar.cpp | 24 ++++++++++--------- .../src/avatars-renderer/Avatar.h | 1 + 3 files changed, 19 insertions(+), 11 deletions(-) diff --git a/interface/src/avatar/AvatarManager.cpp b/interface/src/avatar/AvatarManager.cpp index b9e30a38eb..443d19e473 100644 --- a/interface/src/avatar/AvatarManager.cpp +++ b/interface/src/avatar/AvatarManager.cpp @@ -443,6 +443,11 @@ void AvatarManager::handleRemovedAvatar(const AvatarSharedPointer& removedAvatar avatar->die(); queuePhysicsChange(avatar); + // remove this avatar's entities from the tree now, if we wait (as we did previously) for this Avatar's destructor + // it might not fire until after we create a new instance for the same remote avatar, which creates a race + // on the creation of entities for that avatar instance and the deletion of entities for this instance + avatar->removeAvatarEntitiesFromTree(); + if (removalReason == KillAvatarReason::TheirAvatarEnteredYourBubble) { emit DependencyManager::get()->enteredIgnoreRadius(); } else if (removalReason == KillAvatarReason::AvatarDisconnected) { diff --git a/libraries/avatars-renderer/src/avatars-renderer/Avatar.cpp b/libraries/avatars-renderer/src/avatars-renderer/Avatar.cpp index cadc3b31e3..914a3b7c6e 100644 --- a/libraries/avatars-renderer/src/avatars-renderer/Avatar.cpp +++ b/libraries/avatars-renderer/src/avatars-renderer/Avatar.cpp @@ -131,17 +131,6 @@ Avatar::Avatar(QThread* thread) : } Avatar::~Avatar() { - auto treeRenderer = DependencyManager::get(); - EntityTreePointer entityTree = treeRenderer ? treeRenderer->getTree() : nullptr; - if (entityTree) { - entityTree->withWriteLock([&] { - AvatarEntityMap avatarEntities = getAvatarEntityData(); - for (auto entityID : avatarEntities.keys()) { - entityTree->deleteEntity(entityID, true, true); - } - }); - } - auto geometryCache = DependencyManager::get(); if (geometryCache) { geometryCache->releaseID(_nameRectGeometryID); @@ -386,6 +375,19 @@ void Avatar::updateAvatarEntities() { setAvatarEntityDataChanged(false); } +void Avatar::removeAvatarEntitiesFromTree() { + auto treeRenderer = DependencyManager::get(); + EntityTreePointer entityTree = treeRenderer ? treeRenderer->getTree() : nullptr; + if (entityTree) { + entityTree->withWriteLock([&] { + AvatarEntityMap avatarEntities = getAvatarEntityData(); + for (auto entityID : avatarEntities.keys()) { + entityTree->deleteEntity(entityID, true, true); + } + }); + } +} + void Avatar::relayJointDataToChildren() { forEachChild([&](SpatiallyNestablePointer child) { if (child->getNestableType() == NestableType::Entity) { diff --git a/libraries/avatars-renderer/src/avatars-renderer/Avatar.h b/libraries/avatars-renderer/src/avatars-renderer/Avatar.h index 3482c3c193..4f1c010d84 100644 --- a/libraries/avatars-renderer/src/avatars-renderer/Avatar.h +++ b/libraries/avatars-renderer/src/avatars-renderer/Avatar.h @@ -73,6 +73,7 @@ public: void init(); void updateAvatarEntities(); + void removeAvatarEntitiesFromTree(); void simulate(float deltaTime, bool inView); virtual void simulateAttachments(float deltaTime); From 2b2091290eab1ef6ae3906c1b4f8ad1a09928826 Mon Sep 17 00:00:00 2001 From: SamGondelman Date: Wed, 12 Sep 2018 14:28:39 -0700 Subject: [PATCH 6/6] fix everything --- libraries/fbx/src/FBX.h | 1 - libraries/render-utils/src/Model.cpp | 72 +++++++++++++++------------- libraries/render-utils/src/Model.h | 8 ++-- 3 files changed, 44 insertions(+), 37 deletions(-) diff --git a/libraries/fbx/src/FBX.h b/libraries/fbx/src/FBX.h index 7bd87a4554..fdebb16bc8 100644 --- a/libraries/fbx/src/FBX.h +++ b/libraries/fbx/src/FBX.h @@ -240,7 +240,6 @@ public: QVector vertices; QVector normals; QVector tangents; - mutable QVector normalsAndTangents; // Populated later if needed for blendshapes QVector colors; QVector texCoords; QVector texCoords1; diff --git a/libraries/render-utils/src/Model.cpp b/libraries/render-utils/src/Model.cpp index 2feae1fabb..b9ed43c339 100644 --- a/libraries/render-utils/src/Model.cpp +++ b/libraries/render-utils/src/Model.cpp @@ -317,6 +317,7 @@ bool Model::updateGeometry() { needFullUpdate = true; emit rigReady(); } + return needFullUpdate; } @@ -1033,6 +1034,10 @@ void Model::removeFromScene(const render::ScenePointer& scene, render::Transacti _modelMeshMaterialNames.clear(); _modelMeshRenderItemShapes.clear(); + _blendedVertexBuffers.clear(); + _normalsAndTangents.clear(); + _blendedVertexBuffersInitialized = false; + _addedToScene = false; _renderInfoVertexCount = 0; @@ -1299,25 +1304,26 @@ Blender::Blender(ModelPointer model, int blendNumber, const Geometry::WeakPointe } void Blender::run() { - DETAILED_PROFILE_RANGE_EX(simulation_animation, __FUNCTION__, 0xFFFF0000, 0, { { "url", _model->getURL().toString() } }); QVector vertices; QVector normalsAndTangents; - auto geometry = _geometry.lock(); - if (_model && geometry) { + if (_model && _model->isLoaded()) { + DETAILED_PROFILE_RANGE_EX(simulation_animation, __FUNCTION__, 0xFFFF0000, 0, { { "url", _model->getURL().toString() } }); int offset = 0; int normalsAndTangentsOffset = 0; - auto meshes = geometry->getFBXGeometry().meshes; + auto meshes = _model->getFBXGeometry().meshes; + int meshIndex = 0; foreach (const FBXMesh& mesh, meshes) { - if (mesh.blendshapes.isEmpty()) { + auto modelMeshNormalsAndTangents = _model->_normalsAndTangents.find(meshIndex++); + if (mesh.blendshapes.isEmpty() || modelMeshNormalsAndTangents == _model->_normalsAndTangents.end()) { continue; } vertices += mesh.vertices; - normalsAndTangents += mesh.normalsAndTangents; + normalsAndTangents += modelMeshNormalsAndTangents->second; glm::vec3* meshVertices = vertices.data() + offset; NormalType* meshNormalsAndTangents = normalsAndTangents.data() + normalsAndTangentsOffset; offset += mesh.vertices.size(); - normalsAndTangentsOffset += mesh.normalsAndTangents.size(); + normalsAndTangentsOffset += modelMeshNormalsAndTangents->second.size(); const float NORMAL_COEFFICIENT_SCALE = 0.01f; for (int i = 0, n = qMin(_blendshapeCoefficients.size(), mesh.blendshapes.size()); i < n; i++) { float vertexCoefficient = _blendshapeCoefficients.at(i); @@ -1357,9 +1363,8 @@ void Blender::run() { } // post the result to the ModelBlender, which will dispatch to the model if still alive QMetaObject::invokeMethod(DependencyManager::get().data(), "setBlendedVertices", - Q_ARG(ModelPointer, _model), Q_ARG(int, _blendNumber), - Q_ARG(const Geometry::WeakPointer&, _geometry), Q_ARG(const QVector&, vertices), - Q_ARG(const QVector&, normalsAndTangents)); + Q_ARG(ModelPointer, _model), Q_ARG(int, _blendNumber), Q_ARG(QVector, vertices), + Q_ARG(QVector, normalsAndTangents)); } void Model::setScaleToFit(bool scaleToFit, const glm::vec3& dimensions, bool forceRescale) { @@ -1537,19 +1542,18 @@ bool Model::maybeStartBlender() { return false; } -void Model::setBlendedVertices(int blendNumber, const Geometry::WeakPointer& geometry, - const QVector& vertices, const QVector& normalsAndTangents) { - auto geometryRef = geometry.lock(); - if (!geometryRef || _renderGeometry != geometryRef || blendNumber < _appliedBlendNumber || !_blendedVertexBuffersInitialized) { +void Model::setBlendedVertices(int blendNumber, const QVector& vertices, const QVector& normalsAndTangents) { + if (!isLoaded() || blendNumber < _appliedBlendNumber || !_blendedVertexBuffersInitialized) { return; } _appliedBlendNumber = blendNumber; - const FBXGeometry& fbxGeometry = geometryRef->getFBXGeometry(); + const FBXGeometry& fbxGeometry = getFBXGeometry(); int index = 0; int normalAndTangentIndex = 0; for (int i = 0; i < fbxGeometry.meshes.size(); i++) { const FBXMesh& mesh = fbxGeometry.meshes.at(i); - if (mesh.blendshapes.isEmpty()) { + auto meshNormalsAndTangents = _normalsAndTangents.find(i); + if (mesh.blendshapes.isEmpty() || meshNormalsAndTangents == _normalsAndTangents.end()) { continue; } @@ -1557,18 +1561,19 @@ void Model::setBlendedVertices(int blendNumber, const Geometry::WeakPointer& geo const auto verticesSize = vertexCount * sizeof(glm::vec3); const auto& buffer = _blendedVertexBuffers.find(i); assert(buffer != _blendedVertexBuffers.end()); - buffer->second->resize(mesh.vertices.size() * sizeof(glm::vec3) + mesh.normalsAndTangents.size() * sizeof(NormalType)); + buffer->second->resize(mesh.vertices.size() * sizeof(glm::vec3) + meshNormalsAndTangents->second.size() * sizeof(NormalType)); buffer->second->setSubData(0, verticesSize, (gpu::Byte*) vertices.constData() + index * sizeof(glm::vec3)); - buffer->second->setSubData(verticesSize, mesh.normalsAndTangents.size() * sizeof(NormalType), (const gpu::Byte*) normalsAndTangents.data() + normalAndTangentIndex * sizeof(NormalType)); + buffer->second->setSubData(verticesSize, meshNormalsAndTangents->second.size() * sizeof(NormalType), (const gpu::Byte*) normalsAndTangents.data() + normalAndTangentIndex * sizeof(NormalType)); index += vertexCount; - normalAndTangentIndex += mesh.normalsAndTangents.size(); + normalAndTangentIndex += meshNormalsAndTangents->second.size(); } } void Model::deleteGeometry() { _deleteGeometryCounter++; _blendedVertexBuffers.clear(); + _normalsAndTangents.clear(); _blendedVertexBuffersInitialized = false; _meshStates.clear(); _rig.destroyAnimGraph(); @@ -1634,7 +1639,7 @@ void Model::initializeBlendshapes(const FBXMesh& mesh, int index) { _blendedVertexBuffers[index]->resize(mesh.vertices.size() * sizeof(glm::vec3) + normalsAndTangents.size() * sizeof(NormalType)); _blendedVertexBuffers[index]->setSubData(0, verticesSize, (const gpu::Byte*) mesh.vertices.constData()); _blendedVertexBuffers[index]->setSubData(verticesSize, normalsAndTangents.size() * sizeof(NormalType), (const gpu::Byte*) normalsAndTangents.data()); - mesh.normalsAndTangents = normalsAndTangents; + _normalsAndTangents[index] = normalsAndTangents; } void Model::createRenderItemSet() { @@ -1778,35 +1783,38 @@ ModelBlender::~ModelBlender() { } void ModelBlender::noteRequiresBlend(ModelPointer model) { + Lock lock(_mutex); if (_pendingBlenders < QThread::idealThreadCount()) { if (model->maybeStartBlender()) { _pendingBlenders++; + return; } - return; } - { - Lock lock(_mutex); - _modelsRequiringBlends.insert(model); - } + _modelsRequiringBlends.insert(model); } -void ModelBlender::setBlendedVertices(ModelPointer model, int blendNumber, const Geometry::WeakPointer& geometry, - const QVector& vertices, const QVector& normalsAndTangents) { +void ModelBlender::setBlendedVertices(ModelPointer model, int blendNumber, QVector vertices, QVector normalsAndTangents) { if (model) { - model->setBlendedVertices(blendNumber, geometry, vertices, normalsAndTangents); + model->setBlendedVertices(blendNumber, vertices, normalsAndTangents); } - _pendingBlenders--; { Lock lock(_mutex); - for (auto i = _modelsRequiringBlends.begin(); i != _modelsRequiringBlends.end();) { + _pendingBlenders--; + _modelsRequiringBlends.erase(model); + std::set> modelsToErase; + for (auto i = _modelsRequiringBlends.begin(); i != _modelsRequiringBlends.end(); i++) { auto weakPtr = *i; - _modelsRequiringBlends.erase(i++); // remove front of the set ModelPointer nextModel = weakPtr.lock(); if (nextModel && nextModel->maybeStartBlender()) { _pendingBlenders++; - return; + break; + } else { + modelsToErase.insert(weakPtr); } } + for (auto& weakPtr : modelsToErase) { + _modelsRequiringBlends.erase(weakPtr); + } } } diff --git a/libraries/render-utils/src/Model.h b/libraries/render-utils/src/Model.h index f35048aa2f..447f75dd9d 100644 --- a/libraries/render-utils/src/Model.h +++ b/libraries/render-utils/src/Model.h @@ -144,8 +144,7 @@ public: bool maybeStartBlender(); /// Sets blended vertices computed in a separate thread. - void setBlendedVertices(int blendNumber, const Geometry::WeakPointer& geometry, - const QVector& vertices, const QVector& normalsAndTangents); + void setBlendedVertices(int blendNumber, const QVector& vertices, const QVector& normalsAndTangents); bool isLoaded() const { return (bool)_renderGeometry && _renderGeometry->isGeometryLoaded(); } bool isAddedToScene() const { return _addedToScene; } @@ -345,6 +344,8 @@ public: void addMaterial(graphics::MaterialLayer material, const std::string& parentMaterialName); void removeMaterial(graphics::MaterialPointer material, const std::string& parentMaterialName); + std::unordered_map> _normalsAndTangents; + public slots: void loadURLFinished(bool success); @@ -519,8 +520,7 @@ public: bool shouldComputeBlendshapes() { return _computeBlendshapes; } public slots: - void setBlendedVertices(ModelPointer model, int blendNumber, const Geometry::WeakPointer& geometry, - const QVector& vertices, const QVector& normalsAndTangents); + void setBlendedVertices(ModelPointer model, int blendNumber, QVector vertices, QVector normalsAndTangents); void setComputeBlendshapes(bool computeBlendshapes) { _computeBlendshapes = computeBlendshapes; } private: