From 0fe0db63d7f8d65185e3c296fdf3973f199f216d Mon Sep 17 00:00:00 2001 From: Hans Goudey Date: Mon, 13 Mar 2023 09:07:52 -0400 Subject: [PATCH] Mesh: Optimize BMesh to Mesh conversion with UV maps The BMesh to Mesh conversion does some checks to the UV helper attributes like selection to avoid copying them to the mesh if they don't contain any meaningful data. However, it does this by looping over all faces for every UV map, not in parallel, so it takes up a large portion of the total time in the conversion. This commit moves that to the existing similar checks. On a 1 million face mesh with 3 UV maps, for me this improved the conversion runtime by 75%, from 174ms to 99ms. Before the serial loops took 88ms out of the total. Combining them with the existing loop over faces only increased its runtime from 29 to 40ms. --- .../bmesh/intern/bmesh_mesh_convert.cc | 167 +++++++++--------- 1 file changed, 88 insertions(+), 79 deletions(-) diff --git a/source/blender/bmesh/intern/bmesh_mesh_convert.cc b/source/blender/bmesh/intern/bmesh_mesh_convert.cc index 425b3be3903..edb4c66945b 100644 --- a/source/blender/bmesh/intern/bmesh_mesh_convert.cc +++ b/source/blender/bmesh/intern/bmesh_mesh_convert.cc @@ -1125,14 +1125,54 @@ static void bm_edge_table_build(BMesh &bm, need_uv_seams = (hflag & BM_ELEM_SEAM) != 0; } +/** + * UV map vertex and edge selection, and UV pinning are all stored in separate boolean layers. On + * #Mesh they are only meant to exist if they have a true value, but on #BMesh they currently + * always exist. To avoid creating unnecessary mesh attributes, mark the UV helper layers with no + * true values with the "no copy" flag. + */ static void bm_face_loop_table_build(BMesh &bm, MutableSpan face_table, MutableSpan loop_table, bool &need_select_poly, bool &need_hide_poly, bool &need_sharp_face, - bool &need_material_index) + bool &need_material_index, + Vector &ldata_layers_marked_nocopy) { + const CustomData &ldata = bm.ldata; + Vector vert_sel_layers; + Vector edge_sel_layers; + Vector pin_layers; + for (const int i : IndexRange(CustomData_number_of_layers(&ldata, CD_PROP_FLOAT2))) { + char const *layer_name = CustomData_get_layer_name(&ldata, CD_PROP_FLOAT2, i); + char sub_layer_name[MAX_CUSTOMDATA_LAYER_NAME]; + auto add_bool_layer = [&](Vector &layers, const char *name) { + const int layer_index = CustomData_get_named_layer_index(&ldata, CD_PROP_BOOL, name); + if (layer_index != -1) { + layers.append(layer_index); + } + }; + add_bool_layer(vert_sel_layers, BKE_uv_map_vert_select_name_get(layer_name, sub_layer_name)); + add_bool_layer(edge_sel_layers, BKE_uv_map_edge_select_name_get(layer_name, sub_layer_name)); + add_bool_layer(pin_layers, BKE_uv_map_pin_name_get(layer_name, sub_layer_name)); + } + Array vert_sel_offsets(vert_sel_layers.size()); + Array edge_sel_offsets(edge_sel_layers.size()); + Array pin_offsets(pin_layers.size()); + for (const int i : vert_sel_layers.index_range()) { + vert_sel_offsets[i] = ldata.layers[vert_sel_layers[i]].offset; + } + for (const int i : edge_sel_layers.index_range()) { + edge_sel_offsets[i] = ldata.layers[edge_sel_layers[i]].offset; + } + for (const int i : pin_layers.index_range()) { + pin_offsets[i] = ldata.layers[pin_layers[i]].offset; + } + + Array need_vert_sel(vert_sel_layers.size()); + Array need_edge_sel(edge_sel_layers.size()); + Array need_pin(pin_layers.size()); char hflag = 0; BMIter iter; int face_i = 0; @@ -1150,6 +1190,21 @@ static void bm_face_loop_table_build(BMesh &bm, for ([[maybe_unused]] const int i : IndexRange(face->len)) { BM_elem_index_set(loop, loop_i); /* set_inline */ loop_table[loop_i] = loop; + for (const int i : vert_sel_offsets.index_range()) { + if (BM_ELEM_CD_GET_BOOL(loop, vert_sel_offsets[i])) { + need_vert_sel[i] = true; + } + } + for (const int i : edge_sel_offsets.index_range()) { + if (BM_ELEM_CD_GET_BOOL(loop, edge_sel_offsets[i])) { + need_edge_sel[i] = true; + } + } + for (const int i : pin_offsets.index_range()) { + if (BM_ELEM_CD_GET_BOOL(loop, pin_offsets[i])) { + need_pin[i] = true; + } + } BM_CHECK_ELEMENT(loop); loop = loop->next; loop_i++; @@ -1157,6 +1212,25 @@ static void bm_face_loop_table_build(BMesh &bm, } need_select_poly = (hflag & BM_ELEM_SELECT) != 0; need_hide_poly = (hflag & BM_ELEM_HIDDEN) != 0; + + for (const int i : vert_sel_layers.index_range()) { + if (!need_vert_sel[i]) { + ldata.layers[vert_sel_layers[i]].flag |= CD_FLAG_NOCOPY; + ldata_layers_marked_nocopy.append(vert_sel_layers[i]); + } + } + for (const int i : edge_sel_layers.index_range()) { + if (!need_edge_sel[i]) { + ldata.layers[edge_sel_layers[i]].flag |= CD_FLAG_NOCOPY; + ldata_layers_marked_nocopy.append(edge_sel_layers[i]); + } + } + for (const int i : pin_layers.index_range()) { + if (!need_pin[i]) { + ldata.layers[pin_layers[i]].flag |= CD_FLAG_NOCOPY; + ldata_layers_marked_nocopy.append(pin_layers[i]); + } + } } static void bmesh_block_copy_to_mesh_attributes(const Span copy_info, @@ -1303,14 +1377,10 @@ static void bm_to_mesh_loops(const BMesh &bm, const Span bm_loop void BM_mesh_bm_to_me(Main *bmain, BMesh *bm, Mesh *me, const struct BMeshToMeshParams *params) { + SCOPED_TIMER_AVERAGED(__func__); using namespace blender; - BMFace *f; - BMIter iter; - const int ototvert = me->totvert; - blender::Vector ldata_layers_marked_nocopy; - /* Free custom data. */ CustomData_free(&me->vdata, me->totvert); CustomData_free(&me->edata, me->totedge); @@ -1328,75 +1398,6 @@ void BM_mesh_bm_to_me(Main *bmain, BMesh *bm, Mesh *me, const struct BMeshToMesh me->totpoly = bm->totface; me->act_face = -1; - /* Mark UV selection layers which are all false as 'nocopy'. */ - for (const int layer_index : - IndexRange(CustomData_number_of_layers(&bm->ldata, CD_PROP_FLOAT2))) { - char const *layer_name = CustomData_get_layer_name(&bm->ldata, CD_PROP_FLOAT2, layer_index); - char sub_layer_name[MAX_CUSTOMDATA_LAYER_NAME]; - int vertsel_layer_index = CustomData_get_named_layer_index( - &bm->ldata, CD_PROP_BOOL, BKE_uv_map_vert_select_name_get(layer_name, sub_layer_name)); - int edgesel_layer_index = CustomData_get_named_layer_index( - &bm->ldata, CD_PROP_BOOL, BKE_uv_map_edge_select_name_get(layer_name, sub_layer_name)); - int pin_layer_index = CustomData_get_named_layer_index( - &bm->ldata, CD_PROP_BOOL, BKE_uv_map_pin_name_get(layer_name, sub_layer_name)); - - /* If ever the uv map associated bool layers become optional in BMesh as well (like in Mesh) - * this assert needs to be removed. For now it is a bug if they don't exist. */ - BLI_assert(vertsel_layer_index >= 0 && edgesel_layer_index >= 0 && pin_layer_index >= 0); - - int vertsel_offset = vertsel_layer_index >= 0 ? bm->ldata.layers[vertsel_layer_index].offset : - -1; - int edgesel_offset = edgesel_layer_index >= 0 ? bm->ldata.layers[edgesel_layer_index].offset : - -1; - int pin_offset = pin_layer_index >= 0 ? bm->ldata.layers[pin_layer_index].offset : -1; - - bool need_vertsel = false; - bool need_edgesel = false; - bool need_pin = false; - - BM_ITER_MESH (f, &iter, bm, BM_FACES_OF_MESH) { - BMIter liter; - BMLoop *l; - if (vertsel_layer_index >= 0) { - BM_ITER_ELEM (l, &liter, f, BM_LOOPS_OF_FACE) { - need_vertsel |= BM_ELEM_CD_GET_BOOL(l, vertsel_offset); - } - } - if (edgesel_layer_index >= 0) { - BM_ITER_ELEM (l, &liter, f, BM_LOOPS_OF_FACE) { - need_edgesel |= BM_ELEM_CD_GET_BOOL(l, edgesel_offset); - } - } - if (pin_layer_index >= 0) { - BM_ITER_ELEM (l, &liter, f, BM_LOOPS_OF_FACE) { - need_pin |= BM_ELEM_CD_GET_BOOL(l, pin_offset); - } - } - } - - if (need_vertsel) { - bm->ldata.layers[vertsel_layer_index].flag &= ~CD_FLAG_NOCOPY; - } - else { - bm->ldata.layers[vertsel_layer_index].flag |= CD_FLAG_NOCOPY; - ldata_layers_marked_nocopy.append(vertsel_layer_index); - } - if (need_edgesel) { - bm->ldata.layers[edgesel_layer_index].flag &= ~CD_FLAG_NOCOPY; - } - else { - bm->ldata.layers[edgesel_layer_index].flag |= CD_FLAG_NOCOPY; - ldata_layers_marked_nocopy.append(edgesel_layer_index); - } - if (need_pin) { - bm->ldata.layers[pin_layer_index].flag &= ~CD_FLAG_NOCOPY; - } - else { - bm->ldata.layers[pin_layer_index].flag |= CD_FLAG_NOCOPY; - ldata_layers_marked_nocopy.append(pin_layer_index); - } - } - { CustomData_MeshMasks mask = CD_MASK_MESH; CustomData_MeshMasks_update(&mask, ¶ms->cd_mask_extra); @@ -1420,6 +1421,7 @@ void BM_mesh_bm_to_me(Main *bmain, BMesh *bm, Mesh *me, const struct BMeshToMesh Array edge_table; Array face_table; Array loop_table; + Vector ldata_layers_marked_nocopy; threading::parallel_invoke( me->totface > 1024, [&]() { @@ -1440,7 +1442,8 @@ void BM_mesh_bm_to_me(Main *bmain, BMesh *bm, Mesh *me, const struct BMeshToMesh need_select_poly, need_hide_poly, need_sharp_face, - need_material_index); + need_material_index, + ldata_layers_marked_nocopy); }); bm->elem_index_dirty &= ~(BM_VERT | BM_EDGE | BM_FACE | BM_LOOP); @@ -1524,7 +1527,6 @@ void BM_mesh_bm_to_me(Main *bmain, BMesh *bm, Mesh *me, const struct BMeshToMesh bm_to_mesh_loops(*bm, loop_table, *me); /* Topology could be changed, ensure #CD_MDISPS are ok. */ multires_topology_changed(me); - /* 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; } @@ -1638,6 +1640,7 @@ void BM_mesh_bm_to_me_for_eval(BMesh *bm, Mesh *me, const CustomData_MeshMasks * Array edge_table; Array face_table; Array loop_table; + Vector ldata_layers_marked_nocopy; threading::parallel_invoke( me->totface > 1024, [&]() { @@ -1658,7 +1661,8 @@ void BM_mesh_bm_to_me_for_eval(BMesh *bm, Mesh *me, const CustomData_MeshMasks * need_select_poly, need_hide_poly, need_sharp_face, - need_material_index); + need_material_index, + ldata_layers_marked_nocopy); }); bm->elem_index_dirty &= ~(BM_VERT | BM_EDGE | BM_FACE | BM_LOOP); @@ -1729,7 +1733,12 @@ void BM_mesh_bm_to_me_for_eval(BMesh *bm, Mesh *me, const CustomData_MeshMasks * sharp_face.span, material_index.span); }, - [&]() { bm_to_mesh_loops(*bm, loop_table, *me); }); + [&]() { + bm_to_mesh_loops(*bm, loop_table, *me); + for (const int i : ldata_layers_marked_nocopy) { + bm->ldata.layers[i].flag &= ~CD_FLAG_NOCOPY; + } + }); select_vert.finish(); hide_vert.finish();