From 78fc5ea1c398f70d22cda72be33c105146c0d542 Mon Sep 17 00:00:00 2001 From: Campbell Barton Date: Thu, 5 May 2022 20:36:15 +1000 Subject: [PATCH 1/2] Workaround T81065: Merge UV's when applying modifiers Support merging UV's that share the same vertex and are very close when applying modifiers. This is needed to prevent UV's becoming "detached" which can happen when applying the subdivision surface modifier. This regression was caused by [0] which removed selection threshold for nearby coordinates. While restoring the UV selection threshold could be done - some selection operations that walk around connected UV fans wouldn't behave in a deterministic way (such as select shortest path). There are also other cases where UV's may be compared without a threshold such as tangent calculation and exporters which have their own logic to handling UV's. Also resolves T86896, T89903. [0]: b88dd3b8e7b9c02ae08d4679bb427963c5d21250 Reviewed By: sergey Ref D14841 --- source/blender/blenkernel/BKE_mesh.h | 8 ++++++ source/blender/blenkernel/CMakeLists.txt | 1 + source/blender/editors/object/object_add.cc | 18 ++++++++++++- .../blender/editors/object/object_modifier.cc | 27 +++++++++++++++++++ 4 files changed, 53 insertions(+), 1 deletion(-) diff --git a/source/blender/blenkernel/BKE_mesh.h b/source/blender/blenkernel/BKE_mesh.h index bd9bacb52c1..091f30825ae 100644 --- a/source/blender/blenkernel/BKE_mesh.h +++ b/source/blender/blenkernel/BKE_mesh.h @@ -893,6 +893,14 @@ struct Mesh *BKE_mesh_merge_verts(struct Mesh *mesh, int tot_vtargetmap, int merge_mode); +/** + * Account for custom-data such as UV's becoming detached because of of imprecision + * in custom-data interpolation. + * Without running this operation subdivision surface can cause UV's to be disconnected, + * see: T81065. + */ +void BKE_mesh_merge_customdata_for_apply_modifier(struct Mesh *me); + /* Flush flags. */ /** diff --git a/source/blender/blenkernel/CMakeLists.txt b/source/blender/blenkernel/CMakeLists.txt index 710e2900d78..a57d4d0a2bf 100644 --- a/source/blender/blenkernel/CMakeLists.txt +++ b/source/blender/blenkernel/CMakeLists.txt @@ -199,6 +199,7 @@ set(SRC intern/mesh_iterators.c intern/mesh_mapping.c intern/mesh_merge.c + intern/mesh_merge_customdata.cc intern/mesh_mirror.c intern/mesh_normals.cc intern/mesh_remap.c diff --git a/source/blender/editors/object/object_add.cc b/source/blender/editors/object/object_add.cc index 5b857d2dba1..db8860efdd8 100644 --- a/source/blender/editors/object/object_add.cc +++ b/source/blender/editors/object/object_add.cc @@ -2894,6 +2894,7 @@ static int object_convert_exec(bContext *C, wmOperator *op) Object *ob1, *obact = CTX_data_active_object(C); const short target = RNA_enum_get(op->ptr, "target"); bool keep_original = RNA_boolean_get(op->ptr, "keep_original"); + const bool do_merge_customdata = RNA_boolean_get(op->ptr, "merge_customdata"); const float angle = RNA_float_get(op->ptr, "angle"); const int thickness = RNA_int_get(op->ptr, "thickness"); @@ -3122,6 +3123,10 @@ static int object_convert_exec(bContext *C, wmOperator *op) Mesh *new_mesh = (Mesh *)newob->data; BKE_mesh_nomain_to_mesh(me_eval, new_mesh, newob, &CD_MASK_MESH, true); + if (do_merge_customdata) { + BKE_mesh_merge_customdata_for_apply_modifier(new_mesh); + } + /* Anonymous attributes shouldn't be available on the applied geometry. */ MeshComponent component; component.replace(new_mesh, GeometryOwnershipType::Editable); @@ -3441,7 +3446,11 @@ static void object_convert_ui(bContext *UNUSED(C), wmOperator *op) uiItemR(layout, op->ptr, "target", 0, nullptr, ICON_NONE); uiItemR(layout, op->ptr, "keep_original", 0, nullptr, ICON_NONE); - if (RNA_enum_get(op->ptr, "target") == OB_GPENCIL) { + const int target = RNA_enum_get(op->ptr, "target"); + if (target == OB_MESH) { + uiItemR(layout, op->ptr, "merge_customdata", 0, nullptr, ICON_NONE); + } + else if (target == OB_GPENCIL) { uiItemR(layout, op->ptr, "thickness", 0, nullptr, ICON_NONE); uiItemR(layout, op->ptr, "angle", 0, nullptr, ICON_NONE); uiItemR(layout, op->ptr, "offset", 0, nullptr, ICON_NONE); @@ -3477,6 +3486,13 @@ void OBJECT_OT_convert(wmOperatorType *ot) "Keep Original", "Keep original objects instead of replacing them"); + RNA_def_boolean( + ot->srna, + "merge_customdata", + true, + "Merge UV's", + "Merge UV coordinates that share a vertex to account for imprecision in some modifiers"); + prop = RNA_def_float_rotation(ot->srna, "angle", 0, diff --git a/source/blender/editors/object/object_modifier.cc b/source/blender/editors/object/object_modifier.cc index f90a31a7cbe..3328fe65f2e 100644 --- a/source/blender/editors/object/object_modifier.cc +++ b/source/blender/editors/object/object_modifier.cc @@ -1434,8 +1434,10 @@ static int modifier_apply_exec_ex(bContext *C, wmOperator *op, int apply_as, boo Scene *scene = CTX_data_scene(C); Object *ob = ED_object_active_context(C); ModifierData *md = edit_modifier_property_get(op, ob, 0); + const ModifierTypeInfo *mti = BKE_modifier_get_info((ModifierType)md->type); const bool do_report = RNA_boolean_get(op->ptr, "report"); const bool do_single_user = RNA_boolean_get(op->ptr, "single_user"); + const bool do_merge_customdata = RNA_boolean_get(op->ptr, "merge_customdata"); if (md == nullptr) { return OPERATOR_CANCELLED; @@ -1460,6 +1462,11 @@ static int modifier_apply_exec_ex(bContext *C, wmOperator *op, int apply_as, boo return OPERATOR_CANCELLED; } + if (do_merge_customdata && + (mti->type & (eModifierTypeType_Constructive | eModifierTypeType_Nonconstructive))) { + BKE_mesh_merge_customdata_for_apply_modifier((Mesh *)ob->data); + } + DEG_id_tag_update(&ob->id, ID_RECALC_GEOMETRY); DEG_relations_tag_update(bmain); WM_event_add_notifier(C, NC_OBJECT | ND_MODIFIER, ob); @@ -1518,6 +1525,12 @@ void OBJECT_OT_modifier_apply(wmOperatorType *ot) edit_modifier_properties(ot); edit_modifier_report_property(ot); + RNA_def_boolean( + ot->srna, + "merge_customdata", + true, + "Merge UV's", + "Merge UV coordinates that share a vertex to account for imprecision in some modifiers"); PropertyRNA *prop = RNA_def_boolean(ot->srna, "single_user", false, @@ -1599,11 +1612,18 @@ static int modifier_convert_exec(bContext *C, wmOperator *op) ViewLayer *view_layer = CTX_data_view_layer(C); Object *ob = ED_object_active_context(C); ModifierData *md = edit_modifier_property_get(op, ob, 0); + const ModifierTypeInfo *mti = BKE_modifier_get_info((ModifierType)md->type); + const bool do_merge_customdata = RNA_boolean_get(op->ptr, "merge_customdata"); if (!md || !ED_object_modifier_convert(op->reports, bmain, depsgraph, view_layer, ob, md)) { return OPERATOR_CANCELLED; } + if (do_merge_customdata && + (mti->type & (eModifierTypeType_Constructive | eModifierTypeType_Nonconstructive))) { + BKE_mesh_merge_customdata_for_apply_modifier((Mesh *)ob->data); + } + DEG_id_tag_update(&ob->id, ID_RECALC_GEOMETRY); WM_event_add_notifier(C, NC_OBJECT | ND_MODIFIER, ob); @@ -1631,6 +1651,13 @@ void OBJECT_OT_modifier_convert(wmOperatorType *ot) /* flags */ ot->flag = OPTYPE_REGISTER | OPTYPE_UNDO | OPTYPE_INTERNAL; edit_modifier_properties(ot); + + RNA_def_boolean( + ot->srna, + "merge_customdata", + true, + "Merge UV's", + "Merge UV coordinates that share a vertex to account for imprecision in some modifiers"); } /** \} */ From 756710800c0fd8cc2919ed338a2a897aace535d5 Mon Sep 17 00:00:00 2001 From: Campbell Barton Date: Thu, 5 May 2022 20:40:56 +1000 Subject: [PATCH 2/2] File missing from 78fc5ea1c398f70d22cda72be33c105146c0d542 --- .../intern/mesh_merge_customdata.cc | 137 ++++++++++++++++++ 1 file changed, 137 insertions(+) create mode 100644 source/blender/blenkernel/intern/mesh_merge_customdata.cc diff --git a/source/blender/blenkernel/intern/mesh_merge_customdata.cc b/source/blender/blenkernel/intern/mesh_merge_customdata.cc new file mode 100644 index 00000000000..adaf378ed27 --- /dev/null +++ b/source/blender/blenkernel/intern/mesh_merge_customdata.cc @@ -0,0 +1,137 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ + +/** \file + * \ingroup bke + */ + +#include "MEM_guardedalloc.h" + +#include "DNA_mesh_types.h" +#include "DNA_meshdata_types.h" + +#include "BLI_math.h" +#include "BLI_task.hh" +#include "BLI_utildefines.h" + +#include "BKE_customdata.h" +#include "BKE_mesh.h" +#include "BKE_mesh_mapping.h" +#include "BLI_memarena.h" + +#include "BLI_strict_flags.h" + +using namespace blender; + +enum { + CMP_CLOSE = 0, + CMP_EQUAL = 1, + CMP_APART = 2, +}; + +static int compare_v2_classify(const float uv_a[2], const float uv_b[2]) +{ + if (uv_a[0] == uv_b[0] && uv_a[1] == uv_b[1]) { + return CMP_EQUAL; + } + /* Note that the ULP value is the primary value used to compare relative values + * as the absolute value doesn't account for float precision at difference scales. + * - For subdivision-surface ULP of 3 is sufficient, + * although this value is extremely small. + * - For bevel the URL of 12 is sufficient to merge UV's that appear to be connected + * with bevel on Suzanne beveled 15% with 6 segments. + * + * These values could be tweaked but should be kept on the small side to prevent + * unintentional joining of intentionally dis-connected UV's. + * + * Before v2.91 the threshold was either (`1e-4` or `0.05 / image_size` for selection picking). + * So picking used a threshold of `1e-4` for a 500x500 image and `1e-5` for a 5000x5000 image. + * Given this value worked reasonably well for a long time, the absolute difference should + * never exceed `1e-4` (#STD_UV_CONNECT_LIMIT which is still used in a few areas). */ + const float diff_abs = 1e-12f; + const int diff_ulp = 12; + + if (compare_ff_relative(uv_a[0], uv_b[0], diff_abs, diff_ulp) && + compare_ff_relative(uv_a[1], uv_b[1], diff_abs, diff_ulp)) { + return CMP_CLOSE; + } + return CMP_APART; +} + +static void merge_uvs_for_vertex(const Span loops_for_vert, Span mloopuv_layers) +{ + if (loops_for_vert.size() <= 1) { + return; + } + /* Manipulate a copy of the loop indices, de-duplicating UV's per layer. */ + Vector loops_merge; + loops_merge.reserve(loops_for_vert.size()); + for (MLoopUV *mloopuv : mloopuv_layers) { + BLI_assert(loops_merge.is_empty()); + loops_merge.extend_unchecked(loops_for_vert); + while (loops_merge.size() > 1) { + uint i_last = (uint)loops_merge.size() - 1; + const float *uv_src = mloopuv[loops_merge[0]].uv; + for (uint i = 1; i <= i_last;) { + float *uv_dst = mloopuv[loops_merge[i]].uv; + switch (compare_v2_classify(uv_src, uv_dst)) { + case CMP_CLOSE: { + uv_dst[0] = uv_src[0]; + uv_dst[1] = uv_src[1]; + ATTR_FALLTHROUGH; + } + case CMP_EQUAL: { + loops_merge[i] = loops_merge[i_last--]; + break; + } + case CMP_APART: { + /* Doesn't match, check the next UV. */ + i++; + break; + } + default: { + BLI_assert_unreachable(); + } + } + } + /* Finished de-duplicating with the first index, throw it away. */ + loops_merge[0] = loops_merge[i_last]; + loops_merge.resize(i_last); + } + loops_merge.clear(); + } +} + +void BKE_mesh_merge_customdata_for_apply_modifier(Mesh *me) +{ + if (me->totloop == 0) { + return; + } + const int mloopuv_layers_num = CustomData_number_of_layers(&me->ldata, CD_MLOOPUV); + if (mloopuv_layers_num == 0) { + return; + } + + int *vert_map_mem; + struct MeshElemMap *vert_to_loop; + BKE_mesh_vert_loop_map_create( + &vert_to_loop, &vert_map_mem, me->mpoly, me->mloop, me->totvert, me->totpoly, me->totloop); + + Vector mloopuv_layers; + mloopuv_layers.reserve(mloopuv_layers_num); + for (int a = 0; a < mloopuv_layers_num; a++) { + MLoopUV *mloopuv = static_cast(CustomData_get_layer_n(&me->ldata, CD_MLOOPUV, a)); + mloopuv_layers.append_unchecked(mloopuv); + } + + Span mloopuv_layers_as_span = mloopuv_layers.as_span(); + threading::parallel_for(IndexRange(me->totvert), 1024, [&](IndexRange range) { + for (const int64_t v_index : range) { + MeshElemMap &loops_for_vert = vert_to_loop[v_index]; + Span loops_for_vert_span(loops_for_vert.indices, loops_for_vert.count); + merge_uvs_for_vertex(loops_for_vert_span, mloopuv_layers_as_span); + } + }); + + MEM_freeN(vert_to_loop); + MEM_freeN(vert_map_mem); +}