From f50699d80a20a933af089d403dbceebc4c45992b Mon Sep 17 00:00:00 2001 From: Jacques Lucke Date: Wed, 23 Jul 2025 21:26:51 +0200 Subject: [PATCH] Refactor: Geometry Nodes: use a single string identifier for closure/bundle items Previously, we used `SocketInterfaceKey` as identifier for bundle and closure items. It contained multiple identifiers (although only one was ever used so far). The idea was that multiple identifiers could provide more flexibility. E.g. an Evaluate Closure node could work with closures with slightly different identifier names, or a bundle could be passed into different systems that expect the same data but named differently. The added complexity by allowing for this is greater than I anticipated even though most places didn't even support multiple identifiers yet. In addition to that, it seems like there may be simpler workaround for many situations where multiple identifiers were supposed to help. E.g. one could just add the same value to a bundle twice with different names or one can build a node group that maps a bundle for one system to one for another system. Overall, the complexity of `SocketInterfaceKey` didn't seem worth it, and we can probably just build a better system when we don't allow multiple identifiers per item. Pull Request: https://projects.blender.org/blender/blender/pulls/142947 --- source/blender/blenkernel/BKE_bake_items.hh | 4 +- .../blenkernel/intern/bake_items_serialize.cc | 20 +---- .../intern/geometry_nodes_bundle.cc | 45 ++-------- .../editors/space_node/node_socket_tooltip.cc | 15 ++-- .../editors/space_node/node_sync_sockets.cc | 33 ++++---- .../blender/editors/space_node/space_node.cc | 39 ++++----- .../nodes/NOD_geometry_nodes_bundle.hh | 29 +++---- .../NOD_geometry_nodes_bundle_signature.hh | 4 +- .../nodes/NOD_geometry_nodes_closure_eval.hh | 4 +- .../NOD_geometry_nodes_closure_signature.hh | 8 +- .../blender/nodes/NOD_geometry_nodes_log.hh | 5 +- .../blender/nodes/NOD_socket_interface_key.hh | 36 -------- .../geometry/nodes/node_geo_combine_bundle.cc | 2 +- .../nodes/node_geo_evaluate_closure.cc | 6 +- .../nodes/node_geo_separate_bundle.cc | 2 +- .../nodes/intern/geometry_nodes_bundle.cc | 83 +++++-------------- .../intern/geometry_nodes_bundle_tests.cc | 6 +- .../nodes/intern/geometry_nodes_closure.cc | 22 ++--- .../intern/geometry_nodes_closure_zone.cc | 14 ++-- 19 files changed, 110 insertions(+), 267 deletions(-) delete mode 100644 source/blender/nodes/NOD_socket_interface_key.hh diff --git a/source/blender/blenkernel/BKE_bake_items.hh b/source/blender/blenkernel/BKE_bake_items.hh index faad866aeb4..beabe33c308 100644 --- a/source/blender/blenkernel/BKE_bake_items.hh +++ b/source/blender/blenkernel/BKE_bake_items.hh @@ -14,8 +14,6 @@ #include "BKE_geometry_set.hh" #include "BKE_volume_grid_fwd.hh" -#include "NOD_socket_interface_key.hh" - namespace blender::bke::bake { /** @@ -153,7 +151,7 @@ class StringBakeItem : public BakeItem { class BundleBakeItem : public BakeItem { public: struct Item { - nodes::SocketInterfaceKey key; + std::string key; std::string socket_idname; std::unique_ptr value; }; diff --git a/source/blender/blenkernel/intern/bake_items_serialize.cc b/source/blender/blenkernel/intern/bake_items_serialize.cc index 5eeb3a2b8b4..5aa3aa5342d 100644 --- a/source/blender/blenkernel/intern/bake_items_serialize.cc +++ b/source/blender/blenkernel/intern/bake_items_serialize.cc @@ -1498,10 +1498,7 @@ static void serialize_bake_item(const BakeItem &item, ArrayValue &io_items = *r_io_item.append_array("items"); for (const BundleBakeItem::Item &item : bundle_state_item->items) { DictionaryValue &io_bundle_item = *io_items.append_dict(); - ArrayValue &io_key = *io_bundle_item.append_array("key"); - for (const std::string &identifier : item.key.identifiers()) { - io_key.append_str(identifier); - } + io_bundle_item.append_str("key", item.key); io_bundle_item.append_str("socket_idname", item.socket_idname); io::serialize::DictionaryValue &io_bundle_item_value = *io_bundle_item.append_dict("value"); serialize_bake_item(*item.value, blob_writer, blob_sharing, io_bundle_item_value); @@ -1603,18 +1600,10 @@ static std::unique_ptr deserialize_bake_item(const DictionaryValue &io if (!io_item) { return {}; } - const ArrayValue *io_key = io_item->lookup_array("key"); - if (!io_key) { + const std::optional key = io_item->lookup_str("key"); + if (!key) { return {}; } - Vector key; - for (const auto &io_key_value : io_key->elements()) { - const StringValue *io_key_string = io_key_value->as_string_value(); - if (!io_key_string) { - return {}; - } - key.append(io_key_string->value()); - } const std::optional socket_idname = io_item->lookup_str("socket_idname"); if (!socket_idname) { return {}; @@ -1625,8 +1614,7 @@ static std::unique_ptr deserialize_bake_item(const DictionaryValue &io if (!value) { return {}; } - bundle->items.append(BundleBakeItem::Item{ - nodes::SocketInterfaceKey{std::move(key)}, *socket_idname, std::move(value)}); + bundle->items.append(BundleBakeItem::Item{*key, *socket_idname, std::move(value)}); } return bundle; } diff --git a/source/blender/blenkernel/intern/geometry_nodes_bundle.cc b/source/blender/blenkernel/intern/geometry_nodes_bundle.cc index 7e95aba7d0c..171c175b230 100644 --- a/source/blender/blenkernel/intern/geometry_nodes_bundle.cc +++ b/source/blender/blenkernel/intern/geometry_nodes_bundle.cc @@ -7,32 +7,6 @@ namespace blender::bke { -SocketInterfaceKey::SocketInterfaceKey(std::string identifier) -{ - identifiers_.append(std::move(identifier)); -} - -SocketInterfaceKey::SocketInterfaceKey(Vector identifiers) - : identifiers_(std::move(identifiers)) -{ - BLI_assert(!identifiers_.is_empty()); -} - -Span SocketInterfaceKey::identifiers() const -{ - return identifiers_; -} - -bool SocketInterfaceKey::matches(const SocketInterfaceKey &other) const -{ - for (const std::string &identifier : other.identifiers_) { - if (identifiers_.contains(identifier)) { - return true; - } - } - return false; -} - Bundle::Bundle() = default; Bundle::~Bundle() @@ -77,7 +51,7 @@ Bundle &Bundle::operator=(Bundle &&other) noexcept return *this; } -void Bundle::add_new(SocketInterfaceKey key, const bNodeSocketType &type, const void *value) +void Bundle::add_new(const StringRef key, const bNodeSocketType &type, const void *value) { BLI_assert(!this->contains(key)); BLI_assert(type.geometry_nodes_cpp_type); @@ -88,7 +62,7 @@ void Bundle::add_new(SocketInterfaceKey key, const bNodeSocketType &type, const buffers_.append(buffer); } -bool Bundle::add(const SocketInterfaceKey &key, const bNodeSocketType &type, const void *value) +bool Bundle::add(const StringRef key, const bNodeSocketType &type, const void *value) { if (this->contains(key)) { return false; @@ -97,16 +71,7 @@ bool Bundle::add(const SocketInterfaceKey &key, const bNodeSocketType &type, con return true; } -bool Bundle::add(SocketInterfaceKey &&key, const bNodeSocketType &type, const void *value) -{ - if (this->contains(key)) { - return false; - } - this->add_new(std::move(key), type, value); - return true; -} - -std::optional Bundle::lookup(const SocketInterfaceKey &key) const +std::optional Bundle::lookup(const StringRef key) const { for (const StoredItem &item : items_) { if (item.key.matches(key)) { @@ -116,7 +81,7 @@ std::optional Bundle::lookup(const SocketInterfaceKey &key) const return std::nullopt; } -bool Bundle::remove(const SocketInterfaceKey &key) +bool Bundle::remove(const StringRef key) { const int removed_num = items_.remove_if([&key](StoredItem &item) { if (item.key.matches(key)) { @@ -128,7 +93,7 @@ bool Bundle::remove(const SocketInterfaceKey &key) return removed_num >= 1; } -bool Bundle::contains(const SocketInterfaceKey &key) const +bool Bundle::contains(const StringRef key) const { for (const StoredItem &item : items_) { if (item.key.matches(key)) { diff --git a/source/blender/editors/space_node/node_socket_tooltip.cc b/source/blender/editors/space_node/node_socket_tooltip.cc index 8cdd7f57f42..666c05769ed 100644 --- a/source/blender/editors/space_node/node_socket_tooltip.cc +++ b/source/blender/editors/space_node/node_socket_tooltip.cc @@ -694,14 +694,13 @@ class SocketTooltipBuilder { this->add_text_field_mono(TIP_("Values:")); Vector sorted_items = bundle_log.items; std::sort(sorted_items.begin(), sorted_items.end(), [](const auto &a, const auto &b) { - return BLI_strcasecmp_natural(a.key.identifiers().first().c_str(), - b.key.identifiers().first().c_str()) < 0; + return BLI_strcasecmp_natural(a.key.c_str(), b.key.c_str()) < 0; }); for (const geo_log::BundleValueLog::Item &item : sorted_items) { this->add_space(); const std::string type_name = TIP_(item.type->label); - this->add_text_field_mono(fmt::format( - fmt::runtime("\u2022 \"{}\" ({})\n"), item.key.identifiers().first(), type_name)); + this->add_text_field_mono( + fmt::format(fmt::runtime("\u2022 \"{}\" ({})\n"), item.key, type_name)); } } this->add_space(); @@ -719,8 +718,8 @@ class SocketTooltipBuilder { for (const geo_log::ClosureValueLog::Item &item : closure_log.inputs) { this->add_space(); const std::string type_name = TIP_(item.type->label); - this->add_text_field_mono(fmt::format( - fmt::runtime("\u2022 \"{}\" ({})\n"), item.key.identifiers().first(), type_name)); + this->add_text_field_mono( + fmt::format(fmt::runtime("\u2022 \"{}\" ({})\n"), item.key, type_name)); } } if (!closure_log.outputs.is_empty()) { @@ -729,8 +728,8 @@ class SocketTooltipBuilder { for (const geo_log::ClosureValueLog::Item &item : closure_log.outputs) { this->add_space(); const std::string type_name = TIP_(item.type->label); - this->add_text_field_mono(fmt::format( - fmt::runtime("\u2022 \"{}\" ({})\n"), item.key.identifiers().first(), type_name)); + this->add_text_field_mono( + fmt::format(fmt::runtime("\u2022 \"{}\" ({})\n"), item.key, type_name)); } } } diff --git a/source/blender/editors/space_node/node_sync_sockets.cc b/source/blender/editors/space_node/node_sync_sockets.cc index c8462fdd936..8a28bdb1f75 100644 --- a/source/blender/editors/space_node/node_sync_sockets.cc +++ b/source/blender/editors/space_node/node_sync_sockets.cc @@ -204,12 +204,11 @@ void sync_sockets_separate_bundle(SpaceNode &snode, nodes::socket_items::clear(separate_bundle_node); for (const nodes::BundleSignature::Item &item : sync_state.source_signature->items) { - const StringRefNull name = item.key.identifiers()[0]; NodeGeometrySeparateBundleItem &new_item = *nodes::socket_items::add_item_with_socket_type_and_name< nodes ::SeparateBundleItemsAccessor>( - separate_bundle_node, item.type->type, name.c_str()); - if (const std::optional old_identifier = old_identifiers.lookup_try(name)) { + separate_bundle_node, item.type->type, item.key.c_str()); + if (const std::optional old_identifier = old_identifiers.lookup_try(item.key)) { new_item.identifier = *old_identifier; } } @@ -242,12 +241,11 @@ void sync_sockets_combine_bundle(SpaceNode &snode, bNode &combine_bundle_node, R nodes::socket_items::clear(combine_bundle_node); for (const nodes::BundleSignature::Item &item : sync_state.source_signature->items) { - const StringRefNull name = item.key.identifiers()[0]; NodeGeometryCombineBundleItem &new_item = *nodes::socket_items::add_item_with_socket_type_and_name< nodes ::CombineBundleItemsAccessor>( - combine_bundle_node, item.type->type, name.c_str()); - if (const std::optional old_identifier = old_identifiers.lookup_try(name)) { + combine_bundle_node, item.type->type, item.key.c_str()); + if (const std::optional old_identifier = old_identifiers.lookup_try(item.key)) { new_item.identifier = *old_identifier; } } @@ -291,22 +289,20 @@ void sync_sockets_evaluate_closure(SpaceNode &snode, nodes::socket_items::clear(evaluate_closure_node); for (const nodes::ClosureSignature::Item &item : sync_state.source_signature->inputs) { - const StringRefNull name = item.key.identifiers()[0]; NodeGeometryEvaluateClosureInputItem &new_item = *nodes::socket_items::add_item_with_socket_type_and_name< nodes::EvaluateClosureInputItemsAccessor>( - evaluate_closure_node, item.type->type, name.c_str()); - if (const std::optional old_identifier = old_input_identifiers.lookup_try(name)) { + evaluate_closure_node, item.type->type, item.key.c_str()); + if (const std::optional old_identifier = old_input_identifiers.lookup_try(item.key)) { new_item.identifier = *old_identifier; } } for (const nodes::ClosureSignature::Item &item : sync_state.source_signature->outputs) { - const StringRefNull name = item.key.identifiers()[0]; NodeGeometryEvaluateClosureOutputItem &new_item = *nodes::socket_items::add_item_with_socket_type_and_name< nodes::EvaluateClosureOutputItemsAccessor>( - evaluate_closure_node, item.type->type, name.c_str()); - if (const std::optional old_identifier = old_output_identifiers.lookup_try(name)) { + evaluate_closure_node, item.type->type, item.key.c_str()); + if (const std::optional old_identifier = old_output_identifiers.lookup_try(item.key)) { new_item.identifier = *old_identifier; } } @@ -351,23 +347,22 @@ void sync_sockets_closure(SpaceNode &snode, nodes::socket_items::clear(closure_output_node); for (const nodes::ClosureSignature::Item &item : signature.inputs) { - const StringRefNull name = item.key.identifiers()[0]; NodeGeometryClosureInputItem &new_item = *nodes::socket_items::add_item_with_socket_type_and_name( - closure_output_node, item.type->type, name.c_str()); + closure_output_node, item.type->type, item.key.c_str()); if (item.structure_type) { new_item.structure_type = int(*item.structure_type); } - if (const std::optional old_identifier = old_input_identifiers.lookup_try(name)) { + if (const std::optional old_identifier = old_input_identifiers.lookup_try(item.key)) { new_item.identifier = *old_identifier; } } for (const nodes::ClosureSignature::Item &item : signature.outputs) { - const StringRefNull name = item.key.identifiers()[0]; NodeGeometryClosureOutputItem &new_item = *nodes::socket_items::add_item_with_socket_type_and_name< - nodes::ClosureOutputItemsAccessor>(closure_output_node, item.type->type, name.c_str()); - if (const std::optional old_identifier = old_output_identifiers.lookup_try(name)) { + nodes::ClosureOutputItemsAccessor>( + closure_output_node, item.type->type, item.key.c_str()); + if (const std::optional old_identifier = old_output_identifiers.lookup_try(item.key)) { new_item.identifier = *old_identifier; } } @@ -384,7 +379,7 @@ void sync_sockets_closure(SpaceNode &snode, const nodes::ClosureSignature::Item &input_item = signature.inputs[input_i]; for (const int output_i : signature.outputs.index_range()) { const nodes::ClosureSignature::Item &output_item = signature.outputs[output_i]; - if (input_item.key.matches(output_item.key)) { + if (input_item.key == output_item.key) { internal_links.append({&closure_input_node.output_socket(input_i), &closure_output_node.input_socket(output_i)}); } diff --git a/source/blender/editors/space_node/space_node.cc b/source/blender/editors/space_node/space_node.cc index 16c251582b3..7b71e049012 100644 --- a/source/blender/editors/space_node/space_node.cc +++ b/source/blender/editors/space_node/space_node.cc @@ -63,7 +63,6 @@ #include "WM_types.hh" #include "NOD_node_in_compute_context.hh" -#include "NOD_socket_interface_key.hh" #include "io_utils.hh" @@ -481,7 +480,7 @@ static Vector find_target_sockets_through_contexts( const StringRef query_node_idname, const bool find_all) { - using BundlePath = Vector; + using BundlePath = Vector; struct SocketToCheck { nodes::SocketInContext socket; @@ -557,7 +556,7 @@ static Vector find_target_sockets_through_contexts( if (node->is_type("GeometryNodeCombineBundle")) { const auto &storage = *static_cast(node->storage); BundlePath new_bundle_path = bundle_path; - new_bundle_path.append(nodes::SocketInterfaceKey{storage.items[socket->index()].name}); + new_bundle_path.append(storage.items[socket->index()].name); add_if_new(node.output_socket(0), std::move(new_bundle_path)); continue; } @@ -565,11 +564,10 @@ static Vector find_target_sockets_through_contexts( if (bundle_path.is_empty()) { continue; } - const nodes::SocketInterfaceKey &last_key = bundle_path.last(); + const StringRef last_key = bundle_path.last(); const auto &storage = *static_cast(node->storage); for (const int output_i : IndexRange(storage.items_num)) { - const nodes::SocketInterfaceKey key{storage.items[output_i].name}; - if (last_key.matches(key)) { + if (last_key == storage.items[output_i].name) { add_if_new(node.output_socket(output_i), bundle_path.as_span().drop_back(1)); } } @@ -578,8 +576,7 @@ static Vector find_target_sockets_through_contexts( if (node->is_type("GeometryNodeClosureOutput")) { const auto &closure_storage = *static_cast( node->storage); - const nodes::SocketInterfaceKey key( - closure_storage.output_items.items[socket->index()].name); + const StringRef key = closure_storage.output_items.items[socket->index()].name; const Vector target_sockets = find_target_sockets_through_contexts( node.output_socket(0), compute_context_cache, "GeometryNodeEvaluateClosure", true); for (const auto &target_socket : target_sockets) { @@ -589,7 +586,7 @@ static Vector find_target_sockets_through_contexts( for (const int i : IndexRange(evaluate_storage.output_items.items_num)) { const NodeGeometryEvaluateClosureOutputItem &item = evaluate_storage.output_items.items[i]; - if (key.matches(nodes::SocketInterfaceKey(item.name))) { + if (key == item.name) { add_if_new(evaluate_node.output_socket(i), bundle_path); } } @@ -602,8 +599,7 @@ static Vector find_target_sockets_through_contexts( } const auto &evaluate_storage = *static_cast( node->storage); - const nodes::SocketInterfaceKey key( - evaluate_storage.input_items.items[socket->index() - 1].name); + const StringRef key = evaluate_storage.input_items.items[socket->index() - 1].name; const Vector origin_sockets = find_origin_sockets_through_contexts( node.input_socket(0), compute_context_cache, "GeometryNodeClosureOutput", true); for (const nodes::SocketInContext origin_socket : origin_sockets) { @@ -632,7 +628,7 @@ static Vector find_target_sockets_through_contexts( closure_output_node->storage); for (const int i : IndexRange(closure_output_storage.input_items.items_num)) { const NodeGeometryClosureInputItem &item = closure_output_storage.input_items.items[i]; - if (key.matches(nodes::SocketInterfaceKey(item.name))) { + if (key == item.name) { add_if_new({&closure_context, &closure_input_node->output_socket(i)}, bundle_path); } } @@ -697,7 +693,7 @@ static Vector find_origin_sockets_through_contexts( const StringRef query_node_idname, const bool find_all) { - using BundlePath = Vector; + using BundlePath = Vector; struct SocketToCheck { nodes::SocketInContext socket; @@ -801,8 +797,7 @@ static Vector find_origin_sockets_through_contexts( if (node->is_type("GeometryNodeEvaluateClosure")) { const auto &evaluate_storage = *static_cast( node->storage); - const nodes::SocketInterfaceKey key( - evaluate_storage.output_items.items[socket->index()].name); + const StringRef key = evaluate_storage.output_items.items[socket->index()].name; const Vector origin_sockets = find_origin_sockets_through_contexts( node.input_socket(0), compute_context_cache, "GeometryNodeClosureOutput", true); for (const nodes::SocketInContext origin_socket : origin_sockets) { @@ -818,7 +813,7 @@ static Vector find_origin_sockets_through_contexts( &closure_tree, closure_output_node->identifier, origin_socket.context_hash()}); for (const int i : IndexRange(closure_storage.output_items.items_num)) { const NodeGeometryClosureOutputItem &item = closure_storage.output_items.items[i]; - if (key.matches(nodes::SocketInterfaceKey(item.name))) { + if (key == item.name) { add_if_new({&closure_context, &closure_output_node->input_socket(i)}, bundle_path); } } @@ -834,8 +829,7 @@ static Vector find_origin_sockets_through_contexts( } const auto &output_storage = *static_cast( closure_output_node->storage); - const nodes::SocketInterfaceKey key( - output_storage.input_items.items[socket->index()].name); + const StringRef key = output_storage.input_items.items[socket->index()].name; const bNodeSocket &closure_output_socket = closure_output_node->output_socket(0); const Vector target_sockets = find_target_sockets_through_contexts( {socket.context, &closure_output_socket}, @@ -849,7 +843,7 @@ static Vector find_origin_sockets_through_contexts( for (const int i : IndexRange(evaluate_storage.input_items.items_num)) { const NodeGeometryEvaluateClosureInputItem &item = evaluate_storage.input_items.items[i]; - if (key.matches(nodes::SocketInterfaceKey(item.name))) { + if (key == item.name) { add_if_new(target_node.input_socket(i + 1), bundle_path); } } @@ -860,11 +854,10 @@ static Vector find_origin_sockets_through_contexts( if (bundle_path.is_empty()) { continue; } - const nodes::SocketInterfaceKey &last_key = bundle_path.last(); + const StringRef last_key = bundle_path.last(); const auto &storage = *static_cast(node->storage); for (const int input_i : IndexRange(storage.items_num)) { - const nodes::SocketInterfaceKey key{storage.items[input_i].name}; - if (last_key.matches(key)) { + if (last_key == storage.items[input_i].name) { add_if_new(node.input_socket(input_i), bundle_path.as_span().drop_back(1)); } } @@ -873,7 +866,7 @@ static Vector find_origin_sockets_through_contexts( if (node->is_type("GeometryNodeSeparateBundle")) { const auto &storage = *static_cast(node->storage); BundlePath new_bundle_path = bundle_path; - new_bundle_path.append(nodes::SocketInterfaceKey{storage.items[socket->index()].name}); + new_bundle_path.append(storage.items[socket->index()].name); add_if_new(node.input_socket(0), std::move(new_bundle_path)); continue; } diff --git a/source/blender/nodes/NOD_geometry_nodes_bundle.hh b/source/blender/nodes/NOD_geometry_nodes_bundle.hh index b8d03c6d758..936c1ee62ea 100644 --- a/source/blender/nodes/NOD_geometry_nodes_bundle.hh +++ b/source/blender/nodes/NOD_geometry_nodes_bundle.hh @@ -9,7 +9,6 @@ #include "BKE_node_socket_value.hh" #include "NOD_geometry_nodes_bundle_fwd.hh" #include "NOD_geometry_nodes_values.hh" -#include "NOD_socket_interface_key.hh" #include "DNA_node_types.h" @@ -24,7 +23,7 @@ namespace blender::nodes { class Bundle : public ImplicitSharingMixin { public: struct StoredItem { - SocketInterfaceKey key; + std::string key; const bke::bNodeSocketType *type; void *value; }; @@ -54,28 +53,26 @@ class Bundle : public ImplicitSharingMixin { static BundlePtr create(); - bool add(const SocketInterfaceKey &key, const bke::bNodeSocketType &type, const void *value); - void add_new(SocketInterfaceKey key, const bke::bNodeSocketType &type, const void *value); - void add_override(const SocketInterfaceKey &key, - const bke::bNodeSocketType &type, - const void *value); + bool add(StringRef key, const bke::bNodeSocketType &type, const void *value); + void add_new(StringRef key, const bke::bNodeSocketType &type, const void *value); + void add_override(StringRef key, const bke::bNodeSocketType &type, const void *value); bool add_path(StringRef path, const bke::bNodeSocketType &type, const void *value); void add_path_new(StringRef path, const bke::bNodeSocketType &type, const void *value); void add_path_override(StringRef path, const bke::bNodeSocketType &type, const void *value); - template void add(const SocketInterfaceKey &key, T value); - template void add_override(const SocketInterfaceKey &key, T value); + template void add(StringRef key, T value); + template void add_override(StringRef key, T value); template void add_path(StringRef path, T value); template void add_path_override(StringRef path, T value); - bool remove(const SocketInterfaceKey &key); - bool contains(const SocketInterfaceKey &key) const; + bool remove(StringRef key); + bool contains(StringRef key) const; bool contains_path(StringRef path) const; - std::optional lookup(const SocketInterfaceKey &key) const; + std::optional lookup(StringRef key) const; std::optional lookup_path(Span path) const; std::optional lookup_path(StringRef path) const; - template std::optional lookup(const SocketInterfaceKey &key) const; + template std::optional lookup(StringRef key) const; template std::optional lookup_path(StringRef path) const; bool is_empty() const; @@ -155,7 +152,7 @@ template inline std::optional Bundle::Item::as() const return std::nullopt; } -template inline std::optional Bundle::lookup(const SocketInterfaceKey &key) const +template inline std::optional Bundle::lookup(const StringRef key) const { const std::optional item = this->lookup(key); if (!item) { @@ -188,7 +185,7 @@ template inline void to_stored_type(T &&value, Fn &&fn) } } -template inline void Bundle::add(const SocketInterfaceKey &key, T value) +template inline void Bundle::add(const StringRef key, T value) { to_stored_type(value, [&](const bke::bNodeSocketType &type, const void *value) { this->add(key, type, value); @@ -202,7 +199,7 @@ template inline void Bundle::add_path(const StringRef path, T value) }); } -template inline void Bundle::add_override(const SocketInterfaceKey &key, T value) +template inline void Bundle::add_override(const StringRef key, T value) { to_stored_type(value, [&](const bke::bNodeSocketType &type, const void *value) { this->add_override(key, type, value); diff --git a/source/blender/nodes/NOD_geometry_nodes_bundle_signature.hh b/source/blender/nodes/NOD_geometry_nodes_bundle_signature.hh index 98111a6e83f..b13bd00c60c 100644 --- a/source/blender/nodes/NOD_geometry_nodes_bundle_signature.hh +++ b/source/blender/nodes/NOD_geometry_nodes_bundle_signature.hh @@ -6,13 +6,11 @@ #include "BKE_node.hh" -#include "NOD_socket_interface_key.hh" - namespace blender::nodes { struct BundleSignature { struct Item { - SocketInterfaceKey key; + std::string key; const bke::bNodeSocketType *type = nullptr; }; diff --git a/source/blender/nodes/NOD_geometry_nodes_closure_eval.hh b/source/blender/nodes/NOD_geometry_nodes_closure_eval.hh index 8228bfca29e..1e5f1a06939 100644 --- a/source/blender/nodes/NOD_geometry_nodes_closure_eval.hh +++ b/source/blender/nodes/NOD_geometry_nodes_closure_eval.hh @@ -12,7 +12,7 @@ namespace blender::nodes { struct ClosureEagerEvalParams { struct InputItem { - SocketInterfaceKey key; + std::string key; const bke::bNodeSocketType *type = nullptr; /** * The actual socket value of type bNodeSocketType::geometry_nodes_cpp_type. @@ -22,7 +22,7 @@ struct ClosureEagerEvalParams { }; struct OutputItem { - SocketInterfaceKey key; + std::string key; const bke::bNodeSocketType *type = nullptr; /** * Where the output value should be stored. This is expected to point to uninitialized memory diff --git a/source/blender/nodes/NOD_geometry_nodes_closure_signature.hh b/source/blender/nodes/NOD_geometry_nodes_closure_signature.hh index 93c76694c14..df098db363d 100644 --- a/source/blender/nodes/NOD_geometry_nodes_closure_signature.hh +++ b/source/blender/nodes/NOD_geometry_nodes_closure_signature.hh @@ -6,15 +6,13 @@ #include "BKE_node.hh" -#include "NOD_socket_interface_key.hh" - namespace blender::nodes { /** Describes the names and types of the inputs and outputs of a closure. */ class ClosureSignature { public: struct Item { - SocketInterfaceKey key; + std::string key; const bke::bNodeSocketType *type = nullptr; std::optional structure_type = std::nullopt; }; @@ -22,8 +20,8 @@ class ClosureSignature { Vector inputs; Vector outputs; - std::optional find_input_index(const SocketInterfaceKey &key) const; - std::optional find_output_index(const SocketInterfaceKey &key) const; + std::optional find_input_index(StringRef key) const; + std::optional find_output_index(StringRef key) const; bool matches_exactly(const ClosureSignature &other) const; static bool all_matching_exactly(Span signatures); diff --git a/source/blender/nodes/NOD_geometry_nodes_log.hh b/source/blender/nodes/NOD_geometry_nodes_log.hh index 0e5043b4519..6562e509340 100644 --- a/source/blender/nodes/NOD_geometry_nodes_log.hh +++ b/source/blender/nodes/NOD_geometry_nodes_log.hh @@ -43,7 +43,6 @@ #include "NOD_geometry_nodes_closure_location.hh" #include "NOD_geometry_nodes_warning.hh" -#include "NOD_socket_interface_key.hh" #include "FN_field.hh" @@ -190,7 +189,7 @@ class GridInfoLog : public ValueLog { class BundleValueLog : public ValueLog { public: struct Item { - SocketInterfaceKey key; + std::string key; const bke::bNodeSocketType *type; }; @@ -202,7 +201,7 @@ class BundleValueLog : public ValueLog { class ClosureValueLog : public ValueLog { public: struct Item { - SocketInterfaceKey key; + std::string key; const bke::bNodeSocketType *type; }; diff --git a/source/blender/nodes/NOD_socket_interface_key.hh b/source/blender/nodes/NOD_socket_interface_key.hh deleted file mode 100644 index 6a2bf1ebcb5..00000000000 --- a/source/blender/nodes/NOD_socket_interface_key.hh +++ /dev/null @@ -1,36 +0,0 @@ -/* SPDX-FileCopyrightText: 2025 Blender Authors - * - * SPDX-License-Identifier: GPL-2.0-or-later */ - -#pragma once - -#include - -#include "BLI_vector.hh" - -namespace blender::nodes { - -/** - * A key that identifies values in a bundle or inputs/outputs of a closure. - * Note that this key does not have a hash and thus can't be used in a hash table. This wouldn't - * work well if these items have multiple identifiers for compatibility reasons. While that's not - * used currently, it's good to keep it possible. - */ -class SocketInterfaceKey { - private: - /** May have multiple keys to improve compatibility between systems that use different keys. */ - Vector identifiers_; - - public: - explicit SocketInterfaceKey(Vector identifiers); - explicit SocketInterfaceKey(std::string identifier); - - /** Two keys match when they have one common identifier. */ - bool matches(const SocketInterfaceKey &other) const; - /** Two keys match exactly when the have the same identifiers (the order doesn't matter). */ - bool matches_exactly(const SocketInterfaceKey &other) const; - - Span identifiers() const; -}; - -} // namespace blender::nodes diff --git a/source/blender/nodes/geometry/nodes/node_geo_combine_bundle.cc b/source/blender/nodes/geometry/nodes/node_geo_combine_bundle.cc index e08fa95567e..5ac51512892 100644 --- a/source/blender/nodes/geometry/nodes/node_geo_combine_bundle.cc +++ b/source/blender/nodes/geometry/nodes/node_geo_combine_bundle.cc @@ -116,7 +116,7 @@ static void node_geo_exec(GeoNodeExecParams params) } void *input_ptr = params.low_level_lazy_function_params().try_get_input_data_ptr(i); BLI_assert(input_ptr); - bundle.add(SocketInterfaceKey(name), *stype, input_ptr); + bundle.add(name, *stype, input_ptr); } params.set_output("Bundle", std::move(bundle_ptr)); diff --git a/source/blender/nodes/geometry/nodes/node_geo_evaluate_closure.cc b/source/blender/nodes/geometry/nodes/node_geo_evaluate_closure.cc index b4f463986d4..986f9d2e985 100644 --- a/source/blender/nodes/geometry/nodes/node_geo_evaluate_closure.cc +++ b/source/blender/nodes/geometry/nodes/node_geo_evaluate_closure.cc @@ -218,11 +218,11 @@ const bNodeSocket *evaluate_closure_node_internally_linked_input(const bNodeSock } const NodeGeometryEvaluateClosureOutputItem &output_item = storage.output_items.items[output_socket.index()]; - const SocketInterfaceKey output_key{output_item.name}; + const StringRef output_key = output_item.name; for (const int i : IndexRange(storage.input_items.items_num)) { const NodeGeometryEvaluateClosureInputItem &input_item = storage.input_items.items[i]; - const SocketInterfaceKey input_key{input_item.name}; - if (output_key.matches(input_key)) { + const StringRef input_key = input_item.name; + if (output_key == input_key) { if (!tree.typeinfo->validate_link || tree.typeinfo->validate_link(eNodeSocketDatatype(input_item.socket_type), eNodeSocketDatatype(output_item.socket_type))) diff --git a/source/blender/nodes/geometry/nodes/node_geo_separate_bundle.cc b/source/blender/nodes/geometry/nodes/node_geo_separate_bundle.cc index 7c62ddc3577..e694cd4cd11 100644 --- a/source/blender/nodes/geometry/nodes/node_geo_separate_bundle.cc +++ b/source/blender/nodes/geometry/nodes/node_geo_separate_bundle.cc @@ -123,7 +123,7 @@ static void node_geo_exec(GeoNodeExecParams params) if (!stype || !stype->geometry_nodes_cpp_type) { continue; } - const std::optional value = bundle->lookup(SocketInterfaceKey(name)); + const std::optional value = bundle->lookup(name); if (!value) { params.error_message_add(NodeWarningType::Error, fmt::format(fmt::runtime(TIP_("Value not found: \"{}\"")), name)); diff --git a/source/blender/nodes/intern/geometry_nodes_bundle.cc b/source/blender/nodes/intern/geometry_nodes_bundle.cc index fdf11e6adc7..856697bdc8e 100644 --- a/source/blender/nodes/intern/geometry_nodes_bundle.cc +++ b/source/blender/nodes/intern/geometry_nodes_bundle.cc @@ -12,46 +12,6 @@ namespace blender::nodes { -SocketInterfaceKey::SocketInterfaceKey(std::string identifier) -{ - identifiers_.append(std::move(identifier)); -} - -SocketInterfaceKey::SocketInterfaceKey(Vector identifiers) - : identifiers_(std::move(identifiers)) -{ - BLI_assert(!identifiers_.is_empty()); -} - -Span SocketInterfaceKey::identifiers() const -{ - return identifiers_; -} - -bool SocketInterfaceKey::matches(const SocketInterfaceKey &other) const -{ - for (const std::string &identifier : other.identifiers_) { - if (identifiers_.contains(identifier)) { - return true; - } - } - return false; -} - -bool SocketInterfaceKey::matches_exactly(const SocketInterfaceKey &other) const -{ - for (const std::string &identifier : identifiers_) { - if (std::none_of( - other.identifiers_.begin(), - other.identifiers_.end(), - [&](const std::string &other_identifier) { return other_identifier == identifier; })) - { - return false; - } - } - return true; -} - bool BundleSignature::matches_exactly(const BundleSignature &other) const { if (items.size() != other.items.size()) { @@ -59,7 +19,7 @@ bool BundleSignature::matches_exactly(const BundleSignature &other) const } for (const Item &item : items) { if (std::none_of(other.items.begin(), other.items.end(), [&](const Item &other_item) { - return item.key.matches_exactly(other_item.key); + return item.key == other_item.key; })) { return false; @@ -130,7 +90,7 @@ Bundle &Bundle::operator=(Bundle &&other) noexcept return *this; } -void Bundle::add_new(SocketInterfaceKey key, const bke::bNodeSocketType &type, const void *value) +void Bundle::add_new(const StringRef key, const bke::bNodeSocketType &type, const void *value) { BLI_assert(!this->contains(key)); BLI_assert(type.geometry_nodes_cpp_type); @@ -141,17 +101,13 @@ void Bundle::add_new(SocketInterfaceKey key, const bke::bNodeSocketType &type, c buffers_.append(buffer); } -void Bundle::add_override(const SocketInterfaceKey &key, - const bke::bNodeSocketType &type, - const void *value) +void Bundle::add_override(const StringRef key, const bke::bNodeSocketType &type, const void *value) { this->remove(key); this->add_new(key, type, value); } -bool Bundle::add(const SocketInterfaceKey &key, - const bke::bNodeSocketType &type, - const void *value) +bool Bundle::add(const StringRef key, const bke::bNodeSocketType &type, const void *value) { if (this->contains(key)) { return false; @@ -169,20 +125,21 @@ void Bundle::add_path_override(const StringRef path, BLI_assert(this->is_mutable()); const int sep = path.find_first_of('/'); if (sep == StringRef::not_found) { - this->remove(SocketInterfaceKey{path}); - this->add_new(SocketInterfaceKey{path}, type, value); + const StringRef key = path; + this->remove(key); + this->add_new(key, type, value); return; } const StringRef first_part = path.substr(0, sep); BundlePtr child_bundle; - const std::optional item = this->lookup(SocketInterfaceKey{first_part}); + const std::optional item = this->lookup(first_part); if (item && item->type->type == SOCK_BUNDLE) { child_bundle = static_cast(item->value)->get(); } if (!child_bundle) { child_bundle = Bundle::create(); } - this->remove(SocketInterfaceKey{first_part}); + this->remove(first_part); if (!child_bundle->is_mutable()) { child_bundle = child_bundle->copy(); } @@ -190,9 +147,7 @@ void Bundle::add_path_override(const StringRef path, const_cast(*child_bundle).add_path_override(path.substr(sep + 1), type, value); bke::SocketValueVariant child_bundle_value = bke::SocketValueVariant::From( std::move(child_bundle)); - this->add(SocketInterfaceKey{first_part}, - *bke::node_socket_type_find_static(SOCK_BUNDLE), - &child_bundle_value); + this->add(first_part, *bke::node_socket_type_find_static(SOCK_BUNDLE), &child_bundle_value); } bool Bundle::add_path(StringRef path, const bke::bNodeSocketType &type, const void *value) @@ -210,10 +165,10 @@ void Bundle::add_path_new(StringRef path, const bke::bNodeSocketType &type, cons this->add_path_override(path, type, value); } -std::optional Bundle::lookup(const SocketInterfaceKey &key) const +std::optional Bundle::lookup(const StringRef key) const { for (const StoredItem &item : items_) { - if (item.key.matches(key)) { + if (item.key == key) { return Item{item.type, item.value}; } } @@ -224,7 +179,7 @@ std::optional Bundle::lookup_path(const Span path) cons { BLI_assert(!path.is_empty()); const StringRef first_elem = path[0]; - const std::optional item = this->lookup(SocketInterfaceKey(first_elem)); + const std::optional item = this->lookup(first_elem); if (!item) { return std::nullopt; } @@ -274,10 +229,10 @@ BundlePtr Bundle::copy() const return copy_ptr; } -bool Bundle::remove(const SocketInterfaceKey &key) +bool Bundle::remove(const StringRef key) { const int removed_num = items_.remove_if([&key](StoredItem &item) { - if (item.key.matches(key)) { + if (item.key == key) { item.type->geometry_nodes_cpp_type->destruct(item.value); return true; } @@ -286,10 +241,10 @@ bool Bundle::remove(const SocketInterfaceKey &key) return removed_num >= 1; } -bool Bundle::contains(const SocketInterfaceKey &key) const +bool Bundle::contains(const StringRef key) const { for (const StoredItem &item : items_) { - if (item.key.matches(key)) { + if (item.key == key) { return true; } } @@ -314,7 +269,7 @@ BundleSignature BundleSignature::from_combine_bundle_node(const bNode &node) for (const int i : IndexRange(storage.items_num)) { const NodeGeometryCombineBundleItem &item = storage.items[i]; if (const bke::bNodeSocketType *stype = bke::node_socket_type_find_static(item.socket_type)) { - signature.items.append({nodes::SocketInterfaceKey(item.name), stype}); + signature.items.append({item.name, stype}); } } return signature; @@ -328,7 +283,7 @@ BundleSignature BundleSignature::from_separate_bundle_node(const bNode &node) for (const int i : IndexRange(storage.items_num)) { const NodeGeometrySeparateBundleItem &item = storage.items[i]; if (const bke::bNodeSocketType *stype = bke::node_socket_type_find_static(item.socket_type)) { - signature.items.append({nodes::SocketInterfaceKey(item.name), stype}); + signature.items.append({item.name, stype}); } } return signature; diff --git a/source/blender/nodes/intern/geometry_nodes_bundle_tests.cc b/source/blender/nodes/intern/geometry_nodes_bundle_tests.cc index d82cf22e2dd..0546ec9afe6 100644 --- a/source/blender/nodes/intern/geometry_nodes_bundle_tests.cc +++ b/source/blender/nodes/intern/geometry_nodes_bundle_tests.cc @@ -58,10 +58,10 @@ TEST_F(BundleTest, AddItems) { BundlePtr bundle_ptr = Bundle::create(); Bundle &bundle = const_cast(*bundle_ptr); - bundle.add(SocketInterfaceKey{"a"}, 3); + bundle.add("a", 3); EXPECT_EQ(bundle.size(), 1); - EXPECT_TRUE(bundle.contains(SocketInterfaceKey{"a"})); - EXPECT_EQ(bundle.lookup(SocketInterfaceKey{"a"}), 3); + EXPECT_TRUE(bundle.contains("a")); + EXPECT_EQ(bundle.lookup("a"), 3); } TEST_F(BundleTest, AddLookupPath) diff --git a/source/blender/nodes/intern/geometry_nodes_closure.cc b/source/blender/nodes/intern/geometry_nodes_closure.cc index 40e010534fa..9d62954d4f5 100644 --- a/source/blender/nodes/intern/geometry_nodes_closure.cc +++ b/source/blender/nodes/intern/geometry_nodes_closure.cc @@ -8,22 +8,22 @@ namespace blender::nodes { -std::optional ClosureSignature::find_input_index(const SocketInterfaceKey &key) const +std::optional ClosureSignature::find_input_index(const StringRef key) const { for (const int i : this->inputs.index_range()) { const Item &item = this->inputs[i]; - if (item.key.matches(key)) { + if (item.key == key) { return i; } } return std::nullopt; } -std::optional ClosureSignature::find_output_index(const SocketInterfaceKey &key) const +std::optional ClosureSignature::find_output_index(const StringRef key) const { for (const int i : this->outputs.index_range()) { const Item &item = this->outputs[i]; - if (item.key.matches(key)) { + if (item.key == key) { return i; } } @@ -32,7 +32,7 @@ std::optional ClosureSignature::find_output_index(const SocketInterfaceKey static bool items_equal(const ClosureSignature::Item &a, const ClosureSignature::Item &b) { - if (!a.key.matches_exactly(b.key)) { + if (a.key != b.key) { return false; } if (a.type != b.type) { @@ -94,13 +94,13 @@ ClosureSignature ClosureSignature::from_closure_output_node(const bNode &node) for (const int i : IndexRange(storage.input_items.items_num)) { const NodeGeometryClosureInputItem &item = storage.input_items.items[i]; if (const bke::bNodeSocketType *stype = bke::node_socket_type_find_static(item.socket_type)) { - signature.inputs.append({nodes::SocketInterfaceKey(item.name), stype}); + signature.inputs.append({item.name, stype}); } } for (const int i : IndexRange(storage.output_items.items_num)) { const NodeGeometryClosureOutputItem &item = storage.output_items.items[i]; if (const bke::bNodeSocketType *stype = bke::node_socket_type_find_static(item.socket_type)) { - signature.outputs.append({nodes::SocketInterfaceKey(item.name), stype}); + signature.outputs.append({item.name, stype}); } } return signature; @@ -114,17 +114,13 @@ ClosureSignature ClosureSignature::from_evaluate_closure_node(const bNode &node) for (const int i : IndexRange(storage.input_items.items_num)) { const NodeGeometryEvaluateClosureInputItem &item = storage.input_items.items[i]; if (const bke::bNodeSocketType *stype = bke::node_socket_type_find_static(item.socket_type)) { - signature.inputs.append({nodes::SocketInterfaceKey(item.name), - stype, - nodes::StructureType(item.structure_type)}); + signature.inputs.append({item.name, stype, nodes::StructureType(item.structure_type)}); } } for (const int i : IndexRange(storage.output_items.items_num)) { const NodeGeometryEvaluateClosureOutputItem &item = storage.output_items.items[i]; if (const bke::bNodeSocketType *stype = bke::node_socket_type_find_static(item.socket_type)) { - signature.outputs.append({nodes::SocketInterfaceKey(item.name), - stype, - nodes::StructureType(item.structure_type)}); + signature.outputs.append({item.name, stype, nodes::StructureType(item.structure_type)}); } } return signature; diff --git a/source/blender/nodes/intern/geometry_nodes_closure_zone.cc b/source/blender/nodes/intern/geometry_nodes_closure_zone.cc index f1abfa73219..65480ce0c00 100644 --- a/source/blender/nodes/intern/geometry_nodes_closure_zone.cc +++ b/source/blender/nodes/intern/geometry_nodes_closure_zone.cc @@ -123,11 +123,11 @@ class LazyFunctionForClosureZone : public LazyFunction { for (const int i : IndexRange(storage.input_items.items_num)) { const bNodeSocket &bsocket = zone_.input_node()->output_socket(i); - closure_signature_->inputs.append({SocketInterfaceKey(bsocket.name), bsocket.typeinfo}); + closure_signature_->inputs.append({bsocket.name, bsocket.typeinfo}); } for (const int i : IndexRange(storage.output_items.items_num)) { const bNodeSocket &bsocket = zone_.output_node()->input_socket(i); - closure_signature_->outputs.append({SocketInterfaceKey(bsocket.name), bsocket.typeinfo}); + closure_signature_->outputs.append({bsocket.name, bsocket.typeinfo}); } } @@ -430,7 +430,7 @@ class LazyFunctionForEvaluateClosureNode : public LazyFunction { for (const NodeGeometryEvaluateClosureInputItem &item : Span{node_storage.input_items.items, node_storage.input_items.items_num}) { - if (const std::optional i = signature.find_input_index(SocketInterfaceKey{item.name})) { + if (const std::optional i = signature.find_input_index(item.name)) { const ClosureSignature::Item &closure_item = signature.inputs[*i]; if (!btree_.typeinfo->validate_link(eNodeSocketDatatype(item.socket_type), eNodeSocketDatatype(closure_item.type->type))) @@ -456,8 +456,7 @@ class LazyFunctionForEvaluateClosureNode : public LazyFunction { for (const NodeGeometryEvaluateClosureOutputItem &item : Span{node_storage.output_items.items, node_storage.output_items.items_num}) { - if (const std::optional i = signature.find_output_index(SocketInterfaceKey{item.name})) - { + if (const std::optional i = signature.find_output_index(item.name)) { const ClosureSignature::Item &closure_item = signature.outputs[*i]; if (!btree_.typeinfo->validate_link(eNodeSocketDatatype(closure_item.type->type), eNodeSocketDatatype(item.socket_type))) @@ -502,13 +501,12 @@ class LazyFunctionForEvaluateClosureNode : public LazyFunction { Array> inputs_map(node_storage.input_items.items_num); for (const int i : inputs_map.index_range()) { - inputs_map[i] = closure_signature.find_input_index( - SocketInterfaceKey(node_storage.input_items.items[i].name)); + inputs_map[i] = closure_signature.find_input_index(node_storage.input_items.items[i].name); } Array> outputs_map(node_storage.output_items.items_num); for (const int i : outputs_map.index_range()) { outputs_map[i] = closure_signature.find_output_index( - SocketInterfaceKey(node_storage.output_items.items[i].name)); + node_storage.output_items.items[i].name); } lf::FunctionNode &lf_closure_node = lf_graph.add_function(closure.function());