From fd2d2f225fc1fefb85ce123cbc5f510bb8a97250 Mon Sep 17 00:00:00 2001 From: Julian Eisel Date: Tue, 5 Nov 2024 17:05:15 +0100 Subject: [PATCH] Fix #128744: Reverting single essentials brush reverts all Code used library reloading which would reload all data-blocks from a data-block library. Since the essentials brushes are stored in a single .blend file (one per mode), they would all be reset. In general this violates the design where multiple brushes in a single .blend file should be usable just fine. (A single file per brush is only necessary to allow saving edits to that file without opening it.) Instead of library reloading, delete the brush from the current file and re-link it. The link/append API should probably get support for reloading a single data-block (and optionally its dependencies) but for now this is not supported yet. Pull Request: https://projects.blender.org/blender/blender/pulls/129866 --- source/blender/blenkernel/BKE_asset_edit.hh | 7 +++- .../blender/blenkernel/intern/asset_edit.cc | 38 +++++++++---------- .../editors/sculpt_paint/brush_asset_ops.cc | 10 ++++- 3 files changed, 32 insertions(+), 23 deletions(-) diff --git a/source/blender/blenkernel/BKE_asset_edit.hh b/source/blender/blenkernel/BKE_asset_edit.hh index 691421a141c..bdfc150b709 100644 --- a/source/blender/blenkernel/BKE_asset_edit.hh +++ b/source/blender/blenkernel/BKE_asset_edit.hh @@ -60,7 +60,12 @@ std::optional asset_edit_id_save_as(Main &global_main, ReportList &reports); bool asset_edit_id_save(Main &global_main, const ID &id, ReportList &reports); -bool asset_edit_id_revert(Main &global_main, ID &id, ReportList &reports); +/** + * Relink the asset from the library. This causes the ID to be re-allocated, so its address + * changes. Even in case of failure to reload the asset, \a id will be deleted. + * \return the new address of the reloaded \a id. + */ +ID *asset_edit_id_revert(Main &global_main, ID &id, ReportList &reports); bool asset_edit_id_delete(Main &global_main, ID &id, ReportList &reports); } // namespace blender::bke diff --git a/source/blender/blenkernel/intern/asset_edit.cc b/source/blender/blenkernel/intern/asset_edit.cc index 02cee89a6b5..bca5eb6e424 100644 --- a/source/blender/blenkernel/intern/asset_edit.cc +++ b/source/blender/blenkernel/intern/asset_edit.cc @@ -47,7 +47,8 @@ namespace blender::bke { static ID *asset_link_id(Main &global_main, const ID_Type id_type, const char *filepath, - const char *asset_name) + const char *asset_name, + ReportList *reports = nullptr) { /* Load asset from asset library. */ LibraryLink_Params lapp_params{}; @@ -63,7 +64,7 @@ static ID *asset_link_id(Main &global_main, BKE_blendfile_link_append_context_init_done(lapp_context); - BKE_blendfile_link(lapp_context, nullptr); + BKE_blendfile_link(lapp_context, reports); BKE_blendfile_link_append_context_finalize(lapp_context); @@ -248,24 +249,23 @@ static bool asset_write_in_library(Main &bmain, return success; } -static void asset_reload(Main &global_main, Library *lib, ReportList &reports) +static ID *asset_reload(Main &global_main, ID &id, ReportList *reports) { - /* Fill fresh main database with same datablock as before. */ - LibraryLink_Params lapp_params{}; - lapp_params.bmain = &global_main; - BlendfileLinkAppendContext *lapp_context = BKE_blendfile_link_append_context_new(&lapp_params); - BKE_blendfile_link_append_context_flag_set( - lapp_context, BLO_LIBLINK_FORCE_INDIRECT | BLO_LIBLINK_USE_PLACEHOLDERS, true); + BLI_assert(ID_IS_LINKED(&id)); - BKE_blendfile_link_append_context_library_add(lapp_context, lib->runtime.filepath_abs, nullptr); - BKE_blendfile_library_relocate(lapp_context, &reports, lib, true); - BKE_blendfile_link_append_context_free(lapp_context); + const std::string name = BKE_id_name(id); + const std::string filepath = id.lib->runtime.filepath_abs; + const ID_Type id_type = GS(id.name); - /* Clear temporary tag from relocation. */ - BKE_main_id_tag_all(&global_main, ID_TAG_PRE_EXISTING, false); + /* TODO: There's no API to reload a single data block (and its dependencies) yet. For now + * deleting the brush and re-linking it is the best way to get reloading to work. */ + BKE_id_delete(&global_main, &id); + ID *new_id = asset_link_id(global_main, id_type, filepath.c_str(), name.c_str(), reports); /* Recreate dependency graph to include new IDs. */ DEG_relations_tag_update(&global_main); + + return new_id; } static AssetWeakReference asset_weak_reference_for_user_library( @@ -357,17 +357,13 @@ bool asset_edit_id_save(Main &global_main, const ID &id, ReportList &reports) return true; } -bool asset_edit_id_revert(Main &global_main, ID &id, ReportList &reports) +ID *asset_edit_id_revert(Main &global_main, ID &id, ReportList &reports) { if (!asset_edit_id_is_editable(id)) { - return false; + return nullptr; } - /* Reload entire main, including texture dependencies. This relies on there - * being only a single asset per blend file. */ - asset_reload(global_main, id.lib, reports); - - return true; + return asset_reload(global_main, id, &reports); } bool asset_edit_id_delete(Main &global_main, ID &id, ReportList &reports) diff --git a/source/blender/editors/sculpt_paint/brush_asset_ops.cc b/source/blender/editors/sculpt_paint/brush_asset_ops.cc index 9aabe47f370..f929af3fde2 100644 --- a/source/blender/editors/sculpt_paint/brush_asset_ops.cc +++ b/source/blender/editors/sculpt_paint/brush_asset_ops.cc @@ -788,7 +788,15 @@ static int brush_asset_revert_exec(bContext *C, wmOperator *op) Paint *paint = BKE_paint_get_active_from_context(C); Brush *brush = BKE_paint_brush(paint); - bke::asset_edit_id_revert(*bmain, brush->id, *op->reports); + if (ID *reverted_id = bke::asset_edit_id_revert(*bmain, brush->id, *op->reports)) { + BLI_assert(GS(reverted_id->name) == ID_BR); + BKE_paint_brush_set(paint, reinterpret_cast(reverted_id)); + } + else { + /* bke::asset_edit_id_revert() deleted the brush for sure, even on failure. Fallback to the + * default. */ + BKE_paint_brush_set_default(bmain, paint); + } WM_main_add_notifier(NC_BRUSH | NA_EDITED, nullptr); WM_main_add_notifier(NC_TEXTURE | ND_NODES, nullptr);