Geometry Nodes: Add a 'Group ID' input to the Fill Curve node #114048
|
@ -31,6 +31,11 @@ static void node_declare(NodeDeclarationBuilder &b)
|
|||
{
|
||||
b.add_input<decl::Geometry>("Curve").supported_type(
|
||||
{GeometryComponent::Type::Curve, GeometryComponent::Type::GreasePencil});
|
||||
b.add_input<decl::Int>("Group ID")
|
||||
.supports_field()
|
||||
Douglas-Paul marked this conversation as resolved
|
||||
.hide_value()
|
||||
.description(
|
||||
"An index used to group curves together. Filling is done separately for each group");
|
||||
b.add_output<decl::Geometry>("Mesh");
|
||||
}
|
||||
|
||||
|
@ -75,43 +80,178 @@ static meshintersect::CDT_result<double> do_cdt(const bke::CurvesGeometry &curve
|
|||
return result;
|
||||
}
|
||||
|
||||
/* Converts the CDT result into a Mesh. */
|
||||
static Mesh *cdt_to_mesh(const meshintersect::CDT_result<double> &result)
|
||||
static meshintersect::CDT_result<double> do_cdt_with_mask(const bke::CurvesGeometry &curves,
|
||||
const CDT_output_type output_type,
|
||||
const IndexMask &mask)
|
||||
{
|
||||
const int vert_len = result.vert.size();
|
||||
const int edge_len = result.edge.size();
|
||||
const int face_len = result.face.size();
|
||||
int loop_len = 0;
|
||||
for (const Vector<int> &face : result.face) {
|
||||
loop_len += face.size();
|
||||
}
|
||||
const OffsetIndices points_by_curve = curves.evaluated_points_by_curve();
|
||||
const Span<float3> positions = curves.evaluated_positions();
|
||||
|
||||
Mesh *mesh = BKE_mesh_new_nomain(vert_len, edge_len, face_len, loop_len);
|
||||
MutableSpan<float3> positions = mesh->vert_positions_for_write();
|
||||
mesh->edges_for_write().copy_from(result.edge.as_span().cast<int2>());
|
||||
MutableSpan<int> face_offsets = mesh->face_offsets_for_write();
|
||||
MutableSpan<int> corner_verts = mesh->corner_verts_for_write();
|
||||
int vert_len = 0;
|
||||
mask.foreach_index([&](const int i) { vert_len += points_by_curve[i].size(); });
|
||||
|
||||
for (const int i : IndexRange(result.vert.size())) {
|
||||
positions[i] = float3(float(result.vert[i].x), float(result.vert[i].y), 0.0f);
|
||||
}
|
||||
int i_loop = 0;
|
||||
for (const int i : IndexRange(result.face.size())) {
|
||||
face_offsets[i] = i_loop;
|
||||
for (const int j : result.face[i].index_range()) {
|
||||
corner_verts[i_loop] = result.face[i][j];
|
||||
i_loop++;
|
||||
meshintersect::CDT_input<double> input;
|
||||
input.need_ids = false;
|
||||
input.vert.reinitialize(vert_len);
|
||||
input.face.reinitialize(mask.size());
|
||||
|
||||
Array<int> offsets_data(mask.size() + 1);
|
||||
const OffsetIndices points_by_curve_masked = offset_indices::gather_selected_offsets(
|
||||
Douglas-Paul marked this conversation as resolved
Outdated
Hans Goudey
commented
When writing this sort of code, we're always looking out for how we can make the serial part boil down to just integer addition. Here's my attempt at that:
Not tested, but I think it works logically. In #111061 I'm also refactoring the CDT code to use something more like the When writing this sort of code, we're always looking out for how we can make the serial part boil down to just integer addition. Here's my attempt at that:
```cpp
Array<int> offsets_data(mask.size() + 1);
const OffsetIndices points_by_curve_masked = offset_indices::gather_selected_offsets(
points_by_curve, mask, offsets_data);
mask.foreach_index(GrainSize(1024), [&](const int src_curve, const int dst_curve) {
const IndexRange src_points = points_by_curve[src_curve];
const IndexRange dst_points = points_by_curve_masked[dst_curve];
for (const int i : src_points.index_range()) {
const int src = src_points[i];
const int dst = dst_points[i];
input.vert[dst] = double2(positions[src].x, positions[src].y);
}
input.face[dst_curve].resize(src_points.size());
MutableSpan<int> face_verts = input.face[dst_curve];
array_utils::fill_index_range<int>(input.face[dst_curve], dst_points.start());
});
```
Not tested, but I think it works logically. In #111061 I'm also refactoring the CDT code to use something more like the `OffsetIndices` format, which would remove the need for the "vector of vectors" approach which is much slower.
Douglas Paul
commented
Thank you, this makes a lot of sense. Just like "thinking with Portals", it will take some practice to get used to "thinking with offsets". This inspired me to similarly re-implement Thank you, this makes a lot of sense. Just like "thinking with Portals", it will take some practice to get used to "thinking with offsets".
This inspired me to similarly re-implement `cdts_to_mesh()` using offsets.
|
||||
points_by_curve, mask, offsets_data);
|
||||
|
||||
mask.foreach_index(GrainSize(1024), [&](const int src_curve, const int dst_curve) {
|
||||
const IndexRange src_points = points_by_curve[src_curve];
|
||||
const IndexRange dst_points = points_by_curve_masked[dst_curve];
|
||||
|
||||
for (const int i : src_points.index_range()) {
|
||||
const int src = src_points[i];
|
||||
const int dst = dst_points[i];
|
||||
input.vert[dst] = double2(positions[src].x, positions[src].y);
|
||||
}
|
||||
|
||||
input.face[dst_curve].resize(src_points.size());
|
||||
array_utils::fill_index_range<int>(input.face[dst_curve], dst_points.start());
|
||||
});
|
||||
|
||||
meshintersect::CDT_result<double> result = delaunay_2d_calc(input, output_type);
|
||||
return result;
|
||||
}
|
||||
|
||||
static Array<meshintersect::CDT_result<double>> do_group_aware_cdt(
|
||||
const bke::CurvesGeometry &curves,
|
||||
const CDT_output_type output_type,
|
||||
const Field<int> &group_index_field)
|
||||
{
|
||||
const bke::GeometryFieldContext field_context{curves, ATTR_DOMAIN_CURVE};
|
||||
fn::FieldEvaluator data_evaluator{field_context, curves.curves_num()};
|
||||
data_evaluator.add(group_index_field);
|
||||
data_evaluator.evaluate();
|
||||
const VArray<int> curve_group_ids = data_evaluator.get_evaluated<int>(0);
|
||||
|
||||
if (curve_group_ids.is_single()) {
|
||||
return {do_cdt(curves, output_type)};
|
||||
Douglas-Paul marked this conversation as resolved
Outdated
Hans Goudey
commented
This can have a special case for A simpler way to do this that will help optimization in the future is using a combination of This can have a special case for `curve_group_ids.is_single()` to avoid building selections.
A simpler way to do this that will help optimization in the future is using a combination of `VectorSet<int>` and `IndexMask::from_groups`. There are some examples of that already.
Douglas Paul
commented
Yeah, that had seemed like too much of a corner case to be worth adding special handling for, but I suppose it does make sense since it's low complexity.
Okay, I made the changes based off of how I see things being done in > This can have a special case for `curve_group_ids.is_single()` to avoid building selections.
Yeah, that had seemed like too much of a corner case to be worth adding special handling for, but I suppose it does make sense since it's low complexity.
> A simpler way to do this that will help optimization in the future is using a combination of `VectorSet<int>` and `IndexMask::from_groups`. There are some examples of that already.
Okay, I made the changes based off of how I see things being done in `node_geo_index_of_nearest.cc`. I'm not sure I agree it's _simpler_, but I understand how it's likely more efficient.
Hans Goudey
commented
Right, good point. Less so about simplicity I guess, more about making the pattern recognizable and making it use common code, so we can optimize them all at once at some point. Right, good point. Less so about simplicity I guess, more about making the pattern recognizable and making it use common code, so we can optimize them all at once at some point.
|
||||
}
|
||||
|
||||
/* The delaunay triangulation doesn't seem to return all of the necessary edges, even in
|
||||
const VArraySpan<int> group_ids_span(curve_group_ids);
|
||||
const int domain_size = group_ids_span.size();
|
||||
|
||||
VectorSet<int> group_indexing(group_ids_span);
|
||||
const int groups_num = group_indexing.size();
|
||||
|
||||
IndexMaskMemory mask_memory;
|
||||
Array<IndexMask> group_masks(groups_num);
|
||||
Douglas-Paul marked this conversation as resolved
Hans Goudey
commented
This can be done in the constructor:
This can be done in the constructor:
```cpp
const VectorSet<int> group_indexing(group_ids_span);
const int groups_num = group_indexing.size();
```
Douglas Paul
commented
I was going to let you know these lines were copied from I was going to let you know these lines were copied from `node_geo_index_of_nearest.cc` so you could apply the same fix there, but I see [you already did that](https://projects.blender.org/blender/blender/commit/df4df75317f93516fe7079a455604bc66aa4409b).
|
||||
IndexMask::from_groups<int>(
|
||||
IndexMask(domain_size),
|
||||
mask_memory,
|
||||
[&](const int i) {
|
||||
const int group_id = group_ids_span[i];
|
||||
return group_indexing.index_of(group_id);
|
||||
},
|
||||
group_masks);
|
||||
|
||||
Array<meshintersect::CDT_result<double>> cdt_results(groups_num);
|
||||
|
||||
/* The grain size should be larger as each group gets smaller. */
|
||||
const int avg_group_size = domain_size / groups_num;
|
||||
const int grain_size = std::max(8192 / avg_group_size, 1);
|
||||
threading::parallel_for(IndexRange(groups_num), grain_size, [&](const IndexRange range) {
|
||||
for (const int group_index : range) {
|
||||
const IndexMask &mask = group_masks[group_index];
|
||||
cdt_results[group_index] = do_cdt_with_mask(curves, output_type, mask);
|
||||
}
|
||||
});
|
||||
|
||||
return cdt_results;
|
||||
}
|
||||
|
||||
/* Converts multiple CDT results into a single Mesh. */
|
||||
static Mesh *cdts_to_mesh(const Span<meshintersect::CDT_result<double>> results)
|
||||
{
|
||||
/* Converting a single CDT result to a Mesh would be simple because the indices could be re-used.
|
||||
* However, in the general case here we need to combine several CDT results into a single Mesh,
|
||||
* which requires us to map the original indices to a new set of indices.
|
||||
* In order to allow for parallelization when appropriate, this implementation starts by
|
||||
* determining (for each domain) what range of indices in the final mesh data will be used for
|
||||
* each CDT result. The index ranges are represented as offsets, which are referred to as "group
|
||||
* offsets" to distinguish them from the other types of offsets we need to work with here.
|
||||
* Since it's likely that most invocations will only have a single CDT result, it's important
|
||||
* that case is made as optimal as feasible. */
|
||||
|
||||
Array<int> vert_groups_data(results.size() + 1);
|
||||
Array<int> edge_groups_data(results.size() + 1);
|
||||
Array<int> face_groups_data(results.size() + 1);
|
||||
Array<int> loop_groups_data(results.size() + 1);
|
||||
threading::parallel_for(results.index_range(), 1024, [&](const IndexRange results_range) {
|
||||
for (const int i_result : results_range) {
|
||||
const meshintersect::CDT_result<double> &result = results[i_result];
|
||||
vert_groups_data[i_result] = result.vert.size();
|
||||
Douglas-Paul marked this conversation as resolved
Outdated
Hans Goudey
commented
We should try to avoid duplicating the logic in Also, you can pass We should try to avoid duplicating the logic in `cdt_to_mesh`.
A better approach is probably creating the mesh, then filling each slice corresponding to a CDT result. Then the 1 mesh case is handled by that by default.
Also, you can pass `const Span<meshintersect::CDT_result` rather than a const reference to a vector.
Douglas Paul
commented
I definitely agree it's best to avoid duplicating the logic, but I don't see a way to combine them without a bit of a performance hit to the presumably much more common case of a single group. Could you elaborate on what you mean by "filling each slice corresponding to a CDT result"? It sounds like you're talking about looping over the CDT results and adding each to the mesh, but that's what
Ah, yes, thank you. It will be a while before things like that jump out at me. > We should try to avoid duplicating the logic in `cdt_to_mesh`.
> A better approach is probably creating the mesh, then filling each slice corresponding to a CDT result. Then the 1 mesh case is handled by that by default.
I definitely agree it's best to avoid duplicating the logic, but I don't see a way to combine them without a bit of a performance hit to the presumably much more common case of a single group.
Could you elaborate on what you mean by "filling each slice corresponding to a CDT result"? It sounds like you're talking about looping over the CDT results and adding each to the mesh, but that's what `cdts_to_mesh()` is already doing. I could [eliminate `cdt_to_mesh()`](https://projects.blender.org/Douglas-Paul/blender/commit/dfa046b9223e50d163ac3b6da7a6974d99d7a491), but again I'm not sure if that's worth the added complexity for the simple cases.
> Also, you can pass `const Span<meshintersect::CDT_result` rather than a const reference to a vector.
Ah, yes, thank you. It will be a while before things like that jump out at me.
Hans Goudey
commented
Yes, I'd probably just remove Yes, I'd probably just remove `cdt_to_mesh` and make sure `cdts_to_mesh` isn't doing too much extra work in the 1 mesh case. But yeah, this can be changed later too.
|
||||
edge_groups_data[i_result] = result.edge.size();
|
||||
face_groups_data[i_result] = result.face.size();
|
||||
int loop_len = 0;
|
||||
for (const Vector<int> &face : result.face) {
|
||||
loop_len += face.size();
|
||||
}
|
||||
loop_groups_data[i_result] = loop_len;
|
||||
}
|
||||
});
|
||||
|
||||
const OffsetIndices vert_groups = offset_indices::accumulate_counts_to_offsets(vert_groups_data);
|
||||
const OffsetIndices edge_groups = offset_indices::accumulate_counts_to_offsets(edge_groups_data);
|
||||
const OffsetIndices face_groups = offset_indices::accumulate_counts_to_offsets(face_groups_data);
|
||||
const OffsetIndices loop_groups = offset_indices::accumulate_counts_to_offsets(loop_groups_data);
|
||||
|
||||
Mesh *mesh = BKE_mesh_new_nomain(vert_groups.total_size(),
|
||||
edge_groups.total_size(),
|
||||
face_groups.total_size(),
|
||||
loop_groups.total_size());
|
||||
|
||||
MutableSpan<float3> all_positions = mesh->vert_positions_for_write();
|
||||
MutableSpan<int2> all_edges = mesh->edges_for_write();
|
||||
MutableSpan<int> all_face_offsets = mesh->face_offsets_for_write();
|
||||
MutableSpan<int> all_corner_verts = mesh->corner_verts_for_write();
|
||||
|
||||
threading::parallel_for(results.index_range(), 1024, [&](const IndexRange results_range) {
|
||||
for (const int i_result : results_range) {
|
||||
const meshintersect::CDT_result<double> &result = results[i_result];
|
||||
const IndexRange verts_range = vert_groups[i_result];
|
||||
const IndexRange edges_range = edge_groups[i_result];
|
||||
const IndexRange faces_range = face_groups[i_result];
|
||||
const IndexRange loops_range = loop_groups[i_result];
|
||||
|
||||
MutableSpan<float3> positions = all_positions.slice(verts_range);
|
||||
for (const int i : result.vert.index_range()) {
|
||||
positions[i] = float3(float(result.vert[i].x), float(result.vert[i].y), 0.0f);
|
||||
}
|
||||
|
||||
MutableSpan<int2> edges = all_edges.slice(edges_range);
|
||||
for (const int i : result.edge.index_range()) {
|
||||
edges[i] = int2(result.edge[i].first + verts_range.start(),
|
||||
result.edge[i].second + verts_range.start());
|
||||
}
|
||||
|
||||
MutableSpan<int> face_offsets = all_face_offsets.slice(faces_range);
|
||||
MutableSpan<int> corner_verts = all_corner_verts.slice(loops_range);
|
||||
int i_face_corner = 0;
|
||||
for (const int i_face : result.face.index_range()) {
|
||||
face_offsets[i_face] = i_face_corner + loops_range.start();
|
||||
for (const int i_corner : result.face[i_face].index_range()) {
|
||||
corner_verts[i_face_corner] = result.face[i_face][i_corner] + verts_range.start();
|
||||
i_face_corner++;
|
||||
}
|
||||
}
|
||||
}
|
||||
});
|
||||
|
||||
/* The delaunay triangulation doesn't seem to return all of the necessary all_edges, even in
|
||||
* triangulation mode. */
|
||||
BKE_mesh_calc_edges(mesh, true, false);
|
||||
BKE_mesh_smooth_flag_set(mesh, false);
|
||||
return mesh;
|
||||
}
|
||||
|
||||
static void curve_fill_calculate(GeometrySet &geometry_set, const GeometryNodeCurveFillMode mode)
|
||||
static void curve_fill_calculate(GeometrySet &geometry_set,
|
||||
const GeometryNodeCurveFillMode mode,
|
||||
const Field<int> &group_index)
|
||||
{
|
||||
const CDT_output_type output_type = (mode == GEO_NODE_CURVE_FILL_MODE_NGONS) ?
|
||||
CDT_CONSTRAINTS_VALID_BMESH_WITH_HOLES :
|
||||
|
@ -120,8 +260,9 @@ static void curve_fill_calculate(GeometrySet &geometry_set, const GeometryNodeCu
|
|||
const Curves &curves_id = *geometry_set.get_curves();
|
||||
const bke::CurvesGeometry &curves = curves_id.geometry.wrap();
|
||||
if (curves.curves_num() > 0) {
|
||||
const meshintersect::CDT_result<double> results = do_cdt(curves, output_type);
|
||||
Mesh *mesh = cdt_to_mesh(results);
|
||||
const Array<meshintersect::CDT_result<double>> results = do_group_aware_cdt(
|
||||
curves, output_type, group_index);
|
||||
Mesh *mesh = cdts_to_mesh(results);
|
||||
geometry_set.replace_mesh(mesh);
|
||||
}
|
||||
geometry_set.replace_curves(nullptr);
|
||||
|
@ -140,8 +281,9 @@ static void curve_fill_calculate(GeometrySet &geometry_set, const GeometryNodeCu
|
|||
if (src_curves.curves_num() == 0) {
|
||||
continue;
|
||||
}
|
||||
const meshintersect::CDT_result<double> results = do_cdt(src_curves, output_type);
|
||||
mesh_by_layer[layer_index] = cdt_to_mesh(results);
|
||||
const Array<meshintersect::CDT_result<double>> results = do_group_aware_cdt(
|
||||
src_curves, output_type, group_index);
|
||||
mesh_by_layer[layer_index] = cdts_to_mesh(results);
|
||||
}
|
||||
if (!mesh_by_layer.is_empty()) {
|
||||
InstancesComponent &instances_component =
|
||||
|
@ -172,12 +314,13 @@ static void curve_fill_calculate(GeometrySet &geometry_set, const GeometryNodeCu
|
|||
static void node_geo_exec(GeoNodeExecParams params)
|
||||
{
|
||||
GeometrySet geometry_set = params.extract_input<GeometrySet>("Curve");
|
||||
Field<int> group_index = params.extract_input<Field<int>>("Group ID");
|
||||
|
||||
const NodeGeometryCurveFill &storage = node_storage(params.node());
|
||||
const GeometryNodeCurveFillMode mode = (GeometryNodeCurveFillMode)storage.mode;
|
||||
|
||||
geometry_set.modify_geometry_sets(
|
||||
[&](GeometrySet &geometry_set) { curve_fill_calculate(geometry_set, mode); });
|
||||
[&](GeometrySet &geometry_set) { curve_fill_calculate(geometry_set, mode, group_index); });
|
||||
|
||||
params.set_output("Mesh", std::move(geometry_set));
|
||||
}
|
||||
|
|
.default_value(0)
is unnecessary, that's the implicit default already.