Page MenuHome

Cleanup: Simplify BKE_mesh_nomain_to_mesh
ClosedPublic

Authored by Hans Goudey (HooglyBoogly) on Sep 2 2022, 3:20 PM.

Details

Summary
  • Remove "take ownership" argument which was confusing and always true
    • The argument made ownership very confusing
    • It's better to remove boolean arguments that switch a function's purpose
  • Remove "mask" argument which was basically wrong and not used properly
    • "EVERYTHING" was likely used because developers are wary of removing data
    • Instead use CD_MASK_MESH for its purpose of original mesh data
  • Remove use of shallow copied temporary mesh, which is unnecessary now
  • Split some shape key processing into separate functions, use C++ types
  • Copy fields explicitly rather than using memcpy for the whole struct
  • Use higher level functions and avoid redundant logic
    • The goal is pretty simple actually, and can be built from standard logic
  • Adjust CustomData logic to be consistent with "assign" expectations
    • Now it clears the layer data from the source, and moves the anonymous ID

Diff Detail

Repository
rB Blender

Event Timeline

Hans Goudey (HooglyBoogly) requested review of this revision.Sep 2 2022, 3:20 PM
Hans Goudey (HooglyBoogly) created this revision.
Hans Goudey (HooglyBoogly) edited the summary of this revision. (Show Details)

Fix/simplify CustomData_mergefor CD_ASSIGN expectations

To me it sounds and looks good. But would like if someone more familiar with the modelling module have a look as well.

Regarding your questions: just keep the functionality from before.

source/blender/blenkernel/intern/mesh_convert.cc
1332

typo (evaluation)

Hans Goudey (HooglyBoogly) edited the summary of this revision. (Show Details)
  • Merge master
  • Split freeing animation data out of BKE_mesh_clear_geometry
  • Formatting
  • Rearrange code slightly
  • Move vertex group name list instead of copying
  • Fix typo
Campbell Barton (campbellbarton) added inline comments.
source/blender/blenkernel/intern/mesh_convert.cc
1276–1279

This isn't used, for some reason it doesn't show as a compiler warning, assume it can be removed.

This revision is now accepted and ready to land.Sep 9 2022, 9:13 AM