From 53ea147a51d8a91052b714bb9f235c0eeca2cda3 Mon Sep 17 00:00:00 2001 From: Jacques Lucke Date: Tue, 10 Dec 2024 17:52:44 +0100 Subject: [PATCH] Fix #131598: avoid relying on node tree update code being run before dependency graph is build This avoids assuming that `BKE_ntree_update_main` has run on a node tree before the depsgraph is build. The update code already finds the dependencies to determine if a relations update is necessary or not. To avoid detecting these dependencies again (which requires iterating over all the nested nodes), they were cached on the node group so that they can be used when the depsgraph is build. However, since the update code does not run in all necessary cases yet (#131598), this does not work currently. This patch fixes the situation by removing the optimization of not having to find all dependencies again when the depsgraph is build. This optimization can be introduced again after we can be more sure that the node tree update code runs whenever the node tree changes (which is what #131665 tries, but requires more discussion). Pull Request: https://projects.blender.org/blender/blender/pulls/131685 --- .../blenkernel/intern/node_tree_update.cc | 4 +- source/blender/modifiers/intern/MOD_nodes.cc | 9 ++-- .../nodes/NOD_geometry_nodes_dependencies.hh | 14 ++++-- .../intern/geometry_nodes_dependencies.cc | 45 ++++++++++++++++--- 4 files changed, 56 insertions(+), 16 deletions(-) diff --git a/source/blender/blenkernel/intern/node_tree_update.cc b/source/blender/blenkernel/intern/node_tree_update.cc index 5ea4516193d..bafb73aa385 100644 --- a/source/blender/blenkernel/intern/node_tree_update.cc +++ b/source/blender/blenkernel/intern/node_tree_update.cc @@ -870,8 +870,8 @@ class NodeTreeMainUpdater { void update_eval_dependencies(bNodeTree &ntree) { ntree.ensure_topology_cache(); - nodes::GeometryNodesEvalDependencies new_deps; - nodes::gather_geometry_nodes_eval_dependencies(ntree, new_deps); + nodes::GeometryNodesEvalDependencies new_deps = + nodes::gather_geometry_nodes_eval_dependencies_with_cache(ntree); /* Check if the dependencies have changed. */ if (!ntree.runtime->geometry_nodes_eval_dependencies || diff --git a/source/blender/modifiers/intern/MOD_nodes.cc b/source/blender/modifiers/intern/MOD_nodes.cc index d2475ae4ddf..943277e7991 100644 --- a/source/blender/modifiers/intern/MOD_nodes.cc +++ b/source/blender/modifiers/intern/MOD_nodes.cc @@ -178,10 +178,8 @@ static void update_depsgraph(ModifierData *md, const ModifierUpdateDepsgraphCont DEG_add_node_tree_output_relation(ctx->node, nmd->node_group, "Nodes Modifier"); - BLI_assert(nmd->node_group->runtime->geometry_nodes_eval_dependencies); - /* This intentionally makes a copy because a few extra dependencies are added below. */ nodes::GeometryNodesEvalDependencies eval_deps = - *nmd->node_group->runtime->geometry_nodes_eval_dependencies; + nodes::gather_geometry_nodes_eval_dependencies_recursive(*nmd->node_group); /* Create dependencies to data-blocks referenced by the settings in the modifier. */ find_dependencies_from_settings(nmd->settings, eval_deps); @@ -252,8 +250,9 @@ static bool depends_on_time(Scene * /*scene*/, ModifierData *md) return true; } } - BLI_assert(tree->runtime->geometry_nodes_eval_dependencies); - return tree->runtime->geometry_nodes_eval_dependencies->time_dependent; + nodes::GeometryNodesEvalDependencies eval_deps = + nodes::gather_geometry_nodes_eval_dependencies_recursive(*nmd->node_group); + return eval_deps.time_dependent; } static void foreach_ID_link(ModifierData *md, Object *ob, IDWalkFunc walk, void *user_data) diff --git a/source/blender/nodes/NOD_geometry_nodes_dependencies.hh b/source/blender/nodes/NOD_geometry_nodes_dependencies.hh index 3342268034c..617cf330996 100644 --- a/source/blender/nodes/NOD_geometry_nodes_dependencies.hh +++ b/source/blender/nodes/NOD_geometry_nodes_dependencies.hh @@ -76,10 +76,16 @@ struct GeometryNodesEvalDependencies { }; /** - * Find all evaluation dependencies for the given node tree. - * NOTE: It's assumed that all (indirectly) used node groups are updated already. + * Finds all evaluation dependencies for the given node. This does not include dependencies that + * are passed into the node group. It also may not contain all data-blocks referenced by the node + * tree if some of them can statically be detected to not be used by the evaluation. */ -void gather_geometry_nodes_eval_dependencies(bNodeTree &ntree, - GeometryNodesEvalDependencies &deps); +GeometryNodesEvalDependencies gather_geometry_nodes_eval_dependencies_recursive( + const bNodeTree &ntree); +/** + * Same as above, but assumes that dependencies are already cached on the referenced node groups. + */ +GeometryNodesEvalDependencies gather_geometry_nodes_eval_dependencies_with_cache( + const bNodeTree &ntree); } // namespace blender::nodes diff --git a/source/blender/nodes/intern/geometry_nodes_dependencies.cc b/source/blender/nodes/intern/geometry_nodes_dependencies.cc index a59bb0f1325..829ee67dca1 100644 --- a/source/blender/nodes/intern/geometry_nodes_dependencies.cc +++ b/source/blender/nodes/intern/geometry_nodes_dependencies.cc @@ -60,7 +60,7 @@ void GeometryNodesEvalDependencies::merge(const GeometryNodesEvalDependencies &o this->time_dependent |= other.time_dependent; } -static void add_eval_dependencies_from_socket(bNodeSocket &socket, +static void add_eval_dependencies_from_socket(const bNodeSocket &socket, GeometryNodesEvalDependencies &deps) { if (socket.is_input()) { @@ -133,10 +133,13 @@ static bool node_needs_own_transform(const bNode &node) } } -void gather_geometry_nodes_eval_dependencies(bNodeTree &ntree, GeometryNodesEvalDependencies &deps) +static void gather_geometry_nodes_eval_dependencies( + const bNodeTree &ntree, + GeometryNodesEvalDependencies &deps, + FunctionRef get_group_deps) { ntree.ensure_topology_cache(); - for (bNodeSocket *socket : ntree.all_sockets()) { + for (const bNodeSocket *socket : ntree.all_sockets()) { add_eval_dependencies_from_socket(*socket, deps); } deps.needs_active_camera |= !ntree.nodes_by_type("GeometryNodeInputActiveCamera").is_empty(); @@ -147,8 +150,8 @@ void gather_geometry_nodes_eval_dependencies(bNodeTree &ntree, GeometryNodesEval continue; } const bNodeTree &group = *reinterpret_cast(node->id); - if (group.runtime->geometry_nodes_eval_dependencies) { - deps.merge(*group.runtime->geometry_nodes_eval_dependencies); + if (const GeometryNodesEvalDependencies *group_deps = get_group_deps(group)) { + deps.merge(*group_deps); } } for (const bNode *node : ntree.all_nodes()) { @@ -156,4 +159,36 @@ void gather_geometry_nodes_eval_dependencies(bNodeTree &ntree, GeometryNodesEval } } +GeometryNodesEvalDependencies gather_geometry_nodes_eval_dependencies_with_cache( + const bNodeTree &ntree) +{ + GeometryNodesEvalDependencies deps; + gather_geometry_nodes_eval_dependencies(ntree, deps, [](const bNodeTree &group) { + return group.runtime->geometry_nodes_eval_dependencies.get(); + }); + return deps; +} + +static void gather_geometry_nodes_eval_dependencies_recursive_impl( + const bNodeTree &ntree, Map &deps_by_tree) +{ + if (deps_by_tree.contains(&ntree)) { + return; + } + GeometryNodesEvalDependencies new_deps; + gather_geometry_nodes_eval_dependencies(ntree, new_deps, [&](const bNodeTree &group) { + gather_geometry_nodes_eval_dependencies_recursive_impl(group, deps_by_tree); + return &deps_by_tree.lookup(&group); + }); + deps_by_tree.add(&ntree, std::move(new_deps)); +} + +GeometryNodesEvalDependencies gather_geometry_nodes_eval_dependencies_recursive( + const bNodeTree &ntree) +{ + Map deps_by_tree; + gather_geometry_nodes_eval_dependencies_recursive_impl(ntree, deps_by_tree); + return deps_by_tree.lookup(&ntree); +} + } // namespace blender::nodes