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

Fix: Memory leak writing to builtin attribute from modifier 

If the attribute already existed, but had a different domain or data type
than the user tried to write to it with (i.e. writing to `position` with
a float), then the data from field evaluation wasn't freed.

This restructures the geometry nodes modifier attribute writing process
to solve that problem and remove some of the nested if statements
that made the process confusing.

One case that still doesn't work is writing to a builtin attribute from
a different domain. If `OutputAttribute` gets that feature then that
could be supported here too.

Differential Revision: https://developer.blender.org/D14706
This commit is contained in:
Hans Goudey 2022-04-21 14:05:16 -05:00
parent 9de3ed5c82
commit 65e4c91bec
Notes: blender-bot 2023-02-14 08:38:11 +01:00 
Referenced by issue #97595, Regression: GeoNode attribute output passing does not work in the latest nightly
1 changed files with 26 additions and 22 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -998,30 +998,34 @@ static void store_computed_output_attributes(
const CustomDataType data_type = blender::bke::cpp_type_to_custom_data_type(store.data.type());
const std::optional<AttributeMetaData> meta_data = component.attribute_get_meta_data(
store.name);
if (meta_data.has_value() && meta_data->domain == store.domain &&
meta_data->data_type == data_type) {
/* Copy the data into an existing attribute. */
blender::bke::WriteAttributeLookup write_attribute = component.attribute_try_get_for_write(
store.name);
if (write_attribute) {
write_attribute.varray.set_all(store.data.data());
if (write_attribute.tag_modified_fn) {
write_attribute.tag_modified_fn();
}
}
store.data.type().destruct_n(store.data.data(), store.data.size());
MEM_freeN(store.data.data());
/* Attempt to remove the attribute if it already exists but the domain and type don't match.
* Removing the attribute won't succeed if it is built in and non-removable. */
if (meta_data.has_value() &&
(meta_data->domain != store.domain || meta_data->data_type != data_type)) {
component.attribute_try_delete(store.name);
}
else {
/* Replace the existing attribute with the new data. */
if (meta_data.has_value()) {
component.attribute_try_delete(store.name);
}
component.attribute_try_create(store.name,
store.domain,
blender::bke::cpp_type_to_custom_data_type(store.data.type()),
AttributeInitMove(store.data.data()));
/* 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 (component.attribute_try_create(
store.name,
store.domain,
blender::bke::cpp_type_to_custom_data_type(store.data.type()),
AttributeInitMove(store.data.data()))) {
continue;
}
OutputAttribute attribute = component.attribute_try_get_for_output_only(
store.name, store.domain, data_type);
if (attribute) {
attribute.varray().set_all(store.data.data());
attribute.save();
}
/* We were unable to reuse the data, so it must be destructed and freed. */
store.data.type().destruct_n(store.data.data(), store.data.size());
MEM_freeN(store.data.data());
}
}