tmaq
/
tornavis
1
0
Fork 0
Code Issues 2 Pull Requests 29 Packages Projects 24 Releases Wiki Activity

Fix: Incorrect BMesh to Mesh attribute copying 

The existing logic to copy `BMesh` custom data layers to `Mesh`
attribute arrays was quite complicated, and incorrect in some cases
when the source and destinations didn't have the same layers.
The functions leave a lot to be desired in general, since they have
a lot of redundant complexity that ends up doing the same thing for
every element.

The problem in #104154 was that the "rest_position" attribute overwrote
the mesh positions since it has the same type and the positions weren't
copied. This same problem has shown up in boolean attribute conversion
in the past. Other changes fixed some specific cases but I think a
larger change is the only proper solution.

This patch adds preprocessing before looping over all elements to
find the basic information for copying the relevant layers, taking
layer names into account. The preprocessing makes the hot loops
simpler.

In a simple file with a 1 million vertex grid, I observed a 6%
improvement animation playback framerate in edit mode with a simple
geometry nodes modifier, from 5 to 5.3 FPS.

Fixes #104154, #104348

Pull Request #104421
This commit is contained in:
Hans Goudey 2023-02-13 20:52:02 +01:00
parent 0dfc102531
commit dfacaf4f40
3 changed files with 162 additions and 155 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -137,6 +137,7 @@ bool CustomData_has_referenced(const struct CustomData *data);
* implemented for mloopuv/mloopcol, for now.
*/
void CustomData_data_copy_value(int type, const void *source, void *dest);
void CustomData_data_set_default_value(int type, void *elem);
/**
* Mixes the "value" (e.g. mloopuv uv or mloopcol colors) from one block into
@ -506,6 +507,8 @@ void CustomData_clear_layer_flag(struct CustomData *data, int type, int flag);
void CustomData_bmesh_set_default(struct CustomData *data, void **block);
void CustomData_bmesh_free_block(struct CustomData *data, void **block);
void CustomData_bmesh_alloc_block(struct CustomData *data, void **block);
/**
* Same as #CustomData_bmesh_free_block but zero the memory rather than freeing.
*/
@ -517,23 +520,6 @@ void CustomData_bmesh_free_block_data_exclude_by_type(struct CustomData *data,
void *block,
eCustomDataMask mask_exclude);
/**
* Copy custom data to/from layers as in mesh/derived-mesh, to edit-mesh
* blocks of data. the CustomData's must not be compatible.
*
* \param use_default_init: initializes data which can't be copied,
* typically you'll want to use this if the BM_xxx create function
* is called with BM_CREATE_SKIP_CD flag
*/
void CustomData_to_bmesh_block(const struct CustomData *source,
struct CustomData *dest,
int src_index,
void **dest_block,
bool use_default_init);
void CustomData_from_bmesh_block(const struct CustomData *source,
struct CustomData *dest,
void *src_block,
int dest_index);
/**
* Query info over types.

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

@ -3654,7 +3654,7 @@ void CustomData_bmesh_free_block_data(CustomData *data, void *block)
}
}
static void CustomData_bmesh_alloc_block(CustomData *data, void **block)
void CustomData_bmesh_alloc_block(CustomData *data, void **block)
{
if (*block) {
CustomData_bmesh_free_block(data, block);
@ -3689,19 +3689,23 @@ void CustomData_bmesh_free_block_data_exclude_by_type(CustomData *data,
}
}
static void CustomData_bmesh_set_default_n(CustomData *data, void **block, const int n)
void CustomData_data_set_default_value(const int type, void *elem)
{
int offset = data->layers[n].offset;
const LayerTypeInfo *typeInfo = layerType_getInfo(data->layers[n].type);
const LayerTypeInfo *typeInfo = layerType_getInfo(type);
if (typeInfo->set_default_value) {
typeInfo->set_default_value(POINTER_OFFSET(*block, offset), 1);
typeInfo->set_default_value(elem, 1);
}
else {
memset(POINTER_OFFSET(*block, offset), 0, typeInfo->size);
memset(elem, 0, typeInfo->size);
}
}
static void CustomData_bmesh_set_default_n(CustomData *data, void **block, const int n)
{
const int offset = data->layers[n].offset;
CustomData_data_set_default_value(data->layers[n].type, POINTER_OFFSET(*block, offset));
}
void CustomData_bmesh_set_default(CustomData *data, void **block)
{
if (*block == nullptr) {
@ -3891,8 +3895,8 @@ void CustomData_data_copy_value(int type, const void *source, void *dest)
return;
}
if (typeInfo->copyvalue) {
typeInfo->copyvalue(source, dest, CDT_MIX_NOMIX, 0.0f);
if (typeInfo->copy) {
typeInfo->copy(source, dest, 1);
}
else {
memcpy(dest, source, typeInfo->size);
@ -4067,115 +4071,6 @@ void CustomData_bmesh_interp(CustomData *data,
}
}
void CustomData_to_bmesh_block(const CustomData *source,
CustomData *dest,
int src_index,
void **dest_block,
bool use_default_init)
{
if (*dest_block == nullptr) {
CustomData_bmesh_alloc_block(dest, dest_block);
}
/* copies a layer at a time */
int dest_i = 0;
for (int src_i = 0; src_i < source->totlayer; src_i++) {
/* find the first dest layer with type >= the source type
* (this should work because layers are ordered by type)
*/
while (dest_i < dest->totlayer && dest->layers[dest_i].type < source->layers[src_i].type) {
if (use_default_init) {
CustomData_bmesh_set_default_n(dest, dest_block, dest_i);
}
dest_i++;
}
/* if there are no more dest layers, we're done */
if (dest_i >= dest->totlayer) {
break;
}
/* if we found a matching layer, copy the data */
if (dest->layers[dest_i].type == source->layers[src_i].type) {
int offset = dest->layers[dest_i].offset;
const void *src_data = source->layers[src_i].data;
void *dest_data = POINTER_OFFSET(*dest_block, offset);
const LayerTypeInfo *typeInfo = layerType_getInfo(dest->layers[dest_i].type);
const size_t src_offset = size_t(src_index) * typeInfo->size;
if (typeInfo->copy) {
typeInfo->copy(POINTER_OFFSET(src_data, src_offset), dest_data, 1);
}
else {
memcpy(dest_data, POINTER_OFFSET(src_data, src_offset), typeInfo->size);
}
/* if there are multiple source & dest layers of the same type,
* we don't want to copy all source layers to the same dest, so
* increment dest_i
*/
dest_i++;
}
}
if (use_default_init) {
while (dest_i < dest->totlayer) {
CustomData_bmesh_set_default_n(dest, dest_block, dest_i);
dest_i++;
}
}
}
void CustomData_from_bmesh_block(const CustomData *source,
CustomData *dest,
void *src_block,
int dest_index)
{
/* copies a layer at a time */
int dest_i = 0;
for (int src_i = 0; src_i < source->totlayer; src_i++) {
if (source->layers[src_i].flag & CD_FLAG_NOCOPY) {
continue;
}
/* find the first dest layer with type >= the source type
* (this should work because layers are ordered by type)
*/
while (dest_i < dest->totlayer && dest->layers[dest_i].type < source->layers[src_i].type) {
dest_i++;
}
/* if there are no more dest layers, we're done */
if (dest_i >= dest->totlayer) {
return;
}
/* if we found a matching layer, copy the data */
if (dest->layers[dest_i].type == source->layers[src_i].type) {
const LayerTypeInfo *typeInfo = layerType_getInfo(dest->layers[dest_i].type);
int offset = source->layers[src_i].offset;
const void *src_data = POINTER_OFFSET(src_block, offset);
void *dst_data = POINTER_OFFSET(dest->layers[dest_i].data,
size_t(dest_index) * typeInfo->size);
if (typeInfo->copy) {
typeInfo->copy(src_data, dst_data, 1);
}
else {
memcpy(dst_data, src_data, typeInfo->size);
}
/* if there are multiple source & dest layers of the same type,
* we don't want to copy all source layers to the same dest, so
* increment dest_i
*/
dest_i++;
}
}
}
void CustomData_file_write_info(int type, const char **r_struct_name, int *r_struct_num)
{
const LayerTypeInfo *typeInfo = layerType_getInfo(type);

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

@ -85,6 +85,7 @@
#include "BLI_span.hh"
#include "BLI_string_ref.hh"
#include "BLI_task.hh"
#include "BLI_vector.hh"
#include "BKE_attribute.hh"
#include "BKE_customdata.h"
@ -110,6 +111,7 @@ using blender::IndexRange;
using blender::MutableSpan;
using blender::Span;
using blender::StringRef;
using blender::Vector;
static char bm_edge_flag_from_mflag(const short mflag)
{
@ -164,6 +166,63 @@ static BMFace *bm_face_create_from_mpoly(BMesh &bm,
return BM_face_create(&bm, verts.data(), edges.data(), loops.size(), nullptr, BM_CREATE_SKIP_CD);
}
struct MeshToBMeshLayerInfo {
eCustomDataType type;
/** The layer's position in the BMesh element's data block. */
int bmesh_offset;
/** The mesh's #CustomDataLayer::data. When null, the BMesh block is set to its default value. */
const void *mesh_data;
/** The size of every custom data element. */
size_t elem_size;
};
/**
* Calculate the necessary information to copy every data layer from the Mesh to the BMesh.
*/
static Vector<MeshToBMeshLayerInfo> mesh_to_bm_copy_info_calc(const CustomData &mesh_data,
CustomData &bm_data)
{
Vector<MeshToBMeshLayerInfo> infos;
std::array<int, CD_NUMTYPES> per_type_index;
per_type_index.fill(0);
for (const int i : IndexRange(bm_data.totlayer)) {
CustomDataLayer &bm_layer = bm_data.layers[i];
const eCustomDataType type = eCustomDataType(bm_layer.type);
const int mesh_layer_index =
bm_layer.name[0] == '\0' ?
CustomData_get_layer_index_n(&mesh_data, type, per_type_index[type]) :
CustomData_get_named_layer_index(&mesh_data, type, bm_layer.name);
MeshToBMeshLayerInfo info{};
info.type = type;
info.bmesh_offset = bm_layer.offset;
info.mesh_data = (mesh_layer_index == -1) ? nullptr : mesh_data.layers[mesh_layer_index].data;
info.elem_size = CustomData_get_elem_size(&bm_layer);
infos.append(info);
per_type_index[type]++;
}
return infos;
}
static void mesh_attributes_copy_to_bmesh_block(CustomData &data,
const Span<MeshToBMeshLayerInfo> copy_info,
const int mesh_index,
BMHeader &header)
{
CustomData_bmesh_alloc_block(&data, &header.data);
for (const MeshToBMeshLayerInfo &info : copy_info) {
if (info.mesh_data) {
CustomData_data_copy_value(info.type,
POINTER_OFFSET(info.mesh_data, info.elem_size * mesh_index),
POINTER_OFFSET(header.data, info.bmesh_offset));
}
else {
CustomData_data_set_default_value(info.type, POINTER_OFFSET(header.data, info.bmesh_offset));
}
}
}
void BM_mesh_bm_from_me(BMesh *bm, const Mesh *me, const struct BMeshFromMeshParams *params)
{
if (!me) {
@ -259,6 +318,11 @@ void BM_mesh_bm_from_me(BMesh *bm, const Mesh *me, const struct BMeshFromMeshPar
CustomData_bmesh_merge(&mesh_ldata, &bm->ldata, mask.lmask, CD_SET_DEFAULT, bm, BM_LOOP);
}
const Vector<MeshToBMeshLayerInfo> vert_info = mesh_to_bm_copy_info_calc(mesh_vdata, bm->vdata);
const Vector<MeshToBMeshLayerInfo> edge_info = mesh_to_bm_copy_info_calc(mesh_edata, bm->edata);
const Vector<MeshToBMeshLayerInfo> poly_info = mesh_to_bm_copy_info_calc(mesh_pdata, bm->pdata);
const Vector<MeshToBMeshLayerInfo> loop_info = mesh_to_bm_copy_info_calc(mesh_ldata, bm->ldata);
/* -------------------------------------------------------------------- */
/* Shape Key */
int tot_shape_keys = 0;
@ -393,8 +457,7 @@ void BM_mesh_bm_from_me(BMesh *bm, const Mesh *me, const struct BMeshFromMeshPar
copy_v3_v3(v->no, vert_normals[i]);
}
/* Copy Custom Data */
CustomData_to_bmesh_block(&mesh_vdata, &bm->vdata, i, &v->head.data, true);
mesh_attributes_copy_to_bmesh_block(bm->vdata, vert_info, i, v->head);
/* Set shape key original index. */
if (cd_shape_keyindex_offset != -1) {
@ -432,8 +495,7 @@ void BM_mesh_bm_from_me(BMesh *bm, const Mesh *me, const struct BMeshFromMeshPar
BM_elem_flag_enable(e, BM_ELEM_SMOOTH);
}
/* Copy Custom Data */
CustomData_to_bmesh_block(&mesh_edata, &bm->edata, i, &e->head.data, true);
mesh_attributes_copy_to_bmesh_block(bm->edata, edge_info, i, e->head);
}
if (is_new) {
bm->elem_index_dirty &= ~BM_EDGE; /* Added in order, clear dirty flag. */
@ -491,12 +553,11 @@ void BM_mesh_bm_from_me(BMesh *bm, const Mesh *me, const struct BMeshFromMeshPar
/* Don't use 'j' since we may have skipped some faces, hence some loops. */
BM_elem_index_set(l_iter, totloops++); /* set_ok */
/* Save index of corresponding #MLoop. */
CustomData_to_bmesh_block(&mesh_ldata, &bm->ldata, j++, &l_iter->head.data, true);
mesh_attributes_copy_to_bmesh_block(bm->ldata, loop_info, j, l_iter->head);
j++;
} while ((l_iter = l_iter->next) != l_first);
/* Copy Custom Data */
CustomData_to_bmesh_block(&mesh_pdata, &bm->pdata, i, &f->head.data, true);
mesh_attributes_copy_to_bmesh_block(bm->pdata, poly_info, i, f->head);
if (params->calc_face_normal) {
BM_face_normal_update(f);
@ -998,6 +1059,66 @@ static void convert_bmesh_selection_flags_to_mesh_attributes(BMesh &bm,
}
}
struct BMeshToMeshLayerInfo {
eCustomDataType type;
/** The layer's position in the BMesh element's data block. */
int bmesh_offset;
/** The mesh's #CustomDataLayer::data. When null, the BMesh block is set to its default value. */
void *mesh_data;
/** The size of every custom data element. */
size_t elem_size;
};
/**
* Calculate the necessary information to copy every data layer from the BMesh to the Mesh.
*/
static Vector<BMeshToMeshLayerInfo> bm_to_mesh_copy_info_calc(const CustomData &bm_data,
CustomData &mesh_data)
{
Vector<BMeshToMeshLayerInfo> infos;
std::array<int, CD_NUMTYPES> per_type_index;
per_type_index.fill(0);
for (const int i : IndexRange(mesh_data.totlayer)) {
CustomDataLayer &mesh_layer = mesh_data.layers[i];
const eCustomDataType type = eCustomDataType(mesh_layer.type);
const int bm_layer_index =
mesh_layer.name[0] == '\0' ?
CustomData_get_layer_index_n(&bm_data, type, per_type_index[type]) :
CustomData_get_named_layer_index(&bm_data, type, mesh_layer.name);
/* Skip layers that don't exist in `bm_data` or are explicitly set to not be
* copied. The layers are either set separately or shouldn't exist on the mesh. */
if (bm_layer_index == -1) {
continue;
}
const CustomDataLayer &bm_layer = bm_data.layers[bm_layer_index];
if (bm_layer.flag & CD_FLAG_NOCOPY) {
continue;
}
BMeshToMeshLayerInfo info{};
info.type = type;
info.bmesh_offset = bm_layer.offset;
info.mesh_data = mesh_layer.data;
info.elem_size = CustomData_get_elem_size(&mesh_layer);
infos.append(info);
per_type_index[type]++;
}
return infos;
}
static void bmesh_block_copy_to_mesh_attributes(const Span<BMeshToMeshLayerInfo> copy_info,
const int mesh_index,
const void *block)
{
for (const BMeshToMeshLayerInfo &info : copy_info) {
CustomData_data_copy_value(info.type,
POINTER_OFFSET(block, info.bmesh_offset),
POINTER_OFFSET(info.mesh_data, info.elem_size * mesh_index));
}
}
void BM_mesh_bm_to_me(Main *bmain, BMesh *bm, Mesh *me, const struct BMeshToMeshParams *params)
{
using namespace blender;
@ -1110,6 +1231,11 @@ void BM_mesh_bm_to_me(Main *bmain, BMesh *bm, Mesh *me, const struct BMeshToMesh
CustomData_copy(&bm->pdata, &me->pdata, mask.pmask, CD_SET_DEFAULT, me->totpoly);
}
const Vector<BMeshToMeshLayerInfo> vert_info = bm_to_mesh_copy_info_calc(bm->vdata, me->vdata);
const Vector<BMeshToMeshLayerInfo> edge_info = bm_to_mesh_copy_info_calc(bm->edata, me->edata);
const Vector<BMeshToMeshLayerInfo> poly_info = bm_to_mesh_copy_info_calc(bm->pdata, me->pdata);
const Vector<BMeshToMeshLayerInfo> loop_info = bm_to_mesh_copy_info_calc(bm->ldata, me->ldata);
/* Clear the CD_FLAG_NOCOPY flags for the layers they were temporarily set on */
for (const int i : ldata_layers_marked_nocopy) {
bm->ldata.layers[i].flag &= ~CD_FLAG_NOCOPY;
@ -1147,8 +1273,7 @@ void BM_mesh_bm_to_me(Main *bmain, BMesh *bm, Mesh *me, const struct BMeshToMesh
BM_elem_index_set(v, i); /* set_inline */
/* Copy over custom-data. */
CustomData_from_bmesh_block(&bm->vdata, &me->vdata, v->head.data, i);
bmesh_block_copy_to_mesh_attributes(vert_info, i, v->head.data);
i++;
@ -1174,8 +1299,7 @@ void BM_mesh_bm_to_me(Main *bmain, BMesh *bm, Mesh *me, const struct BMeshToMesh
BM_elem_index_set(e, i); /* set_inline */
/* Copy over custom-data. */
CustomData_from_bmesh_block(&bm->edata, &me->edata, e->head.data, i);
bmesh_block_copy_to_mesh_attributes(edge_info, i, e->head.data);
i++;
BM_CHECK_ELEMENT(e);
@ -1204,8 +1328,7 @@ void BM_mesh_bm_to_me(Main *bmain, BMesh *bm, Mesh *me, const struct BMeshToMesh
mloop[j].e = BM_elem_index_get(l_iter->e);
mloop[j].v = BM_elem_index_get(l_iter->v);
/* Copy over custom-data. */
CustomData_from_bmesh_block(&bm->ldata, &me->ldata, l_iter->head.data, j);
bmesh_block_copy_to_mesh_attributes(loop_info, j, l_iter->head.data);
j++;
BM_CHECK_ELEMENT(l_iter);
@ -1217,8 +1340,7 @@ void BM_mesh_bm_to_me(Main *bmain, BMesh *bm, Mesh *me, const struct BMeshToMesh
me->act_face = i;
}
/* Copy over custom-data. */
CustomData_from_bmesh_block(&bm->pdata, &me->pdata, f->head.data, i);
bmesh_block_copy_to_mesh_attributes(poly_info, i, f->head.data);
i++;
BM_CHECK_ELEMENT(f);
@ -1427,12 +1549,13 @@ static void bm_to_mesh_verts(const BMesh &bm,
MutableSpan<bool> select_vert,
MutableSpan<bool> hide_vert)
{
const Vector<BMeshToMeshLayerInfo> info = bm_to_mesh_copy_info_calc(bm.vdata, mesh.vdata);
MutableSpan<float3> dst_vert_positions = mesh.vert_positions_for_write();
threading::parallel_for(dst_vert_positions.index_range(), 1024, [&](const IndexRange range) {
for (const int vert_i : range) {
const BMVert &src_vert = *bm_verts[vert_i];
copy_v3_v3(dst_vert_positions[vert_i], src_vert.co);
CustomData_from_bmesh_block(&bm.vdata, &mesh.vdata, src_vert.head.data, vert_i);
bmesh_block_copy_to_mesh_attributes(info, vert_i, src_vert.head.data);
}
if (!select_vert.is_empty()) {
for (const int vert_i : range) {
@ -1454,6 +1577,7 @@ static void bm_to_mesh_edges(const BMesh &bm,
MutableSpan<bool> hide_edge,
MutableSpan<bool> sharp_edge)
{
const Vector<BMeshToMeshLayerInfo> info = bm_to_mesh_copy_info_calc(bm.edata, mesh.edata);
MutableSpan<MEdge> dst_edges = mesh.edges_for_write();
threading::parallel_for(dst_edges.index_range(), 512, [&](const IndexRange range) {
for (const int edge_i : range) {
@ -1462,7 +1586,7 @@ static void bm_to_mesh_edges(const BMesh &bm,
dst_edge.v1 = BM_elem_index_get(src_edge.v1);
dst_edge.v2 = BM_elem_index_get(src_edge.v2);
dst_edge.flag = bm_edge_flag_to_mflag(&src_edge);
CustomData_from_bmesh_block(&bm.edata, &mesh.edata, src_edge.head.data, edge_i);
bmesh_block_copy_to_mesh_attributes(info, edge_i, src_edge.head.data);
}
if (!select_edge.is_empty()) {
for (const int edge_i : range) {
@ -1489,6 +1613,7 @@ static void bm_to_mesh_faces(const BMesh &bm,
MutableSpan<bool> hide_poly,
MutableSpan<int> material_indices)
{
const Vector<BMeshToMeshLayerInfo> info = bm_to_mesh_copy_info_calc(bm.pdata, mesh.pdata);
MutableSpan<MPoly> dst_polys = mesh.polys_for_write();
threading::parallel_for(dst_polys.index_range(), 1024, [&](const IndexRange range) {
for (const int face_i : range) {
@ -1497,7 +1622,7 @@ static void bm_to_mesh_faces(const BMesh &bm,
dst_poly.totloop = src_face.len;
dst_poly.loopstart = BM_elem_index_get(BM_FACE_FIRST_LOOP(&src_face));
dst_poly.flag = bm_face_flag_to_mflag(&src_face);
CustomData_from_bmesh_block(&bm.pdata, &mesh.pdata, src_face.head.data, face_i);
bmesh_block_copy_to_mesh_attributes(info, face_i, src_face.head.data);
}
if (!select_poly.is_empty()) {
for (const int face_i : range) {
@ -1519,6 +1644,7 @@ static void bm_to_mesh_faces(const BMesh &bm,
static void bm_to_mesh_loops(const BMesh &bm, const Span<const BMLoop *> bm_loops, Mesh &mesh)
{
const Vector<BMeshToMeshLayerInfo> info = bm_to_mesh_copy_info_calc(bm.ldata, mesh.ldata);
MutableSpan<MLoop> dst_loops = mesh.loops_for_write();
threading::parallel_for(dst_loops.index_range(), 1024, [&](const IndexRange range) {
for (const int loop_i : range) {
@ -1526,7 +1652,7 @@ static void bm_to_mesh_loops(const BMesh &bm, const Span<const BMLoop *> bm_loop
MLoop &dst_loop = dst_loops[loop_i];
dst_loop.v = BM_elem_index_get(src_loop.v);
dst_loop.e = BM_elem_index_get(src_loop.e);
CustomData_from_bmesh_block(&bm.ldata, &mesh.ldata, src_loop.head.data, loop_i);
bmesh_block_copy_to_mesh_attributes(info, loop_i, src_loop.head.data);
}
});
}