Changeset View
Standalone View
source/blender/geometry/intern/mesh_to_curve_convert.cc
- This file was copied from source/blender/nodes/geometry/nodes/legacy/node_geo_mesh_to_curve.cc.
| Show All 9 Lines | |||||
| * GNU General Public License for more details. | * GNU General Public License for more details. | ||||
| * | * | ||||
| * You should have received a copy of the GNU General Public License | * You should have received a copy of the GNU General Public License | ||||
| * along with this program; if not, write to the Free Software Foundation, | * along with this program; if not, write to the Free Software Foundation, | ||||
| * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. | * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. | ||||
| */ | */ | ||||
| #include "BLI_array.hh" | #include "BLI_array.hh" | ||||
| #include "BLI_set.hh" | |||||
| #include "BLI_task.hh" | #include "BLI_task.hh" | ||||
| #include "DNA_mesh_types.h" | #include "DNA_mesh_types.h" | ||||
| #include "DNA_meshdata_types.h" | #include "DNA_meshdata_types.h" | ||||
| #include "BKE_attribute_access.hh" | |||||
| #include "BKE_attribute_math.hh" | #include "BKE_attribute_math.hh" | ||||
| #include "BKE_geometry_set.hh" | |||||
| #include "BKE_spline.hh" | #include "BKE_spline.hh" | ||||
| #include "node_geometry_util.hh" | #include "GEO_mesh_to_curve.hh" | ||||
| using blender::Array; | namespace blender::geometry { | ||||
| namespace blender::nodes { | |||||
| static void geo_node_mesh_to_curve_declare(NodeDeclarationBuilder &b) | |||||
| { | |||||
| b.add_input<decl::Geometry>("Mesh"); | |||||
| b.add_input<decl::String>("Selection"); | |||||
| b.add_output<decl::Geometry>("Curve"); | |||||
| } | |||||
| template<typename T> | template<typename T> | ||||
| static void copy_attribute_to_points(const VArray<T> &source_data, | static void copy_attribute_to_points(const VArray<T> &source_data, | ||||
| Span<int> map, | Span<int> map, | ||||
| MutableSpan<T> dest_data) | MutableSpan<T> dest_data) | ||||
| { | { | ||||
| for (const int point_index : map.index_range()) { | for (const int point_index : map.index_range()) { | ||||
| const int vert_index = map[point_index]; | const int vert_index = map[point_index]; | ||||
| dest_data[point_index] = source_data[vert_index]; | dest_data[point_index] = source_data[vert_index]; | ||||
| } | } | ||||
| } | } | ||||
| static void copy_attributes_to_points(CurveEval &curve, | static void copy_attributes_to_points(CurveEval &curve, | ||||
| const MeshComponent &mesh_component, | const MeshComponent &mesh_component, | ||||
| Span<Vector<int>> point_to_vert_maps) | Span<Vector<int>> point_to_vert_maps) | ||||
| { | { | ||||
| MutableSpan<SplinePtr> splines = curve.splines(); | MutableSpan<SplinePtr> splines = curve.splines(); | ||||
| Set<AttributeIDRef> source_attribute_ids = mesh_component.attribute_ids(); | Set<bke::AttributeIDRef> source_attribute_ids = mesh_component.attribute_ids(); | ||||
| /* Copy builtin control point attributes. */ | /* Copy builtin control point attributes. */ | ||||
| if (source_attribute_ids.contains("tilt")) { | if (source_attribute_ids.contains("tilt")) { | ||||
| const GVArray_Typed<float> tilt_attribute = mesh_component.attribute_get_for_read<float>( | const fn::GVArray_Typed<float> tilt_attribute = mesh_component.attribute_get_for_read<float>( | ||||
| "tilt", ATTR_DOMAIN_POINT, 0.0f); | "tilt", ATTR_DOMAIN_POINT, 0.0f); | ||||
| threading::parallel_for(splines.index_range(), 256, [&](IndexRange range) { | threading::parallel_for(splines.index_range(), 256, [&](IndexRange range) { | ||||
| for (const int i : range) { | for (const int i : range) { | ||||
| copy_attribute_to_points<float>( | copy_attribute_to_points<float>( | ||||
| *tilt_attribute, point_to_vert_maps[i], splines[i]->tilts()); | *tilt_attribute, point_to_vert_maps[i], splines[i]->tilts()); | ||||
| } | } | ||||
| }); | }); | ||||
| source_attribute_ids.remove_contained("tilt"); | source_attribute_ids.remove_contained("tilt"); | ||||
| } | } | ||||
| if (source_attribute_ids.contains("radius")) { | if (source_attribute_ids.contains("radius")) { | ||||
| const GVArray_Typed<float> radius_attribute = mesh_component.attribute_get_for_read<float>( | const fn::GVArray_Typed<float> radius_attribute = mesh_component.attribute_get_for_read<float>( | ||||
| "radius", ATTR_DOMAIN_POINT, 1.0f); | "radius", ATTR_DOMAIN_POINT, 1.0f); | ||||
| threading::parallel_for(splines.index_range(), 256, [&](IndexRange range) { | threading::parallel_for(splines.index_range(), 256, [&](IndexRange range) { | ||||
| for (const int i : range) { | for (const int i : range) { | ||||
| copy_attribute_to_points<float>( | copy_attribute_to_points<float>( | ||||
| *radius_attribute, point_to_vert_maps[i], splines[i]->radii()); | *radius_attribute, point_to_vert_maps[i], splines[i]->radii()); | ||||
| } | } | ||||
| }); | }); | ||||
| source_attribute_ids.remove_contained("radius"); | source_attribute_ids.remove_contained("radius"); | ||||
| } | } | ||||
| /* Don't copy other builtin control point attributes. */ | /* Don't copy attributes that are built-in on meshes but not on curves. */ | ||||
| source_attribute_ids.remove("position"); | for (const bke::AttributeIDRef &attribute_id : source_attribute_ids) { | ||||
| if (mesh_component.attribute_is_builtin(attribute_id)) { | |||||
| source_attribute_ids.remove(attribute_id); | |||||
JacquesLucke: One has to be a bit careful with removing elements from the set you are iterating over. | |||||
HooglyBooglyAuthorUnsubmitted Done Inline ActionsRemoving was pretty simple to add, but I wonder, it looks like a const cast is required (It isn't in the map remove function because the iterator is not internally const-correct. For now there was no real reason to use a separate loop, so I just combined them. I also added a check for unused anonymous attributes. I'll refactor the way attributes are gathered for propagation to make this unnecessary though. diff --git a/source/blender/blenlib/BLI_set.hh b/source/blender/blenlib/BLI_set.hh index a8ccf957f6c..1ac6bdbbd09 100644 --- a/source/blender/blenlib/BLI_set.hh +++ b/source/blender/blenlib/BLI_set.hh @@ -423,6 +423,8 @@ class Set { int64_t total_slots_; int64_t current_slot_; + friend Set; + public: Iterator(const Slot *slots, int64_t total_slots, int64_t current_slot) : slots_(slots), total_slots_(total_slots), current_slot_(current_slot) @@ -467,6 +469,12 @@ class Set { { return !(a != b); } + + protected: + Slot ¤t_slot() const + { + return slots_[current_slot_]; + } }; Iterator begin() const @@ -484,6 +492,19 @@ class Set { return Iterator(slots_.data(), slots_.size(), slots_.size()); } + /** + * Remove the key-value-pair that the iterator is currently pointing at. + * It is valid to call this method while iterating over the map. However, after this method has + * been called, the removed element must not be accessed anymore. + */ + void remove(const Iterator &iterator) + { + Slot &slot = iterator.current_slot(); + BLI_assert(slot.is_occupied()); + slot.remove(); + removed_slots_++; + } + /** * Print common statistics like size and collision count. This is useful for debugging purposes. */ diff --git a/source/blender/blenlib/tests/BLI_set_test.cc b/source/blender/blenlib/tests/BLI_set_test.cc index 3a4733a218f..b2e1f94d22c 100644 --- a/source/blender/blenlib/tests/BLI_set_test.cc +++ b/source/blender/blenlib/tests/BLI_set_test.cc @@ -544,6 +544,33 @@ TEST(set, GenericAlgorithms) EXPECT_EQ(std::count(set.begin(), set.end(), 20), 1); } +TEST(set, RemoveDuringIteration) +{ + Set<int> set; + set.add(2); + set.add(5); + set.add(1); + set.add(2); + set.add(3); + + EXPECT_EQ(set.size(), 5); + + using Iter = Set<int>::Iterator; + Iter begin = set.begin(); + Iter end = set.end(); + for (Iter iter = begin; iter != end; ++iter) { + int item = *iter; + if (item == 2) { + set.remove(iter); + } + } + + EXPECT_EQ(set.size(), 3); + EXPECT_TRUE(set.contains(2)); + EXPECT_TRUE(set.contains(6)); + EXPECT_TRUE(set.contains(3)); +} + /** * Set this to 1 to activate the benchmark. It is disabled by default, because it prints a lot. */ HooglyBoogly: Removing was pretty simple to add, but I wonder, it looks like a const cast is required (It… | |||||
JacquesLuckeUnsubmitted Not Done Inline ActionsThe comment in this patch still says map instead of set. Other than that, it looks fine. Did you actually run the unit test? This part looks wrong already because you add the same value twice: Set<int> set; set.add(2); set.add(5); set.add(1); set.add(2); set.add(3); EXPECT_EQ(set.size(), 5); JacquesLucke: The comment in this patch still says `map` instead of `set`. Other than that, it looks fine. | |||||
| } | |||||
| } | |||||
| /* Copy dynamic control point attributes. */ | for (const bke::AttributeIDRef &attribute_id : source_attribute_ids) { | ||||
| for (const AttributeIDRef &attribute_id : source_attribute_ids) { | const fn::GVArrayPtr mesh_attribute = mesh_component.attribute_try_get_for_read( | ||||
| const GVArrayPtr mesh_attribute = mesh_component.attribute_try_get_for_read(attribute_id, | attribute_id, ATTR_DOMAIN_POINT); | ||||
| ATTR_DOMAIN_POINT); | |||||
| /* Some attributes might not exist if they were builtin attribute on domains that don't | /* Some attributes might not exist if they were builtin attribute on domains that don't | ||||
| * have any elements, i.e. a face attribute on the output of the line primitive node. */ | * have any elements, i.e. a face attribute on the output of the line primitive node. */ | ||||
| if (!mesh_attribute) { | if (!mesh_attribute) { | ||||
| continue; | continue; | ||||
| } | } | ||||
| const CustomDataType data_type = bke::cpp_type_to_custom_data_type(mesh_attribute->type()); | const CustomDataType data_type = bke::cpp_type_to_custom_data_type(mesh_attribute->type()); | ||||
| threading::parallel_for(splines.index_range(), 128, [&](IndexRange range) { | threading::parallel_for(splines.index_range(), 128, [&](IndexRange range) { | ||||
| for (const int i : range) { | for (const int i : range) { | ||||
| /* Create attribute on the spline points. */ | /* Create attribute on the spline points. */ | ||||
| splines[i]->attributes.create(attribute_id, data_type); | splines[i]->attributes.create(attribute_id, data_type); | ||||
| std::optional<GMutableSpan> spline_attribute = splines[i]->attributes.get_for_write( | std::optional<fn::GMutableSpan> spline_attribute = splines[i]->attributes.get_for_write( | ||||
| attribute_id); | attribute_id); | ||||
| BLI_assert(spline_attribute); | BLI_assert(spline_attribute); | ||||
| /* Copy attribute based on the map for this spline. */ | /* Copy attribute based on the map for this spline. */ | ||||
| attribute_math::convert_to_static_type(mesh_attribute->type(), [&](auto dummy) { | attribute_math::convert_to_static_type(mesh_attribute->type(), [&](auto dummy) { | ||||
| using T = decltype(dummy); | using T = decltype(dummy); | ||||
| copy_attribute_to_points<T>( | copy_attribute_to_points<T>( | ||||
| mesh_attribute->typed<T>(), point_to_vert_maps[i], spline_attribute->typed<T>()); | mesh_attribute->typed<T>(), point_to_vert_maps[i], spline_attribute->typed<T>()); | ||||
| }); | }); | ||||
| } | } | ||||
| }); | }); | ||||
| } | } | ||||
| curve.assert_valid_point_attributes(); | curve.assert_valid_point_attributes(); | ||||
| } | } | ||||
| struct CurveFromEdgesOutput { | struct CurveFromEdgesOutput { | ||||
| std::unique_ptr<CurveEval> curve; | std::unique_ptr<CurveEval> curve; | ||||
| Vector<Vector<int>> point_to_vert_maps; | Vector<Vector<int>> point_to_vert_maps; | ||||
| }; | }; | ||||
| static CurveFromEdgesOutput mesh_to_curve(Span<MVert> verts, Span<std::pair<int, int>> edges) | static CurveFromEdgesOutput edges_to_curve(Span<MVert> verts, Span<std::pair<int, int>> edges) | ||||
| { | { | ||||
| std::unique_ptr<CurveEval> curve = std::make_unique<CurveEval>(); | std::unique_ptr<CurveEval> curve = std::make_unique<CurveEval>(); | ||||
| Vector<Vector<int>> point_to_vert_maps; | Vector<Vector<int>> point_to_vert_maps; | ||||
| /* Compute the number of edges connecting to each vertex. */ | /* Compute the number of edges connecting to each vertex. */ | ||||
| Array<int> neighbor_count(verts.size(), 0); | Array<int> neighbor_count(verts.size(), 0); | ||||
| for (const std::pair<int, int> &edge : edges) { | for (const std::pair<int, int> &edge : edges) { | ||||
| neighbor_count[edge.first]++; | neighbor_count[edge.first]++; | ||||
| ▲ Show 20 Lines • Show All 116 Lines • ▼ Show 20 Lines | static CurveFromEdgesOutput edges_to_curve(Span<MVert> verts, Span<std::pair<int, int>> edges) | ||||
| return {std::move(curve), std::move(point_to_vert_maps)}; | return {std::move(curve), std::move(point_to_vert_maps)}; | ||||
| } | } | ||||
| /** | /** | ||||
| * Get a separate array of the indices for edges in a selection (a boolean attribute). | * Get a separate array of the indices for edges in a selection (a boolean attribute). | ||||
| * This helps to make the above algorithm simpler by removing the need to check for selection | * This helps to make the above algorithm simpler by removing the need to check for selection | ||||
| * in many places. | * in many places. | ||||
| */ | */ | ||||
| static Vector<std::pair<int, int>> get_selected_edges(GeoNodeExecParams params, | static Vector<std::pair<int, int>> get_selected_edges(const Mesh &mesh, const IndexMask selection) | ||||
| const MeshComponent &component) | |||||
| { | { | ||||
| const Mesh &mesh = *component.get_for_read(); | |||||
| const std::string selection_name = params.extract_input<std::string>("Selection"); | |||||
| if (!selection_name.empty() && !component.attribute_exists(selection_name)) { | |||||
| params.error_message_add(NodeWarningType::Error, | |||||
| TIP_("No attribute with name \"") + selection_name + "\""); | |||||
| } | |||||
| GVArray_Typed<bool> selection = component.attribute_get_for_read<bool>( | |||||
| selection_name, ATTR_DOMAIN_EDGE, true); | |||||
| Vector<std::pair<int, int>> selected_edges; | Vector<std::pair<int, int>> selected_edges; | ||||
| for (const int i : IndexRange(mesh.totedge)) { | for (const int i : selection) { | ||||
| if (selection[i]) { | |||||
| selected_edges.append({mesh.medge[i].v1, mesh.medge[i].v2}); | selected_edges.append({mesh.medge[i].v1, mesh.medge[i].v2}); | ||||
| } | } | ||||
| } | |||||
| return selected_edges; | return selected_edges; | ||||
| } | } | ||||
| static void geo_node_mesh_to_curve_exec(GeoNodeExecParams params) | /** | ||||
| * Convert the mesh into one or many poly splines. Since splines cannot have branches, | |||||
| * intersections of more than three edges will become breaks in splines. Attributes that | |||||
| * are not built-in on meshes and not curves are transferred to the result curve. | |||||
| */ | |||||
| std::unique_ptr<CurveEval> mesh_to_curve_convert(const MeshComponent &mesh_component, | |||||
| const IndexMask selection) | |||||
| { | { | ||||
| GeometrySet geometry_set = params.extract_input<GeometrySet>("Mesh"); | const Mesh &mesh = *mesh_component.get_for_read(); | ||||
| Vector<std::pair<int, int>> selected_edges = get_selected_edges(*mesh_component.get_for_read(), | |||||
| geometry_set = bke::geometry_set_realize_instances(geometry_set); | selection); | ||||
| CurveFromEdgesOutput output = edges_to_curve({mesh.mvert, mesh.totvert}, selected_edges); | |||||
| if (!geometry_set.has_mesh()) { | copy_attributes_to_points(*output.curve, mesh_component, output.point_to_vert_maps); | ||||
| params.set_output("Curve", GeometrySet()); | return std::move(output.curve); | ||||
| return; | |||||
| } | |||||
| const MeshComponent &component = *geometry_set.get_component_for_read<MeshComponent>(); | |||||
| const Mesh &mesh = *component.get_for_read(); | |||||
| Span<MVert> verts = Span{mesh.mvert, mesh.totvert}; | |||||
| Vector<std::pair<int, int>> selected_edges = get_selected_edges(params, component); | |||||
| if (selected_edges.size() == 0) { | |||||
| params.set_output("Curve", GeometrySet()); | |||||
| return; | |||||
| } | } | ||||
| CurveFromEdgesOutput output = mesh_to_curve(verts, selected_edges); | } // namespace blender::geometry | ||||
| copy_attributes_to_points(*output.curve, component, output.point_to_vert_maps); | |||||
| params.set_output("Curve", GeometrySet::create_with_curve(output.curve.release())); | |||||
| } | |||||
| } // namespace blender::nodes | |||||
| void register_node_type_geo_mesh_to_curve() | |||||
| { | |||||
| static bNodeType ntype; | |||||
| geo_node_type_base( | |||||
| &ntype, GEO_NODE_LEGACY_MESH_TO_CURVE, "Mesh to Curve", NODE_CLASS_GEOMETRY, 0); | |||||
| ntype.declare = blender::nodes::geo_node_mesh_to_curve_declare; | |||||
| ntype.geometry_node_execute = blender::nodes::geo_node_mesh_to_curve_exec; | |||||
| nodeRegisterType(&ntype); | |||||
| } | |||||
One has to be a bit careful with removing elements from the set you are iterating over. blender::Set should support that by design, but it's not part of the "specification" yet.
For blender::Map I added void remove(const BaseIterator &iterator) for this use case. It's even a bit more efficient because it avoids looking up the key again.
The same could be done for blender::Set. Maybe leave a comment about that or add the functionality to blender::Set.
You also don't want to copy over anonymous attributes that don't have strong references anymore.