Assets: Return weak reference by value from asset representation
This is generally more flexible and less error prone. The struct implements a proper descructorfor this anyway. That also makes the separate free function unnecessary-- it's redundant with the destructor.
This commit is contained in:
parent
fd643535bc
commit
8ab23f0a6e
@ -80,10 +80,8 @@ class AssetRepresentation {
|
||||
* Create a weak reference for this asset that can be written to files, but can break under a
|
||||
* number of conditions.
|
||||
* A weak reference can only be created if an asset representation is owned by an asset library.
|
||||
*
|
||||
* Must be freed using #BKE_asset_weak_reference_free().
|
||||
*/
|
||||
AssetWeakReference *make_weak_reference() const;
|
||||
AssetWeakReference make_weak_reference() const;
|
||||
|
||||
StringRefNull get_name() const;
|
||||
ID_Type get_id_type() const;
|
||||
|
@ -57,7 +57,7 @@ const AssetIdentifier &AssetRepresentation::get_identifier() const
|
||||
return identifier_;
|
||||
}
|
||||
|
||||
AssetWeakReference *AssetRepresentation::make_weak_reference() const
|
||||
AssetWeakReference AssetRepresentation::make_weak_reference() const
|
||||
{
|
||||
return AssetWeakReference::make_reference(owner_asset_library_, identifier_);
|
||||
}
|
||||
|
@ -48,12 +48,10 @@ TEST_F(AssetRepresentationTest, weak_reference__current_file)
|
||||
AssetRepresentation &asset = add_dummy_asset(*library, "path/to/an/asset");
|
||||
|
||||
{
|
||||
AssetWeakReference *weak_ref = asset.make_weak_reference();
|
||||
EXPECT_EQ(weak_ref->asset_library_type, ASSET_LIBRARY_LOCAL);
|
||||
EXPECT_EQ(weak_ref->asset_library_identifier, nullptr);
|
||||
EXPECT_STREQ(weak_ref->relative_asset_identifier, "path/to/an/asset");
|
||||
|
||||
BKE_asset_weak_reference_free(&weak_ref);
|
||||
AssetWeakReference weak_ref = asset.make_weak_reference();
|
||||
EXPECT_EQ(weak_ref.asset_library_type, ASSET_LIBRARY_LOCAL);
|
||||
EXPECT_EQ(weak_ref.asset_library_identifier, nullptr);
|
||||
EXPECT_STREQ(weak_ref.relative_asset_identifier, "path/to/an/asset");
|
||||
}
|
||||
}
|
||||
|
||||
@ -65,11 +63,10 @@ TEST_F(AssetRepresentationTest, weak_reference__custom_library)
|
||||
AssetRepresentation &asset = add_dummy_asset(*library, "path/to/an/asset");
|
||||
|
||||
{
|
||||
AssetWeakReference *weak_ref = asset.make_weak_reference();
|
||||
EXPECT_EQ(weak_ref->asset_library_type, ASSET_LIBRARY_CUSTOM);
|
||||
EXPECT_STREQ(weak_ref->asset_library_identifier, "My custom lib");
|
||||
EXPECT_STREQ(weak_ref->relative_asset_identifier, "path/to/an/asset");
|
||||
BKE_asset_weak_reference_free(&weak_ref);
|
||||
AssetWeakReference weak_ref = asset.make_weak_reference();
|
||||
EXPECT_EQ(weak_ref.asset_library_type, ASSET_LIBRARY_CUSTOM);
|
||||
EXPECT_STREQ(weak_ref.asset_library_identifier, "My custom lib");
|
||||
EXPECT_STREQ(weak_ref.relative_asset_identifier, "path/to/an/asset");
|
||||
}
|
||||
}
|
||||
|
||||
@ -79,11 +76,10 @@ TEST_F(AssetRepresentationTest, weak_reference__resolve_to_full_path__current_fi
|
||||
AssetLibrary *library = get_builtin_library_from_type(ASSET_LIBRARY_LOCAL);
|
||||
AssetRepresentation &asset = add_dummy_asset(*library, "path/to/an/asset");
|
||||
|
||||
AssetWeakReference *weak_ref = asset.make_weak_reference();
|
||||
AssetWeakReference weak_ref = asset.make_weak_reference();
|
||||
|
||||
std::string resolved_path = service->resolve_asset_weak_reference_to_full_path(*weak_ref);
|
||||
std::string resolved_path = service->resolve_asset_weak_reference_to_full_path(weak_ref);
|
||||
EXPECT_EQ(resolved_path, "");
|
||||
BKE_asset_weak_reference_free(&weak_ref);
|
||||
}
|
||||
|
||||
/* #AssetLibraryService::resolve_asset_weak_reference_to_full_path(). */
|
||||
@ -94,14 +90,13 @@ TEST_F(AssetRepresentationTest, weak_reference__resolve_to_full_path__custom_lib
|
||||
asset_library_root_);
|
||||
AssetRepresentation &asset = add_dummy_asset(*library, "path/to/an/asset");
|
||||
|
||||
AssetWeakReference *weak_ref = asset.make_weak_reference();
|
||||
AssetWeakReference weak_ref = asset.make_weak_reference();
|
||||
|
||||
std::string expected_path = utils::normalize_path(asset_library_root_ + "/" + "path/") +
|
||||
"to/an/asset";
|
||||
std::string resolved_path = service->resolve_asset_weak_reference_to_full_path(*weak_ref);
|
||||
std::string resolved_path = service->resolve_asset_weak_reference_to_full_path(weak_ref);
|
||||
|
||||
EXPECT_EQ(BLI_path_cmp(resolved_path.c_str(), expected_path.c_str()), 0);
|
||||
BKE_asset_weak_reference_free(&weak_ref);
|
||||
}
|
||||
|
||||
TEST_F(AssetRepresentationTest,
|
||||
@ -112,14 +107,13 @@ TEST_F(AssetRepresentationTest,
|
||||
asset_library_root_);
|
||||
AssetRepresentation &asset = add_dummy_asset(*library, "path\\to\\an\\asset");
|
||||
|
||||
AssetWeakReference *weak_ref = asset.make_weak_reference();
|
||||
AssetWeakReference weak_ref = asset.make_weak_reference();
|
||||
|
||||
std::string expected_path = utils::normalize_path(asset_library_root_ + "\\" + "path\\") +
|
||||
"to\\an\\asset";
|
||||
std::string resolved_path = service->resolve_asset_weak_reference_to_full_path(*weak_ref);
|
||||
std::string resolved_path = service->resolve_asset_weak_reference_to_full_path(weak_ref);
|
||||
|
||||
EXPECT_EQ(BLI_path_cmp(resolved_path.c_str(), expected_path.c_str()), 0);
|
||||
BKE_asset_weak_reference_free(&weak_ref);
|
||||
}
|
||||
|
||||
/* #AssetLibraryService::resolve_asset_weak_reference_to_exploded_path(). */
|
||||
@ -129,18 +123,17 @@ TEST_F(AssetRepresentationTest, weak_reference__resolve_to_exploded_path__curren
|
||||
AssetLibrary *library = get_builtin_library_from_type(ASSET_LIBRARY_LOCAL);
|
||||
AssetRepresentation &asset = add_dummy_asset(*library, "path/to/an/asset");
|
||||
|
||||
AssetWeakReference *weak_ref = asset.make_weak_reference();
|
||||
AssetWeakReference weak_ref = asset.make_weak_reference();
|
||||
|
||||
std::string expected_full_path = utils::normalize_path("path/to/an/asset", 5);
|
||||
std::optional<AssetLibraryService::ExplodedPath> resolved_path =
|
||||
service->resolve_asset_weak_reference_to_exploded_path(*weak_ref);
|
||||
service->resolve_asset_weak_reference_to_exploded_path(weak_ref);
|
||||
|
||||
EXPECT_EQ(*resolved_path->full_path, expected_full_path);
|
||||
EXPECT_EQ(resolved_path->dir_component, "");
|
||||
EXPECT_EQ(resolved_path->group_component, "path");
|
||||
/* ID names may contain slashes. */
|
||||
EXPECT_EQ(resolved_path->name_component, "to/an/asset");
|
||||
BKE_asset_weak_reference_free(&weak_ref);
|
||||
}
|
||||
|
||||
/* #AssetLibraryService::resolve_asset_weak_reference_to_exploded_path(). */
|
||||
@ -151,13 +144,13 @@ TEST_F(AssetRepresentationTest, weak_reference__resolve_to_exploded_path__custom
|
||||
asset_library_root_);
|
||||
AssetRepresentation &asset = add_dummy_asset(*library, "some.blend/Material/asset/name");
|
||||
|
||||
AssetWeakReference *weak_ref = asset.make_weak_reference();
|
||||
AssetWeakReference weak_ref = asset.make_weak_reference();
|
||||
|
||||
std::string expected_full_path = utils::normalize_path(asset_library_root_ +
|
||||
"/some.blend/Material/") +
|
||||
"asset/name";
|
||||
std::optional<AssetLibraryService::ExplodedPath> resolved_path =
|
||||
service->resolve_asset_weak_reference_to_exploded_path(*weak_ref);
|
||||
service->resolve_asset_weak_reference_to_exploded_path(weak_ref);
|
||||
|
||||
EXPECT_EQ(BLI_path_cmp(resolved_path->full_path->c_str(), expected_full_path.c_str()), 0);
|
||||
EXPECT_EQ(BLI_path_cmp_normalized(std::string(resolved_path->dir_component).c_str(),
|
||||
@ -166,7 +159,6 @@ TEST_F(AssetRepresentationTest, weak_reference__resolve_to_exploded_path__custom
|
||||
EXPECT_EQ(resolved_path->group_component, "Material");
|
||||
/* ID names may contain slashes. */
|
||||
EXPECT_EQ(resolved_path->name_component, "asset/name");
|
||||
BKE_asset_weak_reference_free(&weak_ref);
|
||||
}
|
||||
|
||||
/* #AssetLibraryService::resolve_asset_weak_reference_to_exploded_path(). */
|
||||
@ -178,13 +170,13 @@ TEST_F(AssetRepresentationTest,
|
||||
asset_library_root_);
|
||||
AssetRepresentation &asset = add_dummy_asset(*library, "some.blend\\Material\\asset/name");
|
||||
|
||||
AssetWeakReference *weak_ref = asset.make_weak_reference();
|
||||
AssetWeakReference weak_ref = asset.make_weak_reference();
|
||||
|
||||
std::string expected_full_path = utils::normalize_path(asset_library_root_ +
|
||||
"\\some.blend\\Material\\") +
|
||||
"asset/name";
|
||||
std::optional<AssetLibraryService::ExplodedPath> resolved_path =
|
||||
service->resolve_asset_weak_reference_to_exploded_path(*weak_ref);
|
||||
service->resolve_asset_weak_reference_to_exploded_path(weak_ref);
|
||||
|
||||
EXPECT_EQ(BLI_path_cmp(resolved_path->full_path->c_str(), expected_full_path.c_str()), 0);
|
||||
EXPECT_EQ(BLI_path_cmp_normalized(std::string(resolved_path->dir_component).c_str(),
|
||||
@ -193,7 +185,6 @@ TEST_F(AssetRepresentationTest,
|
||||
EXPECT_EQ(resolved_path->group_component, "Material");
|
||||
/* ID names may contain slashes. */
|
||||
EXPECT_EQ(resolved_path->name_component, "asset/name");
|
||||
BKE_asset_weak_reference_free(&weak_ref);
|
||||
}
|
||||
|
||||
} // namespace blender::asset_system::tests
|
||||
|
@ -76,8 +76,6 @@ PreviewImage *BKE_asset_metadata_preview_get_from_id(const AssetMetaData *asset_
|
||||
void BKE_asset_metadata_write(BlendWriter *writer, AssetMetaData *asset_data);
|
||||
void BKE_asset_metadata_read(BlendDataReader *reader, AssetMetaData *asset_data);
|
||||
|
||||
/** Frees the weak reference and its data, and nulls the given pointer. */
|
||||
void BKE_asset_weak_reference_free(AssetWeakReference **weak_ref);
|
||||
AssetWeakReference *BKE_asset_weak_reference_copy(AssetWeakReference *weak_ref);
|
||||
void BKE_asset_weak_reference_write(BlendWriter *writer, const AssetWeakReference *weak_ref);
|
||||
void BKE_asset_weak_reference_read(BlendDataReader *reader, AssetWeakReference *weak_ref);
|
||||
|
@ -46,12 +46,6 @@ AssetWeakReference::~AssetWeakReference()
|
||||
MEM_delete(relative_asset_identifier);
|
||||
}
|
||||
|
||||
void BKE_asset_weak_reference_free(AssetWeakReference **weak_ref)
|
||||
{
|
||||
MEM_delete(*weak_ref);
|
||||
*weak_ref = nullptr;
|
||||
}
|
||||
|
||||
AssetWeakReference *BKE_asset_weak_reference_copy(AssetWeakReference *weak_ref)
|
||||
{
|
||||
if (weak_ref == nullptr) {
|
||||
@ -66,21 +60,21 @@ AssetWeakReference *BKE_asset_weak_reference_copy(AssetWeakReference *weak_ref)
|
||||
return weak_ref_copy;
|
||||
}
|
||||
|
||||
AssetWeakReference *AssetWeakReference::make_reference(
|
||||
AssetWeakReference AssetWeakReference::make_reference(
|
||||
const asset_system::AssetLibrary &library,
|
||||
const asset_system::AssetIdentifier &asset_identifier)
|
||||
{
|
||||
AssetWeakReference *weak_ref = MEM_new<AssetWeakReference>(__func__);
|
||||
AssetWeakReference weak_ref{};
|
||||
|
||||
weak_ref->asset_library_type = library.library_type();
|
||||
weak_ref.asset_library_type = library.library_type();
|
||||
StringRefNull name = library.name();
|
||||
if (!name.is_empty()) {
|
||||
weak_ref->asset_library_identifier = BLI_strdupn(name.c_str(), name.size());
|
||||
weak_ref.asset_library_identifier = BLI_strdupn(name.c_str(), name.size());
|
||||
}
|
||||
|
||||
StringRefNull relative_identifier = asset_identifier.library_relative_identifier();
|
||||
weak_ref->relative_asset_identifier = BLI_strdupn(relative_identifier.c_str(),
|
||||
relative_identifier.size());
|
||||
weak_ref.relative_asset_identifier = BLI_strdupn(relative_identifier.c_str(),
|
||||
relative_identifier.size());
|
||||
|
||||
return weak_ref;
|
||||
}
|
||||
|
@ -51,11 +51,10 @@ void operator_asset_reference_props_register(StructRNA &srna)
|
||||
void operator_asset_reference_props_set(const asset_system::AssetRepresentation &asset,
|
||||
PointerRNA &ptr)
|
||||
{
|
||||
AssetWeakReference *weak_ref = asset.make_weak_reference();
|
||||
RNA_enum_set(&ptr, "asset_library_type", weak_ref->asset_library_type);
|
||||
RNA_string_set(&ptr, "asset_library_identifier", weak_ref->asset_library_identifier);
|
||||
RNA_string_set(&ptr, "relative_asset_identifier", weak_ref->relative_asset_identifier);
|
||||
BKE_asset_weak_reference_free(&weak_ref);
|
||||
const AssetWeakReference weak_ref = asset.make_weak_reference();
|
||||
RNA_enum_set(&ptr, "asset_library_type", weak_ref.asset_library_type);
|
||||
RNA_string_set(&ptr, "asset_library_identifier", weak_ref.asset_library_identifier);
|
||||
RNA_string_set(&ptr, "relative_asset_identifier", weak_ref.relative_asset_identifier);
|
||||
}
|
||||
|
||||
/**
|
||||
|
@ -182,10 +182,9 @@ typedef struct AssetWeakReference {
|
||||
~AssetWeakReference();
|
||||
|
||||
/**
|
||||
* See AssetRepresentation::make_weak_reference(). Must be freed using
|
||||
* #BKE_asset_weak_reference_free().
|
||||
* See AssetRepresentation::make_weak_reference().
|
||||
*/
|
||||
static AssetWeakReference *make_reference(
|
||||
static AssetWeakReference make_reference(
|
||||
const blender::asset_system::AssetLibrary &library,
|
||||
const blender::asset_system::AssetIdentifier &asset_identifier);
|
||||
#endif
|
||||
|
Loading…
Reference in New Issue
Block a user