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

Attributes: make id attribute generic instead of built-in

Browse Source 
The "id" attribute was built-in on curves (point domain), mesh (point domain)
and instances, but not on e.g. point clouds. This mismatch causes issues because
of the special rules regarding built-in attribute propagation. Furthermore, an
implication of this was that the id attribute had to be an integer attribute on
a specific domain, which is not always ideal.

This patch turns the id attribute into a normal generic attribute, except that
some nodes internally modify this attribute in special ways. This may cause some
compatibility breakage in rare cases, but that can generally be easily fixed by
either removing the id attribute or setting it explicitly on the right domain. I
can't think if a feasible way to avoid this unfortunately.

The internal special cases for the id attributes are generally skipped unless
the attribute is an integer attribute on the point/instance domain.

Pull Request: https://projects.blender.org/blender/blender/pulls/146941
This commit is contained in:
Jacques Lucke
2025-09-29 18:47:51 +02:00
parent 1ffa0c4648
commit ae2034e6c5
8 changed files with 82 additions and 54 deletions
Download Patch File Download Diff File Expand all files Collapse all files

3
source/blender/blenkernel/intern/curves_attributes.cc
View File

@@ -189,9 +189,6 @@ static const auto &builtin_attributes()
AttrBuiltinInfo radius(AttrDomain::Point, AttrType::Float);
map.add_new("radius", std::move(radius));
AttrBuiltinInfo id(AttrDomain::Point, AttrType::Int32);
map.add_new("id", std::move(id));
AttrBuiltinInfo tilt(AttrDomain::Point, AttrType::Float);
map.add_new("tilt", std::move(tilt));

9
source/blender/blenkernel/intern/instances_attributes.cc
View File

@@ -28,15 +28,6 @@ static const auto &builtin_attributes()
static auto attributes = []() {
Map<StringRef, AttrBuiltinInfo> map;
/**
* IDs of the instances. They are used for consistency over multiple frames for things like
* motion blur. Proper stable ID data that actually helps when rendering can only be generated
* in some situations, so this vector is allowed to be empty, in which case the index of each
* instance will be used for the final ID.
*/
AttrBuiltinInfo id(bke::AttrDomain::Instance, bke::AttrType::Int32);
map.add_new("id", std::move(id));
AttrBuiltinInfo instance_transform(bke::AttrDomain::Instance, bke::AttrType::Float4x4);
instance_transform.deletable = false;
map.add_new("instance_transform", std::move(instance_transform));

8
source/blender/blenkernel/intern/mesh_attributes.cc
View File

@@ -878,13 +878,6 @@ static GeometryAttributeProviders create_attribute_providers_for_mesh()
point_access,
tag_component_positions_changed);
static BuiltinCustomDataLayerProvider id("id",
AttrDomain::Point,
CD_PROP_INT32,
BuiltinAttributeProvider::Deletable,
point_access,
nullptr);
static const auto material_index_clamp = mf::build::SI1_SO<int, int>(
"Material Index Validate",
[](int value) {
@@ -957,7 +950,6 @@ static GeometryAttributeProviders create_attribute_providers_for_mesh()
&edge_verts,
&corner_vert,
&corner_edge,
&id,
&material_index,
&sharp_face,
&sharp_edge},

30
source/blender/geometry/intern/mix_geometries.cc
View File

@@ -125,19 +125,26 @@ static Map<int, int> create_value_to_first_index_map(const Span<int> values)
}
static Array<int> create_id_index_map(const bke::AttributeAccessor attributes_a,
const bke::AttributeAccessor b_attributes)
const bke::AttributeAccessor b_attributes,
const bke::AttrDomain id_domain)
{
const bke::AttributeReader<int> ids_a = attributes_a.lookup<int>("id");
const bke::AttributeReader<int> ids_b = b_attributes.lookup<int>("id");
const bke::GAttributeReader ids_a = attributes_a.lookup("id");
const bke::GAttributeReader ids_b = b_attributes.lookup("id");
if (!ids_a || !ids_b) {
return {};
}
if (!ids_a.varray.type().is<int>() || !ids_b.varray.type().is<int>()) {
return {};
}
if (ids_a.domain != id_domain || ids_b.domain != id_domain) {
return {};
}
if (sharing_info_equal(ids_a.sharing_info, ids_b.sharing_info)) {
return {};
}
const VArraySpan ids_span_a(*ids_a);
const VArraySpan ids_span_b(*ids_b);
const VArraySpan ids_span_a(ids_a.varray.typed<int>());
const VArraySpan ids_span_b(ids_b.varray.typed<int>());
const Map<int, int> id_map_b = create_value_to_first_index_map(ids_span_b);
Array<int> index_map(ids_span_a.size());
@@ -153,7 +160,8 @@ bke::GeometrySet mix_geometries(bke::GeometrySet a, const bke::GeometrySet &b, c
{
if (Mesh *mesh_a = a.get_mesh_for_write()) {
if (const Mesh *mesh_b = b.get_mesh()) {
Array<int> vert_map = create_id_index_map(mesh_a->attributes(), mesh_b->attributes());
Array<int> vert_map = create_id_index_map(
mesh_a->attributes(), mesh_b->attributes(), bke::AttrDomain::Point);
mix_attributes(mesh_a->attributes_for_write(),
mesh_b->attributes(),
vert_map,
@@ -164,8 +172,8 @@ bke::GeometrySet mix_geometries(bke::GeometrySet a, const bke::GeometrySet &b, c
}
if (PointCloud *points_a = a.get_pointcloud_for_write()) {
if (const PointCloud *points_b = b.get_pointcloud()) {
const Array<int> index_map = create_id_index_map(points_a->attributes(),
points_b->attributes());
const Array<int> index_map = create_id_index_map(
points_a->attributes(), points_b->attributes(), bke::AttrDomain::Point);
mix_attributes(points_a->attributes_for_write(),
points_b->attributes(),
index_map,
@@ -177,7 +185,7 @@ bke::GeometrySet mix_geometries(bke::GeometrySet a, const bke::GeometrySet &b, c
if (const Curves *curves_b = b.get_curves()) {
bke::MutableAttributeAccessor a = curves_a->geometry.wrap().attributes_for_write();
const bke::AttributeAccessor b = curves_b->geometry.wrap().attributes();
const Array<int> index_map = create_id_index_map(a, b);
const Array<int> index_map = create_id_index_map(a, b, bke::AttrDomain::Point);
mix_attributes(
a,
b,
@@ -189,8 +197,8 @@ bke::GeometrySet mix_geometries(bke::GeometrySet a, const bke::GeometrySet &b, c
}
if (bke::Instances *instances_a = a.get_instances_for_write()) {
if (const bke::Instances *instances_b = b.get_instances()) {
const Array<int> index_map = create_id_index_map(instances_a->attributes(),
instances_b->attributes());
const Array<int> index_map = create_id_index_map(
instances_a->attributes(), instances_b->attributes(), bke::AttrDomain::Instance);
mix_attributes(instances_a->attributes_for_write(),
instances_b->attributes(),
index_map,

7
source/blender/geometry/intern/point_merge_by_distance.cc
View File

@@ -106,8 +106,11 @@ PointCloud *point_merge_by_distance(const PointCloud &src_points,
Set<StringRefNull> attribute_ids = src_attributes.all_ids();
/* Transfer the ID attribute if it exists, using the ID of the first merged point. */
if (attribute_ids.contains("id")) {
VArraySpan<int> src = *src_attributes.lookup_or_default<int>("id", bke::AttrDomain::Point, 0);
bke::GAttributeReader src_id_attribute = src_attributes.lookup("id");
if (src_id_attribute && src_id_attribute.domain == bke::AttrDomain::Point &&
src_id_attribute.varray.type().is<int>())
{
VArraySpan<int> src = src_id_attribute.varray.typed<int>();
bke::SpanAttributeWriter<int> dst = dst_attributes.lookup_or_add_for_write_only_span<int>(
"id", bke::AttrDomain::Point);

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

@@ -610,9 +610,11 @@ static void gather_realize_tasks_for_instances(GatherTasksInfo &gather_info,
Span<int> stored_instance_ids;
if (gather_info.create_id_attribute_on_any_component) {
bke::AttributeReader ids = instances.attributes().lookup<int>("id");
if (ids) {
stored_instance_ids = ids.varray.get_internal_span();
bke::GAttributeReader ids = instances.attributes().lookup("id");
if (ids && ids.domain == bke::AttrDomain::Instance && ids.varray.type().is<int>() &&
ids.varray.is_span())
{
stored_instance_ids = ids.varray.get_internal_span().typed<int>();
}
}
@@ -1184,7 +1186,9 @@ static AllPointCloudsInfo preprocess_pointclouds(const bke::GeometrySet &geometr
}
if (info.create_id_attribute) {
bke::GAttributeReader ids_attribute = attributes.lookup("id");
if (ids_attribute) {
if (ids_attribute && ids_attribute.domain == bke::AttrDomain::Point &&
ids_attribute.varray.type().is<int>() && ids_attribute.varray.is_span())
{
pointcloud_info.stored_ids = ids_attribute.varray.get_internal_span().typed<int>();
}
}
@@ -1464,7 +1468,9 @@ static AllMeshesInfo preprocess_meshes(const bke::GeometrySet &geometry_set,
}
if (info.create_id_attribute) {
bke::GAttributeReader ids_attribute = attributes.lookup("id");
if (ids_attribute) {
if (ids_attribute && ids_attribute.domain == bke::AttrDomain::Point &&
ids_attribute.varray.type().is<int>() && ids_attribute.varray.is_span())
{
mesh_info.stored_vertex_ids = ids_attribute.varray.get_internal_span().typed<int>();
}
}
@@ -1915,7 +1921,9 @@ static AllCurvesInfo preprocess_curves(const bke::GeometrySet &geometry_set,
}
if (info.create_id_attribute) {
bke::GAttributeReader id_attribute = attributes.lookup("id");
if (id_attribute) {
if (id_attribute && id_attribute.domain == bke::AttrDomain::Point &&
id_attribute.varray.type().is<int>() && id_attribute.varray.is_span())
{
curve_info.stored_ids = id_attribute.varray.get_internal_span().typed<int>();
}
}

25
source/blender/nodes/geometry/nodes/node_geo_duplicate_elements.cc
View File

@@ -138,6 +138,12 @@ static void copy_stable_id_point(const OffsetIndices<int> offsets,
if (!src_attribute) {
return;
}
if (!ELEM(src_attribute.domain, AttrDomain::Point, AttrDomain::Instance)) {
return;
}
if (!src_attribute.varray.type().is<int>()) {
return;
}
SpanAttributeWriter dst_attribute = dst_attributes.lookup_or_add_for_write_only_span<int>(
"id", AttrDomain::Point);
if (!dst_attribute) {
@@ -217,6 +223,13 @@ static void copy_stable_id_curves(const bke::CurvesGeometry &src_curves,
if (!src_attribute) {
return;
}
if (src_attribute.domain != AttrDomain::Point) {
return;
}
if (!src_attribute.varray.type().is<int>()) {
return;
}
SpanAttributeWriter dst_attribute =
dst_curves.attributes_for_write().lookup_or_add_for_write_only_span<int>("id",
AttrDomain::Point);
@@ -426,6 +439,12 @@ static void copy_stable_id_faces(const Mesh &mesh,
if (!src_attribute) {
return;
}
if (src_attribute.domain != AttrDomain::Point) {
return;
}
if (!src_attribute.varray.type().is<int>()) {
return;
}
SpanAttributeWriter dst_attribute = dst_attributes.lookup_or_add_for_write_only_span<int>(
"id", AttrDomain::Point);
if (!dst_attribute) {
@@ -617,6 +636,12 @@ static void copy_stable_id_edges(const Mesh &mesh,
if (!src_attribute) {
return;
}
if (src_attribute.domain != AttrDomain::Point) {
return;
}
if (!src_attribute.varray.type().is<int>()) {
return;
}
SpanAttributeWriter dst_attribute = dst_attributes.lookup_or_add_for_write_only_span<int>(
"id", AttrDomain::Point);
if (!dst_attribute) {

34
source/blender/nodes/geometry/nodes/node_geo_set_id.cc
View File

@@ -36,22 +36,26 @@ static void set_id_in_component(GeometryComponent &component,
/* Since adding the ID attribute can change the result of the field evaluation (the random value
* node uses the index if the ID is unavailable), make sure that it isn't added before evaluating
* the field. However, as an optimization, use a faster code path when it already exists. */
if (attributes.contains("id")) {
AttributeWriter<int> id_attribute = attributes.lookup_or_add_for_write<int>("id", domain);
evaluator.add_with_destination(id_field, id_attribute.varray);
evaluator.evaluate();
id_attribute.finish();
}
else {
evaluator.add(id_field);
evaluator.evaluate();
const IndexMask selection = evaluator.get_evaluated_selection_as_mask();
const VArray<int> result_ids = evaluator.get_evaluated<int>(0);
SpanAttributeWriter<int> id_attribute = attributes.lookup_or_add_for_write_span<int>("id",
domain);
result_ids.materialize(selection, id_attribute.span);
id_attribute.finish();
if (const std::optional<AttributeMetaData> meta_data = attributes.lookup_meta_data("id")) {
if (meta_data->domain == domain && meta_data->data_type == bke::AttrType::Int32) {
AttributeWriter<int> id_attribute = attributes.lookup_or_add_for_write<int>("id", domain);
evaluator.add_with_destination(id_field, id_attribute.varray);
evaluator.evaluate();
id_attribute.finish();
return;
}
/* There is an ID attribute, but it has the wrong type, so remove it so that it can be
* recreated below. */
attributes.remove("id");
}
evaluator.add(id_field);
evaluator.evaluate();
const IndexMask selection = evaluator.get_evaluated_selection_as_mask();
const VArray<int> result_ids = evaluator.get_evaluated<int>(0);
SpanAttributeWriter<int> id_attribute = attributes.lookup_or_add_for_write_span<int>("id",
domain);
result_ids.materialize(selection, id_attribute.span);
id_attribute.finish();
}
static void node_geo_exec(GeoNodeExecParams params)