diff --git a/interface/src/entities/EntityTreeRenderer.cpp b/interface/src/entities/EntityTreeRenderer.cpp index c1a3d2bf82..57905072e6 100644 --- a/interface/src/entities/EntityTreeRenderer.cpp +++ b/interface/src/entities/EntityTreeRenderer.cpp @@ -67,6 +67,7 @@ void EntityTreeRenderer::update() { void EntityTreeRenderer::render(RenderMode renderMode) { OctreeRenderer::render(renderMode); + deleteReleasedModels(); // seems like as good as any other place to do some memory cleanup } const FBXGeometry* EntityTreeRenderer::getGeometryForEntity(const EntityItem* entityItem) { @@ -76,7 +77,7 @@ const FBXGeometry* EntityTreeRenderer::getGeometryForEntity(const EntityItem* en const RenderableModelEntityItem* constModelEntityItem = dynamic_cast(entityItem); RenderableModelEntityItem* modelEntityItem = const_cast(constModelEntityItem); assert(modelEntityItem); // we need this!!! - Model* model = modelEntityItem->getModel(); + Model* model = modelEntityItem->getModel(this); if (model) { result = &model->getGeometry()->getFBXGeometry(); } @@ -91,7 +92,7 @@ const Model* EntityTreeRenderer::getModelForEntityItem(const EntityItem* entityI RenderableModelEntityItem* modelEntityItem = const_cast(constModelEntityItem); assert(modelEntityItem); // we need this!!! - result = modelEntityItem->getModel(); + result = modelEntityItem->getModel(this); } return result; } @@ -225,7 +226,71 @@ void EntityTreeRenderer::processEraseMessage(const QByteArray& dataByteArray, co static_cast(_tree)->processEraseMessage(dataByteArray, sourceNode); } - - +Model* EntityTreeRenderer::allocateModel(const QString& url) { + Model* model = NULL; + // Make sure we only create and delete models on the thread that owns the EntityTreeRenderer + if (QThread::currentThread() != thread()) { + QMetaObject::invokeMethod(this, "allocateModel", Qt::BlockingQueuedConnection, + Q_RETURN_ARG(Model*, model), + Q_ARG(const QString&, url)); + + return model; + } + model = new Model(); + model->init(); + model->setURL(QUrl(url)); + return model; +} + +Model* EntityTreeRenderer::updateModel(Model* original, const QString& newUrl) { + Model* model = NULL; + + // The caller shouldn't call us if the URL doesn't need to change. But if they + // do, we just return their original back to them. + if (!original || (QUrl(newUrl) == original->getURL())) { + return original; + } + + // Before we do any creating or deleting, make sure we're on our renderer thread + if (QThread::currentThread() != thread()) { + QMetaObject::invokeMethod(this, "updateModel", Qt::BlockingQueuedConnection, + Q_RETURN_ARG(Model*, model), + Q_ARG(Model*, original), + Q_ARG(const QString&, newUrl)); + + return model; + } + + // at this point we know we need to replace the model, and we know we're on the + // correct thread, so we can do all our work. + if (original) { + delete original; // delete the old model... + } + + // create the model and correctly initialize it with the new url + model = new Model(); + model->init(); + model->setURL(QUrl(newUrl)); + + return model; +} + +void EntityTreeRenderer::releaseModel(Model* model) { + // If we're not on the renderer's thread, then remember this model to be deleted later + if (QThread::currentThread() != thread()) { + _releasedModels << model; + } else { // otherwise just delete it right away + delete model; + } +} + +void EntityTreeRenderer::deleteReleasedModels() { + if (_releasedModels.size() > 0) { + foreach(Model* model, _releasedModels) { + delete model; + } + _releasedModels.clear(); + } +} diff --git a/interface/src/entities/EntityTreeRenderer.h b/interface/src/entities/EntityTreeRenderer.h index 2ec9cf2f10..817212fa67 100644 --- a/interface/src/entities/EntityTreeRenderer.h +++ b/interface/src/entities/EntityTreeRenderer.h @@ -63,6 +63,19 @@ public: void renderEntityTypeModel(EntityItem* entity, RenderArgs* args); static QThread* getMainThread(); + + /// if a renderable entity item needs a model, we will allocate it for them + Q_INVOKABLE Model* allocateModel(const QString& url); + + /// if a renderable entity item needs to update the URL of a model, we will handle that for the entity + Q_INVOKABLE Model* updateModel(Model* original, const QString& newUrl); + + /// if a renderable entity item is done with a model, it should return it to us + void releaseModel(Model* model); + + void deleteReleasedModels(); +private: + QList _releasedModels; }; #endif // hifi_EntityTreeRenderer_h diff --git a/interface/src/entities/RenderableModelEntityItem.cpp b/interface/src/entities/RenderableModelEntityItem.cpp index 87bc8eeafc..8c176fb7eb 100644 --- a/interface/src/entities/RenderableModelEntityItem.cpp +++ b/interface/src/entities/RenderableModelEntityItem.cpp @@ -29,10 +29,11 @@ EntityItem* RenderableModelEntityItem::factory(const EntityItemID& entityID, con } RenderableModelEntityItem::~RenderableModelEntityItem() { - - // TODO: we can't just delete this because we may be on the wrong thread. - //delete _model; - _model = NULL; + assert(_myRenderer || !_model); // if we have a model, we need to know our renderer + if (_myRenderer && _model) { + _myRenderer->releaseModel(_model); + _model = NULL; + } }; bool RenderableModelEntityItem::setProperties(const EntityItemProperties& properties, bool forceCopy) { @@ -75,7 +76,8 @@ void RenderableModelEntityItem::render(RenderArgs* args) { if (!_model || _needsModelReload) { // TODO: this getModel() appears to be about 3% of model render time. We should optimize PerformanceTimer perfTimer("getModel"); - getModel(); + EntityTreeRenderer* renderer = static_cast(args->_renderer); + getModel(renderer); } if (_model) { @@ -191,47 +193,40 @@ void RenderableModelEntityItem::render(RenderArgs* args) { } } -Model* RenderableModelEntityItem::getModel() { +Model* RenderableModelEntityItem::getModel(EntityTreeRenderer* renderer) { + Model* result = NULL; + + // make sure our renderer is setup + if (!_myRenderer) { + _myRenderer = renderer; + } + assert(_myRenderer == renderer); // you should only ever render on one renderer + _needsModelReload = false; // this is the reload + + // if we have a URL, then we will want to end up returning a model... if (!getModelURL().isEmpty()) { - // double check our URLS match... + + // if we have a previously allocated model, but it's URL doesn't match + // then we need to let our renderer update our model for us. if (_model && QUrl(getModelURL()) != _model->getURL()) { - delete _model; // delete the old model... - _model = NULL; + result = _model = _myRenderer->updateModel(_model, getModelURL()); _needsSimulation = true; - } - - // if we don't have a model... but our item does have a model URL - if (!_model) { - // Make sure we only create new models on the thread that owns the EntityTreeRenderer - if (QThread::currentThread() != EntityTreeRenderer::getMainThread()) { - - // TODO: how do we better handle this??? we may need this for scripting... - // possible go back to a getModel() service on the TreeRenderer, but have it take a URL - // this would allow the entity items to use that service for loading, and since the - // EntityTreeRenderer is a Q_OBJECT it can call invokeMethod() - - qDebug() << "can't call getModel() on thread other than rendering thread..."; - - //QMetaObject::invokeMethod(this, "getModel", Qt::BlockingQueuedConnection, Q_RETURN_ARG(Model*, _model)); - //qDebug() << "got it... _model=" << _model; - return _model; - } - - _model = new Model(); - _model->init(); - _model->setURL(QUrl(getModelURL())); + } else if (!_model) { // if we don't yet have a model, then we want our renderer to allocate one + result = _model = _myRenderer->allocateModel(getModelURL()); _needsSimulation = true; + } else { // we already have the model we want... + result = _model; } - } else { - // our URL is empty, we should clean up our old model + } else { // if our desired URL is empty, we may need to delete our existing model if (_model) { - delete _model; // delete the old model... - _model = NULL; + _myRenderer->releaseModel(_model); + result = _model = NULL; _needsSimulation = true; } } - return _model; + + return result; } diff --git a/interface/src/entities/RenderableModelEntityItem.h b/interface/src/entities/RenderableModelEntityItem.h index 60714fec15..6125b4f6cd 100644 --- a/interface/src/entities/RenderableModelEntityItem.h +++ b/interface/src/entities/RenderableModelEntityItem.h @@ -36,7 +36,8 @@ public: ModelEntityItem(entityItemID, properties), _model(NULL), _needsSimulation(true), - _needsModelReload(true) { }; + _needsModelReload(true), + _myRenderer(NULL) { }; virtual ~RenderableModelEntityItem(); @@ -48,11 +49,12 @@ public: virtual void somethingChangedNotification() { _needsSimulation = true; } virtual void render(RenderArgs* args); - Model* getModel(); + Model* getModel(EntityTreeRenderer* renderer); private: Model* _model; bool _needsSimulation; bool _needsModelReload; + EntityTreeRenderer* _myRenderer; }; #endif // hifi_RenderableModelEntityItem_h