From 67c78a2df128c841f6f37f8445e6878c73a44476 Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Thu, 1 Mar 2018 09:26:52 -0800 Subject: [PATCH 1/4] remember list of stale proxies --- .../src/entities/EntityTreeHeadlessViewer.cpp | 4 ---- .../entities-renderer/src/EntityTreeRenderer.cpp | 16 +++++----------- libraries/entities/src/EntityTree.cpp | 11 ++++++----- libraries/entities/src/EntityTree.h | 4 ++-- 4 files changed, 13 insertions(+), 22 deletions(-) diff --git a/assignment-client/src/entities/EntityTreeHeadlessViewer.cpp b/assignment-client/src/entities/EntityTreeHeadlessViewer.cpp index d22055b349..81c42cda1e 100644 --- a/assignment-client/src/entities/EntityTreeHeadlessViewer.cpp +++ b/assignment-client/src/entities/EntityTreeHeadlessViewer.cpp @@ -35,10 +35,6 @@ void EntityTreeHeadlessViewer::update() { EntityTreePointer tree = std::static_pointer_cast(_tree); tree->withTryWriteLock([&] { tree->update(); - - // flush final EntityTree references to deleted entities - std::vector deletedEntities; - tree->swapRemovedEntities(deletedEntities); }); } } diff --git a/libraries/entities-renderer/src/EntityTreeRenderer.cpp b/libraries/entities-renderer/src/EntityTreeRenderer.cpp index eecd21e5ac..78279fbec8 100644 --- a/libraries/entities-renderer/src/EntityTreeRenderer.cpp +++ b/libraries/entities-renderer/src/EntityTreeRenderer.cpp @@ -431,18 +431,12 @@ void EntityTreeRenderer::update(bool simulate) { _space->updateProxies(_spaceUpdates); _spaceUpdates.clear(); } - { // flush final EntityTree references to removed entities - std::vector deletedEntities; - tree->swapRemovedEntities(deletedEntities); - { // delete proxies from workload::Space - std::vector deadProxies; + { + std::vector staleProxies; + tree->swapStaleProxies(staleProxies); + { std::unique_lock lock(_spaceLock); - for (auto entity : deletedEntities) { - int32_t spaceIndex = entity->getSpaceIndex(); - disconnect(entity.get(), &EntityItem::spaceUpdate, this, &EntityTreeRenderer::handleSpaceUpdate); - deadProxies.push_back(spaceIndex); - } - _space->deleteProxies(deadProxies); + _space->deleteProxies(staleProxies); } } diff --git a/libraries/entities/src/EntityTree.cpp b/libraries/entities/src/EntityTree.cpp index 27ec868fcf..7fd1625cd5 100644 --- a/libraries/entities/src/EntityTree.cpp +++ b/libraries/entities/src/EntityTree.cpp @@ -697,10 +697,11 @@ void EntityTree::processRemovedEntities(const DeleteEntityOperator& theOperator) _simulation->prepareEntityForDelete(theEntity); } - // We save a pointer to theEntity for external contexts that need it - // but this means: external contexts that remove entities from the tree - // must occasionally swapRemovedEntities() to flush these references. - _removedEntities.push_back(theEntity); + // keep a record of valid stale spaceIndices so they can be removed from the Space + int32_t spaceIndex = theEntity->getSpaceIndex(); + if (spaceIndex != -1) { + _staleProxies.push_back(spaceIndex); + } } } @@ -2476,4 +2477,4 @@ bool EntityTree::removeMaterialFromOverlay(const QUuid& overlayID, graphics::Mat return _removeMaterialFromOverlayOperator(overlayID, material, parentMaterialName); } return false; -} \ No newline at end of file +} diff --git a/libraries/entities/src/EntityTree.h b/libraries/entities/src/EntityTree.h index 17eb360f86..330717bac0 100644 --- a/libraries/entities/src/EntityTree.h +++ b/libraries/entities/src/EntityTree.h @@ -283,7 +283,7 @@ public: void setMyAvatar(std::shared_ptr myAvatar) { _myAvatar = myAvatar; } - void swapRemovedEntities(std::vector& entities) { entities.swap(_removedEntities); } + void swapStaleProxies(std::vector& proxies) { proxies.swap(_staleProxies); } static void setAddMaterialToEntityOperator(std::function addMaterialToEntityOperator) { _addMaterialToEntityOperator = addMaterialToEntityOperator; } static void setRemoveMaterialFromEntityOperator(std::function removeMaterialFromEntityOperator) { _removeMaterialFromEntityOperator = removeMaterialFromEntityOperator; } @@ -415,7 +415,7 @@ private: static std::function _addMaterialToOverlayOperator; static std::function _removeMaterialFromOverlayOperator; - std::vector _removedEntities; + std::vector _staleProxies; }; #endif // hifi_EntityTree_h From 18a6ac057a773ac2dd131f5ced33534ccafc7304 Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Thu, 1 Mar 2018 12:09:28 -0800 Subject: [PATCH 2/4] add Space::clear() method --- libraries/workload/src/workload/Space.cpp | 5 +++++ libraries/workload/src/workload/Space.h | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/libraries/workload/src/workload/Space.cpp b/libraries/workload/src/workload/Space.cpp index e9745e0916..016b246725 100644 --- a/libraries/workload/src/workload/Space.cpp +++ b/libraries/workload/src/workload/Space.cpp @@ -20,6 +20,11 @@ using namespace workload; +void Space::clear() { + _proxies.clear(); + _freeIndices.clear(); +} + int32_t Space::createProxy(const Space::Sphere& newSphere) { if (_freeIndices.empty()) { _proxies.emplace_back(Space::Proxy(newSphere)); diff --git a/libraries/workload/src/workload/Space.h b/libraries/workload/src/workload/Space.h index fb102d49a0..46ea8651bf 100644 --- a/libraries/workload/src/workload/Space.h +++ b/libraries/workload/src/workload/Space.h @@ -70,6 +70,7 @@ public: Space() {} + void clear(); int32_t createProxy(const Sphere& sphere); void deleteProxies(const std::vector& deadIndices); void updateProxies(const std::vector& changedProxies); @@ -79,7 +80,6 @@ public: uint32_t getNumAllocatedProxies() const { return (uint32_t)(_proxies.size()); } void categorizeAndGetChanges(std::vector& changes); - uint32_t copyProxyValues(Proxy* proxies, uint32_t numDestProxies); private: From 7a852a0e31655d8d75de965ff91b0c64a7b6d910 Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Thu, 1 Mar 2018 12:11:30 -0800 Subject: [PATCH 3/4] clear the Space --- libraries/entities-renderer/src/EntityTreeRenderer.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/libraries/entities-renderer/src/EntityTreeRenderer.cpp b/libraries/entities-renderer/src/EntityTreeRenderer.cpp index 78279fbec8..04e8c9a414 100644 --- a/libraries/entities-renderer/src/EntityTreeRenderer.cpp +++ b/libraries/entities-renderer/src/EntityTreeRenderer.cpp @@ -194,6 +194,7 @@ void EntityTreeRenderer::clear() { } // remove all entities from the scene + _space->clear(); auto scene = _viewState->getMain3DScene(); if (scene) { render::Transaction transaction; From 1e1d9907a4840835834d37321e30513c7f3f1850 Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Thu, 1 Mar 2018 12:22:42 -0800 Subject: [PATCH 4/4] also clear _staleProxies when clearing all else --- libraries/entities/src/EntityTree.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/libraries/entities/src/EntityTree.cpp b/libraries/entities/src/EntityTree.cpp index 7fd1625cd5..da80924184 100644 --- a/libraries/entities/src/EntityTree.cpp +++ b/libraries/entities/src/EntityTree.cpp @@ -98,6 +98,7 @@ void EntityTree::eraseAllOctreeElements(bool createNewRoot) { if (_simulation) { _simulation->clearEntities(); } + _staleProxies.clear(); QHash localMap; localMap.swap(_entityMap); this->withWriteLock([&] {