Fix #103410: name collisions between vertex groups and attributes #109910

Merged
Philipp Oeser merged 8 commits from lichtwerk/blender:103410 into main 2023-08-08 10:11:18 +02:00
4 changed files with 29 additions and 32 deletions
Showing only changes of commit 9361ad5f19 - Show all commits

View File

@ -10,6 +10,7 @@
#pragma once
#include "DNA_ID.h"
#include "DNA_object_types.h"
lichtwerk marked this conversation as resolved Outdated

These includes should be unnecessary with forward declarations. It's good to avoid including headers in headers where possible, especially for such a common file such as BKE_attribute.h.

These includes should be unnecessary with forward declarations. It's good to avoid including headers in headers where possible, especially for such a common file such as `BKE_attribute.h`.
#include "BLI_sys_types.h"
@ -24,9 +25,10 @@ struct CustomDataLayer;
struct ID;
struct ReportList;
typedef struct AttrUniqueData {
typedef struct AttributeAndDefgroupUniqueNameData {
lichtwerk marked this conversation as resolved Outdated

It might be nicer to define this right above BKE_id_attribute_and_defgroup_unique_name_check, leaving the top of the file for more generally used / important things like the attribute domain enum definition.

It might be nicer to define this right above `BKE_id_attribute_and_defgroup_unique_name_check`, leaving the top of the file for more generally used / important things like the attribute domain enum definition.
ID *id;
} AttrUniqueData;
bDeformGroup *dg;
} AttributeAndDefgroupUniqueNameData;
/** #Attribute.domain */
typedef enum eAttrDomain {
@ -123,7 +125,7 @@ void BKE_id_attributes_default_color_set(struct ID *id, const char *name);
struct CustomDataLayer *BKE_id_attributes_color_find(const struct ID *id, const char *name);
bool BKE_id_attribute_unique_name_check(void *arg, const char *name);
bool BKE_id_attribute_and_defgroup_unique_name_check(void *arg, const char *name);
bool BKE_id_attribute_calc_unique_name(struct ID *id, const char *name, char *outname);
const char *BKE_uv_map_vert_select_name_get(const char *uv_map_name, char *buffer);

View File

@ -28,11 +28,6 @@ struct MDeformVert;
struct Object;
struct bDeformGroup;
typedef struct DeformGroupUniqueNameData {
ID *id;
bDeformGroup *dg;
} DeformGroupUniqueNameData;
bool BKE_object_supports_vertex_groups(const struct Object *ob);
const struct ListBase *BKE_object_defgroup_list(const struct Object *ob);
struct ListBase *BKE_object_defgroup_list_mutable(struct Object *ob);

View File

@ -224,10 +224,17 @@ bool BKE_id_attribute_rename(ID *id,
return true;
}
bool BKE_id_attribute_unique_name_check(void *arg, const char *name)
bool BKE_id_attribute_and_defgroup_unique_name_check(void *arg, const char *name)
{
AttrUniqueData *data = (AttrUniqueData *)arg;
AttributeAndDefgroupUniqueNameData *data = static_cast<AttributeAndDefgroupUniqueNameData *>(
arg);
/* Checking vertex groups first. */
lichtwerk marked this conversation as resolved
Review

This comment basically says the same thing that the BKE_defgroup_unique_name_check function name does. Best to let the code do the talking here I think-- comments can describe the reasoning, etc. but just saying what the code does on the next line usually shouldn't be necessary

This comment basically says the same thing that the `BKE_defgroup_unique_name_check` function name does. Best to let the code do the talking here I think-- comments can describe the reasoning, etc. but just saying what the code does on the next line usually shouldn't be necessary
if (BKE_defgroup_unique_name_check(data, name)) {
return true;
}
/* Checking attributes next. */
DomainInfo info[ATTR_DOMAIN_NUM];
get_domains(data->id, info);
@ -251,26 +258,18 @@ bool BKE_id_attribute_unique_name_check(void *arg, const char *name)
bool BKE_id_attribute_calc_unique_name(ID *id, const char *name, char *outname)
{
bool success = false;
AttributeAndDefgroupUniqueNameData data{id, nullptr};
AttrUniqueData data{id};
const int name_maxncpy = CustomData_name_maxncpy_calc(name);
/* Set default name if none specified.
* NOTE: We only call IFACE_() if needed to avoid locale lookup overhead. */
BLI_strncpy_utf8(outname, (name && name[0]) ? name : IFACE_("Attribute"), name_maxncpy);
/* Correct name collisions with attributes. */
/* Avoid name collisions with vertex groups and other attributes. */
const char *defname = ""; /* Dummy argument, never used as `name` is never zero length. */
success |= BLI_uniquename_cb(
BKE_id_attribute_unique_name_check, &data, defname, '.', outname, name_maxncpy);
/* Also correct name collisions with vertex groups. */
DeformGroupUniqueNameData data_dg{id, nullptr};
success |= BLI_uniquename_cb(
BKE_defgroup_unique_name_check, &data_dg, defname, '.', outname, name_maxncpy);
return success;
return BLI_uniquename_cb(
BKE_id_attribute_and_defgroup_unique_name_check, &data, defname, '.', outname, name_maxncpy);
}
CustomDataLayer *BKE_id_attribute_new(ID *id,

View File

@ -703,28 +703,29 @@ static bool defgroup_find_name_dupe(const char *name, bDeformGroup *dg, ID *id)
bool BKE_defgroup_unique_name_check(void *arg, const char *name)
{
DeformGroupUniqueNameData *data = static_cast<DeformGroupUniqueNameData *>(arg);
AttributeAndDefgroupUniqueNameData *data = static_cast<AttributeAndDefgroupUniqueNameData *>(
arg);
return defgroup_find_name_dupe(name, data->dg, data->id);
}
void BKE_object_defgroup_unique_name(bDeformGroup *dg, Object *ob)
{
/* Correct name name collisions with vertex groups. */
DeformGroupUniqueNameData data{&ob->id, dg};
BLI_uniquename_cb(
BKE_defgroup_unique_name_check, &data, DATA_("Group"), '.', dg->name, sizeof(dg->name));
/* Also correct name name collisions with (mesh) attributes. */
/* Avoid name collisions with other vertex groups and (mesh) attributes. */
if (ob->type == OB_MESH) {
Mesh *me = static_cast<Mesh *>(ob->data);
AttrUniqueData data_attr{&me->id};
BLI_uniquename_cb(BKE_id_attribute_unique_name_check,
&data_attr,
AttributeAndDefgroupUniqueNameData data{&me->id, dg};
BLI_uniquename_cb(BKE_id_attribute_and_defgroup_unique_name_check,
&data,
DATA_("Group"),
'.',
dg->name,
sizeof(dg->name));
}
else {
AttributeAndDefgroupUniqueNameData data{&ob->id, dg};

AttributeAndDefgroupUniqueNameData data{static_cast<ID *>(ob->data), dg};

This removes the need for the change in defgroup_find_name_dupe

`AttributeAndDefgroupUniqueNameData data{static_cast<ID *>(ob->data), dg};` This removes the need for the change in `defgroup_find_name_dupe`
BLI_uniquename_cb(
BKE_defgroup_unique_name_check, &data, DATA_("Group"), '.', dg->name, sizeof(dg->name));
}
}
float BKE_defvert_find_weight(const MDeformVert *dvert, const int defgroup)