Fix #110650: Drivers not removed if removing their container

This applies to modifiers, constraints and shape keys.
Any driver on such data was not removed with it,
leaving invalid drivers on objects.

With this patch, the drivers are removed, but animation is left untouched.
This is because animation might be reused somewhere else and we don't
want to introduce potential side effects there.

This also adds unit tests for the fixed cases

Pull Request: https://projects.blender.org/blender/blender/pulls/134511

source/blender/blenkernel/BKE_animsys.h

@@ -187,6 +187,25 @@ void BKE_animdata_fix_paths_rename_all(struct ID *ref_id,
  */
 bool BKE_animdata_fix_paths_remove(struct ID *id, const char *prefix);
 
+/**
+ * Remove drivers that have an RNA path starting with `prefix`.
+ *
+ * \return true if any driver was removed.
+ */
+bool BKE_animdata_driver_path_remove(struct ID *id, const char *prefix);
+
+/**
+ * Remove all drivers from the given struct.
+ *
+ * \param type needs to be a struct owned by the given ID.
+ * \param data the actual struct data, needs to be the data for the StructRNA.
+ *
+ * \return true if any driver was removed.
+ */
+bool BKE_animdata_drivers_remove_for_rna_struct(struct ID &owner_id,
+                                                struct StructRNA &type,
+                                                void *data);
+
 /* -------------------------------------- */
 
 typedef struct AnimationBasePathChange {

source/blender/blenkernel/BKE_constraint.h

@@ -243,11 +243,10 @@ struct bConstraint *BKE_constraint_add_for_pose(struct Object *ob,
                                                 const char *name,
                                                 short type);
 
-bool BKE_constraint_remove_ex(ListBase *list, struct Object *ob, struct bConstraint *con);
 /**
  * Remove the specified constraint from the given constraint stack.
  */
-bool BKE_constraint_remove(ListBase *list, struct bConstraint *con);
+bool BKE_constraint_remove_ex(ListBase *list, struct Object *ob, struct bConstraint *con);
 
 /**
  * Apply the specified constraint in the given constraint stack.

source/blender/blenkernel/intern/anim_data.cc

@@ -1216,6 +1216,29 @@ bool BKE_animdata_fix_paths_remove(ID *id, const char *prefix)
   return any_removed;
 }
 
+bool BKE_animdata_driver_path_remove(ID *id, const char *prefix)
+{
+  AnimData *adt = BKE_animdata_from_id(id);
+  if (!adt) {
+    return false;
+  }
+
+  const bool any_removed = fcurves_path_remove_from_listbase(prefix, &adt->drivers);
+  return any_removed;
+}
+
+bool BKE_animdata_drivers_remove_for_rna_struct(ID &owner_id, StructRNA &type, void *data)
+{
+  PointerRNA constraint_ptr = RNA_pointer_create_discrete(&owner_id, &type, data);
+  const std::optional<std::string> base_path = RNA_path_from_ID_to_struct(&constraint_ptr);
+  if (!base_path.has_value()) {
+    /* The data should exist, so the path should always resolve. */
+    BLI_assert_unreachable();
+  }
+
+  return BKE_animdata_driver_path_remove(&owner_id, base_path.value().c_str());
+}
+
 /* Apply Op to All FCurves in Database --------------------------- */
 
 /* Helper for adt_apply_all_fcurves_cb() - Apply wrapped operator to list of F-Curves */

source/blender/blenkernel/intern/constraint.cc

@@ -72,6 +72,8 @@
 
 #include "BIK_api.h"
 
+#include "RNA_prototypes.hh"
+
 #include "DEG_depsgraph.hh"
 #include "DEG_depsgraph_query.hh"
 
@@ -5728,7 +5730,7 @@ void BKE_constraints_free(ListBase *list)
   BKE_constraints_free_ex(list, true);
 }
 
-bool BKE_constraint_remove(ListBase *list, bConstraint *con)
+static bool constraint_remove(ListBase *list, bConstraint *con)
 {
   if (con) {
     BKE_constraint_free_data(con);
@@ -5741,8 +5743,10 @@ bool BKE_constraint_remove(ListBase *list, bConstraint *con)
 
 bool BKE_constraint_remove_ex(ListBase *list, Object *ob, bConstraint *con)
 {
+  BKE_animdata_drivers_remove_for_rna_struct(ob->id, RNA_Constraint, con);
+
   const short type = con->type;
-  if (BKE_constraint_remove(list, con)) {
+  if (constraint_remove(list, con)) {
     /* ITASC needs to be rebuilt once a constraint is removed #26920. */
     if (ELEM(type, CONSTRAINT_TYPE_KINEMATIC, CONSTRAINT_TYPE_SPLINEIK)) {
       BIK_clear_data(ob->pose);

source/blender/blenkernel/intern/object.cc

@@ -137,6 +137,8 @@
 
 #include "ANIM_action_legacy.hh"
 
+#include "RNA_prototypes.hh"
+
 #ifdef WITH_PYTHON
 #  include "BPY_extern.hh"
 #endif
@@ -4459,6 +4461,8 @@ bool BKE_object_shapekey_remove(Main *bmain, Object *ob, KeyBlock *kb)
     return false;
   }
 
+  BKE_animdata_drivers_remove_for_rna_struct(key->id, RNA_ShapeKey, kb);
+
   kb_index = BLI_findindex(&key->block, kb);
   BLI_assert(kb_index != -1);
 

source/blender/editors/object/object_constraint.cc

@@ -2759,7 +2759,7 @@ static int pose_ik_clear_exec(bContext *C, wmOperator * /*op*/)
     for (con = static_cast<bConstraint *>(pchan->constraints.first); con; con = next) {
       next = con->next;
       if (con->type == CONSTRAINT_TYPE_KINEMATIC) {
-        BKE_constraint_remove(&pchan->constraints, con);
+        BKE_constraint_remove_ex(&pchan->constraints, ob, con);
       }
     }
     pchan->constflag &= ~(PCHAN_HAS_IK | PCHAN_HAS_NO_TARGET);

source/blender/editors/object/object_modifier.cc

@@ -384,6 +384,8 @@ static bool object_modifier_remove(
     ob->mode &= ~OB_MODE_PARTICLE_EDIT;
   }
 
+  BKE_animdata_drivers_remove_for_rna_struct(ob->id, RNA_Modifier, md);
+
   BKE_modifier_remove_from_list(ob, md);
   BKE_modifier_free(md);
   BKE_object_free_derived_caches(ob);

source/blender/editors/object/object_relations.cc

@@ -1199,7 +1199,7 @@ static int object_track_clear_exec(bContext *C, wmOperator *op)
                CONSTRAINT_TYPE_LOCKTRACK,
                CONSTRAINT_TYPE_DAMPTRACK))
       {
-        BKE_constraint_remove(&ob->constraints, con);
+        BKE_constraint_remove_ex(&ob->constraints, ob, con);
       }
     }
 

source/blender/makesrna/intern/rna_object.cc

@@ -1630,7 +1630,7 @@ static void rna_Object_constraints_remove(Object *object,
     return;
   }
 
-  BKE_constraint_remove(&object->constraints, con);
+  BKE_constraint_remove_ex(&object->constraints, object, con);
   RNA_POINTER_INVALIDATE(con_ptr);
 
   blender::ed::object::constraint_update(bmain, object);

source/blender/makesrna/intern/rna_pose.cc

@@ -388,7 +388,7 @@ static void rna_PoseChannel_constraints_remove(
     return;
   }
 
-  BKE_constraint_remove(&pchan->constraints, con);
+  BKE_constraint_remove_ex(&pchan->constraints, ob, con);
   RNA_POINTER_INVALIDATE(con_ptr);
 
   blender::ed::object::constraint_update(bmain, ob);

tests/python/bl_animation_drivers.py

@@ -194,6 +194,68 @@ class ContextViewLayerDriverTest(AbstractEmptyDriverTest, unittest.TestCase):
         self.assertPropValue('test_fallback', 321)
 
 
+class SubDataDriverRemovalTest(AbstractEmptyDriverTest, unittest.TestCase):
+
+    def test_remove_object_modifier(self):
+        # Removing a modifier with a driver should also delete the driver
+        modifier = self.obj.modifiers.new("test", 'ARRAY')
+        # No animation data means no drivers.
+        self.assertEqual(self.obj.animation_data, None)
+        self.obj.driver_add('modifiers["test"].count')
+        self.assertEqual(len(self.obj.animation_data.drivers), 1)
+        self.obj.modifiers.remove(modifier)
+        self.assertEqual(len(self.obj.modifiers), 0)
+        self.assertEqual(len(self.obj.animation_data.drivers), 0,
+                         "Removing the modifier should remove the driver on it")
+
+    def test_remove_object_constraint(self):
+        # Using limit distance constraint because that has a property that can have a driver.
+        constraint = self.obj.constraints.new('LIMIT_DISTANCE')
+        constraint.name = "test"
+        # No animation data means no drivers.
+        self.assertEqual(self.obj.animation_data, None)
+        self.obj.driver_add('constraints["test"].distance')
+        self.assertEqual(len(self.obj.animation_data.drivers), 1)
+        self.obj.constraints.remove(constraint)
+        self.assertEqual(len(self.obj.constraints), 0)
+        self.assertEqual(len(self.obj.animation_data.drivers), 0,
+                         "Removing the constraint should remove the driver on it")
+
+    def test_remove_bone_constraint(self):
+        arm = bpy.data.armatures.new('Armature')
+        arm_ob = bpy.data.objects.new('ArmObject', arm)
+        bpy.context.scene.collection.objects.link(arm_ob)
+        bpy.context.view_layer.objects.active = arm_ob
+        bpy.ops.object.mode_set(mode='EDIT')
+        ebone = arm.edit_bones.new(name="test")
+        ebone.tail = (1, 0, 0)
+        bpy.ops.object.mode_set(mode='POSE')
+        pose_bone = arm_ob.pose.bones["test"]
+        constraint = pose_bone.constraints.new('LIMIT_DISTANCE')
+        constraint.name = "test"
+        self.assertEqual(len(pose_bone.constraints), 1)
+        arm_ob.driver_add('pose.bones["test"].constraints["test"].distance')
+        self.assertEqual(len(arm_ob.animation_data.drivers), 1)
+        pose_bone.constraints.remove(constraint)
+        self.assertEqual(len(pose_bone.constraints), 0)
+        self.assertEqual(len(arm_ob.animation_data.drivers), 0,
+                         "Removing the constraint should remove the driver on it")
+
+    def test_remove_shapekey(self):
+        self.obj.shape_key_add(name="base")
+        test_key = self.obj.shape_key_add(name="test")
+        # Due to the weirdness of shapekeys, this is an ID.
+        shape_key_id = self.obj.data.shape_keys
+        self.assertEqual(len(shape_key_id.key_blocks), 2)
+        shape_key_id.driver_add('key_blocks["test"].value')
+        self.assertEqual(len(shape_key_id.animation_data.drivers), 1)
+
+        self.obj.shape_key_remove(test_key)
+        self.assertEqual(len(shape_key_id.key_blocks), 1)
+        self.assertEqual(len(shape_key_id.animation_data.drivers), 0,
+                         "Removing the shape key should remove any driver on it")
+
+
 def main():
     global args
     import argparse