Page MenuHome

Weld Modifier: Initial Implementation
ClosedPublic

Authored by Germano Cavalcante (mano-wii) on Dec 10 2019, 5:11 AM.
Tags
None
Tokens
"Love" token, awarded by Way."Love" token, awarded by threedslider."Burninate" token, awarded by DotBow."Like" token, awarded by Fracture128."Love" token, awarded by MetinSeven."Love" token, awarded by fiendish55."Love" token, awarded by rl.amorato."Love" token, awarded by Dspazio."Love" token, awarded by brilliant_ape."Love" token, awarded by amonpaike.

Details

Summary

Part of T70240

This patch can be split into two commits:

  • BLI_bvhtree_overlap_ex: add 'max_interactions' parameter
  • Weld Modifier: Initial Implementation

It only solves the option described as Full in the proposal.

Still to be done
From proposal

  • Seams: restrict welding to vertices along boundary edges.
  • Edge Collapse: collapse edges below the length threshold.

Other ToDos

  • New icon for the modifier.
  • Some customdata types are not being correctly interpolated (vertex weight).
  • Cache BVHTree.

Obs.: The two rows for modifier icons are already filled. We need to think of a way to organize the new icon.

Diff Detail

Repository
rB Blender

Event Timeline

Germano Cavalcante (mano-wii) edited the summary of this revision. (Show Details)

I cannot build this because it says that BLI_bvhtree_overlap_ex is declared twice:

Looking over the patch, note that am getting asserts / crashes w/ initial stress test:

.

Try scrubbing the playhead, increase the threshold to 0.2 makes it crash more often.

  • Fix bugs related to weld_poly_split_recursive.
  • Use spherical distance.
  • Improve Weld Loop Group detection
  • Comment DEBUG_WELD
  • Support Vertex Groups
Germano Cavalcante (mano-wii) edited the summary of this revision. (Show Details)
Campbell Barton (campbellbarton) requested changes to this revision.EditedDec 11 2019, 2:22 PM

Tested and works very well now, although I did manage to make it hang setting large a threshold.

  • Suggest show threshold as the first option.
  • I think max-interactions variable property is confusing from a user perspective, could rename, eg: Duplicate Limit ... although it's not so much better.
  • Would try to avoid custom-data layer spesific merging here if possible and use custom-data API's instead.
  • It's better not to interpolate vertex normals and set them dirty instead (so they will be recalculated on request) since interpolated normals won't match recalculated normals which will take the new faces into account.

Regarding calculating the welds, I don't think BVH is needed since this is for elements with volume, a KD-tree should be able to handle this. It's just we don't have an API to find overlaps like we do for BVH, although this isn't a large part of the patch and could be changed later if we need.

source/blender/modifiers/intern/MOD_weld.c
60–61

Brief comments would be useful here.

152

(picky), convention is l_curr.

1554–1556

All assignment's to X.

This revision now requires changes to proceed.Dec 11 2019, 2:22 PM
Germano Cavalcante (mano-wii) marked 3 inline comments as done.
  • Comments.
  • Don't interpolate vertex normals.
  • Rename variables.
  • Put show threshold as the first option.
  • Rename Interactions to Duplicate Limit.
  • More Comments
  • Use ICON_AUTOMERGE_OFF instead ICON_DOT

About icons, I don't know how to proceed in that case,
it seems that there is no more slot to add a new modifier icon.
@Andrzej Ambroz (jendrzych), @William Reynish (billreynish), any suggestion?

The plan was to have Weld Modifier for blender 2.82,
but as bcon2 starts tomorrow I don't think there is still time for it.

Regarding the icons sheet - it's more and more crowded and messy. There're some free slots here and there, but we slowly run out of free space. It's obvious, that the sheet must be reorganised or - more likely - we need brand new icon's storing system. I'd go for individual file per icon.

This revision is now accepted and ready to land.Dec 12 2019, 1:15 AM
This revision was automatically updated to reflect the committed changes.

@Andrzej Ambroz (jendrzych) Agreed. The whole concept of an icon sheet is always likely to cause a mess, because the number of icons don't necessarily fit neatly into a series of rows and columns. Let's make a technical design task for this, so we can explore the best solution. For now, you'll just have to keep adding icons to the sheet, even though it's going to be messy.

Way awarded a token.Dec 12 2019, 12:57 PM

amazing addition! Any chance of interior face removal?

in addition. I think w/ smart enough merge it could be quite useful for cutting implied geometry into something.
For example if a mesh cut had a solidify on it just small enough it could heal just to be just the cut area with the implied edge remaining live.

or what about at least having the part that dissolves faces on a checkbox in the mod?