WIP: Core: experiment for embedding linked data-blocks #128545

Jacques Lucke · 2024-10-03T18:01:42+02:00

Jacques Lucke commented

2024-10-03 18:01:42 +02:00

This is an initial experiment for storing linked data-blocks in the .blend that links them. This functionality for implementing is a prerequisite for implementing #125389 in a way that keeps embedded data-blocks linked.

Currently, this patch just treats all linked data-blocks as embedded for simplicity. It seems to work for the simple case I tested. I have a bit of a hard time reasoning about how it behaves with recursively linked data when when embedded data-blocks are linked. Another tricky aspect is that versioning of the embedded data has to happen together with the other local data and not together with the other data from the linked library.

Based on a recent discussion with Dalai, Simon and Pablo, I'm currently using the terms "static linking" and "dynamic linking" in the code. "Dynamic" means that the source data-block is loaded from the source file, and "static" that the data-block is embedded.

I'm posting this here to get some initial feedback on the approach which likely is way to simplistic so far.

The patch also contains a debug utility that I originally wrote for #127729. It servers a similar purpose as blend2json.py but produces more useful data for me right now. It just stores a *.debug.txt file next to every saved .blend file.

This is an initial experiment for storing linked data-blocks in the .blend that links them. This functionality for implementing is a prerequisite for implementing #125389 in a way that keeps embedded data-blocks linked. Currently, this patch just treats all linked data-blocks as embedded for simplicity. It seems to work for the simple case I tested. I have a bit of a hard time reasoning about how it behaves with recursively linked data when when embedded data-blocks are linked. Another tricky aspect is that versioning of the embedded data has to happen together with the other local data and not together with the other data from the linked library. Based on a recent discussion with Dalai, Simon and Pablo, I'm currently using the terms "static linking" and "dynamic linking" in the code. "Dynamic" means that the source data-block is loaded from the source file, and "static" that the data-block is embedded. I'm posting this here to get some initial feedback on the approach which likely is way to simplistic so far. The patch also contains a debug utility that I originally wrote for #127729. It servers a similar purpose as `blend2json.py` but produces more useful data for me right now. It just stores a `*.debug.txt` file next to every saved `.blend` file.

❤️ 1

Jacques Lucke added 5 commits 2024-10-03 18:01:54 +02:00

initial embedded linked fb0bfb9c58

add debug print utility 43d96295b8

progress eebcbc3ddd

fix cc4627ae0c

support reading file again

buildbot/vexp-code-patch-lint Build done.

Details

buildbot/vexp-code-patch-linux-x86_64 Build done.

Details

buildbot/vexp-code-patch-darwin-arm64 Build done.

Details

buildbot/vexp-code-patch-darwin-x86_64 Build done.

Details

buildbot/vexp-code-patch-windows-amd64 Build done.

Details

buildbot/vexp-code-patch-coordinator Build done.

Details

5c2d3d2985

Jacques Lucke commented

2024-10-06 19:06:40 +02:00

@blender-bot build

Bastien Montagne reviewed 2024-10-15 14:57:45 +02:00

Bastien Montagne left a comment

Thanks for this initial prototype! Seems to be simple enough and going into the right direction.

Layout of Embedded Linked IDs in Blendfile

The biggest change I would suggest in this initial protoype is how/where embedded linked IDs are stored.

I think they should rather be stored together with the placeholders of regular directly linked IDs, after their Library data-block:

Avoids need for the new library pointers mapping in readfilecode (library read code already sets ID lib pointer to currlib).
Embedded IDs are still read as 'normal' IDs in readcode. In fact, there should be very limitted amount of changes required in readfile code.
This hopefully will also gives us forward compatibility 'for free', existing read code of older Blender simply reading these embedded IDs and considering them as regular linked ones once read (that'd be my expectations at least, needs to be tested!).
Requires write_libraries to not only write placeholders, but also complete ID data in the embedded case.

All in all, I would find this structure more logical (it would also somewhat match how packed libraries are stored), and again, hopefully makes these embedded IDs 'transparently' load as regular linked IDs in older Blenders.

Versionning

We are going to have a fun time managing the fact that IDs belonging to a given file (a library) may have been written by a different version of Blender (in the blendfile using that library).

one solution could be to store the blendfile version in the ID runtime data itself (during initila reading of the ID in readfile code). However, that would require a complete change in how we handle versioning.

Another solution could be to split each library data in more that one Main data-base for the versioning process, based on the version of the actual blendfile from which the ID data comes from. Or have a different variant of blo_split_main, that would generate Mains based on actual blendfile source of the data, and not the 'owner' libraries...

Naming

Based on a recent discussion with Dalai, Simon and Pablo, I'm currently using the terms "static linking" and "dynamic linking" in the code. "Dynamic" means that the source data-block is loaded from the source file, and "static" that the data-block is embedded.

I would very much like to be part of these discussions, as this is technical topic way more than PR/UX topic. And I strongly disagree with the current naming proposal, would much rather stick to naming used in the design task (just linking for regular linking data, and embedded linking for new worked-on sub-type of linking):

Static vs. dynamic does not mean much (if anything) here, I fail to see how regular linking is dynamic?
Embedded is way more correct and matching what actually happens with this new way to manage linked data, so would really keep that in implementation.
If Embedded is seen as not 'good enough' for user-facing purposes, then something like 'Frozen' or 'Archived' would make more sense than 'Static' imho. But unless really needed, would rather have a single naming for both code and UI.
In virtually everything but some data management tasks, and blendfile read/write, the fact that a linked data is embedded should not matter at all. Embedded linking should be seen and conveyed as a sub-part, a variant of regular linking. I would therefore avoid any attempt to formally give a different name to linking itself.

Thanks for this initial prototype! Seems to be simple enough and going into the right direction. ## Layout of Embedded Linked IDs in Blendfile The biggest change I would suggest in this initial protoype is how/where embedded linked IDs are stored. I think they should rather be stored together with the placeholders of regular directly linked IDs, after their `Library` data-block: - Avoids need for the new library pointers mapping in readfilecode (library read code already sets ID `lib` pointer to `currlib`). - Embedded IDs are still read as 'normal' IDs in readcode. In fact, there should be very limitted amount of changes required in readfile code. - This hopefully will also gives us forward compatibility 'for free', existing read code of older Blender simply reading these embedded IDs and considering them as regular linked ones once read (_that'd be my expectations at least, needs to be tested!_). - Requires `write_libraries` to not only write placeholders, but also complete ID data in the embedded case. All in all, I would find this structure more logical (it would also somewhat match how packed libraries are stored), and again, hopefully makes these embedded IDs 'transparently' load as regular linked IDs in older Blenders. ## Versionning We are going to have a fun time managing the fact that IDs belonging to a given file (a library) may have been written by a different version of Blender (in the blendfile using that library). one solution could be to store the blendfile version in the ID runtime data itself (during initila reading of the ID in readfile code). However, that would require a complete change in how we handle versioning. Another solution could be to split each library data in more that one `Main` data-base for the versioning process, based on the version of the actual blendfile from which the ID data comes from. Or have a different variant of `blo_split_main`, that would generate Mains based on actual blendfile source of the data, and not the 'owner' libraries... ## Naming > Based on a recent discussion with Dalai, Simon and Pablo, I'm currently using the terms "static linking" and "dynamic linking" in the code. "Dynamic" means that the source data-block is loaded from the source file, and "static" that the data-block is embedded. I would very much like to be part of these discussions, as this is technical topic way more than PR/UX topic. And I strongly disagree with the current naming proposal, would much rather stick to naming used in the design task (just `linking` for regular linking data, and `embedded linking` for new worked-on sub-type of linking): - Static vs. dynamic does not mean much (if anything) here, I fail to see how regular linking is dynamic? - Embedded is way more correct and matching what actually happens with this new way to manage linked data, so would really keep that in implementation. - If Embedded is seen as not 'good enough' for user-facing purposes, then something like 'Frozen' or 'Archived' would make more sense than 'Static' imho. But unless really needed, would rather have a single naming for both code and UI. - In virtually everything but some data management tasks, and blendfile read/write, the fact that a linked data is embedded should not matter at all. Embedded linking should be seen and conveyed as a sub-part, a variant of regular linking. I would therefore avoid any attempt to formally give a different name to linking itself.

source/blender/blenloader/intern/readfile.cc

						
				@ -3579,2 +3586,4 @@

				  }

				  /* Used to keep track of data-blocks that were statically linked. */

				  blender::MultiValueMap<const void *, ID *> static_linked_data_blocks_by_old_library;

Bastien Montagne commented

2024-10-15 12:46:14 +02:00

If embedded IDs are written as part of their library in blendfile, there should not be any need for this mapping?

source/blender/blenloader/intern/readfile.cc

						
				@ -3707,3 +3723,3 @@

				      blo_split_main(&mainlist, bfd->main);

				      LISTBASE_FOREACH (Main *, mainvar, &mainlist) {

				        BLI_assert(mainvar->versionfile != 0);

				        if (mainvar->versionfile == 0) {

Bastien Montagne commented

2024-10-15 12:45:10 +02:00

This is going to be an interesting topic to get right. We'll need a way to differentiate the handling of these IDs now, since regular linked one need to be processed according to their source lib blendfile version, while embedded ones need to be handled according to their owning user blendfile version (which may also be a library btw ;) ).

Might end up being simpler to store the 'source version' in each ID runtime data, rather than trying to keep track of this?

This is going to be an interesting topic to get right. We'll need a way to differentiate the handling of these IDs now, since regular linked one need to be processed according to their source lib blendfile version, while embedded ones need to be handled according to their owning user blendfile version (which may also be a library btw ;) ). Might end up being simpler to store the 'source version' in each ID runtime data, rather than trying to keep track of this?

source/blender/blenloader/intern/writefile.cc

						
				@ -1279,3 +1287,3 @@

				  }

				  if (cb_flag & IDWALK_CB_DIRECT_WEAK_LINK) {

				  if (cb_flag & IDWALK_CB_DIRECT_WEAK_LINK && ID_IS_DYNAMIC_LINKED(id)) {

Bastien Montagne commented

2024-10-15 12:37:59 +02:00

Would not check this here, but rather update code of id_lib_indirect_weak_link itself to handle (ignore in fact) embedded linked IDs.

Would not check this here, but rather update code of `id_lib_indirect_weak_link` itself to handle (ignore in fact) embedded linked IDs.

source/blender/blenloader/intern/writefile.cc

						
				@ -1381,3 +1391,3 @@

				   * then the temp ones from override process,

				   * if needed, without duplicating whole code. */

				  Main *bmain = mainvar;

				  blender::Vector<Main *> bmains;

Bastien Montagne commented

2024-10-15 12:34:19 +02:00

Don't think that writing embedded linked IDs together with actual local data is a good idea. Would rather write them as part of their libraries.

source/blender/blenloader/intern/writefile.cc

						
				@ -1384,0 +1394,4 @@

				  LISTBASE_FOREACH (Main *, bmain, &mainlist) {

				    bmains.append(bmain);

				  }

				  if (override_storage) {

Bastien Montagne commented

2024-10-15 11:12:54 +02:00

This is fairly theoretical currently, but the override_storage should be written immediately after the 'local' main (i.e. in second position in the bmains vector), since it also contains purely local IDs.

This is fairly theoretical currently, but the `override_storage` should be written immediately after the 'local' `main` (i.e. in second position in the `bmains` vector), since it also contains purely local IDs.

source/blender/blenloader/intern/writefile.cc

						
				@ -1386,3 +1401,4 @@

				  for (Main *bmain : bmains) {

				    ListBase *lbarray[INDEX_ID_MAX];

				    int a = set_listbasepointers(bmain, lbarray);

				    while (a--) {

Bastien Montagne commented

2024-10-15 12:26:32 +02:00

I would move most of the code in this loop into a new utils (something like write_id). Then this can be called both from here for actual local IDs, and from write_libraries for embedded linked data.

Would even do that as a separate 'refactor' PR to get it in main asap and reduce diffing noise in actual PR?

I would move most of the code in this loop into a new utils (something like `write_id`). Then this can be called both from here for actual local IDs, and from `write_libraries` for embedded linked data. Would even do that as a separate 'refactor' PR to get it in `main` asap and reduce diffing noise in actual PR?

source/blender/makesdna/DNA_ID.h

						
				@ -652,6 +652,8 @@ typedef struct PreviewImage {

				#define ID_MISSING(_id) ((((const ID *)(_id))->tag & ID_TAG_MISSING) != 0)

				#define ID_IS_LINKED(_id) (((const ID *)(_id))->lib != NULL)

				#define ID_IS_DYNAMIC_LINKED(_id) (false)

Bastien Montagne commented

2024-10-15 12:36:11 +02:00

Would rather only have anew ID_IS_LINKED_EMBEDDED for the time being, and check !ID_IS_LINKED_EMBEDDED when only handling 'regular' linked data is needed.

Would rather only have anew `ID_IS_LINKED_EMBEDDED` for the time being, and check `!ID_IS_LINKED_EMBEDDED` when only handling 'regular' linked data is needed.