From acd44cf788494de4cadb1d0265aa9be127948e83 Mon Sep 17 00:00:00 2001 From: Brecht Van Lommel Date: Fri, 22 Mar 2024 18:57:31 +0100 Subject: [PATCH 1/2] Brush Assets: Make revert and delete operators affect all datablocks This is done by reloading or freeing the entire main of the asset, and remapping pointers for all datablocks contained in it. This remapping is primarily needed for the user interface to point to the same datablocks. --- source/blender/blenkernel/BKE_asset.hh | 4 +- .../blenkernel/intern/asset_weak_reference.cc | 109 ++++++++++++++++-- source/blender/blenkernel/intern/blender.cc | 2 +- .../blender/editors/sculpt_paint/paint_ops.cc | 25 +--- 4 files changed, 111 insertions(+), 29 deletions(-) diff --git a/source/blender/blenkernel/BKE_asset.hh b/source/blender/blenkernel/BKE_asset.hh index 6e663a4e459..537fe00eca4 100644 --- a/source/blender/blenkernel/BKE_asset.hh +++ b/source/blender/blenkernel/BKE_asset.hh @@ -86,7 +86,9 @@ void BKE_asset_weak_reference_read(BlendDataReader *reader, AssetWeakReference * * currently used for brush assets and their dependencies. */ Main *BKE_asset_weak_reference_main(const ID *id); -void BKE_asset_weak_reference_main_free(); ID *BKE_asset_weak_reference_ensure(Main &global_main, ID_Type id_type, const AssetWeakReference &weak_ref); +void BKE_asset_weak_reference_main_reload(Main &global_main, Main &main); +void BKE_asset_weak_reference_main_free(Main &global_main, Main &asset_main); +void BKE_asset_weak_reference_main_free_all(); diff --git a/source/blender/blenkernel/intern/asset_weak_reference.cc b/source/blender/blenkernel/intern/asset_weak_reference.cc index 2accc9429d8..d44fb382525 100644 --- a/source/blender/blenkernel/intern/asset_weak_reference.cc +++ b/source/blender/blenkernel/intern/asset_weak_reference.cc @@ -9,7 +9,9 @@ #include #include +#include "BLI_path_util.h" #include "BLI_string.h" +#include "BLI_vector.hh" #include "DNA_space_types.h" @@ -20,10 +22,9 @@ #include "BKE_blendfile_link_append.hh" #include "BKE_idtype.hh" #include "BKE_lib_id.hh" +#include "BKE_lib_remap.hh" #include "BKE_main.hh" -#include "BLI_vector.hh" - #include "BLO_read_write.hh" #include "BLO_readfile.hh" @@ -132,12 +133,9 @@ void BKE_asset_weak_reference_read(BlendDataReader *reader, AssetWeakReference * /* Main database for storing assets that are weak referenced. * * This avoids mixing asset datablocks in the regular main, which leads to naming conflicts and - * confusing user interface. - * - * TODO: Heavily WIP code. */ + * confusing user interface. */ struct AssetWeakReferenceMain { - /* TODO: not sure if this is the best unique identifier. */ std::string filepath; Main *main; @@ -145,12 +143,19 @@ struct AssetWeakReferenceMain { : filepath(std::move(filepath)), main(BKE_main_new()) { main->is_asset_weak_reference_main = true; + BLI_assert(!BLI_path_is_rel(filepath.c_str())); } AssetWeakReferenceMain(const AssetWeakReferenceMain &) = delete; AssetWeakReferenceMain(AssetWeakReferenceMain &&other) : filepath(std::exchange(other.filepath, "")), main(std::exchange(other.main, nullptr)) { } + AssetWeakReferenceMain &operator=(AssetWeakReferenceMain &&other) + { + this->filepath = std::exchange(other.filepath, ""); + this->main = std::exchange(other.main, nullptr); + return *this; + } ~AssetWeakReferenceMain() { @@ -158,8 +163,70 @@ struct AssetWeakReferenceMain { BKE_main_free(main); } } + + void reload(Main &global_main); + void clear_users(Main &global_main); }; +void AssetWeakReferenceMain::reload(Main &global_main) +{ + Main *old_main = this->main; + this->main = BKE_main_new(); + this->main->is_asset_weak_reference_main = true; + + /* Fill fresh main database with same datablock as before. */ + LibraryLink_Params lapp_params{}; + lapp_params.bmain = this->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, true); + BKE_blendfile_link_append_context_flag_set(lapp_context, 0, true); + + BKE_blendfile_link_append_context_library_add(lapp_context, this->filepath.c_str(), nullptr); + + /* Requests all existing datablocks to be append again. */ + ID *old_id; + FOREACH_MAIN_ID_BEGIN (old_main, old_id) { + ID_Type old_id_code = GS(old_id->name); + if (BKE_idtype_idcode_is_linkable(old_id_code)) { + BlendfileLinkAppendContextItem *lapp_item = BKE_blendfile_link_append_context_item_add( + lapp_context, old_id->name + 2, old_id_code, nullptr); + BKE_blendfile_link_append_context_item_library_index_enable(lapp_context, lapp_item, 0); + } + } + FOREACH_MAIN_ID_END; + + BKE_blendfile_link(lapp_context, nullptr); + BKE_blendfile_append(lapp_context, nullptr); + + BKE_blendfile_link_append_context_free(lapp_context); + + BKE_main_id_tag_all(this->main, LIB_TAG_ASSET_MAIN, true); + + /* Remap old to new. */ + bke::id::IDRemapper mappings; + FOREACH_MAIN_ID_BEGIN (old_main, old_id) { + ID *new_id = BKE_libblock_find_name(this->main, GS(old_id->name), old_id->name + 2); + mappings.add(old_id, new_id); + } + FOREACH_MAIN_ID_END; + BKE_libblock_remap_multiple(&global_main, mappings, 0); + + /* Free old database. */ + BKE_main_free(old_main); +} + +void AssetWeakReferenceMain::clear_users(Main &global_main) +{ + /* Remap old to null pointer. */ + bke::id::IDRemapper mappings; + ID *old_id; + FOREACH_MAIN_ID_BEGIN (this->main, old_id) { + mappings.add(old_id, nullptr); + } + FOREACH_MAIN_ID_END; + BKE_libblock_remap_multiple(&global_main, mappings, 0); +} + static Vector &get_weak_reference_mains() { static Vector mains; @@ -196,7 +263,34 @@ static Main &asset_weak_reference_main_ensure(const StringRef filepath) return *get_weak_reference_mains().last().main; } -void BKE_asset_weak_reference_main_free() +void BKE_asset_weak_reference_main_reload(Main &global_main, Main &asset_main) +{ + for (AssetWeakReferenceMain &weak_ref_main : get_weak_reference_mains()) { + if (weak_ref_main.main == &asset_main) { + weak_ref_main.reload(global_main); + return; + } + } + + BLI_assert_unreachable(); +} + +void BKE_asset_weak_reference_main_free(Main &global_main, Main &asset_main) +{ + int index = 0; + for (AssetWeakReferenceMain &weak_ref_main : get_weak_reference_mains()) { + if (weak_ref_main.main == &asset_main) { + weak_ref_main.clear_users(global_main); + get_weak_reference_mains().remove(index); + return; + } + index++; + } + + BLI_assert_unreachable(); +} + +void BKE_asset_weak_reference_main_free_all() { get_weak_reference_mains().clear_and_shrink(); } @@ -247,7 +341,6 @@ ID *BKE_asset_weak_reference_ensure(Main &global_main, BKE_blendfile_link_append_context_free(lapp_context); - /* TODO: only do for new ones? */ BKE_main_id_tag_all(&bmain, LIB_TAG_ASSET_MAIN, true); /* Verify that the name matches. It must for referencing the same asset again to work. */ diff --git a/source/blender/blenkernel/intern/blender.cc b/source/blender/blenkernel/intern/blender.cc index 5982589d583..4a79b0c390f 100644 --- a/source/blender/blenkernel/intern/blender.cc +++ b/source/blender/blenkernel/intern/blender.cc @@ -64,7 +64,7 @@ void BKE_blender_free() /* Free separate data-bases after #BKE_blender_globals_clear in case any data-blocks in the * global main use pointers to asset main data-blocks when they're freed. That generally * shouldn't happen but it's better to be safe. */ - BKE_asset_weak_reference_main_free(); + BKE_asset_weak_reference_main_free_all(); if (G.log.file != nullptr) { fclose(static_cast(G.log.file)); diff --git a/source/blender/editors/sculpt_paint/paint_ops.cc b/source/blender/editors/sculpt_paint/paint_ops.cc index eba648d2c30..877c71d6865 100644 --- a/source/blender/editors/sculpt_paint/paint_ops.cc +++ b/source/blender/editors/sculpt_paint/paint_ops.cc @@ -34,7 +34,6 @@ #include "BKE_context.hh" #include "BKE_image.h" #include "BKE_lib_id.hh" -#include "BKE_lib_override.hh" #include "BKE_lib_remap.hh" #include "BKE_main.hh" #include "BKE_paint.hh" @@ -1248,11 +1247,8 @@ static int brush_asset_delete_exec(bContext *C, wmOperator *op) } if (asset_main != bmain) { - // TODO: hack: no pointer should exist, should do runtime lookup - BKE_libblock_remap(bmain, brush, nullptr, 0); + BKE_asset_weak_reference_main_free(*bmain, *asset_main); } - BKE_id_delete(asset_main, brush); - // TODO: delete whole asset main if empty? refresh_asset_library(C, *library); @@ -1349,14 +1345,13 @@ static void BRUSH_OT_asset_update(wmOperatorType *ot) static bool brush_asset_revert_poll(bContext *C) { - /* TODO: check if there is anything to revert? */ Paint *paint = BKE_paint_get_active_from_context(C); Brush *brush = (paint) ? BKE_paint_brush(paint) : nullptr; if (paint == nullptr || brush == nullptr) { return false; } - return paint->brush_asset_reference && ID_IS_ASSET(brush); + return paint->brush_asset_reference && (brush->id.tag & LIB_TAG_ASSET_MAIN); } static int brush_asset_revert_exec(bContext *C, wmOperator * /*op*/) @@ -1366,19 +1361,11 @@ static int brush_asset_revert_exec(bContext *C, wmOperator * /*op*/) Brush *brush = BKE_paint_brush(paint); Main *asset_main = BKE_main_from_id(bmain, &brush->id); - // TODO: delete and reload dependencies too? - // TODO: hack to make remapping work, should not be needed - // if we can make brush pointer not part of ID management at all - BLI_remlink(&asset_main->brushes, brush); + /* Reload entire main, including texture dependencies. */ + BKE_asset_weak_reference_main_reload(*bmain, *asset_main); - Brush *new_brush = reinterpret_cast( - BKE_asset_weak_reference_ensure(*bmain, ID_BR, *paint->brush_asset_reference)); - - BKE_libblock_remap(bmain, brush, new_brush, 0); - BLI_addtail(&asset_main->brushes, brush); - BKE_id_delete(asset_main, brush); - - WM_main_add_notifier(NC_BRUSH | NA_EDITED, new_brush); + WM_main_add_notifier(NC_BRUSH | NA_EDITED, nullptr); + WM_main_add_notifier(NC_TEXTURE | ND_NODES, nullptr); return OPERATOR_FINISHED; } -- 2.30.2 From 6c9744feee7842b13325384ec757378ad2617080 Mon Sep 17 00:00:00 2001 From: Brecht Van Lommel Date: Mon, 25 Mar 2024 11:53:57 +0100 Subject: [PATCH 2/2] Address comments --- source/blender/blenkernel/intern/asset_weak_reference.cc | 5 ++++- source/blender/editors/sculpt_paint/paint_ops.cc | 3 ++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/source/blender/blenkernel/intern/asset_weak_reference.cc b/source/blender/blenkernel/intern/asset_weak_reference.cc index d44fb382525..a8ecb5eac86 100644 --- a/source/blender/blenkernel/intern/asset_weak_reference.cc +++ b/source/blender/blenkernel/intern/asset_weak_reference.cc @@ -152,6 +152,9 @@ struct AssetWeakReferenceMain { } AssetWeakReferenceMain &operator=(AssetWeakReferenceMain &&other) { + if (this == &other) { + return *this; + } this->filepath = std::exchange(other.filepath, ""); this->main = std::exchange(other.main, nullptr); return *this; @@ -183,7 +186,7 @@ void AssetWeakReferenceMain::reload(Main &global_main) BKE_blendfile_link_append_context_library_add(lapp_context, this->filepath.c_str(), nullptr); - /* Requests all existing datablocks to be append again. */ + /* Requests all existing datablocks to be appended again. */ ID *old_id; FOREACH_MAIN_ID_BEGIN (old_main, old_id) { ID_Type old_id_code = GS(old_id->name); diff --git a/source/blender/editors/sculpt_paint/paint_ops.cc b/source/blender/editors/sculpt_paint/paint_ops.cc index 877c71d6865..c5213b85b95 100644 --- a/source/blender/editors/sculpt_paint/paint_ops.cc +++ b/source/blender/editors/sculpt_paint/paint_ops.cc @@ -1361,7 +1361,8 @@ static int brush_asset_revert_exec(bContext *C, wmOperator * /*op*/) Brush *brush = BKE_paint_brush(paint); Main *asset_main = BKE_main_from_id(bmain, &brush->id); - /* Reload entire main, including texture dependencies. */ + /* Reload entire main, including texture dependencies. This relies on there + * being only a single brush asset per blend file. */ BKE_asset_weak_reference_main_reload(*bmain, *asset_main); WM_main_add_notifier(NC_BRUSH | NA_EDITED, nullptr); -- 2.30.2