From 33bcc4f43092c10d37d4120c860c3a222c05d9f7 Mon Sep 17 00:00:00 2001 From: Julian Eisel Date: Tue, 22 Nov 2022 17:56:36 +0100 Subject: [PATCH 01/25] Initial "All" asset library loading support An "All" asset library can be selected in the Asset Browser and asset view templates now, and that will load all assets from all asset libraries. Preview loading, drag & drop and asset catalogs don't work yet. --- .../blender/asset_system/AS_asset_library.hh | 3 + .../asset_system/intern/asset_library.cc | 7 ++ .../intern/asset_library_service.cc | 40 ++++++++-- .../intern/asset_library_service.hh | 2 + .../asset_system/intern/asset_storage.hh | 2 + .../intern/asset_library_reference_enum.cc | 9 ++- .../editors/asset/intern/asset_list.cc | 26 +++---- source/blender/editors/space_file/file_draw.c | 1 + source/blender/editors/space_file/filelist.cc | 73 ++++++++++++++++--- source/blender/editors/space_file/filesel.c | 8 +- source/blender/makesdna/DNA_asset_defaults.h | 2 +- source/blender/makesdna/DNA_asset_types.h | 3 +- source/blender/makesdna/DNA_space_types.h | 3 + 13 files changed, 140 insertions(+), 39 deletions(-) diff --git a/source/blender/asset_system/AS_asset_library.hh b/source/blender/asset_system/AS_asset_library.hh index 1a558c4e322..ece3426731c 100644 --- a/source/blender/asset_system/AS_asset_library.hh +++ b/source/blender/asset_system/AS_asset_library.hh @@ -120,6 +120,9 @@ Vector all_valid_asset_library_refs(); blender::asset_system::AssetLibrary *AS_asset_library_load( const Main *bmain, const AssetLibraryReference &library_reference); +std::string AS_asset_library_root_path_from_library_ref( + const AssetLibraryReference &library_reference); + /** * Try to find an appropriate location for an asset library root from a file or directory path. * Does not check if \a input_path exists. diff --git a/source/blender/asset_system/intern/asset_library.cc b/source/blender/asset_system/intern/asset_library.cc index 45196a218aa..eebbc8db837 100644 --- a/source/blender/asset_system/intern/asset_library.cc +++ b/source/blender/asset_system/intern/asset_library.cc @@ -64,6 +64,12 @@ bool AS_asset_library_has_any_unsaved_catalogs() return service->has_any_unsaved_catalogs(); } +std::string AS_asset_library_root_path_from_library_ref( + const AssetLibraryReference &library_reference) +{ + return AssetLibraryService::root_path_from_library_ref(library_reference); +} + std::string AS_asset_library_find_suitable_root_path_from_path( const blender::StringRefNull input_path) { @@ -224,6 +230,7 @@ void AssetLibrary::refresh_catalog_simplename(struct AssetMetaData *asset_data) STRNCPY(asset_data->catalog_simple_name, catalog->simple_name.c_str()); } +/* TODO get rid of this. */ Vector all_valid_asset_library_refs() { Vector result; diff --git a/source/blender/asset_system/intern/asset_library_service.cc b/source/blender/asset_system/intern/asset_library_service.cc index 9bd2eecd468..c40e6059f66 100644 --- a/source/blender/asset_system/intern/asset_library_service.cc +++ b/source/blender/asset_system/intern/asset_library_service.cc @@ -68,12 +68,17 @@ AssetLibrary *AssetLibraryService::get_asset_library( return get_asset_library_on_disk(root_path); } - if (library_reference.type == ASSET_LIBRARY_CUSTOM) { - bUserAssetLibrary *user_library = BKE_preferences_asset_library_find_from_index( - &U, library_reference.custom_library_index); - if (user_library) { - return get_asset_library_on_disk(user_library->path); + /* TODO */ + if (library_reference.type == ASSET_LIBRARY_ALL) { + return get_asset_library_current_file(); + } + + if (library_reference.type == ASSET_LIBRARY_CUSTOM) { + std::string root_path = root_path_from_library_ref(library_reference); + + if (!root_path.empty()) { + return get_asset_library_on_disk(root_path); } } @@ -133,6 +138,31 @@ AssetLibrary *AssetLibraryService::get_asset_library_current_file() return lib; } +std::string AssetLibraryService::root_path_from_library_ref( + const AssetLibraryReference &library_reference) +{ + if (ELEM(library_reference.type, ASSET_LIBRARY_ALL, ASSET_LIBRARY_LOCAL)) { + return ""; + } + + const char *top_level_directory = nullptr; + + BLI_assert(library_reference.type == ASSET_LIBRARY_CUSTOM); + BLI_assert(library_reference.custom_library_index >= 0); + + bUserAssetLibrary *user_library = BKE_preferences_asset_library_find_from_index( + &U, library_reference.custom_library_index); + if (user_library) { + top_level_directory = user_library->path; + } + + if (!top_level_directory) { + return ""; + } + + return normalize_directory_path(top_level_directory); +} + void AssetLibraryService::allocate_service_instance() { instance_ = std::make_unique(); diff --git a/source/blender/asset_system/intern/asset_library_service.hh b/source/blender/asset_system/intern/asset_library_service.hh index 6045060e91e..c492b798fc3 100644 --- a/source/blender/asset_system/intern/asset_library_service.hh +++ b/source/blender/asset_system/intern/asset_library_service.hh @@ -54,6 +54,8 @@ class AssetLibraryService { /** Destroy the AssetLibraryService singleton. It will be reallocated by #get() if necessary. */ static void destroy(); + static std::string root_path_from_library_ref(const AssetLibraryReference &library_reference); + AssetLibrary *get_asset_library(const Main *bmain, const AssetLibraryReference &library_reference); diff --git a/source/blender/asset_system/intern/asset_storage.hh b/source/blender/asset_system/intern/asset_storage.hh index 9278b3f41d1..0814c258442 100644 --- a/source/blender/asset_system/intern/asset_storage.hh +++ b/source/blender/asset_system/intern/asset_storage.hh @@ -30,6 +30,8 @@ class AssetStorage { * faster lookups. Not possible until each asset is only represented once in the storage. */ StorageT local_id_assets_; + friend class AssetLibrary; + public: /** See #AssetLibrary::add_external_asset(). */ AssetRepresentation &add_external_asset(StringRef name, std::unique_ptr metadata); diff --git a/source/blender/editors/asset/intern/asset_library_reference_enum.cc b/source/blender/editors/asset/intern/asset_library_reference_enum.cc index 773838a54cd..f88afdd8487 100644 --- a/source/blender/editors/asset/intern/asset_library_reference_enum.cc +++ b/source/blender/editors/asset/intern/asset_library_reference_enum.cc @@ -47,7 +47,7 @@ AssetLibraryReference ED_asset_library_reference_from_enum_value(int value) if (value < ASSET_LIBRARY_CUSTOM) { library.type = value; library.custom_library_index = -1; - BLI_assert(ELEM(value, ASSET_LIBRARY_LOCAL)); + BLI_assert(ELEM(value, ASSET_LIBRARY_ALL, ASSET_LIBRARY_LOCAL)); return library; } @@ -78,8 +78,11 @@ const EnumPropertyItem *ED_asset_library_reference_to_rna_enum_itemf( if (include_local_library) { const EnumPropertyItem predefined_items[] = { - /* For the future. */ - // {ASSET_REPO_BUNDLED, "BUNDLED", 0, "Bundled", "Show the default user assets"}, + {ASSET_LIBRARY_ALL, + "ALL", + ICON_NONE, + "All", + "Show assets from all of the listed asset libraries"}, {ASSET_LIBRARY_LOCAL, "LOCAL", ICON_CURRENT_FILE, diff --git a/source/blender/editors/asset/intern/asset_list.cc b/source/blender/editors/asset/intern/asset_list.cc index bb72c5cc1bb..a14c5eea807 100644 --- a/source/blender/editors/asset/intern/asset_list.cc +++ b/source/blender/editors/asset/intern/asset_list.cc @@ -12,6 +12,8 @@ #include #include +#include "AS_asset_library.hh" + #include "BKE_context.h" #include "BLI_map.hh" @@ -130,16 +132,7 @@ AssetList::AssetList(eFileSelectType filesel_type, const AssetLibraryReference & void AssetList::setup() { FileList *files = filelist_; - - bUserAssetLibrary *user_library = nullptr; - - /* Ensure valid repository, or fall-back to local one. */ - if (library_ref_.type == ASSET_LIBRARY_CUSTOM) { - BLI_assert(library_ref_.custom_library_index >= 0); - - user_library = BKE_preferences_asset_library_find_from_index( - &U, library_ref_.custom_library_index); - } + std::string asset_lib_path = AS_asset_library_root_path_from_library_ref(library_ref_); /* Relevant bits from file_refresh(). */ /* TODO pass options properly. */ @@ -161,13 +154,10 @@ void AssetList::setup() filelist_setindexer(files, use_asset_indexer ? &file_indexer_asset : &file_indexer_noop); char path[FILE_MAXDIR] = ""; - if (user_library) { - BLI_strncpy(path, user_library->path, sizeof(path)); - filelist_setdir(files, path); - } - else { - filelist_setdir(files, path); + if (!asset_lib_path.empty()) { + BLI_strncpy(path, asset_lib_path.c_str(), sizeof(path)); } + filelist_setdir(files, path); } void AssetList::fetch(const bContext &C) @@ -376,7 +366,9 @@ void AssetListStorage::remapID(ID *id_new, ID *id_old) std::optional AssetListStorage::asset_library_reference_to_fileselect_type( const AssetLibraryReference &library_reference) { - switch (library_reference.type) { + switch (eAssetLibraryType(library_reference.type)) { + case ASSET_LIBRARY_ALL: + return FILE_ASSET_LIBRARY_ALL; case ASSET_LIBRARY_CUSTOM: return FILE_ASSET_LIBRARY; case ASSET_LIBRARY_LOCAL: diff --git a/source/blender/editors/space_file/file_draw.c b/source/blender/editors/space_file/file_draw.c index ed0132c6990..d21c5b4fa99 100644 --- a/source/blender/editors/space_file/file_draw.c +++ b/source/blender/editors/space_file/file_draw.c @@ -555,6 +555,7 @@ static void file_draw_preview(const SpaceFile *sfile, /* path is no more static, cannot give it directly to but... */ else if (sfile->browse_mode == FILE_BROWSE_MODE_ASSETS && (file->typeflag & FILE_TYPE_ASSET) != 0) { + /* TODO enable drag & drop support, get path from asset representation. */ char blend_path[FILE_MAX_LIBEXTRA]; if (BLO_library_path_explode(path, blend_path, NULL, NULL)) { diff --git a/source/blender/editors/space_file/filelist.cc b/source/blender/editors/space_file/filelist.cc index dc59bb1286d..b0ac3277961 100644 --- a/source/blender/editors/space_file/filelist.cc +++ b/source/blender/editors/space_file/filelist.cc @@ -320,6 +320,10 @@ static void filelist_readjob_main_assets(FileListReadJob *job_params, bool *stop, bool *do_update, float *progress); +static void filelist_readjob_all_asset_libraries(FileListReadJob *job_params, + bool *stop, + bool *do_update, + float *progress); /* helper, could probably go in BKE actually? */ static int groupname_to_code(const char *group); @@ -854,13 +858,16 @@ static bool is_filtered_lib_type(FileListInternEntry *file, const char *root, FileListFilter *filter) { - char path[FILE_MAX_LIBEXTRA], dir[FILE_MAX_LIBEXTRA], *group, *name; + if (root) { + char path[FILE_MAX_LIBEXTRA], dir[FILE_MAX_LIBEXTRA], *group, *name; - BLI_path_join(path, sizeof(path), root, file->relpath); + BLI_path_join(path, sizeof(path), root, file->relpath); - if (BLO_library_path_explode(path, dir, &group, &name)) { - return is_filtered_id_file_type(file, group, name, filter); + if (BLO_library_path_explode(path, dir, &group, &name)) { + return is_filtered_id_file_type(file, group, name, filter); + } } + return is_filtered_file_type(file, filter); } @@ -1359,11 +1366,10 @@ static bool filelist_checkdir_main(struct FileList *filelist, char *r_dir, const return filelist_checkdir_lib(filelist, r_dir, do_change); } -static bool filelist_checkdir_main_assets(struct FileList * /*filelist*/, - char * /*r_dir*/, - const bool /*do_change*/) +static bool filelist_checkdir_return_always_valid(struct FileList * /*filelist*/, + char * /*r_dir*/, + const bool /*do_change*/) { - /* Main is always valid. */ return true; } @@ -1625,6 +1631,7 @@ static void filelist_cache_previews_push(FileList *filelist, FileDirEntry *entry BLI_thread_queue_push(cache->previews_done, preview); } else { + /* XXX */ if (entry->redirection_path) { BLI_strncpy(preview->filepath, entry->redirection_path, FILE_MAXDIR); } @@ -1766,12 +1773,19 @@ void filelist_settype(FileList *filelist, short type) filelist->tags |= FILELIST_TAGS_USES_MAIN_DATA; break; case FILE_MAIN_ASSET: - filelist->check_dir_fn = filelist_checkdir_main_assets; + filelist->check_dir_fn = filelist_checkdir_return_always_valid; filelist->read_job_fn = filelist_readjob_main_assets; filelist->prepare_filter_fn = prepare_filter_asset_library; filelist->filter_fn = is_filtered_main_assets; filelist->tags |= FILELIST_TAGS_USES_MAIN_DATA | FILELIST_TAGS_NO_THREADS; break; + case FILE_ASSET_LIBRARY_ALL: + filelist->check_dir_fn = filelist_checkdir_return_always_valid; + filelist->read_job_fn = filelist_readjob_all_asset_libraries; + filelist->prepare_filter_fn = prepare_filter_asset_library; + filelist->filter_fn = is_filtered_asset_library; + filelist->tags |= FILELIST_TAGS_USES_MAIN_DATA; + break; default: filelist->check_dir_fn = filelist_checkdir_dir; filelist->read_job_fn = filelist_readjob_dir; @@ -2852,6 +2866,9 @@ void filelist_entry_parent_select_set(FileList *filelist, bool filelist_islibrary(struct FileList *filelist, char *dir, char **r_group) { + if (filelist->asset_library) { + return true; + } return BLO_library_path_explode(filelist->filelist.root, dir, r_group, nullptr); } @@ -3746,6 +3763,10 @@ static void filelist_readjob_main_assets_add_items(FileListReadJob *job_params, */ static bool filelist_contains_main(const FileList *filelist, const Main *bmain) { + if (filelist->asset_library_ref && (filelist->asset_library_ref->type == ASSET_LIBRARY_ALL)) { + return true; + } + const char *blendfile_path = BKE_main_blendfile_path(bmain); return blendfile_path[0] && BLI_path_contains(filelist->filelist.root, blendfile_path); } @@ -3800,6 +3821,40 @@ static void filelist_readjob_main_assets(FileListReadJob *job_params, filelist_readjob_main_assets_add_items(job_params, stop, do_update, progress); } +static void filelist_readjob_all_asset_libraries(FileListReadJob *job_params, + bool *stop, + bool *do_update, + float *progress) +{ + FileList *filelist = job_params->tmp_filelist; /* Use the thread-safe filelist queue. */ + BLI_assert(BLI_listbase_is_empty(&filelist->filelist.entries) && + (filelist->filelist.entries_num == FILEDIR_NBR_ENTRIES_UNSET)); + + filelist_readjob_load_asset_library_data(job_params, do_update); + + /* A valid, but empty file-list from now. */ + filelist->filelist.entries_num = 0; + + filelist_readjob_main_assets_add_items(job_params, stop, do_update, progress); + + /* When only doing partialy reload for main data, we're done. */ + if (job_params->only_main_data) { + return; + } + /* TODO propertly update progress? */ + + for (const AssetLibraryReference &library_ref : asset_system::all_valid_asset_library_refs()) { + if (library_ref.type == ASSET_LIBRARY_LOCAL) { + /* Already added main assets above. */ + continue; + } + std::string library_path = AS_asset_library_root_path_from_library_ref(library_ref); + BLI_strncpy(filelist->filelist.root, library_path.c_str(), sizeof(filelist->filelist.root)); + + filelist_readjob_recursive_dir_add_items(true, job_params, stop, do_update, progress); + } +} + /** * Check if the read-job is requesting a partial reread of the file list only. */ diff --git a/source/blender/editors/space_file/filesel.c b/source/blender/editors/space_file/filesel.c index af2c9d4e757..0b0ab2b0d66 100644 --- a/source/blender/editors/space_file/filesel.c +++ b/source/blender/editors/space_file/filesel.c @@ -423,16 +423,20 @@ static void fileselect_refresh_asset_params(FileAssetSelectParams *asset_params) } switch (library->type) { + case ASSET_LIBRARY_ALL: + base_params->dir[0] = '\0'; + base_params->type = FILE_ASSET_LIBRARY_ALL; + break; case ASSET_LIBRARY_LOCAL: base_params->dir[0] = '\0'; + base_params->type = FILE_MAIN_ASSET; break; case ASSET_LIBRARY_CUSTOM: BLI_assert(user_library); BLI_strncpy(base_params->dir, user_library->path, sizeof(base_params->dir)); + base_params->type = FILE_ASSET_LIBRARY; break; } - base_params->type = (library->type == ASSET_LIBRARY_LOCAL) ? FILE_MAIN_ASSET : - FILE_ASSET_LIBRARY; } void fileselect_refresh_params(SpaceFile *sfile) diff --git a/source/blender/makesdna/DNA_asset_defaults.h b/source/blender/makesdna/DNA_asset_defaults.h index cef4c1270b8..63239f23e4e 100644 --- a/source/blender/makesdna/DNA_asset_defaults.h +++ b/source/blender/makesdna/DNA_asset_defaults.h @@ -21,7 +21,7 @@ #define _DNA_DEFAULT_AssetLibraryReference \ { \ .type = ASSET_LIBRARY_LOCAL, \ - /* Not needed really (should be ignored for #ASSET_LIBRARY_LOCAL), but helps debugging. */ \ + /* Not needed really (should be ignored for anything but #ASSET_LIBRARY_CUSTOM), but helps debugging. */ \ .custom_library_index = -1, \ } diff --git a/source/blender/makesdna/DNA_asset_types.h b/source/blender/makesdna/DNA_asset_types.h index d7e3bcfc919..e492af6e048 100644 --- a/source/blender/makesdna/DNA_asset_types.h +++ b/source/blender/makesdna/DNA_asset_types.h @@ -88,8 +88,7 @@ typedef enum eAssetLibraryType { // ASSET_LIBRARY_BUNDLED = 0, /** Display assets from the current session (current "Main"). */ ASSET_LIBRARY_LOCAL = 1, - /* For the future. Display assets for the current project. */ - // ASSET_LIBRARY_PROJECT = 2, + ASSET_LIBRARY_ALL = 2, /** Display assets from custom asset libraries, as defined in the preferences * (#bUserAssetLibrary). The name will be taken from #FileSelectParams.asset_library_ref.idname diff --git a/source/blender/makesdna/DNA_space_types.h b/source/blender/makesdna/DNA_space_types.h index 36c15b6e106..add85223437 100644 --- a/source/blender/makesdna/DNA_space_types.h +++ b/source/blender/makesdna/DNA_space_types.h @@ -995,12 +995,15 @@ enum eFileDetails { /** File selector types. */ typedef enum eFileSelectType { + FILE_SELECT_TYPE_UNSET = 0, FILE_LOADLIB = 1, FILE_MAIN = 2, /** Load assets from #Main. */ FILE_MAIN_ASSET = 3, /** Load assets of an asset library containing external files. */ FILE_ASSET_LIBRARY = 4, + /** Load all asset libraries. */ + FILE_ASSET_LIBRARY_ALL = 5, FILE_UNIX = 8, FILE_BLENDER = 8, /* don't display relative paths */ -- 2.30.2 From d51212c4f05e38851580f7e9e75d174695dc1b82 Mon Sep 17 00:00:00 2001 From: Julian Eisel Date: Mon, 28 Nov 2022 19:37:00 +0100 Subject: [PATCH 02/25] Integrate "All" library better with the asset system Now it actually loads data from all asset libraries when this is selected. The asset representations still need to be loaded by the file browser backend, this won't change for now. This adds the concept of nested asset libraries, which I'd prefer to keep as implementation detail and not expose in the API. But for now it's needed (for the asset representation loading by the file browser backend). --- .../blender/asset_system/AS_asset_library.hh | 26 ++++- source/blender/asset_system/CMakeLists.txt | 2 + .../asset_system/intern/asset_library.cc | 27 ++++- .../intern/asset_library_service.cc | 108 ++++++++++-------- .../intern/asset_library_service.hh | 8 +- source/blender/asset_system/intern/utils.cc | 30 +++++ source/blender/asset_system/intern/utils.hh | 19 +++ source/blender/editors/space_file/filelist.cc | 37 +++--- 8 files changed, 183 insertions(+), 74 deletions(-) create mode 100644 source/blender/asset_system/intern/utils.cc create mode 100644 source/blender/asset_system/intern/utils.hh diff --git a/source/blender/asset_system/AS_asset_library.hh b/source/blender/asset_system/AS_asset_library.hh index ece3426731c..3bbed9603a5 100644 --- a/source/blender/asset_system/AS_asset_library.hh +++ b/source/blender/asset_system/AS_asset_library.hh @@ -34,7 +34,9 @@ class AssetStorage; * to also include asset indexes and more. */ class AssetLibrary { - bCallbackFuncStore on_save_callback_store_{}; + /** If this is an asset library on disk, the top-level directory path. Normalized using + * #normalize_directory_path().*/ + std::string root_path_; /** Storage for assets (better said their representations) that are considered to be part of this * library. Assets are not automatically loaded into this when loading an asset library. Assets @@ -51,6 +53,12 @@ class AssetLibrary { */ std::unique_ptr asset_storage_; + /** In some cases an asset library is a combination of multiple other ones, these are then + * referenced here. "All" asset library only currently. */ + Vector nested_libs_; /* Non-owning pointers. */ + + bCallbackFuncStore on_save_callback_store_{}; + public: /* Controlled by #ED_asset_catalogs_set_save_catalogs_when_file_is_saved, * for managing the "Save Catalog Changes" in the quit-confirmation dialog box. */ @@ -58,11 +66,16 @@ class AssetLibrary { std::unique_ptr catalog_service; + friend class AssetLibraryService; + public: - AssetLibrary(); + /** + * \param root_path: If this is an asset library on disk, the top-level directory path. + */ + AssetLibrary(StringRef root_path = ""); ~AssetLibrary(); - void load_catalogs(StringRefNull library_root_directory); + void load_catalogs(); /** Load catalogs that have changed on disk. */ void refresh(); @@ -85,6 +98,11 @@ class AssetLibrary { * case when the reference is dangling). */ bool remove_asset(AssetRepresentation &asset); + /** In some cases an asset library is a combination of multiple other ones ("All" asset library + * only currently). Iterate over the contained asset libraries, executing \a fn for each of them. + */ + void foreach_nested(FunctionRef fn); + /** * Remap ID pointers for local ID assets, see #BKE_lib_remap.h. When an ID pointer would be * mapped to null (typically when an ID gets removed), the asset is removed, because we don't @@ -105,6 +123,8 @@ class AssetLibrary { void on_blend_save_post(Main *bmain, PointerRNA **pointers, int num_pointers); + StringRefNull root_path() const; + private: std::optional find_asset_index(const AssetRepresentation &asset); }; diff --git a/source/blender/asset_system/CMakeLists.txt b/source/blender/asset_system/CMakeLists.txt index d00c3c72e3b..c9ef11d33f4 100644 --- a/source/blender/asset_system/CMakeLists.txt +++ b/source/blender/asset_system/CMakeLists.txt @@ -21,6 +21,7 @@ set(SRC intern/asset_library_service.cc intern/asset_representation.cc intern/asset_storage.cc + intern/utils.cc AS_asset_catalog.hh AS_asset_catalog_path.hh @@ -29,6 +30,7 @@ set(SRC AS_asset_representation.hh intern/asset_library_service.hh intern/asset_storage.hh + intern/utils.hh AS_asset_library.h AS_asset_representation.h diff --git a/source/blender/asset_system/intern/asset_library.cc b/source/blender/asset_system/intern/asset_library.cc index eebbc8db837..79c3c55029c 100644 --- a/source/blender/asset_system/intern/asset_library.cc +++ b/source/blender/asset_system/intern/asset_library.cc @@ -22,6 +22,7 @@ #include "asset_library_service.hh" #include "asset_storage.hh" +#include "utils.hh" using namespace blender; using namespace blender::asset_system; @@ -126,8 +127,9 @@ void AS_asset_library_remap_ids(const IDRemapper *mappings) namespace blender::asset_system { -AssetLibrary::AssetLibrary() - : asset_storage_(std::make_unique()), +AssetLibrary::AssetLibrary(StringRef root_path) + : root_path_(utils::normalize_directory_path(root_path)), + asset_storage_(std::make_unique()), catalog_service(std::make_unique()) { } @@ -139,9 +141,9 @@ AssetLibrary::~AssetLibrary() } } -void AssetLibrary::load_catalogs(StringRefNull library_root_directory) +void AssetLibrary::load_catalogs() { - auto catalog_service = std::make_unique(library_root_directory); + auto catalog_service = std::make_unique(root_path_); catalog_service->load_from_disk(); this->catalog_service = std::move(catalog_service); } @@ -167,6 +169,18 @@ bool AssetLibrary::remove_asset(AssetRepresentation &asset) return asset_storage_->remove_asset(asset); } +void AssetLibrary::foreach_nested(FunctionRef fn) +{ + for (AssetLibrary *library : nested_libs_) { + if (!library) { + BLI_assert_unreachable(); + continue; + } + + fn(*library); + } +} + void AssetLibrary::remap_ids_and_remove_invalid(const IDRemapper &mappings) { asset_storage_->remap_ids_and_remove_invalid(mappings); @@ -215,6 +229,11 @@ void AssetLibrary::on_blend_save_post(struct Main *main, } } +StringRefNull AssetLibrary::root_path() const +{ + return root_path_; +} + void AssetLibrary::refresh_catalog_simplename(struct AssetMetaData *asset_data) { if (BLI_uuid_is_nil(asset_data->catalog_id)) { diff --git a/source/blender/asset_system/intern/asset_library_service.cc b/source/blender/asset_system/intern/asset_library_service.cc index c40e6059f66..8a00ebe2a08 100644 --- a/source/blender/asset_system/intern/asset_library_service.cc +++ b/source/blender/asset_system/intern/asset_library_service.cc @@ -4,9 +4,6 @@ * \ingroup asset_system */ -#include "asset_library_service.hh" -#include "AS_asset_library.hh" - #include "BKE_blender.h" #include "BKE_preferences.h" @@ -19,6 +16,10 @@ #include "CLG_log.h" +#include "AS_asset_library.hh" +#include "asset_library_service.hh" +#include "utils.hh" + /* When enabled, use a pre file load handler (#BKE_CB_EVT_LOAD_PRE) callback to destroy the asset * library service. Without this an explicit call from the file loading code is needed to do this, * which is not as nice. @@ -57,69 +58,58 @@ void AssetLibraryService::destroy() AssetLibrary *AssetLibraryService::get_asset_library( const Main *bmain, const AssetLibraryReference &library_reference) { - if (library_reference.type == ASSET_LIBRARY_LOCAL) { - /* For the "Current File" library we get the asset library root path based on main. */ - std::string root_path = bmain ? AS_asset_library_find_suitable_root_path_from_main(bmain) : ""; + const eAssetLibraryType type = eAssetLibraryType(library_reference.type); - if (root_path.empty()) { - /* File wasn't saved yet. */ - return get_asset_library_current_file(); - } + switch (type) { + case ASSET_LIBRARY_LOCAL: { + /* For the "Current File" library we get the asset library root path based on main. */ + std::string root_path = bmain ? AS_asset_library_find_suitable_root_path_from_main(bmain) : + ""; - return get_asset_library_on_disk(root_path); - } - - /* TODO */ - if (library_reference.type == ASSET_LIBRARY_ALL) { - return get_asset_library_current_file(); - } - - if (library_reference.type == ASSET_LIBRARY_CUSTOM) { - std::string root_path = root_path_from_library_ref(library_reference); - - if (!root_path.empty()) { + if (root_path.empty()) { + /* File wasn't saved yet. */ + return get_asset_library_current_file(); + } return get_asset_library_on_disk(root_path); } + case ASSET_LIBRARY_ALL: + return get_asset_library_all(bmain); + case ASSET_LIBRARY_CUSTOM: { + std::string root_path = root_path_from_library_ref(library_reference); + + if (!root_path.empty()) { + return get_asset_library_on_disk(root_path); + } + } break; } return nullptr; } -namespace { -std::string normalize_directory_path(StringRefNull directory) +AssetLibrary *AssetLibraryService::get_asset_library_on_disk(StringRefNull root_path) { - - char dir_normalized[PATH_MAX]; - STRNCPY(dir_normalized, directory.c_str()); - BLI_path_normalize_dir(nullptr, dir_normalized, sizeof(dir_normalized)); - return std::string(dir_normalized); -} -} // namespace - -AssetLibrary *AssetLibraryService::get_asset_library_on_disk(StringRefNull top_level_directory) -{ - BLI_assert_msg(!top_level_directory.is_empty(), + BLI_assert_msg(!root_path.is_empty(), "top level directory must be given for on-disk asset library"); - std::string top_dir_trailing_slash = normalize_directory_path(top_level_directory); + std::string normalized_root_path = utils::normalize_directory_path(root_path); std::unique_ptr *lib_uptr_ptr = on_disk_libraries_.lookup_ptr( - top_dir_trailing_slash); + normalized_root_path); if (lib_uptr_ptr != nullptr) { - CLOG_INFO(&LOG, 2, "get \"%s\" (cached)", top_dir_trailing_slash.c_str()); + CLOG_INFO(&LOG, 2, "get \"%s\" (cached)", normalized_root_path.c_str()); AssetLibrary *lib = lib_uptr_ptr->get(); lib->refresh(); return lib; } - std::unique_ptr lib_uptr = std::make_unique(); + std::unique_ptr lib_uptr = std::make_unique(normalized_root_path); AssetLibrary *lib = lib_uptr.get(); lib->on_blend_save_handler_register(); - lib->load_catalogs(top_dir_trailing_slash); + lib->load_catalogs(); - on_disk_libraries_.add_new(top_dir_trailing_slash, std::move(lib_uptr)); - CLOG_INFO(&LOG, 2, "get \"%s\" (loaded)", top_dir_trailing_slash.c_str()); + on_disk_libraries_.add_new(normalized_root_path, std::move(lib_uptr)); + CLOG_INFO(&LOG, 2, "get \"%s\" (loaded)", normalized_root_path.c_str()); return lib; } @@ -138,6 +128,30 @@ AssetLibrary *AssetLibraryService::get_asset_library_current_file() return lib; } +AssetLibrary *AssetLibraryService::get_asset_library_all(const Main *bmain) +{ + if (all_library_) { + CLOG_INFO(&LOG, 2, "get all lib (cached)"); + all_library_->nested_libs_.clear(); + } + else { + CLOG_INFO(&LOG, 2, "get all lib (loaded)"); + all_library_ = std::make_unique(); + } + + for (AssetLibraryReference &library_ref : all_valid_asset_library_refs()) { + /* Skip self :) */ + if (library_ref.type == ASSET_LIBRARY_ALL) { + continue; + } + + AssetLibrary *nested_lib = get_asset_library(bmain, library_ref); + all_library_->nested_libs_.append(nested_lib); + } + + return all_library_.get(); +} + std::string AssetLibraryService::root_path_from_library_ref( const AssetLibraryReference &library_reference) { @@ -145,22 +159,16 @@ std::string AssetLibraryService::root_path_from_library_ref( return ""; } - const char *top_level_directory = nullptr; - BLI_assert(library_reference.type == ASSET_LIBRARY_CUSTOM); BLI_assert(library_reference.custom_library_index >= 0); bUserAssetLibrary *user_library = BKE_preferences_asset_library_find_from_index( &U, library_reference.custom_library_index); - if (user_library) { - top_level_directory = user_library->path; - } - - if (!top_level_directory) { + if (!user_library || !user_library->path[0]) { return ""; } - return normalize_directory_path(top_level_directory); + return user_library->path; } void AssetLibraryService::allocate_service_instance() diff --git a/source/blender/asset_system/intern/asset_library_service.hh b/source/blender/asset_system/intern/asset_library_service.hh index c492b798fc3..478465d25ed 100644 --- a/source/blender/asset_system/intern/asset_library_service.hh +++ b/source/blender/asset_system/intern/asset_library_service.hh @@ -33,12 +33,15 @@ namespace blender::asset_system { class AssetLibraryService { static std::unique_ptr instance_; - /* Mapping absolute path of the library's top-level directory to the AssetLibrary instance. */ + /* Mapping absolute path of the library's root path (normalize with #normalize_directory_path()!) + * the AssetLibrary instance. */ Map> on_disk_libraries_; /** Library without a known path, i.e. the "Current File" library if the file isn't saved yet. If * the file was saved, a valid path for the library can be determined and #on_disk_libraries_ * above should be used. */ std::unique_ptr current_file_library_; + /** The "all" asset library, merging all other libraries into one. */ + std::unique_ptr all_library_; /* Handlers for managing the life cycle of the AssetLibraryService instance. */ bCallbackFuncStore on_load_callback_store_; @@ -67,6 +70,9 @@ class AssetLibraryService { /** Get the "Current File" asset library. */ AssetLibrary *get_asset_library_current_file(); + /** Get the "All" asset library, merging all others into one. */ + AssetLibrary *get_asset_library_all(const Main *bmain); + /** Returns whether there are any known asset libraries with unsaved catalog edits. */ bool has_any_unsaved_catalogs() const; diff --git a/source/blender/asset_system/intern/utils.cc b/source/blender/asset_system/intern/utils.cc new file mode 100644 index 00000000000..b1e9793634d --- /dev/null +++ b/source/blender/asset_system/intern/utils.cc @@ -0,0 +1,30 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ + +/** \file + * \ingroup asset_system + */ + +#include "BLI_fileops.h" +#include "BLI_path_util.h" +#include "BLI_string.h" + +#include "utils.hh" + +namespace blender::asset_system::utils { + +std::string normalize_directory_path(StringRef directory) +{ + if (directory.is_empty()) { + return ""; + } + + char dir_normalized[PATH_MAX]; + BLI_strncpy(dir_normalized, + directory.data(), + /* + 1 for null terminator. */ + std::min(directory.size() + 1, int64_t(sizeof(dir_normalized)))); + BLI_path_normalize_dir(nullptr, dir_normalized, sizeof(dir_normalized)); + return std::string(dir_normalized); +} + +} // namespace blender::asset_system::utils diff --git a/source/blender/asset_system/intern/utils.hh b/source/blender/asset_system/intern/utils.hh new file mode 100644 index 00000000000..cccc24984ba --- /dev/null +++ b/source/blender/asset_system/intern/utils.hh @@ -0,0 +1,19 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ + +/** \file + * \ingroup asset_system + */ + +#pragma once + +#include "BLI_string_ref.hh" + +namespace blender::asset_system::utils { + +/** + * Returns a normalized directory path with a trailing slash, and a maximum length of #PATH_MAX. + * Slashes are not converted to native format (they probably should be though?). + */ +std::string normalize_directory_path(StringRef directory); + +} // namespace blender::asset_system::utils diff --git a/source/blender/editors/space_file/filelist.cc b/source/blender/editors/space_file/filelist.cc index b0ac3277961..c4cce982896 100644 --- a/source/blender/editors/space_file/filelist.cc +++ b/source/blender/editors/space_file/filelist.cc @@ -320,10 +320,10 @@ static void filelist_readjob_main_assets(FileListReadJob *job_params, bool *stop, bool *do_update, float *progress); -static void filelist_readjob_all_asset_libraries(FileListReadJob *job_params, - bool *stop, - bool *do_update, - float *progress); +static void filelist_readjob_all_asset_library(FileListReadJob *job_params, + bool *stop, + bool *do_update, + float *progress); /* helper, could probably go in BKE actually? */ static int groupname_to_code(const char *group); @@ -1781,7 +1781,7 @@ void filelist_settype(FileList *filelist, short type) break; case FILE_ASSET_LIBRARY_ALL: filelist->check_dir_fn = filelist_checkdir_return_always_valid; - filelist->read_job_fn = filelist_readjob_all_asset_libraries; + filelist->read_job_fn = filelist_readjob_all_asset_library; filelist->prepare_filter_fn = prepare_filter_asset_library; filelist->filter_fn = is_filtered_asset_library; filelist->tags |= FILELIST_TAGS_USES_MAIN_DATA; @@ -3784,6 +3784,8 @@ static void filelist_readjob_asset_library(FileListReadJob *job_params, /* A valid, but empty file-list from now. */ filelist->filelist.entries_num = 0; + BLI_assert(job_params->filelist->asset_library_ref != nullptr); + /* NOP if already read. */ filelist_readjob_load_asset_library_data(job_params, do_update); @@ -3821,10 +3823,10 @@ static void filelist_readjob_main_assets(FileListReadJob *job_params, filelist_readjob_main_assets_add_items(job_params, stop, do_update, progress); } -static void filelist_readjob_all_asset_libraries(FileListReadJob *job_params, - bool *stop, - bool *do_update, - float *progress) +static void filelist_readjob_all_asset_library(FileListReadJob *job_params, + bool *stop, + bool *do_update, + float *progress) { FileList *filelist = job_params->tmp_filelist; /* Use the thread-safe filelist queue. */ BLI_assert(BLI_listbase_is_empty(&filelist->filelist.entries) && @@ -3843,16 +3845,19 @@ static void filelist_readjob_all_asset_libraries(FileListReadJob *job_params, } /* TODO propertly update progress? */ - for (const AssetLibraryReference &library_ref : asset_system::all_valid_asset_library_refs()) { - if (library_ref.type == ASSET_LIBRARY_LOCAL) { - /* Already added main assets above. */ - continue; + BLI_assert(filelist->asset_library != nullptr); + + /* Add assets from asset libraries on disk. */ + filelist->asset_library->foreach_nested([&](asset_system::AssetLibrary &nested_library) { + StringRefNull root_path = nested_library.root_path(); + if (root_path.is_empty()) { + return; } - std::string library_path = AS_asset_library_root_path_from_library_ref(library_ref); - BLI_strncpy(filelist->filelist.root, library_path.c_str(), sizeof(filelist->filelist.root)); + + BLI_strncpy(filelist->filelist.root, root_path.c_str(), sizeof(filelist->filelist.root)); filelist_readjob_recursive_dir_add_items(true, job_params, stop, do_update, progress); - } + }); } /** -- 2.30.2 From 126136baabac679e95d578267d2b4f042dc5f9bd Mon Sep 17 00:00:00 2001 From: Julian Eisel Date: Wed, 30 Nov 2022 20:15:04 +0100 Subject: [PATCH 03/25] Fix missing asset previews and broken drag & drop in "All" library Together with the changes made in master, all this does is making sure the assets are loaded and removed using the correct asset library nested within the "All" library. Now full paths for the assets can be built correctly from the asset identifier, which fixes preview loading and drag & drop. --- .../blender/asset_system/AS_asset_library.hh | 3 +++ .../asset_system/intern/asset_library.cc | 19 +++++++++++++++++- source/blender/editors/space_file/filelist.cc | 20 +++++++++++++------ 3 files changed, 35 insertions(+), 7 deletions(-) diff --git a/source/blender/asset_system/AS_asset_library.hh b/source/blender/asset_system/AS_asset_library.hh index cb75a2540ff..561452d3ac8 100644 --- a/source/blender/asset_system/AS_asset_library.hh +++ b/source/blender/asset_system/AS_asset_library.hh @@ -101,6 +101,9 @@ class AssetLibrary { /** Remove an asset from the library that was added using #add_external_asset() or * #add_local_id_asset(). Can usually be expected to be constant time complexity (worst case may * differ). + * Can also be called when this asset library is just a merged library containing multiple nested + * ones ("All" library). Will then check if it exists in a nested library and remove it. + * * \note This is save to call if \a asset is freed (dangling reference), will not perform any * change then. * \return True on success, false if the asset couldn't be found inside the library (also the diff --git a/source/blender/asset_system/intern/asset_library.cc b/source/blender/asset_system/intern/asset_library.cc index 043d1a45ef2..944e91bcf4d 100644 --- a/source/blender/asset_system/intern/asset_library.cc +++ b/source/blender/asset_system/intern/asset_library.cc @@ -170,7 +170,24 @@ AssetRepresentation &AssetLibrary::add_local_id_asset(StringRef relative_asset_p bool AssetLibrary::remove_asset(AssetRepresentation &asset) { - return asset_storage_->remove_asset(asset); + /* Usual case, only the "All" library differs and uses nested libraries (see below). */ + if (asset_storage_->remove_asset(asset)) { + return true; + } + + /* If asset is not stored in this library, check nested ones (for "All" library). */ + for (AssetLibrary *library : nested_libs_) { + if (!library) { + BLI_assert_unreachable(); + continue; + } + + if (asset_storage_->remove_asset(asset)) { + return true; + } + } + + return false; } void AssetLibrary::foreach_nested(FunctionRef fn) diff --git a/source/blender/editors/space_file/filelist.cc b/source/blender/editors/space_file/filelist.cc index 5039fcdce5a..9d1191ad80e 100644 --- a/source/blender/editors/space_file/filelist.cc +++ b/source/blender/editors/space_file/filelist.cc @@ -2928,6 +2928,11 @@ struct FileListReadJob { * `Materials/Material.001`). */ char cur_relbase[FILE_MAX_LIBEXTRA]; + /** The current asset library to load. Usually the same as #FileList.asset_library, however + * sometimes the #FileList one is a combination of multiple other ones ("All" asset library), + * which need to be loaded individually. Then this can be set to override the #FileList library. + * Use this in all loading code. */ + asset_system::AssetLibrary *load_asset_library; /** Set to request a partial read that only adds files representing #Main data (IDs). Used when * #Main may have received changes of interest (e.g. asset removed or renamed). */ bool only_main_data; @@ -3105,7 +3110,6 @@ static void filelist_readjob_list_lib_add_datablock(FileListReadJob *job_params, const int idcode, const char *group_name) { - FileList *filelist = job_params->tmp_filelist; /* Use the thread-safe filelist queue. */ FileListInternEntry *entry = MEM_cnew(__func__); if (prefix_relpath_with_group_name) { std::string datablock_path = StringRef(group_name) + "/" + datablock_info->name; @@ -3121,13 +3125,13 @@ static void filelist_readjob_list_lib_add_datablock(FileListReadJob *job_params, if (datablock_info->asset_data) { entry->typeflag |= FILE_TYPE_ASSET; - if (filelist->asset_library) { + if (job_params->load_asset_library) { /** XXX Moving out the asset metadata like this isn't great. */ std::unique_ptr metadata = BKE_asset_metadata_move_to_unique_ptr( datablock_info->asset_data); BKE_asset_metadata_free(&datablock_info->asset_data); - entry->asset = &filelist->asset_library->add_external_asset( + entry->asset = &job_params->load_asset_library->add_external_asset( entry->relpath, datablock_info->name, std::move(metadata)); } } @@ -3631,7 +3635,7 @@ static void filelist_readjob_recursive_dir_add_items(const bool do_lib, } /* Only load assets when browsing an asset library. For normal file browsing we return all * entries. `FLF_ASSETS_ONLY` filter can be enabled/disabled by the user. */ - if (filelist->asset_library_ref) { + if (job_params->load_asset_library) { list_lib_options |= LIST_LIB_ASSETS_ONLY; } std::optional lib_entries_num = filelist_readjob_list_lib( @@ -3746,6 +3750,8 @@ static void filelist_readjob_load_asset_library_data(FileListReadJob *job_params * #filelist_readjob_endjob() will move it into the real filelist. */ tmp_filelist->asset_library = AS_asset_library_load(job_params->current_main, *job_params->filelist->asset_library_ref); + /* Set asset library to load (may be overridden later for loading nested ones). */ + job_params->load_asset_library = tmp_filelist->asset_library; *do_update = true; } @@ -3783,8 +3789,8 @@ static void filelist_readjob_main_assets_add_items(FileListReadJob *job_params, entry->local_data.preview_image = BKE_asset_metadata_preview_get_from_id(id_iter->asset_data, id_iter); entry->local_data.id = id_iter; - if (filelist->asset_library) { - entry->asset = &filelist->asset_library->add_local_id_asset(entry->relpath, *id_iter); + if (job_params->load_asset_library) { + entry->asset = &job_params->load_asset_library->add_local_id_asset(entry->relpath, *id_iter); } entries_num++; BLI_addtail(&tmp_entries, entry); @@ -3902,6 +3908,8 @@ static void filelist_readjob_all_asset_library(FileListReadJob *job_params, return; } + /* Override library info to read this library. */ + job_params->load_asset_library = &nested_library; BLI_strncpy(filelist->filelist.root, root_path.c_str(), sizeof(filelist->filelist.root)); filelist_readjob_recursive_dir_add_items(true, job_params, stop, do_update, progress); -- 2.30.2 From fb2303fb739e72e263289d9aeee10031b10a3ec6 Mon Sep 17 00:00:00 2001 From: Julian Eisel Date: Fri, 2 Dec 2022 16:58:47 +0100 Subject: [PATCH 04/25] Avoid ugly nested library storage We actually don't have to do this, since we can just iterate over all loaded libraries after calling the loading for the "All" asset library. --- .../blender/asset_system/AS_asset_library.hh | 32 +++++++------ .../asset_system/intern/asset_library.cc | 46 ++++++------------- .../intern/asset_library_service.cc | 31 +++++++------ .../intern/asset_library_service.hh | 6 ++- source/blender/editors/space_file/filelist.cc | 7 +-- 5 files changed, 56 insertions(+), 66 deletions(-) diff --git a/source/blender/asset_system/AS_asset_library.hh b/source/blender/asset_system/AS_asset_library.hh index a9ed5ff0594..d377f2334b8 100644 --- a/source/blender/asset_system/AS_asset_library.hh +++ b/source/blender/asset_system/AS_asset_library.hh @@ -56,10 +56,6 @@ class AssetLibrary { */ std::unique_ptr asset_storage_; - /** In some cases an asset library is a combination of multiple other ones, these are then - * referenced here. "All" asset library only currently. */ - Vector nested_libs_; /* Non-owning pointers. */ - bCallbackFuncStore on_save_callback_store_{}; public: @@ -78,6 +74,17 @@ class AssetLibrary { AssetLibrary(StringRef root_path = ""); ~AssetLibrary(); + /** + * Execute \a fn for every asset library that is loaded. The asset library is passed to the + * \a fn call. + * + * \param skip_all_library: When true, the \a fn will also be executed for the "All" asset + * library. This is just a combination of the other ones, so usually + * iterating over it is redundant. + */ + static void foreach_loaded(FunctionRef fn, + bool include_all_library = true); + void load_catalogs(); /** Load catalogs that have changed on disk. */ @@ -101,8 +108,6 @@ class AssetLibrary { /** Remove an asset from the library that was added using #add_external_asset() or * #add_local_id_asset(). Can usually be expected to be constant time complexity (worst case may * differ). - * Can also be called when this asset library is just a merged library containing multiple nested - * ones ("All" library). Will then check if it exists in a nested library and remove it. * * \note This is save to call if \a asset is freed (dangling reference), will not perform any * change then. @@ -110,11 +115,6 @@ class AssetLibrary { * case when the reference is dangling). */ bool remove_asset(AssetRepresentation &asset); - /** In some cases an asset library is a combination of multiple other ones ("All" asset library - * only currently). Iterate over the contained asset libraries, executing \a fn for each of them. - */ - void foreach_nested(FunctionRef fn); - /** * Remap ID pointers for local ID assets, see #BKE_lib_remap.h. When an ID pointer would be * mapped to null (typically when an ID gets removed), the asset is removed, because we don't @@ -142,9 +142,6 @@ class AssetLibrary { AssetIdentifier asset_identifier_from_library(StringRef relative_asset_path); StringRefNull root_path() const; - - private: - std::optional find_asset_index(const AssetRepresentation &asset); }; Vector all_valid_asset_library_refs(); @@ -152,6 +149,13 @@ Vector all_valid_asset_library_refs(); } // namespace blender::asset_system /** + * Load the data for an asset library, but not the asset representations themselves (loading these + * is currently not done in the asset system). + * + * For the "All" asset library (#ASSET_LIBRARY_ALL), every other known asset library will be + * loaded as well. So a call to #AssetLibrary::foreach_loaded() can be expected to iterate over all + * libraries. + * * \warning Catalogs are reloaded, invalidating catalog pointers. Do not store catalog pointers, * store CatalogIDs instead and lookup the catalog where needed. */ diff --git a/source/blender/asset_system/intern/asset_library.cc b/source/blender/asset_system/intern/asset_library.cc index 1fe477707a1..b763822c1ac 100644 --- a/source/blender/asset_system/intern/asset_library.cc +++ b/source/blender/asset_system/intern/asset_library.cc @@ -121,9 +121,11 @@ void AS_asset_library_refresh_catalog_simplename(struct ::AssetLibrary *asset_li void AS_asset_library_remap_ids(const IDRemapper *mappings) { AssetLibraryService *service = AssetLibraryService::get(); - service->foreach_loaded_asset_library([mappings](asset_system::AssetLibrary &library) { - library.remap_ids_and_remove_invalid(*mappings); - }); + service->foreach_loaded_asset_library( + [mappings](asset_system::AssetLibrary &library) { + library.remap_ids_and_remove_invalid(*mappings); + }, + true); } namespace blender::asset_system { @@ -142,6 +144,13 @@ AssetLibrary::~AssetLibrary() } } +void AssetLibrary::foreach_loaded(FunctionRef fn, + const bool include_all_library) +{ + AssetLibraryService *service = AssetLibraryService::get(); + service->foreach_loaded_asset_library(fn, include_all_library); +} + void AssetLibrary::load_catalogs() { auto catalog_service = std::make_unique(root_path()); @@ -170,36 +179,7 @@ AssetRepresentation &AssetLibrary::add_local_id_asset(StringRef relative_asset_p bool AssetLibrary::remove_asset(AssetRepresentation &asset) { - /* Usual case, only the "All" library differs and uses nested libraries (see below). */ - if (asset_storage_->remove_asset(asset)) { - return true; - } - - /* If asset is not stored in this library, check nested ones (for "All" library). */ - for (AssetLibrary *library : nested_libs_) { - if (!library) { - BLI_assert_unreachable(); - continue; - } - - if (asset_storage_->remove_asset(asset)) { - return true; - } - } - - return false; -} - -void AssetLibrary::foreach_nested(FunctionRef fn) -{ - for (AssetLibrary *library : nested_libs_) { - if (!library) { - BLI_assert_unreachable(); - continue; - } - - fn(*library); - } + return asset_storage_->remove_asset(asset); } void AssetLibrary::remap_ids_and_remove_invalid(const IDRemapper &mappings) diff --git a/source/blender/asset_system/intern/asset_library_service.cc b/source/blender/asset_system/intern/asset_library_service.cc index 8a00ebe2a08..7f44143fef8 100644 --- a/source/blender/asset_system/intern/asset_library_service.cc +++ b/source/blender/asset_system/intern/asset_library_service.cc @@ -132,7 +132,6 @@ AssetLibrary *AssetLibraryService::get_asset_library_all(const Main *bmain) { if (all_library_) { CLOG_INFO(&LOG, 2, "get all lib (cached)"); - all_library_->nested_libs_.clear(); } else { CLOG_INFO(&LOG, 2, "get all lib (loaded)"); @@ -145,8 +144,8 @@ AssetLibrary *AssetLibraryService::get_asset_library_all(const Main *bmain) continue; } - AssetLibrary *nested_lib = get_asset_library(bmain, library_ref); - all_library_->nested_libs_.append(nested_lib); + /* Ensure all asset libraries are loaded. */ + get_asset_library(bmain, library_ref); } return all_library_.get(); @@ -214,21 +213,25 @@ void AssetLibraryService::app_handler_unregister() bool AssetLibraryService::has_any_unsaved_catalogs() const { - if (current_file_library_ && current_file_library_->catalog_service->has_unsaved_changes()) { - return true; - } + bool has_unsaved_changes = false; - for (const auto &asset_lib_uptr : on_disk_libraries_.values()) { - if (asset_lib_uptr->catalog_service->has_unsaved_changes()) { - return true; - } - } - - return false; + foreach_loaded_asset_library( + [&has_unsaved_changes](AssetLibrary &library) { + if (library.catalog_service->has_unsaved_changes()) { + has_unsaved_changes = true; + } + }, + true); + return has_unsaved_changes; } -void AssetLibraryService::foreach_loaded_asset_library(FunctionRef fn) const +void AssetLibraryService::foreach_loaded_asset_library(FunctionRef fn, + const bool include_all_library) const { + if (include_all_library && all_library_) { + fn(*all_library_); + } + if (current_file_library_) { fn(*current_file_library_); } diff --git a/source/blender/asset_system/intern/asset_library_service.hh b/source/blender/asset_system/intern/asset_library_service.hh index 478465d25ed..1dd9bda7aca 100644 --- a/source/blender/asset_system/intern/asset_library_service.hh +++ b/source/blender/asset_system/intern/asset_library_service.hh @@ -70,13 +70,15 @@ class AssetLibraryService { /** Get the "Current File" asset library. */ AssetLibrary *get_asset_library_current_file(); - /** Get the "All" asset library, merging all others into one. */ + /** Get the "All" asset library, which loads all others and merges them into one. */ AssetLibrary *get_asset_library_all(const Main *bmain); /** Returns whether there are any known asset libraries with unsaved catalog edits. */ bool has_any_unsaved_catalogs() const; - void foreach_loaded_asset_library(FunctionRef fn) const; + /** See AssetLibrary::foreach_loaded(). */ + void foreach_loaded_asset_library(FunctionRef fn, + bool include_all_library) const; protected: /** Allocate a new instance of the service and assign it to `instance_`. */ diff --git a/source/blender/editors/space_file/filelist.cc b/source/blender/editors/space_file/filelist.cc index ca83ff6d51b..60c34ceda5f 100644 --- a/source/blender/editors/space_file/filelist.cc +++ b/source/blender/editors/space_file/filelist.cc @@ -3125,7 +3125,7 @@ static void filelist_readjob_list_lib_add_datablock(FileListReadJob *job_params, if (datablock_info->asset_data) { entry->typeflag |= FILE_TYPE_ASSET; - if (job_params->asset_library) { + if (job_params->load_asset_library) { /* Take ownership over the asset data (shallow copies into unique_ptr managed memory) to * pass it on to the asset system. */ std::unique_ptr metadata = std::make_unique(*datablock_info->asset_data); @@ -3905,8 +3905,9 @@ static void filelist_readjob_all_asset_library(FileListReadJob *job_params, BLI_assert(filelist->asset_library != nullptr); - /* Add assets from asset libraries on disk. */ - filelist->asset_library->foreach_nested([&](asset_system::AssetLibrary &nested_library) { + /* The "All" asset library was loaded, which means all other asset libraries are also loaded. + * Load their assets from disk into the "All" library. */ + asset_system::AssetLibrary::foreach_loaded([&](asset_system::AssetLibrary &nested_library) { StringRefNull root_path = nested_library.root_path(); if (root_path.is_empty()) { return; -- 2.30.2 From af5d225653889bde783f165726ce95786928839a Mon Sep 17 00:00:00 2001 From: Julian Eisel Date: Fri, 2 Dec 2022 19:19:08 +0100 Subject: [PATCH 05/25] Load catalogs for "All" asset library Merges the catalog definitions from all asset libraries in to the storage of the "All" one, builds the catalog tree and refreshes data as needed. This doesn't allow writing changes back to the catalog definition files, so the UI probably shouldn't allow edits. --- .../blender/asset_system/AS_asset_catalog.hh | 21 +++++++++- .../blender/asset_system/AS_asset_library.hh | 2 + .../asset_system/intern/asset_catalog.cc | 27 ++++++++++--- .../asset_system/intern/asset_library.cc | 4 +- .../intern/asset_library_service.cc | 40 ++++++++++++++----- 5 files changed, 77 insertions(+), 17 deletions(-) diff --git a/source/blender/asset_system/AS_asset_catalog.hh b/source/blender/asset_system/AS_asset_catalog.hh index 87b9425c8c6..a7829778fdf 100644 --- a/source/blender/asset_system/AS_asset_catalog.hh +++ b/source/blender/asset_system/AS_asset_catalog.hh @@ -67,6 +67,12 @@ class AssetCatalogService { /** Load asset catalog definitions from the given file or directory. */ void load_from_disk(const CatalogFilePath &file_or_directory_path); + /** + * Duplicate the catalogs from \a other_service into this one. Does not rebuild the tree, this + * needs to be done by the caller (call #rebuild_tree()!). + */ + void add_from_existing(const AssetCatalogService &other_service); + /** * Write the catalog definitions to disk. * @@ -105,6 +111,15 @@ class AssetCatalogService { */ void reload_catalogs(); + /** + * Make sure the tree is updated to the latest collection of catalogs stored in this service. + * Does not depend on a CDF file being available so this can be called on a service that stores + * catalogs that are not stored in a CDF. + * Most API functions that modify catalog data will trigger this, unless otherwise specified (for + * batch operations). + */ + void rebuild_tree(); + /** Return catalog with the given ID. Return nullptr if not found. */ AssetCatalog *find_catalog(CatalogID catalog_id) const; @@ -222,7 +237,6 @@ class AssetCatalogService { const CatalogFilePath &blend_file_path); std::unique_ptr read_into_tree(); - void rebuild_tree(); /** * For every catalog, ensure that its parent path also has a known catalog. @@ -270,6 +284,11 @@ class AssetCatalogCollection { AssetCatalogCollection(AssetCatalogCollection &&other) noexcept = default; std::unique_ptr deep_copy() const; + /** + * Copy the catalogs from \a other and append them to this collection. Copies no other data + * otherwise (but marks as having unsaved changes). + */ + void add_catalogs_from_existing(const AssetCatalogCollection &other); protected: static OwningAssetCatalogMap copy_catalog_map(const OwningAssetCatalogMap &orig); diff --git a/source/blender/asset_system/AS_asset_library.hh b/source/blender/asset_system/AS_asset_library.hh index d377f2334b8..3f2562aa987 100644 --- a/source/blender/asset_system/AS_asset_library.hh +++ b/source/blender/asset_system/AS_asset_library.hh @@ -56,6 +56,8 @@ class AssetLibrary { */ std::unique_ptr asset_storage_; + std::function on_refresh_; + bCallbackFuncStore on_save_callback_store_{}; public: diff --git a/source/blender/asset_system/intern/asset_catalog.cc b/source/blender/asset_system/intern/asset_catalog.cc index 9b47aca1209..7924ceb862c 100644 --- a/source/blender/asset_system/intern/asset_catalog.cc +++ b/source/blender/asset_system/intern/asset_catalog.cc @@ -323,6 +323,11 @@ void AssetCatalogService::load_from_disk(const CatalogFilePath &file_or_director rebuild_tree(); } +void AssetCatalogService::add_from_existing(const AssetCatalogService &other_service) +{ + catalog_collection_->add_catalogs_from_existing(*other_service.catalog_collection_); +} + void AssetCatalogService::load_directory_recursive(const CatalogFilePath &directory_path) { /* TODO(@sybren): implement proper multi-file support. For now, just load @@ -658,15 +663,25 @@ std::unique_ptr AssetCatalogCollection::deep_copy() cons return copy; } +static void copy_catalog_map_into_existing(const OwningAssetCatalogMap &source, + OwningAssetCatalogMap &dest) +{ + for (const auto &orig_catalog_uptr : source.values()) { + auto copy_catalog_uptr = std::make_unique(*orig_catalog_uptr); + dest.add_new(copy_catalog_uptr->catalog_id, std::move(copy_catalog_uptr)); + } +} + +void AssetCatalogCollection::add_catalogs_from_existing(const AssetCatalogCollection &other) +{ + has_unsaved_changes_ = true; + copy_catalog_map_into_existing(other.catalogs_, catalogs_); +} + OwningAssetCatalogMap AssetCatalogCollection::copy_catalog_map(const OwningAssetCatalogMap &orig) { OwningAssetCatalogMap copy; - - for (const auto &orig_catalog_uptr : orig.values()) { - auto copy_catalog_uptr = std::make_unique(*orig_catalog_uptr); - copy.add_new(copy_catalog_uptr->catalog_id, std::move(copy_catalog_uptr)); - } - + copy_catalog_map_into_existing(orig, copy); return copy; } diff --git a/source/blender/asset_system/intern/asset_library.cc b/source/blender/asset_system/intern/asset_library.cc index b763822c1ac..02c5c0dc4f3 100644 --- a/source/blender/asset_system/intern/asset_library.cc +++ b/source/blender/asset_system/intern/asset_library.cc @@ -160,7 +160,9 @@ void AssetLibrary::load_catalogs() void AssetLibrary::refresh() { - this->catalog_service->reload_catalogs(); + if (on_refresh_) { + on_refresh_(); + } } AssetRepresentation &AssetLibrary::add_external_asset(StringRef relative_asset_path, diff --git a/source/blender/asset_system/intern/asset_library_service.cc b/source/blender/asset_system/intern/asset_library_service.cc index 7f44143fef8..430de903db7 100644 --- a/source/blender/asset_system/intern/asset_library_service.cc +++ b/source/blender/asset_system/intern/asset_library_service.cc @@ -16,6 +16,7 @@ #include "CLG_log.h" +#include "AS_asset_catalog_tree.hh" #include "AS_asset_library.hh" #include "asset_library_service.hh" #include "utils.hh" @@ -107,6 +108,8 @@ AssetLibrary *AssetLibraryService::get_asset_library_on_disk(StringRefNull root_ lib->on_blend_save_handler_register(); lib->load_catalogs(); + /* Reload catalogs on refresh. */ + lib->on_refresh_ = [lib]() { lib->catalog_service->reload_catalogs(); }; on_disk_libraries_.add_new(normalized_root_path, std::move(lib_uptr)); CLOG_INFO(&LOG, 2, "get \"%s\" (loaded)", normalized_root_path.c_str()); @@ -117,6 +120,7 @@ AssetLibrary *AssetLibraryService::get_asset_library_current_file() { if (current_file_library_) { CLOG_INFO(&LOG, 2, "get current file lib (cached)"); + current_file_library_->refresh(); } else { CLOG_INFO(&LOG, 2, "get current file lib (loaded)"); @@ -130,14 +134,7 @@ AssetLibrary *AssetLibraryService::get_asset_library_current_file() AssetLibrary *AssetLibraryService::get_asset_library_all(const Main *bmain) { - if (all_library_) { - CLOG_INFO(&LOG, 2, "get all lib (cached)"); - } - else { - CLOG_INFO(&LOG, 2, "get all lib (loaded)"); - all_library_ = std::make_unique(); - } - + /* (Re-)load all other asset libraries. */ for (AssetLibraryReference &library_ref : all_valid_asset_library_refs()) { /* Skip self :) */ if (library_ref.type == ASSET_LIBRARY_ALL) { @@ -148,7 +145,32 @@ AssetLibrary *AssetLibraryService::get_asset_library_all(const Main *bmain) get_asset_library(bmain, library_ref); } - return all_library_.get(); + if (all_library_) { + CLOG_INFO(&LOG, 2, "get all lib (cached)"); + all_library_->refresh(); + return all_library_.get(); + } + + CLOG_INFO(&LOG, 2, "get all lib (loaded)"); + all_library_ = std::make_unique(); + + AssetLibrary &all_library = *all_library_; + auto build_catalogs_fn = [&all_library]() { + /* Start with empty catalog storage. */ + all_library.catalog_service = std::make_unique(); + + /* (Re-)load catalogs on refresh. */ + AssetLibrary::foreach_loaded([&all_library](AssetLibrary &nested) { + nested.catalog_service->reload_catalogs(); + all_library.catalog_service->add_from_existing(*nested.catalog_service); + }); + all_library.catalog_service->rebuild_tree(); + }; + + build_catalogs_fn(); + all_library.on_refresh_ = build_catalogs_fn; + + return &all_library; } std::string AssetLibraryService::root_path_from_library_ref( -- 2.30.2 From a07a2e2369d0290d08e124aa53d56cfd4600e341 Mon Sep 17 00:00:00 2001 From: Julian Eisel Date: Fri, 2 Dec 2022 19:32:46 +0100 Subject: [PATCH 06/25] Avoid redundant loading of catalogs and "All" library processing --- .../blender/asset_system/AS_asset_library.hh | 3 +-- .../intern/asset_library_service.cc | 19 ++++++++++------ source/blender/editors/space_file/filelist.cc | 22 ++++++++++--------- 3 files changed, 25 insertions(+), 19 deletions(-) diff --git a/source/blender/asset_system/AS_asset_library.hh b/source/blender/asset_system/AS_asset_library.hh index 3f2562aa987..6526e3e8382 100644 --- a/source/blender/asset_system/AS_asset_library.hh +++ b/source/blender/asset_system/AS_asset_library.hh @@ -84,8 +84,7 @@ class AssetLibrary { * library. This is just a combination of the other ones, so usually * iterating over it is redundant. */ - static void foreach_loaded(FunctionRef fn, - bool include_all_library = true); + static void foreach_loaded(FunctionRef fn, bool include_all_library); void load_catalogs(); diff --git a/source/blender/asset_system/intern/asset_library_service.cc b/source/blender/asset_system/intern/asset_library_service.cc index 430de903db7..fbcda02025c 100644 --- a/source/blender/asset_system/intern/asset_library_service.cc +++ b/source/blender/asset_system/intern/asset_library_service.cc @@ -155,20 +155,25 @@ AssetLibrary *AssetLibraryService::get_asset_library_all(const Main *bmain) all_library_ = std::make_unique(); AssetLibrary &all_library = *all_library_; - auto build_catalogs_fn = [&all_library]() { + auto build_catalogs_fn = [&all_library](const bool is_first_load) { /* Start with empty catalog storage. */ all_library.catalog_service = std::make_unique(); /* (Re-)load catalogs on refresh. */ - AssetLibrary::foreach_loaded([&all_library](AssetLibrary &nested) { - nested.catalog_service->reload_catalogs(); - all_library.catalog_service->add_from_existing(*nested.catalog_service); - }); + AssetLibrary::foreach_loaded( + [&](AssetLibrary &nested) { + /* On first load the catalogs were read just above, no need to reload. */ + if (!is_first_load) { + nested.catalog_service->reload_catalogs(); + } + all_library.catalog_service->add_from_existing(*nested.catalog_service); + }, + false); all_library.catalog_service->rebuild_tree(); }; - build_catalogs_fn(); - all_library.on_refresh_ = build_catalogs_fn; + build_catalogs_fn(true); + all_library.on_refresh_ = [build_catalogs_fn]() { build_catalogs_fn(false); }; return &all_library; } diff --git a/source/blender/editors/space_file/filelist.cc b/source/blender/editors/space_file/filelist.cc index 60c34ceda5f..2bf91efc610 100644 --- a/source/blender/editors/space_file/filelist.cc +++ b/source/blender/editors/space_file/filelist.cc @@ -3907,18 +3907,20 @@ static void filelist_readjob_all_asset_library(FileListReadJob *job_params, /* The "All" asset library was loaded, which means all other asset libraries are also loaded. * Load their assets from disk into the "All" library. */ - asset_system::AssetLibrary::foreach_loaded([&](asset_system::AssetLibrary &nested_library) { - StringRefNull root_path = nested_library.root_path(); - if (root_path.is_empty()) { - return; - } + asset_system::AssetLibrary::foreach_loaded( + [&](asset_system::AssetLibrary &nested_library) { + StringRefNull root_path = nested_library.root_path(); + if (root_path.is_empty()) { + return; + } - /* Override library info to read this library. */ - job_params->load_asset_library = &nested_library; - BLI_strncpy(filelist->filelist.root, root_path.c_str(), sizeof(filelist->filelist.root)); + /* Override library info to read this library. */ + job_params->load_asset_library = &nested_library; + BLI_strncpy(filelist->filelist.root, root_path.c_str(), sizeof(filelist->filelist.root)); - filelist_readjob_recursive_dir_add_items(true, job_params, stop, do_update, progress); - }); + filelist_readjob_recursive_dir_add_items(true, job_params, stop, do_update, progress); + }, + false); } /** -- 2.30.2 From ecc25bc62ec517420ce0aef47d9a6af761643f22 Mon Sep 17 00:00:00 2001 From: Julian Eisel Date: Fri, 2 Dec 2022 20:23:54 +0100 Subject: [PATCH 07/25] Use "All" library for node add menu building Code was manually building the add menu from all asset libraries, this should be simpler now. --- source/blender/editors/asset/ED_asset_list.hh | 12 ++ .../editors/asset/intern/asset_list.cc | 17 +++ .../editors/space_node/add_menu_assets.cc | 117 ++++++++---------- 3 files changed, 78 insertions(+), 68 deletions(-) diff --git a/source/blender/editors/asset/ED_asset_list.hh b/source/blender/editors/asset/ED_asset_list.hh index 541fa315f77..f60c6752d5f 100644 --- a/source/blender/editors/asset/ED_asset_list.hh +++ b/source/blender/editors/asset/ED_asset_list.hh @@ -15,6 +15,18 @@ struct AssetLibraryReference; struct FileDirEntry; struct bContext; +namespace blender::asset_system { +class AssetLibrary; +} + +/** + * Get the asset library being read into an asset-list and identified using \a library_reference. + * \note The asset library may be loaded asynchronously, so this may return null until it becomes + * available. + */ +blender::asset_system::AssetLibrary *ED_assetlist_library_get( + const AssetLibraryReference &library_reference); + /* Can return false to stop iterating. */ using AssetListIterFn = blender::FunctionRef; void ED_assetlist_iterate(const AssetLibraryReference &library_reference, AssetListIterFn fn); diff --git a/source/blender/editors/asset/intern/asset_list.cc b/source/blender/editors/asset/intern/asset_list.cc index 64636693a58..e086e5702d1 100644 --- a/source/blender/editors/asset/intern/asset_list.cc +++ b/source/blender/editors/asset/intern/asset_list.cc @@ -114,6 +114,7 @@ class AssetList : NonCopyable { void clear(bContext *C); bool needsRefetch() const; + asset_system::AssetLibrary *asset_library() const; void iterate(AssetListIterFn fn) const; bool listen(const wmNotifier ¬ifier) const; int size() const; @@ -180,6 +181,11 @@ bool AssetList::needsRefetch() const return filelist_needs_force_reset(filelist_) || filelist_needs_reading(filelist_); } +asset_system::AssetLibrary *AssetList::asset_library() const +{ + return reinterpret_cast(filelist_asset_library(filelist_)); +} + void AssetList::iterate(AssetListIterFn fn) const { FileList *files = filelist_; @@ -399,6 +405,7 @@ AssetListStorage::AssetListMap &AssetListStorage::global_storage() /** \name C-API * \{ */ +using namespace blender; using namespace blender::ed::asset; void ED_assetlist_storage_fetch(const AssetLibraryReference *library_reference, const bContext *C) @@ -449,6 +456,16 @@ void ED_assetlist_iterate(const AssetLibraryReference &library_reference, AssetL } } +asset_system::AssetLibrary *ED_assetlist_library_get( + const AssetLibraryReference &library_reference) +{ + const AssetList *list = AssetListStorage::lookup_list(library_reference); + if (!list) { + return nullptr; + } + return list->asset_library(); +} + ImBuf *ED_assetlist_asset_image_get(const AssetHandle *asset_handle) { ImBuf *imbuf = filelist_file_getimage(asset_handle->file_data); diff --git a/source/blender/editors/space_node/add_menu_assets.cc b/source/blender/editors/space_node/add_menu_assets.cc index bb5f33f8cf0..2296922e040 100644 --- a/source/blender/editors/space_node/add_menu_assets.cc +++ b/source/blender/editors/space_node/add_menu_assets.cc @@ -49,13 +49,6 @@ struct LibraryAsset { AssetHandle handle; }; -struct LibraryCatalog { - asset_system::AssetLibrary *library; - /* Catalog pointers are not save to store. Use the catalog ID instead and lookup the catalog when - * needed. */ - const asset_system::CatalogID catalog_id; -}; - struct AssetItemTree { asset_system::AssetCatalogTree catalogs; MultiValueMap assets_per_path; @@ -63,14 +56,18 @@ struct AssetItemTree { full_catalog_per_tree_item; }; +static AssetLibraryReference all_library_reference() +{ + AssetLibraryReference all_library_ref{}; + all_library_ref.custom_library_index = -1; + all_library_ref.type = ASSET_LIBRARY_ALL; + return all_library_ref; +} + static bool all_loading_finished() { - for (const AssetLibraryReference &library : asset_system::all_valid_asset_library_refs()) { - if (!ED_assetlist_is_loaded(&library)) { - return false; - } - } - return true; + AssetLibraryReference all_library_ref = all_library_reference(); + return ED_assetlist_is_loaded(&all_library_ref); } static AssetItemTree build_catalog_tree(const bContext &C, const bNodeTree *node_tree) @@ -78,68 +75,52 @@ static AssetItemTree build_catalog_tree(const bContext &C, const bNodeTree *node if (!node_tree) { return {}; } - const Main &bmain = *CTX_data_main(&C); - const Vector all_libraries = asset_system::all_valid_asset_library_refs(); - - /* Merge catalogs from all libraries to deduplicate menu items. Also store the catalog and - * library for each asset ID in order to use them later when retrieving assets and removing - * empty catalogs. */ - Map id_to_catalog_map; - asset_system::AssetCatalogTree catalogs_from_all_libraries; - for (const AssetLibraryReference &library_ref : all_libraries) { - if (asset_system::AssetLibrary *library = AS_asset_library_load(&bmain, library_ref)) { - if (asset_system::AssetCatalogTree *tree = library->catalog_service->get_catalog_tree()) { - tree->foreach_item([&](asset_system::AssetCatalogTreeItem &item) { - const asset_system::CatalogID &id = item.get_catalog_id(); - asset_system::AssetCatalog *catalog = library->catalog_service->find_catalog(id); - catalogs_from_all_libraries.insert_item(*catalog); - id_to_catalog_map.add(item.get_catalog_id(), LibraryCatalog{library, id}); - }); - } - } - } /* Find all the matching node group assets for every catalog path. */ MultiValueMap assets_per_path; - for (const AssetLibraryReference &library_ref : all_libraries) { - AssetFilterSettings type_filter{}; - type_filter.id_types = FILTER_ID_NT; - ED_assetlist_storage_fetch(&library_ref, &C); - ED_assetlist_ensure_previews_job(&library_ref, &C); - ED_assetlist_iterate(library_ref, [&](AssetHandle asset) { - if (!ED_asset_filter_matches_asset(&type_filter, &asset)) { - return true; - } - const AssetMetaData &meta_data = *ED_asset_handle_get_metadata(&asset); - const IDProperty *tree_type = BKE_asset_metadata_idprop_find(&meta_data, "type"); - if (tree_type == nullptr || IDP_Int(tree_type) != node_tree->type) { - return true; - } - if (BLI_uuid_is_nil(meta_data.catalog_id)) { - return true; - } - const LibraryCatalog *library_catalog = id_to_catalog_map.lookup_ptr(meta_data.catalog_id); - if (library_catalog == nullptr) { - return true; - } - const asset_system::AssetCatalog *catalog = - library_catalog->library->catalog_service->find_catalog(library_catalog->catalog_id); - assets_per_path.add(catalog->path, LibraryAsset{library_ref, asset}); - return true; - }); + AssetFilterSettings type_filter{}; + type_filter.id_types = FILTER_ID_NT; + + const AssetLibraryReference all_library_ref = all_library_reference(); + + ED_assetlist_storage_fetch(&all_library_ref, &C); + ED_assetlist_ensure_previews_job(&all_library_ref, &C); + + asset_system::AssetLibrary *all_library = ED_assetlist_library_get(all_library_ref); + if (!all_library) { + return {}; } - /* Build the final tree without any of the catalogs that don't have proper node group assets. */ - asset_system::AssetCatalogTree catalogs_with_node_assets; - catalogs_from_all_libraries.foreach_item([&](asset_system::AssetCatalogTreeItem &item) { - if (!assets_per_path.lookup(item.catalog_path()).is_empty()) { - const asset_system::CatalogID &id = item.get_catalog_id(); - const LibraryCatalog &library_catalog = id_to_catalog_map.lookup(id); - asset_system::AssetCatalog *catalog = library_catalog.library->catalog_service->find_catalog( - id); - catalogs_with_node_assets.insert_item(*catalog); + ED_assetlist_iterate(all_library_ref, [&](AssetHandle asset) { + if (!ED_asset_filter_matches_asset(&type_filter, &asset)) { + return true; } + const AssetMetaData &meta_data = *ED_asset_handle_get_metadata(&asset); + const IDProperty *tree_type = BKE_asset_metadata_idprop_find(&meta_data, "type"); + if (tree_type == nullptr || IDP_Int(tree_type) != node_tree->type) { + return true; + } + if (BLI_uuid_is_nil(meta_data.catalog_id)) { + return true; + } + + const asset_system::AssetCatalog *catalog = all_library->catalog_service->find_catalog( + meta_data.catalog_id); + assets_per_path.add(catalog->path, LibraryAsset{all_library_ref, asset}); + return true; + }); + + /* Build an own tree without any of the catalogs that don't have proper node group assets. */ + asset_system::AssetCatalogTree catalogs_with_node_assets; + asset_system::AssetCatalogTree &catalog_tree = *all_library->catalog_service->get_catalog_tree(); + catalog_tree.foreach_item([&](asset_system::AssetCatalogTreeItem &item) { + if (assets_per_path.lookup(item.catalog_path()).is_empty()) { + return; + } + asset_system::AssetCatalog *catalog = all_library->catalog_service->find_catalog( + item.get_catalog_id()); + catalogs_with_node_assets.insert_item(*catalog); }); /* Build another map storing full asset paths for each tree item, in order to have stable -- 2.30.2 From 11abc1be394ccb148c5cda3ecde6dbe0d5eb4f27 Mon Sep 17 00:00:00 2001 From: Julian Eisel Date: Fri, 2 Dec 2022 20:28:33 +0100 Subject: [PATCH 08/25] Use "All" library for node search menu building Code was manually building the search menu items from all asset libraries, this is simpler now. --- .../editors/space_node/add_node_search.cc | 33 +++++-------------- 1 file changed, 8 insertions(+), 25 deletions(-) diff --git a/source/blender/editors/space_node/add_node_search.cc b/source/blender/editors/space_node/add_node_search.cc index ba060ab3925..85c79d5e57d 100644 --- a/source/blender/editors/space_node/add_node_search.cc +++ b/source/blender/editors/space_node/add_node_search.cc @@ -93,12 +93,15 @@ static void search_items_for_asset_metadata(const bNodeTree &node_tree, search_items.append(std::move(item)); } -static void gather_search_items_for_asset_library(const bContext &C, - const bNodeTree &node_tree, - const AssetLibraryReference &library_ref, - Set &r_added_assets, - Vector &search_items) +static void gather_search_items_for_all_assets(const bContext &C, + const bNodeTree &node_tree, + Set &r_added_assets, + Vector &search_items) { + AssetLibraryReference library_ref{}; + library_ref.custom_library_index = -1; + library_ref.type = ASSET_LIBRARY_ALL; + AssetFilterSettings filter_settings{}; filter_settings.id_types = FILTER_ID_NT; @@ -117,26 +120,6 @@ static void gather_search_items_for_asset_library(const bContext &C, }); } -static void gather_search_items_for_all_assets(const bContext &C, - const bNodeTree &node_tree, - Set &r_added_assets, - Vector &search_items) -{ - int i; - LISTBASE_FOREACH_INDEX (const bUserAssetLibrary *, asset_library, &U.asset_libraries, i) { - AssetLibraryReference library_ref{}; - library_ref.custom_library_index = i; - library_ref.type = ASSET_LIBRARY_CUSTOM; - /* Skip local assets to avoid duplicates when the asset is part of the local file library. */ - gather_search_items_for_asset_library(C, node_tree, library_ref, r_added_assets, search_items); - } - - AssetLibraryReference library_ref{}; - library_ref.custom_library_index = -1; - library_ref.type = ASSET_LIBRARY_LOCAL; - gather_search_items_for_asset_library(C, node_tree, library_ref, r_added_assets, search_items); -} - static void gather_search_items_for_node_groups(const bContext &C, const bNodeTree &node_tree, const Set &local_assets, -- 2.30.2 From 747a9ea263ad97a78878705f020e3587f50734ff Mon Sep 17 00:00:00 2001 From: Julian Eisel Date: Tue, 6 Dec 2022 17:02:05 +0100 Subject: [PATCH 09/25] Make catalogs from "All" library read-only Loading the asset library will create a read-only catalog service. The read-only nature is not dealt with much in the asset catalog code, the using code (e.g. the UI) is responsible for respecting it. --- .../blender/asset_system/AS_asset_catalog.hh | 13 ++++++++ .../asset_system/intern/asset_catalog.cc | 16 ++++++++++ .../intern/asset_library_service.cc | 3 +- .../blender/editors/asset/ED_asset_catalog.hh | 5 ++++ .../editors/asset/intern/asset_catalog.cc | 22 ++++++++++++++ .../blender/editors/asset/intern/asset_ops.cc | 13 +++++++- .../space_file/asset_catalog_tree_view.cc | 30 ++++++++++++++++--- 7 files changed, 96 insertions(+), 6 deletions(-) diff --git a/source/blender/asset_system/AS_asset_catalog.hh b/source/blender/asset_system/AS_asset_catalog.hh index a7829778fdf..1d5401f0b0e 100644 --- a/source/blender/asset_system/AS_asset_catalog.hh +++ b/source/blender/asset_system/AS_asset_catalog.hh @@ -45,11 +45,17 @@ class AssetCatalogService { Vector> undo_snapshots_; Vector> redo_snapshots_; + const bool is_read_only_ = false; + public: static const CatalogFilePath DEFAULT_CATALOG_FILENAME; + struct read_only_tag { + }; + public: AssetCatalogService(); + explicit AssetCatalogService(read_only_tag); explicit AssetCatalogService(const CatalogFilePath &asset_library_root); /** @@ -62,6 +68,13 @@ class AssetCatalogService { void tag_has_unsaved_changes(AssetCatalog *edited_catalog); bool has_unsaved_changes() const; + /** + * Check if this is a read-only service meaning the user shouldn't be able to do edits. This is + * not enforced by internal catalog code, the catalog service user is responsible for it. For + * example the UI should disallow edits. + */ + bool is_read_only() const; + /** Load asset catalog definitions from the files found in the asset library. */ void load_from_disk(); /** Load asset catalog definitions from the given file or directory. */ diff --git a/source/blender/asset_system/intern/asset_catalog.cc b/source/blender/asset_system/intern/asset_catalog.cc index 7924ceb862c..6256b6ee503 100644 --- a/source/blender/asset_system/intern/asset_catalog.cc +++ b/source/blender/asset_system/intern/asset_catalog.cc @@ -44,6 +44,11 @@ AssetCatalogService::AssetCatalogService() { } +AssetCatalogService::AssetCatalogService(read_only_tag) : AssetCatalogService() +{ + const_cast(is_read_only_) = true; +} + AssetCatalogService::AssetCatalogService(const CatalogFilePath &asset_library_root) : AssetCatalogService() { @@ -52,6 +57,8 @@ AssetCatalogService::AssetCatalogService(const CatalogFilePath &asset_library_ro void AssetCatalogService::tag_has_unsaved_changes(AssetCatalog *edited_catalog) { + BLI_assert(!is_read_only_); + if (edited_catalog) { edited_catalog->flags.has_unsaved_changes = true; } @@ -86,6 +93,11 @@ bool AssetCatalogService::has_unsaved_changes() const return catalog_collection_->has_unsaved_changes_; } +bool AssetCatalogService::is_read_only() const +{ + return is_read_only_; +} + void AssetCatalogService::tag_all_catalogs_as_unsaved_changes() { for (auto &catalog : catalog_collection_->catalogs_.values()) { @@ -458,6 +470,8 @@ bool AssetCatalogService::is_catalog_known_with_unsaved_changes(const CatalogID bool AssetCatalogService::write_to_disk(const CatalogFilePath &blend_file_path) { + BLI_assert(!is_read_only_); + if (!write_to_disk_ex(blend_file_path)) { return false; } @@ -631,6 +645,7 @@ void AssetCatalogService::undo() void AssetCatalogService::redo() { + BLI_assert(!is_read_only_); BLI_assert_msg(is_redo_possbile(), "Redo stack is empty"); undo_snapshots_.append(std::move(catalog_collection_)); @@ -640,6 +655,7 @@ void AssetCatalogService::redo() void AssetCatalogService::undo_push() { + BLI_assert(!is_read_only_); std::unique_ptr snapshot = catalog_collection_->deep_copy(); undo_snapshots_.append(std::move(snapshot)); redo_snapshots_.clear(); diff --git a/source/blender/asset_system/intern/asset_library_service.cc b/source/blender/asset_system/intern/asset_library_service.cc index fbcda02025c..80356214e40 100644 --- a/source/blender/asset_system/intern/asset_library_service.cc +++ b/source/blender/asset_system/intern/asset_library_service.cc @@ -157,7 +157,8 @@ AssetLibrary *AssetLibraryService::get_asset_library_all(const Main *bmain) AssetLibrary &all_library = *all_library_; auto build_catalogs_fn = [&all_library](const bool is_first_load) { /* Start with empty catalog storage. */ - all_library.catalog_service = std::make_unique(); + all_library.catalog_service = std::make_unique( + AssetCatalogService::read_only_tag()); /* (Re-)load catalogs on refresh. */ AssetLibrary::foreach_loaded( diff --git a/source/blender/editors/asset/ED_asset_catalog.hh b/source/blender/editors/asset/ED_asset_catalog.hh index 4d61f816085..d236bdec37f 100644 --- a/source/blender/editors/asset/ED_asset_catalog.hh +++ b/source/blender/editors/asset/ED_asset_catalog.hh @@ -6,6 +6,9 @@ * UI/Editor level API for catalog operations, creating richer functionality than the asset system * catalog API provides (which this uses internally). * + * Functions can be expected to not perform any change when #ED_asset_catalogs_read_only() returns + * true. The caller should check. + * * Note that `ED_asset_catalog.h` is part of this API. */ @@ -19,6 +22,8 @@ struct AssetLibrary; +[[nodiscard]] bool ED_asset_catalogs_read_only(const AssetLibrary &library); + blender::asset_system::AssetCatalog *ED_asset_catalog_add( AssetLibrary *library, blender::StringRefNull name, blender::StringRef parent_path = nullptr); void ED_asset_catalog_remove(AssetLibrary *library, diff --git a/source/blender/editors/asset/intern/asset_catalog.cc b/source/blender/editors/asset/intern/asset_catalog.cc index 10aa27a7687..2bfde0aeaae 100644 --- a/source/blender/editors/asset/intern/asset_catalog.cc +++ b/source/blender/editors/asset/intern/asset_catalog.cc @@ -19,6 +19,13 @@ using namespace blender; using namespace blender::asset_system; +bool ED_asset_catalogs_read_only(const ::AssetLibrary &library) +{ + asset_system::AssetCatalogService *catalog_service = AS_asset_library_get_catalog_service( + &library); + return catalog_service->is_read_only(); +} + struct CatalogUniqueNameFnData { const AssetCatalogService &catalog_service; StringRef parent_path; @@ -53,6 +60,9 @@ asset_system::AssetCatalog *ED_asset_catalog_add(::AssetLibrary *library, if (!catalog_service) { return nullptr; } + if (ED_asset_catalogs_read_only(*library)) { + return nullptr; + } std::string unique_name = catalog_name_ensure_unique(*catalog_service, name, parent_path); AssetCatalogPath fullpath = AssetCatalogPath(parent_path) / unique_name; @@ -76,6 +86,9 @@ void ED_asset_catalog_remove(::AssetLibrary *library, const CatalogID &catalog_i BLI_assert_unreachable(); return; } + if (ED_asset_catalogs_read_only(*library)) { + return; + } catalog_service->undo_push(); catalog_service->tag_has_unsaved_changes(nullptr); @@ -93,6 +106,9 @@ void ED_asset_catalog_rename(::AssetLibrary *library, BLI_assert_unreachable(); return; } + if (ED_asset_catalogs_read_only(*library)) { + return; + } AssetCatalog *catalog = catalog_service->find_catalog(catalog_id); @@ -120,6 +136,9 @@ void ED_asset_catalog_move(::AssetLibrary *library, BLI_assert_unreachable(); return; } + if (ED_asset_catalogs_read_only(*library)) { + return; + } AssetCatalog *src_catalog = catalog_service->find_catalog(src_catalog_id); if (!src_catalog) { @@ -161,6 +180,9 @@ void ED_asset_catalogs_save_from_main_path(::AssetLibrary *library, const Main * BLI_assert_unreachable(); return; } + if (ED_asset_catalogs_read_only(*library)) { + return; + } /* Since writing to disk also means loading any on-disk changes, it may be a good idea to store * an undo step. */ diff --git a/source/blender/editors/asset/intern/asset_ops.cc b/source/blender/editors/asset/intern/asset_ops.cc index da4e0794fe8..4189287bc7e 100644 --- a/source/blender/editors/asset/intern/asset_ops.cc +++ b/source/blender/editors/asset/intern/asset_ops.cc @@ -438,7 +438,18 @@ static void ASSET_OT_library_refresh(struct wmOperatorType *ot) static bool asset_catalog_operator_poll(bContext *C) { const SpaceFile *sfile = CTX_wm_space_file(C); - return sfile && ED_fileselect_active_asset_library_get(sfile); + if (!sfile) { + return false; + } + const AssetLibrary *asset_library = ED_fileselect_active_asset_library_get(sfile); + if (!asset_library) { + return false; + } + if (ED_asset_catalogs_read_only(*asset_library)) { + CTX_wm_operator_poll_msg_set(C, "Asset catalogs cannot be edited in this asset library"); + return false; + } + return true; } static int asset_catalog_new_exec(bContext *C, wmOperator *op) diff --git a/source/blender/editors/space_file/asset_catalog_tree_view.cc b/source/blender/editors/space_file/asset_catalog_tree_view.cc index 19b3f135e24..8b7b19387c2 100644 --- a/source/blender/editors/space_file/asset_catalog_tree_view.cc +++ b/source/blender/editors/space_file/asset_catalog_tree_view.cc @@ -119,6 +119,8 @@ class AssetCatalogDropController : public ui::AbstractViewItemDropController { static AssetCatalog *get_drag_catalog(const wmDrag &drag, const ::AssetLibrary &asset_library); static bool has_droppable_asset(const wmDrag &drag, const char **r_disabled_hint); + static bool can_modify_catalogs(const ::AssetLibrary &asset_library, + const char **r_disabled_hint); static bool drop_assets_into_catalog(struct bContext *C, const AssetCatalogTreeView &tree_view, const wmDrag &drag, @@ -321,7 +323,9 @@ void AssetCatalogTreeViewItem::build_context_menu(bContext &C, uiLayout &column) bool AssetCatalogTreeViewItem::supports_renaming() const { - return true; + const AssetCatalogTreeView &tree_view = static_cast( + get_tree_view()); + return !ED_asset_catalogs_read_only(*tree_view.asset_library_); } bool AssetCatalogTreeViewItem::rename(StringRefNull new_name) @@ -360,7 +364,12 @@ AssetCatalogDropController::AssetCatalogDropController(AssetCatalogTreeView &tre bool AssetCatalogDropController::can_drop(const wmDrag &drag, const char **r_disabled_hint) const { if (drag.type == WM_DRAG_ASSET_CATALOG) { - const AssetCatalog *drag_catalog = get_drag_catalog(drag, get_asset_library()); + const ::AssetLibrary &library = get_asset_library(); + if (!can_modify_catalogs(library, r_disabled_hint)) { + return false; + } + + const AssetCatalog *drag_catalog = get_drag_catalog(drag, library); /* NOTE: Technically it's not an issue to allow this (the catalog will just receive a new * path and the catalog system will generate missing parents from the path). But it does * appear broken to users, so disabling entirely. */ @@ -512,6 +521,16 @@ bool AssetCatalogDropController::has_droppable_asset(const wmDrag &drag, return false; } +bool AssetCatalogDropController::can_modify_catalogs(const ::AssetLibrary &library, + const char **r_disabled_hint) +{ + if (ED_asset_catalogs_read_only(library)) { + *r_disabled_hint = "Catalogs cannot be edited in this asset library"; + return false; + } + return true; +} + ::AssetLibrary &AssetCatalogDropController::get_asset_library() const { return *get_view().asset_library_; @@ -579,9 +598,12 @@ bool AssetCatalogTreeViewAllItem::DropController::can_drop(const wmDrag &drag, if (drag.type != WM_DRAG_ASSET_CATALOG) { return false; } + ::AssetLibrary &library = *get_view().asset_library_; + if (!AssetCatalogDropController::can_modify_catalogs(library, r_disabled_hint)) { + return false; + } - const AssetCatalog *drag_catalog = AssetCatalogDropController::get_drag_catalog( - drag, *get_view().asset_library_); + const AssetCatalog *drag_catalog = AssetCatalogDropController::get_drag_catalog(drag, library); if (drag_catalog->path.parent() == "") { *r_disabled_hint = "Catalog is already placed at the highest level"; return false; -- 2.30.2 From e8575bfd4a3a27e9ad636f682ff198907472bec5 Mon Sep 17 00:00:00 2001 From: Julian Eisel Date: Wed, 7 Dec 2022 18:19:15 +0100 Subject: [PATCH 10/25] General cleanup (comments, remove outdated TODO marks, naming) --- source/blender/asset_system/AS_asset_library.hh | 1 - source/blender/asset_system/intern/asset_library.cc | 1 - source/blender/asset_system/intern/asset_storage.hh | 2 -- source/blender/editors/asset/ED_asset_catalog.h | 2 ++ source/blender/editors/asset/ED_asset_catalog.hh | 4 ++++ source/blender/editors/asset/ED_asset_library.h | 7 +++++-- .../asset/intern/asset_library_reference_enum.cc | 12 ++++++------ source/blender/editors/space_file/file_draw.c | 1 - source/blender/editors/space_file/filelist.cc | 1 - source/blender/makesdna/DNA_space_types.h | 1 - 10 files changed, 17 insertions(+), 15 deletions(-) diff --git a/source/blender/asset_system/AS_asset_library.hh b/source/blender/asset_system/AS_asset_library.hh index 6526e3e8382..14d356c703e 100644 --- a/source/blender/asset_system/AS_asset_library.hh +++ b/source/blender/asset_system/AS_asset_library.hh @@ -109,7 +109,6 @@ class AssetLibrary { /** Remove an asset from the library that was added using #add_external_asset() or * #add_local_id_asset(). Can usually be expected to be constant time complexity (worst case may * differ). - * * \note This is save to call if \a asset is freed (dangling reference), will not perform any * change then. * \return True on success, false if the asset couldn't be found inside the library (also the diff --git a/source/blender/asset_system/intern/asset_library.cc b/source/blender/asset_system/intern/asset_library.cc index 02c5c0dc4f3..f5377734b12 100644 --- a/source/blender/asset_system/intern/asset_library.cc +++ b/source/blender/asset_system/intern/asset_library.cc @@ -257,7 +257,6 @@ StringRefNull AssetLibrary::root_path() const return *root_path_; } -/* TODO get rid of this. */ Vector all_valid_asset_library_refs() { Vector result; diff --git a/source/blender/asset_system/intern/asset_storage.hh b/source/blender/asset_system/intern/asset_storage.hh index b4866fa9382..2b4614abca5 100644 --- a/source/blender/asset_system/intern/asset_storage.hh +++ b/source/blender/asset_system/intern/asset_storage.hh @@ -31,8 +31,6 @@ class AssetStorage { * faster lookups. Not possible until each asset is only represented once in the storage. */ StorageT local_id_assets_; - friend class AssetLibrary; - public: /** See #AssetLibrary::add_external_asset(). */ AssetRepresentation &add_external_asset(AssetIdentifier &&identifier, diff --git a/source/blender/editors/asset/ED_asset_catalog.h b/source/blender/editors/asset/ED_asset_catalog.h index 04df381bec9..9f6850b7266 100644 --- a/source/blender/editors/asset/ED_asset_catalog.h +++ b/source/blender/editors/asset/ED_asset_catalog.h @@ -19,6 +19,8 @@ struct Main; void ED_asset_catalogs_save_from_main_path(struct AssetLibrary *library, const struct Main *bmain); +/** Saving catalog edits when the file is saved is a global option shared for each asset library, + * and as such ignores the per asset library #ED_asset_catalogs_read_only(). */ void ED_asset_catalogs_set_save_catalogs_when_file_is_saved(bool should_save); bool ED_asset_catalogs_get_save_catalogs_when_file_is_saved(void); diff --git a/source/blender/editors/asset/ED_asset_catalog.hh b/source/blender/editors/asset/ED_asset_catalog.hh index d236bdec37f..a423ad6f8ad 100644 --- a/source/blender/editors/asset/ED_asset_catalog.hh +++ b/source/blender/editors/asset/ED_asset_catalog.hh @@ -22,6 +22,10 @@ struct AssetLibrary; +/** + * Returns if the catalogs of \a library are allowed to be editable, or if the UI should forbid + * edits. + */ [[nodiscard]] bool ED_asset_catalogs_read_only(const AssetLibrary &library); blender::asset_system::AssetCatalog *ED_asset_catalog_add( diff --git a/source/blender/editors/asset/ED_asset_library.h b/source/blender/editors/asset/ED_asset_library.h index cc0d97054b6..c4baadc23c8 100644 --- a/source/blender/editors/asset/ED_asset_library.h +++ b/source/blender/editors/asset/ED_asset_library.h @@ -29,10 +29,13 @@ AssetLibraryReference ED_asset_library_reference_from_enum_value(int value); * Since this is meant for UI display, skips non-displayable libraries, that is, libraries with an * empty name or path. * - * \param include_local_library: Whether to include the "Current File" library or not. + * \param include_generated: Whether to include libraries that are generated and thus cannot be + * written to. Setting this to false means only custom libraries will be + * included, since they are stored on disk with a single root directory, + * thus have a well defined location that can be written to. */ const struct EnumPropertyItem *ED_asset_library_reference_to_rna_enum_itemf( - bool include_local_library); + bool include_generated); #ifdef __cplusplus } diff --git a/source/blender/editors/asset/intern/asset_library_reference_enum.cc b/source/blender/editors/asset/intern/asset_library_reference_enum.cc index f88afdd8487..d20f3205a77 100644 --- a/source/blender/editors/asset/intern/asset_library_reference_enum.cc +++ b/source/blender/editors/asset/intern/asset_library_reference_enum.cc @@ -70,14 +70,13 @@ AssetLibraryReference ED_asset_library_reference_from_enum_value(int value) return library; } -const EnumPropertyItem *ED_asset_library_reference_to_rna_enum_itemf( - const bool include_local_library) +const EnumPropertyItem *ED_asset_library_reference_to_rna_enum_itemf(const bool include_generated) { EnumPropertyItem *item = nullptr; int totitem = 0; - if (include_local_library) { - const EnumPropertyItem predefined_items[] = { + if (include_generated) { + const EnumPropertyItem generated_items[] = { {ASSET_LIBRARY_ALL, "ALL", ICON_NONE, @@ -91,8 +90,9 @@ const EnumPropertyItem *ED_asset_library_reference_to_rna_enum_itemf( {0, nullptr, 0, nullptr, nullptr}, }; - /* Add predefined items. */ - RNA_enum_items_add(&item, &totitem, predefined_items); + /* Add predefined libraries that are generated and not simple directories that can be written + * to. */ + RNA_enum_items_add(&item, &totitem, generated_items); } /* Add separator if needed. */ diff --git a/source/blender/editors/space_file/file_draw.c b/source/blender/editors/space_file/file_draw.c index 7561cd2f6a2..e85a6cbc0d4 100644 --- a/source/blender/editors/space_file/file_draw.c +++ b/source/blender/editors/space_file/file_draw.c @@ -555,7 +555,6 @@ static void file_draw_preview(const SpaceFile *sfile, /* path is no more static, cannot give it directly to but... */ else if (sfile->browse_mode == FILE_BROWSE_MODE_ASSETS && (file->typeflag & FILE_TYPE_ASSET) != 0) { - /* TODO enable drag & drop support, get path from asset representation. */ char blend_path[FILE_MAX_LIBEXTRA]; if (BLO_library_path_explode(path, blend_path, NULL, NULL)) { diff --git a/source/blender/editors/space_file/filelist.cc b/source/blender/editors/space_file/filelist.cc index 2bf91efc610..234c550d829 100644 --- a/source/blender/editors/space_file/filelist.cc +++ b/source/blender/editors/space_file/filelist.cc @@ -1643,7 +1643,6 @@ static void filelist_cache_previews_push(FileList *filelist, FileDirEntry *entry BLI_thread_queue_push(cache->previews_done, preview); } else { - /* XXX */ if (entry->redirection_path) { BLI_strncpy(preview->filepath, entry->redirection_path, FILE_MAXDIR); } diff --git a/source/blender/makesdna/DNA_space_types.h b/source/blender/makesdna/DNA_space_types.h index bbb6d3e7de3..54edf795e6c 100644 --- a/source/blender/makesdna/DNA_space_types.h +++ b/source/blender/makesdna/DNA_space_types.h @@ -995,7 +995,6 @@ enum eFileDetails { /** File selector types. */ typedef enum eFileSelectType { - FILE_SELECT_TYPE_UNSET = 0, FILE_LOADLIB = 1, FILE_MAIN = 2, /** Load assets from #Main. */ -- 2.30.2 From 3cd93ace247fefff8dec69879bb61ed074d55495 Mon Sep 17 00:00:00 2001 From: Julian Eisel Date: Wed, 7 Dec 2022 18:29:38 +0100 Subject: [PATCH 11/25] Simple progress reporting for all library Progress bar display the file reading (and other operations) is actually broken in master for a while, so this won't actually be reported. Still calculate it for once it's fixed. --- source/blender/editors/space_file/filelist.cc | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/source/blender/editors/space_file/filelist.cc b/source/blender/editors/space_file/filelist.cc index 234c550d829..44f846f8595 100644 --- a/source/blender/editors/space_file/filelist.cc +++ b/source/blender/editors/space_file/filelist.cc @@ -3900,10 +3900,14 @@ static void filelist_readjob_all_asset_library(FileListReadJob *job_params, if (job_params->only_main_data) { return; } - /* TODO propertly update progress? */ + + /* Count how many asset libraries need to be loaded, for progress reporting. Not very precise. */ + int library_count = 0; + asset_system::AssetLibrary::foreach_loaded([&library_count](auto &) { library_count++; }, false); BLI_assert(filelist->asset_library != nullptr); + int libraries_done_count = 0; /* The "All" asset library was loaded, which means all other asset libraries are also loaded. * Load their assets from disk into the "All" library. */ asset_system::AssetLibrary::foreach_loaded( @@ -3917,7 +3921,12 @@ static void filelist_readjob_all_asset_library(FileListReadJob *job_params, job_params->load_asset_library = &nested_library; BLI_strncpy(filelist->filelist.root, root_path.c_str(), sizeof(filelist->filelist.root)); - filelist_readjob_recursive_dir_add_items(true, job_params, stop, do_update, progress); + float progress_this = 0.0f; + filelist_readjob_recursive_dir_add_items( + true, job_params, stop, do_update, &progress_this); + + libraries_done_count++; + *progress = float(libraries_done_count) / library_count; }, false); } -- 2.30.2 From d6df32a6f841836c9b83f116bd95440346cc74fe Mon Sep 17 00:00:00 2001 From: Julian Eisel Date: Wed, 7 Dec 2022 20:00:27 +0100 Subject: [PATCH 12/25] Add basic (empty) asset shelf region --- .../blenloader/intern/versioning_300.cc | 21 +++++++++++++++++++ source/blender/editors/screen/area.c | 3 ++- source/blender/editors/screen/screen_edit.c | 3 ++- .../editors/space_view3d/space_view3d.cc | 17 +++++++++++++++ source/blender/makesdna/DNA_screen_types.h | 3 ++- source/blender/makesrna/intern/rna_screen.c | 1 + 6 files changed, 45 insertions(+), 3 deletions(-) diff --git a/source/blender/blenloader/intern/versioning_300.cc b/source/blender/blenloader/intern/versioning_300.cc index 85078a9902d..2acc95b7339 100644 --- a/source/blender/blenloader/intern/versioning_300.cc +++ b/source/blender/blenloader/intern/versioning_300.cc @@ -3732,5 +3732,26 @@ void blo_do_versions_300(FileData *fd, Library * /*lib*/, Main *bmain) image->seam_margin = 8; } } + + /* Add a properties sidebar to the spreadsheet editor. */ + LISTBASE_FOREACH (bScreen *, screen, &bmain->screens) { + LISTBASE_FOREACH (ScrArea *, area, &screen->areabase) { + LISTBASE_FOREACH (SpaceLink *, sl, &area->spacedata) { + if (sl->spacetype == SPACE_VIEW3D) { + ListBase *regionbase = (sl == area->spacedata.first) ? &area->regionbase : + &sl->regionbase; + ARegion *new_asset_shelf = do_versions_add_region_if_not_found( + regionbase, + RGN_TYPE_ASSET_SHELF, + "asset shelf for view3d (versioning)", + RGN_TYPE_UI); + if (new_asset_shelf != nullptr) { + new_asset_shelf->alignment = RGN_ALIGN_BOTTOM; + new_asset_shelf->flag |= RGN_FLAG_HIDDEN; + } + } + } + } + } } } diff --git a/source/blender/editors/screen/area.c b/source/blender/editors/screen/area.c index a62e027ba03..3721774842c 100644 --- a/source/blender/editors/screen/area.c +++ b/source/blender/editors/screen/area.c @@ -1262,7 +1262,8 @@ bool ED_region_is_overlap(int spacetype, int regiontype) RGN_TYPE_UI, RGN_TYPE_TOOL_PROPS, RGN_TYPE_FOOTER, - RGN_TYPE_TOOL_HEADER)) { + RGN_TYPE_TOOL_HEADER, + RGN_TYPE_ASSET_SHELF)) { return true; } } diff --git a/source/blender/editors/screen/screen_edit.c b/source/blender/editors/screen/screen_edit.c index 14ed5987cc7..0acae5b5ea5 100644 --- a/source/blender/editors/screen/screen_edit.c +++ b/source/blender/editors/screen/screen_edit.c @@ -1435,7 +1435,8 @@ static bScreen *screen_state_to_nonnormal(bContext *C, RGN_TYPE_FOOTER, RGN_TYPE_TOOLS, RGN_TYPE_NAV_BAR, - RGN_TYPE_EXECUTE)) { + RGN_TYPE_EXECUTE, + RGN_TYPE_ASSET_SHELF)) { region->flag |= RGN_FLAG_HIDDEN; } } diff --git a/source/blender/editors/space_view3d/space_view3d.cc b/source/blender/editors/space_view3d/space_view3d.cc index 05fb0c6a720..b2da6630b38 100644 --- a/source/blender/editors/space_view3d/space_view3d.cc +++ b/source/blender/editors/space_view3d/space_view3d.cc @@ -289,6 +289,14 @@ static SpaceLink *view3d_create(const ScrArea * /*area*/, const Scene *scene) region->alignment = RGN_ALIGN_RIGHT; region->flag = RGN_FLAG_HIDDEN; + /* asset shelf */ + region = MEM_cnew("asset shelf for view3d"); + + BLI_addtail(&v3d->regionbase, region); + region->regiontype = RGN_TYPE_ASSET_SHELF; + region->alignment = RGN_ALIGN_BOTTOM; + region->flag = RGN_FLAG_HIDDEN; + /* main region */ region = MEM_cnew("main region for view3d"); @@ -2137,6 +2145,15 @@ void ED_spacetype_view3d() art->draw = view3d_header_region_draw; BLI_addhead(&st->regiontypes, art); + /* regions: asset shelf */ + art = MEM_cnew("spacetype view3d asset shelf region"); + art->regionid = RGN_TYPE_ASSET_SHELF; + art->prefsizey = HEADERY * 4; + art->keymapflag = ED_KEYMAP_UI | ED_KEYMAP_VIEW2D | ED_KEYMAP_FRAMES | ED_KEYMAP_HEADER; + art->init = view3d_header_region_init; + art->draw = view3d_header_region_draw; + BLI_addhead(&st->regiontypes, art); + /* regions: hud */ art = ED_area_type_hud(st->spaceid); BLI_addhead(&st->regiontypes, art); diff --git a/source/blender/makesdna/DNA_screen_types.h b/source/blender/makesdna/DNA_screen_types.h index 4d4bd9ef775..4feab97b6b7 100644 --- a/source/blender/makesdna/DNA_screen_types.h +++ b/source/blender/makesdna/DNA_screen_types.h @@ -657,8 +657,9 @@ typedef enum eRegion_Type { /* Region type used exclusively by internal code and add-ons to register draw callbacks to the XR * context (surface, mirror view). Does not represent any real region. */ RGN_TYPE_XR = 13, + RGN_TYPE_ASSET_SHELF = 14, -#define RGN_TYPE_NUM (RGN_TYPE_XR + 1) +#define RGN_TYPE_NUM (RGN_TYPE_ASSET_SHELF + 1) } eRegion_Type; /* use for function args */ diff --git a/source/blender/makesrna/intern/rna_screen.c b/source/blender/makesrna/intern/rna_screen.c index a65bd613ecf..4d7354421ed 100644 --- a/source/blender/makesrna/intern/rna_screen.c +++ b/source/blender/makesrna/intern/rna_screen.c @@ -26,6 +26,7 @@ const EnumPropertyItem rna_enum_region_type_items[] = { {RGN_TYPE_UI, "UI", 0, "UI", ""}, {RGN_TYPE_TOOLS, "TOOLS", 0, "Tools", ""}, {RGN_TYPE_TOOL_PROPS, "TOOL_PROPS", 0, "Tool Properties", ""}, + {RGN_TYPE_ASSET_SHELF, "ASSET_SHELF", 0, "Asset Shelf", ""}, {RGN_TYPE_PREVIEW, "PREVIEW", 0, "Preview", ""}, {RGN_TYPE_HUD, "HUD", 0, "Floating Region", ""}, {RGN_TYPE_NAV_BAR, "NAVIGATION_BAR", 0, "Navigation Bar", ""}, -- 2.30.2 From 5fc5d742a7c028b273831173ee728ce5649702f0 Mon Sep 17 00:00:00 2001 From: Julian Eisel Date: Tue, 17 Jan 2023 12:48:03 +0100 Subject: [PATCH 13/25] Temp --- source/blender/blenkernel/BKE_screen.h | 4 + source/blender/blenkernel/intern/screen.c | 3 +- source/blender/editors/include/ED_screen.h | 3 +- source/blender/editors/screen/area.c | 19 +- source/blender/editors/screen/screen_edit.c | 50 +++++- .../blender/editors/space_file/space_file.c | 168 +++++------------- source/blender/makesdna/DNA_screen_types.h | 3 + source/blender/windowmanager/intern/wm_draw.c | 5 +- 8 files changed, 125 insertions(+), 130 deletions(-) diff --git a/source/blender/blenkernel/BKE_screen.h b/source/blender/blenkernel/BKE_screen.h index a86953f35cc..08fc785e889 100644 --- a/source/blender/blenkernel/BKE_screen.h +++ b/source/blender/blenkernel/BKE_screen.h @@ -164,6 +164,10 @@ typedef struct ARegionType { void (*init)(struct wmWindowManager *wm, struct ARegion *region); /* exit is called when the region is hidden or removed */ void (*exit)(struct wmWindowManager *wm, struct ARegion *region); + /** Optional callback to decide whether the region should be treated as existing given the + current context. When returning false, the region will be kept in storage, but is not available + to the user in any way. */ + bool (*poll)(const struct bContext *C); /* draw entirely, view changes should be handled here */ void (*draw)(const struct bContext *C, struct ARegion *region); /** diff --git a/source/blender/blenkernel/intern/screen.c b/source/blender/blenkernel/intern/screen.c index 2c896788b20..c03e37cb4b4 100644 --- a/source/blender/blenkernel/intern/screen.c +++ b/source/blender/blenkernel/intern/screen.c @@ -1200,7 +1200,8 @@ static void direct_link_region(BlendDataReader *reader, ARegion *region, int spa BLO_read_list(reader, ®ion->ui_lists); /* The area's search filter is runtime only, so we need to clear the active flag on read. */ - region->flag &= ~RGN_FLAG_SEARCH_FILTER_ACTIVE; + /* Clear runtime flags (e.g. search filter is runtime only). */ + region->flag &= ~(RGN_FLAG_SEARCH_FILTER_ACTIVE | RGN_FLAG_POLL_FAILED); LISTBASE_FOREACH (uiList *, ui_list, ®ion->ui_lists) { ui_list->type = NULL; diff --git a/source/blender/editors/include/ED_screen.h b/source/blender/editors/include/ED_screen.h index 6dfaab663f6..c9d21f6e965 100644 --- a/source/blender/editors/include/ED_screen.h +++ b/source/blender/editors/include/ED_screen.h @@ -292,7 +292,8 @@ void ED_screen_draw_edges(struct wmWindow *win); * for file read and first use, for scaling window, area moves. */ void ED_screen_refresh(struct wmWindowManager *wm, struct wmWindow *win); -void ED_screen_ensure_updated(struct wmWindowManager *wm, +void ED_screen_ensure_updated(struct bContext *C, + struct wmWindowManager *wm, struct wmWindow *win, struct bScreen *screen); void ED_screen_do_listen(struct bContext *C, const struct wmNotifier *note); diff --git a/source/blender/editors/screen/area.c b/source/blender/editors/screen/area.c index 5fc817f3067..d655ac7897d 100644 --- a/source/blender/editors/screen/area.c +++ b/source/blender/editors/screen/area.c @@ -322,7 +322,7 @@ static void region_draw_azones(ScrArea *area, ARegion *region) area_draw_azone(az->x1, az->y1, az->x2, az->y2); } else if (az->type == AZONE_REGION) { - if (az->region) { + if (az->region && !(az->region->flag & RGN_FLAG_POLL_FAILED)) { /* only display tab or icons when the region is hidden */ if (az->region->flag & (RGN_FLAG_HIDDEN | RGN_FLAG_TOO_SMALL)) { region_draw_azone_tab_arrow(area, region, az); @@ -1024,6 +1024,10 @@ static void region_azone_tab_plus(ScrArea *area, AZone *az, ARegion *region) static bool region_azone_edge_poll(const ARegion *region, const bool is_fullscreen) { + if (region->flag & RGN_FLAG_POLL_FAILED) { + return false; + } + const bool is_hidden = (region->flag & (RGN_FLAG_HIDDEN | RGN_FLAG_TOO_SMALL)); if (is_hidden && is_fullscreen) { @@ -1182,7 +1186,7 @@ static void region_overlap_fix(ScrArea *area, ARegion *region) int align1 = 0; const int align = RGN_ALIGN_ENUM_FROM_MASK(region->alignment); for (region_iter = region->prev; region_iter; region_iter = region_iter->prev) { - if (region_iter->flag & RGN_FLAG_HIDDEN) { + if (region_iter->flag & (RGN_FLAG_POLL_FAILED | RGN_FLAG_HIDDEN)) { continue; } @@ -1227,7 +1231,7 @@ static void region_overlap_fix(ScrArea *area, ARegion *region) /* At this point, 'region' is in its final position and still open. * Make a final check it does not overlap any previous 'other side' region. */ for (region_iter = region->prev; region_iter; region_iter = region_iter->prev) { - if (region_iter->flag & RGN_FLAG_HIDDEN) { + if (region_iter->flag & (RGN_FLAG_POLL_FAILED | RGN_FLAG_HIDDEN)) { continue; } if (ELEM(region_iter->alignment, RGN_ALIGN_FLOAT)) { @@ -1345,7 +1349,7 @@ static void region_rect_recursive( prefsizey = UI_DPI_FAC * (region->sizey > 1 ? region->sizey + 0.5f : region->type->prefsizey); } - if (region->flag & RGN_FLAG_HIDDEN) { + if (region->flag & (RGN_FLAG_POLL_FAILED | RGN_FLAG_HIDDEN)) { /* hidden is user flag */ } else if (alignment == RGN_ALIGN_FLOAT) { @@ -1646,7 +1650,8 @@ static void area_calc_totrct(ScrArea *area, const rcti *window_rect) /* used for area initialize below */ static void region_subwindow(ARegion *region) { - bool hidden = (region->flag & (RGN_FLAG_HIDDEN | RGN_FLAG_TOO_SMALL)) != 0; + bool hidden = (region->flag & (RGN_FLAG_POLL_FAILED | RGN_FLAG_HIDDEN | RGN_FLAG_TOO_SMALL)) != + 0; if ((region->alignment & RGN_SPLIT_PREV) && region->prev) { hidden = hidden || (region->prev->flag & (RGN_FLAG_HIDDEN | RGN_FLAG_TOO_SMALL)); @@ -2197,6 +2202,10 @@ static void region_align_info_from_area(ScrArea *area, struct RegionTypeAlignInf } LISTBASE_FOREACH (ARegion *, region, &area->regionbase) { + if (region->flag & RGN_FLAG_POLL_FAILED) { + continue; + } + const int index = region->regiontype; if ((uint)index < RGN_TYPE_NUM) { r_align_info->by_type[index].alignment = RGN_ALIGN_ENUM_FROM_MASK(region->alignment); diff --git a/source/blender/editors/screen/screen_edit.c b/source/blender/editors/screen/screen_edit.c index a62af2b3aef..8f08e302695 100644 --- a/source/blender/editors/screen/screen_edit.c +++ b/source/blender/editors/screen/screen_edit.c @@ -713,8 +713,56 @@ void ED_screens_init(Main *bmain, wmWindowManager *wm) } } -void ED_screen_ensure_updated(wmWindowManager *wm, wmWindow *win, bScreen *screen) +static bool region_poll(const bContext *C, const ARegion *region) { + if (!region->type || !region->type->poll) { + /* Show region by default. */ + return true; + } + + return region->type->poll(C); +} + +static void screen_regions_poll(bContext *C, const wmWindow *win, bScreen *screen) +{ + ScrArea *previous_area = CTX_wm_area(C); + ARegion *previous_region = CTX_wm_region(C); + + bool changed = false; + ED_screen_areas_iter (win, screen, area) { + CTX_wm_area_set(C, area); + + LISTBASE_FOREACH (ARegion *, region, &area->regionbase) { + const int old_region_flag = region->flag; + + region->flag &= ~RGN_FLAG_POLL_FAILED; + + CTX_wm_region_set(C, region); + if (region_poll(C, region) == false) { + region->flag |= RGN_FLAG_POLL_FAILED; + } + + if (old_region_flag != region->flag) { + /* Enforce complete re-init. */ + region->v2d.flag &= ~V2D_IS_INIT; + changed = true; + ED_region_visibility_change_update(C, area, region); + } + } + } + + if (changed) { + screen->do_refresh = true; + // ED_area_tag_redraw(); + } + + CTX_wm_area_set(C, previous_area); + CTX_wm_region_set(C, previous_region); +} + +void ED_screen_ensure_updated(bContext *C, wmWindowManager *wm, wmWindow *win, bScreen *screen) +{ + screen_regions_poll(C, win, screen); if (screen->do_refresh) { ED_screen_refresh(wm, win); } diff --git a/source/blender/editors/space_file/space_file.c b/source/blender/editors/space_file/space_file.c index 95b87f06d96..d45fbf683d2 100644 --- a/source/blender/editors/space_file/space_file.c +++ b/source/blender/editors/space_file/space_file.c @@ -49,60 +49,9 @@ #include "filelist.h" #include "fsmenu.h" -static ARegion *file_ui_region_ensure(ScrArea *area, ARegion *region_prev) -{ - ARegion *region; - - if ((region = BKE_area_find_region_type(area, RGN_TYPE_UI)) != NULL) { - return region; - } - - region = MEM_callocN(sizeof(ARegion), "execute region for file"); - BLI_insertlinkafter(&area->regionbase, region_prev, region); - region->regiontype = RGN_TYPE_UI; - region->alignment = RGN_ALIGN_TOP; - region->flag = RGN_FLAG_DYNAMIC_SIZE; - - return region; -} - -static ARegion *file_execute_region_ensure(ScrArea *area, ARegion *region_prev) -{ - ARegion *region; - - if ((region = BKE_area_find_region_type(area, RGN_TYPE_EXECUTE)) != NULL) { - return region; - } - - region = MEM_callocN(sizeof(ARegion), "execute region for file"); - BLI_insertlinkafter(&area->regionbase, region_prev, region); - region->regiontype = RGN_TYPE_EXECUTE; - region->alignment = RGN_ALIGN_BOTTOM; - region->flag = RGN_FLAG_DYNAMIC_SIZE; - - return region; -} - -static ARegion *file_tool_props_region_ensure(ScrArea *area, ARegion *region_prev) -{ - ARegion *region; - - if ((region = BKE_area_find_region_type(area, RGN_TYPE_TOOL_PROPS)) != NULL) { - return region; - } - - /* add subdiv level; after execute region */ - region = MEM_callocN(sizeof(ARegion), "tool props for file"); - BLI_insertlinkafter(&area->regionbase, region_prev, region); - region->regiontype = RGN_TYPE_TOOL_PROPS; - region->alignment = RGN_ALIGN_RIGHT; - region->flag = RGN_FLAG_HIDDEN; - - return region; -} - /* ******************** default callbacks for file space ***************** */ +/* TODO create regions in versioning? */ static SpaceLink *file_create(const ScrArea *UNUSED(area), const Scene *UNUSED(scene)) { ARegion *region; @@ -131,7 +80,19 @@ static SpaceLink *file_create(const ScrArea *UNUSED(area), const Scene *UNUSED(s region->alignment = RGN_ALIGN_TOP; region->flag |= RGN_FLAG_DYNAMIC_SIZE; - /* Tool props and execute region are added as needed, see file_refresh(). */ + /* execute region */ + region = MEM_callocN(sizeof(ARegion), "execute region for file"); + BLI_addtail(&sfile->regionbase, region); + region->regiontype = RGN_TYPE_EXECUTE; + region->alignment = RGN_ALIGN_BOTTOM; + region->flag = RGN_FLAG_DYNAMIC_SIZE; + + /* tools props region */ + region = MEM_callocN(sizeof(ARegion), "tool props for file"); + BLI_addtail(&sfile->regionbase, region); + region->regiontype = RGN_TYPE_TOOL_PROPS; + region->alignment = RGN_ALIGN_RIGHT; + region->flag = RGN_FLAG_HIDDEN; /* main region */ region = MEM_callocN(sizeof(ARegion), "main region for file"); @@ -184,6 +145,17 @@ static void file_init(wmWindowManager *UNUSED(wm), ScrArea *area) } /* Validate the params right after file read. */ fileselect_refresh_params(sfile); + + ARegion *region_props = BKE_area_find_region_type(area, RGN_TYPE_TOOL_PROPS); + FileSelectParams *params = ED_fileselect_get_active_params(sfile); + if (params && !(region_props->v2d.flag & V2D_IS_INIT)) { + if (params->flag & FILE_HIDE_TOOL_PROPS) { + region_props->flag |= RGN_FLAG_HIDDEN; + } + else { + region_props->flag &= ~RGN_FLAG_HIDDEN; + } + } } static void file_exit(wmWindowManager *wm, ScrArea *area) @@ -231,69 +203,6 @@ static SpaceLink *file_duplicate(SpaceLink *sl) return (SpaceLink *)sfilen; } -static void file_ensure_valid_region_state(bContext *C, - wmWindowManager *wm, - wmWindow *win, - ScrArea *area, - SpaceFile *sfile, - FileSelectParams *params) -{ - ARegion *region_tools = BKE_area_find_region_type(area, RGN_TYPE_TOOLS); - bool needs_init = false; /* To avoid multiple ED_area_init() calls. */ - - BLI_assert(region_tools); - - if (sfile->browse_mode == FILE_BROWSE_MODE_ASSETS) { - file_tool_props_region_ensure(area, region_tools); - - ARegion *region_execute = BKE_area_find_region_type(area, RGN_TYPE_EXECUTE); - if (region_execute) { - ED_region_remove(C, area, region_execute); - needs_init = true; - } - ARegion *region_ui = BKE_area_find_region_type(area, RGN_TYPE_UI); - if (region_ui) { - ED_region_remove(C, area, region_ui); - needs_init = true; - } - } - /* If there's an file-operation, ensure we have the option and execute region */ - else if (sfile->op && !BKE_area_find_region_type(area, RGN_TYPE_TOOL_PROPS)) { - ARegion *region_ui = file_ui_region_ensure(area, region_tools); - ARegion *region_execute = file_execute_region_ensure(area, region_ui); - ARegion *region_props = file_tool_props_region_ensure(area, region_execute); - - if (params->flag & FILE_HIDE_TOOL_PROPS) { - region_props->flag |= RGN_FLAG_HIDDEN; - } - else { - region_props->flag &= ~RGN_FLAG_HIDDEN; - } - - needs_init = true; - } - /* If there's _no_ file-operation, ensure we _don't_ have the option and execute region */ - else if (!sfile->op) { - ARegion *region_props = BKE_area_find_region_type(area, RGN_TYPE_TOOL_PROPS); - ARegion *region_execute = BKE_area_find_region_type(area, RGN_TYPE_EXECUTE); - ARegion *region_ui = file_ui_region_ensure(area, region_tools); - UNUSED_VARS(region_ui); - - if (region_execute) { - ED_region_remove(C, area, region_execute); - needs_init = true; - } - if (region_props) { - ED_region_remove(C, area, region_props); - needs_init = true; - } - } - - if (needs_init) { - ED_area_init(wm, win, area); - } -} - static void file_refresh(const bContext *C, ScrArea *area) { wmWindowManager *wm = CTX_wm_manager(C); @@ -389,11 +298,6 @@ static void file_refresh(const bContext *C, ScrArea *area) sfile->layout->dirty = true; } - /* Might be called with NULL area, see file_main_region_draw() below. */ - if (area) { - file_ensure_valid_region_state((bContext *)C, wm, win, area, sfile, params); - } - ED_area_tag_redraw(area); } @@ -714,6 +618,25 @@ static void file_keymap(struct wmKeyConfig *keyconf) WM_keymap_ensure(keyconf, "File Browser Buttons", SPACE_FILE, 0); } +static bool file_ui_region_poll(const bContext *C) +{ + const SpaceFile *sfile = CTX_wm_space_file(C); + /* Always visible except when browsing assets. */ + return sfile->browse_mode != FILE_BROWSE_MODE_ASSETS; +} + +static bool file_tool_props_region_poll(const bContext *C) +{ + const SpaceFile *sfile = CTX_wm_space_file(C); + return (sfile->browse_mode == FILE_BROWSE_MODE_ASSETS) || (sfile->op != NULL); +} + +static bool file_execution_region_poll(const bContext *C) +{ + const SpaceFile *sfile = CTX_wm_space_file(C); + return sfile->op != NULL; +} + static void file_tools_region_init(wmWindowManager *wm, ARegion *region) { wmKeyMap *keymap; @@ -1085,6 +1008,7 @@ void ED_spacetype_file(void) art = MEM_callocN(sizeof(ARegionType), "spacetype file region"); art->regionid = RGN_TYPE_UI; art->keymapflag = ED_KEYMAP_UI; + art->poll = file_ui_region_poll; art->listener = file_ui_region_listener; art->init = file_ui_region_init; art->draw = file_ui_region_draw; @@ -1094,6 +1018,7 @@ void ED_spacetype_file(void) art = MEM_callocN(sizeof(ARegionType), "spacetype file region"); art->regionid = RGN_TYPE_EXECUTE; art->keymapflag = ED_KEYMAP_UI; + art->poll = file_execution_region_poll; art->listener = file_ui_region_listener; art->init = file_execution_region_init; art->draw = file_execution_region_draw; @@ -1118,6 +1043,7 @@ void ED_spacetype_file(void) art->prefsizex = 240; art->prefsizey = 60; art->keymapflag = ED_KEYMAP_UI; + art->poll = file_tool_props_region_poll; art->listener = file_tool_props_region_listener; art->init = file_tools_region_init; art->draw = file_tools_region_draw; diff --git a/source/blender/makesdna/DNA_screen_types.h b/source/blender/makesdna/DNA_screen_types.h index 4feab97b6b7..9b9346949e7 100644 --- a/source/blender/makesdna/DNA_screen_types.h +++ b/source/blender/makesdna/DNA_screen_types.h @@ -722,6 +722,9 @@ enum { * region's layout pass. so that expansion is still interactive, */ RGN_FLAG_SEARCH_FILTER_UPDATE = (1 << 9), + /** #ARegionType.poll() failed for the current context, and the region should be treated as if it + * wouldn't exist. Runtime only flag. */ + RGN_FLAG_POLL_FAILED = (1 << 10), }; /** #ARegion.do_draw */ diff --git a/source/blender/windowmanager/intern/wm_draw.c b/source/blender/windowmanager/intern/wm_draw.c index 15826af32fa..6a5166e49ca 100644 --- a/source/blender/windowmanager/intern/wm_draw.c +++ b/source/blender/windowmanager/intern/wm_draw.c @@ -894,6 +894,9 @@ static void wm_draw_window_offscreen(bContext *C, wmWindow *win, bool stereo) /* Compute UI layouts for dynamically size regions. */ LISTBASE_FOREACH (ARegion *, region, &area->regionbase) { + if (region->flag & RGN_FLAG_POLL_FAILED) { + continue; + } /* Dynamic region may have been flagged as too small because their size on init is 0. * ARegion.visible is false then, as expected. The layout should still be created then, so * the region size can be updated (it may turn out to be not too small then). */ @@ -1379,7 +1382,7 @@ void wm_draw_update(bContext *C) wm_window_make_drawable(wm, win); /* notifiers for screen redraw */ - ED_screen_ensure_updated(wm, win, screen); + ED_screen_ensure_updated(C, wm, win, screen); wm_draw_window(C, win); wm_draw_update_clear_window(C, win); -- 2.30.2 From fa817e0c717b29467fb548a25663f286dde0d3a5 Mon Sep 17 00:00:00 2001 From: Julian Eisel Date: Wed, 22 Feb 2023 18:29:06 +0100 Subject: [PATCH 14/25] Remove asset-shelf region code This is out of scope of this branch, there's the dedicated `asset-shelf` branch. --- .../blenloader/intern/versioning_300.cc | 19 ------------------- source/blender/editors/screen/area.cc | 3 +-- source/blender/editors/screen/screen_edit.c | 3 +-- source/blender/editors/space_file/filelist.cc | 1 - .../editors/space_view3d/space_view3d.cc | 17 ----------------- source/blender/makesdna/DNA_screen_types.h | 3 +-- source/blender/makesrna/intern/rna_screen.c | 1 - 7 files changed, 3 insertions(+), 44 deletions(-) diff --git a/source/blender/blenloader/intern/versioning_300.cc b/source/blender/blenloader/intern/versioning_300.cc index 846f88499eb..59e4be70989 100644 --- a/source/blender/blenloader/intern/versioning_300.cc +++ b/source/blender/blenloader/intern/versioning_300.cc @@ -4014,24 +4014,5 @@ void blo_do_versions_300(FileData *fd, Library * /*lib*/, Main *bmain) */ { /* Keep this block, even when empty. */ - LISTBASE_FOREACH (bScreen *, screen, &bmain->screens) { - LISTBASE_FOREACH (ScrArea *, area, &screen->areabase) { - LISTBASE_FOREACH (SpaceLink *, sl, &area->spacedata) { - if (sl->spacetype == SPACE_VIEW3D) { - ListBase *regionbase = (sl == area->spacedata.first) ? &area->regionbase : - &sl->regionbase; - ARegion *new_asset_shelf = do_versions_add_region_if_not_found( - regionbase, - RGN_TYPE_ASSET_SHELF, - "asset shelf for view3d (versioning)", - RGN_TYPE_UI); - if (new_asset_shelf != nullptr) { - new_asset_shelf->alignment = RGN_ALIGN_BOTTOM; - new_asset_shelf->flag |= RGN_FLAG_HIDDEN; - } - } - } - } - } } } diff --git a/source/blender/editors/screen/area.cc b/source/blender/editors/screen/area.cc index 24e7f586353..9ac402ec96a 100644 --- a/source/blender/editors/screen/area.cc +++ b/source/blender/editors/screen/area.cc @@ -1247,8 +1247,7 @@ bool ED_region_is_overlap(int spacetype, int regiontype) RGN_TYPE_UI, RGN_TYPE_TOOL_PROPS, RGN_TYPE_FOOTER, - RGN_TYPE_TOOL_HEADER, - RGN_TYPE_ASSET_SHELF)) { + RGN_TYPE_TOOL_HEADER)) { return true; } } diff --git a/source/blender/editors/screen/screen_edit.c b/source/blender/editors/screen/screen_edit.c index eaa1e0cb5d3..6ebd7e96959 100644 --- a/source/blender/editors/screen/screen_edit.c +++ b/source/blender/editors/screen/screen_edit.c @@ -1483,8 +1483,7 @@ static bScreen *screen_state_to_nonnormal(bContext *C, RGN_TYPE_FOOTER, RGN_TYPE_TOOLS, RGN_TYPE_NAV_BAR, - RGN_TYPE_EXECUTE, - RGN_TYPE_ASSET_SHELF)) { + RGN_TYPE_EXECUTE)) { region->flag |= RGN_FLAG_HIDDEN; } } diff --git a/source/blender/editors/space_file/filelist.cc b/source/blender/editors/space_file/filelist.cc index d76224609e4..caea344bc97 100644 --- a/source/blender/editors/space_file/filelist.cc +++ b/source/blender/editors/space_file/filelist.cc @@ -864,7 +864,6 @@ static bool is_filtered_lib_type(FileListInternEntry *file, if (file->typeflag & FILE_TYPE_BLENDERLIB) { return is_filtered_id_file_type(file, file->blentype, file->name, filter); } - return is_filtered_file_type(file, filter); } diff --git a/source/blender/editors/space_view3d/space_view3d.cc b/source/blender/editors/space_view3d/space_view3d.cc index c967ecaacfd..25708ee7a67 100644 --- a/source/blender/editors/space_view3d/space_view3d.cc +++ b/source/blender/editors/space_view3d/space_view3d.cc @@ -290,14 +290,6 @@ static SpaceLink *view3d_create(const ScrArea * /*area*/, const Scene *scene) region->alignment = RGN_ALIGN_RIGHT; region->flag = RGN_FLAG_HIDDEN; - /* asset shelf */ - region = MEM_cnew("asset shelf for view3d"); - - BLI_addtail(&v3d->regionbase, region); - region->regiontype = RGN_TYPE_ASSET_SHELF; - region->alignment = RGN_ALIGN_BOTTOM; - region->flag = RGN_FLAG_HIDDEN; - /* main region */ region = MEM_cnew("main region for view3d"); @@ -2202,15 +2194,6 @@ void ED_spacetype_view3d() art->draw = view3d_header_region_draw; BLI_addhead(&st->regiontypes, art); - /* regions: asset shelf */ - art = MEM_cnew("spacetype view3d asset shelf region"); - art->regionid = RGN_TYPE_ASSET_SHELF; - art->prefsizey = HEADERY * 4; - art->keymapflag = ED_KEYMAP_UI | ED_KEYMAP_VIEW2D | ED_KEYMAP_FRAMES | ED_KEYMAP_HEADER; - art->init = view3d_header_region_init; - art->draw = view3d_header_region_draw; - BLI_addhead(&st->regiontypes, art); - /* regions: hud */ art = ED_area_type_hud(st->spaceid); BLI_addhead(&st->regiontypes, art); diff --git a/source/blender/makesdna/DNA_screen_types.h b/source/blender/makesdna/DNA_screen_types.h index d90dfc9d3f5..da939ec483c 100644 --- a/source/blender/makesdna/DNA_screen_types.h +++ b/source/blender/makesdna/DNA_screen_types.h @@ -657,9 +657,8 @@ typedef enum eRegion_Type { /* Region type used exclusively by internal code and add-ons to register draw callbacks to the XR * context (surface, mirror view). Does not represent any real region. */ RGN_TYPE_XR = 13, - RGN_TYPE_ASSET_SHELF = 14, -#define RGN_TYPE_NUM (RGN_TYPE_ASSET_SHELF + 1) +#define RGN_TYPE_NUM (RGN_TYPE_XR + 1) } eRegion_Type; /* use for function args */ diff --git a/source/blender/makesrna/intern/rna_screen.c b/source/blender/makesrna/intern/rna_screen.c index b03f42f34a3..4d837a374d1 100644 --- a/source/blender/makesrna/intern/rna_screen.c +++ b/source/blender/makesrna/intern/rna_screen.c @@ -26,7 +26,6 @@ const EnumPropertyItem rna_enum_region_type_items[] = { {RGN_TYPE_UI, "UI", 0, "UI", ""}, {RGN_TYPE_TOOLS, "TOOLS", 0, "Tools", ""}, {RGN_TYPE_TOOL_PROPS, "TOOL_PROPS", 0, "Tool Properties", ""}, - {RGN_TYPE_ASSET_SHELF, "ASSET_SHELF", 0, "Asset Shelf", ""}, {RGN_TYPE_PREVIEW, "PREVIEW", 0, "Preview", ""}, {RGN_TYPE_HUD, "HUD", 0, "Floating Region", ""}, {RGN_TYPE_NAV_BAR, "NAVIGATION_BAR", 0, "Navigation Bar", ""}, -- 2.30.2 From 96a4f5b8b12c0516b1953a7884497828133b68dd Mon Sep 17 00:00:00 2001 From: Julian Eisel Date: Thu, 23 Feb 2023 15:06:08 +0100 Subject: [PATCH 15/25] Fix properties region hiding when toggling file and asset browser --- source/blender/blenkernel/BKE_screen.h | 4 +- source/blender/editors/screen/screen_edit.c | 8 ++-- .../blender/editors/space_file/space_file.c | 37 +++++++++++++------ 3 files changed, 32 insertions(+), 17 deletions(-) diff --git a/source/blender/blenkernel/BKE_screen.h b/source/blender/blenkernel/BKE_screen.h index 08fc785e889..892447c1c0d 100644 --- a/source/blender/blenkernel/BKE_screen.h +++ b/source/blender/blenkernel/BKE_screen.h @@ -165,8 +165,8 @@ typedef struct ARegionType { /* exit is called when the region is hidden or removed */ void (*exit)(struct wmWindowManager *wm, struct ARegion *region); /** Optional callback to decide whether the region should be treated as existing given the - current context. When returning false, the region will be kept in storage, but is not available - to the user in any way. */ + * current context. When returning false, the region will be kept in storage, but is not + * available to the user in any way. */ bool (*poll)(const struct bContext *C); /* draw entirely, view changes should be handled here */ void (*draw)(const struct bContext *C, struct ARegion *region); diff --git a/source/blender/editors/screen/screen_edit.c b/source/blender/editors/screen/screen_edit.c index 6ebd7e96959..ca87543b712 100644 --- a/source/blender/editors/screen/screen_edit.c +++ b/source/blender/editors/screen/screen_edit.c @@ -728,7 +728,7 @@ static void screen_regions_poll(bContext *C, const wmWindow *win, bScreen *scree ScrArea *previous_area = CTX_wm_area(C); ARegion *previous_region = CTX_wm_region(C); - bool changed = false; + bool any_changed = false; ED_screen_areas_iter (win, screen, area) { CTX_wm_area_set(C, area); @@ -743,17 +743,17 @@ static void screen_regions_poll(bContext *C, const wmWindow *win, bScreen *scree } if (old_region_flag != region->flag) { + any_changed = true; + /* Enforce complete re-init. */ region->v2d.flag &= ~V2D_IS_INIT; - changed = true; ED_region_visibility_change_update(C, area, region); } } } - if (changed) { + if (any_changed) { screen->do_refresh = true; - // ED_area_tag_redraw(); } CTX_wm_area_set(C, previous_area); diff --git a/source/blender/editors/space_file/space_file.c b/source/blender/editors/space_file/space_file.c index d45fbf683d2..15af68d616d 100644 --- a/source/blender/editors/space_file/space_file.c +++ b/source/blender/editors/space_file/space_file.c @@ -145,17 +145,6 @@ static void file_init(wmWindowManager *UNUSED(wm), ScrArea *area) } /* Validate the params right after file read. */ fileselect_refresh_params(sfile); - - ARegion *region_props = BKE_area_find_region_type(area, RGN_TYPE_TOOL_PROPS); - FileSelectParams *params = ED_fileselect_get_active_params(sfile); - if (params && !(region_props->v2d.flag & V2D_IS_INIT)) { - if (params->flag & FILE_HIDE_TOOL_PROPS) { - region_props->flag |= RGN_FLAG_HIDDEN; - } - else { - region_props->flag &= ~RGN_FLAG_HIDDEN; - } - } } static void file_exit(wmWindowManager *wm, ScrArea *area) @@ -298,6 +287,28 @@ static void file_refresh(const bContext *C, ScrArea *area) sfile->layout->dirty = true; } + if (area) { + ARegion *region_props = BKE_area_find_region_type(area, RGN_TYPE_TOOL_PROPS); + const bool region_flag_old = region_props->flag; + if (!(region_props->v2d.flag & V2D_IS_INIT)) { + if (ED_fileselect_is_asset_browser(sfile)) { + /* Hide by default in asset browser. */ + region_props->flag |= RGN_FLAG_HIDDEN; + } + else { + if (params->flag & FILE_HIDE_TOOL_PROPS) { + region_props->flag |= RGN_FLAG_HIDDEN; + } + else { + region_props->flag &= ~RGN_FLAG_HIDDEN; + } + } + } + if (region_flag_old != region_props->flag) { + ED_region_visibility_change_update((bContext *)C, area, region_props); + } + } + ED_area_tag_redraw(area); } @@ -792,6 +803,10 @@ static int file_space_subtype_get(ScrArea *area) static void file_space_subtype_set(ScrArea *area, int value) { SpaceFile *sfile = area->spacedata.first; + /* Force re-init. */ + LISTBASE_FOREACH (ARegion *, region, &area->regionbase) { + region->v2d.flag &= ~V2D_IS_INIT; + } sfile->browse_mode = value; } -- 2.30.2 From 3fde89c2a88c7d3e17db3d38a2cb20a50d0699f7 Mon Sep 17 00:00:00 2001 From: Julian Eisel Date: Thu, 23 Feb 2023 15:49:58 +0100 Subject: [PATCH 16/25] Add versioning for file browsers in existing workspaces --- .../blenloader/intern/versioning_300.cc | 40 +++++++++++++++++++ .../blender/editors/space_file/space_file.c | 2 +- 2 files changed, 41 insertions(+), 1 deletion(-) diff --git a/source/blender/blenloader/intern/versioning_300.cc b/source/blender/blenloader/intern/versioning_300.cc index 59e4be70989..768e984122d 100644 --- a/source/blender/blenloader/intern/versioning_300.cc +++ b/source/blender/blenloader/intern/versioning_300.cc @@ -4014,5 +4014,45 @@ void blo_do_versions_300(FileData *fd, Library * /*lib*/, Main *bmain) */ { /* Keep this block, even when empty. */ + + /* Some regions used to be added/removed dynamically. Ensure they are always there, there is a + * `ARegionType.poll()` now. */ + LISTBASE_FOREACH (bScreen *, screen, &bmain->screens) { + LISTBASE_FOREACH (ScrArea *, area, &screen->areabase) { + LISTBASE_FOREACH (SpaceLink *, sl, &area->spacedata) { + if (sl->spacetype != SPACE_FILE) { + continue; + } + ListBase *regionbase = (sl == area->spacedata.first) ? &area->regionbase : + &sl->regionbase; + + ARegion *new_region; + if ((new_region = do_versions_add_region_if_not_found( + regionbase, + RGN_TYPE_UI, + "versioning: UI region for space file", + RGN_TYPE_TOOLS))) { + new_region->alignment = RGN_ALIGN_TOP; + new_region->flag |= RGN_FLAG_DYNAMIC_SIZE; + } + if ((new_region = do_versions_add_region_if_not_found( + regionbase, + RGN_TYPE_EXECUTE, + "versioning: execute region for space file", + RGN_TYPE_UI))) { + new_region->alignment = RGN_ALIGN_BOTTOM; + new_region->flag = RGN_FLAG_DYNAMIC_SIZE; + } + if ((new_region = do_versions_add_region_if_not_found( + regionbase, + RGN_TYPE_TOOL_PROPS, + "versioning: tool props region for space file", + RGN_TYPE_EXECUTE))) { + new_region->alignment = RGN_ALIGN_RIGHT; + new_region->flag = RGN_FLAG_HIDDEN; + } + } + } + } } } diff --git a/source/blender/editors/space_file/space_file.c b/source/blender/editors/space_file/space_file.c index 15af68d616d..51d1876dcf7 100644 --- a/source/blender/editors/space_file/space_file.c +++ b/source/blender/editors/space_file/space_file.c @@ -78,7 +78,7 @@ static SpaceLink *file_create(const ScrArea *UNUSED(area), const Scene *UNUSED(s BLI_addtail(&sfile->regionbase, region); region->regiontype = RGN_TYPE_UI; region->alignment = RGN_ALIGN_TOP; - region->flag |= RGN_FLAG_DYNAMIC_SIZE; + region->flag = RGN_FLAG_DYNAMIC_SIZE; /* execute region */ region = MEM_callocN(sizeof(ARegion), "execute region for file"); -- 2.30.2 From 54af101e6e75ab635a0181fc75914bd40eb4a3de Mon Sep 17 00:00:00 2001 From: Julian Eisel Date: Thu, 23 Feb 2023 16:09:02 +0100 Subject: [PATCH 17/25] Remove event handlers for region with failing poll --- source/blender/editors/screen/area.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source/blender/editors/screen/area.cc b/source/blender/editors/screen/area.cc index 9ac402ec96a..72aba3e0039 100644 --- a/source/blender/editors/screen/area.cc +++ b/source/blender/editors/screen/area.cc @@ -2076,7 +2076,7 @@ void ED_region_cursor_set(wmWindow *win, ScrArea *area, ARegion *region) void ED_region_visibility_change_update(bContext *C, ScrArea *area, ARegion *region) { - if (region->flag & RGN_FLAG_HIDDEN) { + if (region->flag & (RGN_FLAG_HIDDEN | RGN_FLAG_POLL_FAILED)) { WM_event_remove_handlers(C, ®ion->handlers); /* Needed to close any open pop-overs which would otherwise remain open, * crashing on attempting to refresh. See: #93410. -- 2.30.2 From 2bbe699bc162049e286e8a9f32336de19eb39408 Mon Sep 17 00:00:00 2001 From: Julian Eisel Date: Thu, 23 Feb 2023 19:18:52 +0100 Subject: [PATCH 18/25] Use region polling in clip editor Makes the region management code quite a bit simpler for the clip editor. A small trade-off is more versioning code. --- source/blender/blenkernel/BKE_screen.h | 3 +- .../blenloader/intern/versioning_300.cc | 92 +++++--- .../blenloader/intern/versioning_common.cc | 25 +- .../blenloader/intern/versioning_common.h | 16 +- .../blender/editors/space_clip/CMakeLists.txt | 1 - .../blender/editors/space_clip/clip_intern.h | 4 - .../blender/editors/space_clip/clip_toolbar.c | 54 ----- .../blender/editors/space_clip/space_clip.c | 218 +++++------------- .../editors/space_sequencer/space_sequencer.c | 71 +++--- 9 files changed, 183 insertions(+), 301 deletions(-) delete mode 100644 source/blender/editors/space_clip/clip_toolbar.c diff --git a/source/blender/blenkernel/BKE_screen.h b/source/blender/blenkernel/BKE_screen.h index 892447c1c0d..e61bbb26533 100644 --- a/source/blender/blenkernel/BKE_screen.h +++ b/source/blender/blenkernel/BKE_screen.h @@ -166,7 +166,8 @@ typedef struct ARegionType { void (*exit)(struct wmWindowManager *wm, struct ARegion *region); /** Optional callback to decide whether the region should be treated as existing given the * current context. When returning false, the region will be kept in storage, but is not - * available to the user in any way. */ + * available to the user in any way. Callbacks can assume that context has the owning area and + * space-data set. */ bool (*poll)(const struct bContext *C); /* draw entirely, view changes should be handled here */ void (*draw)(const struct bContext *C, struct ARegion *region); diff --git a/source/blender/blenloader/intern/versioning_300.cc b/source/blender/blenloader/intern/versioning_300.cc index 768e984122d..443309af65d 100644 --- a/source/blender/blenloader/intern/versioning_300.cc +++ b/source/blender/blenloader/intern/versioning_300.cc @@ -2078,6 +2078,66 @@ static void version_fix_image_format_copy(Main *bmain, ImageFormatData *format) } } +/** + * Some editors would manually manage visibility of regions, or lazy create them based on + * context. Ensure they are always there now, and use the new #ARegionType.poll(). + */ +static void version_ensure_missing_regions(ScrArea *area, SpaceLink *sl) +{ + ListBase *regionbase = (sl == area->spacedata.first) ? &area->regionbase : &sl->regionbase; + + switch (sl->spacetype) { + case SPACE_FILE: { + ARegion *new_region; + + new_region = do_versions_add_region_if_not_found( + regionbase, RGN_TYPE_UI, "versioning: UI region for file", RGN_TYPE_TOOLS); + if (new_region) { + new_region->alignment = RGN_ALIGN_TOP; + new_region->flag |= RGN_FLAG_DYNAMIC_SIZE; + } + + new_region = do_versions_add_region_if_not_found( + regionbase, RGN_TYPE_EXECUTE, "versioning: execute region for file", RGN_TYPE_UI); + if (new_region) { + new_region->alignment = RGN_ALIGN_BOTTOM; + new_region->flag = RGN_FLAG_DYNAMIC_SIZE; + } + + new_region = do_versions_add_region_if_not_found(regionbase, + RGN_TYPE_TOOL_PROPS, + "versioning: tool props region for file", + RGN_TYPE_EXECUTE); + if (new_region) { + new_region->alignment = RGN_ALIGN_RIGHT; + new_region->flag = RGN_FLAG_HIDDEN; + } + break; + } + case SPACE_CLIP: { + ARegion *region; + + region = do_versions_ensure_region( + regionbase, RGN_TYPE_UI, "versioning: properties region for clip", RGN_TYPE_HEADER); + region->alignment = RGN_ALIGN_RIGHT; + region->flag &= ~RGN_FLAG_HIDDEN; + + region = do_versions_ensure_region( + regionbase, RGN_TYPE_CHANNELS, "versioning: channels region for clip", RGN_TYPE_UI); + region->alignment = RGN_ALIGN_LEFT; + region->flag &= ~RGN_FLAG_HIDDEN; + region->v2d.scroll = V2D_SCROLL_BOTTOM; + region->v2d.flag = V2D_VIEWSYNC_AREA_VERTICAL; + + region = do_versions_ensure_region( + regionbase, RGN_TYPE_PREVIEW, "versioning: preview region for clip", RGN_TYPE_WINDOW); + region->flag &= ~RGN_FLAG_HIDDEN; + + break; + } + } +} + /* NOLINTNEXTLINE: readability-function-size */ void blo_do_versions_300(FileData *fd, Library * /*lib*/, Main *bmain) { @@ -4020,37 +4080,7 @@ void blo_do_versions_300(FileData *fd, Library * /*lib*/, Main *bmain) LISTBASE_FOREACH (bScreen *, screen, &bmain->screens) { LISTBASE_FOREACH (ScrArea *, area, &screen->areabase) { LISTBASE_FOREACH (SpaceLink *, sl, &area->spacedata) { - if (sl->spacetype != SPACE_FILE) { - continue; - } - ListBase *regionbase = (sl == area->spacedata.first) ? &area->regionbase : - &sl->regionbase; - - ARegion *new_region; - if ((new_region = do_versions_add_region_if_not_found( - regionbase, - RGN_TYPE_UI, - "versioning: UI region for space file", - RGN_TYPE_TOOLS))) { - new_region->alignment = RGN_ALIGN_TOP; - new_region->flag |= RGN_FLAG_DYNAMIC_SIZE; - } - if ((new_region = do_versions_add_region_if_not_found( - regionbase, - RGN_TYPE_EXECUTE, - "versioning: execute region for space file", - RGN_TYPE_UI))) { - new_region->alignment = RGN_ALIGN_BOTTOM; - new_region->flag = RGN_FLAG_DYNAMIC_SIZE; - } - if ((new_region = do_versions_add_region_if_not_found( - regionbase, - RGN_TYPE_TOOL_PROPS, - "versioning: tool props region for space file", - RGN_TYPE_EXECUTE))) { - new_region->alignment = RGN_ALIGN_RIGHT; - new_region->flag = RGN_FLAG_HIDDEN; - } + version_ensure_missing_regions(area, sl); } } } diff --git a/source/blender/blenloader/intern/versioning_common.cc b/source/blender/blenloader/intern/versioning_common.cc index 80df0a2ba74..d270322d52e 100644 --- a/source/blender/blenloader/intern/versioning_common.cc +++ b/source/blender/blenloader/intern/versioning_common.cc @@ -32,7 +32,7 @@ using blender::StringRef; ARegion *do_versions_add_region_if_not_found(ListBase *regionbase, int region_type, - const char *name, + const char *allocname, int link_after_region_type) { ARegion *link_after_region = nullptr; @@ -45,7 +45,28 @@ ARegion *do_versions_add_region_if_not_found(ListBase *regionbase, } } - ARegion *new_region = static_cast(MEM_callocN(sizeof(ARegion), name)); + ARegion *new_region = static_cast(MEM_callocN(sizeof(ARegion), allocname)); + new_region->regiontype = region_type; + BLI_insertlinkafter(regionbase, link_after_region, new_region); + return new_region; +} + +ARegion *do_versions_ensure_region(ListBase *regionbase, + int region_type, + const char *allocname, + int link_after_region_type) +{ + ARegion *link_after_region = nullptr; + LISTBASE_FOREACH (ARegion *, region, regionbase) { + if (region->regiontype == region_type) { + return region; + } + if (region->regiontype == link_after_region_type) { + link_after_region = region; + } + } + + ARegion *new_region = static_cast(MEM_callocN(sizeof(ARegion), allocname)); new_region->regiontype = region_type; BLI_insertlinkafter(regionbase, link_after_region, new_region); return new_region; diff --git a/source/blender/blenloader/intern/versioning_common.h b/source/blender/blenloader/intern/versioning_common.h index 40f383a27b2..8d17e9db39f 100644 --- a/source/blender/blenloader/intern/versioning_common.h +++ b/source/blender/blenloader/intern/versioning_common.h @@ -19,10 +19,24 @@ struct bNodeTree; extern "C" { #endif +/** + * Check if a region of type \a region_type exists in \a regionbase. Otherwise add it after the + * first region of type \a link_after_region_type. + * \returns null if a region of the given type already existed, otherwise the newly added region. + */ struct ARegion *do_versions_add_region_if_not_found(struct ListBase *regionbase, int region_type, - const char *name, + const char *allocname, int link_after_region_type); +/** + * Check if a region of type \a region_type exists in \a regionbase. Otherwise add it after the + * first region of type \a link_after_region_type. + * \returns either a new, or already existing region. + */ +ARegion *do_versions_ensure_region(ListBase *regionbase, + int region_type, + const char *allocname, + int link_after_region_type); /** * Rename if the ID doesn't exist. diff --git a/source/blender/editors/space_clip/CMakeLists.txt b/source/blender/editors/space_clip/CMakeLists.txt index 15f1e130a64..d9d0a8e7420 100644 --- a/source/blender/editors/space_clip/CMakeLists.txt +++ b/source/blender/editors/space_clip/CMakeLists.txt @@ -34,7 +34,6 @@ set(SRC clip_graph_draw.c clip_graph_ops.c clip_ops.c - clip_toolbar.c clip_utils.c space_clip.c tracking_ops.c diff --git a/source/blender/editors/space_clip/clip_intern.h b/source/blender/editors/space_clip/clip_intern.h index ebdb56f2a12..d94b20df3b5 100644 --- a/source/blender/editors/space_clip/clip_intern.h +++ b/source/blender/editors/space_clip/clip_intern.h @@ -114,10 +114,6 @@ void CLIP_OT_cursor_set(struct wmOperatorType *ot); void CLIP_OT_lock_selection_toggle(struct wmOperatorType *ot); -/* clip_toolbar.c */ - -struct ARegion *ED_clip_has_properties_region(struct ScrArea *area); - /* clip_utils.c */ typedef enum { diff --git a/source/blender/editors/space_clip/clip_toolbar.c b/source/blender/editors/space_clip/clip_toolbar.c deleted file mode 100644 index 017a73503a3..00000000000 --- a/source/blender/editors/space_clip/clip_toolbar.c +++ /dev/null @@ -1,54 +0,0 @@ -/* SPDX-License-Identifier: GPL-2.0-or-later - * Copyright 2011 Blender Foundation. All rights reserved. */ - -/** \file - * \ingroup spclip - */ - -#include - -#include "MEM_guardedalloc.h" - -#include "BLI_blenlib.h" -#include "BLI_utildefines.h" - -#include "BKE_context.h" -#include "BKE_screen.h" - -#include "WM_types.h" - -#include "ED_screen.h" - -#include "clip_intern.h" /* own include */ - -/* ************************ header area region *********************** */ - -/************************** properties ******************************/ - -ARegion *ED_clip_has_properties_region(ScrArea *area) -{ - ARegion *region, *arnew; - - region = BKE_area_find_region_type(area, RGN_TYPE_UI); - if (region) { - return region; - } - - /* add subdiv level; after header */ - region = BKE_area_find_region_type(area, RGN_TYPE_HEADER); - - /* is error! */ - if (region == NULL) { - return NULL; - } - - arnew = MEM_callocN(sizeof(ARegion), "clip properties region"); - - BLI_insertlinkafter(&area->regionbase, region, arnew); - arnew->regiontype = RGN_TYPE_UI; - arnew->alignment = RGN_ALIGN_RIGHT; - - arnew->flag = RGN_FLAG_HIDDEN; - - return arnew; -} diff --git a/source/blender/editors/space_clip/space_clip.c b/source/blender/editors/space_clip/space_clip.c index 3a2cbfdfad3..40eb7dc4456 100644 --- a/source/blender/editors/space_clip/space_clip.c +++ b/source/blender/editors/space_clip/space_clip.c @@ -60,10 +60,6 @@ static void init_preview_region(const Scene *scene, const SpaceClip *sc, ARegion *region) { - region->regiontype = RGN_TYPE_PREVIEW; - region->alignment = RGN_ALIGN_TOP; - region->flag |= RGN_FLAG_HIDDEN; - if (sc->view == SC_VIEW_DOPESHEET) { region->v2d.tot.xmin = -10.0f; region->v2d.tot.ymin = (float)(-area->winy) / 3.0f; @@ -115,78 +111,6 @@ static void init_preview_region(const Scene *scene, } } -static void reinit_preview_region(const bContext *C, ARegion *region) -{ - Scene *scene = CTX_data_scene(C); - ScrArea *area = CTX_wm_area(C); - SpaceClip *sc = CTX_wm_space_clip(C); - - if (sc->view == SC_VIEW_DOPESHEET) { - if ((region->v2d.flag & V2D_VIEWSYNC_AREA_VERTICAL) == 0) { - init_preview_region(scene, area, sc, region); - } - } - else { - if (region->v2d.flag & V2D_VIEWSYNC_AREA_VERTICAL) { - init_preview_region(scene, area, sc, region); - } - } -} - -static ARegion *ED_clip_has_preview_region(const bContext *C, ScrArea *area) -{ - ARegion *region, *arnew; - - region = BKE_area_find_region_type(area, RGN_TYPE_PREVIEW); - if (region) { - return region; - } - - /* add subdiv level; after header */ - region = BKE_area_find_region_type(area, RGN_TYPE_WINDOW); - - /* is error! */ - if (region == NULL) { - return NULL; - } - - arnew = MEM_callocN(sizeof(ARegion), "clip preview region"); - - BLI_insertlinkbefore(&area->regionbase, region, arnew); - init_preview_region(CTX_data_scene(C), area, CTX_wm_space_clip(C), arnew); - - return arnew; -} - -static ARegion *ED_clip_has_channels_region(ScrArea *area) -{ - ARegion *region, *arnew; - - region = BKE_area_find_region_type(area, RGN_TYPE_CHANNELS); - if (region) { - return region; - } - - /* add subdiv level; after header */ - region = BKE_area_find_region_type(area, RGN_TYPE_PREVIEW); - - /* is error! */ - if (region == NULL) { - return NULL; - } - - arnew = MEM_callocN(sizeof(ARegion), "clip channels region"); - - BLI_insertlinkbefore(&area->regionbase, region, arnew); - arnew->regiontype = RGN_TYPE_CHANNELS; - arnew->alignment = RGN_ALIGN_LEFT; - - arnew->v2d.scroll = V2D_SCROLL_BOTTOM; - arnew->v2d.flag = V2D_VIEWSYNC_AREA_VERTICAL; - - return arnew; -} - static void clip_scopes_tag_refresh(ScrArea *area) { SpaceClip *sc = (SpaceClip *)area->spacedata.first; @@ -223,7 +147,7 @@ static void clip_area_sync_frame_from_scene(ScrArea *area, const Scene *scene) /* ******************** default callbacks for clip space ***************** */ -static SpaceLink *clip_create(const ScrArea *area, const Scene *scene) +static SpaceLink *clip_create(const ScrArea *UNUSED(area), const Scene *UNUSED(scene)) { ARegion *region; SpaceClip *sc; @@ -265,7 +189,7 @@ static SpaceLink *clip_create(const ScrArea *area, const Scene *scene) region = MEM_callocN(sizeof(ARegion), "preview for clip"); BLI_addtail(&sc->regionbase, region); - init_preview_region(scene, area, sc, region); + region->regiontype = RGN_TYPE_PREVIEW; /* main region */ region = MEM_callocN(sizeof(ARegion), "main region for clip"); @@ -621,99 +545,28 @@ static void clip_dropboxes(void) WM_dropbox_add(lb, "CLIP_OT_open", clip_drop_poll, clip_drop_copy, NULL, NULL); } -static bool clip_set_region_visible(const bContext *C, - ARegion *region, - const bool is_visible, - const short alignment, - const bool view_all_on_show) +static void clip_refresh(const bContext *C, ScrArea *area) { - bool view_changed = false; + Scene *scene = CTX_data_scene(C); + SpaceClip *sc = (SpaceClip *)area->spacedata.first; - if (is_visible) { - if (region && (region->flag & RGN_FLAG_HIDDEN)) { - region->flag &= ~RGN_FLAG_HIDDEN; - region->v2d.flag &= ~V2D_IS_INIT; - if (view_all_on_show) { - region->v2d.cur = region->v2d.tot; - } - view_changed = true; - } - if (region && region->alignment != alignment) { - region->alignment = alignment; - view_changed = true; + ARegion *region_preview = BKE_area_find_region_type(area, RGN_TYPE_PREVIEW); + if (!(region_preview->v2d.flag & V2D_IS_INIT)) { + init_preview_region(scene, area, sc, region_preview); + region_preview->v2d.cur = region_preview->v2d.tot; + } + /* #V2D_VIEWSYNC_AREA_VERTICAL must always be set for the dopesheet view, in graph view it must + * be unset. This is enforced by region re-initialization. + * That means if it's not set correctly, the view just changed and needs re-initialization */ + else if (sc->view == SC_VIEW_DOPESHEET) { + if ((region_preview->v2d.flag & V2D_VIEWSYNC_AREA_VERTICAL) == 0) { + init_preview_region(scene, area, sc, region_preview); } } else { - if (region && !(region->flag & RGN_FLAG_HIDDEN)) { - region->flag |= RGN_FLAG_HIDDEN; - region->v2d.flag &= ~V2D_IS_INIT; - WM_event_remove_handlers((bContext *)C, ®ion->handlers); - view_changed = true; + if (region_preview->v2d.flag & V2D_VIEWSYNC_AREA_VERTICAL) { + init_preview_region(scene, area, sc, region_preview); } - if (region && region->alignment != RGN_ALIGN_NONE) { - region->alignment = RGN_ALIGN_NONE; - view_changed = true; - } - } - - return view_changed; -} - -static void clip_refresh(const bContext *C, ScrArea *area) -{ - wmWindowManager *wm = CTX_wm_manager(C); - wmWindow *window = CTX_wm_window(C); - Scene *scene = CTX_data_scene(C); - SpaceClip *sc = (SpaceClip *)area->spacedata.first; - ARegion *region_main = BKE_area_find_region_type(area, RGN_TYPE_WINDOW); - ARegion *region_tools = BKE_area_find_region_type(area, RGN_TYPE_TOOLS); - ARegion *region_preview = ED_clip_has_preview_region(C, area); - ARegion *region_properties = ED_clip_has_properties_region(area); - ARegion *region_channels = ED_clip_has_channels_region(area); - bool main_visible = false, preview_visible = false, tools_visible = false; - bool properties_visible = false, channels_visible = false; - bool view_changed = false; - - switch (sc->view) { - case SC_VIEW_CLIP: - main_visible = true; - preview_visible = false; - tools_visible = true; - properties_visible = true; - channels_visible = false; - break; - case SC_VIEW_GRAPH: - main_visible = false; - preview_visible = true; - tools_visible = false; - properties_visible = false; - channels_visible = false; - - reinit_preview_region(C, region_preview); - break; - case SC_VIEW_DOPESHEET: - main_visible = false; - preview_visible = true; - tools_visible = false; - properties_visible = false; - channels_visible = true; - - reinit_preview_region(C, region_preview); - break; - } - - view_changed |= clip_set_region_visible(C, region_main, main_visible, RGN_ALIGN_NONE, false); - view_changed |= clip_set_region_visible( - C, region_properties, properties_visible, RGN_ALIGN_RIGHT, false); - view_changed |= clip_set_region_visible(C, region_tools, tools_visible, RGN_ALIGN_LEFT, false); - view_changed |= clip_set_region_visible( - C, region_preview, preview_visible, RGN_ALIGN_NONE, true); - view_changed |= clip_set_region_visible( - C, region_channels, channels_visible, RGN_ALIGN_LEFT, false); - - if (view_changed) { - ED_area_init(wm, window, area); - ED_area_tag_redraw(area); } BKE_movieclip_user_set_frame(&sc->user, scene->r.cfra); @@ -781,6 +634,12 @@ static void movieclip_main_area_set_view2d(const bContext *C, ARegion *region) region->v2d.cur.ymax /= h; } +static bool clip_main_region_poll(const bContext *C) +{ + const SpaceClip *sclip = CTX_wm_space_clip(C); + return ELEM(sclip->view, SC_VIEW_CLIP); +} + /* add handlers, stuff you only do once or on area/region changes */ static void clip_main_region_init(wmWindowManager *wm, ARegion *region) { @@ -935,6 +794,12 @@ static void clip_main_region_listener(const wmRegionListenerParams *params) /****************** preview region ******************/ +static bool clip_preview_region_poll(const bContext *C) +{ + const SpaceClip *sclip = CTX_wm_space_clip(C); + return ELEM(sclip->view, SC_VIEW_GRAPH, SC_VIEW_DOPESHEET); +} + static void clip_preview_region_init(wmWindowManager *wm, ARegion *region) { wmKeyMap *keymap; @@ -1062,6 +927,12 @@ static void clip_preview_region_listener(const wmRegionListenerParams *UNUSED(pa /****************** channels region ******************/ +static bool clip_channels_region_poll(const bContext *C) +{ + const SpaceClip *sclip = CTX_wm_space_clip(C); + return ELEM(sclip->view, SC_VIEW_DOPESHEET); +} + static void clip_channels_region_init(wmWindowManager *wm, ARegion *region) { wmKeyMap *keymap; @@ -1138,6 +1009,12 @@ static void clip_header_region_listener(const wmRegionListenerParams *params) /****************** tools region ******************/ +static bool clip_tools_region_poll(const bContext *C) +{ + const SpaceClip *sclip = CTX_wm_space_clip(C); + return ELEM(sclip->view, SC_VIEW_CLIP); +} + /* add handlers, stuff you only do once or on area/region changes */ static void clip_tools_region_init(wmWindowManager *wm, ARegion *region) { @@ -1188,6 +1065,12 @@ static void clip_props_region_listener(const wmRegionListenerParams *params) /****************** properties region ******************/ +static bool clip_properties_region_poll(const bContext *C) +{ + const SpaceClip *sclip = CTX_wm_space_clip(C); + return ELEM(sclip->view, SC_VIEW_CLIP); +} + /* add handlers, stuff you only do once or on area/region changes */ static void clip_properties_region_init(wmWindowManager *wm, ARegion *region) { @@ -1292,6 +1175,7 @@ void ED_spacetype_clip(void) /* regions: main window */ art = MEM_callocN(sizeof(ARegionType), "spacetype clip region"); art->regionid = RGN_TYPE_WINDOW; + art->poll = clip_main_region_poll; art->init = clip_main_region_init; art->draw = clip_main_region_draw; art->listener = clip_main_region_listener; @@ -1303,6 +1187,7 @@ void ED_spacetype_clip(void) art = MEM_callocN(sizeof(ARegionType), "spacetype clip region preview"); art->regionid = RGN_TYPE_PREVIEW; art->prefsizey = 240; + art->poll = clip_preview_region_poll; art->init = clip_preview_region_init; art->draw = clip_preview_region_draw; art->listener = clip_preview_region_listener; @@ -1315,6 +1200,7 @@ void ED_spacetype_clip(void) art->regionid = RGN_TYPE_UI; art->prefsizex = UI_SIDEBAR_PANEL_WIDTH; art->keymapflag = ED_KEYMAP_FRAMES | ED_KEYMAP_UI; + art->poll = clip_properties_region_poll; art->init = clip_properties_region_init; art->draw = clip_properties_region_draw; art->listener = clip_properties_region_listener; @@ -1326,6 +1212,7 @@ void ED_spacetype_clip(void) art->regionid = RGN_TYPE_TOOLS; art->prefsizex = UI_SIDEBAR_PANEL_WIDTH; art->keymapflag = ED_KEYMAP_FRAMES | ED_KEYMAP_UI; + art->poll = clip_tools_region_poll; art->listener = clip_props_region_listener; art->init = clip_tools_region_init; art->draw = clip_tools_region_draw; @@ -1351,6 +1238,7 @@ void ED_spacetype_clip(void) art->regionid = RGN_TYPE_CHANNELS; art->prefsizex = UI_COMPACT_PANEL_WIDTH; art->keymapflag = ED_KEYMAP_FRAMES | ED_KEYMAP_UI; + art->poll = clip_channels_region_poll; art->listener = clip_channels_region_listener; art->init = clip_channels_region_init; art->draw = clip_channels_region_draw; diff --git a/source/blender/editors/space_sequencer/space_sequencer.c b/source/blender/editors/space_sequencer/space_sequencer.c index 5661208dc15..95ca2d4e491 100644 --- a/source/blender/editors/space_sequencer/space_sequencer.c +++ b/source/blender/editors/space_sequencer/space_sequencer.c @@ -144,7 +144,6 @@ static SpaceLink *sequencer_create(const ScrArea *UNUSED(area), const Scene *sce BLI_addtail(&sseq->regionbase, region); region->regiontype = RGN_TYPE_PREVIEW; region->alignment = RGN_ALIGN_TOP; - region->flag |= RGN_FLAG_HIDDEN; /* For now, aspect ratio should be maintained, and zoom is clamped within sane default limits. */ region->v2d.keepzoom = V2D_KEEPASPECT | V2D_KEEPZOOM | V2D_LIMITZOOM; region->v2d.minzoom = 0.001f; @@ -247,17 +246,6 @@ static void sequencer_refresh(const bContext *C, ScrArea *area) switch (sseq->view) { case SEQ_VIEW_SEQUENCE: - if (region_main && (region_main->flag & RGN_FLAG_HIDDEN)) { - region_main->flag &= ~RGN_FLAG_HIDDEN; - region_main->v2d.flag &= ~V2D_IS_INIT; - view_changed = true; - } - if (region_preview && !(region_preview->flag & RGN_FLAG_HIDDEN)) { - region_preview->flag |= RGN_FLAG_HIDDEN; - region_preview->v2d.flag &= ~V2D_IS_INIT; - WM_event_remove_handlers((bContext *)C, ®ion_preview->handlers); - view_changed = true; - } if (region_main && region_main->alignment != RGN_ALIGN_NONE) { region_main->alignment = RGN_ALIGN_NONE; view_changed = true; @@ -268,17 +256,11 @@ static void sequencer_refresh(const bContext *C, ScrArea *area) } break; case SEQ_VIEW_PREVIEW: - if (region_main && !(region_main->flag & RGN_FLAG_HIDDEN)) { - region_main->flag |= RGN_FLAG_HIDDEN; - region_main->v2d.flag &= ~V2D_IS_INIT; - WM_event_remove_handlers((bContext *)C, ®ion_main->handlers); - view_changed = true; - } - if (region_preview && (region_preview->flag & RGN_FLAG_HIDDEN)) { - region_preview->flag &= ~RGN_FLAG_HIDDEN; - region_preview->v2d.flag &= ~V2D_IS_INIT; + /* Reset scrolling when preview region just appears. */ + if (!(region_preview->v2d.flag & V2D_IS_INIT)) { region_preview->v2d.cur = region_preview->v2d.tot; - view_changed = true; + /* Only redraw, don't re-init. */ + ED_area_tag_redraw(area); } if (region_main && region_main->alignment != RGN_ALIGN_NONE) { region_main->alignment = RGN_ALIGN_NONE; @@ -297,17 +279,10 @@ static void sequencer_refresh(const bContext *C, ScrArea *area) /* We reuse hidden region's size, allows to find same layout as before if we just switch * between one 'full window' view and the combined one. This gets lost if we switch to both * 'full window' views before, though... Better than nothing. */ - if (region_main->flag & RGN_FLAG_HIDDEN) { - region_main->flag &= ~RGN_FLAG_HIDDEN; - region_main->v2d.flag &= ~V2D_IS_INIT; - region_preview->sizey = (int)(height - region_main->sizey); - view_changed = true; - } - if (region_preview->flag & RGN_FLAG_HIDDEN) { - region_preview->flag &= ~RGN_FLAG_HIDDEN; - region_preview->v2d.flag &= ~V2D_IS_INIT; + if (!(region_preview->v2d.flag & V2D_IS_INIT)) { region_preview->v2d.cur = region_preview->v2d.tot; region_main->sizey = (int)(height - region_preview->sizey); + region_preview->sizey = (int)(height - region_main->sizey); view_changed = true; } if (region_main->alignment != RGN_ALIGN_NONE) { @@ -331,23 +306,12 @@ static void sequencer_refresh(const bContext *C, ScrArea *area) ARegion *region_channels = sequencer_find_region(area, RGN_TYPE_CHANNELS); if (sseq->view == SEQ_VIEW_SEQUENCE) { - if (region_channels && (region_channels->flag & RGN_FLAG_HIDDEN)) { - region_channels->flag &= ~RGN_FLAG_HIDDEN; - region_channels->v2d.flag &= ~V2D_IS_INIT; - view_changed = true; - } if (region_channels && region_channels->alignment != RGN_ALIGN_LEFT) { region_channels->alignment = RGN_ALIGN_LEFT; view_changed = true; } } else { - if (region_channels && !(region_channels->flag & RGN_FLAG_HIDDEN)) { - region_channels->flag |= RGN_FLAG_HIDDEN; - region_channels->v2d.flag &= ~V2D_IS_INIT; - WM_event_remove_handlers((bContext *)C, ®ion_channels->handlers); - view_changed = true; - } if (region_channels && region_channels->alignment != RGN_ALIGN_NONE) { region_channels->alignment = RGN_ALIGN_NONE; view_changed = true; @@ -508,6 +472,13 @@ static void sequencer_gizmos(void) } /* *********************** sequencer (main) region ************************ */ + +static bool sequencer_main_region_poll(const bContext *C) +{ + const SpaceSeq *sseq = CTX_wm_space_seq(C); + return ELEM(sseq->view, SEQ_VIEW_SEQUENCE, SEQ_VIEW_SEQUENCE_PREVIEW); +} + /* Add handlers, stuff you only do once or on area/region changes. */ static void sequencer_main_region_init(wmWindowManager *wm, ARegion *region) { @@ -761,6 +732,13 @@ static void sequencer_tools_region_draw(const bContext *C, ARegion *region) ED_region_panels(C, region); } /* *********************** preview region ************************ */ + +static bool sequencer_preview_region_poll(const bContext *C) +{ + const SpaceSeq *sseq = CTX_wm_space_seq(C); + return ELEM(sseq->view, SEQ_VIEW_PREVIEW, SEQ_VIEW_SEQUENCE_PREVIEW); +} + static void sequencer_preview_region_init(wmWindowManager *wm, ARegion *region) { wmKeyMap *keymap; @@ -983,6 +961,12 @@ static void sequencer_id_remap(ScrArea *UNUSED(area), /* ************************************* */ +static bool sequencer_channel_region_poll(const bContext *C) +{ + const SpaceSeq *sseq = CTX_wm_space_seq(C); + return ELEM(sseq->view, SEQ_VIEW_SEQUENCE); +} + /* add handlers, stuff you only do once or on area/region changes */ static void sequencer_channel_region_init(wmWindowManager *wm, ARegion *region) { @@ -1070,6 +1054,7 @@ void ED_spacetype_sequencer(void) /* Main window. */ art = MEM_callocN(sizeof(ARegionType), "spacetype sequencer region"); art->regionid = RGN_TYPE_WINDOW; + art->poll = sequencer_main_region_poll; art->init = sequencer_main_region_init; art->draw = sequencer_main_region_draw; art->draw_overlay = sequencer_main_region_draw_overlay; @@ -1084,6 +1069,7 @@ void ED_spacetype_sequencer(void) /* Preview. */ art = MEM_callocN(sizeof(ARegionType), "spacetype sequencer region"); art->regionid = RGN_TYPE_PREVIEW; + art->poll = sequencer_preview_region_poll; art->init = sequencer_preview_region_init; art->layout = sequencer_preview_region_layout; art->on_view2d_changed = sequencer_preview_region_view2d_changed; @@ -1122,6 +1108,7 @@ void ED_spacetype_sequencer(void) art->regionid = RGN_TYPE_CHANNELS; art->prefsizex = UI_COMPACT_PANEL_WIDTH; art->keymapflag = ED_KEYMAP_UI; + art->poll = sequencer_channel_region_poll; art->init = sequencer_channel_region_init; art->draw = sequencer_channel_region_draw; art->listener = sequencer_main_region_listener; -- 2.30.2 From ca7b094bcca08cb84921cf4823df69f7774c8e63 Mon Sep 17 00:00:00 2001 From: Julian Eisel Date: Thu, 23 Feb 2023 19:26:31 +0100 Subject: [PATCH 19/25] Use poll functions for Preferences execution region Previously we'd hide the "execution" region (containing the Save Preferences button) when the header is visible, which would contain the button instead. It was possible to unhide it, which made it behave glitchy. Now it reliably "removes" is and brings it back when needed (it's not really removed, only from what the user can tell). --- source/blender/editors/screen/screen_ops.c | 5 ----- source/blender/editors/space_userpref/space_userpref.c | 10 +++++++++- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/source/blender/editors/screen/screen_ops.c b/source/blender/editors/screen/screen_ops.c index 80e5da43423..76c24b54a98 100644 --- a/source/blender/editors/screen/screen_ops.c +++ b/source/blender/editors/screen/screen_ops.c @@ -5079,11 +5079,6 @@ static int userpref_show_exec(bContext *C, wmOperator *op) region->flag |= RGN_FLAG_HIDDEN; ED_region_visibility_change_update(C, area, region); - /* And also show the region with "Load & Save" buttons. */ - region = BKE_area_find_region_type(area, RGN_TYPE_EXECUTE); - region->flag &= ~RGN_FLAG_HIDDEN; - ED_region_visibility_change_update(C, area, region); - return OPERATOR_FINISHED; } BKE_report(op->reports, RPT_ERROR, "Failed to open window!"); diff --git a/source/blender/editors/space_userpref/space_userpref.c b/source/blender/editors/space_userpref/space_userpref.c index 1516435c6fc..b4fe9179188 100644 --- a/source/blender/editors/space_userpref/space_userpref.c +++ b/source/blender/editors/space_userpref/space_userpref.c @@ -65,7 +65,7 @@ static SpaceLink *userpref_create(const ScrArea *area, const Scene *UNUSED(scene BLI_addtail(&spref->regionbase, region); region->regiontype = RGN_TYPE_EXECUTE; region->alignment = RGN_ALIGN_BOTTOM | RGN_SPLIT_PREV; - region->flag |= RGN_FLAG_DYNAMIC_SIZE | RGN_FLAG_HIDDEN; + region->flag |= RGN_FLAG_DYNAMIC_SIZE; /* main region */ region = MEM_callocN(sizeof(ARegion), "main region for userpref"); @@ -162,6 +162,13 @@ static void userpref_navigation_region_draw(const bContext *C, ARegion *region) ED_region_panels(C, region); } +static bool userpref_execute_region_poll(const bContext *C) +{ + const ScrArea *area = CTX_wm_area(C); + const ARegion *region_header = BKE_area_find_region_type(area, RGN_TYPE_HEADER); + return !region_header->visible; +} + /* add handlers, stuff you only do once or on area/region changes */ static void userpref_execute_region_init(wmWindowManager *wm, ARegion *region) { @@ -243,6 +250,7 @@ void ED_spacetype_userpref(void) art = MEM_callocN(sizeof(ARegionType), "spacetype userpref region"); art->regionid = RGN_TYPE_EXECUTE; art->prefsizey = HEADERY; + art->poll = userpref_execute_region_poll; art->init = userpref_execute_region_init; art->layout = ED_region_panels_layout; art->draw = ED_region_panels_draw; -- 2.30.2 From e58ac364af261c3fde3bab9d38fd985fc24a68ca Mon Sep 17 00:00:00 2001 From: Julian Eisel Date: Mon, 20 Mar 2023 13:03:19 +0100 Subject: [PATCH 20/25] Fix broken sequencer when used from existing workspace --- source/blender/blenloader/intern/versioning_300.cc | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/source/blender/blenloader/intern/versioning_300.cc b/source/blender/blenloader/intern/versioning_300.cc index 94682458c98..b05d440ad93 100644 --- a/source/blender/blenloader/intern/versioning_300.cc +++ b/source/blender/blenloader/intern/versioning_300.cc @@ -4257,6 +4257,20 @@ void blo_do_versions_300(FileData *fd, Library * /*lib*/, Main *bmain) LISTBASE_FOREACH (ScrArea *, area, &screen->areabase) { LISTBASE_FOREACH (SpaceLink *, sl, &area->spacedata) { version_ensure_missing_regions(area, sl); + + const ListBase *regionbase = (sl == area->spacedata.first) ? &area->regionbase : + &sl->regionbase; + if (sl->spacetype == SPACE_SEQ) { + ARegion *region_main = BKE_region_find_in_listbase_by_type(regionbase, + RGN_TYPE_WINDOW); + region_main->flag &= ~RGN_FLAG_HIDDEN; + region_main->alignment = RGN_ALIGN_NONE; + + ARegion *region_preview = BKE_region_find_in_listbase_by_type(regionbase, + RGN_TYPE_PREVIEW); + region_preview->flag &= ~RGN_FLAG_HIDDEN; + region_preview->alignment = RGN_ALIGN_NONE; + } } } } -- 2.30.2 From 63b8e31a9ce25794ba69a672b4ba8366f0522601 Mon Sep 17 00:00:00 2001 From: Julian Eisel Date: Mon, 20 Mar 2023 15:01:23 +0100 Subject: [PATCH 21/25] Fix missing sequencer channels region when reading from old files --- source/blender/blenloader/intern/versioning_300.cc | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/source/blender/blenloader/intern/versioning_300.cc b/source/blender/blenloader/intern/versioning_300.cc index b05d440ad93..2052c193274 100644 --- a/source/blender/blenloader/intern/versioning_300.cc +++ b/source/blender/blenloader/intern/versioning_300.cc @@ -2280,6 +2280,13 @@ static void version_ensure_missing_regions(ScrArea *area, SpaceLink *sl) break; } + case SPACE_SEQ: { + do_versions_ensure_region(regionbase, + RGN_TYPE_CHANNELS, + "versioning: channels region for sequencer", + RGN_TYPE_TOOLS); + break; + } } } -- 2.30.2 From b5e69fd1954f1daa527d6f02dc1ab12450b4d80c Mon Sep 17 00:00:00 2001 From: Julian Eisel Date: Mon, 20 Mar 2023 15:03:56 +0100 Subject: [PATCH 22/25] Avoid region state management in sequencer refresh callback Ensure a specific state with versioning, then the refreshing can be simplified quite a bit. --- .../blenloader/intern/versioning_300.cc | 6 ++ .../editors/space_sequencer/space_sequencer.c | 81 ++++++------------- 2 files changed, 31 insertions(+), 56 deletions(-) diff --git a/source/blender/blenloader/intern/versioning_300.cc b/source/blender/blenloader/intern/versioning_300.cc index 2052c193274..69868630d74 100644 --- a/source/blender/blenloader/intern/versioning_300.cc +++ b/source/blender/blenloader/intern/versioning_300.cc @@ -4265,6 +4265,8 @@ void blo_do_versions_300(FileData *fd, Library * /*lib*/, Main *bmain) LISTBASE_FOREACH (SpaceLink *, sl, &area->spacedata) { version_ensure_missing_regions(area, sl); + /* Ensure expected region state. Previously this was modified to hide/unhide regions. */ + const ListBase *regionbase = (sl == area->spacedata.first) ? &area->regionbase : &sl->regionbase; if (sl->spacetype == SPACE_SEQ) { @@ -4277,6 +4279,10 @@ void blo_do_versions_300(FileData *fd, Library * /*lib*/, Main *bmain) RGN_TYPE_PREVIEW); region_preview->flag &= ~RGN_FLAG_HIDDEN; region_preview->alignment = RGN_ALIGN_NONE; + + ARegion *region_channels = BKE_region_find_in_listbase_by_type(regionbase, + RGN_TYPE_CHANNELS); + region_channels->alignment = RGN_ALIGN_LEFT; } } } diff --git a/source/blender/editors/space_sequencer/space_sequencer.c b/source/blender/editors/space_sequencer/space_sequencer.c index ae566f56f64..20182a5d020 100644 --- a/source/blender/editors/space_sequencer/space_sequencer.c +++ b/source/blender/editors/space_sequencer/space_sequencer.c @@ -245,16 +245,6 @@ static void sequencer_refresh(const bContext *C, ScrArea *area) bool view_changed = false; switch (sseq->view) { - case SEQ_VIEW_SEQUENCE: - if (region_main && region_main->alignment != RGN_ALIGN_NONE) { - region_main->alignment = RGN_ALIGN_NONE; - view_changed = true; - } - if (region_preview && region_preview->alignment != RGN_ALIGN_NONE) { - region_preview->alignment = RGN_ALIGN_NONE; - view_changed = true; - } - break; case SEQ_VIEW_PREVIEW: /* Reset scrolling when preview region just appears. */ if (!(region_preview->v2d.flag & V2D_IS_INIT)) { @@ -262,60 +252,39 @@ static void sequencer_refresh(const bContext *C, ScrArea *area) /* Only redraw, don't re-init. */ ED_area_tag_redraw(area); } - if (region_main && region_main->alignment != RGN_ALIGN_NONE) { - region_main->alignment = RGN_ALIGN_NONE; - view_changed = true; - } - if (region_preview && region_preview->alignment != RGN_ALIGN_NONE) { + if (region_preview->alignment != RGN_ALIGN_NONE) { region_preview->alignment = RGN_ALIGN_NONE; view_changed = true; } break; - case SEQ_VIEW_SEQUENCE_PREVIEW: - if (region_main && region_preview) { - /* Get available height (without DPI correction). */ - const float height = (area->winy - ED_area_headersize()) / UI_SCALE_FAC; + case SEQ_VIEW_SEQUENCE_PREVIEW: { + /* Get available height (without DPI correction). */ + const float height = (area->winy - ED_area_headersize()) / UI_SCALE_FAC; - /* We reuse hidden region's size, allows to find same layout as before if we just switch - * between one 'full window' view and the combined one. This gets lost if we switch to both - * 'full window' views before, though... Better than nothing. */ - if (!(region_preview->v2d.flag & V2D_IS_INIT)) { - region_preview->v2d.cur = region_preview->v2d.tot; - region_main->sizey = (int)(height - region_preview->sizey); - region_preview->sizey = (int)(height - region_main->sizey); - view_changed = true; - } - if (region_main->alignment != RGN_ALIGN_NONE) { - region_main->alignment = RGN_ALIGN_NONE; - view_changed = true; - } - if (region_preview->alignment != RGN_ALIGN_TOP) { - region_preview->alignment = RGN_ALIGN_TOP; - view_changed = true; - } - /* Final check that both preview and main height are reasonable. */ - if (region_preview->sizey < 10 || region_main->sizey < 10 || - region_preview->sizey + region_main->sizey > height) { - region_preview->sizey = roundf(height * 0.4f); - region_main->sizey = (int)(height - region_preview->sizey); - view_changed = true; - } + /* We reuse hidden region's size, allows to find same layout as before if we just switch + * between one 'full window' view and the combined one. This gets lost if we switch to both + * 'full window' views before, though... Better than nothing. */ + if (!(region_preview->v2d.flag & V2D_IS_INIT)) { + region_preview->v2d.cur = region_preview->v2d.tot; + region_main->sizey = (int)(height - region_preview->sizey); + region_preview->sizey = (int)(height - region_main->sizey); + view_changed = true; + } + if (region_preview->alignment != RGN_ALIGN_TOP) { + region_preview->alignment = RGN_ALIGN_TOP; + view_changed = true; + } + /* Final check that both preview and main height are reasonable. */ + if (region_preview->sizey < 10 || region_main->sizey < 10 || + region_preview->sizey + region_main->sizey > height) { + region_preview->sizey = roundf(height * 0.4f); + region_main->sizey = (int)(height - region_preview->sizey); + view_changed = true; } break; - } - - ARegion *region_channels = sequencer_find_region(area, RGN_TYPE_CHANNELS); - if (sseq->view == SEQ_VIEW_SEQUENCE) { - if (region_channels && region_channels->alignment != RGN_ALIGN_LEFT) { - region_channels->alignment = RGN_ALIGN_LEFT; - view_changed = true; - } - } - else { - if (region_channels && region_channels->alignment != RGN_ALIGN_NONE) { - region_channels->alignment = RGN_ALIGN_NONE; - view_changed = true; } + case SEQ_VIEW_SEQUENCE: + break; } if (view_changed) { -- 2.30.2 From b43b416d1f58b4dd5fcfb2b9f02c6d55624ca560 Mon Sep 17 00:00:00 2001 From: Julian Eisel Date: Mon, 20 Mar 2023 15:32:45 +0100 Subject: [PATCH 23/25] Address review comments --- source/blender/blenkernel/BKE_screen.h | 6 ++-- .../blenloader/intern/versioning_300.cc | 34 ++++++++----------- .../blenloader/intern/versioning_common.cc | 2 +- 3 files changed, 20 insertions(+), 22 deletions(-) diff --git a/source/blender/blenkernel/BKE_screen.h b/source/blender/blenkernel/BKE_screen.h index e61bbb26533..1af2bbffff6 100644 --- a/source/blender/blenkernel/BKE_screen.h +++ b/source/blender/blenkernel/BKE_screen.h @@ -164,10 +164,12 @@ typedef struct ARegionType { void (*init)(struct wmWindowManager *wm, struct ARegion *region); /* exit is called when the region is hidden or removed */ void (*exit)(struct wmWindowManager *wm, struct ARegion *region); - /** Optional callback to decide whether the region should be treated as existing given the + /** + * Optional callback to decide whether the region should be treated as existing given the * current context. When returning false, the region will be kept in storage, but is not * available to the user in any way. Callbacks can assume that context has the owning area and - * space-data set. */ + * space-data set. + */ bool (*poll)(const struct bContext *C); /* draw entirely, view changes should be handled here */ void (*draw)(const struct bContext *C, struct ARegion *region); diff --git a/source/blender/blenloader/intern/versioning_300.cc b/source/blender/blenloader/intern/versioning_300.cc index 69868630d74..ad7459e3d67 100644 --- a/source/blender/blenloader/intern/versioning_300.cc +++ b/source/blender/blenloader/intern/versioning_300.cc @@ -2233,29 +2233,25 @@ static void version_ensure_missing_regions(ScrArea *area, SpaceLink *sl) switch (sl->spacetype) { case SPACE_FILE: { - ARegion *new_region; - - new_region = do_versions_add_region_if_not_found( - regionbase, RGN_TYPE_UI, "versioning: UI region for file", RGN_TYPE_TOOLS); - if (new_region) { - new_region->alignment = RGN_ALIGN_TOP; - new_region->flag |= RGN_FLAG_DYNAMIC_SIZE; + if (ARegion *ui_region = do_versions_add_region_if_not_found( + regionbase, RGN_TYPE_UI, "versioning: UI region for file", RGN_TYPE_TOOLS)) { + ui_region->alignment = RGN_ALIGN_TOP; + ui_region->flag |= RGN_FLAG_DYNAMIC_SIZE; } - new_region = do_versions_add_region_if_not_found( - regionbase, RGN_TYPE_EXECUTE, "versioning: execute region for file", RGN_TYPE_UI); - if (new_region) { - new_region->alignment = RGN_ALIGN_BOTTOM; - new_region->flag = RGN_FLAG_DYNAMIC_SIZE; + if (ARegion *exec_region = do_versions_add_region_if_not_found( + regionbase, RGN_TYPE_EXECUTE, "versioning: execute region for file", RGN_TYPE_UI)) { + exec_region->alignment = RGN_ALIGN_BOTTOM; + exec_region->flag = RGN_FLAG_DYNAMIC_SIZE; } - new_region = do_versions_add_region_if_not_found(regionbase, - RGN_TYPE_TOOL_PROPS, - "versioning: tool props region for file", - RGN_TYPE_EXECUTE); - if (new_region) { - new_region->alignment = RGN_ALIGN_RIGHT; - new_region->flag = RGN_FLAG_HIDDEN; + if (ARegion *tool_props_region = do_versions_add_region_if_not_found( + regionbase, + RGN_TYPE_TOOL_PROPS, + "versioning: tool props region for file", + RGN_TYPE_EXECUTE)) { + tool_props_region->alignment = RGN_ALIGN_RIGHT; + tool_props_region->flag = RGN_FLAG_HIDDEN; } break; } diff --git a/source/blender/blenloader/intern/versioning_common.cc b/source/blender/blenloader/intern/versioning_common.cc index d270322d52e..31e8c7620dc 100644 --- a/source/blender/blenloader/intern/versioning_common.cc +++ b/source/blender/blenloader/intern/versioning_common.cc @@ -66,7 +66,7 @@ ARegion *do_versions_ensure_region(ListBase *regionbase, } } - ARegion *new_region = static_cast(MEM_callocN(sizeof(ARegion), allocname)); + ARegion *new_region = MEM_cnew(allocname); new_region->regiontype = region_type; BLI_insertlinkafter(regionbase, link_after_region, new_region); return new_region; -- 2.30.2 From 38c14def4cf42eb7871ca32537d691d55cded79d Mon Sep 17 00:00:00 2001 From: Julian Eisel Date: Wed, 5 Apr 2023 13:04:27 +0200 Subject: [PATCH 24/25] From review: Avoid repetitive context queries in poll functions --- source/blender/blenkernel/BKE_screen.h | 12 ++++++++++- source/blender/editors/screen/screen_edit.c | 15 +++++++++++--- .../blender/editors/space_clip/space_clip.cc | 20 +++++++++---------- .../blender/editors/space_file/space_file.c | 12 +++++------ .../editors/space_sequencer/space_sequencer.c | 12 +++++------ .../editors/space_userpref/space_userpref.c | 5 ++--- 6 files changed, 47 insertions(+), 29 deletions(-) diff --git a/source/blender/blenkernel/BKE_screen.h b/source/blender/blenkernel/BKE_screen.h index d2fd6d8f5d6..63462e689e3 100644 --- a/source/blender/blenkernel/BKE_screen.h +++ b/source/blender/blenkernel/BKE_screen.h @@ -155,6 +155,16 @@ typedef struct wmRegionMessageSubscribeParams { struct ARegion *region; } wmRegionMessageSubscribeParams; +typedef struct RegionPollParams { + /** Context in case non-WM context is needed. Any screen/area/region context should be obtained + * using the members below instead (avoids many context queries on redraws). */ + const struct bContext *context; + + const struct bScreen *screen; + const struct ScrArea *area; + const struct ARegion *region; +} RegionPollParams; + typedef struct ARegionType { struct ARegionType *next, *prev; @@ -170,7 +180,7 @@ typedef struct ARegionType { * available to the user in any way. Callbacks can assume that context has the owning area and * space-data set. */ - bool (*poll)(const struct bContext *C); + bool (*poll)(const RegionPollParams *params); /* draw entirely, view changes should be handled here */ void (*draw)(const struct bContext *C, struct ARegion *region); /** diff --git a/source/blender/editors/screen/screen_edit.c b/source/blender/editors/screen/screen_edit.c index 0bbcb9bb8e2..30a2c884b37 100644 --- a/source/blender/editors/screen/screen_edit.c +++ b/source/blender/editors/screen/screen_edit.c @@ -713,14 +713,23 @@ void ED_screens_init(Main *bmain, wmWindowManager *wm) } } -static bool region_poll(const bContext *C, const ARegion *region) +static bool region_poll(const bContext *C, + const bScreen *screen, + const ScrArea *area, + const ARegion *region) { if (!region->type || !region->type->poll) { /* Show region by default. */ return true; } - return region->type->poll(C); + RegionPollParams params = {0}; + params.context = C; + params.screen = screen; + params.area = area; + params.region = region; + + return region->type->poll(¶ms); } static void screen_regions_poll(bContext *C, const wmWindow *win, bScreen *screen) @@ -738,7 +747,7 @@ static void screen_regions_poll(bContext *C, const wmWindow *win, bScreen *scree region->flag &= ~RGN_FLAG_POLL_FAILED; CTX_wm_region_set(C, region); - if (region_poll(C, region) == false) { + if (region_poll(C, screen, area, region) == false) { region->flag |= RGN_FLAG_POLL_FAILED; } diff --git a/source/blender/editors/space_clip/space_clip.cc b/source/blender/editors/space_clip/space_clip.cc index 6a63a97ecad..5543730ec19 100644 --- a/source/blender/editors/space_clip/space_clip.cc +++ b/source/blender/editors/space_clip/space_clip.cc @@ -633,9 +633,9 @@ static void movieclip_main_area_set_view2d(const bContext *C, ARegion *region) region->v2d.cur.ymax /= h; } -static bool clip_main_region_poll(const bContext *C) +static bool clip_main_region_poll(const RegionPollParams *params) { - const SpaceClip *sclip = CTX_wm_space_clip(C); + const SpaceClip *sclip = static_cast(params->area->spacedata.first); return ELEM(sclip->view, SC_VIEW_CLIP); } @@ -794,9 +794,9 @@ static void clip_main_region_listener(const wmRegionListenerParams *params) /****************** preview region ******************/ -static bool clip_preview_region_poll(const bContext *C) +static bool clip_preview_region_poll(const RegionPollParams *params) { - const SpaceClip *sclip = CTX_wm_space_clip(C); + const SpaceClip *sclip = static_cast(params->area->spacedata.first); return ELEM(sclip->view, SC_VIEW_GRAPH, SC_VIEW_DOPESHEET); } @@ -925,9 +925,9 @@ static void clip_preview_region_listener(const wmRegionListenerParams * /*params /****************** channels region ******************/ -static bool clip_channels_region_poll(const bContext *C) +static bool clip_channels_region_poll(const RegionPollParams *params) { - const SpaceClip *sclip = CTX_wm_space_clip(C); + const SpaceClip *sclip = static_cast(params->area->spacedata.first); return ELEM(sclip->view, SC_VIEW_DOPESHEET); } @@ -1005,9 +1005,9 @@ static void clip_header_region_listener(const wmRegionListenerParams *params) /****************** tools region ******************/ -static bool clip_tools_region_poll(const bContext *C) +static bool clip_tools_region_poll(const RegionPollParams *params) { - const SpaceClip *sclip = CTX_wm_space_clip(C); + const SpaceClip *sclip = static_cast(params->area->spacedata.first); return ELEM(sclip->view, SC_VIEW_CLIP); } @@ -1061,9 +1061,9 @@ static void clip_props_region_listener(const wmRegionListenerParams *params) /****************** properties region ******************/ -static bool clip_properties_region_poll(const bContext *C) +static bool clip_properties_region_poll(const RegionPollParams *params) { - const SpaceClip *sclip = CTX_wm_space_clip(C); + const SpaceClip *sclip = static_cast(params->area->spacedata.first); return ELEM(sclip->view, SC_VIEW_CLIP); } diff --git a/source/blender/editors/space_file/space_file.c b/source/blender/editors/space_file/space_file.c index d55a1caa120..019f1fdbccb 100644 --- a/source/blender/editors/space_file/space_file.c +++ b/source/blender/editors/space_file/space_file.c @@ -629,22 +629,22 @@ static void file_keymap(struct wmKeyConfig *keyconf) WM_keymap_ensure(keyconf, "File Browser Buttons", SPACE_FILE, 0); } -static bool file_ui_region_poll(const bContext *C) +static bool file_ui_region_poll(const RegionPollParams *params) { - const SpaceFile *sfile = CTX_wm_space_file(C); + const SpaceFile *sfile = (SpaceFile *)params->area->spacedata.first; /* Always visible except when browsing assets. */ return sfile->browse_mode != FILE_BROWSE_MODE_ASSETS; } -static bool file_tool_props_region_poll(const bContext *C) +static bool file_tool_props_region_poll(const RegionPollParams *params) { - const SpaceFile *sfile = CTX_wm_space_file(C); + const SpaceFile *sfile = (SpaceFile *)params->area->spacedata.first; return (sfile->browse_mode == FILE_BROWSE_MODE_ASSETS) || (sfile->op != NULL); } -static bool file_execution_region_poll(const bContext *C) +static bool file_execution_region_poll(const RegionPollParams *params) { - const SpaceFile *sfile = CTX_wm_space_file(C); + const SpaceFile *sfile = (SpaceFile *)params->area->spacedata.first; return sfile->op != NULL; } diff --git a/source/blender/editors/space_sequencer/space_sequencer.c b/source/blender/editors/space_sequencer/space_sequencer.c index c2620f9e982..ea34b92fe2a 100644 --- a/source/blender/editors/space_sequencer/space_sequencer.c +++ b/source/blender/editors/space_sequencer/space_sequencer.c @@ -440,9 +440,9 @@ static void sequencer_gizmos(void) /* *********************** sequencer (main) region ************************ */ -static bool sequencer_main_region_poll(const bContext *C) +static bool sequencer_main_region_poll(const RegionPollParams *params) { - const SpaceSeq *sseq = CTX_wm_space_seq(C); + const SpaceSeq *sseq = (SpaceSeq *)params->area->spacedata.first; return ELEM(sseq->view, SEQ_VIEW_SEQUENCE, SEQ_VIEW_SEQUENCE_PREVIEW); } @@ -700,9 +700,9 @@ static void sequencer_tools_region_draw(const bContext *C, ARegion *region) } /* *********************** preview region ************************ */ -static bool sequencer_preview_region_poll(const bContext *C) +static bool sequencer_preview_region_poll(const RegionPollParams *params) { - const SpaceSeq *sseq = CTX_wm_space_seq(C); + const SpaceSeq *sseq = (SpaceSeq *)params->area->spacedata.first; return ELEM(sseq->view, SEQ_VIEW_PREVIEW, SEQ_VIEW_SEQUENCE_PREVIEW); } @@ -928,9 +928,9 @@ static void sequencer_id_remap(ScrArea *UNUSED(area), /* ************************************* */ -static bool sequencer_channel_region_poll(const bContext *C) +static bool sequencer_channel_region_poll(const RegionPollParams *params) { - const SpaceSeq *sseq = CTX_wm_space_seq(C); + const SpaceSeq *sseq = (SpaceSeq *)params->area->spacedata.first; return ELEM(sseq->view, SEQ_VIEW_SEQUENCE); } diff --git a/source/blender/editors/space_userpref/space_userpref.c b/source/blender/editors/space_userpref/space_userpref.c index c6224001e32..f8d31d5e70a 100644 --- a/source/blender/editors/space_userpref/space_userpref.c +++ b/source/blender/editors/space_userpref/space_userpref.c @@ -156,10 +156,9 @@ static void userpref_navigation_region_draw(const bContext *C, ARegion *region) ED_region_panels(C, region); } -static bool userpref_execute_region_poll(const bContext *C) +static bool userpref_execute_region_poll(const RegionPollParams *params) { - const ScrArea *area = CTX_wm_area(C); - const ARegion *region_header = BKE_area_find_region_type(area, RGN_TYPE_HEADER); + const ARegion *region_header = BKE_area_find_region_type(params->area, RGN_TYPE_HEADER); return !region_header->visible; } -- 2.30.2 From 73ec577c03c048cd2e5f016edd0d8ad3a6c20b18 Mon Sep 17 00:00:00 2001 From: Julian Eisel Date: Wed, 5 Apr 2023 13:17:24 +0200 Subject: [PATCH 25/25] Remove unused context parameter --- source/blender/blenkernel/BKE_screen.h | 6 ++---- source/blender/editors/screen/screen_edit.c | 17 ++--------------- 2 files changed, 4 insertions(+), 19 deletions(-) diff --git a/source/blender/blenkernel/BKE_screen.h b/source/blender/blenkernel/BKE_screen.h index 63462e689e3..6ce89b3448d 100644 --- a/source/blender/blenkernel/BKE_screen.h +++ b/source/blender/blenkernel/BKE_screen.h @@ -156,13 +156,11 @@ typedef struct wmRegionMessageSubscribeParams { } wmRegionMessageSubscribeParams; typedef struct RegionPollParams { - /** Context in case non-WM context is needed. Any screen/area/region context should be obtained - * using the members below instead (avoids many context queries on redraws). */ - const struct bContext *context; - const struct bScreen *screen; const struct ScrArea *area; const struct ARegion *region; + + /* For now only WM context members here, could add the scene or even #bContext if needed. */ } RegionPollParams; typedef struct ARegionType { diff --git a/source/blender/editors/screen/screen_edit.c b/source/blender/editors/screen/screen_edit.c index 30a2c884b37..88762a19778 100644 --- a/source/blender/editors/screen/screen_edit.c +++ b/source/blender/editors/screen/screen_edit.c @@ -713,10 +713,7 @@ void ED_screens_init(Main *bmain, wmWindowManager *wm) } } -static bool region_poll(const bContext *C, - const bScreen *screen, - const ScrArea *area, - const ARegion *region) +static bool region_poll(const bScreen *screen, const ScrArea *area, const ARegion *region) { if (!region->type || !region->type->poll) { /* Show region by default. */ @@ -724,7 +721,6 @@ static bool region_poll(const bContext *C, } RegionPollParams params = {0}; - params.context = C; params.screen = screen; params.area = area; params.region = region; @@ -734,20 +730,14 @@ static bool region_poll(const bContext *C, static void screen_regions_poll(bContext *C, const wmWindow *win, bScreen *screen) { - ScrArea *previous_area = CTX_wm_area(C); - ARegion *previous_region = CTX_wm_region(C); - bool any_changed = false; ED_screen_areas_iter (win, screen, area) { - CTX_wm_area_set(C, area); - LISTBASE_FOREACH (ARegion *, region, &area->regionbase) { const int old_region_flag = region->flag; region->flag &= ~RGN_FLAG_POLL_FAILED; - CTX_wm_region_set(C, region); - if (region_poll(C, screen, area, region) == false) { + if (region_poll(screen, area, region) == false) { region->flag |= RGN_FLAG_POLL_FAILED; } @@ -764,9 +754,6 @@ static void screen_regions_poll(bContext *C, const wmWindow *win, bScreen *scree if (any_changed) { screen->do_refresh = true; } - - CTX_wm_area_set(C, previous_area); - CTX_wm_region_set(C, previous_region); } void ED_screen_ensure_updated(bContext *C, wmWindowManager *wm, wmWindow *win, bScreen *screen) -- 2.30.2