Page MenuHome

Add pressure forces to cloth sim
ClosedPublic

Authored by Sebastian Parborg (zeddb) on Aug 13 2019, 4:23 PM.
Tags
None
Tokens
"Dat Boi" token, awarded by Pipeliner."Love" token, awarded by mistajuliax."Love" token, awarded by vr_sebas."Love" token, awarded by Noss."Like" token, awarded by acme_pjz."Love" token, awarded by yebyte."Love" token, awarded by nate066."Love" token, awarded by Mets."Mountain of Wealth" token, awarded by marcog."Love" token, awarded by eversimo.

Details

Summary

Here is my port of T30941 to blender 2.8.

This adds the ability to simulate air pressure inside a closed cloth surface.

I think it should be good to go. Perhaps a bit more polishing on the UI is needed.

Diff Detail

Repository
rB Blender

Event Timeline

Jacques Lucke (JacquesLucke) added inline comments.
source/blender/makesdna/DNA_cloth_types.h
102

I think this property names can be improved. cloth_pressure does not make much sense to me. Why not just call it something like air_pressure_difference?

For the others: initial_volume and pressure_factor (factor seems to fit better than scale here imo).

source/blender/makesrna/intern/rna_cloth.c
785

Don't shorten names unnecessarily.

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

Simplified the volume calculation method and unified the method into a new math function.
I made the other parts of blender that uses this method use this function as well.

source/blender/blenkernel/BKE_cloth.h
92

initial_mesh_volume (based on comment)

source/blender/blenlib/BLI_math_geom.h
95

This should not compute 6x the volume, but the actual volume.
Update the code and comments and the caller and callee side.

source/blender/makesdna/DNA_cloth_types.h
101

This comment is useless.

102

Put comments on the lines before the property, for better formatting. Also, sentences start with a capital letter.

Typo difference,

What unit is the pressure_difference in? To make sense in the formula below it should not have a unit. However, the term difference suggests, that you subtract inner from outer pressure (or the other way around). This would result in a value that still has a unit.
Maybe this is fine, I don't know. It just does not seem to make sense just based on the units, which suggests that there is something wrong.

104

A volume is an intrinsic property of a closed mesh, it feels weird to have it exposed as property. It feels more like an initial_volume_override or so.

source/blender/makesrna/intern/rna_cloth.c
788

Typos parameter and initial.

source/blender/physics/intern/BPH_mass_spring.cpp
603

First calculate all the forces from effectors, then the forces from the pressure. Don't try to interleave it. This way you do not get all those if (effectors) and if (calc_pressure).

The next step would be to move the different kinds of force-calculations into separate functions, but this is more a refactor and can be done afterwards.

source/blender/physics/intern/implicit_blender.c
1500

Where does the / 3.0f come from?

1505

No need for empty lines here.

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

Updated with the latest feedback

Jacques Lucke (JacquesLucke) requested changes to this revision.Nov 27 2019, 1:58 PM
Jacques Lucke (JacquesLucke) added inline comments.
source/blender/blenkernel/intern/cloth.c
1756

Is that change intended?

source/blender/editors/uvedit/uvedit_parametrizer.c
2052

Please try not to follow this code style where you define all the variables at the top before they are used. Try to make the vertical scope of variables as small as possible.

2057

Would actually be quite nice to have a function volume_tri_tetrahedron_v3 here (that uses volume_tri_tetrahedron_signed_v3_6x internally).

source/blender/physics/intern/BPH_mass_spring.cpp
563

This variable is not really necessary anymore, just make the check in the if statement.

614

Make comments a full sentence, same below.

source/blender/physics/intern/implicit.h
185

Should also have signed and 6x in the name.

This revision now requires changes to proceed.Nov 27 2019, 1:58 PM
Sebastian Parborg (zeddb) marked 5 inline comments as done.

Updated with latest feedback

The code looks good to me now. I tested it a bit yesterday, and it seemed to work as expected, didn't have any crash. Hope you did some more testing of the functionality.

This revision is now accepted and ready to land.Nov 27 2019, 2:49 PM

Yes, I've tested this quite a bit. Fingers crossed that I didn't miss anything :P

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

It is clang format that does this. But I can remove this part of the patch if you wish.

I wonder if it would be now easy to add a true buoyancy force (when negative conversely emulating objects filled with a liquid) - isn't it basically a pressure gradient along the gravity vector?

I wonder if it would be now easy to add a true buoyancy force (when negative conversely emulating objects filled with a liquid) - isn't it basically a pressure gradient along the gravity vector?

I had tried it long time ago, it seems that it causes instabilities during the simulation. Maybe it's time to try it again.

source/blender/physics/intern/implicit_blender.c
1498

While testing my D6442 buoyancy patch I found an old bug: this calc_nor_area_tri function is wrong and returns double the area of the triangle. As a result, this (and my) patch effectively has a hidden additional factor of 2 for the actually applied pressure. This is not much of a problem in practice, but it makes understanding the precise mathematical meaning of the input parameters even more difficult.

source/blender/physics/intern/implicit_blender.c
1498

Then fix it in your new patch. :P

There is no use of adding more comments for this merged patch.

Sebastian Parborg (zeddb) marked an inline comment as done.Dec 19 2019, 4:15 PM
Sebastian Parborg (zeddb) added inline comments.
source/blender/physics/intern/implicit_blender.c
1498

I committed a fix for this into master. Now the area calculation should be correct.

Thanks for pointing this out!