Animation: Weight Paint select more/less for faces #105607
|
@ -4410,6 +4410,8 @@ def km_face_mask(params):
|
|||
{"properties": [("deselect", False)]}),
|
||||
("paint.face_select_linked_pick", {"type": 'L', "value": 'PRESS', "shift": True},
|
||||
{"properties": [("deselect", True)]}),
|
||||
("paint.face_select_more", {"type": 'NUMPAD_PLUS', "value": 'PRESS', "ctrl": True}, None),
|
||||
("paint.face_select_less", {"type": 'NUMPAD_MINUS', "value": 'PRESS', "ctrl": True}, None),
|
||||
])
|
||||
|
||||
return keymap
|
||||
|
|
|
@ -2011,6 +2011,9 @@ class VIEW3D_MT_select_paint_mask(Menu):
|
|||
layout.operator("paint.face_select_all", text="None").action = 'DESELECT'
|
||||
layout.operator("paint.face_select_all", text="Invert").action = 'INVERT'
|
||||
|
||||
layout.operator("paint.face_select_more")
|
||||
layout.operator("paint.face_select_less")
|
||||
|
||||
layout.separator()
|
||||
|
||||
layout.operator("view3d.select_box")
|
||||
|
|
|
@ -419,6 +419,11 @@ void paintface_select_linked(struct bContext *C,
|
|||
struct Object *ob,
|
||||
const int mval[2],
|
||||
bool select);
|
||||
/** Grow the selection of faces.
|
||||
* \param face_step If true will also select faces that only touch on the corner.
|
||||
*/
|
||||
void paintface_select_more(struct Mesh *mesh, bool face_step);
|
||||
void paintface_select_less(struct Mesh *mesh, bool face_step);
|
||||
bool paintface_minmax(struct Object *ob, float r_min[3], float r_max[3]);
|
||||
|
||||
void paintface_hide(struct bContext *C, struct Object *ob, bool unselected);
|
||||
|
|
|
@ -350,6 +350,131 @@ 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> poly_edges,
|
||||
|
||||
blender::Span<MEdge> edges,
|
||||
blender::Span<bool> select_vert,
|
||||
const bool face_step)
|
||||
{
|
||||
for (const int edge_index : poly_edges) {
|
||||
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;
|
||||
|
||||
bke::MutableAttributeAccessor attributes = mesh->attributes_for_write();
|
||||
bke::SpanAttributeWriter<bool> select_poly = attributes.lookup_or_add_for_write_span<bool>(
|
||||
".select_poly", ATTR_DOMAIN_FACE);
|
||||
bke::SpanAttributeWriter<bool> select_vert = attributes.lookup_or_add_for_write_span<bool>(
|
||||
".select_vert", ATTR_DOMAIN_POINT);
|
||||
const VArray<bool> hide_poly = attributes.lookup_or_default<bool>(
|
||||
".hide_poly", ATTR_DOMAIN_FACE, false);
|
||||
|
||||
const Span<MPoly> polys = mesh->polys();
|
||||
Hans Goudey
commented
This is feeling a bit picky, sorry about that, but might as well extract this check for the poly instead of using a This is feeling a bit picky, sorry about that, but might as well extract this check for the poly instead of using a `break` like below (`poly_has_unselected_neighbour`) I think it makes sense for them to be consistent anyway.
|
||||
const Span<int> corner_edges = mesh->corner_edges();
|
||||
const Span<MEdge> edges = mesh->edges();
|
||||
|
||||
threading::parallel_for(select_poly.span.index_range(), 1024, [&](const IndexRange range) {
|
||||
for (const int i : range) {
|
||||
if (select_poly.span[i] || hide_poly[i]) {
|
||||
continue;
|
||||
}
|
||||
const MPoly &poly = polys[i];
|
||||
if (poly_has_selected_neighbor(corner_edges.slice(poly.loopstart, poly.totloop),
|
||||
Sybren A. Stüvel
commented
The condition can be avoided by doing something like:
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.
|
||||
edges,
|
||||
Hans Goudey
commented
Blender uses American English spelling, so Blender uses American English spelling, so `neighbor` instead of `neighbor`
|
||||
select_vert.span,
|
||||
face_step)) {
|
||||
select_poly.span[i] = true;
|
||||
}
|
||||
}
|
||||
Hans Goudey
commented
It might be clearer for these functions to have slightly lower level arguments, like 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.
This might be my limited understanding of C++ but I can't get this to work. Edit since it seems to not have linked to your comment. 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
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.
|
||||
});
|
||||
|
||||
select_poly.finish();
|
||||
ChrisLend marked this conversation as resolved
Outdated
Hans Goudey
commented
Replace the Replace the `BitVector` reference with `const BitSpan`, that will give a better idea of ownership and the fact that this doesn't need to modify the data.
|
||||
select_vert.finish();
|
||||
}
|
||||
|
||||
static bool poly_has_unselected_neighbor(blender::Span<int> poly_edges,
|
||||
blender::Span<MEdge> edges,
|
||||
Hans Goudey
commented
It's not a big deal, but the function could be a bit simpler without the It's not a big deal, but the function could be a bit simpler without the `unselected_neighbor` variable. It's nice to assign a variable a meaningful value in the same expression you initialize it, and that's not really the case here. Given the function name, returning early reads as "poly has an unselected neighbor" anyway.
|
||||
blender::BitSpan verts_of_unselected_faces,
|
||||
const bool face_step)
|
||||
ChrisLend marked this conversation as resolved
Outdated
Hans Goudey
commented
What do you think about removing the What do you think about removing the `.test()` and using `BitRef`'s implicit bool conversion?
|
||||
{
|
||||
for (const int edge_index : poly_edges) {
|
||||
const MEdge &edge = edges[edge_index];
|
||||
if (face_step) {
|
||||
if (verts_of_unselected_faces[edge.v1] || verts_of_unselected_faces[edge.v2]) {
|
||||
return true;
|
||||
}
|
||||
}
|
||||
ChrisLend marked this conversation as resolved
Outdated
Hans Goudey
commented
I assume you're using I assume you're using `std::vector<bool>` because it works as a bitmap internally? `blender::BitVector` should be a better choice here. See the reasoning at the top of that header.
|
||||
else {
|
||||
if (verts_of_unselected_faces[edge.v1] && verts_of_unselected_faces[edge.v2]) {
|
||||
return true;
|
||||
ChrisLend marked this conversation as resolved
Outdated
Hans Goudey
commented
Small thing, but maybe it's a bit clearer to write Small thing, but maybe it's a bit clearer to write `polys.index_range()` than `select_poly.span.index_range()`. That's just semantically closer to the goal of iterating over all polys.
|
||||
}
|
||||
}
|
||||
}
|
||||
return false;
|
||||
}
|
||||
|
||||
void paintface_select_less(Mesh *mesh, const bool face_step)
|
||||
{
|
||||
using namespace blender;
|
||||
|
||||
bke::MutableAttributeAccessor attributes = mesh->attributes_for_write();
|
||||
bke::SpanAttributeWriter<bool> select_poly = attributes.lookup_or_add_for_write_span<bool>(
|
||||
".select_poly", ATTR_DOMAIN_FACE);
|
||||
const VArray<bool> hide_poly = attributes.lookup_or_default<bool>(
|
||||
".hide_poly", ATTR_DOMAIN_FACE, false);
|
||||
|
||||
const Span<MPoly> polys = mesh->polys();
|
||||
Hans Goudey
commented
What do you think about splitting this part into a separate function-- something like I think that would make the logic here a bit simpler, and avoid the need for breaking inside the loop. Something similar would be helpful above too. What do you think about splitting this part into a separate function-- something like ` bool poly_has_unselected_neighbor(Span<MLoop> poly_loops, bool face_step)`?
I think that would make the logic here a bit simpler, and avoid the need for breaking inside the loop.
Something similar would be helpful above too.
I've split up this part. the argument list is a bit long though so I am a bit unsure if that is an improvement. Let me know what you think I've split up this part. the argument list is a bit long though so I am a bit unsure if that is an improvement. Let me know what you think
Hans Goudey
commented
`poly_loops` is generally the name for a span containing the loops of a single polygon. The `poly` argument could be removed by slicing the `loops` span before passing it to the function.
|
||||
const Span<int> corner_verts = mesh->corner_verts();
|
||||
Hans Goudey
commented
`MLoop` has been replaced by two arrays in `main`. This loop can be simplified now:
```
for (const int vert : corner_verts.slice(poly.loopstart, poly.totloop)) {
...
}
```
|
||||
const Span<int> corner_edges = mesh->corner_edges();
|
||||
const Span<MEdge> edges = mesh->edges();
|
||||
|
||||
BitVector<> verts_of_unselected_faces(mesh->totvert);
|
||||
|
||||
/* Find all vertices of unselected faces to help find neighboring faces after. */
|
||||
Hans Goudey
commented
Might as well change this to I realize that's a bit nitpicky, just hoping you might agree and appreciate the more literal semantic argument :P Might as well change this to `polys.index_range()` instead of `select_poly.span.index_range()` for the same reason I mentioned earlier too-- it just says more clearly "we're iterating over all faces" rather than "we're iterating over the face selection".
I realize that's a bit nitpicky, just hoping you might agree and appreciate the more literal semantic argument :P
|
||||
for (const int i : polys.index_range()) {
|
||||
if (select_poly.span[i]) {
|
||||
continue;
|
||||
}
|
||||
const MPoly &poly = polys[i];
|
||||
for (const int vert : corner_verts.slice(poly.loopstart, poly.totloop)) {
|
||||
Hans Goudey
commented
`vert_index` -> `vert` here too, though I mentioned that in chat
|
||||
verts_of_unselected_faces[vert].set(true);
|
||||
}
|
||||
}
|
||||
|
||||
threading::parallel_for(polys.index_range(), 1024, [&](const IndexRange range) {
|
||||
for (const int i : range) {
|
||||
if (!select_poly.span[i] || hide_poly[i]) {
|
||||
continue;
|
||||
}
|
||||
const MPoly &poly = polys[i];
|
||||
if (poly_has_unselected_neighbor(corner_edges.slice(poly.loopstart, poly.totloop),
|
||||
edges,
|
||||
verts_of_unselected_faces,
|
||||
face_step)) {
|
||||
select_poly.span[i] = false;
|
||||
}
|
||||
}
|
||||
});
|
||||
|
||||
select_poly.finish();
|
||||
}
|
||||
|
||||
bool paintface_deselect_all_visible(bContext *C, Object *ob, int action, bool flush_flags)
|
||||
{
|
||||
using namespace blender;
|
||||
|
|
|
@ -378,6 +378,8 @@ void BRUSH_OT_sculpt_curves_falloff_preset(struct wmOperatorType *ot);
|
|||
void PAINT_OT_face_select_linked(struct wmOperatorType *ot);
|
||||
void PAINT_OT_face_select_linked_pick(struct wmOperatorType *ot);
|
||||
void PAINT_OT_face_select_all(struct wmOperatorType *ot);
|
||||
void PAINT_OT_face_select_more(struct wmOperatorType *ot);
|
||||
void PAINT_OT_face_select_less(struct wmOperatorType *ot);
|
||||
void PAINT_OT_face_select_hide(struct wmOperatorType *ot);
|
||||
|
||||
void PAINT_OT_face_vert_reveal(struct wmOperatorType *ot);
|
||||
|
|
|
@ -1518,6 +1518,8 @@ void ED_operatortypes_paint(void)
|
|||
WM_operatortype_append(PAINT_OT_face_select_linked);
|
||||
WM_operatortype_append(PAINT_OT_face_select_linked_pick);
|
||||
WM_operatortype_append(PAINT_OT_face_select_all);
|
||||
WM_operatortype_append(PAINT_OT_face_select_more);
|
||||
WM_operatortype_append(PAINT_OT_face_select_less);
|
||||
WM_operatortype_append(PAINT_OT_face_select_hide);
|
||||
|
||||
WM_operatortype_append(PAINT_OT_face_vert_reveal);
|
||||
|
|
|
@ -693,6 +693,68 @@ void PAINT_OT_face_select_all(wmOperatorType *ot)
|
|||
WM_operator_properties_select_all(ot);
|
||||
}
|
||||
|
||||
static int paint_select_more_exec(bContext *C, wmOperator *op)
|
||||
{
|
||||
Object *ob = CTX_data_active_object(C);
|
||||
Mesh *mesh = BKE_mesh_from_object(ob);
|
||||
if (mesh == NULL || mesh->totpoly == 0) {
|
||||
return OPERATOR_CANCELLED;
|
||||
}
|
||||
|
||||
const bool face_step = RNA_boolean_get(op->ptr, "face_step");
|
||||
paintface_select_more(mesh, face_step);
|
||||
paintface_flush_flags(C, ob, true, false);
|
||||
|
||||
ED_region_tag_redraw(CTX_wm_region(C));
|
||||
return OPERATOR_FINISHED;
|
||||
}
|
||||
|
||||
void PAINT_OT_face_select_more(wmOperatorType *ot)
|
||||
{
|
||||
ot->name = "Select More";
|
||||
ot->description = "Select Faces connected to existing selection";
|
||||
ot->idname = "PAINT_OT_face_select_more";
|
||||
|
||||
ot->exec = paint_select_more_exec;
|
||||
ot->poll = facemask_paint_poll;
|
||||
|
||||
ot->flag = OPTYPE_REGISTER | OPTYPE_UNDO;
|
||||
|
||||
RNA_def_boolean(
|
||||
ot->srna, "face_step", true, "Face Step", "Also select faces that only touch on a corner");
|
||||
}
|
||||
|
||||
static int paint_select_less_exec(bContext *C, wmOperator *op)
|
||||
{
|
||||
Object *ob = CTX_data_active_object(C);
|
||||
Mesh *mesh = BKE_mesh_from_object(ob);
|
||||
if (mesh == NULL || mesh->totpoly == 0) {
|
||||
return OPERATOR_CANCELLED;
|
||||
}
|
||||
|
||||
const bool face_step = RNA_boolean_get(op->ptr, "face_step");
|
||||
paintface_select_less(mesh, face_step);
|
||||
paintface_flush_flags(C, ob, true, false);
|
||||
|
||||
ED_region_tag_redraw(CTX_wm_region(C));
|
||||
return OPERATOR_FINISHED;
|
||||
}
|
||||
|
||||
void PAINT_OT_face_select_less(wmOperatorType *ot)
|
||||
{
|
||||
ot->name = "Select Less";
|
||||
ot->description = "Deselect Faces connected to existing selection";
|
||||
ot->idname = "PAINT_OT_face_select_less";
|
||||
|
||||
ot->exec = paint_select_less_exec;
|
||||
ot->poll = facemask_paint_poll;
|
||||
|
||||
ot->flag = OPTYPE_REGISTER | OPTYPE_UNDO;
|
||||
|
||||
RNA_def_boolean(
|
||||
ot->srna, "face_step", true, "Face Step", "Also deselect faces that only touch on a corner");
|
||||
}
|
||||
|
||||
static int vert_select_all_exec(bContext *C, wmOperator *op)
|
||||
{
|
||||
Object *ob = CTX_data_active_object(C);
|
||||
|
|
Canonical variable name for the edges of a face is
poly_edges