From e615947d6ac3c3098dd6e5eec5e799d411cb3ad7 Mon Sep 17 00:00:00 2001 From: Bastien Montagne Date: Mon, 17 Apr 2023 17:17:02 +0200 Subject: [PATCH] BKE: Rework ID swap code to properly handle embedded ID pointers. While embedded IDs are usually considered as private local data of their owner ID, some areas of code, like the depsgraph, can consider them as regular IDs in some aspects. So when swapping IDs, also properly 'counter-swap' their potential embedded IDs, such that the pointers to the embedded IDs remain as before swapping, even though the data of the embedded IDs is swapped. The main target of this change is memfile undo code. There, newly read IDs are swapped with their oldder version, so that the old address contains the new data. This allows to avoid rebuilding some of the depsgraph. Doing the same thing for embedded IDs should reduce even further the needs for depsgrah rebuilds on undo steps. This commit also gives more control over the remapping of 'self' ID pointers inside themselves. --- source/blender/blenkernel/BKE_lib_id.h | 21 +++- source/blender/blenkernel/intern/brush.cc | 2 +- source/blender/blenkernel/intern/lib_id.c | 108 ++++++++++++++++-- .../blender/blenkernel/intern/lib_override.cc | 2 +- source/blender/blenkernel/intern/paint.cc | 2 +- source/blender/blenloader/intern/readfile.cc | 16 ++- 6 files changed, 129 insertions(+), 22 deletions(-) diff --git a/source/blender/blenkernel/BKE_lib_id.h b/source/blender/blenkernel/BKE_lib_id.h index ca734872502..a96791707e8 100644 --- a/source/blender/blenkernel/BKE_lib_id.h +++ b/source/blender/blenkernel/BKE_lib_id.h @@ -464,18 +464,27 @@ struct ID *BKE_id_copy_for_use_in_bmain(struct Main *bmain, const struct ID *id) * Does a mere memory swap over the whole IDs data (including type-specific memory). * \note Most internal ID data itself is not swapped (only IDProperties are). * - * \param bmain: May be NULL, in which case there will be no remapping of internal pointers to - * itself. + * \param bmain: May be NULL, in which case there is no guarantee that internal remapping of ID + * pointers to themselves will be complete (reguarding depsgraph and/or runtime data updates). + * \param do_self_remap: Whether to remap internal pointers to itself or not. + * \param self_remap_flags: Flags controlling self remapping, see BKE_lib_remap.h. */ -void BKE_lib_id_swap(struct Main *bmain, struct ID *id_a, struct ID *id_b); +void BKE_lib_id_swap(struct Main *bmain, + struct ID *id_a, + struct ID *id_b, + const bool do_self_remap, + const int self_remap_flags); /** * Does a mere memory swap over the whole IDs data (including type-specific memory). * \note All internal ID data itself is also swapped. * - * \param bmain: May be NULL, in which case there will be no remapping of internal pointers to - * itself. + * For parameters description, see #BKE_lib_id_swap above. */ -void BKE_lib_id_swap_full(struct Main *bmain, struct ID *id_a, struct ID *id_b); +void BKE_lib_id_swap_full(struct Main *bmain, + struct ID *id_a, + struct ID *id_b, + const bool do_self_remap, + const int self_remap_flags); /** * Sort given \a id into given \a lb list, using case-insensitive comparison of the id names. diff --git a/source/blender/blenkernel/intern/brush.cc b/source/blender/blenkernel/intern/brush.cc index b72f19abd8a..01652c7e2c3 100644 --- a/source/blender/blenkernel/intern/brush.cc +++ b/source/blender/blenkernel/intern/brush.cc @@ -416,7 +416,7 @@ static int brush_undo_preserve_cb(LibraryIDLinkCallbackData *cb_data) static void brush_undo_preserve(BlendLibReader *reader, ID *id_new, ID *id_old) { /* Whole Brush is preserved across undo-steps. */ - BKE_lib_id_swap(nullptr, id_new, id_old); + BKE_lib_id_swap(nullptr, id_new, id_old, false, 0); /* `id_new` now has content from `id_old`, we need to ensure those old ID pointers are valid. * NOTE: Since we want to re-use all old pointers here, code is much simpler than for Scene. */ diff --git a/source/blender/blenkernel/intern/lib_id.c b/source/blender/blenkernel/intern/lib_id.c index 0d9ff62a2ed..638aa6d9b7b 100644 --- a/source/blender/blenkernel/intern/lib_id.c +++ b/source/blender/blenkernel/intern/lib_id.c @@ -765,14 +765,38 @@ ID *BKE_id_copy_for_use_in_bmain(Main *bmain, const ID *id) return newid; } +static void id_embedded_swap(ID **embedded_id_a, + ID **embedded_id_b, + const bool do_full_id, + struct IDRemapper *remapper_id_a, + struct IDRemapper *remapper_id_b); + /** * Does a mere memory swap over the whole IDs data (including type-specific memory). * \note Most internal ID data itself is not swapped (only IDProperties are). */ -static void id_swap(Main *bmain, ID *id_a, ID *id_b, const bool do_full_id) +static void id_swap(Main *bmain, + ID *id_a, + ID *id_b, + const bool do_full_id, + const bool do_self_remap, + struct IDRemapper *input_remapper_id_a, + struct IDRemapper *input_remapper_id_b, + const int self_remap_flags) { BLI_assert(GS(id_a->name) == GS(id_b->name)); + struct IDRemapper *remapper_id_a = input_remapper_id_a; + struct IDRemapper *remapper_id_b = input_remapper_id_b; + if (do_self_remap) { + if (remapper_id_a == NULL) { + remapper_id_a = BKE_id_remapper_create(); + } + if (remapper_id_b == NULL) { + remapper_id_b = BKE_id_remapper_create(); + } + } + const IDTypeInfo *id_type = BKE_idtype_get_info_from_id(id_a); BLI_assert(id_type != NULL); const size_t id_struct_size = id_type->struct_size; @@ -799,21 +823,87 @@ static void id_swap(Main *bmain, ID *id_a, ID *id_b, const bool do_full_id) id_b->recalc = id_a_back.recalc; } - if (bmain != NULL) { - /* Swap will have broken internal references to itself, restore them. */ - BKE_libblock_relink_ex(bmain, id_a, id_b, id_a, ID_REMAP_SKIP_NEVER_NULL_USAGE); - BKE_libblock_relink_ex(bmain, id_b, id_a, id_b, ID_REMAP_SKIP_NEVER_NULL_USAGE); + id_embedded_swap((ID **)BKE_ntree_ptr_from_id(id_a), + (ID **)BKE_ntree_ptr_from_id(id_b), + do_full_id, + remapper_id_a, + remapper_id_b); + if (GS(id_a->name) == ID_SCE) { + Scene *scene_a = (Scene *)id_a; + Scene *scene_b = (Scene *)id_b; + id_embedded_swap((ID **)&scene_a->master_collection, + (ID **)&scene_b->master_collection, + do_full_id, + remapper_id_a, + remapper_id_b); + } + + if (remapper_id_a != NULL) { + BKE_id_remapper_add(remapper_id_a, id_b, id_a); + } + if (remapper_id_b != NULL) { + BKE_id_remapper_add(remapper_id_b, id_a, id_b); + } + + /* Finalize remapping of internal referrences to self broken by swapping, if requested. */ + if (do_self_remap) { + LinkNode ids = {.next = NULL, .link = id_a}; + BKE_libblock_relink_multiple( + bmain, &ids, ID_REMAP_TYPE_REMAP, remapper_id_a, self_remap_flags); + ids.link = id_b; + BKE_libblock_relink_multiple( + bmain, &ids, ID_REMAP_TYPE_REMAP, remapper_id_b, self_remap_flags); + } + + if (input_remapper_id_a == NULL && remapper_id_a != NULL) { + BKE_id_remapper_free(remapper_id_a); + } + if (input_remapper_id_b == NULL && remapper_id_b != NULL) { + BKE_id_remapper_free(remapper_id_b); } } -void BKE_lib_id_swap(Main *bmain, ID *id_a, ID *id_b) +/* Conceptually, embedded IDs are part of their owner's data. However, some parts of the code + * (like e.g. the depsgraph) may treat them as independant IDs, so swapping them here and + * switching their pointers in the owner IDs allows to help not break cached relationships and + * such (by preserving the pointer values). */ +static void id_embedded_swap(ID **embedded_id_a, + ID **embedded_id_b, + const bool do_full_id, + struct IDRemapper *remapper_id_a, + struct IDRemapper *remapper_id_b) { - id_swap(bmain, id_a, id_b, false); + if (embedded_id_a != NULL && *embedded_id_a != NULL) { + BLI_assert(embedded_id_b != NULL && *embedded_id_b != NULL); + + /* Do not remap internal references to itself here, since embedded IDs pointers also need to be + * potentially remapped in owner ID's data, which will also handle embedded IDs data. */ + id_swap( + NULL, *embedded_id_a, *embedded_id_b, do_full_id, false, remapper_id_a, remapper_id_b, 0); + /* Manual 'remap' of owning embedded pointer in owner ID. */ + SWAP(ID *, *embedded_id_a, *embedded_id_b); + + /* Restore internal pointers to the swapped embedded IDs in their owners' data. This also + * includes the potential self-references inside the embedded IDs themselves. */ + if (remapper_id_a != NULL) { + BKE_id_remapper_add(remapper_id_a, *embedded_id_b, *embedded_id_a); + } + if (remapper_id_b != NULL) { + BKE_id_remapper_add(remapper_id_b, *embedded_id_a, *embedded_id_b); + } + } } -void BKE_lib_id_swap_full(Main *bmain, ID *id_a, ID *id_b) +void BKE_lib_id_swap( + Main *bmain, ID *id_a, ID *id_b, const bool do_self_remap, const int self_remap_flags) { - id_swap(bmain, id_a, id_b, true); + id_swap(bmain, id_a, id_b, false, do_self_remap, NULL, NULL, self_remap_flags); +} + +void BKE_lib_id_swap_full( + Main *bmain, ID *id_a, ID *id_b, const bool do_self_remap, const int self_remap_flags) +{ + id_swap(bmain, id_a, id_b, true, do_self_remap, NULL, NULL, self_remap_flags); } bool id_single_user(bContext *C, ID *id, PointerRNA *ptr, PropertyRNA *prop) diff --git a/source/blender/blenkernel/intern/lib_override.cc b/source/blender/blenkernel/intern/lib_override.cc index eec961d6dd6..1afe4d4d280 100644 --- a/source/blender/blenkernel/intern/lib_override.cc +++ b/source/blender/blenkernel/intern/lib_override.cc @@ -3784,7 +3784,7 @@ void BKE_lib_override_library_main_unused_cleanup(Main *bmain) static void lib_override_id_swap(Main *bmain, ID *id_local, ID *id_temp) { - BKE_lib_id_swap(bmain, id_local, id_temp); + BKE_lib_id_swap(bmain, id_local, id_temp, true, 0); /* We need to keep these tags from temp ID into orig one. * ID swap does not swap most of ID data itself. */ id_local->tag |= (id_temp->tag & LIB_TAG_LIB_OVERRIDE_NEED_RESYNC); diff --git a/source/blender/blenkernel/intern/paint.cc b/source/blender/blenkernel/intern/paint.cc index 04784fe9378..b67696432d4 100644 --- a/source/blender/blenkernel/intern/paint.cc +++ b/source/blender/blenkernel/intern/paint.cc @@ -130,7 +130,7 @@ static void palette_undo_preserve(BlendLibReader * /*reader*/, ID *id_new, ID *i /* NOTE: We do not care about potential internal references to self here, Palette has none. */ /* NOTE: We do not swap IDProperties, as dealing with potential ID pointers in those would be * fairly delicate. */ - BKE_lib_id_swap(nullptr, id_new, id_old); + BKE_lib_id_swap(nullptr, id_new, id_old, false, 0); std::swap(id_new->properties, id_old->properties); } diff --git a/source/blender/blenloader/intern/readfile.cc b/source/blender/blenloader/intern/readfile.cc index 1f1c5a48758..55d1bb29568 100644 --- a/source/blender/blenloader/intern/readfile.cc +++ b/source/blender/blenloader/intern/readfile.cc @@ -72,6 +72,7 @@ #include "BKE_lib_id.h" #include "BKE_lib_override.h" #include "BKE_lib_query.h" +#include "BKE_lib_remap.h" #include "BKE_main.h" /* for Main */ #include "BKE_main_idmap.h" #include "BKE_material.h" @@ -3095,10 +3096,17 @@ static void read_libblock_undo_restore_at_old_address(FileData *fd, Main *main, BLI_remlink(old_lb, id_old); BLI_remlink(new_lb, id); - /* We do not need any remapping from this call here, since no ID pointer is valid in the data - * currently (they are all pointing to old addresses, and need to go through `lib_link` - * process). So we can pass nullptr for the Main pointer parameter. */ - BKE_lib_id_swap_full(nullptr, id, id_old); + /* We do need remapping of internal pointers to the ID itself here. + * + * Passing a NULL BMain means that not all potential runtime data (like collections' parent + * pointers etc.) will be up-to-date. However, this should not be a problem here, since these + * data are re-generated later in fileread process anyway.. */ + BKE_lib_id_swap_full(nullptr, + id, + id_old, + true, + ID_REMAP_SKIP_NEVER_NULL_USAGE | ID_REMAP_SKIP_UPDATE_TAGGING | + ID_REMAP_SKIP_USER_REFCOUNT); /* Special temporary usage of this pointer, necessary for the `undo_preserve` call after * lib-linking to restore some data that should never be affected by undo, e.g. the 3D cursor of -- 2.30.2