Assets: Implement Traits #105841

Open
Harley Acheson wants to merge 2 commits from Harley/blender:AssetTraits into main

When changing the target branch, be careful to rebase the branch in your fork to match. See documentation.
6 changed files with 149 additions and 3 deletions

View File

@ -26,6 +26,7 @@ struct IDProperty;
struct PreviewImage;
typedef void (*PreSaveFn)(void *asset_ptr, struct AssetMetaData *asset_data);
typedef void (*EnsureTraitsFn)(void *asset_ptr, struct AssetMetaData *asset_data);
typedef struct AssetTypeInfo {
/**
@ -33,6 +34,8 @@ typedef struct AssetTypeInfo {
* saved.
*/
PreSaveFn pre_save_fn;
/* Callback for custom traits. */
EnsureTraitsFn ensure_traits_fn;
} AssetTypeInfo;
struct AssetMetaData *BKE_asset_metadata_create(void);
@ -60,6 +63,19 @@ struct AssetTagEnsureResult BKE_asset_metadata_tag_ensure(struct AssetMetaData *
const char *name);
void BKE_asset_metadata_tag_remove(struct AssetMetaData *asset_data, struct AssetTag *tag);
struct AssetTraitEnsureResult {
struct AssetTrait *trait;
/* Set to false if a trait with this value was already present. */
bool is_new;
};
/**
* Make sure there is a trait with value \a value, create one if needed.
*/
struct AssetTraitEnsureResult BKE_asset_metadata_trait_ensure(struct AssetMetaData *asset_data,
const char *value);
void BKE_asset_metadata_trait_remove(struct AssetMetaData *asset_data, struct AssetTrait *trait);
/** Clean up the catalog ID (white-spaces removed, length reduced, etc.) and assign it. */
void BKE_asset_metadata_catalog_id_clear(struct AssetMetaData *asset_data);
void BKE_asset_metadata_catalog_id_set(struct AssetMetaData *asset_data,

View File

@ -81,11 +81,12 @@ AssetMetaData::~AssetMetaData()
MEM_SAFE_FREE(copyright);
MEM_SAFE_FREE(license);
BLI_freelistN(&tags);
BLI_freelistN(&traits);
}
static AssetTag *asset_metadata_tag_add(AssetMetaData *asset_data, const char *const name)
{
AssetTag *tag = (AssetTag *)MEM_callocN(sizeof(*tag), __func__);
AssetTag *tag = MEM_cnew<AssetTag>(__func__);
STRNCPY(tag->name, name);
BLI_addtail(&asset_data->tags, tag);
@ -110,7 +111,8 @@ AssetTagEnsureResult BKE_asset_metadata_tag_ensure(AssetMetaData *asset_data, co
return result;
}
AssetTag *tag = (AssetTag *)BLI_findstring(&asset_data->tags, name, offsetof(AssetTag, name));
AssetTag *tag = static_cast<AssetTag *>(
BLI_findstring(&asset_data->tags, name, offsetof(AssetTag, name)));
if (tag) {
result.tag = tag;
Harley marked this conversation as resolved Outdated

Can be removed. I'd for traits we just want to ensure a specific trait is set, we never want the name to be changed (like no automatic .001 suffix). So _ensure() is all we need.

Can be removed. I'd for traits we just want to ensure a specific trait is set, we never want the name to be changed (like no automatic `.001` suffix). So `_ensure()` is all we need.
@ -134,6 +136,44 @@ void BKE_asset_metadata_tag_remove(AssetMetaData *asset_data, AssetTag *tag)
BLI_assert(BLI_listbase_count(&asset_data->tags) == asset_data->tot_tags);
}
static AssetTrait *asset_metadata_trait_add(AssetMetaData *asset_data, const char *const value)
{
Harley marked this conversation as resolved
Review

MEM_cnew<AssetTrait>(__func__) is simpler in C++ code

`MEM_cnew<AssetTrait>(__func__)` is simpler in C++ code
AssetTrait *trait = MEM_cnew<AssetTrait>(__func__);
BLI_strncpy(trait->value, value, sizeof(trait->value));
BLI_addtail(&asset_data->traits, trait);
return trait;
}
struct AssetTraitEnsureResult BKE_asset_metadata_trait_ensure(AssetMetaData *asset_data,
const char *value)
{
struct AssetTraitEnsureResult result = {nullptr};
if (!value[0]) {
return result;
}
Harley marked this conversation as resolved
Review
`static_cast<AssetTrait *>` (https://wiki.blender.org/wiki/Style_Guide/C_Cpp#C.2B.2B_Type_Cast)
AssetTrait *trait = static_cast<AssetTrait *>(
BLI_findstring(&asset_data->traits, value, offsetof(AssetTrait, value)));
if (trait) {
result.trait = trait;
result.is_new = false;
return result;
}
trait = asset_metadata_trait_add(asset_data, value);
result.trait = trait;
result.is_new = true;
return result;
}
void BKE_asset_metadata_trait_remove(AssetMetaData *asset_data, AssetTrait *trait)
{
BLI_assert(BLI_findindex(&asset_data->traits, trait) >= 0);
BLI_freelinkN(&asset_data->traits, trait);
}
void BKE_asset_library_reference_init_default(AssetLibraryReference *library_ref)
{
memcpy(library_ref, DNA_struct_default_get(AssetLibraryReference), sizeof(*library_ref));
@ -212,6 +252,10 @@ void BKE_asset_metadata_write(BlendWriter *writer, AssetMetaData *asset_data)
LISTBASE_FOREACH (AssetTag *, tag, &asset_data->tags) {
BLO_write_struct(writer, AssetTag, tag);
}
LISTBASE_FOREACH (AssetTrait *, trait, &asset_data->traits) {
BLO_write_struct(writer, AssetTrait, trait);
}
}
void BKE_asset_metadata_read(BlendDataReader *reader, AssetMetaData *asset_data)
@ -230,4 +274,5 @@ void BKE_asset_metadata_read(BlendDataReader *reader, AssetMetaData *asset_data)
BLO_read_data_address(reader, &asset_data->license);
BLO_read_list(reader, &asset_data->tags);
BLI_assert(BLI_listbase_count(&asset_data->tags) == asset_data->tot_tags);
BLO_read_list(reader, &asset_data->traits);
}

View File

@ -61,12 +61,13 @@ using namespace blender::bke::idprop;
* "copyright": "<copyright>",
* "license": "<license>",
* "tags": ["<tag>"],
* "traits": ["<trait>"],
* "properties": [..]
* }]
* }
* \endcode
*
* NOTE: entries, author, description, copyright, license, tags and properties are optional
* NOTE: entries, author, description, copyright, license, tags, traits and properties are optional
* attributes.
*
* NOTE: File browser uses name and idcode separate. Inside the index they are joined together like
@ -83,6 +84,7 @@ constexpr StringRef ATTRIBUTE_ENTRIES_AUTHOR("author");
constexpr StringRef ATTRIBUTE_ENTRIES_COPYRIGHT("copyright");
constexpr StringRef ATTRIBUTE_ENTRIES_LICENSE("license");
constexpr StringRef ATTRIBUTE_ENTRIES_TAGS("tags");
constexpr StringRef ATTRIBUTE_ENTRIES_TRAITS("traits");
constexpr StringRef ATTRIBUTE_ENTRIES_PROPERTIES("properties");
/** Abstract class for #BlendFile and #AssetIndexFile. */
@ -229,6 +231,21 @@ struct AssetEntryReader {
}
}
void add_traits_to_meta_data(AssetMetaData *asset_data) const
{
const DictionaryValue::LookupValue *value_ptr = lookup.lookup_ptr(ATTRIBUTE_ENTRIES_TRAITS);
if (value_ptr == nullptr) {
return;
}
const ArrayValue *array_value = (*value_ptr)->as_array_value();
const ArrayValue::Items &elements = array_value->elements();
for (const ArrayValue::Item &item : elements) {
const StringRefNull trait_value = item->as_string_value()->value();
BKE_asset_metadata_trait_ensure(asset_data, trait_value.c_str());
}
}
void add_properties_to_meta_data(AssetMetaData *asset_data) const
{
BLI_assert(asset_data->properties == nullptr);
@ -309,6 +326,17 @@ struct AssetEntryWriter {
}
}
void add_traits(const ListBase /* AssetTrait */ *asset_traits)
{
ArrayValue *traits = new ArrayValue();
attributes.append_as(std::pair(ATTRIBUTE_ENTRIES_TRAITS, traits));
ArrayValue::Items &trait_items = traits->elements();
LISTBASE_FOREACH (AssetTrait *, trait, asset_traits) {
trait_items.append_as(new StringValue(trait->value));
}
}
void add_properties(const IDProperty *properties)
{
std::unique_ptr<Value> value = convert_to_serialize_values(properties);
@ -347,6 +375,10 @@ static void init_value_from_file_indexer_entry(AssetEntryWriter &result,
result.add_tags(&asset_data.tags);
}
if (!BLI_listbase_is_empty(&asset_data.traits)) {
result.add_traits(&asset_data.traits);
}
if (asset_data.properties != nullptr) {
result.add_properties(asset_data.properties);
}
@ -431,6 +463,7 @@ static void init_indexer_entry_from_value(FileIndexerEntry &indexer_entry,
asset_data->catalog_id = entry.get_catalog_id();
entry.add_tags_to_meta_data(asset_data);
entry.add_traits_to_meta_data(asset_data);
entry.add_properties_to_meta_data(asset_data);
}

View File

@ -83,6 +83,13 @@ void ED_assets_pre_save(Main *bmain)
continue;
}
const IDTypeInfo *id_type_info = BKE_idtype_get_info_from_id(id);
BKE_asset_metadata_trait_ensure(id->asset_data, id_type_info->name);
if (id_type_info->asset_type_info && id_type_info->asset_type_info->ensure_traits_fn) {
id_type_info->asset_type_info->ensure_traits_fn(id, id->asset_data);
}
if (id->asset_data->local_type_info->pre_save_fn) {
id->asset_data->local_type_info->pre_save_fn(id, id->asset_data);
}

View File

@ -36,6 +36,15 @@ typedef struct AssetTag {
char name[64]; /* MAX_NAME */
} AssetTag;
Harley marked this conversation as resolved Outdated

Would say: "Managed by Blender, not the user".

Also note that the trait should never be translated for storage (only when displaying in the UI).

Would say: "Managed by Blender, not the user". Also note that the trait should never be translated for storage (only when displaying in the UI).
/**
* \brief Keywords describing type characteristics of an asset. Managed by Blender, not the user.
* These should never be translated for storage, only when displayed in the UI.
*/
Harley marked this conversation as resolved Outdated

Think calling this value makes more sense. Same for the tags actually, but yeah, not worth bothering.

Think calling this `value` makes more sense. Same for the tags actually, but yeah, not worth bothering.
typedef struct AssetTrait {
Harley marked this conversation as resolved Outdated

Just rename it to value, makes more sense to me.

Just rename it to `value`, makes more sense to me.
struct AssetTrait *next, *prev;
char value[64]; /* MAX_NAME */
} AssetTrait;
#
#
typedef struct AssetFilterSettings {
@ -100,6 +109,9 @@ typedef struct AssetMetaData {
short tot_tags;
char _pad[4];
/** See #AssetTrait. */
Harley marked this conversation as resolved Outdated

Trailing empty line, what an eyesore :)

Trailing empty line, what an eyesore :)
ListBase traits;
Review

Not sure what Julian thinks about this, personally I think it's better these days to use an array of pointers in DNA rather than a linked list. i.e. AssetTrait **. That makes adding and removing elements a bit more complex, but it means code can be type safe when dealing with an array of traits, because we can pass around Span<const AssetTrait *>. That's really nice, in comparison with just ListBase.

Not sure what Julian thinks about this, personally I think it's better these days to use an array of pointers in DNA rather than a linked list. i.e. `AssetTrait **`. That makes adding and removing elements a bit more complex, but it means code can be type safe when dealing with an array of traits, because we can pass around `Span<const AssetTrait *>`. That's really nice, in comparison with just `ListBase`.
} AssetMetaData;
typedef enum eAssetLibraryType {

View File

@ -136,6 +136,14 @@ static void rna_AssetMetaData_tag_remove(ID *id,
RNA_POINTER_INVALIDATE(tag_ptr);
}
static char *rna_AssetTrait_path(const PointerRNA *ptr)
{
const AssetTrait *asset_trait = static_cast<AssetTrait *>(ptr->data);
char asset_trait_value_esc[sizeof(asset_trait->value) * 2];
BLI_str_escape(asset_trait_value_esc, asset_trait->value, sizeof(asset_trait_value_esc));
return BLI_sprintfN("asset_data.traits[\"%s\"]", asset_trait_value_esc);
}
static IDProperty **rna_AssetMetaData_idprops(PointerRNA *ptr)
{
AssetMetaData *asset_data = static_cast<AssetMetaData *>(ptr->data);
@ -440,6 +448,21 @@ static void rna_def_asset_tags_api(BlenderRNA *brna, PropertyRNA *cprop)
RNA_def_parameter_clear_flags(parm, PROP_THICK_WRAP, ParameterFlag(0));
}
static void rna_def_asset_trait(BlenderRNA *brna)
{
StructRNA *srna;
PropertyRNA *prop;
srna = RNA_def_struct(brna, "AssetTrait", NULL);
RNA_def_struct_path_func(srna, "rna_AssetTrait_path");
RNA_def_struct_ui_text(srna, "Asset Trait", "Keywords describing type characteristics of an asset (managed by Blender, not the user)");
prop = RNA_def_property(srna, "value", PROP_STRING, PROP_NONE);
RNA_def_property_string_maxlength(prop, MAX_NAME);
RNA_def_property_ui_text(prop, "Value", "The identifier that makes up this asset trait");
RNA_def_struct_name_property(srna, prop);
}
static void rna_def_asset_data(BlenderRNA *brna)
{
StructRNA *srna;
@ -507,6 +530,15 @@ static void rna_def_asset_data(BlenderRNA *brna)
RNA_def_property_int_funcs(prop, nullptr, nullptr, "rna_AssetMetaData_active_tag_range");
RNA_def_property_ui_text(prop, "Active Tag", "Index of the tag set for editing");
prop = RNA_def_property(srna, "traits", PROP_COLLECTION, PROP_NONE);
RNA_def_property_struct_type(prop, "AssetTrait");
RNA_def_property_editable_func(prop, "rna_AssetMetaData_editable");
RNA_def_property_ui_text(
prop,
"Asset Traits",
"Keywords describing type characteristics of an asset. Used to determine how an asset "
"should behave, and to provide additional information and filtering options to the user");
prop = RNA_def_property(srna, "catalog_id", PROP_STRING, PROP_NONE);
RNA_def_property_string_funcs(prop,
"rna_AssetMetaData_catalog_id_get",
@ -614,6 +646,7 @@ void RNA_def_asset(BlenderRNA *brna)
RNA_define_animate_sdna(false);
rna_def_asset_tag(brna);
rna_def_asset_trait(brna);
rna_def_asset_data(brna);
rna_def_asset_library_reference(brna);
rna_def_asset_handle(brna);