griefith/test2
1
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

Fix #141024: Normalize active vertex group properly

Browse Source 
When manually setting a group's vertex weight, auto-normalization would
fail in some circumstances, such as when all other groups are locked.

The root cause of this issue was our approach for ensuring that the
weight specified by the user remained as-is when possible during
normalization. Rather than "when possible", it erroneously *always*
ensured the weight stayed as-is even when that made normalization
impossible. It came down to this:

1. Normalization is done as a post process, with no knowledge of what
   changes were just made to the weights.
2. In order to (try to) make up for that and ensure that the just-set
   weight remains as the user specified, the active group was
   temporarily locked during normalization, which could prevent
   normalization in some cases.

This PR fixes the issue by introducing a new internal-only concept of
"soft locked" vertex groups to the normalization functions, intended to
be used in exactly these cases where there are weights that have just
been set and we want to avoid altering them when possible. Soft-locked
groups are left untouched whenever normalization is achievable without
touching them, but are still modified if normalization can't be achieved
otherwise.

This has been implemented by introducing a new bool array alongside the
"locked" bool array in the core normalization functions.  Although all
uses in this PR only ever specify a single group as "soft locked", using
a bool array will make it easy to use this concept in other weight
painting tools in the future, which may modify more than one group at
once.

Pull Request: https://projects.blender.org/blender/blender/pulls/141045
This commit is contained in:
Nathan Vegdahl
2025-07-08 15:20:07 +02:00
committed by Gitea
parent caddde1eb7
commit 69433a181e
4 changed files with 252 additions and 38 deletions
Download Patch File Download Diff File Expand all files Collapse all files

31
source/blender/blenkernel/BKE_deform.hh
View File

@@ -241,7 +241,7 @@ void BKE_defvert_flip_merged(MDeformVert *dvert, const int *flip_map, int flip_m
* Note: this ignores whether groups are locked or not, and will therefore
* happily modify even locked groups.
*
* See #BKE_defvert_normalize_lock_map() for parameter documentation.
* See #BKE_defvert_normalize_ex() for parameter documentation.
*/
void BKE_defvert_normalize(MDeformVert &dvert);
@@ -251,10 +251,20 @@ void BKE_defvert_normalize(MDeformVert &dvert);
* Note: this ignores whether groups are locked or not, and will therefore
* happily modify even locked groups.
*
* See #BKE_defvert_normalize_lock_map() for parameter documentation.
* See #BKE_defvert_normalize_ex() for parameter documentation.
*/
void BKE_defvert_normalize_subset(MDeformVert &dvert, blender::Span<bool> subset_flags);
/**
* Normalize a subset of vertex group weights among themselves, but leaving
* locked groups unmodified.
*
* See #BKE_defvert_normalize_ex() for parameter documentation.
*/
void BKE_defvert_normalize_lock_map(MDeformVert &dvert,
blender::Span<bool> subset_flags,
blender::Span<bool> lock_flags);
/**
* Normalize the vertex groups of a vertex, with all the bells and whistles.
*
@@ -273,10 +283,21 @@ void BKE_defvert_normalize_subset(MDeformVert &dvert, blender::Span<bool> subset
* normalization. May be empty, indicating no locked groups. If not empty, its
* length must match the number of vertex groups in the source data (e.g. the
* mesh).
*
* \param soft_lock_flags: span of bools with `true` indicating a set of vertex
* groups that are "soft locked". The intended use case for this is to "protect"
* weights that have just been set by a tool or operator during post-process
* normalization. When possible, only non-soft-locked weights will be modified
* to achieve normalization, but if necessary soft-locked will also be modified.
* NOTE: in theory this could be used for purposes other than "just set" groups,
* but corner cases are handled with that use case in mind. May be empty,
* indicating no "soft locked" groups. If not empty, its length must match the
* number of vertex groups in the source data (e.g. the mesh).
*/
void BKE_defvert_normalize_lock_map(MDeformVert &dvert,
blender::Span<bool> subset_flags,
blender::Span<bool> lock_flags);
void BKE_defvert_normalize_ex(MDeformVert &dvert,
blender::Span<bool> vgroup_subset,
blender::Span<bool> lock_flags,
blender::Span<bool> soft_lock_flags);
/* Utilities to 'extract' a given vgroup into a simple float array,
* for verts, but also edges/faces/loops. */

96
source/blender/blenkernel/intern/deform.cc
View File

@@ -230,20 +230,29 @@ void BKE_defvert_remap(MDeformVert *dvert, const int *map, const int map_len)
void BKE_defvert_normalize_subset(MDeformVert &dvert, blender::Span<bool> subset_flags)
{
BKE_defvert_normalize_lock_map(dvert, subset_flags, {});
BKE_defvert_normalize_ex(dvert, subset_flags, {}, {});
}
void BKE_defvert_normalize(MDeformVert &dvert)
{
BKE_defvert_normalize_lock_map(dvert, {}, {});
BKE_defvert_normalize_ex(dvert, {}, {}, {});
}
void BKE_defvert_normalize_lock_map(MDeformVert &dvert,
blender::Span<bool> subset_flags,
blender::Span<bool> lock_flags)
{
BKE_defvert_normalize_ex(dvert, subset_flags, lock_flags, {});
}
void BKE_defvert_normalize_ex(MDeformVert &dvert,
blender::Span<bool> subset_flags,
blender::Span<bool> lock_flags,
blender::Span<bool> soft_lock_flags)
{
const bool use_subset = !subset_flags.is_empty();
const bool use_locks = !lock_flags.is_empty();
const bool use_soft_locks = !soft_lock_flags.is_empty();
/* Note: confusingly, `totweight` isn't the total weight on the vertex, it's
* the number of vertex groups assigned to the vertex. It's a DNA field, so
@@ -287,7 +296,9 @@ void BKE_defvert_normalize_lock_map(MDeformVert &dvert,
/* Collect weights. */
float total_locked_weight = 0.0f;
float total_regular_weight = 0.0f; /* Unlocked. */
float total_soft_locked_weight = 0.0f;
float total_regular_weight = 0.0f; /* Neither locked nor soft locked. */
int soft_locked_group_count = 0;
for (MDeformWeight &dw : vertex_weights) {
if (use_subset && !subset_flags[dw.def_nr]) {
/* Not part of the subset being normalized. */
@@ -298,6 +309,10 @@ void BKE_defvert_normalize_lock_map(MDeformVert &dvert,
/* Locked. */
total_locked_weight += dw.weight;
}
else if (use_soft_locks && soft_lock_flags[dw.def_nr]) {
total_soft_locked_weight += dw.weight;
soft_locked_group_count++;
}
else {
total_regular_weight += dw.weight;
}
@@ -305,13 +320,73 @@ void BKE_defvert_normalize_lock_map(MDeformVert &dvert,
const float available_weight = max_ff(0.0f, 1.0f - total_locked_weight);
/* Special case: all non-locked vertex groups have zero weight. */
if (total_regular_weight <= 0.0f) {
/* Special case: all non-hard-locked vertex groups have zero weight.
*
* Note: conceptually this if condition is checking for `== 0.0`, because
* negative weights shouldn't be possible. We're just being paranoid with
* the `<=`. */
if (total_regular_weight <= 0.0f && total_soft_locked_weight <= 0.0f) {
/* There isn't any "right" thing to do here.
*
* What we choose to do is: if there are any soft-locked groups on the
* vertex, distribute the needed weight equally among them. If there are no
* soft-locked groups on the vertex, we do nothing. The rationale behind
* this is that:
*
* 1. Zero-weight groups should typically be treated the same as unassigned
* groups.
* 2. But soft-locked groups can be treated specially: since their intended
* use case is indicating vertex groups that have just now had their
* weights set, we know they were intentionally set. Therefore even when
* zero-weight we can consider them assigned.
*
* There isn't any deep truth behind this approach, but after discussion
* with a few people I (Nathan Vegdahl) think in practice this is likely to
* be the least surprising behavior to users (out of several bad options).
* In particular, when the user modifies weights with auto-normalize
* enabled, they expect Blender to ensure normalized weights whenever
* possible (see issue #141024), and this approach achieves that.
*
* However, this approach is very much worth revisiting if it ends up
* causing other problems. */
if (soft_locked_group_count == 0) {
return;
}
const float weight = available_weight / soft_locked_group_count;
for (MDeformWeight &dw : vertex_weights) {
if (!subset_flags.is_empty() && !subset_flags[dw.def_nr]) {
/* Not part of the subset being normalized. */
continue;
}
if (!lock_flags.is_empty() && lock_flags[dw.def_nr]) {
/* Locked. */
continue;
}
if (use_soft_locks && soft_lock_flags[dw.def_nr]) {
dw.weight = weight;
}
}
return;
}
/* Compute scale factor for unlocked group weights. */
const float scale = available_weight / total_regular_weight;
/* Compute scale factors for soft-locked and regular group weights. */
float soft_locked_scale;
float regular_scale;
const bool must_adjust_soft_locked = total_soft_locked_weight >= available_weight ||
total_regular_weight <= 0.0f;
if (must_adjust_soft_locked) {
soft_locked_scale = available_weight / total_soft_locked_weight;
regular_scale = 0.0f;
}
else {
soft_locked_scale = 1.0f;
regular_scale = (available_weight - total_soft_locked_weight) / total_regular_weight;
}
/* Normalize the weights via scaling by the appropriate factors. */
for (MDeformWeight &dw : vertex_weights) {
@@ -325,7 +400,12 @@ void BKE_defvert_normalize_lock_map(MDeformVert &dvert,
continue;
}
dw.weight *= scale;
if (use_soft_locks && soft_lock_flags[dw.def_nr]) {
dw.weight *= soft_locked_scale;
}
else {
dw.weight *= regular_scale;
}
/* In case of division errors with very low weights. */
CLAMP(dw.weight, 0.0f, 1.0f);

133
source/blender/blenkernel/intern/deform_test.cc
View File

@@ -20,7 +20,7 @@ TEST(vertex_weights_normalize, EmptyWeights)
MDeformVert vert = {weights, 0, 0};
BKE_defvert_normalize_lock_map(vert, {}, {});
BKE_defvert_normalize_ex(vert, {}, {}, {});
}
TEST(vertex_weights_normalize, SingleWeight)
@@ -31,38 +31,48 @@ TEST(vertex_weights_normalize, SingleWeight)
/* Excluded from normalized set: shouldn't be touched. */
weights[0].weight = 0.5;
BKE_defvert_normalize_lock_map(vert, {false}, {false});
BKE_defvert_normalize_ex(vert, {false}, {false}, {false});
EXPECT_FLOAT_EQ(0.5, weights[0].weight);
/* Locked: shouldn't be touched. */
weights[0].weight = 0.5;
BKE_defvert_normalize_lock_map(vert, {true}, {true});
BKE_defvert_normalize_ex(vert, {true}, {true}, {false});
EXPECT_FLOAT_EQ(0.5, weights[0].weight);
/* Unlocked: should get normalized to 1.0. */
weights[0].weight = 0.5;
BKE_defvert_normalize_lock_map(vert, {true}, {false});
BKE_defvert_normalize_ex(vert, {true}, {false}, {false});
EXPECT_FLOAT_EQ(1.0, weights[0].weight);
/* Unlocked and soft-locked: should get normalized to 1.0. */
weights[0].weight = 0.5;
BKE_defvert_normalize_ex(vert, {true}, {false}, {true});
EXPECT_FLOAT_EQ(1.0, weights[0].weight);
/* Locked and soft-locked: shouldn't be touched (locked takes precedent). */
weights[0].weight = 0.5;
BKE_defvert_normalize_ex(vert, {true}, {true}, {true});
EXPECT_FLOAT_EQ(0.5, weights[0].weight);
/* An empty "subset" flag list should be equivalent to everything being included. */
weights[0].weight = 0.5;
BKE_defvert_normalize_lock_map(vert, {}, {false});
BKE_defvert_normalize_ex(vert, {}, {false}, {false});
EXPECT_FLOAT_EQ(1.0, weights[0].weight);
/* An empty "locked" flag list should be equivalent to everything being unlocked. */
weights[0].weight = 0.5;
BKE_defvert_normalize_lock_map(vert, {true}, {});
BKE_defvert_normalize_ex(vert, {true}, {}, {false});
EXPECT_FLOAT_EQ(1.0, weights[0].weight);
/* Zero weight: single-group vertices are special cased for some reason to be
* set to 1.0. */
weights[0].weight = 0.0;
BKE_defvert_normalize_lock_map(vert, {}, {});
BKE_defvert_normalize_ex(vert, {}, {}, {});
EXPECT_FLOAT_EQ(1.0, weights[0].weight);
/* Zero weight locked: shouldn't be touched. */
weights[0].weight = 0.0;
BKE_defvert_normalize_lock_map(vert, {}, {true});
BKE_defvert_normalize_ex(vert, {}, {true}, {});
EXPECT_FLOAT_EQ(0.0, weights[0].weight);
}
@@ -76,36 +86,36 @@ TEST(vertex_weights_normalize, TwoWeights)
/* Both excluded from normalized set: shouldn't be touched. */
weights[0].weight = 0.25;
weights[1].weight = 0.25;
BKE_defvert_normalize_lock_map(vert, {false, false}, {false, false});
BKE_defvert_normalize_ex(vert, {false, false}, {false, false}, {false, false});
EXPECT_FLOAT_EQ(0.25, weights[0].weight);
EXPECT_FLOAT_EQ(0.25, weights[1].weight);
/* One included: included one should be set to 1.0. */
weights[0].weight = 0.25;
weights[1].weight = 0.25;
BKE_defvert_normalize_lock_map(vert, {false, true}, {false, false});
BKE_defvert_normalize_ex(vert, {false, true}, {false, false}, {false, false});
EXPECT_FLOAT_EQ(0.25, weights[0].weight);
EXPECT_FLOAT_EQ(1.0, weights[1].weight);
/* Both included: should be normalized together. */
weights[0].weight = 0.25;
weights[1].weight = 0.25;
BKE_defvert_normalize_lock_map(vert, {true, true}, {false, false});
BKE_defvert_normalize_ex(vert, {true, true}, {false, false}, {false, false});
EXPECT_FLOAT_EQ(0.5, weights[0].weight);
EXPECT_FLOAT_EQ(0.5, weights[1].weight);
/* All flag arrays being empty should mean: included and unlocked. So this
* should behave as a simple normalization across both groups. */
/* All flag arrays being empty should mean: included, unlocked, and not "just
* set". So this should behave as a simple normalization across both groups. */
weights[0].weight = 0.25;
weights[1].weight = 0.25;
BKE_defvert_normalize_lock_map(vert, {}, {});
BKE_defvert_normalize_ex(vert, {}, {}, {});
EXPECT_FLOAT_EQ(0.5, weights[0].weight);
EXPECT_FLOAT_EQ(0.5, weights[1].weight);
/* Both included but locked: shouldn't be touched. */
weights[0].weight = 0.25;
weights[1].weight = 0.25;
BKE_defvert_normalize_lock_map(vert, {}, {true, true});
BKE_defvert_normalize_ex(vert, {}, {true, true}, {false, false});
EXPECT_FLOAT_EQ(0.25, weights[0].weight);
EXPECT_FLOAT_EQ(0.25, weights[1].weight);
@@ -113,16 +123,105 @@ TEST(vertex_weights_normalize, TwoWeights)
* slack for normalization. */
weights[0].weight = 0.25;
weights[1].weight = 0.25;
BKE_defvert_normalize_lock_map(vert, {}, {true, false});
BKE_defvert_normalize_ex(vert, {}, {true, false}, {false, false});
EXPECT_FLOAT_EQ(0.25, weights[0].weight);
EXPECT_FLOAT_EQ(0.75, weights[1].weight);
/* Only one marked as soft-locked: soft-locked shouldn't be touched, the other
* should pick up the slack for normalization. */
weights[0].weight = 0.25;
weights[1].weight = 0.25;
BKE_defvert_normalize_ex(vert, {}, {false, false}, {true, false});
EXPECT_FLOAT_EQ(0.25, weights[0].weight);
EXPECT_FLOAT_EQ(0.75, weights[1].weight);
/* One locked, the other marked as soft-locked: soft-locked should pick up the
* slack for normalization. */
weights[0].weight = 0.25;
weights[1].weight = 0.25;
BKE_defvert_normalize_ex(vert, {}, {true, false}, {false, true});
EXPECT_FLOAT_EQ(0.25, weights[0].weight);
EXPECT_FLOAT_EQ(0.75, weights[1].weight);
/* Zero weight: shouldn't be touched. */
weights[0].weight = 0.0;
weights[1].weight = 0.0;
BKE_defvert_normalize_lock_map(vert, {}, {false, false});
BKE_defvert_normalize_ex(vert, {}, {false, false}, {false, false});
EXPECT_FLOAT_EQ(0.0, weights[0].weight);
EXPECT_FLOAT_EQ(0.0, weights[1].weight);
/* Zero weight with one group soft-locked: soft-locked should pick up the slack. */
weights[0].weight = 0.0;
weights[1].weight = 0.0;
BKE_defvert_normalize_ex(vert, {}, {false, false}, {false, true});
EXPECT_FLOAT_EQ(0.0, weights[0].weight);
EXPECT_FLOAT_EQ(1.0, weights[1].weight);
/* Zero weight with both groups soft-locked: both should pick up the slack equally. */
weights[0].weight = 0.0;
weights[1].weight = 0.0;
BKE_defvert_normalize_ex(vert, {}, {false, false}, {true, true});
EXPECT_FLOAT_EQ(0.5, weights[0].weight);
EXPECT_FLOAT_EQ(0.5, weights[1].weight);
}
TEST(vertex_weights_normalize, FourWeights)
{
/* Note the out-of-order `def_nr`, which is part of this test. Further below,
* we write the weights ordered to line up with the boolean arrays to make
* things easier to follow. */
MDeformWeight weights[4];
weights[0].def_nr = 3;
weights[1].def_nr = 0;
weights[2].def_nr = 1;
weights[3].def_nr = 2;
MDeformVert vert = {weights, 4, 0};
/* One locked, one soft-locked: the remaining two should pick up the slack. */
weights[1].weight = 0.125;
weights[2].weight = 0.125;
weights[3].weight = 0.125;
weights[0].weight = 0.0625;
BKE_defvert_normalize_ex(vert, {}, {true, false, false, false}, {false, false, true, false});
EXPECT_FLOAT_EQ(0.125, weights[1].weight);
EXPECT_FLOAT_EQ(0.75 * 2.0 / 3.0, weights[2].weight);
EXPECT_FLOAT_EQ(0.125, weights[3].weight);
EXPECT_FLOAT_EQ(0.75 / 3.0, weights[0].weight);
/* One locked, two soft-locked: the remaining one should pick up the slack. */
weights[1].weight = 0.125;
weights[2].weight = 0.125;
weights[3].weight = 0.125;
weights[0].weight = 0.125;
BKE_defvert_normalize_ex(vert, {}, {true, false, false, false}, {false, true, true, false});
EXPECT_FLOAT_EQ(0.125, weights[1].weight);
EXPECT_FLOAT_EQ(0.125, weights[2].weight);
EXPECT_FLOAT_EQ(0.125, weights[3].weight);
EXPECT_FLOAT_EQ(0.625, weights[0].weight);
/* One locked, one soft-locked, and the rest zero-weight: the soft-locked one
* should pick up the slack. */
weights[1].weight = 0.125;
weights[2].weight = 0.0;
weights[3].weight = 0.125;
weights[0].weight = 0.0;
BKE_defvert_normalize_ex(vert, {}, {true, false, false, false}, {false, false, true, false});
EXPECT_FLOAT_EQ(0.125, weights[1].weight);
EXPECT_FLOAT_EQ(0.0, weights[2].weight);
EXPECT_FLOAT_EQ(0.875, weights[3].weight);
EXPECT_FLOAT_EQ(0.0, weights[0].weight);
/* One locked, two soft-locked, and the last zero-weight: the soft-locked ones
* should pick up the slack. */
weights[1].weight = 0.125;
weights[2].weight = 0.125;
weights[3].weight = 0.25;
weights[0].weight = 0.0;
BKE_defvert_normalize_ex(vert, {}, {true, false, false, false}, {false, true, true, false});
EXPECT_FLOAT_EQ(0.125, weights[1].weight);
EXPECT_FLOAT_EQ(0.875 / 3.0, weights[2].weight);
EXPECT_FLOAT_EQ(0.875 * 2.0 / 3.0, weights[3].weight);
EXPECT_FLOAT_EQ(0.0f, weights[0].weight);
}
} // namespace blender::bke::tests

30
source/blender/editors/object/object_vgroup.cc
View File

@@ -1380,6 +1380,11 @@ static void vgroup_levels_subset(Object *ob,
* \param lock_active: If true, the active vertex group is temporarily locked
* during this normalization process.
*
* \param soft_lock_active: If true, the active vertex group is treated as "soft
* locked". See `BKE_defvert_normalize_ex()`'s documentation for details of what
* that means. Note: because locking is a stronger restriction, if `lock_active`
* is true then this parameter has no effect.
*
* \return True if modification to weights might have happened, false if
* modification was impossible (e.g. due to all groups being locked.)
*/
@@ -1387,6 +1392,7 @@ static bool vgroup_normalize_all(Object *ob,
const bool *vgroup_validmap,
const int vgroup_tot,
const bool lock_active,
const bool soft_lock_active,
ReportList *reports,
std::optional<int> current_frame = {})
{
@@ -1421,6 +1427,12 @@ static bool vgroup_normalize_all(Object *ob,
lock_flags[def_nr] = true;
}
Vector<bool> soft_lock_flags;
if (soft_lock_active && !lock_active) {
soft_lock_flags = Vector(vgroup_tot, false);
soft_lock_flags[def_nr] = true;
}
const bool all_locked = !lock_flags.contains(false);
if (all_locked) {
BKE_report(reports, RPT_ERROR, "All groups are locked");
@@ -1431,7 +1443,7 @@ static bool vgroup_normalize_all(Object *ob,
MDeformVert *dv = dvert_array[i];
/* in case its not selected */
if (dv) {
BKE_defvert_normalize_lock_map(*dv, subset_flags, lock_flags);
BKE_defvert_normalize_ex(*dv, subset_flags, lock_flags, soft_lock_flags);
}
}
}
@@ -1446,12 +1458,12 @@ static bool vgroup_normalize_all(Object *ob,
* If the currently active vertex group is for a deform bone, normalize all
* vertex groups that are for deform bones.
*
* \param lock_active: If true, the active vertex group will be left untouched,
* and the remaining deform groups will be normalized to occupy the remaining
* weight not used by it.
* \param soft_lock_active: If true, the active vertex group is treated as "soft
* locked". See `BKE_defvert_normalize_ex()`'s documentation for details of what
* that means.
*/
static void vgroup_normalize_all_deform_if_active_is_deform(Object *ob,
const bool lock_active,
const bool soft_lock_active,
ReportList *reports,
std::optional<int> current_frame = {})
{
@@ -1465,7 +1477,8 @@ static void vgroup_normalize_all_deform_if_active_is_deform(Object *ob,
const bool *vgroup_validmap = BKE_object_defgroup_subset_from_select_type(
ob, WT_VGROUP_BONE_DEFORM, &vgroup_tot, &subset_count);
vgroup_normalize_all(ob, vgroup_validmap, vgroup_tot, lock_active, reports, current_frame);
vgroup_normalize_all(
ob, vgroup_validmap, vgroup_tot, false, soft_lock_active, reports, current_frame);
MEM_SAFE_FREE(vgroup_validmap);
}
@@ -3187,10 +3200,11 @@ static wmOperatorStatus vertex_group_normalize_all_exec(bContext *C, wmOperator
if (ob->type == OB_GREASE_PENCIL) {
int current_frame = CTX_data_scene(C)->r.cfra;
changed = vgroup_normalize_all(
ob, vgroup_validmap, vgroup_tot, lock_active, op->reports, current_frame);
ob, vgroup_validmap, vgroup_tot, lock_active, false, op->reports, current_frame);
}
else {
changed = vgroup_normalize_all(ob, vgroup_validmap, vgroup_tot, lock_active, op->reports);
changed = vgroup_normalize_all(
ob, vgroup_validmap, vgroup_tot, lock_active, false, op->reports);
}
}