Improve handling of embedded data-blocks (root nodetrees and master collections) #69169

New Issue

Brecht Van Lommel · 2019-08-26T13:25:32+02:00

Brecht Van Lommel commented

2019-08-26 13:25:32 +02:00

Embedded data-blocks often require specific handling in ID management (freeing, duplicating, referencing, RNA paths, etc.).

Characteristics of embedded IDs:

They have one and only one owner ID.
They are flagged as such with LIB_EMBEDDED_DATA. This owner ID takes care of their management (creation, duplication, deletion...).
** IDTypeInfo provides a way to get that unique owner ID from an embedded one.
They are not stored in Main database.
However, they can be referenced by other IDs (e.g. master collection of scene can be a parent of many other collections). New embedded IDs are created with LIB_TAG_NO_MAIN, however existing old ones may not have it, and there is no do_version to set it on those...
They do have their own animation data.
Their name is not unique.
They do have a valid session_uuid.
They are stored in .blend files as 'regular' data, they do not have the bhead->code set to their ID type.

Proposals for Improvements

Here are ideas to help improve handling of the embedded IDs.

1 - Storing a reference to owner ID in embedded ones

While 'loop-back' ID pointers can be problematic, their support in general ID management code is now fairly safe and sound. This would solve the current issue where we have to loop over all potential owner IDs to find the proper one.

- Store that id_owner pointer in ID struct itself, or in data of each specific ID type? D15838, cd49fee741
  ** ~~Would rather go for generic pointer in ID struct.~~

2 - Tags and Library of embedded IDs should match their owner's

Embedded IDs should share the same 'ID management' tags as their owner ID (if owner ID is LIB_TAG_NO_MAIN, its embedded ones are too, etc.). Embedded IDs are not stored in G_MAIN, but are often handled as such, can be referenced by other regular IDs (like master collection being parent of other collections), etc. Similarly, they should also always share the same Library pointer as their owners.

- Needs to ensure tags are in sync with their owners (need to check exactly which ones need it, document it, and probably add an API to set tags that ensures this behavior). This should wait for #88555/T90610 to be addressed.
- Needs to ensure lib pointers are always in sync. //Seems to already be the case, 788952705c just adds some extra asserts to double-check that on file read.

3 - Generic 'foreach' looper over embedded pointers only

This could help making some of IDManagement related tasks more generic when dealing with embedded pointers, needs to be investigated further.

There can be two approaches:

Add an extra flag option to the existing foreach_id API (aka BKE_lib_query). This imply IDTypeInfo.foreach_id callbacks of types that may own embedded IDs to be aware of this option, to optimize their code by only processing those embedded pointers. Least effort solution, but requires exposing some info from generic foreach_id code into types callbacks. not necessarily a bad thing though.
Add a new dedicated callback to IDTypeInfo, that would be somewhat the complement to the existing owner_pointer_get one.

Original task (note that since 2019, we clearly moved towards the second solution described below):

bNodeTree is currently both a datablock, and embedded in materials. Same for collection and master collection of a scene.

This leads to messy datablock management and problems creating RNA paths (e.g. #68971).

A solution to this could be to make bNodeTree node longer a datablock. Instead it would be embedded in Material, World, Light, and a new NodeGroup datablock.
This will require Python API breakage and some relatively complicated versioning, but the resulting design should be cleaner.

Another solution is to design a proper handling of those private data-blocks all around our code base (ID/Lib management in BKE_library area, RNA handling of IDs, etc.). We already do that to some point, but so far it has mostly been hacks around issues on a case-by-case basis.
Solution to #68971 already added a new ID flag to mark those private datablocks, e.g. we could extend RNA handling of IDs to detect that one and automatically consider those structs as the other private sub-data ones (like e.g. render settings from scene, etc.).

Embedded data-blocks often require specific handling in ID management (freeing, duplicating, referencing, RNA paths, etc.). Characteristics of embedded IDs: * They have one and only one owner ID. **They are flagged as such with `LIB_EMBEDDED_DATA`.** This owner ID takes care of their management (creation, duplication, deletion...). ** IDTypeInfo provides a way to get that unique owner ID from an embedded one. * They are not stored in Main database. **However, they can be referenced by other IDs *(e.g. master collection of scene can be a parent of many other collections)*.** New embedded IDs are created with `LIB_TAG_NO_MAIN`, however existing old ones may not have it, and there is no `do_version` to set it on those... * They do have their own animation data. * Their name is not unique. * They do have a valid `session_uuid`. * They are stored in .blend files as 'regular' data, they do not have the `bhead->code` set to their ID type. Proposals for Improvements **** Here are ideas to help improve handling of the embedded IDs. 1 - Storing a reference to owner ID in embedded ones ---------------------------------------------------------------- While 'loop-back' ID pointers can be problematic, their support in general ID management code is now fairly safe and sound. This would solve the current issue where we have to loop over all potential owner IDs to find the proper one. * - [x] Store that `id_owner` pointer in `ID` struct itself, or in data of each specific ID type? [D15838](https://archive.blender.org/developer/D15838), cd49fee741 ** *~~Would rather go for generic pointer in `ID` struct.~~* 2 - Tags and Library of embedded IDs should match their owner's ----------------------------------------------------------------------------- Embedded IDs should share the same 'ID management' tags as their owner ID (if owner ID is `LIB_TAG_NO_MAIN`, its embedded ones are too, etc.). Embedded IDs are not stored in `G_MAIN`, but are often handled as such, can be referenced by other regular IDs (like master collection being parent of other collections), etc. Similarly, they should also always share the same Library pointer as their owners. * - [ ] Needs to ensure tags are in sync with their owners (need to check exactly which ones need it, document it, and probably add an API to set tags that ensures this behavior). *This should wait for #88555/T90610 to be addressed.* * - [x] Needs to ensure lib pointers are always in sync. //Seems to already be the case, 788952705c just adds some extra asserts to double-check that on file read. 3 - Generic 'foreach' looper over embedded pointers only ------------------------------------------------------------------- This could help making some of IDManagement related tasks more generic when dealing with embedded pointers, needs to be investigated further. There can be two approaches: - Add an extra flag option to the existing `foreach_id` API (aka `BKE_lib_query`). This imply `IDTypeInfo.foreach_id` callbacks of types that may own embedded IDs to be aware of this option, to optimize their code by only processing those embedded pointers. *Least effort solution, but requires exposing some info from generic foreach_id code into types callbacks. not necessarily a bad thing though.* - Add a new dedicated callback to `IDTypeInfo`, that would be somewhat the complement to the existing `owner_pointer_get` one. ----------------------- *Original task (note that since 2019, we clearly moved towards the second solution described below):* `bNodeTree` is currently both a datablock, and embedded in materials. Same for collection and master collection of a scene. This leads to messy datablock management and problems creating RNA paths (e.g. #68971). A solution to this could be to make `bNodeTree` node longer a datablock. Instead it would be embedded in `Material`, `World`, `Light`, and a new `NodeGroup` datablock. This will require Python API breakage and some relatively complicated versioning, but the resulting design should be cleaner. Another solution is to design a proper handling of those private data-blocks all around our code base (ID/Lib management in BKE_library area, RNA handling of IDs, etc.). We already do that to some point, but so far it has mostly been hacks around issues on a case-by-case basis. Solution to #68971 already added a new ID flag to mark those private datablocks, e.g. we could extend RNA handling of IDs to detect that one and automatically consider those structs as the other private sub-data ones (like e.g. render settings from scene, etc.).

Brecht Van Lommel commented

2019-08-26 13:25:32 +02:00

Added subscriber: @brecht

blender-admin commented

2019-08-26 13:25:32 +02:00

#86119 was marked as duplicate of this issue

Jacques Lucke commented

2019-08-26 14:21:27 +02:00

Added subscriber: @JacquesLucke

MACHIN3 commented

2019-08-28 21:45:34 +02:00

Added subscriber: @MACHIN3

Bastien Montagne changed title from ~~Improve embedded nodetree storage~~ to Improve embedded nodetree/master collection storage & handling

2019-08-29 15:51:25 +02:00

Pipeliner commented

2019-11-28 18:30:24 +01:00

Added subscriber: @Pipeliner

Bartosz Moniewski commented

2020-01-27 00:35:38 +01:00

Added subscriber: @BartekMoniewski

Ryan Inch commented

2020-05-17 03:16:30 +02:00

Added subscriber: @Imaginer

Bastien Montagne changed title from ~~Improve embedded nodetree/master collection storage & handling~~ to Improve handling of embedded data-blocks (root nodetrees and master collections)

2021-02-25 11:38:31 +01:00

Bastien Montagne commented

2021-10-27 15:02:01 +02:00

Added subscriber: @mont29

Brecht Van Lommel commented

2021-11-02 19:03:28 +01:00

This seems generally fine.

I'd personally try to save memory and put id_owner in specific datablocks that are embedded, but not a big deal either way.

This seems generally fine. I'd personally try to save memory and put `id_owner` in specific datablocks that are embedded, but not a big deal either way.

Philipp Oeser removed the

Download

What's New

Blender Studio

Manual

Developers Blog

Documentation

Benchmark

Blender Conference

Development Fund

One-time Donations

Improve handling of embedded data-blocks (root nodetrees and master collections) #69169

1 - Storing a reference to owner ID in embedded ones

2 - Tags and Library of embedded IDs should match their owner's

3 - Generic 'foreach' looper over embedded pointers only