Page MenuHome

Add cloth pressure vertex group and unlock cloth shrink values range
ClosedPublic

Authored by Sebastian Parborg (zeddb) on Mon, Dec 2, 6:29 PM.

Details

Summary

Introduced a way to specify cloth pressure force influence with a vertex group.
This will allow users to only have pressure affect certain parts of the mesh.

In addition to this, the "shrink factor" is now also unlocked to allow negative values and thus allowing the cloth mesh to grow as well.

Diff Detail

Repository
rB Blender

Event Timeline

Jacques Lucke (JacquesLucke) requested changes to this revision.Tue, Dec 3, 10:11 AM

The face skipping in the volume calculation seems wrong. Functionality wise that looks good. Could not test it right now because I'm fighting some other linking issue...

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

Might be fine, just wondering, why did you remove these checks?

746

Typical case for a comment that should be encoded in the variable name.

source/blender/makesdna/DNA_cloth_types.h
97

Maybe use "double the edge length" or something similar?

source/blender/makesrna/intern/rna_cloth.c
616–617

Why is -FLT_MAX better than FLT_MIN? (I see that a lot in Blender, I just don't know why.)

849

Faces with a vertex that has zero weight will be excluded from the volume calculation.

That seems to be a weird decision. The volume calculation algorithm does not make any sense when you simply skip some triangles...

I wonder why this vertex group should have any impact on the volume calculation. It should only have an impact on what forces are applied based on the volume difference.

source/blender/physics/intern/BPH_mass_spring.cpp
658–689

I'd probably nest things a bit differently here, buuut it's fine.

659

Sentences end with a dot. (same "issue" in more new comments)

672

Is it really necessary to have this skip_face thing here? It looks like everything should work out fine when some weight is zero in BPH_mass_spring_force_pressure.

687

In my opinion, the weights input for such a low level function should not allowed to be zero.
Possible nicer solutions are:

  • Always passing some weights (e.g. (1, 1, 1))
  • Having a second BPH_mass_spring_force_pressure_weighted function that takes weights as additional input.
  • Do the actual weight computation outside of BPH_mass_spring_force_pressure. Note that in this case the function would have to return the raw weights and should not apply them directly on the ImplicitData struct.

In this particular case, it is not super bad, but one should be careful with allowing null as parameter. They increase the possible states a function can be in exponentially. That increases the complexity and often makes it unclear at which point an error actually occurs. Also having more branches in low level code means slower execution performance, especially when the branch is not easy to predict (I've shown you the graph ;D).

I'm also fine if you don't change this, just wanted to write this here. :)

This revision now requires changes to proceed.Tue, Dec 3, 10:11 AM
Sebastian Parborg (zeddb) marked 5 inline comments as done.

Updated patch fixing some of the feedback comments.

Sebastian Parborg (zeddb) marked 2 inline comments as done.

Fixed the remaining comments.

Sebastian Parborg (zeddb) marked 2 inline comments as done.Tue, Dec 3, 3:12 PM
Sebastian Parborg (zeddb) added inline comments.
source/blender/blenkernel/intern/cloth.c
729

Because they should not be needed.

If these are not set, then the vgroup should be 0 which will make the dvert check bellow always be false as def_nr can only be >= 0.
We have checks and asserts in the code that will trigger if def_nr is below zero.

I just thought it would be good to remove this so there is no confusion in the future.
I was a but perplexed as well. But I did some research and test and came to the conclusion that these checks are not needed.

746

Right, I can't come up with a good name though. And delta in technically correct...

source/blender/makesrna/intern/rna_cloth.c
616–617

FLT_MIN is not the highest negative number you can have but the one closest to zero.
So 0.0000000000000001 etc

This revision is now accepted and ready to land.Wed, Dec 4, 11:16 AM