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

Refactor: Cleanup Library FileData handling.

Browse Source 
Add an explicit 'is owning' tag for libraries' FileData pointer, and
factorize code cleaning it up into a small util function.

Also allows to get rid of the weird ugly exception case in `link_end`
code for the 'root' library filedata, which is usually owned by caller
code and should not be freed here.

Pull Request: https://projects.blender.org/blender/blender/pulls/142723
This commit is contained in:
Bastien Montagne
2025-07-21 17:59:17 +02:00
committed by Bastien Montagne
parent 828bd83734
commit 7c7c68fd7a
3 changed files with 32 additions and 20 deletions
Download Patch File Download Diff File Expand all files Collapse all files

8
source/blender/blenkernel/BKE_library.hh
View File

@@ -23,7 +23,15 @@ struct LibraryRuntime {
/* Used for efficient calculations of unique names. */
UniqueName_Map *name_map = nullptr;
/**
* Filedata (i.e. opened blendfile) source of this library data.
*/
FileData *filedata = nullptr;
/**
* Whether this library is owning its filedata pointer (and therefore should take care of
* releasing it as part of the readfile process).
*/
bool is_filedata_owner = false;
/**
* Run-time only, absolute file-path (set on read).

1
source/blender/blenkernel/intern/library.cc
View File

@@ -86,6 +86,7 @@ static void library_copy_data(Main *bmain,
}
library_dst->runtime = MEM_new<LibraryRuntime>(__func__, *library_src->runtime);
library_dst->runtime->filedata = nullptr;
library_dst->runtime->is_filedata_owner = false;
library_dst->runtime->name_map = nullptr;
}

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

@@ -2431,6 +2431,19 @@ static void lib_link_scenes_check_set(Main *bmain)
/** \name Read ID: Library
* \{ */
static void library_filedata_release(Library *lib)
{
if (lib->runtime->filedata) {
BLI_assert(lib->runtime->versionfile != 0);
if (lib->runtime->is_filedata_owner) {
blo_filedata_free(lib->runtime->filedata);
}
lib->runtime->filedata = nullptr;
}
lib->runtime->is_filedata_owner = false;
}
static void direct_link_library(FileData *fd, Library *lib, Main *main)
{
/* Make sure we have full path in lib->runtime->filepath_abs */
@@ -3995,11 +4008,7 @@ BlendFileData *blo_read_file_internal(FileData *fd, const char *filepath)
LISTBASE_FOREACH_MUTABLE (Library *, lib, &bfd->main->libraries) {
/* Now we can clear this runtime library filedata, it is not needed anymore. */
if (lib->runtime->filedata) {
BLI_assert(lib->runtime->versionfile != 0);
blo_filedata_free(lib->runtime->filedata);
lib->runtime->filedata = nullptr;
}
library_filedata_release(lib);
/* If no data-blocks were read from a library (should only happen when all references to a
* library's data are `ID_FLAG_INDIRECT_WEAK_LINK`), its versionfile will still be zero and
* it can be deleted.
@@ -4010,7 +4019,7 @@ BlendFileData *blo_read_file_internal(FileData *fd, const char *filepath)
* valid version number as the file was read to search for the linked IDs.
* - In case the library blendfile does not exist, its local Library ID will get the version
* of the current local Main (i.e. the loaded blendfile). */
else if (lib->runtime->versionfile == 0) {
if (lib->runtime->versionfile == 0) {
#ifndef NDEBUG
ID *id_iter;
FOREACH_MAIN_ID_BEGIN (bfd->main, id_iter) {
@@ -4552,6 +4561,8 @@ static Main *library_link_begin(Main *mainvar,
fd->fd_bmain = mainl;
if (mainl->curlib) {
mainl->curlib->runtime->filedata = fd;
/* This filedata is owned and managed by the calling code. */
mainl->curlib->runtime->is_filedata_owner = false;
}
/* needed for do_version */
@@ -4759,20 +4770,10 @@ void BLO_library_link_end(Main *mainl,
LISTBASE_FOREACH (Library *, lib, &params->bmain->libraries) {
/* Now we can clear this runtime library filedata, it is not needed anymore. */
if (lib->runtime->filedata == reinterpret_cast<FileData *>(*bh)) {
/* The filedata is owned and managed by caller code, only clear matching library pointer. */
lib->runtime->filedata = nullptr;
}
else if (lib->runtime->filedata) {
/* In case other libraries had to be read as dependencies of the main linked one, they need
* to be cleared here.
*
* TODO: In the future, could be worth keeping them in case data are linked from several
* libraries at once? To avoid closing and re-opening the same file several times. Would need
* a global cleanup callback then once all linking is done, though. */
blo_filedata_free(lib->runtime->filedata);
lib->runtime->filedata = nullptr;
}
/* TODO: In the future, could be worth keeping them in case data are linked from several
* libraries at once? To avoid closing and re-opening the same file several times. Would need
* a global cleanup callback then once all linking is done, though. */
library_filedata_release(lib);
}
*bh = reinterpret_cast<BlendHandle *>(fd);
@@ -5002,6 +5003,7 @@ static FileData *read_library_file_data(FileData *basefd, Main *bmain, Main *lib
fd->libmap = oldnewmap_new();
lib_bmain->curlib->runtime->filedata = fd;
lib_bmain->curlib->runtime->is_filedata_owner = true;
lib_bmain->versionfile = fd->fileversion;
/* subversion */
@@ -5010,6 +5012,7 @@ static FileData *read_library_file_data(FileData *basefd, Main *bmain, Main *lib
}
else {
lib_bmain->curlib->runtime->filedata = nullptr;
lib_bmain->curlib->runtime->is_filedata_owner = false;
lib_bmain->curlib->id.tag |= ID_TAG_MISSING;
/* Set lib version to current main one... Makes assert later happy. */
lib_bmain->versionfile = lib_bmain->curlib->runtime->versionfile = bmain->versionfile;