Deleting bone constraints when the armature layer isn't active crashes Blender #80464

New Issue

Chia · 2020-09-04T11:49:51+02:00

Chia commented

2020-09-04 11:49:51 +02:00

System Information
Operating system: Win 10
Graphics card: RTX2070

Blender Version
Broken: 2.90, 2.91 alpha
Worked: 2.83.5
Caused by 608d9b5aa1: UI: Add shortcuts for constraint panels

Short description of error
Deleting bone constraints when the armature layer isn't active crashes Blender

Exact steps for others to reproduce the error
The attached file has 2 bones in 1 armature. Bone.001 is constrained to Bone, and the two bones are in separate armature layers. With both layers active, deleting the constraint in Bone.001 works normally. But if layer 16 isn't active, doing the same thing will crash Blender.
crashtest_2_90.blend

**System Information** Operating system: Win 10 Graphics card: RTX2070 **Blender Version** Broken: 2.90, 2.91 alpha Worked: 2.83.5 Caused by 608d9b5aa1: UI: Add shortcuts for constraint panels **Short description of error** Deleting bone constraints when the armature layer isn't active crashes Blender **Exact steps for others to reproduce the error** The attached file has 2 bones in 1 armature. Bone.001 is constrained to Bone, and the two bones are in separate armature layers. With both layers active, deleting the constraint in Bone.001 works normally. But if layer 16 isn't active, doing the same thing will crash Blender. [crashtest_2_90.blend](https://archive.blender.org/developer/F8839639/crashtest_2_90.blend)

Chia commented

2020-09-04 11:49:51 +02:00

Added subscriber: @alphis

Philipp Oeser commented

2020-09-04 12:06:59 +02:00

Added subscriber: @lichtwerk

Philipp Oeser self-assigned this 2020-09-04 12:06:59 +02:00

Philipp Oeser commented

2020-09-04 12:06:59 +02:00

Changed status from 'Needs Triage' to: 'Confirmed'

Philipp Oeser commented

2020-09-04 12:06:59 +02:00

Can confirm.

Philipp Oeser commented

2020-09-04 14:50:41 +02:00

Caused by 608d9b5aa1

Prior to 608d9b5aa1, the constraint was gotten using context [CTX_data_pointer_get_type(C, "constraint", &RNA_Constraint) -- which is valid for bones on hidden layers].
After 608d9b5aa1, the constraint is found (or isnt) using edit_constraint_property_get [this is not valid for bones on hidden layers because internally BKE_pose_channel_active checks if the bone is on an active layer].

Some observations:

Every operator using edit_constraint_property_get doesnt work for bones on inactive layers [delete, moveup, movedown, move to index (drag n drop nowadays)]
- moveup, movedown, move to index check if they could find a constraint beforehand though (dont crash)
- delete crashes (doesnt check if a constraint could actually be found)
Every operator using edit_constraint_property_get for constraint data doesnt work for bones on inactive layers [stretchto_reset, limitdistance_reset, childof_set_inverse, ...]
- these all check if they could find a constraint beforehand though (dont crash)

This is because the poll function is using context to get the constraint, the operators themselves use edit_constraint_property_get which leads to inconsistent/unexpected results.

Possible solutions:

- let the delete operator just work with the context constraint again (like prior to 608d9b5aa1) -- allows for deleting constraints on bones in inactive layers
- check if we could get a constraint -- prevents the crash, but does not allow for deleting constraints on bones in inactive layers
- make the poll edit_constraint_poll_generic be as strict as the operators -- dont use context to get the constraint, but something like edit_constraint_property_get
- make the operators be more graceful and let them act on bones on hidden layers -- let edit_constraint_property_get actually use context

Whatever we decide upon: this might be a candidate for 2.90.1? Will check on all solutions, might go for the quick fix - [x] first, then try [4]?

Note: Adding constraints also doesnt work for bones on inactive layers [that was the case in 2.79 as well -- it is also using BKE_pose_channel_active]
Note: edit_constraint_invoke_properties also uses said context

Caused by 608d9b5aa1 Prior to 608d9b5aa1, the constraint was gotten using **context** [CTX_data_pointer_get_type(C, "constraint", &RNA_Constraint) -- which is valid for bones on hidden layers]. After 608d9b5aa1, the constraint is found (or isnt) using `edit_constraint_property_get` [this is **not** valid for bones on hidden layers because internally `BKE_pose_channel_active` checks if the bone is on an active layer]. Some observations: - Every operator using `edit_constraint_property_get` doesnt work for bones on inactive layers [delete, moveup, movedown, move to index (drag n drop nowadays)] - moveup, movedown, move to index check if they could find a constraint beforehand though (dont crash) - delete crashes (doesnt check if a constraint could actually be found) - Every operator using `edit_constraint_property_get` for constraint data doesnt work for bones on inactive layers [stretchto_reset, limitdistance_reset, childof_set_inverse, ...] - these all check if they could find a constraint beforehand though (dont crash) This is because the poll function is using **context** to get the constraint, the operators themselves use **edit_constraint_property_get** which leads to inconsistent/unexpected results. Possible solutions: - - [x] let the delete operator just work with the context constraint again (like prior to 608d9b5aa1) -- allows for deleting constraints on bones in inactive layers - - [x] check if we could get a constraint -- prevents the crash, but does **not** allow for deleting constraints on bones in inactive layers - - [x] make the poll `edit_constraint_poll_generic` be as strict as the operators -- dont use **context** to get the constraint, but something like **edit_constraint_property_get** - - [x] make the operators be more graceful and let them act on bones on hidden layers -- let **edit_constraint_property_get** actually use **context** Whatever we decide upon: this might be a candidate for 2.90.1? Will check on all solutions, might go for the quick fix - [x] first, then try [4]? Note: Adding constraints also doesnt work for bones on inactive layers [that was the case in 2.79 as well -- it is also using `BKE_pose_channel_active`] Note: `edit_constraint_invoke_properties` also uses said **context**

Sybren A. Stüvel commented

2020-09-04 15:15:26 +02:00

Added subscriber: @dr.sybren

Sybren A. Stüvel commented

2020-09-04 15:15:26 +02:00

Good find @lichtwerk. I think it's important to have the poll function actually in sync with the rest of the operator; without that, it's rather useless. And first doing - [x] and then trying - [x] sounds good to me.