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
This commit is contained in:
Hans Goudey 2021-11-02 15:16:52 -05:00
parent 7aa311e539
commit 18392cef17
Notes: blender-bot 2023-02-14 04:56:36 +01:00
Referenced by issue #92848, Geometry Nodes: Attribute Capture crash when joining curves
Referenced by issue #92652, Attribute capture causes flickering in render (Geometrynodes)
4 changed files with 64 additions and 0 deletions

View File

@ -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<AttributeIDRef> new_order);
bool foreach_attribute(const AttributeForeachCallback callback,
const AttributeDomain domain) const;
};

View File

@ -835,6 +835,26 @@ bool CustomDataAttributes::foreach_attribute(const AttributeForeachCallback call
return true;
}
void CustomDataAttributes::reorder(Span<AttributeIDRef> new_order)
{
BLI_assert(new_order.size() == data.totlayer);
Map<AttributeIDRef, int> old_order;
old_order.reserve(data.totlayer);
Array<CustomDataLayer> 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
/* -------------------------------------------------------------------- */

View File

@ -530,6 +530,24 @@ static void join_instance_groups_volume(Span<GeometryInstanceGroup> 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<AttributeIDRef, AttributeKind> &info,
MutableSpan<SplinePtr> splines)
{
Vector<AttributeIDRef> 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<GeometryInstanceGroup> 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<GeometryInstanceGroup> set_groups, G
{GEO_COMPONENT_TYPE_CURVE},
attributes,
static_cast<GeometryComponent &>(dst_component));
sort_curve_point_attributes(attributes, curve->splines());
}
GeometrySet geometry_set_realize_instances(const GeometrySet &geometry_set)

View File

@ -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<AttributeIDRef, AttributeMetaData> &info,
MutableSpan<SplinePtr> splines)
{
Vector<AttributeIDRef> 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<AttributeIDRef, AttributeMetaData> &
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<GeometrySet> src_geometry_sets, GeometrySet &result)