From 43e6f110f474979d2999ced11ae11af73f92f525 Mon Sep 17 00:00:00 2001 From: Bastien Montagne Date: Wed, 24 May 2023 12:11:10 +0200 Subject: [PATCH 1/3] Fix (unreported) ID copying code tagging embedded IDs as no-main data. Embedded IDs (master collection of scene, etc.) do not exist in the Main data-base. However, their tags should follow these from their owners. So e.g. if a scene is in Main, its master collection should not be tagged as no-main. NOTE: this is somewhat also related to our ID tags sanitizing TODO task (#88555). Found while invesigating #107913. --- source/blender/blenkernel/intern/lib_id.c | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/source/blender/blenkernel/intern/lib_id.c b/source/blender/blenkernel/intern/lib_id.c index 1d38ec8ffba..83390842415 100644 --- a/source/blender/blenkernel/intern/lib_id.c +++ b/source/blender/blenkernel/intern/lib_id.c @@ -1346,14 +1346,18 @@ void BKE_libblock_copy_ex(Main *bmain, const ID *id, ID **r_newid, const int ori ID *new_id = *r_newid; int flag = orig_flag; - const bool is_private_id_data = (id->flag & LIB_EMBEDDED_DATA) != 0; + const bool is_embedded_id = (id->flag & LIB_EMBEDDED_DATA) != 0; BLI_assert((flag & LIB_ID_CREATE_NO_MAIN) != 0 || bmain != NULL); BLI_assert((flag & LIB_ID_CREATE_NO_MAIN) != 0 || (flag & LIB_ID_CREATE_NO_ALLOCATE) == 0); BLI_assert((flag & LIB_ID_CREATE_NO_MAIN) != 0 || (flag & LIB_ID_CREATE_LOCAL) == 0); - /* 'Private ID' data handling. */ - if ((bmain != NULL) && is_private_id_data) { + /* Embedded ID handling. + * + * NOTE: This makes copying code of embedded IDs non-reentrant (i.e. copying an embedded ID as + * part of another embedded ID would not work properly). This is not an issue currently, but may + * need to be addressed in the future. */ + if ((bmain != NULL) && is_embedded_id) { flag |= LIB_ID_CREATE_NO_MAIN; } @@ -1391,6 +1395,11 @@ void BKE_libblock_copy_ex(Main *bmain, const ID *id, ID **r_newid, const int ori new_id->flag = (new_id->flag & ~copy_idflag_mask) | (id->flag & copy_idflag_mask); + /* 'Private ID' data handling. */ + if (is_embedded_id && (orig_flag & LIB_ID_CREATE_NO_MAIN) == 0) { + new_id->tag &= ~LIB_TAG_NO_MAIN; + } + /* We do not want any handling of user-count in code duplicating the data here, we do that all * at once in id_copy_libmanagement_cb() at the end. */ const int copy_data_flag = orig_flag | LIB_ID_CREATE_NO_USER_REFCOUNT; From babdfc2294828c5491189766efdee367ba9f33aa Mon Sep 17 00:00:00 2001 From: Bastien Montagne Date: Wed, 24 May 2023 17:37:42 +0200 Subject: [PATCH 2/3] Fix #107913: LibOverride: Hard Crash Opening Blender File with overriden active scene. The main issue was the fact that if a Scene is overridden, it's content will be fully invalidated when updating the liboverride at the end of the file reading process. Since the FileData keeps a pointer to the active view layer, it needs to be udated then. As a side consequence, the liblinking of global data also needs to happen before liboverrides are updated. --- .../blender/blenkernel/intern/lib_override.cc | 14 ++++++++++++-- source/blender/blenloader/intern/readfile.cc | 17 +++++++++++++++-- 2 files changed, 27 insertions(+), 4 deletions(-) diff --git a/source/blender/blenkernel/intern/lib_override.cc b/source/blender/blenkernel/intern/lib_override.cc index 06b4c9854ea..1902981a30a 100644 --- a/source/blender/blenkernel/intern/lib_override.cc +++ b/source/blender/blenkernel/intern/lib_override.cc @@ -2667,8 +2667,8 @@ static bool lib_override_library_main_resync_id_skip_check(ID *id, /** * Clear 'unreachable' tag of existing liboverrides if they are using another reachable liboverride * (typical case: Mesh object which only relationship to the rest of the liboverride hierarchy is - * though its 'parent' pointer (i.e. rest of the hierarchy has no actual relationship to this mesh - * object). Sadge. + * through its 'parent' pointer (i.e. rest of the hierarchy has no actual relationship to this mesh + * object). * * Logic and rational of this function are very similar to these of * #lib_override_hierarchy_dependencies_recursive_tag_from, but withing specific resync context. @@ -4440,10 +4440,20 @@ void BKE_lib_override_library_main_unused_cleanup(Main *bmain) static void lib_override_id_swap(Main *bmain, ID *id_local, ID *id_temp) { + /* Ensure ViewLayers are in sync in case a Scene is being swapped, and prevent any further resync + * during the swapping itself. */ + if (GS(id_local->name) == ID_SCE) { + BKE_scene_view_layers_synced_ensure(reinterpret_cast(id_local)); + BKE_scene_view_layers_synced_ensure(reinterpret_cast(id_temp)); + } + BKE_layer_collection_resync_forbid(); + 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_LIBOVERRIDE_NEED_RESYNC); + + BKE_layer_collection_resync_allow(); } void BKE_lib_override_library_update(Main *bmain, ID *local) diff --git a/source/blender/blenloader/intern/readfile.cc b/source/blender/blenloader/intern/readfile.cc index 1d5e83559dd..02bcf57e553 100644 --- a/source/blender/blenloader/intern/readfile.cc +++ b/source/blender/blenloader/intern/readfile.cc @@ -3956,15 +3956,30 @@ BlendFileData *blo_read_file_internal(FileData *fd, const char *filepath) BKE_main_id_tag_all(bfd->main, LIB_TAG_NEW, false); + /* Must happen before applying liboverrides, as this process may fully invalidate e.g. view + * layer pointers in case a Scene is a liboverride. */ + link_global(fd, bfd); + /* Now that all our data-blocks are loaded, * we can re-generate overrides from their references. */ if ((fd->flags & FD_FLAGS_IS_MEMFILE) == 0) { /* Do not apply in undo case! */ fd->reports->duration.lib_overrides = PIL_check_seconds_timer(); + std::string cur_view_layer_name = bfd->cur_view_layer != nullptr ? + bfd->cur_view_layer->name : + ""; + BKE_lib_override_library_main_validate(bfd->main, fd->reports->reports); BKE_lib_override_library_main_update(bfd->main); + /* In case the current scene is a liboverride, while the ID pointer itself remains valid, + * above update of liboverrides will have completely invalidated its old content, so the + * current viewlayer needs to be searched for again. */ + if (bfd->cur_view_layer != nullptr) { + bfd->cur_view_layer = BKE_view_layer_find(bfd->curscene, cur_view_layer_name.c_str()); + } + /* FIXME Temporary 'fix' to a problem in how temp ID are copied in * `BKE_lib_override_library_main_update`, see #103062. * Proper fix involves first addressing #90610. */ @@ -3978,8 +3993,6 @@ BlendFileData *blo_read_file_internal(FileData *fd, const char *filepath) /* Make all relative paths, relative to the open blend file. */ fix_relpaths_library(fd->relabase, bfd->main); - - link_global(fd, bfd); /* as last */ } fd->mainlist = nullptr; /* Safety, this is local variable, shall not be used afterward. */ From 43a31d3c93f9af08f3a87d54cc5c85ba8944f3a0 Mon Sep 17 00:00:00 2001 From: Bastien Montagne Date: Wed, 24 May 2023 18:01:18 +0200 Subject: [PATCH 3/3] Fix #107913: Creation of liboverrides of Scenes should not be allowed. It was still possible to create liboverrides of Scene IDs from the Outliner. Scenes are currently not officially supported, RNA API allows to create them for experimental purposes, but UI should not. --- source/blender/editors/space_outliner/outliner_tools.cc | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/source/blender/editors/space_outliner/outliner_tools.cc b/source/blender/editors/space_outliner/outliner_tools.cc index 37c32aa46b8..6e38739a6fe 100644 --- a/source/blender/editors/space_outliner/outliner_tools.cc +++ b/source/blender/editors/space_outliner/outliner_tools.cc @@ -1043,6 +1043,14 @@ static void id_override_library_create_hierarchy_pre_process_fn(bContext *C, return; } + if (!ID_IS_OVERRIDABLE_LIBRARY_HIERARCHY(id_root_reference)) { + BKE_reportf(reports, + RPT_WARNING, + "Could not create library override from data-block '%s', as it is not overridable", + id_root_reference->name); + return; + } + BLI_assert(do_hierarchy); UNUSED_VARS_NDEBUG(do_hierarchy);