blender
/
blender
136
406
Fork 589
Code Issues 6.3k Pull Requests 766 Projects 19 Wiki Activity

Refactor: Weight Paint Select Linked Faces #104577

Merged
Christoph Lendenfeld merged 15 commits from ChrisLend/blender:weight_paint_refactor_face_select into main 2023-02-23 08:26:44 +01:00
Conversation 3 Commits 15 Files Changed 1 +12 -13
5 changed files with 12 additions and 13 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit 1d596ddc55 - Show all commits

2
release/datafiles/locale

@ -1 +1 @@
Subproject commit 4331c8e76c2f42b9fd903716c333d6cdeaa5cebd
Subproject commit 8d04896a130c12eb440b7b9ac33bf9dc152506b5

2
release/scripts/addons

@ -1 +1 @@
Subproject commit b3f0ffc587d197b37eac9a1566d1d24b7bee7d9a
Subproject commit 648124b64779822fff8d708c4f9d779231f0c680

2
release/scripts/addons_contrib

@ -1 +1 @@
Subproject commit 14ab9273409ea0231d08ba6e86fdc73d4e459e99
Subproject commit 8f4599c4fb6ab2386f2498814c203fd676b2ccf2

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

@ -215,8 +215,8 @@ void paintface_reveal(bContext *C, Object *ob, const bool select)
/**
* Join all edges of each poly in the AtomicDisjointSet. This can be used to find out which polys
ChrisLend marked this conversation as resolved
Hans Goudey commented 2023-02-10 16:42:01 +01:00
Review
Copy Link

Mesh *mesh -> Mesh &mesh

Using a reference indicates that the pointer won't be null

`Mesh *mesh` -> `Mesh &mesh` Using a reference indicates that the pointer won't be null
* are connected to each other.
* @param islands Is expected to be of length mesh->totedge.
* @param skip_seams Polys separated by a seam will be treated as not connected.
* \param islands Is expected to be of length mesh->totedge.
ChrisLend marked this conversation as resolved
Hans Goudey commented 2023-02-10 18:26:22 +01:00
Review
Copy Link

@param islands -> \param islands:
https://wiki.blender.org/wiki/Style_Guide/C_Cpp#Comments

`@param islands` -> `\param islands:` https://wiki.blender.org/wiki/Style_Guide/C_Cpp#Comments
* \param skip_seams Polys separated by a seam will be treated as not connected.
*/
static void build_poly_connections(blender::AtomicDisjointSet &islands,
Mesh &mesh,
@ -240,17 +240,17 @@ static void build_poly_connections(blender::AtomicDisjointSet &islands,
}
const MPoly &poly = polys[poly_index];
for (const int outer_loop_index : IndexRange(0, poly.totloop)) {
ChrisLend marked this conversation as resolved
Hans Goudey commented 2023-02-10 18:35:54 +01:00
Review
Copy Link

I suggest using a separate const Span<MLoop> poly_loops = loops.slice(poly.loopstart, poly.totloop); variable. Then the next loop can use for (const int outer_loop_index : poly_loops.index_range()) { and you don't have to add poly.totloop every time a loop in the poly is accessed.

I suggest using a separate `const Span<MLoop> poly_loops = loops.slice(poly.loopstart, poly.totloop);` variable. Then the next loop can use `for (const int outer_loop_index : poly_loops.index_range()) {` and you don't have to add `poly.totloop` every time a loop in the poly is accessed.
const MLoop *outer_mloop = &loops[poly.loopstart + outer_loop_index];
if (skip_seams && (edges[outer_mloop->e].flag & ME_SEAM) != 0) {
const MLoop &outer_mloop = loops[poly.loopstart + outer_loop_index];
Hans Goudey commented 2023-02-10 18:27:31 +01:00
Review
Copy Link

Use references instead of pointers, that should be your default in C++ code.

Use references instead of pointers, that should be your default in C++ code.
Christoph Lendenfeld commented 2023-02-16 10:02:23 +01:00
Review
Copy Link

sure thing, missed that
out of curiosity: what is the reason for it?

sure thing, missed that out of curiosity: what is the reason for it?
Hans Goudey commented 2023-02-16 13:51:55 +01:00
Review
Copy Link

There are quite a few benefits-- they indicate non-null, they allow using . instead of ->, they can't be indexed (so they can't be mistaken for pointers to arrays). There's more info here:

https://stackoverflow.com/questions/57483/what-are-the-differences-between-a-pointer-variable-and-a-reference-variable

There are quite a few benefits-- they indicate non-null, they allow using `.` instead of `->`, they can't be indexed (so they can't be mistaken for pointers to arrays). There's more info here: https://stackoverflow.com/questions/57483/what-are-the-differences-between-a-pointer-variable-and-a-reference-variable
👍 1
if (skip_seams && (edges[outer_mloop.e].flag & ME_SEAM) != 0) {
continue;
}
for (int inner_loop_index = outer_loop_index + 1; inner_loop_index < poly.totloop;
inner_loop_index++) {
const MLoop *inner_mloop = &loops[poly.loopstart + inner_loop_index];
if (skip_seams && (edges[inner_mloop->e].flag & ME_SEAM) != 0) {
const MLoop &inner_mloop = loops[poly.loopstart + inner_loop_index];
if (skip_seams && (edges[inner_mloop.e].flag & ME_SEAM) != 0) {
continue;
}
islands.join(inner_mloop->e, outer_mloop->e);
islands.join(inner_mloop.e, outer_mloop.e);
}
}
}
@ -311,12 +311,11 @@ void paintface_select_linked(bContext *C, Object *ob, const int mval[2], const b
return;
}
Vector<int> indices;
bke::MutableAttributeAccessor attributes = me->attributes_for_write();
ChrisLend marked this conversation as resolved
Hans Goudey commented 2023-02-10 18:31:00 +01:00
Review
Copy Link

Declare this closer to where it's first used, right above if (mval) {

Declare this closer to where it's first used, right above `if (mval) {`
bke::SpanAttributeWriter<bool> select_poly = attributes.lookup_or_add_for_write_span<bool>(
".select_poly", ATTR_DOMAIN_FACE);
Vector<int> indices;
if (mval) {
uint index = uint(-1);
if (!ED_mesh_pick_face(C, ob, mval, ED_MESH_PICK_DEFAULT_FACE_DIST, &index)) {

2
source/tools

@ -1 +1 @@
Subproject commit e133fc08cd3254bb3d3bd1345028c8486700bca4
Subproject commit 27826b4aed138b7e3aed882d0eab96740c3a55c7