create collision render geometry only when needed

libraries/entities-renderer/src/RenderableModelEntityItem.cpp

@@ -29,6 +29,9 @@
 #include "RenderableModelEntityItem.h"
 #include "RenderableEntityItem.h"
 
+static CollisionRenderMeshCache collisionMeshCache;
+
+
 EntityItemPointer RenderableModelEntityItem::factory(const EntityItemID& entityID, const EntityItemProperties& properties) {
     EntityItemPointer entity{ new RenderableModelEntityItem(entityID, properties.getDimensionsInitialized()) };
     entity->setProperties(properties);
@@ -434,11 +437,28 @@ void RenderableModelEntityItem::render(RenderArgs* args) {
             // Remap textures for the next frame to avoid flicker
             remapTextures();
 
-            // update whether the model should be showing collision mesh
+            // update whether the model should be showing collision mesh (this may flag for fixupInScene)
-            // (this may flag for fixupInScene)
+            ShapeType type = getShapeType();
-            bool shouldShowCollisionMesh = getShapeType() != SHAPE_TYPE_STATIC_MESH &&
+            bool shouldShowCollisionGeometry = type != SHAPE_TYPE_STATIC_MESH &&
+                type != SHAPE_TYPE_NONE &&
                 (args->_debugFlags & (int)RenderArgs::RENDER_DEBUG_HULLS) > 0;
-            _model->setShowCollisionMesh(shouldShowCollisionMesh);
+            if (shouldShowCollisionGeometry != _showCollisionGeometry) {
+                _showCollisionGeometry = shouldShowCollisionGeometry;
+                if (_showCollisionGeometry) {
+                    // NOTE: it is OK if _collisionMeshKey is nullptr
+                    model::MeshPointer mesh = collisionMeshCache.getMesh(_collisionMeshKey);
+                    // NOTE: the model will render the collisionGeometry if it has one
+                    _model->setCollisionMesh(mesh);
+                } else {
+                    // release mesh
+                    if (_collisionMeshKey) {
+                        collisionMeshCache.releaseMesh(_collisionMeshKey);
+                    }
+                    // clear model's collision geometry
+                    model::MeshPointer mesh = nullptr;
+                    _model->setCollisionMesh(mesh);
+                }
+            }
 
             if (_model->needsFixupInScene()) {
                 render::PendingChanges pendingChanges;
@@ -954,21 +974,15 @@ void RenderableModelEntityItem::computeShapeInfo(ShapeInfo& info) {
     }
 }
 
-static CollisionRenderMeshCache collisionMeshCache;
-
 void RenderableModelEntityItem::setCollisionShape(const btCollisionShape* shape) {
     const void* key = static_cast<const void*>(shape);
     if (_collisionMeshKey != key) {
         if (_collisionMeshKey) {
-            // releasing the shape is not strictly necessary, but
-            // we do it as hint to the cache's garbage collection system
             collisionMeshCache.releaseMesh(_collisionMeshKey);
         }
         _collisionMeshKey = key;
-        model::MeshPointer mesh = collisionMeshCache.getMesh(_collisionMeshKey);
+        // toggle _showCollisionGeometry forces re-evaluation later
-        if (_model) {
+        _showCollisionGeometry = !_showCollisionGeometry;
-            _model->setCollisionMesh(mesh);
-        }
     }
 }
 

libraries/entities-renderer/src/RenderableModelEntityItem.h

@@ -119,6 +119,7 @@ private:
     bool getAnimationFrame();
 
     bool _needsJointSimulation { false };
+    bool _showCollisionGeometry { false };
     const void* _collisionMeshKey { nullptr };
 };
 

libraries/render-utils/src/Model.cpp

@@ -118,29 +118,8 @@ Model::~Model() {
 
 AbstractViewStateInterface* Model::_viewState = NULL;
 
-void Model::setShowCollisionMesh(bool value) {
-    if (_showCollisionGeometry != value) {
-        _showCollisionGeometry = value;
-        _needsFixupInScene = true;
-    }
-}
-
 bool Model::needsFixupInScene() const {
-    if ((_needsFixupInScene || !_addedToScene) && !_needsReload && isLoaded()) {
+    return (_needsFixupInScene || !_addedToScene) && !_needsReload && isLoaded();
-        if (_showCollisionGeometry && _collisionGeometry) {
-            return true;
-        }
-        if (!_meshStates.isEmpty() || (_renderGeometry && _renderGeometry->getMeshes().empty())) {
-            if (_needsUpdateTextures) {
-                if (!_renderGeometry->areTexturesLoaded()) {
-                    return false;
-                }
-                _needsUpdateTextures = false;
-            }
-            return true;
-        }
-    }
-    return false;
 }
 
 // TODO?: should we combine translation and rotation into single method to avoid double-work?
@@ -610,13 +589,13 @@ void Model::setVisibleInScene(bool newValue, std::shared_ptr<render::Scene> scen
 bool Model::addToScene(std::shared_ptr<render::Scene> scene,
                        render::PendingChanges& pendingChanges,
                        render::Item::Status::Getters& statusGetters) {
-    bool readyToRender = (_showCollisionGeometry && _collisionGeometry) || isLoaded();
+    bool readyToRender = _collisionGeometry || isLoaded();
     if (!_addedToScene && readyToRender) {
         createRenderItemSet();
     }
 
     bool somethingAdded = false;
-    if (_showCollisionGeometry && _collisionGeometry) {
+    if (_collisionGeometry) {
         if (_collisionRenderItems.empty()) {
             foreach (auto renderItem, _collisionRenderItemsSet) {
                 auto item = scene->allocateID();
@@ -1237,7 +1216,7 @@ AABox Model::getRenderableMeshBound() const {
 }
 
 void Model::createRenderItemSet() {
-    if (_showCollisionGeometry && _collisionGeometry) {
+    if (_collisionGeometry) {
         if (_collisionRenderItemsSet.empty()) {
             createCollisionRenderItemSet();
         }
@@ -1332,7 +1311,7 @@ bool Model::initWhenReady(render::ScenePointer scene) {
     render::PendingChanges pendingChanges;
 
     bool addedPendingChanges = false;
-    if (_showCollisionGeometry && _collisionGeometry) {
+    if (_collisionGeometry) {
         foreach (auto renderItem, _collisionRenderItemsSet) {
             auto item = scene->allocateID();
             auto renderPayload = std::make_shared<MeshPartPayload::Payload>(renderItem);
@@ -1374,13 +1353,12 @@ public:
 };
 
 void Model::setCollisionMesh(model::MeshPointer mesh) {
-    _collisionGeometry = std::make_shared<CollisionRenderGeometry>(mesh);
+    if (mesh) {
-
+        _collisionGeometry = std::make_shared<CollisionRenderGeometry>(mesh);
-    // TODO: At the moment we create the collision mesh for every model that has a collision shape
+    } else {
-    // as soon as we know the shape, but we SHOULD only ever create the render mesh when we need it.
+        _collisionGeometry.reset();
-    if (_showCollisionGeometry) {
-        _needsFixupInScene = true;
     }
+    _needsFixupInScene = true;
 }
 
 ModelBlender::ModelBlender() :

libraries/render-utils/src/Model.h

@@ -82,8 +82,6 @@ public:
     void setVisibleInScene(bool newValue, std::shared_ptr<render::Scene> scene);
     bool needsFixupInScene() const;
 
-    void setShowCollisionMesh(bool value);
-
     bool readyToAddToScene(RenderArgs* renderArgs = nullptr) const {
         return !_needsReload && isRenderable() && isActive();
     }
@@ -395,7 +393,6 @@ protected:
     bool _needsFixupInScene { true }; // needs to be removed/re-added to scene
     bool _needsReload { true };
     bool _needsUpdateClusterMatrices { true };
-    bool _showCollisionGeometry { false };
     mutable bool _needsUpdateTextures { true };
 
     friend class ModelMeshPartPayload;