Fix: Explicitly prevent capturing built-in attribute with bad mata data #110433

Open
Iliya Katushenock wants to merge 7 commits from mod_moder/blender:tmp_fix_rewrite_builtin_attr into main

When changing the target branch, be careful to rebase the branch in your fork to match. See documentation.
9 changed files with 197 additions and 103 deletions

View File

@ -63,6 +63,11 @@ struct AttributeMetaData {
eAttrDomain domain;
eCustomDataType data_type;
constexpr AttributeMetaData(eAttrDomain src_domain, eCustomDataType src_data_type)
: domain(src_domain), data_type(src_data_type)
{
}
constexpr friend bool operator==(AttributeMetaData a, AttributeMetaData b)
{
return (a.domain == b.domain) && (a.data_type == b.data_type);
@ -197,14 +202,31 @@ struct AttributeValidator {
*/
const fn::multi_function::MultiFunction *function;
/**
* Static meta data for the attribute, if needed.
*/
const std::optional<AttributeMetaData> meta_data;
AttributeValidator() = default;
AttributeValidator(const fn::multi_function::MultiFunction &src_function)
: function(&src_function)
{
}
AttributeValidator(const fn::multi_function::MultiFunction &src_function,
const AttributeMetaData src_meta_data)
: function(&src_function), meta_data(src_meta_data)
{
}
operator bool() const
{
return this->function != nullptr;
return this->function != nullptr || meta_data.has_value();
}
/**
* Return a field that creates corrected attribute values.
*/
fn::GField validate_field_if_necessary(const fn::GField &field) const;
bool validate_meta_data_if_necessary(AttributeMetaData) const;
};
/**

View File

@ -964,6 +964,14 @@ fn::GField AttributeValidator::validate_field_if_necessary(const fn::GField &fie
return field;
}
bool AttributeValidator::validate_meta_data_if_necessary(const AttributeMetaData other) const
{
if (meta_data.has_value()) {
return other == meta_data;
}
return true;
}
Vector<AttributeTransferData> retrieve_attributes_for_transfer(
const bke::AttributeAccessor src_attributes,
bke::MutableAttributeAccessor dst_attributes,

View File

@ -424,25 +424,27 @@ static ComponentAttributeProviders create_attribute_providers_for_curve()
return std::clamp<int8_t>(value, BEZIER_HANDLE_FREE, BEZIER_HANDLE_ALIGN);
},
mf::build::exec_presets::AllSpanOrSingle());
static BuiltinCustomDataLayerProvider handle_type_right("handle_type_right",
ATTR_DOMAIN_POINT,
CD_PROP_INT8,
CD_PROP_INT8,
BuiltinAttributeProvider::Creatable,
BuiltinAttributeProvider::Deletable,
point_access,
tag_component_topology_changed,
AttributeValidator{&handle_type_clamp});
static BuiltinCustomDataLayerProvider handle_type_right(
"handle_type_right",
ATTR_DOMAIN_POINT,
CD_PROP_INT8,
CD_PROP_INT8,
BuiltinAttributeProvider::Creatable,
BuiltinAttributeProvider::Deletable,
point_access,
tag_component_topology_changed,
AttributeValidator(handle_type_clamp, AttributeMetaData(ATTR_DOMAIN_POINT, CD_PROP_INT8)));
static BuiltinCustomDataLayerProvider handle_type_left("handle_type_left",
ATTR_DOMAIN_POINT,
CD_PROP_INT8,
CD_PROP_INT8,
BuiltinAttributeProvider::Creatable,
BuiltinAttributeProvider::Deletable,
point_access,
tag_component_topology_changed,
AttributeValidator{&handle_type_clamp});
static BuiltinCustomDataLayerProvider handle_type_left(
"handle_type_left",
ATTR_DOMAIN_POINT,
CD_PROP_INT8,
CD_PROP_INT8,
BuiltinAttributeProvider::Creatable,
BuiltinAttributeProvider::Deletable,
point_access,
tag_component_topology_changed,
AttributeValidator(handle_type_clamp, AttributeMetaData(ATTR_DOMAIN_POINT, CD_PROP_INT8)));
static BuiltinCustomDataLayerProvider nurbs_weight("nurbs_weight",
ATTR_DOMAIN_POINT,
@ -457,15 +459,16 @@ static ComponentAttributeProviders create_attribute_providers_for_curve()
"NURBS Order Validate",
[](int8_t value) { return std::max<int8_t>(value, 0); },
mf::build::exec_presets::AllSpanOrSingle());
static BuiltinCustomDataLayerProvider nurbs_order("nurbs_order",
ATTR_DOMAIN_CURVE,
CD_PROP_INT8,
CD_PROP_INT8,
BuiltinAttributeProvider::Creatable,
BuiltinAttributeProvider::Deletable,
curve_access,
tag_component_topology_changed,
AttributeValidator{&nurbs_order_clamp});
static BuiltinCustomDataLayerProvider nurbs_order(
"nurbs_order",
ATTR_DOMAIN_CURVE,
CD_PROP_INT8,
CD_PROP_INT8,
BuiltinAttributeProvider::Creatable,
BuiltinAttributeProvider::Deletable,
curve_access,
tag_component_topology_changed,
AttributeValidator(nurbs_order_clamp, AttributeMetaData(ATTR_DOMAIN_CURVE, CD_PROP_INT8)));
static const auto normal_mode_clamp = mf::build::SI1_SO<int8_t, int8_t>(
"Normal Mode Validate",
@ -473,15 +476,16 @@ static ComponentAttributeProviders create_attribute_providers_for_curve()
return std::clamp<int8_t>(value, NORMAL_MODE_MINIMUM_TWIST, NORMAL_MODE_Z_UP);
},
mf::build::exec_presets::AllSpanOrSingle());
static BuiltinCustomDataLayerProvider normal_mode("normal_mode",
ATTR_DOMAIN_CURVE,
CD_PROP_INT8,
CD_PROP_INT8,
BuiltinAttributeProvider::Creatable,
BuiltinAttributeProvider::Deletable,
curve_access,
tag_component_normals_changed,
AttributeValidator{&normal_mode_clamp});
static BuiltinCustomDataLayerProvider normal_mode(
"normal_mode",
ATTR_DOMAIN_CURVE,
CD_PROP_INT8,
CD_PROP_INT8,
BuiltinAttributeProvider::Creatable,
BuiltinAttributeProvider::Deletable,
curve_access,
tag_component_normals_changed,
AttributeValidator(normal_mode_clamp, AttributeMetaData(ATTR_DOMAIN_CURVE, CD_PROP_INT8)));
static const auto knots_mode_clamp = mf::build::SI1_SO<int8_t, int8_t>(
"Knots Mode Validate",
@ -489,15 +493,16 @@ static ComponentAttributeProviders create_attribute_providers_for_curve()
return std::clamp<int8_t>(value, NURBS_KNOT_MODE_NORMAL, NURBS_KNOT_MODE_ENDPOINT_BEZIER);
},
mf::build::exec_presets::AllSpanOrSingle());
static BuiltinCustomDataLayerProvider nurbs_knots_mode("knots_mode",
ATTR_DOMAIN_CURVE,
CD_PROP_INT8,
CD_PROP_INT8,
BuiltinAttributeProvider::Creatable,
BuiltinAttributeProvider::Deletable,
curve_access,
tag_component_topology_changed,
AttributeValidator{&knots_mode_clamp});
static BuiltinCustomDataLayerProvider nurbs_knots_mode(
"knots_mode",
ATTR_DOMAIN_CURVE,
CD_PROP_INT8,
CD_PROP_INT8,
BuiltinAttributeProvider::Creatable,
BuiltinAttributeProvider::Deletable,
curve_access,
tag_component_topology_changed,
AttributeValidator(knots_mode_clamp, AttributeMetaData(ATTR_DOMAIN_CURVE, CD_PROP_INT8)));
static const auto curve_type_clamp = mf::build::SI1_SO<int8_t, int8_t>(
"Curve Type Validate",
@ -505,29 +510,31 @@ static ComponentAttributeProviders create_attribute_providers_for_curve()
return std::clamp<int8_t>(value, CURVE_TYPE_CATMULL_ROM, CURVE_TYPES_NUM);
},
mf::build::exec_presets::AllSpanOrSingle());
static BuiltinCustomDataLayerProvider curve_type("curve_type",
ATTR_DOMAIN_CURVE,
CD_PROP_INT8,
CD_PROP_INT8,
BuiltinAttributeProvider::Creatable,
BuiltinAttributeProvider::Deletable,
curve_access,
tag_component_curve_types_changed,
AttributeValidator{&curve_type_clamp});
static BuiltinCustomDataLayerProvider curve_type(
"curve_type",
ATTR_DOMAIN_CURVE,
CD_PROP_INT8,
CD_PROP_INT8,
BuiltinAttributeProvider::Creatable,
BuiltinAttributeProvider::Deletable,
curve_access,
tag_component_curve_types_changed,
AttributeValidator(curve_type_clamp, AttributeMetaData(ATTR_DOMAIN_CURVE, CD_PROP_INT8)));
static const auto resolution_clamp = mf::build::SI1_SO<int, int>(
"Resolution Validate",
[](int value) { return std::max<int>(value, 1); },
mf::build::exec_presets::AllSpanOrSingle());
static BuiltinCustomDataLayerProvider resolution("resolution",
ATTR_DOMAIN_CURVE,
CD_PROP_INT32,
CD_PROP_INT32,
BuiltinAttributeProvider::Creatable,
BuiltinAttributeProvider::Deletable,
curve_access,
tag_component_topology_changed,
AttributeValidator{&resolution_clamp});
static BuiltinCustomDataLayerProvider resolution(
"resolution",
ATTR_DOMAIN_CURVE,
CD_PROP_INT32,
CD_PROP_INT32,
BuiltinAttributeProvider::Creatable,
BuiltinAttributeProvider::Deletable,
curve_access,
tag_component_topology_changed,
AttributeValidator(resolution_clamp, AttributeMetaData(ATTR_DOMAIN_CURVE, CD_PROP_INT32)));
static BuiltinCustomDataLayerProvider cyclic("cyclic",
ATTR_DOMAIN_CURVE,

View File

@ -1124,29 +1124,32 @@ static ComponentAttributeProviders create_attribute_providers_for_mesh()
return std::clamp<int>(value, 0, std::numeric_limits<short>::max());
},
mf::build::exec_presets::AllSpanOrSingle());
static BuiltinCustomDataLayerProvider material_index("material_index",
ATTR_DOMAIN_FACE,
CD_PROP_INT32,
CD_PROP_INT32,
BuiltinAttributeProvider::Creatable,
BuiltinAttributeProvider::Deletable,
face_access,
nullptr,
AttributeValidator{&material_index_clamp});
static BuiltinCustomDataLayerProvider material_index(
"material_index",
ATTR_DOMAIN_FACE,
CD_PROP_INT32,
CD_PROP_INT32,
BuiltinAttributeProvider::Creatable,
BuiltinAttributeProvider::Deletable,
face_access,
nullptr,
AttributeValidator(material_index_clamp,
AttributeMetaData(ATTR_DOMAIN_FACE, CD_PROP_INT32)));
static const auto int2_index_clamp = mf::build::SI1_SO<int2, int2>(
"Index Validate",
[](int2 value) { return math::max(value, int2(0)); },
mf::build::exec_presets::AllSpanOrSingle());
static BuiltinCustomDataLayerProvider edge_verts(".edge_verts",
ATTR_DOMAIN_EDGE,
CD_PROP_INT32_2D,
CD_PROP_INT32_2D,
BuiltinAttributeProvider::Creatable,
BuiltinAttributeProvider::NonDeletable,
edge_access,
nullptr,
AttributeValidator{&int2_index_clamp});
static BuiltinCustomDataLayerProvider edge_verts(
".edge_verts",
ATTR_DOMAIN_EDGE,
CD_PROP_INT32_2D,
CD_PROP_INT32_2D,
BuiltinAttributeProvider::Creatable,
BuiltinAttributeProvider::NonDeletable,
edge_access,
nullptr,
AttributeValidator(int2_index_clamp, AttributeMetaData(ATTR_DOMAIN_EDGE, CD_PROP_INT32_2D)));
/* Note: This clamping is more of a last resort, since it's quite easy to make an
* invalid mesh that will crash Blender by arbitrarily editing this attribute. */
@ -1154,24 +1157,26 @@ static ComponentAttributeProviders create_attribute_providers_for_mesh()
"Index Validate",
[](int value) { return std::max(value, 0); },
mf::build::exec_presets::AllSpanOrSingle());
static BuiltinCustomDataLayerProvider corner_vert(".corner_vert",
ATTR_DOMAIN_CORNER,
CD_PROP_INT32,
CD_PROP_INT32,
BuiltinAttributeProvider::Creatable,
BuiltinAttributeProvider::NonDeletable,
corner_access,
nullptr,
AttributeValidator{&int_index_clamp});
static BuiltinCustomDataLayerProvider corner_edge(".corner_edge",
ATTR_DOMAIN_CORNER,
CD_PROP_INT32,
CD_PROP_INT32,
BuiltinAttributeProvider::Creatable,
BuiltinAttributeProvider::NonDeletable,
corner_access,
nullptr,
AttributeValidator{&int_index_clamp});
static BuiltinCustomDataLayerProvider corner_vert(
".corner_vert",
ATTR_DOMAIN_CORNER,
CD_PROP_INT32,
CD_PROP_INT32,
BuiltinAttributeProvider::Creatable,
BuiltinAttributeProvider::NonDeletable,
corner_access,
nullptr,
AttributeValidator(int_index_clamp, AttributeMetaData(ATTR_DOMAIN_CORNER, CD_PROP_INT32)));
static BuiltinCustomDataLayerProvider corner_edge(
".corner_edge",
ATTR_DOMAIN_CORNER,
CD_PROP_INT32,
CD_PROP_INT32,
BuiltinAttributeProvider::Creatable,
BuiltinAttributeProvider::NonDeletable,
corner_access,
nullptr,
AttributeValidator(int_index_clamp, AttributeMetaData(ATTR_DOMAIN_CORNER, CD_PROP_INT32)));
static BuiltinCustomDataLayerProvider sharp_face("sharp_face",
ATTR_DOMAIN_FACE,

View File

@ -497,6 +497,10 @@ bool try_capture_field_on_geometry(GeometryComponent &component,
const bke::GeometryFieldContext field_context{component, domain};
const bke::AttributeValidator validator = attributes.lookup_validator(attribute_id);
if (!validator.validate_meta_data_if_necessary({domain, data_type})) {
return false;
}
const std::optional<AttributeMetaData> meta_data = attributes.lookup_meta_data(attribute_id);
const bool attribute_matches = meta_data &&
attribute_kind_matches(*meta_data, domain, data_type);

View File

@ -336,16 +336,22 @@ static int run_node_group_exec(bContext *C, wmOperator *op)
bke::GeometrySet geometry_orig = get_original_geometry_eval_copy(*object);
Vector<std::string> r_errors;
bke::GeometrySet new_geometry = nodes::execute_geometry_nodes_on_geometry(
*node_tree,
op->properties,
compute_context,
std::move(geometry_orig),
r_errors,
[&](nodes::GeoNodesLFUserData &user_data) {
user_data.operator_data = &operator_eval_data;
user_data.log_socket_values = false;
});
for (const std::string &error : r_errors) {
BKE_report(op->reports, RPT_ERROR, error.data());
}
store_result_geometry(*bmain, *scene, *object, std::move(new_geometry));
DEG_id_tag_update(static_cast<ID *>(object->data), ID_RECALC_GEOMETRY);

View File

@ -1130,15 +1130,21 @@ static void modifyGeometry(ModifierData *md,
bke::ModifierComputeContext modifier_compute_context{nullptr, nmd->modifier.name};
Vector<std::string> r_errors;
geometry_set = nodes::execute_geometry_nodes_on_geometry(
tree,
nmd->settings.properties,
modifier_compute_context,
std::move(geometry_set),
r_errors,
[&](nodes::GeoNodesLFUserData &user_data) {
user_data.modifier_data = &modifier_eval_data;
});
for (const std::string &error : r_errors) {
BKE_modifier_set_error(ctx->object, md, error.data());
}
if (logging_enabled(ctx)) {
nmd_orig->runtime->eval_log = std::move(eval_log);
}

View File

@ -57,6 +57,7 @@ bke::GeometrySet execute_geometry_nodes_on_geometry(
const IDProperty *properties,
const ComputeContext &base_compute_context,
bke::GeometrySet input_geometry,
Vector<std::string> &r_errors,
FunctionRef<void(nodes::GeoNodesLFUserData &)> fill_user_data);
void update_input_properties_from_node_tree(const bNodeTree &tree,

View File

@ -24,6 +24,8 @@
#include "FN_field_cpp_type.hh"
#include "FN_lazy_function_execute.hh"
#include <fmt/format.h>
namespace lf = blender::fn::lazy_function;
namespace geo_log = blender::nodes::geo_eval_log;
@ -456,13 +458,36 @@ static MultiValueMap<eAttrDomain, OutputAttributeInfo> find_output_attributes_to
return outputs_by_domain;
}
static StringRefNull component_name(const bke::GeometryComponent::Type type)
{
switch (type) {
case bke::GeometryComponent::Type::Mesh:
return "Mesh";
case bke::GeometryComponent::Type::PointCloud:
return "PointCloud";
case bke::GeometryComponent::Type::Curve:
return "Curve";
case bke::GeometryComponent::Type::Instance:
return "Instance";
case bke::GeometryComponent::Type::Edit:
return "Edit";
case bke::GeometryComponent::Type::GreasePencil:
return "GreasePencil";
case bke::GeometryComponent::Type::Volume:
return "Volume";
}
BLI_assert_unreachable();
return "";
}
/**
* The computed values are stored in newly allocated arrays. They still have to be moved to the
* actual geometry.
*/
static Vector<OutputAttributeToStore> compute_attributes_to_store(
const bke::GeometrySet &geometry,
const MultiValueMap<eAttrDomain, OutputAttributeInfo> &outputs_by_domain)
const MultiValueMap<eAttrDomain, OutputAttributeInfo> &outputs_by_domain,
Vector<std::string> &r_errors)
{
Vector<OutputAttributeToStore> attributes_to_store;
for (const auto component_type : {bke::GeometryComponent::Type::Mesh,
@ -486,7 +511,15 @@ static Vector<OutputAttributeToStore> compute_attributes_to_store(
fn::FieldEvaluator field_evaluator{field_context, domain_size};
for (const OutputAttributeInfo &output_info : outputs_info) {
const CPPType &type = output_info.field.cpp_type();
const eCustomDataType data_type = bke::cpp_type_to_custom_data_type(type);
const bke::AttributeValidator validator = attributes.lookup_validator(output_info.name);
if (!validator.validate_meta_data_if_necessary({domain, data_type})) {
r_errors.append(
fmt::format(TIP_("Output Attribute {} can not be captured by {} component"),
output_info.name.data(),
component_name(component_type).data()));
continue;
}
OutputAttributeToStore store{
component_type,
domain,
@ -548,6 +581,7 @@ static void store_computed_output_attributes(
static void store_output_attributes(bke::GeometrySet &geometry,
const bNodeTree &tree,
const IDProperty *properties,
Vector<std::string> &r_errors,
Span<GMutablePointer> output_values)
{
/* All new attribute values have to be computed before the geometry is actually changed. This is
@ -555,7 +589,7 @@ static void store_output_attributes(bke::GeometrySet &geometry,
MultiValueMap<eAttrDomain, OutputAttributeInfo> outputs_by_domain =
find_output_attributes_to_store(tree, properties, output_values);
Vector<OutputAttributeToStore> attributes_to_store = compute_attributes_to_store(
geometry, outputs_by_domain);
geometry, outputs_by_domain, r_errors);
store_computed_output_attributes(geometry, attributes_to_store);
}
@ -564,6 +598,7 @@ bke::GeometrySet execute_geometry_nodes_on_geometry(
const IDProperty *properties,
const ComputeContext &base_compute_context,
bke::GeometrySet input_geometry,
Vector<std::string> &r_errors,
const FunctionRef<void(nodes::GeoNodesLFUserData &)> fill_user_data)
{
const nodes::GeometryNodesLazyFunctionGraphInfo &lf_graph_info =
@ -653,7 +688,7 @@ bke::GeometrySet execute_geometry_nodes_on_geometry(
}
bke::GeometrySet output_geometry = std::move(*param_outputs[0].get<bke::GeometrySet>());
store_output_attributes(output_geometry, btree, properties, param_outputs);
store_output_attributes(output_geometry, btree, properties, r_errors, param_outputs);
for (GMutablePointer &ptr : param_outputs) {
ptr.destruct();