blender
/
blender
136
406
Fork 589
Code Issues 6.3k Pull Requests 766 Projects 19 Wiki Activity

Geometry Nodes: Avoid copying attributes before reordering #117910

Open
Iliya Katushenock wants to merge 13 commits from mod_moder/blender:tmp_sort_elements_del_att into main
pull from: mod_moder/blender:tmp_sort_elements_del_att
merge into: blender:main

When changing the target branch, be careful to rebase the branch in your fork to match. See documentation.
Conversation 5 Commits 13 Files Changed 5 +161 -94
5 changed files with 161 additions and 94 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
source/blender/blenkernel/BKE_curves.hh
View File

@ -853,6 +853,8 @@ CurvesGeometry curves_copy_curve_selection(
const IndexMask &curves_to_copy,
const AnonymousAttributePropagationInfo &propagation_info);
CurvesGeometry curves_new_no_attributes(int point_num, int curve_num);
std::array<int, CURVE_TYPES_NUM> calculate_type_counts(const VArray<int8_t> &types);
/* -------------------------------------------------------------------- */

2
source/blender/blenkernel/BKE_pointcloud.hh
View File

@ -46,6 +46,8 @@ struct PointCloudRuntime {
MEM_CXX_CLASS_ALLOC_FUNCS("PointCloudRuntime");
};
PointCloud *pointcloud_new_no_attributes(int totpoint);
} // namespace blender::bke
void *BKE_pointcloud_add(Main *bmain, const char *name);

10
source/blender/blenkernel/intern/curves_geometry.cc
View File

@ -1403,6 +1403,16 @@ void CurvesGeometry::remove_attributes_based_on_types()
}
}
CurvesGeometry curves_new_no_attributes(int point_num, int curve_num)
{
CurvesGeometry curves(0, curve_num);
curves.point_num = point_num;
CustomData_free_layer_named(&curves.point_data, "position", 0);
return curves;
}
/** \} */
/* -------------------------------------------------------------------- */

12
source/blender/blenkernel/intern/pointcloud.cc
View File

@ -398,3 +398,15 @@ void BKE_pointcloud_batch_cache_free(PointCloud *pointcloud)
BKE_pointcloud_batch_cache_free_cb(pointcloud);
}
}
namespace blender::bke {
PointCloud *pointcloud_new_no_attributes(int totpoint)
{
PointCloud *pointcloud = BKE_pointcloud_new_nomain(0);
pointcloud->totpoint = totpoint;
CustomData_free_layer_named(&pointcloud->pdata, "position", 0);
return pointcloud;
}
} // namespace blender::bke

229
source/blender/geometry/intern/reorder.cc
View File

@ -14,6 +14,7 @@
#include "BLI_array.hh"
#include "BLI_array_utils.hh"
#include "BLI_multi_value_map.hh"
#include "BLI_string_ref.hh"
#include "DNA_curves_types.h"
#include "DNA_mesh_types.h"
@ -41,15 +42,20 @@ components_supported_reordering()
return supported_types_and_domains;
}
static void reorder_attributes_group_to_group(const bke::AttributeAccessor src_attributes,
const bke::AttrDomain domain,
const OffsetIndices<int> src_offsets,
const OffsetIndices<int> dst_offsets,
const Span<int> old_by_new_map,
bke::MutableAttributeAccessor dst_attributes)
static void reorder_attributes_group_to_group(
const bke::AttributeAccessor src_attributes,
const bke::AttrDomain domain,
const bke::AnonymousAttributePropagationInfo &propagation_info,
const OffsetIndices<int> src_offsets,
const OffsetIndices<int> dst_offsets,
const Span<int> old_by_new_map,
bke::MutableAttributeAccessor dst_attributes)
{
src_attributes.for_all(
[&](const bke::AttributeIDRef &id, const bke::AttributeMetaData meta_data) {
if (id.is_anonymous() && !propagation_info.propagate(id.anonymous_id())) {
return true;
}
if (meta_data.domain != domain) {
return true;
}
@ -86,16 +92,34 @@ static Array<int> invert_permutation(const Span<int> permutation)
return data;
}
static void reorder_mesh_verts_exec(const Mesh &src_mesh,
const Span<int> old_by_new_map,
Mesh &dst_mesh)
static void copy_and_reorder_mesh_verts(
const Mesh &src_mesh,
const Span<int> old_by_new_map,
const bke::AnonymousAttributePropagationInfo &propagation_info,
Mesh &dst_mesh)
{
bke::gather_attributes(src_mesh.attributes(),
const bke::AttributeAccessor src_attributes = src_mesh.attributes();
bke::MutableAttributeAccessor dst_attributes = dst_mesh.attributes_for_write();
bke::copy_attributes(
src_attributes, bke::AttrDomain::Edge, propagation_info, {}, dst_attributes);
bke::copy_attributes(
src_attributes, bke::AttrDomain::Face, propagation_info, {}, dst_attributes);
implicit_sharing::free_shared_data(&dst_mesh.face_offset_indices,
&dst_mesh.runtime->face_offsets_sharing_info);
implicit_sharing::copy_shared_pointer(src_mesh.face_offset_indices,
src_mesh.runtime->face_offsets_sharing_info,
&dst_mesh.face_offset_indices,
&dst_mesh.runtime->face_offsets_sharing_info);
bke::copy_attributes(
src_attributes, bke::AttrDomain::Corner, propagation_info, {}, dst_attributes);
bke::gather_attributes(src_attributes,
bke::AttrDomain::Point,
{},
propagation_info,
{},
old_by_new_map,
dst_mesh.attributes_for_write());
dst_attributes);
const Array<int> new_by_old_map = invert_permutation(old_by_new_map);
array_utils::gather(new_by_old_map.as_span(),
dst_mesh.edges().cast<int>(),
@ -104,57 +128,79 @@ static void reorder_mesh_verts_exec(const Mesh &src_mesh,
new_by_old_map.as_span(), dst_mesh.corner_verts(), dst_mesh.corner_verts_for_write());
}
static void reorder_mesh_edges_exec(const Mesh &src_mesh,
const Span<int> old_by_new_map,
Mesh &dst_mesh)
static void copy_and_reorder_mesh_edges(
const Mesh &src_mesh,
const Span<int> old_by_new_map,
const bke::AnonymousAttributePropagationInfo &propagation_info,
Mesh &dst_mesh)
{
bke::gather_attributes(src_mesh.attributes(),
bke::AttrDomain::Edge,
{},
{},
old_by_new_map,
dst_mesh.attributes_for_write());
const bke::AttributeAccessor src_attributes = src_mesh.attributes();
bke::MutableAttributeAccessor dst_attributes = dst_mesh.attributes_for_write();
bke::copy_attributes(
src_attributes, bke::AttrDomain::Point, propagation_info, {}, dst_attributes);
bke::copy_attributes(
src_attributes, bke::AttrDomain::Face, propagation_info, {}, dst_attributes);
implicit_sharing::free_shared_data(&dst_mesh.face_offset_indices,
&dst_mesh.runtime->face_offsets_sharing_info);
implicit_sharing::copy_shared_pointer(src_mesh.face_offset_indices,
src_mesh.runtime->face_offsets_sharing_info,
mod_moder marked this conversation as resolved
Hans Goudey commented 2024-02-16 20:53:33 +01:00
Review
Copy Link

Pretty sure this is unnecessary now

Pretty sure this is unnecessary now
&dst_mesh.face_offset_indices,
&dst_mesh.runtime->face_offsets_sharing_info);
bke::copy_attributes(
src_attributes, bke::AttrDomain::Corner, propagation_info, {}, dst_attributes);
bke::gather_attributes(
src_attributes, bke::AttrDomain::Edge, propagation_info, {}, old_by_new_map, dst_attributes);
const Array<int> new_by_old_map = invert_permutation(old_by_new_map);
array_utils::gather(
new_by_old_map.as_span(), dst_mesh.corner_edges(), dst_mesh.corner_edges_for_write());
}
static void reorder_mesh_faces_exec(const Mesh &src_mesh,
const Span<int> old_by_new_map,
Mesh &dst_mesh)
static void copy_and_reorder_mesh_faces(
const Mesh &src_mesh,
const Span<int> old_by_new_map,
const bke::AnonymousAttributePropagationInfo &propagation_info,
Mesh &dst_mesh)
{
bke::gather_attributes(src_mesh.attributes(),
bke::AttrDomain::Face,
{},
{},
old_by_new_map,
dst_mesh.attributes_for_write());
const bke::AttributeAccessor src_attributes = src_mesh.attributes();
bke::MutableAttributeAccessor dst_attributes = dst_mesh.attributes_for_write();
bke::copy_attributes(
src_attributes, bke::AttrDomain::Point, propagation_info, {}, dst_attributes);
bke::copy_attributes(
src_attributes, bke::AttrDomain::Edge, propagation_info, {}, dst_attributes);
bke::gather_attributes(
src_attributes, bke::AttrDomain::Face, propagation_info, {}, old_by_new_map, dst_attributes);
const Span<int> old_offsets = src_mesh.face_offsets();
MutableSpan<int> new_offsets = dst_mesh.face_offsets_for_write();
offset_indices::gather_group_sizes(old_offsets, old_by_new_map, new_offsets);
offset_indices::accumulate_counts_to_offsets(new_offsets);
reorder_attributes_group_to_group(src_mesh.attributes(),
reorder_attributes_group_to_group(src_attributes,
bke::AttrDomain::Corner,
propagation_info,
old_offsets,
new_offsets.as_span(),
old_by_new_map,
dst_mesh.attributes_for_write());
dst_attributes);
}
static void reorder_mesh_exec(const Mesh &src_mesh,
const Span<int> old_by_new_map,
const bke::AttrDomain domain,
Mesh &dst_mesh)
static void copy_and_reorder_mesh(const Mesh &src_mesh,
const Span<int> old_by_new_map,
const bke::AttrDomain domain,
const bke::AnonymousAttributePropagationInfo &propagation_info,
Mesh &dst_mesh)
{
switch (domain) {
case bke::AttrDomain::Point:
reorder_mesh_verts_exec(src_mesh, old_by_new_map, dst_mesh);
copy_and_reorder_mesh_verts(src_mesh, old_by_new_map, propagation_info, dst_mesh);
break;
case bke::AttrDomain::Edge:
reorder_mesh_edges_exec(src_mesh, old_by_new_map, dst_mesh);
copy_and_reorder_mesh_edges(src_mesh, old_by_new_map, propagation_info, dst_mesh);
break;
case bke::AttrDomain::Face:
reorder_mesh_faces_exec(src_mesh, old_by_new_map, dst_mesh);
copy_and_reorder_mesh_faces(src_mesh, old_by_new_map, propagation_info, dst_mesh);
break;
default:
break;
@ -163,13 +209,14 @@ static void reorder_mesh_exec(const Mesh &src_mesh,
dst_mesh.tag_topology_changed();
}
static void reorder_points_exec(const PointCloud &src_pointcloud,
const Span<int> old_by_new_map,
PointCloud &dst_pointcloud)
static void copy_and_reorder_points(const PointCloud &src_pointcloud,
const Span<int> old_by_new_map,
const bke::AnonymousAttributePropagationInfo &propagation_info,
PointCloud &dst_pointcloud)
{
bke::gather_attributes(src_pointcloud.attributes(),
bke::AttrDomain::Point,
{},
propagation_info,
{},
old_by_new_map,
dst_pointcloud.attributes_for_write());
@ -177,95 +224,89 @@ static void reorder_points_exec(const PointCloud &src_pointcloud,
dst_pointcloud.tag_radii_changed();
}
static void reorder_curves_exec(const bke::CurvesGeometry &src_curves,
const Span<int> old_by_new_map,
bke::CurvesGeometry &dst_curves)
static void copy_and_reorder_curves(const bke::CurvesGeometry &src_curves,
const Span<int> old_by_new_map,
const bke::AnonymousAttributePropagationInfo &propagation_info,
bke::CurvesGeometry &dst_curves)
{
bke::gather_attributes(src_curves.attributes(),
const bke::AttributeAccessor src_attributes = src_curves.attributes();
bke::MutableAttributeAccessor dst_attributes = dst_curves.attributes_for_write();
bke::gather_attributes(src_attributes,
bke::AttrDomain::Curve,
{},
propagation_info,
{},
old_by_new_map,
dst_curves.attributes_for_write());
dst_attributes);
const Span<int> old_offsets = src_curves.offsets();
MutableSpan<int> new_offsets = dst_curves.offsets_for_write();
offset_indices::gather_group_sizes(old_offsets, old_by_new_map, new_offsets);
offset_indices::accumulate_counts_to_offsets(new_offsets);
reorder_attributes_group_to_group(src_curves.attributes(),
reorder_attributes_group_to_group(src_attributes,
bke::AttrDomain::Point,
propagation_info,
old_offsets,
new_offsets.as_span(),
old_by_new_map,
dst_curves.attributes_for_write());
dst_attributes);
dst_curves.tag_topology_changed();
}
static void reorder_instaces_exec(const bke::Instances &src_instances,
const Span<int> old_by_new_map,
bke::Instances &dst_instances)
static void copy_and_reorder_instaces(
const bke::Instances &src_instances,
const Span<int> old_by_new_map,
const bke::AnonymousAttributePropagationInfo &propagation_info,
bke::Instances &dst_instances)
{
dst_instances.resize(src_instances.instances_num());
bke::gather_attributes(src_instances.attributes(),
bke::AttrDomain::Instance,
{},
propagation_info,
{},
old_by_new_map,
dst_instances.attributes_for_write());
}
static void clean_unused_attributes(const bke::AnonymousAttributePropagationInfo &propagation_info,
bke::MutableAttributeAccessor attributes)
{
Vector<std::string> unused_ids;
attributes.for_all([&](const bke::AttributeIDRef &id, const bke::AttributeMetaData meta_data) {
if (!id.is_anonymous()) {
return true;
}
if (meta_data.data_type == CD_PROP_STRING) {
return true;
}
if (propagation_info.propagate(id.anonymous_id())) {
return true;
}
unused_ids.append(id.name());
return true;
});
for (const std::string &unused_id : unused_ids) {
attributes.remove(unused_id);
for (const bke::InstanceReference &reference : src_instances.references()) {
dst_instances.add_reference(reference);
}
BLI_assert(src_instances.references() == dst_instances.references());
const Span<float4x4> old_transforms = src_instances.transforms();
MutableSpan<float4x4> new_transforms = dst_instances.transforms_for_write();
array_utils::gather(old_transforms, old_by_new_map, new_transforms);
}
Mesh *reorder_mesh(const Mesh &src_mesh,
Span<int> old_by_new_map,
bke::AttrDomain domain,
const Span<int> old_by_new_map,
const bke::AttrDomain domain,
const bke::AnonymousAttributePropagationInfo &propagation_info)
{
Mesh *dst_mesh = BKE_mesh_copy_for_eval(&src_mesh);
clean_unused_attributes(propagation_info, dst_mesh->attributes_for_write());
reorder_mesh_exec(src_mesh, old_by_new_map, domain, *dst_mesh);
Mesh *dst_mesh = bke::mesh_new_no_attributes(
src_mesh.verts_num, src_mesh.edges_num, src_mesh.faces_num, src_mesh.corners_num);
BKE_mesh_copy_parameters(dst_mesh, &src_mesh);
copy_and_reorder_mesh(src_mesh, old_by_new_map, domain, propagation_info, *dst_mesh);
return dst_mesh;
}
PointCloud *reorder_points(const PointCloud &src_pointcloud,
Span<int> old_by_new_map,
const Span<int> old_by_new_map,
const bke::AnonymousAttributePropagationInfo &propagation_info)
{
PointCloud *dst_pointcloud = BKE_pointcloud_copy_for_eval(&src_pointcloud);
clean_unused_attributes(propagation_info, dst_pointcloud->attributes_for_write());
reorder_points_exec(src_pointcloud, old_by_new_map, *dst_pointcloud);
PointCloud *dst_pointcloud = bke::pointcloud_new_no_attributes(src_pointcloud.totpoint);
copy_and_reorder_points(src_pointcloud, old_by_new_map, propagation_info, *dst_pointcloud);
mod_moder marked this conversation as resolved Outdated
Hans Goudey commented 2024-02-14 17:17:01 +01:00
Outdated
Review
Copy Link

This will allocate the builtin attributes only to free them a moment later. I do think it's better to use some function like create_mesh_no_attributes.

This will allocate the builtin attributes only to free them a moment later. I do think it's better to use some function like `create_mesh_no_attributes`.
return dst_pointcloud;
}
mod_moder marked this conversation as resolved Outdated
Hans Goudey commented 2024-02-07 17:07:00 +01:00
Outdated
Review
Copy Link

Actually, this will be a regression in some cases, because the builtin attributes and face offsets won't be shared anymore.

Actually, this will be a regression in some cases, because the builtin attributes and face offsets won't be shared anymore.
Iliya Katushenock commented 2024-02-07 17:23:43 +01:00
Outdated
Review
Copy Link

Face offsets can be shared?
Isn't bke::copy_attributes copy built-in attributes?

Face offsets can be shared? Isn't `bke::copy_attributes` copy built-in attributes?
Hans Goudey commented 2024-02-07 17:28:13 +01:00
Outdated
Review
Copy Link

BKE_mesh_new_nomain creates the attributes. Once they already exist, they won't be shared by copy_attributes. And yes, face offsets can be shared.

`BKE_mesh_new_nomain` creates the attributes. Once they already exist, they won't be shared by `copy_attributes`. And yes, face offsets can be shared.
Iliya Katushenock commented 2024-02-09 18:50:57 +01:00
Outdated
Review
Copy Link

Sorry for merging with main and changes in the same commit.

Sorry for merging with main and changes in the same commit.
bke::CurvesGeometry reorder_curves_geometry(
const bke::CurvesGeometry &src_curves,
Span<int> old_by_new_map,
const Span<int> old_by_new_map,
const bke::AnonymousAttributePropagationInfo &propagation_info)
{
bke::CurvesGeometry dst_curves = bke::CurvesGeometry(src_curves);
clean_unused_attributes(propagation_info, dst_curves.attributes_for_write());
reorder_curves_exec(src_curves, old_by_new_map, dst_curves);
bke::CurvesGeometry dst_curves = bke::curves_new_no_attributes(src_curves.points_num(),
src_curves.curves_num());
copy_and_reorder_curves(src_curves, old_by_new_map, propagation_info, dst_curves);
return dst_curves;
}
@ -274,19 +315,19 @@ Curves *reorder_curves(const Curves &src_curves,
const bke::AnonymousAttributePropagationInfo &propagation_info)
{
const bke::CurvesGeometry src_curve_geometry = src_curves.geometry.wrap();
Curves *dst_curves = BKE_curves_copy_for_eval(&src_curves);
Curves *dst_curves = bke::curves_new_nomain(src_curve_geometry.points_num(),
mod_moder marked this conversation as resolved Outdated
Hans Goudey commented 2024-02-16 20:53:51 +01:00
Outdated
Review
Copy Link

The same optimization would apply to curves and point clouds

The same optimization would apply to curves and point clouds
Iliya Katushenock commented 2024-02-16 21:29:01 +01:00
Outdated
Review
Copy Link

Not sure this is actually optimization rather than just new abstraction for future optimizations of how data block with custom data were allocated. But i can add new functions to construct point cloud/curves/ and so now.

Not sure this is actually optimization rather than just new abstraction for future optimizations of how data block with custom data were allocated. But i can add new functions to construct point cloud/curves/ and so now.
Hans Goudey commented 2024-02-16 21:40:10 +01:00
Outdated
Review
Copy Link

It's worth testing then, I think that's reasonable since the goal of this whole PR is optimization.

It's worth testing then, I think that's reasonable since the goal of this whole PR is optimization.
Iliya Katushenock commented 2024-02-16 21:49:53 +01:00
Outdated
Review
Copy Link

I mean, right now we still allocate custom data for mesh, but delete it just in time. This can be done both in reorder.cc and in mesh.cc, but here is no much change, while this allocation is even did.

I mean, right now we still allocate custom data for mesh, but delete it just in time. This can be done both in `reorder.cc` and in `mesh.cc`, but here is no much change, while this allocation is even did.
src_curve_geometry.curves_num());
dst_curves->geometry.wrap() = reorder_curves_geometry(
src_curve_geometry, old_by_new_map, propagation_info);
return dst_curves;
}
bke::Instances *reorder_instaces(const bke::Instances &src_instances,
Span<int> old_by_new_map,
const Span<int> old_by_new_map,
const bke::AnonymousAttributePropagationInfo &propagation_info)
{
bke::Instances *dst_instances = new bke::Instances(src_instances);
clean_unused_attributes(propagation_info, dst_instances->attributes_for_write());
reorder_instaces_exec(src_instances, old_by_new_map, *dst_instances);
bke::Instances *dst_instances = new bke::Instances();
copy_and_reorder_instaces(src_instances, old_by_new_map, propagation_info, *dst_instances);
return dst_instances;
}