Page MenuHome
Differential D16009 Diff 57842 source/blender/nodes/geometry/nodes/node_geo_curve_fillet.cc

Changeset View

Standalone View

View Options

source/blender/nodes/geometry/nodes/node_geo_curve_fillet.cc

Show All 23 Lines b.add_input<decl::Float>(N_("Radius"))
.min(0.0f) .min(0.0f)
.max(FLT_MAX) .max(FLT_MAX)
.subtype(PropertySubType::PROP_DISTANCE) .subtype(PropertySubType::PROP_DISTANCE)
.default_value(0.25f) .default_value(0.25f)
.supports_field(); .supports_field();
b.add_input<decl::Bool>(N_("Limit Radius")) b.add_input<decl::Bool>(N_("Limit Radius"))
.description( .description(
N_("Limit the maximum value of the radius in order to avoid overlapping fillets")); N_("Limit the maximum value of the radius in order to avoid overlapping fillets"));
b.add_input<decl::Bool>(N_("Selection")).default_value(true).hide_value().supports_field();
b.add_output<decl::Geometry>(N_("Curve")); b.add_output<decl::Geometry>(N_("Curve"));
} }
HooglyBooglyUnsubmitted
Done
Inline Actions

On other nodes the selection goes right below the geometry input, it would probably make sense to be consistent here.

HooglyBoogly: On other nodes the selection goes right below the geometry input, it would probably make sense…
yemountAuthorUnsubmitted
Done
Inline Actions

Moved.

yemount: Moved.
static void node_layout(uiLayout *layout, bContext *UNUSED(C), PointerRNA *ptr) static void node_layout(uiLayout *layout, bContext *UNUSED(C), PointerRNA *ptr)
{ {
uiItemR(layout, ptr, "mode", UI_ITEM_R_EXPAND, nullptr, ICON_NONE); uiItemR(layout, ptr, "mode", UI_ITEM_R_EXPAND, nullptr, ICON_NONE);
} }
static void node_init(bNodeTree *UNUSED(tree), bNode *node) static void node_init(bNodeTree *UNUSED(tree), bNode *node)
{ {
NodeGeometryCurveFillet *data = MEM_cnew<NodeGeometryCurveFillet>(__func__); NodeGeometryCurveFillet *data = MEM_cnew<NodeGeometryCurveFillet>(__func__);
data->mode = GEO_NODE_CURVE_FILLET_BEZIER; data->mode = GEO_NODE_CURVE_FILLET_BEZIER;
node->storage = data; node->storage = data;
} }
static void node_update(bNodeTree *ntree, bNode *node) static void node_update(bNodeTree *ntree, bNode *node)
{ {
const NodeGeometryCurveFillet &storage = node_storage(*node); const NodeGeometryCurveFillet &storage = node_storage(*node);
const GeometryNodeCurveFilletMode mode = (GeometryNodeCurveFilletMode)storage.mode; const GeometryNodeCurveFilletMode mode = (GeometryNodeCurveFilletMode)storage.mode;
bNodeSocket *poly_socket = ((bNodeSocket *)node->inputs.first)->next; bNodeSocket *poly_socket = ((bNodeSocket *)node->inputs.first)->next;
HooglyBooglyUnsubmitted
Not Done
Inline Actions

The order of sockets matters with this problem because of the socket order is used here to hide the "count" socket in Bezier mode IIRC.

This is a common pattern in nodes that hide or show sockets. It's really brittle and we're hoping to improve it in the future by moving this info to the node declaration above

HooglyBoogly: The order of sockets matters with this problem because of the socket order is used here to hide…
nodeSetSocketAvailability(ntree, poly_socket, mode == GEO_NODE_CURVE_FILLET_POLY); nodeSetSocketAvailability(ntree, poly_socket, mode == GEO_NODE_CURVE_FILLET_POLY);
} }
static void node_geo_exec(GeoNodeExecParams params) static void node_geo_exec(GeoNodeExecParams params)
{ {
GeometrySet geometry_set = params.extract_input<GeometrySet>("Curve"); GeometrySet geometry_set = params.extract_input<GeometrySet>("Curve");
const NodeGeometryCurveFillet &storage = node_storage(params.node()); const NodeGeometryCurveFillet &storage = node_storage(params.node());
const GeometryNodeCurveFilletMode mode = (GeometryNodeCurveFilletMode)storage.mode; const GeometryNodeCurveFilletMode mode = (GeometryNodeCurveFilletMode)storage.mode;
Field<float> radius_field = params.extract_input<Field<float>>("Radius"); Field<float> radius_field = params.extract_input<Field<float>>("Radius");
const bool limit_radius = params.extract_input<bool>("Limit Radius"); const bool limit_radius = params.extract_input<bool>("Limit Radius");
Field<bool> selection_field = params.extract_input<Field<bool>>("Selection");
std::optional<Field<int>> count_field; std::optional<Field<int>> count_field;
if (mode == GEO_NODE_CURVE_FILLET_POLY) { if (mode == GEO_NODE_CURVE_FILLET_POLY) {
count_field.emplace(params.extract_input<Field<int>>("Count")); count_field.emplace(params.extract_input<Field<int>>("Count"));
} }
geometry_set.modify_geometry_sets([&](GeometrySet &geometry_set) { geometry_set.modify_geometry_sets([&](GeometrySet &geometry_set) {
if (!geometry_set.has_curves()) { if (!geometry_set.has_curves()) {
return; return;
} }
const Curves &curves_id = *geometry_set.get_curves_for_read(); const Curves &curves_id = *geometry_set.get_curves_for_read();
const bke::CurvesGeometry &curves = bke::CurvesGeometry::wrap(curves_id.geometry); const bke::CurvesGeometry &curves = bke::CurvesGeometry::wrap(curves_id.geometry);
bke::CurvesFieldContext context{curves, ATTR_DOMAIN_POINT}; bke::CurvesFieldContext context{curves, ATTR_DOMAIN_POINT};
fn::FieldEvaluator evaluator{context, curves.points_num()}; fn::FieldEvaluator evaluator{context, curves.points_num()};
evaluator.set_selection(selection_field);
evaluator.add(radius_field); evaluator.add(radius_field);
switch (mode) { switch (mode) {
case GEO_NODE_CURVE_FILLET_BEZIER: { case GEO_NODE_CURVE_FILLET_BEZIER: {
evaluator.evaluate(); evaluator.evaluate();
IndexMask selected_points = evaluator.get_evaluated_selection_as_mask();
if (selected_points.is_empty()) {
break;
}
bke::CurvesGeometry dst_curves = geometry::fillet_curves_bezier( bke::CurvesGeometry dst_curves = geometry::fillet_curves_bezier(
curves, curves.curves_range(), evaluator.get_evaluated<float>(0), limit_radius); curves,
selected_points,
evaluator.get_evaluated<float>(0), limit_radius);
Curves *dst_curves_id = bke::curves_new_nomain(std::move(dst_curves)); Curves *dst_curves_id = bke::curves_new_nomain(std::move(dst_curves));
bke::curves_copy_parameters(curves_id, *dst_curves_id); bke::curves_copy_parameters(curves_id, *dst_curves_id);
geometry_set.replace_curves(dst_curves_id); geometry_set.replace_curves(dst_curves_id);
break; break;
} }
case GEO_NODE_CURVE_FILLET_POLY: { case GEO_NODE_CURVE_FILLET_POLY: {
evaluator.add(*count_field); evaluator.add(*count_field);
evaluator.evaluate(); evaluator.evaluate();
IndexMask selected_points = evaluator.get_evaluated_selection_as_mask();
if (selected_points.is_empty()) {
break;
}
bke::CurvesGeometry dst_curves = geometry::fillet_curves_poly( bke::CurvesGeometry dst_curves = geometry::fillet_curves_poly(
curves, curves,
curves.curves_range(), selected_points,
HooglyBooglyUnsubmitted
Done
Inline Actions

You could pass the selected_points variable here instead of retrieving it again, right?

HooglyBoogly: You could pass the `selected_points` variable here instead of retrieving it again, right?
evaluator.get_evaluated<float>(0), evaluator.get_evaluated<float>(0),
evaluator.get_evaluated<int>(1), evaluator.get_evaluated<int>(1),
limit_radius); limit_radius);
Curves *dst_curves_id = bke::curves_new_nomain(std::move(dst_curves)); Curves *dst_curves_id = bke::curves_new_nomain(std::move(dst_curves));
bke::curves_copy_parameters(curves_id, *dst_curves_id); bke::curves_copy_parameters(curves_id, *dst_curves_id);
geometry_set.replace_curves(dst_curves_id); geometry_set.replace_curves(dst_curves_id);
break; break;
} }
} }
ModerUnsubmitted
Not Done
Inline Actions
fn::FieldEvaluator evaluator{context, curves.points_num()};
evaluator.set_selection(selection_field);
evaluator.add(radius_field);
+ if (mode == GEO_NODE_CURVE_FILLET_POLY){
+ evaluator.add(*count_field);
+ }
- switch (mode) {
- case GEO_NODE_CURVE_FILLET_BEZIER: {
- evaluator.evaluate();
- IndexMask selected_points = evaluator.get_evaluated_selection_as_mask();
- if (selected_points.is_empty()) {
- break;
- }
- bke::CurvesGeometry dst_curves = geometry::fillet_curves_bezier(
+ evaluator.evaluate();
+
+ IndexMask selected_points = evaluator.get_evaluated_selection_as_mask();
+ if (selected_points.is_empty()) {
+ break;
+ }
+
+ VArray<float> point_radius = evaluator.get_evaluated<float>(0);
+
+ bke::CurvesGeometry dst_curves;
+
+ switch (mode) {
+ case GEO_NODE_CURVE_FILLET_BEZIER: {
+ dst_curves = geometry::fillet_curves_bezier(
curves,
- selected_points,
- evaluator.get_evaluated<float>(0), limit_radius);
- Curves *dst_curves_id = bke::curves_new_nomain(std::move(dst_curves));
- bke::curves_copy_parameters(curves_id, *dst_curves_id);
- geometry_set.replace_curves(dst_curves_id);
+ dst_curves,
+ std::move(selected_points),
+ point_radius, limit_radius);
break;
}
case GEO_NODE_CURVE_FILLET_POLY: {
- evaluator.add(*count_field);
- evaluator.evaluate();
- IndexMask selected_points = evaluator.get_evaluated_selection_as_mask();
- if (selected_points.is_empty()) {
- break;
- }
- bke::CurvesGeometry dst_curves = geometry::fillet_curves_poly(
+ dst_curves = geometry::fillet_curves_poly(
curves,
+ dst_curves,
selected_points,
- evaluator.get_evaluated<float>(0),
+ std::move(selected_points),
evaluator.get_evaluated<int>(1),
limit_radius);
- Curves *dst_curves_id = bke::curves_new_nomain(std::move(dst_curves));
- bke::curves_copy_parameters(curves_id, *dst_curves_id);
- geometry_set.replace_curves(dst_curves_id);
break;
}
}
- });
+ Curves *dst_curves_id = bke::curves_new_nomain(std::move(dst_curves));
+ bke::curves_copy_parameters(curves_id, *dst_curves_id);
+ geometry_set.replace_curves(dst_curves_id); });
params.set_output("Curve", std::move(geometry_set));

It seems that now more than one argument needs to be received in both places. Better to do it once. Other opinions are possible, just sensations

Moder: It seems that now more than one argument needs to be received in both places. Better to do it…
}); });
params.set_output("Curve", std::move(geometry_set)); params.set_output("Curve", std::move(geometry_set));
} }
} // namespace blender::nodes::node_geo_curve_fillet_cc } // namespace blender::nodes::node_geo_curve_fillet_cc
void register_node_type_geo_curve_fillet() void register_node_type_geo_curve_fillet()
Show All 15 Lines