Page MenuHome

Edit Mesh: `AutoMerge` with option `Split Edges`
ClosedPublic

Authored by Germano Cavalcante (mano-wii) on Thu, Aug 22, 11:55 PM.

Diff Detail

Repository
rB Blender

Event Timeline

  • Cleanup and Last Fixes
  • Fix random crash (Use BM_mesh_elem_toolflags_ensure)
  • Fix assert in BM_edge_exists
  • Use BVHtree to optimize the search for intersections.

Tests were done using overlap between two BVHTrees (Edges and Vertices) and with a new function called BLI_bvhtree_find_duplicate_fast.

The results were similar but with a slight advantage to the find_duplicate_fast solution.

The tests were done on a 2x subdivided suzanne and a dense high poly mesh.

Here are the results:
t1: Default AutoMerge Time (No Split);
t2: AutoMerge + Split time;
fac: t1 / t2;

1 BVHTree Solution:t1t2fac
Split all suzanne edges:0.0181400.0568980.318820
Move away suzanne verts:0.0171290.0200900.852576
Merge single suzanne vert:0.0175310.0142271.232292
Split single suzanne edge:0.0174060.0191800.907499
Move away verts of high poly mesh:0.2804410.3070280.913406

For reference here are values without BVHTree:

Loop over all combinations (O^2)t1t2fac
Split all suzanne edges:0.0184730.2170000.085130
Move away suzanne verts:0.0179980.1650780.109026
Merge single suzanne vert:0.0177770.0145941.218125
Split single suzanne edge:0.0174570.0153891.134390
Move away verts of high poly mesh:0.28240729.0293630.009728
Campbell Barton (campbellbarton) requested changes to this revision.Mon, Aug 26, 3:53 AM
Campbell Barton (campbellbarton) added inline comments.
source/blender/blenlib/intern/BLI_kdopbvh.c
1526

Assuming this is called fast because it optimizes for speed over quality.

This function should document the trade off's it makes.

eg:

https://developer.blender.org/diffusion/B/browse/master/source/blender/blenlib/intern/BLI_kdtree.c;459d76ec5106a949f85c121a3d977a65af560f4c$738-755

source/blender/editors/mesh/editmesh_select.c
204–206

Could use BM_vert_pair_share_face_check_cb.

207–210

This isn't checking if the face is hidden.

376

Prefer to avoid adding numbers onto variable names and instead name them in a way that tells us how it's different. eg: cuts_iter_other or cuts_iter_adjacent.

449

Single .

This revision now requires changes to proceed.Mon, Aug 26, 3:53 AM
Germano Cavalcante (mano-wii) marked 2 inline comments as done.EditedMon, Aug 26, 5:46 AM
Germano Cavalcante (mano-wii) updated this revision to Diff 17509.
  • Use BLI_qsort_r
  • Cleanup: Rename functions
  • Check if face is hidden
  • Document the trade off of the new BVHTree function
Germano Cavalcante (mano-wii) marked 2 inline comments as done.Mon, Aug 26, 5:55 AM
Germano Cavalcante (mano-wii) updated this revision to Diff 17510.
  • Single . in comment
source/blender/editors/mesh/editmesh_select.c
211

this can be made into a utility function similar to BM_vert_pair_share_face_check_cb

Would also pick the *best* face since this will cause an error with edges between concave faces, which may also be joined by an adjacent face which the edge crosses.

There are a few ways this could be tested, one would be to calculate two normals on either side of the vertices with BM_face_calc_normal_subset, if they're flipped - the edge runs between concave part of a face and can be skipped, we could also check that this edge doesn't cross any edges of the face - although not sure that's needed.

290

Would call this DEBUG_TIME (typically used elsewhere).

292

Even with ifdef's like this, they should be outside the function otherwise adding inline function could cause an error - for eg.

Germano Cavalcante (mano-wii) marked 4 inline comments as done.Mon, Aug 26, 3:38 PM
Germano Cavalcante (mano-wii) updated this revision to Diff 17516.
  • Create and use new BM_vert_pair_shared_face_cb
  • Fix python error
  • Remove COMPARE_TIME directive
Campbell Barton (campbellbarton) requested changes to this revision.Mon, Aug 26, 6:13 PM
Campbell Barton (campbellbarton) added inline comments.
source/blender/editors/mesh/editmesh_select.c
211

This is no longer used.

233

should be static.

233

f isn't used,. use UNUSED(f)

456

This gives me:

/src/blender/source/blender/editors/mesh/editmesh_select.c: In function ‘EDBM_automerge_and_split’:
/src/blender/source/blender/editors/mesh/editmesh_select.c:456:15: error: flexible array member in union
  456 |           int as_int[];

use int as_int[0];

469

Shadows previous value, needs to be named differently.

This revision now requires changes to proceed.Mon, Aug 26, 6:13 PM
Germano Cavalcante (mano-wii) marked 5 inline comments as done.EditedMon, Aug 26, 6:24 PM
Germano Cavalcante (mano-wii) updated this revision to Diff 17523.
  • Remove unused embm_face_split_vert_pair;
  • Use static;
  • as_int[] in union to as_int[0]
  • rename cuts_len to e_cuts_len to avoid shadowing.
  • Rename property Split to Split Edges & Faces
This revision was not accepted when it landed; it landed in state Needs Review.
This revision was automatically updated to reflect the committed changes.