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.
Details
Details
Diff Detail
Diff Detail
Event Timeline
| 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. | |
Comment Actions
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 ↗ | (On Diff #17096) | This should not compute 6x the volume, but the actual volume. |
| 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. | |
| 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 | ||
| 605–606 | 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. | |
| source/blender/blenkernel/intern/cloth.c | ||
|---|---|---|
| 1756–1758 | Is that change intended? | |
| source/blender/editors/uvedit/uvedit_parametrizer.c | ||
| 2052 ↗ | (On Diff #19874) | 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 ↗ | (On Diff #19874) | 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. | |
Comment Actions
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.
Comment Actions
Yes, I've tested this quite a bit. Fingers crossed that I didn't miss anything :P
| source/blender/blenkernel/intern/cloth.c | ||
|---|---|---|
| 1756–1758 | It is clang format that does this. But I can remove this part of the patch if you wish. | |
Comment Actions
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?
Comment Actions
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. | |
| 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! | |