From 4d39b6b3f4911123290c776fbaee7b0bcb4e3011 Mon Sep 17 00:00:00 2001 From: Jacques Lucke Date: Thu, 29 Dec 2022 19:36:15 +0100 Subject: [PATCH] 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 --- .../blender/editors/space_node/node_draw.cc | 4 ++- source/blender/modifiers/intern/MOD_nodes.cc | 30 ++++++++++++++++ .../nodes/NOD_geometry_nodes_lazy_function.hh | 14 +++++++- .../blender/nodes/NOD_geometry_nodes_log.hh | 2 ++ .../intern/geometry_nodes_lazy_function.cc | 18 +++++++--- .../nodes/intern/geometry_nodes_log.cc | 34 ++++++++++++------- 6 files changed, 83 insertions(+), 19 deletions(-) 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 &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(area->spacedata.first); + if (sl->spacetype == SPACE_NODE) { + const SpaceNode &snode = *reinterpret_cast(sl); + if (const std::optional 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(); + + Set 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 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 *side_effect_nodes; + const MultiValueMap *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 *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 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(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 bsockets = lf_graph_info_.mapping.bsockets_by_lf_socket_map.lookup(&lf_socket); if (bsockets.is_empty()) { return; } - GeoNodesLFUserData *user_data = dynamic_cast(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 get_modifier_for_node_editor(const Space return ObjectAndModifier{object, used_modifier}; } +std::optional GeoModifierLog::get_compute_context_hash_for_node_editor( + const SpaceNode &snode, const StringRefNull modifier_name) +{ + Vector tree_path = snode.treepath; + if (tree_path.is_empty()) { + return std::nullopt; + } + ComputeContextBuilder compute_context_builder; + compute_context_builder.push(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(*node); + } + return compute_context_builder.hash(); +} + GeoTreeLog *GeoModifierLog::get_tree_log_for_node_editor(const SpaceNode &snode) { std::optional 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 tree_path = snode.treepath; - if (tree_path.is_empty()) { - return nullptr; + if (const std::optional 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( - 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(*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)