[Bf-blender-cvs] [267ba329ece] geometry-nodes-point-separate-node: Point separate node: Refactor, add comments, clean up

[Hans Goudey]

Commit: 267ba329ece56b9bd30abb4866ff75e437cb7f83
Author: Hans Goudey
Date:   Tue Dec 1 22:33:47 2020 -0500
Branches: geometry-nodes-point-separate-node
https://developer.blender.org/rB267ba329ece56b9bd30abb4866ff75e437cb7f83

Point separate node: Refactor, add comments, clean up

Make more of the logic generic so that it can more easily work on more
component types in the future. Basically anything that handles attributes
can be generic.

===================================================================

M	source/blender/nodes/geometry/nodes/node_geo_point_separate.cc

===================================================================

diff --git a/source/blender/nodes/geometry/nodes/node_geo_point_separate.cc b/source/blender/nodes/geometry/nodes/node_geo_point_separate.cc
index 81a193723b2..1fa2c87c95e 100644
--- a/source/blender/nodes/geometry/nodes/node_geo_point_separate.cc
+++ b/source/blender/nodes/geometry/nodes/node_geo_point_separate.cc
@@ -39,10 +39,10 @@ static bNodeSocketTemplate geo_node_point_instance_out[] = {
 
 namespace blender::nodes {
 
-static void fill_attribute_from_input(ReadAttributePtr input_attribute,
-                                      WriteAttributePtr out_attribute_a,
-                                      WriteAttributePtr out_attribute_b,
-                                      Span<bool> a_or_b)
+static void fill_new_attribute_from_input(ReadAttributePtr input_attribute,
+                                          WriteAttributePtr out_attribute_a,
+                                          WriteAttributePtr out_attribute_b,
+                                          Span<bool> a_or_b)
 {
   fn::GSpan in_span = input_attribute->get_span();
   int i_a = 0;
@@ -59,36 +59,44 @@ static void fill_attribute_from_input(ReadAttributePtr input_attribute,
   }
 }
 
-static void separate_component_attributes(const PointCloudComponent &component,
-                                          GeometrySet *out_set_a,
-                                          GeometrySet *out_set_b,
-                                          const int a_total,
-                                          const int b_total,
+/**
+ * Move the original attribute values to the two output components.
+ *
+ * \note This assumes a consistent ordering of indices before and after the split.
+ * This is trivial for simple components like point clouds, but for meshes a map between
+ * input and output indices might be necessary.
+ */
+static void separate_component_attributes(const GeometryComponent &in_component,
+                                          GeometryComponent &out_component_a,
+                                          GeometryComponent &out_component_b,
                                           Span<bool> a_or_b)
 {
-  /* Start fresh with new pointclouds. */
-  out_set_a->replace_pointcloud(BKE_pointcloud_new_nomain(a_total));
-  out_set_b->replace_pointcloud(BKE_pointcloud_new_nomain(b_total));
-  PointCloudComponent &out_component_a = out_set_a->get_component_for_write<PointCloudComponent>();
-  PointCloudComponent &out_component_b = out_set_b->get_component_for_write<PointCloudComponent>();
+  Set<std::string> attribute_names = in_component.attribute_names();
 
-  Set<std::string> attribute_names = component.attribute_names();
   for (const std::string &name : attribute_names) {
-    ReadAttributePtr attribute = component.attribute_try_get_for_read(name);
+    ReadAttributePtr attribute = in_component.attribute_try_get_for_read(name);
     BLI_assert(attribute);
 
     const CustomDataType data_type = bke::cpp_type_to_custom_data_type(attribute->cpp_type());
     const AttributeDomain domain = attribute->domain();
-    if (!component.attribute_is_builtin(name)) {
-      const bool create_success_a = out_component_a.attribute_try_create(name, domain, data_type);
-      const bool create_success_b = out_component_b.attribute_try_create(name, domain, data_type);
-      if (!(create_success_a && create_success_b)) {
-        /* Since the input attribute is from the same component type,
-         * it should always be possible to create the new attributes. */
+
+    /* Don't try to create the attribute on the new component if it already exists. Built-in
+     * attributes will already exist on new components by definition. It should always be possible
+     * to recreate the attribute on the same component type. Also, if one of the new components
+     * has the attribute the other one should have it too, but check independently to be safe. */
+    if (!out_component_a.has_attribute(name)) {
+      if (!out_component_a.attribute_try_create(name, domain, data_type)) {
+        BLI_assert(false);
+        continue;
+      }
+    }
+    if (!out_component_b.has_attribute(name)) {
+      if (!out_component_b.attribute_try_create(name, domain, data_type)) {
         BLI_assert(false);
         continue;
       }
     }
+
     WriteAttributePtr out_attribute_a = out_component_a.attribute_try_get_for_write(name);
     WriteAttributePtr out_attribute_b = out_component_b.attribute_try_get_for_write(name);
     if (!out_attribute_a || !out_attribute_b) {
@@ -96,11 +104,22 @@ static void separate_component_attributes(const PointCloudComponent &component,
       continue;
     }
 
-    fill_attribute_from_input(
+    fill_new_attribute_from_input(
         std::move(attribute), std::move(out_attribute_a), std::move(out_attribute_b), a_or_b);
   }
 }
 
+/* TODO: How to override based on component type without adding it as an argument? */
+static void create_new_components(GeometrySet *out_set_a,
+                                  GeometrySet *out_set_b,
+                                  const int a_total,
+                                  const int b_total)
+{
+  /* Start fresh with new pointclouds. */
+  out_set_a->replace_pointcloud(BKE_pointcloud_new_nomain(a_total));
+  out_set_b->replace_pointcloud(BKE_pointcloud_new_nomain(b_total));
+}
+
 /**
  * Find total in each new set and find which of the output sets each point will belong to.
  */
@@ -127,7 +146,7 @@ static Array<bool> calculate_split(const GeometryComponent &component,
   }
   *r_b_total = in_total - *r_a_total;
 
-  return a_or_b;
+  return a_or_b; /* TODO: Can't return a span here? How to do that? */
 }
 
 /* Much of the attribute code can be handled generically for every geometry component type. */
@@ -142,8 +161,12 @@ static void separate_component_type(const Component &component,
   int b_total;
   Array<bool> a_or_b = calculate_split(component, mask_name, threshold, &a_total, &b_total);
 
-  separate_component_attributes(
-      component, out_set_a, out_set_b, a_total, b_total, a_or_b.as_span());
+  /* Start fresh with new components since the size will change anyway. */
+  create_new_components(out_set_a, out_set_b, a_total, b_total);
+
+  GeometryComponent &out_component_a = out_set_a->get_component_for_write<Component>();
+  GeometryComponent &out_component_b = out_set_b->get_component_for_write<Component>();
+  separate_component_attributes(component, out_component_a, out_component_b, a_or_b.as_span());
 }
 
 static void geo_node_point_separate_exec(GeoNodeExecParams params)