[Bf-blender-cvs] [e3a420c4773] master: Fix T76199 Bevel materials "bleed" over faces.

Howard Trickey noreply at git.blender.org
Fri Jun 26 12:43:28 CEST 2020


Commit: e3a420c4773a1d1a79c21f15137066d4411d3574
Author: Howard Trickey
Date:   Fri Jun 26 06:42:20 2020 -0400
Branches: master
https://developer.blender.org/rBe3a420c4773a1d1a79c21f15137066d4411d3574

Fix T76199 Bevel materials "bleed" over faces.

When there is an odd number of segments, bevel has an ambiguous
choice as to which side face to use to copy face attributes from
and to use for UV (and other loops that have math function) interpolation.
We used to make choice arbitrarily, which led to visually inconsistent
results. Now there is tie-breaking code, face with lexicographic lowest
value in vector with these elements:
  (1) connected component (in math-layer space) id
  (2) selected (0) vs unselected (1)
  (3) material index
  (4,5,6): z,x,y components of face center, in that order.

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

M	source/blender/bmesh/tools/bmesh_bevel.c

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

diff --git a/source/blender/bmesh/tools/bmesh_bevel.c b/source/blender/bmesh/tools/bmesh_bevel.c
index 10f93a8f095..e971e465eca 100644
--- a/source/blender/bmesh/tools/bmesh_bevel.c
+++ b/source/blender/bmesh/tools/bmesh_bevel.c
@@ -174,6 +174,26 @@ typedef struct ProfileSpacing {
   float fullness;
 } ProfileSpacing;
 
+/**
+ * If the mesh has custom data Loop layers that 'have math' we use this
+ * data to help decide which face to use as representative when there
+ * is an ambiguous choice as to which face to use, which happens
+ * when there is an odd number of segments.
+ *
+ * The face_compent field of the following will only be set if there are an odd
+ * number of segments. The it uses BMFace indices to index into it, so will
+ * only be valid as long BMFaces are not added or deleted in the BMesh.
+ * "Connected Component" here means connected in UV space:
+ * i.e., one face is directly connected to another if they share an edge and
+ * all of Loop UV custom layers are contiguous across that edge.
+ */
+typedef struct MathLayerInfo {
+  /** A connected-component id for each BMFace in the mesh. */
+  int *face_component;
+  /** Does the mesh have any custom loop uv layers? */
+  bool has_math_layers;
+} MathLayerInfo;
+
 /**
  * An element in a cyclic boundary of a Vertex Mesh (VMesh), placed on each side of beveled edges
  * where each profile starts, or on each side of a miter.
@@ -297,6 +317,8 @@ typedef struct BevelParams {
   ProfileSpacing pro_spacing;
   /** Parameter values for evenly spaced profile points for the miter profiles. */
   ProfileSpacing pro_spacing_miter;
+  /** Information about 'math' loop layers, like UV layers. */
+  MathLayerInfo math_layer_info;
   /** Blender units to offset each side of a beveled edge. */
   float offset;
   /** How offset is measured; enum defined in bmesh_operators.h. */
@@ -733,12 +755,13 @@ static BMFace *bev_create_quad_ex(BMesh *bm,
                                   BMEdge *e2,
                                   BMEdge *e3,
                                   BMEdge *e4,
+                                  BMFace *frep,
                                   int mat_nr)
 {
   BMVert *varr[4] = {v1, v2, v3, v4};
   BMFace *farr[4] = {f1, f2, f3, f4};
   BMEdge *earr[4] = {e1, e2, e3, e4};
-  return bev_create_ngon(bm, varr, 4, farr, f1, earr, mat_nr, true);
+  return bev_create_ngon(bm, varr, 4, farr, frep, earr, mat_nr, true);
 }
 
 /* Is Loop layer layer_index contiguous across shared vertex of l1 and l2? */
@@ -752,7 +775,8 @@ static bool contig_ldata_across_loops(BMesh *bm, BMLoop *l1, BMLoop *l2, int lay
 }
 
 /* Are all loop layers with have math (e.g., UVs)
- * contiguous from face f1 to face f2 across edge e? */
+ * contiguous from face f1 to face f2 across edge e?
+ */
 static bool contig_ldata_across_edge(BMesh *bm, BMEdge *e, BMFace *f1, BMFace *f2)
 {
   BMLoop *lef1, *lef2;
@@ -764,41 +788,202 @@ static bool contig_ldata_across_edge(BMesh *bm, BMEdge *e, BMFace *f1, BMFace *f
     return true;
   }
 
-  v1 = e->v1;
-  v2 = e->v2;
   if (!BM_edge_loop_pair(e, &lef1, &lef2)) {
     return false;
   }
+  /* If faces are oriented consistently around e,
+   * should now have lef1 and lef2 being f1 and f2 in either order.
+   */
   if (lef1->f == f2) {
     SWAP(BMLoop *, lef1, lef2);
   }
-
-  if (lef1->v == v1) {
-    lv1f1 = lef1;
-    lv2f1 = BM_face_other_edge_loop(f1, e, v2);
+  if (lef1->f != f1 || lef2 != lef2) {
+    return false;
   }
-  else {
-    lv2f1 = lef1;
-    lv1f1 = BM_face_other_edge_loop(f1, e, v1);
+  v1 = lef1->v;
+  v2 = lef2->v;
+  BLI_assert((v1 == e->v1 && v2 == e->v2) || (v1 == e->v2 && v2 == e->v1));
+  lv1f1 = lef1;
+  lv2f1 = lef1->next;
+  lv1f2 = lef2->next;
+  lv2f2 = lef2;
+  BLI_assert(lv1f1->v == v1 && lv1f1->f == f1 && lv2f1->v == v2 && lv2f1->f == f1 &&
+             lv1f2->v == v1 && lv1f2->f == f2 && lv2f2->v == v2 && lv2f2->f == f2);
+  for (i = 0; i < bm->ldata.totlayer; i++) {
+    if (CustomData_layer_has_math(&bm->ldata, i)) {
+      if (!contig_ldata_across_loops(bm, lv1f1, lv1f2, i) ||
+          !contig_ldata_across_loops(bm, lv2f1, lv2f2, i)) {
+        return false;
+      }
+    }
   }
+  return true;
+}
+
+/*
+ * Set up the fields of bp->math_layer_info.
+ * We always set has_math_layers to the correct value.
+ * Only if there are UV layers and the number of segments is odd,
+ * we need to calculate connected face components in UV space.
+ */
+static void math_layer_info_init(BevelParams *bp, BMesh *bm)
+{
+  int i, f, stack_top, totface, current_component;
+  int bmf_index, bmf_other_index;
+  int *face_component;
+  BMFace *bmf, *bmf_other;
+  BMEdge *bme;
+  BMFace **stack;
+  BMIter eiter, fiter;
 
-  if (lef2->v == v1) {
-    lv1f2 = lef2;
-    lv2f2 = BM_face_other_edge_loop(f2, e, v2);
+  bp->math_layer_info.has_math_layers = false;
+  bp->math_layer_info.face_component = NULL;
+  for (i = 0; i < bm->ldata.totlayer; i++) {
+    if (CustomData_has_layer(&bm->ldata, CD_MLOOPUV)) {
+      bp->math_layer_info.has_math_layers = true;
+      break;
+    }
   }
-  else {
-    lv2f2 = lef2;
-    lv1f2 = BM_face_other_edge_loop(f2, e, v1);
+  if (!bp->math_layer_info.has_math_layers || (bp->seg % 2) == 0) {
+    return;
   }
 
-  for (i = 0; i < bm->ldata.totlayer; i++) {
-    if (CustomData_layer_has_math(&bm->ldata, i) &&
-        (!contig_ldata_across_loops(bm, lv1f1, lv1f2, i) ||
-         !contig_ldata_across_loops(bm, lv2f1, lv2f2, i))) {
-      return false;
+  BM_mesh_elem_index_ensure(bm, BM_FACE);
+  BM_mesh_elem_table_ensure(bm, BM_FACE);
+  totface = bm->totface;
+  face_component = BLI_memarena_alloc(bp->mem_arena, totface * sizeof(int));
+  bp->math_layer_info.face_component = face_component;
+
+  /* Set all component ids by DFS from faces with unassigned components. */
+  for (f = 0; f < totface; f++) {
+    face_component[f] = -1;
+  }
+  current_component = -1;
+
+  /* Use an array as a stack. Stack size can't exceed double total faces. */
+  stack = MEM_malloc_arrayN(2 * totface, sizeof(BMFace *), __func__);
+  for (f = 0; f < totface; f++) {
+    if (face_component[f] == -1) {
+      stack_top = 0;
+      current_component++;
+      BLI_assert(stack_top < 2 * totface);
+      stack[stack_top] = BM_face_at_index(bm, f);
+      while (stack_top >= 0) {
+        bmf = stack[stack_top];
+        stack_top--;
+        bmf_index = BM_elem_index_get(bmf);
+        if (face_component[bmf_index] != -1) {
+          continue;
+        }
+        face_component[bmf_index] = current_component;
+        /* Neighbors are faces that share an edge with bmf and
+         * are where contig_ldata_across_edge(...) is true for the
+         * shared edge and two faces.
+         */
+        BM_ITER_ELEM (bme, &eiter, bmf, BM_EDGES_OF_FACE) {
+          BM_ITER_ELEM (bmf_other, &fiter, bme, BM_FACES_OF_EDGE) {
+            if (bmf_other != bmf) {
+              bmf_other_index = BM_elem_index_get(bmf_other);
+              if (face_component[bmf_other_index] != -1) {
+                continue;
+              }
+              if (contig_ldata_across_edge(bm, bme, bmf, bmf_other)) {
+                stack_top++;
+                BLI_assert(stack_top < 2 * totface);
+                stack[stack_top] = bmf_other;
+              }
+            }
+          }
+        }
+      }
     }
   }
-  return true;
+  MEM_freeN(stack);
+}
+
+/* Use a tie-breaking rule to choose a representative face when
+ * there are number of choices, face[0], face[1], ..., face[nfaces].
+ * This is needed when there are an odd number of segments, and the center
+ * segmment (and its continuation into vmesh) can usually arbitrarily be
+ * the previous face or the next face.
+ * Or, for the center polygon of a corner, all of the faces around
+ * the vertex are possible choices.
+ * If we just choose randomly, the resulting UV maps or material
+ * assignment can look ugly/inconsistent.
+ * Allow for the case when args are null.
+ */
+static BMFace *choose_rep_face(BevelParams *bp, BMFace **face, int nfaces)
+{
+  int f, bmf_index, value_index, best_f, i;
+  BMFace *bmf;
+  float cent[3];
+#define VEC_VALUE_LEN 6
+  float(*value_vecs)[VEC_VALUE_LEN] = NULL;
+  bool *still_viable = NULL;
+  int num_viable = 0;
+
+  value_vecs = BLI_array_alloca(value_vecs, nfaces);
+  still_viable = BLI_array_alloca(still_viable, nfaces);
+  for (int f = 0; f < nfaces; f++) {
+    bmf = face[f];
+    if (bmf == NULL) {
+      still_viable[f] = false;
+      continue;
+    }
+    still_viable[f] = true;
+    num_viable++;
+    bmf_index = BM_elem_index_get(bmf);
+    value_index = 0;
+    /* First tie-breaker: lower math-layer connected component id. */
+    value_vecs[f][value_index++] = bp->math_layer_info.face_component ?
+                                       (float)bp->math_layer_info.face_component[bmf_index] :
+                                       0.0f;
+    /* Next tie-breaker: selected face beats unselected one. */
+    value_vecs[f][value_index++] = BM_elem_flag_test(bmf, BM_ELEM_SELECT) ? 0.0f : 1.0f;
+    /* Next tie-breaker: lower material index. */
+    value_vecs[f][value_index++] = bmf->mat_nr >= 0 ? (float)bmf->mat_nr : 0.0f;
+    /* Next three tie-breakers: z, x, y components of face center. */
+    BM_face_calc_center_bounds(bmf, cent);
+    value_vecs[f][value_index++] = cent[2];
+    value_vecs[f][value_index++] = cent[0];
+    value_vecs[f][value_index++] = cent[1];
+    BLI_assert(value_index == VEC_VALUE_LEN);
+  }
+
+  /* Look for a face that has a unique minimum value for in a value_index,
+   * trying each value_index in turn until find a unique minimum.
+   */
+  best_f = -1;
+  for (value_index = 0; num_viable > 1 && value_index < VEC_VALUE_LEN; value_index++) {
+    for (f = 0; f < nfaces; f++) {
+      if (!still_viable[f] || f == best_f) {
+        continue;
+      }
+      if (best_f == -1) {
+        best_f = f;
+        continue;
+      }
+      if (value_vecs[f][value_index] < value_vecs[best_f][value_index]) {
+        best_f = f;
+        /* Previous f's are now not viable any more. */
+        for (i = f - 1; i >= 0; i--) {
+          if (still_viable[i]) {
+            still_viable[i] = false;
+            num_viable--;
+          }
+        }


@@ Diff output truncated at 10240 characters. @@



More information about the Bf-blender-cvs mailing list