From 5812bc7d8908015b8a06d240da7964be0a19a2c3 Mon Sep 17 00:00:00 2001 From: Campbell Barton Date: Fri, 12 Mar 2021 00:34:21 +1100 Subject: [PATCH] Cleanup: split file read and setup into separate steps Currently file loading performs almost all reloading logic even in the case loading the file fails, causing the file to be in a state that isn't well defined: undo is cleared, timers are canceled & scripts are re-registered. --- source/blender/blenkernel/BKE_blendfile.h | 53 ++++---- .../blender/blenkernel/intern/blender_undo.c | 14 ++- source/blender/blenkernel/intern/blendfile.c | 115 +++++++++--------- .../blender/windowmanager/intern/wm_files.c | 64 +++++----- 4 files changed, 128 insertions(+), 118 deletions(-) diff --git a/source/blender/blenkernel/BKE_blendfile.h b/source/blender/blenkernel/BKE_blendfile.h index f73c4a70809..429e294a337 100644 --- a/source/blender/blenkernel/BKE_blendfile.h +++ b/source/blender/blenkernel/BKE_blendfile.h @@ -23,6 +23,7 @@ extern "C" { #endif +struct BlendFileData; struct BlendFileReadParams; struct ID; struct Main; @@ -31,36 +32,32 @@ struct ReportList; struct UserDef; struct bContext; -bool BKE_blendfile_read_ex(struct bContext *C, - const char *filepath, - const struct BlendFileReadParams *params, - struct ReportList *reports, - /* Extra args. */ - const bool startup_update_defaults, - const char *startup_app_template); -bool BKE_blendfile_read(struct bContext *C, - const char *filepath, - const struct BlendFileReadParams *params, - struct ReportList *reports); +void BKE_blendfile_read_setup_ex(struct bContext *C, + struct BlendFileData *bfd, + const struct BlendFileReadParams *params, + struct ReportList *reports, + /* Extra args. */ + const bool startup_update_defaults, + const char *startup_app_template); -bool BKE_blendfile_read_from_memory_ex(struct bContext *C, - const void *filebuf, - int filelength, - const struct BlendFileReadParams *params, - struct ReportList *reports, - /* Extra args. */ - const bool startup_update_defaults, - const char *startup_app_template); -bool BKE_blendfile_read_from_memory(struct bContext *C, - const void *filebuf, - int filelength, - const struct BlendFileReadParams *params, - struct ReportList *reports); +void BKE_blendfile_read_setup(struct bContext *C, + struct BlendFileData *bfd, + const struct BlendFileReadParams *params, + struct ReportList *reports); -bool BKE_blendfile_read_from_memfile(struct bContext *C, - struct MemFile *memfile, - const struct BlendFileReadParams *params, - struct ReportList *reports); +struct BlendFileData *BKE_blendfile_read(const char *filepath, + const struct BlendFileReadParams *params, + struct ReportList *reports); + +struct BlendFileData *BKE_blendfile_read_from_memory(const void *filebuf, + int filelength, + const struct BlendFileReadParams *params, + struct ReportList *reports); + +struct BlendFileData *BKE_blendfile_read_from_memfile(struct Main *bmain, + struct MemFile *memfile, + const struct BlendFileReadParams *params, + struct ReportList *reports); void BKE_blendfile_read_make_empty(struct bContext *C); struct UserDef *BKE_blendfile_userdef_read(const char *filepath, struct ReportList *reports); diff --git a/source/blender/blenkernel/intern/blender_undo.c b/source/blender/blenkernel/intern/blender_undo.c index d826aaf24e3..9f8a30722c2 100644 --- a/source/blender/blenkernel/intern/blender_undo.c +++ b/source/blender/blenkernel/intern/blender_undo.c @@ -77,7 +77,12 @@ bool BKE_memfile_undo_decode(MemFileUndoData *mfu, G.fileflags |= G_FILE_NO_UI; if (UNDO_DISK) { - success = BKE_blendfile_read(C, mfu->filename, &(const struct BlendFileReadParams){0}, NULL); + const struct BlendFileReadParams params = {0}; + struct BlendFileData *bfd = BKE_blendfile_read(mfu->filename, ¶ms, NULL); + if (bfd != NULL) { + BKE_blendfile_read_setup(C, bfd, ¶ms, NULL); + success = true; + } } else { struct BlendFileReadParams params = {0}; @@ -85,7 +90,12 @@ bool BKE_memfile_undo_decode(MemFileUndoData *mfu, if (!use_old_bmain_data) { params.skip_flags |= BLO_READ_SKIP_UNDO_OLD_MAIN; } - success = BKE_blendfile_read_from_memfile(C, &mfu->memfile, ¶ms, NULL); + struct BlendFileData *bfd = BKE_blendfile_read_from_memfile( + bmain, &mfu->memfile, ¶ms, NULL); + if (bfd != NULL) { + BKE_blendfile_read_setup(C, bfd, ¶ms, NULL); + success = true; + } } /* Restore, bmain has been re-allocated. */ diff --git a/source/blender/blenkernel/intern/blendfile.c b/source/blender/blenkernel/intern/blendfile.c index c3edd84fd43..d3dfe60c444 100644 --- a/source/blender/blenkernel/intern/blendfile.c +++ b/source/blender/blenkernel/intern/blendfile.c @@ -429,15 +429,46 @@ static void handle_subversion_warning(Main *main, ReportList *reports) } } -bool BKE_blendfile_read_ex(bContext *C, - const char *filepath, - const struct BlendFileReadParams *params, - ReportList *reports, - /* Extra args. */ - const bool startup_update_defaults, - const char *startup_app_template) +/** + * Shared setup function that makes the data from `bfd` into the current blend file, + * replacing the contents of #G.main. + * This uses the bfd #BKE_blendfile_read and similarly named functions. + * + * This is done in a separate step so the caller may perform actions after it is known the file + * loaded correctly but before the file replaces the existing blend file contents. + */ +void BKE_blendfile_read_setup_ex(bContext *C, + BlendFileData *bfd, + const struct BlendFileReadParams *params, + ReportList *reports, + /* Extra args. */ + const bool startup_update_defaults, + const char *startup_app_template) { + if (startup_update_defaults) { + if ((params->skip_flags & BLO_READ_SKIP_DATA) == 0) { + BLO_update_defaults_startup_blend(bfd->main, startup_app_template); + } + } + setup_app_blend_file_data(C, bfd, params, reports); + BLO_blendfiledata_free(bfd); +} +void BKE_blendfile_read_setup(bContext *C, + BlendFileData *bfd, + const struct BlendFileReadParams *params, + ReportList *reports) +{ + BKE_blendfile_read_setup_ex(C, bfd, params, reports, false, NULL); +} + +/** + * \return Blend file data, this must be passed to #BKE_blendfile_read_setup when non-NULL. + */ +struct BlendFileData *BKE_blendfile_read(const char *filepath, + const struct BlendFileReadParams *params, + ReportList *reports) +{ /* Don't print startup file loading. */ if (params->is_startup == false) { printf("Read blend: %s\n", filepath); @@ -446,69 +477,40 @@ bool BKE_blendfile_read_ex(bContext *C, BlendFileData *bfd = BLO_read_from_file(filepath, params->skip_flags, reports); if (bfd) { handle_subversion_warning(bfd->main, reports); - if (startup_update_defaults) { - if ((params->skip_flags & BLO_READ_SKIP_DATA) == 0) { - BLO_update_defaults_startup_blend(bfd->main, startup_app_template); - } - } - setup_app_blend_file_data(C, bfd, params, reports); - BLO_blendfiledata_free(bfd); } else { BKE_reports_prependf(reports, "Loading '%s' failed: ", filepath); } - return (bfd != NULL); + return bfd; } -bool BKE_blendfile_read(bContext *C, - const char *filepath, - const struct BlendFileReadParams *params, - ReportList *reports) -{ - return BKE_blendfile_read_ex(C, filepath, params, reports, false, NULL); -} - -bool BKE_blendfile_read_from_memory_ex(bContext *C, - const void *filebuf, - int filelength, - const struct BlendFileReadParams *params, - ReportList *reports, - /* Extra args. */ - const bool startup_update_defaults, - const char *startup_app_template) +/** + * \return Blend file data, this must be passed to #BKE_blendfile_read_setup when non-NULL. + */ +struct BlendFileData *BKE_blendfile_read_from_memory(const void *filebuf, + int filelength, + const struct BlendFileReadParams *params, + ReportList *reports) { BlendFileData *bfd = BLO_read_from_memory(filebuf, filelength, params->skip_flags, reports); if (bfd) { - if (startup_update_defaults) { - if ((params->skip_flags & BLO_READ_SKIP_DATA) == 0) { - BLO_update_defaults_startup_blend(bfd->main, startup_app_template); - } - } - setup_app_blend_file_data(C, bfd, params, reports); - BLO_blendfiledata_free(bfd); + /* Pass. */ } else { BKE_reports_prepend(reports, "Loading failed: "); } - return (bfd != NULL); + return bfd; } -bool BKE_blendfile_read_from_memory(bContext *C, - const void *filebuf, - int filelength, - const struct BlendFileReadParams *params, - ReportList *reports) +/** + * \return Blend file data, this must be passed to #BKE_blendfile_read_setup when non-NULL. + * \note `memfile` is the undo buffer. + */ +struct BlendFileData *BKE_blendfile_read_from_memfile(Main *bmain, + struct MemFile *memfile, + const struct BlendFileReadParams *params, + ReportList *reports) { - return BKE_blendfile_read_from_memory_ex(C, filebuf, filelength, params, reports, false, NULL); -} - -/* memfile is the undo buffer */ -bool BKE_blendfile_read_from_memfile(bContext *C, - struct MemFile *memfile, - const struct BlendFileReadParams *params, - ReportList *reports) -{ - Main *bmain = CTX_data_main(C); BlendFileData *bfd = BLO_read_from_memfile( bmain, BKE_main_blendfile_path(bmain), memfile, params, reports); if (bfd) { @@ -519,14 +521,11 @@ bool BKE_blendfile_read_from_memfile(bContext *C, BLI_assert(BLI_listbase_is_empty(&bfd->main->wm)); BLI_assert(BLI_listbase_is_empty(&bfd->main->workspaces)); BLI_assert(BLI_listbase_is_empty(&bfd->main->screens)); - - setup_app_blend_file_data(C, bfd, params, reports); - BLO_blendfiledata_free(bfd); } else { BKE_reports_prepend(reports, "Loading failed: "); } - return (bfd != NULL); + return bfd; } /** diff --git a/source/blender/windowmanager/intern/wm_files.c b/source/blender/windowmanager/intern/wm_files.c index 8a8baad0df0..3bf925af2ce 100644 --- a/source/blender/windowmanager/intern/wm_files.c +++ b/source/blender/windowmanager/intern/wm_files.c @@ -729,17 +729,19 @@ bool WM_file_read(bContext *C, const char *filepath, ReportList *reports) /* also exit screens and editors */ wm_window_match_init(C, &wmbase); - success = BKE_blendfile_read( - C, - filepath, - /* Loading preferences when the user intended to load a regular file is a security risk, - * because the excluded path list is also loaded. - * Further it's just confusing if a user loads a file and various preferences change. */ - &(const struct BlendFileReadParams){ - .is_startup = false, - .skip_flags = BLO_READ_SKIP_USERDEF, - }, - reports); + const struct BlendFileReadParams params = { + .is_startup = false, + /* Loading preferences when the user intended to load a regular file is a security + * risk, because the excluded path list is also loaded. Further it's just confusing + * if a user loads a file and various preferences change. */ + .skip_flags = BLO_READ_SKIP_USERDEF, + }; + + struct BlendFileData *bfd = BKE_blendfile_read(filepath, ¶ms, reports); + if (bfd != NULL) { + BKE_blendfile_read_setup(C, bfd, ¶ms, reports); + success = true; + } /* BKE_file_read sets new Main into context. */ Main *bmain = CTX_data_main(C); @@ -1040,15 +1042,17 @@ void wm_homefile_read(bContext *C, if (!use_factory_settings || (filepath_startup[0] != '\0')) { if (BLI_access(filepath_startup, R_OK) == 0) { - success = BKE_blendfile_read_ex(C, - filepath_startup, - &(const struct BlendFileReadParams){ - .is_startup = true, - .skip_flags = skip_flags | BLO_READ_SKIP_USERDEF, - }, - NULL, - update_defaults && use_data, - app_template); + const struct BlendFileReadParams params = { + .is_startup = true, + .skip_flags = skip_flags | BLO_READ_SKIP_USERDEF, + }; + + struct BlendFileData *bfd = BKE_blendfile_read(filepath_startup, ¶ms, NULL); + if (bfd != NULL) { + BKE_blendfile_read_setup_ex( + C, bfd, ¶ms, NULL, update_defaults && use_data, app_template); + success = true; + } } if (success) { is_factory_startup = filepath_startup_is_factory; @@ -1069,16 +1073,16 @@ void wm_homefile_read(bContext *C, } if (success == false) { - success = BKE_blendfile_read_from_memory_ex(C, - datatoc_startup_blend, - datatoc_startup_blend_size, - &(const struct BlendFileReadParams){ - .is_startup = true, - .skip_flags = skip_flags, - }, - NULL, - true, - NULL); + const struct BlendFileReadParams params = { + .is_startup = true, + .skip_flags = skip_flags, + }; + struct BlendFileData *bfd = BKE_blendfile_read_from_memory( + datatoc_startup_blend, datatoc_startup_blend_size, ¶ms, NULL); + if (bfd != NULL) { + BKE_blendfile_read_setup_ex(C, bfd, ¶ms, NULL, true, NULL); + success = true; + } if (use_data && BLI_listbase_is_empty(&wmbase)) { wm_clear_default_size(C);