Geometry nodes: Crash for cycle links in node group when try to find viewer node #102882

New Issue

Christophe Mermoud · 2022-11-30T18:18:18+01:00

2022-11-30 18:18:18 +01:00

System Information
Operating system: Linux-4.15.0-120-generic-x86_64-with-glibc2.27 64 Bits
Graphics card: GeForce GTX 680/PCIe/SSE2 NVIDIA Corporation 4.5.0 NVIDIA 450.80.02

Blender Version
Broken: version: 3.4.0 Beta, branch: blender-v3.4-release, commit 5c1cc79cf4 date: 2022-11-30 16:24, hash: 5c1cc79cf4
Worked: Never. This bug involves the previewer node and was already present at branch blender-v3.4-release, commit c55d38f00b

Short description of error

Error   : EXCEPTION_ACCESS_VIOLATION
Address : 0x00007FF6088DCCC5
Module  : blender.exe
Thread  : 000035d8

Description of the problem:
ensure_geometry_nodes_lazy_function_graph can returen nullptr for tree with link cycle

static const lf::FunctionNode &find_viewer_lf_node(const bNode &viewer_bnode)
{
  return *blender::nodes::ensure_geometry_nodes_lazy_function_graph(viewer_bnode.owner_tree())->mapping.viewer_node_map.lookup(&viewer_bnode);
}

static const lf::FunctionNode &find_group_lf_node(const bNode &group_bnode)
{
  return *blender::nodes::ensure_geometry_nodes_lazy_function_graph(group_bnode.owner_tree())->mapping.group_node_map.lookup(&group_bnode);
}

Exact steps for others to reproduce the error

Open the file and connect the reroute nodes.

**System Information** Operating system: Linux-4.15.0-120-generic-x86_64-with-glibc2.27 64 Bits Graphics card: GeForce GTX 680/PCIe/SSE2 NVIDIA Corporation 4.5.0 NVIDIA 450.80.02 **Blender Version** Broken: version: 3.4.0 Beta, branch: blender-v3.4-release, commit 5c1cc79cf4ea9b557d3ee843f275074441b90894 date: 2022-11-30 16:24, hash: `5c1cc79cf4` Worked: Never. This bug involves the previewer node and was already present at branch blender-v3.4-release, commit c55d38f00b8c0e6ae8bda9cc66614afe28fb3fc9 **Short description of error** ``` Error : EXCEPTION_ACCESS_VIOLATION Address : 0x00007FF6088DCCC5 Module : blender.exe Thread : 000035d8 ``` Description of the problem: `ensure_geometry_nodes_lazy_function_graph` can returen nullptr for tree with link cycle ```lang=c++ static const lf::FunctionNode &find_viewer_lf_node(const bNode &viewer_bnode) { return *blender::nodes::ensure_geometry_nodes_lazy_function_graph(viewer_bnode.owner_tree())->mapping.viewer_node_map.lookup(&viewer_bnode); } ``` ``` static const lf::FunctionNode &find_group_lf_node(const bNode &group_bnode) { return *blender::nodes::ensure_geometry_nodes_lazy_function_graph(group_bnode.owner_tree())->mapping.group_node_map.lookup(&group_bnode); } ``` **Exact steps for others to reproduce the error** Open the file and connect the reroute nodes.

Christophe Mermoud commented

2022-11-30 18:18:18 +01:00

Added subscriber: @ChristopheMermoud

Iliya Katushenock commented

2022-11-30 18:19:24 +01:00

Added subscriber: @mod_moder

Iliya Katushenock commented

2022-11-30 18:19:24 +01:00

I check ...

Iliya Katushenock commented

2022-11-30 18:24:50 +01:00

Changed status from 'Needs Triage' to: 'Confirmed'

Iliya Katushenock commented

2022-11-30 18:24:50 +01:00

I think find problem

The search for preview nodes does not take into account that the tree may be cyclic. Because of this, there may be a reading from the 0.
In general, it should be possible to learn about the tree problem and log it as a preview node problem.

P3360: (An Untitled Masterwork)

diff --git a/source/blender/modifiers/intern/MOD_nodes.cc b/source/blender/modifiers/intern/MOD_nodes.cc
index c2b14262b4f..791c93425ec 100644
--- a/source/blender/modifiers/intern/MOD_nodes.cc
+++ b/source/blender/modifiers/intern/MOD_nodes.cc
@@ -780,203 +780,221 @@ static void initialize_group_input(NodesModifierData &nmd,
       interface_socket.type);
   if (nmd.settings.properties == nullptr) {
     socket_type.get_geometry_nodes_cpp_value(interface_socket, r_value);
     return;
   }
   const IDProperty *property = IDP_GetPropertyFromGroup(nmd.settings.properties,
                                                         interface_socket.identifier);
   if (property == nullptr) {
     socket_type.get_geometry_nodes_cpp_value(interface_socket, r_value);
     return;
   }
   if (!id_property_type_matches_socket(interface_socket, *property)) {
     socket_type.get_geometry_nodes_cpp_value(interface_socket, r_value);
     return;
   }
 
   if (!input_has_attribute_toggle(*nmd.node_group, input_index)) {
     init_socket_cpp_value_from_property(*property, socket_data_type, r_value);
     return;
   }
 
   const IDProperty *property_use_attribute = IDP_GetPropertyFromGroup(
       nmd.settings.properties, (interface_socket.identifier + use_attribute_suffix).c_str());
   const IDProperty *property_attribute_name = IDP_GetPropertyFromGroup(
       nmd.settings.properties, (interface_socket.identifier + attribute_name_suffix).c_str());
   if (property_use_attribute == nullptr || property_attribute_name == nullptr) {
     init_socket_cpp_value_from_property(*property, socket_data_type, r_value);
     return;
   }
 
   const bool use_attribute = IDP_Int(property_use_attribute) != 0;
   if (use_attribute) {
     const StringRef attribute_name{IDP_String(property_attribute_name)};
     if (!blender::bke::allow_procedural_attribute_access(attribute_name)) {
       init_socket_cpp_value_from_property(*property, socket_data_type, r_value);
       return;
     }
     auto attribute_input = std::make_shared<blender::bke::AttributeFieldInput>(
         attribute_name, *socket_type.base_cpp_type);
     GField attribute_field{std::move(attribute_input), 0};
     const auto *value_or_field_cpp_type = ValueOrFieldCPPType::get_from_self(
         *socket_type.geometry_nodes_cpp_type);
     BLI_assert(value_or_field_cpp_type != nullptr);
     value_or_field_cpp_type->construct_from_field(r_value, std::move(attribute_field));
   }
   else {
     init_socket_cpp_value_from_property(*property, socket_data_type, r_value);
   }
 }
 
-static const lf::FunctionNode &find_viewer_lf_node(const bNode &viewer_bnode)
+static const lf::FunctionNode *find_viewer_lf_node(const bNode &viewer_bnode)
 {
-  return *blender::nodes::ensure_geometry_nodes_lazy_function_graph(viewer_bnode.owner_tree())
-              ->mapping.viewer_node_map.lookup(&viewer_bnode);
+  const blender::nodes::GeometryNodesLazyFunctionGraphInfo *viewer_graph = blender::nodes::ensure_geometry_nodes_lazy_function_graph(viewer_bnode.owner_tree());
+  /* Maybe nullptr if node tree has cycle links. */
+  if (viewer_graph){
+    return viewer_graph->mapping.viewer_node_map.lookup(&viewer_bnode);
+  }
+  return nullptr;
 }
-static const lf::FunctionNode &find_group_lf_node(const bNode &group_bnode)
+static const lf::FunctionNode *find_group_lf_node(const bNode &group_bnode)
 {
-  return *blender::nodes::ensure_geometry_nodes_lazy_function_graph(group_bnode.owner_tree())
-              ->mapping.group_node_map.lookup(&group_bnode);
+  const blender::nodes::GeometryNodesLazyFunctionGraphInfo *group_graph = blender::nodes::ensure_geometry_nodes_lazy_function_graph(group_bnode.owner_tree());
+  
+  if (group_graph){
+    return group_graph->mapping.group_node_map.lookup(&group_bnode);
+  }
+  return nullptr;
 }
 
 static void find_side_effect_nodes_for_viewer_path(
     const ViewerPath &viewer_path,
+    ModifierData &md,
     const NodesModifierData &nmd,
     const ModifierEvalContext &ctx,
     MultiValueMap<blender::ComputeContextHash, const lf::FunctionNode *> &r_side_effect_nodes)
 {
   const std::optional<blender::ed::viewer_path::ViewerPathForGeometryNodesViewer> parsed_path =
       blender::ed::viewer_path::parse_geometry_nodes_viewer(viewer_path);
   if (!parsed_path.has_value()) {
     return;
   }
   if (parsed_path->object != DEG_get_original_object(ctx.object)) {
     return;
   }
   if (parsed_path->modifier_name != nmd.modifier.name) {
     return;
   }
 
   blender::ComputeContextBuilder compute_context_builder;
   compute_context_builder.push<blender::bke::ModifierComputeContext>(parsed_path->modifier_name);
 
   const bNodeTree *group = nmd.node_group;
   Stack<const bNode *> group_node_stack;
   for (const StringRefNull group_node_name : parsed_path->group_node_names) {
     const bNode *found_node = nullptr;
     for (const bNode *node : group->group_nodes()) {
       if (node->name == group_node_name) {
         found_node = node;
         break;
       }
     }
     if (found_node == nullptr) {
       return;
     }
     if (found_node->id == nullptr) {
       return;
     }
     if (found_node->is_muted()) {
       return;
     }
     group_node_stack.push(found_node);
     group = reinterpret_cast<bNodeTree *>(found_node->id);
     compute_context_builder.push<blender::bke::NodeGroupComputeContext>(group_node_name);
   }
 
   const bNode *found_viewer_node = nullptr;
   for (const bNode *viewer_node : group->nodes_by_type("GeometryNodeViewer")) {
     if (viewer_node->name == parsed_path->viewer_node_name) {
       found_viewer_node = viewer_node;
       break;
     }
   }
   if (found_viewer_node == nullptr) {
     return;
   }
 
   /* Not only mark the viewer node as having side effects, but also all group nodes it is contained
    * in. */
-  r_side_effect_nodes.add(compute_context_builder.hash(),
-                          &find_viewer_lf_node(*found_viewer_node));
+  const lf::FunctionNode *viewer_lf = find_viewer_lf_node(*found_viewer_node);
+  if (!viewer_lf){
+    BKE_modifier_set_error(ctx.object, &md, "Can not use viewer node");
+    return;
+  }
+  r_side_effect_nodes.add(compute_context_builder.hash(), viewer_lf);
   compute_context_builder.pop();
   while (!compute_context_builder.is_empty()) {
-    r_side_effect_nodes.add(compute_context_builder.hash(),
-                            &find_group_lf_node(*group_node_stack.pop()));
+    const lf::FunctionNode *group_lf = find_group_lf_node(*group_node_stack.pop());
+    if (group_lf){
+      r_side_effect_nodes.add(compute_context_builder.hash(), group_lf);
+    }else{
+      BKE_modifier_set_error(ctx.object, &md, "Can not use viewer node");
+    }
     compute_context_builder.pop();
   }
 }
 
 static void find_side_effect_nodes(
     const NodesModifierData &nmd,
+    ModifierData &md,
     const ModifierEvalContext &ctx,
     MultiValueMap<blender::ComputeContextHash, const lf::FunctionNode *> &r_side_effect_nodes)
 {
   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);
     const WorkSpace *workspace = BKE_workspace_active_get(window->workspace_hook);
-    find_side_effect_nodes_for_viewer_path(workspace->viewer_path, nmd, ctx, r_side_effect_nodes);
+    find_side_effect_nodes_for_viewer_path(workspace->viewer_path, md, nmd, ctx, r_side_effect_nodes);
     LISTBASE_FOREACH (const ScrArea *, area, &screen->areabase) {
       const SpaceLink *sl = static_cast<SpaceLink *>(area->spacedata.first);
       if (sl->spacetype == SPACE_SPREADSHEET) {
         const SpaceSpreadsheet &sspreadsheet = *reinterpret_cast<const SpaceSpreadsheet *>(sl);
         find_side_effect_nodes_for_viewer_path(
-            sspreadsheet.viewer_path, nmd, ctx, r_side_effect_nodes);
+            sspreadsheet.viewer_path, md, nmd, ctx, r_side_effect_nodes);
       }
       if (sl->spacetype == SPACE_VIEW3D) {
         const View3D &v3d = *reinterpret_cast<const View3D *>(sl);
-        find_side_effect_nodes_for_viewer_path(v3d.viewer_path, nmd, ctx, r_side_effect_nodes);
+        find_side_effect_nodes_for_viewer_path(v3d.viewer_path, md, nmd, ctx, r_side_effect_nodes);
       }
     }
   }
 }
 
 static void clear_runtime_data(NodesModifierData *nmd)
 {
   if (nmd->runtime_eval_log != nullptr) {
     delete static_cast<GeoModifierLog *>(nmd->runtime_eval_log);
     nmd->runtime_eval_log = nullptr;
   }
 }
 
 struct OutputAttributeInfo {
   GField field;
   StringRefNull name;
 };
 
 struct OutputAttributeToStore {
   GeometryComponentType component_type;
   eAttrDomain domain;
   StringRefNull name;
   GMutableSpan data;
 };
 
 /**
  * The output attributes are organized based on their domain, because attributes on the same domain
  * can be evaluated together.
  */
 static MultiValueMap<eAttrDomain, OutputAttributeInfo> find_output_attributes_to_store(
     const NodesModifierData &nmd, const bNode &output_node, Span<GMutablePointer> output_values)
 {
   MultiValueMap<eAttrDomain, OutputAttributeInfo> outputs_by_domain;
   for (const bNodeSocket *socket : output_node.input_sockets().drop_front(1).drop_back(1)) {
     if (!socket_type_has_attribute_toggle(*socket)) {
       continue;
     }
 
     const std::string prop_name = socket->identifier + attribute_name_suffix;
     const IDProperty *prop = IDP_GetPropertyFromGroup(nmd.settings.properties, prop_name.c_str());
     if (prop == nullptr) {
       continue;
     }
     const StringRefNull attribute_name = IDP_String(prop);
     if (attribute_name.is_empty()) {
       continue;
     }
     if (!blender::bke::allow_procedural_attribute_access(attribute_name)) {
       continue;
     }
@@ -1060,132 +1078,133 @@ static void store_computed_output_attributes(
     if (meta_data.has_value() &&
         (meta_data->domain != store.domain || meta_data->data_type != data_type)) {
       attributes.remove(store.name);
     }
 
     /* Try to create the attribute reusing the stored buffer. This will only succeed if the
      * attribute didn't exist before, or if it existed but was removed above. */
     if (attributes.add(store.name,
                        store.domain,
                        blender::bke::cpp_type_to_custom_data_type(store.data.type()),
                        blender::bke::AttributeInitMoveArray(store.data.data()))) {
       continue;
     }
 
     blender::bke::GAttributeWriter attribute = attributes.lookup_or_add_for_write(
         store.name, store.domain, data_type);
     if (attribute) {
       attribute.varray.set_all(store.data.data());
       attribute.finish();
     }
 
     /* We were unable to reuse the data, so it must be destructed and freed. */
     store.data.type().destruct_n(store.data.data(), store.data.size());
     MEM_freeN(store.data.data());
   }
 }
 
 static void store_output_attributes(GeometrySet &geometry,
                                     const NodesModifierData &nmd,
                                     const bNode &output_node,
                                     Span<GMutablePointer> output_values)
 {
   /* All new attribute values have to be computed before the geometry is actually changed. This is
    * necessary because some fields might depend on attributes that are overwritten. */
   MultiValueMap<eAttrDomain, OutputAttributeInfo> outputs_by_domain =
       find_output_attributes_to_store(nmd, output_node, output_values);
   Vector<OutputAttributeToStore> attributes_to_store = compute_attributes_to_store(
       geometry, outputs_by_domain);
   store_computed_output_attributes(geometry, attributes_to_store);
 }
 
 /**
  * Evaluate a node group to compute the output geometry.
  */
 static GeometrySet compute_geometry(
     const bNodeTree &btree,
     const blender::nodes::GeometryNodesLazyFunctionGraphInfo &lf_graph_info,
     const bNode &output_node,
     GeometrySet input_geometry_set,
     NodesModifierData *nmd,
+    ModifierData *md,
     const ModifierEvalContext *ctx)
 {
   const blender::nodes::GeometryNodeLazyFunctionGraphMapping &mapping = lf_graph_info.mapping;
 
   Span<const lf::OutputSocket *> graph_inputs = mapping.group_input_sockets;
   Vector<const lf::InputSocket *> graph_outputs;
   for (const bNodeSocket *bsocket : output_node.input_sockets().drop_back(1)) {
     const lf::InputSocket &socket = mapping.dummy_socket_map.lookup(bsocket)->as_input();
     graph_outputs.append(&socket);
   }
 
   Array<GMutablePointer> param_inputs(graph_inputs.size());
   Array<GMutablePointer> param_outputs(graph_outputs.size());
   Array<std::optional<lf::ValueUsage>> param_input_usages(graph_inputs.size());
   Array<lf::ValueUsage> param_output_usages(graph_outputs.size(), lf::ValueUsage::Used);
   Array<bool> param_set_outputs(graph_outputs.size(), false);
 
   blender::nodes::GeometryNodesLazyFunctionLogger lf_logger(lf_graph_info);
   blender::nodes::GeometryNodesLazyFunctionSideEffectProvider lf_side_effect_provider;
 
   lf::GraphExecutor graph_executor{
       lf_graph_info.graph, graph_inputs, graph_outputs, &lf_logger, &lf_side_effect_provider};
 
   blender::nodes::GeoNodesModifierData geo_nodes_modifier_data;
   geo_nodes_modifier_data.depsgraph = ctx->depsgraph;
   geo_nodes_modifier_data.self_object = ctx->object;
   auto eval_log = std::make_unique<GeoModifierLog>();
   if (logging_enabled(ctx)) {
     geo_nodes_modifier_data.eval_log = eval_log.get();
   }
   MultiValueMap<blender::ComputeContextHash, const lf::FunctionNode *> r_side_effect_nodes;
-  find_side_effect_nodes(*nmd, *ctx, r_side_effect_nodes);
+  find_side_effect_nodes(*nmd, *md, *ctx, r_side_effect_nodes);
   geo_nodes_modifier_data.side_effect_nodes = &r_side_effect_nodes;
   blender::nodes::GeoNodesLFUserData user_data;
   user_data.modifier_data = &geo_nodes_modifier_data;
   blender::bke::ModifierComputeContext modifier_compute_context{nullptr, nmd->modifier.name};
   user_data.compute_context = &modifier_compute_context;
 
   blender::LinearAllocator<> allocator;
   Vector<GMutablePointer> inputs_to_destruct;
 
   int input_index;
   LISTBASE_FOREACH_INDEX (bNodeSocket *, interface_socket, &btree.inputs, input_index) {
     if (interface_socket->type == SOCK_GEOMETRY && input_index == 0) {
       param_inputs[input_index] = &input_geometry_set;
       continue;
     }
 
     const CPPType *type = interface_socket->typeinfo->geometry_nodes_cpp_type;
     BLI_assert(type != nullptr);
     void *value = allocator.allocate(type->size(), type->alignment());
     initialize_group_input(*nmd, *interface_socket, input_index, value);
     param_inputs[input_index] = {type, value};
     inputs_to_destruct.append({type, value});
   }
 
   for (const int i : graph_outputs.index_range()) {
     const lf::InputSocket &socket = *graph_outputs[i];
     const CPPType &type = socket.type();
     void *buffer = allocator.allocate(type.size(), type.alignment());
     param_outputs- [x] = {type, buffer};
   }
 
   lf::Context lf_context;
   lf_context.storage = graph_executor.init_storage(allocator);
   lf_context.user_data = &user_data;
   lf::BasicParams lf_params{graph_executor,
                             param_inputs,
                             param_outputs,
                             param_input_usages,
                             param_output_usages,
                             param_set_outputs};
   graph_executor.execute(lf_params, lf_context);
   graph_executor.destruct_storage(lf_context.storage);
 
   for (GMutablePointer &ptr : inputs_to_destruct) {
     ptr.destruct();
   }
 
   GeometrySet output_geometry_set = std::move(*static_cast<GeometrySet *>(param_outputs- [x].get()));
   store_output_attributes(output_geometry_set, *nmd, output_node, param_outputs);
 
@@ -1255,101 +1274,101 @@ static void modifyGeometry(ModifierData *md,
 {
   NodesModifierData *nmd = reinterpret_cast<NodesModifierData *>(md);
   if (nmd->node_group == nullptr) {
     return;
   }
 
   const bNodeTree &tree = *nmd->node_group;
   tree.ensure_topology_cache();
   check_property_socket_sync(ctx->object, md);
 
   const bNode *output_node = tree.group_output_node();
   if (output_node == nullptr) {
     BKE_modifier_set_error(ctx->object, md, "Node group must have a group output node");
     geometry_set.clear();
     return;
   }
 
   Span<const bNodeSocket *> group_outputs = output_node->input_sockets().drop_back(1);
   if (group_outputs.is_empty()) {
     BKE_modifier_set_error(ctx->object, md, "Node group must have an output socket");
     geometry_set.clear();
     return;
   }
 
   const bNodeSocket *first_output_socket = group_outputs[0];
   if (!STREQ(first_output_socket->idname, "NodeSocketGeometry")) {
     BKE_modifier_set_error(ctx->object, md, "Node group's first output must be a geometry");
     geometry_set.clear();
     return;
   }
 
   const blender::nodes::GeometryNodesLazyFunctionGraphInfo *lf_graph_info =
       blender::nodes::ensure_geometry_nodes_lazy_function_graph(tree);
   if (lf_graph_info == nullptr) {
     BKE_modifier_set_error(ctx->object, md, "Cannot evaluate node group");
     geometry_set.clear();
     return;
   }
 
   bool use_orig_index_verts = false;
   bool use_orig_index_edges = false;
   bool use_orig_index_polys = false;
   if (geometry_set.has_mesh()) {
     const Mesh &mesh = *geometry_set.get_mesh_for_read();
     use_orig_index_verts = CustomData_has_layer(&mesh.vdata, CD_ORIGINDEX);
     use_orig_index_edges = CustomData_has_layer(&mesh.edata, CD_ORIGINDEX);
     use_orig_index_polys = CustomData_has_layer(&mesh.pdata, CD_ORIGINDEX);
   }
 
   geometry_set = compute_geometry(
-      tree, *lf_graph_info, *output_node, std::move(geometry_set), nmd, ctx);
+      tree, *lf_graph_info, *output_node, std::move(geometry_set), nmd, md, ctx);
 
   if (geometry_set.has_mesh()) {
     /* Add #CD_ORIGINDEX layers if they don't exist already. This is required because the
      * #eModifierTypeFlag_SupportsMapping flag is set. If the layers did not exist before, it is
      * assumed that the output mesh does not have a mapping to the original mesh. */
     Mesh &mesh = *geometry_set.get_mesh_for_write();
     if (use_orig_index_verts) {
       CustomData_add_layer(&mesh.vdata, CD_ORIGINDEX, CD_SET_DEFAULT, nullptr, mesh.totvert);
     }
     if (use_orig_index_edges) {
       CustomData_add_layer(&mesh.edata, CD_ORIGINDEX, CD_SET_DEFAULT, nullptr, mesh.totedge);
     }
     if (use_orig_index_polys) {
       CustomData_add_layer(&mesh.pdata, CD_ORIGINDEX, CD_SET_DEFAULT, nullptr, mesh.totpoly);
     }
   }
 }
 
 static Mesh *modifyMesh(ModifierData *md, const ModifierEvalContext *ctx, Mesh *mesh)
 {
   GeometrySet geometry_set = GeometrySet::create_with_mesh(mesh, GeometryOwnershipType::Editable);
 
   modifyGeometry(md, ctx, geometry_set);
 
   Mesh *new_mesh = geometry_set.get_component_for_write<MeshComponent>().release();
   if (new_mesh == nullptr) {
     return BKE_mesh_new_nomain(0, 0, 0, 0, 0);
   }
   return new_mesh;
 }
 
 static void modifyGeometrySet(ModifierData *md,
                               const ModifierEvalContext *ctx,
                               GeometrySet *geometry_set)
 {
   modifyGeometry(md, ctx, *geometry_set);
 }
 
 struct AttributeSearchData {
   uint32_t object_session_uid;
   char modifier_name[MAX_NAME];
   char socket_identifier[MAX_NAME];
   bool is_output;
 };
 /* This class must not have a destructor, since it is used by buttons and freed with #MEM_freeN. */
 BLI_STATIC_ASSERT(std::is_trivially_destructible_v<AttributeSearchData>, "");
 
 static NodesModifierData *get_modifier_data(Main &bmain,
                                             const wmWindowManager &wm,
                                             const AttributeSearchData &data)

I think find problem The search for preview nodes does not take into account that the tree may be cyclic. Because of this, there may be a reading from the 0. In general, it should be possible to learn about the tree problem and log it as a preview node problem. [P3360: (An Untitled Masterwork)](https://archive.blender.org/developer/P3360.txt) ```diff diff --git a/source/blender/modifiers/intern/MOD_nodes.cc b/source/blender/modifiers/intern/MOD_nodes.cc index c2b14262b4f..791c93425ec 100644 --- a/source/blender/modifiers/intern/MOD_nodes.cc +++ b/source/blender/modifiers/intern/MOD_nodes.cc @@ -780,203 +780,221 @@ static void initialize_group_input(NodesModifierData &nmd, interface_socket.type); if (nmd.settings.properties == nullptr) { socket_type.get_geometry_nodes_cpp_value(interface_socket, r_value); return; } const IDProperty *property = IDP_GetPropertyFromGroup(nmd.settings.properties, interface_socket.identifier); if (property == nullptr) { socket_type.get_geometry_nodes_cpp_value(interface_socket, r_value); return; } if (!id_property_type_matches_socket(interface_socket, *property)) { socket_type.get_geometry_nodes_cpp_value(interface_socket, r_value); return; } if (!input_has_attribute_toggle(*nmd.node_group, input_index)) { init_socket_cpp_value_from_property(*property, socket_data_type, r_value); return; } const IDProperty *property_use_attribute = IDP_GetPropertyFromGroup( nmd.settings.properties, (interface_socket.identifier + use_attribute_suffix).c_str()); const IDProperty *property_attribute_name = IDP_GetPropertyFromGroup( nmd.settings.properties, (interface_socket.identifier + attribute_name_suffix).c_str()); if (property_use_attribute == nullptr || property_attribute_name == nullptr) { init_socket_cpp_value_from_property(*property, socket_data_type, r_value); return; } const bool use_attribute = IDP_Int(property_use_attribute) != 0; if (use_attribute) { const StringRef attribute_name{IDP_String(property_attribute_name)}; if (!blender::bke::allow_procedural_attribute_access(attribute_name)) { init_socket_cpp_value_from_property(*property, socket_data_type, r_value); return; } auto attribute_input = std::make_shared<blender::bke::AttributeFieldInput>( attribute_name, *socket_type.base_cpp_type); GField attribute_field{std::move(attribute_input), 0}; const auto *value_or_field_cpp_type = ValueOrFieldCPPType::get_from_self( *socket_type.geometry_nodes_cpp_type); BLI_assert(value_or_field_cpp_type != nullptr); value_or_field_cpp_type->construct_from_field(r_value, std::move(attribute_field)); } else { init_socket_cpp_value_from_property(*property, socket_data_type, r_value); } } -static const lf::FunctionNode &find_viewer_lf_node(const bNode &viewer_bnode) +static const lf::FunctionNode *find_viewer_lf_node(const bNode &viewer_bnode) { - return *blender::nodes::ensure_geometry_nodes_lazy_function_graph(viewer_bnode.owner_tree()) - ->mapping.viewer_node_map.lookup(&viewer_bnode); + const blender::nodes::GeometryNodesLazyFunctionGraphInfo *viewer_graph = blender::nodes::ensure_geometry_nodes_lazy_function_graph(viewer_bnode.owner_tree()); + /* Maybe nullptr if node tree has cycle links. */ + if (viewer_graph){ + return viewer_graph->mapping.viewer_node_map.lookup(&viewer_bnode); + } + return nullptr; } -static const lf::FunctionNode &find_group_lf_node(const bNode &group_bnode) +static const lf::FunctionNode *find_group_lf_node(const bNode &group_bnode) { - return *blender::nodes::ensure_geometry_nodes_lazy_function_graph(group_bnode.owner_tree()) - ->mapping.group_node_map.lookup(&group_bnode); + const blender::nodes::GeometryNodesLazyFunctionGraphInfo *group_graph = blender::nodes::ensure_geometry_nodes_lazy_function_graph(group_bnode.owner_tree()); + + if (group_graph){ + return group_graph->mapping.group_node_map.lookup(&group_bnode); + } + return nullptr; } static void find_side_effect_nodes_for_viewer_path( const ViewerPath &viewer_path, + ModifierData &md, const NodesModifierData &nmd, const ModifierEvalContext &ctx, MultiValueMap<blender::ComputeContextHash, const lf::FunctionNode *> &r_side_effect_nodes) { const std::optional<blender::ed::viewer_path::ViewerPathForGeometryNodesViewer> parsed_path = blender::ed::viewer_path::parse_geometry_nodes_viewer(viewer_path); if (!parsed_path.has_value()) { return; } if (parsed_path->object != DEG_get_original_object(ctx.object)) { return; } if (parsed_path->modifier_name != nmd.modifier.name) { return; } blender::ComputeContextBuilder compute_context_builder; compute_context_builder.push<blender::bke::ModifierComputeContext>(parsed_path->modifier_name); const bNodeTree *group = nmd.node_group; Stack<const bNode *> group_node_stack; for (const StringRefNull group_node_name : parsed_path->group_node_names) { const bNode *found_node = nullptr; for (const bNode *node : group->group_nodes()) { if (node->name == group_node_name) { found_node = node; break; } } if (found_node == nullptr) { return; } if (found_node->id == nullptr) { return; } if (found_node->is_muted()) { return; } group_node_stack.push(found_node); group = reinterpret_cast<bNodeTree *>(found_node->id); compute_context_builder.push<blender::bke::NodeGroupComputeContext>(group_node_name); } const bNode *found_viewer_node = nullptr; for (const bNode *viewer_node : group->nodes_by_type("GeometryNodeViewer")) { if (viewer_node->name == parsed_path->viewer_node_name) { found_viewer_node = viewer_node; break; } } if (found_viewer_node == nullptr) { return; } /* Not only mark the viewer node as having side effects, but also all group nodes it is contained * in. */ - r_side_effect_nodes.add(compute_context_builder.hash(), - &find_viewer_lf_node(*found_viewer_node)); + const lf::FunctionNode *viewer_lf = find_viewer_lf_node(*found_viewer_node); + if (!viewer_lf){ + BKE_modifier_set_error(ctx.object, &md, "Can not use viewer node"); + return; + } + r_side_effect_nodes.add(compute_context_builder.hash(), viewer_lf); compute_context_builder.pop(); while (!compute_context_builder.is_empty()) { - r_side_effect_nodes.add(compute_context_builder.hash(), - &find_group_lf_node(*group_node_stack.pop())); + const lf::FunctionNode *group_lf = find_group_lf_node(*group_node_stack.pop()); + if (group_lf){ + r_side_effect_nodes.add(compute_context_builder.hash(), group_lf); + }else{ + BKE_modifier_set_error(ctx.object, &md, "Can not use viewer node"); + } compute_context_builder.pop(); } } static void find_side_effect_nodes( const NodesModifierData &nmd, + ModifierData &md, const ModifierEvalContext &ctx, MultiValueMap<blender::ComputeContextHash, const lf::FunctionNode *> &r_side_effect_nodes) { 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); const WorkSpace *workspace = BKE_workspace_active_get(window->workspace_hook); - find_side_effect_nodes_for_viewer_path(workspace->viewer_path, nmd, ctx, r_side_effect_nodes); + find_side_effect_nodes_for_viewer_path(workspace->viewer_path, md, nmd, ctx, r_side_effect_nodes); LISTBASE_FOREACH (const ScrArea *, area, &screen->areabase) { const SpaceLink *sl = static_cast<SpaceLink *>(area->spacedata.first); if (sl->spacetype == SPACE_SPREADSHEET) { const SpaceSpreadsheet &sspreadsheet = *reinterpret_cast<const SpaceSpreadsheet *>(sl); find_side_effect_nodes_for_viewer_path( - sspreadsheet.viewer_path, nmd, ctx, r_side_effect_nodes); + sspreadsheet.viewer_path, md, nmd, ctx, r_side_effect_nodes); } if (sl->spacetype == SPACE_VIEW3D) { const View3D &v3d = *reinterpret_cast<const View3D *>(sl); - find_side_effect_nodes_for_viewer_path(v3d.viewer_path, nmd, ctx, r_side_effect_nodes); + find_side_effect_nodes_for_viewer_path(v3d.viewer_path, md, nmd, ctx, r_side_effect_nodes); } } } } static void clear_runtime_data(NodesModifierData *nmd) { if (nmd->runtime_eval_log != nullptr) { delete static_cast<GeoModifierLog *>(nmd->runtime_eval_log); nmd->runtime_eval_log = nullptr; } } struct OutputAttributeInfo { GField field; StringRefNull name; }; struct OutputAttributeToStore { GeometryComponentType component_type; eAttrDomain domain; StringRefNull name; GMutableSpan data; }; /** * The output attributes are organized based on their domain, because attributes on the same domain * can be evaluated together. */ static MultiValueMap<eAttrDomain, OutputAttributeInfo> find_output_attributes_to_store( const NodesModifierData &nmd, const bNode &output_node, Span<GMutablePointer> output_values) { MultiValueMap<eAttrDomain, OutputAttributeInfo> outputs_by_domain; for (const bNodeSocket *socket : output_node.input_sockets().drop_front(1).drop_back(1)) { if (!socket_type_has_attribute_toggle(*socket)) { continue; } const std::string prop_name = socket->identifier + attribute_name_suffix; const IDProperty *prop = IDP_GetPropertyFromGroup(nmd.settings.properties, prop_name.c_str()); if (prop == nullptr) { continue; } const StringRefNull attribute_name = IDP_String(prop); if (attribute_name.is_empty()) { continue; } if (!blender::bke::allow_procedural_attribute_access(attribute_name)) { continue; } @@ -1060,132 +1078,133 @@ static void store_computed_output_attributes( if (meta_data.has_value() && (meta_data->domain != store.domain || meta_data->data_type != data_type)) { attributes.remove(store.name); } /* Try to create the attribute reusing the stored buffer. This will only succeed if the * attribute didn't exist before, or if it existed but was removed above. */ if (attributes.add(store.name, store.domain, blender::bke::cpp_type_to_custom_data_type(store.data.type()), blender::bke::AttributeInitMoveArray(store.data.data()))) { continue; } blender::bke::GAttributeWriter attribute = attributes.lookup_or_add_for_write( store.name, store.domain, data_type); if (attribute) { attribute.varray.set_all(store.data.data()); attribute.finish(); } /* We were unable to reuse the data, so it must be destructed and freed. */ store.data.type().destruct_n(store.data.data(), store.data.size()); MEM_freeN(store.data.data()); } } static void store_output_attributes(GeometrySet &geometry, const NodesModifierData &nmd, const bNode &output_node, Span<GMutablePointer> output_values) { /* All new attribute values have to be computed before the geometry is actually changed. This is * necessary because some fields might depend on attributes that are overwritten. */ MultiValueMap<eAttrDomain, OutputAttributeInfo> outputs_by_domain = find_output_attributes_to_store(nmd, output_node, output_values); Vector<OutputAttributeToStore> attributes_to_store = compute_attributes_to_store( geometry, outputs_by_domain); store_computed_output_attributes(geometry, attributes_to_store); } /** * Evaluate a node group to compute the output geometry. */ static GeometrySet compute_geometry( const bNodeTree &btree, const blender::nodes::GeometryNodesLazyFunctionGraphInfo &lf_graph_info, const bNode &output_node, GeometrySet input_geometry_set, NodesModifierData *nmd, + ModifierData *md, const ModifierEvalContext *ctx) { const blender::nodes::GeometryNodeLazyFunctionGraphMapping &mapping = lf_graph_info.mapping; Span<const lf::OutputSocket *> graph_inputs = mapping.group_input_sockets; Vector<const lf::InputSocket *> graph_outputs; for (const bNodeSocket *bsocket : output_node.input_sockets().drop_back(1)) { const lf::InputSocket &socket = mapping.dummy_socket_map.lookup(bsocket)->as_input(); graph_outputs.append(&socket); } Array<GMutablePointer> param_inputs(graph_inputs.size()); Array<GMutablePointer> param_outputs(graph_outputs.size()); Array<std::optional<lf::ValueUsage>> param_input_usages(graph_inputs.size()); Array<lf::ValueUsage> param_output_usages(graph_outputs.size(), lf::ValueUsage::Used); Array<bool> param_set_outputs(graph_outputs.size(), false); blender::nodes::GeometryNodesLazyFunctionLogger lf_logger(lf_graph_info); blender::nodes::GeometryNodesLazyFunctionSideEffectProvider lf_side_effect_provider; lf::GraphExecutor graph_executor{ lf_graph_info.graph, graph_inputs, graph_outputs, &lf_logger, &lf_side_effect_provider}; blender::nodes::GeoNodesModifierData geo_nodes_modifier_data; geo_nodes_modifier_data.depsgraph = ctx->depsgraph; geo_nodes_modifier_data.self_object = ctx->object; auto eval_log = std::make_unique<GeoModifierLog>(); if (logging_enabled(ctx)) { geo_nodes_modifier_data.eval_log = eval_log.get(); } MultiValueMap<blender::ComputeContextHash, const lf::FunctionNode *> r_side_effect_nodes; - find_side_effect_nodes(*nmd, *ctx, r_side_effect_nodes); + find_side_effect_nodes(*nmd, *md, *ctx, r_side_effect_nodes); geo_nodes_modifier_data.side_effect_nodes = &r_side_effect_nodes; blender::nodes::GeoNodesLFUserData user_data; user_data.modifier_data = &geo_nodes_modifier_data; blender::bke::ModifierComputeContext modifier_compute_context{nullptr, nmd->modifier.name}; user_data.compute_context = &modifier_compute_context; blender::LinearAllocator<> allocator; Vector<GMutablePointer> inputs_to_destruct; int input_index; LISTBASE_FOREACH_INDEX (bNodeSocket *, interface_socket, &btree.inputs, input_index) { if (interface_socket->type == SOCK_GEOMETRY && input_index == 0) { param_inputs[input_index] = &input_geometry_set; continue; } const CPPType *type = interface_socket->typeinfo->geometry_nodes_cpp_type; BLI_assert(type != nullptr); void *value = allocator.allocate(type->size(), type->alignment()); initialize_group_input(*nmd, *interface_socket, input_index, value); param_inputs[input_index] = {type, value}; inputs_to_destruct.append({type, value}); } for (const int i : graph_outputs.index_range()) { const lf::InputSocket &socket = *graph_outputs[i]; const CPPType &type = socket.type(); void *buffer = allocator.allocate(type.size(), type.alignment()); param_outputs- [x] = {type, buffer}; } lf::Context lf_context; lf_context.storage = graph_executor.init_storage(allocator); lf_context.user_data = &user_data; lf::BasicParams lf_params{graph_executor, param_inputs, param_outputs, param_input_usages, param_output_usages, param_set_outputs}; graph_executor.execute(lf_params, lf_context); graph_executor.destruct_storage(lf_context.storage); for (GMutablePointer &ptr : inputs_to_destruct) { ptr.destruct(); } GeometrySet output_geometry_set = std::move(*static_cast<GeometrySet *>(param_outputs- [x].get())); store_output_attributes(output_geometry_set, *nmd, output_node, param_outputs); @@ -1255,101 +1274,101 @@ static void modifyGeometry(ModifierData *md, { NodesModifierData *nmd = reinterpret_cast<NodesModifierData *>(md); if (nmd->node_group == nullptr) { return; } const bNodeTree &tree = *nmd->node_group; tree.ensure_topology_cache(); check_property_socket_sync(ctx->object, md); const bNode *output_node = tree.group_output_node(); if (output_node == nullptr) { BKE_modifier_set_error(ctx->object, md, "Node group must have a group output node"); geometry_set.clear(); return; } Span<const bNodeSocket *> group_outputs = output_node->input_sockets().drop_back(1); if (group_outputs.is_empty()) { BKE_modifier_set_error(ctx->object, md, "Node group must have an output socket"); geometry_set.clear(); return; } const bNodeSocket *first_output_socket = group_outputs[0]; if (!STREQ(first_output_socket->idname, "NodeSocketGeometry")) { BKE_modifier_set_error(ctx->object, md, "Node group's first output must be a geometry"); geometry_set.clear(); return; } const blender::nodes::GeometryNodesLazyFunctionGraphInfo *lf_graph_info = blender::nodes::ensure_geometry_nodes_lazy_function_graph(tree); if (lf_graph_info == nullptr) { BKE_modifier_set_error(ctx->object, md, "Cannot evaluate node group"); geometry_set.clear(); return; } bool use_orig_index_verts = false; bool use_orig_index_edges = false; bool use_orig_index_polys = false; if (geometry_set.has_mesh()) { const Mesh &mesh = *geometry_set.get_mesh_for_read(); use_orig_index_verts = CustomData_has_layer(&mesh.vdata, CD_ORIGINDEX); use_orig_index_edges = CustomData_has_layer(&mesh.edata, CD_ORIGINDEX); use_orig_index_polys = CustomData_has_layer(&mesh.pdata, CD_ORIGINDEX); } geometry_set = compute_geometry( - tree, *lf_graph_info, *output_node, std::move(geometry_set), nmd, ctx); + tree, *lf_graph_info, *output_node, std::move(geometry_set), nmd, md, ctx); if (geometry_set.has_mesh()) { /* Add #CD_ORIGINDEX layers if they don't exist already. This is required because the * #eModifierTypeFlag_SupportsMapping flag is set. If the layers did not exist before, it is * assumed that the output mesh does not have a mapping to the original mesh. */ Mesh &mesh = *geometry_set.get_mesh_for_write(); if (use_orig_index_verts) { CustomData_add_layer(&mesh.vdata, CD_ORIGINDEX, CD_SET_DEFAULT, nullptr, mesh.totvert); } if (use_orig_index_edges) { CustomData_add_layer(&mesh.edata, CD_ORIGINDEX, CD_SET_DEFAULT, nullptr, mesh.totedge); } if (use_orig_index_polys) { CustomData_add_layer(&mesh.pdata, CD_ORIGINDEX, CD_SET_DEFAULT, nullptr, mesh.totpoly); } } } static Mesh *modifyMesh(ModifierData *md, const ModifierEvalContext *ctx, Mesh *mesh) { GeometrySet geometry_set = GeometrySet::create_with_mesh(mesh, GeometryOwnershipType::Editable); modifyGeometry(md, ctx, geometry_set); Mesh *new_mesh = geometry_set.get_component_for_write<MeshComponent>().release(); if (new_mesh == nullptr) { return BKE_mesh_new_nomain(0, 0, 0, 0, 0); } return new_mesh; } static void modifyGeometrySet(ModifierData *md, const ModifierEvalContext *ctx, GeometrySet *geometry_set) { modifyGeometry(md, ctx, *geometry_set); } struct AttributeSearchData { uint32_t object_session_uid; char modifier_name[MAX_NAME]; char socket_identifier[MAX_NAME]; bool is_output; }; /* This class must not have a destructor, since it is used by buttons and freed with #MEM_freeN. */ BLI_STATIC_ASSERT(std::is_trivially_destructible_v<AttributeSearchData>, ""); static NodesModifierData *get_modifier_data(Main &bmain, const wmWindowManager &wm, const AttributeSearchData &data) ```

Iliya Katushenock commented

2022-11-30 19:46:02 +01:00

Added subscriber: @JacquesLucke

Iliya Katushenock changed title from ~~Geometry nodes: Crash on recursive reference to a field~~ to Geometry nodes: Crash for cycle links in node group when try to find viewer node

2022-11-30 19:56:01 +01:00

Iliya Katushenock commented

2022-11-30 19:56:20 +01:00

@ChristopheMermoud Thank you for reporting this!

Jacques Lucke was assigned by Iliya Katushenock

2023-01-31 19:09:02 +01:00

Philipp Oeser removed the

Download

What's New

Blender Studio

Manual

Developers Blog

Documentation

Benchmark

Blender Conference

Development Fund

One-time Donations

Geometry nodes: Crash for cycle links in node group when try to find viewer node #102882