Commit: efaeed1c1704b84187ff3d17a595c1df1f13c65c Author: Julian Eisel Date: Mon Nov 7 15:11:27 2022 +0100 Branches: temp-asset-representation https://developer.blender.org/rBefaeed1c1704b84187ff3d17a595c1df1f13c65c
Refactor how asset representations are added to an asset library =================================================================== M source/blender/blenkernel/BKE_asset.h M source/blender/blenkernel/BKE_asset_library.hh M source/blender/blenkernel/BKE_asset_representation.hh M source/blender/blenkernel/intern/asset.cc M source/blender/blenkernel/intern/asset_library.cc M source/blender/blenkernel/intern/asset_representation.cc M source/blender/editors/space_file/file_indexer.cc M source/blender/editors/space_file/filelist.cc M source/blender/makesdna/DNA_asset_types.h =================================================================== diff --git a/source/blender/blenkernel/BKE_asset.h b/source/blender/blenkernel/BKE_asset.h index 379a16432cb..14d1b80c73a 100644 --- a/source/blender/blenkernel/BKE_asset.h +++ b/source/blender/blenkernel/BKE_asset.h @@ -79,3 +79,12 @@ bool BKE_asset_representation_is_local_id(const AssetRepresentation *asset) #ifdef __cplusplus } #endif + +#ifdef __cplusplus + +# include <memory> + +[[nodiscard]] std::unique_ptr<AssetMetaData> BKE_asset_metadata_move_to_unique_ptr( + AssetMetaData *asset_data); + +#endif diff --git a/source/blender/blenkernel/BKE_asset_library.hh b/source/blender/blenkernel/BKE_asset_library.hh index e089e30c345..091df6d8edd 100644 --- a/source/blender/blenkernel/BKE_asset_library.hh +++ b/source/blender/blenkernel/BKE_asset_library.hh @@ -51,11 +51,6 @@ struct AssetLibrary { std::string root_path; std::unique_ptr<AssetCatalogService> catalog_service; - /** Container to store asset representations, managed by whatever manages this library, not by - * the library itself. So this really is arbitrary storage as far as #AssetLibrary is concerned - * (allowing the API user to manage partial library storage and partial loading, so only relevant - * parts of a library are kept in memory). */ - AssetStorage asset_storage; AssetLibrary(); ~AssetLibrary(); @@ -65,6 +60,9 @@ struct AssetLibrary { /** Load catalogs that have changed on disk. */ void refresh(); + AssetRepresentation &add_external_asset(std::unique_ptr<AssetMetaData> metadata); + AssetRepresentation &add_local_id_asset(const ID &id); + /** * Update `catalog_simple_name` by looking up the asset's catalog by its ID. * @@ -80,6 +78,14 @@ struct AssetLibrary { private: bCallbackFuncStore on_save_callback_store_{}; + + /** Container to store asset representations. Assets are not automatically loaded into this when + * loading an asset library. Assets have to be loaded externally and added to this storage via + * #add_external_asset() or #add_local_id_asset(). + * So this really is arbitrary storage as far as #AssetLibrary is concerned (allowing the API + * user to manage partial library storage and partial loading, so only relevant parts of a + * library are kept in memory). */ + AssetStorage asset_storage_; }; Vector<AssetLibraryReference> all_valid_asset_library_refs(); diff --git a/source/blender/blenkernel/BKE_asset_representation.hh b/source/blender/blenkernel/BKE_asset_representation.hh index 32f5dd44f43..d2e167b3f35 100644 --- a/source/blender/blenkernel/BKE_asset_representation.hh +++ b/source/blender/blenkernel/BKE_asset_representation.hh @@ -31,10 +31,7 @@ class AssetRepresentation { explicit AssetRepresentation(std::unique_ptr<AssetMetaData> metadata); /** Constructs an asset representation for an ID stored in the current file. This makes the asset * local and fully editable. */ - explicit AssetRepresentation(ID &id); - - /* TODO this doesn't make sense. Remove this. */ - explicit AssetRepresentation(AssetMetaData &&metadata); + explicit AssetRepresentation(const ID &id); AssetMetaData &get_metadata() const; /** Returns if this asset is stored inside this current file, and as such fully editable. */ diff --git a/source/blender/blenkernel/intern/asset.cc b/source/blender/blenkernel/intern/asset.cc index 67802b1d6b4..7103e017847 100644 --- a/source/blender/blenkernel/intern/asset.cc +++ b/source/blender/blenkernel/intern/asset.cc @@ -27,21 +27,31 @@ using namespace blender; AssetMetaData *BKE_asset_metadata_create() { - AssetMetaData *asset_data = (AssetMetaData *)MEM_callocN(sizeof(*asset_data), __func__); - memcpy(asset_data, DNA_struct_default_get(AssetMetaData), sizeof(*asset_data)); - return asset_data; + const AssetMetaData *default_metadata = DNA_struct_default_get(AssetMetaData); + return MEM_new<AssetMetaData>(__func__, *default_metadata); } void BKE_asset_metadata_free(AssetMetaData **asset_data) { - if ((*asset_data)->properties) { - IDP_FreeProperty((*asset_data)->properties); + (*asset_data)->~AssetMetaData(); + MEM_SAFE_FREE(*asset_data); +} + +AssetMetaData::~AssetMetaData() +{ + if (properties) { + IDP_FreeProperty(properties); } - MEM_SAFE_FREE((*asset_data)->author); - MEM_SAFE_FREE((*asset_data)->description); - BLI_freelistN(&(*asset_data)->tags); + MEM_SAFE_FREE(author); + MEM_SAFE_FREE(description); + BLI_freelistN(&tags); +} - MEM_SAFE_FREE(*asset_data); +std::unique_ptr<AssetMetaData> BKE_asset_metadata_move_to_unique_ptr(AssetMetaData *asset_data) +{ + std::unique_ptr unique_asset_data = std::make_unique<AssetMetaData>(*asset_data); + *asset_data = *DNA_struct_default_get(AssetMetaData); + return unique_asset_data; } static AssetTag *asset_metadata_tag_add(AssetMetaData *asset_data, const char *const name) diff --git a/source/blender/blenkernel/intern/asset_library.cc b/source/blender/blenkernel/intern/asset_library.cc index 0e14ad2b4cc..a6243876d34 100644 --- a/source/blender/blenkernel/intern/asset_library.cc +++ b/source/blender/blenkernel/intern/asset_library.cc @@ -124,6 +124,16 @@ void AssetLibrary::refresh() this->catalog_service->reload_catalogs(); } +AssetRepresentation &AssetLibrary::add_external_asset(std::unique_ptr<AssetMetaData> metadata) +{ + return asset_storage_.append(std::make_unique<AssetRepresentation>(std::move(metadata))); +} + +AssetRepresentation &AssetLibrary::add_local_id_asset(const ID &id) +{ + return asset_storage_.append(std::make_unique<AssetRepresentation>(id)); +} + namespace { void asset_library_on_save_post(struct Main *main, struct PointerRNA **pointers, diff --git a/source/blender/blenkernel/intern/asset_representation.cc b/source/blender/blenkernel/intern/asset_representation.cc index df10ab35d24..7123d7aba55 100644 --- a/source/blender/blenkernel/intern/asset_representation.cc +++ b/source/blender/blenkernel/intern/asset_representation.cc @@ -17,12 +17,7 @@ AssetRepresentation::AssetRepresentation(std::unique_ptr<AssetMetaData> metadata { } -AssetRepresentation::AssetRepresentation(ID &id) : local_id_metadata_(id.asset_data) -{ -} - -AssetRepresentation::AssetRepresentation(AssetMetaData &&metadata) - : metadata_(std::make_unique<AssetMetaData>(metadata)) +AssetRepresentation::AssetRepresentation(const ID &id) : local_id_metadata_(id.asset_data) { } diff --git a/source/blender/editors/space_file/file_indexer.cc b/source/blender/editors/space_file/file_indexer.cc index ec631eb48b3..8520ac34122 100644 --- a/source/blender/editors/space_file/file_indexer.cc +++ b/source/blender/editors/space_file/file_indexer.cc @@ -67,8 +67,9 @@ void ED_file_indexer_entries_extend_from_datablock_infos( } } -static void ED_file_indexer_entry_free(void *indexer_entry) +static void ED_file_indexer_entry_free(void *indexer_entry_ptr) { + FileIndexerEntry *indexer_entry = static_cast<FileIndexerEntry *>(indexer_entry_ptr); MEM_freeN(indexer_entry); } diff --git a/source/blender/editors/space_file/filelist.cc b/source/blender/editors/space_file/filelist.cc index 9d806be61f5..8c389a35b3a 100644 --- a/source/blender/editors/space_file/filelist.cc +++ b/source/blender/editors/space_file/filelist.cc @@ -2993,9 +2993,13 @@ static FileListInternEntry *filelist_readjob_list_lib_group_create(const int idc return entry; } +/** + * \warning: This "steals" the asset metadata from \a datablock_info. Not great design but fixing + * this requires redesigning things on the caller side for proper ownership management. + */ static void filelist_readjob_list_lib_add_datablock(FileList *filelist, ListBase *entries, - const BLODataBlockInfo *datablock_info, + BLODataBlockInfo *datablock_info, const bool prefix_relpath_with_group_name, const int idcode, const char *group_name) @@ -3015,11 +3019,12 @@ static void filelist_readjob_list_lib_add_datablock(FileList *filelist, entry->typeflag |= FILE_TYPE_ASSET; if (filelist->asset_library) { - /* TODO copying asset metadata like this does a shallow copy. E.g. custom properties are - * not duplicated properly. */ - std::unique_ptr asset = std::make_unique<bke::AssetRepresentation>( - std::move(*datablock_info->asset_data)); - entry->asset = &filelist->asset_library->asset_storage.append(std::move(asset)); + /** XXX Moving out the asset metadata like this isn't great. */ + std::unique_ptr metadata = BKE_asset_metadata_move_to_unique_ptr( + datablock_info->asset_data); + BKE_asset_metadata_free(&datablock_info->asset_data); + + entry->asset = &filelist->asset_library->add_external_asset(std::move(metadata)); } } } @@ -3048,7 +3053,7 @@ static void filelist_readjob_list_lib_add_from_indexer_entries( const bool prefix_relpath_with_group_name) { for (const LinkNode *ln = indexer_entries->entries; ln; ln = ln->next) { - const FileIndexerEntry *indexer_entry = (const FileIndexerEntry *)ln->link; + FileIndexerEntry *indexer_entry = static_cast<FileIndexerEntry *>(ln->link); const char *group_name = BKE_idtype_idcode_to_name(indexer_entry->idcode); filelist_readjob_list_lib_add_datablock(filelist, entries, @@ -3691,8 +3696,7 @@ static void filelist_readjob_main_assets_add_items(FileListReadJob *job_params, id_iter); entry->local_data.id = id_iter; if (filelist->asset_library) { - std::unique_ptr asset = std::make_unique<bke::AssetRepresentation>(*id_iter); - entry->asset = &filelist->asset_library->asset_storage.append(std::move(asset)); + entry->asset = &filelist->asset_library->add_local_id_asset(*id_iter); } entries_num++; BLI_addtail(&tmp_entries, entry); diff --git a/source/blender/makesdna @@ Diff output truncated at 10240 characters. @@ _______________________________________________ Bf-blender-cvs mailing list Bf-blender-cvs@blender.org List details, subscription details or unsubscribe: https://lists.blender.org/mailman/listinfo/bf-blender-cvs