From 95055af668337cbb07dbb2a0bb0b0adafdc39351 Mon Sep 17 00:00:00 2001 From: Hans Goudey Date: Thu, 30 Jun 2022 19:01:02 -0500 Subject: [PATCH] Fix T99309: Duplicate elements deletes instance attributes The node had incorrect handling of instance attributes. For the instance component, instead of using the instance domain over the point domain, it just looked through instances recursively when retrieving attributes. We never want to do that, since we only work on one geometry at a time. Instead, just switch out the instance domain for the point domain like the set position node. --- .../nodes/node_geo_duplicate_elements.cc | 81 +++++++++---------- 1 file changed, 37 insertions(+), 44 deletions(-) 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 691f341b518..108a9443d58 100644 --- a/source/blender/nodes/geometry/nodes/node_geo_duplicate_elements.cc +++ b/source/blender/nodes/geometry/nodes/node_geo_duplicate_elements.cc @@ -61,13 +61,11 @@ struct IndexAttributes { * \{ */ static Map gather_attributes_without_id( - const GeometrySet &geometry_set, - const GeometryComponentType component_type, - const bool include_instances) + const GeometrySet &geometry_set, const GeometryComponentType component_type) { Map attributes; geometry_set.gather_attributes_for_propagation( - {component_type}, component_type, include_instances, attributes); + {component_type}, component_type, false, attributes); attributes.remove("id"); return attributes; }; @@ -186,26 +184,21 @@ static void copy_stable_id_point(const Span offsets, dst_attribute.save(); } -/* The attributes for the point (also instance) duplicated elements are stored sequentially - * (1,1,1,2,2,2,3,3,3,etc) They can be copied by using a simple offset array. For each domain, if - * elements are ordered differently a custom function is called to copy the attributes. - */ - -static void copy_point_attributes_without_id(GeometrySet &geometry_set, - const GeometryComponentType component_type, - const bool include_instances, - const Span offsets, - const IndexMask selection, - const GeometryComponent &src_component, - GeometryComponent &dst_component) +static void copy_attributes_without_id(GeometrySet &geometry_set, + const GeometryComponentType component_type, + const eAttrDomain domain, + const Span offsets, + const IndexMask selection, + const GeometryComponent &src_component, + GeometryComponent &dst_component) { - Map attributes = gather_attributes_without_id( - geometry_set, component_type, include_instances); + const Map attributes = gather_attributes_without_id( + geometry_set, component_type); for (const Map::Item entry : attributes.items()) { const AttributeIDRef attribute_id = entry.key; ReadAttributeLookup src_attribute = src_component.attribute_try_get_for_read(attribute_id); - if (!src_attribute || src_attribute.domain != ATTR_DOMAIN_POINT) { + if (!src_attribute || src_attribute.domain != domain) { continue; } eAttrDomain out_domain = src_attribute.domain; @@ -245,7 +238,7 @@ static void copy_curve_attributes_without_id(const GeometrySet &geometry_set, CurveComponent &dst_component) { Map attributes = gather_attributes_without_id( - geometry_set, GEO_COMPONENT_TYPE_CURVE, false); + geometry_set, GEO_COMPONENT_TYPE_CURVE); for (const Map::Item entry : attributes.items()) { const AttributeIDRef attribute_id = entry.key; @@ -426,7 +419,7 @@ static void copy_face_attributes_without_id(GeometrySet &geometry_set, GeometryComponent &dst_component) { Map attributes = gather_attributes_without_id( - geometry_set, GEO_COMPONENT_TYPE_MESH, false); + geometry_set, GEO_COMPONENT_TYPE_MESH); for (const Map::Item entry : attributes.items()) { const AttributeIDRef attribute_id = entry.key; @@ -639,7 +632,7 @@ static void copy_edge_attributes_without_id(GeometrySet &geometry_set, GeometryComponent &dst_component) { Map attributes = gather_attributes_without_id( - geometry_set, GEO_COMPONENT_TYPE_MESH, false); + geometry_set, GEO_COMPONENT_TYPE_MESH); for (const Map::Item entry : attributes.items()) { const AttributeIDRef attribute_id = entry.key; @@ -840,7 +833,7 @@ static void duplicate_points_curve(GeometrySet &geometry_set, dst_component.replace(new_curves_id, GeometryOwnershipType::Editable); Map attributes = gather_attributes_without_id( - geometry_set, GEO_COMPONENT_TYPE_CURVE, false); + geometry_set, GEO_COMPONENT_TYPE_CURVE); for (const Map::Item entry : attributes.items()) { const AttributeIDRef attribute_id = entry.key; @@ -925,13 +918,13 @@ static void duplicate_points_mesh(GeometrySet &geometry_set, MeshComponent dst_component; dst_component.replace(new_mesh, GeometryOwnershipType::Editable); - copy_point_attributes_without_id(geometry_set, - GEO_COMPONENT_TYPE_MESH, - false, - offsets, - selection, - src_component, - dst_component); + copy_attributes_without_id(geometry_set, + GEO_COMPONENT_TYPE_MESH, + ATTR_DOMAIN_POINT, + offsets, + selection, + src_component, + dst_component); copy_stable_id_point(offsets, src_component, dst_component); @@ -972,13 +965,13 @@ static void duplicate_points_pointcloud(GeometrySet &geometry_set, PointCloudComponent dst_component; dst_component.replace(pointcloud, GeometryOwnershipType::Editable); - copy_point_attributes_without_id(geometry_set, - GEO_COMPONENT_TYPE_POINT_CLOUD, - false, - offsets, - selection, - src_points, - dst_component); + copy_attributes_without_id(geometry_set, + GEO_COMPONENT_TYPE_POINT_CLOUD, + ATTR_DOMAIN_POINT, + offsets, + selection, + src_points, + dst_component); copy_stable_id_point(offsets, src_points, dst_component); @@ -1076,13 +1069,13 @@ static void duplicate_instances(GeometrySet &geometry_set, dst_instances.instance_reference_handles().slice(range).fill(new_handle); } - copy_point_attributes_without_id(geometry_set, - GEO_COMPONENT_TYPE_INSTANCES, - true, - offsets, - selection, - src_instances, - dst_instances); + copy_attributes_without_id(geometry_set, + GEO_COMPONENT_TYPE_INSTANCES, + ATTR_DOMAIN_INSTANCE, + offsets, + selection, + src_instances, + dst_instances); if (attribute_outputs.duplicate_index) { create_duplicate_index_attribute(