Fix: Attribute layers can be skipped in Mesh to BMesh conversion

In some situations, layers were filled with their default value
when converting from Mesh to BMesh (entering edit mode, for example).
This was caused by the recently added "copy mesh to bmesh" or "merge
mesh to bmesh" custom data functions creating a difference custom
data format than was used for the copying functions used later.

`CustomData_to_bmesh_block` is not robust enough to handle simple
differences in layout between the layout of the source and result
CustomData layers, because it relies on the order of the types and
the number of layers within each type.

As a fix, make the "mesh to bmesh" special case more explicit in
the conversion functions. This makes the difference in the API
smaller, which is a nice improvement anwyay.

Fixes T101796

4 files changed, 84 insertions, 86 deletions

diff --git a/source/blender/blenkernel/BKE_customdata.h b/source/blender/blenkernel/BKE_customdata.h
index 61f3a0e1d5e..f761e28cbb4 100644
--- a/source/blender/blenkernel/BKE_customdata.h
+++ b/source/blender/blenkernel/BKE_customdata.h
@@ -146,15 +146,6 @@ void CustomData_copy(const struct CustomData *source,
                      eCDAllocType alloctype,
                      int totelem);
 
-/**
- * Like #CustomData_copy but skips copying layers that are stored as flags on #BMesh.
- */
-void CustomData_copy_mesh_to_bmesh(const struct CustomData *source,
-                                   struct CustomData *dest,
-                                   eCustomDataMask mask,
-                                   eCDAllocType alloctype,
-                                   int totelem);
-
 /* BMESH_TODO, not really a public function but readfile.c needs it */
 void CustomData_update_typemap(struct CustomData *data);
 
@@ -169,15 +160,6 @@ bool CustomData_merge(const struct CustomData *source,
                       int totelem);
 
 /**
- * Like #CustomData_copy but skips copying layers that are stored as flags on #BMesh.
- */
-bool CustomData_merge_mesh_to_bmesh(const struct CustomData *source,
-                                    struct CustomData *dest,
-                                    eCustomDataMask mask,
-                                    eCDAllocType alloctype,
-                                    int totelem);
-
-/**
  * Reallocate custom data to a new element count. If the new size is larger, the new values use
  * the #CD_CONSTRUCT behavior, so trivial types must be initialized by the caller. After being
  * resized, the #CustomData does not contain any referenced layers.
@@ -197,6 +179,14 @@ bool CustomData_bmesh_merge(const struct CustomData *source,
                             char htype);
 
 /**
+ * Remove layers that aren't stored in BMesh or are stored as flags on BMesh.
+ * The `layers` array of the returned #CustomData must be freed, but may be null.
+ * Used during conversion of #Mesh data to #BMesh storage format.
+ */
+CustomData CustomData_shallow_copy_remove_non_bmesh_attributes(const CustomData *src,
+                                                               eCustomDataMask mask);
+
+/**
  * NULL's all members and resets the #CustomData.typemap.
  */
 void CustomData_reset(struct CustomData *data);
diff --git a/source/blender/blenkernel/intern/customdata.cc b/source/blender/blenkernel/intern/customdata.cc
index be99a8ee87b..6f2390fc28f 100644
--- a/source/blender/blenkernel/intern/customdata.cc
+++ b/source/blender/blenkernel/intern/customdata.cc
@@ -2382,16 +2382,21 @@ static bool attribute_stored_in_bmesh_flag(const StringRef name)
               "material_index");
 }
 
-static CustomData shallow_copy_remove_non_bmesh_attributes(const CustomData &src)
+CustomData CustomData_shallow_copy_remove_non_bmesh_attributes(const CustomData *src,
+                                                               const eCustomDataMask mask)
 {
   Vector<CustomDataLayer> dst_layers;
-  for (const CustomDataLayer &layer : Span<CustomDataLayer>{src.layers, src.totlayer}) {
-    if (!attribute_stored_in_bmesh_flag(layer.name)) {
-      dst_layers.append(layer);
+  for (const CustomDataLayer &layer : Span<CustomDataLayer>{src->layers, src->totlayer}) {
+    if (attribute_stored_in_bmesh_flag(layer.name)) {
+      continue;
+    }
+    if (!(mask & CD_TYPE_AS_MASK(layer.type))) {
+      continue;
     }
+    dst_layers.append(layer);
   }
 
-  CustomData dst = src;
+  CustomData dst = *src;
   dst.layers = static_cast<CustomDataLayer *>(
       MEM_calloc_arrayN(dst_layers.size(), sizeof(CustomDataLayer), __func__));
   dst.totlayer = dst_layers.size();
@@ -2402,18 +2407,6 @@ static CustomData shallow_copy_remove_non_bmesh_attributes(const CustomData &src
   return dst;
 }
 
-bool CustomData_merge_mesh_to_bmesh(const CustomData *source,
-                                    CustomData *dest,
-                                    const eCustomDataMask mask,
-                                    const eCDAllocType alloctype,
-                                    const int totelem)
-{
-  CustomData source_copy = shallow_copy_remove_non_bmesh_attributes(*source);
-  const bool result = CustomData_merge(&source_copy, dest, mask, alloctype, totelem);
-  MEM_SAFE_FREE(source_copy.layers);
-  return result;
-}
-
 void CustomData_realloc(CustomData *data, const int old_size, const int new_size)
 {
   BLI_assert(new_size >= 0);
@@ -2463,17 +2456,6 @@ void CustomData_copy(const CustomData *source,
   CustomData_merge(source, dest, mask, alloctype, totelem);
 }
 
-void CustomData_copy_mesh_to_bmesh(const CustomData *source,
-                                   CustomData *dest,
-                                   const eCustomDataMask mask,
-                                   const eCDAllocType alloctype,
-                                   const int totelem)
-{
-  CustomData source_copy = shallow_copy_remove_non_bmesh_attributes(*source);
-  CustomData_copy(&source_copy, dest, mask, alloctype, totelem);
-  MEM_SAFE_FREE(source_copy.layers);
-}
-
 static void customData_free_layer__internal(CustomDataLayer *layer, const int totelem)
 {
   const LayerTypeInfo *typeInfo;
@@ -3697,7 +3679,7 @@ bool CustomData_bmesh_merge(const CustomData *source,
     destold.layers = static_cast<CustomDataLayer *>(MEM_dupallocN(destold.layers));
   }
 
-  if (CustomData_merge_mesh_to_bmesh(source, dest, mask, alloctype, 0) == false) {
+  if (CustomData_merge(source, dest, mask, alloctype, 0) == false) {
     if (destold.layers) {
       MEM_freeN(destold.layers);
     }
diff --git a/source/blender/bmesh/intern/bmesh_construct.c b/source/blender/bmesh/intern/bmesh_construct.c
index 3ee9fa7aee4..22900ecef85 100644
--- a/source/blender/bmesh/intern/bmesh_construct.c
+++ b/source/blender/bmesh/intern/bmesh_construct.c
@@ -507,26 +507,32 @@ void BM_mesh_copy_init_customdata_from_mesh_array(BMesh *bm_dst,
 
   for (int i = 0; i < me_src_array_len; i++) {
     const Mesh *me_src = me_src_array[i];
+    CustomData mesh_vdata = CustomData_shallow_copy_remove_non_bmesh_attributes(
+        &me_src->vdata, CD_MASK_BMESH.vmask);
+    CustomData mesh_edata = CustomData_shallow_copy_remove_non_bmesh_attributes(
+        &me_src->edata, CD_MASK_BMESH.emask);
+    CustomData mesh_pdata = CustomData_shallow_copy_remove_non_bmesh_attributes(
+        &me_src->pdata, CD_MASK_BMESH.lmask);
+    CustomData mesh_ldata = CustomData_shallow_copy_remove_non_bmesh_attributes(
+        &me_src->ldata, CD_MASK_BMESH.pmask);
+
     if (i == 0) {
-      CustomData_copy_mesh_to_bmesh(
-          &me_src->vdata, &bm_dst->vdata, CD_MASK_BMESH.vmask, CD_SET_DEFAULT, 0);
-      CustomData_copy_mesh_to_bmesh(
-          &me_src->edata, &bm_dst->edata, CD_MASK_BMESH.emask, CD_SET_DEFAULT, 0);
-      CustomData_copy_mesh_to_bmesh(
-          &me_src->ldata, &bm_dst->ldata, CD_MASK_BMESH.lmask, CD_SET_DEFAULT, 0);
-      CustomData_copy_mesh_to_bmesh(
-          &me_src->pdata, &bm_dst->pdata, CD_MASK_BMESH.pmask, CD_SET_DEFAULT, 0);
+      CustomData_copy(&me_src->vdata, &bm_dst->vdata, CD_MASK_BMESH.vmask, CD_SET_DEFAULT, 0);
+      CustomData_copy(&me_src->edata, &bm_dst->edata, CD_MASK_BMESH.emask, CD_SET_DEFAULT, 0);
+      CustomData_copy(&me_src->ldata, &bm_dst->ldata, CD_MASK_BMESH.lmask, CD_SET_DEFAULT, 0);
+      CustomData_copy(&me_src->pdata, &bm_dst->pdata, CD_MASK_BMESH.pmask, CD_SET_DEFAULT, 0);
     }
     else {
-      CustomData_merge_mesh_to_bmesh(
-          &me_src->vdata, &bm_dst->vdata, CD_MASK_BMESH.vmask, CD_SET_DEFAULT, 0);
-      CustomData_merge_mesh_to_bmesh(
-          &me_src->edata, &bm_dst->edata, CD_MASK_BMESH.emask, CD_SET_DEFAULT, 0);
-      CustomData_merge_mesh_to_bmesh(
-          &me_src->ldata, &bm_dst->ldata, CD_MASK_BMESH.lmask, CD_SET_DEFAULT, 0);
-      CustomData_merge_mesh_to_bmesh(
-          &me_src->pdata, &bm_dst->pdata, CD_MASK_BMESH.pmask, CD_SET_DEFAULT, 0);
+      CustomData_merge(&me_src->vdata, &bm_dst->vdata, CD_MASK_BMESH.vmask, CD_SET_DEFAULT, 0);
+      CustomData_merge(&me_src->edata, &bm_dst->edata, CD_MASK_BMESH.emask, CD_SET_DEFAULT, 0);
+      CustomData_merge(&me_src->ldata, &bm_dst->ldata, CD_MASK_BMESH.lmask, CD_SET_DEFAULT, 0);
+      CustomData_merge(&me_src->pdata, &bm_dst->pdata, CD_MASK_BMESH.pmask, CD_SET_DEFAULT, 0);
     }
+
+    MEM_SAFE_FREE(mesh_vdata.layers);
+    MEM_SAFE_FREE(mesh_edata.layers);
+    MEM_SAFE_FREE(mesh_pdata.layers);
+    MEM_SAFE_FREE(mesh_ldata.layers);
   }
 
   CustomData_bmesh_init_pool(&bm_dst->vdata, allocsize->totvert, BM_VERT);
diff --git a/source/blender/bmesh/intern/bmesh_mesh_convert.cc b/source/blender/bmesh/intern/bmesh_mesh_convert.cc
index 82d707c7e54..d65cac08db8 100644
--- a/source/blender/bmesh/intern/bmesh_mesh_convert.cc
+++ b/source/blender/bmesh/intern/bmesh_mesh_convert.cc
@@ -129,6 +129,10 @@ static BMFace *bm_face_create_from_mpoly(BMesh &bm,
 
 void BM_mesh_bm_from_me(BMesh *bm, const Mesh *me, const struct BMeshFromMeshParams *params)
 {
+  if (!me) {
+    /* Sanity check. */
+    return;
+  }
   const bool is_new = !(bm->totvert || (bm->vdata.totlayer || bm->edata.totlayer ||
                                         bm->pdata.totlayer || bm->ldata.totlayer));
   KeyBlock *actkey;
@@ -136,19 +140,35 @@ void BM_mesh_bm_from_me(BMesh *bm, const Mesh *me, const struct BMeshFromMeshPar
   CustomData_MeshMasks mask = CD_MASK_BMESH;
   CustomData_MeshMasks_update(&mask, &params->cd_mask_extra);
 
-  if (!me || !me->totvert) {
-    if (me && is_new) { /* No verts? still copy custom-data layout. */
-      CustomData_copy_mesh_to_bmesh(&me->vdata, &bm->vdata, mask.vmask, CD_CONSTRUCT, 0);
-      CustomData_copy_mesh_to_bmesh(&me->edata, &bm->edata, mask.emask, CD_CONSTRUCT, 0);
-      CustomData_copy_mesh_to_bmesh(&me->ldata, &bm->ldata, mask.lmask, CD_CONSTRUCT, 0);
-      CustomData_copy_mesh_to_bmesh(&me->pdata, &bm->pdata, mask.pmask, CD_CONSTRUCT, 0);
+  CustomData mesh_vdata = CustomData_shallow_copy_remove_non_bmesh_attributes(&me->vdata,
+                                                                              mask.vmask);
+  CustomData mesh_edata = CustomData_shallow_copy_remove_non_bmesh_attributes(&me->edata,
+                                                                              mask.emask);
+  CustomData mesh_pdata = CustomData_shallow_copy_remove_non_bmesh_attributes(&me->pdata,
+                                                                              mask.pmask);
+  CustomData mesh_ldata = CustomData_shallow_copy_remove_non_bmesh_attributes(&me->ldata,
+                                                                              mask.lmask);
+  BLI_SCOPED_DEFER([&]() {
+    MEM_SAFE_FREE(mesh_vdata.layers);
+    MEM_SAFE_FREE(mesh_edata.layers);
+    MEM_SAFE_FREE(mesh_pdata.layers);
+    MEM_SAFE_FREE(mesh_ldata.layers);
+  });
+
+  if (me->totvert == 0) {
+    if (is_new) {
+      /* No verts? still copy custom-data layout. */
+      CustomData_copy(&mesh_vdata, &bm->vdata, mask.vmask, CD_CONSTRUCT, 0);
+      CustomData_copy(&mesh_edata, &bm->edata, mask.emask, CD_CONSTRUCT, 0);
+      CustomData_copy(&mesh_pdata, &bm->pdata, mask.pmask, CD_CONSTRUCT, 0);
+      CustomData_copy(&mesh_ldata, &bm->ldata, mask.lmask, CD_CONSTRUCT, 0);
 
       CustomData_bmesh_init_pool(&bm->vdata, me->totvert, BM_VERT);
       CustomData_bmesh_init_pool(&bm->edata, me->totedge, BM_EDGE);
       CustomData_bmesh_init_pool(&bm->ldata, me->totloop, BM_LOOP);
       CustomData_bmesh_init_pool(&bm->pdata, me->totpoly, BM_FACE);
     }
-    return; /* Sanity check. */
+    return;
   }
 
   const float(*vert_normals)[3] = nullptr;
@@ -157,16 +177,16 @@ void BM_mesh_bm_from_me(BMesh *bm, const Mesh *me, const struct BMeshFromMeshPar
   }
 
   if (is_new) {
-    CustomData_copy_mesh_to_bmesh(&me->vdata, &bm->vdata, mask.vmask, CD_SET_DEFAULT, 0);
-    CustomData_copy_mesh_to_bmesh(&me->edata, &bm->edata, mask.emask, CD_SET_DEFAULT, 0);
-    CustomData_copy_mesh_to_bmesh(&me->ldata, &bm->ldata, mask.lmask, CD_SET_DEFAULT, 0);
-    CustomData_copy_mesh_to_bmesh(&me->pdata, &bm->pdata, mask.pmask, CD_SET_DEFAULT, 0);
+    CustomData_copy(&mesh_vdata, &bm->vdata, mask.vmask, CD_SET_DEFAULT, 0);
+    CustomData_copy(&mesh_edata, &bm->edata, mask.emask, CD_SET_DEFAULT, 0);
+    CustomData_copy(&mesh_pdata, &bm->pdata, mask.pmask, CD_SET_DEFAULT, 0);
+    CustomData_copy(&mesh_ldata, &bm->ldata, mask.lmask, CD_SET_DEFAULT, 0);
   }
   else {
-    CustomData_bmesh_merge(&me->vdata, &bm->vdata, mask.vmask, CD_SET_DEFAULT, bm, BM_VERT);
-    CustomData_bmesh_merge(&me->edata, &bm->edata, mask.emask, CD_SET_DEFAULT, bm, BM_EDGE);
-    CustomData_bmesh_merge(&me->ldata, &bm->ldata, mask.lmask, CD_SET_DEFAULT, bm, BM_LOOP);
-    CustomData_bmesh_merge(&me->pdata, &bm->pdata, mask.pmask, CD_SET_DEFAULT, bm, BM_FACE);
+    CustomData_bmesh_merge(&mesh_vdata, &bm->vdata, mask.vmask, CD_SET_DEFAULT, bm, BM_VERT);
+    CustomData_bmesh_merge(&mesh_edata, &bm->edata, mask.emask, CD_SET_DEFAULT, bm, BM_EDGE);
+    CustomData_bmesh_merge(&mesh_pdata, &bm->pdata, mask.pmask, CD_SET_DEFAULT, bm, BM_FACE);
+    CustomData_bmesh_merge(&mesh_ldata, &bm->ldata, mask.lmask, CD_SET_DEFAULT, bm, BM_LOOP);
   }
 
   /* -------------------------------------------------------------------- */
@@ -302,7 +322,7 @@ void BM_mesh_bm_from_me(BMesh *bm, const Mesh *me, const struct BMeshFromMeshPar
     }
 
     /* Copy Custom Data */
-    CustomData_to_bmesh_block(&me->vdata, &bm->vdata, i, &v->head.data, true);
+    CustomData_to_bmesh_block(&mesh_vdata, &bm->vdata, i, &v->head.data, true);
 
     /* Set shape key original index. */
     if (cd_shape_keyindex_offset != -1) {
@@ -338,7 +358,7 @@ void BM_mesh_bm_from_me(BMesh *bm, const Mesh *me, const struct BMeshFromMeshPar
     }
 
     /* Copy Custom Data */
-    CustomData_to_bmesh_block(&me->edata, &bm->edata, i, &e->head.data, true);
+    CustomData_to_bmesh_block(&mesh_edata, &bm->edata, i, &e->head.data, true);
   }
   if (is_new) {
     bm->elem_index_dirty &= ~BM_EDGE; /* Added in order, clear dirty flag. */
@@ -397,11 +417,11 @@ void BM_mesh_bm_from_me(BMesh *bm, const Mesh *me, const struct BMeshFromMeshPar
       BM_elem_index_set(l_iter, totloops++); /* set_ok */
 
       /* Save index of corresponding #MLoop. */
-      CustomData_to_bmesh_block(&me->ldata, &bm->ldata, j++, &l_iter->head.data, true);
+      CustomData_to_bmesh_block(&mesh_ldata, &bm->ldata, j++, &l_iter->head.data, true);
     } while ((l_iter = l_iter->next) != l_first);
 
     /* Copy Custom Data */
-    CustomData_to_bmesh_block(&me->pdata, &bm->pdata, i, &f->head.data, true);
+    CustomData_to_bmesh_block(&mesh_pdata, &bm->pdata, i, &f->head.data, true);
 
     if (params->calc_face_normal) {
       BM_face_normal_update(f);
@@ -951,10 +971,10 @@ void BM_mesh_bm_to_me(Main *bmain, BMesh *bm, Mesh *me, const struct BMeshToMesh
   {
     CustomData_MeshMasks mask = CD_MASK_MESH;
     CustomData_MeshMasks_update(&mask, &params->cd_mask_extra);
-    CustomData_copy_mesh_to_bmesh(&bm->vdata, &me->vdata, mask.vmask, CD_SET_DEFAULT, me->totvert);
-    CustomData_copy_mesh_to_bmesh(&bm->edata, &me->edata, mask.emask, CD_SET_DEFAULT, me->totedge);
-    CustomData_copy_mesh_to_bmesh(&bm->ldata, &me->ldata, mask.lmask, CD_SET_DEFAULT, me->totloop);
-    CustomData_copy_mesh_to_bmesh(&bm->pdata, &me->pdata, mask.pmask, CD_SET_DEFAULT, me->totpoly);
+    CustomData_copy(&bm->vdata, &me->vdata, mask.vmask, CD_SET_DEFAULT, me->totvert);
+    CustomData_copy(&bm->edata, &me->edata, mask.emask, CD_SET_DEFAULT, me->totedge);
+    CustomData_copy(&bm->ldata, &me->ldata, mask.lmask, CD_SET_DEFAULT, me->totloop);
+    CustomData_copy(&bm->pdata, &me->pdata, mask.pmask, CD_SET_DEFAULT, me->totpoly);
   }
 
   CustomData_add_layer(&me->vdata, CD_MVERT, CD_SET_DEFAULT, nullptr, me->totvert);