Fix #102103: Copy/Paste of nodes is not handling ID references properly. #3

Closed
Bastien Montagne wants to merge 1 commits from tmp-node-copypaste into tmp-libquery-subdata-foreachid

When changing the target branch, be careful to rebase the branch in your fork to match. See documentation.

View File

@ -7,7 +7,9 @@
#include "BKE_context.hh" #include "BKE_context.hh"
#include "BKE_global.hh" #include "BKE_global.hh"
#include "BKE_lib_id.hh" #include "BKE_lib_id.hh"
#include "BKE_lib_query.hh"
#include "BKE_main.hh" #include "BKE_main.hh"
#include "BKE_main_idmap.hh"
#include "BKE_node.hh" #include "BKE_node.hh"
#include "BKE_node_runtime.hh" #include "BKE_node_runtime.hh"
#include "BKE_report.hh" #include "BKE_report.hh"
@ -25,6 +27,18 @@
namespace blender::ed::space_node { namespace blender::ed::space_node {
struct NodeClipboardItemIDInfo {
/* Name and library filepath form a unique identifer for a given ID. */
std::string id_name = "";
/* Note: Library reference is stored as an absolute path. Since the Node clippboard is runtime
* data, persistent over new blendfiles opening, this should guarantee that identical IDs from
* identical libraries can be matched accordingly, even accross several blendfiles. */
std::string library_path = "";
/* The validated ID pointer (may be the same as the original one, or a new one). */
std::optional<ID *> new_id = {};
};
struct NodeClipboardItem { struct NodeClipboardItem {
bNode *node; bNode *node;
/** /**
@ -33,7 +47,9 @@ struct NodeClipboardItem {
*/ */
rctf draw_rect; rctf draw_rect;
/* Extra info to validate the node on creation. Otherwise we may reference missing data. */ /* Extra info to validate the IDs used by the node on creation. Otherwise it may reference
* missing data. */
ID *id; ID *id;
std::string id_name; std::string id_name;
std::string library_name; std::string library_name;
@ -43,6 +59,13 @@ struct NodeClipboard {
Vector<NodeClipboardItem> nodes; Vector<NodeClipboardItem> nodes;
Vector<bNodeLink> links; Vector<bNodeLink> links;
/* A mapping of all ID references from nodes in the clipboard, to information allowing to find
* their valid matching counterpart in current Main data when pasting the nodes back. Entries are
* added when adding nodes to the clipboard, and they are updated when pasting the nodes back
* into current Main. */
Map<ID *, NodeClipboardItemIDInfo> old_ids_to_idinfo;
/** Completely empty the current clipboard content. */
void clear() void clear()
{ {
for (NodeClipboardItem &item : this->nodes) { for (NodeClipboardItem &item : this->nodes) {
@ -50,39 +73,140 @@ struct NodeClipboard {
} }
this->nodes.clear_and_shrink(); this->nodes.clear_and_shrink();
this->links.clear_and_shrink(); this->links.clear_and_shrink();
this->old_ids_to_idinfo.clear_and_shrink();
} }
/** /**
* Replace node IDs that are no longer available in the current file. Return false when one or * Find valid current pointers for all IDs used by the nodes in the clipboard.
* more IDs are lost. *
* DO NOT update the nodes' pointers here though, as this would affect the clipboard content,
* which is no desired here. It should remain as in original state, such that e.g. one can copy
* nodes in file `A.blend`, open file `B.blend`, paste nodes (and lose some of the invlaid ID
* references in file `B.blend`), and then open again file `A.blend`, paste nodes, and lose no ID
* references.
*/ */
bool validate() bool paste_validate_id_references(Main &bmain)
{ {
bool ok = true; bool is_valid = true;
IDNameLib_Map *bmain_id_map = nullptr;
/* Clear any potentially previously found `new_id` valid pointers in #old_ids_to_idinfo values,
* and populate a temporary mapping from absolute library paths to existing Library IDs in
* given Main. */
Map<std::string, Library *> libraries_path_to_id;
for (NodeClipboardItemIDInfo &id_info : this->old_ids_to_idinfo.values()) {
id_info.new_id.reset();
if (!id_info.library_path.empty() && !libraries_path_to_id.contains(id_info.library_path)) {
libraries_path_to_id.add(
id_info.library_path,
static_cast<Library *>(BLI_findstring(&bmain.libraries,
id_info.library_path.c_str(),
offsetof(Library, runtime.filepath_abs))));
}
}
/* Find a new valid ID pointer for all ID usages in given node.
*
* NOTE: Due to the fact that the clipboard survives file loading, only name (including IDType)
* and libpath pairs can be used here.
* - UID cannot be trusted accross file load.
* - ID pointer itself cannot be trusted accross undo/redo and fileload. */
auto validate_id_fn = [this, &is_valid, &bmain, &bmain_id_map, &libraries_path_to_id](
LibraryIDLinkCallbackData *cb_data) -> int {
ID *old_id = *(cb_data->id_pointer);
if (!old_id) {
return IDWALK_RET_NOP;
}
if (!this->old_ids_to_idinfo.contains(old_id)) {
BLI_assert_msg(
0, "Missing entry in the old ID data of the node clipboard, should not happen");
is_valid = false;
return IDWALK_RET_NOP;
}
NodeClipboardItemIDInfo &id_info = this->old_ids_to_idinfo.lookup(old_id);
if (!id_info.new_id) {
if (!bmain_id_map) {
bmain_id_map = BKE_main_idmap_create(&bmain, false, nullptr, MAIN_IDMAP_TYPE_NAME);
}
Library *new_id_lib = libraries_path_to_id.lookup_default(id_info.library_path, nullptr);
if (id_info.library_path.empty() || new_id_lib) {
id_info.new_id = BKE_main_idmap_lookup_name(
bmain_id_map, GS(id_info.id_name.c_str()), id_info.id_name.c_str() + 2, new_id_lib);
}
else {
/* No matching library found, so there is no possible matching ID either. */
id_info.new_id = nullptr;
}
}
if (*(id_info.new_id) == nullptr) {
is_valid = false;
}
return IDWALK_RET_NOP;
};
for (NodeClipboardItem &item : this->nodes) { for (NodeClipboardItem &item : this->nodes) {
bNode &node = *item.node; BKE_library_foreach_subdata_id(
/* Reassign each loop since we may clear, open a new file where the ID is valid, and paste &bmain,
* again. */ nullptr,
node.id = item.id; nullptr,
[&item](LibraryForeachIDData *data) { bke::node_node_foreach_id(item.node, data); },
if (node.id) { validate_id_fn,
const ListBase *lb = which_libbase(G_MAIN, GS(item.id_name.c_str())); nullptr,
if (BLI_findindex(lb, item.id) == -1) { IDWALK_READONLY);
/* May assign null. */
node.id = static_cast<ID *>(
BLI_findstring(lb, item.id_name.c_str() + 2, offsetof(ID, name) + 2));
if (!node.id) {
ok = false;
}
}
}
} }
return ok; if (bmain_id_map) {
BKE_main_idmap_destroy(bmain_id_map);
bmain_id_map = nullptr;
}
return is_valid;
} }
void add_node(const bNode &node, /**
* Ensure that a newly pasted copy of a node from the clipboard has valid ID references, as
* ensured by #paste_validate_id_references.
*/
void paste_update_node_id_references(bNode &node)
{
/* Update all old ID pointers in given node by new, valid ones. */
auto update_id_fn = [this](LibraryIDLinkCallbackData *cb_data) -> int {
ID *old_id = *(cb_data->id_pointer);
if (!old_id) {
return IDWALK_RET_NOP;
}
if (!this->old_ids_to_idinfo.contains(old_id)) {
BLI_assert_msg(
0, "Missing entry in the old ID data of the node clipboard, should not happen");
*(cb_data->id_pointer) = nullptr;
return IDWALK_RET_NOP;
}
NodeClipboardItemIDInfo &id_info = this->old_ids_to_idinfo.lookup(old_id);
if (!id_info.new_id) {
BLI_assert_msg(
0,
"Unset new ID value for an old ID reference in the node clipboard, should not happen");
*(cb_data->id_pointer) = nullptr;
return IDWALK_RET_NOP;
}
*(cb_data->id_pointer) = *(id_info.new_id);
if (cb_data->cb_flag & IDWALK_CB_USER) {
id_us_plus(*(cb_data->id_pointer));
}
return IDWALK_RET_NOP;
};
BKE_library_foreach_subdata_id(
nullptr,
nullptr,
nullptr,
[&node](LibraryForeachIDData *data) { bke::node_node_foreach_id(&node, data); },
update_id_fn,
nullptr,
IDWALK_NOP);
}
/** Add a new node to the clipboard. */
void copy_add_node(const bNode &node,
Map<const bNode *, bNode *> &node_map, Map<const bNode *, bNode *> &node_map,
Map<const bNodeSocket *, bNodeSocket *> &socket_map) Map<const bNodeSocket *, bNodeSocket *> &socket_map)
{ {
@ -92,16 +216,39 @@ struct NodeClipboard {
nullptr, node, LIB_ID_CREATE_NO_USER_REFCOUNT | LIB_ID_CREATE_NO_MAIN, false, socket_map); nullptr, node, LIB_ID_CREATE_NO_USER_REFCOUNT | LIB_ID_CREATE_NO_MAIN, false, socket_map);
node_map.add_new(&node, new_node); node_map.add_new(&node, new_node);
/* Find a new valid ID pointer for all ID usages in given node. */
auto ensure_id_info_fn = [this](LibraryIDLinkCallbackData *cb_data) -> int {
ID *old_id = *(cb_data->id_pointer);
if (!old_id) {
}
if (this->old_ids_to_idinfo.contains(old_id)) {
return IDWALK_RET_NOP;
}
NodeClipboardItemIDInfo id_info;
if (old_id) {
id_info.id_name = old_id->name;
if (ID_IS_LINKED(old_id)) {
id_info.library_path = old_id->lib->runtime.filepath_abs;
}
}
this->old_ids_to_idinfo.add(old_id, std::move(id_info));
return IDWALK_RET_NOP;
};
BKE_library_foreach_subdata_id(
nullptr,
nullptr,
nullptr,
[&node](LibraryForeachIDData *data) {
bke::node_node_foreach_id(const_cast<bNode *>(&node), data);
},
ensure_id_info_fn,
nullptr,
IDWALK_READONLY);
NodeClipboardItem item; NodeClipboardItem item;
item.draw_rect = node.runtime->totr; item.draw_rect = node.runtime->totr;
item.node = new_node; item.node = new_node;
item.id = new_node->id;
if (item.id) {
item.id_name = new_node->id->name;
if (ID_IS_LINKED(new_node->id)) {
item.library_name = new_node->id->lib->runtime.filepath_abs;
}
}
this->nodes.append(std::move(item)); this->nodes.append(std::move(item));
} }
}; };
@ -129,7 +276,7 @@ static int node_clipboard_copy_exec(bContext *C, wmOperator * /*op*/)
for (const bNode *node : tree.all_nodes()) { for (const bNode *node : tree.all_nodes()) {
if (node->flag & SELECT) { if (node->flag & SELECT) {
clipboard.add_node(*node, node_map, socket_map); clipboard.copy_add_node(*node, node_map, socket_map);
} }
} }
@ -184,21 +331,20 @@ void NODE_OT_clipboard_copy(wmOperatorType *ot)
static int node_clipboard_paste_exec(bContext *C, wmOperator *op) static int node_clipboard_paste_exec(bContext *C, wmOperator *op)
{ {
Main *bmain = CTX_data_main(C);
SpaceNode &snode = *CTX_wm_space_node(C); SpaceNode &snode = *CTX_wm_space_node(C);
bNodeTree &tree = *snode.edittree; bNodeTree &tree = *snode.edittree;
NodeClipboard &clipboard = get_node_clipboard(); NodeClipboard &clipboard = get_node_clipboard();
const bool is_valid = clipboard.validate();
if (clipboard.nodes.is_empty()) { if (clipboard.nodes.is_empty()) {
BKE_report(op->reports, RPT_ERROR, "The internal clipboard is empty"); BKE_report(op->reports, RPT_ERROR, "The internal clipboard is empty");
return OPERATOR_CANCELLED; return OPERATOR_CANCELLED;
} }
if (!is_valid) { if (!clipboard.paste_validate_id_references(*bmain)) {
BKE_report(op->reports, BKE_report(op->reports,
RPT_WARNING, RPT_WARNING,
"Some nodes references could not be restored, will be left empty"); "Some nodes references to other IDs could not be restored, will be left empty");
} }
ED_preview_kill_jobs(CTX_wm_manager(C), CTX_data_main(C)); ED_preview_kill_jobs(CTX_wm_manager(C), CTX_data_main(C));
@ -214,8 +360,12 @@ static int node_clipboard_paste_exec(bContext *C, wmOperator *op)
const char *disabled_hint = nullptr; const char *disabled_hint = nullptr;
if (node.typeinfo->poll_instance && node.typeinfo->poll_instance(&node, &tree, &disabled_hint)) if (node.typeinfo->poll_instance && node.typeinfo->poll_instance(&node, &tree, &disabled_hint))
{ {
/* Do not access referenced ID pointers here, as they are still the old ones, which may be
* invalid. */
bNode *new_node = bke::node_copy_with_mapping( bNode *new_node = bke::node_copy_with_mapping(
&tree, node, LIB_ID_COPY_DEFAULT, true, socket_map); &tree, node, LIB_ID_CREATE_NO_USER_REFCOUNT, true, socket_map);
/* Update the newly copied node's ID references. */
clipboard.paste_update_node_id_references(*new_node);
/* Reset socket shape in case a node is copied to a different tree type. */ /* Reset socket shape in case a node is copied to a different tree type. */
LISTBASE_FOREACH (bNodeSocket *, socket, &new_node->inputs) { LISTBASE_FOREACH (bNodeSocket *, socket, &new_node->inputs) {
socket->display_shape = SOCK_DISPLAY_SHAPE_CIRCLE; socket->display_shape = SOCK_DISPLAY_SHAPE_CIRCLE;
@ -306,7 +456,6 @@ static int node_clipboard_paste_exec(bContext *C, wmOperator *op)
update_multi_input_indices_for_removed_links(*new_node); update_multi_input_indices_for_removed_links(*new_node);
} }
Main *bmain = CTX_data_main(C);
ED_node_tree_propagate_change(C, bmain, &tree); ED_node_tree_propagate_change(C, bmain, &tree);
/* Pasting nodes can create arbitrary new relations because nodes can reference IDs. */ /* Pasting nodes can create arbitrary new relations because nodes can reference IDs. */
DEG_relations_tag_update(bmain); DEG_relations_tag_update(bmain);
@ -358,6 +507,5 @@ void ED_node_clipboard_free()
{ {
using namespace blender::ed::space_node; using namespace blender::ed::space_node;
NodeClipboard &clipboard = get_node_clipboard(); NodeClipboard &clipboard = get_node_clipboard();
clipboard.validate();
clipboard.clear(); clipboard.clear();
} }