[Bf-blender-cvs] [a803dbe7ed8] master: Geometry Nodes: Use common utility for copying attribute data

Hans Goudey noreply at git.blender.org
Wed Oct 19 19:39:57 CEST 2022


Commit: a803dbe7ed80df577b648f9e289aaed2a2cc1700
Author: Hans Goudey
Date:   Wed Oct 19 12:38:48 2022 -0500
Branches: master
https://developer.blender.org/rBa803dbe7ed80df577b648f9e289aaed2a2cc1700

Geometry Nodes: Use common utility for copying attribute data

Attribute copying often uses identical logic for copying selected
elements or copying with an index map. Instead of reimplementing
this in each file, use the common implementation in the array_utils
namespace. This makes the commonality more obvious, gives improved
performance (this implementation is multithreaded), reduces binary
size (I observed a 173KB reduction), and probably reduces compile time.

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

M	source/blender/blenkernel/intern/curves_geometry.cc
M	source/blender/blenkernel/intern/instances.cc
M	source/blender/blenlib/BLI_array_utils.hh
M	source/blender/blenlib/intern/array_utils.cc
M	source/blender/nodes/geometry/nodes/node_geo_delete_geometry.cc
M	source/blender/nodes/geometry/nodes/node_geo_dual_mesh.cc
M	source/blender/nodes/geometry/nodes/node_geo_duplicate_elements.cc
M	source/blender/nodes/geometry/nodes/node_geo_extrude_mesh.cc
M	source/blender/nodes/geometry/nodes/node_geo_instance_on_points.cc

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

diff --git a/source/blender/blenkernel/intern/curves_geometry.cc b/source/blender/blenkernel/intern/curves_geometry.cc
index 29f8d62545f..d65c31139fc 100644
--- a/source/blender/blenkernel/intern/curves_geometry.cc
+++ b/source/blender/blenkernel/intern/curves_geometry.cc
@@ -9,6 +9,7 @@
 
 #include "MEM_guardedalloc.h"
 
+#include "BLI_array_utils.hh"
 #include "BLI_bounds.hh"
 #include "BLI_index_mask_ops.hh"
 #include "BLI_length_parameterize.hh"
@@ -1111,21 +1112,11 @@ static void copy_between_buffers(const CPPType &type,
                         src_range.size());
 }
 
-template<typename T>
-static void copy_with_map(const Span<T> src, const Span<int> map, MutableSpan<T> dst)
-{
-  threading::parallel_for(map.index_range(), 1024, [&](const IndexRange range) {
-    for (const int i : range) {
-      dst[i] = src[map[i]];
-    }
-  });
-}
-
 static void copy_with_map(const GSpan src, const Span<int> map, GMutableSpan dst)
 {
   attribute_math::convert_to_static_type(src.type(), [&](auto dummy) {
     using T = decltype(dummy);
-    copy_with_map(src.typed<T>(), map, dst.typed<T>());
+    array_utils::gather(src.typed<T>(), map, dst.typed<T>());
   });
 }
 
diff --git a/source/blender/blenkernel/intern/instances.cc b/source/blender/blenkernel/intern/instances.cc
index 2f8acf477e2..4675562e927 100644
--- a/source/blender/blenkernel/intern/instances.cc
+++ b/source/blender/blenkernel/intern/instances.cc
@@ -1,5 +1,6 @@
 /* SPDX-License-Identifier: GPL-2.0-or-later */
 
+#include "BLI_array_utils.hh"
 #include "BLI_cpp_type_make.hh"
 #include "BLI_rand.hh"
 #include "BLI_task.hh"
@@ -107,18 +108,6 @@ blender::Span<InstanceReference> Instances::references() const
   return references_;
 }
 
-template<typename T>
-static void copy_data_based_on_mask(Span<T> src, MutableSpan<T> dst, IndexMask mask)
-{
-  BLI_assert(src.data() != dst.data());
-  using namespace blender;
-  threading::parallel_for(mask.index_range(), 1024, [&](IndexRange range) {
-    for (const int i : range) {
-      dst[i] = src[mask[i]];
-    }
-  });
-}
-
 void Instances::remove(const IndexMask mask)
 {
   using namespace blender;
@@ -129,11 +118,14 @@ void Instances::remove(const IndexMask mask)
     return;
   }
 
+  const Span<int> old_handles = this->reference_handles();
   Vector<int> new_handles(mask.size());
-  copy_data_based_on_mask<int>(this->reference_handles(), new_handles, mask);
+  array_utils::gather(old_handles, mask.indices(), new_handles.as_mutable_span());
   reference_handles_ = std::move(new_handles);
+
+  const Span<float4x4> old_tansforms = this->transforms();
   Vector<float4x4> new_transforms(mask.size());
-  copy_data_based_on_mask<float4x4>(this->transforms(), new_transforms, mask);
+  array_utils::gather(old_tansforms, mask.indices(), new_transforms.as_mutable_span());
   transforms_ = std::move(new_transforms);
 
   const bke::CustomDataAttributes &src_attributes = attributes_;
@@ -150,11 +142,8 @@ void Instances::remove(const IndexMask mask)
         GSpan src = *src_attributes.get_for_read(id);
         dst_attributes.create(id, meta_data.data_type);
         GMutableSpan dst = *dst_attributes.get_for_write(id);
+        array_utils::gather(src, mask.indices(), dst);
 
-        attribute_math::convert_to_static_type(src.type(), [&](auto dummy) {
-          using T = decltype(dummy);
-          copy_data_based_on_mask<T>(src.typed<T>(), dst.typed<T>(), mask);
-        });
         return true;
       },
       ATTR_DOMAIN_INSTANCE);
diff --git a/source/blender/blenlib/BLI_array_utils.hh b/source/blender/blenlib/BLI_array_utils.hh
index 95b3bde10f4..264ac00e034 100644
--- a/source/blender/blenlib/BLI_array_utils.hh
+++ b/source/blender/blenlib/BLI_array_utils.hh
@@ -39,6 +39,11 @@ inline void copy(const Span<T> src,
  */
 void gather(const GVArray &src, IndexMask indices, GMutableSpan dst, int64_t grain_size = 4096);
 
+/**
+ * Fill the destination span by gathering indexed values from the `src` array.
+ */
+void gather(GSpan src, IndexMask indices, GMutableSpan dst, int64_t grain_size = 4096);
+
 /**
  * Fill the destination span by gathering indexed values from the `src` array.
  */
diff --git a/source/blender/blenlib/intern/array_utils.cc b/source/blender/blenlib/intern/array_utils.cc
index a837d6aceec..2a231228dcb 100644
--- a/source/blender/blenlib/intern/array_utils.cc
+++ b/source/blender/blenlib/intern/array_utils.cc
@@ -28,4 +28,9 @@ void gather(const GVArray &src,
   });
 }
 
+void gather(const GSpan src, const IndexMask indices, GMutableSpan dst, const int64_t grain_size)
+{
+  gather(GVArray::ForSpan(src), indices, dst, grain_size);
+}
+
 }  // namespace blender::array_utils
diff --git a/source/blender/nodes/geometry/nodes/node_geo_delete_geometry.cc b/source/blender/nodes/geometry/nodes/node_geo_delete_geometry.cc
index a433a0df9b0..3e48a9fd923 100644
--- a/source/blender/nodes/geometry/nodes/node_geo_delete_geometry.cc
+++ b/source/blender/nodes/geometry/nodes/node_geo_delete_geometry.cc
@@ -4,6 +4,7 @@
 #include "UI_resources.h"
 
 #include "BLI_array.hh"
+#include "BLI_array_utils.hh"
 
 #include "DNA_mesh_types.h"
 #include "DNA_meshdata_types.h"
@@ -23,15 +24,9 @@ namespace blender::nodes::node_geo_delete_geometry_cc {
 using blender::bke::CustomDataAttributes;
 
 template<typename T>
-static void copy_data_based_on_mask(Span<T> data, MutableSpan<T> r_data, IndexMask mask)
-{
-  for (const int i_out : mask.index_range()) {
-    r_data[i_out] = data[mask[i_out]];
-  }
-}
-
-template<typename T>
-static void copy_data_based_on_map(Span<T> src, MutableSpan<T> dst, Span<int> index_map)
+static void copy_data_based_on_map(const Span<T> src,
+                                   const Span<int> index_map,
+                                   MutableSpan<T> dst)
 {
   for (const int i_src : index_map.index_range()) {
     const int i_dst = index_map[i_src];
@@ -55,26 +50,17 @@ static void copy_attributes(const Map<AttributeIDRef, AttributeKind> &attributes
     if (!attribute) {
       continue;
     }
-
     /* Only copy if it is on a domain we want. */
     if (!domains.contains(attribute.domain)) {
       continue;
     }
     const eCustomDataType data_type = bke::cpp_type_to_custom_data_type(attribute.varray.type());
-
     GSpanAttributeWriter result_attribute = dst_attributes.lookup_or_add_for_write_only_span(
         attribute_id, attribute.domain, data_type);
-
     if (!result_attribute) {
       continue;
     }
-
-    attribute_math::convert_to_static_type(data_type, [&](auto dummy) {
-      using T = decltype(dummy);
-      VArraySpan<T> span{attribute.varray.typed<T>()};
-      MutableSpan<T> out_span = result_attribute.span.typed<T>();
-      out_span.copy_from(span);
-    });
+    attribute.varray.materialize(result_attribute.span.data());
     result_attribute.finish();
   }
 }
@@ -95,26 +81,19 @@ static void copy_attributes_based_on_mask(const Map<AttributeIDRef, AttributeKin
     if (!attribute) {
       continue;
     }
-
     /* Only copy if it is on a domain we want. */
     if (domain != attribute.domain) {
       continue;
     }
     const eCustomDataType data_type = bke::cpp_type_to_custom_data_type(attribute.varray.type());
-
     GSpanAttributeWriter result_attribute = dst_attributes.lookup_or_add_for_write_only_span(
         attribute_id, attribute.domain, data_type);
-
     if (!result_attribute) {
       continue;
     }
 
-    attribute_math::convert_to_static_type(data_type, [&](auto dummy) {
-      using T = decltype(dummy);
-      VArraySpan<T> span{attribute.varray.typed<T>()};
-      MutableSpan<T> out_span = result_attribute.span.typed<T>();
-      copy_data_based_on_mask(span, out_span, mask);
-    });
+    array_utils::gather(attribute.varray, mask, result_attribute.span);
+
     result_attribute.finish();
   }
 }
@@ -131,16 +110,13 @@ static void copy_attributes_based_on_map(const Map<AttributeIDRef, AttributeKind
     if (!attribute) {
       continue;
     }
-
     /* Only copy if it is on a domain we want. */
     if (domain != attribute.domain) {
       continue;
     }
     const eCustomDataType data_type = bke::cpp_type_to_custom_data_type(attribute.varray.type());
-
     GSpanAttributeWriter result_attribute = dst_attributes.lookup_or_add_for_write_only_span(
         attribute_id, attribute.domain, data_type);
-
     if (!result_attribute) {
       continue;
     }
@@ -149,7 +125,7 @@ static void copy_attributes_based_on_map(const Map<AttributeIDRef, AttributeKind
       using T = decltype(dummy);
       VArraySpan<T> span{attribute.varray.typed<T>()};
       MutableSpan<T> out_span = result_attribute.span.typed<T>();
-      copy_data_based_on_map(span, out_span, index_map);
+      copy_data_based_on_map(span, index_map, out_span);
     });
     result_attribute.finish();
   }
diff --git a/source/blender/nodes/geometry/nodes/node_geo_dual_mesh.cc b/source/blender/nodes/geometry/nodes/node_geo_dual_mesh.cc
index 84e63845b84..9b1c13bf563 100644
--- a/source/blender/nodes/geometry/nodes/node_geo_dual_mesh.cc
+++ b/source/blender/nodes/geometry/nodes/node_geo_dual_mesh.cc
@@ -1,5 +1,6 @@
 /* SPDX-License-Identifier: GPL-2.0-or-later */
 
+#include "BLI_array_utils.hh"
 #include "BLI_task.hh"
 
 #include "DNA_mesh_types.h"
@@ -105,18 +106,6 @@ static void copy_data_based_on_pairs(Span<T> data,
   }
 }
 
-/* Copy using the map. */
-template<typename T>
-static void copy_data_based_on_new_to_old_map(Span<T> data,
-                                              MutableSpan<T> r_data,
-                                              const Span<int> new_to_old_map)
-{
-  for (const int i : r_data.index_range()) {
-    const int old_i = new_to_old_map[i];
-    r_data[i] = data[old_i];
-  }
-}
-
 /**
  * Transfers the attributes from the original mesh to the new mesh using the following logic:
  * - If the attribute was on the face domain it is now on the point domain, and this is true
@@ -168,7 +157,6 @@ static void transfer_attributes(
         src_attribute.varray.type());
     GSpanAttributeWriter dst_attribute = dst_attributes.lookup_or_add_for_write_only_span(
         attribute_id, out_domain, data_type);
-
     if (!dst_attribute) {
       continue;
     }
@@ -177,20 +165,24 @@ static void transfer_

@@ Diff output truncated at 10240 characters. @@



More information about the Bf-blender-cvs mailing list