blender
/
blender
134
398
Fork 580
Code Issues 6.3k Pull Requests 757 Projects 19 Wiki Activity

Fix #105152: Removing color attribute doesn't update active #105871

Manually merged
Brecht Van Lommel merged 6 commits from bonj/blender:fix/remove-color-attribute into main 2023-03-19 10:28:23 +01:00
Conversation 10 Commits 6 Files Changed 2 +52 -45
2 changed files with 52 additions and 45 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -370,6 +370,26 @@ CustomDataLayer *BKE_id_attribute_duplicate(ID *id, const char *name, ReportList
return BKE_id_attribute_search(id, uniquename, CD_MASK_PROP_ALL, ATTR_DOMAIN_MASK_ALL);
}
static int color_name_to_index(ID *id, const char *name)
{
const CustomDataLayer *layer = BKE_id_attribute_search(
id, name, CD_MASK_COLOR_ALL, ATTR_DOMAIN_MASK_COLOR);
return BKE_id_attribute_to_index(id, layer, ATTR_DOMAIN_MASK_COLOR, CD_MASK_COLOR_ALL);
}
static int color_clamp_index(ID *id, int index)
{
const int length = BKE_id_attributes_length(id, ATTR_DOMAIN_MASK_COLOR, CD_MASK_COLOR_ALL);
return min_ii(index, length - 1);
}
static const char *color_name_from_index(ID *id, int index)
{
const CustomDataLayer *layer = BKE_id_attribute_from_index(
id, index, ATTR_DOMAIN_MASK_COLOR, CD_MASK_COLOR_ALL);
return (layer) ? layer->name : nullptr;
}
bool BKE_id_attribute_remove(ID *id, const char *name, ReportList *reports)
{
using namespace blender;
@ -403,19 +423,23 @@ bool BKE_id_attribute_remove(ID *id, const char *name, ReportList *reports)
BM_data_layer_free_named(em->bm, data, BKE_uv_map_pin_name_get(name, buffer_src));
}
}
/* Because it's possible that name is owned by the layer and will be freed
* when freeing the layer, do these checks before freeing. */
/* Update active and default color attributes. */
const bool is_active_color_attribute = name == StringRef(mesh->active_color_attribute);
const bool is_default_color_attribute = name == StringRef(mesh->default_color_attribute);
if (BM_data_layer_free_named(em->bm, data, name)) {
if (is_active_color_attribute) {
MEM_SAFE_FREE(mesh->active_color_attribute);
}
else if (is_default_color_attribute) {
MEM_SAFE_FREE(mesh->default_color_attribute);
}
return true;
const int active_index = color_name_to_index(id, mesh->active_color_attribute);
const int default_index = color_name_to_index(id, mesh->default_color_attribute);
if (!BM_data_layer_free_named(em->bm, data, name)) {
return false;
}
if (is_active_color_attribute) {
BKE_id_attributes_active_color_set(
id, color_name_from_index(id, color_clamp_index(id, active_index)));
}
if (is_default_color_attribute) {
BKE_id_attributes_default_color_set(
id, color_name_from_index(id, color_clamp_index(id, default_index)));
}
return true;
}
}
return false;
@ -429,7 +453,6 @@ bool BKE_id_attribute_remove(ID *id, const char *name, ReportList *reports)
}
if (GS(id->name) == ID_ME) {
std::optional<blender::bke::AttributeMetaData> metadata = attributes->lookup_meta_data(name);
if (metadata->data_type == CD_PROP_FLOAT2) {
/* remove UV sub-attributes. */
@ -438,6 +461,24 @@ bool BKE_id_attribute_remove(ID *id, const char *name, ReportList *reports)
BKE_id_attribute_remove(id, BKE_uv_map_edge_select_name_get(name, buffer_src), reports);
BKE_id_attribute_remove(id, BKE_uv_map_pin_name_get(name, buffer_src), reports);
}
/* Update active and default color attributes. */
Mesh *mesh = reinterpret_cast<Mesh *>(id);
const bool is_active_color_attribute = name == StringRef(mesh->active_color_attribute);
const bool is_default_color_attribute = name == StringRef(mesh->default_color_attribute);
const int active_index = color_name_to_index(id, mesh->active_color_attribute);
const int default_index = color_name_to_index(id, mesh->default_color_attribute);
bonj marked this conversation as resolved Outdated
Hans Goudey commented 2023-03-18 22:02:06 +01:00
Outdated
Review
Copy Link

I'd suggest flipping the condition here and returning early instead of indenting the rest of the logic.

I'd suggest flipping the condition here and returning early instead of indenting the rest of the logic.
if (!attributes->remove(name)) {
return false;
}
if (is_active_color_attribute) {
BKE_id_attributes_active_color_set(
id, color_name_from_index(id, color_clamp_index(id, active_index)));
}
if (is_default_color_attribute) {
BKE_id_attributes_default_color_set(
id, color_name_from_index(id, color_clamp_index(id, default_index)));
}
return true;
}
return attributes->remove(name);

34
source/blender/editors/geometry/geometry_attributes.cc
View File

@ -107,36 +107,6 @@ static int geometry_attribute_add_exec(bContext *C, wmOperator *op)
return OPERATOR_FINISHED;
}
static void next_color_attribute(ID *id, const StringRefNull name, bool is_render)
{
const CustomDataLayer *layer = BKE_id_attributes_color_find(id, name.c_str());
int index = BKE_id_attribute_to_index(id, layer, ATTR_DOMAIN_MASK_COLOR, CD_MASK_COLOR_ALL);
index++;
layer = BKE_id_attribute_from_index(id, index, ATTR_DOMAIN_MASK_COLOR, CD_MASK_COLOR_ALL);
if (!layer) {
index = 0;
layer = BKE_id_attribute_from_index(id, index, ATTR_DOMAIN_MASK_COLOR, CD_MASK_COLOR_ALL);
}
if (layer) {
if (is_render) {
BKE_id_attributes_active_color_set(id, layer->name);
}
else {
BKE_id_attributes_default_color_set(id, layer->name);
}
}
}
static void next_color_attributes(ID *id, const StringRefNull name)
{
next_color_attribute(id, name, false); /* active */
next_color_attribute(id, name, true); /* render */
}
void GEOMETRY_OT_attribute_add(wmOperatorType *ot)
{
/* identifiers */
@ -182,8 +152,6 @@ static int geometry_attribute_remove_exec(bContext *C, wmOperator *op)
ID *id = static_cast<ID *>(ob->data);
CustomDataLayer *layer = BKE_id_attributes_active_get(id);
next_color_attributes(id, layer->name);
if (!BKE_id_attribute_remove(id, layer->name, op->reports)) {
return OPERATOR_CANCELLED;
}
@ -436,8 +404,6 @@ static int geometry_color_attribute_remove_exec(bContext *C, wmOperator *op)
return OPERATOR_CANCELLED;
}
next_color_attributes(id, active_name);
if (!BKE_id_attribute_remove(id, active_name.c_str(), op->reports)) {
return OPERATOR_CANCELLED;
}