Regression: Python: removing a color attribute may leave a mesh without an active color attribute #105152

New Issue

Closed

opened 2023-02-23 19:27:27 +01:00 by Scurest · 8 comments

Scurest commented

2023-02-23 19:27:27 +01:00

Contributor

System Information
Operating system: Linux

Blender Version
Broken: 3.6.0-alpha+main.e497a50aa207
Worked: 3.2

Caused by 6514bb05ea

Short description of error
Removing a color attribute via the mesh.color_attributes.remove API can leave the mesh without an active/active render color attribute.

This is yet another continuation of #103761.

Exact steps for others to reproduce the error

Create a cube and add some color attributes.
Delete the active attribute with the Python console, eg: C.object.data.color_attributes.remove(C.object.data.color_attributes[1]).
Observe that the mesh is left with no active (highlighted) or active render (the little camera icon) color attribute.

Deleting the attribute through the UI, or using Python in 3.2 works correctly.

**System Information** Operating system: Linux **Blender Version** Broken: 3.6.0-alpha+main.e497a50aa207 Worked: 3.2 Caused by 6514bb05ea **Short description of error** Removing a color attribute via the `mesh.color_attributes.remove` API can leave the mesh without an active/active render color attribute. This is yet another continuation of #103761. ![ColorAttribBug.png](/attachments/2accaa8b-404d-4466-bafa-5650c3fc9278) **Exact steps for others to reproduce the error** 1. Create a cube and add some color attributes. 2. Delete the active attribute with the Python console, eg: `C.object.data.color_attributes.remove(C.object.data.color_attributes[1])`. 3. Observe that the mesh is left with no active (highlighted) or active render (the little camera icon) color attribute. Deleting the attribute through the UI, or using Python in 3.2 works correctly.

ColorAttribBug.png

10 KiB

Scurest added the

labels 2023-02-23 19:27:28 +01:00

Iliya Katushenock added the

Sculpt, Paint & Texture

labels 2023-02-23 19:32:07 +01:00

Hans Goudey added

Sculpt, Paint & Texture

and removed

labels 2023-02-23 21:43:52 +01:00

Hans Goudey commented

2023-02-23 21:45:28 +01:00

Member

I looked into this a bit. It seems possible to fix. I just noticed that this has never even worked properly from the UI. When you delete the last attribute in the list, the first attribute becomes active rather than the previous one. That's because it's just calling next_color_attribute before deleting the attribute. So this behavior needs some general fixing/cleanup.

I looked into this a bit. It seems possible to fix. I just noticed that this has never even worked properly from the UI. When you delete the last attribute in the list, the first attribute becomes active rather than the previous one. That's because it's just calling `next_color_attribute` before deleting the attribute. So this behavior needs some general fixing/cleanup.

Hans Goudey added

and removed

labels 2023-02-23 21:45:50 +01:00

Hans Goudey added this to the 3.5 milestone 2023-02-23 21:46:15 +01:00

Iliya Katushenock commented

2023-02-23 21:58:40 +01:00

Member

When you delete the last attribute in the list, the first attribute becomes active rather than the previous one.

I wrote this oddity in the comments of an old report, not sure if it's waiting for more verification...

> When you delete the last attribute in the list, the first attribute becomes active rather than the previous one. I wrote this oddity in the comments of an old report, not sure if it's waiting for more verification...

Philipp Oeser commented

2023-02-24 10:21:28 +01:00

Member

Note : mesh.vertex_colors.remove() is also still not handled at all

Note : `mesh.vertex_colors.remove()` is also still not handled at all

Philipp Oeser changed title from ~~Python: removing a color attribute may leave a mesh without an active color attribute~~ to Regression: Python: removing a color attribute may leave a mesh without an active color attribute

2023-02-24 11:11:47 +01:00

Joseph Eagar commented

2023-02-27 11:24:19 +01:00

Member

Do you think this is something that should be handled in BKE_id_attribute_remove?

Do you think this is something that should be handled in `BKE_id_attribute_remove`?

Joseph Eagar commented

2023-02-27 11:37:15 +01:00

Member

So I guess we need to subclass CustomDataAttributeProvider for this?

So I guess we need to subclass `CustomDataAttributeProvider` for this?

Hans Goudey commented

2023-02-27 19:16:13 +01:00

Member

If we want to handle it in the C++ attribute API, which probably makes sense, a good approach might be to add another callback to MutableAttributeAccessor which could be called after removing an attribute. Adding more logic to CustomDataAttributeProvider might work too, but it's also used in situations that don't need the logic, and it would probably be better to keep that part generic.

If we want to handle it in the C++ attribute API, which probably makes sense, a good approach might be to add another callback to `MutableAttributeAccessor` which could be called after removing an attribute. Adding more logic to `CustomDataAttributeProvider` might work too, but it's also used in situations that don't need the logic, and it would probably be better to keep that part generic.

Thomas Dinges commented

2023-03-14 15:42:55 +01:00

Owner

We are one week away from Bcon4 and this is still scheduled for 3.5. Any updates here?

We are one week away from Bcon4 and this is still scheduled for 3.5. Any updates here?

Jorijn de Graaf commented

2023-03-17 23:53:37 +01:00

Contributor

If nobody is working on this, I could give it a try.
Not an ideal solution but here's something that seems to work, at least in edit mode:

const bool is_active_color_attribute = name == StringRef(mesh->active_color_attribute);
const CustomDataLayer *active_layer_old = BKE_id_attribute_search(
    id, mesh->active_color_attribute, CD_MASK_COLOR_ALL, ATTR_DOMAIN_MASK_COLOR);
int active_index = BKE_id_attribute_to_index(
    id, active_layer_old, ATTR_DOMAIN_MASK_COLOR, CD_MASK_COLOR_ALL);
const bool is_default_color_attribute = name == StringRef(mesh->default_color_attribute);
const CustomDataLayer *default_layer_old = BKE_id_attribute_search(
    id, mesh->default_color_attribute, CD_MASK_COLOR_ALL, ATTR_DOMAIN_MASK_COLOR);
int default_index = BKE_id_attribute_to_index(
    id, default_layer_old, ATTR_DOMAIN_MASK_COLOR, CD_MASK_COLOR_ALL);

if (BM_data_layer_free_named(em->bm, data, name)) {
  if (is_active_color_attribute) {
    const CustomDataLayer *active_layer_new = BKE_id_attribute_from_index(
        id, active_index, ATTR_DOMAIN_MASK_COLOR, CD_MASK_COLOR_ALL);
    BKE_id_attributes_active_color_set(
        id, (active_layer_new) ? active_layer_new->name : nullptr);
  }
  if (is_default_color_attribute) {
    const CustomDataLayer *default_layer_new = BKE_id_attribute_from_index(
        id, default_index, ATTR_DOMAIN_MASK_COLOR, CD_MASK_COLOR_ALL);
    BKE_id_attributes_default_color_set(
        id, (default_layer_new) ? default_layer_new->name : nullptr);
  }
  return true;
}

Basically just get the index, then remove the attribute, and find the attribute that corresponds with that index now.
I'll see about cleaning this up tomorrow, making sure the index is in range etc.

If nobody is working on this, I could give it a try. Not an ideal solution but here's something that seems to work, at least in edit mode: ```cc const bool is_active_color_attribute = name == StringRef(mesh->active_color_attribute); const CustomDataLayer *active_layer_old = BKE_id_attribute_search( id, mesh->active_color_attribute, CD_MASK_COLOR_ALL, ATTR_DOMAIN_MASK_COLOR); int active_index = BKE_id_attribute_to_index( id, active_layer_old, ATTR_DOMAIN_MASK_COLOR, CD_MASK_COLOR_ALL); const bool is_default_color_attribute = name == StringRef(mesh->default_color_attribute); const CustomDataLayer *default_layer_old = BKE_id_attribute_search( id, mesh->default_color_attribute, CD_MASK_COLOR_ALL, ATTR_DOMAIN_MASK_COLOR); int default_index = BKE_id_attribute_to_index( id, default_layer_old, ATTR_DOMAIN_MASK_COLOR, CD_MASK_COLOR_ALL); if (BM_data_layer_free_named(em->bm, data, name)) { if (is_active_color_attribute) { const CustomDataLayer *active_layer_new = BKE_id_attribute_from_index( id, active_index, ATTR_DOMAIN_MASK_COLOR, CD_MASK_COLOR_ALL); BKE_id_attributes_active_color_set( id, (active_layer_new) ? active_layer_new->name : nullptr); } if (is_default_color_attribute) { const CustomDataLayer *default_layer_new = BKE_id_attribute_from_index( id, default_index, ATTR_DOMAIN_MASK_COLOR, CD_MASK_COLOR_ALL); BKE_id_attributes_default_color_set( id, (default_layer_new) ? default_layer_new->name : nullptr); } return true; } ``` Basically just get the index, then remove the attribute, and find the attribute that corresponds with that index now. I'll see about cleaning this up tomorrow, making sure the index is in range etc.

Jorijn de Graaf referenced a pull request that will close this issue

2023-03-18 04:15:32 +01:00

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

Hans Goudey referenced this issue from a commit

2023-03-19 00:57:38 +01:00

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

Hans Goudey closed this issue

2023-03-19 00:58:17 +01:00

Blender Bot added

and removed

labels 2023-03-19 00:58:18 +01:00

Sign in to join this conversation.

No Label

Animation & Rigging

Asset Browser Project Overview

Automated Testing

Blender Asset Bundle

Dependency Graph

Development Management

EEVEE & Viewport

Images & Movies

Motion Tracking

Nodes & Physics

Pipeline, Assets & IO

Platforms, Builds & Tests

Render & Cycles

Render Pipeline

Sculpt, Paint & Texture

Video Sequencer

Virtual Reality

Blender 2.8 Project

Milestone 1: Basic, Local Asset Browser

Good First Issue

Animation & Rigging

Development Management

EEVEE & Viewport

Nodes & Physics

Pipeline, Assets & IO

Platforms, Builds & Tests

Render & Cycles

Sculpt, Paint & Texture

Needs Info from Developers

Needs Information from User

No Milestone

No project

No Assignees

7 Participants

Notifications

Due Date

The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: blender/blender#105152

No description provided.