[Bf-blender-cvs] [90ea1b76434] master: Nodes: Use persistent integer to identify to nodes
Hans Goudey
noreply at git.blender.org
Thu Dec 1 22:11:05 CET 2022
Commit: 90ea1b76434fe175e99d43da8f463f28a108d0ad
Author: Hans Goudey
Date: Thu Dec 1 14:53:27 2022 -0600
Branches: master
https://developer.blender.org/rB90ea1b76434fe175e99d43da8f463f28a108d0ad
Nodes: Use persistent integer to identify to nodes
This patch adds an integer identifier to nodes that doesn't change when
the node name changes. This identifier can be used by different systems
to reference a node. This may be important to store caches and simulation
states per node, because otherwise those would always be invalidated
when a node name changes.
Additionally, this kind of identifier could make some things more efficient,
because with it an integer is enough to identify a node and one does not
have to store the node name.
I observed a 10% improvement in evaluation time in a file with an extreme
number of simple math nodes, due to reduced logging overhead-- from
0.226s to 0.205s.
Differential Revision: https://developer.blender.org/D15775
===================================================================
M source/blender/blenkernel/BKE_compute_contexts.hh
M source/blender/blenkernel/BKE_node.h
M source/blender/blenkernel/BKE_node_runtime.hh
M source/blender/blenkernel/intern/compute_contexts.cc
M source/blender/blenkernel/intern/node.cc
M source/blender/blenkernel/intern/node_runtime.cc
M source/blender/blenkernel/intern/node_tree_update.cc
M source/blender/blenkernel/intern/viewer_path.cc
M source/blender/editors/include/ED_viewer_path.hh
M source/blender/editors/space_node/node_draw.cc
M source/blender/editors/space_node/node_geometry_attribute_search.cc
M source/blender/editors/space_node/node_group.cc
M source/blender/editors/util/ed_viewer_path.cc
M source/blender/makesdna/DNA_node_types.h
M source/blender/makesdna/DNA_viewer_path_types.h
M source/blender/modifiers/intern/MOD_nodes.cc
M source/blender/nodes/NOD_geometry_nodes_log.hh
M source/blender/nodes/intern/geometry_nodes_lazy_function.cc
M source/blender/nodes/intern/geometry_nodes_log.cc
M source/blender/nodes/intern/node_geometry_exec.cc
M source/blender/nodes/shader/node_shader_tree.cc
===================================================================
diff --git a/source/blender/blenkernel/BKE_compute_contexts.hh b/source/blender/blenkernel/BKE_compute_contexts.hh
index a8f0022f49b..8e4bbbba524 100644
--- a/source/blender/blenkernel/BKE_compute_contexts.hh
+++ b/source/blender/blenkernel/BKE_compute_contexts.hh
@@ -8,6 +8,8 @@
#include "BLI_compute_context.hh"
+struct bNode;
+
namespace blender::bke {
class ModifierComputeContext : public ComputeContext {
@@ -32,12 +34,20 @@ class NodeGroupComputeContext : public ComputeContext {
private:
static constexpr const char *s_static_type = "NODE_GROUP";
- std::string node_name_;
+ int32_t node_id_;
+
+#ifdef DEBUG
+ std::string debug_node_name_;
+#endif
public:
- NodeGroupComputeContext(const ComputeContext *parent, std::string node_name);
+ NodeGroupComputeContext(const ComputeContext *parent, int32_t node_id);
+ NodeGroupComputeContext(const ComputeContext *parent, const bNode &node);
- StringRefNull node_name() const;
+ int32_t node_id() const
+ {
+ return node_id_;
+ }
private:
void print_current_in_line(std::ostream &stream) const override;
diff --git a/source/blender/blenkernel/BKE_node.h b/source/blender/blenkernel/BKE_node.h
index 2cd2fa9ac62..74d6f8becd1 100644
--- a/source/blender/blenkernel/BKE_node.h
+++ b/source/blender/blenkernel/BKE_node.h
@@ -668,6 +668,7 @@ void nodeUnlinkNode(struct bNodeTree *ntree, struct bNode *node);
* Find the first available, non-duplicate name for a given node.
*/
void nodeUniqueName(struct bNodeTree *ntree, struct bNode *node);
+void nodeUniqueID(struct bNodeTree *ntree, struct bNode *node);
/**
* Delete node, associated animation data and ID user count.
@@ -687,16 +688,17 @@ namespace blender::bke {
/**
* \note keeps socket list order identical, for copying links.
- * \note `unique_name` should usually be true, unless the \a dst_tree is temporary,
- * or the names can already be assumed valid.
+ * \param use_unique: If true, make sure the node's identifier and name are unique in the new
+ * tree. Must be *true* if the \a dst_tree had nodes that weren't in the source node's tree.
+ * Must be *false* when simply copying a node tree, so that identifiers don't change.
*/
bNode *node_copy_with_mapping(bNodeTree *dst_tree,
const bNode &node_src,
int flag,
- bool unique_name,
+ bool use_unique,
Map<const bNodeSocket *, bNodeSocket *> &new_socket_map);
-bNode *node_copy(bNodeTree *dst_tree, const bNode &src_node, int flag, bool unique_name);
+bNode *node_copy(bNodeTree *dst_tree, const bNode &src_node, int flag, bool use_unique);
} // namespace blender::bke
diff --git a/source/blender/blenkernel/BKE_node_runtime.hh b/source/blender/blenkernel/BKE_node_runtime.hh
index 56d51169934..a2241557315 100644
--- a/source/blender/blenkernel/BKE_node_runtime.hh
+++ b/source/blender/blenkernel/BKE_node_runtime.hh
@@ -8,6 +8,7 @@
#include "BLI_multi_value_map.hh"
#include "BLI_utility_mixins.hh"
#include "BLI_vector.hh"
+#include "BLI_vector_set.hh"
#include "DNA_node_types.h"
@@ -24,6 +25,36 @@ class NodeDeclaration;
struct GeometryNodesLazyFunctionGraphInfo;
} // namespace blender::nodes
+namespace blender {
+
+struct NodeIDHash {
+ uint64_t operator()(const bNode *node) const
+ {
+ return node->identifier;
+ }
+ uint64_t operator()(const int32_t id) const
+ {
+ return id;
+ }
+};
+
+struct NodeIDEquality {
+ bool operator()(const bNode *a, const bNode *b) const
+ {
+ return a->identifier == b->identifier;
+ }
+ bool operator()(const bNode *a, const int32_t b) const
+ {
+ return a->identifier == b;
+ }
+ bool operator()(const int32_t a, const bNode *b) const
+ {
+ return this->operator()(b, a);
+ }
+};
+
+} // namespace blender
+
namespace blender::bke {
class bNodeTreeRuntime : NonCopyable, NonMovable {
@@ -46,6 +77,13 @@ class bNodeTreeRuntime : NonCopyable, NonMovable {
*/
uint8_t runtime_flag = 0;
+ /**
+ * Storage of nodes based on their identifier. Also used as a contiguous array of nodes to
+ * allow simpler and more cache friendly iteration. Supports lookup by integer or by node.
+ * Unlike other caches, this is maintained eagerly while changing the tree.
+ */
+ VectorSet<bNode *, DefaultProbingStrategy, NodeIDHash, NodeIDEquality> nodes_by_id;
+
/** Execution data.
*
* XXX It would be preferable to completely move this data out of the underlying node tree,
@@ -91,7 +129,6 @@ class bNodeTreeRuntime : NonCopyable, NonMovable {
mutable std::atomic<int> allow_use_dirty_topology_cache = 0;
/** Only valid when #topology_cache_is_dirty is false. */
- Vector<bNode *> nodes;
Vector<bNodeLink *> links;
Vector<bNodeSocket *> sockets;
Vector<bNodeSocket *> input_sockets;
@@ -292,6 +329,28 @@ inline bool topology_cache_is_available(const bNodeSocket &socket)
/** \name #bNodeTree Inline Methods
* \{ */
+inline blender::Span<const bNode *> bNodeTree::all_nodes() const
+{
+ return this->runtime->nodes_by_id.as_span();
+}
+
+inline blender::Span<bNode *> bNodeTree::all_nodes()
+{
+ return this->runtime->nodes_by_id;
+}
+
+inline bNode *bNodeTree::node_by_id(const int32_t identifier)
+{
+ bNode *const *node = this->runtime->nodes_by_id.lookup_key_ptr_as(identifier);
+ return node ? *node : nullptr;
+}
+
+inline const bNode *bNodeTree::node_by_id(const int32_t identifier) const
+{
+ const bNode *const *node = this->runtime->nodes_by_id.lookup_key_ptr_as(identifier);
+ return node ? *node : nullptr;
+}
+
inline blender::Span<bNode *> bNodeTree::nodes_by_type(const blender::StringRefNull type_idname)
{
BLI_assert(blender::bke::node_tree_runtime::topology_cache_is_available(*this));
@@ -329,18 +388,6 @@ inline blender::Span<bNode *> bNodeTree::toposort_right_to_left()
return this->runtime->toposort_right_to_left;
}
-inline blender::Span<const bNode *> bNodeTree::all_nodes() const
-{
- BLI_assert(blender::bke::node_tree_runtime::topology_cache_is_available(*this));
- return this->runtime->nodes;
-}
-
-inline blender::Span<bNode *> bNodeTree::all_nodes()
-{
- BLI_assert(blender::bke::node_tree_runtime::topology_cache_is_available(*this));
- return this->runtime->nodes;
-}
-
inline blender::Span<const bNode *> bNodeTree::group_nodes() const
{
BLI_assert(blender::bke::node_tree_runtime::topology_cache_is_available(*this));
diff --git a/source/blender/blenkernel/intern/compute_contexts.cc b/source/blender/blenkernel/intern/compute_contexts.cc
index 026706d363e..4d0fba49f85 100644
--- a/source/blender/blenkernel/intern/compute_contexts.cc
+++ b/source/blender/blenkernel/intern/compute_contexts.cc
@@ -1,5 +1,7 @@
/* SPDX-License-Identifier: GPL-2.0-or-later */
+#include "DNA_node_types.h"
+
#include "BKE_compute_contexts.hh"
namespace blender::bke {
@@ -17,22 +19,30 @@ void ModifierComputeContext::print_current_in_line(std::ostream &stream) const
stream << "Modifier: " << modifier_name_;
}
-NodeGroupComputeContext::NodeGroupComputeContext(const ComputeContext *parent,
- std::string node_name)
- : ComputeContext(s_static_type, parent), node_name_(std::move(node_name))
+NodeGroupComputeContext::NodeGroupComputeContext(const ComputeContext *parent, const int node_id)
+ : ComputeContext(s_static_type, parent), node_id_(node_id)
{
hash_.mix_in(s_static_type, strlen(s_static_type));
- hash_.mix_in(node_name_.data(), node_name_.size());
+ hash_.mix_in(&node_id_, sizeof(int32_t));
}
-StringRefNull NodeGroupComputeContext::node_name() const
+NodeGroupComputeContext::NodeGroupComputeContext(const ComputeContext *parent, const bNode &node)
+ : NodeGroupComputeContext(parent, node.identifier)
{
- return node_name_;
+#ifdef DEBUG
+ debug_node_name_ = node.name;
+#endif
}
void NodeGroupComputeContext::print_current_in_line(std::ostream &stream) const
{
- stream << "Node: " << node_name_;
+#ifdef DEBUG
+ if (!debug_node_name_.empty()) {
+ stream << "Node: " << debug_node_name_;
+ return;
+ }
+#endif
+ stream << "Node ID: " << node_id_;
}
} // namespace blender::bke
diff --git a/source/blender/blenkernel/intern/node.cc b/source/blender/blenkernel/intern/node.cc
index 7c437f8fa9f..957150441ea 100644
--- a/source/blender/blenkernel/intern/node.cc
+++ b/source/blender/blenkernel/intern/node.cc
@@ -36,6 +36,7 @@
#include "BLI_listbase.h"
#include "BLI_map.hh"
#include "BLI_path_util.h"
+#include "BLI_rand.hh"
#include "BLI_set.hh"
#include "BLI_stack.hh"
#include "BLI_string.h"
@@ -84,6 +85,8 @@
#include "BLO_read_write.h"
+#include "PIL_time.h"
+
#define NODE_DEFAULT_MAX_WIDTH 700
using blender::Array;
@@ -146,12 +149,14 @@ static void ntree_copy_data(Main * /*bmain*/, ID *id_dst, const ID *id_src, cons
Map<const bNode *, bNode *> node_map;
Map<const bNodeSocket *, bNodeSocket *> socket_map;
+ ntree_dst->runtime->nodes_by_id.reserve(ntree_src->all_nodes().size());
BLI_listbase_clear(&ntree_dst->nodes);
LISTBASE_FOREACH (const bNode *, src_node, &ntree_src->nodes) {
/* Don't find a unique name for every node, since they should have valid names already. */
bNode *new_node = blender::bke::node_copy_with_mapping(
ntree_dst, *src_node, flag_subdata, false, socket_map);
node_map.add(src_node, new_node);
+ ntree_dst->runtime->nodes_by_id.add_new(new_node);
}
/* copy links */
@@ -679,6 +684,15 @@ void ntreeBlendReadData(BlendDataReader *reader, ID *owner_id, bNodeTree *ntree)
node->runtime = MEM_new<bNodeRuntime>(__func__);
node->typeinfo = nullptr;
+ /* Create the `nodes_by_id` cache eagerly so it can be expected to be valid. Because
+ * we create it here we also have to check for zero identifiers from previous versions. */
+ if (ntree->runtime->nodes_by_id.contains_as(node->identifier)) {
+ nodeUniqueID(ntree, node);
+ }
+ else {
+ ntree->runtime->nodes_by_id.add_new(node);
+ }
+
BLO_read_list(reader, &node->inputs);
BLO_read_list(reader, &node->outputs);
@@ -764,6 +778,7 @@ void ntreeBlendReadData(BlendDataReade
@@ Diff output truncated at 10240 characters. @@
More information about the Bf-blender-cvs
mailing list