[Bf-blender-cvs] [f35e15647b7] soc-2021-porting-modifiers-to-nodes-merge-by-distance: Changes based on Review by Hans Goudey (HooglyBoogly)

Fabian Schempp noreply at git.blender.org
Sun Oct 10 22:15:57 CEST 2021


Commit: f35e15647b7f0ebd38e69d58fddf5e42ff678450
Author: Fabian Schempp
Date:   Sun Oct 10 22:15:52 2021 +0200
Branches: soc-2021-porting-modifiers-to-nodes-merge-by-distance
https://developer.blender.org/rBf35e15647b7f0ebd38e69d58fddf5e42ff678450

Changes based on Review by Hans Goudey (HooglyBoogly)

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

M	source/blender/blenlib/BLI_multi_value_map.hh
M	source/blender/geometry/GEO_mesh_merge_by_distance.hh
M	source/blender/geometry/intern/mesh_merge_by_distance.cc
M	source/blender/geometry/intern/pointcloud_merge_by_distance.cc
M	source/blender/makesrna/intern/rna_nodetree.c
M	source/blender/modifiers/CMakeLists.txt
M	source/blender/modifiers/intern/MOD_weld.cc
M	source/blender/nodes/geometry/nodes/node_geo_merge_by_distance.cc

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

diff --git a/source/blender/blenlib/BLI_multi_value_map.hh b/source/blender/blenlib/BLI_multi_value_map.hh
index d3073c98417..3f1f8333ed3 100644
--- a/source/blender/blenlib/BLI_multi_value_map.hh
+++ b/source/blender/blenlib/BLI_multi_value_map.hh
@@ -144,6 +144,14 @@ template<typename Key, typename Value> class MultiValueMap {
     return map_.keys();
   }
 
+  /**
+   * Return how many keys are currently stored in the Map.
+   */
+  int64_t size() const
+  {
+    return std::distance(this->keys().begin(), this->keys().end());
+  }
+
   /**
    * NOTE: This signature will change when the implementation changes.
    */
diff --git a/source/blender/geometry/GEO_mesh_merge_by_distance.hh b/source/blender/geometry/GEO_mesh_merge_by_distance.hh
index 3bf32b01e2e..01a40d88e33 100644
--- a/source/blender/geometry/GEO_mesh_merge_by_distance.hh
+++ b/source/blender/geometry/GEO_mesh_merge_by_distance.hh
@@ -24,15 +24,11 @@
 
 namespace blender::geometry {
 
-/* Weld mode is only used when welding mesh data. */
 enum class WeldMode {
   all = 0,
   connected = 1,
 };
 
-WeldMode weld_mode_from_int(const int16_t type);
-int16_t weld_mode_to_int(const WeldMode weld_mode);
-
 struct Mesh *mesh_merge_by_distance(struct Mesh *mesh,
                                     const Span<bool> mask,
                                     const float merge_distance,
diff --git a/source/blender/geometry/intern/mesh_merge_by_distance.cc b/source/blender/geometry/intern/mesh_merge_by_distance.cc
index 9eb2fe1afd2..5b98bbd2b87 100644
--- a/source/blender/geometry/intern/mesh_merge_by_distance.cc
+++ b/source/blender/geometry/intern/mesh_merge_by_distance.cc
@@ -1542,31 +1542,6 @@ struct WeldVertexCluster {
 
 namespace blender::geometry {
 
-WeldMode weld_mode_from_int(const short type)
-{
-  switch (static_cast<WeldMode>(type)) {
-    case WeldMode::all:
-      return WeldMode::all;
-    case WeldMode::connected:
-      return WeldMode::connected;
-  }
-  BLI_assert_unreachable();
-  return WeldMode::all;
-}
-
-int16_t weld_mode_to_int(const WeldMode weld_mode)
-{
-  switch (weld_mode) {
-    case WeldMode::all:
-      return static_cast<int16_t>(WeldMode::all);
-    case WeldMode::connected:
-      return static_cast<int16_t>(WeldMode::connected);
-  }
-
-  BLI_assert_unreachable();
-  return static_cast<int16_t>(WeldMode::all);
-}
-
 Mesh *mesh_merge_by_distance(Mesh *mesh,
                              const Span<bool> mask,
                              const float merge_distance,
diff --git a/source/blender/geometry/intern/pointcloud_merge_by_distance.cc b/source/blender/geometry/intern/pointcloud_merge_by_distance.cc
index 6e5e18980fb..b7540f250e1 100644
--- a/source/blender/geometry/intern/pointcloud_merge_by_distance.cc
+++ b/source/blender/geometry/intern/pointcloud_merge_by_distance.cc
@@ -34,6 +34,7 @@
 using blender::MutableSpan;
 
 namespace blender::geometry {
+const static int POINT_NOT_MERGED = -1;
 
 static KDTree_3d *build_kdtree(Span<float3> positions, Span<bool> selection)
 {
@@ -76,8 +77,8 @@ static void build_merge_map(Span<float3> positions,
           CallbackData &callback_data = *static_cast<CallbackData *>(user_data);
           int target_point_index = callback_data.index;
           if (source_point_index != target_point_index &&
-              callback_data.merge_map[source_point_index] == -1 &&
-              callback_data.merge_map[target_point_index] == -1 &&
+              callback_data.merge_map[source_point_index] == POINT_NOT_MERGED &&
+              callback_data.merge_map[target_point_index] == POINT_NOT_MERGED &&
               callback_data.selection[source_point_index] &&
               callback_data.selection[target_point_index]) {
             callback_data.merge_map[source_point_index] = target_point_index;
@@ -95,80 +96,58 @@ PointCloud *pointcloud_merge_by_distance(PointCloudComponent &pointcloud_compone
                                          Span<bool> selection)
 {
   const PointCloud &src_pointcloud = *pointcloud_component.get_for_read();
-  Array<int> merge_map(src_pointcloud.totpoint, -1);
+  Array<int> merge_map(src_pointcloud.totpoint, POINT_NOT_MERGED);
   Span<float3> positions((const float3 *)src_pointcloud.co, src_pointcloud.totpoint);
 
   build_merge_map(positions, merge_map, merge_threshold, selection);
 
   MultiValueMap<int, int> copy_map;
   for (const int i : merge_map.index_range()) {
-    if (merge_map[i] != -1) {
+    if (merge_map[i] != POINT_NOT_MERGED) {
       copy_map.add(merge_map[i], i);
     }
   }
 
-  int copy_count = std::distance(copy_map.keys().begin(), copy_map.keys().end());
-
-  PointCloud *pointcloud = BKE_pointcloud_new_nomain(copy_count);
+  PointCloud *pointcloud = BKE_pointcloud_new_nomain(copy_map.size());
   PointCloudComponent dst_component;
   dst_component.replace(pointcloud, GeometryOwnershipType::Editable);
 
-  int index_new = 0;
-  for (const int index_old : copy_map.keys()) {
-    Span<int> merged_points = copy_map.lookup(index_old);
-    if (merged_points.size() > 0) {
-      copy_v3_v3(pointcloud->co[index_new], src_pointcloud.co[index_old]);
-      pointcloud->radius[index_new] = src_pointcloud.radius[index_old];
-      float weight = 1 / (float(merged_points.size() + 1));
-      for (const int j : merged_points) {
-        add_v3_v3(pointcloud->co[index_new], src_pointcloud.co[j]);
-        pointcloud->radius[index_new] += src_pointcloud.radius[j];
-      }
-      mul_v3_fl(pointcloud->co[index_new], weight);
-      pointcloud->radius[index_new] *= weight;
-    }
-    index_new++;
-  }
-
   pointcloud_component.attribute_foreach(
       [&](const bke::AttributeIDRef &attribute_id, const AttributeMetaData &meta_data) {
         fn::GVArrayPtr read_attribute = pointcloud_component.attribute_get_for_read(
             attribute_id, meta_data.domain, meta_data.data_type);
 
-        if (dst_component.attribute_try_create(
+        if (!dst_component.attribute_exists(attribute_id) &&
+            !dst_component.attribute_try_create(
                 attribute_id, meta_data.domain, meta_data.data_type, AttributeInitDefault())) {
+          return true;
+        }
 
-          bke::OutputAttribute target_attribute = dst_component.attribute_try_get_for_output_only(
-              attribute_id, meta_data.domain, meta_data.data_type);
+        bke::OutputAttribute target_attribute = dst_component.attribute_try_get_for_output_only(
+            attribute_id, meta_data.domain, meta_data.data_type);
 
-          if (target_attribute->is_empty()) {
-            target_attribute.save();
-            return true;
-          }
+        blender::attribute_math::convert_to_static_type(meta_data.data_type, [&](auto dummy) {
+          using T = decltype(dummy);
+          const fn::GVArray_Typed<T> src_span = read_attribute->typed<T>();
+
+          attribute_math::DefaultMixer<T> mixer(target_attribute.as_span<T>());
 
-          blender::attribute_math::convert_to_static_type(meta_data.data_type, [&](auto dummy) {
-            using T = decltype(dummy);
-            const fn::GVArray_Typed<T> src_span = read_attribute->typed<T>();
-
-            attribute_math::DefaultMixer<T> mixer(target_attribute.as_span<T>());
-
-            index_new = 0;
-            for (const int index_old : copy_map.keys()) {
-              Span<int> merged_points = copy_map.lookup(index_old);
-              if (merged_points.size() > 0) {
-                float weight = 1 / (float(merged_points.size() + 1));
-                mixer.mix_in(index_new, src_span[index_old], weight);
-                for (const int j : merged_points) {
-                  mixer.mix_in(index_new, src_span[j], weight);
-                }
+          int index_new = 0;
+          for (const int index_old : copy_map.keys()) {
+            Span<int> merged_points = copy_map.lookup(index_old);
+            if (merged_points.size() > 0) {
+              float weight = 1.0f / (float(merged_points.size() + 1.0f));
+              mixer.mix_in(index_new, src_span[index_old], weight);
+              for (const int j : merged_points) {
+                mixer.mix_in(index_new, src_span[j], weight);
               }
-              index_new++;
             }
-            mixer.finalize();
-          });
+            index_new++;
+          }
+          mixer.finalize();
+        });
 
-          target_attribute.save();
-        }
+        target_attribute.save();
         return true;
       });
 
diff --git a/source/blender/makesrna/intern/rna_nodetree.c b/source/blender/makesrna/intern/rna_nodetree.c
index 684ff182988..6556eaa8b50 100644
--- a/source/blender/makesrna/intern/rna_nodetree.c
+++ b/source/blender/makesrna/intern/rna_nodetree.c
@@ -10754,21 +10754,21 @@ static void def_geo_string_to_curves(StructRNA *srna)
   RNA_def_property_update(prop, NC_NODE | NA_EDITED, "rna_Node_update");
 }
 
-/* Weld mode is only used when welding mesh data. */
-const EnumPropertyItem rna_enum_geometry_nodes_weld_mode_items[] = {
-    {0, "ALL", 0, "All", "Full merge by distance"},
-    {1, "CONNECTED", 0, "Connected", "Only merge along the edges"},
-    {0, NULL, 0, NULL, NULL},
-};
-
 static void def_geo_merge_by_distance(StructRNA *srna)
 {
+  static const EnumPropertyItem rna_enum_geometry_nodes_weld_mode_items[] = {
+      {0, "ALL", 0, "All", "Full merge by distance."},
+      {1, "CONNECTED", 0, "Connected", "Only merge along the edges"},
+      {0, NULL, 0, NULL, NULL},
+  };
+
   PropertyRNA *prop;
 
   prop = RNA_def_property(srna, "merge_mode", PROP_ENUM, PROP_NONE);
   RNA_def_property_enum_sdna(prop, NULL, "custom1");
   RNA_def_property_enum_items(prop, rna_enum_geometry_nodes_weld_mode_items);
-  RNA_def_property_ui_text(prop, "Mode", "Mode defines the merge rule");
+  RNA_def_property_ui_text(
+      prop, "Mode", "Mode defines the merge rule. Only used when welding mesh data.");
   RNA_def_property_update(prop, NC_NODE | NA_EDITED, "rna_Node_update");
 }
 /* -------------------------------------------------------------------------- */
diff --git a/source/blender/modifiers/CMakeLists.txt b/source/blender/modifiers/CMakeLists.txt
index 0a1f3918aaa..75045263b1d 100644
--- a/source/blender/modifiers/CMakeLists.txt
+++ b/source/blender/modifiers/C

@@ Diff output truncated at 10240 characters. @@



More information about the Bf-blender-cvs mailing list