Fix (studio-reported) liboverride: hierarchy root ensure fails in some cases.

The code ensuring a valid liboverride hierarchy root ID pointer for all
liboverrides in a given Main would fail in some cases, and crash on
asserts (in Debug builds) or due to corrupted data later in code
(release builds).

The main issue fixed here is re-entry in case of dependency loops (a
same ID being checked more than once within the same chain of recursive
calls to ensure its hierarchy root is valid). Solved the usual way now,
using the intermediate 'PROCESSING' flag instead of setting immediately
the 'PROCESSED' one, when recursively processing the chain of
dependencies.

A second issue fixed in that code was that in some cases, it could leave
the invalid existing hierarchy root pointer unchanged, because it could
not find a 'valid enough' alternative.

NOTE: This data corruption is presumably caused by 'make local'
operations on liboverride hierarchy root IDs. This will be addressed as
a second commit.

For reference, issue is reproducible when opening
`/promo/splash/040_0010.lighting.splash.blend` from Pets SVN repo r3106.
This commit is contained in:
Bastien Montagne
2023-10-10 16:43:44 +02:00
parent 02ee5a7693
commit c5e10920f9

View File

@@ -1573,9 +1573,15 @@ static ID *lib_override_root_find(Main *bmain, ID *id, const int curr_level, int
BKE_lib_override_library_get(bmain, id, nullptr, &id_owner);
return lib_override_root_find(bmain, id_owner, curr_level + 1, &best_level_placeholder);
}
/* This way we won't process again that ID, should we encounter it again through another
* relationship hierarchy. */
entry->tags |= MAINIDRELATIONS_ENTRY_TAGS_PROCESSED;
if (entry->tags & MAINIDRELATIONS_ENTRY_TAGS_INPROGRESS) {
/* Re-processing an entry already being processed higher in the callgraph (re-entry caused by a
* dependency loops). Just do nothing, there is no more usefull info to provide here. */
return nullptr;
}
/* Flag this entry to avoid re-processing it in case some dependency loop leads to it again
* downwards in the callstack. */
entry->tags |= MAINIDRELATIONS_ENTRY_TAGS_INPROGRESS;
int best_level_candidate = curr_level;
ID *best_root_id_candidate = id;
@@ -1618,6 +1624,11 @@ static ID *lib_override_root_find(Main *bmain, ID *id, const int curr_level, int
BLI_assert(best_root_id_candidate != nullptr);
BLI_assert((best_root_id_candidate->flag & LIB_EMBEDDED_DATA_LIB_OVERRIDE) == 0);
/* This way this ID won't be processed again, should it be encountered again through another
* relationship hierarchy. */
entry->tags &= ~MAINIDRELATIONS_ENTRY_TAGS_INPROGRESS;
entry->tags |= MAINIDRELATIONS_ENTRY_TAGS_PROCESSED;
*r_best_level = best_level_candidate;
return best_root_id_candidate;
}
@@ -1663,7 +1674,10 @@ static void lib_override_root_hierarchy_set(
bmain->relations->relations_from_pointers, id->override_library->reference));
BLI_assert(entry != nullptr);
bool do_replace_root = false;
/* Enforce replacing hierarchy root if the current one is invalid. */
bool do_replace_root = (!id->override_library->hierarchy_root ||
!ID_IS_OVERRIDE_LIBRARY_REAL(id->override_library->hierarchy_root) ||
id->override_library->hierarchy_root->lib != id->lib);
for (MainIDRelationsEntryItem *from_id_entry = entry->from_ids; from_id_entry != nullptr;
from_id_entry = from_id_entry->next)
{
@@ -1770,6 +1784,7 @@ void BKE_lib_override_library_main_hierarchy_root_ensure(Main *bmain)
}
BKE_main_relations_tag_set(bmain, MAINIDRELATIONS_ENTRY_TAGS_PROCESSED, false);
BKE_main_relations_tag_set(bmain, MAINIDRELATIONS_ENTRY_TAGS_INPROGRESS, false);
int best_level = 0;
ID *id_root = lib_override_root_find(bmain, id, best_level, &best_level);