Fix #107573: Purging orphan data deletes used data.

The logic in the initial commit (97dd107070) was broken in some cases,
and would end up tagging as unused IDs that had valid usages. It is
reverted by this commit..

For this new solution to #98029 (deleting unused archipelagos of data),
the logic handling dependency loops detection is reworked to rely on a
new tag in the relations entry of the relevant IDs, instead of
pre-tagging as unused the ID itself.

Further more, when a dependency loop is detected, the IDs in it cannot
be immediately tagged as unused, since it may be that the entry point
of that loop is later detected as actually used. So their relations
entries are not tagged as processed, and only the entry point of the
potential archipelago can be checked in that case, outside of the
recursive processing of dependencies.

The other IDs of the archipelago will then be processed again later, in
a state where at least one ID in the archipelago has a known state for
sure, which then allows for a safe evaluation of the other related data.

This commit should be backported to 3.3LTS.
This commit is contained in:
Bastien Montagne 2023-05-03 16:01:25 +02:00
parent 109c1b92cd
commit ac1ac6be9a
2 changed files with 102 additions and 32 deletions

View File

@ -79,15 +79,26 @@ typedef enum eMainIDRelationsEntryTags {
/* Generic tag marking the entry as to be processed. */
MAINIDRELATIONS_ENTRY_TAGS_DOIT = 1 << 0,
/* Generic tag marking the entry as processed in the `to` direction (i.e. we processed the IDs
* used by this item). */
MAINIDRELATIONS_ENTRY_TAGS_PROCESSED_TO = 1 << 1,
/* Generic tag marking the entry as processed in the `from` direction (i.e. we processed the IDs
* using by this item). */
MAINIDRELATIONS_ENTRY_TAGS_PROCESSED_FROM = 1 << 2,
/* Generic tag marking the entry as processed in the `to` direction (i.e. the IDs used by this
* item have been processed). */
MAINIDRELATIONS_ENTRY_TAGS_PROCESSED_TO = 1 << 4,
/* Generic tag marking the entry as processed in the `from` direction (i.e. the IDs using this
* item have been processed). */
MAINIDRELATIONS_ENTRY_TAGS_PROCESSED_FROM = 1 << 5,
/* Generic tag marking the entry as processed. */
MAINIDRELATIONS_ENTRY_TAGS_PROCESSED = MAINIDRELATIONS_ENTRY_TAGS_PROCESSED_TO |
MAINIDRELATIONS_ENTRY_TAGS_PROCESSED_FROM,
/* Generic tag marking the entry as being processed in the `to` direction (i.e. the IDs used by
* this item are being processed). Useful for dependency loops detection and handling. */
MAINIDRELATIONS_ENTRY_TAGS_INPROGRESS_TO = 1 << 8,
/* Generic tag marking the entry as being processed in the `from` direction (i.e. the IDs using
* this item are being processed). Useful for dependency loops detection and handling. */
MAINIDRELATIONS_ENTRY_TAGS_INPROGRESS_FROM = 1 << 9,
/* Generic tag marking the entry as being processed. Useful for dependency loops detection and
* handling. */
MAINIDRELATIONS_ENTRY_TAGS_INPROGRESS = MAINIDRELATIONS_ENTRY_TAGS_INPROGRESS_TO |
MAINIDRELATIONS_ENTRY_TAGS_INPROGRESS_FROM,
} eMainIDRelationsEntryTags;
typedef struct MainIDRelations {

View File

@ -657,7 +657,10 @@ void BKE_library_ID_test_usages(Main *bmain, void *idv, bool *is_used_local, boo
}
/* ***** IDs usages.checking/tagging. ***** */
static void lib_query_unused_ids_tag_recurse(Main *bmain,
/* Returns `true` if given ID is detected as part of at least one dependency loop, false otherwise.
*/
static bool lib_query_unused_ids_tag_recurse(Main *bmain,
const int tag,
const bool do_local_ids,
const bool do_linked_ids,
@ -667,31 +670,40 @@ static void lib_query_unused_ids_tag_recurse(Main *bmain,
/* We should never deal with embedded, not-in-main IDs here. */
BLI_assert((id->flag & LIB_EMBEDDED_DATA) == 0);
if ((!do_linked_ids && ID_IS_LINKED(id)) || (!do_local_ids && !ID_IS_LINKED(id))) {
return;
}
MainIDRelationsEntry *id_relations = BLI_ghash_lookup(bmain->relations->relations_from_pointers,
id);
if ((id_relations->tags & MAINIDRELATIONS_ENTRY_TAGS_PROCESSED) != 0) {
return;
return false;
}
else if ((id_relations->tags & MAINIDRELATIONS_ENTRY_TAGS_INPROGRESS) != 0) {
/* This ID has not yet been fully processed. If this condition is reached, it means this is a
* dependency loop case. */
return true;
}
if ((!do_linked_ids && ID_IS_LINKED(id)) || (!do_local_ids && !ID_IS_LINKED(id))) {
id_relations->tags |= MAINIDRELATIONS_ENTRY_TAGS_PROCESSED;
return false;
}
id_relations->tags |= MAINIDRELATIONS_ENTRY_TAGS_PROCESSED;
if ((id->tag & tag) != 0) {
return;
id_relations->tags |= MAINIDRELATIONS_ENTRY_TAGS_PROCESSED;
return false;
}
if ((id->flag & LIB_FAKEUSER) != 0) {
/* This ID is forcefully kept around, and therefore never unused, no need to check it further.
*/
return;
id_relations->tags |= MAINIDRELATIONS_ENTRY_TAGS_PROCESSED;
return false;
}
if (ELEM(GS(id->name), ID_WM, ID_WS, ID_SCE, ID_SCR, ID_LI)) {
/* Some 'root' ID types are never unused (even though they may not have actual users), unless
* their actual user-count is set to 0. */
return;
id_relations->tags |= MAINIDRELATIONS_ENTRY_TAGS_PROCESSED;
return false;
}
if (ELEM(GS(id->name), ID_IM)) {
@ -699,7 +711,8 @@ static void lib_query_unused_ids_tag_recurse(Main *bmain,
* orphaned/unused data. */
Image *image = (Image *)id;
if (image->source == IMA_SRC_VIEWER) {
return;
id_relations->tags |= MAINIDRELATIONS_ENTRY_TAGS_PROCESSED;
return false;
}
}
@ -714,12 +727,8 @@ static void lib_query_unused_ids_tag_recurse(Main *bmain,
* First recursively check all its valid users, if all of them can be tagged as
* unused, then we can tag this ID as such too. */
bool has_valid_from_users = false;
/* Preemptively consider this ID as unused. That way if there is a loop of dependency leading
* back to it, it won't create a fake 'valid user' detection.
* NOTE: there are some cases (like when fake user is set, or some ID types) which are never
* 'indirectly unused'. However, these have already been checked and early-returned above, so any
* ID reaching this point of the function can be tagged. */
id->tag |= tag;
bool is_part_of_dependency_loop = false;
id_relations->tags |= MAINIDRELATIONS_ENTRY_TAGS_INPROGRESS;
for (MainIDRelationsEntryItem *id_from_item = id_relations->from_ids; id_from_item != NULL;
id_from_item = id_from_item->next)
{
@ -736,24 +745,42 @@ static void lib_query_unused_ids_tag_recurse(Main *bmain,
BLI_assert(id_from != NULL);
}
lib_query_unused_ids_tag_recurse(
bmain, tag, do_local_ids, do_linked_ids, id_from, r_num_tagged);
if (lib_query_unused_ids_tag_recurse(
bmain, tag, do_local_ids, do_linked_ids, id_from, r_num_tagged))
{
/* Dependency loop case, ignore the `id_from` tag value here (as it should not be considered
* as valid yet), and presume that this is a 'valid user' case for now. . */
is_part_of_dependency_loop = true;
continue;
}
if ((id_from->tag & tag) == 0) {
has_valid_from_users = true;
break;
}
}
if (has_valid_from_users) {
/* This ID has 'valid' users, clear the 'tag as unused' preemptively set above. */
id->tag &= ~tag;
}
else {
/* This ID has no 'valid' users, its 'unused' tag preemptively set above can be kept. */
if (!has_valid_from_users && !is_part_of_dependency_loop) {
/* Tag the ID as unused, only in case it is not part of a dependency loop. */
id->tag |= tag;
if (r_num_tagged != NULL) {
r_num_tagged[INDEX_ID_NULL]++;
r_num_tagged[BKE_idtype_idcode_to_index(GS(id->name))]++;
}
}
/* This ID is not being processed anymore.
*
* However, we can only tag is as sucessfully processed if either it was detected as part of a
* valid usage hierarchy, or, if detected as unused, if it was not part of a dependency loop.
*
* Otherwise, this is an undecided state, it will be resolved at the entry point of this
* recursive process for the root id (see below in #BKE_lib_query_unused_ids_tag calling code).
*/
id_relations->tags &= ~MAINIDRELATIONS_ENTRY_TAGS_INPROGRESS;
if (has_valid_from_users || !is_part_of_dependency_loop) {
id_relations->tags |= MAINIDRELATIONS_ENTRY_TAGS_PROCESSED;
}
return is_part_of_dependency_loop;
}
void BKE_lib_query_unused_ids_tag(Main *bmain,
@ -789,7 +816,39 @@ void BKE_lib_query_unused_ids_tag(Main *bmain,
BKE_main_relations_create(bmain, 0);
FOREACH_MAIN_ID_BEGIN (bmain, id) {
lib_query_unused_ids_tag_recurse(bmain, tag, do_local_ids, do_linked_ids, id, r_num_tagged);
if (lib_query_unused_ids_tag_recurse(
bmain, tag, do_local_ids, do_linked_ids, id, r_num_tagged)) {
/* This root processed ID is part of one or more dependency loops.
*
* If it was not tagged, and its matching relations entry is not marked as processed, it
* means that it's the first encountered entry point of an 'unused archipelago' (i.e. the
* entry point to a set of IDs with relationships to each other, but no 'valid usage'
* relations to the current Blender file (like being part of a scene, etc.).
*
* So the entry can be tagged as processed, and the ID tagged as unused. */
if ((id->tag & tag) == 0) {
MainIDRelationsEntry *id_relations = BLI_ghash_lookup(
bmain->relations->relations_from_pointers, id);
if ((id_relations->tags & MAINIDRELATIONS_ENTRY_TAGS_PROCESSED) == 0) {
id_relations->tags |= MAINIDRELATIONS_ENTRY_TAGS_PROCESSED;
id->tag |= tag;
if (r_num_tagged != NULL) {
r_num_tagged[INDEX_ID_NULL]++;
r_num_tagged[BKE_idtype_idcode_to_index(GS(id->name))]++;
}
}
}
}
#ifndef NDEBUG
/* Relation entry for the root processed ID should always be marked as processed now. */
MainIDRelationsEntry *id_relations = BLI_ghash_lookup(
bmain->relations->relations_from_pointers, id);
if ((id_relations->tags & MAINIDRELATIONS_ENTRY_TAGS_PROCESSED) == 0) {
BLI_assert((id_relations->tags & MAINIDRELATIONS_ENTRY_TAGS_PROCESSED) != 0);
}
BLI_assert((id_relations->tags & MAINIDRELATIONS_ENTRY_TAGS_INPROGRESS) == 0);
#endif
}
FOREACH_MAIN_ID_END;
BKE_main_relations_free(bmain);