Changeset View
Standalone View
source/blender/modifiers/intern/MOD_weld.c
| Show First 20 Lines • Show All 377 Lines • ▼ Show 20 Lines | static void weld_vert_ctx_alloc_and_setup(const uint mvert_len, | ||||
| /* Vert Context. */ | /* Vert Context. */ | ||||
| uint wvert_len = 0; | uint wvert_len = 0; | ||||
| WeldVert *wvert, *wv; | WeldVert *wvert, *wv; | ||||
| wvert = MEM_mallocN(sizeof(*wvert) * mvert_len, __func__); | wvert = MEM_mallocN(sizeof(*wvert) * mvert_len, __func__); | ||||
| wv = &wvert[0]; | wv = &wvert[0]; | ||||
| uint *v_dest_iter = &r_vert_dest_map[0]; | uint *v_dest_iter = &r_vert_dest_map[0]; | ||||
| for (uint i = 0; i < mvert_len; i++, v_dest_iter++) { | for (uint i = 0; i < mvert_len; i++, v_dest_iter++) { | ||||
mano-wii: Is it really necessary to allocate these arrays `mvert` and `combined_verts`?
Apparently the… | |||||
Done Inline ActionsThis came from complex solidify. Originally it didn't have that, but the chain merging was quite severe on meshes with ordered vertices like text. Honestly it is also quite bad with the usual weld modifier. D7224 changed that in solidify and I want to keep it like that. If you can think of a different way of getting rid of that chaining with a good result, propose it, but I found this (even though it is order dependend) is fairly good in practice and the order dependency does usually not become a problem (See T75032#896741 for the things I thought of how to solve this). Also the calculated positions could potentially be used as final positions in the future, removing some of the extra overhead. What I am more concerned with here is, that with very high merging thresholds various spikes appear. I know the reason for it, but it's not trivial to fix. weasel: This came from complex solidify. Originally it didn't have that, but the chain merging was… | |||||
| if (*v_dest_iter != OUT_OF_CONTEXT) { | if (*v_dest_iter != OUT_OF_CONTEXT) { | ||||
| wv->vert_dest = *v_dest_iter; | wv->vert_dest = *v_dest_iter; | ||||
| wv->vert_orig = i; | wv->vert_orig = i; | ||||
| wv++; | wv++; | ||||
| wvert_len++; | wvert_len++; | ||||
| } | } | ||||
Not Done Inline ActionsI'm not sure if "weld_fix_vert_map" is an appropriate name mano-wii: I'm not sure if `"weld_fix_vert_map"` is an appropriate name
A comment here would be useful to… | |||||
Done Inline ActionsAgreed. Just for reference here: The vert_dest_map prior to this function is supposed to converge to the correct value when iterated (for fixpoints). This function will finalize this map by doing the iteration for every vert and writing the result. It also sets all verts that didn't get merged to OUT_OF_CONTEXT. weasel: Agreed. Just for reference here: The `vert_dest_map` prior to this function is supposed to… | |||||
Done Inline ActionsRemove or change this unused parameter to uint UNUSED(v_mask_act). mano-wii: Remove or change this unused parameter to `uint UNUSED(v_mask_act)`.
This avoids warnings on… | |||||
| } | } | ||||
| *r_wvert = MEM_reallocN(wvert, sizeof(*wvert) * wvert_len); | *r_wvert = MEM_reallocN(wvert, sizeof(*wvert) * wvert_len); | ||||
| *r_wvert_len = wvert_len; | *r_wvert_len = wvert_len; | ||||
| } | } | ||||
| static void weld_vert_groups_setup(const uint mvert_len, | static void weld_vert_groups_setup(const uint mvert_len, | ||||
| const uint wvert_len, | const uint wvert_len, | ||||
| Show All 15 Lines | if (vert_dest != OUT_OF_CONTEXT) { | ||||
| *group_map_iter = ELEM_MERGED; | *group_map_iter = ELEM_MERGED; | ||||
| } | } | ||||
| else { | else { | ||||
| *group_map_iter = wgroups_len; | *group_map_iter = wgroups_len; | ||||
| wgroups_len++; | wgroups_len++; | ||||
| } | } | ||||
| } | } | ||||
| else { | else { | ||||
| *group_map_iter = OUT_OF_CONTEXT; | *group_map_iter = OUT_OF_CONTEXT; | ||||
Done Inline ActionsFrom what I can see here. Neither 'medge' nor 'totegde' are being used. mano-wii: From what I can see here. Neither 'medge' nor 'totegde' are being used. | |||||
| } | } | ||||
| } | } | ||||
| struct WeldGroup *wgroups = MEM_callocN(sizeof(*wgroups) * wgroups_len, __func__); | struct WeldGroup *wgroups = MEM_callocN(sizeof(*wgroups) * wgroups_len, __func__); | ||||
| const WeldVert *wv = &wvert[0]; | const WeldVert *wv = &wvert[0]; | ||||
| for (uint i = wvert_len; i--; wv++) { | for (uint i = wvert_len; i--; wv++) { | ||||
| uint group_index = r_vert_groups_map[wv->vert_dest]; | uint group_index = r_vert_groups_map[wv->vert_dest]; | ||||
| ▲ Show 20 Lines • Show All 952 Lines • ▼ Show 20 Lines | #endif | ||||
| MEM_freeN(wvert); | MEM_freeN(wvert); | ||||
| MEM_freeN(edge_ctx_map); | MEM_freeN(edge_ctx_map); | ||||
| MEM_freeN(wedge); | MEM_freeN(wedge); | ||||
| } | } | ||||
| static void weld_mesh_context_free(WeldMesh *weld_mesh) | static void weld_mesh_context_free(WeldMesh *weld_mesh) | ||||
| { | { | ||||
| MEM_freeN(weld_mesh->vert_groups); | MEM_freeN(weld_mesh->vert_groups); | ||||
| MEM_freeN(weld_mesh->vert_groups_buffer); | MEM_freeN(weld_mesh->vert_groups_buffer); | ||||
Not Done Inline ActionsWhy was this line removed? mano-wii: Why was this line removed? | |||||
Done Inline ActionsAs weld_mesh should be setup in weld_mesh_context_create I removed vert_groups_map from it and made it an argument to the functions that need it. The problem is that this free will not be called if no vertices where merged, but the memory would still have been allocated. So I wanted both cases in one and put it at the bottom of doWeld. I think of vert_dest_map as the backbone of the whole modifier, because it is used all throughout. It should not be part of weld_mesh, which is only used for one (conditional) step of the modifier. weasel: As `weld_mesh` should be setup in `weld_mesh_context_create` I removed `vert_groups_map` from… | |||||
| MEM_freeN(weld_mesh->edge_groups); | MEM_freeN(weld_mesh->edge_groups); | ||||
| MEM_freeN(weld_mesh->edge_groups_buffer); | MEM_freeN(weld_mesh->edge_groups_buffer); | ||||
| MEM_freeN(weld_mesh->edge_groups_map); | MEM_freeN(weld_mesh->edge_groups_map); | ||||
| MEM_freeN(weld_mesh->wloop); | MEM_freeN(weld_mesh->wloop); | ||||
| MEM_freeN(weld_mesh->wpoly); | MEM_freeN(weld_mesh->wpoly); | ||||
| MEM_freeN(weld_mesh->loop_map); | MEM_freeN(weld_mesh->loop_map); | ||||
| MEM_freeN(weld_mesh->poly_map); | MEM_freeN(weld_mesh->poly_map); | ||||
| ▲ Show 20 Lines • Show All 159 Lines • ▼ Show 20 Lines | if (index_a < index_b) { | ||||
| const float dist_sq = len_squared_v3v3(mvert[index_a].co, mvert[index_b].co); | const float dist_sq = len_squared_v3v3(mvert[index_a].co, mvert[index_b].co); | ||||
| BLI_assert(dist_sq <= ((data->merge_dist_sq + FLT_EPSILON) * 3)); | BLI_assert(dist_sq <= ((data->merge_dist_sq + FLT_EPSILON) * 3)); | ||||
| return dist_sq <= data->merge_dist_sq; | return dist_sq <= data->merge_dist_sq; | ||||
| } | } | ||||
| return false; | return false; | ||||
| } | } | ||||
| #endif | #endif | ||||
| struct WeldVertexCluster { | |||||
| float co[3]; | |||||
| uint merged_verts; | |||||
| uint edge_other_vert[2]; | |||||
| uint edge_users; | |||||
| }; | |||||
| static Mesh *weldModifier_doWeld(WeldModifierData *wmd, const ModifierEvalContext *ctx, Mesh *mesh) | static Mesh *weldModifier_doWeld(WeldModifierData *wmd, const ModifierEvalContext *ctx, Mesh *mesh) | ||||
| { | { | ||||
| Mesh *result = mesh; | Mesh *result = mesh; | ||||
| Object *ob = ctx->object; | Object *ob = ctx->object; | ||||
| BLI_bitmap *v_mask = NULL; | BLI_bitmap *v_mask = NULL; | ||||
| int v_mask_act = 0; | int v_mask_act = 0; | ||||
| Show All 23 Lines | if (dvert) { | ||||
| } | } | ||||
| } | } | ||||
| } | } | ||||
| /* From the original index of the vertex. | /* From the original index of the vertex. | ||||
| * This indicates which vert it is or is going to be merged. */ | * This indicates which vert it is or is going to be merged. */ | ||||
| uint *vert_dest_map = MEM_malloc_arrayN(totvert, sizeof(*vert_dest_map), __func__); | uint *vert_dest_map = MEM_malloc_arrayN(totvert, sizeof(*vert_dest_map), __func__); | ||||
| uint vert_kill_len = 0; | uint vert_kill_len = 0; | ||||
| if (wmd->mode == MOD_WELD_ALL_MODE) | |||||
| #ifdef USE_BVHTREEKDOP | #ifdef USE_BVHTREEKDOP | ||||
| { | { | ||||
| /* Get overlap map. */ | /* Get overlap map. */ | ||||
| struct BVHTreeFromMesh treedata; | struct BVHTreeFromMesh treedata; | ||||
| BVHTree *bvhtree = bvhtree_from_mesh_verts_ex(&treedata, | BVHTree *bvhtree = bvhtree_from_mesh_verts_ex(&treedata, | ||||
| mvert, | mvert, | ||||
| totvert, | totvert, | ||||
| false, | false, | ||||
| ▲ Show 20 Lines • Show All 77 Lines • ▼ Show 20 Lines | for (uint i = 0; i < totvert; i++) { | ||||
| } | } | ||||
| vert_dest_map[i] = OUT_OF_CONTEXT; | vert_dest_map[i] = OUT_OF_CONTEXT; | ||||
| } | } | ||||
| BLI_kdtree_3d_balance(tree); | BLI_kdtree_3d_balance(tree); | ||||
| vert_kill_len = BLI_kdtree_3d_calc_duplicates_fast( | vert_kill_len = BLI_kdtree_3d_calc_duplicates_fast( | ||||
| tree, wmd->merge_dist, true, (int *)vert_dest_map); | tree, wmd->merge_dist, true, (int *)vert_dest_map); | ||||
| BLI_kdtree_3d_free(tree); | BLI_kdtree_3d_free(tree); | ||||
| } | } | ||||
Done Inline ActionsHere instead of a comment, you can use a BLI_assert(MOD_WELD_ALL_MODE). mano-wii: Here instead of a comment, you can use a `BLI_assert(MOD_WELD_ALL_MODE)`.
Thus, in addition to… | |||||
| #endif | #endif | ||||
| else { | |||||
| BLI_assert(wmd->mode == MOD_WELD_CONNECTED_MODE); | |||||
| MEdge *medge, *me; | |||||
| float edgedir[3]; | |||||
| medge = mesh->medge; | |||||
| totvert = mesh->totvert; | |||||
| totedge = mesh->totedge; | |||||
| struct WeldVertexCluster *vert_clusters = MEM_calloc_arrayN(totvert, sizeof(*vert_clusters), __func__); | |||||
| struct WeldVertexCluster *vc = &vert_clusters[0]; | |||||
| for (uint i = 0; i < totvert; i++, vc++) { | |||||
| copy_v3_v3(vc->co, mvert[i].co); | |||||
| } | |||||
| float merge_dist_sq = square_f(wmd->merge_dist); | |||||
| range_vn_u(vert_dest_map, totvert, 0); | |||||
| /* Collapse Edges that are shorter than the threshold. */ | |||||
| me = &medge[0]; | |||||
| for (uint i = 0; i < totedge; i++, me++) { | |||||
| uint v1 = me->v1; | |||||
| uint v2 = me->v2; | |||||
| while (v1 != vert_dest_map[v1]) { | |||||
| v1 = vert_dest_map[v1]; | |||||
| } | |||||
| while (v2 != vert_dest_map[v2]) { | |||||
| v2 = vert_dest_map[v2]; | |||||
| } | |||||
| if (v1 == v2) { | |||||
| continue; | |||||
| } | |||||
| if (v_mask && (!BLI_BITMAP_TEST(v_mask, v1) || !BLI_BITMAP_TEST(v_mask, v2))) { | |||||
| continue; | |||||
| } | |||||
| if (v1 > v2) { | |||||
| SWAP(uint, v1, v2); | |||||
| } | |||||
| struct WeldVertexCluster *v1_cluster = &vert_clusters[v1]; | |||||
| struct WeldVertexCluster *v2_cluster = &vert_clusters[v2]; | |||||
| sub_v3_v3v3(edgedir, v2_cluster->co, v1_cluster->co); | |||||
| const float dist_sq = len_squared_v3(edgedir); | |||||
| if (dist_sq <= merge_dist_sq) { | |||||
| float influence = (v2_cluster->merged_verts + 1) / | |||||
| (float)(v1_cluster->merged_verts + v2_cluster->merged_verts + 2); | |||||
| madd_v3_v3fl(v1_cluster->co, edgedir, influence); | |||||
| v1_cluster->merged_verts += v2_cluster->merged_verts + 1; | |||||
| vert_dest_map[v2] = v1; | |||||
| vert_kill_len++; | |||||
| } | |||||
| } | |||||
| MEM_freeN(vert_clusters); | |||||
| for (uint i = 0; i < totvert; i++) { | |||||
| if (i == vert_dest_map[i]) { | |||||
| vert_dest_map[i] = OUT_OF_CONTEXT; | |||||
| } | |||||
| else { | |||||
| uint v = i; | |||||
| while (v != vert_dest_map[v] && vert_dest_map[v] != OUT_OF_CONTEXT) { | |||||
| v = vert_dest_map[v]; | |||||
| } | |||||
| vert_dest_map[v] = v; | |||||
| vert_dest_map[i] = v; | |||||
| } | |||||
| } | |||||
| } | |||||
| if (v_mask) { | if (v_mask) { | ||||
| MEM_freeN(v_mask); | MEM_freeN(v_mask); | ||||
| } | } | ||||
| if (vert_kill_len) { | if (vert_kill_len) { | ||||
| WeldMesh weld_mesh; | WeldMesh weld_mesh; | ||||
| weld_mesh_context_create(mesh, vert_dest_map, vert_kill_len, &weld_mesh); | weld_mesh_context_create(mesh, vert_dest_map, vert_kill_len, &weld_mesh); | ||||
| ▲ Show 20 Lines • Show All 197 Lines • ▼ Show 20 Lines | static Mesh *modifyMesh(ModifierData *md, const ModifierEvalContext *ctx, Mesh *mesh) | ||||
| WeldModifierData *wmd = (WeldModifierData *)md; | WeldModifierData *wmd = (WeldModifierData *)md; | ||||
| return weldModifier_doWeld(wmd, ctx, mesh); | return weldModifier_doWeld(wmd, ctx, mesh); | ||||
| } | } | ||||
| static void initData(ModifierData *md) | static void initData(ModifierData *md) | ||||
| { | { | ||||
| WeldModifierData *wmd = (WeldModifierData *)md; | WeldModifierData *wmd = (WeldModifierData *)md; | ||||
| BLI_assert(MEMCMP_STRUCT_AFTER_IS_ZERO(wmd, modifier)); | BLI_assert(MEMCMP_STRUCT_AFTER_IS_ZERO(wmd, modifier)); | ||||
Done Inline ActionsIt will probably be necessary to create a versioning (in ...\source\blender\blenloader\intern\versioning_290.c) to adapt old files. mano-wii: It will probably be necessary to create a versioning (in `... | |||||
Done Inline ActionsI dont think so. I made sure current "Full" Mode is mode=0 so when old files get loaded it will automatically choose Full (AFAIK and tested) weasel: I dont think so. I made sure current "Full" Mode is mode=0 so when old files get loaded it will… | |||||
| MEMCPY_STRUCT_AFTER(wmd, DNA_struct_default_get(WeldModifierData), modifier); | MEMCPY_STRUCT_AFTER(wmd, DNA_struct_default_get(WeldModifierData), modifier); | ||||
| } | } | ||||
| static void requiredDataMask(Object *UNUSED(ob), | static void requiredDataMask(Object *UNUSED(ob), | ||||
| ModifierData *md, | ModifierData *md, | ||||
| CustomData_MeshMasks *r_cddata_masks) | CustomData_MeshMasks *r_cddata_masks) | ||||
| { | { | ||||
| Show All 9 Lines | |||||
| { | { | ||||
| uiLayout *layout = panel->layout; | uiLayout *layout = panel->layout; | ||||
| PointerRNA ob_ptr; | PointerRNA ob_ptr; | ||||
| PointerRNA *ptr = modifier_panel_get_property_pointers(panel, &ob_ptr); | PointerRNA *ptr = modifier_panel_get_property_pointers(panel, &ob_ptr); | ||||
| uiLayoutSetPropSep(layout, true); | uiLayoutSetPropSep(layout, true); | ||||
| uiItemR(layout, ptr, "mode", 0, NULL, ICON_NONE); | |||||
| uiItemR(layout, ptr, "merge_threshold", 0, IFACE_("Distance"), ICON_NONE); | uiItemR(layout, ptr, "merge_threshold", 0, IFACE_("Distance"), ICON_NONE); | ||||
| modifier_vgroup_ui(layout, ptr, &ob_ptr, "vertex_group", "invert_vertex_group", NULL); | modifier_vgroup_ui(layout, ptr, &ob_ptr, "vertex_group", "invert_vertex_group", NULL); | ||||
| modifier_panel_end(layout, ptr); | modifier_panel_end(layout, ptr); | ||||
| } | } | ||||
| static void panelRegister(ARegionType *region_type) | static void panelRegister(ARegionType *region_type) | ||||
| { | { | ||||
| ▲ Show 20 Lines • Show All 41 Lines • Show Last 20 Lines | |||||
Is it really necessary to allocate these arrays mvert and combined_verts?
Apparently the idea is to get the average coordinate value.
This does not seem to be a good solution as the result may depend on the order in which the vertices are tested.
Also allocating and reading values on different arrays can be inefficient and memory consuming.
Isn't the idea here just to detect the edges that collapse?
That being the case, at first glance I don't really see a good reason for these arrays.