From 00a36cbf242779c4995c7f5d67eeefee979b96c2 Mon Sep 17 00:00:00 2001 From: Bastien Montagne Date: Wed, 6 Sep 2023 15:44:30 +0200 Subject: [PATCH] Fix #111970: Regression: Crash with assertion after delete scene when some 3D Views have Local Collection enabled This is a nasty gathering of several issues, main one being that 'local collections' of 3DViews are still updated immediately instead of the deferred update used for all other viewlayer cases. The change in a16bcb6576 led to internal references to the Scene's master collection to become invalid, which is the expected behavior. But this turns the Scene's view_layers into invalid state too. Ideally, there should never be resync of viewlayers of a scene being deleted anyways. For now, take the (hopefully!) safe approach of explicitely forbidding any viewlayer update during ID deletion process, and deferring it at the end of the process. Note that this change may also give some marginal gerformance improvements in some rare edge cases (like deleting a very heavy scene with many collections and 'local collection' 3DViews ?). --- .../blenkernel/intern/lib_id_delete.cc | 31 +++++++++++++++++-- 1 file changed, 28 insertions(+), 3 deletions(-) diff --git a/source/blender/blenkernel/intern/lib_id_delete.cc b/source/blender/blenkernel/intern/lib_id_delete.cc index 41e73465f6d..e80580a07f0 100644 --- a/source/blender/blenkernel/intern/lib_id_delete.cc +++ b/source/blender/blenkernel/intern/lib_id_delete.cc @@ -81,7 +81,7 @@ void BKE_libblock_free_datablock(ID *id, const int /*flag*/) BLI_assert_msg(0, "IDType Missing IDTypeInfo"); } -void BKE_id_free_ex(Main *bmain, void *idv, int flag, const bool use_flag_from_idtag) +static int id_free(Main *bmain, void *idv, int flag, const bool use_flag_from_idtag) { ID *id = static_cast(idv); @@ -129,7 +129,7 @@ void BKE_id_free_ex(Main *bmain, void *idv, int flag, const bool use_flag_from_i } if ((flag & LIB_ID_FREE_NO_MAIN) == 0 && key != nullptr) { - BKE_id_free_ex(bmain, &key->id, flag, use_flag_from_idtag); + id_free(bmain, &key->id, flag, use_flag_from_idtag); } BKE_libblock_free_datablock(id, flag); @@ -169,6 +169,23 @@ void BKE_id_free_ex(Main *bmain, void *idv, int flag, const bool use_flag_from_i if ((flag & LIB_ID_FREE_NOT_ALLOCATED) == 0) { MEM_freeN(id); } + + return flag; +} + +void BKE_id_free_ex(Main *bmain, void *idv, int flag, const bool use_flag_from_idtag) +{ + /* ViewLayer resync needs to be delayed during Scene freeing, since internal relationships + * between the Scene's master collection and its view_layers become invalid (due to remapping . + */ + BKE_layer_collection_resync_forbid(); + + flag = id_free(bmain, idv, flag, use_flag_from_idtag); + + BKE_layer_collection_resync_allow(); + if (bmain && (flag & LIB_ID_FREE_NO_MAIN) == 0) { + BKE_main_collection_sync_remap(bmain); + } } void BKE_id_free(Main *bmain, void *idv) @@ -340,6 +357,11 @@ static size_t id_delete(Main *bmain, BKE_main_unlock(bmain); + /* ViewLayer resync needs to be delayed during Scene freeing, since internal relationships + * between the Scene's master collection and its view_layers become invalid (due to remapping . + */ + BKE_layer_collection_resync_forbid(); + /* In usual reversed order, such that all usage of a given ID, even 'never nullptr' ones, * have been already cleared when we reach it * (e.g. Objects being processed before meshes, they'll have already released their 'reference' @@ -363,12 +385,15 @@ static size_t id_delete(Main *bmain, ID_REAL_USERS(id), (id->tag & LIB_TAG_EXTRAUSER_SET) != 0 ? 1 : 0); } - BKE_id_free_ex(bmain, id, free_flag, !do_tagged_deletion); + id_free(bmain, id, free_flag, !do_tagged_deletion); ++num_datablocks_deleted; } } } + BKE_layer_collection_resync_allow(); + BKE_main_collection_sync_remap(bmain); + bmain->is_memfile_undo_written = false; return num_datablocks_deleted; }