Page MenuHome

Add the ability to create internal springs to the cloth sim
Needs RevisionPublic

Authored by Sebastian Parborg (zeddb) on Sep 13 2019, 3:53 PM.

Details

Summary

This will make closed surfaces behave more like a soft body.

As always, I'm a bit uncertain with the naming of options. So I'm open to alternative naming if there is something more proper.

On of the things that is left to do is to decide what to do when dynamic base mesh is on. Having a dynamic base mesh will force recalculation of springs and will currently not work with this. I'm guessing I could simply disable internal springs if dynamic base mesh is on?

@Campbell Barton (campbellbarton) @Germano Cavalcante (mano-wii) sometimes (with suzanne for example) this triggers and asset in blender/blenlib/intern/BLI_kdopbvh.c:1292
BLI_assert(!((flag & BVH_OVERLAP_RETURN_PAIRS) && (flag & ~BVH_OVERLAP_BREAK_ON_FIRST)));

I do not set these flags with my code, so I'm guessing this is an issue with defaults?

Diff Detail

Event Timeline

Jacques Lucke (JacquesLucke) requested changes to this revision.Sep 13 2019, 4:27 PM
Jacques Lucke (JacquesLucke) added inline comments.
release/scripts/startup/bl_ui/properties_physics_cloth.py
186

I think title case should be used here, i.e. Max Spring Length. Same below.

source/blender/blenkernel/intern/cloth.c
1390

Should maybe named something like find_internal_spring_target_vertex.

1391

Should this be r_rayhit and be the last parameter?

1394

max_distance?

1396

Are there cases when we want this to be false?

1459

Looks like rayhit->index is two different things in different cases here. Sometimes it is an index into the looptri array, and sometimes a vertex index. You should avoid using the same variable for that.

In fact, I don't see why BVHTreeRayHit should be passed from the outside at all.

1513

use_internal_springs

1529

Can you just use a separate code path for the case when internal springs are built? It feels unnecessary to squeeze this into this loop and then have a condition in every iteration.

1545

Wait, every spring is separately allocated? Can you use some mempool?

1547

I'm surprised to see that you handled failed allocation here. That's rarely done in Blender, but cannot hurt to do. Just wonder if it is worth the additional code path.

source/blender/makesdna/DNA_cloth_types.h
151

Comments should end with a dot.

152

Just "internal" does not tell me that this is about springs. So either put this into a separate struct that only contains information about the (internal) springs, or call it something like internal_spring_max_length.
Same below and for the RNA property names.

This revision now requires changes to proceed.Sep 13 2019, 4:27 PM
source/blender/blenkernel/intern/cloth.c
1391

I'm not sure what you mean here?

1396

Yes, there are certain setups where you do not what it to check for normals.
When using meshes like Suzanne this can be useful to make sure that the eyes are mounted to the eyelids.

1529

But then we would have to iterate over the mesh two times. But perhaps looping over two times in not that big of an issue?

1545

I simply copied the spring creation code. This is how it is done in the later part of this file.

1547

Same as above, this is simply how the other code does it too.

I guess we could rewrite it if you feel that it would be better.

source/blender/blenkernel/intern/cloth.c
1391

Usually parameters that return stuff to the caller should have the r_ prefix and be at the end of the parameter list.
However, BVHTreeRayHit shouldn't be a parameter in the first place, so it doesn't matter here.

1529

Doing the raycast is far more expensive than having a loop over all vertices. So an additional loop should not make the performance worse in any noticeable way.