griefith/test
1
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

Revert "Refactor: Move 'need link' and 'need expand' ID tags to runtime readfile data."

Browse Source 
Creates a very mysterious crash in nodetree code when using deprecated
'full undo'. Needs more investigation, we need to understand what's
happening here!

This reverts commit 2612b27e42.
This commit is contained in:
Bastien Montagne
2025-02-01 19:35:12 +01:00
parent 8abac73cbd
commit aff2cf97a1
4 changed files with 49 additions and 71 deletions
Download Patch File Download Diff File Expand all files Collapse all files

29
source/blender/blenloader/BLO_readfile.hh
View File

@@ -489,8 +489,7 @@ using BLOExpandDoitCallback = void (*)(void *fdhandle, Main *mainvar, void *idv)
/**
* Loop over all ID data in Main to mark relations.
* Set #ID_Readfile_Data::Tags.needs_expanding to mark expanding. Flags get
* cleared after expanding.
* Set (id->tag & ID_TAG_NEED_EXPAND) to mark expanding. Flags get cleared after expanding.
*
* \param fdhandle: usually file-data, or own handle. May be nullptr.
* \param mainvar: the Main database to expand.
@@ -546,24 +545,7 @@ short BLO_version_from_file(const char *filepath);
*/
struct ID_Readfile_Data {
struct Tags {
/* General ID reading related tags. */
/**
* Mark ID placeholders for linked data-blocks needing to be read from their library
* blendfiles.
*/
bool is_link_placeholder : 1;
/**
* Mark IDs needing to be expanded (only done once). See #BLO_expand_main.
*/
bool needs_expanding : 1;
/**
* Mark IDs needing to be 'lib-linked', i.e. to get their pointers to other data-blocks
* updated from the 'UID' values stored in `.blend` files to the new, actual pointers.
*/
bool needs_linking : 1;
/* Specific ID-type reading/versioning related tags. */
bool is_id_link_placeholder : 1;
/**
* Set when this ID used a legacy Action, in which case it also should pick
@@ -581,13 +563,6 @@ struct ID_Readfile_Data {
*/
ID_Readfile_Data::Tags BLO_readfile_id_runtime_tags(ID &id);
/**
* Create the `readfile_data` if needed, and return `id->runtime.readfile_data->tags`.
*
* Use it instead of #BLO_readfile_id_runtime_tags when tags need to be set.
*/
ID_Readfile_Data::Tags &BLO_readfile_id_runtime_tags_for_write(ID &id);
/**
* Free the ID_Readfile_Data of all IDs in this bmain and all their embedded IDs.
*

71
source/blender/blenloader/intern/readfile.cc
View File

@@ -2133,12 +2133,6 @@ ID_Readfile_Data::Tags BLO_readfile_id_runtime_tags(ID &id)
return id.runtime.readfile_data->tags;
}
ID_Readfile_Data::Tags &BLO_readfile_id_runtime_tags_for_write(ID &id)
{
readfile_id_runtime_data_ensure(id);
return id.runtime.readfile_data->tags;
}
void BLO_readfile_id_runtime_data_free(ID &id)
{
MEM_SAFE_FREE(id.runtime.readfile_data);
@@ -2221,7 +2215,7 @@ static void direct_link_id_common(BlendDataReader *reader,
id->runtime.readfile_data->tags = id_read_tags;
}
if (BLO_readfile_id_runtime_tags(*id).is_link_placeholder) {
if (BLO_readfile_id_runtime_tags(*id).is_id_link_placeholder) {
/* For placeholder we only need to set the tag and properly initialize generic ID fields above,
* no further data to read. */
return;
@@ -2328,8 +2322,8 @@ static bool scene_validate_setscene__liblink(Scene *sce, const int totscene)
/* This runs per library (before each libraries #Main has been joined),
* so we can't step into other libraries since `totscene` is only for this library.
*
* Also, other libraries may not have been linked yet, while we could check for
* #ID_Readfile_Data::Tags.needs_linking the library pointer check is sufficient. */
* Also, other libraries may not have been linked yet,
* while we could check #ID_TAG_NEED_LINK the library pointer check is sufficient. */
if (sce->id.lib != sce_iter->id.lib) {
return true;
}
@@ -2533,7 +2527,7 @@ static bool direct_link_id(FileData *fd,
/* Read part of datablock that is common between real and embedded datablocks. */
direct_link_id_common(&reader, main->curlib, id, id_old, tag, id_read_tags);
if (BLO_readfile_id_runtime_tags(*id).is_link_placeholder) {
if (BLO_readfile_id_runtime_tags(*id).is_id_link_placeholder) {
/* For placeholder we only need to set the tag, no further data to read. */
id->tag = tag;
return true;
@@ -2788,8 +2782,8 @@ static void read_libblock_undo_restore_identical(
/* Do not add ID_TAG_NEW here, this should not be needed/used in undo case anyway (as
* this is only for do_version-like code), but for sake of consistency, and also because
* it will tell us which ID is re-used from old Main, and which one is actually newly read. */
/* Also do not set #ID_Readfile_Data::Tags.needs_linking, this ID will never be re-liblinked,
* hence that tag will never be cleared, leading to critical issue in link/append code. */
/* Also do not add ID_TAG_NEED_LINK, this ID will never be re-liblinked, hence that tag will
* never be cleared, leading to critical issue in link/append code. */
/* Some tags need to be preserved here. */
id_old->tag = ((id_tag | ID_TAG_UNDO_OLD_ID_REUSED_UNCHANGED) & ~ID_TAG_KEEP_ON_UNDO) |
(id_old->tag & ID_TAG_KEEP_ON_UNDO);
@@ -2862,8 +2856,7 @@ static void read_libblock_undo_restore_at_old_address(FileData *fd, Main *main,
* lib-linking to restore some data that should never be affected by undo, e.g. the 3D cursor of
* #Scene. */
id_old->orig_id = id;
id_old->tag |= ID_TAG_UNDO_OLD_ID_REREAD_IN_PLACE;
BLO_readfile_id_runtime_tags_for_write(*id_old).needs_linking = true;
id_old->tag |= ID_TAG_UNDO_OLD_ID_REREAD_IN_PLACE | ID_TAG_NEED_LINK;
BLI_addtail(new_lb, id_old);
BLI_addtail(old_lb, id);
@@ -2974,7 +2967,6 @@ static BHead *read_libblock(FileData *fd,
Main *main,
BHead *bhead,
int id_tag,
ID_Readfile_Data::Tags id_read_tags,
const bool placeholder_set_indirect_extern,
ID **r_id)
{
@@ -3043,12 +3035,13 @@ static BHead *read_libblock(FileData *fd,
/* Set tag for new datablock to indicate lib linking and versioning needs
* to be done still. */
id_tag |= ID_TAG_NEW;
id_read_tags.needs_linking = true;
id_tag |= (ID_TAG_NEED_LINK | ID_TAG_NEW);
ID_Readfile_Data::Tags id_read_tags{};
if (bhead->code == ID_LINK_PLACEHOLDER) {
/* Read placeholder for linked datablock. */
id_read_tags.is_link_placeholder = true;
id_read_tags.is_id_link_placeholder = true;
if (placeholder_set_indirect_extern) {
if (id->flag & ID_FLAG_INDIRECT_WEAK_LINK) {
@@ -3362,7 +3355,7 @@ static void lib_link_all(FileData *fd, Main *bmain)
/* This ID has been re-used from 'old' bmain. Since it was therefore unchanged across
* current undo step, and old IDs re-use their old memory address, we do not need to liblink
* it at all. */
BLI_assert(!BLO_readfile_id_runtime_tags(*id).needs_linking);
BLI_assert((id->tag & ID_TAG_NEED_LINK) == 0);
/* Some data that should be persistent, like the 3DCursor or the tool settings, are
* stored in IDs affected by undo, like Scene. So this requires some specific handling. */
@@ -3376,7 +3369,7 @@ static void lib_link_all(FileData *fd, Main *bmain)
continue;
}
if (BLO_readfile_id_runtime_tags(*id).needs_linking) {
if ((id->tag & ID_TAG_NEED_LINK) != 0) {
/* Not all original pointer values can be considered as valid.
* Handling of DNA deprecated data should never be needed in undo case. */
const LibraryForeachIDFlag flag = IDWALK_NO_ORIG_POINTERS_ACCESS | IDWALK_INCLUDE_UI |
@@ -3387,7 +3380,7 @@ static void lib_link_all(FileData *fd, Main *bmain)
after_liblink_id_process(&reader, id);
BLO_readfile_id_runtime_tags_for_write(*id).needs_linking = false;
id->tag &= ~ID_TAG_NEED_LINK;
}
/* Some data that should be persistent, like the 3DCursor or the tool settings, are
@@ -3409,7 +3402,7 @@ static void lib_link_all(FileData *fd, Main *bmain)
/* Double check we do not have any 'need link' tag remaining, this should never be the case once
* this function has run. */
FOREACH_MAIN_ID_BEGIN (bmain, id) {
BLI_assert(!BLO_readfile_id_runtime_tags(*id).needs_linking);
BLI_assert((id->tag & ID_TAG_NEED_LINK) == 0);
}
FOREACH_MAIN_ID_END;
#endif
@@ -3724,7 +3717,7 @@ BlendFileData *blo_read_file_internal(FileData *fd, const char *filepath)
* to the file format definition. So we can use the entry at the
* end of mainlist, added in direct_link_library. */
Main *libmain = static_cast<Main *>(mainlist.last);
bhead = read_libblock(fd, libmain, bhead, 0, {}, true, nullptr);
bhead = read_libblock(fd, libmain, bhead, 0, true, nullptr);
}
break;
/* in 2.50+ files, the file identifier for screens is patched, forward compatibility */
@@ -3740,7 +3733,7 @@ BlendFileData *blo_read_file_internal(FileData *fd, const char *filepath)
bhead = blo_bhead_next(fd, bhead);
}
else {
bhead = read_libblock(fd, bfd->main, bhead, ID_TAG_LOCAL, {}, false, nullptr);
bhead = read_libblock(fd, bfd->main, bhead, ID_TAG_LOCAL, false, nullptr);
}
}
else {
@@ -4148,7 +4141,7 @@ static void expand_doit_library(void *fdhandle, Main *mainvar, void *old)
if (id == nullptr) {
/* ID has not been read yet, add placeholder to the main of the
* library it belongs to, so that it will be read later. */
read_libblock(fd, libmain, bhead, fd->id_tag_extra | ID_TAG_INDIRECT, {}, false, &id);
read_libblock(fd, libmain, bhead, fd->id_tag_extra | ID_TAG_INDIRECT, false, &id);
BLI_assert(id != nullptr);
id_sort_by_name(which_libbase(libmain, GS(id->name)), id, static_cast<ID *>(id->prev));
@@ -4161,7 +4154,7 @@ static void expand_doit_library(void *fdhandle, Main *mainvar, void *old)
else {
/* Convert any previously read weak link to regular link
* to signal that we want to read this data-block. */
if (BLO_readfile_id_runtime_tags(*id).is_link_placeholder) {
if (BLO_readfile_id_runtime_tags(*id).is_id_link_placeholder) {
id->flag &= ~ID_FLAG_INDIRECT_WEAK_LINK;
}
@@ -4192,10 +4185,8 @@ static void expand_doit_library(void *fdhandle, Main *mainvar, void *old)
/* Data-block in same library. */
ID *id = library_id_is_yet_read(fd, mainvar, bhead);
if (id == nullptr) {
ID_Readfile_Data::Tags id_read_tags{};
id_read_tags.needs_expanding = true;
read_libblock(
fd, mainvar, bhead, fd->id_tag_extra | ID_TAG_INDIRECT, id_read_tags, false, &id);
fd, mainvar, bhead, fd->id_tag_extra | ID_TAG_NEED_EXPAND | ID_TAG_INDIRECT, false, &id);
BLI_assert(id != nullptr);
id_sort_by_name(which_libbase(mainvar, GS(id->name)), id, static_cast<ID *>(id->prev));
}
@@ -4203,7 +4194,7 @@ static void expand_doit_library(void *fdhandle, Main *mainvar, void *old)
/* Convert any previously read weak link to regular link to signal that we want to read this
* data-block. Note that this function also visits already-loaded data-blocks, and thus their
* `readfile_data` field might already have been freed. */
if (BLO_readfile_id_runtime_tags(*id).is_link_placeholder) {
if (BLO_readfile_id_runtime_tags(*id).is_id_link_placeholder) {
id->flag &= ~ID_FLAG_INDIRECT_WEAK_LINK;
}
@@ -4257,7 +4248,7 @@ void BLO_expand_main(void *fdhandle, Main *mainvar, BLOExpandDoitCallback callba
ID *id_iter;
FOREACH_MAIN_ID_BEGIN (mainvar, id_iter) {
if (!BLO_readfile_id_runtime_tags(*id_iter).needs_expanding) {
if ((id_iter->tag & ID_TAG_NEED_EXPAND) == 0) {
continue;
}
@@ -4273,7 +4264,7 @@ void BLO_expand_main(void *fdhandle, Main *mainvar, BLOExpandDoitCallback callba
BKE_library_foreach_ID_link(nullptr, id_iter, expand_cb, &expander, flag);
do_it = true;
BLO_readfile_id_runtime_tags_for_write(*id_iter).needs_expanding = false;
id_iter->tag &= ~ID_TAG_NEED_EXPAND;
}
FOREACH_MAIN_ID_END;
}
@@ -4303,9 +4294,7 @@ static ID *link_named_part(
if (id == nullptr) {
/* not read yet */
const int tag = ((force_indirect ? ID_TAG_INDIRECT : ID_TAG_EXTERN) | fd->id_tag_extra);
ID_Readfile_Data::Tags id_read_tags{};
id_read_tags.needs_expanding = true;
read_libblock(fd, mainl, bhead, tag, id_read_tags, false, &id);
read_libblock(fd, mainl, bhead, tag | ID_TAG_NEED_EXPAND, false, &id);
if (id) {
/* sort by name in list */
@@ -4619,7 +4608,7 @@ static int has_linked_ids_to_read(Main *mainvar)
while (a--) {
LISTBASE_FOREACH (ID *, id, lbarray[a]) {
if (BLO_readfile_id_runtime_tags(*id).is_link_placeholder &&
if (BLO_readfile_id_runtime_tags(*id).is_id_link_placeholder &&
!(id->flag & ID_FLAG_INDIRECT_WEAK_LINK))
{
return true;
@@ -4652,13 +4641,13 @@ static void read_library_linked_id(
library_parent_filepath(mainvar->curlib));
}
BLO_readfile_id_runtime_tags_for_write(*id).is_link_placeholder = false;
id->runtime.readfile_data->tags.is_id_link_placeholder = false;
id->flag &= ~ID_FLAG_INDIRECT_WEAK_LINK;
if (bhead) {
BLO_readfile_id_runtime_tags_for_write(*id).needs_expanding = true;
id->tag |= ID_TAG_NEED_EXPAND;
// printf("read lib block %s\n", id->name);
read_libblock(fd, mainvar, bhead, id->tag, BLO_readfile_id_runtime_tags(*id), false, r_id);
read_libblock(fd, mainvar, bhead, id->tag, false, r_id);
}
else {
CLOG_INFO(&LOG,
@@ -4697,7 +4686,7 @@ static void read_library_linked_ids(FileData *basefd,
while (id) {
ID *id_next = static_cast<ID *>(id->next);
if (BLO_readfile_id_runtime_tags(*id).is_link_placeholder &&
if (BLO_readfile_id_runtime_tags(*id).is_id_link_placeholder &&
!(id->flag & ID_FLAG_INDIRECT_WEAK_LINK))
{
BLI_remlink(lbarray[a], id);
@@ -4761,7 +4750,7 @@ static void read_library_clear_weak_links(FileData *basefd, ListBase *mainlist,
/* This function also visits already-loaded data-blocks, and thus their
* `readfile_data` field might already have been freed. */
if (BLO_readfile_id_runtime_tags(*id).is_link_placeholder &&
if (BLO_readfile_id_runtime_tags(*id).is_id_link_placeholder &&
(id->flag & ID_FLAG_INDIRECT_WEAK_LINK))
{
CLOG_INFO(&LOG, 3, "Dropping weak link to '%s'", id->name);

6
source/blender/blenloader/intern/versioning_legacy.cc
View File

@@ -485,7 +485,8 @@ void blo_do_versions_pre250(FileData *fd, Library *lib, Main *bmain)
/* tex->extend and tex->imageflag have changed: */
Tex *tex = static_cast<Tex *>(bmain->textures.first);
while (tex) {
if (BLO_readfile_id_runtime_tags(tex->id).needs_linking) {
if (tex->id.tag & ID_TAG_NEED_LINK) {
if (tex->extend == 0) {
if (tex->xrepeat || tex->yrepeat) {
tex->extend = TEX_REPEAT;
@@ -2269,8 +2270,7 @@ void blo_do_versions_pre250(FileData *fd, Library *lib, Main *bmain)
part->id.lib = ob->id.lib;
part->id.us--;
BLO_readfile_id_runtime_tags_for_write(part->id).needs_linking =
BLO_readfile_id_runtime_tags(ob->id).needs_linking;
part->id.tag |= (ob->id.tag & ID_TAG_NEED_LINK);
psys->totpart = 0;
psys->flag = PSYS_CURRENT;

14
source/blender/makesdna/DNA_ID.h
View File

@@ -874,6 +874,20 @@ enum {
*/
ID_TAG_PRE_EXISTING = 1 << 13,
/**
* Tag used internally in `readfile.cc`, to mark IDs needing to be expanded (only done once).
*
* RESET_AFTER_USE
*/
ID_TAG_NEED_EXPAND = 1 << 14,
/**
* Tag used internally in `readfile.cc`, to mark IDs needing to be 'lib-linked', i.e. to get
* their pointers to other data-blocks updated from the 'UID' values stored in `.blend` files to
* the new, actual pointers.
*
* RESET_AFTER_USE
*/
ID_TAG_NEED_LINK = 1 << 16,
/**
* ID is being re-used from the old Main (instead of read from memfile), during memfile undo
* processing, because it was detected as unchanged.