Merge pull request #11421 from jherico/fix/transform_log_spam

Don't add items to the scene graph until they have a valid parent chain

libraries/entities-renderer/src/EntityTreeRenderer.cpp

@@ -159,6 +159,78 @@ void EntityTreeRenderer::shutdown() {
     clear(); // always clear() on shutdown
 }
 
+void EntityTreeRenderer::addPendingEntities(const render::ScenePointer& scene, render::Transaction& transaction) {
+    // Clear any expired entities 
+    // FIXME should be able to use std::remove_if, but it fails due to some 
+    // weird compilation error related to EntityItemID assignment operators
+    for (auto itr = _entitiesToAdd.begin(); _entitiesToAdd.end() != itr; ) {
+        if (itr->second.expired()) {
+            _entitiesToAdd.erase(itr++);
+        } else {
+            ++itr;
+        }
+    }
+
+    if (!_entitiesToAdd.empty()) {
+        std::unordered_set<EntityItemID> processedIds;
+        for (const auto& entry : _entitiesToAdd) {
+            auto entity = entry.second.lock();
+            if (!entity) {
+                continue;
+            }
+
+            // Path to the parent transforms is not valid, 
+            // don't add to the scene graph yet
+            if (!entity->isParentPathComplete()) {
+                continue;
+            }
+
+            auto entityID = entity->getEntityItemID();
+            processedIds.insert(entityID);
+            auto renderable = EntityRenderer::addToScene(*this, entity, scene, transaction);
+            if (renderable) {
+                _entitiesInScene.insert({ entityID, renderable });
+            }
+        }
+
+
+        if (!processedIds.empty()) {
+            for (const auto& processedId : processedIds) {
+                _entitiesToAdd.erase(processedId);
+            }
+        }
+    }
+}
+
+void EntityTreeRenderer::updateChangedEntities(const render::ScenePointer& scene, render::Transaction& transaction) {
+    std::unordered_set<EntityItemID> changedEntities;
+    _changedEntitiesGuard.withWriteLock([&] {
+#if 0
+        // FIXME Weird build failure in latest VC update that fails to compile when using std::swap
+        changedEntities.swap(_changedEntities);
+#else
+        changedEntities.insert(_changedEntities.begin(), _changedEntities.end());
+        _changedEntities.clear();
+#endif
+    });
+
+    for (const auto& entityId : changedEntities) {
+        auto renderable = renderableForEntityId(entityId);
+        if (!renderable) {
+            continue;
+        }
+        _renderablesToUpdate.insert({ entityId, renderable });
+    }
+
+    if (!_renderablesToUpdate.empty()) {
+        for (const auto& entry : _renderablesToUpdate) {
+            const auto& renderable = entry.second;
+            renderable->updateInScene(scene, transaction);
+        }
+        _renderablesToUpdate.clear();
+    }
+}
+
 void EntityTreeRenderer::update(bool simulate) {
     PerformanceTimer perfTimer("ETRupdate");
     if (_tree && !_shuttingDown) {
@@ -178,30 +250,11 @@ void EntityTreeRenderer::update(bool simulate) {
             }
         }
 
-        std::unordered_set<EntityItemID> changedEntities;
-        // FIXME Weird build failure in latest VC update that fails to compile when using std::swap
-        _changedEntitiesGuard.withWriteLock([&] {
-            changedEntities.insert(_changedEntities.begin(), _changedEntities.end());
-            _changedEntities.clear();
-        });
-
-        for (const auto& entityId : changedEntities) {
-            auto renderable = renderableForEntityId(entityId);
-            if (!renderable) {
-                continue;
-            }
-
-            _renderablesToUpdate.insert({ entityId, renderable });
-        }
-
         auto scene = _viewState->getMain3DScene();
-        if (scene && !_renderablesToUpdate.empty()) {
+        if (scene) {
             render::Transaction transaction;
-            for (const auto& entry : _renderablesToUpdate) {
-                const auto& renderable = entry.second;
-                renderable->updateInScene(scene, transaction);
-            }
-            _renderablesToUpdate.clear();
+            addPendingEntities(scene, transaction);
+            updateChangedEntities(scene, transaction);
             scene->enqueueTransaction(transaction);
         }
     }
@@ -689,8 +742,12 @@ void EntityTreeRenderer::mouseMoveEvent(QMouseEvent* event) {
 }
 
 void EntityTreeRenderer::deletingEntity(const EntityItemID& entityID) {
+    // If it's in the pending queue, remove it
+    _entitiesToAdd.erase(entityID);
+
     auto itr = _entitiesInScene.find(entityID);
     if (_entitiesInScene.end() == itr) {
+        // Not in the scene, and no longer potentially in the pending queue, we're done
         return;
     }
         
@@ -725,25 +782,10 @@ void EntityTreeRenderer::addingEntity(const EntityItemID& entityID) {
     checkAndCallPreload(entityID);
     auto entity = std::static_pointer_cast<EntityTree>(_tree)->findEntityByID(entityID);
     if (entity) {
-        addEntityToScene(entity);
+        _entitiesToAdd.insert({ entity->getEntityItemID(),  entity });
     }
 }
 
-void EntityTreeRenderer::addEntityToScene(const EntityItemPointer& entity) {
-    // here's where we add the entity payload to the scene
-    auto scene = _viewState->getMain3DScene();
-    if (!scene) {
-        qCWarning(entitiesrenderer) << "EntityTreeRenderer::addEntityToScene(), Unexpected null scene, possibly during application shutdown";
-        return;
-    }
-
-    auto renderable = EntityRenderer::addToScene(*this, entity, scene);
-    if (renderable) {
-        _entitiesInScene[entity->getEntityItemID()] = renderable;
-    }
-}
-
-
 void EntityTreeRenderer::entityScriptChanging(const EntityItemID& entityID, bool reload) {
     checkAndCallPreload(entityID, reload, true);
 }

libraries/entities-renderer/src/EntityTreeRenderer.h

@@ -36,6 +36,7 @@ namespace render { namespace entities {
     class EntityRenderer;
     using EntityRendererPointer = std::shared_ptr<EntityRenderer>;
     using EntityRendererWeakPointer = std::weak_ptr<EntityRenderer>;
+
 } }
 
 // Allow the use of std::unordered_map with QUuid keys
@@ -157,12 +158,13 @@ protected:
     }
 
 private:
+    void addPendingEntities(const render::ScenePointer& scene, render::Transaction& transaction);
+    void updateChangedEntities(const render::ScenePointer& scene, render::Transaction& transaction);
     EntityRendererPointer renderableForEntity(const EntityItemPointer& entity) const { return renderableForEntityId(entity->getID()); }
     render::ItemID renderableIdForEntity(const EntityItemPointer& entity) const { return renderableIdForEntityId(entity->getID()); }
 
     void resetEntitiesScriptEngine();
 
-    void addEntityToScene(const EntityItemPointer& entity);
     bool findBestZoneAndMaybeContainingEntities(QVector<EntityItemID>* entitiesContainingAvatar = nullptr);
 
     bool applyLayeredZones();
@@ -260,6 +262,7 @@ private:
 
     std::unordered_map<EntityItemID, EntityRendererPointer> _renderablesToUpdate;
     std::unordered_map<EntityItemID, EntityRendererPointer> _entitiesInScene;
+    std::unordered_map<EntityItemID, EntityItemWeakPointer> _entitiesToAdd;
     // For Scene.shouldRenderEntities
     QList<EntityItemID> _entityIDsLastInScene;
 

libraries/entities-renderer/src/RenderableEntityItem.cpp

@@ -187,7 +187,7 @@ void EntityRenderer::render(RenderArgs* args) {
 // Methods called by the EntityTreeRenderer
 //
 
-EntityRenderer::Pointer EntityRenderer::addToScene(EntityTreeRenderer& renderer, const EntityItemPointer& entity, const ScenePointer& scene) {
+EntityRenderer::Pointer EntityRenderer::addToScene(EntityTreeRenderer& renderer, const EntityItemPointer& entity, const ScenePointer& scene, Transaction& transaction) {
     EntityRenderer::Pointer result;
     if (!entity) {
         return result;
@@ -245,9 +245,7 @@ EntityRenderer::Pointer EntityRenderer::addToScene(EntityTreeRenderer& renderer,
     }
 
     if (result) {
-        Transaction transaction;
         result->addToScene(scene, transaction);
-        scene->enqueueTransaction(transaction);
     }
 
     return result;

libraries/entities-renderer/src/RenderableEntityItem.h

@@ -30,7 +30,7 @@ class EntityRenderer : public QObject, public std::enable_shared_from_this<Entit
 
 public:
     static void initEntityRenderers();
-    static Pointer addToScene(EntityTreeRenderer& renderer, const EntityItemPointer& entity, const ScenePointer& scene);
+    static Pointer addToScene(EntityTreeRenderer& renderer, const EntityItemPointer& entity, const ScenePointer& scene, Transaction& transaction);
 
     // Allow classes to override this to interact with the user
     virtual bool wantsHandControllerPointerEvents() const { return false; }

libraries/render/src/render/Forward.h

@@ -28,6 +28,7 @@ namespace render {
     class Scene;
     using ScenePointer = std::shared_ptr<Scene>;
     class ShapePipeline;
+    class Transaction;
 }
 
 using RenderArgs = render::Args;

libraries/shared/src/SpatiallyNestable.cpp

@@ -1182,3 +1182,19 @@ void SpatiallyNestable::dump(const QString& prefix) const {
         parent->dump(prefix + "    ");
     }
 }
+
+bool SpatiallyNestable::isParentPathComplete() const {
+    static const QUuid IDENTITY;
+    QUuid parentID = getParentID();
+    if (parentID.isNull() || parentID == IDENTITY) {
+        return true;
+    }
+
+    bool success = false;
+    SpatiallyNestablePointer parent = getParentPointer(success);
+    if (!success || !parent) {
+        return false;
+    }
+
+    return parent->isParentPathComplete();
+}

libraries/shared/src/SpatiallyNestable.h

@@ -66,6 +66,10 @@ public:
 
     static QString nestableTypeToString(NestableType nestableType);
 
+
+    virtual bool isParentPathComplete() const;
+
+
     // world frame
     virtual const Transform getTransform(bool& success, int depth = 0) const;
     virtual const Transform getTransform() const;