Details

Reviewers

Summary

This patch exposes functionality for performing partial mesh updates
for normal calculation and face tessellation while transforming a mesh.

The partial update data only needs to be generated once,
afterwards the cached connectivity information can be reused
(with the exception of changing proportional editing radius).

Currently this is only used for transform, in the future it could be
used for other operators as well as the transform panel.

The best-case overall speedup while transforming geometry is about
1.45x since the time to update a small number of normals and faces is
negligible.

For an additional speedup partial face tessellation is multi-threaded,
this gives ~15x speedup on my system (timing tessellation alone).
Exact results depend on the number of CPU cores available.

Notes:

Partial updates to normals is by @Germano Cavalcante (mano-wii) from patch D11283.
Logic from this patch can be used to multi-thread all face tessellation (not only partial updates).
Partial updates could be used for other tools that deform/transform vertices, although there aren't many cases where it seems urgent.

As mentioned in the commit message: the vertex transform panel is a candidate for this, although this needs changes to the interface code to support this functionality.

Most other deforming operators aren't interactive, or they use operator redo.

Diff Detail

Repository

rBS Blender Staging

Branch

temp-editmesh-partial-updates

Build Status

Buildable 14946
Build 14946: arc lint + arc unit

Event Timeline

Campbell Barton (campbellbarton) requested review of this revision.Jun 4 2021, 4:11 AM

Campbell Barton (campbellbarton) created this revision.

Harbormaster completed remote builds in B14945: Diff 37837.Jun 4 2021, 4:11 AM

Improve code comments.
Rename functions and variables.
Remove temporary bitmap tags from the BMPartialUpdate struct (only needed when building).
Reserve more edges than vertices since common grid topologies use around twice as many.
Add BMPartialUpdate_Params struct to support generating update data for different purposes (only normals & tessellation are used currently).

Harbormaster completed remote builds in B14946: Diff 37838.Jun 4 2021, 4:16 AM

TheRedWaxPolice (TheRedWaxPolice) added a subscriber: TheRedWaxPolice (TheRedWaxPolice).Jun 4 2021, 4:20 AM

More code comments
Cleanup function names for transform custom data private API.

Harbormaster completed remote builds in B14949: Diff 37841.Jun 4 2021, 5:15 AM

Perform a full update when all vertices are selected.

The additional cost of generating the partial connectivity data isn't justified when all data needs to be updated.

Harbormaster completed remote builds in B14950: Diff 37842.Jun 4 2021, 6:08 AM

Multi thread partial face tessellation

Harbormaster completed remote builds in B14951: Diff 37843.Jun 4 2021, 8:50 AM

Campbell Barton (campbellbarton) retitled this revision from Partial edit-mesh updating for transform to Edit Mesh: partial updates for normal and face tessellation.Jun 4 2021, 8:51 AM

Campbell Barton (campbellbarton) edited the summary of this revision. (Show Details)

Campbell Barton (campbellbarton) added a reviewer: Germano Cavalcante (mano-wii).

Remove debug timing

Harbormaster completed remote builds in B14952: Diff 37844.Jun 4 2021, 8:53 AM

Move redundant header

Harbormaster completed remote builds in B14953: Diff 37845.Jun 4 2021, 8:54 AM

Campbell Barton (campbellbarton) edited the summary of this revision. (Show Details)Jun 4 2021, 8:55 AM

Campbell Barton (campbellbarton) edited the summary of this revision. (Show Details)Jun 4 2021, 8:59 AM

Campbell Barton (campbellbarton) edited the summary of this revision. (Show Details)Jun 4 2021, 9:03 AM

Minor improvement to comments

Harbormaster completed remote builds in B14954: Diff 37847.Jun 4 2021, 9:06 AM

Use compiler attributes for partial update functions.
Include bmesh_partial_update.h from bmesh.h (avoid referencing intern/bmesh_partial_update.h in files outside the BMesh API)

Harbormaster completed remote builds in B14955: Diff 37848.Jun 4 2021, 9:13 AM

Campbell Barton (campbellbarton) edited the summary of this revision. (Show Details)Jun 4 2021, 9:16 AM

Campbell Barton (campbellbarton) mentioned this in T88550: Mesh Optimization Project Progress.Jun 4 2021, 9:22 AM

Check face count instead of edges
Note that edge count in some ways a better number to check since it will cause ngons with many sides
to use threaded logic even if there aren't so many faces.

However there edges won't be set if the caller is only calculating tessellation (not vertex normals).

Harbormaster completed remote builds in B14956: Diff 37849.Jun 4 2021, 9:32 AM

Don't allocate vert/edge arrays when only tessellation data is requested.

Harbormaster completed remote builds in B14957: Diff 37850.Jun 4 2021, 9:44 AM

RedMser (RedMser) added a subscriber: RedMser (RedMser).Jun 4 2021, 10:18 AM

Good to see this code being worked on :)
It would be interesting to see some numbers regarding multithreaded tessellation.
Some observations:

source/blender/blenkernel/intern/editmesh.c
156	Use comment style guide for new code?
source/blender/bmesh/intern/bmesh_mesh.c
499	I think this description was accidentally removed.
source/blender/bmesh/intern/bmesh_mesh_partial_update.c
176	It seems very unusual for radial loops to have the same vertices. `BM_DISK_EDGE_NEXT` doesn't supply all faces?
source/blender/bmesh/intern/bmesh_mesh_tessellate.c
241	It somehow seems a bit contradictory with the code. Is 1024 a very small number? Should the number of cores be counted? Is this a TODO?
source/blender/editors/transform/transform_convert_mesh.c
373	Perhaps this function could be removed to avoid confusion with the name. (customdata -> customdatacorrect)
1752	Wouldn't it be `!equals_v3v3(td_mirror->loc, td_mirror->iloc)` (with `!`)? Also `!BM_elem_flag_test(v_mirr_other, BM_ELEM_TAG)` seems redundant in this case, `td_mirror->extra` and `td->extra` are never repeated.

Campbell Barton (campbellbarton) marked 2 inline comments as done.Jun 4 2021, 2:29 PM

Campbell Barton (campbellbarton) added inline comments.

source/blender/blenkernel/intern/editmesh.c
156	This comment was copied but doesn't really apply, removing.
source/blender/bmesh/intern/bmesh_mesh.c
499	Will add back (removal was from D11283 it seems)
source/blender/bmesh/intern/bmesh_mesh_partial_update.c
176	This ensures each face is only handled once, without this check both edges that a shared by the vertex and the face-loop will walk over the face.
source/blender/bmesh/intern/bmesh_mesh_tessellate.c
241	This isn't an accident or a TODO, but the text should be clarified. between 500-2000 faces tessellation for quads and tris at least, is very fast. around 1000 the difference between single and multi threaded isn't all that much. on my system multi-threaded is a little faster but I have 32 cores, on systems with fewer cores the gain is likely not so much compared with the overhead of running callbacks, so I chose to set the limit at 1024.
source/blender/editors/transform/transform_convert_mesh.c
373	Agree, I missed this one.
1752	I'll need to double check this logic.