griefith/test2
1
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

Fix: Geometry nodes reorders attributes in some cases

Browse Source 
Currently attribute names are often added to a hash map that doesn't
maintain the original order. This is convenient for developers but it's
better for users if attributes aren't arbitrarily reordered. Instead,
use `VectorSet` which gives the benefit of a hash map but maintains the
insertion order. The main downside is the loss of O(1) removal which we
benefited from in a few cases. I used string comparison instead for now.

Fixes #144491, #122994

Pull Request: https://projects.blender.org/blender/blender/pulls/145668
This commit is contained in:
Hans Goudey
2025-09-04 13:59:44 +02:00
committed by Hans Goudey
parent aee7943c06
commit 5418f0bc68
9 changed files with 137 additions and 117 deletions
Download Patch File Download Diff File Expand all files Collapse all files

17
source/blender/blenkernel/BKE_geometry_set.hh
View File

@@ -256,12 +256,17 @@ struct GeometrySet {
bool include_instances,
AttributeForeachCallback callback) const;
void gather_attributes_for_propagation(
Span<GeometryComponent::Type> component_types,
GeometryComponent::Type dst_component_type,
bool include_instances,
const AttributeFilter &attribute_filter,
Map<StringRef, AttributeDomainAndType> &r_attributes) const;
struct GatheredAttributes {
VectorSet<StringRef, 16> names;
Vector<AttributeDomainAndType, 16> kinds;
void add(const StringRef name, const AttributeDomainAndType &kind);
};
void gather_attributes_for_propagation(Span<GeometryComponent::Type> component_types,
GeometryComponent::Type dst_component_type,
bool include_instances,
const AttributeFilter &attribute_filter,
GatheredAttributes &r_attributes) const;
Vector<GeometryComponent::Type> gather_component_types(bool include_instances,
bool ignore_empty) const;

28
source/blender/blenkernel/intern/geometry_set.cc
View File

@@ -701,12 +701,26 @@ bool attribute_is_builtin_on_component_type(const GeometryComponent::Type type,
return false;
}
void GeometrySet::GatheredAttributes::add(const StringRef name, const AttributeDomainAndType &kind)
{
const int index = this->names.index_of_or_add(name);
if (index >= this->kinds.size()) {
this->kinds.append(AttributeDomainAndType{kind.domain, kind.data_type});
}
else {
this->kinds[index].domain = bke::attribute_domain_highest_priority(
{this->kinds[index].domain, kind.domain});
this->kinds[index].data_type = bke::attribute_data_type_highest_complexity(
{this->kinds[index].data_type, kind.data_type});
}
}
void GeometrySet::gather_attributes_for_propagation(
const Span<GeometryComponent::Type> component_types,
const GeometryComponent::Type dst_component_type,
bool include_instances,
const AttributeFilter &attribute_filter,
Map<StringRef, AttributeDomainAndType> &r_attributes) const
GatheredAttributes &r_attributes) const
{
this->attribute_foreach(
component_types,
@@ -735,17 +749,7 @@ void GeometrySet::gather_attributes_for_propagation(
domain = AttrDomain::Point;
}
auto add_info = [&](AttributeDomainAndType *attribute_kind) {
attribute_kind->domain = domain;
attribute_kind->data_type = meta_data.data_type;
};
auto modify_info = [&](AttributeDomainAndType *attribute_kind) {
attribute_kind->domain = bke::attribute_domain_highest_priority(
{attribute_kind->domain, domain});
attribute_kind->data_type = bke::attribute_data_type_highest_complexity(
{attribute_kind->data_type, meta_data.data_type});
};
r_attributes.add_or_modify(attribute_id, add_info, modify_info);
r_attributes.add(attribute_id, AttributeDomainAndType{domain, meta_data.data_type});
});
}

26
source/blender/geometry/intern/join_geometries.cc
View File

@@ -15,10 +15,10 @@ using bke::AttributeDomainAndType;
using bke::GeometryComponent;
using bke::GeometrySet;
static Map<StringRef, AttributeDomainAndType> get_final_attribute_info(
static GeometrySet::GatheredAttributes get_final_attribute_info(
const Span<const GeometryComponent *> components, const Span<StringRef> ignored_attributes)
{
Map<StringRef, AttributeDomainAndType> info;
GeometrySet::GatheredAttributes info;
for (const GeometryComponent *component : components) {
component->attributes()->foreach_attribute([&](const bke::AttributeIter &iter) {
@@ -28,17 +28,7 @@ static Map<StringRef, AttributeDomainAndType> get_final_attribute_info(
if (iter.data_type == bke::AttrType::String) {
return;
}
info.add_or_modify(
iter.name,
[&](AttributeDomainAndType *meta_data_final) {
*meta_data_final = {iter.domain, iter.data_type};
},
[&](AttributeDomainAndType *meta_data_final) {
meta_data_final->data_type = bke::attribute_data_type_highest_complexity(
{meta_data_final->data_type, iter.data_type});
meta_data_final->domain = bke::attribute_domain_highest_priority(
{meta_data_final->domain, iter.domain});
});
info.add(iter.name, AttributeDomainAndType{iter.domain, iter.data_type});
});
}
@@ -75,12 +65,12 @@ void join_attributes(const Span<const GeometryComponent *> src_components,
GeometryComponent &result,
const Span<StringRef> ignored_attributes)
{
const Map<StringRef, AttributeDomainAndType> info = get_final_attribute_info(src_components,
ignored_attributes);
const GeometrySet::GatheredAttributes info = get_final_attribute_info(src_components,
ignored_attributes);
for (const MapItem<StringRef, AttributeDomainAndType> item : info.items()) {
const StringRef attribute_id = item.key;
const AttributeDomainAndType &meta_data = item.value;
for (const int i : info.names.index_range()) {
const StringRef attribute_id = info.names[i];
const AttributeDomainAndType &meta_data = info.kinds[i];
bke::GSpanAttributeWriter write_attribute =
result.attributes_for_write()->lookup_or_add_for_write_only_span(

116
source/blender/geometry/intern/realize_instances.cc
View File

@@ -911,7 +911,7 @@ static void gather_attribute_propagation_components_with_custom_depths(
}
}
static Map<StringRef, AttributeDomainAndType> gather_attributes_to_propagate(
static bke::GeometrySet::GatheredAttributes gather_attributes_to_propagate(
const bke::GeometrySet &geometry,
const bke::GeometryComponent::Type component_type,
const RealizeInstancesOptions &options,
@@ -941,7 +941,7 @@ static Map<StringRef, AttributeDomainAndType> gather_attributes_to_propagate(
}
/* Actually gather the attributes to propagate from the found components. */
Map<StringRef, AttributeDomainAndType> attributes_to_propagate;
bke::GeometrySet::GatheredAttributes attributes_to_propagate;
for (const bke::GeometryComponentPtr &component : components) {
const bke::AttributeAccessor attributes = *component->attributes();
attributes.foreach_attribute([&](const bke::AttributeIter &iter) {
@@ -980,16 +980,7 @@ static Map<StringRef, AttributeDomainAndType> gather_attributes_to_propagate(
dst_domain = AttrDomain::Point;
}
}
auto add = [&](AttributeDomainAndType *kind) {
kind->domain = dst_domain;
kind->data_type = iter.data_type;
};
auto modify = [&](AttributeDomainAndType *kind) {
kind->domain = bke::attribute_domain_highest_priority({kind->domain, dst_domain});
kind->data_type = bke::attribute_data_type_highest_complexity(
{kind->data_type, iter.data_type});
};
attributes_to_propagate.add_or_modify(iter.name, add, modify);
attributes_to_propagate.add(iter.name, AttributeDomainAndType{dst_domain, iter.data_type});
});
}
@@ -1007,13 +998,15 @@ static OrderedAttributes gather_generic_instance_attributes_to_propagate(
const RealizeInstancesOptions &options,
const VariedDepthOptions &varied_depth_option)
{
Map<StringRef, AttributeDomainAndType> attributes_to_propagate = gather_attributes_to_propagate(
bke::GeometrySet::GatheredAttributes attributes_to_propagate = gather_attributes_to_propagate(
in_geometry_set, bke::GeometryComponent::Type::Instance, options, varied_depth_option);
attributes_to_propagate.pop_try("id");
OrderedAttributes ordered_attributes;
for (const auto item : attributes_to_propagate.items()) {
ordered_attributes.ids.add_new(item.key);
ordered_attributes.kinds.append(item.value);
for (const int i : attributes_to_propagate.names.index_range()) {
if (attributes_to_propagate.names[i] == "id") {
continue;
}
ordered_attributes.ids.add_new(attributes_to_propagate.names[i]);
ordered_attributes.kinds.append(attributes_to_propagate.kinds[i]);
}
return ordered_attributes;
}
@@ -1123,15 +1116,23 @@ static OrderedAttributes gather_generic_pointcloud_attributes_to_propagate(
bool &r_create_radii,
bool &r_create_id)
{
Map<StringRef, AttributeDomainAndType> attributes_to_propagate = gather_attributes_to_propagate(
bke::GeometrySet::GatheredAttributes attributes_to_propagate = gather_attributes_to_propagate(
in_geometry_set, bke::GeometryComponent::Type::PointCloud, options, varied_depth_option);
attributes_to_propagate.remove("position");
r_create_id = attributes_to_propagate.pop_try("id").has_value();
r_create_radii = attributes_to_propagate.pop_try("radius").has_value();
OrderedAttributes ordered_attributes;
for (const auto item : attributes_to_propagate.items()) {
ordered_attributes.ids.add_new(item.key);
ordered_attributes.kinds.append(item.value);
for (const int i : attributes_to_propagate.names.index_range()) {
if (attributes_to_propagate.names[i] == "position") {
continue;
}
if (attributes_to_propagate.names[i] == "id") {
r_create_id = true;
continue;
}
if (attributes_to_propagate.names[i] == "radius") {
r_create_radii = true;
continue;
}
ordered_attributes.ids.add_new(attributes_to_propagate.names[i]);
ordered_attributes.kinds.append(attributes_to_propagate.kinds[i]);
}
return ordered_attributes;
}
@@ -1352,19 +1353,29 @@ static OrderedAttributes gather_generic_mesh_attributes_to_propagate(
bool &r_create_id,
bool &r_create_material_index)
{
Map<StringRef, AttributeDomainAndType> attributes_to_propagate = gather_attributes_to_propagate(
bke::GeometrySet::GatheredAttributes attributes_to_propagate = gather_attributes_to_propagate(
in_geometry_set, bke::GeometryComponent::Type::Mesh, options, varied_depth_option);
attributes_to_propagate.remove("position");
attributes_to_propagate.remove(".edge_verts");
attributes_to_propagate.remove(".corner_vert");
attributes_to_propagate.remove(".corner_edge");
attributes_to_propagate.remove("custom_normal");
r_create_id = attributes_to_propagate.pop_try("id").has_value();
r_create_material_index = attributes_to_propagate.pop_try("material_index").has_value();
OrderedAttributes ordered_attributes;
for (const auto item : attributes_to_propagate.items()) {
ordered_attributes.ids.add_new(item.key);
ordered_attributes.kinds.append(item.value);
for (const int i : attributes_to_propagate.names.index_range()) {
if (ELEM(attributes_to_propagate.names[i],
"position",
".edge_verts",
".corner_vert",
".corner_edge",
"custom_normal"))
{
continue;
}
if (attributes_to_propagate.names[i] == "id") {
r_create_id = true;
continue;
}
if (attributes_to_propagate.names[i] == "material_index") {
r_create_material_index = true;
continue;
}
ordered_attributes.ids.add_new(attributes_to_propagate.names[i]);
ordered_attributes.kinds.append(attributes_to_propagate.kinds[i]);
}
return ordered_attributes;
}
@@ -1836,18 +1847,25 @@ static OrderedAttributes gather_generic_curve_attributes_to_propagate(
const VariedDepthOptions &varied_depth_option,
bool &r_create_id)
{
Map<StringRef, AttributeDomainAndType> attributes_to_propagate = gather_attributes_to_propagate(
bke::GeometrySet::GatheredAttributes attributes_to_propagate = gather_attributes_to_propagate(
in_geometry_set, bke::GeometryComponent::Type::Curve, options, varied_depth_option);
attributes_to_propagate.remove("position");
attributes_to_propagate.remove("radius");
attributes_to_propagate.remove("handle_right");
attributes_to_propagate.remove("handle_left");
attributes_to_propagate.remove("custom_normal");
r_create_id = attributes_to_propagate.pop_try("id").has_value();
OrderedAttributes ordered_attributes;
for (const auto item : attributes_to_propagate.items()) {
ordered_attributes.ids.add_new(item.key);
ordered_attributes.kinds.append(item.value);
for (const int i : attributes_to_propagate.names.index_range()) {
if (ELEM(attributes_to_propagate.names[i],
"position",
"radius",
"handle_left",
"handle_right",
"custom_normal"))
{
continue;
}
if (attributes_to_propagate.names[i] == "id") {
r_create_id = true;
continue;
}
ordered_attributes.ids.add_new(attributes_to_propagate.names[i]);
ordered_attributes.kinds.append(attributes_to_propagate.kinds[i]);
}
return ordered_attributes;
}
@@ -2191,12 +2209,12 @@ static OrderedAttributes gather_generic_grease_pencil_attributes_to_propagate(
const RealizeInstancesOptions &options,
const VariedDepthOptions &varied_depth_options)
{
Map<StringRef, AttributeDomainAndType> attributes_to_propagate = gather_attributes_to_propagate(
bke::GeometrySet::GatheredAttributes attributes_to_propagate = gather_attributes_to_propagate(
in_geometry_set, bke::GeometryComponent::Type::GreasePencil, options, varied_depth_options);
OrderedAttributes ordered_attributes;
for (auto &&item : attributes_to_propagate.items()) {
ordered_attributes.ids.add_new(item.key);
ordered_attributes.kinds.append(item.value);
for (const int i : attributes_to_propagate.names.index_range()) {
ordered_attributes.ids.add_new(attributes_to_propagate.names[i]);
ordered_attributes.kinds.append(attributes_to_propagate.kinds[i]);
}
return ordered_attributes;
}

16
source/blender/nodes/geometry/nodes/node_geo_distribute_points_on_faces.cc
View File

@@ -290,7 +290,7 @@ BLI_NOINLINE static void interpolate_attribute(const Mesh &mesh,
BLI_NOINLINE static void propagate_existing_attributes(
const Mesh &mesh,
const Map<StringRef, AttributeDomainAndType> &attributes,
const GeometrySet::GatheredAttributes &attributes,
PointCloud &points,
const Span<float3> bary_coords,
const Span<int> tri_indices)
@@ -298,9 +298,12 @@ BLI_NOINLINE static void propagate_existing_attributes(
const AttributeAccessor mesh_attributes = mesh.attributes();
MutableAttributeAccessor point_attributes = points.attributes_for_write();
for (MapItem<StringRef, AttributeDomainAndType> entry : attributes.items()) {
const StringRef attribute_id = entry.key;
const bke::AttrType output_data_type = entry.value.data_type;
for (const int i : attributes.names.index_range()) {
const StringRef attribute_id = attributes.names[i];
const bke::AttrType output_data_type = attributes.kinds[i].data_type;
if (attribute_id == "position") {
continue;
}
GAttributeReader src = mesh_attributes.lookup(attribute_id);
if (!src) {
@@ -559,16 +562,13 @@ static void point_distribution_calculate(GeometrySet &geometry_set,
geometry_set.replace_pointcloud(pointcloud);
Map<StringRef, AttributeDomainAndType> attributes;
GeometrySet::GatheredAttributes attributes;
geometry_set.gather_attributes_for_propagation({GeometryComponent::Type::Mesh},
GeometryComponent::Type::PointCloud,
false,
params.get_attribute_filter("Points"),
attributes);
/* Position is set separately. */
attributes.remove("position");
propagate_existing_attributes(mesh, attributes, *pointcloud, bary_coords, tri_indices);
const bool use_legacy_normal = params.node().custom2 != 0;

15
source/blender/nodes/geometry/nodes/node_geo_instance_on_points.cc
View File

@@ -49,7 +49,7 @@ static void add_instances_from_component(
const GeometrySet &instance,
const fn::FieldContext &field_context,
const GeoNodeExecParams &params,
const Map<StringRef, AttributeDomainAndType> &attributes_to_propagate)
const bke::GeometrySet::GatheredAttributes &attributes_to_propagate)
{
const AttrDomain domain = AttrDomain::Point;
const int domain_num = src_attributes.domain_size(domain);
@@ -154,9 +154,12 @@ static void add_instances_from_component(
}
bke::MutableAttributeAccessor dst_attributes = dst_component.attributes_for_write();
for (const auto item : attributes_to_propagate.items()) {
const StringRef id = item.key;
const bke::AttrType data_type = item.value.data_type;
for (const int i : attributes_to_propagate.names.index_range()) {
if (ELEM(attributes_to_propagate.names[i], "position", ".reference_index")) {
continue;
}
const StringRef id = attributes_to_propagate.names[i];
const bke::AttrType data_type = attributes_to_propagate.kinds[i].data_type;
const bke::GAttributeReader src = src_attributes.lookup(id, AttrDomain::Point, data_type);
if (!src) {
/* Domain interpolation can fail if the source domain is empty. */
@@ -195,14 +198,12 @@ static void node_geo_exec(GeoNodeExecParams params)
GeometryComponent::Type::PointCloud,
GeometryComponent::Type::Curve};
Map<StringRef, AttributeDomainAndType> attributes_to_propagate;
bke::GeometrySet::GatheredAttributes attributes_to_propagate;
geometry_set.gather_attributes_for_propagation(types,
GeometryComponent::Type::Instance,
false,
attribute_filter,
attributes_to_propagate);
attributes_to_propagate.remove("position");
attributes_to_propagate.remove(".reference_index");
for (const GeometryComponent::Type type : types) {
if (geometry_set.has(type)) {

15
source/blender/nodes/geometry/nodes/node_geo_instances_to_points.cc
View File

@@ -60,19 +60,20 @@ static void convert_instances_to_points(GeometrySet &geometry_set,
point_radii.finish();
const bke::AttributeAccessor src_attributes = instances.attributes();
Map<StringRef, AttributeDomainAndType> attributes_to_propagate;
bke::GeometrySet::GatheredAttributes attributes_to_propagate;
geometry_set.gather_attributes_for_propagation({GeometryComponent::Type::Instance},
GeometryComponent::Type::PointCloud,
false,
attribute_filter,
attributes_to_propagate);
/* These two attributes are added by the implicit inputs above. */
attributes_to_propagate.remove("position");
attributes_to_propagate.remove("radius");
for (const auto item : attributes_to_propagate.items()) {
const StringRef id = item.key;
const bke::AttrType type = item.value.data_type;
for (const int i : attributes_to_propagate.names.index_range()) {
/* These two attributes are added by the implicit inputs above. */
if (ELEM(attributes_to_propagate.names[i], "position", "radius")) {
continue;
}
const StringRef id = attributes_to_propagate.names[i];
const bke::AttrType type = attributes_to_propagate.kinds[i].data_type;
const GAttributeReader src = src_attributes.lookup(id);
if (selection.size() == instances.instances_num() && src.sharing_info && src.varray.is_span())

13
source/blender/nodes/geometry/nodes/node_geo_mesh_to_points.cc
View File

@@ -108,18 +108,19 @@ static void geometry_set_mesh_to_points(GeometrySet &geometry_set,
array_utils::gather(evaluator.get_evaluated(1), selection, radius.span);
radius.finish();
Map<StringRef, AttributeDomainAndType> attributes;
bke::GeometrySet::GatheredAttributes attributes;
geometry_set.gather_attributes_for_propagation({GeometryComponent::Type::Mesh},
GeometryComponent::Type::PointCloud,
false,
attribute_filter,
attributes);
attributes.remove("radius");
attributes.remove("position");
for (MapItem<StringRef, AttributeDomainAndType> entry : attributes.items()) {
const StringRef attribute_id = entry.key;
const bke::AttrType data_type = entry.value.data_type;
for (const int i : attributes.names.index_range()) {
if (ELEM(attributes.names[i], "position", "radius")) {
continue;
}
const StringRef attribute_id = attributes.names[i];
const bke::AttrType data_type = attributes.kinds[i].data_type;
const bke::GAttributeReader src = src_attributes.lookup(attribute_id, domain, data_type);
if (!src) {
/* Domain interpolation can fail if the source domain is empty. */

8
source/blender/nodes/geometry/nodes/node_geo_points_to_vertices.cc
View File

@@ -45,7 +45,7 @@ static void geometry_set_points_to_vertices(GeometrySet &geometry_set,
selection_evaluator.evaluate();
const IndexMask selection = selection_evaluator.get_evaluated_as_mask(0);
Map<StringRef, AttributeDomainAndType> attributes;
bke::GeometrySet::GatheredAttributes attributes;
geometry_set.gather_attributes_for_propagation({GeometryComponent::Type::PointCloud},
GeometryComponent::Type::Mesh,
false,
@@ -66,9 +66,9 @@ static void geometry_set_points_to_vertices(GeometrySet &geometry_set,
const AttributeAccessor src_attributes = points->attributes();
MutableAttributeAccessor dst_attributes = mesh->attributes_for_write();
for (MapItem<StringRef, AttributeDomainAndType> entry : attributes.items()) {
const StringRef id = entry.key;
const bke::AttrType data_type = entry.value.data_type;
for (const int i : attributes.names.index_range()) {
const StringRef id = attributes.names[i];
const bke::AttrType data_type = attributes.kinds[i].data_type;
const GAttributeReader src = src_attributes.lookup(id);
if (selection.size() == points->totpoint && src.sharing_info && src.varray.is_span()) {
const bke::AttributeInitShared init(src.varray.get_internal_span().data(),