split work out of EntityTree::update(), call it preUpdate()

assignment-client/src/entities/EntityTreeHeadlessViewer.cpp

@@ -37,6 +37,7 @@ void EntityTreeHeadlessViewer::update() {
     if (_tree) {
         EntityTreePointer tree = std::static_pointer_cast<EntityTree>(_tree);
         tree->withTryWriteLock([&] {
+            tree->preUpdate();
             tree->update();
         });
     }

interface/src/Application.cpp

@@ -6399,6 +6399,8 @@ void Application::update(float deltaTime) {
         PROFILE_RANGE(simulation_physics, "Simulation");
         PerformanceTimer perfTimer("simulation");
 
+        getEntities()->preUpdate();
+
         if (_physicsEnabled) {
             auto t0 = std::chrono::high_resolution_clock::now();
             auto t1 = t0;

libraries/entities-renderer/src/EntityTreeRenderer.cpp

@@ -474,6 +474,12 @@ void EntityTreeRenderer::updateChangedEntities(const render::ScenePointer& scene
     }
 }
 
+void EntityTreeRenderer::preUpdate() {
+    if (_tree && !_shuttingDown) {
+        _tree->preUpdate();
+    }
+}
+
 void EntityTreeRenderer::update(bool simulate) {
     PROFILE_RANGE(simulation_physics, "ETR::update");
     PerformanceTimer perfTimer("ETRupdate");

libraries/entities-renderer/src/EntityTreeRenderer.h

@@ -78,6 +78,7 @@ public:
     void setSetPrecisionPickingOperator(std::function<void(unsigned int, bool)> setPrecisionPickingOperator) { _setPrecisionPickingOperator = setPrecisionPickingOperator; }
 
     void shutdown();
+    void preUpdate();
     void update(bool simulate);
 
     EntityTreePointer getTree() { return std::static_pointer_cast<EntityTree>(_tree); }

libraries/entities/src/EntitySimulation.cpp

@@ -176,17 +176,26 @@ void EntitySimulation::addEntity(EntityItemPointer entity) {
 void EntitySimulation::changeEntity(EntityItemPointer entity) {
     QMutexLocker lock(&_mutex);
     assert(entity);
-    if (!entity->isSimulated()) {
-        // This entity was either never added to the simulation or has been removed
-        // (probably for pending delete), so we don't want to keep a pointer to it
-        // on any internal lists.
-        return;
-    }
+    _changedEntities.insert(entity);
+}
 
+void EntitySimulation::processChangedEntities() {
+    QMutexLocker lock(&_mutex);
+    PROFILE_RANGE_EX(simulation_physics, "processChangedEntities", 0xffff00ff, (uint64_t)_changedEntities.size());
+    for (auto& entity : _changedEntities) {
+        if (entity->isSimulated()) {
+            processChangedEntity(entity);
+        }
+    }
+    _changedEntities.clear();
+}
+
+void EntitySimulation::processChangedEntity(const EntityItemPointer& entity) {
+    uint32_t dirtyFlags = entity->getDirtyFlags();
+    /* TODO? maybe add to _entitiesToSort when DIRTY_POSITION is set?
     // Although it is not the responsibility of the EntitySimulation to sort the tree for EXTERNAL changes
     // it IS responsibile for triggering deletes for entities that leave the bounds of the domain, hence
     // we must check for that case here, however we rely on the change event to have set DIRTY_POSITION flag.
-    uint32_t dirtyFlags = entity->getDirtyFlags();
     if (dirtyFlags & Simulation::DIRTY_POSITION) {
         AACube domainBounds(glm::vec3((float)-HALF_TREE_SCALE), (float)TREE_SCALE);
         bool success;
@@ -198,25 +207,29 @@ void EntitySimulation::changeEntity(EntityItemPointer entity) {
             return;
         }
     }
+    */
 
-    if (dirtyFlags & Simulation::DIRTY_LIFETIME) {
-        if (entity->isMortal()) {
-            _mortalEntities.insert(entity);
-            uint64_t expiry = entity->getExpiry();
-            if (expiry < _nextExpiry) {
-                _nextExpiry = expiry;
+    if (dirtyFlags & (Simulation::DIRTY_LIFETIME | Simulation::DIRTY_UPDATEABLE)) {
+        if (dirtyFlags & Simulation::DIRTY_LIFETIME) {
+            if (entity->isMortal()) {
+                _mortalEntities.insert(entity);
+                uint64_t expiry = entity->getExpiry();
+                if (expiry < _nextExpiry) {
+                    _nextExpiry = expiry;
+                }
+            } else {
+                _mortalEntities.remove(entity);
             }
-        } else {
-            _mortalEntities.remove(entity);
         }
-        entity->clearDirtyFlags(Simulation::DIRTY_LIFETIME);
+        if (dirtyFlags & Simulation::DIRTY_UPDATEABLE) {
+            if (entity->needsToCallUpdate()) {
+                _entitiesToUpdate.insert(entity);
+            } else {
+                _entitiesToUpdate.remove(entity);
+            }
+        }
+        entity->clearDirtyFlags(Simulation::DIRTY_LIFETIME | Simulation::DIRTY_UPDATEABLE);
     }
-    if (entity->needsToCallUpdate()) {
-        _entitiesToUpdate.insert(entity);
-    } else {
-        _entitiesToUpdate.remove(entity);
-    }
-    changeEntityInternal(entity);
 }
 
 void EntitySimulation::clearEntities() {

libraries/entities/src/EntitySimulation.h

@@ -13,6 +13,7 @@
 #define hifi_EntitySimulation_h
 
 #include <limits>
+#include <unordered_set>
 
 #include <QtCore/QObject>
 #include <QSet>
@@ -82,13 +83,15 @@ public:
     /// \param entity pointer to EntityItem that needs to be put on the entitiesToDelete list and removed from others.
     virtual void prepareEntityForDelete(EntityItemPointer entity);
 
+    void processChangedEntities();
+
 protected:
     // These pure virtual methods are protected because they are not to be called will-nilly. The base class
     // calls them in the right places.
     virtual void updateEntitiesInternal(uint64_t now) = 0;
     virtual void addEntityInternal(EntityItemPointer entity) = 0;
     virtual void removeEntityInternal(EntityItemPointer entity);
-    virtual void changeEntityInternal(EntityItemPointer entity) = 0;
+    virtual void processChangedEntity(const EntityItemPointer& entity);
     virtual void clearEntitiesInternal() = 0;
 
     void expireMortalEntities(uint64_t now);
@@ -114,11 +117,11 @@ private:
 
     // We maintain multiple lists, each for its distinct purpose.
     // An entity may be in more than one list.
+    std::unordered_set<EntityItemPointer> _changedEntities; // all changes this frame
     SetOfEntities _allEntities; // tracks all entities added the simulation
     SetOfEntities _mortalEntities; // entities that have an expiry
     uint64_t _nextExpiry;
 
-
     SetOfEntities _entitiesToUpdate; // entities that need to call EntityItem::update()
 };
 

libraries/entities/src/EntityTree.cpp

@@ -2188,11 +2188,19 @@ void EntityTree::addToNeedsParentFixupList(EntityItemPointer entity) {
     _needsParentFixup.append(entity);
 }
 
+void EntityTree::preUpdate() {
+    withWriteLock([&] {
+        fixupNeedsParentFixups();
+        if (_simulation) {
+            _simulation->processChangedEntities();
+        }
+    });
+}
+
 void EntityTree::update(bool simulate) {
     PROFILE_RANGE(simulation_physics, "UpdateTree");
     PerformanceTimer perfTimer("updateTree");
     withWriteLock([&] {
-        fixupNeedsParentFixups();
         if (simulate && _simulation) {
             _simulation->updateEntities();
             {

libraries/entities/src/EntityTree.h

@@ -109,9 +109,10 @@ public:
 
     virtual void releaseSceneEncodeData(OctreeElementExtraEncodeData* extraEncodeData) const override;
 
-    virtual void update() override { update(true); }
-
-    void update(bool simulate);
+    // Why preUpdate() and update()?
+    // Because sometimes we need to do stuff between the two.
+    void preUpdate() override;
+    void update(bool simulate = true) override;
 
     // The newer API...
     void postAddEntity(EntityItemPointer entityItem);

libraries/entities/src/SimpleEntitySimulation.cpp

@@ -85,7 +85,9 @@ void SimpleEntitySimulation::removeEntityInternal(EntityItemPointer entity) {
     _entitiesThatNeedSimulationOwner.remove(entity);
 }
 
-void SimpleEntitySimulation::changeEntityInternal(EntityItemPointer entity) {
+void SimpleEntitySimulation::processChangedEntity(const EntityItemPointer& entity) {
+    EntitySimulation::processChangedEntity(entity);
+
     uint32_t flags = entity->getDirtyFlags();
     if ((flags & Simulation::DIRTY_SIMULATOR_ID) || (flags & Simulation::DIRTY_VELOCITIES)) {
         if (entity->getSimulatorID().isNull()) {

libraries/entities/src/SimpleEntitySimulation.h

@@ -31,7 +31,7 @@ protected:
     void updateEntitiesInternal(uint64_t now) override;
     void addEntityInternal(EntityItemPointer entity) override;
     void removeEntityInternal(EntityItemPointer entity) override;
-    void changeEntityInternal(EntityItemPointer entity) override;
+    void processChangedEntity(const EntityItemPointer& entity) override;
     void clearEntitiesInternal() override;
 
     void sortEntitiesThatMoved() override;

libraries/octree/src/Octree.h

@@ -145,7 +145,10 @@ public:
     virtual bool rootElementHasData() const { return false; }
     virtual void releaseSceneEncodeData(OctreeElementExtraEncodeData* extraEncodeData) const { }
 
-    virtual void update() { } // nothing to do by default
+    // Why preUpdate() and update()?
+    // Because EntityTree needs them.
+    virtual void preUpdate() { }
+    virtual void update(bool simulate = true) { }
 
     OctreeElementPointer getRoot() { return _rootElement; }
 

libraries/octree/src/OctreePersistThread.cpp

@@ -242,6 +242,7 @@ bool OctreePersistThread::backupCurrentFile() {
 }
 
 void OctreePersistThread::process() {
+    _tree->preUpdate();
     _tree->update();
 
     auto now = std::chrono::steady_clock::now();

libraries/physics/src/PhysicalEntitySimulation.cpp

@@ -131,10 +131,10 @@ void PhysicalEntitySimulation::takeDeadAvatarEntities(SetOfEntities& deadEntitie
     _deadAvatarEntities.clear();
 }
 
-void PhysicalEntitySimulation::changeEntityInternal(EntityItemPointer entity) {
+void PhysicalEntitySimulation::processChangedEntity(const EntityItemPointer& entity) {
+    EntitySimulation::processChangedEntity(entity);
+
     // queue incoming changes: from external sources (script, EntityServer, etc) to physics engine
-    QMutexLocker lock(&_mutex);
-    assert(entity);
     EntityMotionState* motionState = static_cast<EntityMotionState*>(entity->getPhysicsInfo());
     uint8_t region = _space->getRegion(entity->getSpaceIndex());
     bool shouldBePhysical = region < workload::Region::R3 && entity->shouldBePhysical();

libraries/physics/src/PhysicalEntitySimulation.h

@@ -72,7 +72,7 @@ protected: // only called by EntitySimulation
     virtual void updateEntitiesInternal(uint64_t now) override;
     virtual void addEntityInternal(EntityItemPointer entity) override;
     virtual void removeEntityInternal(EntityItemPointer entity) override;
-    virtual void changeEntityInternal(EntityItemPointer entity) override;
+    void processChangedEntity(const EntityItemPointer& entity) override;
     virtual void clearEntitiesInternal() override;
 
     void removeOwnershipData(EntityMotionState* motionState);