Fix T94659: crash when deleting instances

The crash was caused by using `modify_geometry_sets` to modify
instances, which does not generally work unfortunately.
The intended behavior was wrong anyway. In instances mode,
only top level instances should be deleted.

Also removed the old error handling because it doesn't look like it
ever worked. all_is_error remained false all the time.
Furthermore, updating it was not thread safe.

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

1 files changed, 23 insertions, 23 deletions

diff --git a/source/blender/nodes/geometry/nodes/node_geo_separate_geometry.cc b/source/blender/nodes/geometry/nodes/node_geo_separate_geometry.cc
index 844d3c88e36..63da7399c3e 100644
--- a/source/blender/nodes/geometry/nodes/node_geo_separate_geometry.cc
+++ b/source/blender/nodes/geometry/nodes/node_geo_separate_geometry.cc
@@ -59,38 +59,38 @@ static void node_geo_exec(GeoNodeExecParams params)
   const NodeGeometrySeparateGeometry &storage = node_storage(params.node());
   const AttributeDomain domain = static_cast<AttributeDomain>(storage.domain);
 
-  bool all_is_error = false;
-  GeometrySet second_set(geometry_set);
-  if (params.output_is_required("Selection")) {
-    geometry_set.modify_geometry_sets([&](GeometrySet &geometry_set) {
-      bool this_is_error = false;
+  auto separate_geometry_maybe_recursively = [&](bool invert) {
+    bool is_error;
+    if (domain == ATTR_DOMAIN_INSTANCE) {
+      /* Only delete top level instances. */
       separate_geometry(geometry_set,
                         domain,
                         GEO_NODE_DELETE_GEOMETRY_MODE_ALL,
                         selection_field,
-                        false,
-                        this_is_error);
-      all_is_error &= this_is_error;
-    });
+                        invert,
+                        is_error);
+    }
+    else {
+      geometry_set.modify_geometry_sets([&](GeometrySet &geometry_set) {
+        separate_geometry(geometry_set,
+                          domain,
+                          GEO_NODE_DELETE_GEOMETRY_MODE_ALL,
+                          selection_field,
+                          invert,
+                          is_error);
+      });
+    }
+  };
+
+  GeometrySet second_set(geometry_set);
+  if (params.output_is_required("Selection")) {
+    separate_geometry_maybe_recursively(false);
     params.set_output("Selection", std::move(geometry_set));
   }
   if (params.output_is_required("Inverted")) {
-    second_set.modify_geometry_sets([&](GeometrySet &geometry_set) {
-      bool this_is_error = false;
-      separate_geometry(geometry_set,
-                        domain,
-                        GEO_NODE_DELETE_GEOMETRY_MODE_ALL,
-                        selection_field,
-                        true,
-                        this_is_error);
-      all_is_error &= this_is_error;
-    });
+    separate_geometry_maybe_recursively(true);
     params.set_output("Inverted", std::move(second_set));
   }
-  if (all_is_error) {
-    /* Only show this if none of the instances/components actually changed. */
-    params.error_message_add(NodeWarningType::Info, TIP_("No geometry with given domain"));
-  }
 }
 
 }  // namespace blender::nodes::node_geo_separate_geometry_cc