Geometry Nodes: don't delete existing attribute before new attribute is computed

This fixes the behavior of some nodes when the same attribute
name is used for input and output. If both attributes have a
different type, they can't exist at the same time. Therefore,
the input attribute has to be removed in order to create the
output attribute.

Previously, the input attribute was remove before it was used
in any computations. Now, the output is written to a temporary
buffer and only later saved in the geometry component. This
allows both attributes to coexist within the node.

The temporary attribute is only create when necessary. The
normal case without name collisions still works the same
as before.

Differential Revision: https://developer.blender.org/D10109

Ref T83793.
This commit is contained in:
Jacques Lucke 2021-01-14 15:52:08 +01:00
parent 53bd58993e
commit e5ee7e9a2d
Notes: blender-bot 2023-02-14 05:59:31 +01:00
Referenced by issue #83793, Adjust the decision making for result attribute types
15 changed files with 263 additions and 57 deletions

View File

@ -65,6 +65,62 @@ template<> struct DefaultHash<GeometryComponentType> {
};
} // namespace blender
class GeometryComponent;
/**
* An #OutputAttributePtr wraps a #WriteAttributePtr that might not be stored in its final
* destination yet. Therefore, once the attribute has been filled with data, the #save method has
* to be called, to store the attribute where it belongs (possibly by replacing an existing
* attribute with the same name).
*
* This is useful for example in the Attribute Color Ramp node, when the same attribute name is
* used as input and output. Typically the input is a float attribute, and the output is a color.
* Those two attributes cannot exist at the same time, due to a name collision. To handle this
* situation well, first the output colors have to be computed before the input floats are deleted.
* Therefore, the outputs have to be written to a temporary buffer that replaces the existing
* attribute once all computations are done.
*/
class OutputAttributePtr {
private:
blender::bke::WriteAttributePtr attribute_;
public:
OutputAttributePtr() = default;
OutputAttributePtr(blender::bke::WriteAttributePtr attribute);
OutputAttributePtr(GeometryComponent &component,
AttributeDomain domain,
std::string name,
CustomDataType data_type);
~OutputAttributePtr();
/* Returns false, when this wrapper is empty. */
operator bool() const
{
return static_cast<bool>(attribute_);
}
/* Get a reference to the underlying #WriteAttribute. */
blender::bke::WriteAttribute &get()
{
BLI_assert(attribute_);
return *attribute_;
}
blender::bke::WriteAttribute &operator*()
{
return *attribute_;
}
blender::bke::WriteAttribute *operator->()
{
return attribute_.get();
}
void save();
void apply_span_and_save();
};
/**
* This is the base class for specialized geometry component types.
*/
@ -185,14 +241,17 @@ class GeometryComponent {
}
/**
* Returns the attribute with the given parameters if it exists.
* If an exact match does not exist, other attributes with the same name are deleted and a new
* attribute is created if possible.
* If an attribute with the given params exist, it is returned.
* If no attribute with the given name exists, it is created and returned.
* If an attribute with the given name but different domain or type exists, a temporary attribute
* is created that has to be saved after the output has been computed. This avoids deleting
* another attribute, before a computation is finished.
*
* This might return no attribute when the attribute cannot exist on the component.
*/
blender::bke::WriteAttributePtr attribute_try_ensure_for_write(
const blender::StringRef attribute_name,
const AttributeDomain domain,
const CustomDataType data_type);
OutputAttributePtr attribute_try_get_for_output(const blender::StringRef attribute_name,
const AttributeDomain domain,
const CustomDataType data_type);
};
template<typename T>

View File

@ -40,8 +40,10 @@ static CLG_LogRef LOG = {"bke.attribute_access"};
using blender::float3;
using blender::Set;
using blender::StringRef;
using blender::StringRefNull;
using blender::bke::ReadAttributePtr;
using blender::bke::WriteAttributePtr;
using blender::fn::GMutableSpan;
/* Can't include BKE_object_deform.h right now, due to an enum forward declaration. */
extern "C" MDeformVert *BKE_object_defgroup_data_create(ID *id);
@ -250,6 +252,54 @@ template<typename T> class ArrayWriteAttribute final : public WriteAttribute {
}
};
/* This is used by the #OutputAttributePtr class. */
class TemporaryWriteAttribute final : public WriteAttribute {
public:
GMutableSpan data;
GeometryComponent &component;
std::string final_name;
TemporaryWriteAttribute(AttributeDomain domain,
GMutableSpan data,
GeometryComponent &component,
std::string final_name)
: WriteAttribute(domain, data.type(), data.size()),
data(data),
component(component),
final_name(std::move(final_name))
{
}
~TemporaryWriteAttribute() override
{
if (data.data() != nullptr) {
cpp_type_.destruct_n(data.data(), data.size());
MEM_freeN(data.data());
}
}
void get_internal(const int64_t index, void *r_value) const override
{
data.type().copy_to_uninitialized(data[index], r_value);
}
void set_internal(const int64_t index, const void *value) override
{
data.type().copy_to_initialized(value, data[index]);
}
void initialize_span(const bool UNUSED(write_only)) override
{
array_buffer_ = data.data();
array_is_temporary_ = false;
}
void apply_span_if_necessary() override
{
/* Do nothing, because the span contains the attribute itself already. */
}
};
template<typename T> class ArrayReadAttribute final : public ReadAttribute {
private:
Span<T> data_;
@ -762,30 +812,117 @@ blender::bke::ReadAttributePtr GeometryComponent::attribute_get_constant_for_rea
return attribute;
}
WriteAttributePtr GeometryComponent::attribute_try_ensure_for_write(const StringRef attribute_name,
const AttributeDomain domain,
const CustomDataType data_type)
OutputAttributePtr GeometryComponent::attribute_try_get_for_output(const StringRef attribute_name,
const AttributeDomain domain,
const CustomDataType data_type)
{
BLI_assert(this->attribute_domain_with_type_supported(domain, data_type));
const blender::fn::CPPType *cpp_type = blender::bke::custom_data_type_to_cpp_type(data_type);
BLI_assert(cpp_type != nullptr);
WriteAttributePtr attribute = this->attribute_try_get_for_write(attribute_name);
if (attribute && attribute->domain() == domain && attribute->cpp_type() == *cpp_type) {
return attribute;
/* If the attribute doesn't exist, make a new one with the correct type. */
if (!attribute) {
this->attribute_try_create(attribute_name, domain, data_type);
attribute = this->attribute_try_get_for_write(attribute_name);
return OutputAttributePtr(std::move(attribute));
}
if (attribute) {
if (!this->attribute_try_delete(attribute_name)) {
return {};
}
/* If an existing attribute has a matching domain and type, just use that. */
if (attribute->domain() == domain && attribute->cpp_type() == *cpp_type) {
return OutputAttributePtr(std::move(attribute));
}
if (!this->attribute_domain_with_type_supported(domain, data_type)) {
return {};
/* Otherwise create a temporary buffer to use before saving the new attribute. */
return OutputAttributePtr(*this, domain, attribute_name, data_type);
}
/* Construct from an attribute that already exists in the geometry component. */
OutputAttributePtr::OutputAttributePtr(WriteAttributePtr attribute)
: attribute_(std::move(attribute))
{
}
/* Construct a temporary attribute that has to replace an existing one later on. */
OutputAttributePtr::OutputAttributePtr(GeometryComponent &component,
AttributeDomain domain,
std::string final_name,
CustomDataType data_type)
{
const blender::fn::CPPType *cpp_type = blender::bke::custom_data_type_to_cpp_type(data_type);
BLI_assert(cpp_type != nullptr);
const int domain_size = component.attribute_domain_size(domain);
void *buffer = MEM_malloc_arrayN(domain_size, cpp_type->size(), __func__);
cpp_type->construct_default_n(buffer, domain_size);
attribute_ = std::make_unique<blender::bke::TemporaryWriteAttribute>(
domain, GMutableSpan{*cpp_type, buffer, domain_size}, component, std::move(final_name));
}
/* Store the computed attribute. If it was stored from the beginning already, nothing is done. This
* might delete another attribute with the same name. */
void OutputAttributePtr::save()
{
if (!attribute_) {
CLOG_WARN(&LOG, "Trying to save an attribute that does not exist anymore.");
return;
}
if (!this->attribute_try_create(attribute_name, domain, data_type)) {
return {};
blender::bke::TemporaryWriteAttribute *attribute =
dynamic_cast<blender::bke::TemporaryWriteAttribute *>(attribute_.get());
if (attribute == nullptr) {
/* The attribute is saved already. */
attribute_.reset();
return;
}
return this->attribute_try_get_for_write(attribute_name);
StringRefNull name = attribute->final_name;
const blender::fn::CPPType &cpp_type = attribute->cpp_type();
/* Delete an existing attribute with the same name if necessary. */
attribute->component.attribute_try_delete(name);
if (!attribute->component.attribute_try_create(
name, attribute_->domain(), attribute_->custom_data_type())) {
/* Cannot create the target attribute for some reason. */
CLOG_WARN(&LOG,
"Creating the '%s' attribute with type '%s' failed.",
name.c_str(),
cpp_type.name().c_str());
attribute_.reset();
return;
}
WriteAttributePtr new_attribute = attribute->component.attribute_try_get_for_write(name);
GMutableSpan temp_span = attribute->data;
GMutableSpan new_span = new_attribute->get_span_for_write_only();
BLI_assert(temp_span.size() == new_span.size());
/* Currently we copy over the attribute. In the future we want to reuse the buffer. */
cpp_type.move_to_initialized_n(temp_span.data(), new_span.data(), new_span.size());
new_attribute->apply_span();
attribute_.reset();
}
OutputAttributePtr::~OutputAttributePtr()
{
if (attribute_) {
CLOG_ERROR(&LOG, "Forgot to call #save or #apply_span_and_save.");
}
}
/* Utility function to call #apply_span and #save in the right order. */
void OutputAttributePtr::apply_span_and_save()
{
BLI_assert(attribute_);
attribute_->apply_span();
this->save();
}
/** \} */

View File

@ -161,6 +161,14 @@ class GMutableSpan {
void *operator[](int64_t index)
{
BLI_assert(index >= 0);
BLI_assert(index < size_);
return POINTER_OFFSET(data_, type_->size() * index);
}
void *operator[](int64_t index) const
{
BLI_assert(index >= 0);
BLI_assert(index < size_);
return POINTER_OFFSET(data_, type_->size() * index);
}

View File

@ -41,12 +41,12 @@ static void align_rotations_on_component(GeometryComponent &component,
const NodeGeometryAlignRotationToVector &storage = *(const NodeGeometryAlignRotationToVector *)
node.storage;
WriteAttributePtr rotation_attribute = component.attribute_try_ensure_for_write(
OutputAttributePtr rotation_attribute = component.attribute_try_get_for_output(
"rotation", ATTR_DOMAIN_POINT, CD_PROP_FLOAT3);
if (!rotation_attribute) {
return;
}
MutableSpan<float3> rotations = rotation_attribute->get_span().typed<float3>();
MutableSpan<float3> rotations = rotation_attribute->get_span<float3>();
FloatReadAttribute factors = params.get_input_attribute<float>(
"Factor", component, ATTR_DOMAIN_POINT, 1.0f);
@ -85,7 +85,7 @@ static void align_rotations_on_component(GeometryComponent &component,
rotations[i] = new_rotation;
}
rotation_attribute->apply_span();
rotation_attribute.apply_span_and_save();
}
static void geo_node_align_rotation_to_vector_exec(GeoNodeExecParams params)

View File

@ -46,7 +46,7 @@ static void execute_on_component(const GeoNodeExecParams &params, GeometryCompon
/* Once we support more domains at the user level, we have to decide how the result domain is
* chosen. */
const AttributeDomain result_domain = ATTR_DOMAIN_POINT;
WriteAttributePtr attribute_result = component.attribute_try_ensure_for_write(
OutputAttributePtr attribute_result = component.attribute_try_get_for_output(
result_name, result_domain, result_type);
if (!attribute_result) {
return;
@ -64,7 +64,7 @@ static void execute_on_component(const GeoNodeExecParams &params, GeometryCompon
BKE_colorband_evaluate(color_ramp, data_in[i], data_out[i]);
}
attribute_result->apply_span();
attribute_result.apply_span_and_save();
}
static void geo_node_attribute_color_ramp_exec(GeoNodeExecParams params)

View File

@ -242,7 +242,7 @@ static void attribute_compare_calc(GeometryComponent &component, const GeoNodeEx
/* Get result attribute first, in case it has to overwrite one of the existing attributes. */
const std::string result_name = params.get_input<std::string>("Result");
WriteAttributePtr attribute_result = component.attribute_try_ensure_for_write(
OutputAttributePtr attribute_result = component.attribute_try_get_for_output(
result_name, result_domain, result_type);
if (!attribute_result) {
return;
@ -260,8 +260,7 @@ static void attribute_compare_calc(GeometryComponent &component, const GeoNodeEx
return;
}
BooleanWriteAttribute attribute_result_bool = *attribute_result;
MutableSpan<bool> result_span = attribute_result_bool.get_span_for_write_only();
MutableSpan<bool> result_span = attribute_result->get_span_for_write_only<bool>();
/* Use specific types for correct equality operations, but for other operations we use implicit
* conversions and float comparison. In other words, the comparison is not element-wise. */
@ -300,7 +299,7 @@ static void attribute_compare_calc(GeometryComponent &component, const GeoNodeEx
do_math_operation(*attribute_a, *attribute_b, operation, result_span);
}
attribute_result_bool.apply_span();
attribute_result.apply_span_and_save();
}
static void geo_node_attribute_compare_exec(GeoNodeExecParams params)

View File

@ -68,7 +68,7 @@ static void fill_attribute(GeometryComponent &component, const GeoNodeExecParams
return;
}
WriteAttributePtr attribute = component.attribute_try_ensure_for_write(
OutputAttributePtr attribute = component.attribute_try_get_for_output(
attribute_name, domain, data_type);
if (!attribute) {
return;
@ -79,33 +79,31 @@ static void fill_attribute(GeometryComponent &component, const GeoNodeExecParams
const float value = params.get_input<float>("Value_001");
MutableSpan<float> attribute_span = attribute->get_span_for_write_only<float>();
attribute_span.fill(value);
attribute->apply_span();
break;
}
case CD_PROP_FLOAT3: {
const float3 value = params.get_input<float3>("Value");
MutableSpan<float3> attribute_span = attribute->get_span_for_write_only<float3>();
attribute_span.fill(value);
attribute->apply_span();
break;
}
case CD_PROP_COLOR: {
const Color4f value = params.get_input<Color4f>("Value_002");
MutableSpan<Color4f> attribute_span = attribute->get_span_for_write_only<Color4f>();
attribute_span.fill(value);
attribute->apply_span();
break;
}
case CD_PROP_BOOL: {
const bool value = params.get_input<bool>("Value_003");
MutableSpan<bool> attribute_span = attribute->get_span_for_write_only<bool>();
attribute_span.fill(value);
attribute->apply_span();
break;
}
default:
break;
}
attribute.apply_span_and_save();
}
static void geo_node_attribute_fill_exec(GeoNodeExecParams params)

View File

@ -107,7 +107,7 @@ static void attribute_math_calc(GeometryComponent &component, const GeoNodeExecP
/* Get result attribute first, in case it has to overwrite one of the existing attributes. */
const std::string result_name = params.get_input<std::string>("Result");
WriteAttributePtr attribute_result = component.attribute_try_ensure_for_write(
OutputAttributePtr attribute_result = component.attribute_try_get_for_output(
result_name, result_domain, result_type);
if (!attribute_result) {
return;
@ -123,6 +123,7 @@ static void attribute_math_calc(GeometryComponent &component, const GeoNodeExecP
}
do_math_operation(*attribute_a, *attribute_b, *attribute_result, operation);
attribute_result.save();
}
static void geo_node_attribute_math_exec(GeoNodeExecParams params)

View File

@ -136,7 +136,7 @@ static void attribute_mix_calc(GeometryComponent &component, const GeoNodeExecPa
result_domain = result_attribute_read->domain();
}
WriteAttributePtr attribute_result = component.attribute_try_ensure_for_write(
OutputAttributePtr attribute_result = component.attribute_try_get_for_output(
result_name, result_domain, result_type);
if (!attribute_result) {
return;
@ -155,6 +155,7 @@ static void attribute_mix_calc(GeometryComponent &component, const GeoNodeExecPa
*attribute_a,
*attribute_b,
*attribute_result);
attribute_result.save();
}
static void geo_node_attribute_mix_exec(GeoNodeExecParams params)

View File

@ -151,7 +151,7 @@ static void randomize_attribute(GeometryComponent &component,
return;
}
WriteAttributePtr attribute = component.attribute_try_ensure_for_write(
OutputAttributePtr attribute = component.attribute_try_get_for_output(
attribute_name, domain, data_type);
if (!attribute) {
return;
@ -179,6 +179,8 @@ static void randomize_attribute(GeometryComponent &component,
default:
break;
}
attribute.save();
}
static void geo_node_random_attribute_exec(GeoNodeExecParams params)

View File

@ -345,7 +345,7 @@ static void attribute_vector_math_calc(GeometryComponent &component,
/* Get result attribute first, in case it has to overwrite one of the existing attributes. */
const std::string result_name = params.get_input<std::string>("Result");
WriteAttributePtr attribute_result = component.attribute_try_ensure_for_write(
OutputAttributePtr attribute_result = component.attribute_try_get_for_output(
result_name, result_domain, result_type);
if (!attribute_result) {
return;
@ -390,6 +390,7 @@ static void attribute_vector_math_calc(GeometryComponent &component,
*attribute_a, *attribute_b, *attribute_c, *attribute_result, operation);
break;
}
attribute_result.save();
}
static void geo_node_attribute_vector_math_exec(GeoNodeExecParams params)

View File

@ -333,27 +333,27 @@ static void geo_node_point_distribute_exec(GeoNodeExecParams params)
point_component.replace(pointcloud);
{
Int32WriteAttribute stable_id_attribute = point_component.attribute_try_ensure_for_write(
OutputAttributePtr stable_id_attribute = point_component.attribute_try_get_for_output(
"id", ATTR_DOMAIN_POINT, CD_PROP_INT32);
MutableSpan<int> stable_ids_span = stable_id_attribute.get_span();
MutableSpan<int> stable_ids_span = stable_id_attribute->get_span<int>();
stable_ids_span.copy_from(stable_ids);
stable_id_attribute.apply_span();
stable_id_attribute.apply_span_and_save();
}
{
Float3WriteAttribute normals_attribute = point_component.attribute_try_ensure_for_write(
OutputAttributePtr normals_attribute = point_component.attribute_try_get_for_output(
"normal", ATTR_DOMAIN_POINT, CD_PROP_FLOAT3);
MutableSpan<float3> normals_span = normals_attribute.get_span();
MutableSpan<float3> normals_span = normals_attribute->get_span<float3>();
normals_span.copy_from(normals);
normals_attribute.apply_span();
normals_attribute.apply_span_and_save();
}
{
Float3WriteAttribute rotations_attribute = point_component.attribute_try_ensure_for_write(
OutputAttributePtr rotations_attribute = point_component.attribute_try_get_for_output(
"rotation", ATTR_DOMAIN_POINT, CD_PROP_FLOAT3);
MutableSpan<float3> rotations_span = rotations_attribute.get_span();
MutableSpan<float3> rotations_span = rotations_attribute->get_span<float3>();
rotations_span.copy_from(rotations);
rotations_attribute.apply_span();
rotations_attribute.apply_span_and_save();
}
params.set_output("Geometry", std::move(geometry_set_out));

View File

@ -104,13 +104,13 @@ static void point_rotate_on_component(GeometryComponent &component,
const bNode &node = params.node();
const NodeGeometryRotatePoints &storage = *(const NodeGeometryRotatePoints *)node.storage;
WriteAttributePtr rotation_attribute = component.attribute_try_ensure_for_write(
OutputAttributePtr rotation_attribute = component.attribute_try_get_for_output(
"rotation", ATTR_DOMAIN_POINT, CD_PROP_FLOAT3);
if (!rotation_attribute) {
return;
}
MutableSpan<float3> rotations = rotation_attribute->get_span().typed<float3>();
MutableSpan<float3> rotations = rotation_attribute->get_span<float3>();
const int domain_size = rotations.size();
if (storage.type == GEO_NODE_POINT_ROTATE_TYPE_AXIS_ANGLE) {
@ -138,7 +138,7 @@ static void point_rotate_on_component(GeometryComponent &component,
}
}
rotation_attribute->apply_span();
rotation_attribute.apply_span_and_save();
}
static void geo_node_point_rotate_exec(GeoNodeExecParams params)

View File

@ -34,7 +34,7 @@ namespace blender::nodes {
static void execute_on_component(GeoNodeExecParams params, GeometryComponent &component)
{
Float3WriteAttribute scale_attribute = component.attribute_try_ensure_for_write(
OutputAttributePtr scale_attribute = component.attribute_try_get_for_output(
"scale", ATTR_DOMAIN_POINT, CD_PROP_FLOAT3);
ReadAttributePtr attribute = params.get_input_attribute(
"Factor", component, ATTR_DOMAIN_POINT, CD_PROP_FLOAT3, nullptr);
@ -43,12 +43,12 @@ static void execute_on_component(GeoNodeExecParams params, GeometryComponent &co
}
Span<float3> data = attribute->get_span<float3>();
MutableSpan<float3> scale_span = scale_attribute.get_span();
MutableSpan<float3> scale_span = scale_attribute->get_span<float3>();
for (const int i : scale_span.index_range()) {
scale_span[i] = scale_span[i] * data[i];
}
scale_attribute.apply_span();
scale_attribute.apply_span_and_save();
}
static void geo_node_point_scale_exec(GeoNodeExecParams params)

View File

@ -34,7 +34,7 @@ namespace blender::nodes {
static void execute_on_component(GeoNodeExecParams params, GeometryComponent &component)
{
Float3WriteAttribute position_attribute = component.attribute_try_ensure_for_write(
OutputAttributePtr position_attribute = component.attribute_try_get_for_output(
"position", ATTR_DOMAIN_POINT, CD_PROP_FLOAT3);
ReadAttributePtr attribute = params.get_input_attribute(
"Translation", component, ATTR_DOMAIN_POINT, CD_PROP_FLOAT3, nullptr);
@ -43,12 +43,12 @@ static void execute_on_component(GeoNodeExecParams params, GeometryComponent &co
}
Span<float3> data = attribute->get_span<float3>();
MutableSpan<float3> scale_span = position_attribute.get_span();
MutableSpan<float3> scale_span = position_attribute->get_span<float3>();
for (const int i : scale_span.index_range()) {
scale_span[i] = scale_span[i] + data[i];
}
position_attribute.apply_span();
position_attribute.apply_span_and_save();
}
static void geo_node_point_translate_exec(GeoNodeExecParams params)