From 3de5f73a1faffa53dfb13f1526c54b1bde03dfa3 Mon Sep 17 00:00:00 2001 From: Zach Pomerantz Date: Sun, 27 Mar 2016 17:29:11 -0700 Subject: [PATCH 1/4] Fixup model on tex load --- .../src/RenderableModelEntityItem.cpp | 6 ++---- .../src/model-networking/ModelCache.cpp | 7 +++---- .../src/model-networking/ModelCache.h | 4 ---- libraries/render-utils/src/Model.cpp | 21 ++++++++++--------- libraries/render-utils/src/Model.h | 6 ++++-- 5 files changed, 20 insertions(+), 24 deletions(-) diff --git a/libraries/entities-renderer/src/RenderableModelEntityItem.cpp b/libraries/entities-renderer/src/RenderableModelEntityItem.cpp index e18f85211f..3909472f6c 100644 --- a/libraries/entities-renderer/src/RenderableModelEntityItem.cpp +++ b/libraries/entities-renderer/src/RenderableModelEntityItem.cpp @@ -133,10 +133,8 @@ void RenderableModelEntityItem::remapTextures() { return; // nothing to do if the model has not yet loaded } - auto& geometry = _model->getGeometry()->getGeometry(); - if (!_originalTexturesRead) { - _originalTextures = geometry->getTextures(); + _originalTextures = _model->getTextures(); _originalTexturesRead = true; // Default to _originalTextures to avoid remapping immediately and lagging on load @@ -152,7 +150,7 @@ void RenderableModelEntityItem::remapTextures() { auto newTextures = parseTexturesToMap(textures); if (newTextures != _currentTextures) { - geometry->setTextures(newTextures); + _model->setTextures(newTextures); _currentTextures = newTextures; } } diff --git a/libraries/model-networking/src/model-networking/ModelCache.cpp b/libraries/model-networking/src/model-networking/ModelCache.cpp index 2a6f33b964..88b05ef5f3 100644 --- a/libraries/model-networking/src/model-networking/ModelCache.cpp +++ b/libraries/model-networking/src/model-networking/ModelCache.cpp @@ -270,6 +270,9 @@ void Geometry::setTextures(const QVariantMap& textureMap) { material->setTextures(textureMap); _areTexturesLoaded = false; + + // If we only use cached textures, they should all be loaded + areTexturesLoaded(); } } } else { @@ -279,8 +282,6 @@ void Geometry::setTextures(const QVariantMap& textureMap) { bool Geometry::areTexturesLoaded() const { if (!_areTexturesLoaded) { - _hasTransparentTextures = false; - for (auto& material : _materials) { // Check if material textures are loaded if (std::any_of(material->_textures.cbegin(), material->_textures.cend(), @@ -293,8 +294,6 @@ bool Geometry::areTexturesLoaded() const { const auto albedoTexture = material->_textures[NetworkMaterial::MapChannel::ALBEDO_MAP]; if (albedoTexture.texture && albedoTexture.texture->getGPUTexture()) { material->resetOpacityMap(); - - _hasTransparentTextures |= material->getKey().isTranslucent(); } } diff --git a/libraries/model-networking/src/model-networking/ModelCache.h b/libraries/model-networking/src/model-networking/ModelCache.h index dad7883a6a..5598f15a62 100644 --- a/libraries/model-networking/src/model-networking/ModelCache.h +++ b/libraries/model-networking/src/model-networking/ModelCache.h @@ -74,9 +74,6 @@ public: void setTextures(const QVariantMap& textureMap); virtual bool areTexturesLoaded() const; - // Returns true if any albedo texture has a non-masking alpha channel. - // This can only be known after areTexturesLoaded(). - bool hasTransparentTextures() const { return _hasTransparentTextures; } protected: friend class GeometryMappingResource; @@ -91,7 +88,6 @@ protected: private: mutable bool _areTexturesLoaded { false }; - mutable bool _hasTransparentTextures { false }; }; /// A geometry loaded from the network. diff --git a/libraries/render-utils/src/Model.cpp b/libraries/render-utils/src/Model.cpp index 0bd2687169..39ffc9dd9a 100644 --- a/libraries/render-utils/src/Model.cpp +++ b/libraries/render-utils/src/Model.cpp @@ -76,14 +76,9 @@ AbstractViewStateInterface* Model::_viewState = NULL; bool Model::needsFixupInScene() const { if (readyToAddToScene()) { - // Once textures are loaded, fixup if they are now transparent - if (_needsUpdateTransparentTextures && _geometry->getGeometry()->areTexturesLoaded()) { - _needsUpdateTransparentTextures = false; - bool hasTransparentTextures = _geometry->getGeometry()->hasTransparentTextures(); - if (_hasTransparentTextures != hasTransparentTextures) { - _hasTransparentTextures = hasTransparentTextures; - return true; - } + if (_needsUpdateTextures && _geometry->getGeometry()->areTexturesLoaded()) { + _needsUpdateTextures = false; + return true; } if (!_readyWhenAdded) { return true; @@ -791,6 +786,13 @@ int Model::getLastFreeJointIndex(int jointIndex) const { return (isActive() && jointIndex != -1) ? getFBXGeometry().joints.at(jointIndex).freeLineage.last() : -1; } +void Model::setTextures(const QVariantMap& textures) { + if (isLoaded()) { + _needsUpdateTextures = true; + _geometry->getGeometry()->setTextures(textures); + } +} + void Model::setURL(const QUrl& url) { // don't recreate the geometry if it's the same URL if (_url == url && _geometry && _geometry->getURL() == url) { @@ -807,8 +809,7 @@ void Model::setURL(const QUrl& url) { } _needsReload = true; - _needsUpdateTransparentTextures = true; - _hasTransparentTextures = false; + _needsUpdateTextures = true; _meshGroupsKnown = false; invalidCalculatedMeshBoxes(); deleteGeometry(); diff --git a/libraries/render-utils/src/Model.h b/libraries/render-utils/src/Model.h index e4a8fa3b36..03760378f0 100644 --- a/libraries/render-utils/src/Model.h +++ b/libraries/render-utils/src/Model.h @@ -129,6 +129,9 @@ public: /// Returns a reference to the shared collision geometry. const NetworkGeometry::Pointer& getCollisionGeometry() const { return _collisionGeometry; } + const QVariantMap getTextures() const { assert(isLoaded()); return _geometry->getGeometry()->getTextures(); } + void setTextures(const QVariantMap& textures); + /// Provided as a convenience, will crash if !isLoaded() // And so that getGeometry() isn't chained everywhere const FBXGeometry& getFBXGeometry() const { assert(isLoaded()); return getGeometry()->getGeometry()->getGeometry(); } @@ -385,9 +388,8 @@ protected: bool _readyWhenAdded { false }; bool _needsReload { true }; bool _needsUpdateClusterMatrices { true }; - mutable bool _needsUpdateTransparentTextures { true }; - mutable bool _hasTransparentTextures { false }; bool _showCollisionHull { false }; + mutable bool _needsUpdateTextures { true }; friend class ModelMeshPartPayload; RigPointer _rig; From 984bacdae3527a8e8bb5f3efe5cba15612aa6af8 Mon Sep 17 00:00:00 2001 From: Zach Pomerantz Date: Wed, 30 Mar 2016 18:09:02 -0700 Subject: [PATCH 2/4] Rearrange model entity render to reduce flicker --- .../src/RenderableModelEntityItem.cpp | 76 ++++++++++--------- 1 file changed, 39 insertions(+), 37 deletions(-) diff --git a/libraries/entities-renderer/src/RenderableModelEntityItem.cpp b/libraries/entities-renderer/src/RenderableModelEntityItem.cpp index 3909472f6c..5b8e4a644a 100644 --- a/libraries/entities-renderer/src/RenderableModelEntityItem.cpp +++ b/libraries/entities-renderer/src/RenderableModelEntityItem.cpp @@ -364,41 +364,16 @@ void RenderableModelEntityItem::render(RenderArgs* args) { assert(getType() == EntityTypes::Model); if (hasModel()) { - if (_model) { - render::ScenePointer scene = AbstractViewStateInterface::instance()->getMain3DScene(); - - // check to see if when we added our models to the scene they were ready, if they were not ready, then - // fix them up in the scene - bool shouldShowCollisionHull = (args->_debugFlags & (int)RenderArgs::RENDER_DEBUG_HULLS) > 0; - if (_model->needsFixupInScene() || _showCollisionHull != shouldShowCollisionHull) { - _showCollisionHull = shouldShowCollisionHull; - render::PendingChanges pendingChanges; - - _model->removeFromScene(scene, pendingChanges); - - render::Item::Status::Getters statusGetters; - makeEntityItemStatusGetters(getThisPointer(), statusGetters); - _model->addToScene(scene, pendingChanges, statusGetters, _showCollisionHull); - - scene->enqueuePendingChanges(pendingChanges); - } - - // FIXME: this seems like it could be optimized if we tracked our last known visible state in - // the renderable item. As it stands now the model checks it's visible/invisible state - // so most of the time we don't do anything in this function. - _model->setVisibleInScene(getVisible(), scene); - } - - - remapTextures(); + // Prepare the current frame { - // float alpha = getLocalRenderAlpha(); - if (!_model || _needsModelReload) { // TODO: this getModel() appears to be about 3% of model render time. We should optimize PerformanceTimer perfTimer("getModel"); EntityTreeRenderer* renderer = static_cast(args->_renderer); getModel(renderer); + + // Remap textures immediately after loading to avoid flicker + remapTextures(); } if (_model) { @@ -429,15 +404,42 @@ void RenderableModelEntityItem::render(RenderArgs* args) { } }); updateModelBounds(); + } + } - // Check if the URL has changed - // Do this last as the getModel is queued for the next frame, - // and we need to keep state directing the model to reinitialize - auto& currentURL = getParsedModelURL(); - if (currentURL != _model->getURL()) { - // Defer setting the url to the render thread - getModel(_myRenderer); - } + // Enqueue updates for the next frame + if (_model) { + + render::ScenePointer scene = AbstractViewStateInterface::instance()->getMain3DScene(); + + // FIXME: this seems like it could be optimized if we tracked our last known visible state in + // the renderable item. As it stands now the model checks it's visible/invisible state + // so most of the time we don't do anything in this function. + _model->setVisibleInScene(getVisible(), scene); + + // 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 + // fix them up in the scene + bool shouldShowCollisionHull = (args->_debugFlags & (int)RenderArgs::RENDER_DEBUG_HULLS) > 0; + if (_model->needsFixupInScene() || _showCollisionHull != shouldShowCollisionHull) { + _showCollisionHull = shouldShowCollisionHull; + render::PendingChanges pendingChanges; + + _model->removeFromScene(scene, pendingChanges); + + render::Item::Status::Getters statusGetters; + makeEntityItemStatusGetters(getThisPointer(), statusGetters); + _model->addToScene(scene, pendingChanges, statusGetters, _showCollisionHull); + + scene->enqueuePendingChanges(pendingChanges); + } + + auto& currentURL = getParsedModelURL(); + if (currentURL != _model->getURL()) { + // Defer setting the url to the render thread + getModel(_myRenderer); } } } else { From 377a20e98304832b98657b2b923119acf2e2c0b8 Mon Sep 17 00:00:00 2001 From: Zach Pomerantz Date: Wed, 30 Mar 2016 18:54:12 -0700 Subject: [PATCH 3/4] Use default texs for empty json --- .../entities-renderer/src/RenderableModelEntityItem.cpp | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/libraries/entities-renderer/src/RenderableModelEntityItem.cpp b/libraries/entities-renderer/src/RenderableModelEntityItem.cpp index 5b8e4a644a..b510f58b12 100644 --- a/libraries/entities-renderer/src/RenderableModelEntityItem.cpp +++ b/libraries/entities-renderer/src/RenderableModelEntityItem.cpp @@ -116,11 +116,18 @@ QVariantMap RenderableModelEntityItem::parseTexturesToMap(QString textures) { QJsonParseError error; QJsonDocument texturesJson = QJsonDocument::fromJson(textures.toUtf8(), &error); + // If textures are invalid, revert to original textures if (error.error != QJsonParseError::NoError) { qCWarning(entitiesrenderer) << "Could not evaluate textures property value:" << textures; return _originalTextures; } + QVariantMap texturesMap = texturesJson.toVariant().toMap(); + // If textures are unset, revert to original textures + if (texturesMap.isEmpty()) { + return _originalTextures; + } + return texturesJson.toVariant().toMap(); } From e4cba1433393ca903ac5db5b511935a9a70a713d Mon Sep 17 00:00:00 2001 From: Zach Pomerantz Date: Wed, 30 Mar 2016 18:54:25 -0700 Subject: [PATCH 4/4] Avoid recreating model items when adding to scene --- .../src/RenderableModelEntityItem.cpp | 2 - libraries/render-utils/src/Model.cpp | 97 ++++++++----------- libraries/render-utils/src/Model.h | 5 +- 3 files changed, 44 insertions(+), 60 deletions(-) diff --git a/libraries/entities-renderer/src/RenderableModelEntityItem.cpp b/libraries/entities-renderer/src/RenderableModelEntityItem.cpp index b510f58b12..a9dcb3883c 100644 --- a/libraries/entities-renderer/src/RenderableModelEntityItem.cpp +++ b/libraries/entities-renderer/src/RenderableModelEntityItem.cpp @@ -434,8 +434,6 @@ void RenderableModelEntityItem::render(RenderArgs* args) { _showCollisionHull = shouldShowCollisionHull; render::PendingChanges pendingChanges; - _model->removeFromScene(scene, pendingChanges); - render::Item::Status::Getters statusGetters; makeEntityItemStatusGetters(getThisPointer(), statusGetters); _model->addToScene(scene, pendingChanges, statusGetters, _showCollisionHull); diff --git a/libraries/render-utils/src/Model.cpp b/libraries/render-utils/src/Model.cpp index 39ffc9dd9a..3a5928f3d1 100644 --- a/libraries/render-utils/src/Model.cpp +++ b/libraries/render-utils/src/Model.cpp @@ -541,43 +541,6 @@ void Model::setVisibleInScene(bool newValue, std::shared_ptr scen } } - -bool Model::addToScene(std::shared_ptr scene, render::PendingChanges& pendingChanges, bool showCollisionHull) { - - if ((!_meshGroupsKnown || showCollisionHull != _showCollisionHull) && isLoaded()) { - _showCollisionHull = showCollisionHull; - segregateMeshGroups(); - } - - bool somethingAdded = false; - - foreach (auto renderItem, _modelMeshRenderItemsSet) { - auto item = scene->allocateID(); - auto renderPayload = std::make_shared(renderItem); - pendingChanges.resetItem(item, renderPayload); - pendingChanges.updateItem(item, [](ModelMeshPartPayload& data) { - data.notifyLocationChanged(); - }); - _modelMeshRenderItems.insert(item, renderPayload); - somethingAdded = true; - } - - foreach (auto renderItem, _collisionRenderItemsSet) { - auto item = scene->allocateID(); - auto renderPayload = std::make_shared(renderItem); - pendingChanges.resetItem(item, renderPayload); - pendingChanges.updateItem(item, [](MeshPartPayload& data) { - data.notifyLocationChanged(); - }); - _collisionRenderItems.insert(item, renderPayload); - somethingAdded = true; - } - - _readyWhenAdded = readyToAddToScene(); - - return somethingAdded; -} - bool Model::addToScene(std::shared_ptr scene, render::PendingChanges& pendingChanges, render::Item::Status::Getters& statusGetters, @@ -589,28 +552,48 @@ bool Model::addToScene(std::shared_ptr scene, bool somethingAdded = false; - foreach (auto renderItem, _modelMeshRenderItemsSet) { - auto item = scene->allocateID(); - auto renderPayload = std::make_shared(renderItem); - renderPayload->addStatusGetters(statusGetters); - pendingChanges.resetItem(item, renderPayload); - pendingChanges.updateItem(item, [](ModelMeshPartPayload& data) { - data.notifyLocationChanged(); - }); - _modelMeshRenderItems.insert(item, renderPayload); - somethingAdded = true; + if (_modelMeshRenderItems.size()) { + for (auto item : _modelMeshRenderItems.keys()) { + pendingChanges.updateItem(item, [](ModelMeshPartPayload& data) { + data.notifyLocationChanged(); + }); + } + } else { + for (auto renderItem : _modelMeshRenderItemsSet) { + auto item = scene->allocateID(); + auto renderPayload = std::make_shared(renderItem); + if (statusGetters.size()) { + renderPayload->addStatusGetters(statusGetters); + } + pendingChanges.resetItem(item, renderPayload); + pendingChanges.updateItem(item, [](ModelMeshPartPayload& data) { + data.notifyLocationChanged(); + }); + _modelMeshRenderItems.insert(item, renderPayload); + somethingAdded = true; + } } - foreach (auto renderItem, _collisionRenderItemsSet) { - auto item = scene->allocateID(); - auto renderPayload = std::make_shared(renderItem); - renderPayload->addStatusGetters(statusGetters); - pendingChanges.resetItem(item, renderPayload); - pendingChanges.updateItem(item, [](MeshPartPayload& data) { - data.notifyLocationChanged(); - }); - _collisionRenderItems.insert(item, renderPayload); - somethingAdded = true; + if (_collisionRenderItems.size()) { + for (auto item : _collisionRenderItems.keys()) { + pendingChanges.updateItem(item, [](MeshPartPayload& data) { + data.notifyLocationChanged(); + }); + } + } else { + for (auto renderItem : _collisionRenderItemsSet) { + auto item = scene->allocateID(); + auto renderPayload = std::make_shared(renderItem); + if (statusGetters.size()) { + renderPayload->addStatusGetters(statusGetters); + } + pendingChanges.resetItem(item, renderPayload); + pendingChanges.updateItem(item, [](MeshPartPayload& data) { + data.notifyLocationChanged(); + }); + _collisionRenderItems.insert(item, renderPayload); + somethingAdded = true; + } } _readyWhenAdded = readyToAddToScene(); diff --git a/libraries/render-utils/src/Model.h b/libraries/render-utils/src/Model.h index 03760378f0..239d0e7c28 100644 --- a/libraries/render-utils/src/Model.h +++ b/libraries/render-utils/src/Model.h @@ -87,7 +87,10 @@ public: bool initWhenReady(render::ScenePointer scene); bool addToScene(std::shared_ptr scene, render::PendingChanges& pendingChanges, - bool showCollisionHull = false); + bool showCollisionHull = false) { + auto getters = render::Item::Status::Getters(0); + return addToScene(scene, pendingChanges, getters, showCollisionHull); + } bool addToScene(std::shared_ptr scene, render::PendingChanges& pendingChanges, render::Item::Status::Getters& statusGetters,