From 6d347cf44f3018afbf1172eee5d30f454f706388 Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Thu, 15 Feb 2018 17:40:15 -0800 Subject: [PATCH] revert deadlock, remove Space stubbery in EntityTree --- assignment-client/CMakeLists.txt | 2 +- interface/src/Application.cpp | 10 ++ interface/src/Application.h | 3 + libraries/avatars-renderer/CMakeLists.txt | 1 - libraries/entities-renderer/CMakeLists.txt | 1 - libraries/entities/CMakeLists.txt | 2 +- libraries/entities/src/EntityItem.cpp | 11 +-- libraries/entities/src/EntityTree.cpp | 108 ++++++--------------- libraries/entities/src/EntityTree.h | 4 - libraries/physics/CMakeLists.txt | 1 - libraries/script-engine/CMakeLists.txt | 1 - tests/entities/CMakeLists.txt | 2 +- tests/render-perf/CMakeLists.txt | 2 +- 13 files changed, 52 insertions(+), 96 deletions(-) diff --git a/assignment-client/CMakeLists.txt b/assignment-client/CMakeLists.txt index 7750c0a105..c73e8e1d34 100644 --- a/assignment-client/CMakeLists.txt +++ b/assignment-client/CMakeLists.txt @@ -13,7 +13,7 @@ setup_memory_debugger() link_hifi_libraries( audio avatars octree gpu graphics fbx entities networking animation recording shared script-engine embedded-webserver - controllers physics plugins midi image workload + controllers physics plugins midi image ) add_dependencies(${TARGET_NAME} oven) diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp index 8bfb1dfbd4..8a3bdfca0e 100644 --- a/interface/src/Application.cpp +++ b/interface/src/Application.cpp @@ -4477,6 +4477,8 @@ void Application::init() { _physicsEngine->init(); EntityTreePointer tree = getEntities()->getTree(); + connect(tree.get(), &EntityTree::deletingEntity, this, &Application::deletingEntity, Qt::QueuedConnection); + connect(tree.get(), &EntityTree::addingEntity, this, &Application::addingEntity, Qt::QueuedConnection); _entitySimulation->init(tree, _physicsEngine, &_entityEditSender); tree->setSimulation(_entitySimulation); @@ -7606,4 +7608,12 @@ void Application::saveNextPhysicsStats(QString filename) { _physicsEngine->saveNextPhysicsStats(filename); } +void Application::addingEntity(const EntityItemID& entityID) { + // TODO: Andrew to implement this +} + +void Application::deletingEntity(const EntityItemID& entityID) { + // TODO: Andrew to implement this +} + #include "Application.moc" diff --git a/interface/src/Application.h b/interface/src/Application.h index 0435425d5f..3c2dee03e7 100644 --- a/interface/src/Application.h +++ b/interface/src/Application.h @@ -391,6 +391,9 @@ public slots: const QString getPreferredCursor() const { return _preferredCursor.get(); } void setPreferredCursor(const QString& cursor); + void addingEntity(const EntityItemID& entityID); + void deletingEntity(const EntityItemID& entityID); + private slots: void onDesktopRootItemCreated(QQuickItem* qmlContext); void onDesktopRootContextCreated(QQmlContext* qmlContext); diff --git a/libraries/avatars-renderer/CMakeLists.txt b/libraries/avatars-renderer/CMakeLists.txt index d3d61ea3fa..40e1607b2a 100644 --- a/libraries/avatars-renderer/CMakeLists.txt +++ b/libraries/avatars-renderer/CMakeLists.txt @@ -14,6 +14,5 @@ include_hifi_library_headers(audio) include_hifi_library_headers(entities) include_hifi_library_headers(octree) include_hifi_library_headers(task) -include_hifi_library_headers(workload) target_bullet() diff --git a/libraries/entities-renderer/CMakeLists.txt b/libraries/entities-renderer/CMakeLists.txt index 92f78e357e..0f33a73e40 100644 --- a/libraries/entities-renderer/CMakeLists.txt +++ b/libraries/entities-renderer/CMakeLists.txt @@ -14,7 +14,6 @@ include_hifi_library_headers(entities) include_hifi_library_headers(avatars) include_hifi_library_headers(controllers) include_hifi_library_headers(task) -include_hifi_library_headers(workload) target_bullet() target_polyvox() diff --git a/libraries/entities/CMakeLists.txt b/libraries/entities/CMakeLists.txt index 3d43330ebc..d6d9058e44 100644 --- a/libraries/entities/CMakeLists.txt +++ b/libraries/entities/CMakeLists.txt @@ -1,4 +1,4 @@ set(TARGET_NAME entities) setup_hifi_library(Network Script) include_directories(SYSTEM "${OPENSSL_INCLUDE_DIR}") -link_hifi_libraries(shared workload networking octree avatars graphics) +link_hifi_libraries(shared networking octree avatars graphics) diff --git a/libraries/entities/src/EntityItem.cpp b/libraries/entities/src/EntityItem.cpp index 503f55e072..8ae00dee99 100644 --- a/libraries/entities/src/EntityItem.cpp +++ b/libraries/entities/src/EntityItem.cpp @@ -2365,15 +2365,14 @@ QList EntityItem::getActionsOfType(EntityDynamicType typeT void EntityItem::locationChanged(bool tellPhysics) { requiresRecalcBoxes(); - EntityTreePointer tree = getTree(); - if (tree) { - if (tellPhysics) { - _flags |= Simulation::DIRTY_TRANSFORM; + if (tellPhysics) { + _flags |= Simulation::DIRTY_TRANSFORM; + EntityTreePointer tree = getTree(); + if (tree) { tree->entityChanged(getThisPointer()); } - glm::vec4 sphere(getWorldPosition(), 0.5f * glm::length(getScaledDimensions())); - tree->queueUpdateSpaceProxy(_spaceIndex, sphere); } + // TODO: Andrew to connect this to the Space instance in Application SpatiallyNestable::locationChanged(tellPhysics); // tell all the children, also somethingChangedNotification(); } diff --git a/libraries/entities/src/EntityTree.cpp b/libraries/entities/src/EntityTree.cpp index 5854357661..60bcc85575 100644 --- a/libraries/entities/src/EntityTree.cpp +++ b/libraries/entities/src/EntityTree.cpp @@ -560,49 +560,37 @@ void EntityTree::setSimulation(EntitySimulationPointer simulation) { } void EntityTree::deleteEntity(const EntityItemID& entityID, bool force, bool ignoreWarnings) { - EntityItemPointer entity; - { - QReadLocker locker(&_entityMapLock); - entity = _entityMap.value(entityID); - } - if (!entity) { + EntityTreeElementPointer containingElement = getContainingElement(entityID); + if (!containingElement) { if (!ignoreWarnings) { - qCWarning(entities) << "EntityTree::deleteEntity() on unknown entityID=" << entityID; + qCWarning(entities) << "EntityTree::deleteEntity() on non-existent entityID=" << entityID; } return; } - if (entity->getLocked() && !force) { + + EntityItemPointer existingEntity = containingElement->getEntityWithEntityItemID(entityID); + if (!existingEntity) { + if (!ignoreWarnings) { + qCWarning(entities) << "EntityTree::deleteEntity() on non-existant entity item with entityID=" << entityID; + } + return; + } + + if (existingEntity->getLocked() && !force) { if (!ignoreWarnings) { qCDebug(entities) << "ERROR! EntityTree::deleteEntity() trying to delete locked entity. entityID=" << entityID; } return; } - if (!ignoreWarnings) { - if (!entity->getElement()) { - qCWarning(entities) << "EntityTree::deleteEntity() found entity with entityID=" << entityID - << " in map but it doesn't know its containing element"; - } else { - // check for element mismatch - EntityItemPointer otherEntity = entity->getElement()->getEntityWithEntityItemID(entityID); - if (!otherEntity) { - qCWarning(entities) << "EntityTree::deleteEntity() on entity with entityID=" << entityID - << " but the containing element of record cannot find it"; - } else if (otherEntity != entity) { - qCWarning(entities) << "EntityTree::deleteEntity() on entity with entityID=" << entityID - << " but the containing element of record found a different entity"; - } - } - } - unhookChildAvatar(entityID); emit deletingEntity(entityID); - emit deletingEntityPointer(entity.get()); + emit deletingEntityPointer(existingEntity.get()); // NOTE: callers must lock the tree before using this method DeleteEntityOperator theOperator(getThisPointer(), entityID); - entity->forEachDescendant([&](SpatiallyNestablePointer descendant) { + existingEntity->forEachDescendant([&](SpatiallyNestablePointer descendant) { auto descendantID = descendant->getID(); theOperator.addEntityIDToDeleteList(descendantID); emit deletingEntity(descendantID); @@ -632,46 +620,34 @@ void EntityTree::deleteEntities(QSet entityIDs, bool force, bool i // NOTE: callers must lock the tree before using this method DeleteEntityOperator theOperator(getThisPointer()); foreach(const EntityItemID& entityID, entityIDs) { - EntityItemPointer entity; - { - QReadLocker locker(&_entityMapLock); - entity = _entityMap.value(entityID); - } - if (!entity) { + EntityTreeElementPointer containingElement = getContainingElement(entityID); + if (!containingElement) { if (!ignoreWarnings) { - qCWarning(entities) << "EntityTree::deleteEntities() on unknown entityID=" << entityID; + qCWarning(entities) << "EntityTree::deleteEntities() on non-existent entityID=" << entityID; } continue; } - if (entity->getLocked() && !force) { + + EntityItemPointer existingEntity = containingElement->getEntityWithEntityItemID(entityID); + if (!existingEntity) { + if (!ignoreWarnings) { + qCWarning(entities) << "EntityTree::deleteEntities() on non-existent entity item with entityID=" << entityID; + } + continue; + } + + if (existingEntity->getLocked() && !force) { if (!ignoreWarnings) { qCDebug(entities) << "ERROR! EntityTree::deleteEntities() trying to delete locked entity. entityID=" << entityID; } continue; } - if (!ignoreWarnings) { - if (!entity->getElement()) { - qCWarning(entities) << "EntityTree::deleteEntities() found entity with entityID=" << entityID - << " in map but it doesn't know its containing element"; - } else { - // check for element mismatch - EntityItemPointer otherEntity = entity->getElement()->getEntityWithEntityItemID(entityID); - if (!otherEntity) { - qCWarning(entities) << "EntityTree::deleteEntities() on entity with entityID=" << entityID - << " but the containing element of record cannot find it"; - } else if (otherEntity != entity) { - qCWarning(entities) << "EntityTree::deleteEntities() on entity with entityID=" << entityID - << " but the containing element of record found a different entity"; - } - } - } - // tell our delete operator about this entityID unhookChildAvatar(entityID); theOperator.addEntityIDToDeleteList(entityID); emit deletingEntity(entityID); - emit deletingEntityPointer(entity.get()); + emit deletingEntityPointer(existingEntity.get()); } if (theOperator.getEntities().size() > 0) { @@ -1233,10 +1209,6 @@ bool EntityTree::verifyNonce(const QString& certID, const QString& nonce, Entity return verificationSuccess; } -void EntityTree::queueUpdateSpaceProxy(int32_t index, const glm::vec4& sphere) { - _spaceUpdates.push_back(std::pair(index, sphere)); -} - void EntityTree::processChallengeOwnershipRequestPacket(ReceivedMessage& message, const SharedNodePointer& sourceNode) { int certIDByteArraySize; int textByteArraySize; @@ -1838,12 +1810,6 @@ void EntityTree::update(bool simulate) { } }); } - - // process Space queues - _space.deleteProxies(_spaceDeletes); - _spaceDeletes.clear(); - _space.updateProxies(_spaceUpdates); - _spaceUpdates.clear(); } quint64 EntityTree::getAdjustedConsiderSince(quint64 sinceTime) { @@ -2013,25 +1979,11 @@ void EntityTree::addEntityMapEntry(EntityItemPointer entity) { return; } _entityMap.insert(id, entity); - - // create a proxy for the entity in the workload/Space container - glm::vec4 sphere(entity->getWorldPosition(), 0.5f * glm::length(entity->getScaledDimensions())); - int32_t spaceIndex = _space.createProxy(sphere); - entity->setSpaceIndex(spaceIndex); } void EntityTree::clearEntityMapEntry(const EntityItemID& id) { - // this method only called by DeleteEntityOperator QWriteLocker locker(&_entityMapLock); - QHash::iterator itr = _entityMap.find(id); - if (itr != _entityMap.end()) { - // queue delete from _space BEFORE removing from map - int32_t spaceIndex = itr.value()->getSpaceIndex(); - assert(spaceIndex != -1); - _spaceDeletes.push_back(spaceIndex); - - _entityMap.erase(itr); - } + _entityMap.remove(id); } void EntityTree::debugDumpMap() { diff --git a/libraries/entities/src/EntityTree.h b/libraries/entities/src/EntityTree.h index c2d0b229b6..3b75929e2a 100644 --- a/libraries/entities/src/EntityTree.h +++ b/libraries/entities/src/EntityTree.h @@ -17,7 +17,6 @@ #include #include -#include class EntityTree; using EntityTreePointer = std::shared_ptr; @@ -390,9 +389,6 @@ private: void validatePop(const QString& certID, const EntityItemID& entityItemID, const SharedNodePointer& senderNode, bool isRetryingValidation); std::shared_ptr _myAvatar{ nullptr }; - workload::Space _space; - std::vector< std::pair > _spaceUpdates; - std::vector< int32_t > _spaceDeletes; }; #endif // hifi_EntityTree_h diff --git a/libraries/physics/CMakeLists.txt b/libraries/physics/CMakeLists.txt index c9df2dfc77..ad082c1a6e 100644 --- a/libraries/physics/CMakeLists.txt +++ b/libraries/physics/CMakeLists.txt @@ -7,6 +7,5 @@ include_hifi_library_headers(avatars) include_hifi_library_headers(audio) include_hifi_library_headers(octree) include_hifi_library_headers(animation) -include_hifi_library_headers(workload) target_bullet() diff --git a/libraries/script-engine/CMakeLists.txt b/libraries/script-engine/CMakeLists.txt index 6ef8438bf5..31436bbf8b 100644 --- a/libraries/script-engine/CMakeLists.txt +++ b/libraries/script-engine/CMakeLists.txt @@ -20,4 +20,3 @@ endif () link_hifi_libraries(shared networking octree gpu procedural graphics model-networking ktx recording avatars fbx entities controllers animation audio physics image midi) # ui includes gl, but link_hifi_libraries does not use transitive includes, so gl must be explicit include_hifi_library_headers(gl) -include_hifi_library_headers(workload) diff --git a/tests/entities/CMakeLists.txt b/tests/entities/CMakeLists.txt index b1bfcfd8a5..6d0bf9f149 100644 --- a/tests/entities/CMakeLists.txt +++ b/tests/entities/CMakeLists.txt @@ -7,7 +7,7 @@ setup_memory_debugger() set_target_properties(${TARGET_NAME} PROPERTIES FOLDER "Tests/manual-tests/") # link in the shared libraries -link_hifi_libraries(entities avatars shared workload octree gpu graphics fbx networking animation audio gl) +link_hifi_libraries(entities avatars shared octree gpu graphics fbx networking animation audio gl) if (WIN32) add_dependency_external_projects(wasapi) diff --git a/tests/render-perf/CMakeLists.txt b/tests/render-perf/CMakeLists.txt index ed13aadfa3..7e61d9550c 100644 --- a/tests/render-perf/CMakeLists.txt +++ b/tests/render-perf/CMakeLists.txt @@ -18,7 +18,7 @@ link_hifi_libraries( render render-utils graphics fbx model-networking entities entities-renderer audio avatars script-engine - physics procedural midi qml ui workload + physics procedural midi qml ui ${PLATFORM_GL_BACKEND} )