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

Nodes: wrap int in MenuValue type for menu sockets

Browse Source 
Previously, we always just used `int` when dealing with menu values on the C++
side. It's currently the only type where we have the same base type (`int`) for
two different socket types (integer and menu sockets). This has some downsides:
* It requires special cases in some places.
* There is no function from static base type to socket type (which is useful for
  some utilities like `SocketValueVariant`).
* It implicitly allows operations on menu values that shouldn't be allowed such
  as arithmetic operations and conversions to and from other types.

This patch adds a new `MenuValue` type that is used for menu sockets in Geometry
Nodes and the (CPU) Compositor, clarifying the distinction between integer and
menu values.

Pull Request: https://projects.blender.org/blender/blender/pulls/144476
This commit is contained in:
Jacques Lucke
2025-08-13 15:43:37 +02:00
parent 09d357f878
commit c90a137a27
23 changed files with 111 additions and 50 deletions
Download Patch File Download Diff File Expand all files Collapse all files

3
source/blender/blenkernel/intern/cpp_types.cc
View File

@@ -15,6 +15,7 @@
#include "NOD_geometry_nodes_closure.hh"
#include "NOD_geometry_nodes_list.hh"
#include "NOD_geometry_nodes_values.hh"
#include "NOD_menu_value.hh"
#include "DNA_meshdata_types.h"
@@ -34,6 +35,7 @@ BLI_CPP_TYPE_MAKE(Image *, CPPTypeFlags::BasicType)
BLI_CPP_TYPE_MAKE(Material *, CPPTypeFlags::BasicType)
BLI_CPP_TYPE_MAKE(MStringProperty, CPPTypeFlags::None);
BLI_CPP_TYPE_MAKE(blender::nodes::MenuValue, CPPTypeFlags::None);
BLI_CPP_TYPE_MAKE(blender::nodes::BundlePtr, CPPTypeFlags::None);
BLI_CPP_TYPE_MAKE(blender::nodes::ClosurePtr, CPPTypeFlags::None);
BLI_CPP_TYPE_MAKE(blender::nodes::ListPtr, CPPTypeFlags::None);
@@ -60,6 +62,7 @@ void BKE_cpp_types_init()
BLI_CPP_TYPE_REGISTER(Material *);
BLI_CPP_TYPE_REGISTER(MStringProperty);
BLI_CPP_TYPE_REGISTER(blender::nodes::MenuValue);
BLI_CPP_TYPE_REGISTER(blender::nodes::BundlePtr);
BLI_CPP_TYPE_REGISTER(blender::nodes::ClosurePtr);
BLI_CPP_TYPE_REGISTER(blender::nodes::ListPtr);

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

@@ -91,6 +91,7 @@
#include "NOD_geometry_nodes_dependencies.hh"
#include "NOD_geometry_nodes_gizmos.hh"
#include "NOD_geometry_nodes_lazy_function.hh"
#include "NOD_menu_value.hh"
#include "NOD_node_declaration.hh"
#include "NOD_register.hh"
#include "NOD_shader.h"
@@ -4966,6 +4967,9 @@ std::optional<eNodeSocketDatatype> geo_nodes_base_cpp_type_to_socket_type(const
if (type.is<math::Quaternion>()) {
return SOCK_ROTATION;
}
if (type.is<nodes::MenuValue>()) {
return SOCK_MENU;
}
if (type.is<float4x4>()) {
return SOCK_MATRIX;
}

18
source/blender/blenkernel/intern/node_socket_value.cc
View File

@@ -16,6 +16,7 @@
#include "NOD_geometry_nodes_bundle.hh"
#include "NOD_geometry_nodes_closure.hh"
#include "NOD_geometry_nodes_list.hh"
#include "NOD_menu_value.hh"
#include "BLI_color.hh"
#include "BLI_math_rotation_types.hh"
@@ -58,6 +59,9 @@ template<typename T> static std::optional<eNodeSocketDatatype> static_type_to_so
if constexpr (is_single_or_field_or_grid_v<T, math::Quaternion>) {
return SOCK_ROTATION;
}
if constexpr (is_same_any_v<T, nodes::MenuValue, fn::Field<nodes::MenuValue>>) {
return SOCK_MENU;
}
if constexpr (is_same_any_v<T, float4x4, fn::Field<float4x4>>) {
return SOCK_MATRIX;
}
@@ -115,7 +119,7 @@ static bool static_type_is_base_socket_type(const eNodeSocketDatatype socket_typ
case SOCK_STRING:
return std::is_same_v<T, std::string>;
case SOCK_MENU:
return std::is_same_v<T, int>;
return std::is_same_v<T, nodes::MenuValue>;
case SOCK_BUNDLE:
return std::is_same_v<T, nodes::BundlePtr>;
case SOCK_CLOSURE:
@@ -297,6 +301,10 @@ void SocketValueVariant::store_single(const eNodeSocketDatatype socket_type, con
value_.emplace<math::Quaternion>(*static_cast<const math::Quaternion *>(value));
break;
}
case SOCK_MENU: {
value_.emplace<nodes::MenuValue>(*static_cast<const nodes::MenuValue *>(value));
break;
}
case SOCK_MATRIX: {
value_.emplace<float4x4>(*static_cast<const float4x4 *>(value));
break;
@@ -442,7 +450,7 @@ void *SocketValueVariant::allocate_single(const eNodeSocketDatatype socket_type)
case SOCK_STRING:
return value_.allocate<std::string>();
case SOCK_MENU:
return value_.allocate<int>();
return value_.allocate<nodes::MenuValue>();
case SOCK_BUNDLE:
return value_.allocate<nodes::BundlePtr>();
case SOCK_CLOSURE:
@@ -487,9 +495,6 @@ bool SocketValueVariant::valid_for_socket(eNodeSocketDatatype socket_type) const
if (kind_ == Kind::None) {
return false;
}
if (socket_type == SOCK_MENU) {
return socket_type_ == SOCK_INT;
}
return socket_type_ == socket_type;
}
@@ -532,6 +537,9 @@ INSTANTIATE(Material *)
INSTANTIATE(float4x4)
INSTANTIATE(fn::Field<float4x4>)
INSTANTIATE(nodes::MenuValue)
INSTANTIATE(fn::Field<nodes::MenuValue>)
#ifdef WITH_OPENVDB
INSTANTIATE(GVolumeGrid)
#endif

6
source/blender/compositor/COM_result.hh
View File

@@ -23,6 +23,8 @@
#include "GPU_shader.hh"
#include "GPU_texture.hh"
#include "NOD_menu_value.hh"
#include "COM_domain.hh"
#include "COM_meta_data.hh"
@@ -133,8 +135,8 @@ class Result {
* which will be identical to that stored in the data_ member. The active variant member depends
* on the type of the result. This member is uninitialized and should not be used if the result
* is not a single value. */
std::variant<float, float2, float3, float4, int32_t, int2, bool, std::string> single_value_ =
0.0f;
std::variant<float, float2, float3, float4, int32_t, int2, bool, std::string, nodes::MenuValue>
single_value_ = 0.0f;
/* The domain of the result. This only matters if the result was not a single value. See the
* discussion in COM_domain.hh for more information. */
Domain domain_ = Domain::identity();

2
source/blender/compositor/intern/input_single_value_operation.cc
View File

@@ -76,7 +76,7 @@ void InputSingleValueOperation::execute()
}
case SOCK_MENU: {
const int32_t value = input_socket_->default_value_typed<bNodeSocketValueMenu>()->value;
result.set_single_value(value);
result.set_single_value(nodes::MenuValue(value));
break;
}
case SOCK_STRING: {

3
source/blender/compositor/intern/multi_function_procedure_operation.cc
View File

@@ -265,7 +265,8 @@ mf::Variable *MultiFunctionProcedureOperation::get_constant_input_variable(DInpu
}
case SOCK_MENU: {
const int32_t value = input->default_value_typed<bNodeSocketValueMenu>()->value;
constant_function = &procedure_.construct_function<mf::CustomMF_Constant<int32_t>>(value);
constant_function = &procedure_.construct_function<mf::CustomMF_Constant<nodes::MenuValue>>(
value);
break;
}
case SOCK_STRING: {

4
source/blender/compositor/intern/result.cc
View File

@@ -328,7 +328,7 @@ const CPPType &Result::cpp_type(const ResultType type)
case ResultType::Bool:
return CPPType::get<bool>();
case ResultType::Menu:
return CPPType::get<int32_t>();
return CPPType::get<nodes::MenuValue>();
case ResultType::String:
return CPPType::get<std::string>();
}
@@ -446,7 +446,7 @@ void Result::allocate_single_value()
this->set_single_value(false);
break;
case ResultType::Menu:
this->set_single_value(0);
this->set_single_value(nodes::MenuValue(0));
break;
case ResultType::String:
this->set_single_value(std::string(""));

11
source/blender/editors/space_node/node_socket_tooltip.cc
View File

@@ -21,6 +21,7 @@
#include "DNA_material_types.h"
#include "NOD_geometry_nodes_log.hh"
#include "NOD_menu_value.hh"
#include "NOD_node_declaration.hh"
#include "NOD_socket.hh"
@@ -362,7 +363,7 @@ class SocketTooltipBuilder {
return true;
}
void build_tooltip_value_enum(const int item_identifier)
void build_tooltip_value_enum(const nodes::MenuValue menu_item)
{
const auto *storage = socket_.default_value_typed<bNodeSocketValueMenu>();
if (!storage->enum_items || storage->has_conflict()) {
@@ -370,7 +371,7 @@ class SocketTooltipBuilder {
return;
}
const bke::RuntimeNodeEnumItem *enum_item = storage->enum_items->find_item_by_identifier(
item_identifier);
menu_item.value);
if (!enum_item) {
return;
}
@@ -482,12 +483,12 @@ class SocketTooltipBuilder {
}
if (socket_.type == SOCK_MENU) {
if (!value_type.is<int>()) {
if (!value_type.is<nodes::MenuValue>()) {
this->build_tooltip_value_unknown();
return;
}
const int item_identifier = *value.get<int>();
this->build_tooltip_value_enum(item_identifier);
const nodes::MenuValue menu_item = *value.get<nodes::MenuValue>();
this->build_tooltip_value_enum(menu_item);
return;
}

2
source/blender/makesrna/intern/rna_node_socket.cc
View File

@@ -586,7 +586,7 @@ int rna_NodeSocketStandard_menu_default(PointerRNA *ptr, PropertyRNA * /*prop*/)
return 0;
}
auto *decl = static_cast<const blender::nodes::decl::Menu *>(sock->runtime->declaration);
return decl->default_value;
return decl->default_value.value;
}
/* using a context update function here, to avoid searching the node if possible */

1
source/blender/nodes/CMakeLists.txt
View File

@@ -129,6 +129,7 @@ set(SRC
NOD_inverse_eval_path.hh
NOD_inverse_eval_run.hh
NOD_math_functions.hh
NOD_menu_value.hh
NOD_multi_function.hh
NOD_nested_node_id.hh
NOD_node_declaration.hh

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

@@ -26,6 +26,7 @@
#include "NOD_derived_node_tree.hh"
#include "NOD_geometry_nodes_lazy_function.hh"
#include "NOD_geometry_nodes_values.hh"
#include "NOD_menu_value.hh"
namespace blender::nodes {
@@ -132,7 +133,7 @@ class GeoNodeExecParams {
return value_variant;
}
else if constexpr (std::is_enum_v<T>) {
return T(value_variant.extract<int>());
return T(value_variant.extract<MenuValue>().value);
}
else {
T value = value_variant.extract<T>();
@@ -177,7 +178,7 @@ class GeoNodeExecParams {
return value_variant;
}
else if constexpr (std::is_enum_v<T>) {
return T(value_variant.get<int>());
return T(value_variant.get<MenuValue>().value);
}
else {
T value = value_variant.get<T>();

35
source/blender/nodes/NOD_menu_value.hh Normal file
View File

@@ -0,0 +1,35 @@
/* SPDX-FileCopyrightText: 2025 Blender Authors
*
* SPDX-License-Identifier: GPL-2.0-or-later */
#pragma once
#include "BLI_hash.hh"
#include "BLI_struct_equality_utils.hh"
namespace blender::nodes {
/**
* Don't use integer for menus directly, so that there is a each static single value type maps to
* exactly one socket type. Also it avoids accidentally casting the menu value to other types.
*/
struct MenuValue {
int value = 0;
MenuValue() = default;
explicit MenuValue(const int value) : value(value) {}
template<typename EnumT, BLI_ENABLE_IF((std::is_enum_v<EnumT>))>
MenuValue(const EnumT value) : value(int(value))
{
}
uint64_t hash() const
{
return get_default_hash(this->value);
}
BLI_STRUCT_EQUALITY_OPERATORS_1(MenuValue, value)
};
} // namespace blender::nodes

7
source/blender/nodes/NOD_socket_declarations.hh
View File

@@ -4,6 +4,7 @@
#pragma once
#include "NOD_menu_value.hh"
#include "NOD_node_declaration.hh"
#include "RNA_types.hh"
@@ -228,7 +229,7 @@ class Menu : public SocketDeclaration {
public:
static constexpr eNodeSocketDatatype static_socket_type = SOCK_MENU;
int32_t default_value;
MenuValue default_value;
bool is_expanded = false;
ImplicitSharingPtr<bke::RuntimeNodeEnumItems> items;
@@ -244,7 +245,7 @@ class Menu : public SocketDeclaration {
class MenuBuilder : public SocketDeclarationBuilder<Menu> {
public:
MenuBuilder &default_value(int32_t value);
MenuBuilder &default_value(MenuValue value);
/** Draw the menu items next to each other instead of as a drop-down menu. */
MenuBuilder &expanded(bool value = true);
@@ -598,7 +599,7 @@ inline StringBuilder &StringBuilder::subtype(PropertySubType subtype)
/** \name #MenuBuilder Inline Methods
* \{ */
inline MenuBuilder &MenuBuilder::default_value(const int32_t value)
inline MenuBuilder &MenuBuilder::default_value(const MenuValue value)
{
decl_->default_value = value;
return *this;

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

@@ -163,7 +163,7 @@ class MenuSwitchFn : public mf::MultiFunction {
: enum_def_(enum_def), type_(type)
{
mf::SignatureBuilder builder{"Menu Switch", signature_};
builder.single_input<int>("Menu");
builder.single_input<MenuValue>("Menu");
for (const NodeEnumItem &enum_item : enum_def.items()) {
builder.single_input(enum_item.name, type);
}
@@ -176,24 +176,24 @@ class MenuSwitchFn : public mf::MultiFunction {
{
const int value_inputs_start = 1;
const int inputs_num = enum_def_.items_num;
const VArray<int> values = params.readonly_single_input<int>(0, "Menu");
const VArray<MenuValue> values = params.readonly_single_input<MenuValue>(0, "Menu");
/* Use one extra mask at the end for invalid indices. */
const int invalid_index = inputs_num;
GMutableSpan output = params.uninitialized_single_output(
signature_.params.index_range().last(), "Output");
auto find_item_index = [&](const int value) -> int {
auto find_item_index = [&](const MenuValue value) -> int {
for (const int i : enum_def_.items().index_range()) {
const NodeEnumItem &item = enum_def_.items()[i];
if (item.identifier == value) {
if (item.identifier == value.value) {
return i;
}
}
return invalid_index;
};
if (const std::optional<int> value = values.get_if_single()) {
if (const std::optional<MenuValue> value = values.get_if_single()) {
const int index = find_item_index(*value);
if (index < inputs_num) {
const GVArray inputs = params.readonly_single_input(value_inputs_start + index);
@@ -258,19 +258,19 @@ class LazyFunctionForMenuSwitchNode : public LazyFunction {
{
SocketValueVariant condition_variant = params.get_input<SocketValueVariant>(0);
if (condition_variant.is_context_dependent_field() && can_be_field_) {
this->execute_field(condition_variant.get<Field<int>>(), params);
this->execute_field(condition_variant.get<Field<MenuValue>>(), params);
}
else {
this->execute_single(condition_variant.get<int>(), params);
this->execute_single(condition_variant.get<MenuValue>(), params);
}
}
void execute_single(const int condition, lf::Params &params) const
void execute_single(const MenuValue condition, lf::Params &params) const
{
for (const int i : IndexRange(enum_def_.items_num)) {
const NodeEnumItem &enum_item = enum_def_.items_array[i];
const int input_index = i + 1;
if (enum_item.identifier == condition) {
if (enum_item.identifier == condition.value) {
SocketValueVariant *value_to_forward =
params.try_get_input_data_ptr_or_request<SocketValueVariant>(input_index);
if (value_to_forward == nullptr) {
@@ -289,7 +289,7 @@ class LazyFunctionForMenuSwitchNode : public LazyFunction {
set_default_remaining_node_outputs(params, node_);
}
void execute_field(Field<int> condition, lf::Params &params) const
void execute_field(Field<MenuValue> condition, lf::Params &params) const
{
/* When the condition is a non-constant field, we need all inputs. */
const int values_num = this->enum_def_.items_num;
@@ -347,10 +347,10 @@ class LazyFunctionForMenuSwitchSocketUsage : public lf::LazyFunction {
}
}
else {
const int32_t value = condition_variant.get<int>();
const MenuValue value = condition_variant.get<MenuValue>();
for (const int i : IndexRange(enum_def_.items_num)) {
const NodeEnumItem &enum_item = enum_def_.items()[i];
params.set_output(i, value == enum_item.identifier);
params.set_output(i, value.value == enum_item.identifier);
}
}
}
@@ -365,12 +365,12 @@ class MenuSwitchOperation : public NodeOperation {
void execute() override
{
Result &output = this->get_result("Output");
const int32_t menu_identifier = this->get_input("Menu").get_single_value<int32_t>();
const MenuValue menu_identifier = this->get_input("Menu").get_single_value<MenuValue>();
const NodeEnumDefinition &enum_definition = node_storage(bnode()).enum_definition;
for (const int i : IndexRange(enum_definition.items_num)) {
const NodeEnumItem &enum_item = enum_definition.items()[i];
if (enum_item.identifier != menu_identifier) {
if (enum_item.identifier != menu_identifier.value) {
continue;
}
const std::string identifier = MenuSwitchItemsAccessor::socket_identifier_for_item(

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

@@ -48,7 +48,7 @@ static void node_declare(NodeDeclarationBuilder &b)
b.add_input<decl::Vector>("Position").implicit_field(NODE_DEFAULT_INPUT_POSITION_FIELD);
b.add_input<decl::Menu>("Interpolation")
.static_items(interpolation_mode_items)
.default_value(int(InterpolationMode::TriLinear))
.default_value(InterpolationMode::TriLinear)
.description("How to interpolate the values between neighboring voxels");
b.add_output(data_type, "Value").dependent_field({1});

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

@@ -202,7 +202,8 @@ class LazyFunctionForSwitchNode : public LazyFunction {
ColorGeometry4f,
std::string,
math::Quaternion,
float4x4>([&](auto type_tag) {
float4x4,
MenuValue>([&](auto type_tag) {
using T = typename decltype(type_tag)::type;
if constexpr (std::is_void_v<T>) {
BLI_assert_unreachable();

4
source/blender/nodes/geometry/nodes/node_geo_triangulate.cc
View File

@@ -68,10 +68,10 @@ static void node_declare(NodeDeclarationBuilder &b)
b.add_input<decl::Bool>("Selection").default_value(true).field_on_all().hide_value();
b.add_input<decl::Menu>("Quad Method")
.static_items(rna_node_geometry_triangulate_quad_method_items)
.default_value(int(geometry::TriangulateQuadMode::ShortEdge))
.default_value(geometry::TriangulateQuadMode::ShortEdge)
.description("Method for splitting the quads into triangles");
b.add_input<decl::Menu>("N-gon Method")
.default_value(int(geometry::TriangulateNGonMode::Beauty))
.default_value(geometry::TriangulateNGonMode::Beauty)
.static_items(rna_node_geometry_triangulate_ngon_method_items)
.description("Method for splitting the n-gons into triangles");
}

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

@@ -67,7 +67,7 @@ static void node_declare(NodeDeclarationBuilder &b)
b.add_input<decl::Bool>("Rotate").default_value(true).description("Rotate islands for best fit");
b.add_input<decl::Menu>("Method")
.static_items(shape_method_items)
.default_value(int(ShapeMethod::Aabb))
.default_value(ShapeMethod::Aabb)
.description("Method used for packing UV islands");
}

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

@@ -13,6 +13,7 @@
#include "NOD_geometry.hh"
#include "NOD_geometry_nodes_execute.hh"
#include "NOD_geometry_nodes_lazy_function.hh"
#include "NOD_menu_value.hh"
#include "NOD_node_declaration.hh"
#include "NOD_socket.hh"
@@ -564,7 +565,7 @@ static bke::SocketValueVariant init_socket_cpp_value_from_property(
}
case SOCK_MENU: {
int value = IDP_Int(&property);
return bke::SocketValueVariant(std::move(value));
return bke::SocketValueVariant::From(MenuValue(value));
}
case SOCK_OBJECT: {
ID *id = IDP_Id(&property);

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

@@ -335,7 +335,7 @@ static BaseSocketDeclarationBuilder &build_interface_socket_declaration(
case SOCK_MENU: {
const auto &value = node_interface::get_socket_data_as<bNodeSocketValueMenu>(io_socket);
decl = &b.add_socket<decl::Menu>(name, identifier, in_out)
.default_value(value.value)
.default_value(MenuValue(value.value))
.expanded(io_socket.flag & NODE_INTERFACE_SOCKET_MENU_EXPANDED);
break;
}

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

@@ -37,6 +37,7 @@
#include "NOD_geometry_nodes_bundle.hh"
#include "NOD_geometry_nodes_closure.hh"
#include "NOD_menu_value.hh"
#include "NOD_node_declaration.hh"
#include "NOD_socket.hh"
@@ -1125,15 +1126,15 @@ static bke::bNodeSocketType *make_socket_type_string(PropertySubType subtype)
static bke::bNodeSocketType *make_socket_type_menu()
{
bke::bNodeSocketType *socktype = make_standard_socket_type(SOCK_MENU, PROP_NONE);
socktype->base_cpp_type = &blender::CPPType::get<int>();
socktype->base_cpp_type = &blender::CPPType::get<nodes::MenuValue>();
socktype->get_base_cpp_value = [](const void *socket_value, void *r_value) {
*(int *)r_value = ((bNodeSocketValueMenu *)socket_value)->value;
new (r_value) nodes::MenuValue(((bNodeSocketValueMenu *)socket_value)->value);
};
socktype->get_geometry_nodes_cpp_value = [](const void *socket_value) {
const int value = ((bNodeSocketValueMenu *)socket_value)->value;
return SocketValueVariant(value);
const nodes::MenuValue value{((bNodeSocketValueMenu *)socket_value)->value};
return SocketValueVariant::From(value);
};
static SocketValueVariant default_value{0};
static SocketValueVariant default_value = SocketValueVariant::From(nodes::MenuValue());
socktype->geometry_nodes_default_value = &default_value;
return socktype;
}

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

@@ -573,7 +573,7 @@ bNodeSocket &Menu::build(bNodeTree &ntree, bNode &node) const
this->identifier.c_str(),
this->name.c_str());
((bNodeSocketValueMenu *)socket.default_value)->value = this->default_value;
((bNodeSocketValueMenu *)socket.default_value)->value = this->default_value.value;
this->set_common_flags(socket);
return socket;
}

3
source/blender/nodes/intern/socket_usage_inference.cc
View File

@@ -5,6 +5,7 @@
#include <regex>
#include "NOD_geometry_nodes_execute.hh"
#include "NOD_menu_value.hh"
#include "NOD_multi_function.hh"
#include "NOD_node_declaration.hh"
#include "NOD_node_in_compute_context.hh"
@@ -1540,7 +1541,7 @@ bool InputSocketUsageParams::menu_input_may_be(const StringRef identifier,
/* The value is unknown, so it may be the requested enum value. */
return true;
}
return value.get_known<int>() == enum_value;
return value.get_known<MenuValue>().value == enum_value;
}
} // namespace blender::nodes::socket_usage_inference