Page MenuHome

Fix T65357: wrong facemap indices after applying a boolean modifier
ClosedPublic

Authored by Philipp Oeser (lichtwerk) on Jun 3 2019, 5:56 PM.

Diff Detail

Repository
rB Blender

Event Timeline

Harbormaster completed remote builds in B3807: Diff 15750.

adding a layer "by hand" with CD_DEFAULT (initializes data correctly to -1) seems to do the trick, not sure though if this should/could be handled in a more generic way?

Brecht Van Lommel (brecht) requested changes to this revision.EditedJun 3 2019, 6:19 PM

I think there is a deeper issue here, booleans should not have to do anything specifically for facemaps.

BM_mesh_bm_from_me seems to be using CD_CALLOC, using CD_DEFAULT instead may help?

This revision now requires changes to proceed.Jun 3 2019, 6:19 PM

I think there is a deeper issue here, booleans should not have to do anything specifically for facemaps.
BM_mesh_bm_from_me seems to be using CD_CALLOC, using CD_DEFAULT instead may help?

That was my first intuition as well, but that "fails" as well. Digging deeper into CustomData_bmesh_merge (think I am close...)

Some findings:

  • in CustomData_bmesh_merge, CustomData_merge is called [in which customData_add_layer__internal would take care of layer creation/data init nicely if CD_DEFAULT is used], but we are calling it with totelem=0
  • (this is just for detecting if a change to customdata would take place at all...)
  • dont see another place where CD_DEFAULT would kick in
  • then CustomData_bmesh_copy_data copies data of existing layers into the (merged) customdata-layout [this initializes with 0 and then copies stuff over], so no call to LayerTypeInfo->set_default()...

So instead of the previous solution, I have added code to CustomData_bmesh_copy_data that would take care of setting to default, will post in a bit...
This seems to work nicely (e.g. also vertex colors are now initialized to white)

Philipp Oeser (lichtwerk) updated this revision to Diff 15843.EditedJun 7 2019, 4:18 PM
  • Revert "Fix T65357: wrong facemap indices after applying a boolean modifier"
  • set customdata to default for layers present in dest but not source in 'CustomData_bmesh_copy_data'

note: it might be good to pass an option to CustomData_bmesh_copy_data to controll whether this is done or not (since this called from multiple places)

This is fine, I'll commit a modified version that uses the same logic as CustomData_to_bmesh_block to avoid having two variations.

As far as I can tell it is not needed to make this optional.

This revision is now accepted and ready to land.Jun 17 2019, 7:37 PM
This revision was automatically updated to reflect the committed changes.