fix for extruding edges giving incorrect (swapped) loop data for new faces.

1 files changed, 44 insertions, 55 deletions

diff --git a/source/blender/bmesh/operators/bmo_extrude.c b/source/blender/bmesh/operators/bmo_extrude.c
index 2b0f08ef815..627351ead11 100644
--- a/source/blender/bmesh/operators/bmo_extrude.c
+++ b/source/blender/bmesh/operators/bmo_extrude.c
@@ -132,46 +132,36 @@ void bmo_extrude_discrete_faces_exec(BMesh *bm, BMOperator *op)
  * This function won't crash if its not but won't work right either.
  * \a e_b is the new edge.
  *
- * \note this function could be exposed as an api call if other areas need it,
- * so far only extrude does.
+ * \note The edge this face comes from needs to be from the first and second verts fo the face.
+ * The caller must ensure this else we will copy from the wrong source.
  */
-static void bm_extrude_copy_face_loop_attributes(BMesh *bm, BMFace *f, BMEdge *e_a, BMEdge *e_b)
+static void bm_extrude_copy_face_loop_attributes(BMesh *bm, BMFace *f)
 {
-	/* 'a' is the starting edge #e, 'b' is the final edge #newedge */
-	BMLoop *l_dst_a = BM_face_edge_share_loop(f, e_a);
-	BMLoop *l_dst_b = BM_face_edge_share_loop(f, e_b);
-	/* we could only have a face on one-or the other edges,
-	 * check if either side of the face has an adjacent face */
-	BMLoop *l_src_1;
-	BMLoop *l_src_2;
-
-	/* there is no l_src_b */
-
-	/* sanity */
-	BLI_assert(l_dst_a->f == l_dst_b->f);
-
-	if (l_dst_a != l_dst_a->radial_next) {
-		l_src_1 = l_dst_a->radial_next;
-		l_src_2 = l_src_1->next;
-	}
-	else if (l_dst_b != l_dst_b->radial_next) {
-		l_src_2 = l_dst_b->radial_next;
-		l_src_1 = l_src_2->next;
-	}
-	else {
-		/* no new faces on either edge, nothing to copy from */
+	/* edge we are extruded from */
+	BMLoop *l_first_0 = BM_FACE_FIRST_LOOP(f);
+	BMLoop *l_first_1 = l_first_0->next;
+	BMLoop *l_first_2 = l_first_1->next;
+	BMLoop *l_first_3 = l_first_2->next;
+
+	BMLoop *l_other_0;
+	BMLoop *l_other_1;
+
+	if (UNLIKELY(l_first_0 == l_first_0->radial_next)) {
 		return;
 	}
 
-	BM_elem_attrs_copy(bm, bm, l_src_1->f, l_dst_a->f);
-	BM_elem_flag_disable(f, BM_ELEM_HIDDEN); /* possibly we copy from a hidden face */
+	l_other_0 = BM_edge_other_loop(l_first_0->e, l_first_0);
+	l_other_1 = BM_edge_other_loop(l_first_0->e, l_first_1);
 
 	/* copy data */
-	BM_elem_attrs_copy(bm, bm, l_src_2, l_dst_a);
-	BM_elem_attrs_copy(bm, bm, l_src_2, l_dst_b->next);
+	BM_elem_attrs_copy(bm, bm, l_other_0->f, f);
+	BM_elem_flag_disable(f, BM_ELEM_HIDDEN);  /* possibly we copy from a hidden face */
+
+	BM_elem_attrs_copy(bm, bm, l_other_0, l_first_0);
+	BM_elem_attrs_copy(bm, bm, l_other_0, l_first_3);
 
-	BM_elem_attrs_copy(bm, bm, l_src_1, l_dst_a->next);
-	BM_elem_attrs_copy(bm, bm, l_src_1, l_dst_b);
+	BM_elem_attrs_copy(bm, bm, l_other_1, l_first_1);
+	BM_elem_attrs_copy(bm, bm, l_other_1, l_first_2);
 }
 
 /* Disable the skin root flag on the input vert, assumes that the vert
@@ -189,7 +179,7 @@ void bmo_extrude_edge_only_exec(BMesh *bm, BMOperator *op)
 	BMOIter siter;
 	BMOperator dupeop;
 	BMFace *f;
-	BMEdge *e, *e2;
+	BMEdge *e, *e_new;
 	
 	BMO_ITER (e, &siter, bm, op, "edges", BM_EDGE) {
 		BMO_elem_flag_enable(bm, e, EXT_INPUT);
@@ -210,27 +200,26 @@ void bmo_extrude_edge_only_exec(BMesh *bm, BMOperator *op)
 
 	for (e = BMO_iter_new(&siter, bm, &dupeop, "boundarymap", 0); e; e = BMO_iter_step(&siter)) {
 		BMVert *f_verts[4];
-		e2 = BMO_iter_map_value(&siter);
-		e2 = *(BMEdge **)e2;
+		e_new = *(BMEdge **)BMO_iter_map_value(&siter);
 
 		if (e->l && e->v1 != e->l->v) {
 			f_verts[0] = e->v1;
 			f_verts[1] = e->v2;
-			f_verts[2] = e2->v2;
-			f_verts[3] = e2->v1;
+			f_verts[2] = e_new->v2;
+			f_verts[3] = e_new->v1;
 		}
 		else {
-			f_verts[3] = e->v1;
-			f_verts[2] = e->v2;
-			f_verts[1] = e2->v2;
-			f_verts[0] = e2->v1;
+			f_verts[0] = e->v2;
+			f_verts[1] = e->v1;
+			f_verts[2] = e_new->v1;
+			f_verts[3] = e_new->v2;
 		}
 		/* not sure what to do about example face, pass NULL for now */
 		f = BM_face_create_quad_tri_v(bm, f_verts, 4, NULL, FALSE);
-		bm_extrude_copy_face_loop_attributes(bm, f, e, e2);
+		bm_extrude_copy_face_loop_attributes(bm, f);
 		
 		if (BMO_elem_flag_test(bm, e, EXT_INPUT))
-			e = e2;
+			e = e_new;
 		
 		BMO_elem_flag_enable(bm, f, EXT_KEEP);
 		BMO_elem_flag_enable(bm, e, EXT_KEEP);
@@ -271,7 +260,7 @@ void bmo_extrude_face_region_exec(BMesh *bm, BMOperator *op)
 	BMOperator dupeop, delop;
 	BMOIter siter;
 	BMIter iter, fiter, viter;
-	BMEdge *e, *newedge;
+	BMEdge *e, *e_new;
 	BMVert *v, *v2;
 	BMFace *f;
 	int found, fwd, delorig = FALSE;
@@ -398,37 +387,37 @@ void bmo_extrude_face_region_exec(BMesh *bm, BMOperator *op)
 			continue;
 		}
 
-		newedge = *(BMEdge **)BMO_iter_map_value(&siter);
+		e_new = *(BMEdge **)BMO_iter_map_value(&siter);
 
-		if (!newedge) {
+		if (!e_new) {
 			continue;
 		}
 
 		/* orient loop to give same normal as a loop of newedge
 		 * if it exists (will be an extruded face),
 		 * else same normal as a loop of e, if it exists */
-		if (!newedge->l)
+		if (!e_new->l)
 			fwd = !e->l || !(e->l->v == e->v1);
 		else
-			fwd = (newedge->l->v == newedge->v1);
+			fwd = (e_new->l->v == e_new->v1);
 
 		
 		if (fwd) {
 			f_verts[0] = e->v1;
 			f_verts[1] = e->v2;
-			f_verts[2] = newedge->v2;
-			f_verts[3] = newedge->v1;
+			f_verts[2] = e_new->v2;
+			f_verts[3] = e_new->v1;
 		}
 		else {
-			f_verts[3] = e->v1;
-			f_verts[2] = e->v2;
-			f_verts[1] = newedge->v2;
-			f_verts[0] = newedge->v1;
+			f_verts[0] = e->v2;
+			f_verts[1] = e->v1;
+			f_verts[2] = e_new->v1;
+			f_verts[3] = e_new->v2;
 		}
 
 		/* not sure what to do about example face, pass NULL for now */
 		f = BM_face_create_quad_tri_v(bm, f_verts, 4, NULL, FALSE);
-		bm_extrude_copy_face_loop_attributes(bm, f, e, newedge);
+		bm_extrude_copy_face_loop_attributes(bm, f);
 	}
 
 	/* link isolated vert */