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

Refactor: Nodes: improve socket value variant construction

Browse Source 
Previously, there was the issue that `SocketValueVariant` had a constructor that
took a forwarding reference as parameter. This was problematic, because this
could potentially hide copy/move constructors which is not intentional. This
patch makes sure that these special constructors are not overridden and adds two
static utility functions to make sure it's still straight forward to construct
the `SocketValueVariant` on a single line.

Clangd also warned about this case.

Pull Request: https://projects.blender.org/blender/blender/pulls/140842
This commit is contained in:
Jacques Lucke
2025-06-23 10:08:43 +02:00
parent 41e93e3d9b
commit ea618c2027
12 changed files with 69 additions and 39 deletions
Download Patch File Download Diff File Expand all files Collapse all files

38
source/blender/blenkernel/BKE_node_socket_value.hh
View File

@@ -81,12 +81,30 @@ class SocketValueVariant {
* Create an empty variant. This is not valid for any socket type yet.
*/
SocketValueVariant() = default;
SocketValueVariant(const SocketValueVariant &other) = default;
SocketValueVariant(SocketValueVariant &&other) = default;
SocketValueVariant &operator=(const SocketValueVariant &other) = default;
SocketValueVariant &operator=(SocketValueVariant &&other) = default;
~SocketValueVariant() = default;
/**
* Create a variant based on the given value. This works for primitive types, #GField and
* #Field<T>.
* Create a variant based on the given value. This works for primitive types. For more complex
* types use #set explicity. Alternatively, one can use the #From or #ConstructIn utilities.
*/
template<typename T> explicit SocketValueVariant(T &&value);
template<typename T,
/* The enable-if is necessary to avoid overridding the copy/moveconstructors. */
BLI_ENABLE_IF((std::is_trivial_v<std::decay_t<T>> ||
is_same_any_v<std::decay_t<T>, std::string>))>
explicit SocketValueVariant(T &&value)
{
this->set(std::forward<T>(value));
}
/** Construct a #SocketValueVariant at the given pointer from the given value. */
template<typename T> static SocketValueVariant &ConstructIn(void *ptr, T &&value);
/** Create a new #SocketValueVariant from the given value. */
template<typename T> static SocketValueVariant From(T &&value);
/**
* \return True if the stored value is valid for a specific socket type. This is mainly meant to
@@ -174,9 +192,19 @@ class SocketValueVariant {
template<typename T> void store_impl(T value);
};
template<typename T> inline SocketValueVariant::SocketValueVariant(T &&value)
template<typename T>
inline SocketValueVariant &SocketValueVariant::ConstructIn(void *ptr, T &&value)
{
this->set(std::forward<T>(value));
SocketValueVariant *value_variant = new (ptr) SocketValueVariant();
value_variant->set(std::forward<T>(value));
return *value_variant;
}
template<typename T> inline SocketValueVariant SocketValueVariant::From(T &&value)
{
SocketValueVariant value_variant;
value_variant.set(std::forward<T>(value));
return value_variant;
}
template<typename T> inline void SocketValueVariant::set(T &&value)

6
source/blender/blenkernel/intern/bake_items_socket.cc
View File

@@ -238,7 +238,7 @@ Array<std::unique_ptr<BakeItem>> move_socket_values_to_bake_items(const Span<voi
std::shared_ptr<AttributeFieldInput> attribute_field = make_attribute_field(base_type);
r_attribute_map.add(item->name(), attribute_field->attribute_name());
fn::GField field{attribute_field};
new (r_value) SocketValueVariant(std::move(field));
SocketValueVariant::ConstructIn(r_value, std::move(field));
return true;
}
#ifdef WITH_OPENVDB
@@ -251,7 +251,7 @@ Array<std::unique_ptr<BakeItem>> move_socket_values_to_bake_items(const Span<voi
return false;
}
if (grid_socket_type == socket_type) {
new (r_value) SocketValueVariant(*item->grid);
bke::SocketValueVariant::ConstructIn(r_value, std::move(*item->grid));
return true;
}
return false;
@@ -287,7 +287,7 @@ Array<std::unique_ptr<BakeItem>> move_socket_values_to_bake_items(const Span<voi
bundle.add(item.key, *stype, buffer);
stype->geometry_nodes_cpp_type->destruct(buffer);
}
new (r_value) SocketValueVariant(std::move(bundle_ptr));
bke::SocketValueVariant::ConstructIn(r_value, std::move(bundle_ptr));
return true;
}
return false;

3
source/blender/nodes/NOD_geometry_exec.hh
View File

@@ -186,8 +186,7 @@ class GeoNodeExecParams {
{
using StoredT = std::decay_t<T>;
if constexpr (stored_as_SocketValueVariant_v<StoredT>) {
SocketValueVariant value_variant(std::forward<T>(value));
this->set_output(identifier, std::move(value_variant));
this->set_output(identifier, SocketValueVariant::From(std::forward<T>(value)));
}
else {
#ifndef NDEBUG

2
source/blender/nodes/geometry/nodes/node_geo_index_switch.cc
View File

@@ -309,7 +309,7 @@ class LazyFunctionForIndexSwitchNode : public LazyFunction {
GField output_field(FieldOperation::Create(std::move(switch_fn), std::move(input_fields)));
void *output_ptr = params.get_output_data_ptr(0);
new (output_ptr) SocketValueVariant(std::move(output_field));
SocketValueVariant::ConstructIn(output_ptr, std::move(output_field));
params.output_set(0);
}
};

2
source/blender/nodes/geometry/nodes/node_geo_menu_switch.cc
View File

@@ -319,7 +319,7 @@ class LazyFunctionForMenuSwitchNode : public LazyFunction {
GField output_field{FieldOperation::Create(std::move(multi_function), std::move(item_fields))};
void *output_ptr = params.get_output_data_ptr(0);
new (output_ptr) SocketValueVariant(std::move(output_field));
SocketValueVariant::ConstructIn(output_ptr, std::move(output_field));
params.output_set(0);
}
};

2
source/blender/nodes/geometry/nodes/node_geo_switch.cc
View File

@@ -179,7 +179,7 @@ class LazyFunctionForSwitchNode : public LazyFunction {
{std::move(condition), std::move(false_field), std::move(true_field)})};
void *output_ptr = params.get_output_data_ptr(0);
new (output_ptr) SocketValueVariant(std::move(output_field));
SocketValueVariant::ConstructIn(output_ptr, std::move(output_field));
params.output_set(0);
}

5
source/blender/nodes/intern/geometry_nodes_closure_zone.cc
View File

@@ -139,7 +139,8 @@ class LazyFunctionForClosureZone : public LazyFunction {
params.set_output(zone_info_.indices.outputs.border_link_usages[i], true);
}
if (!U.experimental.use_bundle_and_closure_nodes) {
params.set_output(zone_info_.indices.outputs.main[0], bke::SocketValueVariant(ClosurePtr()));
params.set_output(zone_info_.indices.outputs.main[0],
bke::SocketValueVariant::From(ClosurePtr()));
return;
}
@@ -256,7 +257,7 @@ class LazyFunctionForClosureZone : public LazyFunction {
std::make_shared<ClosureEvalLog>())};
params.set_output(zone_info_.indices.outputs.main[0],
bke::SocketValueVariant(std::move(closure)));
bke::SocketValueVariant::From(std::move(closure)));
}
};

10
source/blender/nodes/intern/geometry_nodes_execute.cc
View File

@@ -565,7 +565,7 @@ static void init_socket_cpp_value_from_property(const IDProperty &property,
}
case SOCK_STRING: {
std::string value = IDP_String(&property);
new (r_value) bke::SocketValueVariant(std::move(value));
bke::SocketValueVariant::ConstructIn(r_value, std::move(value));
break;
}
case SOCK_MENU: {
@@ -662,14 +662,14 @@ static void initialize_group_input(const bNodeTree &tree,
if (attribute_name && bke::allow_procedural_attribute_access(*attribute_name)) {
fn::GField attribute_field = bke::AttributeFieldInput::Create(*attribute_name,
*typeinfo->base_cpp_type);
new (r_value) bke::SocketValueVariant(std::move(attribute_field));
bke::SocketValueVariant::ConstructIn(r_value, std::move(attribute_field));
}
else if (is_layer_selection_field(io_input)) {
const IDProperty *property_layer_name = properties.lookup_key_as(io_input.identifier);
StringRef layer_name = IDP_String(property_layer_name);
const fn::GField selection_field(
std::make_shared<bke::NamedLayerSelectionFieldInput>(layer_name), 0);
new (r_value) bke::SocketValueVariant(std::move(selection_field));
fn::GField selection_field(std::make_shared<bke::NamedLayerSelectionFieldInput>(layer_name),
0);
bke::SocketValueVariant::ConstructIn(r_value, std::move(selection_field));
}
else {
init_socket_cpp_value_from_property(*property, socket_data_type, r_value);

6
source/blender/nodes/intern/geometry_nodes_foreach_geometry_element_zone.cc
View File

@@ -916,8 +916,7 @@ void LazyFunctionForReduceForeachGeometryElement::handle_main_items_and_geometry
*base_cpp_type,
make_anonymous_attribute_socket_inspection_string(
parent_.output_bnode_.output_socket(parent_.indices_.main.bsocket_outer[item_i])));
SocketValueVariant attribute_value_variant{GField(std::move(attribute_field))};
params.set_output(1 + item_i, std::move(attribute_value_variant));
params.set_output(1 + item_i, SocketValueVariant::From(GField(std::move(attribute_field))));
}
/* Output the original geometry with potentially additional attributes. */
@@ -1183,9 +1182,8 @@ void LazyFunctionForReduceForeachGeometryElement::handle_generation_items_group(
base_cpp_type,
make_anonymous_attribute_socket_inspection_string(
parent_.output_bnode_.output_socket(2 + node_storage.main_items.items_num + item_i)));
SocketValueVariant attribute_value_variant{GField(std::move(attribute_field))};
params.set_output(parent_.indices_.generation.lf_outer[item_i],
std::move(attribute_value_variant));
bke::SocketValueVariant::From(GField(std::move(attribute_field))));
}
}

2
source/blender/nodes/intern/geometry_nodes_lazy_function.cc
View File

@@ -303,7 +303,7 @@ class LazyFunctionForGeometryNode : public LazyFunction {
std::move(socket_inspection_name));
void *r_value = params.get_output_data_ptr(lf_index);
new (r_value) SocketValueVariant(GField(std::move(attribute_field)));
SocketValueVariant::ConstructIn(r_value, GField(std::move(attribute_field)));
params.output_set(lf_index);
}

24
source/blender/nodes/intern/node_declaration.cc
View File

@@ -916,40 +916,44 @@ namespace implicit_field_inputs {
static void position(const bNode & /*node*/, void *r_value)
{
new (r_value) bke::SocketValueVariant(bke::AttributeFieldInput::Create<float3>("position"));
bke::SocketValueVariant::ConstructIn(r_value,
bke::AttributeFieldInput::Create<float3>("position"));
}
static void normal(const bNode & /*node*/, void *r_value)
{
new (r_value)
bke::SocketValueVariant(fn::Field<float3>(std::make_shared<bke::NormalFieldInput>()));
bke::SocketValueVariant::ConstructIn(
r_value, fn::Field<float3>(std::make_shared<bke::NormalFieldInput>()));
}
static void index(const bNode & /*node*/, void *r_value)
{
new (r_value) bke::SocketValueVariant(fn::Field<int>(std::make_shared<fn::IndexFieldInput>()));
bke::SocketValueVariant::ConstructIn(r_value,
fn::Field<int>(std::make_shared<fn::IndexFieldInput>()));
}
static void id_or_index(const bNode & /*node*/, void *r_value)
{
new (r_value)
bke::SocketValueVariant(fn::Field<int>(std::make_shared<bke::IDAttributeFieldInput>()));
bke::SocketValueVariant::ConstructIn(
r_value, fn::Field<int>(std::make_shared<bke::IDAttributeFieldInput>()));
}
static void instance_transform(const bNode & /*node*/, void *r_value)
{
new (r_value)
bke::SocketValueVariant(bke::AttributeFieldInput::Create<float4x4>("instance_transform"));
bke::SocketValueVariant::ConstructIn(
r_value, bke::AttributeFieldInput::Create<float4x4>("instance_transform"));
}
static void handle_left(const bNode & /*node*/, void *r_value)
{
new (r_value) bke::SocketValueVariant(bke::AttributeFieldInput::Create<float3>("handle_left"));
bke::SocketValueVariant::ConstructIn(r_value,
bke::AttributeFieldInput::Create<float3>("handle_left"));
}
static void handle_right(const bNode & /*node*/, void *r_value)
{
new (r_value) bke::SocketValueVariant(bke::AttributeFieldInput::Create<float3>("handle_right"));
bke::SocketValueVariant::ConstructIn(r_value,
bke::AttributeFieldInput::Create<float3>("handle_right"));
}
} // namespace implicit_field_inputs

8
source/blender/nodes/intern/node_socket.cc
View File

@@ -995,9 +995,9 @@ static bke::bNodeSocketType *make_socket_type_bundle()
};
socktype->geometry_nodes_cpp_type = &blender::CPPType::get<SocketValueVariant>();
socktype->get_geometry_nodes_cpp_value = [](const void * /*socket_value*/, void *r_value) {
new (r_value) SocketValueVariant(nodes::BundlePtr());
SocketValueVariant::ConstructIn(r_value, nodes::BundlePtr());
};
static SocketValueVariant default_value{nodes::BundlePtr()};
static SocketValueVariant default_value = SocketValueVariant::From(nodes::BundlePtr());
socktype->geometry_nodes_default_cpp_value = &default_value;
return socktype;
}
@@ -1011,9 +1011,9 @@ static bke::bNodeSocketType *make_socket_type_closure()
};
socktype->geometry_nodes_cpp_type = &blender::CPPType::get<SocketValueVariant>();
socktype->get_geometry_nodes_cpp_value = [](const void * /*socket_value*/, void *r_value) {
new (r_value) SocketValueVariant(nodes::ClosurePtr());
SocketValueVariant::ConstructIn(r_value, nodes::ClosurePtr());
};
static SocketValueVariant default_value{nodes::ClosurePtr()};
static SocketValueVariant default_value = SocketValueVariant::From(nodes::ClosurePtr());
socktype->geometry_nodes_default_cpp_value = &default_value;
return socktype;
}