From 9efc23bb690eaed1718adf727a406140a1647d3d Mon Sep 17 00:00:00 2001 From: Jacques Lucke Date: Mon, 9 Jun 2025 12:08:24 +0200 Subject: [PATCH] Fix #140018: no warning when using bake node or simulation in zone Baking and storing simulation state within loops or closures is not supported. Previously, attempting to use the bake node or simulation zone in such a zone would just silently fail. Now there is an error on the node and the bake settings are grayed out. Pull Request: https://projects.blender.org/blender/blender/pulls/140041 --- source/blender/editors/include/ED_node.hh | 9 +- .../blender/editors/space_node/space_node.cc | 94 +++++++++++-------- source/blender/nodes/CMakeLists.txt | 1 + .../nodes/NOD_geometry_nodes_lazy_function.hh | 8 +- source/blender/nodes/NOD_nested_node_id.hh | 20 ++++ .../nodes/geometry/include/NOD_geo_bake.hh | 1 + .../nodes/geometry/nodes/node_geo_bake.cc | 20 ++-- .../geometry/nodes/node_geo_simulation.cc | 7 ++ .../intern/geometry_nodes_lazy_function.cc | 36 +------ 9 files changed, 112 insertions(+), 84 deletions(-) create mode 100644 source/blender/nodes/NOD_nested_node_id.hh diff --git a/source/blender/editors/include/ED_node.hh b/source/blender/editors/include/ED_node.hh index 476d35d34ca..9ce12992737 100644 --- a/source/blender/editors/include/ED_node.hh +++ b/source/blender/editors/include/ED_node.hh @@ -14,6 +14,7 @@ #include "BKE_compute_context_cache_fwd.hh" #include "NOD_geometry_nodes_closure_location.hh" +#include "NOD_nested_node_id.hh" #include "ED_node_c.hh" @@ -82,7 +83,10 @@ void std_node_socket_colors_get(int socket_type, float *r_color); /** * Find the nested node id of a currently visible node in the root tree. */ -std::optional find_nested_node_id_in_root(const SpaceNode &snode, const bNode &node); +std::optional find_nested_node_id_in_root(const SpaceNode &snode, + const bNode &node); +std::optional find_nested_node_id_in_root( + const bNodeTree &root_tree, const ComputeContext *compute_context, const int node_id); struct ObjectAndModifier { const Object *object; @@ -112,6 +116,9 @@ bool node_editor_is_for_geometry_nodes_modifier(const SpaceNode &snode, bke::ComputeContextCache &compute_context_cache, const bNodeSocket &socket); +[[nodiscard]] const ComputeContext *compute_context_for_edittree_node( + const SpaceNode &snode, bke::ComputeContextCache &compute_context_cache, const bNode &node); + /** * Attempts to find a compute context that the closure is evaluated in. If none is found, null is * returned. If multiple are found, it currently picks the first one it finds which is somewhat diff --git a/source/blender/editors/space_node/space_node.cc b/source/blender/editors/space_node/space_node.cc index be03f905179..b04369adfa9 100644 --- a/source/blender/editors/space_node/space_node.cc +++ b/source/blender/editors/space_node/space_node.cc @@ -268,51 +268,53 @@ float2 space_node_group_offset(const SpaceNode &snode) return float2(0); } -static const bNode *group_node_by_name(const bNodeTree &ntree, StringRef name) -{ - for (const bNode *node : ntree.group_nodes()) { - if (node->name == name) { - return node; - } - } - return nullptr; -} - -std::optional find_nested_node_id_in_root(const SpaceNode &snode, const bNode &query_node) +std::optional find_nested_node_id_in_root(const SpaceNode &snode, + const bNode &query_node) { BLI_assert(snode.edittree->runtime->nodes_by_id.contains(const_cast(&query_node))); + bke::ComputeContextCache compute_context_cache; + const ComputeContext *compute_context = compute_context_for_edittree_node( + snode, compute_context_cache, query_node); + if (!compute_context) { + return {}; + } + return find_nested_node_id_in_root(*snode.nodetree, compute_context, query_node.identifier); +} - std::optional id_in_node; - const char *group_node_name = nullptr; - const bNode *node = &query_node; - LISTBASE_FOREACH_BACKWARD (const bNodeTreePath *, path, &snode.treepath) { - const bNodeTree *ntree = path->nodetree; - ntree->ensure_topology_cache(); - if (group_node_name) { - node = group_node_by_name(*ntree, group_node_name); +std::optional find_nested_node_id_in_root( + const bNodeTree &root_tree, const ComputeContext *compute_context, const int node_id) +{ + nodes::FoundNestedNodeID found; + Vector node_ids; + for (const ComputeContext *context = compute_context; context != nullptr; + context = context->parent()) + { + if (const auto *node_context = dynamic_cast(context)) { + node_ids.append(node_context->node_id()); } - bool found = false; - for (const bNestedNodeRef &ref : ntree->nested_node_refs_span()) { - if (node->is_group()) { - if (ref.path.node_id == node->identifier && ref.path.id_in_node == id_in_node) { - group_node_name = path->node_name; - id_in_node = ref.id; - found = true; - break; - } - } - else if (ref.path.node_id == node->identifier) { - group_node_name = path->node_name; - id_in_node = ref.id; - found = true; - break; - } + else if (dynamic_cast(context) != nullptr) { + found.is_in_loop = true; } - if (!found) { - return std::nullopt; + else if (dynamic_cast(context) != nullptr) { + found.is_in_simulation = true; + } + else if (dynamic_cast(context) != + nullptr) + { + found.is_in_loop = true; + } + else if (dynamic_cast(context) != nullptr) { + found.is_in_closure = true; } } - return id_in_node; + std::reverse(node_ids.begin(), node_ids.end()); + node_ids.append(node_id); + const bNestedNodeRef *nested_node_ref = root_tree.nested_node_ref_from_node_id_path(node_ids); + if (nested_node_ref == nullptr) { + return std::nullopt; + } + found.id = nested_node_ref->id; + return found; } std::optional get_modifier_for_node_editor(const SpaceNode &snode) @@ -645,6 +647,22 @@ const ComputeContext *compute_context_for_edittree_socket( return compute_context_for_zones(zone_stack, compute_context_cache, context); } +const ComputeContext *compute_context_for_edittree_node( + const SpaceNode &snode, bke::ComputeContextCache &compute_context_cache, const bNode &node) +{ + const ComputeContext *context = compute_context_for_edittree(snode, compute_context_cache); + if (!context) { + return nullptr; + } + const bke::bNodeTreeZones *zones = snode.edittree->zones(); + if (!zones) { + return nullptr; + } + const bke::bNodeTreeZone *zone = zones->get_zone_by_node(node.identifier); + const Vector zone_stack = zones->get_zones_to_enter_from_root(zone); + return compute_context_for_zones(zone_stack, compute_context_cache, context); +} + /* ******************** default callbacks for node space ***************** */ static SpaceLink *node_create(const ScrArea * /*area*/, const Scene * /*scene*/) diff --git a/source/blender/nodes/CMakeLists.txt b/source/blender/nodes/CMakeLists.txt index cc44f827003..2176230e60f 100644 --- a/source/blender/nodes/CMakeLists.txt +++ b/source/blender/nodes/CMakeLists.txt @@ -122,6 +122,7 @@ set(SRC NOD_inverse_eval_run.hh NOD_math_functions.hh NOD_multi_function.hh + NOD_nested_node_id.hh NOD_node_declaration.hh NOD_node_extra_info.hh NOD_node_in_compute_context.hh diff --git a/source/blender/nodes/NOD_geometry_nodes_lazy_function.hh b/source/blender/nodes/NOD_geometry_nodes_lazy_function.hh index b4dc788238b..39cfe4a00a7 100644 --- a/source/blender/nodes/NOD_geometry_nodes_lazy_function.hh +++ b/source/blender/nodes/NOD_geometry_nodes_lazy_function.hh @@ -27,6 +27,7 @@ #include "NOD_geometry_nodes_log.hh" #include "NOD_multi_function.hh" +#include "NOD_nested_node_id.hh" #include "BLI_compute_context.hh" #include "BLI_math_quaternion_types.hh" @@ -461,13 +462,6 @@ std::string make_anonymous_attribute_socket_inspection_string(const bNodeSocket std::string make_anonymous_attribute_socket_inspection_string(StringRef node_name, StringRef socket_name); -struct FoundNestedNodeID { - int id; - bool is_in_simulation = false; - bool is_in_loop = false; - bool is_in_closure = false; -}; - std::optional find_nested_node_id(const GeoNodesUserData &user_data, const int node_id); diff --git a/source/blender/nodes/NOD_nested_node_id.hh b/source/blender/nodes/NOD_nested_node_id.hh new file mode 100644 index 00000000000..5bccc267c10 --- /dev/null +++ b/source/blender/nodes/NOD_nested_node_id.hh @@ -0,0 +1,20 @@ +/* SPDX-FileCopyrightText: 2025 Blender Authors + * + * SPDX-License-Identifier: GPL-2.0-or-later */ + +#pragma once + +namespace blender::nodes { + +/** + * Utility struct to store information about a nested node id. Also see #bNestedNodeRef. + * Sometimes these IDs can only be used when they are at the top level and not within zones. + */ +struct FoundNestedNodeID { + int id; + bool is_in_simulation = false; + bool is_in_loop = false; + bool is_in_closure = false; +}; + +} // namespace blender::nodes diff --git a/source/blender/nodes/geometry/include/NOD_geo_bake.hh b/source/blender/nodes/geometry/include/NOD_geo_bake.hh index 4112aa96962..e5644b88add 100644 --- a/source/blender/nodes/geometry/include/NOD_geo_bake.hh +++ b/source/blender/nodes/geometry/include/NOD_geo_bake.hh @@ -111,6 +111,7 @@ struct BakeDrawContext { bool bake_still; bool is_baked; std::optional bake_target; + bool is_bakeable_in_current_context; }; [[nodiscard]] bool get_bake_draw_context(const bContext *C, diff --git a/source/blender/nodes/geometry/nodes/node_geo_bake.cc b/source/blender/nodes/geometry/nodes/node_geo_bake.cc index 29a2b402718..117dad29c4f 100644 --- a/source/blender/nodes/geometry/nodes/node_geo_bake.cc +++ b/source/blender/nodes/geometry/nodes/node_geo_bake.cc @@ -489,6 +489,12 @@ static void node_extra_info(NodeExtraInfoParams ¶ms) if (!get_bake_draw_context(¶ms.C, params.node, ctx)) { return; } + if (!ctx.is_bakeable_in_current_context) { + NodeExtraInfoRow row; + row.text = TIP_("Can't bake in zone"); + row.icon = ICON_ERROR; + params.rows.append(std::move(row)); + } if (ctx.is_baked) { NodeExtraInfoRow row; row.text = get_baked_string(ctx); @@ -503,7 +509,7 @@ static void node_layout(uiLayout *layout, bContext *C, PointerRNA *ptr) if (!get_bake_draw_context(C, node, ctx)) { return; } - + uiLayoutSetActive(layout, ctx.is_bakeable_in_current_context); uiLayoutSetEnabled(layout, ID_IS_EDITABLE(ctx.object)); uiLayout *col = &layout->column(false); { @@ -524,6 +530,7 @@ static void node_layout_ex(uiLayout *layout, bContext *C, PointerRNA *ptr) return; } + uiLayoutSetActive(layout, ctx.is_bakeable_in_current_context); uiLayoutSetEnabled(layout, ID_IS_EDITABLE(ctx.object)); { @@ -631,14 +638,15 @@ bool get_bake_draw_context(const bContext *C, const bNode &node, BakeDrawContext } r_ctx.object = object_and_modifier->object; r_ctx.nmd = object_and_modifier->nmd; - const std::optional bake_id = ed::space_node::find_nested_node_id_in_root(*r_ctx.snode, - *r_ctx.node); + const std::optional bake_id = ed::space_node::find_nested_node_id_in_root( + *r_ctx.snode, *r_ctx.node); if (!bake_id) { return false; } + r_ctx.is_bakeable_in_current_context = !bake_id->is_in_loop && !bake_id->is_in_closure; r_ctx.bake = nullptr; for (const NodesModifierBake &iter_bake : Span(r_ctx.nmd->bakes, r_ctx.nmd->bakes_num)) { - if (iter_bake.id == *bake_id) { + if (iter_bake.id == bake_id->id) { r_ctx.bake = &iter_bake; break; } @@ -653,7 +661,7 @@ bool get_bake_draw_context(const bContext *C, const bNode &node, BakeDrawContext const bke::bake::ModifierCache &cache = *r_ctx.nmd->runtime->cache; std::lock_guard lock{cache.mutex}; if (const std::unique_ptr *node_cache_ptr = - cache.bake_cache_by_id.lookup_ptr(*bake_id)) + cache.bake_cache_by_id.lookup_ptr(bake_id->id)) { const bke::bake::BakeNodeCache &node_cache = **node_cache_ptr; if (!node_cache.bake.frames.is_empty()) { @@ -663,7 +671,7 @@ bool get_bake_draw_context(const bContext *C, const bNode &node, BakeDrawContext } } else if (const std::unique_ptr *node_cache_ptr = - cache.simulation_cache_by_id.lookup_ptr(*bake_id)) + cache.simulation_cache_by_id.lookup_ptr(bake_id->id)) { const bke::bake::SimulationNodeCache &node_cache = **node_cache_ptr; if (!node_cache.bake.frames.is_empty() && diff --git a/source/blender/nodes/geometry/nodes/node_geo_simulation.cc b/source/blender/nodes/geometry/nodes/node_geo_simulation.cc index 0d0bf8013e5..f4f9f616b9c 100644 --- a/source/blender/nodes/geometry/nodes/node_geo_simulation.cc +++ b/source/blender/nodes/geometry/nodes/node_geo_simulation.cc @@ -225,6 +225,7 @@ static void node_layout_ex(uiLayout *layout, bContext *C, PointerRNA *current_no if (!get_bake_draw_context(C, output_node, ctx)) { return; } + uiLayoutSetActive(layout, ctx.is_bakeable_in_current_context); draw_simulation_state(C, layout, ntree, output_node); @@ -865,6 +866,12 @@ static void node_extra_info(NodeExtraInfoParams ¶ms) if (!get_bake_draw_context(¶ms.C, params.node, ctx)) { return; } + if (!ctx.is_bakeable_in_current_context) { + NodeExtraInfoRow row; + row.text = TIP_("Can't bake in zone"); + row.icon = ICON_ERROR; + params.rows.append(std::move(row)); + } if (ctx.is_baked) { NodeExtraInfoRow row; row.text = get_baked_string(ctx); diff --git a/source/blender/nodes/intern/geometry_nodes_lazy_function.cc b/source/blender/nodes/intern/geometry_nodes_lazy_function.cc index b1bf2481b0b..89a068ac5d9 100644 --- a/source/blender/nodes/intern/geometry_nodes_lazy_function.cc +++ b/source/blender/nodes/intern/geometry_nodes_lazy_function.cc @@ -47,6 +47,8 @@ #include "BKE_node_tree_zones.hh" #include "BKE_type_conversions.hh" +#include "ED_node.hh" + #include "FN_lazy_function_graph_executor.hh" #include "DEG_depsgraph_query.hh" @@ -4302,38 +4304,8 @@ void GeoNodesLocalUserData::ensure_tree_logger(const GeoNodesUserData &user_data std::optional find_nested_node_id(const GeoNodesUserData &user_data, const int node_id) { - FoundNestedNodeID found; - Vector node_ids; - for (const ComputeContext *context = user_data.compute_context; context != nullptr; - context = context->parent()) - { - if (const auto *node_context = dynamic_cast(context)) { - node_ids.append(node_context->node_id()); - } - else if (dynamic_cast(context) != nullptr) { - found.is_in_loop = true; - } - else if (dynamic_cast(context) != nullptr) { - found.is_in_simulation = true; - } - else if (dynamic_cast(context) != - nullptr) - { - found.is_in_loop = true; - } - else if (dynamic_cast(context) != nullptr) { - found.is_in_closure = true; - } - } - std::reverse(node_ids.begin(), node_ids.end()); - node_ids.append(node_id); - const bNestedNodeRef *nested_node_ref = - user_data.call_data->root_ntree->nested_node_ref_from_node_id_path(node_ids); - if (nested_node_ref == nullptr) { - return std::nullopt; - } - found.id = nested_node_ref->id; - return found; + return ed::space_node::find_nested_node_id_in_root( + *user_data.call_data->root_ntree, user_data.compute_context, node_id); } GeoNodesOperatorDepsgraphs::~GeoNodesOperatorDepsgraphs()