From 870f75b790db7eaa2e81da430ffc0fce5bd4ebe3 Mon Sep 17 00:00:00 2001 From: Piotr Makal Date: Thu, 19 Jun 2025 09:33:52 -0400 Subject: [PATCH] Fix #79163: Bevel operation produces disconnected UVs #139595. Fix #79163 bug related to the bevel operation producing disconnected UVs for new bevel faces. This change replaces previous approach using scattered and selective usage of functions: bev_merge_uvs, bev_merge_edge_uvs and bev_merge_end_uvs with one coherent technique for all stages of the bevel operation. It is utilizing a concept of loop (BMLoop) buckets to keep track of UV vertices that should be merged at the end of bevel operation by a single call to bevel_merge_uvs function. This approach doesn't touch initial UV position calculation done by interpolation algorithm in bev_create_ngon function and keeps the concept of representative faces (called frep, facerep or rep_face in code) to help decide to which bucket specific loops should be assigned. This is from PR https://projects.blender.org/blender/blender/pulls/139595, which has more explanation and discussion. --- source/blender/bmesh/tools/bmesh_bevel.cc | 549 +++++++++++++------- tests/files/modeling/bevel_regression.blend | 4 +- tests/files/modeling/modifiers.blend | 4 +- tests/python/bevel_operator.py | 6 +- 4 files changed, 361 insertions(+), 202 deletions(-) diff --git a/source/blender/bmesh/tools/bmesh_bevel.cc b/source/blender/bmesh/tools/bmesh_bevel.cc index 8645b3a29b9..9d5b312b066 100644 --- a/source/blender/bmesh/tools/bmesh_bevel.cc +++ b/source/blender/bmesh/tools/bmesh_bevel.cc @@ -17,13 +17,14 @@ #include "DNA_modifier_types.h" #include "BLI_alloca.h" +#include "BLI_map.hh" #include "BLI_math_base.h" -#include "BLI_math_base_safe.h" #include "BLI_math_geom.h" #include "BLI_math_matrix.h" #include "BLI_math_rotation.h" #include "BLI_math_vector.h" #include "BLI_memarena.h" +#include "BLI_set.hh" #include "BLI_utildefines.h" #include "BLI_vector.hh" @@ -31,6 +32,7 @@ #include "BKE_customdata.hh" #include "BKE_deform.hh" #include "BKE_mesh.hh" +#include "BKE_mesh_mapping.hh" #include "eigen_capi.h" @@ -39,6 +41,8 @@ #include "./intern/bmesh_private.hh" +using blender::Map; +using blender::Set; using blender::Vector; // #define BEVEL_DEBUG_TIME @@ -197,6 +201,19 @@ struct MathLayerInfo { bool has_math_layers; }; +/** + * Auxiliary structure representing bevel face created by `bev_create_ngon` function. It holds + * reference to both newly create ngon and a representative face (from the original mesh) it is + * attached to. This information helps with merging UVs - bevel faces that share the same + * `attached_frep` pointer should have their neighboring UV verts connected. + */ +struct UVFace { + /** `BMesh` face which this `UVFace` represents. */ + BMFace *f; + /** `BMFace` of the original mesh to which bevel face `f` is attached in UV space. */ + BMFace *attached_frep; +}; + /** * An element in a cyclic boundary of a Vertex Mesh (VMesh), placed on each side of beveled edges * where each profile starts, or on each side of a miter. @@ -313,12 +330,22 @@ enum AngleKind { ANGLE_LARGER = 1, }; +/** Container for loops representing UV verts which should be merged together in a UV map. */ +using UVVertBucket = Set; + +/** Mapping of vertex to UV vert buckets (i.e. loops belonging to that key `BMVert`). */ +using UVVertMap = Map>; + /** Bevel parameters and state. */ struct BevelParams { /** Records BevVerts made: key BMVert*, value BevVert* */ GHash *vert_hash; /** Records new faces: key BMFace*, value one of {VERT/EDGE/RECON}_POLY. */ GHash *face_hash; + /** Records `UVFace` made: key `BMFace*`, value `UVFace*`. */ + GHash *uv_face_hash; + /** Container which keeps track of UV vert connectivity in different UV maps. */ + Vector uv_vert_maps; /** * Use for all allocations while bevel runs. * \note If we need to free we can switch to `BLI_mempool`. @@ -535,6 +562,12 @@ static BevVert *find_bevvert(BevelParams *bp, BMVert *bmv) return static_cast(BLI_ghash_lookup(bp->vert_hash, bmv)); } +/* Find the `UVFace` corresponding to `bmf` face. */ +static UVFace *find_uv_face(BevelParams *bp, BMFace *bmf) +{ + return static_cast(BLI_ghash_lookup(bp->uv_face_hash, bmf)); +} + /** * Find the EdgeHalf representing the other end of e->e. * \return other end's BevVert in *r_bvother, if r_bvother is provided. That may not have @@ -604,6 +637,202 @@ static bool edges_face_connected_at_vert(BMEdge *bme1, BMEdge *bme2) return false; } +/** + * Create and register new `UVFace` object based on a new face (`BMFace *fnew`); and assign proper + * representative face from either `frep` or `frep_arr` arguments. + */ +static UVFace *register_uv_face(BevelParams *bp, BMFace *fnew, BMFace *frep, BMFace **frep_arr) +{ + UVFace *uv_face = (UVFace *)BLI_memarena_alloc(bp->mem_arena, sizeof(UVFace)); + uv_face->f = fnew; + uv_face->attached_frep = nullptr; + if (frep_arr && frep_arr[0]) { + /* Choosing first face from `frep_arr` is an arbitrary choice but for our algorithm it doesn't + * matter. Usually the difference in `frep` and `frep_arr` is that the latter is used when + * loops' custom data for a new bevel face is interpolated between multiple original mesh faces + * on top of which this new bevel face is being constructed; in such case original faces + * _should_ be already connected in UV space (i.e. no seam) and we handle such scenarios when + * setting proper value for `is_orig_uv_verts_connected` variable. */ + uv_face->attached_frep = frep_arr[0]; + } + else if (frep) { + uv_face->attached_frep = frep; + } + + BLI_ghash_insert(bp->uv_face_hash, fnew, uv_face); + return uv_face; +} + +/** + * Update UV vert map with new loops (`BMLoop`) from a face (`uv_face->f`) to keep track of proper + * UV connectivity. This data will help with merging UV verts later. Loops stored in the same UV + * vert bucket will be merged together (their UV positions). + */ +static void update_uv_vert_map(BevelParams *bp, + UVFace *uv_face, + BMVert *bv, + Map *nv_bv_map) +{ + if (!uv_face || !uv_face->attached_frep) { + return; + } + + for (UVVertMap &uv_vert_map : bp->uv_vert_maps) { + BMIter iter; + BMLoop *l; + BM_ITER_ELEM (l, &iter, uv_face->f, BM_LOOPS_OF_FACE) { + Vector *uv_vert_buckets = uv_vert_map.lookup_ptr(l->v); + if (!uv_vert_buckets) { + /* New vertex (and a corresponding loop) needs to be registered. No need for further UV + * connectivity search, we just create a new bucket and move on. */ + uv_vert_map.add(l->v, Vector{{l}}); + continue; + } + + /* `orig_v` should always point to a vertex which takes part in a bevel operation and comes + * from the original mesh. This vertex is equivalent to what is stored in the `BevVert::v` + * field. */ + BMVert *orig_v = nv_bv_map ? nv_bv_map->lookup(l->v) : bv; + BMLoop *orig_l = BM_face_vert_share_loop(uv_face->attached_frep, orig_v); + BLI_assert(orig_l != nullptr); + + bool is_bucket_found = false; + BMIter iter2; + BMLoop *l2; + BM_ITER_ELEM (l2, &iter2, l->v, BM_LOOPS_OF_VERT) { + if (l == l2) { + continue; + } + + UVFace *uv_face2 = find_uv_face(bp, l2->f); + if (!uv_face2) { + continue; + } + + BMLoop *orig_l2 = BM_face_vert_share_loop(uv_face2->attached_frep, orig_v); + BLI_assert(orig_l2 != nullptr); + + bool is_orig_uv_verts_connected = false; + Vector &orig_uv_vert_buckets = uv_vert_map.lookup(orig_v); + for (UVVertBucket &orig_uv_vert_bucket : orig_uv_vert_buckets) { + if (orig_uv_vert_bucket.contains(orig_l) && orig_uv_vert_bucket.contains(orig_l2)) { + is_orig_uv_verts_connected = true; + break; + } + } + + /* Add loop `l` to the existing bucket containing neighboring loop `l2` if either of those + * conditions are met: + * 1. Neighboring faces (represented by `UVFace` objects) have the same representative face + * attached to them. + * 2. If representative faces attached to faces containing both loops (`l` and `l2`) are + * different but otherwise connected in UV space (`orig_l` and `orig_l2` are + * overlapping) for original input mesh. */ + if (uv_face->attached_frep == uv_face2->attached_frep || is_orig_uv_verts_connected) { + for (UVVertBucket &uv_vert_bucket : *uv_vert_buckets) { + if (uv_vert_bucket.contains(l2)) { + uv_vert_bucket.add(l); + is_bucket_found = true; + break; + } + } + } + if (is_bucket_found) { + break; + } + } + if (!is_bucket_found) { + uv_vert_buckets->append(UVVertBucket{l}); + } + } + } +} + +/** + * Determine UV vert connectivity based on provided `BMVert *v`. If UV loop data is available, + * iterate through loops of vertex `v`, fetching UV position for each loop and checking against + * already evaluated ones. If UV coords are overlapping (delta smaller than `STD_UV_CONNECT_LIMIT`) + * then add those loops to the same UV vert bucket. If UV verts are not overlapping they will end + * up in separate buckets. Those buckets are later utilized during UV merging process, i.e. UV + * verts which will end up in the same bucket will be merged together. + */ +static void determine_uv_vert_connectivity(BevelParams *bp, BMesh *bm, BMVert *v) +{ + int num_uv_layers = CustomData_number_of_layers(&bm->ldata, CD_PROP_FLOAT2); + BLI_assert(bp->uv_vert_maps.size() == num_uv_layers); + + for (int i = 0; i < num_uv_layers; ++i) { + int uv_data_offset = CustomData_get_n_offset(&bm->ldata, CD_PROP_FLOAT2, i); + Vector uv_vert_buckets; + BMIter iter; + BMLoop *l; + BM_ITER_ELEM (l, &iter, v, BM_LOOPS_OF_VERT) { + float *luv = BM_ELEM_CD_GET_FLOAT_P(l, uv_data_offset); + bool is_overlap_found = false; + for (UVVertBucket &uv_vert_bucket : uv_vert_buckets) { + for (BMLoop *l2 : uv_vert_bucket) { + float *luv2 = BM_ELEM_CD_GET_FLOAT_P(l2, uv_data_offset); + if (compare_v2v2(luv, luv2, STD_UV_CONNECT_LIMIT)) { + uv_vert_bucket.add(l); + is_overlap_found = true; + break; + } + } + if (is_overlap_found) { + break; + } + } + if (!is_overlap_found) { + uv_vert_buckets.append(UVVertBucket{l}); + } + } + + /* For now `determine_uv_vert_connectivity` is expected to be run at the beginning of the bevel + * operation, determining connectivity for each vertex `v` (from the original mesh). We expect + * `bp->uv_vert_maps[i]` to not contain vertex `v` at this point in time. */ + BLI_assert(bp->uv_vert_maps[i].contains(v) == false); + bp->uv_vert_maps[i].add_new(v, uv_vert_buckets); + } +} + +/** + * Merge UVs based on data gathered in `bm->uv_vert_maps`. If UV verts are in the same bucket, + * merge them together. Note that this function exists purely because of imperfections in initial + * UV position calculations using interpolation approach (`BM_loop_interp_from_face` function). + */ +static void bevel_merge_uvs(BevelParams *bp, BMesh *bm) +{ + int num_uv_layers = CustomData_number_of_layers(&bm->ldata, CD_PROP_FLOAT2); + BLI_assert(bp->uv_vert_maps.size() == num_uv_layers); + + for (int i = 0; i < num_uv_layers; ++i) { + int uv_data_offset = CustomData_get_n_offset(&bm->ldata, CD_PROP_FLOAT2, i); + for (Vector &uv_vert_buckets : bp->uv_vert_maps[i].values()) { + for (UVVertBucket &uv_vert_bucket : uv_vert_buckets) { + /* Using face weights instead of mean average because it produces slightly better results, + * although this is purely empirical and subjective. */ + float weight_sum = 0.0f; + float uv[2] = {0.0f, 0.0f}; + for (BMLoop *l : uv_vert_bucket) { + float *luv = BM_ELEM_CD_GET_FLOAT_P(l, uv_data_offset); + float weighted_luv[2] = {0.0f, 0.0f}; + float face_area = BM_face_calc_area(l->f); + mul_v2_v2fl(weighted_luv, luv, face_area); + add_v2_v2(uv, weighted_luv); + weight_sum += face_area; + } + if (uv_vert_bucket.size() > 1 && weight_sum > 0.0f) { + mul_v2_fl(uv, 1.0f / weight_sum); + for (BMLoop *l : uv_vert_bucket) { + float *luv = BM_ELEM_CD_GET_FLOAT_P(l, uv_data_offset); + copy_v2_v2(luv, uv); + } + } + } + } + } +} + /** * Return a good representative face (for materials, etc.) for faces * created around/near BoundVert v. @@ -675,18 +904,24 @@ static BMFace *boundvert_rep_face(BoundVert *v, BMFace **r_fother) * * \note ALL face creation goes through this function, this is important to keep! */ -static BMFace *bev_create_ngon(BMesh *bm, +static BMFace *bev_create_ngon(BevelParams *bp, + BMesh *bm, BMVert **vert_arr, const int totv, BMFace **face_arr, BMFace *facerep, BMEdge **snap_edge_arr, + BMVert *bv, + Map *nv_bv_map, int mat_nr, bool do_interp) { BMFace *f = BM_face_create_verts(bm, vert_arr, totv, facerep, BM_CREATE_NOP, true); + if (!f) { + return nullptr; + } - if ((facerep || (face_arr && face_arr[0])) && f) { + if ((facerep || (face_arr && face_arr[0]))) { BM_elem_attrs_copy(bm, facerep ? facerep : face_arr[0], f); if (do_interp) { int i = 0; @@ -722,20 +957,22 @@ static BMFace *bev_create_ngon(BMesh *bm, } } - /* Not essential for bevels own internal logic, - * this is done so the operator can select newly created geometry. */ - if (f) { - BM_elem_flag_enable(f, BM_ELEM_TAG); - BMIter iter; - BMEdge *bme; - BM_ITER_ELEM (bme, &iter, f, BM_EDGES_OF_FACE) { - flag_out_edge(bm, bme); - } + BM_elem_flag_enable(f, BM_ELEM_TAG); + BMIter iter; + BMEdge *bme; + BM_ITER_ELEM (bme, &iter, f, BM_EDGES_OF_FACE) { + /* Not essential for bevels own internal logic, this is done so the operator can select newly + * created geometry. */ + flag_out_edge(bm, bme); } if (mat_nr >= 0) { f->mat_nr = short(mat_nr); } + + UVFace *uv_face = register_uv_face(bp, f, facerep, face_arr); + update_uv_vert_map(bp, uv_face, bv, nv_bv_map); + return f; } @@ -815,6 +1052,26 @@ static void swap_face_components(int *face_component, int totface, int c1, int c } } +/** + * Initialize `bp->uv_vert_maps` to the size equal to the number of UV layers. + */ +static void uv_vert_map_init(BevelParams *bp, BMesh *bm) +{ + int num_uv_layers = CustomData_number_of_layers(&bm->ldata, CD_PROP_FLOAT2); + bp->uv_vert_maps.clear(); + bp->uv_vert_maps.resize(num_uv_layers); +} + +/** + * Remove vertex `v` from all UV maps in `bp->uv_vert_maps`. + */ +static void uv_vert_map_pop(BevelParams *bp, BMVert *v) +{ + for (UVVertMap &uv_vert_map : bp->uv_vert_maps) { + uv_vert_map.pop_try(v); + } +} + /* * Set up the fields of bp->math_layer_info. * We always set has_math_layers to the correct value. @@ -1022,80 +1279,6 @@ static BMFace *choose_rep_face(BevelParams *bp, BMFace **face, int nfaces) #undef VEC_VALUE_LEN } -/* Merge (using average) all the UV values for loops of v's faces. - * Caller should ensure that no seams are violated by doing this. */ -static void bev_merge_uvs(BMesh *bm, BMVert *v) -{ - int num_of_uv_layers = CustomData_number_of_layers(&bm->ldata, CD_PROP_FLOAT2); - - for (int i = 0; i < num_of_uv_layers; i++) { - int cd_loop_uv_offset = CustomData_get_n_offset(&bm->ldata, CD_PROP_FLOAT2, i); - - if (cd_loop_uv_offset == -1) { - return; - } - - int n = 0; - float uv[2] = {0.0f, 0.0f}; - BMIter iter; - BMLoop *l; - BM_ITER_ELEM (l, &iter, v, BM_LOOPS_OF_VERT) { - float *luv = BM_ELEM_CD_GET_FLOAT_P(l, cd_loop_uv_offset); - add_v2_v2(uv, luv); - n++; - } - if (n > 1) { - mul_v2_fl(uv, 1.0f / float(n)); - BM_ITER_ELEM (l, &iter, v, BM_LOOPS_OF_VERT) { - float *luv = BM_ELEM_CD_GET_FLOAT_P(l, cd_loop_uv_offset); - copy_v2_v2(luv, uv); - } - } - } -} - -/* Merge (using average) the UV values for two specific loops of v: those for faces containing v, - * and part of faces that share edge bme. */ -static void bev_merge_edge_uvs(BMesh *bm, BMEdge *bme, BMVert *v) -{ - int num_of_uv_layers = CustomData_number_of_layers(&bm->ldata, CD_PROP_FLOAT2); - - BMLoop *l1 = nullptr; - BMLoop *l2 = nullptr; - BMIter iter; - BMLoop *l; - BM_ITER_ELEM (l, &iter, v, BM_LOOPS_OF_VERT) { - if (l->e == bme) { - l1 = l; - } - else if (l->prev->e == bme) { - l2 = l; - } - } - if (l1 == nullptr || l2 == nullptr) { - return; - } - - for (int i = 0; i < num_of_uv_layers; i++) { - int cd_loop_uv_offset = CustomData_get_n_offset(&bm->ldata, CD_PROP_FLOAT2, i); - - if (cd_loop_uv_offset == -1) { - return; - } - - float uv[2] = {0.0f, 0.0f}; - float *luv = BM_ELEM_CD_GET_FLOAT_P(l1, cd_loop_uv_offset); - add_v2_v2(uv, luv); - luv = BM_ELEM_CD_GET_FLOAT_P(l2, cd_loop_uv_offset); - add_v2_v2(uv, luv); - mul_v2_fl(uv, 0.5f); - luv = BM_ELEM_CD_GET_FLOAT_P(l1, cd_loop_uv_offset); - copy_v2_v2(luv, uv); - luv = BM_ELEM_CD_GET_FLOAT_P(l2, cd_loop_uv_offset); - copy_v2_v2(luv, uv); - } -} - /* Calculate coordinates of a point a distance d from v on e->e and return it in slideco. */ static void slide_dist(EdgeHalf *e, BMVert *v, float d, float r_slideco[3]) { @@ -2652,21 +2835,6 @@ static void set_bound_vert_seams(BevVert *bv, bool mark_seam, bool mark_sharp) } } -static int count_bound_vert_seams(BevVert *bv) -{ - if (!bv->any_seam) { - return 0; - } - - int ans = 0; - for (int i = 0; i < bv->edgecount; i++) { - if (bv->edges[i].is_seam) { - ans++; - } - } - return ans; -} - /* Is e between two faces with a 180 degree angle between their normals? */ static bool eh_on_plane(EdgeHalf *e) { @@ -4940,7 +5108,8 @@ static void build_center_ngon(BevelParams *bp, BMesh *bm, BevVert *bv, int mat_n ve.append(nullptr); } } while ((v = v->next) != vm->boundstart); - BMFace *f = bev_create_ngon(bm, vv.data(), vv.size(), vf.data(), frep, ve.data(), mat_nr, true); + BMFace *f = bev_create_ngon( + bp, bm, vv.data(), vv.size(), vf.data(), frep, ve.data(), bv->v, nullptr, mat_nr, true); record_face_kind(bp, f, F_VERT); } @@ -5530,32 +5699,13 @@ static void bevel_build_rings(BevelParams *bp, BMesh *bm, BevVert *bv, BoundVert se[2] = se[1] != nullptr ? se[1] : se[3]; } } - BMFace *r_f = bev_create_ngon(bm, bmvs, 4, fr, nullptr, se, mat_nr, true); + BMFace *r_f = bev_create_ngon( + bp, bm, bmvs, 4, fr, nullptr, se, bv->v, nullptr, mat_nr, true); record_face_kind(bp, r_f, F_VERT); } } } while ((bndv = bndv->next) != vm->boundstart); - /* Fix UVs along center lines if even number of segments. */ - if (!odd) { - bndv = vm->boundstart; - do { - int i = bndv->index; - if (!bndv->any_seam) { - for (int ring = 1; ring < ns2; ring++) { - BMVert *v_uv = mesh_vert(vm, i, ring, ns2)->v; - if (v_uv) { - bev_merge_uvs(bm, v_uv); - } - } - } - } while ((bndv = bndv->next) != vm->boundstart); - BMVert *bmv = mesh_vert(vm, 0, ns2, ns2)->v; - if (bp->affect_type == BEVEL_AFFECT_VERTICES || count_bound_vert_seams(bv) <= 1) { - bev_merge_uvs(bm, bmv); - } - } - /* Center ngon. */ if (odd) { if (bp->affect_type == BEVEL_AFFECT_EDGES) { @@ -5563,8 +5713,17 @@ static void bevel_build_rings(BevelParams *bp, BMesh *bm, BevVert *bv, BoundVert if (bv->any_seam) { frep = frep_for_center_poly(bp, bv); } - BMFace *cen_f = bev_create_ngon( - bm, center_verts, n_bndv, center_face_interps, frep, center_edge_snaps, mat_nr, true); + BMFace *cen_f = bev_create_ngon(bp, + bm, + center_verts, + n_bndv, + center_face_interps, + frep, + center_edge_snaps, + bv->v, + nullptr, + mat_nr, + true); record_face_kind(bp, cen_f, F_VERT); } else { @@ -5698,12 +5857,15 @@ static void bevel_build_cutoff(BevelParams *bp, BMesh *bm, BevVert *bv) /* Create the profile cutoff face for this boundvert. */ // repface = boundvert_rep_face(bndv, nullptr); - bev_create_ngon(bm, + bev_create_ngon(bp, + bm, face_bmverts, bp->seg + 2 + build_center_face, nullptr, nullptr, nullptr, + bv->v, + nullptr, bp->mat_nr, true); } while ((bndv = bndv->next) != bv->vmesh->boundstart); @@ -5716,7 +5878,8 @@ static void bevel_build_cutoff(BevelParams *bp, BMesh *bm, BevVert *bv) face_bmverts[i] = mesh_vert(bv->vmesh, i, 1, 0)->v; } - bev_create_ngon(bm, face_bmverts, n_bndv, nullptr, nullptr, nullptr, bp->mat_nr, true); + bev_create_ngon( + bp, bm, face_bmverts, n_bndv, nullptr, nullptr, nullptr, bv->v, nullptr, bp->mat_nr, true); } } @@ -5780,8 +5943,17 @@ static BMFace *bevel_build_poly(BevelParams *bp, BMesh *bm, BevVert *bv) BMFace *f; if (n > 2) { - f = bev_create_ngon( - bm, bmverts.data(), n, bmfaces.data(), repface, bmedges.data(), bp->mat_nr, true); + f = bev_create_ngon(bp, + bm, + bmverts.data(), + n, + bmfaces.data(), + repface, + bmedges.data(), + bv->v, + nullptr, + bp->mat_nr, + true); record_face_kind(bp, f, F_VERT); } else { @@ -6568,8 +6740,8 @@ static bool bev_rebuild_polygon(BMesh *bm, BevelParams *bp, BMFace *f) { bool do_rebuild = false; Vector vv; - Vector vv_fix; Vector ee; + Map nv_bv_map; /* New vertex to the (original) bevel vertex mapping. */ BMIter liter; BMLoop *l; @@ -6634,10 +6806,9 @@ static bool bev_rebuild_polygon(BMesh *bm, BevelParams *bp, BMFace *f) if (!on_profile_start) { vv.append(v->nv.v); ee.append(bme); + nv_bv_map.add(v->nv.v, l->v); } while (v != vend) { - /* Check for special case: multi-segment 3rd face opposite a beveled edge with no vmesh. */ - bool corner3special = (vm->mesh_kind == M_NONE && v->ebev != e && v->ebev != eprev); if (go_ccw) { int i = v->index; int kstart, kend; @@ -6659,9 +6830,7 @@ static bool bev_rebuild_polygon(BMesh *bm, BevelParams *bp, BMFace *f) if (bmv) { vv.append(bmv); ee.append(bme); /* TODO: Maybe better edge here. */ - if (corner3special && v->ebev && !bv->any_seam && k != vm->seg) { - vv_fix.append(bmv); - } + nv_bv_map.add(bmv, l->v); } } v = v->next; @@ -6688,9 +6857,7 @@ static bool bev_rebuild_polygon(BMesh *bm, BevelParams *bp, BMFace *f) if (bmv) { vv.append(bmv); ee.append(bme); - if (corner3special && v->ebev && !bv->any_seam && k != 0) { - vv_fix.append(bmv); - } + nv_bv_map.add(bmv, l->v); } } v = v->prev; @@ -6701,15 +6868,13 @@ static bool bev_rebuild_polygon(BMesh *bm, BevelParams *bp, BMFace *f) else { vv.append(l->v); ee.append(l->e); + nv_bv_map.add(l->v, l->v); // We keep the old vertex, i.e. mapping to itself. } } if (do_rebuild) { const int64_t n = vv.size(); - BMFace *f_new = bev_create_ngon(bm, vv.data(), n, nullptr, f, nullptr, -1, true); - - for (int64_t k = 0; k < vv_fix.size(); k++) { - bev_merge_uvs(bm, vv_fix[k]); - } + BMFace *f_new = bev_create_ngon( + bp, bm, vv.data(), n, nullptr, f, nullptr, nullptr, &nv_bv_map, -1, true); /* Copy attributes from old edges. */ BLI_assert(n == ee.size()); @@ -6766,25 +6931,23 @@ static bool bev_rebuild_polygon(BMesh *bm, BevelParams *bp, BMFace *f) return do_rebuild; } -/* All polygons touching v need rebuilding because beveling v has made new vertices. */ -static void bevel_rebuild_existing_polygons(BMesh *bm, BevelParams *bp, BMVert *v) +/* All polygons touching `v` need rebuilding because beveling `v` has made new vertices. */ +static void bevel_rebuild_existing_polygons(BMesh *bm, + BevelParams *bp, + BMVert *v, + Set &rebuilt_orig_faces) { - void *faces_stack[BM_DEFAULT_ITER_STACK_SIZE]; - int faces_len, f_index; - BMFace **faces = static_cast(BM_iter_as_arrayN( - bm, BM_FACES_OF_VERT, v, &faces_len, faces_stack, BM_DEFAULT_ITER_STACK_SIZE)); - - if (LIKELY(faces != nullptr)) { - for (f_index = 0; f_index < faces_len; f_index++) { - BMFace *f = faces[f_index]; + BMIter iter; + BMFace *f; + BM_ITER_ELEM (f, &iter, v, BM_FACES_OF_VERT) { + /* Deletion of original mesh faces that are being rebuild is deferred thus we have to perform + * a check against `rebuilt_orig_faces` container - previous calls to + * `bevel_rebuild_existing_polygons` could have already rebuilt faces touching vertex `v`. */ + if (!rebuilt_orig_faces.contains(f)) { if (bev_rebuild_polygon(bm, bp, f)) { - BM_face_kill(bm, f); + rebuilt_orig_faces.add(f); } } - - if (faces != (BMFace **)faces_stack) { - MEM_freeN(faces); - } } } @@ -6838,17 +7001,6 @@ static void bevel_reattach_wires(BMesh *bm, BevelParams *bp, BMVert *v) } } -static void bev_merge_end_uvs(BMesh *bm, BevVert *bv, EdgeHalf *e) -{ - VMesh *vm = bv->vmesh; - - int nseg = e->seg; - int i = e->leftv->index; - for (int k = 1; k < nseg; k++) { - bev_merge_uvs(bm, mesh_vert(vm, i, 0, k)->v); - } -} - /* * Is this BevVert the special case of a weld (no vmesh) where there are * four edges total, two are beveled, and the other two are on opposite sides? @@ -6966,6 +7118,11 @@ static void bevel_build_edge_polygons(BMesh *bm, BevelParams *bp, BMEdge *bme) BMVert *verts[4]; verts[0] = bmv1; verts[1] = bmv2; + + Map nv_bv_map; /* New vertex to the (original) bevel vertex mapping. */ + nv_bv_map.add(verts[0], bv1->v); + nv_bv_map.add(verts[1], bv2->v); + int odd = nseg % 2; int mid = nseg / 2; BMEdge *center_bme = nullptr; @@ -6981,6 +7138,8 @@ static void bevel_build_edge_polygons(BMesh *bm, BevelParams *bp, BMEdge *bme) for (int k = 1; k <= nseg; k++) { verts[3] = mesh_vert(vm1, i1, 0, k)->v; verts[2] = mesh_vert(vm2, i2, 0, nseg - k)->v; + nv_bv_map.add(verts[3], bv1->v); + nv_bv_map.add(verts[2], bv2->v); BMFace *r_f; if (odd && k == mid + 1) { if (e1->is_seam) { @@ -6996,11 +7155,13 @@ static void bevel_build_edge_polygons(BMesh *bm, BevelParams *bp, BMEdge *bme) edges[0] = edges[1] = bme; edges[2] = edges[3] = nullptr; } - r_f = bev_create_ngon(bm, verts, 4, nullptr, f_choice, edges, mat_nr, true); + r_f = bev_create_ngon( + bp, bm, verts, 4, nullptr, f_choice, edges, nullptr, &nv_bv_map, mat_nr, true); } else { /* Straddles but not a seam: interpolate left half in f1, right half in f2. */ - r_f = bev_create_ngon(bm, verts, 4, faces, f_choice, nullptr, mat_nr, true); + r_f = bev_create_ngon( + bp, bm, verts, 4, faces, f_choice, nullptr, nullptr, &nv_bv_map, mat_nr, true); } } else if (odd && k == center_adj_k && e1->is_seam) { @@ -7020,24 +7181,28 @@ static void bevel_build_edge_polygons(BMesh *bm, BevelParams *bp, BMEdge *bme) edges[2] = edges[3] = nullptr; f_interp = f2; } - r_f = bev_create_ngon(bm, verts, 4, nullptr, f_interp, edges, mat_nr, true); + r_f = bev_create_ngon( + bp, bm, verts, 4, nullptr, f_interp, edges, nullptr, &nv_bv_map, mat_nr, true); } else if (!odd && k == mid) { /* Left poly that touches an even center line on right. */ BMEdge *edges[4] = {nullptr, nullptr, bme, bme}; - r_f = bev_create_ngon(bm, verts, 4, nullptr, f1, edges, mat_nr, true); + r_f = bev_create_ngon( + bp, bm, verts, 4, nullptr, f1, edges, nullptr, &nv_bv_map, mat_nr, true); center_bme = BM_edge_exists(verts[2], verts[3]); BLI_assert(center_bme != nullptr); } else if (!odd && k == mid + 1) { /* Right poly that touches an even center line on left. */ BMEdge *edges[4] = {bme, bme, nullptr, nullptr}; - r_f = bev_create_ngon(bm, verts, 4, nullptr, f2, edges, mat_nr, true); + r_f = bev_create_ngon( + bp, bm, verts, 4, nullptr, f2, edges, nullptr, &nv_bv_map, mat_nr, true); } else { /* Doesn't cross or touch the center line, so interpolate in appropriate f1 or f2. */ BMFace *f = (k <= mid) ? f1 : f2; - r_f = bev_create_ngon(bm, verts, 4, nullptr, f, nullptr, mat_nr, true); + r_f = bev_create_ngon( + bp, bm, verts, 4, nullptr, f, nullptr, nullptr, &nv_bv_map, mat_nr, true); } record_face_kind(bp, r_f, F_EDGE); /* Tag the long edges: those out of verts[0] and verts[2]. */ @@ -7051,23 +7216,6 @@ static void bevel_build_edge_polygons(BMesh *bm, BevelParams *bp, BMEdge *bme) verts[0] = verts[3]; verts[1] = verts[2]; } - if (!odd) { - if (!e1->is_seam) { - bev_merge_edge_uvs(bm, center_bme, mesh_vert(vm1, i1, 0, mid)->v); - } - if (!e2->is_seam) { - bev_merge_edge_uvs(bm, center_bme, mesh_vert(vm2, i2, 0, mid)->v); - } - } - - /* Fix UVs along end edge joints. A NOP unless other side built already. */ - /* TODO: If some seam, may want to do selective merge. */ - if (!bv1->any_seam && bv1->vmesh->mesh_kind == M_NONE) { - bev_merge_end_uvs(bm, bv1, e1); - } - if (!bv2->any_seam && bv2->vmesh->mesh_kind == M_NONE) { - bev_merge_end_uvs(bm, bv2, e2); - } /* Copy edge data to first and last edge. */ BMEdge *bme1 = BM_edge_exists(bmv1, bmv2); @@ -7552,8 +7700,7 @@ static float geometry_collide_offset(BevelParams *bp, EdgeHalf *eb) * The intersection of these two corner vectors is the collapse point. * The length of edge B divided by the projection of these vectors onto edge B * is the number of 'offsets' that can be accommodated. */ - float offsets_projected_on_B = safe_divide(ka + cos1 * kb, sin1) + - safe_divide(kc + cos2 * kb, sin2); + float offsets_projected_on_B = (ka + cos1 * kb) / sin1 + (kc + cos2 * kb) / sin2; if (offsets_projected_on_B > BEVEL_EPSILON) { offsets_projected_on_B = bp->offset * (len_v3v3(vb->co, vc->co) / offsets_projected_on_B); if (offsets_projected_on_B > BEVEL_EPSILON) { @@ -7803,8 +7950,10 @@ void BM_mesh_bevel(BMesh *bm, bp.face_hash = BLI_ghash_ptr_new(__func__); BLI_ghash_flag_set(bp.face_hash, GHASH_FLAG_ALLOW_DUPES); + bp.uv_face_hash = BLI_ghash_ptr_new(__func__); math_layer_info_init(&bp, bm); + uv_vert_map_init(&bp, bm); /* Analyze input vertices, sorting edges and assigning initial new vertex positions. */ BM_ITER_MESH (v, &iter, bm, BM_VERTS_OF_MESH) { @@ -7813,6 +7962,7 @@ void BM_mesh_bevel(BMesh *bm, if (!limit_offset && bv) { build_boundary(&bp, bv, true); } + determine_uv_vert_connectivity(&bp, bm, v); } } @@ -7875,20 +8025,28 @@ void BM_mesh_bevel(BMesh *bm, } /* Rebuild face polygons around affected vertices. */ + Set rebuilt_orig_faces; BM_ITER_MESH (v, &iter, bm, BM_VERTS_OF_MESH) { if (BM_elem_flag_test(v, BM_ELEM_TAG)) { - bevel_rebuild_existing_polygons(bm, &bp, v); + bevel_rebuild_existing_polygons(bm, &bp, v, rebuilt_orig_faces); bevel_reattach_wires(bm, &bp, v); } } + for (BMFace *f : rebuilt_orig_faces) { + BM_face_kill(bm, f); + } + BM_ITER_MESH_MUTABLE (v, v_next, &iter, bm, BM_VERTS_OF_MESH) { if (BM_elem_flag_test(v, BM_ELEM_TAG)) { BLI_assert(find_bevvert(&bp, v) != nullptr); + uv_vert_map_pop(&bp, v); BM_vert_kill(bm, v); } } + bevel_merge_uvs(&bp, bm); + if (bp.harden_normals) { bevel_harden_normals(&bp, bm); } @@ -7924,6 +8082,7 @@ void BM_mesh_bevel(BMesh *bm, /* Primary free. */ BLI_ghash_free(bp.vert_hash, nullptr, nullptr); BLI_ghash_free(bp.face_hash, nullptr, nullptr); + BLI_ghash_free(bp.uv_face_hash, nullptr, nullptr); BLI_memarena_free(bp.mem_arena); #ifdef BEVEL_DEBUG_TIME diff --git a/tests/files/modeling/bevel_regression.blend b/tests/files/modeling/bevel_regression.blend index 21049322cc8..ea2740e9ca0 100644 --- a/tests/files/modeling/bevel_regression.blend +++ b/tests/files/modeling/bevel_regression.blend @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:a71b06b46de076d45e8d96679270e9a8bd1fc205044beba047d574387e60ee7d -size 264374 +oid sha256:e607207e05462f8256e3452d436977cfc271fb2ec7c5a4a9f489351d5e484950 +size 1857388 diff --git a/tests/files/modeling/modifiers.blend b/tests/files/modeling/modifiers.blend index 59b3888d828..5618e72314c 100644 --- a/tests/files/modeling/modifiers.blend +++ b/tests/files/modeling/modifiers.blend @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:441dab52843a8e94cac46bfec5dc35c3bca9d84b980872773903d807bfcd43f4 -size 2291591 +oid sha256:960ed934aebe7fe4a55cb36763e46c61693c1beba7b47794729ed3e421b784da +size 10244843 diff --git a/tests/python/bevel_operator.py b/tests/python/bevel_operator.py index 98edd2989fe..311e0478d76 100644 --- a/tests/python/bevel_operator.py +++ b/tests/python/bevel_operator.py @@ -52,7 +52,7 @@ def main(): SpecMeshTest('Cube_test_11', 'Cube_test', 'Cube_result_11', [OperatorSpecEditMode('bevel', {'offset': 0.4, 'segments': 5, 'profile': 1.0}, 'EDGE', {8, 10, 7}, )]), - SpecMeshTest("test 12", 'Cube_test', 'Cube_result_12', + SpecMeshTest("Cube_test_12", 'Cube_test', 'Cube_result_12', [OperatorSpecEditMode('bevel', {'offset': 0.4, 'segments': 8}, 'EDGE', {0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11}, )]), @@ -217,7 +217,7 @@ def main(): SpecMeshTest('Tray', 'Tray', 'Tray_result_1', [OperatorSpecEditMode('bevel', {'offset': 0.01, 'segments': 2}, 'EDGE', {0, 1, 6, 7, 12, 14, 16, 17}, )]), - SpecMeshTest("test 73", 'Bumptop', 'Bumptop_result_1', + SpecMeshTest("Bumptop", 'Bumptop', 'Bumptop_result_1', [OperatorSpecEditMode('bevel', {'offset': 0.1, 'segments': 4}, 'EDGE', {33, 4, 38, 8, 41, 10, 42, 12, 14, 17, 24, 31}, )]), @@ -228,7 +228,7 @@ def main(): [OperatorSpecEditMode('bevel', {'offset': 0.05, 'segments': 2}, 'EDGE', {19, 20, 23, 15}, )]), # 75 - SpecMeshTest("test 77", 'Cube_hn_test', 'Cube_hn_result_1', + SpecMeshTest("Cube_hn_test", 'Cube_hn_test', 'Cube_hn_result_1', [OperatorSpecEditMode('bevel', {'offset': 0.2, 'harden_normals': True}, 'EDGE', {8}, )]), SpecMeshTest('Blocksteps_test_1', 'Blocksteps_test', 'Blocksteps_result_1', [OperatorSpecEditMode('bevel',