blender
/
blender
129
398
Fork 572
Code Issues 6.2k Pull Requests 747 Projects 19 Wiki Activity

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
This commit is contained in:
Hans Goudey 2022-10-13 12:42:12 -05:00
parent b2627dff49
commit 6f190c669f
Notes: blender-bot 2023-11-17 20:37:48 +01:00 
Referenced by commit 2111fab88a, Fix: Missing part of previous BMesh bug fix
Referenced by issue #101796, Sculpting: Face Sets in files saved in 3.3 are deleted in 3.4 after switching to edit mode
Referenced by issue #115060, Discovered typo in bmesh code
4 changed files with 84 additions and 86 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

26
source/blender/blenkernel/BKE_customdata.h
View File

@ -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);
@ -168,15 +159,6 @@ bool CustomData_merge(const struct CustomData *source,
eCDAllocType alloctype,
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
@ -196,6 +178,14 @@ bool CustomData_bmesh_merge(const struct CustomData *source,
struct BMesh *bm,
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.
*/

40
source/blender/blenkernel/intern/customdata.cc
View File

@ -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);
}

38
source/blender/bmesh/intern/bmesh_construct.c
View File

@ -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);

66
source/blender/bmesh/intern/bmesh_mesh_convert.cc
View File

@ -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);