From a803dbe7ed80df577b648f9e289aaed2a2cc1700 Mon Sep 17 00:00:00 2001 From: Hans Goudey Date: Wed, 19 Oct 2022 12:38:48 -0500 Subject: 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. --- .../geometry/nodes/node_geo_delete_geometry.cc | 40 +++---------- .../nodes/geometry/nodes/node_geo_dual_mesh.cc | 46 +++++++-------- .../geometry/nodes/node_geo_duplicate_elements.cc | 25 +++------ .../nodes/geometry/nodes/node_geo_extrude_mesh.cc | 65 ++++++++-------------- .../geometry/nodes/node_geo_instance_on_points.cc | 14 +---- 5 files changed, 61 insertions(+), 129 deletions(-) (limited to 'source/blender/nodes/geometry') 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 -static void copy_data_based_on_mask(Span data, MutableSpan r_data, IndexMask mask) -{ - for (const int i_out : mask.index_range()) { - r_data[i_out] = data[mask[i_out]]; - } -} - -template -static void copy_data_based_on_map(Span src, MutableSpan dst, Span index_map) +static void copy_data_based_on_map(const Span src, + const Span index_map, + MutableSpan 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 &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 span{attribute.varray.typed()}; - MutableSpan out_span = result_attribute.span.typed(); - 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 span{attribute.varray.typed()}; - MutableSpan out_span = result_attribute.span.typed(); - 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 span{attribute.varray.typed()}; MutableSpan out_span = result_attribute.span.typed(); - 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 data, } } -/* Copy using the map. */ -template -static void copy_data_based_on_new_to_old_map(Span data, - MutableSpan r_data, - const Span 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_attributes( using T = decltype(dummy); VArraySpan span{src_attribute.varray.typed()}; MutableSpan dst_span = dst_attribute.span.typed(); - if (src_attribute.domain == ATTR_DOMAIN_FACE) { - dst_span.take_front(span.size()).copy_from(span); - if (keep_boundaries) { - copy_data_based_on_pairs(span, dst_span, boundary_vertex_to_relevant_face_map); - } - } - else if (src_attribute.domain == ATTR_DOMAIN_POINT) { - copy_data_based_on_vertex_types(span, dst_span, vertex_types, keep_boundaries); - } - else if (src_attribute.domain == ATTR_DOMAIN_EDGE) { - copy_data_based_on_new_to_old_map(span, dst_span, new_to_old_edges_map); - } - else { - copy_data_based_on_new_to_old_map(span, dst_span, new_to_old_face_corners_map); + switch (src_attribute.domain) { + case ATTR_DOMAIN_POINT: + copy_data_based_on_vertex_types(span, dst_span, vertex_types, keep_boundaries); + break; + case ATTR_DOMAIN_EDGE: + array_utils::gather(span, new_to_old_edges_map, dst_span); + break; + case ATTR_DOMAIN_FACE: + dst_span.take_front(span.size()).copy_from(span); + if (keep_boundaries) { + copy_data_based_on_pairs(span, dst_span, boundary_vertex_to_relevant_face_map); + } + break; + case ATTR_DOMAIN_CORNER: + array_utils::gather(span, new_to_old_face_corners_map, dst_span); + break; + default: + BLI_assert_unreachable(); } }); dst_attribute.finish(); diff --git a/source/blender/nodes/geometry/nodes/node_geo_duplicate_elements.cc b/source/blender/nodes/geometry/nodes/node_geo_duplicate_elements.cc index 70583625a7a..486f900aca5 100644 --- a/source/blender/nodes/geometry/nodes/node_geo_duplicate_elements.cc +++ b/source/blender/nodes/geometry/nodes/node_geo_duplicate_elements.cc @@ -1,5 +1,6 @@ /* SPDX-License-Identifier: GPL-2.0-or-later */ +#include "BLI_array_utils.hh" #include "BLI_map.hh" #include "BLI_noise.hh" #include "BLI_span.hh" @@ -105,16 +106,6 @@ static void threaded_slice_fill(Span offsets, }); } -template -static void threaded_mapped_copy(const Span mapping, const Span src, MutableSpan dst) -{ - threading::parallel_for(mapping.index_range(), 512, [&](IndexRange range) { - for (const int i : range) { - dst[i] = src[mapping[i]]; - } - }); -} - static void copy_hashed_ids(const Span src, const int hash, MutableSpan dst) { for (const int i : src.index_range()) { @@ -440,17 +431,17 @@ static void copy_face_attributes_without_id(GeometrySet &geometry_set, MutableSpan dst = dst_attribute.span.typed(); switch (out_domain) { - case ATTR_DOMAIN_FACE: - threaded_slice_fill(offsets, selection, src, dst); + case ATTR_DOMAIN_POINT: + array_utils::gather(src, vert_mapping, dst); break; case ATTR_DOMAIN_EDGE: - threaded_mapped_copy(edge_mapping, src, dst); + array_utils::gather(src, edge_mapping, dst); break; - case ATTR_DOMAIN_POINT: - threaded_mapped_copy(vert_mapping, src, dst); + case ATTR_DOMAIN_FACE: + threaded_slice_fill(offsets, selection, src, dst); break; case ATTR_DOMAIN_CORNER: - threaded_mapped_copy(loop_mapping, src, dst); + array_utils::gather(src, loop_mapping, dst); break; default: break; @@ -653,7 +644,7 @@ static void copy_edge_attributes_without_id(GeometrySet &geometry_set, threaded_slice_fill(offsets, selection, src, dst); break; case ATTR_DOMAIN_POINT: - threaded_mapped_copy(point_mapping, src, dst); + array_utils::gather(src, point_mapping, dst); break; default: break; diff --git a/source/blender/nodes/geometry/nodes/node_geo_extrude_mesh.cc b/source/blender/nodes/geometry/nodes/node_geo_extrude_mesh.cc index d348d886ad6..151ba3e59cc 100644 --- a/source/blender/nodes/geometry/nodes/node_geo_extrude_mesh.cc +++ b/source/blender/nodes/geometry/nodes/node_geo_extrude_mesh.cc @@ -1,5 +1,6 @@ /* SPDX-License-Identifier: GPL-2.0-or-later */ +#include "BLI_array_utils.hh" #include "BLI_disjoint_set.hh" #include "BLI_task.hh" #include "BLI_vector_set.hh" @@ -175,24 +176,6 @@ static MPoly new_poly(const int loopstart, const int totloop) return poly; } -template void copy_with_indices(MutableSpan dst, Span src, Span indices) -{ - BLI_assert(dst.size() == indices.size()); - for (const int i : dst.index_range()) { - dst[i] = src[indices[i]]; - } -} - -template void copy_with_mask(MutableSpan dst, Span src, IndexMask mask) -{ - BLI_assert(dst.size() == mask.size()); - threading::parallel_for(mask.index_range(), 512, [&](const IndexRange range) { - for (const int i : range) { - dst[i] = src[mask[i]]; - } - }); -} - /** * \param get_mix_indices_fn: Returns a Span of indices of the source points to mix for every * result point. @@ -265,26 +248,24 @@ static void extrude_mesh_vertices(Mesh &mesh, } GSpanAttributeWriter attribute = attributes.lookup_or_add_for_write_span( id, meta_data.domain, meta_data.data_type); - attribute_math::convert_to_static_type(meta_data.data_type, [&](auto dummy) { - using T = decltype(dummy); - MutableSpan data = attribute.span.typed(); - switch (attribute.domain) { - case ATTR_DOMAIN_POINT: { - /* New vertices copy the attribute values from their source vertex. */ - copy_with_mask(data.slice(new_vert_range), data.as_span(), selection); - break; - } - case ATTR_DOMAIN_EDGE: { + switch (attribute.domain) { + case ATTR_DOMAIN_POINT: + /* New vertices copy the attribute values from their source vertex. */ + array_utils::gather(attribute.span, selection, attribute.span.slice(new_vert_range)); + break; + case ATTR_DOMAIN_EDGE: + attribute_math::convert_to_static_type(meta_data.data_type, [&](auto dummy) { + using T = decltype(dummy); + MutableSpan data = attribute.span.typed(); /* New edge values are mixed from of all the edges connected to the source vertex. */ copy_with_mixing(data.slice(new_edge_range), data.as_span(), [&](const int i) { return vert_to_edge_map[selection[i]].as_span(); }); - break; - } - default: - BLI_assert_unreachable(); - } - }); + }); + break; + default: + BLI_assert_unreachable(); + } attribute.finish(); return true; @@ -524,13 +505,14 @@ static void extrude_mesh_edges(Mesh &mesh, switch (attribute.domain) { case ATTR_DOMAIN_POINT: { /* New vertices copy the attribute values from their source vertex. */ - copy_with_indices(data.slice(new_vert_range), data.as_span(), new_vert_indices); + array_utils::gather( + data.as_span(), new_vert_indices.as_span(), data.slice(new_vert_range)); break; } case ATTR_DOMAIN_EDGE: { /* Edges parallel to original edges copy the edge attributes from the original edges. */ MutableSpan duplicate_data = data.slice(duplicate_edge_range); - copy_with_mask(duplicate_data, data.as_span(), edge_selection); + array_utils::gather(data.as_span(), edge_selection, duplicate_data); /* Edges connected to original vertices mix values of selected connected edges. */ MutableSpan connect_data = data.slice(connect_edge_range); @@ -910,17 +892,18 @@ static void extrude_mesh_face_regions(Mesh &mesh, switch (attribute.domain) { case ATTR_DOMAIN_POINT: { /* New vertices copy the attributes from their original vertices. */ - copy_with_indices(data.slice(new_vert_range), data.as_span(), new_vert_indices); + array_utils::gather( + data.as_span(), new_vert_indices.as_span(), data.slice(new_vert_range)); break; } case ATTR_DOMAIN_EDGE: { /* Edges parallel to original edges copy the edge attributes from the original edges. */ MutableSpan boundary_data = data.slice(boundary_edge_range); - copy_with_indices(boundary_data, data.as_span(), boundary_edge_indices); + array_utils::gather(data.as_span(), boundary_edge_indices.as_span(), boundary_data); /* Edges inside of face regions also just duplicate their source data. */ MutableSpan new_inner_data = data.slice(new_inner_edge_range); - copy_with_indices(new_inner_data, data.as_span(), new_inner_edge_indices); + array_utils::gather(data.as_span(), new_inner_edge_indices.as_span(), new_inner_data); /* Edges connected to original vertices mix values of selected connected edges. */ MutableSpan connect_data = data.slice(connect_edge_range); @@ -932,8 +915,8 @@ static void extrude_mesh_face_regions(Mesh &mesh, case ATTR_DOMAIN_FACE: { /* New faces on the side of extrusions get the values from the corresponding selected * face. */ - copy_with_indices( - data.slice(side_poly_range), data.as_span(), edge_extruded_face_indices); + array_utils::gather( + data.as_span(), edge_extruded_face_indices.as_span(), data.slice(side_poly_range)); break; } case ATTR_DOMAIN_CORNER: { diff --git a/source/blender/nodes/geometry/nodes/node_geo_instance_on_points.cc b/source/blender/nodes/geometry/nodes/node_geo_instance_on_points.cc index affeb43e33b..64546684186 100644 --- a/source/blender/nodes/geometry/nodes/node_geo_instance_on_points.cc +++ b/source/blender/nodes/geometry/nodes/node_geo_instance_on_points.cc @@ -2,6 +2,7 @@ #include "DNA_collection_types.h" +#include "BLI_array_utils.hh" #include "BLI_hash.h" #include "BLI_task.hh" @@ -172,18 +173,7 @@ static void add_instances_from_component( dst_attribute_opt = instance_attributes.get_for_write(attribute_id); } BLI_assert(dst_attribute_opt); - const GMutableSpan dst_attribute = dst_attribute_opt->slice(start_len, select_len); - threading::parallel_for(selection.index_range(), 1024, [&](IndexRange selection_range) { - attribute_math::convert_to_static_type(attribute_kind.data_type, [&](auto dummy) { - using T = decltype(dummy); - VArray src = src_attribute.typed(); - MutableSpan dst = dst_attribute.typed(); - for (const int range_i : selection_range) { - const int i = selection[range_i]; - dst[range_i] = src[i]; - } - }); - }); + array_utils::gather(src_attribute, selection, dst_attribute_opt->slice(start_len, select_len)); } } -- cgit v1.2.3