Merge pull request #7169 from samcake/red

Fix the crash when Particle system entities are created and removed quickly

interface/src/Application.cpp

@@ -3773,8 +3773,9 @@ void Application::displaySide(RenderArgs* renderArgs, Camera& theCamera, bool se
     // The pending changes collecting the changes here
     render::PendingChanges pendingChanges;
 
+    // FIXME: Move this out of here!, Background / skybox should be driven by the enityt content just like the other entities
     // Background rendering decision
-    if (BackgroundRenderData::_item == 0) {
+    if (!render::Item::isValidID(BackgroundRenderData::_item)) {
         auto backgroundRenderData = make_shared<BackgroundRenderData>();
         auto backgroundRenderPayload = make_shared<BackgroundRenderData::Payload>(backgroundRenderData);
         BackgroundRenderData::_item = _main3DScene->allocateID();
@@ -3799,8 +3800,9 @@ void Application::displaySide(RenderArgs* renderArgs, Camera& theCamera, bool se
         }
     }
 
+    // FIXME: Move this out of here!, WorldBox should be driven by the entity content just like the other entities
     // Make sure the WorldBox is in the scene
-    if (WorldBoxRenderData::_item == 0) {
+    if (!render::Item::isValidID(WorldBoxRenderData::_item)) {
         auto worldBoxRenderData = make_shared<WorldBoxRenderData>();
         auto worldBoxRenderPayload = make_shared<WorldBoxRenderData::Payload>(worldBoxRenderData);
 

interface/src/avatar/Avatar.cpp

@@ -315,6 +315,7 @@ bool Avatar::addToScene(AvatarSharedPointer self, std::shared_ptr<render::Scene>
 
 void Avatar::removeFromScene(AvatarSharedPointer self, std::shared_ptr<render::Scene> scene, render::PendingChanges& pendingChanges) {
     pendingChanges.removeItem(_renderItemID);
+    render::Item::clearID(_renderItemID);
     _skeletonModel.removeFromScene(scene, pendingChanges);
     getHead()->getFaceModel().removeFromScene(scene, pendingChanges);
     for (auto& attachmentModel : _attachmentModels) {
@@ -323,7 +324,7 @@ void Avatar::removeFromScene(AvatarSharedPointer self, std::shared_ptr<render::S
 }
 
 void Avatar::updateRenderItem(render::PendingChanges& pendingChanges) {
-    if (_renderItemID != render::Item::INVALID_ITEM_ID) {
+    if (render::Item::isValidID(_renderItemID)) {
         pendingChanges.updateItem<render::Payload<AvatarData>>(_renderItemID, [](render::Payload<AvatarData>& p) {});
     }
 }

interface/src/ui/overlays/Overlay.cpp

@@ -229,6 +229,6 @@ bool Overlay::addToScene(Overlay::Pointer overlay, std::shared_ptr<render::Scene
 
 void Overlay::removeFromScene(Overlay::Pointer overlay, std::shared_ptr<render::Scene> scene, render::PendingChanges& pendingChanges) {
     pendingChanges.removeItem(_renderItemID);
-    _renderItemID = render::Item::INVALID_ITEM_ID;
+    render::Item::clearID(_renderItemID);
 }
 

interface/src/ui/overlays/Overlays.cpp

@@ -94,7 +94,7 @@ void Overlays::cleanupOverlaysToDelete() {
                 Overlay::Pointer overlay = _overlaysToDelete.takeLast();
 
                 auto itemID = overlay->getRenderItemID();
-                if (itemID != render::Item::INVALID_ITEM_ID) {
+                if (render::Item::isValidID(itemID)) {
                     overlay->removeFromScene(overlay, scene, pendingChanges);
                 }
             } while (!_overlaysToDelete.isEmpty());
@@ -241,7 +241,7 @@ bool Overlays::editOverlay(unsigned int id, const QScriptValue& properties) {
             render::ItemKey itemKey = render::payloadGetKey(thisOverlay);
             if (itemKey != oldItemKey) {
                 auto itemID = thisOverlay->getRenderItemID();
-                if (itemID != render::Item::INVALID_ITEM_ID) {
+                if (render::Item::isValidID(itemID)) {
                     render::ScenePointer scene = qApp->getMain3DScene();
                     render::PendingChanges pendingChanges;
                     pendingChanges.resortItem(itemID, oldItemKey, itemKey);

libraries/entities-renderer/src/RenderableEntityItem.h

@@ -66,10 +66,11 @@ public:
 
     void removeFromScene(EntityItemPointer self, std::shared_ptr<render::Scene> scene, render::PendingChanges& pendingChanges) {
         pendingChanges.removeItem(_myItem);
+        render::Item::clearID(_myItem);
     }
 
     void notifyChanged() {
-        if (_myItem == render::Item::INVALID_ITEM_ID) {
+        if (!render::Item::isValidID(_myItem)) {
             return;
         }
 

libraries/entities-renderer/src/RenderableModelEntityItem.cpp

@@ -251,6 +251,7 @@ bool RenderableModelEntityItem::addToScene(EntityItemPointer self, std::shared_p
 void RenderableModelEntityItem::removeFromScene(EntityItemPointer self, std::shared_ptr<render::Scene> scene,
                                                 render::PendingChanges& pendingChanges) {
     pendingChanges.removeItem(_myMetaItem);
+    render::Item::clearID(_myMetaItem);
     if (_model) {
         _model->removeFromScene(scene, pendingChanges);
     }

libraries/entities-renderer/src/RenderableParticleEffectEntityItem.cpp

@@ -179,6 +179,7 @@ void RenderableParticleEffectEntityItem::removeFromScene(EntityItemPointer self,
                                                          render::PendingChanges& pendingChanges) {
     pendingChanges.removeItem(_renderItemId);
     _scene = nullptr;
+    render::Item::clearID(_renderItemId);
 };
 
 void RenderableParticleEffectEntityItem::update(const quint64& now) {
@@ -199,7 +200,8 @@ void RenderableParticleEffectEntityItem::update(const quint64& now) {
 }
 
 void RenderableParticleEffectEntityItem::updateRenderItem() {
-    if (!_scene) {
+    // this 2 tests are synonyms for this class, but we would like to get rid of the _scene pointer ultimately
+    if (!_scene || !render::Item::isValidID(_renderItemId)) { 
         return;
     }
     if (!getVisible()) {
@@ -312,7 +314,7 @@ void RenderableParticleEffectEntityItem::createPipelines() {
 }
 
 void RenderableParticleEffectEntityItem::notifyBoundChanged() {
-    if (_renderItemId == render::Item::INVALID_ITEM_ID) {
+    if (!render::Item::isValidID(_renderItemId)) {
         return;
     }
     render::PendingChanges pendingChanges;

libraries/entities-renderer/src/RenderablePolyVoxEntityItem.cpp

@@ -572,6 +572,7 @@ void RenderablePolyVoxEntityItem::removeFromScene(EntityItemPointer self,
                                                   std::shared_ptr<render::Scene> scene,
                                                   render::PendingChanges& pendingChanges) {
     pendingChanges.removeItem(_myItem);
+    render::Item::clearID(_myItem);
 }
 
 namespace render {

libraries/entities-renderer/src/RenderableZoneEntityItem.cpp

@@ -230,6 +230,7 @@ bool RenderableZoneEntityItem::addToScene(EntityItemPointer self, std::shared_pt
 void RenderableZoneEntityItem::removeFromScene(EntityItemPointer self, std::shared_ptr<render::Scene> scene,
                                                 render::PendingChanges& pendingChanges) {
     pendingChanges.removeItem(_myMetaItem);
+    render::Item::clearID(_myMetaItem);
     if (_model) {
         _model->removeFromScene(scene, pendingChanges);
     }
@@ -237,7 +238,7 @@ void RenderableZoneEntityItem::removeFromScene(EntityItemPointer self, std::shar
 
 
 void RenderableZoneEntityItem::notifyBoundChanged() {
-    if (_myMetaItem == render::Item::INVALID_ITEM_ID) {
+    if (!render::Item::isValidID(_myMetaItem)) {
         return;
     }
     render::PendingChanges pendingChanges;

libraries/render/src/render/Item.h

@@ -217,6 +217,10 @@ public:
     static const ID INVALID_ITEM_ID = 0;
     static const ItemCell INVALID_CELL = -1;
 
+    // Convenient function to clear an ID or check it s valid
+    static void clearID(ID& id) { id = INVALID_ITEM_ID; }
+    static bool isValidID(const ID id) { return id != INVALID_ITEM_ID; }
+
     // Bound is the AABBox fully containing this item
     typedef AABox Bound;