Mesh: Avoid creating incorrect original index layers
Currently, whenever any BMesh is converted to a Mesh (except for edit mode switching), original index (`CD_ORIGINDEX`) layers are added. This is incorrect, because many operations just convert some Mesh into a BMesh and then back, but they shouldn't make any assumption about where their input mesh came from. It might even come from a primitive in geometry nodes, where there are no original indices at all. Conceptually, mesh original indices should be filled by the modifier stack when first creating the evaluated mesh. So that's where they're moved in this patch. A separate function now fills the indices with their default (0,1,2,3...) values. The way the mesh wrapper system defers the BMesh to Mesh conversion makes this a bit less obvious though. The old behavior is incorrect, but it's also slower, because three arrays the size of the mesh's vertices, edges, and faces had to be allocated and filled during the BMesh to Mesh conversion, which just ends up putting more pressure on the cache. In the many cases where original indices aren't used, I measured an **8% speedup** for the conversion (from 76.5ms to 70.7ms). Generally there is an assumption that BMesh is "original" and Mesh is "evaluated". After this patch, that assumption isn't quite as strong, but it still exists for two reasons. First, original indices are added whenever converting a BMesh "wrapper" to a Mesh. Second, original indices are not added to the BMesh at the beginning of evaluation, which assumes that every BMesh in the viewport is original and doesn't need the mapping. Differential Revision: https://developer.blender.org/D14018
This commit is contained in:
parent
960783a21a
commit
edb15b8458
Notes:
blender-bot
2023-02-14 00:20:19 +01:00
Referenced by issue #96241, 3.1: Potential candidates for corrective releases Referenced by issue #94495, Geometry nodes: Split edges node leads to a crash in edit mode
|
@ -85,6 +85,13 @@ struct Mesh *BKE_mesh_from_bmesh_for_eval_nomain(struct BMesh *bm,
|
|||
const struct CustomData_MeshMasks *cd_mask_extra,
|
||||
const struct Mesh *me_settings);
|
||||
|
||||
/**
|
||||
* Add original index (#CD_ORIGINDEX) layers if they don't already exist. This is meant to be used
|
||||
* when creating an evaluated mesh from an original edit mode mesh, to allow mapping from the
|
||||
* evaluated vertices to the originals.
|
||||
*/
|
||||
void BKE_mesh_ensure_default_orig_index_customdata(struct Mesh *mesh);
|
||||
|
||||
/**
|
||||
* Find the index of the loop in 'poly' which references vertex,
|
||||
* returns -1 if not found
|
||||
|
|
|
@ -708,6 +708,7 @@ static Mesh *create_orco_mesh(Object *ob, Mesh *me, BMEditMesh *em, int layer)
|
|||
|
||||
if (em) {
|
||||
mesh = BKE_mesh_from_bmesh_for_eval_nomain(em->bm, nullptr, me);
|
||||
BKE_mesh_ensure_default_orig_index_customdata(mesh);
|
||||
}
|
||||
else {
|
||||
mesh = BKE_mesh_copy_for_eval(me, true);
|
||||
|
@ -1521,6 +1522,12 @@ static void editbmesh_calc_modifiers(struct Depsgraph *depsgraph,
|
|||
em_input, &final_datamask, nullptr, mesh_input);
|
||||
}
|
||||
|
||||
/* The mesh from edit mode should not have any original index layers already, since those
|
||||
* are added during evaluation when necessary and are redundant on an original mesh. */
|
||||
BLI_assert(CustomData_get_layer(&em_input->bm->pdata, CD_ORIGINDEX) == nullptr &&
|
||||
CustomData_get_layer(&em_input->bm->edata, CD_ORIGINDEX) == nullptr &&
|
||||
CustomData_get_layer(&em_input->bm->pdata, CD_ORIGINDEX) == nullptr);
|
||||
|
||||
/* Clear errors before evaluation. */
|
||||
BKE_modifiers_clear_errors(ob);
|
||||
|
||||
|
@ -1559,6 +1566,7 @@ static void editbmesh_calc_modifiers(struct Depsgraph *depsgraph,
|
|||
else if (isPrevDeform && mti->dependsOnNormals && mti->dependsOnNormals(md)) {
|
||||
if (mesh_final == nullptr) {
|
||||
mesh_final = BKE_mesh_from_bmesh_for_eval_nomain(em_input->bm, nullptr, mesh_input);
|
||||
BKE_mesh_ensure_default_orig_index_customdata(mesh_final);
|
||||
ASSERT_IS_VALID_MESH(mesh_final);
|
||||
}
|
||||
BLI_assert(deformed_verts != nullptr);
|
||||
|
|
|
@ -1199,6 +1199,23 @@ Mesh *BKE_mesh_from_bmesh_for_eval_nomain(BMesh *bm,
|
|||
return mesh;
|
||||
}
|
||||
|
||||
static void ensure_orig_index_layer(CustomData &data, const int size)
|
||||
{
|
||||
if (CustomData_has_layer(&data, CD_ORIGINDEX)) {
|
||||
return;
|
||||
}
|
||||
int *indices = (int *)CustomData_add_layer(&data, CD_ORIGINDEX, CD_DEFAULT, nullptr, size);
|
||||
range_vn_i(indices, size, 0);
|
||||
}
|
||||
|
||||
void BKE_mesh_ensure_default_orig_index_customdata(Mesh *mesh)
|
||||
{
|
||||
BLI_assert(mesh->runtime.wrapper_type == ME_WRAPPER_TYPE_MDATA);
|
||||
ensure_orig_index_layer(mesh->vdata, mesh->totvert);
|
||||
ensure_orig_index_layer(mesh->edata, mesh->totedge);
|
||||
ensure_orig_index_layer(mesh->pdata, mesh->totpoly);
|
||||
}
|
||||
|
||||
BoundBox *BKE_mesh_boundbox_get(Object *ob)
|
||||
{
|
||||
/* This is Object-level data access,
|
||||
|
|
|
@ -129,6 +129,16 @@ static void mesh_wrapper_ensure_mdata_isolated(void *userdata)
|
|||
BMEditMesh *em = me->edit_mesh;
|
||||
BM_mesh_bm_to_me_for_eval(em->bm, me, &me->runtime.cd_mask_extra);
|
||||
|
||||
/* Adding original index layers assumes that all BMesh mesh wrappers are created from
|
||||
* original edit mode meshes (the only case where adding original indices makes sense).
|
||||
* If that assumption is broken, the layers might be incorrect in that they might not
|
||||
* actually be "original".
|
||||
*
|
||||
* There is also a performance aspect, where this also assumes that original indices are
|
||||
* always needed when converting an edit mesh to a mesh. That might be wrong, but it's not
|
||||
* harmful. */
|
||||
BKE_mesh_ensure_default_orig_index_customdata(me);
|
||||
|
||||
EditMeshData *edit_data = me->runtime.edit_data;
|
||||
if (edit_data->vertexCos) {
|
||||
BKE_mesh_vert_coords_apply(me, edit_data->vertexCos);
|
||||
|
|
|
@ -1194,10 +1194,6 @@ void BM_mesh_bm_to_me_for_eval(BMesh *bm, Mesh *me, const CustomData_MeshMasks *
|
|||
me->totloop = bm->totloop;
|
||||
me->totpoly = bm->totface;
|
||||
|
||||
CustomData_add_layer(&me->vdata, CD_ORIGINDEX, CD_CALLOC, nullptr, bm->totvert);
|
||||
CustomData_add_layer(&me->edata, CD_ORIGINDEX, CD_CALLOC, nullptr, bm->totedge);
|
||||
CustomData_add_layer(&me->pdata, CD_ORIGINDEX, CD_CALLOC, nullptr, bm->totface);
|
||||
|
||||
CustomData_add_layer(&me->vdata, CD_MVERT, CD_CALLOC, nullptr, bm->totvert);
|
||||
CustomData_add_layer(&me->edata, CD_MEDGE, CD_CALLOC, nullptr, bm->totedge);
|
||||
CustomData_add_layer(&me->ldata, CD_MLOOP, CD_CALLOC, nullptr, bm->totloop);
|
||||
|
@ -1225,7 +1221,6 @@ void BM_mesh_bm_to_me_for_eval(BMesh *bm, Mesh *me, const CustomData_MeshMasks *
|
|||
MEdge *medge = me->medge;
|
||||
MLoop *mloop = me->mloop;
|
||||
MPoly *mpoly = me->mpoly;
|
||||
int *index, add_orig;
|
||||
unsigned int i, j;
|
||||
|
||||
const int cd_vert_bweight_offset = CustomData_get_offset(&bm->vdata, CD_BWEIGHT);
|
||||
|
@ -1238,11 +1233,6 @@ void BM_mesh_bm_to_me_for_eval(BMesh *bm, Mesh *me, const CustomData_MeshMasks *
|
|||
|
||||
me->runtime.deformed_only = true;
|
||||
|
||||
/* Don't add origindex layer if one already exists. */
|
||||
add_orig = !CustomData_has_layer(&bm->pdata, CD_ORIGINDEX);
|
||||
|
||||
index = (int *)CustomData_get_layer(&me->vdata, CD_ORIGINDEX);
|
||||
|
||||
BM_ITER_MESH_INDEX (eve, &iter, bm, BM_VERTS_OF_MESH, i) {
|
||||
MVert *mv = &mvert[i];
|
||||
|
||||
|
@ -1256,15 +1246,10 @@ void BM_mesh_bm_to_me_for_eval(BMesh *bm, Mesh *me, const CustomData_MeshMasks *
|
|||
mv->bweight = BM_ELEM_CD_GET_FLOAT_AS_UCHAR(eve, cd_vert_bweight_offset);
|
||||
}
|
||||
|
||||
if (add_orig) {
|
||||
*index++ = i;
|
||||
}
|
||||
|
||||
CustomData_from_bmesh_block(&bm->vdata, &me->vdata, eve->head.data, i);
|
||||
}
|
||||
bm->elem_index_dirty &= ~BM_VERT;
|
||||
|
||||
index = (int *)CustomData_get_layer(&me->edata, CD_ORIGINDEX);
|
||||
BM_ITER_MESH_INDEX (eed, &iter, bm, BM_EDGES_OF_MESH, i) {
|
||||
MEdge *med = &medge[i];
|
||||
|
||||
|
@ -1291,13 +1276,9 @@ void BM_mesh_bm_to_me_for_eval(BMesh *bm, Mesh *me, const CustomData_MeshMasks *
|
|||
}
|
||||
|
||||
CustomData_from_bmesh_block(&bm->edata, &me->edata, eed->head.data, i);
|
||||
if (add_orig) {
|
||||
*index++ = i;
|
||||
}
|
||||
}
|
||||
bm->elem_index_dirty &= ~BM_EDGE;
|
||||
|
||||
index = (int *)CustomData_get_layer(&me->pdata, CD_ORIGINDEX);
|
||||
j = 0;
|
||||
BM_ITER_MESH_INDEX (efa, &iter, bm, BM_FACES_OF_MESH, i) {
|
||||
BMLoop *l_iter;
|
||||
|
@ -1324,10 +1305,6 @@ void BM_mesh_bm_to_me_for_eval(BMesh *bm, Mesh *me, const CustomData_MeshMasks *
|
|||
} while ((l_iter = l_iter->next) != l_first);
|
||||
|
||||
CustomData_from_bmesh_block(&bm->pdata, &me->pdata, efa->head.data, i);
|
||||
|
||||
if (add_orig) {
|
||||
*index++ = i;
|
||||
}
|
||||
}
|
||||
bm->elem_index_dirty &= ~(BM_FACE | BM_LOOP);
|
||||
|
||||
|
|
Loading…
Reference in New Issue