From f3186389b0094b0478df5adb2da917ac722acd7f Mon Sep 17 00:00:00 2001 From: Bastien Montagne Date: Tue, 30 Aug 2022 14:20:53 +0200 Subject: Fix T100586: libOverride resync could remove too many data-blocks. Do not delete 'orphaned' overrides when their reference is missing because the library file itself is missing. --- source/blender/blenkernel/intern/lib_override.cc | 29 +++++++++++++++++++++--- 1 file changed, 26 insertions(+), 3 deletions(-) (limited to 'source/blender/blenkernel/intern') diff --git a/source/blender/blenkernel/intern/lib_override.cc b/source/blender/blenkernel/intern/lib_override.cc index 03ad4b1cd5f..0e09c36fd64 100644 --- a/source/blender/blenkernel/intern/lib_override.cc +++ b/source/blender/blenkernel/intern/lib_override.cc @@ -2081,9 +2081,13 @@ static bool lib_override_library_resync(Main *bmain, } /* Also deal with old overrides that went missing in new linked data - only for real local * overrides for now, not those who are linked. */ - else if (id->tag & LIB_TAG_MISSING && !ID_IS_LINKED(id)) { - BLI_assert(ID_IS_OVERRIDE_LIBRARY(id)); - if (!BKE_lib_override_library_is_user_edited(id)) { + else if (id->tag & LIB_TAG_MISSING && !ID_IS_LINKED(id) && ID_IS_OVERRIDE_LIBRARY(id)) { + if (ID_IS_OVERRIDE_LIBRARY_REAL(id) && + id->override_library->reference->lib->id.tag & LIB_TAG_MISSING) { + /* Do not delete overrides which reference is missing because the library itself is missing + * (ref. T100586). */ + } + else if (!BKE_lib_override_library_is_user_edited(id)) { /* If user never edited them, we can delete them. */ id->tag |= LIB_TAG_DOIT; id->tag &= ~LIB_TAG_MISSING; @@ -2395,6 +2399,11 @@ static void lib_override_library_main_resync_on_library_indirect_level( continue; } + /* Do not attempt to resync from missing data. */ + if (((id->tag | id->override_library->reference->tag) & LIB_TAG_MISSING) != 0) { + continue; + } + if (id->override_library->flag & IDOVERRIDE_LIBRARY_FLAG_NO_HIERARCHY) { /* This ID is not part of an override hierarchy. */ continue; @@ -2423,6 +2432,11 @@ static void lib_override_library_main_resync_on_library_indirect_level( continue; } + /* Do not attempt to resync from missing data. */ + if (((id->tag | id->override_library->reference->tag) & LIB_TAG_MISSING) != 0) { + continue; + } + if (id->override_library->flag & IDOVERRIDE_LIBRARY_FLAG_NO_HIERARCHY) { /* This ID is not part of an override hierarchy. */ BLI_assert((id->tag & LIB_TAG_LIB_OVERRIDE_NEED_RESYNC) == 0); @@ -2550,6 +2564,15 @@ static void lib_override_library_main_resync_on_library_indirect_level( /* Check there are no left-over IDs needing resync from the current (or higher) level of indirect * library level. */ FOREACH_MAIN_ID_BEGIN (bmain, id) { + if (!ID_IS_OVERRIDE_LIBRARY(id)) { + continue; + } + /* Do not attempt to resync to/from missing data. */ + if (((id->tag | (ID_IS_OVERRIDE_LIBRARY_REAL(id) ? id->override_library->reference->tag : 0)) & + LIB_TAG_MISSING) != 0) { + continue; + } + const bool is_valid_tagged_need_resync = ((id->tag & LIB_TAG_LIB_OVERRIDE_NEED_RESYNC) == 0 || lib_override_resync_id_lib_level_is_valid( id, library_indirect_level - 1, false)); -- cgit v1.2.3 From 34ff27025d442f6b5e65075e8c8b6aaad8e400b4 Mon Sep 17 00:00:00 2001 From: Bastien Montagne Date: Tue, 30 Aug 2022 14:35:17 +0200 Subject: Cleanup: Remove one level of indentation by early continue in a loop. --- source/blender/blenkernel/intern/lib_override.cc | 297 ++++++++++++----------- 1 file changed, 152 insertions(+), 145 deletions(-) (limited to 'source/blender/blenkernel/intern') diff --git a/source/blender/blenkernel/intern/lib_override.cc b/source/blender/blenkernel/intern/lib_override.cc index 0e09c36fd64..94642ad2116 100644 --- a/source/blender/blenkernel/intern/lib_override.cc +++ b/source/blender/blenkernel/intern/lib_override.cc @@ -1815,68 +1815,70 @@ static bool lib_override_library_resync(Main *bmain, id->tag |= LIB_TAG_MISSING; } - if ((id->lib == id_root->lib) && ID_IS_OVERRIDE_LIBRARY(id)) { - /* While this should not happen in typical cases (and won't be properly supported here), - * user is free to do all kind of very bad things, including having different local - * overrides of a same linked ID in a same hierarchy. */ - IDOverrideLibrary *id_override_library = BKE_lib_override_library_get( - bmain, id, nullptr, nullptr); - - if (id_override_library->hierarchy_root != id_root->override_library->hierarchy_root) { - continue; - } + if ((id->lib != id_root->lib) || !ID_IS_OVERRIDE_LIBRARY(id)) { + continue; + } - ID *reference_id = id_override_library->reference; - if (GS(reference_id->name) != GS(id->name)) { - switch (GS(id->name)) { - case ID_KE: - reference_id = reinterpret_cast(BKE_key_from_id(reference_id)); - break; - case ID_GR: - BLI_assert(GS(reference_id->name) == ID_SCE); - reference_id = reinterpret_cast( - reinterpret_cast(reference_id)->master_collection); - break; - case ID_NT: - reference_id = reinterpret_cast(ntreeFromID(id)); - break; - default: - break; - } + /* While this should not happen in typical cases (and won't be properly supported here), + * user is free to do all kind of very bad things, including having different local + * overrides of a same linked ID in a same hierarchy. */ + IDOverrideLibrary *id_override_library = BKE_lib_override_library_get( + bmain, id, nullptr, nullptr); + + if (id_override_library->hierarchy_root != id_root->override_library->hierarchy_root) { + continue; + } + + ID *reference_id = id_override_library->reference; + if (GS(reference_id->name) != GS(id->name)) { + switch (GS(id->name)) { + case ID_KE: + reference_id = reinterpret_cast(BKE_key_from_id(reference_id)); + break; + case ID_GR: + BLI_assert(GS(reference_id->name) == ID_SCE); + reference_id = reinterpret_cast( + reinterpret_cast(reference_id)->master_collection); + break; + case ID_NT: + reference_id = reinterpret_cast(ntreeFromID(id)); + break; + default: + break; } - if (reference_id == nullptr) { - /* Can happen e.g. when there is a local override of a shapekey, but the matching linked - * obdata (mesh etc.) does not have any shapekey anymore. */ + } + if (reference_id == nullptr) { + /* Can happen e.g. when there is a local override of a shapekey, but the matching linked + * obdata (mesh etc.) does not have any shapekey anymore. */ + continue; + } + BLI_assert(GS(reference_id->name) == GS(id->name)); + + if (!BLI_ghash_haskey(linkedref_to_old_override, reference_id)) { + BLI_ghash_insert(linkedref_to_old_override, reference_id, id); + if (!ID_IS_OVERRIDE_LIBRARY_REAL(id) || (id->tag & LIB_TAG_DOIT) == 0) { continue; } - BLI_assert(GS(reference_id->name) == GS(id->name)); + if ((id->override_library->reference->tag & LIB_TAG_DOIT) == 0) { + /* We have an override, but now it does not seem to be necessary to override that ID + * anymore. Check if there are some actual overrides from the user, otherwise assume + * that we can get rid of this local override. */ + LISTBASE_FOREACH (IDOverrideLibraryProperty *, op, &id->override_library->properties) { + if (!ELEM(op->rna_prop_type, PROP_POINTER, PROP_COLLECTION)) { + id->override_library->reference->tag |= LIB_TAG_DOIT; + break; + } - if (!BLI_ghash_haskey(linkedref_to_old_override, reference_id)) { - BLI_ghash_insert(linkedref_to_old_override, reference_id, id); - if (!ID_IS_OVERRIDE_LIBRARY_REAL(id) || (id->tag & LIB_TAG_DOIT) == 0) { - continue; - } - if ((id->override_library->reference->tag & LIB_TAG_DOIT) == 0) { - /* We have an override, but now it does not seem to be necessary to override that ID - * anymore. Check if there are some actual overrides from the user, otherwise assume - * that we can get rid of this local override. */ - LISTBASE_FOREACH (IDOverrideLibraryProperty *, op, &id->override_library->properties) { - if (!ELEM(op->rna_prop_type, PROP_POINTER, PROP_COLLECTION)) { + bool do_break = false; + LISTBASE_FOREACH (IDOverrideLibraryPropertyOperation *, opop, &op->operations) { + if ((opop->flag & IDOVERRIDE_LIBRARY_FLAG_IDPOINTER_MATCH_REFERENCE) == 0) { id->override_library->reference->tag |= LIB_TAG_DOIT; + do_break = true; break; } - - bool do_break = false; - LISTBASE_FOREACH (IDOverrideLibraryPropertyOperation *, opop, &op->operations) { - if ((opop->flag & IDOVERRIDE_LIBRARY_FLAG_IDPOINTER_MATCH_REFERENCE) == 0) { - id->override_library->reference->tag |= LIB_TAG_DOIT; - do_break = true; - break; - } - } - if (do_break) { - break; - } + } + if (do_break) { + break; } } } @@ -1919,60 +1921,62 @@ static bool lib_override_library_resync(Main *bmain, ListBase *lb; FOREACH_MAIN_LISTBASE_BEGIN (bmain, lb) { FOREACH_MAIN_LISTBASE_ID_BEGIN (lb, id) { - if (id->tag & LIB_TAG_DOIT && id->newid != nullptr && id->lib == id_root_reference->lib) { - ID *id_override_new = id->newid; - ID *id_override_old = static_cast(BLI_ghash_lookup(linkedref_to_old_override, id)); + if ((id->tag & LIB_TAG_DOIT) == 0 || id->newid == nullptr || + id->lib != id_root_reference->lib) { + continue; + } + ID *id_override_new = id->newid; + ID *id_override_old = static_cast(BLI_ghash_lookup(linkedref_to_old_override, id)); - BLI_assert((id_override_new->tag & LIB_TAG_LIB_OVERRIDE_NEED_RESYNC) == 0); - - /* We need to 'move back' newly created override into its proper library (since it was - * duplicated from the reference ID with 'no main' option, it should currently be the same - * as the reference ID one). */ - 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; - /* 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; - } + BLI_assert((id_override_new->tag & LIB_TAG_LIB_OVERRIDE_NEED_RESYNC) == 0); + + /* We need to 'move back' newly created override into its proper library (since it was + * duplicated from the reference ID with 'no main' option, it should currently be the same + * as the reference ID one). */ + 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; + /* 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; + } - if (id_override_old != nullptr) { - /* Swap the names between old override ID and new one. */ - char id_name_buf[MAX_ID_NAME]; - memcpy(id_name_buf, id_override_old->name, sizeof(id_name_buf)); - memcpy(id_override_old->name, id_override_new->name, sizeof(id_override_old->name)); - memcpy(id_override_new->name, id_name_buf, sizeof(id_override_new->name)); - - BLI_insertlinkreplace(lb, id_override_old, id_override_new); - id_override_old->tag |= LIB_TAG_NO_MAIN; - id_override_new->tag &= ~LIB_TAG_NO_MAIN; - - lib_override_object_posemode_transfer(id_override_new, id_override_old); - - if (ID_IS_OVERRIDE_LIBRARY_REAL(id_override_new)) { - BLI_assert(ID_IS_OVERRIDE_LIBRARY_REAL(id_override_old)); - - id_override_new->override_library->flag = id_override_old->override_library->flag; - - /* Copy over overrides rules from old override ID to new one. */ - BLI_duplicatelist(&id_override_new->override_library->properties, - &id_override_old->override_library->properties); - IDOverrideLibraryProperty *op_new = static_cast( - id_override_new->override_library->properties.first); - IDOverrideLibraryProperty *op_old = static_cast( - id_override_old->override_library->properties.first); - for (; op_new; op_new = op_new->next, op_old = op_old->next) { - lib_override_library_property_copy(op_new, op_old); - } + if (id_override_old != nullptr) { + /* Swap the names between old override ID and new one. */ + char id_name_buf[MAX_ID_NAME]; + memcpy(id_name_buf, id_override_old->name, sizeof(id_name_buf)); + memcpy(id_override_old->name, id_override_new->name, sizeof(id_override_old->name)); + memcpy(id_override_new->name, id_name_buf, sizeof(id_override_new->name)); + + BLI_insertlinkreplace(lb, id_override_old, id_override_new); + id_override_old->tag |= LIB_TAG_NO_MAIN; + id_override_new->tag &= ~LIB_TAG_NO_MAIN; + + lib_override_object_posemode_transfer(id_override_new, id_override_old); + + if (ID_IS_OVERRIDE_LIBRARY_REAL(id_override_new)) { + BLI_assert(ID_IS_OVERRIDE_LIBRARY_REAL(id_override_old)); + + id_override_new->override_library->flag = id_override_old->override_library->flag; + + /* Copy over overrides rules from old override ID to new one. */ + BLI_duplicatelist(&id_override_new->override_library->properties, + &id_override_old->override_library->properties); + IDOverrideLibraryProperty *op_new = static_cast( + id_override_new->override_library->properties.first); + IDOverrideLibraryProperty *op_old = static_cast( + id_override_old->override_library->properties.first); + for (; op_new; op_new = op_new->next, op_old = op_old->next) { + lib_override_library_property_copy(op_new, op_old); } - - BLI_addtail(no_main_ids_list, id_override_old); - } - else { - /* Add to proper main list, ensure unique name for local ID, sort, and clear relevant - * tags. */ - BKE_libblock_management_main_add(bmain, id_override_new); } + + BLI_addtail(no_main_ids_list, id_override_old); + } + else { + /* Add to proper main list, ensure unique name for local ID, sort, and clear relevant + * tags. */ + BKE_libblock_management_main_add(bmain, id_override_new); } } FOREACH_MAIN_LISTBASE_ID_END; @@ -1991,53 +1995,56 @@ static bool lib_override_library_resync(Main *bmain, * remapped, and all new local override IDs have gotten their proper original names, otherwise * override operations based on those ID names would fail. */ FOREACH_MAIN_ID_BEGIN (bmain, id) { - if (id->tag & LIB_TAG_DOIT && id->newid != nullptr && id->lib == id_root_reference->lib) { - ID *id_override_new = id->newid; - if (!ID_IS_OVERRIDE_LIBRARY_REAL(id_override_new)) { - continue; - } - ID *id_override_old = static_cast(BLI_ghash_lookup(linkedref_to_old_override, id)); + if ((id->tag & LIB_TAG_DOIT) == 0 || id->newid == nullptr || + id->lib != id_root_reference->lib) { + continue; + } - if (id_override_old == nullptr) { - continue; - } - if (ID_IS_OVERRIDE_LIBRARY_REAL(id_override_old)) { - /* Apply rules on new override ID using old one as 'source' data. */ - /* Note that since we already remapped ID pointers in old override IDs to new ones, we - * can also apply ID pointer override rules safely here. */ - PointerRNA rnaptr_src, rnaptr_dst; - RNA_id_pointer_create(id_override_old, &rnaptr_src); - RNA_id_pointer_create(id_override_new, &rnaptr_dst); - - /* We remove any operation tagged with `IDOVERRIDE_LIBRARY_FLAG_IDPOINTER_MATCH_REFERENCE`, - * that way the potentially new pointer will be properly kept, when old one is still valid - * too (typical case: assigning new ID to some usage, while old one remains used elsewhere - * in the override hierarchy). */ - LISTBASE_FOREACH_MUTABLE ( - IDOverrideLibraryProperty *, op, &id_override_new->override_library->properties) { - LISTBASE_FOREACH_MUTABLE (IDOverrideLibraryPropertyOperation *, opop, &op->operations) { - if (opop->flag & IDOVERRIDE_LIBRARY_FLAG_IDPOINTER_MATCH_REFERENCE) { - lib_override_library_property_operation_clear(opop); - BLI_freelinkN(&op->operations, opop); - } - } - if (BLI_listbase_is_empty(&op->operations)) { - BKE_lib_override_library_property_delete(id_override_new->override_library, op); + ID *id_override_new = id->newid; + if (!ID_IS_OVERRIDE_LIBRARY_REAL(id_override_new)) { + continue; + } + + ID *id_override_old = static_cast(BLI_ghash_lookup(linkedref_to_old_override, id)); + if (id_override_old == nullptr) { + continue; + } + + if (ID_IS_OVERRIDE_LIBRARY_REAL(id_override_old)) { + /* Apply rules on new override ID using old one as 'source' data. */ + /* Note that since we already remapped ID pointers in old override IDs to new ones, we + * can also apply ID pointer override rules safely here. */ + PointerRNA rnaptr_src, rnaptr_dst; + RNA_id_pointer_create(id_override_old, &rnaptr_src); + RNA_id_pointer_create(id_override_new, &rnaptr_dst); + + /* We remove any operation tagged with `IDOVERRIDE_LIBRARY_FLAG_IDPOINTER_MATCH_REFERENCE`, + * that way the potentially new pointer will be properly kept, when old one is still valid + * too (typical case: assigning new ID to some usage, while old one remains used elsewhere + * in the override hierarchy). */ + LISTBASE_FOREACH_MUTABLE ( + IDOverrideLibraryProperty *, op, &id_override_new->override_library->properties) { + LISTBASE_FOREACH_MUTABLE (IDOverrideLibraryPropertyOperation *, opop, &op->operations) { + if (opop->flag & IDOVERRIDE_LIBRARY_FLAG_IDPOINTER_MATCH_REFERENCE) { + lib_override_library_property_operation_clear(opop); + BLI_freelinkN(&op->operations, opop); } } - - RNA_struct_override_apply(bmain, - &rnaptr_dst, - &rnaptr_src, - nullptr, - id_override_new->override_library, - do_hierarchy_enforce ? - RNA_OVERRIDE_APPLY_FLAG_IGNORE_ID_POINTERS : - RNA_OVERRIDE_APPLY_FLAG_NOP); + if (BLI_listbase_is_empty(&op->operations)) { + BKE_lib_override_library_property_delete(id_override_new->override_library, op); + } } - BLI_linklist_prepend(&id_override_old_list, id_override_old); + RNA_struct_override_apply(bmain, + &rnaptr_dst, + &rnaptr_src, + nullptr, + id_override_new->override_library, + do_hierarchy_enforce ? RNA_OVERRIDE_APPLY_FLAG_IGNORE_ID_POINTERS : + RNA_OVERRIDE_APPLY_FLAG_NOP); } + + BLI_linklist_prepend(&id_override_old_list, id_override_old); } FOREACH_MAIN_ID_END; -- cgit v1.2.3 From afa4f8f3ce0260262e98ee3804a7ae40d931e0d9 Mon Sep 17 00:00:00 2001 From: Bastien Montagne Date: Tue, 30 Aug 2022 15:17:16 +0200 Subject: LibOverride: Minor resync optimization by removing unuecessary processing. Not much to gain here, but can make resync faster by a few percents when dealing with linked overrides and such. --- source/blender/blenkernel/intern/lib_override.cc | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) (limited to 'source/blender/blenkernel/intern') diff --git a/source/blender/blenkernel/intern/lib_override.cc b/source/blender/blenkernel/intern/lib_override.cc index 94642ad2116..29f8b26c296 100644 --- a/source/blender/blenkernel/intern/lib_override.cc +++ b/source/blender/blenkernel/intern/lib_override.cc @@ -1802,6 +1802,10 @@ static bool lib_override_library_resync(Main *bmain, lib_override_hierarchy_dependencies_recursive_tag(&data); FOREACH_MAIN_ID_BEGIN (bmain, id) { + if ((id->lib != id_root->lib) || !ID_IS_OVERRIDE_LIBRARY(id)) { + continue; + } + /* IDs that get fully removed from linked data remain as local overrides (using place-holder * linked IDs as reference), but they are often not reachable from any current valid local * override hierarchy anymore. This will ensure they get properly deleted at the end of this @@ -1815,10 +1819,6 @@ static bool lib_override_library_resync(Main *bmain, id->tag |= LIB_TAG_MISSING; } - if ((id->lib != id_root->lib) || !ID_IS_OVERRIDE_LIBRARY(id)) { - continue; - } - /* While this should not happen in typical cases (and won't be properly supported here), * user is free to do all kind of very bad things, including having different local * overrides of a same linked ID in a same hierarchy. */ @@ -2401,6 +2401,11 @@ static void lib_override_library_main_resync_on_library_indirect_level( if (!ID_IS_OVERRIDE_LIBRARY_REAL(id)) { continue; } + + if (!lib_override_resync_id_lib_level_is_valid(id, library_indirect_level, true)) { + continue; + } + if (id->tag & (LIB_TAG_DOIT | LIB_TAG_MISSING)) { /* We already processed that ID as part of another ID's hierarchy. */ continue; -- cgit v1.2.3