From 5e0c77594a18a10a3ae9bdf6bd6953786b006bac Mon Sep 17 00:00:00 2001 From: Bastien Montagne Date: Wed, 19 Apr 2023 20:24:40 +0200 Subject: [PATCH 1/2] LibOverride: Restore local references to virtual linked liboverrides on resync. When resyncing linked liboverride data, new IDs may be created that do not exist in actual library file (since the lib file has not been resynced). If such 'virtual linked liboverrides' data-blocks are used locally (e.g. by adding such object to a local collection), on next file read they will be detected as missing. Now resync code will list such missing linked IDs that were liboverrides, and try to re-use them when matching (by root name and library) with newly generated virtual liboverrides. The process may not be 100% perfect, especially a perfect one-to-one remapping cannot be ensured if source data keep being changed over and over (because of the order in which virtual linked liboverrides generated by resync may change over time). However, in practice this should fix the vast majority of issues, especially if sane naming practices are used on IDs. --------------- For the record, an attempt was made to write liboverride data together with the placeholders for linked IDs in .blendfile. In theory, this should ensure a perfect and fully valid re-usage of such IDs. However, for this to work, not opnly the liboverride data of linked IDs need to be written on disk, but also all ID references in this data has to be considered as directly linked, to ensure that such liboverride data can be re-read properly. Otherwise, these placeholders would get a liboverride data with NULL ID pointers, which is useless. Such change feels way to intrusive for the very limited benefit, so for now would consider current solution as the best option. --- .../blender/blenkernel/intern/lib_override.cc | 119 +++++++++++++++++- source/blender/blenloader/intern/readfile.cc | 22 +++- source/blender/makesdna/DNA_ID.h | 3 + 3 files changed, 140 insertions(+), 4 deletions(-) diff --git a/source/blender/blenkernel/intern/lib_override.cc b/source/blender/blenkernel/intern/lib_override.cc index 0d67b2d97e0..e06808655cb 100644 --- a/source/blender/blenkernel/intern/lib_override.cc +++ b/source/blender/blenkernel/intern/lib_override.cc @@ -7,6 +7,8 @@ #include #include +#include +#include #include "CLG_log.h" @@ -1718,6 +1720,91 @@ static void lib_override_library_remap(Main *bmain, BLI_linklist_free(nomain_ids, nullptr); } +/** + * Mapping to find suitable missing linked liboverrides to replace by the newly generated linked + * liboverrides during resync process. + * + * \note About Order: + * In most cases, if there are several virtual linked liboverrides generated with the same base + * name (like `OBCube.001`, `OBCube.002`, etc.), this mapping system will find the correct one, for + * the following reasons: + * - Order of creation of these virtual IDs in resync process is expected to be stable (i.e. + * several runs of resync code based on the same linked data would re-create the same virtual + * liboverride IDs in the same order); + * - Order of creation and usage of the mapping data (a FIFO queue) also ensures that the missing + * placeholder `OBCube.001` is always 're-used' before `OBCube.002`. + * + * In case linked data keep being modified, these conditions may fail and the mapping may start to + * return 'wrong' results. However, this is considered as an acceptable limitation here, since this + * is mainly a 'best effort' to recover from situations that should not be hapenning in the first + * place. + */ + +using LibOverrideMissingIDsData_Key = const std::pair; +using LibOverrideMissingIDsData_Value = std::deque; +using LibOverrideMissingIDsData = + std::map; + +/* Return a key suitable for the missing IDs mapping, i.e. a pair of + * ``. + * + * So e.g. returns `<"OBMyObject", lib>` for ID from `lib` with names like `"OBMyObject"`, + * `"OBMyObject.002"`, `"OBMyObject.12345"`, and so on, but _not_ for e.g. `"OBMyObject.12.002"`. + */ +static LibOverrideMissingIDsData_Key lib_override_library_resync_missing_id_key(ID *id) +{ + std::string id_name_key(id->name); + const size_t last_key_index = id_name_key.find_last_not_of("0123456789"); + + BLI_assert(last_key_index != std::string::npos); + + if (id_name_key[last_key_index] == '.') { + id_name_key.resize(last_key_index); + } + + return LibOverrideMissingIDsData_Key(id_name_key, id->lib); +} + +static LibOverrideMissingIDsData lib_override_library_resync_build_missing_ids_data(Main *bmain) +{ + LibOverrideMissingIDsData missing_ids; + ID *id_iter; + FOREACH_MAIN_ID_BEGIN (bmain, id_iter) { + if (!ID_IS_LINKED(id_iter)) { + continue; + } + const int required_tags = (LIB_TAG_MISSING | LIB_TAG_LIB_OVERRIDE_NEED_RESYNC); + if ((id_iter->tag & required_tags) != required_tags) { + continue; + } + + LibOverrideMissingIDsData_Key key = lib_override_library_resync_missing_id_key(id_iter); + std::pair value = missing_ids.try_emplace( + key, LibOverrideMissingIDsData_Value()); + value.first->second.push_back(id_iter); + } + FOREACH_MAIN_ID_END; + + return missing_ids; +} + +static ID *lib_override_library_resync_search_missing_ids_data( + LibOverrideMissingIDsData &missing_ids, ID *id_override) +{ + LibOverrideMissingIDsData_Key key = lib_override_library_resync_missing_id_key(id_override); + const LibOverrideMissingIDsData::iterator value = missing_ids.find(key); + if (value == missing_ids.end()) { + return nullptr; + } + if (value->second.empty()) { + return nullptr; + } + ID *match_id = value->second.front(); + value->second.pop_front(); + return match_id; +} + static bool lib_override_library_resync(Main *bmain, Scene *scene, ViewLayer *view_layer, @@ -1931,6 +2018,11 @@ static bool lib_override_library_resync(Main *bmain, return success; } + /* Get a mapping of all missing linked IDs that were liboverrides, to search for 'old + * liboverrides' for newly created ones that do not alredy have one, in next step. */ + LibOverrideMissingIDsData missing_ids_data = lib_override_library_resync_build_missing_ids_data( + bmain); + ListBase *lb; FOREACH_MAIN_LISTBASE_BEGIN (bmain, lb) { FOREACH_MAIN_LISTBASE_ID_BEGIN (lb, id) { @@ -1949,6 +2041,26 @@ static bool lib_override_library_resync(Main *bmain, BLI_assert(/*!ID_IS_LINKED(id_override_new) || */ id_override_new->lib == id->lib); BLI_assert(id_override_old == nullptr || id_override_old->lib == id_root->lib); id_override_new->lib = id_root->lib; + + /* The old override may have been created as linked data and then referenced by local data + * during a previous Blender session, in which case it became directly linked and a reference + * to it was stored in the local .blend file. however, since that linked liboverride ID does + * not actually exist in the original library file, on next file read it is lost and marked + * as missing ID.*/ + if (id_override_old == nullptr && ID_IS_LINKED(id_override_new)) { + id_override_old = lib_override_library_resync_search_missing_ids_data(missing_ids_data, + id_override_new); + BLI_assert(id_override_old == nullptr || id_override_old->lib == id_override_new->lib); + if (id_override_old != nullptr) { + BLI_ghash_insert(linkedref_to_old_override, id, id_override_old); + CLOG_INFO(&LOG, + 2, + "Found missing linked old override best-match %s for new linked override %s", + id_override_old->name, + id_override_new->name); + } + } + /* Remap step below will tag directly linked ones properly as needed. */ if (ID_IS_LINKED(id_override_new)) { id_override_new->tag |= LIB_TAG_INDIRECT; @@ -1967,7 +2079,12 @@ static bool lib_override_library_resync(Main *bmain, lib_override_object_posemode_transfer(id_override_new, id_override_old); - if (ID_IS_OVERRIDE_LIBRARY_REAL(id_override_new)) { + /* Missing old liboverrides cannot transfer their override rules to new liboverride. + * This is fine though, since these are expected to only be 'virtual' linked overrides + * generated by resync of linked overrides. So nothing is expected to be overridden here. + */ + if (ID_IS_OVERRIDE_LIBRARY_REAL(id_override_new) && + (id_override_old->tag & LIB_TAG_MISSING) == 0) { BLI_assert(ID_IS_OVERRIDE_LIBRARY_REAL(id_override_old)); id_override_new->override_library->flag = id_override_old->override_library->flag; diff --git a/source/blender/blenloader/intern/readfile.cc b/source/blender/blenloader/intern/readfile.cc index 87446026c44..7cddb7258c1 100644 --- a/source/blender/blenloader/intern/readfile.cc +++ b/source/blender/blenloader/intern/readfile.cc @@ -2753,7 +2753,11 @@ static void fix_relpaths_library(const char *basepath, Main *main) /** \name Read Library Data Block * \{ */ -static ID *create_placeholder(Main *mainvar, const short idcode, const char *idname, const int tag) +static ID *create_placeholder(Main *mainvar, + const short idcode, + const char *idname, + const int tag, + const bool was_liboverride) { ListBase *lb = which_libbase(mainvar, idcode); ID *ph_id = static_cast(BKE_libblock_alloc_notest(idcode)); @@ -2766,6 +2770,14 @@ static ID *create_placeholder(Main *mainvar, const short idcode, const char *idn ph_id->us = ID_FAKE_USERS(ph_id); ph_id->icon_id = 0; + if (was_liboverride) { + /* 'Abuse' `LIB_TAG_LIB_OVERRIDE_NEED_RESYNC` to mark that placeholder missing linked ID as + * being a liboverride. + * + * This will be used by the liboverride resync process, see #lib_override_library_resync. */ + ph_id->tag |= LIB_TAG_LIB_OVERRIDE_NEED_RESYNC; + } + BLI_addtail(lb, ph_id); id_sort_by_name(lb, ph_id, nullptr); @@ -4384,7 +4396,7 @@ static ID *link_named_part( else if (use_placeholders) { /* XXX flag part is weak! */ id = create_placeholder( - mainl, idcode, name, force_indirect ? LIB_TAG_INDIRECT : LIB_TAG_EXTERN); + mainl, idcode, name, force_indirect ? LIB_TAG_INDIRECT : LIB_TAG_EXTERN, false); } else { id = nullptr; @@ -4701,7 +4713,11 @@ static void read_library_linked_id( /* Generate a placeholder for this ID (simplified version of read_libblock actually...). */ if (r_id) { - *r_id = is_valid ? create_placeholder(mainvar, GS(id->name), id->name + 2, id->tag) : + *r_id = is_valid ? create_placeholder(mainvar, + GS(id->name), + id->name + 2, + id->tag, + id->override_library != nullptr) : nullptr; } } diff --git a/source/blender/makesdna/DNA_ID.h b/source/blender/makesdna/DNA_ID.h index 6346abf4211..168342aeaeb 100644 --- a/source/blender/makesdna/DNA_ID.h +++ b/source/blender/makesdna/DNA_ID.h @@ -788,6 +788,9 @@ enum { /** * ID is a library override that needs re-sync to its linked reference. * + * \note Also used by readfile code when creating a missing ID placeholder if it is detected as + * being a linked liboverride ID. + * * RESET_NEVER */ LIB_TAG_LIB_OVERRIDE_NEED_RESYNC = 1 << 8, -- 2.30.2 From 56373841561696d84bcdf95953e867c1d9e3b60b Mon Sep 17 00:00:00 2001 From: Bastien Montagne Date: Mon, 8 May 2023 17:24:36 +0200 Subject: [PATCH 2/2] Fix after merging main. --- source/blender/blenkernel/intern/lib_override.cc | 2 +- source/blender/blenloader/intern/readfile.cc | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/source/blender/blenkernel/intern/lib_override.cc b/source/blender/blenkernel/intern/lib_override.cc index ff6367bab39..528bce8ef3c 100644 --- a/source/blender/blenkernel/intern/lib_override.cc +++ b/source/blender/blenkernel/intern/lib_override.cc @@ -1810,7 +1810,7 @@ static LibOverrideMissingIDsData lib_override_library_resync_build_missing_ids_d if (!ID_IS_LINKED(id_iter)) { continue; } - const int required_tags = (LIB_TAG_MISSING | LIB_TAG_LIB_OVERRIDE_NEED_RESYNC); + const int required_tags = (LIB_TAG_MISSING | LIB_TAG_LIBOVERRIDE_NEED_RESYNC); if ((id_iter->tag & required_tags) != required_tags) { continue; } diff --git a/source/blender/blenloader/intern/readfile.cc b/source/blender/blenloader/intern/readfile.cc index dcc4ef02af1..229b1960fe3 100644 --- a/source/blender/blenloader/intern/readfile.cc +++ b/source/blender/blenloader/intern/readfile.cc @@ -2774,11 +2774,11 @@ static ID *create_placeholder(Main *mainvar, ph_id->icon_id = 0; if (was_liboverride) { - /* 'Abuse' `LIB_TAG_LIB_OVERRIDE_NEED_RESYNC` to mark that placeholder missing linked ID as + /* 'Abuse' `LIB_TAG_LIBOVERRIDE_NEED_RESYNC` to mark that placeholder missing linked ID as * being a liboverride. * * This will be used by the liboverride resync process, see #lib_override_library_resync. */ - ph_id->tag |= LIB_TAG_LIB_OVERRIDE_NEED_RESYNC; + ph_id->tag |= LIB_TAG_LIBOVERRIDE_NEED_RESYNC; } BLI_addtail(lb, ph_id); -- 2.30.2