Page MenuHome
Differential D15317 Diff 55485 source/blender/blenkernel/intern/deform.c

Changeset View

Standalone View

View Options

source/blender/blenkernel/intern/deform.c

Show First 20 Lines • Show All 566 Lines • ▼ Show 20 Lines
void BKE_object_defgroup_active_index_set(Object *ob, const int new_index) void BKE_object_defgroup_active_index_set(Object *ob, const int new_index)
{ {
/* Cast away const just for the accessor. */ /* Cast away const just for the accessor. */
int *index = (int *)object_defgroup_active_index_get_p(ob); int *index = (int *)object_defgroup_active_index_get_p(ob);
*index = new_index; *index = new_index;
} }
int *BKE_object_defgroup_flip_map(const Object *ob, int *flip_map_len, const bool use_default) static int *object_defgroup_unlocked_flip_map_ex(const Object *ob,
int *flip_map_len,
const bool use_default,
const bool use_only_unlocked)
{ {
const ListBase *defbase = BKE_object_defgroup_list(ob); const ListBase *defbase = BKE_object_defgroup_list(ob);
int defbase_tot = *flip_map_len = BLI_listbase_count(defbase); int defbase_tot = *flip_map_len = BLI_listbase_count(defbase);
campbellbartonUnsubmitted
Done
Inline Actions

flip_map_len can be passed in as a return argument & calculated in this funciton.

if (use_only_unlocked) {
  /* count unlocked. */
}
else {
  /* use `defbase_tot` */
}

Avoids 2x calls to BLI_listbase_count for the common code-path too (while not a problem, it's not necessary either).

campbellbarton: `flip_map_len` can be passed in as a return argument & calculated in this funciton. ``` if…
campbellbartonUnsubmitted
Done
Inline Actions

Prefer to keep defbase_tot, accessing the return argument everywhere reads worse and makes the patch noisier.

campbellbarton: Prefer to keep `defbase_tot`, accessing the return argument everywhere reads worse and makes…
if (defbase_tot == 0) { if (defbase_tot == 0) {
return NULL; return NULL;
} }
bDeformGroup *dg; bDeformGroup *dg;
char name_flip[sizeof(dg->name)]; char name_flip[sizeof(dg->name)];
int i, flip_num, *map = MEM_mallocN(defbase_tot * sizeof(int), __func__); int i, flip_num;
int *map = MEM_mallocN(defbase_tot * sizeof(int), __func__);
campbellbartonUnsubmitted
Done
Inline Actions

Assigning an allocation to an int should warn, e.g:

source/blender/blenkernel/intern/deform.c: In function ‘object_defgroup_unlocked_flip_map_ex’:
source/blender/blenkernel/intern/deform.c:602:21: warning: initialization of ‘int’ from ‘void *’ makes integer from pointer without a cast [-Wint-conversion]
  602 |   int i, flip_num = MEM_mallocN(*flip_map_len * sizeof(int), __func__);
      |                     ^~~~~~~~~~~
campbellbarton: Assigning an allocation to an int should warn, e.g: ``` source/blender/blenkernel/intern/deform.
for (i = 0; i < defbase_tot; i++) { for (int i = 0; i < defbase_tot; i++) {
map[i] = -1; map[i] = -1;
} }
campbellbartonUnsubmitted
Done
Inline Actions

There is no need to count unlocked groups now as flip_map_len doesn't change anymore for unlocked groups.

campbellbarton: There is no need to count unlocked groups now as `flip_map_len` doesn't change anymore for…
for (dg = defbase->first, i = 0; dg; dg = dg->next, i++) { for (dg = defbase->first, i = 0; dg; dg = dg->next, i++) {
if (map[i] == -1) { /* may be calculated previously */ if (map[i] == -1) { /* may be calculated previously */
/* in case no valid value is found, use this */ /* in case no valid value is found, use this */
if (use_default) { if (use_default) {
map[i] = i; map[i] = i;
} }
if (use_only_unlocked && (dg->flag & DG_LOCK_WEIGHT)) {
continue;
}
BLI_string_flip_side_name(name_flip, dg->name, false, sizeof(name_flip)); BLI_string_flip_side_name(name_flip, dg->name, false, sizeof(name_flip));
if (!STREQ(name_flip, dg->name)) { if (!STREQ(name_flip, dg->name)) {
flip_num = BKE_object_defgroup_name_index(ob, name_flip); flip_num = BKE_object_defgroup_name_index(ob, name_flip);
if (flip_num >= 0) { if (flip_num != -1) {
map[i] = flip_num; map[i] = flip_num;
map[flip_num] = i; /* save an extra lookup */ map[flip_num] = i; /* save an extra lookup */
} }
} }
} }
} }
return map; return map;
} }
int *BKE_object_defgroup_flip_map(const Object *ob, int *flip_map_len, const bool use_default)
{
return object_defgroup_unlocked_flip_map_ex(ob, flip_map_len, use_default, false);
}
int *BKE_object_defgroup_flip_map_unlocked(const Object *ob,
int *flip_map_len,
const bool use_default)
{
return object_defgroup_unlocked_flip_map_ex(ob, flip_map_len, use_default, true);
}
int *BKE_object_defgroup_flip_map_single(const Object *ob, int *BKE_object_defgroup_flip_map_single(const Object *ob,
int *flip_map_len, int *flip_map_len,
const bool use_default, const bool use_default,
int defgroup) int defgroup)
{ {
const ListBase *defbase = BKE_object_defgroup_list(ob); const ListBase *defbase = BKE_object_defgroup_list(ob);
int defbase_tot = *flip_map_len = BLI_listbase_count(defbase); int defbase_tot = *flip_map_len = BLI_listbase_count(defbase);
if (defbase_tot == 0) { if (defbase_tot == 0) {
return NULL; return NULL;
} }
bDeformGroup *dg; bDeformGroup *dg;
char name_flip[sizeof(dg->name)]; char name_flip[sizeof(dg->name)];
int i, flip_num, *map = MEM_mallocN(defbase_tot * sizeof(int), __func__); int i, flip_num, *map = MEM_mallocN(defbase_tot * sizeof(int), __func__);
for (i = 0; i < defbase_tot; i++) { for (i = 0; i < defbase_tot; i++) {
map[i] = use_default ? i : -1; map[i] = use_default ? i : -1;
} }
dg = BLI_findlink(defbase, defgroup); dg = BLI_findlink(defbase, defgroup);
BLI_string_flip_side_name(name_flip, dg->name, false, sizeof(name_flip)); BLI_string_flip_side_name(name_flip, dg->name, false, sizeof(name_flip));
if (!STREQ(name_flip, dg->name)) { if (!STREQ(name_flip, dg->name)) {
flip_num = BKE_object_defgroup_name_index(ob, name_flip); flip_num = BKE_object_defgroup_name_index(ob, name_flip);
if (flip_num != -1) { if (flip_num != -1) {
map[defgroup] = flip_num; map[defgroup] = flip_num;
map[flip_num] = defgroup; map[flip_num] = defgroup;
} }
} }
return map; return map;
} }
int BKE_object_defgroup_flip_index(const Object *ob, int index, const bool use_default) int BKE_object_defgroup_flip_index(const Object *ob, int index, const bool use_default)
{ {
const ListBase *defbase = BKE_object_defgroup_list(ob); const ListBase *defbase = BKE_object_defgroup_list(ob);
bDeformGroup *dg = BLI_findlink(defbase, index); bDeformGroup *dg = BLI_findlink(defbase, index);
int flip_index = -1; int flip_index = -1;
if (dg) { if (dg) {
campbellbartonUnsubmitted
Done
Inline Actions

This is a reasonable amount of code duplication, suggest to have a static function object_defgroup_unlocked_flip_map_ex(ob, flip_map_len, use_default, use_only_unlocked). Then there there should only need to be minor differences without having to duplicate logic.

If including the unlocked logic in-line complicates things too much, it would also be fine to clear locked groups in a second loop when use_only_unlocked is set. Even though that incurs redundant calculation, this particular function I don't see it causing any differences users are going to notice.

campbellbarton: This is a reasonable amount of code duplication, suggest to have a static function…
nrupsisAuthorUnsubmitted
Done
Inline Actions

I 100% agree there is too much duplication of logic (was throwing this up for some people to test). I do like the idea of abstracting the core logic into its own method. I'll go ahead and do that.

nrupsis: I 100% agree there is too much duplication of logic (was throwing this up for some people to…
char name_flip[sizeof(dg->name)]; char name_flip[sizeof(dg->name)];
BLI_string_flip_side_name(name_flip, dg->name, false, sizeof(name_flip)); BLI_string_flip_side_name(name_flip, dg->name, false, sizeof(name_flip));
if (!STREQ(name_flip, dg->name)) { if (!STREQ(name_flip, dg->name)) {
flip_index = BKE_object_defgroup_name_index(ob, name_flip); flip_index = BKE_object_defgroup_name_index(ob, name_flip);
} }
} }
▲ Show 20 Lines • Show All 944 Lines • Show Last 20 Lines