blender
/
blender
136
404
Fork 586
Code Issues 6.2k Pull Requests 761 Projects 19 Wiki Activity

Attributes: Integrate implicit sharing with the attribute API #107059

Merged
Jacques Lucke merged 8 commits from HooglyBoogly/blender:implicit-sharing-attribute-api into main 2023-04-19 11:21:21 +02:00
Conversation 1 Commits 8 Files Changed 83 +75 -57
14 changed files with 75 additions and 57 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit 5c578d29e1 - Show all commits

24
source/blender/blenkernel/BKE_attribute.hh
View File

@ -84,8 +84,10 @@ struct AttributeInit {
DefaultValue,
/** #AttributeInitVArray. */
VArray,
/** #AttributeInitData. */
/** #AttributeInitMoveArray. */
MoveArray,
/** #AttributeInitShared. */
Shared,
};
Type type;
AttributeInit(const Type type) : type(type) {}
@ -121,15 +123,25 @@ struct AttributeInitVArray : public AttributeInit {
* Sometimes data is created before a geometry component is available. In that case, it's
* preferable to move data directly to the created attribute to avoid a new allocation and a copy.
*
* Implicit sharing info can be provided if the array is shared. In that case the sharing info
* has ownership of the provided array.
* The array must be allocated with MEM_*, since `attribute_try_create` will free the array if it
* can't be used directly, and that is generally how Blender expects custom data to be allocated.
*/
struct AttributeInitData : public AttributeInit {
struct AttributeInitMoveArray : public AttributeInit {
void *data = nullptr;
AttributeInitMoveArray(void *data) : AttributeInit(Type::MoveArray), data(data) {}
};
/**
* Create a shared attribute by adding a user to a shared data array.
* The sharing info has ownership of the provided contiguous array.
*/
struct AttributeInitShared : public AttributeInit {
const void *data = nullptr;
const ImplicitSharingInfo *sharing_info = nullptr;
AttributeInitData(const void *data, const ImplicitSharingInfo *sharing_info)
: AttributeInit(Type::MoveArray), data(data), sharing_info(sharing_info)
AttributeInitShared(const void *data, const ImplicitSharingInfo &sharing_info)
: AttributeInit(Type::Shared), data(data), sharing_info(&sharing_info)
{
}
};

4
source/blender/blenkernel/intern/DerivedMesh.cc
View File

@ -658,12 +658,12 @@ static void mesh_calc_modifiers(struct Depsgraph *depsgraph,
ASSERT_IS_VALID_MESH(mesh_final);
}
MutableAttributeAccessor attributes = mesh_final->attributes_for_write();
const AttributeReader<float3> positions = attributes.lookup<float3>("position");
const AttributeReader positions = attributes.lookup<float3>("position");
if (positions) {
attributes.add<float3>(
"rest_position",
ATTR_DOMAIN_POINT,
AttributeInitData(positions.varray.common_info().data, positions.sharing_info));
AttributeInitShared(positions.varray.common_info().data, *positions.sharing_info));
Jacques Lucke commented 2023-04-18 23:10:38 +02:00
Outdated
Review
Copy Link

I think you make too many implicit assumptions here:

  • The attribute is definitely shared.
  • The attribute is definitely stored as continuous array.

Both assumptions generally have to be checked for.

I think you make too many implicit assumptions here: * The attribute is definitely shared. * The attribute is definitely stored as continuous array. Both assumptions generally have to be checked for.
Hans Goudey commented 2023-04-18 23:18:36 +02:00
Outdated
Review
Copy Link

I thought it was reasonable to assume that sharing_info != nullptr implies varray.is_span(), but I can also check the latter explicitly too. I guess this will get nicer when attributes don't always have to be stored as contiguous arrays.

I should definitely check for sharing_info here though. I did it elsewhere and forgot here.

I thought it was reasonable to assume that `sharing_info != nullptr` implies `varray.is_span()`, but I can also check the latter explicitly too. I guess this will get nicer when attributes don't always have to be stored as contiguous arrays. I should definitely check for `sharing_info` here though. I did it elsewhere and forgot here.
}
}

23
source/blender/blenkernel/intern/attribute_access.cc
View File

@ -206,7 +206,20 @@ static bool add_builtin_type_custom_data_layer_from_init(CustomData &custom_data
return true;
}
case AttributeInit::Type::MoveArray: {
const AttributeInitData &init = static_cast<const AttributeInitData &>(initializer);
void *src_data = static_cast<const AttributeInitMoveArray &>(initializer).data;
const void *stored_data = CustomData_add_layer_with_data(
&custom_data, data_type, src_data, domain_num, nullptr);
if (stored_data == nullptr) {
return false;
}
if (stored_data != src_data) {
MEM_freeN(src_data);
return true;
}
return true;
}
case AttributeInit::Type::Shared: {
const AttributeInitShared &init = static_cast<const AttributeInitShared &>(initializer);
const void *stored_data = CustomData_add_layer_with_data(
&custom_data, data_type, const_cast<void *>(init.data), domain_num, init.sharing_info);
return stored_data != nullptr;
@ -281,7 +294,13 @@ static bool add_custom_data_layer_from_attribute_init(const AttributeIDRef &attr
break;
}
case AttributeInit::Type::MoveArray: {
const AttributeInitData &init = static_cast<const AttributeInitData &>(initializer);
void *data = static_cast<const AttributeInitMoveArray &>(initializer).data;
add_generic_custom_data_layer_with_existing_data(
custom_data, data_type, attribute_id, domain_num, data, nullptr);
break;
}
case AttributeInit::Type::Shared: {
const AttributeInitShared &init = static_cast<const AttributeInitShared &>(initializer);
add_generic_custom_data_layer_with_existing_data(custom_data,
data_type,
attribute_id,

2
source/blender/blenkernel/intern/geometry_fields.cc
View File

@ -474,7 +474,7 @@ bool try_capture_field_on_geometry(GeometryComponent &component,
}
attributes.remove(attribute_id);
if (attributes.add(attribute_id, domain, data_type, bke::AttributeInitData(buffer, nullptr))) {
if (attributes.add(attribute_id, domain, data_type, bke::AttributeInitMoveArray(buffer))) {
return true;
}

2
source/blender/editors/curves/intern/curves_ops.cc
View File

@ -803,7 +803,7 @@ static int curves_set_selection_domain_exec(bContext *C, wmOperator *op)
if (!attributes.add(".selection",
domain,
bke::cpp_type_to_custom_data_type(type),
bke::AttributeInitData(dst, nullptr))) {
bke::AttributeInitMoveArray(dst))) {
MEM_freeN(dst);
}
}

3
source/blender/editors/geometry/geometry_attributes.cc
View File

@ -709,8 +709,7 @@ bool ED_geometry_attribute_convert(Mesh *mesh,
void *new_data = MEM_malloc_arrayN(varray.size(), cpp_type.size(), __func__);
varray.materialize_to_uninitialized(new_data);
attributes.remove(name_copy);
if (!attributes.add(
name_copy, dst_domain, dst_type, bke::AttributeInitData(new_data, nullptr))) {
if (!attributes.add(name_copy, dst_domain, dst_type, bke::AttributeInitMoveArray(new_data))) {
MEM_freeN(new_data);
}

2
source/blender/editors/sculpt_paint/curves_sculpt_selection.cc
View File

@ -25,7 +25,7 @@ bke::SpanAttributeWriter<float> float_selection_ensure(Curves &curves_id)
attributes.remove(".selection");
attributes.add(
".selection", meta_data->domain, CD_PROP_FLOAT, bke::AttributeInitData(dst, nullptr));
".selection", meta_data->domain, CD_PROP_FLOAT, bke::AttributeInitMoveArray(dst));
}
}
else {

2
source/blender/geometry/intern/mesh_split_edges.cc
View File

@ -154,7 +154,7 @@ static void add_new_edges(Mesh &mesh,
attributes.add(new_data.local_id,
ATTR_DOMAIN_EDGE,
bke::cpp_type_to_custom_data_type(new_data.type),
bke::AttributeInitData(new_data.array, nullptr));
bke::AttributeInitMoveArray(new_data.array));
}
}

6
source/blender/modifiers/intern/MOD_nodes.cc
View File

@ -1086,10 +1086,8 @@ static void store_computed_output_attributes(
/* Try to create the attribute reusing the stored buffer. This will only succeed if the
* attribute didn't exist before, or if it existed but was removed above. */
if (attributes.add(store.name,
store.domain,
data_type,
bke::AttributeInitData(store.data.data(), nullptr))) {
if (attributes.add(
store.name, store.domain, data_type, bke::AttributeInitMoveArray(store.data.data()))) {
continue;
}

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

@ -161,15 +161,13 @@ static void add_instances_from_component(
bke::MutableAttributeAccessor dst_attributes = dst_component.attributes_for_write();
for (const auto item : attributes_to_propagate.items()) {
const AttributeIDRef &id = item.key;
const eCustomDataType type = item.value.data_type;
const bke::GAttributeReader src = src_attributes.lookup_or_default(
id, ATTR_DOMAIN_POINT, type);
if (src.varray.size() == dst_component.instances_num()) {
dst_attributes.add(id,
ATTR_DOMAIN_INSTANCE,
type,
bke::AttributeInitData(src.varray.common_info().data, src.sharing_info));
const bke::GAttributeReader src = src_attributes.lookup(id, ATTR_DOMAIN_POINT);
const eCustomDataType type = bke::cpp_type_to_custom_data_type(src.varray.type());
if (src.varray.size() == dst_component.instances_num() && src.sharing_info) {
const bke::AttributeInitShared init(src.varray.get_internal_span().data(),
*src.sharing_info);
dst_attributes.add(id, ATTR_DOMAIN_INSTANCE, type, init);
}
else {
GSpanAttributeWriter dst = dst_attributes.lookup_or_add_for_write_only_span(

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

@ -71,12 +71,11 @@ static void convert_instances_to_points(GeometrySet &geometry_set,
const AttributeIDRef &id = item.key;
const eCustomDataType type = item.value.data_type;
const GAttributeReader src = src_attributes.lookup_or_default(id, ATTR_DOMAIN_INSTANCE, type);
if (selection.size() == instances.instances_num()) {
dst_attributes.add(id,
ATTR_DOMAIN_POINT,
type,
bke::AttributeInitData(src.varray.common_info().data, src.sharing_info));
const GAttributeReader src = src_attributes.lookup(id);
if (selection.size() == instances.instances_num() && src.sharing_info) {
const bke::AttributeInitShared init(src.varray.get_internal_span().data(),
*src.sharing_info);
dst_attributes.add(id, ATTR_DOMAIN_POINT, type, init);
}
else {
GSpanAttributeWriter dst = dst_attributes.lookup_or_add_for_write_only_span(

12
source/blender/nodes/geometry/nodes/node_geo_interpolate_curves.cc
View File

@ -574,17 +574,13 @@ static void interpolate_curve_attributes(bke::CurvesGeometry &child_curves,
if (id.is_anonymous() && !propagation_info.propagate(id.anonymous_id())) {
return true;
}
const eCustomDataType type = meta_data.data_type;
if (type == CD_PROP_STRING) {
if (meta_data.data_type == CD_PROP_STRING) {
return true;
}
const GAttributeReader src = point_attributes.lookup(id, ATTR_DOMAIN_POINT, type);
children_attributes.add(
id,
ATTR_DOMAIN_CURVE,
type,
bke::AttributeInitData(src.varray.common_info().data, src.sharing_info));
const GAttributeReader src = point_attributes.lookup(id);
const bke::AttributeInitShared init(src.varray.get_internal_span().data(), *src.sharing_info);
children_attributes.add(id, ATTR_DOMAIN_CURVE, meta_data.data_type, init);
return true;
});
}

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

@ -83,15 +83,13 @@ static void geometry_set_mesh_to_points(GeometrySet &geometry_set,
PointCloud *pointcloud;
if (share_position) {
/* Create an empty point cloud so the positions can be easily shared with a generic utility. */
/* Create an empty point cloud so the positions can be shared. */
pointcloud = BKE_pointcloud_new_nomain(0);
CustomData_free_layer_named(&pointcloud->pdata, "position", pointcloud->totpoint);
pointcloud->totpoint = mesh->totvert;
const bke::AttributeReader src = src_attributes.lookup<float3>("position");
pointcloud->attributes_for_write().add<float3>(
"position",
ATTR_DOMAIN_POINT,
bke::AttributeInitData(src.varray.common_info().data, src.sharing_info));
const bke::AttributeInitShared init(src.varray.get_internal_span().data(), *src.sharing_info);
pointcloud->attributes_for_write().add<float3>("position", ATTR_DOMAIN_POINT, init);
}
else {
pointcloud = BKE_pointcloud_new_nomain(selection.size());
@ -117,11 +115,10 @@ static void geometry_set_mesh_to_points(GeometrySet &geometry_set,
const AttributeIDRef attribute_id = entry.key;
const eCustomDataType data_type = entry.value.data_type;
const bke::GAttributeReader src = src_attributes.lookup(attribute_id, domain, data_type);
if (share_arrays && src.domain == domain) {
dst_attributes.add(attribute_id,
ATTR_DOMAIN_POINT,
data_type,
bke::AttributeInitData(src.varray.common_info().data, src.sharing_info));
if (share_arrays && src.domain == domain && src.sharing_info) {
const bke::AttributeInitShared init(src.varray.get_internal_span().data(),
*src.sharing_info);
dst_attributes.add(attribute_id, ATTR_DOMAIN_POINT, data_type, init);
}
else {
GSpanAttributeWriter dst = dst_attributes.lookup_or_add_for_write_only_span(

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

@ -51,6 +51,7 @@ static void geometry_set_points_to_vertices(
Mesh *mesh;
if (selection.size() == points->totpoint) {
/* Create a mesh without positions so the attribute can be shared. */
mesh = BKE_mesh_new_nomain(0, 0, 0, 0);
CustomData_free_layer_named(&mesh->vdata, "position", mesh->totvert);
mesh->totvert = selection.size();
@ -66,11 +67,10 @@ static void geometry_set_points_to_vertices(
const AttributeIDRef id = entry.key;
const eCustomDataType data_type = entry.value.data_type;
const GAttributeReader src = src_attributes.lookup(id);
if (selection.size() == points->totpoint) {
dst_attributes.add(id,
ATTR_DOMAIN_POINT,
data_type,
bke::AttributeInitData(src.varray.common_info().data, src.sharing_info));
if (selection.size() == points->totpoint && src.sharing_info) {
const bke::AttributeInitShared init(src.varray.get_internal_span().data(),
*src.sharing_info);
dst_attributes.add(id, ATTR_DOMAIN_POINT, data_type, init);
}
else {
GSpanAttributeWriter dst = dst_attributes.lookup_or_add_for_write_only_span(