[Bf-blender-cvs] [4d39b6b3f49] master: Geometry Nodes: skip logging socket values for invisible trees

Jacques Lucke noreply at git.blender.org
Thu Dec 29 19:36:55 CET 2022


Commit: 4d39b6b3f4911123290c776fbaee7b0bcb4e3011
Author: Jacques Lucke
Date:   Thu Dec 29 19:36:15 2022 +0100
Branches: master
https://developer.blender.org/rB4d39b6b3f4911123290c776fbaee7b0bcb4e3011

Geometry Nodes: skip logging socket values for invisible trees

Geometry nodes used to log all socket values during evaluation.
This allowed the user to hover over any socket (that was evaluated)
to see its last value. The problem is that in large (nested) node trees,
the number of sockets becomes huge, causing a lot of performance
and memory overhead (in extreme cases, more than 70% of the
total execution time).

This patch changes it so, that only socket values are logged that the
user is likely to investigate. The simple heuristic is that socket values
of the currently visible node tree are logged.

The downside is that when the user changes the visible node tree, it
won't have any logged values until it is reevaluated. I updated the
tooltip message for that case to be a bit more precise.
If user feedback suggests that this new behavior is too annoying, we
can always add a UI option to log all socket values again. That shouldn't
be done without an actual need though because it takes up UI space.

Differential Revision: https://developer.blender.org/D16884

===================================================================

M	source/blender/editors/space_node/node_draw.cc
M	source/blender/modifiers/intern/MOD_nodes.cc
M	source/blender/nodes/NOD_geometry_nodes_lazy_function.hh
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

===================================================================

diff --git a/source/blender/editors/space_node/node_draw.cc b/source/blender/editors/space_node/node_draw.cc
index 669961351cb..bb9c16ada54 100644
--- a/source/blender/editors/space_node/node_draw.cc
+++ b/source/blender/editors/space_node/node_draw.cc
@@ -1117,7 +1117,9 @@ static char *node_socket_get_tooltip(const bContext *C,
       output << *socket_inspection_str;
     }
     else {
-      output << TIP_("The socket value has not been computed yet");
+      output << TIP_(
+          "Unknown socket value. Either the socket was not used or its value was not logged "
+          "during the last evaluation");
     }
   }
 
diff --git a/source/blender/modifiers/intern/MOD_nodes.cc b/source/blender/modifiers/intern/MOD_nodes.cc
index 258971c3565..ebd5bf351ea 100644
--- a/source/blender/modifiers/intern/MOD_nodes.cc
+++ b/source/blender/modifiers/intern/MOD_nodes.cc
@@ -923,6 +923,31 @@ static void find_side_effect_nodes(
   }
 }
 
+static void find_socket_log_contexts(const NodesModifierData &nmd,
+                                     const ModifierEvalContext &ctx,
+                                     Set<blender::ComputeContextHash> &r_socket_log_contexts)
+{
+  Main *bmain = DEG_get_bmain(ctx.depsgraph);
+  wmWindowManager *wm = (wmWindowManager *)bmain->wm.first;
+  if (wm == nullptr) {
+    return;
+  }
+  LISTBASE_FOREACH (const wmWindow *, window, &wm->windows) {
+    const bScreen *screen = BKE_workspace_active_screen_get(window->workspace_hook);
+    LISTBASE_FOREACH (const ScrArea *, area, &screen->areabase) {
+      const SpaceLink *sl = static_cast<SpaceLink *>(area->spacedata.first);
+      if (sl->spacetype == SPACE_NODE) {
+        const SpaceNode &snode = *reinterpret_cast<const SpaceNode *>(sl);
+        if (const std::optional<blender::ComputeContextHash> hash = blender::nodes::geo_eval_log::
+                GeoModifierLog::get_compute_context_hash_for_node_editor(snode,
+                                                                         nmd.modifier.name)) {
+          r_socket_log_contexts.add(*hash);
+        }
+      }
+    }
+  }
+}
+
 static void clear_runtime_data(NodesModifierData *nmd)
 {
   if (nmd->runtime_eval_log != nullptr) {
@@ -1122,8 +1147,13 @@ static GeometrySet compute_geometry(
   geo_nodes_modifier_data.depsgraph = ctx->depsgraph;
   geo_nodes_modifier_data.self_object = ctx->object;
   auto eval_log = std::make_unique<GeoModifierLog>();
+
+  Set<blender::ComputeContextHash> socket_log_contexts;
   if (logging_enabled(ctx)) {
     geo_nodes_modifier_data.eval_log = eval_log.get();
+
+    find_socket_log_contexts(*nmd, *ctx, socket_log_contexts);
+    geo_nodes_modifier_data.socket_log_contexts = &socket_log_contexts;
   }
   MultiValueMap<blender::ComputeContextHash, const lf::FunctionNode *> r_side_effect_nodes;
   find_side_effect_nodes(*nmd, *ctx, r_side_effect_nodes);
diff --git a/source/blender/nodes/NOD_geometry_nodes_lazy_function.hh b/source/blender/nodes/NOD_geometry_nodes_lazy_function.hh
index f21100414b3..84c264f4976 100644
--- a/source/blender/nodes/NOD_geometry_nodes_lazy_function.hh
+++ b/source/blender/nodes/NOD_geometry_nodes_lazy_function.hh
@@ -48,7 +48,15 @@ struct GeoNodesModifierData {
    * Some nodes should be executed even when their output is not used (e.g. active viewer nodes and
    * the node groups they are contained in).
    */
-  const MultiValueMap<ComputeContextHash, const lf::FunctionNode *> *side_effect_nodes;
+  const MultiValueMap<ComputeContextHash, const lf::FunctionNode *> *side_effect_nodes = nullptr;
+  /**
+   * Controls in which compute contexts we want to log socket values. Logging them in all contexts
+   * can result in slowdowns. In the majority of cases, the logged socket values are freed without
+   * being looked at anyway.
+   *
+   * If this is null, all socket values will be logged.
+   */
+  const Set<ComputeContextHash> *socket_log_contexts = nullptr;
 };
 
 /**
@@ -64,6 +72,10 @@ struct GeoNodesLFUserData : public lf::UserData {
    * evaluated.
    */
   const ComputeContext *compute_context = nullptr;
+  /**
+   * Log socket values in the current compute context. Child contexts might use logging again.
+   */
+  bool log_socket_values = true;
 };
 
 /**
diff --git a/source/blender/nodes/NOD_geometry_nodes_log.hh b/source/blender/nodes/NOD_geometry_nodes_log.hh
index d99dff21c6d..95e54f1b9a5 100644
--- a/source/blender/nodes/NOD_geometry_nodes_log.hh
+++ b/source/blender/nodes/NOD_geometry_nodes_log.hh
@@ -332,6 +332,8 @@ class GeoModifierLog {
   /**
    * Utility accessor to logged data.
    */
+  static std::optional<ComputeContextHash> get_compute_context_hash_for_node_editor(
+      const SpaceNode &snode, StringRefNull modifier_name);
   static GeoTreeLog *get_tree_log_for_node_editor(const SpaceNode &snode);
   static const ViewerNodeLog *find_viewer_node_log_for_path(const ViewerPath &viewer_path);
 };
diff --git a/source/blender/nodes/intern/geometry_nodes_lazy_function.cc b/source/blender/nodes/intern/geometry_nodes_lazy_function.cc
index c537fe67422..57033e89bf7 100644
--- a/source/blender/nodes/intern/geometry_nodes_lazy_function.cc
+++ b/source/blender/nodes/intern/geometry_nodes_lazy_function.cc
@@ -667,6 +667,10 @@ class LazyFunctionForGroupNode : public LazyFunction {
                                                  group_node_.identifier};
     GeoNodesLFUserData group_user_data = *user_data;
     group_user_data.compute_context = &compute_context;
+    if (user_data->modifier_data->socket_log_contexts) {
+      group_user_data.log_socket_values = user_data->modifier_data->socket_log_contexts->contains(
+          compute_context.hash());
+    }
 
     lf::Context group_context = context;
     group_context.user_data = &group_user_data;
@@ -1305,17 +1309,21 @@ void GeometryNodesLazyFunctionLogger::log_socket_value(
     const GPointer value,
     const fn::lazy_function::Context &context) const
 {
+  GeoNodesLFUserData *user_data = dynamic_cast<GeoNodesLFUserData *>(context.user_data);
+  BLI_assert(user_data != nullptr);
+  if (!user_data->log_socket_values) {
+    return;
+  }
+  if (user_data->modifier_data->eval_log == nullptr) {
+    return;
+  }
+
   const Span<const bNodeSocket *> bsockets =
       lf_graph_info_.mapping.bsockets_by_lf_socket_map.lookup(&lf_socket);
   if (bsockets.is_empty()) {
     return;
   }
 
-  GeoNodesLFUserData *user_data = dynamic_cast<GeoNodesLFUserData *>(context.user_data);
-  BLI_assert(user_data != nullptr);
-  if (user_data->modifier_data->eval_log == nullptr) {
-    return;
-  }
   geo_eval_log::GeoTreeLogger &tree_logger =
       user_data->modifier_data->eval_log->get_local_tree_logger(*user_data->compute_context);
   for (const bNodeSocket *bsocket : bsockets) {
diff --git a/source/blender/nodes/intern/geometry_nodes_log.cc b/source/blender/nodes/intern/geometry_nodes_log.cc
index eb2e9bd9015..660f878c1f7 100644
--- a/source/blender/nodes/intern/geometry_nodes_log.cc
+++ b/source/blender/nodes/intern/geometry_nodes_log.cc
@@ -513,6 +513,23 @@ static std::optional<ObjectAndModifier> get_modifier_for_node_editor(const Space
   return ObjectAndModifier{object, used_modifier};
 }
 
+std::optional<ComputeContextHash> GeoModifierLog::get_compute_context_hash_for_node_editor(
+    const SpaceNode &snode, const StringRefNull modifier_name)
+{
+  Vector<const bNodeTreePath *> tree_path = snode.treepath;
+  if (tree_path.is_empty()) {
+    return std::nullopt;
+  }
+  ComputeContextBuilder compute_context_builder;
+  compute_context_builder.push<bke::ModifierComputeContext>(modifier_name);
+  for (const int i : tree_path.index_range().drop_back(1)) {
+    /* The tree path contains the name of the node but not its ID. */
+    const bNode *node = nodeFindNodebyName(tree_path[i]->nodetree, tree_path[i + 1]->node_name);
+    compute_context_builder.push<bke::NodeGroupComputeContext>(*node);
+  }
+  return compute_context_builder.hash();
+}
+
 GeoTreeLog *GeoModifierLog::get_tree_log_for_node_editor(const SpaceNode &snode)
 {
   std::optional<ObjectAndModifier> object_and_modifier = get_modifier_for_node_editor(snode);
@@ -524,19 +541,12 @@ GeoTreeLog *GeoModifierLog::get_tree_log_for_node_editor(const SpaceNode &snode)
   if (modifier_log == nullptr) {
     return nullptr;
   }
-  Vector<const bNodeTreePath *> tree_path = snode.treepath;
-  if (tree_path.is_empty()) {
-    return nullptr;
+  if (const std::optional<ComputeContextHash> hash =
+          GeoModifierLog::get_compute_context_hash_for_node_editor(
+              snode, object_and_modifier->nmd->modifier.name)) {
+    return &modifier_log->get_tree_log(*hash);
   }
-  ComputeContextBuilder compute_context_builder;
-  compute_context_builder.push<bke::ModifierComputeContext>(
-      object_and_modifier->nmd->modifier.name);
-  for (const int i : tree_path.index_range().drop_back(1)) {
-    /* The tree path contains the name of the node but not its ID. */
-    const bNode *node = nodeFindNodebyName(tree_path[i]->nodetree, tree_path[i + 1]->node_name);
-    compute_context_builder.push<bke::NodeGroupComputeContext>(*node);
-  }
-  return &modifier_log->get_tree_log(compute_context_builder.hash());
+  return nullptr;
 }
 
 const ViewerNodeLog *GeoModifierLog::find_viewer_node_log_for_path(const ViewerPath &viewer_path)



More information about the Bf-blender-cvs mailing list