Page MenuHome

Add argument to DM_to_mesh() function to take ownership over the DM
ClosedPublic

Authored by Sergey Sharybin (sergey) on Feb 18 2015, 8:04 PM.

Details

Summary

The idea is pretty simple: instead of making temporary copy of all the
related custom data layers just pass the ownership from the DM to the
mesh.

This is really handy in cases when you've got DM which you need to
convert to Mesh datablock and wouldn't need that DM after conversion
anyway.

Foe example, render database conversion, exporters and even Modifier
Apply will benefit from this option.

Diff Detail

Repository
rB Blender

Event Timeline

Sergey Sharybin (sergey) retitled this revision from to Experiment with more optimal DM to Mesh conversion.
Sergey Sharybin (sergey) updated this object.

Some changes which makes the patch ready for full review i think:

  • Solved memory leak. It was caused by using wrong allocation type for loop data.
  • Passing custom data ownership seems indeed be correct now, there's no leaks, no double-frees, no bad memory access happening. Extra eye here wouldn't hurt tho.
  • Restricted to CDDM with needsFree only.

Fix memory leak when source DM has tessfaces

Fixed leaks of layers which are in the DM but not in the requested conversion mask

Sergey Sharybin (sergey) retitled this revision from Experiment with more optimal DM to Mesh conversion to Add argument to DM_to_mesh() function to take ownership over the DM.Feb 20 2015, 4:43 PM
Sergey Sharybin (sergey) updated this object.
Sergey Sharybin (sergey) updated this object.

Fix for wrong allocation type passed to layers add, causing memory leaks

O

source/blender/blenkernel/intern/mesh.c
2259–2260

Not sure about removing the release here, its possible that dm isn't a CDDM, therefor assign will be set to false internally, in this case dm->release(dm) wont run, when it did before.

source/blender/editors/object/object_modifier.c
617–618

same concern as above.

source/blender/blenkernel/intern/mesh.c
2259–2260

It will. If take_ownership is true the function DM_to_mesh will always release the derived mesh, even if it's not CDDM or of it's needsFree != 1.

source/blender/blenkernel/intern/customdata.c
1562

mask should be CustomDataMask not int.

*picky* would call CustomData_free_typemask(struct CustomData *data, int totelem, CustomDataMask mask),

matches CustomData_number_of_layers_typemask, and is a bit more clearly freeing by type.

This revision is now accepted and ready to land.Feb 27 2015, 12:04 PM
Sergey Sharybin (sergey) edited edge metadata.

Addressed feedback from Cambo

This revision was automatically updated to reflect the committed changes.