Page MenuHome

Edit Mesh: partial updates for normal and face tessellation
ClosedPublic

Authored by Campbell Barton (campbellbarton) on Jun 4 2021, 4:11 AM.

Details

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 14955
Build 14955: 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.
  • 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).
  • More code comments
  • Cleanup function names for transform custom data private API.
  • 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.
  • Multi thread partial face tessellation
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) edited the summary of this revision. (Show Details)
  • Remove debug timing
  • Minor improvement to comments
  • 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)

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).

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

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
240

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
377

Perhaps this function could be removed to avoid confusion with the name. (customdata -> customdatacorrect)

1756

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
240

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
377

Agree, I missed this one.

1756

I'll need to double check this logic.

Campbell Barton (campbellbarton) marked 2 inline comments as done.
  • Clarify what is meant by a small number of faces (WRT multi-threading).
  • Rename customdata function that wasn't renamed when it should have been.
  • Add back comment accidentally removed.
Campbell Barton (campbellbarton) marked 3 inline comments as done and an inline comment as not done.Jun 4 2021, 2:37 PM
  • Simplify logic for tagging mirror vertices
Campbell Barton (campbellbarton) marked an inline comment as done.Jun 4 2021, 2:58 PM
Campbell Barton (campbellbarton) added inline comments.
source/blender/editors/transform/transform_convert_mesh.c
1756

As far as I can see this equals_v3v3 wasn't needed, and your right, the tag is always set.

  • Update based on discussion with mano-wii
This revision is now accepted and ready to land.Jun 4 2021, 3:20 PM
  • Fix for normal calculation logic, additional faces are needed for waiting vertex normals