Assets: Implement Traits #105841

Allow Assets to have system-defined keywords to provide information
about the type of the asset.

This is WIP implementation for task #105808 for design #105807

This is currently just a copy of the code that implements tags. So this includes things we don't need like user-facing addition and deletion of traits, but I needed to make sure that these operations worked and were saved correctly. I imagine these being removed after this is confirmed correct. This doesn't change ED_asset_filter_matches_asset because I was unable to find that visible to test.

Allow Assets to have system-defined keywords to provide information about the type of the asset. --- This is WIP implementation for task #105808 for design #105807 This is currently just a copy of the code that implements tags. So this includes things we don't need like user-facing addition and deletion of traits, but I needed to make sure that these operations worked and were saved correctly. I imagine these being removed after this is confirmed correct. This doesn't change `ED_asset_filter_matches_asset` because I was unable to find that visible to test.

I think we can remove most of the editing capabilities. Let's for now only have a list or traits that is managed internally, with no API functions to edit them. I guess we'd want that later to support custom asset types managed by add-ons, but we may want to use different ways to define those (e.g. a bl_asset_traits property for a new asset type definition).

Plus, we indeed don't need any UI for the traits. We might want to display the traits as non-editable tags in the tags list, for informal purposes.

I think we can remove most of the editing capabilities. Let's for now only have a list or traits that is managed internally, with no API functions to edit them. I guess we'd want that later to support custom asset types managed by add-ons, but we may want to use different ways to define those (e.g. a `bl_asset_traits` property for a new asset type definition). Plus, we indeed don't need any UI for the traits. We might want to display the traits as non-editable tags in the tags list, for informal purposes.

@JulianEisel - Sorry for the delay; took a while to get rid of all my extras without leaving any cruft.

@JulianEisel @Harley what is the status of this patch?

Could you remove the ensure_traits_fn callbacks for now as noted in #105808 (comment)? We don't need most of these traits yet, I rather add traits as needed, and don't set a whole bunch of unused traits just because they might be useful in future. (Although to a degree we may have to do that for forwards compatibility.)

Also, if a patch is ready for review, please make sure to remove the "WIP:" from the title. This is meant for patches that you still intend to work on independently of the review process.

Could you remove the `ensure_traits_fn` callbacks for now as noted in https://projects.blender.org/blender/blender/issues/105808#issuecomment-905308? We don't need most of these traits yet, I rather add traits as needed, and don't set a whole bunch of unused traits just because they might be useful in future. (Although to a degree we may have to do that for forwards compatibility.) Also, if a patch is ready for review, please make sure to remove the "WIP:" from the title. This is meant for patches that you still intend to work on independently of the review process.

@JulianEisel - Why would we store the UI description as trait?

I removed that section so didn't really look at it again, but I think that when I was examining the data there the names had multiple words CamelCased together but the descriptions were nicer. But again, that code is gone now anyway.

What does System-defined mean? The OS?

I changed it all to your requested terminology, but I do think that "system-defined" was the best counterpoint to "user-defined" as it helped explain the similarities and differences. Now we have "User defined tag" and a very similar looking thing as "Keywords describing type characteristics of an asset." Don't think anyone would think system-defined had anything to do with the OS or the platform - just implies not defined or managed by the user.

> @JulianEisel - Why would we store the UI description as trait? I removed that section so didn't really look at it again, but I *think* that when I was examining the data there the names had multiple words CamelCased together but the descriptions were nicer. But again, that code is gone now anyway. > What does System-defined mean? The OS? I changed it all to your requested terminology, but I do think that "system-defined" was the best counterpoint to "user-defined" as it helped explain the similarities and differences. Now we have "User defined tag" and a very similar looking thing as "Keywords describing type characteristics of an asset." Don't think anyone would think system-defined had anything to do with the OS or the platform - just implies not defined or managed by the user.

I think this is almost ready, just some smaller points.

Could the get_obdata_uiname stuff be replaced with use of the IDTypeInfo of ob->data? It's a bit of a shame to add another set of defines and another switch with Blender trying to move away from that sort of thing and become more easily extensible.

Also, I know it's simple to use ListBase in DNA, but I wonder if it's worth using an array here instead. That makes it a bit more complex to add and remove items, but it makes everything else much simpler, since there would actually be type safety, etc. For example, the traits could mostly be passed as Span<AssetTrait>. The next and prev pointers could also be removed. That's my personal agenda of moving away from linked lists coming though, but maybe you find it reasonable too.

Could the `get_obdata_uiname` stuff be replaced with use of the `IDTypeInfo` of `ob->data`? It's a bit of a shame to add another set of defines and another switch with Blender trying to move away from that sort of thing and become more easily extensible. Also, I know it's simple to use `ListBase` in DNA, but I wonder if it's worth using an array here instead. That makes it a bit more complex to add and remove items, but it makes everything else much simpler, since there would actually be type safety, etc. For example, the traits could mostly be passed as `Span<AssetTrait>`. The `next` and `prev` pointers could also be removed. That's my personal agenda of moving away from linked lists coming though, but maybe you find it reasonable too.

Oops, meant to just comment, now I can't remove myself as a reviewer though :/

Could the get_obdata_uiname stuff be replaced with use of the IDTypeInfo of ob->data?

We could have a const char *ui_name in there and assign that to the ID_UI_NAME_ value indeed. Similar to id_code, id_filter and main_listbase_index.
Unfortunately the #defines would still be necessary if we want to keep the UI names in the RNA enum in sync and static.

Also, I know it's simple to use ListBase in DNA, but I wonder if it's worth using an array here instead.

For runtime there should be some kind of hash-table inside AssetRepresentation (and/or maybe inside an AssetMetaData_Runtime?). The listbase is more or less just for DNA storage. Don't think it's a big issue, not worth the complexity IMO.

> Could the `get_obdata_uiname` stuff be replaced with use of the `IDTypeInfo` of `ob->data`? We could have a `const char *ui_name` in there and assign that to the `ID_UI_NAME_` value indeed. Similar to `id_code`, `id_filter` and `main_listbase_index`. Unfortunately the `#define`s would still be necessary if we want to keep the UI names in the RNA enum in sync and static. > Also, I know it's simple to use `ListBase` in DNA, but I wonder if it's worth using an array here instead. For runtime there should be some kind of hash-table inside `AssetRepresentation` (and/or maybe inside an `AssetMetaData_Runtime`?). The listbase is more or less just for DNA storage. Don't think it's a big issue, not worth the complexity IMO.

Don't think it's a big issue, not worth the complexity IMO.

I'd argue using ListBase everywhere gives its own pervasive complexity that's less obvious when adding it in the first place. But fair enough. I guess the need to store these at all is just a future-proofing thing?

>Don't think it's a big issue, not worth the complexity IMO. I'd argue using `ListBase` everywhere gives its own pervasive complexity that's less obvious when adding it in the first place. But fair enough. I guess the need to store these at all is just a future-proofing thing?

I guess the need to store these at all is just a future-proofing thing?

They are stored in the asset metadata so this type information becomes available when loading asset representations, without loading the entire asset. Or do you mean the object data traits? Yeah we don't actually use them yet, so this is a future proofing thing indeed.

> I guess the need to store these at all is just a future-proofing thing? They are stored in the asset metadata so this type information becomes available when loading asset representations, without loading the entire asset. Or do you mean the object data traits? Yeah we don't actually use them yet, so this is a future proofing thing indeed.

Talked to Bastien, he rejects the idea of something like a ui_name field in IDTypeInfo for this purpose, because then the object callback would have to get the IDTypeInfo of the object data, which creates dependencies between ID types.

So nothing to change there from the current patch, we stick to the #defines and the switch-case for the object data.

Talked to Bastien, he rejects the idea of something like a `ui_name` field in `IDTypeInfo` for this purpose, because then the object callback would have to get the `IDTypeInfo` of the object data, which creates dependencies between ID types. So nothing to change there from the current patch, we stick to the `#define`s and the switch-case for the object data.

@JulianEisel

Should be done with just the following notes:

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

I couldn't tell from your comment if you wanted this done for traits, tags, both, or none. So I left it alone. Can certainly do whatever you prefer.

...so let's do it on file save in ED_assets_pre_save()

I did so. But that does mean that assets loaded from a prior session have these while newly-created ones will not until saved. Maybe not a problem at all, and probably just stating the obvious.

@JulianEisel Should be done with just the following notes: > Think calling this value makes more sense. Same for the tags actually, but yeah, not worth bothering. I couldn't tell from your comment if you wanted this done for traits, tags, both, or none. So I left it alone. Can certainly do whatever you prefer. > ...so let's do it on file save in ED_assets_pre_save() I did so. But that does mean that assets loaded from a prior session have these while newly-created ones will not until saved. Maybe not a problem at all, and probably just stating the obvious.

Almost ready!

@JulianEisel - Just rename it to value, makes more sense to me.

Done. But I also changed other related phrasing around it, so in comments, argument names, etc for consistancy.

Cast this to the enum type, so that the compiler warns when a new enumerator is introduced but not added here.

I did and I was surprised that it worked. Our Object types enum was untyped so I assumed if I added one (here using ObjectType) that it would result in a billion warnings/errors all over the place. But seems to be fine with it. Cool.

> @JulianEisel - Just rename it to value, makes more sense to me. Done. But I also changed other related phrasing around it, so in comments, argument names, etc for consistancy. > Cast this to the enum type, so that the compiler warns when a new enumerator is introduced but not added here. I did and I was surprised that it worked. Our Object types enum was untyped so I assumed if I added one (here using `ObjectType`) that it would result in a billion warnings/errors all over the place. But seems to be fine with it. Cool.

Committed fe7540d39a to add an ObjectType typedef, shouldn't cause any merge conflicts. We should really have that, there are plenty of switch-cases that should cover all enumerators.

Committed fe7540d39a to add an `ObjectType` typedef, shouldn't cause any merge conflicts. We should really have that, there are plenty of `switch`-`cases` that should cover all enumerators.

@JulianEisel - This PR seems to be done? I don't think your last comment (about ObjectType) requested any changes here, but correct me if I am misunderstanding.

Minor points, this is pretty much ready.

@blender-bot build

I would like to have some sign off by @mont29 and/or @brecht on the idea of storing UI names as asset traits. This patch stores the IDTypeInfo.name and OB_UI_NAME_XXX as asset traits, so they become relevant for compatibility.

I would like to have some sign off by @mont29 and/or @brecht on the idea of storing UI names as asset traits. This patch stores the `IDTypeInfo.name` and `OB_UI_NAME_XXX` as asset traits, so they become relevant for compatibility.

I had not seen the design of asset traits before, and I left questions in #105807.

It's not clear to me what it means exactly to have e.g. a "Mesh" trait on an asset. Does it guarantee there exist some interface to query it as a Mesh datablock? Though when used together with an "Object" trait I guess it means there you can get an Object with type mesh instead? But maybe that object has geometry nodes generating curves from the mesh? Is a distinction needed between an asset being a datablock, or being available to be retrieved as a databock?

To me it seems there can be quite a bit of nuance here, where you may need a tooltip to describe it also. And at that point maybe you might as well use an identifier, and then map that to some UI name and description. Which can then also be translated or updated over time.

I'd suggest to clearly define the meaning of traits and have an understanding of how UI and tools are intended to use them, before creating them and storing them in a way that requires us to think about compatibility. Maybe for the first version of traits it's better to only add those that have a specific meaning, like a "use this node group as a mesh modifier" trait.

I had not seen the design of asset traits before, and I left questions in #105807. It's not clear to me what it means exactly to have e.g. a "Mesh" trait on an asset. Does it guarantee there exist some interface to query it as a Mesh datablock? Though when used together with an "Object" trait I guess it means there you can get an Object with type mesh instead? But maybe that object has geometry nodes generating curves from the mesh? Is a distinction needed between an asset being a datablock, or being available to be retrieved as a databock? To me it seems there can be quite a bit of nuance here, where you may need a tooltip to describe it also. And at that point maybe you might as well use an identifier, and then map that to some UI name and description. Which can then also be translated or updated over time. I'd suggest to clearly define the meaning of traits and have an understanding of how UI and tools are intended to use them, before creating them and storing them in a way that requires us to think about compatibility. Maybe for the first version of traits it's better to only add those that have a specific meaning, like a "use this node group as a mesh modifier" trait.

 @ -104,0 +115,4 @@
   return trait;
 }
 AssetTrait *BKE_asset_metadata_trait_add(AssetMetaData *asset_data, const char *name)

 @ -27,0 +30,4 @@
  */
 typedef struct AssetTrait {
   struct AssetTrait *next, *prev;
   char name[64];

 @ -91,0 +103,4 @@
   ListBase traits; /* AssetTrait */
   short active_trait;
   /** Number of traits to avoid counting. Can be moved with tot_tags to runtime data. */
   short tot_traits;

 @ -440,0 +534,4 @@
   RNA_def_struct_name_property(srna, prop);
 }
 static void rna_def_asset_traits_api(BlenderRNA *brna, PropertyRNA *cprop)

 @ -507,0 +641,4 @@
   RNA_def_property_struct_type(prop, "AssetTrait");
   RNA_def_property_editable_func(prop, "rna_AssetMetaData_editable");
   RNA_def_property_ui_text(prop,
                            "Traits",

Labels Milestones

Assets: Implement Traits #105841

Reviewers

 @ -507,0 +646,4 @@
                            "general asset management");
   rna_def_asset_traits_api(brna, prop);
   prop = RNA_def_property(srna, "active_trait", PROP_INT, PROP_NONE);

 @ -1044,0 +1045,4 @@
 {
   bNodeTree &node_tree = *static_cast<bNodeTree *>(asset_ptr);
   BLI_assert(GS(node_tree.id.name) == ID_NT);
   BKE_asset_metadata_trait_ensure(asset_data, node_tree.typeinfo->ui_description);

 @ -1223,0 +1225,4 @@
 {
   Object *ob = (Object *)asset_ptr;
   BLI_assert(GS(ob->id.name) == ID_OB);
   BKE_asset_metadata_trait_ensure(asset_data, get_obdata_defname(ob->type));

 @ -41,1 +41,4 @@
   BKE_asset_metadata_trait_ensure(id->asset_data, "ID");
   BKE_asset_metadata_trait_ensure(id->asset_data, id_type_info->name);
   BKE_asset_metadata_trait_ensure(id->asset_data, id->name + 2);

 @ -25,2 +25,4 @@
 } AssetTag;
 /**
  * \brief System-defined trait.

 @ -26,1 +26,4 @@
 /**
  * \brief System-defined trait.
  * Defined separately from AssetTag because we might want to vary name length.

 @ -136,0 +141,4 @@
   return BLI_sprintfN("asset_data.traits[\"%s\"]", asset_trait_name_esc);
 }
 static int rna_AssetTrait_editable(PointerRNA *ptr, const char **r_info)

 @ -507,0 +550,4 @@
   RNA_def_property_ui_text(prop,
                            "Asset Traits",
                            "Custom traits (name tokens) for the asset, used for filtering and "
                            "general asset management");

 @ -89,2 +99,4 @@
   char _pad[4];
   /** System defined keywords for describing the type of asset. */

 @ -42,1 +48,4 @@
   /** Tags and Traits to match against. These are newly allocated, and compared against the
    * #AssetMetaData.tags and #AssetMetaData.traits. */
   ListBase tags;     /* AssetTag */
   ListBase traits;   /* AssetTrait */

 @ -2144,6 +2152,47 @@ static const char *get_obdata_defname(int type)
   }
 }
 static const char *get_obdata_uiname(int type)

 @ -1223,3 +1231,3 @@
 static AssetTypeInfo AssetType_OB = {
     /*pre_save_fn*/ object_asset_pre_save,
 };
     /*EnsureTraitsFn*/ object_asset_ensure_traits};

 @ -35,2 +35,4 @@
 } AssetTag;
 /**
  * \brief Keywords describing type characteristics of an asset. Not user-defined.

 @ -46,1 +46,4 @@
 /** UI Names for object types. These may be written to files,
  *  so changing these should be done with extreme caution. */
 #define OB_UINAME_MESH "Mesh"

 @ -440,0 +452,4 @@
   srna = RNA_def_struct(brna, "AssetTrait", NULL);
   RNA_def_struct_path_func(srna, "rna_AssetTrait_path");
   RNA_def_struct_ui_text(srna, "Asset Trait", "System defined trait (name token)");

 @ -43,0 +46,4 @@
   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);
   }

 @ -2147,0 +2156,4 @@
 /* Return a string identifying the object type that is *not* translated. */
 static const char *get_obdata_uiname(int type)
 {
   switch (type) {

 @ -101,0 +110,4 @@
   /** See #AssetTrait. */
   ListBase traits;

 @ -48,0 +52,4 @@
 #define OB_UI_NAME_SURF "Surface"
 #define OB_UI_NAME_METABALL "Metaball"
 #define OB_UI_NAME_FONT "Text"
 #define OB_UI_NAME_CURVES "Hair Curves"

Download

What's New

Blender Studio

Manual

Developers Blog

Documentation

Benchmark

Blender Conference

Development Fund

One-time Donations

Labels Milestones

Assets: Implement Traits #105841

Reviewers