revert deadlock, remove Space stubbery in EntityTree

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)

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"

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);

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()

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()

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)

libraries/entities/src/EntityItem.cpp

@@ -2365,15 +2365,14 @@ QList<EntityDynamicPointer> EntityItem::getActionsOfType(EntityDynamicType typeT
 
 void EntityItem::locationChanged(bool tellPhysics) {
     requiresRecalcBoxes();
-    EntityTreePointer tree = getTree();
+    if (tellPhysics) {
-    if (tree) {
+        _flags |= Simulation::DIRTY_TRANSFORM;
-        if (tellPhysics) {
+        EntityTreePointer tree = getTree();
-            _flags |= Simulation::DIRTY_TRANSFORM;
+        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();
 }

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;
+    EntityTreeElementPointer containingElement = getContainingElement(entityID);
-    {
+    if (!containingElement) {
-        QReadLocker locker(&_entityMapLock);
-        entity = _entityMap.value(entityID);
-    }
-    if (!entity) {
         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<EntityItemID> 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;
+        EntityTreeElementPointer containingElement = getContainingElement(entityID);
-        {
+        if (!containingElement) {
-            QReadLocker locker(&_entityMapLock);
-            entity = _entityMap.value(entityID);
-        }
-        if (!entity) {
             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<int32_t, glm::vec4>(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<EntityItemID, EntityItemPointer>::iterator itr = _entityMap.find(id);
+    _entityMap.remove(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);
-    }
 }
 
 void EntityTree::debugDumpMap() {

libraries/entities/src/EntityTree.h

@@ -17,7 +17,6 @@
 
 #include <Octree.h>
 #include <SpatialParentFinder.h>
-#include <workload/Space.h>
 
 class EntityTree;
 using EntityTreePointer = std::shared_ptr<EntityTree>;
@@ -390,9 +389,6 @@ private:
     void validatePop(const QString& certID, const EntityItemID& entityItemID, const SharedNodePointer& senderNode, bool isRetryingValidation);
 
     std::shared_ptr<AvatarData> _myAvatar{ nullptr };
-    workload::Space _space;
-    std::vector< std::pair<int32_t, glm::vec4> > _spaceUpdates;
-    std::vector< int32_t > _spaceDeletes;
 };
 
 #endif // hifi_EntityTree_h

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()

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)

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)

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}
 )