blender
/
blender
133
398
Fork 579
Code Issues 6.3k Pull Requests 746 Projects 19 Wiki Activity

Animation: Weight Paint select more/less for faces #105607

Merged
Christoph Lendenfeld merged 13 commits from ChrisLend/blender:weight_paint_grow_sel_face into main 2023-03-31 14:53:12 +02:00
Conversation 6 Commits 13 Files Changed 7 +28 -16
1 changed files with 28 additions and 16 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit d05334b691 - Show all commits

44
source/blender/editors/mesh/editface.cc
View File

@ -350,6 +350,29 @@ void paintface_select_linked(bContext *C, Object *ob, const int mval[2], const b
paintface_flush_flags(C, ob, true, false);
}
static bool poly_has_selected_neighbor(blender::Span<int> edge_indices,
Hans Goudey commented 2023-03-24 14:46:03 +01:00
Outdated
Review
Copy Link

Canonical variable name for the edges of a face is poly_edges

Canonical variable name for the edges of a face is `poly_edges`
blender::Span<MEdge> edges,
blender::Span<bool> select_vert,
const bool face_step)
{
for (const int edge_index : edge_indices) {
const MEdge &edge = edges[edge_index];
/* If a poly is selected, all of its verts are selected too, meaning that neighboring faces
* will have some vertices selected. */
if (face_step) {
if (select_vert[edge.v1] || select_vert[edge.v2]) {
return true;
}
}
else {
if (select_vert[edge.v1] && select_vert[edge.v2]) {
return true;
}
}
}
return false;
}
void paintface_select_more(Mesh *mesh, const bool face_step)
{
using namespace blender;
@ -372,22 +395,11 @@ void paintface_select_more(Mesh *mesh, const bool face_step)
continue;
}
const MPoly &poly = polys[i];
for (const int edge_index : corner_edges.slice(poly.loopstart, poly.totloop)) {
const MEdge &edge = edges[edge_index];
/* If a poly is selected, all of its verts are selected too, meaning that neighboring faces
* will have some vertices selected. */
bool selected_neighbor = false;
if (face_step) {
selected_neighbor = select_vert.span[edge.v1] || select_vert.span[edge.v2];
}
else {
selected_neighbor = select_vert.span[edge.v1] && select_vert.span[edge.v2];
}
if (selected_neighbor) {
select_poly.span[i] = true;
break;
}
if (poly_has_selected_neighbor(corner_edges.slice(poly.loopstart, poly.totloop),
Sybren A. Stüvel commented 2023-03-31 14:28:56 +02:00
Review
Copy Link

The condition can be avoided by doing something like:

const bool has_selected_neighbour = poly_has_selected_neighbor(...);
select_poly.span[i] |= has_selected_neighbour;

Not sure if it's worth it though, you choose @ChrisLend. Could be applied below as well.

The condition can be avoided by doing something like: ```cpp const bool has_selected_neighbour = poly_has_selected_neighbor(...); select_poly.span[i] |= has_selected_neighbour; ``` Not sure if it's worth it though, you choose @ChrisLend. Could be applied below as well.
Christoph Lendenfeld commented 2023-03-31 14:46:15 +02:00
Review
Copy Link

had a look at it but I think it's a bit clearer if the bool is set explicitly so I left it as is

had a look at it but I think it's a bit clearer if the bool is set explicitly so I left it as is
edges,
Hans Goudey commented 2023-03-23 15:19:50 +01:00
Outdated
Review
Copy Link

Blender uses American English spelling, so neighbor instead of neighbor

Blender uses American English spelling, so `neighbor` instead of `neighbor`
Christoph Lendenfeld commented 2023-03-24 14:10:04 +01:00
Outdated
Review
Copy Link

thanks, that always gets me

thanks, that always gets me
select_vert.span,
face_step)) {
select_poly.span[i] = true;
}
}
Hans Goudey commented 2023-03-09 17:01:38 +01:00
Outdated
Review
Copy Link

It might be clearer for these functions to have slightly lower level arguments, like (Mesh &mesh, const bool face_step) in this case. That separates the abstraction levels more clearly, and means this function could be used in other situations where the context is different or the update tags aren't the same.

It might be clearer for these functions to have slightly lower level arguments, like `(Mesh &mesh, const bool face_step)` in this case. That separates the abstraction levels more clearly, and means this function could be used in other situations where the context is different or the update tags aren't the same.
Christoph Lendenfeld commented 2023-03-16 10:53:36 +01:00
Outdated
Review
Copy Link

This might be my limited understanding of C++ but I can't get this to work.
Could it be that because the header file is ED_mesh.h meaning it's pure C so it doesn't understand references?

Edit since it seems to not have linked to your comment.
It was about passing in Mesh &mesh instead of bContext and Object

This might be my limited understanding of C++ but I can't get this to work. Could it be that because the header file is `ED_mesh.h` meaning it's pure C so it doesn't understand references? Edit since it seems to not have linked to your comment. It was about passing in `Mesh &mesh` instead of bContext and Object
Hans Goudey commented 2023-03-18 02:19:02 +01:00
Outdated
Review
Copy Link

Yeah right, a couple options-- keep a function with this signature in the public header, and a static function with the signature I suggested, or use the signature I suggested with a pointer instead of a reference (that's probably the better option IMO).

Mainly I think it's nice to avoid just using the object argument to retrieve the mesh, and it's nice to avoid the null check because this function really shouldn't be concerned with whether there is no mesh, that's the job of somewhere else.

Yeah right, a couple options-- keep a function with this signature in the public header, and a static function with the signature I suggested, or use the signature I suggested with a pointer instead of a reference (that's probably the better option IMO). Mainly I think it's nice to avoid just using the object argument to retrieve the mesh, and it's nice to avoid the null check because this function really shouldn't be concerned with whether there is no mesh, that's the job of somewhere else.
Christoph Lendenfeld commented 2023-03-23 10:55:00 +01:00
Outdated
Review
Copy Link

now takes a Mesh*
a potential clean up is to check if that can be done for other functions as well

now takes a `Mesh*` a potential clean up is to check if that can be done for other functions as well
});