[Bf-blender-cvs] [6dc7266cf1f] master: Fix T80464: Crash deleting bone constraints when the armature layer is not active

[Philipp Oeser]

Commit: 6dc7266cf1f4ee4e6e6eaa7e4949fcff11372479
Author: Philipp Oeser
Date:   Wed Sep 9 14:10:47 2020 +0200
Branches: master
https://developer.blender.org/rB6dc7266cf1f4ee4e6e6eaa7e4949fcff11372479

Fix T80464: Crash deleting bone constraints when the armature layer is
not active

Caused by {rB608d9b5aa1f1}

Prior to rB608d9b5aa1f1, the constraint was gotten using **context**
[CTX_data_pointer_get_type(C, "constraint", &RNA_Constraint) -- which is
valid for bones on hidden layers].
After rB608d9b5aa1f1, 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 were:
- [1] let the delete operator just work with the context constraint
again (like prior to rB608d9b5aa1f1) -- allows for deleting constraints
on bones in inactive layers
- [2] check if we could get a constraint -- prevents the crash, but does
**not** allow for deleting constraints on bones in inactive layers
- [3] 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**
- [4] make the operators be more graceful and let them act on bones on
hidden layers -- let **edit_constraint_property_get** actually use the
same **context**

This patch implements [4], so poll an doperators are now in sync.
- prevents reported crash
- also enables operators for bone constraints on hidden layers
- also enables drag and drop reordering of constraints on hidden layers

This might be a candidate for 2.90.1? (if it is, take care to include
prior "Refactor getting constraints" refactoring commit)

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`]

Maniphest Tasks: T80464

Differential Revision: https://developer.blender.org/D8805

===================================================================

M	source/blender/editors/object/object_constraint.c

===================================================================

diff --git a/source/blender/editors/object/object_constraint.c b/source/blender/editors/object/object_constraint.c
index 9da5b774af2..2f9917a2674 100644
--- a/source/blender/editors/object/object_constraint.c
+++ b/source/blender/editors/object/object_constraint.c
@@ -793,7 +793,7 @@ static bool edit_constraint_invoke_properties(bContext *C,
   return false;
 }
 
-static bConstraint *edit_constraint_property_get(wmOperator *op, Object *ob, int type)
+static bConstraint *edit_constraint_property_get(bContext *C, wmOperator *op, Object *ob, int type)
 {
   char constraint_name[MAX_NAME];
   int owner = RNA_enum_get(op->ptr, "owner");
@@ -802,30 +802,10 @@ static bConstraint *edit_constraint_property_get(wmOperator *op, Object *ob, int
 
   RNA_string_get(op->ptr, "constraint", constraint_name);
 
-  if (owner == EDIT_CONSTRAINT_OWNER_OBJECT) {
-    list = &ob->constraints;
-  }
-  else if (owner == EDIT_CONSTRAINT_OWNER_BONE) {
-    bPoseChannel *pchan = BKE_pose_channel_active(ob);
-    if (pchan) {
-      list = &pchan->constraints;
-    }
-    else {
-#if 0
-      if (G.debug & G_DEBUG) {
-        printf("edit_constraint_property_get: No active bone for object '%s'\n",
-               (ob) ? ob->id.name + 2 : "<None>");
-      }
-#endif
-      return NULL;
-    }
+  if (owner == EDIT_CONSTRAINT_OWNER_BONE) {
+    list = ED_object_pose_constraint_list(C);
   }
   else {
-#if 0
-    if (G.debug & G_DEBUG) {
-      printf("edit_constraint_property_get: defaulting to getting list in the standard way\n");
-    }
-#endif
     list = ED_object_constraint_active_list(ob);
   }
 
@@ -855,7 +835,7 @@ static int stretchto_reset_exec(bContext *C, wmOperator *op)
 {
   Main *bmain = CTX_data_main(C);
   Object *ob = ED_object_active_context(C);
-  bConstraint *con = edit_constraint_property_get(op, ob, CONSTRAINT_TYPE_STRETCHTO);
+  bConstraint *con = edit_constraint_property_get(C, op, ob, CONSTRAINT_TYPE_STRETCHTO);
   bStretchToConstraint *data = (con) ? (bStretchToConstraint *)con->data : NULL;
 
   /* despite 3 layers of checks, we may still not be able to find a constraint */
@@ -910,7 +890,7 @@ static int limitdistance_reset_exec(bContext *C, wmOperator *op)
 {
   Main *bmain = CTX_data_main(C);
   Object *ob = ED_object_active_context(C);
-  bConstraint *con = edit_constraint_property_get(op, ob, CONSTRAINT_TYPE_DISTLIMIT);
+  bConstraint *con = edit_constraint_property_get(C, op, ob, CONSTRAINT_TYPE_DISTLIMIT);
   bDistLimitConstraint *data = (con) ? (bDistLimitConstraint *)con->data : NULL;
 
   /* despite 3 layers of checks, we may still not be able to find a constraint */
@@ -982,7 +962,7 @@ static int childof_set_inverse_exec(bContext *C, wmOperator *op)
 {
   Main *bmain = CTX_data_main(C);
   Object *ob = ED_object_active_context(C);
-  bConstraint *con = edit_constraint_property_get(op, ob, CONSTRAINT_TYPE_CHILDOF);
+  bConstraint *con = edit_constraint_property_get(C, op, ob, CONSTRAINT_TYPE_CHILDOF);
   bChildOfConstraint *data = (con) ? (bChildOfConstraint *)con->data : NULL;
 
   /* despite 3 layers of checks, we may still not be able to find a constraint */
@@ -1036,7 +1016,7 @@ static int childof_clear_inverse_exec(bContext *C, wmOperator *op)
 {
   Main *bmain = CTX_data_main(C);
   Object *ob = ED_object_active_context(C);
-  bConstraint *con = edit_constraint_property_get(op, ob, CONSTRAINT_TYPE_CHILDOF);
+  bConstraint *con = edit_constraint_property_get(C, op, ob, CONSTRAINT_TYPE_CHILDOF);
   bChildOfConstraint *data = (con) ? (bChildOfConstraint *)con->data : NULL;
 
   if (data == NULL) {
@@ -1090,7 +1070,7 @@ static int followpath_path_animate_exec(bContext *C, wmOperator *op)
 {
   Main *bmain = CTX_data_main(C);
   Object *ob = ED_object_active_context(C);
-  bConstraint *con = edit_constraint_property_get(op, ob, CONSTRAINT_TYPE_FOLLOWPATH);
+  bConstraint *con = edit_constraint_property_get(C, op, ob, CONSTRAINT_TYPE_FOLLOWPATH);
   bFollowPathConstraint *data = (con) ? (bFollowPathConstraint *)con->data : NULL;
 
   bAction *act = NULL;
@@ -1234,7 +1214,7 @@ static int objectsolver_set_inverse_exec(bContext *C, wmOperator *op)
 {
   Main *bmain = CTX_data_main(C);
   Object *ob = ED_object_active_context(C);
-  bConstraint *con = edit_constraint_property_get(op, ob, CONSTRAINT_TYPE_OBJECTSOLVER);
+  bConstraint *con = edit_constraint_property_get(C, op, ob, CONSTRAINT_TYPE_OBJECTSOLVER);
   bObjectSolverConstraint *data = (con) ? (bObjectSolverConstraint *)con->data : NULL;
 
   /* despite 3 layers of checks, we may still not be able to find a constraint */
@@ -1296,7 +1276,7 @@ static int objectsolver_clear_inverse_exec(bContext *C, wmOperator *op)
 {
   Main *bmain = CTX_data_main(C);
   Object *ob = ED_object_active_context(C);
-  bConstraint *con = edit_constraint_property_get(op, ob, CONSTRAINT_TYPE_OBJECTSOLVER);
+  bConstraint *con = edit_constraint_property_get(C, op, ob, CONSTRAINT_TYPE_OBJECTSOLVER);
   bObjectSolverConstraint *data = (con) ? (bObjectSolverConstraint *)con->data : NULL;
 
   if (data == NULL) {
@@ -1444,7 +1424,7 @@ static int constraint_delete_exec(bContext *C, wmOperator *op)
 {
   Main *bmain = CTX_data_main(C);
   Object *ob = ED_object_active_context(C);
-  bConstraint *con = edit_constraint_property_get(op, ob, 0);
+  bConstraint *con = edit_constraint_property_get(C, op, ob, 0);
   ListBase *lb = ED_object_constraint_list_from_constraint(ob, con, NULL);
 
   /* Store name temporarily for report. */
@@ -1510,7 +1490,7 @@ void CONSTRAINT_OT_delete(wmOperatorType *ot)
 static int constraint_move_down_exec(bContext *C, wmOperator *op)
 {
   Object *ob = ED_object_active_context(C);
-  bConstraint *con = edit_constraint_property_get(op, ob, 0);
+  bConstraint *con = edit_constraint_property_get(C, op, ob, 0);
 
   if (con && con->next) {
     ListBase *conlist = ED_object_constraint_list_from_constraint(ob, con, NULL);
@@ -1565,7 +1545,7 @@ void CONSTRAINT_OT_move_down(wmOperatorType *ot)
 static int constraint_move_up_exec(bContext *C, wmOperator *op)
 {
   Object *ob = ED_object_active_context(C);
-  bConstraint *con = edit_constraint_property_get(op, ob, 0);
+  bConstraint *con = edit_constraint_property_get(C, op, ob, 0);
 
   if (con && con->prev) {
     ListBase *conlist = ED_object_constraint_list_from_constraint(ob, con, NULL);
@@ -1618,7 +1598,7 @@ void CONSTRAINT_OT_move_up(wmOperatorType *ot)
 static int constraint_move_to_index_exec(bContext *C, wmOperator *op)
 {
   Object *ob = ED_object_active_context(C);
-  bConstraint *con = edit_constraint_property_get(op, ob, 0);
+  bConstraint *con = edit_constraint_property_get(C, op, ob, 0);
 
   int new_index = RNA_int_get(op->ptr, "index");
   if (new_index < 0) {