From 09fe9735fac987c6656699089b41190db6aebb27 Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Thu, 24 Jan 2019 14:57:54 -0800 Subject: [PATCH] plug grab leak, more correct names --- interface/src/avatar/AvatarManager.cpp | 1 + interface/src/avatar/MyAvatar.cpp | 4 +-- interface/src/avatar/OtherAvatar.cpp | 2 +- .../src/avatars-renderer/Avatar.cpp | 32 +++++++++++++------ .../src/avatars-renderer/Avatar.h | 8 +++-- 5 files changed, 32 insertions(+), 15 deletions(-) diff --git a/interface/src/avatar/AvatarManager.cpp b/interface/src/avatar/AvatarManager.cpp index b9c7dc729d..2a83724f9e 100644 --- a/interface/src/avatar/AvatarManager.cpp +++ b/interface/src/avatar/AvatarManager.cpp @@ -452,6 +452,7 @@ void AvatarManager::handleRemovedAvatar(const AvatarSharedPointer& removedAvatar _spaceProxiesToDelete.push_back(avatar->getSpaceIndex()); } AvatarHashMap::handleRemovedAvatar(avatar, removalReason); + avatar->tearDownGrabs(); avatar->die(); queuePhysicsChange(avatar); diff --git a/interface/src/avatar/MyAvatar.cpp b/interface/src/avatar/MyAvatar.cpp index 2ccc540105..7bb8d38ffa 100755 --- a/interface/src/avatar/MyAvatar.cpp +++ b/interface/src/avatar/MyAvatar.cpp @@ -817,7 +817,7 @@ void MyAvatar::simulate(float deltaTime, bool inView) { // and all of its joints, now update our attachements. Avatar::simulateAttachments(deltaTime); relayJointDataToChildren(); - if (updateGrabs()) { + if (applyGrabChanges()) { _cauterizationNeedsUpdate = true; } @@ -5310,7 +5310,7 @@ void MyAvatar::releaseGrab(const QUuid& grabID) { _avatarGrabsLock.withWriteLock([&] { if (_avatarGrabData.remove(grabID)) { - _deletedAvatarGrabs.push_back(grabID); + _grabsToDelete.push_back(grabID); tellHandler = true; } }); diff --git a/interface/src/avatar/OtherAvatar.cpp b/interface/src/avatar/OtherAvatar.cpp index 1f4fdc5e52..803e4be610 100644 --- a/interface/src/avatar/OtherAvatar.cpp +++ b/interface/src/avatar/OtherAvatar.cpp @@ -225,7 +225,7 @@ void OtherAvatar::simulate(float deltaTime, bool inView) { { PROFILE_RANGE(simulation, "grabs"); - updateGrabs(); + applyGrabChanges(); } updateFadingStatus(); diff --git a/libraries/avatars-renderer/src/avatars-renderer/Avatar.cpp b/libraries/avatars-renderer/src/avatars-renderer/Avatar.cpp index d27a3026af..7cdc82d7e5 100644 --- a/libraries/avatars-renderer/src/avatars-renderer/Avatar.cpp +++ b/libraries/avatars-renderer/src/avatars-renderer/Avatar.cpp @@ -324,8 +324,8 @@ void Avatar::removeAvatarEntitiesFromTree() { } } -bool Avatar::updateGrabs() { - if (!_avatarGrabDataChanged && _changedAvatarGrabs.empty() && _deletedAvatarGrabs.empty()) { +bool Avatar::applyGrabChanges() { + if (!_avatarGrabDataChanged && _grabsToChange.empty() && _grabsToDelete.empty()) { // early exit for most common case: nothing to do return false; } @@ -340,11 +340,11 @@ bool Avatar::updateGrabs() { GrabPointer grab = std::make_shared(); grab->fromByteArray(_avatarGrabData.value(grabID)); _avatarGrabs[grabID] = grab; - _changedAvatarGrabs.insert(grabID); + _grabsToChange.insert(grabID); } else { bool changed = itr->second->fromByteArray(_avatarGrabData.value(grabID)); if (changed) { - _changedAvatarGrabs.insert(grabID); + _grabsToChange.insert(grabID); } } } @@ -353,7 +353,7 @@ bool Avatar::updateGrabs() { // delete _avatarGrabs VectorOfIDs undeleted; - for (const auto& id : _deletedAvatarGrabs) { + for (const auto& id : _grabsToDelete) { MapOfGrabs::iterator itr = _avatarGrabs.find(id); if (itr == _avatarGrabs.end()) { continue; @@ -370,11 +370,11 @@ bool Avatar::updateGrabs() { undeleted.push_back(id); } } - _deletedAvatarGrabs = std::move(undeleted); + _grabsToDelete = std::move(undeleted); // change _avatarGrabs and add Actions to target SetOfIDs unchanged; - for (const auto& id : _changedAvatarGrabs) { + for (const auto& id : _grabsToChange) { MapOfGrabs::iterator itr = _avatarGrabs.find(id); if (itr == _avatarGrabs.end()) { continue; @@ -396,7 +396,7 @@ bool Avatar::updateGrabs() { unchanged.insert(id); } } - _changedAvatarGrabs = std::move(unchanged); + _grabsToChange = std::move(unchanged); }); return grabAddedOrRemoved; } @@ -423,6 +423,20 @@ void Avatar::accumulateGrabPositions(std::map& g }); } +void Avatar::tearDownGrabs() { + _avatarGrabsLock.withWriteLock([&] { + for (const auto& entry : _avatarGrabs) { + _grabsToDelete.push_back(entry.first); + } + _grabsToChange.clear(); + }); + applyGrabChanges(); + if (!_grabsToDelete.empty()) { + // some grabs failed to delete, which is a possible "leak", so we log about it + qWarning() << "Failed to tearDown" << _grabsToDelete.size() << "grabs for Avatar" << getID(); + } +} + void Avatar::relayJointDataToChildren() { forEachChild([&](SpatiallyNestablePointer child) { if (child->getNestableType() == NestableType::Entity) { @@ -1925,7 +1939,7 @@ void Avatar::clearAvatarGrabData(const QUuid& id) { AvatarData::clearAvatarGrabData(id); _avatarGrabsLock.withWriteLock([&] { if (_avatarGrabs.find(id) == _avatarGrabs.end()) { - _deletedAvatarGrabs.push_back(id); + _grabsToDelete.push_back(id); } }); } diff --git a/libraries/avatars-renderer/src/avatars-renderer/Avatar.h b/libraries/avatars-renderer/src/avatars-renderer/Avatar.h index 75ae697603..1c3768e937 100644 --- a/libraries/avatars-renderer/src/avatars-renderer/Avatar.h +++ b/libraries/avatars-renderer/src/avatars-renderer/Avatar.h @@ -441,6 +441,8 @@ public: void accumulateGrabPositions(std::map& grabAccumulators); + void tearDownGrabs(); + signals: void targetScaleChanged(float targetScale); @@ -543,7 +545,7 @@ protected: // protected methods... bool isLookingAtMe(AvatarSharedPointer avatar) const; - bool updateGrabs(); + bool applyGrabChanges(); void relayJointDataToChildren(); void fade(render::Transaction& transaction, render::Transition::Type type); @@ -637,8 +639,8 @@ protected: using MapOfGrabs = std::map; MapOfGrabs _avatarGrabs; - SetOfIDs _changedAvatarGrabs; // updated grab IDs -- changes needed to entities or physics - VectorOfIDs _deletedAvatarGrabs; // deleted grab IDs -- changes needed to entities or physics + SetOfIDs _grabsToChange; // updated grab IDs -- changes needed to entities or physics + VectorOfIDs _grabsToDelete; // deleted grab IDs -- changes needed to entities or physics }; #endif // hifi_Avatar_h