bugfix [#25712] Deletion of vertex groups under script control causes incorrect reassignment of vertices in other groups

vgroup functions were mixing up active group and one passed as an argument.

also made other changes.
- removed superfluous call to defvert_find_index() in vgroup_delete_object_mode(), was also doing unnecessary NULL check on each loop.
- remove paranoid NULL check from ED_vgroup_vert_remove, callers all check for valid 'ob'

1 files changed, 53 insertions, 68 deletions

diff --git a/source/blender/editors/object/object_vgroup.c b/source/blender/editors/object/object_vgroup.c
index 41d9ca93a9b..bd7983004eb 100644
--- a/source/blender/editors/object/object_vgroup.c
+++ b/source/blender/editors/object/object_vgroup.c
@@ -30,6 +30,7 @@
 #include <string.h>
 #include <stddef.h>
 #include <math.h>
+#include <assert.h>
 
 #include "MEM_guardedalloc.h"
 
@@ -168,8 +169,7 @@ int ED_vgroup_give_parray(ID *id, MDeformVert ***dvert_arr, int *dvert_tot)
 						return 0;
 					}
 
-					i = 0;
-					for (eve=em->verts.first; eve; eve=eve->next) i++;
+					i= BLI_countlist(&em->verts);
 
 					*dvert_arr= MEM_mallocN(sizeof(void*)*i, "vgroup parray from me");
 					*dvert_tot = i;
@@ -492,23 +492,10 @@ void ED_vgroup_vert_remove(Object *ob, bDeformGroup	*dg, int vertnum)
 	/* This routine removes the vertex from the specified
 	 * deform group.
 	 */
-
-	int def_nr;
-
-	/* if the object is NULL abort
-	 */
-	if(!ob)
+	const int def_nr= defgroup_find_index(ob, dg);
+	if(def_nr < 0)
 		return;
 
-	/* get the deform number that cooresponds
-	 * to this deform group, and abort if it
-	 * can not be found.
-	 */
-	def_nr = defgroup_find_index(ob, dg);
-	if(def_nr < 0) return;
-
-	/* call another routine to do the work
-	 */
 	ED_vgroup_nr_vert_remove(ob, def_nr, vertnum);
 }
 
@@ -1128,55 +1115,52 @@ static void vgroup_delete_update_users(Object *ob, int id)
 
 static void vgroup_delete_object_mode(Object *ob, bDeformGroup *dg)
 {
-	MDeformVert *dvert, *dvert_array=NULL;
+	MDeformVert *dvert_array=NULL;
 	int i, e, dvert_tot=0;
+	const int dg_index= BLI_findindex(&ob->defbase, dg);
+
+	assert(dg_index > -1);
 	
 	ED_vgroup_give_array(ob->data, &dvert_array, &dvert_tot);
 
 	if(dvert_array) {
-		for(i = 0; i < dvert_tot; i++) {
-			dvert = dvert_array + i;
-			if(dvert) {
-				if(defvert_find_index(dvert, (ob->actdef-1)))
-					ED_vgroup_vert_remove(ob, dg, i);
-			}
+		MDeformVert *dvert;
+		for(i= 0, dvert= dvert_array; i < dvert_tot; i++, dvert++) {
+			ED_vgroup_vert_remove(ob, dg, i); /* ok if the dg isnt in this dvert, will continue silently */
 		}
 
-		for(i = 0; i < dvert_tot; i++) {
-			dvert = dvert_array+i;
-			if(dvert) {
-				for(e = 0; e < dvert->totweight; e++) {
-					if(dvert->dw[e].def_nr > (ob->actdef-1))
-						dvert->dw[e].def_nr--;
+		for(i= 0, dvert= dvert_array; i < dvert_tot; i++, dvert++) {
+			for(e = 0; e < dvert->totweight; e++) {
+				if(dvert->dw[e].def_nr > dg_index) {
+					dvert->dw[e].def_nr--;
 				}
 			}
 		}
 	}
 
-	vgroup_delete_update_users(ob, ob->actdef);
+	vgroup_delete_update_users(ob, dg_index + 1);
 
-	/* Update the active deform index if necessary */
-	if(ob->actdef == BLI_countlist(&ob->defbase))
-		ob->actdef--;
-  
 	/* Remove the group */
 	BLI_freelinkN(&ob->defbase, dg);
+
+	/* Update the active deform index if necessary */
+	if(ob->actdef > dg_index)
+		ob->actdef--;
+	if(ob->actdef < 1 && ob->defbase.first)
+		ob->actdef= 1;
+
 }
 
 /* only in editmode */
 /* removes from active defgroup, if allverts==0 only selected vertices */
-static void vgroup_active_remove_verts(Object *ob, int allverts)
+static void vgroup_active_remove_verts(Object *ob, const int allverts, bDeformGroup *dg)
 {
 	EditVert *eve;
 	MDeformVert *dvert;
 	MDeformWeight *newdw;
-	bDeformGroup *dg, *eg;
+	bDeformGroup *eg;
 	int	i;
 
-	dg=BLI_findlink(&ob->defbase, ob->actdef-1);
-	if(!dg)
-		return;
-
 	if(ob->type == OB_MESH) {
 		Mesh *me= ob->data;
 		EditMesh *em = BKE_mesh_get_editmesh(me);
@@ -1226,15 +1210,15 @@ static void vgroup_active_remove_verts(Object *ob, int allverts)
 	}
 }
 
-static void vgroup_delete_edit_mode(Object *ob, bDeformGroup *defgroup)
+static void vgroup_delete_edit_mode(Object *ob, bDeformGroup *dg)
 {
 	int i;
+	const int dg_index= BLI_findindex(&ob->defbase, dg);
 
-	if(!ob->actdef)
-		return;
+	assert(dg_index > -1);
 
 	/* Make sure that no verts are using this group */
-	vgroup_active_remove_verts(ob, 1);
+	vgroup_active_remove_verts(ob, TRUE, dg);
 
 	/* Make sure that any verts with higher indices are adjusted accordingly */
 	if(ob->type==OB_MESH) {
@@ -1248,7 +1232,7 @@ static void vgroup_delete_edit_mode(Object *ob, bDeformGroup *defgroup)
 
 			if(dvert)
 				for(i=0; i<dvert->totweight; i++)
-					if(dvert->dw[i].def_nr > (ob->actdef-1))
+					if(dvert->dw[i].def_nr > dg_index)
 						dvert->dw[i].def_nr--;
 		}
 		BKE_mesh_end_editmesh(me, em);
@@ -1263,24 +1247,26 @@ static void vgroup_delete_edit_mode(Object *ob, bDeformGroup *defgroup)
 			tot= lt->pntsu*lt->pntsv*lt->pntsw;
 			for(a=0, bp= lt->def; a<tot; a++, bp++, dvert++) {
 				for(i=0; i<dvert->totweight; i++){
-					if(dvert->dw[i].def_nr > (ob->actdef-1))
+					if(dvert->dw[i].def_nr > dg_index)
 						dvert->dw[i].def_nr--;
 				}
 			}
 		}
 	}
 
-	vgroup_delete_update_users(ob, ob->actdef);
+	vgroup_delete_update_users(ob, dg_index + 1);
+
+	/* Remove the group */
+	BLI_freelinkN (&ob->defbase, dg);
 
 	/* Update the active deform index if necessary */
-	if(ob->actdef==BLI_countlist(&ob->defbase))
+	if(ob->actdef > dg_index)
 		ob->actdef--;
-	
-	/* Remove the group */
-	BLI_freelinkN (&ob->defbase, defgroup);
-	
+	if(ob->actdef < 1 && ob->defbase.first)
+		ob->actdef= 1;
+
 	/* remove all dverts */
-	if(ob->actdef==0) {
+	if(ob->defbase.first == NULL) {
 		if(ob->type==OB_MESH) {
 			Mesh *me= ob->data;
 			CustomData_free_layer_active(&me->vdata, CD_MDEFORMVERT, me->totvert);
@@ -1417,22 +1403,14 @@ static void vgroup_assign_verts(Object *ob, float weight)
 /* removes from all defgroup, if allverts==0 only selected vertices */
 static void vgroup_remove_verts(Object *ob, int allverts)
 {
-	int actdef, defCount;
-	
-	actdef= ob->actdef;
-	defCount= BLI_countlist(&ob->defbase);
-	
-	if(defCount == 0)
-		return;
-	
 	/* To prevent code redundancy, we just use vgroup_active_remove_verts, but that
 	 * only operates on the active vgroup. So we iterate through all groups, by changing
 	 * active group index
 	 */
-	for(ob->actdef= 1; ob->actdef <= defCount; ob->actdef++)
-		vgroup_active_remove_verts(ob, allverts);
-		
-	ob->actdef= actdef;
+	bDeformGroup *dg;
+	for(dg= ob->defbase.first; dg; dg= dg->next) {
+		vgroup_active_remove_verts(ob, allverts, dg);
+	}
 }
 
 /********************** vertex group operators *********************/
@@ -1552,8 +1530,15 @@ static int vertex_group_remove_from_exec(bContext *C, wmOperator *op)
 
 	if(RNA_boolean_get(op->ptr, "all"))
 		vgroup_remove_verts(ob, 0);
-	else
-		vgroup_active_remove_verts(ob, 0);
+	else {
+		bDeformGroup *dg= BLI_findlink(&ob->defbase, ob->actdef - 1);
+
+		if(dg == NULL) {
+			return OPERATOR_CANCELLED;
+		}
+
+		vgroup_active_remove_verts(ob, FALSE, dg);
+	}
 
 	DAG_id_tag_update(&ob->id, OB_RECALC_DATA);
 	WM_event_add_notifier(C, NC_GEOM|ND_DATA, ob->data);