blender
/
blender
134
421
Fork 604
Code Issues 6.4k Pull Requests 770 Projects 19 Wiki Activity

Mesh: Rewrite split edges algorithm #110661

Merged
Hans Goudey merged 78 commits from HooglyBoogly/blender:split-edges-rewrite-2 into main 2023-08-30 14:23:49 +02:00
Conversation 8 Commits 78 Files Changed 1 +12 -21
1 changed files with 12 additions and 21 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit 026e41b5f0 - Show all commits

33
source/blender/geometry/intern/mesh_split_edges.cc
View File

@ -294,7 +294,7 @@ static void calc_updated_corner_verts(const int orig_verts_num,
}
/**
* When each of an edge's vertices only have a single grouped edge fan, the vertices will not be
* When each of an edge's vertices only have a single grouped corner fan, the vertices will not be
* split, meaning all output edges will reuse the same vertices.
*/
static BitVector<> calc_deduplicated_new_edges(const int orig_verts_num,
@ -321,10 +321,9 @@ static Vector<int2> add_new_edges(const OffsetIndices<int> faces,
const Span<int> orig_corner_edges,
const BitSpan selection,
const Span<int> new_corner_verts,
const int new_edge_start,
const BitSpan output_single_edge,
MutableSpan<int2> edges,
MutableSpan<int> new_corner_edges,
MutableSpan<int> corner_edges,
Vector<int2> &new_edges,
Vector<int> &new_edge_orig_indices)
{
@ -332,23 +331,22 @@ static Vector<int2> add_new_edges(const OffsetIndices<int> faces,
for (const int i : faces.index_range()) {
const IndexRange face = faces[i];
for (const int corner : face) {
const int corner_next = bke::mesh::face_corner_next(face, corner);
const int edge = orig_corner_edges[corner];
if (!selection[edge]) {
continue;
}
const int vert_1 = new_corner_verts[corner];
const int vert_2 = new_corner_verts[bke::mesh::face_corner_next(face, corner)];
if (output_single_edge[edge]) {
BLI_assert(OrderedEdge(edges[edge]) == OrderedEdge(vert_1, vert_2));
const int2 new_edge(new_corner_verts[corner], new_corner_verts[corner_next]);
if (!selection[edge] || output_single_edge[edge]) {
/* Even if an edge wasn't selected, its vertex indices can be updated when neighboring
* edges are selected and new vertices are created for the corner fans of its vertices. */
edges[edge] = new_edge;
continue;
}
if (edge_used[edge]) {
HooglyBoogly marked this conversation as resolved Outdated
Wannes Malfait commented 2023-08-29 22:29:14 +02:00
Outdated
Review
Copy Link

Took me a while to realize what new_edge_offsets was storing. Something like num_new_edges_per_old_edge would be clearer to me (but also too long probably). I'm fine if you leave it as is, since the current name does make sense as well in hindsight. What confused me was that "offset" is also used in other arrays like offset_no_merge or merged_offsets in a different way (cumulative offsets).

Took me a while to realize what `new_edge_offsets` was storing. Something like `num_new_edges_per_old_edge` would be clearer to me (but also too long probably). I'm fine if you leave it as is, since the current name does make sense as well in hindsight. What confused me was that "offset" is also used in other arrays like `offset_no_merge` or `merged_offsets` in a different way (cumulative offsets).
new_corner_edges[corner] = new_edge_start + new_edges.size();
new_edges.append(int2(vert_1, vert_2));
corner_edges[corner] = edges.size() + new_edges.size();
new_edges.append(new_edge);
new_edge_orig_indices.append(edge);
}
else {
edges[edge] = int2(vert_1, vert_2);
edges[edge] = new_edge;
edge_used[edge].set();
}
HooglyBoogly marked this conversation as resolved Outdated
Jacques Lucke commented 2023-08-02 16:13:51 +02:00
Outdated
Review
Copy Link

Should use the r_ prefix.

Should use the `r_` prefix.
}
HooglyBoogly marked this conversation as resolved Outdated
Wannes Malfait commented 2023-08-29 22:34:30 +02:00
Outdated
Review
Copy Link

I was not able to figure out why this line is there. I see that this is different from what is set in the loop, but it seems wrong to me to have this here:

  • last() is not necessarily a selected edge
  • I thought the offsets should point to the index the new edge would be located at. Now the last one is pointing out of bounds?

Probably there's something I'm not getting

I was not able to figure out why this line is there. I see that this is different from what is set in the loop, but it seems wrong to me to have this here: - `last()` is not necessarily a selected edge - I thought the offsets should point to the index the new edge would be located at. Now the last one is pointing out of bounds? Probably there's something I'm not getting
Hans Goudey commented 2023-08-29 23:07:33 +02:00
Outdated
Review
Copy Link

Ah, sorry, the extra element is left over from when I was using OffsetIndices with this array (which requires the extra).

Ah, sorry, the extra element is left over from when I was using `OffsetIndices` with this array (which requires the extra).
@ -423,7 +421,7 @@ static Array<int> calc_new_to_old_vert_map(const IndexMask &affected_verts,
void split_edges(Mesh &mesh,
const IndexMask &selected_edges,
const bke::AnonymousAttributePropagationInfo &propagation_info)
const bke::AnonymousAttributePropagationInfo & /*propagation_info*/)
{
const int orig_verts_num = mesh.totvert;
const Span<int2> orig_edges = mesh.edges();
@ -431,16 +429,10 @@ void split_edges(Mesh &mesh,
const Array<int> orig_corner_edges = mesh.corner_edges();
IndexMaskMemory memory;
const IndexMask unselected_edges = selected_edges.complement(orig_edges.index_range(), memory);
const IndexMask affected_verts = vert_selection_from_edge(
orig_edges, selected_edges, orig_verts_num, memory);
const BitVector<> selection_bits = selection_to_bit_vector(selected_edges, orig_edges.size());
const bke::LooseEdgeCache &loose_edges = mesh.loose_edges();
const IndexMask selected_loose_edges = loose_edges.count > 0 ?
IndexMask::from_bits(selected_edges,
loose_edges.is_loose_bits,
memory) :
IndexMask();
Array<int> vert_to_corner_offsets;
Array<int> vert_to_corner_indices;
@ -495,7 +487,6 @@ void split_edges(Mesh &mesh,
orig_corner_edges,
selection_bits,
new_corner_verts,
orig_edges.size(),
single_edges,
mesh.edges_for_write(),
mesh.corner_edges_for_write(),
HooglyBoogly marked this conversation as resolved Outdated
Wannes Malfait commented 2023-08-29 22:25:34 +02:00
Outdated
Review
Copy Link

This is also used for edges, so using vert in the comment and variable name is maybe not the best idea. Maybe a generic element would be good enough?

This is also used for edges, so using `vert` in the comment and variable name is maybe not the best idea. Maybe a generic `element` would be good enough?