From 814de72bc2ea2cbfb055f131b6e8f0a8d77dcbfb Mon Sep 17 00:00:00 2001 From: Emil Axelsson Date: Fri, 15 Dec 2017 18:51:22 +0100 Subject: [PATCH] Fix bugs in asset management --- .../properties/numericalproperty.inl | 4 +- include/openspace/scene/asset.h | 4 +- src/engine/openspaceengine.cpp | 1 + src/scene/asset.cpp | 88 ++++++++----------- src/scene/assetloader.cpp | 7 +- src/scene/assetmanager.cpp | 1 + 6 files changed, 46 insertions(+), 59 deletions(-) diff --git a/include/openspace/properties/numericalproperty.inl b/include/openspace/properties/numericalproperty.inl index 671e2e7fd2..88104ab3cf 100644 --- a/include/openspace/properties/numericalproperty.inl +++ b/include/openspace/properties/numericalproperty.inl @@ -367,12 +367,12 @@ void NumericalProperty::setMaxValue(T value) { template T NumericalProperty::steppingValue() const { - return _steppingValue; + return _stepping; } template void NumericalProperty::setSteppingValue(T value) { - _steppingValue = std::move(value); + _stepping = std::move(value); } template diff --git a/include/openspace/scene/asset.h b/include/openspace/scene/asset.h index 63b5ddf3f1..503e003afd 100644 --- a/include/openspace/scene/asset.h +++ b/include/openspace/scene/asset.h @@ -136,8 +136,8 @@ public: private: void setState(State state); - void requiredAssetChangedState(std::shared_ptr a); - void requestedAssetChangedState(std::shared_ptr a); + void requiredAssetChangedState(std::shared_ptr a, Asset::State state); + void requestedAssetChangedState(std::shared_ptr a, Asset::State state); bool isSyncResolveReady(); diff --git a/src/engine/openspaceengine.cpp b/src/engine/openspaceengine.cpp index a7babb4b51..fbc731ab93 100644 --- a/src/engine/openspaceengine.cpp +++ b/src/engine/openspaceengine.cpp @@ -438,6 +438,7 @@ void OpenSpaceEngine::destroy() { _engine->parallelConnection().signalDisconnect(); } + _engine->assetManager().deinitialize(); _engine->_scene = nullptr; _engine->_syncEngine->removeSyncables(_engine->timeManager().getSyncables()); diff --git a/src/scene/asset.cpp b/src/scene/asset.cpp index 0f725ff275..439f0c0356 100644 --- a/src/scene/asset.cpp +++ b/src/scene/asset.cpp @@ -109,22 +109,20 @@ void Asset::setState(Asset::State state) { for (auto& requiringAsset : _requiringAssets) { if (std::shared_ptr a = requiringAsset.lock()) { - a->requiredAssetChangedState(thisAsset); + a->requiredAssetChangedState(thisAsset, state); } } for (auto& requestingAsset : _requestingAssets) { if (std::shared_ptr a = requestingAsset.lock()) { - a->requestedAssetChangedState(thisAsset); + a->requestedAssetChangedState(thisAsset, state); } } } -void Asset::requiredAssetChangedState(std::shared_ptr child) { +void Asset::requiredAssetChangedState(std::shared_ptr child, Asset::State childState) { ghoul_assert(!isInitialized(), "Required asset changing state while parent asset is initialized"); - - State childState = child->state(); if (childState == State::SyncResolved) { if (isSyncResolveReady()) { setState(State::SyncResolved); @@ -134,8 +132,7 @@ void Asset::requiredAssetChangedState(std::shared_ptr child) { } } -void Asset::requestedAssetChangedState(std::shared_ptr child) { - State childState = child->state(); +void Asset::requestedAssetChangedState(std::shared_ptr child, Asset::State childState) { if (child->hasInitializedParent()) { if (childState == State::Loaded) { child->startSynchronizations(); @@ -163,9 +160,6 @@ void Asset::addSynchronization(std::shared_ptr synchron void Asset::syncStateChanged(std::shared_ptr synchronization, ResourceSynchronization::State state) { - ghoul_assert(!isInitialized(), - "Synchronization changing state while asset is initialized"); - if (state == ResourceSynchronization::State::Resolved) { if (!isSynchronized() && isSyncResolveReady()) { setState(State::SyncResolved); @@ -178,17 +172,16 @@ void Asset::syncStateChanged(std::shared_ptr synchroniz bool Asset::isSyncResolveReady() { std::vector> requiredAssets = this->requiredAssets(); - auto pendingRequiredAsset = std::find_if( + auto unsynchronizedAsset = std::find_if( requiredAssets.begin(), requiredAssets.end(), [](std::shared_ptr& a) { - return a->state() == Asset::State::Loaded || - a->state() == Asset::State::Synchronizing; + return !a->isSynchronized(); } ); - if (pendingRequiredAsset != requiredAssets.end()) { - // Not considered resolved if all children are not resolved + if (unsynchronizedAsset != requiredAssets.end()) { + // Not considered resolved if there is one or more unresolved children return false; } @@ -333,9 +326,13 @@ bool Asset::isInitialized() const { } bool Asset::startSynchronizations() { - // Do not attempt to resync if this is already initialized - if (state() == State::Initialized) { - return false; + for (auto& child : requestedAssets()) { + child->startSynchronizations(); + } + + // Do not attempt to resync if this is already done + if (isSyncingOrResolved()) { + return state() != State::SyncResolved; } setState(State::Synchronizing); @@ -348,9 +345,6 @@ bool Asset::startSynchronizations() { foundUnresolved = true; } } - for (auto& child : requestedAssets()) { - child->startSynchronizations(); - } // Now synchronize its own synchronizations. for (const auto& s : ownSynchronizations()) { @@ -360,7 +354,7 @@ bool Asset::startSynchronizations() { } } // If all syncs are resolved (or no syncs exist), mark as resolved. - if (!foundUnresolved) { + if (!isInitialized() && !foundUnresolved) { setState(State::SyncResolved); } return foundUnresolved; @@ -419,7 +413,6 @@ bool Asset::restartAllSynchronizations() { float Asset::requiredSynchronizationProgress() { std::vector> assets = requiredSubTreeAssets(); return syncProgress(assets); - } float Asset::requestedSynchronizationProgress() { @@ -428,13 +421,11 @@ float Asset::requestedSynchronizationProgress() { } void Asset::load() { - LDEBUG("Loading asset " << id()); - if (isLoaded()) { return; } - bool loaded = loader()->loadAsset(shared_from_this()); + bool loaded = loader()->loadAsset(shared_from_this()); setState(loaded ? State::Loaded : State::LoadingFailed); } @@ -446,8 +437,6 @@ void Asset::unloadIfUnwanted() { } void Asset::unload() { - LDEBUG("Unloading asset " << id()); - if (!isLoaded()) { return; } @@ -653,20 +642,28 @@ void Asset::require(std::shared_ptr child) { return; } _requiredAssets.push_back(child); - - child->_requiringAssets.push_back(shared_from_this()); if (!child->isLoaded()) { child->load(); } - - if (isSynchronized() && child->isLoaded() && !child->isSynchronized()) { - child->startSynchronizations(); + if (!child->isLoaded()) { + unrequire(child); } - if (isInitialized() && child->isSynchronized() && !child->isInitialized()) { - child->initialize(); + if (isSynchronized()) { + if (child->isLoaded() && !child->isSynchronized()) { + child->startSynchronizations(); + } + } + + if (isInitialized()) { + if (child->isSynchronized() && !child->isInitialized()) { + child->initialize(); + } + if (!child->isInitialized()) { + unrequire(child); + } } } @@ -717,29 +714,18 @@ void Asset::request(std::shared_ptr child) { return; } _requestedAssets.push_back(child); - child->_requestingAssets.push_back(shared_from_this()); if (!child->isLoaded()) { child->load(); } - if (!child->isLoaded()) { - unrequest(child); + + if (isSynchronized() && child->isLoaded() && !child->isSynchronized()) { + child->startSynchronizations(); } - if (isSynchronized()) { - if (child->isLoaded() && !child->isSynchronized()) { - child->startSynchronizations(); - } - } - - if (isInitialized()) { - if (child->isSynchronized() && !child->isInitialized()) { - child->initialize(); - } - if (!child->isInitialized()) { - unrequest(child); - } + if (isInitialized() && child->isSynchronized() && !child->isInitialized()) { + child->initialize(); } } diff --git a/src/scene/assetloader.cpp b/src/scene/assetloader.cpp index a6cd04ed6c..568fa8d5df 100644 --- a/src/scene/assetloader.cpp +++ b/src/scene/assetloader.cpp @@ -94,9 +94,11 @@ AssetLoader::~AssetLoader() { void AssetLoader::trackAsset(std::shared_ptr asset) { _trackedAssets.emplace(asset->id(), asset); + setUpAssetLuaTable(asset.get()); } void AssetLoader::untrackAsset(Asset* asset) { + tearDownAssetLuaTable(asset); auto it = _trackedAssets.find(asset->id()); if (it != _trackedAssets.end()) { _trackedAssets.erase(it); @@ -210,7 +212,7 @@ void AssetLoader::tearDownAssetLuaTable(Asset* asset) { bool AssetLoader::loadAsset(std::shared_ptr asset) { std::shared_ptr parentAsset = _currentAsset; - setUpAssetLuaTable(asset.get()); + setCurrentAsset(asset); ghoul::OnScopeExit e([this, parentAsset] { setCurrentAsset(parentAsset); @@ -218,7 +220,6 @@ bool AssetLoader::loadAsset(std::shared_ptr asset) { if (!FileSys.fileExists(asset->assetFilePath())) { LERROR("Could not load asset '" << asset->assetFilePath() << "': File does not exist."); - tearDownAssetLuaTable(asset.get()); return false; } @@ -226,7 +227,6 @@ bool AssetLoader::loadAsset(std::shared_ptr asset) { ghoul::lua::runScriptFile(*_luaState, asset->assetFilePath()); } catch (const ghoul::lua::LuaRuntimeException& e) { LERROR("Could not load asset '" << asset->assetFilePath() << "': " << e.message); - tearDownAssetLuaTable(asset.get()); return false; } @@ -234,7 +234,6 @@ bool AssetLoader::loadAsset(std::shared_ptr asset) { } void AssetLoader::unloadAsset(std::shared_ptr asset) { - tearDownAssetLuaTable(asset.get()); for (int ref : _onInitializationFunctionRefs[asset.get()]) { luaL_unref(*_luaState, LUA_REGISTRYINDEX, ref); } diff --git a/src/scene/assetmanager.cpp b/src/scene/assetmanager.cpp index 1022ac1790..e2b7bb36cc 100644 --- a/src/scene/assetmanager.cpp +++ b/src/scene/assetmanager.cpp @@ -54,6 +54,7 @@ void AssetManager::initialize() { void AssetManager::deinitialize() { _assetLoader->rootAsset()->deinitialize(); _assetLoader->removeAssetListener(this); + _assetLoader = nullptr; } bool AssetManager::update() {