[Bf-blender-cvs] [59f8061a346] master: Assets: Refactor asset representation storage
Julian Eisel
noreply at git.blender.org
Thu Nov 17 12:20:16 CET 2022
Commit: 59f8061a346cdea268307c3d4b07024b4db35d02
Author: Julian Eisel
Date: Wed Nov 16 15:43:43 2022 +0100
Branches: master
https://developer.blender.org/rB59f8061a346cdea268307c3d4b07024b4db35d02
Assets: Refactor asset representation storage
- Move code to manage storage to own class in own file, separates
concerns and different levels of abstraction better.
- Store local ID assets separately in the storage class for more
efficient lookups (e.g. for ID remapping).
- Make API function names and comments more complete.
===================================================================
M source/blender/asset_system/AS_asset_library.hh
M source/blender/asset_system/AS_asset_representation.hh
M source/blender/asset_system/CMakeLists.txt
M source/blender/asset_system/intern/asset_library.cc
A source/blender/asset_system/intern/asset_storage.cc
A source/blender/asset_system/intern/asset_storage.hh
===================================================================
diff --git a/source/blender/asset_system/AS_asset_library.hh b/source/blender/asset_system/AS_asset_library.hh
index c4d9706a9c1..4bd5ee3446d 100644
--- a/source/blender/asset_system/AS_asset_library.hh
+++ b/source/blender/asset_system/AS_asset_library.hh
@@ -20,11 +20,13 @@
struct AssetLibrary;
struct AssetLibraryReference;
struct AssetMetaData;
+struct IDRemapper;
struct Main;
namespace blender::asset_system {
class AssetRepresentation;
+class AssetStorage;
/**
* AssetLibrary provides access to an asset library's data.
@@ -54,12 +56,24 @@ struct AssetLibrary {
* loading a different file).
*/
AssetRepresentation &add_external_asset(StringRef name, std::unique_ptr<AssetMetaData> metadata);
+ /** See #AssetLibrary::add_external_asset(). */
AssetRepresentation &add_local_id_asset(ID &id);
/** Remove an asset from the library that was added using #add_external_asset() or
- * #add_local_id_asset().
- * \return True on success, false if the asset couldn't be found inside the library. */
+ * #add_local_id_asset(). Can usually be expected to be constant time complexity (worst case may
+ * differ).
+ * \note This is save to call if \a asset is freed (dangling reference), will not perform any
+ * change then.
+ * \return True on success, false if the asset couldn't be found inside the library (also the
+ * case when the reference is dangling). */
bool remove_asset(AssetRepresentation &asset);
+ /**
+ * Remap ID pointers for local ID assets, see #BKE_lib_remap.h. When an ID pointer would be
+ * mapped to null (typically when an ID gets removed), the asset is removed, because we don't
+ * support such empty/null assets.
+ */
+ void remap_ids_and_remove_invalid(const IDRemapper &mappings);
+
/**
* Update `catalog_simple_name` by looking up the asset's catalog by its ID.
*
@@ -73,8 +87,6 @@ struct AssetLibrary {
void on_blend_save_post(Main *bmain, PointerRNA **pointers, int num_pointers);
- void remap_ids(const struct IDRemapper &mappings);
-
private:
bCallbackFuncStore on_save_callback_store_{};
@@ -91,7 +103,7 @@ struct AssetLibrary {
* already in memory and which not. Neither do we keep track of how many parts of Blender are
* using an asset or an asset library, which is needed to know when assets can be freed.
*/
- Set<std::unique_ptr<AssetRepresentation>> asset_storage_;
+ std::unique_ptr<AssetStorage> asset_storage_;
std::optional<int> find_asset_index(const AssetRepresentation &asset);
};
diff --git a/source/blender/asset_system/AS_asset_representation.hh b/source/blender/asset_system/AS_asset_representation.hh
index 66c49c445dc..15e10d6c98f 100644
--- a/source/blender/asset_system/AS_asset_representation.hh
+++ b/source/blender/asset_system/AS_asset_representation.hh
@@ -23,6 +23,7 @@ namespace blender::asset_system {
*/
class AssetRepresentation {
friend struct AssetLibrary;
+ friend class AssetStorage;
struct ExternalAsset {
std::string name;
diff --git a/source/blender/asset_system/CMakeLists.txt b/source/blender/asset_system/CMakeLists.txt
index 8bf7a135155..05f03c2bfc5 100644
--- a/source/blender/asset_system/CMakeLists.txt
+++ b/source/blender/asset_system/CMakeLists.txt
@@ -19,12 +19,14 @@ set(SRC
intern/asset_library.cc
intern/asset_library_service.cc
intern/asset_representation.cc
+ intern/asset_storage.cc
AS_asset_catalog.hh
AS_asset_catalog_path.hh
AS_asset_library.hh
AS_asset_representation.hh
intern/asset_library_service.hh
+ intern/asset_storage.hh
AS_asset_library.h
AS_asset_representation.h
diff --git a/source/blender/asset_system/intern/asset_library.cc b/source/blender/asset_system/intern/asset_library.cc
index a7add445c99..90d83d369d6 100644
--- a/source/blender/asset_system/intern/asset_library.cc
+++ b/source/blender/asset_system/intern/asset_library.cc
@@ -10,7 +10,6 @@
#include "AS_asset_library.hh"
#include "AS_asset_representation.hh"
-#include "BKE_lib_remap.h"
#include "BKE_main.h"
#include "BKE_preferences.h"
@@ -18,10 +17,10 @@
#include "BLI_path_util.h"
#include "BLI_set.hh"
-#include "DNA_asset_types.h"
#include "DNA_userdef_types.h"
#include "asset_library_service.hh"
+#include "asset_storage.hh"
using namespace blender;
using namespace blender::asset_system;
@@ -107,13 +106,16 @@ void AS_asset_library_refresh_catalog_simplename(struct ::AssetLibrary *asset_li
void AS_asset_library_remap_ids(const IDRemapper *mappings)
{
AssetLibraryService *service = AssetLibraryService::get();
- service->foreach_loaded_asset_library(
- [mappings](asset_system::AssetLibrary &library) { library.remap_ids(*mappings); });
+ service->foreach_loaded_asset_library([mappings](asset_system::AssetLibrary &library) {
+ library.remap_ids_and_remove_invalid(*mappings);
+ });
}
namespace blender::asset_system {
-AssetLibrary::AssetLibrary() : catalog_service(std::make_unique<AssetCatalogService>())
+AssetLibrary::AssetLibrary()
+ : catalog_service(std::make_unique<AssetCatalogService>()),
+ asset_storage_(std::make_unique<AssetStorage>())
{
}
@@ -139,25 +141,22 @@ void AssetLibrary::refresh()
AssetRepresentation &AssetLibrary::add_external_asset(StringRef name,
std::unique_ptr<AssetMetaData> metadata)
{
- return *asset_storage_.lookup_key_or_add(
- std::make_unique<AssetRepresentation>(name, std::move(metadata)));
+ return asset_storage_->add_external_asset(name, std::move(metadata));
}
AssetRepresentation &AssetLibrary::add_local_id_asset(ID &id)
{
- return *asset_storage_.lookup_key_or_add(std::make_unique<AssetRepresentation>(id));
+ return asset_storage_->add_local_id_asset(id);
}
bool AssetLibrary::remove_asset(AssetRepresentation &asset)
{
- /* Create a "fake" unique_ptr to figure out the hash for the pointed to asset representation. The
- * standard requires that this is the same for all unique_ptr's wrapping the same address. */
- std::unique_ptr<AssetRepresentation> fake_asset_ptr{&asset};
-
- const bool was_removed = asset_storage_.remove_as(fake_asset_ptr);
- /* Make sure the contained storage is not destructed. */
- fake_asset_ptr.release();
- return was_removed;
+ return asset_storage_->remove_asset(asset);
+}
+
+void AssetLibrary::remap_ids_and_remove_invalid(const IDRemapper &mappings)
+{
+ asset_storage_->remap_ids_and_remove_invalid(mappings);
}
namespace {
@@ -203,28 +202,6 @@ void AssetLibrary::on_blend_save_post(struct Main *main,
}
}
-void AssetLibrary::remap_ids(const IDRemapper &mappings)
-{
- Set<AssetRepresentation *> removed_id_assets;
-
- for (auto &asset_uptr : asset_storage_) {
- if (!asset_uptr->is_local_id()) {
- continue;
- }
-
- IDRemapperApplyResult result = BKE_id_remapper_apply(
- &mappings, &asset_uptr->local_asset_id_, ID_REMAP_APPLY_DEFAULT);
- if (result == ID_REMAP_RESULT_SOURCE_UNASSIGNED) {
- removed_id_assets.add(asset_uptr.get());
- }
- }
-
- /* Remove the assets from storage. */
- for (AssetRepresentation *asset : removed_id_assets) {
- remove_asset(*asset);
- }
-}
-
void AssetLibrary::refresh_catalog_simplename(struct AssetMetaData *asset_data)
{
if (BLI_uuid_is_nil(asset_data->catalog_id)) {
diff --git a/source/blender/asset_system/intern/asset_storage.cc b/source/blender/asset_system/intern/asset_storage.cc
new file mode 100644
index 00000000000..d4dcc0ce73c
--- /dev/null
+++ b/source/blender/asset_system/intern/asset_storage.cc
@@ -0,0 +1,79 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+
+/** \file
+ * \ingroup asset_system
+ */
+
+#include "AS_asset_representation.hh"
+
+#include "DNA_ID.h"
+#include "DNA_asset_types.h"
+
+#include "BKE_lib_remap.h"
+
+#include "asset_storage.hh"
+
+namespace blender::asset_system {
+
+AssetRepresentation &AssetStorage::add_local_id_asset(ID &id)
+{
+ return *local_id_assets_.lookup_key_or_add(std::make_unique<AssetRepresentation>(id));
+}
+
+AssetRepresentation &AssetStorage::add_external_asset(StringRef name,
+ std::unique_ptr<AssetMetaData> metadata)
+{
+ return *external_assets_.lookup_key_or_add(
+ std::make_unique<AssetRepresentation>(name, std::move(metadata)));
+}
+
+bool AssetStorage::remove_asset(AssetRepresentation &asset)
+{
+ auto remove_if_contained_fn = [&asset](StorageT &storage) {
+ /* Create a "fake" unique_ptr to figure out the hash for the pointed to asset representation.
+ * The standard requires that this is the same for all unique_ptr's wrapping the same address.
+ */
+ std::unique_ptr<AssetRepresentation> fake_asset_ptr{&asset};
+
+ const std::unique_ptr<AssetRepresentation> *real_asset_ptr = storage.lookup_key_ptr_as(
+ fake_asset_ptr);
+ /* Make sure the contained storage is not destructed. */
+ fake_asset_ptr.release();
+
+ if (!real_asset_ptr) {
+ return false;
+ }
+
+ storage.remove_contained(*real_asset_ptr);
+ return true;
+ };
+
+ if (remove_if_contained_fn(local_id_assets_)) {
+ return true;
+ }
+ return remove_if_contained_fn(external_assets_);
+}
+
+void AssetStorage::remap_ids_and_remove_invalid(const IDRemapper &mappings)
+{
+ Set<AssetRepresentation *> removed_assets{};
+
+ for (auto &asset_ptr : local_id_assets_) {
+ AssetRepresentation &asset = *asset_ptr;
+ BLI_assert(asset.is_local_id());
+
+ const IDRemapperApplyResult result = BKE_id_remapper_apply(
+ &mappings, &asset.local_asset_id_, ID_REMAP_APPLY_DEFAULT);
+
+ /* Entirely remove assets whose ID is unset. We don't want assets with a null ID pointer. */
+ if (result == ID_REMAP_RESULT_SOURCE_UNASSIGNED) {
+ removed_assets.add(&asset);
+ }
+ }
+
+ for (AssetRepresentation *asset : removed_assets) {
+ remove_asset(*asset);
+ }
+}
+
+} // namespace blender::asset_system
diff --git a/source/blender/asset_system/intern/asset_storage.hh b/source/blender/asset_system/intern/asset_storage.hh
new file mode 100644
index 00000000000..9278b3f41d1
--- /dev/null
+++ b/source/blender/asset_system/intern/asset_storage.hh
@@ -0,0 +1,46 @@
+/* SPDX-License-Ide
@@ Diff output truncated at 10240 characters. @@
More information about the Bf-blender-cvs
mailing list