blender
/
blender
136
411
Fork 595
Code Issues 6.3k Pull Requests 774 Projects 19 Wiki Activity

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
Conversation 7 Commits 8 Files Changed 4 +9 -14
3 changed files with 9 additions and 14 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit abfd99b950 - Show all commits

13
source/blender/blenkernel/BKE_attribute.h
View File

@ -9,9 +9,6 @@
#pragma once
#include "DNA_ID.h"
#include "DNA_object_types.h"
#include "BLI_sys_types.h"
lichtwerk marked this conversation as resolved Outdated
Hans Goudey commented 2023-07-14 14:26:31 +02:00
Outdated
Review
Copy Link

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 "BKE_customdata.h"
@ -25,11 +22,6 @@ struct CustomDataLayer;
struct ID;
struct ReportList;
typedef struct AttributeAndDefgroupUniqueNameData {
ID *id;
bDeformGroup *dg;
} AttributeAndDefgroupUniqueNameData;
/** #Attribute.domain */
typedef enum eAttrDomain {
ATTR_DOMAIN_AUTO = -1, /* Use for nodes to choose automatically based on other data. */
@ -125,6 +117,11 @@ void BKE_id_attributes_default_color_set(struct ID *id, const char *name);
const struct CustomDataLayer *BKE_id_attributes_color_find(const struct ID *id, const char *name);
typedef struct AttributeAndDefgroupUniqueNameData {
struct ID *id;
struct bDeformGroup *dg;
lichtwerk marked this conversation as resolved
Hans Goudey commented 2023-07-14 15:37:26 +02:00
Review
Copy Link

This should be forward declared at the top of the file right before CustomData

This should be forward declared at the top of the file right before `CustomData`
} AttributeAndDefgroupUniqueNameData;
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);

2
source/blender/blenkernel/intern/attribute.cc
View File

@ -229,12 +229,10 @@ bool BKE_id_attribute_and_defgroup_unique_name_check(void *arg, const char *name
AttributeAndDefgroupUniqueNameData *data = static_cast<AttributeAndDefgroupUniqueNameData *>(
arg);
/* Checking vertex groups first. */
if (BKE_defgroup_unique_name_check(data, name)) {
lichtwerk marked this conversation as resolved
Hans Goudey commented 2023-07-14 14:24:20 +02:00
Review
Copy Link

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
return true;
}
/* Checking attributes next. */
DomainInfo info[ATTR_DOMAIN_NUM];
get_domains(data->id, info);

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

@ -448,9 +448,6 @@ bool BKE_object_supports_vertex_groups(const Object *ob)
const ListBase *BKE_id_defgroup_list_get(const ID *id)
{
switch (GS(id->name)) {
case ID_OB: {
return BKE_object_defgroup_list((const Object *)id);
}
case ID_ME: {
lichtwerk marked this conversation as resolved Outdated
Hans Goudey commented 2023-07-14 14:19:47 +02:00
Outdated
Review
Copy Link

This function is meant to be called on object data, adding this is misleading compared to the other cases IMO

This function is meant to be called on object data, adding this is misleading compared to the other cases IMO
const Mesh *me = (const Mesh *)id;
return &me->vertex_group_names;
@ -687,7 +684,10 @@ int BKE_object_defgroup_flip_index(const Object *ob, int index, const bool use_d
static bool defgroup_find_name_dupe(const char *name, bDeformGroup *dg, ID *id)
{
const ListBase *defbase = BKE_id_defgroup_list_get(id);
const ListBase *defbase = (GS(id->name) == ID_OB) ?
BKE_object_defgroup_list((const Object *)id) :
BKE_id_defgroup_list_get(id);
bDeformGroup *curdef;
for (curdef = static_cast<bDeformGroup *>(defbase->first); curdef; curdef = curdef->next) {