From 32b15f90ab2ea8c86dcf5ea06d0f48353ac6407c Mon Sep 17 00:00:00 2001 From: Wojtek Figat Date: Mon, 22 Apr 2024 18:10:58 +0200 Subject: [PATCH] Minor improvements --- Flax.sln.DotSettings | 1 + Source/Engine/Content/Asset.cpp | 3 +- Source/Engine/Content/Asset.h | 2 +- Source/Engine/Content/BinaryAsset.cpp | 6 +-- .../Engine/Content/Loading/ContentLoadTask.h | 41 ----------------- .../Content/Loading/ContentLoadingManager.cpp | 3 +- .../Content/Loading/Tasks/LoadAssetDataTask.h | 7 +-- .../Content/Loading/Tasks/LoadAssetTask.h | 46 +++++++++---------- Source/Engine/Threading/Task.h | 18 +++----- 9 files changed, 36 insertions(+), 91 deletions(-) diff --git a/Flax.sln.DotSettings b/Flax.sln.DotSettings index 6d72e44ed..96456601b 100644 --- a/Flax.sln.DotSettings +++ b/Flax.sln.DotSettings @@ -236,6 +236,7 @@ True True True + DisabledByUser True Blue True diff --git a/Source/Engine/Content/Asset.cpp b/Source/Engine/Content/Asset.cpp index 30771899f..ddff7fc8a 100644 --- a/Source/Engine/Content/Asset.cpp +++ b/Source/Engine/Content/Asset.cpp @@ -587,8 +587,8 @@ bool Asset::IsInternalType() const bool Asset::onLoad(LoadAssetTask* task) { - // It may fail when task is cancelled and new one is created later (don't crash but just end with an error) if (task->Asset.Get() != this || Platform::AtomicRead(&_loadingTask) == 0) + // It may fail when task is cancelled and new one was created later (don't crash but just end with an error) return true; Locker.Lock(); @@ -601,7 +601,6 @@ bool Asset::onLoad(LoadAssetTask* task) } const bool isLoaded = result == LoadResult::Ok; const bool failed = !isLoaded; - LoadState state = LoadState::Loaded; Platform::AtomicStore(&_loadState, (int64)(isLoaded ? LoadState::Loaded : LoadState::LoadFailed)); if (failed) { diff --git a/Source/Engine/Content/Asset.h b/Source/Engine/Content/Asset.h index c4b89935d..d1256b7c7 100644 --- a/Source/Engine/Content/Asset.h +++ b/Source/Engine/Content/Asset.h @@ -103,7 +103,7 @@ public: public: /// - /// Gets the path to the asset storage file. In Editor it reflects the actual file, in cooked Game, it fakes the Editor path to be informative for developers. + /// Gets the path to the asset storage file. In Editor, it reflects the actual file, in cooked Game, it fakes the Editor path to be informative for developers. /// API_PROPERTY() virtual const String& GetPath() const = 0; diff --git a/Source/Engine/Content/BinaryAsset.cpp b/Source/Engine/Content/BinaryAsset.cpp index 7735d8b28..692fc9329 100644 --- a/Source/Engine/Content/BinaryAsset.cpp +++ b/Source/Engine/Content/BinaryAsset.cpp @@ -507,8 +507,7 @@ public: /// /// The asset. InitAssetTask(BinaryAsset* asset) - : ContentLoadTask(Type::Custom) - , _asset(asset) + : _asset(asset) , _dataLock(asset->Storage->Lock()) { } @@ -527,8 +526,6 @@ protected: AssetReference ref = _asset.Get(); if (ref == nullptr) return Result::MissingReferences; - - // Prepare auto storage = ref->Storage; auto factory = (BinaryAssetFactoryBase*)Content::GetAssetFactory(ref->GetTypeName()); ASSERT(factory); @@ -548,7 +545,6 @@ protected: _dataLock.Release(); _asset = nullptr; - // Base ContentLoadTask::OnEnd(); } }; diff --git a/Source/Engine/Content/Loading/ContentLoadTask.h b/Source/Engine/Content/Loading/ContentLoadTask.h index d8d44b50a..2e2608612 100644 --- a/Source/Engine/Content/Loading/ContentLoadTask.h +++ b/Source/Engine/Content/Loading/ContentLoadTask.h @@ -15,52 +15,11 @@ class ContentLoadTask : public Task friend LoadingThread; public: - /// - /// Describes work type - /// - DECLARE_ENUM_3(Type, Custom, LoadAsset, LoadAssetData); - /// /// Describes work result value /// DECLARE_ENUM_5(Result, Ok, AssetLoadError, MissingReferences, LoadDataError, TaskFailed); -private: - /// - /// Task type - /// - Type _type; - -protected: - /// - /// Initializes a new instance of the class. - /// - /// The task type. - ContentLoadTask(const Type type) - : _type(type) - { - } - -public: - /// - /// Gets a task type. - /// - FORCE_INLINE Type GetType() const - { - return _type; - } - -public: - /// - /// Checks if async task is loading given asset resource - /// - /// Target asset to check - /// True if is loading that asset, otherwise false - bool IsLoading(Asset* asset) const - { - return _type == Type::LoadAsset && HasReference((Object*)asset); - } - protected: virtual Result run() = 0; diff --git a/Source/Engine/Content/Loading/ContentLoadingManager.cpp b/Source/Engine/Content/Loading/ContentLoadingManager.cpp index 68d06b3d2..d0efbd197 100644 --- a/Source/Engine/Content/Loading/ContentLoadingManager.cpp +++ b/Source/Engine/Content/Loading/ContentLoadingManager.cpp @@ -4,6 +4,7 @@ #include "ContentLoadTask.h" #include "Engine/Core/Log.h" #include "Engine/Core/Math/Math.h" +#include "Engine/Core/Collections/Array.h" #include "Engine/Platform/CPUInfo.h" #include "Engine/Platform/Thread.h" #include "Engine/Platform/ConditionVariable.h" @@ -212,7 +213,7 @@ void ContentLoadingManagerService::Dispose() String ContentLoadTask::ToString() const { - return String::Format(TEXT("Content Load Task {0} ({1})"), ToString(GetType()), (int32)GetState()); + return String::Format(TEXT("Content Load Task ({})"), (int32)GetState()); } void ContentLoadTask::Enqueue() diff --git a/Source/Engine/Content/Loading/Tasks/LoadAssetDataTask.h b/Source/Engine/Content/Loading/Tasks/LoadAssetDataTask.h index 60ebba6cd..3fbd34835 100644 --- a/Source/Engine/Content/Loading/Tasks/LoadAssetDataTask.h +++ b/Source/Engine/Content/Loading/Tasks/LoadAssetDataTask.h @@ -15,28 +15,24 @@ class LoadAssetDataTask : public ContentLoadTask { private: - WeakAssetReference _asset; // Don't keep ref to the asset (so it can be unloaded if none using it, task will fail then) AssetChunksFlag _chunks; FlaxStorage::LockData _dataLock; public: - /// /// Initializes a new instance of the class. /// /// The asset to load. /// The chunks to load. LoadAssetDataTask(BinaryAsset* asset, AssetChunksFlag chunks) - : ContentLoadTask(Type::LoadAssetData) - , _asset(asset) + : _asset(asset) , _chunks(chunks) , _dataLock(asset->Storage->Lock()) { } public: - // [ContentLoadTask] bool HasReference(Object* obj) const override { @@ -44,7 +40,6 @@ public: } protected: - // [ContentLoadTask] Result run() override { diff --git a/Source/Engine/Content/Loading/Tasks/LoadAssetTask.h b/Source/Engine/Content/Loading/Tasks/LoadAssetTask.h index f71f2b4a6..24bd66119 100644 --- a/Source/Engine/Content/Loading/Tasks/LoadAssetTask.h +++ b/Source/Engine/Content/Loading/Tasks/LoadAssetTask.h @@ -15,38 +15,35 @@ class LoadAssetTask : public ContentLoadTask { public: - /// /// Initializes a new instance of the class. /// /// The asset to load. LoadAssetTask(Asset* asset) - : ContentLoadTask(Type::LoadAsset) - , Asset(asset) + : Asset(asset) { } ~LoadAssetTask() { - if (Asset) + auto asset = Asset.Get(); + if (asset) { - Asset->Locker.Lock(); - if (Platform::AtomicRead(&Asset->_loadingTask) == (intptr)this) + asset->Locker.Lock(); + if (Platform::AtomicRead(&asset->_loadingTask) == (intptr)this) { - Platform::AtomicStore(&Asset->_loadState, (int64)Asset::LoadState::LoadFailed); - Platform::AtomicStore(&Asset->_loadingTask, 0); + Platform::AtomicStore(&asset->_loadState, (int64)Asset::LoadState::LoadFailed); + Platform::AtomicStore(&asset->_loadingTask, 0); LOG(Error, "Loading asset \'{0}\' result: {1}.", ToString(), ToString(Result::TaskFailed)); } - Asset->Locker.Unlock(); + asset->Locker.Unlock(); } } public: - WeakAssetReference Asset; public: - // [ContentLoadTask] bool HasReference(Object* obj) const override { @@ -54,7 +51,6 @@ public: } protected: - // [ContentLoadTask] Result run() override { @@ -68,32 +64,36 @@ protected: // Call loading if (ref->onLoad(this)) return Result::AssetLoadError; - return Result::Ok; } + void OnFail() override { - if (Asset) + auto asset = Asset.Get(); + if (asset) { - Asset->Locker.Lock(); - if (Platform::AtomicRead(&Asset->_loadingTask) == (intptr)this) - Platform::AtomicStore(&Asset->_loadingTask, 0); - Asset->Locker.Unlock(); Asset = nullptr; + asset->Locker.Lock(); + if (Platform::AtomicRead(&asset->_loadingTask) == (intptr)this) + Platform::AtomicStore(&asset->_loadingTask, 0); + asset->Locker.Unlock(); } // Base ContentLoadTask::OnFail(); } + void OnEnd() override { - if (Asset) + auto asset = Asset.Get(); + if (asset) { - Asset->Locker.Lock(); - if (Platform::AtomicRead(&Asset->_loadingTask) == (intptr)this) - Platform::AtomicStore(&Asset->_loadingTask, 0); - Asset->Locker.Unlock(); Asset = nullptr; + asset->Locker.Lock(); + if (Platform::AtomicRead(&asset->_loadingTask) == (intptr)this) + Platform::AtomicStore(&asset->_loadingTask, 0); + asset->Locker.Unlock(); + asset = nullptr; } // Base diff --git a/Source/Engine/Threading/Task.h b/Source/Engine/Threading/Task.h index d2d77a2e9..c741da73d 100644 --- a/Source/Engine/Threading/Task.h +++ b/Source/Engine/Threading/Task.h @@ -7,7 +7,6 @@ #include "Engine/Core/NonCopyable.h" #include "Engine/Core/Enums.h" #include "Engine/Core/Types/TimeSpan.h" -#include "Engine/Core/Collections/Array.h" #include "Engine/Platform/Platform.h" /// @@ -188,8 +187,8 @@ public: /// The tasks list to wait for. /// The maximum amount of milliseconds to wait for the task to finish it's job. Timeout smaller/equal 0 will result in infinite waiting. /// True if any task failed or has been canceled or has timeout, otherwise false. - template - static bool WaitAll(Array& tasks, double timeoutMilliseconds = -1) + template + static bool WaitAll(Array& tasks, double timeoutMilliseconds = -1) { for (int32 i = 0; i < tasks.Count(); i++) { @@ -300,27 +299,22 @@ public: /// /// Cancels all the tasks from the list and clears it. /// - template - static void CancelAll(Array& tasks) + template + static void CancelAll(Array& tasks) { for (int32 i = 0; i < tasks.Count(); i++) - { tasks[i]->Cancel(); - } tasks.Clear(); } protected: /// - /// Executes this task. - /// It should be called by the task consumer (thread pool or other executor of this task type). - /// It calls run() and handles result). + /// Executes this task. It should be called by the task consumer (thread pool or other executor of this task type). It calls run() and handles result). /// void Execute(); /// - /// Runs the task specified operations - /// Does not handles any task related logic, only performs the actual job. + /// Runs the task specified operations. It does not handle any task related logic, but only performs the actual job. /// /// The task execution result. Returns true if failed, otherwise false. virtual bool Run() = 0;