diff --git a/source/blender/blenkernel/BKE_lib_remap.hh b/source/blender/blenkernel/BKE_lib_remap.hh index d95647a46f6..6717d75021e 100644 --- a/source/blender/blenkernel/BKE_lib_remap.hh +++ b/source/blender/blenkernel/BKE_lib_remap.hh @@ -22,6 +22,7 @@ #include "BLI_compiler_attrs.h" #include "BLI_map.hh" +#include "BLI_set.hh" #include "BLI_span.hh" #include "BLI_utildefines.h" @@ -48,10 +49,11 @@ enum { */ ID_REMAP_SKIP_NEVER_NULL_USAGE = 1 << 1, /** - * This tells the callback func to flag with #LIB_DOIT all IDs - * using target one with a 'never NULL' pointer (like e.g. #Object.data). + * Store in the #IDRemapper all IDs using target one with a 'never NULL' pointer (like e.g. + * #Object.data), when such ID usage has (or should have) been remapped to `nullptr`. See also + * #ID_REMAP_FORCE_NEVER_NULL_USAGE and #ID_REMAP_SKIP_NEVER_NULL_USAGE. */ - ID_REMAP_FLAG_NEVER_NULL_USAGE = 1 << 2, + ID_REMAP_STORE_NEVER_NULL_USAGE = 1 << 2, /** * This tells the callback func to force setting IDs * using target one with a 'never NULL' pointer to NULL. @@ -163,12 +165,8 @@ void BKE_libblock_remap(Main *bmain, void *old_idv, void *new_idv, int remap_fla /** * Unlink given \a id from given \a bmain * (does not touch to indirect, i.e. library, usages of the ID). - * - * \param do_flag_never_null: If true, all IDs using \a idv in a 'non-NULL' way are flagged by - * #LIB_TAG_DOIT flag (quite obviously, 'non-NULL' usages can never be unlinked by this function). */ -void BKE_libblock_unlink(Main *bmain, void *idv, bool do_flag_never_null, bool do_skip_indirect) - ATTR_NONNULL(); +void BKE_libblock_unlink(Main *bmain, void *idv, bool do_skip_indirect) ATTR_NONNULL(); /** * Similar to libblock_remap, but only affects IDs used by given \a idv ID. @@ -263,10 +261,17 @@ class IDRemapper { blender::Map mappings_; IDTypeFilter source_types_ = 0; + /** + * Store all IDs using another ID with the 'NEVER_NULL' flag, which have (or + * should have been) remapped to `nullptr`. + */ + blender::Set never_null_users_; + public: void clear(void) { mappings_.clear(); + never_null_users_.clear(); source_types_ = 0; } @@ -302,6 +307,16 @@ class IDRemapper { IDRemapperApplyOptions options, ID *id_self = nullptr) const; + void never_null_users_add(ID *id) + { + never_null_users_.add(id); + } + + const blender::Set &never_null_users(void) const + { + return never_null_users_; + } + /** Iterate over all remapping pairs in the remapper, and call the callback function on them. */ void iter(IDRemapperIterFunction func, void *user_data) const { diff --git a/source/blender/blenkernel/intern/lib_id_delete.cc b/source/blender/blenkernel/intern/lib_id_delete.cc index 428f8a1239a..1fa5ee7f56f 100644 --- a/source/blender/blenkernel/intern/lib_id_delete.cc +++ b/source/blender/blenkernel/intern/lib_id_delete.cc @@ -20,6 +20,7 @@ #include "BLI_linklist.h" #include "BLI_listbase.h" +#include "BLI_set.hh" #include "BLI_vector.hh" #include "BKE_anim_data.h" @@ -214,7 +215,7 @@ void BKE_id_free_us(Main *bmain, void *idv) /* test users */ } if (id->us == 0) { - BKE_libblock_unlink(bmain, id, false, false); + BKE_libblock_unlink(bmain, id, false); BKE_id_free(bmain, id); } @@ -234,7 +235,7 @@ static size_t id_delete(Main *bmain, const int free_flag = LIB_ID_FREE_NO_UI_USER | (do_tagged_deletion ? LIB_ID_FREE_NO_MAIN | LIB_ID_FREE_NO_USER_REFCOUNT : 0); - const int remapping_flags = (ID_REMAP_FLAG_NEVER_NULL_USAGE | ID_REMAP_FORCE_NEVER_NULL_USAGE | + const int remapping_flags = (ID_REMAP_STORE_NEVER_NULL_USAGE | ID_REMAP_FORCE_NEVER_NULL_USAGE | ID_REMAP_FORCE_INTERNAL_RUNTIME_POINTERS | extra_remapping_flags); ListBase tagged_deleted_ids = {nullptr}; @@ -293,13 +294,17 @@ static size_t id_delete(Main *bmain, } } - /* Will tag 'never nullptr' users of this ID too. + /* Will store in remapper all 'never nullptr' users of this ID too. * * NOTE: #BKE_libblock_unlink() cannot be used here, since it would ignore indirect * links, this can lead to nasty crashing here in second, actual deleting loop. * Also, this will also flag users of deleted data that cannot be unlinked * (object using deleted obdata, etc.), so that they also get deleted. */ BKE_libblock_remap_multiple_locked(bmain, id_remapper, remapping_flags); + /* Tag cleared 'never-null' user IDs for deletion too. */ + for (ID *never_null_user_id : id_remapper.never_null_users()) { + never_null_user_id->tag |= tag; + } id_remapper.clear(); } diff --git a/source/blender/blenkernel/intern/lib_remap.cc b/source/blender/blenkernel/intern/lib_remap.cc index ad04b2a3801..61d019add87 100644 --- a/source/blender/blenkernel/intern/lib_remap.cc +++ b/source/blender/blenkernel/intern/lib_remap.cc @@ -256,8 +256,11 @@ static int foreach_libblock_remap_callback(LibraryIDLinkCallbackData *cb_data) skip_reference); #endif - if ((id_remap_data->flag & ID_REMAP_FLAG_NEVER_NULL_USAGE) && (cb_flag & IDWALK_CB_NEVER_NULL)) { - id_owner->tag |= LIB_TAG_DOIT; + if ((id_remap_data->flag & ID_REMAP_STORE_NEVER_NULL_USAGE) && + (cb_flag & IDWALK_CB_NEVER_NULL) && + (expected_mapping_result == ID_REMAP_RESULT_SOURCE_UNASSIGNED)) + { + id_remapper.never_null_users_add(id_owner); } /* Special hack in case it's Object->data and we are in edit mode, and new_id is not nullptr @@ -715,13 +718,9 @@ void BKE_libblock_remap_multiple(Main *bmain, IDRemapper &mappings, const int re BKE_main_unlock(bmain); } -void BKE_libblock_unlink(Main *bmain, - void *idv, - const bool do_flag_never_null, - const bool do_skip_indirect) +void BKE_libblock_unlink(Main *bmain, void *idv, const bool do_skip_indirect) { - const int remap_flags = (do_skip_indirect ? ID_REMAP_SKIP_INDIRECT_USAGE : 0) | - (do_flag_never_null ? ID_REMAP_FLAG_NEVER_NULL_USAGE : 0); + const int remap_flags = (do_skip_indirect ? ID_REMAP_SKIP_INDIRECT_USAGE : 0); BKE_main_lock(bmain); diff --git a/source/blender/blenkernel/intern/lib_remap_test.cc b/source/blender/blenkernel/intern/lib_remap_test.cc index 32977e3c2c4..f4ba8a26350 100644 --- a/source/blender/blenkernel/intern/lib_remap_test.cc +++ b/source/blender/blenkernel/intern/lib_remap_test.cc @@ -32,6 +32,8 @@ #include "MEM_guardedalloc.h" +using namespace blender::bke::id; + namespace blender::bke::tests { class TestData { @@ -298,7 +300,7 @@ TEST(lib_remap, never_null_usage_flag_not_requested_on_delete) EXPECT_EQ(context.test_data.object->id.tag & LIB_TAG_DOIT, 0); } -TEST(lib_remap, never_null_usage_flag_requested_on_delete) +TEST(lib_remap, never_null_usage_storage_requested_on_delete) { Context context; @@ -306,14 +308,19 @@ TEST(lib_remap, never_null_usage_flag_requested_on_delete) EXPECT_EQ(context.test_data.object->data, context.test_data.mesh); EXPECT_EQ(context.test_data.object->id.tag & LIB_TAG_DOIT, 0); - /* Never null usage is requested so the flag should be set. */ - BKE_libblock_remap(context.test_data.bmain, - context.test_data.mesh, - nullptr, - ID_REMAP_SKIP_NEVER_NULL_USAGE | ID_REMAP_FLAG_NEVER_NULL_USAGE); + /* Never null usage is requested so the owner ID (the Object) should be added to the set. */ + IDRemapper remapper; + remapper.add(&context.test_data.mesh->id, nullptr); + BKE_libblock_remap_multiple_locked( + context.test_data.bmain, + remapper, + (ID_REMAP_SKIP_NEVER_NULL_USAGE | ID_REMAP_STORE_NEVER_NULL_USAGE)); + + /* Never null usages unassignement is not enforced (no #ID_REMAP_FORCE_NEVER_NULL_USAGE), so the + * obdta should still use the original mesh. */ EXPECT_EQ(context.test_data.object->data, context.test_data.mesh); EXPECT_NE(context.test_data.object->data, nullptr); - EXPECT_EQ(context.test_data.object->id.tag & LIB_TAG_DOIT, LIB_TAG_DOIT); + EXPECT_TRUE(remapper.never_null_users().contains(&context.test_data.object->id)); } TEST(lib_remap, never_null_usage_flag_not_requested_on_remap) @@ -332,7 +339,7 @@ TEST(lib_remap, never_null_usage_flag_not_requested_on_remap) EXPECT_EQ(context.test_data.object->id.tag & LIB_TAG_DOIT, 0); } -TEST(lib_remap, never_null_usage_flag_requested_on_remap) +TEST(lib_remap, never_null_usage_storage_requested_on_remap) { Context context; Mesh *other_mesh = BKE_mesh_add(context.test_data.bmain, nullptr); @@ -341,13 +348,16 @@ TEST(lib_remap, never_null_usage_flag_requested_on_remap) EXPECT_EQ(context.test_data.object->data, context.test_data.mesh); EXPECT_EQ(context.test_data.object->id.tag & LIB_TAG_DOIT, 0); - /* Never null usage is requested so the flag should be set. */ - BKE_libblock_remap(context.test_data.bmain, - context.test_data.mesh, - other_mesh, - ID_REMAP_SKIP_NEVER_NULL_USAGE | ID_REMAP_FLAG_NEVER_NULL_USAGE); + /* Never null usage is requested, but the obdata is remapped to another Mesh, not to `nullptr`, + * so the `never_null_users` set should remain empty. */ + IDRemapper remapper; + remapper.add(&context.test_data.mesh->id, &other_mesh->id); + BKE_libblock_remap_multiple_locked( + context.test_data.bmain, + remapper, + (ID_REMAP_SKIP_NEVER_NULL_USAGE | ID_REMAP_STORE_NEVER_NULL_USAGE)); EXPECT_EQ(context.test_data.object->data, other_mesh); - EXPECT_EQ(context.test_data.object->id.tag & LIB_TAG_DOIT, LIB_TAG_DOIT); + EXPECT_TRUE(remapper.never_null_users().is_empty()); } /** \} */