cleanup logic around creating RenderItems

libraries/entities-renderer/src/RenderableModelEntityItem.cpp

@@ -177,6 +177,24 @@ void RenderableModelEntityItem::doInitialModelSimulation() {
     _needsInitialSimulation = false;
 }
 
+// TODO: we need a solution for changes to the postion/rotation/etc of a model...
+// this current code path only addresses that in this setup case... not the changing/moving case
+bool RenderableModelEntityItem::readyToAddToScene(RenderArgs* renderArgs) {
+    if (!_model && renderArgs) {
+        // TODO: this getModel() appears to be about 3% of model render time. We should optimize
+        PerformanceTimer perfTimer("getModel");
+        EntityTreeRenderer* renderer = static_cast<EntityTreeRenderer*>(renderArgs->_renderer);
+        getModel(renderer);
+    }
+    if (renderArgs && _model && _needsInitialSimulation && _model->isActive() && _model->isLoaded()) {
+        // make sure to simulate so everything gets set up correctly for rendering
+        doInitialModelSimulation();
+        _model->renderSetup(renderArgs);
+    }
+    bool ready = !_needsInitialSimulation && _model && _model->readyToAddToScene();
+    return ready;
+}
+
 class RenderableModelEntityItemMeta {
 public:
     RenderableModelEntityItemMeta(EntityItemPointer entity) : entity(entity){ }
@@ -215,21 +233,21 @@ namespace render {
 bool RenderableModelEntityItem::addToScene(EntityItemPointer self, std::shared_ptr<render::Scene> scene, 
                                             render::PendingChanges& pendingChanges) {
     _myMetaItem = scene->allocateID();
-    
+
     auto renderData = std::make_shared<RenderableModelEntityItemMeta>(self);
     auto renderPayload = std::make_shared<RenderableModelEntityItemMeta::Payload>(renderData);
-    
+
     pendingChanges.resetItem(_myMetaItem, renderPayload);
-    
+
     if (_model) {
         render::Item::Status::Getters statusGetters;
         makeEntityItemStatusGetters(getThisPointer(), statusGetters);
-        
+
-        // note: we don't care if the model fails to add items, we always added our meta item and therefore we return
+        // note: we don't mind if the model fails to add, we'll retry (in render()) until it succeeds
-        // true so that the system knows our meta item is in the scene!
+        _model->addToScene(scene, pendingChanges, statusGetters);
-        _model->addToScene(scene, pendingChanges, statusGetters, _showCollisionHull);
     }
 
+    // we've successfully added _myMetaItem so we always return true
     return true;
 }
 
@@ -416,19 +434,20 @@ void RenderableModelEntityItem::render(RenderArgs* args) {
             // Remap textures for the next frame to avoid flicker
             remapTextures();
 
-            // check to see if when we added our models to the scene they were ready, if they were not ready, then
+            // update whether the model should be showing collision mesh
-            // fix them up in the scene
+            // (this may flag for fixupInScene)
-            bool shouldShowCollisionHull = (args->_debugFlags & (int)RenderArgs::RENDER_DEBUG_HULLS) > 0
+            bool shouldShowCollisionHull = getShapeType() != SHAPE_TYPE_STATIC_MESH &&
-                && getShapeType() == SHAPE_TYPE_COMPOUND;
+                (args->_debugFlags & (int)RenderArgs::RENDER_DEBUG_HULLS) > 0;
-            if (_model->needsFixupInScene() || _showCollisionHull != shouldShowCollisionHull) {
+            _model->setShowCollisionMesh(shouldShowCollisionHull);
-                _showCollisionHull = shouldShowCollisionHull;
+
+            if (_model->needsFixupInScene()) {
                 render::PendingChanges pendingChanges;
 
                 _model->removeFromScene(scene, pendingChanges);
 
                 render::Item::Status::Getters statusGetters;
                 makeEntityItemStatusGetters(getThisPointer(), statusGetters);
-                _model->addToScene(scene, pendingChanges, statusGetters, _showCollisionHull);
+                _model->addToScene(scene, pendingChanges, statusGetters);
 
                 scene->enqueuePendingChanges(pendingChanges);
             }

libraries/entities-renderer/src/RenderableModelEntityItem.h

@@ -114,8 +114,6 @@ private:
 
     render::ItemID _myMetaItem{ render::Item::INVALID_ITEM_ID };
 
-    bool _showCollisionHull = false;
-
     bool getAnimationFrame();
 
     bool _needsJointSimulation { false };

libraries/entities-renderer/src/RenderableZoneEntityItem.cpp

@@ -122,7 +122,7 @@ void RenderableZoneEntityItem::render(RenderArgs* args) {
                     _model->removeFromScene(scene, pendingChanges);
                     render::Item::Status::Getters statusGetters;
                     makeEntityItemStatusGetters(getThisPointer(), statusGetters);
-                    _model->addToScene(scene, pendingChanges, false);
+                    _model->addToScene(scene, pendingChanges);
                     
                     scene->enqueuePendingChanges(pendingChanges);
                     

libraries/render-utils/src/MeshPartPayload.cpp

@@ -414,8 +414,7 @@ ShapeKey ModelMeshPartPayload::getShapeKey() const {
     // if our index is ever out of range for either meshes or networkMeshes, then skip it, and set our _meshGroupsKnown
     // to false to rebuild out mesh groups.
     if (_meshIndex < 0 || _meshIndex >= (int)networkMeshes.size() || _meshIndex > geometry.meshes.size()) {
-        _model->_meshGroupsKnown = false; // regenerate these lists next time around.
+        _model->_needsFixupInScene = true; // trigger remove/add cycle
-        _model->_readyWhenAdded = false; // in case any of our users are using scenes
         _model->invalidCalculatedMeshBoxes(); // if we have to reload, we need to assume our mesh boxes are all invalid
         return ShapeKey::Builder::invalid();
     }
@@ -533,7 +532,7 @@ void ModelMeshPartPayload::startFade() {
 void ModelMeshPartPayload::render(RenderArgs* args) const {
     PerformanceTimer perfTimer("ModelMeshPartPayload::render");
 
-    if (!_model->_readyWhenAdded || !_model->_isVisible) {
+    if (!_model->addedToScene() || !_model->isVisible()) {
         return; // bail asap
     }
 

libraries/render-utils/src/Model.cpp

@@ -100,7 +100,6 @@ Model::Model(RigPointer rig, QObject* parent) :
     _calculatedMeshPartBoxesValid(false),
     _calculatedMeshBoxesValid(false),
     _calculatedMeshTrianglesValid(false),
-    _meshGroupsKnown(false),
     _isWireframe(false),
     _rig(rig)
 {
@@ -121,19 +120,33 @@ Model::~Model() {
 
 AbstractViewStateInterface* Model::_viewState = NULL;
 
+void Model::setShowCollisionMesh(bool value) {
+    if (_showCollisionHull != value) {
+        _showCollisionHull = value;
+        _needsFixupInScene = true;
+    }
+}
+
 bool Model::needsFixupInScene() const {
-    if (readyToAddToScene()) {
+    if ((_needsFixupInScene || !_addedToScene) && !_needsReload && isLoaded()) {
-        if (_needsUpdateTextures && _renderGeometry->areTexturesLoaded()) {
+        if (_showCollisionHull && _collisionGeometry) {
-            _needsUpdateTextures = false;
             return true;
         }
-        if (!_readyWhenAdded) {
+        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?
+// (figure out where we call these)
 void Model::setTranslation(const glm::vec3& translation) {
     _translation = translation;
     updateRenderItems();
@@ -172,6 +185,9 @@ void Model::setOffset(const glm::vec3& offset) {
 }
 
 void Model::updateRenderItems() {
+    if (!_addedToScene) {
+        return;
+    }
 
     _needsUpdateClusterMatrices = true;
     _renderItemsNeedUpdate = false;
@@ -574,8 +590,8 @@ void Model::renderSetup(RenderArgs* args) {
         }
     }
 
-    if (!_meshGroupsKnown && isLoaded()) {
+    if (!_addedToScene && isLoaded()) {
-        createRenderItems();
+        createRenderItemSet();
     }
 }
 
@@ -596,43 +612,46 @@ 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,
+                       render::Item::Status::Getters& statusGetters) {
-                       bool showCollisionHull) {
+    bool readyToRender = (_showCollisionHull && _collisionGeometry) || isLoaded();
-    if ((!_meshGroupsKnown || showCollisionHull != _showCollisionHull) && isLoaded()) {
+    if (!_addedToScene && readyToRender) {
-        _showCollisionHull = showCollisionHull;
+        createRenderItemSet();
-        createRenderItems();
     }
 
     bool somethingAdded = false;
-
+    if (_showCollisionHull && _collisionGeometry) {
-    if (_modelMeshRenderItems.empty()) {
+        if (_collisionRenderItems.empty()) {
-        foreach (auto renderItem, _modelMeshRenderItemsSet) {
+            foreach (auto renderItem, _collisionRenderItemsSet) {
-            auto item = scene->allocateID();
+                auto item = scene->allocateID();
-            auto renderPayload = std::make_shared<ModelMeshPartPayload::Payload>(renderItem);
+                auto renderPayload = std::make_shared<MeshPartPayload::Payload>(renderItem);
-            if (statusGetters.size()) {
+                if (statusGetters.size()) {
-                renderPayload->addStatusGetters(statusGetters);
+                    renderPayload->addStatusGetters(statusGetters);
+                }
+                pendingChanges.resetItem(item, renderPayload);
+                _collisionRenderItems.insert(item, renderPayload);
             }
-            pendingChanges.resetItem(item, renderPayload);
+            somethingAdded = !_collisionRenderItems.empty();
-            _modelMeshRenderItems.insert(item, renderPayload);
-            somethingAdded = true;
         }
-    }
+    } else {
-    if (_collisionRenderItems.empty()) {
+        if (_modelMeshRenderItems.empty()) {
-        foreach (auto renderItem, _collisionRenderItemsSet) {
+            foreach (auto renderItem, _modelMeshRenderItemsSet) {
-            auto item = scene->allocateID();
+                auto item = scene->allocateID();
-            auto renderPayload = std::make_shared<MeshPartPayload::Payload>(renderItem);
+                auto renderPayload = std::make_shared<ModelMeshPartPayload::Payload>(renderItem);
-            if (statusGetters.size()) {
+                if (statusGetters.size()) {
-                renderPayload->addStatusGetters(statusGetters);
+                    renderPayload->addStatusGetters(statusGetters);
+                }
+                pendingChanges.resetItem(item, renderPayload);
+                _modelMeshRenderItems.insert(item, renderPayload);
             }
-            pendingChanges.resetItem(item, renderPayload);
+            somethingAdded = !_modelMeshRenderItems.empty();
-            _collisionRenderItems.insert(item, renderPayload);
-            somethingAdded = true;
         }
     }
 
-    updateRenderItems();
+    if (somethingAdded) {
-
+        _addedToScene = true;
-    _readyWhenAdded = readyToAddToScene();
+        updateRenderItems();
+        _needsFixupInScene = false;
+    }
 
     return somethingAdded;
 }
@@ -643,13 +662,13 @@ void Model::removeFromScene(std::shared_ptr<render::Scene> scene, render::Pendin
     }
     _modelMeshRenderItems.clear();
     _modelMeshRenderItemsSet.clear();
+
     foreach (auto item, _collisionRenderItems.keys()) {
         pendingChanges.removeItem(item);
     }
     _collisionRenderItems.clear();
     _collisionRenderItemsSet.clear();
-    _meshGroupsKnown = false;
+    _addedToScene = false;
-    _readyWhenAdded = false;
 }
 
 void Model::renderDebugMeshBoxes(gpu::Batch& batch) {
@@ -804,6 +823,7 @@ int Model::getLastFreeJointIndex(int jointIndex) const {
 void Model::setTextures(const QVariantMap& textures) {
     if (isLoaded()) {
         _needsUpdateTextures = true;
+        _needsFixupInScene = true;
         _renderGeometry->setTextures(textures);
     }
 }
@@ -825,8 +845,8 @@ void Model::setURL(const QUrl& url) {
 
     _needsReload = true;
     _needsUpdateTextures = true;
-    _meshGroupsKnown = false;
     _visualGeometryRequestFailed = false;
+    _needsFixupInScene = true;
     invalidCalculatedMeshBoxes();
     deleteGeometry();
 
@@ -1236,21 +1256,21 @@ AABox Model::getRenderableMeshBound() const {
     }
 }
 
-void Model::createRenderItems() {
+void Model::createRenderItemSet() {
-    Geometry::Pointer geometry;
-    bool showingCollisionHull = false;
     if (_showCollisionHull && _collisionGeometry) {
-        if (isCollisionLoaded()) {
+        if (_collisionRenderItemsSet.empty()) {
-            geometry = _collisionGeometry;
+            createCollisionRenderItemSet();
-            showingCollisionHull = true;
-        } else {
-            return;
         }
     } else {
-        assert(isLoaded());
+        if (_modelMeshRenderItemsSet.empty()) {
-        geometry = _renderGeometry;
+            createVisibleRenderItemSet();
+        }
     }
-    const auto& meshes = geometry->getMeshes();
+};
+
+void Model::createVisibleRenderItemSet() {
+    assert(isLoaded());
+    const auto& meshes = _renderGeometry->getMeshes();
 
     // all of our mesh vectors must match in size
     if ((int)meshes.size() != _meshStates.size()) {
@@ -1259,13 +1279,9 @@ void Model::createRenderItems() {
     }
 
     // We should not have any existing renderItems if we enter this section of code
-    Q_ASSERT(_modelMeshRenderItems.isEmpty());
     Q_ASSERT(_modelMeshRenderItemsSet.isEmpty());
-    Q_ASSERT(_collisionRenderItems.isEmpty());
-    Q_ASSERT(_collisionRenderItemsSet.isEmpty());
 
     _modelMeshRenderItemsSet.clear();
-    _collisionRenderItemsSet.clear();
 
     Transform transform;
     transform.setTranslation(_translation);
@@ -1280,60 +1296,98 @@ void Model::createRenderItems() {
     uint32_t numMeshes = (uint32_t)meshes.size();
     for (uint32_t i = 0; i < numMeshes; i++) {
         const auto& mesh = meshes.at(i);
-        if (mesh) {
+        if (!mesh) {
+            continue;
+        }
 
-            // Create the render payloads
+        // Create the render payloads
-            int numParts = (int)mesh->getNumParts();
+        int numParts = (int)mesh->getNumParts();
-            for (int partIndex = 0; partIndex < numParts; partIndex++) {
+        for (int partIndex = 0; partIndex < numParts; partIndex++) {
-                if (showingCollisionHull) {
+            _modelMeshRenderItemsSet << std::make_shared<ModelMeshPartPayload>(this, i, partIndex, shapeID, transform, offset);
-                    if (_collisionHullMaterials.empty()) {
+            shapeID++;
-                        initCollisionHullMaterials();
-                    }
-                    _collisionRenderItemsSet << std::make_shared<MeshPartPayload>(mesh, partIndex, _collisionHullMaterials[partIndex % NUM_COLLISION_HULL_COLORS], transform, offset);
-                } else {
-                    _modelMeshRenderItemsSet << std::make_shared<ModelMeshPartPayload>(this, i, partIndex, shapeID, transform, offset);
-                }
-
-                shapeID++;
-            }
         }
     }
-    _meshGroupsKnown = true;
+}
+
+void Model::createCollisionRenderItemSet() {
+    assert((bool)_collisionGeometry);
+    if (_collisionHullMaterials.empty()) {
+        initCollisionHullMaterials();
+    }
+
+    const auto& meshes = _collisionGeometry->getMeshes();
+
+    // We should not have any existing renderItems if we enter this section of code
+    Q_ASSERT(_collisionRenderItemsSet.isEmpty());
+
+    Transform transform;
+    transform.setTranslation(_translation);
+    transform.setRotation(_rotation);
+
+    Transform offset;
+    offset.setScale(_scale);
+    offset.postTranslate(_offset);
+
+    // Run through all of the meshes, and place them into their segregated, but unsorted buckets
+    uint32_t numMeshes = (uint32_t)meshes.size();
+    for (uint32_t i = 0; i < numMeshes; i++) {
+        const auto& mesh = meshes.at(i);
+        if (!mesh) {
+            continue;
+        }
+
+        // Create the render payloads
+        int numParts = (int)mesh->getNumParts();
+        for (int partIndex = 0; partIndex < numParts; partIndex++) {
+            model::MaterialPointer& material =  _collisionHullMaterials[partIndex % NUM_COLLISION_HULL_COLORS];
+            _collisionRenderItemsSet << std::make_shared<MeshPartPayload>(mesh, partIndex, material, transform, offset);
+        }
+    }
+}
+
+bool Model::isRenderable() const {
+    return !_meshStates.isEmpty() || (isLoaded() && _renderGeometry->getMeshes().empty());
 }
 
 bool Model::initWhenReady(render::ScenePointer scene) {
-    if (isActive() && isRenderable() && !_meshGroupsKnown && isLoaded()) {
+    // NOTE: this only called by SkeletonModel
-        createRenderItems();
+    if (_addedToScene || !isRenderable()) {
+        return false;
+    }
 
-        render::PendingChanges pendingChanges;
+    createRenderItemSet();
 
-        Transform transform;
+    render::PendingChanges pendingChanges;
-        transform.setTranslation(_translation);
-        transform.setRotation(_rotation);
 
-        Transform offset;
+    bool addedPendingChanges = false;
-        offset.setScale(_scale);
+    if (_showCollisionHull && _collisionGeometry) {
-        offset.postTranslate(_offset);
-
-        foreach (auto renderItem, _modelMeshRenderItemsSet) {
-            auto item = scene->allocateID();
-            auto renderPayload = std::make_shared<ModelMeshPartPayload::Payload>(renderItem);
-            _modelMeshRenderItems.insert(item, renderPayload);
-            pendingChanges.resetItem(item, renderPayload);
-        }
         foreach (auto renderItem, _collisionRenderItemsSet) {
             auto item = scene->allocateID();
             auto renderPayload = std::make_shared<MeshPartPayload::Payload>(renderItem);
             _collisionRenderItems.insert(item, renderPayload);
             pendingChanges.resetItem(item, renderPayload);
         }
-        scene->enqueuePendingChanges(pendingChanges);
+        addedPendingChanges = !_collisionRenderItems.empty();
-        updateRenderItems();
+    } else {
-
+        foreach (auto renderItem, _modelMeshRenderItemsSet) {
-        _readyWhenAdded = true;
+            auto item = scene->allocateID();
-        return true;
+            auto renderPayload = std::make_shared<ModelMeshPartPayload::Payload>(renderItem);
+            _modelMeshRenderItems.insert(item, renderPayload);
+            pendingChanges.resetItem(item, renderPayload);
+        }
+        addedPendingChanges = !_modelMeshRenderItems.empty();
     }
-    return false;
+    _addedToScene = addedPendingChanges;
+    if (addedPendingChanges) {
+        scene->enqueuePendingChanges(pendingChanges);
+        // NOTE: updateRender items enqueues identical pendingChanges (using a lambda)
+        // so it looks like we're doing double work here, but I don't want to remove the call
+        // for fear there is some sideeffect we'll miss. -- Andrew 2016.07.21
+        // TODO: figure out if we really need this call to updateRenderItems() or not.
+        updateRenderItems();
+    }
+
+    return true;
 }
 
 class CollisionRenderGeometry : public Geometry {

libraries/render-utils/src/Model.h

@@ -81,24 +81,25 @@ public:
     // new Scene/Engine rendering support
     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();
     }
     bool needsReload() const { return _needsReload; }
     bool initWhenReady(render::ScenePointer scene);
     bool addToScene(std::shared_ptr<render::Scene> scene,
-                    render::PendingChanges& pendingChanges,
+                    render::PendingChanges& pendingChanges) {
-                    bool showCollisionHull = false) {
         auto getters = render::Item::Status::Getters(0);
-        return addToScene(scene, pendingChanges, getters, showCollisionHull);
+        return addToScene(scene, pendingChanges, getters);
     }
     bool addToScene(std::shared_ptr<render::Scene> scene,
                     render::PendingChanges& pendingChanges,
-                    render::Item::Status::Getters& statusGetters,
+                    render::Item::Status::Getters& statusGetters);
-                    bool showCollisionHull = false);
     void removeFromScene(std::shared_ptr<render::Scene> scene, render::PendingChanges& pendingChanges);
     void renderSetup(RenderArgs* args);
-    bool isRenderable() const { return !_meshStates.isEmpty() || (isActive() && _renderGeometry->getMeshes().empty()); }
+    bool isRenderable() const;
 
     bool isVisible() const { return _isVisible; }
 
@@ -239,6 +240,7 @@ public:
 
     // returns 'true' if needs fullUpdate after geometry change
     bool updateGeometry();
+    void setCollisionMesh(model::MeshPointer mesh);
 
     void setLoadingPriority(float priority) { _loadingPriority = priority; }
 
@@ -249,9 +251,9 @@ public slots:
 signals:
     void setURLFinished(bool success);
     void setCollisionModelURLFinished(bool success);
-    void setCollisionMesh(model::MeshPointer mesh);
 
 protected:
+    bool addedToScene() const { return _addedToScene; }
 
     void setPupilDilation(float dilation) { _pupilDilation = dilation; }
     float getPupilDilation() const { return _pupilDilation; }
@@ -377,10 +379,11 @@ protected:
 
     void recalculateMeshBoxes(bool pickAgainstTriangles = false);
 
-    void createRenderItems(); // used to calculate our list of translucent vs opaque meshes
+    void createRenderItemSet();
+    void createVisibleRenderItemSet();
+    void createCollisionRenderItemSet();
     static model::MaterialPointer _collisionHullMaterial;
 
-    bool _meshGroupsKnown;
     bool _isWireframe;
 
 
@@ -397,7 +400,8 @@ protected:
     QSet<std::shared_ptr<ModelMeshPartPayload>> _modelMeshRenderItemsSet;
     QMap<render::ItemID, render::PayloadPointer> _modelMeshRenderItems;
 
-    bool _readyWhenAdded { false };
+    bool _addedToScene { false }; // has been added to scene
+    bool _needsFixupInScene { true }; // needs to be removed/re-added to scene
     bool _needsReload { true };
     bool _needsUpdateClusterMatrices { true };
     bool _showCollisionHull { false };