From a6cea64b3ad745615acbce48342b8100fff4cb3a Mon Sep 17 00:00:00 2001 From: Bastien Montagne Date: Tue, 7 Oct 2025 16:11:47 +0200 Subject: [PATCH] Fix #147461: Implement packed data support for generic ID copy-paste. Adding support for packed IDs in the PartialWriteContext was completely missed in initial work... This somewhat complexifies the handling of libraries in PartialWriteContext, as packed IDs need to be handled in specific ways. It also exposes `bke::library::ensure_archive_library` as a public function, to avoid duplicating this complex logic of finding or creating a new archive library for a given ID. Finally, prevent attempts to copy a packed ID for now, as pasting them is not supported (linking/appending these directly is not supported). Pull Request: https://projects.blender.org/blender/blender/pulls/147468 --- source/blender/blenkernel/BKE_library.hh | 14 +++++ .../blender/blenkernel/intern/asset_edit.cc | 3 ++ source/blender/blenkernel/intern/blendfile.cc | 52 +++++++++++++++++-- source/blender/blenkernel/intern/library.cc | 31 ++++++----- .../editors/armature/pose_transform.cc | 6 +++ .../blender/editors/render/render_shading.cc | 5 ++ .../editors/space_outliner/outliner_edit.cc | 5 ++ 7 files changed, 99 insertions(+), 17 deletions(-) diff --git a/source/blender/blenkernel/BKE_library.hh b/source/blender/blenkernel/BKE_library.hh index 5895579356e..e400ae87633 100644 --- a/source/blender/blenkernel/BKE_library.hh +++ b/source/blender/blenkernel/BKE_library.hh @@ -92,6 +92,20 @@ void pack_linked_id_hierarchy(Main &bmain, ID &root_id); */ void main_cleanup_parent_archives(Main &bmain); +/** + * Ensure that there is a valid archive library in given `bmain`, for the given `id`, + * `reference_library` and `id_deep_hash` parameters. + * + * \note Typically, both the `reference_library` and `id_deep_hash` are the same as the `id` + * library and deephash, but in some cases they may still differ (see e.g. + * #PartialWriteContext::ensure_library). + * + * \return the archive library. `is_new` is set to `true` if a new archive library had to be + * created, false if an existing one could be re-used. + */ +Library *ensure_archive_library( + Main &bmain, ID &id, Library &reference_library, const IDHash &id_deep_hash, bool &is_new); + }; // namespace blender::bke::library /** #LibraryRuntime.tag */ diff --git a/source/blender/blenkernel/intern/asset_edit.cc b/source/blender/blenkernel/intern/asset_edit.cc index f964a0ac6ce..e0a7bf89d8b 100644 --- a/source/blender/blenkernel/intern/asset_edit.cc +++ b/source/blender/blenkernel/intern/asset_edit.cc @@ -160,6 +160,9 @@ static bool asset_write_in_library(Main &bmain, ID &id = const_cast(id_const); + /* This is not expected to ever happen currently from this codepath. */ + BLI_assert(!ID_IS_PACKED(&id)); + PartialWriteContext lib_write_ctx{bmain}; ID *new_id = lib_write_ctx.id_add(&id, {(PartialWriteContext::IDAddOperations::MAKE_LOCAL | diff --git a/source/blender/blenkernel/intern/blendfile.cc b/source/blender/blenkernel/intern/blendfile.cc index 8c20c2a5f0b..f4d5497d16d 100644 --- a/source/blender/blenkernel/intern/blendfile.cc +++ b/source/blender/blenkernel/intern/blendfile.cc @@ -1846,6 +1846,10 @@ ID *PartialWriteContext::id_add_copy(const ID *id, const bool regenerate_session LIB_ID_COPY_ASSET_METADATA); ctx_root_id = BKE_id_copy_in_lib(nullptr, id->lib, id, std::nullopt, nullptr, copy_flags); ctx_root_id->tag |= ID_TAG_TEMP_MAIN; + /* It is critical to preserve the deep hash here, as the copy put in the partial write context is + * expected to be a perfect duplicate of the packed ID (including all of its dependencies). This + * will also be used on paste for deduplication. */ + ctx_root_id->deep_hash = id->deep_hash; /* Ensure that the newly copied ID has a library in temp local bmain if it was linked. * While this could be optimized out in case the ID is made local in the context, this adds * complexity as default ID management code like 'make local' code will create invalid bmain @@ -1896,12 +1900,39 @@ Library *PartialWriteContext::ensure_library(ID *ctx_id) if (!ID_IS_LINKED(ctx_id)) { return nullptr; } - blender::StringRefNull lib_path = ctx_id->lib->runtime->filepath_abs; - Library *ctx_lib = this->libraries_map_.lookup_default(lib_path, nullptr); - if (!ctx_lib) { - ctx_lib = reinterpret_cast(id_add_copy(&ctx_id->lib->id, true)); - this->libraries_map_.add(lib_path, ctx_lib); + + Library *src_lib = ctx_id->lib; + const bool is_archive_lib = (src_lib->flag & LIBRARY_FLAG_IS_ARCHIVE) != 0; + Library *src_base_lib = is_archive_lib ? src_lib->archive_parent_library : src_lib; + BLI_assert(src_base_lib); + BLI_assert((is_archive_lib && src_lib != src_base_lib && !ctx_id->deep_hash.is_null()) || + (!is_archive_lib && src_lib == src_base_lib && ctx_id->deep_hash.is_null())); + + blender::StringRefNull lib_path = src_base_lib->runtime->filepath_abs; + Library *ctx_base_lib = this->libraries_map_.lookup_default(lib_path, nullptr); + if (!ctx_base_lib) { + ctx_base_lib = reinterpret_cast(id_add_copy(&src_base_lib->id, true)); + this->libraries_map_.add(lib_path, ctx_base_lib); } + /* The mapping should only contain real libraries, never packed ones. */ + BLI_assert(!ctx_base_lib || (ctx_base_lib->flag & LIBRARY_FLAG_IS_ARCHIVE) == 0); + + /* There is a valid context base library, now find or create a valid archived library if needed. + */ + Library *ctx_lib = ctx_base_lib; + if (is_archive_lib) { + /* Leave the creation of a new archive library to the Library code, when needed, instead of + * using the write context's own `id_add_copy` util. Both are doing different and complex + * things, but for archive libraries the Library code should be mostly usable 'as-is'. */ + bool is_new = false; + ctx_lib = blender::bke::library::ensure_archive_library( + this->bmain, *ctx_id, *ctx_lib, ctx_id->deep_hash, is_new); + if (is_new) { + ctx_lib->id.tag |= ID_TAG_TEMP_MAIN; + BKE_main_idmap_insert_id(this->bmain.id_map, &ctx_lib->id); + } + } + ctx_id->lib = ctx_lib; return ctx_lib; } @@ -1942,6 +1973,9 @@ ID *PartialWriteContext::id_add( } /* The given ID may have already been added (either explicitly or as a dependency) before. */ + /* NOTE: This should not be needed currently (as this is only used as temporary partial copy of + * the current main Main data-base, so ID's runtime session_uid should be enough), but in the + * future it might also be good to lookup by ID deep hash for packed data? */ ID *ctx_root_id = BKE_main_idmap_lookup_uid(matching_uid_map_, id->session_uid); if (ctx_root_id) { /* If the root orig ID is already in the context, assume all of its dependencies are as well. @@ -1996,6 +2030,14 @@ ID *PartialWriteContext::id_add( cb_data, options); operations_final = ((operations_per_id & MASK_PER_ID_USAGE) | (operations_final & ~MASK_PER_ID_USAGE)); + if (ID_IS_PACKED(orig_deps_id) && (operations_final & MAKE_LOCAL) == 0) { + /* To ensure that their deep hash still matches with their 'context' copy, packed IDs that + * are not made local (i.e. 'unpacked'): + * - Must also include all of their dependencies. + * - Should never duplicate or clear their dependencies. */ + operations_final |= ADD_DEPENDENCIES; + operations_final &= ~(DUPLICATE_DEPENDENCIES | CLEAR_DEPENDENCIES); + } } const bool add_dependencies = (operations_final & ADD_DEPENDENCIES) != 0; diff --git a/source/blender/blenkernel/intern/library.cc b/source/blender/blenkernel/intern/library.cc index 5e69ba8b578..ea97c06a697 100644 --- a/source/blender/blenkernel/intern/library.cc +++ b/source/blender/blenkernel/intern/library.cc @@ -460,36 +460,41 @@ static Library *add_archive_library(Main &bmain, Library &reference_library) return archive_library; } -static Library *get_archive_library(Main &bmain, ID *for_id, const IDHash &for_id_deep_hash) +Library *blender::bke::library::ensure_archive_library( + Main &bmain, ID &id, Library &reference_library, const IDHash &id_deep_hash, bool &is_new) { - Library *reference_library = for_id->lib; - BLI_assert(reference_library && (reference_library->flag & LIBRARY_FLAG_IS_ARCHIVE) == 0); + BLI_assert(ID_IS_LINKED(&id)); + BLI_assert((reference_library.flag & LIBRARY_FLAG_IS_ARCHIVE) == 0); Library *archive_library = nullptr; - for (Library *lib_iter : reference_library->runtime->archived_libraries) { + for (Library *lib_iter : reference_library.runtime->archived_libraries) { BLI_assert((lib_iter->flag & LIBRARY_FLAG_IS_ARCHIVE) != 0); BLI_assert(lib_iter->archive_parent_library != nullptr); - BLI_assert(lib_iter->archive_parent_library == reference_library); + BLI_assert(lib_iter->archive_parent_library == &reference_library); /* Check if current archive library already contains an ID of same type and name. */ - if (BKE_main_namemap_contain_name(bmain, lib_iter, GS(for_id->name), BKE_id_name(*for_id))) { + if (BKE_main_namemap_contain_name(bmain, lib_iter, GS(id.name), BKE_id_name(id))) { #ifndef NDEBUG ID *packed_id = BKE_libblock_find_name_and_library( - &bmain, GS(for_id->name), BKE_id_name(*for_id), BKE_id_name(lib_iter->id)); + &bmain, GS(id.name), BKE_id_name(id), BKE_id_name(lib_iter->id)); BLI_assert_msg( - packed_id && packed_id->deep_hash != for_id_deep_hash, + packed_id && packed_id->deep_hash != id_deep_hash, "An already packed ID with same deep hash as the one to be packed, should have already " "be found and used (deduplication) before reaching this code-path"); #endif - UNUSED_VARS_NDEBUG(for_id_deep_hash); + UNUSED_VARS_NDEBUG(id_deep_hash); continue; } archive_library = lib_iter; break; } if (!archive_library) { - archive_library = add_archive_library(bmain, *reference_library); + archive_library = add_archive_library(bmain, reference_library); + is_new = true; } - BLI_assert(reference_library->runtime->archived_libraries.contains(archive_library)); + else { + is_new = false; + } + BLI_assert(reference_library.runtime->archived_libraries.contains(archive_library)); return archive_library; } @@ -540,7 +545,9 @@ static void pack_linked_id(Main &bmain, /* Find an existing archive Library not containing a 'version' of this ID yet (to prevent names * collisions). */ - Library *archive_lib = get_archive_library(bmain, linked_id, linked_id_deep_hash); + bool is_new; + Library *archive_lib = ensure_archive_library( + bmain, *linked_id, *linked_id->lib, linked_id_deep_hash, is_new); auto copied_id_process = [&archive_lib, &deep_hashes, &ids_to_remap, &id_remapper, &already_packed_ids]( diff --git a/source/blender/editors/armature/pose_transform.cc b/source/blender/editors/armature/pose_transform.cc index d647f3a3d10..6d6bfe184b5 100644 --- a/source/blender/editors/armature/pose_transform.cc +++ b/source/blender/editors/armature/pose_transform.cc @@ -797,6 +797,12 @@ static wmOperatorStatus pose_copy_exec(bContext *C, wmOperator *op) BKE_report(op->reports, RPT_ERROR, "No pose to copy"); return OPERATOR_CANCELLED; } + if (ID_IS_PACKED(&ob->id)) { + /* Direct link/append of packed IDs is not supported currently, so neither is their + * copy/pasting. */ + BKE_report(op->reports, RPT_ERROR, "Cannot copy/paste packed data"); + return OPERATOR_CANCELLED; + } /* Sets chan->flag to POSE_KEY if bone selected. */ set_pose_keys(ob); diff --git a/source/blender/editors/render/render_shading.cc b/source/blender/editors/render/render_shading.cc index 2b8fe615756..94e79f5a508 100644 --- a/source/blender/editors/render/render_shading.cc +++ b/source/blender/editors/render/render_shading.cc @@ -2716,6 +2716,11 @@ static wmOperatorStatus copy_material_exec(bContext *C, wmOperator *op) if (ma == nullptr) { return OPERATOR_CANCELLED; } + if (ID_IS_PACKED(&ma->id)) { + /* Direct link/append of packed IDs is not supported currently, so neither is their + * copy/pasting. */ + return OPERATOR_CANCELLED; + } Main *bmain = CTX_data_main(C); PartialWriteContext copybuffer{*bmain}; diff --git a/source/blender/editors/space_outliner/outliner_edit.cc b/source/blender/editors/space_outliner/outliner_edit.cc index 2b3d85a3d32..03d6a9aa66f 100644 --- a/source/blender/editors/space_outliner/outliner_edit.cc +++ b/source/blender/editors/space_outliner/outliner_edit.cc @@ -817,6 +817,11 @@ static int outliner_id_copy_tag(SpaceOutliner *space_outliner, /* Add selected item and all of its dependencies to the copy buffer. */ if (tselem->flag & TSE_SELECTED && ELEM(tselem->type, TSE_SOME_ID, TSE_LAYER_COLLECTION)) { + if (ID_IS_PACKED(tselem->id)) { + /* Direct link/append of packed IDs is not supported currently, so neither is their + * copy/pasting. */ + continue; + } copybuffer.id_add(tselem->id, PartialWriteContext::IDAddOptions{ (PartialWriteContext::IDAddOperations::SET_FAKE_USER |