Page MenuHome

Bypass EditMesh to Mesh conversion (initial step)
Needs ReviewPublic

Authored by Campbell Barton (campbellbarton) on Wed, Mar 18, 4:31 AM.
Tokens
"Like" token, awarded by lucky3."Love" token, awarded by samgreen."Love" token, awarded by szap."Burninate" token, awarded by amonpaike."Yellow Medal" token, awarded by franMarz."Burninate" token, awarded by mywa880."Mountain of Wealth" token, awarded by EitanSomething.

Details

Summary

This is an initial patch to avoid converting EditMesh to Mesh.

This patch makes the following changes:

  • Mesh data blocks in the modifier stack can be "thin wrapped" edit-mesh data (empty mesh data, with pointers to the EditMesh and EditMeshData).
  • Any users of these meshes that need the full mesh data can ensure this data exists (delaying edit-mesh conversion until it's needed).
  • Currently, this means there is no EditMesh to Mesh conversion for edit-mode for many deform modifiers, giving significant performance improvements (about 2x).
  • RNA Object.to_mesh initialized mesh data for Cycles & exporters.

In this patch a define is checked for, currently no functional changes unless the developer defines USE_EDITMESH_THIN_WRAP (exact details of how this is enabled can be tweaked).

Once this patch is applied, each modifier with a deformVertsEM callback which runs BKE_mesh_from_editmesh_thick_wrap_ensure needs to be updated to remove this call.
In most cases this can be removed once there is a utility to get vertex weights from both a mesh and edit-mesh vertices.

Performance

Testing playback of a mesh with ~98k tris and two deform modifiers gives a little over 2x performance improvement here.

Note that draw_cache_extract_mesh.c is accessing normals and coordinates more than I would like. In the current patch this checks for deformed coordinates on each access.

Eventually we should be able to avoid passing vertex coordinates to the GPU so many times to reduce the overhead.

Currently it's not practical to split the code into separate branches for deform/non-deform cases. With different logic for updating the coordinate buffer, we might be able to avoid checking for deformed coords on every access.

Possible Changes

Here are some changes I'm considering.

  • Instead of having thin-wrapped mesh which uses a boolean to check we could have a data-type, so mesh->runtime.data_type could be: ME_GEOMTYPE_MDATA, ME_GEOMTYPE_BMESH and later perhaps ME_GEOMTYPE_SUBSURF.
  • Currently the mesh extraction code is calculating data from the edit-mesh (such as tangents and normals). This is awkward as there is logic in the modifier calculation code to calculate this for mesh data, see: editbmesh_calc_modifier_final_normals. This area could be cleaned up, making sure calculations happen in editbmesh_calc_modifier_final_normals, even with an edit-mesh.

Remaining Work

  • Ensure lazy initialization doesn't cause threading bugs. As long as the lazy initializing the mesh data happens in modifiers (for the mesh being created by the modifier stack). This isn't needed, however we will most likely want locking for the final id_eval, as well as modifiers that depend on other objects geometry.
  • Checking UV tangents are being correctly calculated before & after this patch is applied.
  • Check on custom normal editing.
  • Remove or update BKE_mesh_foreach_mapped_loop, as it's not used. This currently uses Mesh loop layer even for edit-mesh data when MESH_FOREACH_USE_NORMAL is enabled.
  • Support statvis "Distort" display option with coordinates deformed by modifiers.

Proposal

  • Finish this patch to a basic level:
    • Resolve lazy initializing multi-threading for modifiers that depend on geometry.
    • Ensure tangents work correctly (needs testing).
    • Ensure normal editing works correctly (needs testing).
  • Incrementally remove BKE_mesh_from_editmesh_thick_wrap_ensure for modifiers deformVertsEM callbacks.
  • Ensure there are no regressions (UV-mapping, UV-tangents, auto-smooth, normal-editing, crazy-space, snapping to edit-mesh data).
  • Remove pre-processor checks and enable this by default.

Note: this could go into master and be disabled at any point, initially this was my plan but as the patch gets a bit noisy. I'm not as convinced this is worth doing.

Further Work

  • Support modifiers passing thin-wrapped meshes between modifiers (eModifierTypeFlag_AcceptsBMesh). This shouldn't be such a big task, it's mostly a matter of making sure there are no regressions.
  • Support bypassing normal calculation in wire-frame mode (for 2.7x performance in wire-frame mode - if we want to aim for this)

Diff Detail

Repository
rB Blender
Branch
TEMP-MESH-OPTIMIZE-v2 (branched from master)
Build Status
Buildable 7206
Build 7206: arc lint + arc unit

Event Timeline

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

Ensure mesh data is available for Object.to_mesh(), Cycles now works.

  • Fix drawing tangents in edit-mode

This seems broadly the right direction to me. My main concern is naming and conventions in API functions, how to know which ones work on a regular mesh, on a bmesh mesh, or both.

Instead of having thin-wrapped mesh which uses a boolean to check we could have a data-type, so mesh->runtime.data_type could be: ME_GEOMTYPE_MDATA, ME_GEOMTYPE_BMESH and later perhaps ME_GEOMTYPE_SUBSURF.

I think that would be better, I find the thin/thick wrap terminology unclear.

Currently the mesh extraction code is calculating data from the edit-mesh (such as tangents and normals). This is awkward as there is logic in the modifier calculation code to calculate this for mesh data, see: editbmesh_calc_modifier_final_normals. This area could be cleaned up, making sure calculations happen in editbmesh_calc_modifier_final_normals, even with an edit-mesh.

Yes, computation like that should happen in depsgraph evaluation rather than drawing code whenever possible.

source/blender/blenkernel/BKE_editmesh_cache.h
41

This can be an inline function if it's performance critical, no need for it to be a macro.

source/blender/blenkernel/intern/editmesh.c
266–272

To me it seems like BKE_mesh_minmax should have this logic, rather than every place that calls BKE_mesh_minmax?

In general we need good (naming) conventions on this in the BKE mesh API, it's quite fuzzy to me now.

General approach looks fine to me too, not much more to say at this point, did not actually check the code in details.

Instead of having thin-wrapped mesh which uses a boolean to check we could have a data-type, so mesh->runtime.data_type could be: ME_GEOMTYPE_MDATA, ME_GEOMTYPE_BMESH and later perhaps ME_GEOMTYPE_SUBSURF.

I think that would be better, I find the thin/thick wrap terminology unclear.

Fully agree as well, looks much cleaner and easier to extend to me.