From 18392cef17ddb40857fdb2462d8bad50f389029b Mon Sep 17 00:00:00 2001 From: Hans Goudey Date: Tue, 2 Nov 2021 15:16:52 -0500 Subject: Fix T92652: Joining curves breaks point attribute order Currently the curve to mesh node relies on the order of attributes being the same on every spline. This is a worthwhile assumption, because it will allow removing the attribute name storage duplication on every spline in the future. However, the join geometry node broke this order, since it just created new attributes without any regard to the order. To fix this, I added a "reorder" step after all the merged attributes have been created, the types have been joined, etc. It should be possible to change this code so that the attributes are added with the right order in the first place, but I would like to do that after refactoring spline attribute storage, and not for 3.0. Differential Revision: https://developer.blender.org/D13074 --- source/blender/blenkernel/BKE_attribute_access.hh | 5 +++++ source/blender/blenkernel/intern/attribute_access.cc | 20 ++++++++++++++++++++ .../blenkernel/intern/geometry_set_instances.cc | 19 +++++++++++++++++++ .../nodes/geometry/nodes/node_geo_join_geometry.cc | 20 ++++++++++++++++++++ 4 files changed, 64 insertions(+) (limited to 'source') diff --git a/source/blender/blenkernel/BKE_attribute_access.hh b/source/blender/blenkernel/BKE_attribute_access.hh index cf19eb29a0e..6a87375e5e2 100644 --- a/source/blender/blenkernel/BKE_attribute_access.hh +++ b/source/blender/blenkernel/BKE_attribute_access.hh @@ -372,6 +372,11 @@ class CustomDataAttributes { void *buffer); bool remove(const AttributeIDRef &attribute_id); + /** + * Change the order of the attributes to match the order of IDs in the argument. + */ + void reorder(Span new_order); + bool foreach_attribute(const AttributeForeachCallback callback, const AttributeDomain domain) const; }; diff --git a/source/blender/blenkernel/intern/attribute_access.cc b/source/blender/blenkernel/intern/attribute_access.cc index aeb66dc5a33..cd394a4ca42 100644 --- a/source/blender/blenkernel/intern/attribute_access.cc +++ b/source/blender/blenkernel/intern/attribute_access.cc @@ -835,6 +835,26 @@ bool CustomDataAttributes::foreach_attribute(const AttributeForeachCallback call return true; } +void CustomDataAttributes::reorder(Span new_order) +{ + BLI_assert(new_order.size() == data.totlayer); + + Map old_order; + old_order.reserve(data.totlayer); + Array old_layers(Span(data.layers, data.totlayer)); + for (const int i : old_layers.index_range()) { + old_order.add_new(attribute_id_from_custom_data_layer(old_layers[i]), i); + } + + MutableSpan layers(data.layers, data.totlayer); + for (const int i : layers.index_range()) { + const int old_index = old_order.lookup(new_order[i]); + layers[i] = old_layers[old_index]; + } + + CustomData_update_typemap(&data); +} + } // namespace blender::bke /* -------------------------------------------------------------------- */ diff --git a/source/blender/blenkernel/intern/geometry_set_instances.cc b/source/blender/blenkernel/intern/geometry_set_instances.cc index 35c6a7df2c7..82fc14e7772 100644 --- a/source/blender/blenkernel/intern/geometry_set_instances.cc +++ b/source/blender/blenkernel/intern/geometry_set_instances.cc @@ -530,6 +530,24 @@ static void join_instance_groups_volume(Span set_groups, } } +/** + * Curve point domain attributes must be in the same order on every spline. The order might have + * been different on separate instances, so ensure that all splines have the same order. Note that + * because #Map is used, the order is not necessarily consistent every time, but it is the same for + * every spline, and that's what matters. + */ +static void sort_curve_point_attributes(const Map &info, + MutableSpan splines) +{ + Vector new_order; + for (const AttributeIDRef attribute_id : info.keys()) { + new_order.append(attribute_id); + } + for (SplinePtr &spline : splines) { + spline->attributes.reorder(new_order); + } +} + static void join_instance_groups_curve(Span set_groups, GeometrySet &result) { CurveEval *curve = join_curve_splines_and_builtin_attributes(set_groups); @@ -550,6 +568,7 @@ static void join_instance_groups_curve(Span set_groups, G {GEO_COMPONENT_TYPE_CURVE}, attributes, static_cast(dst_component)); + sort_curve_point_attributes(attributes, curve->splines()); } GeometrySet geometry_set_realize_instances(const GeometrySet &geometry_set) diff --git a/source/blender/nodes/geometry/nodes/node_geo_join_geometry.cc b/source/blender/nodes/geometry/nodes/node_geo_join_geometry.cc index cd385f364e9..1422bf2e475 100644 --- a/source/blender/nodes/geometry/nodes/node_geo_join_geometry.cc +++ b/source/blender/nodes/geometry/nodes/node_geo_join_geometry.cc @@ -356,6 +356,24 @@ static void ensure_control_point_attribute(const AttributeIDRef &attribute_id, } } +/** + * Curve point domain attributes must be in the same order on every spline. The order might have + * been different on separate instances, so ensure that all splines have the same order. Note that + * because #Map is used, the order is not necessarily consistent every time, but it is the same for + * every spline, and that's what matters. + */ +static void sort_curve_point_attributes(const Map &info, + MutableSpan splines) +{ + Vector new_order; + for (const AttributeIDRef attribute_id : info.keys()) { + new_order.append(attribute_id); + } + for (SplinePtr &spline : splines) { + spline->attributes.reorder(new_order); + } +} + /** * Fill data for an attribute on the new curve based on all source curves. */ @@ -409,6 +427,8 @@ static void join_curve_attributes(const Map & ensure_control_point_attribute(attribute_id, meta_data.data_type, src_components, result); } } + + sort_curve_point_attributes(info, result.splines()); } static void join_curve_components(MutableSpan src_geometry_sets, GeometrySet &result) -- cgit v1.2.3