From b9becc47de4ead8ea4c7d43977f29b18fb3ae6d5 Mon Sep 17 00:00:00 2001 From: Bastien Montagne Date: Sat, 8 Jul 2023 16:16:49 +0200 Subject: [PATCH] Core: ID naming: Add 'global' namemap. This new namemap allows to generate new ID names which are unique in current Main (for a given ID type), regardless of the library they belong to. This new feature will be used by library override to try to reduce name collisions with its linked reference IDs when more than one override exists. It is not intended to be used for general ID naming. --- source/blender/blenkernel/BKE_main.h | 4 + source/blender/blenkernel/BKE_main_namemap.h | 8 +- source/blender/blenkernel/intern/blendfile.cc | 9 +- source/blender/blenkernel/intern/lib_id.c | 2 +- .../blender/blenkernel/intern/lib_id_test.cc | 95 ++++++++++++++- source/blender/blenkernel/intern/main.c | 3 + .../blender/blenkernel/intern/main_namemap.cc | 113 ++++++++++++++---- 7 files changed, 199 insertions(+), 35 deletions(-) diff --git a/source/blender/blenkernel/BKE_main.h b/source/blender/blenkernel/BKE_main.h index e0f0b9fe11e..fcea5e67d59 100644 --- a/source/blender/blenkernel/BKE_main.h +++ b/source/blender/blenkernel/BKE_main.h @@ -240,6 +240,10 @@ typedef struct Main { /** Used for efficient calculations of unique names. */ struct UniqueName_Map *name_map; + /* Used for efficient calculations of unique names. Covers all names in current Main, including + * linked data ones. */ + struct UniqueName_Map *name_map_global; + struct MainLock *lock; } Main; diff --git a/source/blender/blenkernel/BKE_main_namemap.h b/source/blender/blenkernel/BKE_main_namemap.h index 70d897827cb..b85b2228174 100644 --- a/source/blender/blenkernel/BKE_main_namemap.h +++ b/source/blender/blenkernel/BKE_main_namemap.h @@ -45,9 +45,15 @@ void BKE_main_namemap_clear(struct Main *bmain) ATTR_NONNULL(); * * In case of name collisions, the name will be adjusted to be unique. * + * \param do_unique_in_bmain if `true`, ensure that the final name is unique in the whole Main (for + * the given ID type), not only in the set of IDs from the same library. + * * \return true if the name had to be adjusted for uniqueness. */ -bool BKE_main_namemap_get_name(struct Main *bmain, struct ID *id, char *name) ATTR_NONNULL(); +bool BKE_main_namemap_get_name(struct Main *bmain, + struct ID *id, + char *name, + const bool do_unique_in_bmain) ATTR_NONNULL(); /** * Remove a given name from usage. diff --git a/source/blender/blenkernel/intern/blendfile.cc b/source/blender/blenkernel/intern/blendfile.cc index c2e5ce97d05..d6bf4fa807a 100644 --- a/source/blender/blenkernel/intern/blendfile.cc +++ b/source/blender/blenkernel/intern/blendfile.cc @@ -337,15 +337,10 @@ static void swap_old_bmain_data_for_blendfile(ReuseOldBMainData *reuse_data, con SWAP(ListBase, *new_lb, *old_lb); - /* Since all IDs here are supposed to be local, no need to call #BKE_main_namemap_clear. */ /* TODO: Could add per-IDType control over namemaps clearing, if this becomes a performances * concern. */ - if (old_bmain->name_map != nullptr) { - BKE_main_namemap_destroy(&old_bmain->name_map); - } - if (new_bmain->name_map != nullptr) { - BKE_main_namemap_destroy(&new_bmain->name_map); - } + BKE_main_namemap_clear(old_bmain); + BKE_main_namemap_clear(new_bmain); /* Original 'new' IDs have been moved into the old listbase and will be discarded (deleted). * Original 'old' IDs have been moved into the new listbase and are being reused (kept). diff --git a/source/blender/blenkernel/intern/lib_id.c b/source/blender/blenkernel/intern/lib_id.c index 2a44edcea0c..714af1ef616 100644 --- a/source/blender/blenkernel/intern/lib_id.c +++ b/source/blender/blenkernel/intern/lib_id.c @@ -1625,7 +1625,7 @@ bool BKE_id_new_name_validate( BLI_str_utf8_invalid_strip(name, strlen(name)); } - result = BKE_main_namemap_get_name(bmain, id, name); + result = BKE_main_namemap_get_name(bmain, id, name, false); BLI_strncpy(id->name + 2, name, sizeof(id->name) - 2); id_sort_by_name(lb, id, NULL); diff --git a/source/blender/blenkernel/intern/lib_id_test.cc b/source/blender/blenkernel/intern/lib_id_test.cc index b3b9c9f71e2..f75bde3c957 100644 --- a/source/blender/blenkernel/intern/lib_id_test.cc +++ b/source/blender/blenkernel/intern/lib_id_test.cc @@ -57,6 +57,8 @@ TEST(lib_id_main_sort, local_ids_1) EXPECT_TRUE(ctx.bmain->objects.first == id_a); EXPECT_TRUE(ctx.bmain->objects.last == id_c); test_lib_id_main_sort_check_order({id_a, id_b, id_c}); + + EXPECT_EQ(ctx.bmain->name_map_global, nullptr); } static void change_lib(Main *bmain, ID *id, Library *lib) @@ -66,7 +68,7 @@ static void change_lib(Main *bmain, ID *id, Library *lib) } BKE_main_namemap_remove_name(bmain, id, id->name + 2); id->lib = lib; - BKE_main_namemap_get_name(bmain, id, id->name + 2); + BKE_main_namemap_get_name(bmain, id, id->name + 2, false); } static void change_name(Main *bmain, ID *id, const char *name) @@ -108,6 +110,8 @@ TEST(lib_id_main_sort, linked_ids_1) test_lib_id_main_sort_check_order({id_c, id_a, id_b}); EXPECT_TRUE(BKE_main_namemap_validate(ctx.bmain)); + + EXPECT_EQ(ctx.bmain->name_map_global, nullptr); } TEST(lib_id_main_unique_name, local_ids_1) @@ -131,6 +135,8 @@ TEST(lib_id_main_unique_name, local_ids_1) test_lib_id_main_sort_check_order({id_a, id_c, id_b}); EXPECT_TRUE(BKE_main_namemap_validate(ctx.bmain)); + + EXPECT_EQ(ctx.bmain->name_map_global, nullptr); } TEST(lib_id_main_unique_name, linked_ids_1) @@ -170,6 +176,73 @@ TEST(lib_id_main_unique_name, linked_ids_1) test_lib_id_main_sort_check_order({id_c, id_a, id_b}); EXPECT_TRUE(BKE_main_namemap_validate(ctx.bmain)); + + EXPECT_EQ(ctx.bmain->name_map_global, nullptr); +} + +static void change_name_global(Main *bmain, ID *id, const char *name) +{ + BKE_main_namemap_remove_name(bmain, id, id->name + 2); + BLI_strncpy(id->name + 2, name, MAX_NAME); + + BKE_main_namemap_get_name(bmain, id, id->name + 2, true); + + id_sort_by_name(&bmain->objects, id, nullptr); +} + +TEST(lib_id_main_global_unique_name, linked_ids_1) +{ + LibIDMainSortTestContext ctx; + EXPECT_TRUE(BLI_listbase_is_empty(&ctx.bmain->libraries)); + + Library *lib_a = static_cast(BKE_id_new(ctx.bmain, ID_LI, "LI_A")); + Library *lib_b = static_cast(BKE_id_new(ctx.bmain, ID_LI, "LI_B")); + ID *id_c = static_cast(BKE_id_new(ctx.bmain, ID_OB, "OB_C")); + ID *id_a = static_cast(BKE_id_new(ctx.bmain, ID_OB, "OB_A")); + ID *id_b = static_cast(BKE_id_new(ctx.bmain, ID_OB, "OB_B")); + + EXPECT_TRUE(BKE_main_namemap_validate(ctx.bmain)); + + change_lib(ctx.bmain, id_a, lib_a); + id_sort_by_name(&ctx.bmain->objects, id_a, nullptr); + change_lib(ctx.bmain, id_b, lib_b); + id_sort_by_name(&ctx.bmain->objects, id_b, nullptr); + + change_name_global(ctx.bmain, id_b, "OB_A"); + EXPECT_NE(ctx.bmain->name_map_global, nullptr); + EXPECT_STREQ(id_b->name + 2, "OB_A.001"); + EXPECT_STREQ(id_a->name + 2, "OB_A"); + EXPECT_TRUE(ctx.bmain->objects.first == id_c); + EXPECT_TRUE(ctx.bmain->objects.last == id_b); + test_lib_id_main_sort_check_order({id_c, id_a, id_b}); + + EXPECT_TRUE(BKE_main_namemap_validate(ctx.bmain)); + + change_lib(ctx.bmain, id_b, lib_a); + id_sort_by_name(&ctx.bmain->objects, id_b, nullptr); + change_name_global(ctx.bmain, id_b, "OB_C"); + EXPECT_STREQ(id_b->name + 2, "OB_C.001"); + EXPECT_STREQ(id_a->name + 2, "OB_A"); + EXPECT_STREQ(id_c->name + 2, "OB_C"); + change_name_global(ctx.bmain, id_a, "OB_C"); + EXPECT_STREQ(id_b->name + 2, "OB_C.001"); + EXPECT_STREQ(id_a->name + 2, "OB_C.002"); + EXPECT_STREQ(id_c->name + 2, "OB_C"); + EXPECT_TRUE(ctx.bmain->objects.first == id_c); + EXPECT_TRUE(ctx.bmain->objects.last == id_a); + test_lib_id_main_sort_check_order({id_c, id_b, id_a}); + + EXPECT_TRUE(BKE_main_namemap_validate(ctx.bmain)); + + change_name(ctx.bmain, id_b, "OB_C"); + EXPECT_STREQ(id_b->name + 2, "OB_C"); + EXPECT_STREQ(id_a->name + 2, "OB_C.002"); + EXPECT_STREQ(id_c->name + 2, "OB_C"); + EXPECT_TRUE(ctx.bmain->objects.first == id_c); + EXPECT_TRUE(ctx.bmain->objects.last == id_a); + test_lib_id_main_sort_check_order({id_c, id_b, id_a}); + + EXPECT_TRUE(BKE_main_namemap_validate(ctx.bmain)); } TEST(lib_id_main_unique_name, ids_sorted_by_default) @@ -183,6 +256,8 @@ TEST(lib_id_main_unique_name, ids_sorted_by_default) test_lib_id_main_sort_check_order({id_bar, id_baz, id_foo, id_yes}); EXPECT_TRUE(BKE_main_namemap_validate(ctx.bmain)); + + EXPECT_EQ(ctx.bmain->name_map_global, nullptr); } static ID *add_id_in_library(Main *bmain, const char *name, Library *lib) @@ -213,6 +288,8 @@ TEST(lib_id_main_unique_name, ids_sorted_by_default_with_libraries) test_lib_id_main_sort_check_order({id_bar, id_baz, id_foo, id_yes, id_l1a, id_l1c, id_l2b}); EXPECT_TRUE(BKE_main_namemap_validate(ctx.bmain)); + + EXPECT_EQ(ctx.bmain->name_map_global, nullptr); } TEST(lib_id_main_unique_name, name_too_long_handling) @@ -231,6 +308,8 @@ TEST(lib_id_main_unique_name, name_too_long_handling) EXPECT_STREQ(id_c->name + 2, "Name_That_Has_Too_Long_Number_Suffix.1234567890"); /* Unchanged */ EXPECT_TRUE(BKE_main_namemap_validate(ctx.bmain)); + + EXPECT_EQ(ctx.bmain->name_map_global, nullptr); } TEST(lib_id_main_unique_name, create_equivalent_numeric_suffixes) @@ -292,6 +371,8 @@ TEST(lib_id_main_unique_name, create_equivalent_numeric_suffixes) EXPECT_STREQ(id_k->name + 2, "Foo..004"); EXPECT_TRUE(BKE_main_namemap_validate(ctx.bmain)); + + EXPECT_EQ(ctx.bmain->name_map_global, nullptr); } TEST(lib_id_main_unique_name, zero_suffix_is_never_assigned) @@ -309,6 +390,8 @@ TEST(lib_id_main_unique_name, zero_suffix_is_never_assigned) EXPECT_STREQ(id_003->name + 2, "Foo.003"); EXPECT_TRUE(BKE_main_namemap_validate(ctx.bmain)); + + EXPECT_EQ(ctx.bmain->name_map_global, nullptr); } TEST(lib_id_main_unique_name, remove_after_dup_get_original_name) @@ -328,6 +411,8 @@ TEST(lib_id_main_unique_name, remove_after_dup_get_original_name) EXPECT_STREQ(id_a->name + 2, "Foo"); EXPECT_TRUE(BKE_main_namemap_validate(ctx.bmain)); + + EXPECT_EQ(ctx.bmain->name_map_global, nullptr); } TEST(lib_id_main_unique_name, name_number_suffix_assignment) @@ -398,6 +483,8 @@ TEST(lib_id_main_unique_name, name_number_suffix_assignment) EXPECT_STREQ(id_fo180->name + 2, "Fo.180"); EXPECT_TRUE(BKE_main_namemap_validate(ctx.bmain)); + + EXPECT_EQ(ctx.bmain->name_map_global, nullptr); } TEST(lib_id_main_unique_name, renames_with_duplicates) @@ -424,6 +511,8 @@ TEST(lib_id_main_unique_name, renames_with_duplicates) EXPECT_STREQ(id_b->name + 2, "Bar"); EXPECT_TRUE(BKE_main_namemap_validate(ctx.bmain)); + + EXPECT_EQ(ctx.bmain->name_map_global, nullptr); } TEST(lib_id_main_unique_name, names_are_unique_per_id_type) @@ -439,6 +528,8 @@ TEST(lib_id_main_unique_name, names_are_unique_per_id_type) EXPECT_STREQ(id_c->name + 2, "Foo.001"); EXPECT_TRUE(BKE_main_namemap_validate(ctx.bmain)); + + EXPECT_EQ(ctx.bmain->name_map_global, nullptr); } TEST(lib_id_main_unique_name, name_huge_number_suffix) @@ -454,6 +545,8 @@ TEST(lib_id_main_unique_name, name_huge_number_suffix) EXPECT_STREQ(id_b->name + 2, "SuperLong.001"); EXPECT_TRUE(BKE_main_namemap_validate(ctx.bmain)); + + EXPECT_EQ(ctx.bmain->name_map_global, nullptr); } } // namespace blender::bke::tests diff --git a/source/blender/blenkernel/intern/main.c b/source/blender/blenkernel/intern/main.c index d5a578c3555..a2dd0b99b0b 100644 --- a/source/blender/blenkernel/intern/main.c +++ b/source/blender/blenkernel/intern/main.c @@ -147,6 +147,9 @@ void BKE_main_free(Main *mainvar) if (mainvar->name_map) { BKE_main_namemap_destroy(&mainvar->name_map); } + if (mainvar->name_map_global) { + BKE_main_namemap_destroy(&mainvar->name_map_global); + } BLI_spin_end((SpinLock *)mainvar->lock); MEM_freeN(mainvar->lock); diff --git a/source/blender/blenkernel/intern/main_namemap.cc b/source/blender/blenkernel/intern/main_namemap.cc index 527c619845f..074efb4ee56 100644 --- a/source/blender/blenkernel/intern/main_namemap.cc +++ b/source/blender/blenkernel/intern/main_namemap.cc @@ -203,6 +203,9 @@ void BKE_main_namemap_clear(Main *bmain) if (bmain_iter->name_map != nullptr) { BKE_main_namemap_destroy(&bmain_iter->name_map); } + if (bmain_iter->name_map_global != nullptr) { + BKE_main_namemap_destroy(&bmain_iter->name_map_global); + } for (Library *lib_iter = static_cast(bmain_iter->libraries.first); lib_iter != nullptr; lib_iter = static_cast(lib_iter->id.next)) @@ -214,16 +217,19 @@ void BKE_main_namemap_clear(Main *bmain) } } -static void main_namemap_populate(UniqueName_Map *name_map, Main *bmain, ID *ignore_id) +/* `do_global` will generate a namemap for all IDs in current Main, regardless of their library. + * Note that duplicates (e.g.local ID and linked ID with same name) will only generate a single + * entry in the map then. */ +static void main_namemap_populate( + UniqueName_Map *name_map, Main *bmain, Library *library, ID *ignore_id, const bool do_global) { BLI_assert_msg(name_map != nullptr, "name_map should not be null"); for (UniqueName_TypeMap &type_map : name_map->type_maps) { type_map.base_name_to_num_suffix.clear(); } - Library *library = ignore_id->lib; ID *id; FOREACH_MAIN_ID_BEGIN (bmain, id) { - if ((id == ignore_id) || (id->lib != library)) { + if ((id == ignore_id) || (!do_global && (id->lib != library))) { continue; } UniqueName_TypeMap *type_map = name_map->find_by_type(GS(id->name)); @@ -232,7 +238,16 @@ static void main_namemap_populate(UniqueName_Map *name_map, Main *bmain, ID *ign /* Insert the full name into the set. */ UniqueName_Key key; STRNCPY(key.name, id->name + 2); - type_map->full_names.add(key); + if (!type_map->full_names.add(key)) { + /* Do not assert, this code is also used by #BKE_main_namemap_validate_and_fix, where + * duplicates are expected. */ +#if 0 + BLI_assert_msg(do_global, + "The key (name) already exists in the namemap, should only happen when " + "`do_global` is true."); +#endif + continue; + } /* Get the name and number parts ("name.number"). */ int number = MIN_NUMBER; @@ -248,29 +263,62 @@ static void main_namemap_populate(UniqueName_Map *name_map, Main *bmain, ID *ign /* Get the name map object used for the given Main/ID. * Lazily creates and populates the contents of the name map, if ensure_created is true. * NOTE: if the contents are populated, the name of the given ID itself is not added. */ -static UniqueName_Map *get_namemap_for(Main *bmain, ID *id, bool ensure_created) +static UniqueName_Map *get_namemap_for(Main *bmain, + ID *id, + const bool ensure_created, + const bool do_global) { + if (do_global) { + if (ensure_created && bmain->name_map_global == nullptr) { + bmain->name_map_global = BKE_main_namemap_create(); + main_namemap_populate(bmain->name_map_global, bmain, id->lib, id, true); + } + return bmain->name_map_global; + } + if (id->lib != nullptr) { if (ensure_created && id->lib->runtime.name_map == nullptr) { id->lib->runtime.name_map = BKE_main_namemap_create(); - main_namemap_populate(id->lib->runtime.name_map, bmain, id); + main_namemap_populate(id->lib->runtime.name_map, bmain, id->lib, id, false); } return id->lib->runtime.name_map; } if (ensure_created && bmain->name_map == nullptr) { bmain->name_map = BKE_main_namemap_create(); - main_namemap_populate(bmain->name_map, bmain, id); + main_namemap_populate(bmain->name_map, bmain, id->lib, id, false); } return bmain->name_map; } -bool BKE_main_namemap_get_name(Main *bmain, ID *id, char *name) +/* Tries to add given name to the given name_map, returns `true` if added, `false` if it was + * already in the namemap. */ +static bool namemap_add_name(UniqueName_Map *name_map, ID *id, const char *name, const int number) +{ + BLI_assert(strlen(name) < MAX_NAME); + UniqueName_TypeMap *type_map = name_map->find_by_type(GS(id->name)); + BLI_assert(type_map != nullptr); + + UniqueName_Key key; + /* Remove full name from the set. */ + STRNCPY(key.name, name); + if (!type_map->full_names.add(key)) { + /* Name already in this namemap, nothing else to do. */ + return false; + } + + UniqueName_Value &val = type_map->base_name_to_num_suffix.lookup_or_add(key, {}); + val.mark_used(number); + return true; +} + +bool BKE_main_namemap_get_name(Main *bmain, ID *id, char *name, const bool do_unique_in_bmain) { #ifndef __GNUC__ /* GCC warns with `nonull-compare`. */ BLI_assert(bmain != nullptr); BLI_assert(id != nullptr); #endif - UniqueName_Map *name_map = get_namemap_for(bmain, id, true); + UniqueName_Map *name_map = get_namemap_for(bmain, id, true, do_unique_in_bmain); + UniqueName_Map *name_map_other = get_namemap_for(bmain, id, false, !do_unique_in_bmain); BLI_assert(name_map != nullptr); BLI_assert(strlen(name) < MAX_NAME); UniqueName_TypeMap *type_map = name_map->find_by_type(GS(id->name)); @@ -304,6 +352,9 @@ bool BKE_main_namemap_get_name(Main *bmain, ID *id, char *name) STRNCPY(key.name, name); type_map->full_names.add(key); } + if (name_map_other != nullptr) { + namemap_add_name(name_map_other, id, name, number); + } return is_name_changed; } @@ -338,6 +389,9 @@ bool BKE_main_namemap_get_name(Main *bmain, ID *id, char *name) /* All good, add final name to the set. */ STRNCPY(key.name, name); type_map->full_names.add(key); + if (name_map_other != nullptr) { + namemap_add_name(name_map_other, id, name, number); + } break; } @@ -349,22 +403,8 @@ bool BKE_main_namemap_get_name(Main *bmain, ID *id, char *name) return is_name_changed; } -void BKE_main_namemap_remove_name(Main *bmain, ID *id, const char *name) +static void namemap_remove_name(UniqueName_Map *name_map, ID *id, const char *name) { -#ifndef __GNUC__ /* GCC warns with `nonull-compare`. */ - BLI_assert(bmain != nullptr); - BLI_assert(id != nullptr); - BLI_assert(name != nullptr); -#endif - /* Name is empty or not initialized yet, nothing to remove. */ - if (name[0] == '\0') { - return; - } - - UniqueName_Map *name_map = get_namemap_for(bmain, id, false); - if (name_map == nullptr) { - return; - } BLI_assert(strlen(name) < MAX_NAME); UniqueName_TypeMap *type_map = name_map->find_by_type(GS(id->name)); BLI_assert(type_map != nullptr); @@ -388,6 +428,29 @@ void BKE_main_namemap_remove_name(Main *bmain, ID *id, const char *name) val->mark_unused(number); } +void BKE_main_namemap_remove_name(Main *bmain, ID *id, const char *name) +{ +#ifndef __GNUC__ /* GCC warns with `nonull-compare`. */ + BLI_assert(bmain != nullptr); + BLI_assert(id != nullptr); + BLI_assert(name != nullptr); +#endif + /* Name is empty or not initialized yet, nothing to remove. */ + if (name[0] == '\0') { + return; + } + + UniqueName_Map *name_map_local = get_namemap_for(bmain, id, false, false); + if (name_map_local != nullptr) { + namemap_remove_name(name_map_local, id, name); + } + + UniqueName_Map *name_map_global = get_namemap_for(bmain, id, false, true); + if (name_map_global != nullptr) { + namemap_remove_name(name_map_global, id, name); + } +} + struct Uniqueness_Key { char name[MAX_ID_NAME]; Library *lib; @@ -452,7 +515,7 @@ static bool main_namemap_validate_and_fix(Main *bmain, const bool do_fix) } } - UniqueName_Map *name_map = get_namemap_for(bmain, id_iter, false); + UniqueName_Map *name_map = get_namemap_for(bmain, id_iter, false, false); if (name_map == nullptr) { continue; }